From 34639e462254966c32e39581883edcc73dc537f2 Mon Sep 17 00:00:00 2001 From: Elliot Mawby Date: Thu, 1 Aug 2024 10:41:03 -0700 Subject: [PATCH 1/4] Add option to default to HMS over FCM if a device supports both This is migrating over the PR from the player model pr https://github.com/OneSignal/OneSignal-Android-SDK/pull/1984 --- .../core/internal/device/impl/DeviceService.kt | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt index fa592539de..132100ac1f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt @@ -1,5 +1,6 @@ package com.onesignal.core.internal.device.impl +import android.content.Context import android.content.pm.PackageManager import com.onesignal.common.AndroidUtils import com.onesignal.core.internal.application.IApplicationService @@ -10,6 +11,7 @@ internal class DeviceService(private val _applicationService: IApplicationServic companion object { private const val HMS_CORE_SERVICES_PACKAGE = "com.huawei.hwid" // = HuaweiApiAvailability.SERVICES_PACKAGE private const val GOOGLE_PLAY_SERVICES_PACKAGE = "com.google.android.gms" // = GoogleApiAvailability.GOOGLE_PLAY_SERVICES_PACKAGE + private const val PREFER_HMS_METADATA_NAME = "com.onesignal.preferHMS" private const val HMS_AVAILABLE_SUCCESSFUL = 0 } @@ -37,7 +39,17 @@ internal class DeviceService(private val _applicationService: IApplicationServic override val deviceType: IDeviceService.DeviceType get() { if (supportsADM()) return IDeviceService.DeviceType.Fire - if (supportsGooglePush()) return IDeviceService.DeviceType.Android + + val supportsHMS: Boolean = supportsHMS + val supportsFCM = supportsGooglePush() + + if (supportsFCM && supportsHMS) { + val context: Context = _applicationService.appContext + val preferHMS = AndroidUtils.getManifestMetaBoolean(context, PREFER_HMS_METADATA_NAME) + return if (preferHMS) IDeviceService.DeviceType.Huawei else IDeviceService.DeviceType.Android + } + + if (supportsFCM) return IDeviceService.DeviceType.Android // Some Huawei devices have both FCM & HMS support, but prefer FCM (Google push) over HMS if (supportsHMS) return IDeviceService.DeviceType.Huawei From 4809f024df8ba06995402c365b73a212716e54c1 Mon Sep 17 00:00:00 2001 From: Elliot Mawby Date: Fri, 2 Aug 2024 13:19:19 -0700 Subject: [PATCH 2/4] fix nits --- .../com/onesignal/core/internal/device/impl/DeviceService.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt index 132100ac1f..227bcb619b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt @@ -40,11 +40,11 @@ internal class DeviceService(private val _applicationService: IApplicationServic get() { if (supportsADM()) return IDeviceService.DeviceType.Fire - val supportsHMS: Boolean = supportsHMS + val supportsHMS = supportsHMS val supportsFCM = supportsGooglePush() if (supportsFCM && supportsHMS) { - val context: Context = _applicationService.appContext + val context = _applicationService.appContext val preferHMS = AndroidUtils.getManifestMetaBoolean(context, PREFER_HMS_METADATA_NAME) return if (preferHMS) IDeviceService.DeviceType.Huawei else IDeviceService.DeviceType.Android } From 12b5c44f82cea616b22139329470f59d59afd50e Mon Sep 17 00:00:00 2001 From: Elliot Mawby Date: Fri, 2 Aug 2024 14:25:45 -0700 Subject: [PATCH 3/4] Add unit tests for preferHMS devicetype behavior This creates a new DeviceServiceTests file and moves supportsHMS and supportsGooglePush to the IDeviceService interface --- .../core/internal/device/IDeviceService.kt | 3 + .../internal/device/impl/DeviceService.kt | 4 +- .../internal/device/DeviceServiceTests.kt | 81 +++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IDeviceService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IDeviceService.kt index 0b8d2d3635..5cda91bc70 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IDeviceService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/IDeviceService.kt @@ -11,6 +11,9 @@ interface IDeviceService { val hasAllHMSLibrariesForPushKit: Boolean val hasFCMLibrary: Boolean val jetpackLibraryStatus: JetpackLibraryStatus + val supportsHMS: Boolean + + fun supportsGooglePush(): Boolean enum class JetpackLibraryStatus { MISSING, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt index 227bcb619b..3e641833e7 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/device/impl/DeviceService.kt @@ -74,7 +74,7 @@ internal class DeviceService(private val _applicationService: IApplicationServic return IDeviceService.JetpackLibraryStatus.OK } - private fun supportsGooglePush(): Boolean { + override fun supportsGooglePush(): Boolean { // 1. If app does not have the FCM library it won't support Google push return if (!hasFCMLibrary) false else isGMSInstalledAndEnabled @@ -116,7 +116,7 @@ internal class DeviceService(private val _applicationService: IApplicationServic } } - private val supportsHMS: Boolean + override val supportsHMS: Boolean get() { // 1. App should have the HMSAvailability for best detection and must have PushKit libraries return if (!hasHMSAvailabilityLibrary() || !hasAllHMSLibrariesForPushKit) false else isHMSCoreInstalledAndEnabled() diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt new file mode 100644 index 0000000000..5011e8e8d2 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt @@ -0,0 +1,81 @@ +package com.onesignal.core.internal.device + +import androidx.test.core.app.ApplicationProvider +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.common.AndroidUtils +import com.onesignal.core.internal.device.impl.DeviceService +import com.onesignal.mocks.MockHelper +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.spyk + +@RobolectricTest +class DeviceServiceTests : FunSpec({ + test("devicetype is Huawei when preferHMS manifest value is true when a device supports HMS and FCM") { + // Given + val mockApplicationService = MockHelper.applicationService() + every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() + + val mockDeviceService = + spyk( + DeviceService( + mockApplicationService, + ), + ) + every { mockDeviceService.supportsHMS } returns true + every { mockDeviceService.supportsGooglePush() } returns true + mockkObject(AndroidUtils) + every { AndroidUtils.getManifestMetaBoolean(ApplicationProvider.getApplicationContext(), "com.onesignal.preferHMS") } returns true + + // When + val deviceType = mockDeviceService.deviceType + + // Then + deviceType shouldBe IDeviceService.DeviceType.Huawei + } + + test("devicetype is FCM when preferHMS manifest value is false when a device supports HMS and FCM") { + // Given + val mockApplicationService = MockHelper.applicationService() + every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() + + val mockDeviceService = + spyk( + DeviceService( + mockApplicationService, + ), + ) + every { mockDeviceService.supportsHMS } returns true + every { mockDeviceService.supportsGooglePush() } returns true + mockkObject(AndroidUtils) + every { AndroidUtils.getManifestMetaBoolean(ApplicationProvider.getApplicationContext(), "com.onesignal.preferHMS") } returns false + + // When + val deviceType = mockDeviceService.deviceType + + // Then + deviceType shouldBe IDeviceService.DeviceType.Android + } + + test("devicetype is FCM when preferHMS manifest value is missing when a device supports HMS and FCM") { + // Given + val mockApplicationService = MockHelper.applicationService() + every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() + + val mockDeviceService = + spyk( + DeviceService( + mockApplicationService, + ), + ) + every { mockDeviceService.supportsHMS } returns true + every { mockDeviceService.supportsGooglePush() } returns true + // When + val deviceType = mockDeviceService.deviceType + + // Then + deviceType shouldBe IDeviceService.DeviceType.Android + } +}) From 4a1ae404855dad747d61482edd1497ddeae3c887 Mon Sep 17 00:00:00 2001 From: Elliot Mawby Date: Fri, 2 Aug 2024 14:45:20 -0700 Subject: [PATCH 4/4] Use private mocks class in unit test --- .../internal/device/DeviceServiceTests.kt | 48 ++++++++----------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt index 5011e8e8d2..21f31fe174 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/device/DeviceServiceTests.kt @@ -15,15 +15,7 @@ import io.mockk.spyk class DeviceServiceTests : FunSpec({ test("devicetype is Huawei when preferHMS manifest value is true when a device supports HMS and FCM") { // Given - val mockApplicationService = MockHelper.applicationService() - every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() - - val mockDeviceService = - spyk( - DeviceService( - mockApplicationService, - ), - ) + val mockDeviceService = Mocks().deviceService every { mockDeviceService.supportsHMS } returns true every { mockDeviceService.supportsGooglePush() } returns true mockkObject(AndroidUtils) @@ -38,15 +30,7 @@ class DeviceServiceTests : FunSpec({ test("devicetype is FCM when preferHMS manifest value is false when a device supports HMS and FCM") { // Given - val mockApplicationService = MockHelper.applicationService() - every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() - - val mockDeviceService = - spyk( - DeviceService( - mockApplicationService, - ), - ) + val mockDeviceService = Mocks().deviceService every { mockDeviceService.supportsHMS } returns true every { mockDeviceService.supportsGooglePush() } returns true mockkObject(AndroidUtils) @@ -61,17 +45,10 @@ class DeviceServiceTests : FunSpec({ test("devicetype is FCM when preferHMS manifest value is missing when a device supports HMS and FCM") { // Given - val mockApplicationService = MockHelper.applicationService() - every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() - - val mockDeviceService = - spyk( - DeviceService( - mockApplicationService, - ), - ) + val mockDeviceService = Mocks().deviceService every { mockDeviceService.supportsHMS } returns true every { mockDeviceService.supportsGooglePush() } returns true + // When val deviceType = mockDeviceService.deviceType @@ -79,3 +56,20 @@ class DeviceServiceTests : FunSpec({ deviceType shouldBe IDeviceService.DeviceType.Android } }) + +private class Mocks { + val applicationService = + run { + val mockApplicationService = MockHelper.applicationService() + every { mockApplicationService.appContext } returns ApplicationProvider.getApplicationContext() + mockApplicationService + } + + val deviceService: DeviceService by lazy { + spyk( + DeviceService( + applicationService, + ), + ) + } +}