From dedab04246786d5625d48b7a9cba2e45a91bf723 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 9 Oct 2025 16:54:03 -0700 Subject: [PATCH 1/6] Detect timezone changes on new sessions Tack onto existing SessionListener's TrackSessionStartOperation call so this update would be combined. Otherwise, because the TrackSessionStartOperation has flush = true, the timing may mean 2 user update requests are made when it could be combined. Cons: the Session Listener is encroaching beyond what its scope should be. --- .../session/internal/session/impl/SessionListener.kt | 4 ++++ .../onesignal/user/internal/properties/PropertiesModel.kt | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt index 8d2161aa65..db0c96c4e8 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt @@ -11,6 +11,7 @@ import com.onesignal.session.internal.session.ISessionService import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.TrackSessionEndOperation import com.onesignal.user.internal.operations.TrackSessionStartOperation +import com.onesignal.user.internal.properties.PropertiesModelStore /** * The [SessionListener] is responsible for subscribing itself as an [ISessionLifecycleHandler] @@ -33,6 +34,7 @@ internal class SessionListener( private val _sessionService: ISessionService, private val _configModelStore: ConfigModelStore, private val _identityModelStore: IdentityModelStore, + private val _propertiesModelStore: PropertiesModelStore, private val _outcomeEventsController: IOutcomeEventsController, ) : IStartableService, ISessionLifecycleHandler { override fun start() { @@ -40,6 +42,8 @@ internal class SessionListener( } override fun onSessionStarted() { + // Detect any user updates to send with the TrackSessionStartOperation + _propertiesModelStore.model.update() _operationRepo.enqueue(TrackSessionStartOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId), true) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt index cd6a42e6d6..ab04ac393e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt @@ -1,5 +1,6 @@ package com.onesignal.user.internal.properties +import com.onesignal.common.TimeUtils import com.onesignal.common.modeling.MapModel import com.onesignal.common.modeling.Model import org.json.JSONObject @@ -122,4 +123,8 @@ class PropertiesModel : Model() { return null } + + fun update() { + timezone = TimeUtils.getTimeZoneId() + } } From 6018f4d987733aef11b1a9ca7c15ecfc1d0bb401 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 9 Oct 2025 17:29:13 -0700 Subject: [PATCH 2/6] No longer hydrate timezone from the server After the implementation to start updating timezone changes on new sessions, hydrating timezone from the user refresh call will conflict if the timing between the update and refresh requests is not perfect. For example, imagine this scenario if we still hydrate this property: 1. On a new session, timezone detects as changed from US to UK. A property update is enqueued. 2. This request is sent and the refresh user call returns (which still contains timezone as US) 3. The SDK hydrates timezone as US 4. On the next session, timezone is detected as changed from US to UK. --- .../impl/executors/RefreshUserOperationExecutor.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt index cec48b05a9..ecd0e59fb9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt @@ -89,9 +89,8 @@ internal class RefreshUserOperationExecutor( } } - if (response.properties.timezoneId != null) { - propertiesModel.timezone = response.properties.timezoneId - } + // No longer hydrate timezone from remote, set locally + propertiesModel.update() val subscriptionModels = mutableListOf() for (subscription in response.subscriptions) { From e433a4f1c6dc9d315b25ffb3e5fb3559f63c68c8 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 16 Oct 2025 12:17:35 -0700 Subject: [PATCH 3/6] Let UserManager refresh timezone changes on focus Let the UserManager refresh the changes to properties model, instead of SessionListener. The SubscriptionManager already refreshes the push subscription. --- .../internal/session/impl/SessionListener.kt | 4 ---- .../com/onesignal/user/internal/UserManager.kt | 14 +++++++++++++- .../impl/executors/RefreshUserOperationExecutor.kt | 3 ++- .../user/internal/properties/PropertiesModel.kt | 5 ----- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt index db0c96c4e8..8d2161aa65 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt @@ -11,7 +11,6 @@ import com.onesignal.session.internal.session.ISessionService import com.onesignal.user.internal.identity.IdentityModelStore import com.onesignal.user.internal.operations.TrackSessionEndOperation import com.onesignal.user.internal.operations.TrackSessionStartOperation -import com.onesignal.user.internal.properties.PropertiesModelStore /** * The [SessionListener] is responsible for subscribing itself as an [ISessionLifecycleHandler] @@ -34,7 +33,6 @@ internal class SessionListener( private val _sessionService: ISessionService, private val _configModelStore: ConfigModelStore, private val _identityModelStore: IdentityModelStore, - private val _propertiesModelStore: PropertiesModelStore, private val _outcomeEventsController: IOutcomeEventsController, ) : IStartableService, ISessionLifecycleHandler { override fun start() { @@ -42,8 +40,6 @@ internal class SessionListener( } override fun onSessionStarted() { - // Detect any user updates to send with the TrackSessionStartOperation - _propertiesModelStore.model.update() _operationRepo.enqueue(TrackSessionStartOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId), true) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt index 934d233183..7b31cac811 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt @@ -2,9 +2,12 @@ package com.onesignal.user.internal import com.onesignal.common.IDManager import com.onesignal.common.OneSignalUtils +import com.onesignal.common.TimeUtils import com.onesignal.common.events.EventProducer import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.core.internal.application.IApplicationLifecycleHandler +import com.onesignal.core.internal.application.IApplicationService import com.onesignal.core.internal.language.ILanguageContext import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -26,7 +29,8 @@ internal open class UserManager( private val _identityModelStore: IdentityModelStore, private val _propertiesModelStore: PropertiesModelStore, private val _languageContext: ILanguageContext, -) : IUserManager, ISingletonModelStoreChangeHandler { + private val _applicationService: IApplicationService, +) : IUserManager, IApplicationLifecycleHandler, ISingletonModelStoreChangeHandler { override val onesignalId: String get() = if (IDManager.isLocalId(_identityModel.onesignalId)) "" else _identityModel.onesignalId @@ -55,6 +59,7 @@ internal open class UserManager( } init { + _applicationService.addApplicationLifecycleHandler(this) _identityModelStore.subscribe(this) } @@ -260,4 +265,11 @@ internal open class UserManager( } } } + + override fun onFocus(firedOnSubscribe: Boolean) { + // Detect any user properties updates that changed + _propertiesModel.timezone = TimeUtils.getTimeZoneId() + } + + override fun onUnfocused() { } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt index ecd0e59fb9..d7bfa0f671 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt @@ -1,6 +1,7 @@ package com.onesignal.user.internal.operations.impl.executors import com.onesignal.common.NetworkUtils +import com.onesignal.common.TimeUtils import com.onesignal.common.exceptions.BackendException import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.core.internal.config.ConfigModelStore @@ -90,7 +91,7 @@ internal class RefreshUserOperationExecutor( } // No longer hydrate timezone from remote, set locally - propertiesModel.update() + propertiesModel.timezone = TimeUtils.getTimeZoneId() val subscriptionModels = mutableListOf() for (subscription in response.subscriptions) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt index ab04ac393e..cd6a42e6d6 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt @@ -1,6 +1,5 @@ package com.onesignal.user.internal.properties -import com.onesignal.common.TimeUtils import com.onesignal.common.modeling.MapModel import com.onesignal.common.modeling.Model import org.json.JSONObject @@ -123,8 +122,4 @@ class PropertiesModel : Model() { return null } - - fun update() { - timezone = TimeUtils.getTimeZoneId() - } } From f225c3298a32625feafca709ab60116a57512425 Mon Sep 17 00:00:00 2001 From: Nan Date: Mon, 20 Oct 2025 12:01:26 -0700 Subject: [PATCH 4/6] tests: pass application service --- .../com/onesignal/user/internal/UserManagerTests.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt index 41e836eb9b..64039ccc76 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt @@ -26,7 +26,7 @@ class UserManagerTests : FunSpec({ every { languageContext.language = capture(languageSlot) } answers { } val userManager = - UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), MockHelper.propertiesModelStore(), languageContext) + UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), MockHelper.propertiesModelStore(), languageContext, MockHelper.applicationService()) // When userManager.setLanguage("new-language") @@ -44,7 +44,7 @@ class UserManagerTests : FunSpec({ } val userManager = - UserManager(mockSubscriptionManager, identityModelStore, MockHelper.propertiesModelStore(), MockHelper.languageContext()) + UserManager(mockSubscriptionManager, identityModelStore, MockHelper.propertiesModelStore(), MockHelper.languageContext(), MockHelper.applicationService()) // When val externalId = userManager.externalId @@ -63,7 +63,7 @@ class UserManagerTests : FunSpec({ } val userManager = - UserManager(mockSubscriptionManager, identityModelStore, MockHelper.propertiesModelStore(), MockHelper.languageContext()) + UserManager(mockSubscriptionManager, identityModelStore, MockHelper.propertiesModelStore(), MockHelper.languageContext(), MockHelper.applicationService()) // When val alias1 = userManager.aliases["my-alias-key1"] @@ -102,7 +102,7 @@ class UserManagerTests : FunSpec({ } val userManager = - UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), propertiesModelStore, MockHelper.languageContext()) + UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), propertiesModelStore, MockHelper.languageContext(), MockHelper.applicationService()) // When val tag1 = propertiesModelStore.model.tags["my-tag-key1"] @@ -141,7 +141,7 @@ class UserManagerTests : FunSpec({ it.tags["my-tag-key1"] = "my-tag-value1" } - val userManager = UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), propertiesModelStore, MockHelper.languageContext()) + val userManager = UserManager(mockSubscriptionManager, MockHelper.identityModelStore(), propertiesModelStore, MockHelper.languageContext(), MockHelper.applicationService()) // When val tagSnapshot1 = userManager.getTags() @@ -174,6 +174,7 @@ class UserManagerTests : FunSpec({ MockHelper.identityModelStore(), MockHelper.propertiesModelStore(), MockHelper.languageContext(), + MockHelper.applicationService(), ) // When From a4a6d5e6cc245188a6d3e5f138aefea5ffe6462c Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 23 Oct 2025 11:31:33 -0700 Subject: [PATCH 5/6] Add tests for timezone * Add UserManagerTests that onFocus will update timezone * Update an existing test in RefreshUserOperationExecutorTests that tests a successful getUser request and hydration already. Let's add on the timezone component to this hydration, which is that timezone is no longer hydrated from the server but set locally. * Add TimeUtilsTest class testing getTimeZoneId --- .../com/onesignal/common/TimeUtilsTest.kt | 48 +++++++ .../user/internal/UserManagerTests.kt | 35 +++++ .../RefreshUserOperationExecutorTests.kt | 121 +++++++++++------- 3 files changed, 155 insertions(+), 49 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/TimeUtilsTest.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/TimeUtilsTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/TimeUtilsTest.kt new file mode 100644 index 0000000000..b470727370 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/TimeUtilsTest.kt @@ -0,0 +1,48 @@ +package com.onesignal.common + +import android.os.Build +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.string.shouldNotBeEmpty +import io.kotest.matchers.string.shouldNotContain +import java.time.ZoneId +import java.util.TimeZone + +@RobolectricTest +class TimeUtilsTest : FunSpec({ + + test("getTimeZoneId returns correct time zone id") { + // Given + val expected = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + ZoneId.systemDefault().id + } else { + TimeZone.getDefault().id + } + + // When + val actual = TimeUtils.getTimeZoneId() + + // Then + actual shouldBe expected + actual.shouldNotBeEmpty() + } + + test("getTimeZoneId returns valid timezone format") { + // When + val timeZoneId = TimeUtils.getTimeZoneId() + + // Then + timeZoneId.shouldNotBeEmpty() + timeZoneId shouldNotBe "" + + // Valid timezone IDs follow IANA format patterns: + // - Continental zones: "America/New_York", "Europe/London" + // - UTC variants: "UTC", "GMT" + // - Offset formats: "GMT+05:30", "UTC-08:00" + // Should not contain spaces or invalid characters + timeZoneId.shouldNotContain(" ") + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt index 64039ccc76..f39602c0c3 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt @@ -1,5 +1,6 @@ package com.onesignal.user.internal +import com.onesignal.common.TimeUtils import com.onesignal.core.internal.language.ILanguageContext import com.onesignal.mocks.MockHelper import com.onesignal.user.internal.subscriptions.ISubscriptionManager @@ -10,8 +11,10 @@ import io.kotest.matchers.shouldNotBe import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.runs import io.mockk.slot +import io.mockk.unmockkObject import io.mockk.verify class UserManagerTests : FunSpec({ @@ -192,4 +195,36 @@ class UserManagerTests : FunSpec({ verify(exactly = 1) { mockSubscriptionManager.addSmsSubscription("+15558675309") } verify(exactly = 1) { mockSubscriptionManager.removeSmsSubscription("+15558675309") } } + + test("onFocus updates timezone") { + // Given + val mockTimeZone = "Europe/Foo" + mockkObject(TimeUtils) + every { TimeUtils.getTimeZoneId() } returns mockTimeZone + + val mockPropertiesModelStore = MockHelper.propertiesModelStore() + + val userManager = + UserManager( + mockk(), + MockHelper.identityModelStore(), + mockPropertiesModelStore, + MockHelper.languageContext(), + MockHelper.applicationService(), + ) + + val propertiesModel = mockPropertiesModelStore.model + propertiesModel.timezone shouldNotBe mockTimeZone + + try { + // When + userManager.onFocus(firedOnSubscribe = false) + + // Then + propertiesModel.timezone shouldBe mockTimeZone + } finally { + // Clean up the mock + unmockkObject(TimeUtils) + } + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt index 07acdd60cb..2689d761af 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/RefreshUserOperationExecutorTests.kt @@ -1,5 +1,6 @@ package com.onesignal.user.internal.operations +import com.onesignal.common.TimeUtils import com.onesignal.common.exceptions.BackendException import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.core.internal.operations.ExecutionResult @@ -27,7 +28,9 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.runs +import io.mockk.unmockkObject class RefreshUserOperationExecutorTests : FunSpec({ val appId = "appId" @@ -37,13 +40,24 @@ class RefreshUserOperationExecutorTests : FunSpec({ val remoteSubscriptionId1 = "remote-subscriptionId1" val remoteSubscriptionId2 = "remote-subscriptionId2" - test("refresh user is successful") { + test("refresh user is successful and models are hydrated properly") { // Given + val localTimeZone = "Europe/Local" + val remoteTimeZone = "Europe/Remote" + mockkObject(TimeUtils) + every { TimeUtils.getTimeZoneId() } returns localTimeZone + + val localCountry = "US" + val remoteCountry = "VT" + val localLanguage = "fr" + val remoteLanguage = "it" + val remoteTags = mapOf("tagKey1" to "remote-1", "tagKey2" to "remote-2") + val mockUserBackendService = mockk() coEvery { mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) } returns CreateUserResponse( mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId, "aliasLabel1" to "aliasValue1"), - PropertiesObject(country = "US"), + PropertiesObject(country = remoteCountry, language = remoteLanguage, timezoneId = remoteTimeZone, tags = remoteTags), listOf( SubscriptionObject(existingSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "on-backend-push-token"), SubscriptionObject(remoteSubscriptionId1, SubscriptionObjectType.ANDROID_PUSH, enabled = true, token = "pushToken2"), @@ -61,8 +75,8 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockPropertiesModelStore = MockHelper.propertiesModelStore() val mockPropertiesModel = PropertiesModel() mockPropertiesModel.onesignalId = remoteOneSignalId - mockPropertiesModel.country = "VT" - mockPropertiesModel.language = "language" + mockPropertiesModel.country = localCountry + mockPropertiesModel.language = localLanguage every { mockPropertiesModelStore.model } returns mockPropertiesModel every { mockPropertiesModelStore.replace(any(), any()) } just runs @@ -84,7 +98,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -97,40 +111,49 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) - // When - val response = loginUserOperationExecutor.execute(operations) - - // Then - response.result shouldBe ExecutionResult.SUCCESS - coVerify(exactly = 1) { - mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) - mockIdentityModelStore.replace( - withArg { - it["aliasLabel1"] shouldBe "aliasValue1" - }, - ModelChangeTags.HYDRATE, - ) - mockPropertiesModelStore.replace( - withArg { - it.country shouldBe "US" - it.language shouldBe null - }, - ModelChangeTags.HYDRATE, - ) - mockSubscriptionsModelStore.replaceAll( - withArg { - it.count() shouldBe 2 - it[0].id shouldBe remoteSubscriptionId2 - it[0].type shouldBe SubscriptionType.EMAIL - it[0].optedIn shouldBe true - it[0].address shouldBe "name@company.com" - it[1].id shouldBe existingSubscriptionId1 - it[1].type shouldBe SubscriptionType.PUSH - it[1].optedIn shouldBe true - it[1].address shouldBe onDevicePushToken - }, - ModelChangeTags.HYDRATE, - ) + try { + // When + val response = refreshUserOperationExecutor.execute(operations) + + // Then + response.result shouldBe ExecutionResult.SUCCESS + coVerify(exactly = 1) { + mockUserBackendService.getUser(appId, IdentityConstants.ONESIGNAL_ID, remoteOneSignalId) + mockIdentityModelStore.replace( + withArg { + it["aliasLabel1"] shouldBe "aliasValue1" + }, + ModelChangeTags.HYDRATE, + ) + // The properties model should be set with appropriate remote and local values + mockPropertiesModelStore.replace( + withArg { + it.onesignalId shouldBe remoteOneSignalId + it.country shouldBe remoteCountry + it.language shouldBe remoteLanguage + it.tags shouldBe remoteTags + it.timezone shouldBe localTimeZone // timezone is set locally + }, + ModelChangeTags.HYDRATE, + ) + mockSubscriptionsModelStore.replaceAll( + withArg { + it.count() shouldBe 2 + it[0].id shouldBe remoteSubscriptionId2 + it[0].type shouldBe SubscriptionType.EMAIL + it[0].optedIn shouldBe true + it[0].address shouldBe "name@company.com" + it[1].id shouldBe existingSubscriptionId1 + it[1].type shouldBe SubscriptionType.PUSH + it[1].optedIn shouldBe true + it[1].address shouldBe onDevicePushToken + }, + ModelChangeTags.HYDRATE, + ) + } + } finally { + // Clean up the mock + unmockkObject(TimeUtils) } } @@ -159,7 +182,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockSubscriptionsModelStore = mockk() val mockBuildUserService = mockk() - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -173,7 +196,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) // When - val response = loginUserOperationExecutor.execute(operations) + val response = refreshUserOperationExecutor.execute(operations) // Then response.result shouldBe ExecutionResult.SUCCESS @@ -198,7 +221,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockSubscriptionsModelStore = mockk() val mockBuildUserService = mockk() - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -212,7 +235,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) // When - val response = loginUserOperationExecutor.execute(operations) + val response = refreshUserOperationExecutor.execute(operations) // Then response.result shouldBe ExecutionResult.FAIL_RETRY @@ -233,7 +256,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockSubscriptionsModelStore = mockk() val mockBuildUserService = mockk() - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -247,7 +270,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) // When - val response = loginUserOperationExecutor.execute(operations) + val response = refreshUserOperationExecutor.execute(operations) // Then response.result shouldBe ExecutionResult.FAIL_NORETRY @@ -268,7 +291,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockBuildUserService = mockk() every { mockBuildUserService.getRebuildOperationsIfCurrentUser(any(), any()) } returns null - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -282,7 +305,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) // When - val response = loginUserOperationExecutor.execute(operations) + val response = refreshUserOperationExecutor.execute(operations) // Then response.result shouldBe ExecutionResult.FAIL_NORETRY @@ -305,7 +328,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val mockConfigModelStore = MockHelper.configModelStore().also { it.model.opRepoPostCreateRetryUpTo = 1_000 } val newRecordState = getNewRecordState(mockConfigModelStore).also { it.add(remoteOneSignalId) } - val loginUserOperationExecutor = + val refreshUserOperationExecutor = RefreshUserOperationExecutor( mockUserBackendService, mockIdentityModelStore, @@ -319,7 +342,7 @@ class RefreshUserOperationExecutorTests : FunSpec({ val operations = listOf(RefreshUserOperation(appId, remoteOneSignalId)) // When - val response = loginUserOperationExecutor.execute(operations) + val response = refreshUserOperationExecutor.execute(operations) // Then response.result shouldBe ExecutionResult.FAIL_RETRY From 14b5d76ab494938aa5c21a5d0eea88bc70bb20c2 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 23 Oct 2025 11:33:56 -0700 Subject: [PATCH 6/6] nit: remove extraneous non-null assert It's already non-null --- .../operations/impl/executors/LoginUserOperationExecutor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt index 8a24454a6d..46968b3e71 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt @@ -146,7 +146,7 @@ internal class LoginUserOperationExecutor( var identities = mapOf() var subscriptions = mapOf() val properties = mutableMapOf() - properties["timezone_id"] = TimeUtils.getTimeZoneId()!! + properties["timezone_id"] = TimeUtils.getTimeZoneId() properties["language"] = _languageContext.language if (createUserOperation.externalId != null) {