From 0212f1149bd6f0fe7afe65d9776086336022efbd Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 14 Nov 2023 09:56:52 -0500 Subject: [PATCH 1/3] Synchronize initWithContext --- .../com/onesignal/internal/OneSignalImp.kt | 237 ++++++++++-------- .../onesignal/internal/OneSignalImplTests.kt | 42 ++++ 2 files changed, 177 insertions(+), 102 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 979ea310e2..b7216fde7c 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -131,6 +131,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { private var _consentRequired: Boolean? = null private var _consentGiven: Boolean? = null private var _disableGMSMissingPrompt: Boolean? = null + private val initLock: Any = Any() private val loginLock: Any = Any() private val listOfModules = @@ -173,128 +174,161 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { // do not do this again if already initialized if (isInitialized) { + Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized") return true } - PreferenceStoreFix.ensureNoObfuscatedPrefStore(context) + synchronized(initLock) { + // check whether we've been initialized again, now that we have the lock + if (isInitialized) { + Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized") + return true + } - // start the application service. This is called explicitly first because we want - // to make sure it has the context provided on input, for all other startable services - // to depend on if needed. - val applicationService = services.getService() - (applicationService as ApplicationService).start(context) + Logging.log(LogLevel.DEBUG, "initWithContext: SDK initializing") - // Give the logging singleton access to the application service to support visual logging. - Logging.applicationService = applicationService + PreferenceStoreFix.ensureNoObfuscatedPrefStore(context) - // get the current config model, if there is one - configModel = services.getService().model - sessionModel = services.getService().model + // start the application service. This is called explicitly first because we want + // to make sure it has the context provided on input, for all other startable services + // to depend on if needed. + val applicationService = services.getService() + (applicationService as ApplicationService).start(context) - // initWithContext is called by our internal services/receivers/activites but they do not provide - // an appId (they don't know it). If the app has never called the external initWithContext - // prior to our services/receivers/activities we will blow up, as no appId has been established. - if (appId == null && !configModel!!.hasProperty(ConfigModel::appId.name)) { - Logging.warn("initWithContext called without providing appId, and no appId has been established!") - return false - } + // Give the logging singleton access to the application service to support visual logging. + Logging.applicationService = applicationService - var forceCreateUser = false - // if the app id was specified as input, update the config model with it - if (appId != null) { - if (!configModel!!.hasProperty(ConfigModel::appId.name) || configModel!!.appId != appId) { - forceCreateUser = true + // get the current config model, if there is one + configModel = services.getService().model + sessionModel = services.getService().model + + // initWithContext is called by our internal services/receivers/activites but they do not provide + // an appId (they don't know it). If the app has never called the external initWithContext + // prior to our services/receivers/activities we will blow up, as no appId has been established. + if (appId == null && !configModel!!.hasProperty(ConfigModel::appId.name)) { + Logging.warn("initWithContext called without providing appId, and no appId has been established!") + return false } - configModel!!.appId = appId - } - // if requires privacy consent was set prior to init, set it in the model now - if (_consentRequired != null) { - configModel!!.consentRequired = _consentRequired!! - } + var forceCreateUser = false + // if the app id was specified as input, update the config model with it + if (appId != null) { + if (!configModel!!.hasProperty(ConfigModel::appId.name) || configModel!!.appId != appId) { + forceCreateUser = true + } + configModel!!.appId = appId + } - // if privacy consent was set prior to init, set it in the model now - if (_consentGiven != null) { - configModel!!.consentGiven = _consentGiven!! - } + // if requires privacy consent was set prior to init, set it in the model now + if (_consentRequired != null) { + configModel!!.consentRequired = _consentRequired!! + } - if (_disableGMSMissingPrompt != null) { - configModel!!.disableGMSMissingPrompt = _disableGMSMissingPrompt!! - } + // if privacy consent was set prior to init, set it in the model now + if (_consentGiven != null) { + configModel!!.consentGiven = _consentGiven!! + } - // "Inject" the services required by this main class - _location = services.getService() - _user = services.getService() - _session = services.getService() - iam = services.getService() - _notifications = services.getService() - operationRepo = services.getService() - propertiesModelStore = services.getService() - identityModelStore = services.getService() - subscriptionModelStore = services.getService() - preferencesService = services.getService() - - // Instantiate and call the IStartableServices - startupService = services.getService() - startupService!!.bootstrap() - - if (forceCreateUser || !identityModelStore!!.model.hasProperty(IdentityConstants.ONESIGNAL_ID)) { - val legacyPlayerId = preferencesService!!.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID) - if (legacyPlayerId == null) { - Logging.debug("initWithContext: creating new device-scoped user") - createAndSwitchToNewUser() - operationRepo!!.enqueue( - LoginUserOperation( - configModel!!.appId, - identityModelStore!!.model.onesignalId, - identityModelStore!!.model.externalId, - ), - ) - } else { - Logging.debug("initWithContext: creating user linked to subscription $legacyPlayerId") + if (_disableGMSMissingPrompt != null) { + configModel!!.disableGMSMissingPrompt = _disableGMSMissingPrompt!! + } - // Converting a 4.x SDK to the 5.x SDK. We pull the legacy user sync values to create the subscription model, then enqueue - // a specialized `LoginUserFromSubscriptionOperation`, which will drive fetching/refreshing of the local user - // based on the subscription ID we do have. - val legacyUserSyncString = - preferencesService!!.getString( + // "Inject" the services required by this main class + _location = services.getService() + _user = services.getService() + _session = services.getService() + iam = services.getService() + _notifications = services.getService() + operationRepo = services.getService() + propertiesModelStore = services.getService() + identityModelStore = services.getService() + subscriptionModelStore = services.getService() + preferencesService = services.getService() + + // Instantiate and call the IStartableServices + startupService = services.getService() + startupService!!.bootstrap() + + if (forceCreateUser || !identityModelStore!!.model.hasProperty(IdentityConstants.ONESIGNAL_ID)) { + val legacyPlayerId = preferencesService!!.getString( + PreferenceStores.ONESIGNAL, + PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID + ) + if (legacyPlayerId == null) { + Logging.debug("initWithContext: creating new device-scoped user") + createAndSwitchToNewUser() + operationRepo!!.enqueue( + LoginUserOperation( + configModel!!.appId, + identityModelStore!!.model.onesignalId, + identityModelStore!!.model.externalId, + ), + ) + } else { + Logging.debug("initWithContext: creating user linked to subscription $legacyPlayerId") + + // Converting a 4.x SDK to the 5.x SDK. We pull the legacy user sync values to create the subscription model, then enqueue + // a specialized `LoginUserFromSubscriptionOperation`, which will drive fetching/refreshing of the local user + // based on the subscription ID we do have. + val legacyUserSyncString = + preferencesService!!.getString( + PreferenceStores.ONESIGNAL, + PreferenceOneSignalKeys.PREFS_LEGACY_USER_SYNCVALUES, + ) + var suppressBackendOperation = false + + if (legacyUserSyncString != null) { + val legacyUserSyncJSON = JSONObject(legacyUserSyncString) + val notificationTypes = legacyUserSyncJSON.getInt("notification_types") + + val pushSubscriptionModel = SubscriptionModel() + pushSubscriptionModel.id = legacyPlayerId + pushSubscriptionModel.type = SubscriptionType.PUSH + pushSubscriptionModel.optedIn = + notificationTypes != SubscriptionStatus.NO_PERMISSION.value && notificationTypes != SubscriptionStatus.UNSUBSCRIBE.value + pushSubscriptionModel.address = + legacyUserSyncJSON.safeString("identifier") ?: "" + pushSubscriptionModel.status = SubscriptionStatus.fromInt(notificationTypes) + ?: SubscriptionStatus.NO_PERMISSION + configModel!!.pushSubscriptionId = legacyPlayerId + subscriptionModelStore!!.add( + pushSubscriptionModel, + ModelChangeTags.NO_PROPOGATE + ) + suppressBackendOperation = true + } + + createAndSwitchToNewUser(suppressBackendOperation = suppressBackendOperation) + + operationRepo!!.enqueue( + LoginUserFromSubscriptionOperation( + configModel!!.appId, + identityModelStore!!.model.onesignalId, + legacyPlayerId + ), + ) + preferencesService!!.saveString( PreferenceStores.ONESIGNAL, - PreferenceOneSignalKeys.PREFS_LEGACY_USER_SYNCVALUES, + PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID, + null ) - var suppressBackendOperation = false - - if (legacyUserSyncString != null) { - val legacyUserSyncJSON = JSONObject(legacyUserSyncString) - val notificationTypes = legacyUserSyncJSON.getInt("notification_types") - - val pushSubscriptionModel = SubscriptionModel() - pushSubscriptionModel.id = legacyPlayerId - pushSubscriptionModel.type = SubscriptionType.PUSH - pushSubscriptionModel.optedIn = notificationTypes != SubscriptionStatus.NO_PERMISSION.value && notificationTypes != SubscriptionStatus.UNSUBSCRIBE.value - pushSubscriptionModel.address = legacyUserSyncJSON.safeString("identifier") ?: "" - pushSubscriptionModel.status = SubscriptionStatus.fromInt(notificationTypes) ?: SubscriptionStatus.NO_PERMISSION - configModel!!.pushSubscriptionId = legacyPlayerId - subscriptionModelStore!!.add(pushSubscriptionModel, ModelChangeTags.NO_PROPOGATE) - suppressBackendOperation = true } - - createAndSwitchToNewUser(suppressBackendOperation = suppressBackendOperation) - + } else { + Logging.debug("initWithContext: using cached user ${identityModelStore!!.model.onesignalId}") operationRepo!!.enqueue( - LoginUserFromSubscriptionOperation(configModel!!.appId, identityModelStore!!.model.onesignalId, legacyPlayerId), + RefreshUserOperation( + configModel!!.appId, + identityModelStore!!.model.onesignalId + ) ) - preferencesService!!.saveString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID, null) } - } else { - Logging.debug("initWithContext: using cached user ${identityModelStore!!.model.onesignalId}") - operationRepo!!.enqueue(RefreshUserOperation(configModel!!.appId, identityModelStore!!.model.onesignalId)) - } - startupService!!.start() + startupService!!.start() - isInitialized = true + isInitialized = true - return true + return true + } } override fun login( @@ -304,7 +338,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)") if (!isInitialized) { - Logging.log(LogLevel.ERROR, "Must call 'initWithContext' before using Login") + throw Exception("Must call 'initWithContext' before 'login'") } var currentIdentityExternalId: String? = null @@ -378,8 +412,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { Logging.log(LogLevel.DEBUG, "logout()") if (!isInitialized) { - Logging.log(LogLevel.ERROR, "Must call 'initWithContext' before using Login") - return + throw Exception("Must call 'initWithContext' before 'logout'") } // only allow one login/logout at a time diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt new file mode 100644 index 0000000000..a540e36055 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt @@ -0,0 +1,42 @@ +package com.onesignal.internal + +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import io.kotest.assertions.throwables.shouldThrowUnit +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.runner.junit4.KotestTestRunner +import org.junit.runner.RunWith + +@RunWith(KotestTestRunner::class) +class OneSignalImplTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + test("attempting login before initWithContext throws exception") { + // Given + val os = OneSignalImp() + + // When + val exception = shouldThrowUnit { + os.login("login-id") + } + + // Then + exception.message shouldBe "Must call 'initWithContext' before 'login'" + } + + test("attempting logout before initWithContext throws exception") { + // Given + val os = OneSignalImp() + + // When + val exception = shouldThrowUnit { + os.logout() + } + + // Then + exception.message shouldBe "Must call 'initWithContext' before 'logout'" + } +}) From f89a8295a8b702f66d63d2e1febca40bd927c17e Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 8 Nov 2023 07:23:42 -0500 Subject: [PATCH 2/3] Rename OneSignalImplTests.kt -> OneSignalImpTests Fix linting errors with OneSignalImp and OneSignalImpTests --- .../com/onesignal/internal/OneSignalImp.kt | 19 ++++++++++--------- ...ignalImplTests.kt => OneSignalImpTests.kt} | 16 +++++++++------- 2 files changed, 19 insertions(+), 16 deletions(-) rename OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/{OneSignalImplTests.kt => OneSignalImpTests.kt} (77%) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index b7216fde7c..0932c0b589 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -250,10 +250,11 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { startupService!!.bootstrap() if (forceCreateUser || !identityModelStore!!.model.hasProperty(IdentityConstants.ONESIGNAL_ID)) { - val legacyPlayerId = preferencesService!!.getString( - PreferenceStores.ONESIGNAL, - PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID - ) + val legacyPlayerId = + preferencesService!!.getString( + PreferenceStores.ONESIGNAL, + PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID, + ) if (legacyPlayerId == null) { Logging.debug("initWithContext: creating new device-scoped user") createAndSwitchToNewUser() @@ -293,7 +294,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { configModel!!.pushSubscriptionId = legacyPlayerId subscriptionModelStore!!.add( pushSubscriptionModel, - ModelChangeTags.NO_PROPOGATE + ModelChangeTags.NO_PROPOGATE, ) suppressBackendOperation = true } @@ -304,13 +305,13 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { LoginUserFromSubscriptionOperation( configModel!!.appId, identityModelStore!!.model.onesignalId, - legacyPlayerId + legacyPlayerId, ), ) preferencesService!!.saveString( PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_PLAYER_ID, - null + null, ) } } else { @@ -318,8 +319,8 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { operationRepo!!.enqueue( RefreshUserOperation( configModel!!.appId, - identityModelStore!!.model.onesignalId - ) + identityModelStore!!.model.onesignalId, + ), ) } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt similarity index 77% rename from OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt rename to OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt index a540e36055..2f855a51d1 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImplTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt @@ -9,7 +9,7 @@ import io.kotest.runner.junit4.KotestTestRunner import org.junit.runner.RunWith @RunWith(KotestTestRunner::class) -class OneSignalImplTests : FunSpec({ +class OneSignalImpTests : FunSpec({ beforeAny { Logging.logLevel = LogLevel.NONE } @@ -19,9 +19,10 @@ class OneSignalImplTests : FunSpec({ val os = OneSignalImp() // When - val exception = shouldThrowUnit { - os.login("login-id") - } + val exception = + shouldThrowUnit { + os.login("login-id") + } // Then exception.message shouldBe "Must call 'initWithContext' before 'login'" @@ -32,9 +33,10 @@ class OneSignalImplTests : FunSpec({ val os = OneSignalImp() // When - val exception = shouldThrowUnit { - os.logout() - } + val exception = + shouldThrowUnit { + os.logout() + } // Then exception.message shouldBe "Must call 'initWithContext' before 'logout'" From 6b958ec6439dbadd14218a9ee993c1db291b9c4a Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 14 Nov 2023 09:58:27 -0500 Subject: [PATCH 3/3] Remove isInitialized checked prior to obtaining initLock --- .../src/main/java/com/onesignal/internal/OneSignalImp.kt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 0932c0b589..573e0b449f 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -172,14 +172,8 @@ internal class OneSignalImp : IOneSignal, IServiceProvider { ): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context, appId: $appId)") - // do not do this again if already initialized - if (isInitialized) { - Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized") - return true - } - synchronized(initLock) { - // check whether we've been initialized again, now that we have the lock + // do not do this again if already initialized if (isInitialized) { Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized") return true