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/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) { 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..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 @@ -89,9 +90,8 @@ internal class RefreshUserOperationExecutor( } } - if (response.properties.timezoneId != null) { - propertiesModel.timezone = response.properties.timezoneId - } + // No longer hydrate timezone from remote, set locally + propertiesModel.timezone = TimeUtils.getTimeZoneId() val subscriptionModels = mutableListOf() for (subscription in response.subscriptions) { 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 41e836eb9b..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({ @@ -26,7 +29,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 +47,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 +66,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 +105,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 +144,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 +177,7 @@ class UserManagerTests : FunSpec({ MockHelper.identityModelStore(), MockHelper.propertiesModelStore(), MockHelper.languageContext(), + MockHelper.applicationService(), ) // When @@ -191,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