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 f0f59fea1..cb956962f 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,24 +131,19 @@ internal class OneSignalImp : IOneSignal, override val debug: IDebugManager = DebugManager() override val session: ISessionManager - get() = - waitAndReturn { services.getService() } + get() = if (isReadyFor("Session")) services.getService() else UninitializedSessionManager override val notifications: INotificationsManager - get() = - waitAndReturn { services.getService() } + get() = if (isReadyFor("Notifications")) services.getService() else UninitializedNotificationsManager override val location: ILocationManager - get() = - waitAndReturn { services.getService() } + get() = if (isReadyFor("Location")) services.getService() else UninitializedLocationManager override val inAppMessages: IInAppMessagesManager - get() = - waitAndReturn { services.getService() } + get() = if (isReadyFor("InAppMessages")) services.getService() else UninitializedInAppMessagesManager override val user: IUserManager - get() = - waitAndReturn { services.getService() } + get() = if (isReadyFor("User")) services.getService() else UninitializedUserManager // Services required by this class // WARNING: OperationRepo depends on OperationModelStore which in-turn depends @@ -491,9 +486,7 @@ internal class OneSignalImp : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - waitForInit(operationName = "login") - - if (isMissing(externalId, "login: externalId")) return + if (!isReadyFor("login") || isMissing(externalId, "login: externalId")) return val context = loginHelper.switchUser(externalId, jwtBearerToken).context ?: return @@ -503,7 +496,7 @@ internal class OneSignalImp : IOneSignal, override fun logout() { Logging.log(LogLevel.DEBUG, "Calling deprecated logout()") - waitForInit(operationName = "logout") + if (!isReadyFor("logout")) return val context = logoutHelper.switchUser() ?: return @@ -516,7 +509,7 @@ internal class OneSignalImp : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})") - waitForInit(operationName = "updateUserJwt") + if (!isReadyFor("updateUserJwt")) return if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return @@ -526,12 +519,12 @@ internal class OneSignalImp : IOneSignal, } override fun addUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) { - waitForInit(operationName = "addUserJwtInvalidatedListener") + if (!isReadyFor("addUserJwtInvalidatedListener")) return jwtTokenStore.addUserJwtInvalidatedListener(listener) } override fun removeUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) { - waitForInit(operationName = "removeUserJwtInvalidatedListener") + if (!isReadyFor("removeUserJwtInvalidatedListener")) return jwtTokenStore.removeUserJwtInvalidatedListener(listener) } @@ -543,6 +536,30 @@ internal class OneSignalImp : IOneSignal, override fun getAllServices(c: Class): List = services.getAllServices(c) + // Log and return false instead of throwing when init never started or failed, so callers can + // skip the call or hand out a stand-in manager. + private fun isReadyFor(operationName: String): Boolean = + ignoreIfNotInitialized(operationName) { waitForInit(operationName) } == null + + private suspend fun isReadyForSuspend(operationName: String): Boolean = + ignoreIfNotInitialized(operationName) { suspendUntilInit(operationName) } == null + + /** Runs [wait] and returns the init failure it threw, after logging it, or null once init succeeded. */ + private inline fun ignoreIfNotInitialized( + operationName: String, + wait: () -> Unit, + ): IllegalStateException? = + try { + wait() + null + } catch (e: CancellationException) { + // A subclass of IllegalStateException, so it must be rethrown before the catch below. + throw e + } catch (e: IllegalStateException) { + Logging.error("'$operationName' ignored: ${e.message}", e) + e + } + /** * Blocking version that waits for initialization to complete. * Uses runBlocking to bridge to the suspend implementation. @@ -680,13 +697,6 @@ internal class OneSignalImp : IOneSignal, return getter() } - // Blocks until init completes; [waitForInit] throws on NOT_STARTED / FAILED and - // returns once initState == SUCCESS. - private fun waitAndReturn(getter: () -> T): T { - waitForInit() - return getter() - } - private fun blockingGet(getter: () -> T): T { try { if (AndroidUtils.isRunningOnMainThread()) { @@ -713,27 +723,27 @@ internal class OneSignalImp : IOneSignal, override suspend fun getSession(): ISessionManager = withContext(ioDispatcher) { - suspendAndReturn { services.getService() } + if (isReadyForSuspend("Session")) services.getService() else UninitializedSessionManager } override suspend fun getNotifications(): INotificationsManager = withContext(ioDispatcher) { - suspendAndReturn { services.getService() } + if (isReadyForSuspend("Notifications")) services.getService() else UninitializedNotificationsManager } override suspend fun getLocation(): ILocationManager = withContext(ioDispatcher) { - suspendAndReturn { services.getService() } + if (isReadyForSuspend("Location")) services.getService() else UninitializedLocationManager } override suspend fun getInAppMessages(): IInAppMessagesManager = withContext(ioDispatcher) { - suspendAndReturn { services.getService() } + if (isReadyForSuspend("InAppMessages")) services.getService() else UninitializedInAppMessagesManager } override suspend fun getUser(): IUserManager = withContext(ioDispatcher) { - suspendAndReturn { services.getService() } + if (isReadyForSuspend("User")) services.getService() else UninitializedUserManager } override suspend fun getConsentRequired(): Boolean = @@ -841,9 +851,7 @@ internal class OneSignalImp : IOneSignal, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - // suspendUntilInit throws on NOT_STARTED / FAILED (preserving initFailureException as the - // cause), and only returns once initState == SUCCESS — so no post-check is needed here. - suspendUntilInit(operationName = "login") + if (!isReadyForSuspend("login")) return@withContext if (isMissing(externalId, "login: externalId")) return@withContext @@ -898,17 +906,8 @@ internal class OneSignalImp : IOneSignal, /** Waits for init; returns a NOT_INITIALIZED failure instead of throwing when it never started or failed. */ internal suspend fun awaitInitForLogin(): OneSignalResult? = - try { - suspendUntilInit(operationName = "login") - null - } catch (e: CancellationException) { - // A subclass of IllegalStateException, so it must be rethrown before the catch below. - throw e - } catch (e: IllegalStateException) { - // Callers can ignore the result, so the failure has to show up in the log. - Logging.error("login(externalId, profile) skipped: ${e.message}", e) - OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, e.message, cause = e) - } + ignoreIfNotInitialized("login") { suspendUntilInit(operationName = "login") } + ?.let { OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, it.message, cause = it) } private fun loginFailureMessage(wait: OperationWaitResult): String = wait.httpStatusCode?.let { "Login did not complete (HTTP $it)." } ?: "Login did not complete." @@ -919,11 +918,7 @@ internal class OneSignalImp : IOneSignal, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})") - suspendUntilInit(operationName = "updateUserJwt") - - if (!isInitialized) { - throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'") - } + if (!isReadyForSuspend("updateUserJwt")) return@withContext if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) { return@withContext @@ -937,7 +932,7 @@ internal class OneSignalImp : IOneSignal, withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "logoutSuspend()") - suspendUntilInit(operationName = "logout") + if (!isReadyForSuspend("logout")) return@withContext val context = logoutHelper.switchUser() ?: return@withContext logoutHelper.enqueueLogout(context) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/UninitializedManagers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/UninitializedManagers.kt new file mode 100644 index 000000000..f1e261e93 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/UninitializedManagers.kt @@ -0,0 +1,154 @@ +package com.onesignal.internal + +import com.onesignal.inAppMessages.IInAppMessageClickListener +import com.onesignal.inAppMessages.IInAppMessageLifecycleListener +import com.onesignal.inAppMessages.IInAppMessagesManager +import com.onesignal.location.ILocationManager +import com.onesignal.notifications.INotificationClickListener +import com.onesignal.notifications.INotificationLifecycleListener +import com.onesignal.notifications.INotificationsManager +import com.onesignal.notifications.IPermissionObserver +import com.onesignal.session.ISessionManager +import com.onesignal.user.IUserManager +import com.onesignal.user.state.IUserStateObserver +import com.onesignal.user.subscriptions.IPushSubscription +import com.onesignal.user.subscriptions.IPushSubscriptionObserver + +// Handed out by the accessors when init never started or failed, so calls do nothing instead of +// crashing. The accessor already logged the call, so these stay silent. + +@Suppress("TooManyFunctions") +internal object UninitializedUserManager : IUserManager { + override val pushSubscription: IPushSubscription = UninitializedPushSubscription + override val onesignalId = "" + override val externalId = "" + + override fun setLanguage(value: String) {} + + override fun addAlias( + label: String, + id: String, + ) {} + + override fun addAliases(aliases: Map) {} + + override fun removeAlias(label: String) {} + + override fun removeAliases(labels: Collection) {} + + override fun addEmail(email: String) {} + + override fun removeEmail(email: String) {} + + override fun addSms(sms: String) {} + + override fun removeSms(sms: String) {} + + override fun addTag( + key: String, + value: String, + ) {} + + override fun addTags(tags: Map) {} + + override fun removeTag(key: String) {} + + override fun removeTags(keys: Collection) {} + + override fun getTags(): Map = emptyMap() + + override fun addObserver(observer: IUserStateObserver) {} + + override fun removeObserver(observer: IUserStateObserver) {} + + override fun trackEvent( + name: String, + properties: Map?, + ) {} +} + +internal object UninitializedPushSubscription : IPushSubscription { + override val id = "" + override val token = "" + override val optedIn = false + + override fun optIn() {} + + override fun optOut() {} + + override fun addObserver(observer: IPushSubscriptionObserver) {} + + override fun removeObserver(observer: IPushSubscriptionObserver) {} +} + +@Suppress("TooManyFunctions") +internal object UninitializedNotificationsManager : INotificationsManager { + override val permission = false + override val canRequestPermission = false + + override suspend fun requestPermission(fallbackToSettings: Boolean) = false + + override fun removeNotification(id: Int) {} + + override fun removeGroupedNotifications(group: String) {} + + override fun clearAllNotifications() {} + + override fun addPermissionObserver(observer: IPermissionObserver) {} + + override fun removePermissionObserver(observer: IPermissionObserver) {} + + override fun addForegroundLifecycleListener(listener: INotificationLifecycleListener) {} + + override fun removeForegroundLifecycleListener(listener: INotificationLifecycleListener) {} + + override fun addClickListener(listener: INotificationClickListener) {} + + override fun removeClickListener(listener: INotificationClickListener) {} +} + +internal object UninitializedInAppMessagesManager : IInAppMessagesManager { + override var paused: Boolean + get() = false + set(_) {} + + override fun addTrigger( + key: String, + value: String, + ) {} + + override fun addTriggers(triggers: Map) {} + + override fun removeTrigger(key: String) {} + + override fun removeTriggers(keys: Collection) {} + + override fun clearTriggers() {} + + override fun addLifecycleListener(listener: IInAppMessageLifecycleListener) {} + + override fun removeLifecycleListener(listener: IInAppMessageLifecycleListener) {} + + override fun addClickListener(listener: IInAppMessageClickListener) {} + + override fun removeClickListener(listener: IInAppMessageClickListener) {} +} + +internal object UninitializedLocationManager : ILocationManager { + override var isShared: Boolean + get() = false + set(_) {} + + override suspend fun requestPermission() = false +} + +internal object UninitializedSessionManager : ISessionManager { + override fun addOutcome(name: String) {} + + override fun addUniqueOutcome(name: String) {} + + override fun addOutcomeWithValue( + name: String, + value: Float, + ) {} +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt index 55d7b6ece..02f3c9b2a 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendTests.kt @@ -8,7 +8,6 @@ import com.onesignal.OneSignalUserProfile import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.internal.OneSignalImp -import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import kotlinx.coroutines.runBlocking @@ -263,34 +262,20 @@ class SDKInitSuspendTests : FunSpec({ } } - test("login should throw exception when initWithContext is never called") { - // Given + test("login is ignored when initWithContext is never called") { val oneSignalImp = OneSignalImp() - // When/Then - should throw exception immediately - val exception = - shouldThrow { - oneSignalImp.login("testUser", null) - } + oneSignalImp.login("testUser", null) - // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before 'login'" + oneSignalImp.isInitialized shouldBe false } - test("loginSuspend should throw exception when initWithContext is never called") { - // Given + test("loginSuspend is ignored when initWithContext is never called") { val oneSignalImp = OneSignalImp() - // When/Then - should throw exception immediately - runBlocking { - val exception = - shouldThrow { - oneSignalImp.loginSuspend("testUser", null) - } + runBlocking { oneSignalImp.loginSuspend("testUser", null) } - // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before 'login'" - } + oneSignalImp.isInitialized shouldBe false } test("composite login returns NOT_INITIALIZED when initWithContext is never called") { @@ -305,19 +290,14 @@ class SDKInitSuspendTests : FunSpec({ } } - test("logoutSuspend should throw exception when initWithContext is never called") { - // Given + test("logoutSuspend and updateUserJwtSuspend are ignored when initWithContext is never called") { val oneSignalImp = OneSignalImp() - // When/Then - should throw exception immediately runBlocking { - val exception = - shouldThrow { - oneSignalImp.logoutSuspend() - } - - // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before 'logout'" + oneSignalImp.logoutSuspend() + oneSignalImp.updateUserJwtSuspend("user", "token") } + + oneSignalImp.isInitialized shouldBe false } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt index 7f5c9e855..840e9e0b6 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitTests.kt @@ -11,6 +11,11 @@ import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys.PREFS_LEG import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.internal.OneSignalImp +import com.onesignal.internal.UninitializedInAppMessagesManager +import com.onesignal.internal.UninitializedLocationManager +import com.onesignal.internal.UninitializedNotificationsManager +import com.onesignal.internal.UninitializedSessionManager +import com.onesignal.internal.UninitializedUserManager import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.maps.shouldContain @@ -101,7 +106,7 @@ class SDKInitTests : FunSpec({ .commit() } - test("accessor instances after initWithContext without appID shows the failure reason") { + test("accessors after initWithContext without appID return stand-ins") { // Given val context = getApplicationContext() val os = OneSignalImp() @@ -110,14 +115,8 @@ class SDKInitTests : FunSpec({ os.initWithContext(context) // Then - val ex = shouldThrow { - os.user.onesignalId // Should trigger waitUntilInitInternal → throw failure message - } - // The main exception preserves the caller's stack trace - ex.message shouldBe "OneSignal initWithContext failed." - // The detailed failure reason is in the suppressed exception - ex.suppressed.size shouldBe 1 - ex.suppressed[0].message shouldBe "suspendInitInternal: no appId provided or found in local storage. Please pass a valid appId to initWithContext()." + os.user shouldBe UninitializedUserManager + os.user.onesignalId shouldBe "" // Calling initWithContext with an appID after the failure should not throw anymore val result = os.initWithContext(context, "appID") @@ -126,24 +125,23 @@ class SDKInitTests : FunSpec({ os.isInitialized shouldBe true } - test("OneSignal accessors throw before calling initWithContext") { + test("OneSignal accessors return stand-ins before calling initWithContext") { val os = OneSignalImp() - shouldThrow { - os.user - } - shouldThrow { - os.inAppMessages - } - shouldThrow { - os.session - } - shouldThrow { - os.notifications - } - shouldThrow { - os.location + os.user shouldBe UninitializedUserManager + os.inAppMessages shouldBe UninitializedInAppMessagesManager + os.session shouldBe UninitializedSessionManager + os.notifications shouldBe UninitializedNotificationsManager + os.location shouldBe UninitializedLocationManager + runBlocking { + os.getUser() shouldBe UninitializedUserManager + os.getNotifications().requestPermission(true) shouldBe false } + + os.user.addTag("key", "value") + os.user.pushSubscription.id shouldBe "" + os.inAppMessages.paused = true + os.inAppMessages.paused shouldBe false } test("initWithContext returns gracefully when Android user is locked") { @@ -159,10 +157,8 @@ class SDKInitTests : FunSpec({ // Then // Init runs asynchronously and fails the locked-storage check, transitioning to FAILED. - // Reading an accessor blocks until that terminal state and surfaces the failure. - shouldThrow { - os.user.onesignalId - } + // Reading an accessor blocks until that terminal state and then hands out the stand-in. + os.user shouldBe UninitializedUserManager os.isInitialized shouldBe false unmockkObject(AndroidUtils) @@ -511,13 +507,9 @@ class SDKInitTests : FunSpec({ os.initWithContext(brokenContext, "appId") } - // Then: SDK transitions to FAILED, accessor throws with the cast as suppressed cause. - val ex = - shouldThrow { - os.user.onesignalId - } - ex.message shouldBe "OneSignal initWithContext failed." - ex.suppressed.any { it is ClassCastException } shouldBe true + // Then: SDK transitions to FAILED and the accessor hands out the stand-in. + os.isInitialized shouldBe false + os.user shouldBe UninitializedUserManager // And: a retry with a working context succeeds (FAILED is not "accessible", // so the retry legitimately re-enters init). @@ -677,32 +669,20 @@ class SDKInitTests : FunSpec({ os.isInitialized shouldBe true } - test("login should throw exception when initWithContext is never called") { - // Given + test("login is ignored when initWithContext is never called") { val oneSignalImp = OneSignalImp() - // When/Then - should throw exception immediately - val exception = - shouldThrow { - oneSignalImp.login("testUser", null) - } + oneSignalImp.login("testUser", null) - // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before 'login'" + oneSignalImp.isInitialized shouldBe false } - test("logout should throw exception when initWithContext is never called") { - // Given + test("logout is ignored when initWithContext is never called") { val oneSignalImp = OneSignalImp() - // When/Then - should throw exception immediately - val exception = - shouldThrow { - oneSignalImp.logout() - } + oneSignalImp.logout() - // Should throw immediately because isInitialized is false - exception.message shouldBe "Must call 'initWithContext' before 'logout'" + oneSignalImp.isInitialized shouldBe false } // Regression test for the SessionService NPE during in-flight background init. diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt index 051d89fbc..b645e12cd 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt @@ -1,12 +1,12 @@ package com.onesignal.internal import com.onesignal.ErrorCode +import com.onesignal.IUserJwtInvalidatedListener import com.onesignal.LoginData import com.onesignal.OneSignalResult import com.onesignal.OneSignalUserProfile 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 kotlinx.coroutines.CancellationException @@ -40,29 +40,31 @@ class OneSignalImpTests : FunSpec({ field.set(os, value) } - test("attempting login before initWithContext throws exception") { - // Given + test("login, logout, updateUserJwt and the JWT listeners before initWithContext are ignored") { val os = OneSignalImp() + Logging.logLevel = LogLevel.NONE + val listener = IUserJwtInvalidatedListener { } - // When - val exception = - shouldThrowUnit { - os.login("login-id") - } + os.login("login-id") + os.login("") + os.logout() + os.updateUserJwt("user", "token") + os.addUserJwtInvalidatedListener(listener) + os.removeUserJwtInvalidatedListener(listener) - // Then - exception.message shouldBe "Must call 'initWithContext' before 'login'" + os.isInitialized shouldBe false } - test("login with empty externalId before init still throws") { + test("login and logout after a failed init are ignored") { val os = OneSignalImp() + Logging.logLevel = LogLevel.NONE + setPrivateField(os, "initState", InitState.FAILED) - val exception = - shouldThrowUnit { - os.login("") - } + os.login("login-id") + os.logout() + runBlocking { os.loginSuspend("login-id") } - exception.message shouldBe "Must call 'initWithContext' before 'login'" + os.isInitialized shouldBe false } test("login with empty externalId after init does not switch user") { @@ -91,20 +93,6 @@ class OneSignalImpTests : FunSpec({ os.updateUserJwt("user", "") } - 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'" - } - test("waitForInit returns synchronously without dispatching when initState is SUCCESS") { val os = OneSignalImp() markInitialized(os)