From aa74a5101d186578570d9310f6bb2327a9e48bb1 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 16 Oct 2026 11:58:02 -0500 Subject: [PATCH 1/6] fix: [SDK-5538] never crash on invalid init or calls before init OneSignalImpV2 becomes the never-crash implementation (the default; the kill switch still selects OneSignalImp): - initWithContext accepts a null context or appId. An empty, null or NUL appId falls back to a validated cached appId; with none, init is skipped and logged. - Init state lives in InitGate: waiters follow retries, a cancelled suspend init ends in FAILED, a retry skips services that already bootstrapped, and consent set while init finishes is kept. - Calls before init log and return a default. Accessors hand out the deferred managers, whose queued registrations replay before services start and before SUCCESS. OneSignalImp only takes the nullable signatures; a null context still throws there. Co-authored-by: Cursor --- OneSignalSDK/detekt/detekt-baseline-core.xml | 5 +- .../src/main/java/com/onesignal/IOneSignal.kt | 32 +- .../src/main/java/com/onesignal/OneSignal.kt | 37 +- .../core/internal/startup/StartupService.kt | 10 +- .../java/com/onesignal/internal/InitGate.kt | 132 ++ .../java/com/onesignal/internal/InitState.kt | 8 +- .../com/onesignal/internal/OneSignalImp.kt | 8 +- .../com/onesignal/internal/OneSignalImpV2.kt | 620 ++++------ .../user/internal/AppIdResolutionV2.kt | 37 + .../application/SDKInitSuspendTests.kt | 50 +- .../application/SDKInitSuspendV2Tests.kt | 253 ++++ .../internal/application/SDKInitV2Tests.kt | 1075 +++++++++++++++++ .../internal/startup/StartupServiceTests.kt | 14 + .../com/onesignal/internal/InitGateTests.kt | 92 ++ .../onesignal/internal/OneSignalImpV2Tests.kt | 247 ++++ .../user/internal/AppIdHelperV2Tests.kt | 315 +++++ 16 files changed, 2439 insertions(+), 496 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/AppIdResolutionV2.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/AppIdHelperV2Tests.kt diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index 2feb49e8f0..a46e9d40e5 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -310,6 +310,7 @@ PrintStackTrace:TrackGooglePurchase.kt$TrackGooglePurchase.<no name provided>$t RethrowCaughtException:OSDatabase.kt$OSDatabase$throw e ReturnCount:AppIdResolution.kt$fun resolveAppId( inputAppId: String?, configModel: ConfigModel, preferencesService: IPreferencesService, ): AppIdResolution + ReturnCount:AppIdResolutionV2.kt$internal fun resolveAppIdV2( inputAppId: String?, configModel: ConfigModel, preferencesService: IPreferencesService, ): AppIdResolution ReturnCount:BackgroundManager.kt$BackgroundManager$override fun cancelRunBackgroundServices(): Boolean ReturnCount:ConfigModel.kt$ConfigModel$override fun createModelForProperty( property: String, jsonObject: JSONObject, ): Model? ReturnCount:ExecutorsIvExtensions.kt$internal fun resolveIvBackendParams( op: Operation, onesignalId: String, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): IvBackendParams @@ -327,6 +328,8 @@ ReturnCount:Model.kt$Model$protected fun getOptIntProperty( name: String, create: (() -> Int?)? = null, ): Int? ReturnCount:Model.kt$Model$protected fun getOptLongProperty( name: String, create: (() -> Long?)? = null, ): Long? ReturnCount:Model.kt$Model$protected inline fun <reified T : Enum<T>> getOptEnumProperty(name: String): T? + ReturnCount:OneSignalImp.kt$OneSignalImp$override suspend fun initWithContextSuspend( context: Context?, appId: String?, ): Boolean + ReturnCount:OneSignalImpV2.kt$OneSignalImpV2$override suspend fun initWithContextSuspend( context: Context?, appId: String?, ): Boolean ReturnCount:OperationModelStore.kt$OperationModelStore$override fun create(jsonObject: JSONObject?): Operation? ReturnCount:OperationModelStore.kt$OperationModelStore$private fun isValidOperation(jsonObject: JSONObject): Boolean ReturnCount:OperationRepo.kt$OperationRepo$private fun shouldSuppressAnonymousOp(op: Operation): Boolean @@ -640,7 +643,5 @@ UnusedPrivateMember:OperationRepo.kt$OperationRepo$private val _time: ITime UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'") UseCheckOrError:OneSignalImp.kt$OneSignalImp$throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.") - UseCheckOrError:OneSignalImpV2.kt$OneSignalImpV2$throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'") - UseCheckOrError:OneSignalImpV2.kt$OneSignalImpV2$throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.") diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt index 1fbcd7837e..a7c86f2109 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt @@ -22,6 +22,9 @@ interface IOneSignal { /** * The user manager for accessing user-scoped * management. + * + * Before init succeeds this is a stand-in: listener and setting registrations are replayed once + * init succeeds, other calls are logged and ignored. */ val user: IUserManager @@ -79,13 +82,15 @@ interface IOneSignal { * Initialize the OneSignal SDK. This should be called during startup of the application. * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached + * app ID is used; if none is cached, initialization is skipped and an error is logged. * - * @return true if the SDK could be successfully initialized, false otherwise. + * @return true once initialization has started (or is already running or done), false for a + * null [context] or an immediate startup failure. Check [isInitialized] for the outcome. */ fun initWithContext( - context: Context, - appId: String, + context: Context?, + appId: String?, ): Boolean /** @@ -165,42 +170,43 @@ interface IOneSignal { * Initialize the OneSignal SDK, suspend until initialization is completed * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached + * app ID is used; if none is cached, initialization is skipped and an error is logged. * * @return true if the SDK could be successfully initialized, false otherwise. */ suspend fun initWithContextSuspend( - context: Context, + context: Context?, appId: String? = null, ): Boolean /** * Get the session manager without blocking the calling thread. - * Suspends until the SDK is initialized. + * Suspends while initialization is in progress; if the SDK is not initialized, returns a stand-in (see [user]). */ suspend fun getSession(): ISessionManager /** * Get the notifications manager without blocking the calling thread. - * Suspends until the SDK is initialized. + * Suspends while initialization is in progress; if the SDK is not initialized, returns a stand-in (see [user]). */ suspend fun getNotifications(): INotificationsManager /** * Get the location manager without blocking the calling thread. - * Suspends until the SDK is initialized. + * Suspends while initialization is in progress; if the SDK is not initialized, returns a stand-in (see [user]). */ suspend fun getLocation(): ILocationManager /** * Get the in-app messages manager without blocking the calling thread. - * Suspends until the SDK is initialized. + * Suspends while initialization is in progress; if the SDK is not initialized, returns a stand-in (see [user]). */ suspend fun getInAppMessages(): IInAppMessagesManager /** * Get the user manager without blocking the calling thread. - * Suspends until the SDK is initialized. + * Suspends while initialization is in progress; if the SDK is not initialized, returns a stand-in (see [user]). */ suspend fun getUser(): IUserManager @@ -266,8 +272,8 @@ interface IOneSignal { suspend fun logoutSuspend() /** - * Update the JWT bearer token associated with [externalId] (suspend version). Suspends - * until SDK initialization is complete, then stores the JWT and wakes the operation queue. + * Update the JWT bearer token associated with [externalId] (suspend version). Suspends while + * initialization is in progress, then stores the JWT; if the SDK is not initialized it logs instead. * * @param externalId The external ID the JWT belongs to. * @param token The new JWT bearer token issued by your backend. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt index f834e3cc3f..fb885b8add 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt @@ -37,41 +37,44 @@ object OneSignal { get() = oneSignal.sdkVersion /** - * The user manager for accessing user-scoped management. Initialized only after [initWithContext] - * has been called, and initialized with a device-scoped user until (or if) [login] has been - * called. + * The user manager for accessing user-scoped management, with a device-scoped user until (or + * if) [login] is called. Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are + * replayed once init succeeds, other calls are logged and ignored. */ @JvmStatic val User: IUserManager get() = oneSignal.user /** - * The session manager for accessing session-scoped management. Initialized only after [initWithContext] - * has been called. + * The session manager for accessing session-scoped management. + * Before [initWithContext] succeeds this is a stand-in: outcome calls are logged and ignored. */ @JvmStatic val Session: ISessionManager get() = oneSignal.session /** - * The notification manager for accessing device-scoped notification management. Initialized - * only after [initWithContext] has been called. + * The notification manager for accessing device-scoped notification management. + * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are + * replayed once init succeeds, other calls are logged and ignored. */ @JvmStatic val Notifications: INotificationsManager get() = oneSignal.notifications /** - * The location manager for accessing device-scoped location management. Initialized - * only after [initWithContext] has been called. + * The location manager for accessing device-scoped location management. + * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are + * replayed once init succeeds, other calls are logged and ignored. */ @JvmStatic val Location: ILocationManager get() = oneSignal.location /** - * The In App Messaging manager for accessing device-scoped IAP management. Initialized - * only after [initWithContext] has been called. + * The In App Messaging manager for accessing device-scoped IAP management. + * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are + * replayed once init succeeds, other calls are logged and ignored. */ @JvmStatic val InAppMessages: IInAppMessagesManager @@ -125,12 +128,13 @@ object OneSignal { * Initialize the OneSignal SDK. This should be called during startup of the application. * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached + * app ID is used; if none is cached, initialization is skipped and an error is logged. */ @JvmStatic fun initWithContext( - context: Context, - appId: String, + context: Context?, + appId: String?, ) { oneSignal.initWithContext(context, appId) } @@ -141,12 +145,13 @@ object OneSignal { * Uses Dispatchers.IO internally to prevent ANRs and optimize for I/O operations. * * @param context Application context is recommended for SDK operations - * @param appId The application ID the OneSignal SDK is bound to. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached + * app ID is used; if none is cached, initialization is skipped and an error is logged. * @return Boolean indicating if initialization was successful. */ @JvmStatic suspend fun initWithContextSuspend( - context: Context, + context: Context?, appId: String? = null, ): Boolean { return oneSignal.initWithContextSuspend(context, appId) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt index 250910ca79..c773a9df0b 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt @@ -7,8 +7,16 @@ import com.onesignal.debug.internal.logging.Logging internal class StartupService( private val services: ServiceProvider, ) { + private val bootstrapped = mutableSetOf() + + // Retryable after a failure: services that already bootstrapped are skipped, so none subscribes twice. fun bootstrap() { - services.getAllServices().forEach { it.bootstrap() } + services.getAllServices().forEach { + if (it !in bootstrapped) { + it.bootstrap() + bootstrapped += it + } + } } // schedule to start all startable services using OneSignal dispatcher diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt new file mode 100644 index 0000000000..ae097e9b66 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt @@ -0,0 +1,132 @@ +package com.onesignal.internal + +import com.onesignal.debug.internal.logging.Logging +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.runBlocking + +/** + * Init state for [OneSignalImpV2] and the waits on it. Never throws: callers that need the SDK + * get `false` and a log line when init has not started or has failed. + */ +internal class InitGate { + private val lock = Any() + + // Replaced by every new attempt, so a retry after FAILED never reuses a completed signal. + @Volatile + private var completion = CompletableDeferred() + + @Volatile + var state: InitState = InitState.NOT_STARTED + private set + + // Created by the call that started the attempt, so its stack points at the app's init call. + @Volatile + var failure: Exception? = null + private set + + @Volatile + private var failureTraceLogged = false + + val isSuccess: Boolean get() = state == InitState.SUCCESS + + /** Starts a new attempt and returns true, or returns false if one is running or has succeeded. */ + fun tryStart(): Boolean = + synchronized(lock) { + if (state.isSDKAccessible()) return false + failure = IllegalStateException("OneSignal initWithContext failed.") + failureTraceLogged = false + state = InitState.IN_PROGRESS + completion = CompletableDeferred() + true + } + + fun addFailureCause(cause: Throwable) { + failure?.addSuppressed(cause) + } + + // State and signal change together, so a waiter can't wake up holding the next attempt's signal. + fun complete(terminal: InitState) { + require(terminal == InitState.SUCCESS || terminal == InitState.FAILED) { + "complete requires a terminal state, got $terminal" + } + synchronized(lock) { + state = terminal + completion.complete(Unit) + } + } + + /** Waits out in-progress attempts, including a retry that starts while waiting. True only on SUCCESS. */ + suspend fun await(operation: SdkOperation? = null): Boolean { + while (true) { + val (observed, signal) = synchronized(lock) { state to completion } + when (observed) { + InitState.SUCCESS -> return true + InitState.NOT_STARTED, InitState.FAILED -> return false + InitState.IN_PROGRESS -> awaitAttempt(signal, operation) + } + } + } + + /** Blocking form of [awaitReady]. On the main thread this blocks until init finishes. */ + fun isReady( + operation: SdkOperation, + member: String? = null, + log: Boolean = true, + ): Boolean { + if (isSuccess) return true + // Dispatcher-less: resumes on the caller's own event loop, never on a busy IO worker. + return runBlocking { awaitReady(operation, member, log) } + } + + suspend fun awaitReady( + operation: SdkOperation, + member: String? = null, + log: Boolean = true, + ): Boolean { + if (await(operation)) return true + if (log) logNotReady(target(operation, member)) + return false + } + + fun notReadyMessage(target: String): String = + if (state == InitState.FAILED) { + "'$target' ignored: initWithContext failed." + } else { + "'$target' ignored: call initWithContext with a valid appId first." + } + + fun logQueued( + operation: SdkOperation, + member: String, + ) { + Logging.warn("'${target(operation, member)}' queued until initWithContext succeeds.") + } + + private fun logNotReady(target: String) { + val message = notReadyMessage(target) + val trace = failure.takeIf { state == InitState.FAILED && !failureTraceLogged } + if (trace == null) { + Logging.error(message) + return + } + failureTraceLogged = true + Logging.error(message, trace) + } + + private suspend fun awaitAttempt( + signal: CompletableDeferred, + operation: SdkOperation?, + ) { + Logging.debug("Waiting for init to complete...") + val startTime = System.currentTimeMillis() + signal.await() + val elapsed = System.currentTimeMillis() - startTime + val before = operation?.let { " before '$it'" } ?: "" + Logging.debug("OneSignalImpV2 initialization completed$before (took ${elapsed}ms)") + } +} + +private fun target( + operation: SdkOperation, + member: String?, +) = member?.let { "$operation.$it" } ?: "$operation" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt index 4de391be86..0481d6c4be 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt @@ -9,13 +9,13 @@ package com.onesignal.internal internal enum class InitState { /** * SDK initialization has not yet started. - * Calling SDK-dependent methods in this state will throw an exception. + * SDK-dependent calls log and return a default; they never throw. */ NOT_STARTED, /** * SDK initialization is currently in progress. - * Calls that require initialization will block (via a latch) until this completes. + * Calls that require initialization wait (block or suspend) until this completes. */ IN_PROGRESS, @@ -26,8 +26,8 @@ internal enum class InitState { SUCCESS, /** - * SDK initialization has failed due to an unrecoverable error (e.g., missing app ID). - * All dependent operations should fail fast or throw until re-initialized. + * SDK initialization has failed (e.g., no usable app ID, locked storage). + * SDK-dependent calls log and return a default until a retry of initWithContext succeeds. */ FAILED, 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 7325c27971..6dcd267df1 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 @@ -344,10 +344,11 @@ internal class OneSignalImp : IOneSignal, */ @Suppress("ReturnCount", "TooGenericExceptionCaught") override fun initWithContext( - context: Context, - appId: String, + context: Context?, + appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "Calling deprecated initWithContext(context: $context, appId: $appId)") + requireNotNull(context) { "initWithContext: context is required" } // Warm OneSignalDispatchers on a dedicated daemon thread so the first production caller // of suspendifyOnIO / launchOnSerialIO doesn't pay the ThreadPoolExecutor + dispatcher + @@ -753,10 +754,11 @@ internal class OneSignalImp : IOneSignal, } override suspend fun initWithContextSuspend( - context: Context, + context: Context?, appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context, appId: $appId)") + requireNotNull(context) { "initWithContext: context is required" } // Same warm-up as the synchronous variant. Reaching this entry point on the main thread // (e.g. SyncJobService.onStartJob -> suspendifyOnIO -> initWithContext(context)) pays the diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt index 182bbdff5a..af69ce9f87 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt @@ -48,36 +48,24 @@ import com.onesignal.user.internal.LoginHelper import com.onesignal.user.internal.LogoutHelper import com.onesignal.user.internal.UserSwitcher import com.onesignal.user.internal.identity.IdentityModelStore +import com.onesignal.user.internal.isUsableAppId import com.onesignal.user.internal.jwt.JwtTokenStore import com.onesignal.user.internal.properties.PropertiesModelStore -import com.onesignal.user.internal.resolveAppId +import com.onesignal.user.internal.resolveAppIdV2 import com.onesignal.user.internal.subscriptions.SubscriptionModelStore -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext internal class OneSignalImpV2 : IOneSignal, IServiceProvider { - // Reset every time the synchronized(initLock) block flips state to IN_PROGRESS so that - // a retry-after-FAILED gets a fresh latch instead of an already-completed one. Mutated only - // under initLock; reads outside that lock must local-capture before suspending on it. - @Volatile - private var suspendCompletion = CompletableDeferred() - - @Volatile - private var initState: InitState = InitState.NOT_STARTED - - // Save the exception pointing to the caller that triggered init, not the async worker thread. - private var initFailureException: Exception? = null + internal val gate = InitGate() private var observabilityManager: IObservabilityLifecycleManager? = null override val sdkVersion: String = OneSignalUtils.sdkVersion override val isInitialized: Boolean - get() = initState == InitState.SUCCESS + get() = gate.isSuccess override var consentRequired: Boolean get() = @@ -87,9 +75,9 @@ internal class OneSignalImpV2 : IOneSignal, _consentRequired == true } set(value) { - _consentRequired = value - if (isInitialized) { - configModel.consentRequired = value + synchronized(configLock) { + _consentRequired = value + if (configApplied) configModel.consentRequired = value } } @@ -101,13 +89,15 @@ internal class OneSignalImpV2 : IOneSignal, _consentGiven == true } set(value) { - val oldValue = _consentGiven - _consentGiven = value - if (isInitialized) { - configModel.consentGiven = value - if (oldValue != value && value) { - operationRepo.forceExecuteOperations() + val grantedNow = + synchronized(configLock) { + val oldValue = _consentGiven + _consentGiven = value + if (configApplied) configModel.consentGiven = value + configApplied && oldValue != value && value } + if (grantedNow) { + operationRepo.forceExecuteOperations() } } @@ -119,34 +109,65 @@ internal class OneSignalImpV2 : IOneSignal, _disableGMSMissingPrompt == true } set(value) { - _disableGMSMissingPrompt = value - if (isInitialized) { - configModel.disableGMSMissingPrompt = value + synchronized(configLock) { + _disableGMSMissingPrompt = value + if (configApplied) configModel.disableGMSMissingPrompt = value } } // we hardcode the DebugManager implementation so it can be used prior to calling `initWithContext` override val debug: IDebugManager = DebugManager() + private val sessionSource = GatedSource(SdkOperation.SESSION, ISessionManager::class.java) + private val notificationsSource = GatedSource(SdkOperation.NOTIFICATIONS, INotificationsManager::class.java) + private val locationSource = GatedSource(SdkOperation.LOCATION, ILocationManager::class.java) + private val inAppMessagesSource = GatedSource(SdkOperation.IN_APP_MESSAGES, IInAppMessagesManager::class.java) + private val userSource = GatedSource(SdkOperation.USER, IUserManager::class.java) + + private val deferredSession = DeferredSessionManager(sessionSource) + private val deferredNotifications = DeferredNotificationsManager(notificationsSource) + private val deferredLocation = DeferredLocationManager(locationSource) + private val deferredInAppMessages = DeferredInAppMessagesManager(inAppMessagesSource) + private val deferredUser = DeferredUserManager(userSource) + private val deferredJwtListeners = DeferredJwtListeners(GatedSource(SdkOperation.JWT_LISTENERS, JwtTokenStore::class.java)) + override val session: ISessionManager - get() = - waitAndReturn { services.getService() } + get() = sessionSource.peek() ?: deferredSession override val notifications: INotificationsManager - get() = - waitAndReturn { services.getService() } + get() = notificationsSource.peek() ?: deferredNotifications override val location: ILocationManager - get() = - waitAndReturn { services.getService() } + get() = locationSource.peek() ?: deferredLocation override val inAppMessages: IInAppMessagesManager - get() = - waitAndReturn { services.getService() } + get() = inAppMessagesSource.peek() ?: deferredInAppMessages override val user: IUserManager - get() = - waitAndReturn { services.getService() } + get() = userSource.peek() ?: deferredUser + + private inner class GatedSource( + private val operation: SdkOperation, + private val type: Class, + ) : ManagerSource { + override fun get(member: String): T? = if (gate.isReady(operation, member)) services.getService(type) else null + + override fun peek(): T? = if (gate.isReady(operation, log = false)) services.getService(type) else null + + override suspend fun await(member: String): T? = if (gate.awaitReady(operation, member)) services.getService(type) else null + + override fun logQueued(member: String) = gate.logQueued(operation, member) + + suspend fun awaitQuietly(): T? = if (gate.awaitReady(operation, log = false)) services.getService(type) else null + } + + private fun replayDeferredCalls() { + deferredUser.replay(services.getService()) + deferredNotifications.replay(services.getService()) + deferredInAppMessages.replay(services.getService()) + deferredLocation.replay(services.getService()) + deferredJwtListeners.replay(services.getService()) + } // Services required by this class // WARNING: OperationRepo depends on OperationModelStore which in-turn depends @@ -185,14 +206,8 @@ internal class OneSignalImpV2 : IOneSignal, } }.build() - // Off-main work routes through OneSignal's centralized dispatcher pool rather than ad-hoc - // threads / Dispatchers.IO. - // - // Resolved per-access (get()) rather than captured once: an eager `val` would construct the - // lazy `pools.IO` dispatcher during OneSignalImp construction on the main thread (before - // prewarm() runs), and would pin this instance to one pool generation -- so after - // resetForTest() swaps `pools` and shutdownNow()'s the old executor, a reused instance would - // dispatch onto a dead pool. The getter always reads the current generation's warm dispatcher. + // Read per access: an eager val would build the IO pool on main before prewarm() and keep + // pointing at a dead pool after resetForTest() swaps generations. private val ioDispatcher: CoroutineDispatcher get() = OneSignalDispatchers.IO // get the current config model, if there is one @@ -200,7 +215,14 @@ internal class OneSignalImpV2 : IOneSignal, private var _consentRequired: Boolean? = null private var _consentGiven: Boolean? = null private var _disableGMSMissingPrompt: Boolean? = null - private val initLock: Any = Any() + + // Guards the pre-init fields above and [configApplied], so a setter racing updateConfig is never lost. + private val configLock: Any = Any() + private var configApplied = false + + // Set once services are bootstrapped; a retry after FAILED must not subscribe listeners twice. + private var startupService: StartupService? = null + private val bootstrapper = StartupService(services) private val loginLogoutLock: Any = Any() private val applicationServiceLock: Any = Any() @@ -252,9 +274,11 @@ internal class OneSignalImpV2 : IOneSignal, // lazy supplier — `enabledFeatureFlags` is read per-event, so resolving the manager // can be deferred until services have bootstrapped. val featureManagerProvider = { services.getService() } - observabilityManager = - LoggerLifecycleManager(context = context, featureManagerProvider = featureManagerProvider) - .also { it.initializeFromCachedConfig() } + if (observabilityManager == null) { + observabilityManager = + LoggerLifecycleManager(context = context, featureManagerProvider = featureManagerProvider) + .also { it.initializeFromCachedConfig() } + } logStartupDiagnostics(context) PreferenceStoreFix.ensureNoObfuscatedPrefStore(context) @@ -309,85 +333,79 @@ internal class OneSignalImpV2 : IOneSignal, } } + // Copies values set before init into the model; setters write through from here on. private fun updateConfig() { - // if requires privacy consent was set prior to init, set it in the model now - if (_consentRequired != null) { - configModel.consentRequired = _consentRequired!! - } - - // if privacy consent was set prior to init, set it in the model now - if (_consentGiven != null) { - configModel.consentGiven = _consentGiven!! - } - - if (_disableGMSMissingPrompt != null) { - configModel.disableGMSMissingPrompt = _disableGMSMissingPrompt!! + synchronized(configLock) { + _consentRequired?.let { configModel.consentRequired = it } + _consentGiven?.let { configModel.consentGiven = it } + _disableGMSMissingPrompt?.let { configModel.disableGMSMissingPrompt = it } + configApplied = true } } - private fun bootstrapServices(): StartupService { - val startupService = StartupService(services) - // bootstrap all services - startupService.bootstrap() - return startupService + private fun bootstrapOnce(context: Context): StartupService { + startupService?.let { return it } + + initEssentials(context) + val started = bootstrapper.also { it.bootstrap() } + startupService = started + + // Now that the IoC container is ready, subscribe the observability lifecycle + // manager to config store events so it reacts to fresh remote config. + observabilityManager?.subscribeToConfigStore(services.getService()) + // The event recorder is a container service, so it can only be handed over now. The resolve + // cannot fail in practice (bootstrap already built the feature manager it needs), and the + // hand-over is fail-open inside the manager. + val eventRecorder = services.getServiceOrNull() + if (eventRecorder != null) { + observabilityManager?.attachEventRecorder(eventRecorder) + } else { + Logging.warn("OneSignal: event recorder unavailable, observability events will not ship") + } + return started } /** - * Fire-and-forget init. The actual work runs asynchronously on the IO pool, so this method - * cannot know the real outcome before it returns. - * - * Return value: always `true` once init has been kicked off (or is already in progress/done). - * It does NOT reflect init success — a failure surfaces later, either via a subsequent - * accessor throwing or by querying [isInitialized]. Callers (including wrapper SDKs) must not - * branch on this boolean to detect init failure; use the suspending [initWithContextSuspend], - * which returns an accurate result, when the outcome matters. + * Fire-and-forget: true once init is dispatched, running or done; false only for a null [context] + * or a synchronous start failure. Use [initWithContextSuspend] or [isInitialized] for the outcome. */ @Suppress("ReturnCount", "TooGenericExceptionCaught") override fun initWithContext( - context: Context, - appId: String, + context: Context?, + appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "Calling deprecated initWithContext(context: $context, appId: $appId)") + if (context == null) { + Logging.error(MISSING_CONTEXT_MESSAGE) + return false + } + // Warm OneSignalDispatchers on a dedicated daemon thread so the first production caller // of suspendifyOnIO / launchOnSerialIO doesn't pay the ThreadPoolExecutor + dispatcher + // scope construction cost on the main thread (observed as 5-20s main-thread blocks at the // first foreground/background lifecycle event). prewarm() is idempotent and fire-and-forget. OneSignalDispatchers.prewarm() - synchronized(initLock) { - if (initState.isSDKAccessible()) { - Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized or in progress") - return true - } - - // Publish state and its associated metadata atomically. - initFailureException = IllegalStateException("OneSignal initWithContext failed.") - initState = InitState.IN_PROGRESS - // Fresh latch for this init attempt -- prevents retry-after-FAILED waiters from - // observing the prior init's already-completed deferred. - suspendCompletion = CompletableDeferred() + if (!gate.tryStart()) { + Logging.log(LogLevel.DEBUG, "initWithContext: SDK already initialized or in progress") + return true } - // Tight try/catch around the only step that can throw synchronously *before* the - // dispatch (or runBlocking). ApplicationService.start does - // `context.applicationContext as Application`, which can ClassCastException in - // restricted hosts (Robolectric custom application factories, instrumentation context - // wrappers, multi-process content provider init order). If we don't transition to - // FAILED here, IN_PROGRESS would never be retired — every subsequent - // [initWithContext] would short-circuit (IN_PROGRESS counts as "accessible") and every - // accessor would deadlock on suspendCompletion.await(). Failures *inside* - // [internalInit] are owned by its own try/catch. + // The only step that can throw before dispatch (the Application cast can fail in restricted + // hosts). It must end in FAILED, or IN_PROGRESS is never retired and every accessor blocks + // forever. Failures inside [internalInit] are caught there. try { ensureApplicationServiceStarted(context) } catch (e: Exception) { - initFailureException?.addSuppressed(e) - completeInit(InitState.FAILED) - throw e + gate.addFailureCause(e) + gate.complete(InitState.FAILED) + Logging.error("initWithContext failed to start the application service", e) + return false } // Dispatch init asynchronously so this method never blocks the caller. Callers that - // need to wait (accessors, login, logout) will block via suspendCompletion. + // need to wait (accessors, login, logout) block on the gate. suspendifyOnIO { internalInit(context, appId) } @@ -402,11 +420,6 @@ internal class OneSignalImpV2 : IOneSignal, return initWithContextSuspend(context, null) } - // ReturnCount: 4 returns vs detekt's limit of 2. The function intentionally has - // multiple early-return failure paths (user-locked, missing appId, exception). The - // existing detekt baseline entry was for the unannotated signature; once we added - // @Suppress(TooGenericExceptionCaught) the baseline ID no longer matched, so we - // suppress ReturnCount inline rather than churning the baseline. @Suppress("ReturnCount", "TooGenericExceptionCaught") private fun internalInit( context: Context, @@ -418,60 +431,41 @@ internal class OneSignalImpV2 : IOneSignal, // to device storage like SharedPreferences. if (!AndroidUtils.isAndroidUserUnlocked(context)) { Logging.warn("initWithContext called when device storage is locked, no user data is accessible!") - completeInit(InitState.FAILED) + gate.complete(InitState.FAILED) return false } - initEssentials(context) - - val startupService = bootstrapServices() - - // Now that the IoC container is ready, subscribe the observability lifecycle - // manager to config store events so it reacts to fresh remote config. - observabilityManager?.subscribeToConfigStore(services.getService()) - // The event recorder is a container service, so it can only be handed over now. The resolve - // cannot fail in practice (bootstrap already built the feature manager it needs), and the - // hand-over is fail-open inside the manager. - val eventRecorder = services.getServiceOrNull() - if (eventRecorder != null) { - observabilityManager?.attachEventRecorder(eventRecorder) - } else { - Logging.warn("OneSignal: event recorder unavailable, observability events will not ship") - } + val startupService = bootstrapOnce(context) - val result = resolveAppId(appId, configModel, preferencesService) + val result = resolveAppIdV2(appId, configModel, preferencesService) if (result.failed) { - val message = "suspendInitInternal: no appId provided or found in local storage. Please pass a valid appId to initWithContext()." - val exception = IllegalStateException(message) - // attach the real crash cause to the init failure exception that will be throw shortly after - initFailureException?.addSuppressed(exception) - Logging.warn(message) - completeInit(InitState.FAILED) + val message = "initWithContext skipped: no valid appId was passed and none is cached. Pass your OneSignal app ID to initWithContext()." + gate.addFailureCause(IllegalStateException(message)) + // Null is also how background entry points (e.g. SyncJobService) start init, so it is not an app error. + if (appId == null) Logging.warn(message) else Logging.error(message) + gate.complete(InitState.FAILED) return false } + if (appId != null && !isUsableAppId(appId)) { + Logging.warn("initWithContext: appId is empty or contains a null byte, using the cached app ID instead.") + } configModel.appId = result.appId!! // safe because failed is false val forceCreateUser = result.forceCreateUser updateConfig() userSwitcher.initUser(forceCreateUser) + // Before start, so services never act on a setting the app already changed; before SUCCESS, + // so no direct call can overtake a queued one. Replayed calls must not go through the gate. + replayDeferredCalls() startupService.scheduleStart() - completeInit(InitState.SUCCESS) + gate.complete(InitState.SUCCESS) return true } catch (e: Exception) { - // Any unchecked throw from initEssentials / bootstrapServices / subscribeToConfigStore / - // attachEventRecorder / updateConfig / userSwitcher.initUser / startupService.scheduleStart would otherwise - // leave initState at IN_PROGRESS forever and `suspendCompletion` uncompleted — - // accessors and re-entrant suspend callers (e.g. SyncJobService) would deadlock on - // `await()`. Reach a terminal state via [completeInit] (atomic state+completion) and - // return `false`. The throw is captured on `initFailureException.addSuppressed(...)` - // so a subsequent accessor surfaces it to the caller. We deliberately don't rethrow - // here: the synchronous [initWithContext] dispatches this via `suspendifyOnIO { ... }` - // (which has no caller to rethrow to), so returning `false` on every path — including - // [initWithContextSuspend] — gives one consistent shape. Callers can query - // `initFailureException` (or just the returned `Boolean`) to react. + // Never rethrow: the sync initWithContext runs this via suspendifyOnIO, with no caller to + // catch it. Reaching FAILED unblocks every waiter; the cause is attached for the next log. Logging.error("OneSignal: internalInit threw unexpectedly; marking init FAILED", e) - initFailureException?.addSuppressed(e) - completeInit(InitState.FAILED) + gate.addFailureCause(e) + if (!gate.isSuccess) gate.complete(InitState.FAILED) return false } } @@ -482,9 +476,7 @@ internal class OneSignalImpV2 : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - waitForInit(operationName = "login") - - if (isMissing(externalId, "login: externalId")) return + if (!gate.isReady(SdkOperation.LOGIN) || isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM)) return val context = loginHelper.switchUser(externalId, jwtBearerToken).context ?: return @@ -494,7 +486,7 @@ internal class OneSignalImpV2 : IOneSignal, override fun logout() { Logging.log(LogLevel.DEBUG, "Calling deprecated logout()") - waitForInit(operationName = "logout") + if (!gate.isReady(SdkOperation.LOGOUT)) return val context = logoutHelper.switchUser() ?: return @@ -507,24 +499,18 @@ internal class OneSignalImpV2 : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})") - waitForInit(operationName = "updateUserJwt") + if (!gate.isReady(SdkOperation.UPDATE_USER_JWT)) return - if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return + if (isMissing(externalId, SdkOperation.UPDATE_USER_JWT, EXTERNAL_ID_PARAM) || isMissing(token, SdkOperation.UPDATE_USER_JWT, TOKEN_PARAM)) return jwtTokenStore.putJwt(externalId, token) // Wake the queue so any deferred ops can dispatch with the fresh token. operationRepo.forceExecuteOperations() } - override fun addUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) { - waitForInit(operationName = "addUserJwtInvalidatedListener") - jwtTokenStore.addUserJwtInvalidatedListener(listener) - } + override fun addUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) = deferredJwtListeners.add(listener) - override fun removeUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) { - waitForInit(operationName = "removeUserJwtInvalidatedListener") - jwtTokenStore.removeUserJwtInvalidatedListener(listener) - } + override fun removeUserJwtInvalidatedListener(listener: IUserJwtInvalidatedListener) = deferredJwtListeners.remove(listener) override fun hasService(c: Class): Boolean = services.hasService(c) @@ -534,138 +520,7 @@ internal class OneSignalImpV2 : IOneSignal, override fun getAllServices(c: Class): List = services.getAllServices(c) - /** - * Blocking version that waits for initialization to complete. - * Uses runBlocking to bridge to the suspend implementation. - * Waits indefinitely until init completes and logs how long it took. - * - * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") - */ - private fun waitForInit(operationName: String? = null) { - if (initState == InitState.SUCCESS) { - return - } - // Intentionally dispatcher-less: wait on the calling thread's own event loop so resuming - // when init completes never depends on a free worker in a busy pool. - runBlocking { - waitUntilInitInternal(operationName) - } - } - - /** - * Atomically transitions [initState] to a terminal state (SUCCESS or FAILED) - * and completes [suspendCompletion] so any waiters unblock. - * - * Both the state write and the completion run under [initLock]. This closes a - * race where another caller could observe the terminal state in between the two - * writes, call `initWithContext` to flip back to IN_PROGRESS, and replace - * [suspendCompletion] with a fresh deferred — at which point this thread's - * completion would prematurely unblock waiters of the *second* init. - */ - private fun completeInit(terminalState: InitState) { - require(terminalState == InitState.SUCCESS || terminalState == InitState.FAILED) { - "completeInit requires a terminal state, got $terminalState" - } - synchronized(initLock) { - initState = terminalState - suspendCompletion.complete(Unit) - } - } - - /** - * Suspend version that waits for initialization to complete. - * Waits indefinitely until init completes and logs how long it took. - * - * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") - */ - private suspend fun suspendUntilInit(operationName: String? = null) { - waitUntilInitInternal(operationName) - } - - /** - * Common implementation for waiting until initialization completes. - * Waits indefinitely until init completes (SUCCESS or FAILED) to ensure consistent state. - * Logs how long initialization took when it completes. - * - * @param operationName Optional operation name to include in error messages (e.g., "login", "logout") - */ - private suspend fun waitUntilInitInternal(operationName: String? = null) { - // Local-capture state + deferred under initLock so we await on the same generation - // we observed (a concurrent retry-after-FAILED can replace `suspendCompletion`). - val observedState: InitState - val completionToAwait: CompletableDeferred - synchronized(initLock) { - observedState = initState - completionToAwait = suspendCompletion - } - - when (observedState) { - InitState.NOT_STARTED -> throw IllegalStateException(notInitializedMessage(operationName)) - - InitState.IN_PROGRESS -> awaitInitCompletion(completionToAwait, operationName) - - InitState.FAILED -> { - throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.") - } - - else -> { - // SUCCESS - already initialized, no need to wait - } - } - } - - private fun notInitializedMessage(operationName: String?): String = - if (operationName != null) { - "Must call 'initWithContext' before '$operationName'" - } else { - "Must call 'initWithContext' before use" - } - - private suspend fun awaitInitCompletion( - completionToAwait: CompletableDeferred, - operationName: String?, - ) { - Logging.debug("Waiting for init to complete...") - - val startTime = System.currentTimeMillis() - - // Wait indefinitely until init actually completes - ensures consistent state - // Function only returns when initState is SUCCESS or FAILED - // Non-blocking when called from coroutines. But if waitForInit() (which uses runBlocking) - // is called from the main thread, it blocks the main thread until init completes, which can - // ANR. This is intentional: an ANR is recoverable, an uncaught throw is not. To avoid it, - // call SDK methods from background threads or use the suspend API. - completionToAwait.await() - - // Log how long initialization took - val elapsed = System.currentTimeMillis() - startTime - val message = - if (operationName != null) { - "OneSignalImp initialization completed before '$operationName' (took ${elapsed}ms)" - } else { - "OneSignalImp initialization completed (took ${elapsed}ms)" - } - Logging.debug(message) - - // Re-check state after waiting - init might have failed during the wait - if (initState == InitState.FAILED) { - throw initFailureException ?: IllegalStateException("Initialization failed. Cannot proceed.") - } - // initState is guaranteed to be SUCCESS here - consistent state - } - - private suspend fun suspendAndReturn(getter: () -> T): T { - suspendUntilInit() - 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() - } - + // Callers only reach this after checking [isInitialized], and SUCCESS is terminal. private fun blockingGet(getter: () -> T): T { try { if (AndroidUtils.isRunningOnMainThread()) { @@ -676,136 +531,93 @@ internal class OneSignalImpV2 : IOneSignal, // because Looper.getMainLooper() is not mocked. This is safe to ignore. Logging.debug("Could not check main thread status (likely in test environment): ${e.message}") } - if (initState == InitState.SUCCESS) { - return getter() - } - - // Intentionally dispatcher-less (see waitForInit). - return runBlocking { - suspendAndReturn(getter) - } + return getter() } // =============================== // Suspend API Implementation // =============================== - override suspend fun getSession(): ISessionManager = - withContext(ioDispatcher) { - suspendAndReturn { services.getService() } - } + override suspend fun getSession(): ISessionManager = awaitManager(sessionSource) ?: deferredSession - override suspend fun getNotifications(): INotificationsManager = - withContext(ioDispatcher) { - suspendAndReturn { services.getService() } - } + override suspend fun getNotifications(): INotificationsManager = awaitManager(notificationsSource) ?: deferredNotifications - override suspend fun getLocation(): ILocationManager = - withContext(ioDispatcher) { - suspendAndReturn { services.getService() } - } + override suspend fun getLocation(): ILocationManager = awaitManager(locationSource) ?: deferredLocation - override suspend fun getInAppMessages(): IInAppMessagesManager = - withContext(ioDispatcher) { - suspendAndReturn { services.getService() } - } + override suspend fun getInAppMessages(): IInAppMessagesManager = awaitManager(inAppMessagesSource) ?: deferredInAppMessages - override suspend fun getUser(): IUserManager = - withContext(ioDispatcher) { - suspendAndReturn { services.getService() } - } + override suspend fun getUser(): IUserManager = awaitManager(userSource) ?: deferredUser - override suspend fun getConsentRequired(): Boolean = - withContext(ioDispatcher) { - configModel.consentRequired ?: (_consentRequired == true) - } + // Suspends, never blocks, through an in-progress init; the returned deferred manager logs on use. + private suspend fun awaitManager(source: GatedSource): T? = withContext(ioDispatcher) { source.awaitQuietly() } - override suspend fun setConsentRequired(required: Boolean) = - withContext(ioDispatcher) { - _consentRequired = required - configModel.consentRequired = required - } + // The properties hold the value locally until init applies it, so these are safe before init. + override suspend fun getConsentRequired(): Boolean = afterInit(SdkOperation.GET_CONSENT_REQUIRED) { consentRequired } - override suspend fun getConsentGiven(): Boolean = - withContext(ioDispatcher) { - configModel.consentGiven ?: (_consentGiven == true) - } + override suspend fun setConsentRequired(required: Boolean) = afterInit(SdkOperation.SET_CONSENT_REQUIRED) { consentRequired = required } - override suspend fun setConsentGiven(value: Boolean) = - withContext(ioDispatcher) { - val oldValue = _consentGiven - _consentGiven = value - configModel.consentGiven = value - if (oldValue != value && value) { - operationRepo.forceExecuteOperations() - } - } + override suspend fun getConsentGiven(): Boolean = afterInit(SdkOperation.GET_CONSENT_GIVEN) { consentGiven } - override suspend fun getDisableGMSMissingPrompt(): Boolean = - withContext(ioDispatcher) { - configModel.disableGMSMissingPrompt - } + override suspend fun setConsentGiven(value: Boolean) = afterInit(SdkOperation.SET_CONSENT_GIVEN) { consentGiven = value } - override suspend fun setDisableGMSMissingPrompt(value: Boolean) = + override suspend fun getDisableGMSMissingPrompt(): Boolean = afterInit(SdkOperation.GET_DISABLE_GMS_MISSING_PROMPT) { disableGMSMissingPrompt } + + override suspend fun setDisableGMSMissingPrompt(value: Boolean) = afterInit(SdkOperation.SET_DISABLE_GMS_MISSING_PROMPT) { disableGMSMissingPrompt = value } + + // Waits out an in-progress init so a value set now is not lost between updateConfig and SUCCESS. + private suspend fun afterInit( + operation: SdkOperation, + block: () -> T, + ): T = withContext(ioDispatcher) { - _disableGMSMissingPrompt = value - configModel.disableGMSMissingPrompt = value + gate.await(operation) + block() } override suspend fun initWithContextSuspend( - context: Context, + context: Context?, appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context, appId: $appId)") + if (context == null) { + Logging.error(MISSING_CONTEXT_MESSAGE) + return false + } + // Same warm-up as the synchronous variant. Reaching this entry point on the main thread // (e.g. SyncJobService.onStartJob -> suspendifyOnIO -> initWithContext(context)) pays the // cold-init cost on the dispatcher used to enter [withContext] below, so warm // OneSignalDispatchers on a background thread before we touch [ioDispatcher]. OneSignalDispatchers.prewarm() - // Observing a terminal SUCCESS must never require an IO worker (see waitForInit). - if (initState == InitState.SUCCESS) { + // Observing a terminal SUCCESS must never require an IO worker (see InitGate.isReady). + if (gate.isSuccess) { return true } - // Local-capture under the lock so that even if a concurrent retry-after-FAILED - // resets `suspendCompletion`, we await on the same generation we observed. - // Null means this caller is the one that starts init. - val completionToAwait: CompletableDeferred? = - synchronized(initLock) { - if (initState.isSDKAccessible()) { - suspendCompletion - } else { - initState = InitState.IN_PROGRESS - // Fresh latch for this init attempt. - suspendCompletion = CompletableDeferred() - // Only the call that actually starts init owns the failure-attribution exception. - // Re-entrant callers must not overwrite it -- otherwise the failure stack trace - // would point at the SyncJobService coroutine instead of the original initiator. - initFailureException = IllegalStateException("OneSignal initWithContext failed.") - null - } - } - - if (completionToAwait != null) { - // Another caller has already started (or completed) init. Honor this method's - // contract by suspending until initialization is *fully* completed -- not just - // kicked off. This closes a race where re-entrant suspend callers (e.g. the - // SyncJobService entry point) would otherwise - // proceed to use IBackgroundService implementations like SessionService whose - // bootstrap() had not yet run, NPE'ing on still-null model fields. + if (!gate.tryStart()) { + // Another caller started init. Suspend until it is fully done, not just kicked off, so + // re-entrant callers (e.g. SyncJobService) never use services whose bootstrap has not run. + // Awaited on the caller's own context so the wait never queues behind init's own work. Logging.log(LogLevel.DEBUG, "initWithContext: init already in progress or completed, awaiting completion") - // Awaited on the caller's own context: waiting for, and resuming from, another - // caller's init must not queue behind the work that init itself is doing. - completionToAwait.await() - return initState == InitState.SUCCESS + return gate.await(SdkOperation.INIT_WITH_CONTEXT) } // Only the caller that actually runs init needs the IO dispatcher, to keep its disk // and network work off the calling thread. - return withContext(ioDispatcher) { - internalInit(context, appId) + var started = false + try { + return withContext(ioDispatcher) { + started = true + internalInit(context, appId) + } + } finally { + // A caller cancelled before dispatch would otherwise leave this attempt IN_PROGRESS forever. + if (!started) { + gate.addFailureCause(IllegalStateException("initWithContext was cancelled before init ran")) + gate.complete(InitState.FAILED) + } } } @@ -815,11 +627,9 @@ internal class OneSignalImpV2 : 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 (!gate.awaitReady(SdkOperation.LOGIN)) return@withContext - if (isMissing(externalId, "login: externalId")) return@withContext + if (isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM)) return@withContext val context = loginHelper.switchUser(externalId, jwtBearerToken).context ?: return@withContext loginHelper.enqueueLogin(context) @@ -838,9 +648,11 @@ internal class OneSignalImpV2 : IOneSignal, return@withContext OneSignalResult.failure(ErrorCode.INVALID_ARGUMENT, it) } - awaitInitForLogin()?.let { return@withContext it } + if (!gate.awaitReady(SdkOperation.LOGIN)) { + return@withContext OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, gate.notReadyMessage("login")) + } - if (isMissing(externalId, "login: externalId") || externalId == null) { + if (isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM) || externalId == null) { return@withContext OneSignalResult.failure( ErrorCode.INVALID_ARGUMENT, "login: externalId is required", @@ -870,20 +682,6 @@ internal class OneSignalImpV2 : IOneSignal, OneSignalResult.success(loginHelper.loginData(externalId, profile, fallbackOnesignalId = switched.onesignalId)) } - /** 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) - } - private fun loginFailureMessage(wait: OperationWaitResult): String = wait.httpStatusCode?.let { "Login did not complete (HTTP $it)." } ?: "Login did not complete." @@ -893,13 +691,9 @@ internal class OneSignalImpV2 : IOneSignal, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})") - suspendUntilInit(operationName = "updateUserJwt") + if (!gate.awaitReady(SdkOperation.UPDATE_USER_JWT)) return@withContext - if (!isInitialized) { - throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'") - } - - if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) { + if (isMissing(externalId, SdkOperation.UPDATE_USER_JWT, EXTERNAL_ID_PARAM) || isMissing(token, SdkOperation.UPDATE_USER_JWT, TOKEN_PARAM)) { return@withContext } @@ -911,9 +705,15 @@ internal class OneSignalImpV2 : IOneSignal, withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "logoutSuspend()") - suspendUntilInit(operationName = "logout") + if (!gate.awaitReady(SdkOperation.LOGOUT)) return@withContext val context = logoutHelper.switchUser() ?: return@withContext logoutHelper.enqueueLogout(context) } + + private companion object { + const val MISSING_CONTEXT_MESSAGE = "initWithContext skipped: context is required." + const val EXTERNAL_ID_PARAM = "externalId" + const val TOKEN_PARAM = "token" + } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/AppIdResolutionV2.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/AppIdResolutionV2.kt new file mode 100644 index 0000000000..3ac339d1aa --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/AppIdResolutionV2.kt @@ -0,0 +1,37 @@ +package com.onesignal.user.internal + +import com.onesignal.common.hasNullByte +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.preferences.IPreferencesService +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys +import com.onesignal.core.internal.preferences.PreferenceStores + +internal fun resolveAppIdV2( + inputAppId: String?, + configModel: ConfigModel, + preferencesService: IPreferencesService, +): AppIdResolution { + val validInputAppId = inputAppId?.takeIf { isUsableAppId(it) } + + // Case 1: AppId provided as input + if (validInputAppId != null) { + val forceCreateUser = !configModel.hasProperty(ConfigModel::appId.name) || configModel.appId != validInputAppId + return AppIdResolution(appId = validInputAppId, forceCreateUser = forceCreateUser, failed = false) + } + + // Case 2: No appId provided, but configModel has one. + // Cached values can be invalid on upgraded installs, so they are validated too. + if (configModel.hasProperty(ConfigModel::appId.name) && isUsableAppId(configModel.appId)) { + return AppIdResolution(appId = configModel.appId, forceCreateUser = false, failed = false) + } + + // Case 3: No appId provided, no configModel appId - try legacy + val legacyAppId = preferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + if (isUsableAppId(legacyAppId)) { + return AppIdResolution(appId = legacyAppId, forceCreateUser = true, failed = false) + } + + return AppIdResolution(appId = null, forceCreateUser = false, failed = true) +} + +internal fun isUsableAppId(appId: String?): Boolean = !appId.isNullOrEmpty() && !hasNullByte(appId) 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 55d7b6eceb..ae3b424c26 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 @@ -54,56 +54,12 @@ class SDKInitSuspendTests : FunSpec({ } test("initWithContextSuspend with null appId fails when configModel has no appId") { - // Given - val context = getApplicationContext() - - // COMPLETE STATE RESET: Clear ALL SharedPreferences and wait for completion - val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) - prefs.edit().clear().commit() - - // Clear any other potential SharedPreferences files - val otherPrefs = context.getSharedPreferences("com.onesignal", Context.MODE_PRIVATE) - otherPrefs.edit().clear().commit() - - // Clear any other potential preference stores that might exist - try { - val allPrefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) - allPrefs.edit().clear().commit() - } catch (e: Exception) { - // Ignore any errors during cleanup - } - - // Wait longer to ensure all cleanup operations are complete - Thread.sleep(100) - - // Verify cleanup worked - this should be empty - val verifyPrefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) - val allKeys = verifyPrefs.all - if (allKeys.isNotEmpty()) { - println("WARNING: SharedPreferences still contains keys after cleanup: $allKeys") - // Force clear again - verifyPrefs.edit().clear().commit() - Thread.sleep(50) - } - - // Create a completely fresh OneSignalImp instance for this test + // Fresh prefs files: other tests' background inits write the shared store, which made this flaky. + val context = IsolatedPrefsContext(getApplicationContext()) val os = OneSignalImp() runBlocking { - // When - val result = os.initWithContextSuspend(context, null) - - // Debug output for CI/CD troubleshooting - println("DEBUG: initWithContextSuspend result = $result") - println("DEBUG: os.isInitialized = ${os.isInitialized}") - - // Additional debug: Check what's in SharedPreferences after the call - val debugPrefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) - val debugKeys = debugPrefs.all - println("DEBUG: SharedPreferences after initWithContextSuspend: $debugKeys") - - // Then - should return false because no appId is provided and configModel doesn't have an appId - result shouldBe false + os.initWithContextSuspend(context, null) shouldBe false os.isInitialized shouldBe false } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt new file mode 100644 index 0000000000..3bd9d64a38 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt @@ -0,0 +1,253 @@ +package com.onesignal.core.internal.application + +import android.content.Context +import androidx.test.core.app.ApplicationProvider.getApplicationContext +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.ErrorCode +import com.onesignal.OneSignalUserProfile +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.internal.OneSignalImpV2 +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout + +@RobolectricTest +class SDKInitSuspendV2Tests : FunSpec({ + + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + afterAny { + val context = getApplicationContext() + + // AGGRESSIVE CLEANUP: Clear ALL SharedPreferences to ensure complete isolation + val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) + prefs.edit().clear().commit() + + // Also clear any other potential SharedPreferences files + val otherPrefs = context.getSharedPreferences("com.onesignal", Context.MODE_PRIVATE) + otherPrefs.edit().clear().commit() + + // Wait longer to ensure cleanup is complete + Thread.sleep(50) + } + + // ===== INITIALIZATION TESTS ===== + + test("initWithContextSuspend with appId returns true") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // When + val result = os.initWithContextSuspend(context, "testAppId") + + // Then + result shouldBe true + os.isInitialized shouldBe true + } + } + + test("initWithContextSuspend with null appId fails when configModel has no appId") { + // Fresh prefs files: other tests' background inits write the shared store, which made this flaky. + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + + runBlocking { + os.initWithContextSuspend(context, null) shouldBe false + os.isInitialized shouldBe false + } + } + + test("initWithContextSuspend is idempotent") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // When + val result1 = os.initWithContextSuspend(context, "testAppId") + val result2 = os.initWithContextSuspend(context, "testAppId") + val result3 = os.initWithContextSuspend(context, "testAppId") + + // Then + result1 shouldBe true + result2 shouldBe true + result3 shouldBe true + os.isInitialized shouldBe true + } + } + + // ===== LOGIN TESTS ===== + + test("login suspend method works after initWithContextSuspend") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + val testExternalId = "testUser123" + + runBlocking { + // When + val initResult = os.initWithContextSuspend(context, "testAppId") + initResult shouldBe true + + // Login with timeout - demonstrates suspend method works correctly + try { + withTimeout(2000) { // 2 second timeout + os.login(testExternalId) + } + // If we get here, login completed successfully (unlikely in test env) + os.isInitialized shouldBe true + } catch (e: kotlinx.coroutines.TimeoutCancellationException) { + // Expected timeout due to operation queue processing in test environment + // This proves the suspend method is working correctly + os.isInitialized shouldBe true + println("Login suspend method works correctly - timed out as expected due to operation queue") + } + } + } + + // Note: Tests for null appId removed since appId is now non-nullable + + test("login suspend method with JWT token") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + val testExternalId = "testUser789" + val jwtToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." + + runBlocking { + // When + val initResult = os.initWithContextSuspend(context, "testAppId") + initResult shouldBe true + + try { + withTimeout(2000) { // 2 second timeout + os.login(testExternalId, jwtToken) + } + os.isInitialized shouldBe true + } catch (e: kotlinx.coroutines.TimeoutCancellationException) { + // Expected timeout due to operation queue processing + os.isInitialized shouldBe true + println("Login with JWT suspend method works correctly - timed out as expected due to operation queue") + } + } + } + + // ===== LOGOUT TESTS ===== + + test("logout suspend method works after initWithContextSuspend") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // When + val initResult = os.initWithContextSuspend(context, "testAppId") + initResult shouldBe true + + // Logout with timeout - demonstrates suspend method works correctly + try { + withTimeout(2000) { // 2 second timeout + os.logout() + } + // If we get here, logout completed successfully (unlikely in test env) + os.isInitialized shouldBe true + } catch (e: kotlinx.coroutines.TimeoutCancellationException) { + // Expected timeout due to operation queue processing in test environment + // This proves the suspend method is working correctly + os.isInitialized shouldBe true + println("Logout suspend method works correctly - timed out as expected due to operation queue") + } + } + } + + // ===== INTEGRATION TESTS ===== + + test("multiple login calls work correctly") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // When + val initResult = os.initWithContextSuspend(context, "testAppId") + initResult shouldBe true + + try { + withTimeout(3000) { // 3 second timeout for multiple operations + os.login("user1") + os.login("user2") + os.login("user3") + } + os.isInitialized shouldBe true + } catch (e: kotlinx.coroutines.TimeoutCancellationException) { + // Expected timeout due to operation queue processing + os.isInitialized shouldBe true + println("Multiple login calls suspend method works correctly - timed out as expected due to operation queue") + } + } + } + + test("login and logout sequence works correctly") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // When + val initResult = os.initWithContextSuspend(context, "testAppId") + initResult shouldBe true + + try { + withTimeout(3000) { // 3 second timeout for sequence + os.login("user1") + os.logout() + os.login("user2") + } + os.isInitialized shouldBe true + } catch (e: kotlinx.coroutines.TimeoutCancellationException) { + // Expected timeout due to operation queue processing + os.isInitialized shouldBe true + println("Login/logout sequence suspend methods work correctly - timed out as expected due to operation queue") + } + } + } + + test("suspend login, logout and updateUserJwt are ignored without throwing when initWithContext is never called") { + val oneSignalImp = OneSignalImpV2() + + runBlocking { + oneSignalImp.loginSuspend("testUser", null) + oneSignalImp.logoutSuspend() + oneSignalImp.updateUserJwtSuspend("testUser", "token") + } + + oneSignalImp.isInitialized shouldBe false + } + + test("composite login returns NOT_INITIALIZED when initWithContext is never called") { + val oneSignalImp = OneSignalImpV2() + + val result = runBlocking { oneSignalImp.login("testUser", OneSignalUserProfile()) } + + result.data shouldBe null + result.error!!.first.code shouldBe ErrorCode.NOT_INITIALIZED + } + + test("suspend consent accessors work before initWithContext") { + val oneSignalImp = OneSignalImpV2() + + runBlocking { + oneSignalImp.setConsentRequired(true) + oneSignalImp.setConsentGiven(true) + oneSignalImp.getConsentRequired() shouldBe true + oneSignalImp.getConsentGiven() shouldBe true + oneSignalImp.getDisableGMSMissingPrompt() shouldBe false + } + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt new file mode 100644 index 0000000000..0003062def --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt @@ -0,0 +1,1075 @@ +package com.onesignal.core.internal.application + +import android.content.Context +import android.content.ContextWrapper +import android.content.SharedPreferences +import androidx.test.core.app.ApplicationProvider.getApplicationContext +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.IUserJwtInvalidatedListener +import com.onesignal.common.AndroidUtils +import com.onesignal.common.threading.OneSignalDispatchers +import com.onesignal.core.internal.operations.IOperationRepo +import com.onesignal.core.internal.operations.impl.OperationRepo +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID +import com.onesignal.debug.ILogListener +import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.internal.InitGate +import com.onesignal.internal.InitState +import com.onesignal.internal.OneSignalImpV2 +import com.onesignal.user.internal.UserManager +import com.onesignal.user.internal.jwt.JwtTokenStore +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.maps.shouldContain +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkObject +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +@RobolectricTest +class SDKInitV2Tests : FunSpec({ + + /** + * Helper function to wait for OneSignal initialization to complete. + * @param oneSignalImp The OneSignalImpV2 instance to wait for + * @param maxAttempts Maximum number of attempts (default: 100) + * @param sleepMs Sleep duration between attempts in milliseconds (default: 20) + */ + fun waitForInitialization(oneSignalImp: OneSignalImpV2, maxAttempts: Int = 100, sleepMs: Long = 20) { + var attempts = 0 + while (!oneSignalImp.isInitialized && attempts < maxAttempts) { + Thread.sleep(sleepMs) + attempts++ + } + oneSignalImp.isInitialized shouldBe true + } + + fun withSaturatedOneSignalIo(block: () -> Unit) { + val running = CountDownLatch(2) + val release = CountDownLatch(1) + repeat(4) { + OneSignalDispatchers.launchOnIO { + running.countDown() + release.await(30, TimeUnit.SECONDS) + } + } + + try { + running.await(5, TimeUnit.SECONDS) shouldBe true + block() + } finally { + release.countDown() + } + } + + fun runOnThreadAndWait(block: () -> Unit) { + val error = AtomicReference(null) + val thread = + Thread { + try { + block() + } catch (t: Throwable) { + error.set(t) + } + } + // Daemon so a stuck waitForInit() (e.g. if a dispatch is rejected under pool pressure) + // can never keep the test JVM alive and trip the 6h CI timeout. + thread.isDaemon = true + thread.start() + thread.join(2_000) + + thread.isAlive shouldBe false + error.get() shouldBe null + } + + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + afterAny { + val context = getApplicationContext() + val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) + prefs.edit() + .clear() + .commit() + + // Also clear any other potential SharedPreferences files + val otherPrefs = context.getSharedPreferences("com.onesignal", Context.MODE_PRIVATE) + otherPrefs.edit() + .clear() + .commit() + } + + test("accessor after initWithContext without appID returns a default instead of throwing") { + // Given + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + + // When + os.initWithContext(context) + + // Then + os.user.onesignalId shouldBe "" + os.isInitialized shouldBe false + + // A later initWithContext with an appID still initializes + val result = os.initWithContext(context, "appID") + waitForInitialization(os) + result shouldBe true + os.isInitialized shouldBe true + } + + test("OneSignal accessors return defaults before calling initWithContext") { + val os = OneSignalImpV2() + + os.user.onesignalId shouldBe "" + os.user.externalId shouldBe "" + os.user.getTags() shouldBe emptyMap() + os.user.pushSubscription.id shouldBe "" + os.user.pushSubscription.optedIn shouldBe false + os.user.addTag("key", "value") + os.notifications.permission shouldBe false + runBlocking { os.notifications.requestPermission(true) } shouldBe false + os.inAppMessages.paused shouldBe false + os.inAppMessages.addTrigger("key", "value") + os.session.addOutcome("outcome") + os.location.isShared shouldBe false + os.isInitialized shouldBe false + } + + test("accessor references captured before initWithContext forward once init succeeds") { + val os = OneSignalImpV2() + val user = os.user + + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "appId") + waitForInitialization(os) + + user.addTag("key", "value") + os.user.getTags() shouldContain ("key" to "value") + user.getTags() shouldContain ("key" to "value") + } + + test("accessor references captured after a failed init forward once a retry succeeds") { + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + os.initWithContext(context, "") + val user = os.user + os.isInitialized shouldBe false + + os.initWithContext(context, "appId") + waitForInitialization(os) + + user.addTag("key", "value") + os.user.getTags() shouldContain ("key" to "value") + user.getTags() shouldContain ("key" to "value") + } + + test("a retry after a failed init does not enqueue duplicate operations") { + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + os.initWithContext(context, "") + os.user + os.isInitialized shouldBe false + + os.initWithContext(context, "appId") + waitForInitialization(os) + os.user.addTag("retryKey", "value") + + val repo = os.getService(IOperationRepo::class.java) as OperationRepo + fun tagOps() = synchronized(repo.queue) { repo.queue.count { it.operation.toString().contains("retryKey") } } + repeat(50) { if (tagOps() == 0) Thread.sleep(20) } + Thread.sleep(200) + tagOps() shouldBe 1 + } + + test("observers and listeners registered before init are applied once init succeeds") { + val os = OneSignalImpV2() + os.user.addObserver(mockk(relaxed = true)) + // Core-only test: the in-app messages module is absent, so replaying this throws and must not break init. + os.inAppMessages.addClickListener(mockk(relaxed = true)) + + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "appId") + waitForInitialization(os) + + (os.user as UserManager).changeHandlersNotifier.hasSubscribers shouldBe true + os.isInitialized shouldBe true + } + + test("pre-init registrations are replayed before init reports success") { + val os = OneSignalImpV2() + var stateAtReplay: InitState? = null + // The in-app messages module is absent here, so its replayed call logs synchronously on the init thread. + val listener = ILogListener { if (it.entry.contains("Replaying 'addClickListener'")) stateAtReplay = os.gate.state } + Logging.addListener(listener) + try { + os.inAppMessages.addClickListener(mockk(relaxed = true)) + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "appId") + waitForInitialization(os) + } finally { + Logging.removeListener(listener) + } + + stateAtReplay shouldBe InitState.IN_PROGRESS + } + + test("a JWT invalidated listener added before init is replayed, and a removal before init cancels it") { + val os = OneSignalImpV2() + val fired = CountDownLatch(1) + val removedFired = CountDownLatch(1) + val removed = IUserJwtInvalidatedListener { removedFired.countDown() } + os.addUserJwtInvalidatedListener { fired.countDown() } + os.addUserJwtInvalidatedListener(removed) + os.removeUserJwtInvalidatedListener(removed) + + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "appId") + waitForInitialization(os) + val store = os.getService(JwtTokenStore::class.java) + store.putJwt("ext", "token") + store.invalidateJwt("ext") + + fired.await(2, TimeUnit.SECONDS) shouldBe true + removedFired.await(200, TimeUnit.MILLISECONDS) shouldBe false + } + + test("an empty appId that falls back to a cached appId logs a warning, not an error") { + val context = IsolatedPrefsContext(getApplicationContext()) + context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE).edit().putString(PREFS_LEGACY_APP_ID, "cachedAppId").commit() + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + val os = OneSignalImpV2() + try { + os.initWithContext(context, "") + waitForInitialization(os) + } finally { + Logging.removeListener(listener) + } + + os.isInitialized shouldBe true + events.filter { it.level == LogLevel.ERROR && it.entry.contains("appId") } shouldBe emptyList() + events.count { it.level == LogLevel.WARN && it.entry.contains("using the cached app ID") } shouldBe 1 + } + + test("a call before init logs one error naming the call, with the failure trace once per attempt") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val os = OneSignalImpV2() + os.user.addTag("key", "value") + events.filter { it.level == LogLevel.ERROR }.map { it.entry.substringAfter("] ") } shouldBe + listOf("'User.addTag' ignored: call initWithContext with a valid appId first.") + + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "") + os.user + events.clear() + os.user.addTag("key", "value") + os.user.addTag("key", "value") + + val errors = events.filter { it.level == LogLevel.ERROR && it.entry.contains("'User.addTag'") } + errors.size shouldBe 2 + errors.count { it.entry.contains("OneSignal initWithContext failed.") } shouldBe 1 + } finally { + Logging.removeListener(listener) + } + } + + test("a consent change made after init applied the stored values is not lost") { + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + os.consentGiven = false + os.initWithContext(context, "appId") + waitForInitialization(os) + + // Re-enter the window between updateConfig and SUCCESS, where the setter used to write only the local copy. + val state = InitGate::class.java.getDeclaredField("state").apply { isAccessible = true } + val repo = os.getService(IOperationRepo::class.java) as OperationRepo + val forced = OperationRepo::class.java.getDeclaredField("forcedDuringExecute").apply { isAccessible = true } + synchronized(repo.queue) { + repo.queue.clear() + forced.set(repo, false) + } + state.set(os.gate, InitState.IN_PROGRESS) + os.consentGiven = true + state.set(os.gate, InitState.SUCCESS) + + os.consentGiven shouldBe true + forced.get(repo) shouldBe true + } + + test("cancelling initWithContextSuspend before init runs does not leave init stuck in progress") { + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + + runBlocking { + launch { + cancel() + os.initWithContextSuspend(context, "appId") + }.join() + } + + os.gate.state shouldBe InitState.FAILED + os.initWithContext(context, "appId") shouldBe true + waitForInitialization(os) + } + + test("initWithContext with a null context returns false without throwing") { + val os = OneSignalImpV2() + + os.initWithContext(null, "appId") shouldBe false + runBlocking { os.initWithContextSuspend(null, "appId") } shouldBe false + os.isInitialized shouldBe false + } + + test("initWithContext returns gracefully when Android user is locked") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + mockkObject(AndroidUtils) + every { AndroidUtils.isAndroidUserUnlocked(any()) } returns false + + // When + os.initWithContext(context, "appId") + + // Then + // Init runs asynchronously and fails the locked-storage check, transitioning to FAILED. + // Reading an accessor blocks until that terminal state, then returns a default. + os.user.onesignalId shouldBe "" + os.login("user") + os.isInitialized shouldBe false + + unmockkObject(AndroidUtils) + } + + test("initWithContext is successful when Android user is unlocked") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + mockkObject(AndroidUtils) + every { AndroidUtils.isAndroidUserUnlocked(any()) } returns true + + // When + os.initWithContext(context, "appId") + + // Then + waitForInitialization(os) + os.isInitialized shouldBe true + + unmockkObject(AndroidUtils) + } + + test("initWithContext with no appId succeeds when configModel has appId") { + // Given + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) + + // Set up a legacy appId in SharedPreferences to simulate a previous test scenario + // This simulates the case where a previous test has set an appId that can be resolved + prefs.edit() + .putString(PREFS_LEGACY_APP_ID, "testAppId") // Set legacy appId + .commit() + + // When - no appId passed; resolved from the legacy SharedPreferences value. + os.initWithContext(context) + + // Then - init completes successfully because an appId was resolvable. + waitForInitialization(os) + os.isInitialized shouldBe true + } + + test("initWithContext with an empty or null appId and nothing cached skips init without throwing") { + for (appId in listOf("", null)) { + // Fresh stores: buffered prefs writes from earlier tests can land after afterAny clears. + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + os.initWithContext(context, appId) + + // Blocks until init reaches its terminal state, then returns a default. + os.user.onesignalId shouldBe "" + os.login("user") + os.isInitialized shouldBe false + } + } + + test("initWithContext with an empty appId falls back to the cached appId") { + val context = IsolatedPrefsContext(getApplicationContext()) + context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE).edit() + .putString(PREFS_LEGACY_APP_ID, "cachedAppId") + .commit() + val os = OneSignalImpV2() + + os.initWithContext(context, "") + + waitForInitialization(os) + os.isInitialized shouldBe true + } + + test("initWithContext returns immediately and completes init asynchronously") { + // Background-threaded init is the default: initWithContext dispatches internalInit to a + // background dispatcher and returns immediately, even while internalInit is parked on a + // slow SharedPreferences read. isInitialized only flips to true once init completes. + val trigger = CountDownLatch(1) + val context = getApplicationContext() + val blockingPrefContext = BlockingPrefsContext(context, trigger) + val os = OneSignalImpV2() + + // Returns right away despite the blocked prefs read happening on the background worker. + os.initWithContext(blockingPrefContext, "appId") shouldBe true + os.isInitialized shouldBe false + + trigger.countDown() + waitForInitialization(os) + os.isInitialized shouldBe true + } + + test("accessors will be blocked if call too early after initWithContext with appId") { + // Given + // block SharedPreference before calling init + val trigger = CountDownLatch(1) + val context = getApplicationContext() + val blockingPrefContext = BlockingPrefsContext(context, trigger) + val os = OneSignalImpV2() + + val accessorThread = + Thread { + os.initWithContext(blockingPrefContext, "appId") + os.user // This should block until either trigger is released or timed out + } + + // Daemon so a stuck waitForInit() can never keep the test JVM alive. + accessorThread.isDaemon = true + accessorThread.start() + accessorThread.join(500) + + accessorThread.isAlive shouldBe true + + // release the lock on SharedPreferences + trigger.countDown() + + accessorThread.join(1000) + accessorThread.isAlive shouldBe false + os.isInitialized shouldBe true + } + + test("ensure adding tags right after initWithContext with appId is successful") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + val tagKey = "tagKey" + val tagValue = "tagValue" + val testTags = mapOf(tagKey to tagValue) + + // When + os.initWithContext(context, "appId") + os.user.addTags(testTags) + + // Then + val tags = os.user.getTags() + tags shouldContain (tagKey to tagValue) + } + + test("ensure login called right after initWithContext can set externalId correctly") { + // Given + // block SharedPreference before calling init + val trigger = CountDownLatch(1) + val context = getApplicationContext() + val blockingPrefContext = BlockingPrefsContext(context, trigger) + val os = OneSignalImpV2() + val externalId = "testUser" + + val accessorThread = + Thread { + os.initWithContext(blockingPrefContext, "appId") + os.login(externalId) + + // Wait for background login operation to complete with polling + var attempts = 0 + while (os.user.externalId != externalId && attempts < 50) { + Thread.sleep(20) + attempts++ + } + } + + // Daemon so a stuck waitForInit() can never keep the test JVM alive. + accessorThread.isDaemon = true + accessorThread.start() + accessorThread.join(500) + + // initWithContext should return immediately (non-blocking) + // but isInitialized won't be true until internalInit completes + // which requires SharedPreferences to be unblocked + accessorThread.isAlive shouldBe true + + // release the lock on SharedPreferences so internalInit can complete + trigger.countDown() + + // Wait for initialization to complete (internalInit runs asynchronously) + var initAttempts = 0 + while (!os.isInitialized && initAttempts < 50) { + Thread.sleep(20) + initAttempts++ + } + + os.isInitialized shouldBe true + + accessorThread.join(500) + accessorThread.isAlive shouldBe false + os.user.externalId shouldBe externalId + } + + test("a push subscription should be created right after initWithContext") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + os.initWithContext(context, "appId") + + // When + val pushSub = os.user.pushSubscription + + // Then + pushSub shouldNotBe null + pushSub.token shouldNotBe null + } + + test("login changes externalId from initial state after init") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + val testExternalId = "uniqueTestUser_${System.currentTimeMillis()}" // Use unique ID to avoid conflicts + + // When + os.initWithContext(context, "appId") + val initialExternalId = os.user.externalId + os.login(testExternalId) + + // Wait for background login operation to complete with polling + var attempts = 0 + while (os.user.externalId != testExternalId && attempts < 50) { + Thread.sleep(20) + attempts++ + } + + val finalExternalId = os.user.externalId + + // Then - Verify the complete login flow + // 1. Login should set the external ID to our test value + finalExternalId shouldBe testExternalId + + // 2. Login should change the external ID (regardless of initial state) + // This makes the test resilient to state contamination while still testing the flow + finalExternalId shouldNotBe initialExternalId + + // 3. If we're in a clean state, initial should be empty (but don't fail if not) + // This documents the expected behavior without making the test brittle + if (initialExternalId.isEmpty()) { + // Clean state detected - this is the ideal scenario + println("✅ Clean state: initial externalId was empty as expected") + } else { + // State contamination detected - log it but don't fail + println("⚠️ State contamination: initial externalId was '$initialExternalId' (expected empty)") + } + + // Clean up after ourselves to avoid polluting subsequent tests + os.logout() + + // Wait for logout to complete with polling + var logoutAttempts = 0 + while (os.user.externalId.isNotEmpty() && logoutAttempts < 50) { + Thread.sleep(20) + logoutAttempts++ + } + } + + test("accessor instances after multiple initWithContext calls are consistent") { + // Given + val context = getApplicationContext() + val os = OneSignalImpV2() + + // When + os.initWithContext(context, "appId") + + // Wait for initialization to complete before accessing user + waitForInitialization(os) + + // Give additional time for coroutines to settle, especially in CI/CD + Thread.sleep(50) + + val oldUser = os.user + + // Second init from some internal class + os.initWithContext(context) + + // Wait for second initialization to complete + waitForInitialization(os) + + // Give additional time for coroutines to settle after second init + Thread.sleep(50) + + val newUser = os.user + + // Then + oldUser shouldBe newUser + } + + test("integration: full user workflow after initialization") { + val context = getApplicationContext() + val os = OneSignalImpV2() + val testExternalId = "test-user" + val tags = mapOf("test" to "integration", "version" to "1.0") + + os.initWithContext(context, "appId") + + // Test user workflow + // init + val initialExternalId = os.user.externalId + + // Handle state contamination gracefully - if externalId is not empty, logout first + if (initialExternalId.isNotEmpty()) { + println("⚠️ State contamination detected: initial externalId was '$initialExternalId' (expected empty)") + os.logout() + + // Wait for logout to complete with polling + var cleanupAttempts = 0 + while (os.user.externalId.isNotEmpty() && cleanupAttempts < 50) { + Thread.sleep(20) + cleanupAttempts++ + } + + val cleanedExternalId = os.user.externalId + cleanedExternalId shouldBe "" + } else { + initialExternalId shouldBe "" + } + + // login + os.login(testExternalId) + + // Wait for background login operation to complete with polling (CI-safe) + run { + var attempts = 0 + val maxAttempts = 200 // 4 seconds total at 20ms intervals + while (os.user.externalId != testExternalId && attempts < maxAttempts) { + Thread.sleep(20) + attempts++ + } + } + + os.user.externalId shouldBe testExternalId + + // addTags and getTags + os.user.addTags(tags) + val retrievedTags = os.user.getTags() + retrievedTags shouldContain ("test" to "integration") + retrievedTags shouldContain ("version" to "1.0") + + // logout + os.logout() + + // Wait for background logout operation to complete with polling + var attempts = 0 + while (os.user.externalId.isNotEmpty() && attempts < 50) { + Thread.sleep(20) + attempts++ + } + + os.user.externalId shouldBe "" + } + + test("initWithContext recovers when ensureApplicationServiceStarted throws synchronously") { + // Given: a context whose applicationContext is NOT an Application instance. + // ApplicationService.start does `context.applicationContext as Application`, which + // will throw ClassCastException — simulates restricted hosts (Robolectric custom + // application factories, instrumentation context wrappers, multi-process content + // provider init order). Without recovery, initState would be left at IN_PROGRESS + // forever and accessors would deadlock on suspendCompletion.await(). + val baseContext = getApplicationContext() + val brokenContext = + object : ContextWrapper(baseContext) { + override fun getApplicationContext(): Context = ContextWrapper(baseContext) + } + val os = OneSignalImpV2() + + // When: synchronous failure during the bootstrap step. + os.initWithContext(brokenContext, "appId") shouldBe false + + // Then: SDK transitions to FAILED, and the accessor returns a default. + os.user.onesignalId shouldBe "" + os.isInitialized shouldBe false + + // And: a retry with a working context succeeds (FAILED is not "accessible", + // so the retry legitimately re-enters init). + os.initWithContext(baseContext, "appId") shouldBe true + waitForInitialization(os) + } + + test("accessor returns once init is SUCCESS even when the IO dispatcher is saturated (OneSignal-Flutter-SDK#1163)") { + // Regression test for the Flutter main-thread ANR. getServiceWithFeatureGate used to + // ALWAYS route through waitAndReturn -> waitForInit -> runBlocking(OneSignalDispatchers.IO), + // even after init had already reached SUCCESS. On Flutter's main-thread lifecycleInit that + // parks the UI thread on a cold-start-contended IO pool (OneSignal-Flutter-SDK#1163). The + // fix adds a lock-free SUCCESS fast-path, centralized in waitForInit(). + // + // We reproduce the contention faithfully: saturate OneSignalDispatchers.IO so that the + // pre-fix runBlocking(OneSignalDispatchers.IO) would park indefinitely behind the busy + // workers, then assert the accessor still returns. With the fix it answers synchronously; + // without it the accessor thread would still be parked when we check. + val context = getApplicationContext() + val os = OneSignalImpV2() + + os.initWithContext(context, "appId") + waitForInitialization(os) + + withSaturatedOneSignalIo { + // When: a SUCCESS-state accessor is read while the IO pool is fully saturated. + val result = arrayOfNulls(1) + runOnThreadAndWait { result[0] = os.user } + + // Then: the fast-path answered synchronously and the thread finished. The pre-fix + // runBlocking(OneSignalDispatchers.IO) path would still be parked here. + result[0] shouldNotBe null + } + } + + test("login returns once init is SUCCESS even when the IO dispatcher is saturated (OneSignal-Flutter-SDK#1163)") { + // Companion regression test proving the SUCCESS fast-path is centralized in waitForInit(), + // so it covers the operation paths (login / logout / updateUserJwt / JWT listeners), not + // just the service accessors. login() used to route through + // waitForInit -> runBlocking(OneSignalDispatchers.IO) even after init reached SUCCESS, so a + // saturated IO pool would park the caller indefinitely. + val context = getApplicationContext() + val os = OneSignalImpV2() + + os.initWithContext(context, "appId") + waitForInitialization(os) + + withSaturatedOneSignalIo { + // When: login() is invoked while the IO pool is fully saturated. waitForInit must + // short-circuit (the subsequent enqueue is fire-and-forget via suspendifyOnIO and does + // not block the caller). + runOnThreadAndWait { os.login("externalId") } + + // Then: the fast-path answered synchronously and the call returned. The pre-fix + // runBlocking(OneSignalDispatchers.IO) path would still be parked here. + } + } + + test("consent getters return once init is SUCCESS even when the IO dispatcher is saturated") { + val context = getApplicationContext() + val os = OneSignalImpV2() + + os.initWithContext(context, "appId") + waitForInitialization(os) + + withSaturatedOneSignalIo { + val results = arrayOfNulls(3) + runOnThreadAndWait { + results[0] = os.consentRequired + results[1] = os.consentGiven + results[2] = os.disableGMSMissingPrompt + } + + results.toList() shouldBe listOf(false, false, false) + } + } + + test("initWithContextSuspend returns once init is SUCCESS even when the IO dispatcher is saturated") { + // Observing a terminal SUCCESS must not require an IO worker. + val context = getApplicationContext() + val os = OneSignalImpV2() + + os.initWithContext(context, "appId") + waitForInitialization(os) + + withSaturatedOneSignalIo { + val result = arrayOfNulls(1) + runOnThreadAndWait { result[0] = runBlocking { os.initWithContextSuspend(context, "appId") } } + + result[0] shouldBe true + } + } + + test("initWithContextSuspend awaiting an in-flight init does not need an IO worker") { + // Waiting on another caller's init must not queue behind the work that init is doing, + // otherwise a caller arriving while init is IN_PROGRESS stalls until the pool drains. + val context = getApplicationContext() + val os = OneSignalImpV2() + + val started = CountDownLatch(1) + val trigger = CountDownLatch(1) + val blockingCtx = + object : ContextWrapper(context) { + override fun getSharedPreferences(name: String, mode: Int): SharedPreferences { + started.countDown() + trigger.await(30, TimeUnit.SECONDS) + return super.getSharedPreferences(name, mode) + } + } + + // Hold init at IN_PROGRESS, then fill the pool so no worker is left for a second caller. + os.initWithContext(blockingCtx, "appId") + started.await(5, TimeUnit.SECONDS) shouldBe true + os.isInitialized shouldBe false + + // The stalled init already holds one worker, so one more blocking task fills the pool + // and the rest back up the queue. + val release = CountDownLatch(1) + val occupied = CountDownLatch(1) + repeat(4) { + OneSignalDispatchers.launchOnIO { + occupied.countDown() + release.await(30, TimeUnit.SECONDS) + } + } + + try { + occupied.await(5, TimeUnit.SECONDS) shouldBe true + + val startedWaiting = CountDownLatch(1) + val waiter = + Thread { + runBlocking { + startedWaiting.countDown() + os.initWithContextSuspend(context, "appId") + } + } + waiter.isDaemon = true + waiter.start() + + startedWaiting.await(5, TimeUnit.SECONDS) shouldBe true + Thread.sleep(200) + // Still blocked, because the init it is waiting on has not been released yet. + waiter.isAlive shouldBe true + + trigger.countDown() + + // Resuming is driven by the latch, not by a free worker, so this completes while + // the queued tasks above still own the pool. + waiter.join(5_000) + waiter.isAlive shouldBe false + } finally { + trigger.countDown() + release.countDown() + } + + os.isInitialized shouldBe true + } + + test("login and logout are ignored without throwing when initWithContext is never called") { + val oneSignalImp = OneSignalImpV2() + + oneSignalImp.login("testUser", null) + oneSignalImp.logout() + oneSignalImp.updateUserJwt("testUser", "token") + + oneSignalImp.isInitialized shouldBe false + } + + // Regression test for the SessionService NPE during in-flight background init. + // + // Background: when init is in flight, internalInit() has not yet called bootstrapServices(), + // which means IBootstrapService implementations like SessionService have null model fields. + // SyncJobService re-enters via initWithContextSuspend(context, null) and then immediately + // calls runBackgroundServices(); if initWithContextSuspend returns true while init is still + // in progress (the old behavior), the loop hits SessionService.endSession() → NPE. + // + // The fix: initWithContextSuspend must suspend until init is *fully* completed, not just + // kicked off. This test verifies that contract by checking that os.isInitialized is true + // at the moment the re-entrant suspend call returns. + test("initWithContextSuspend with in-flight init waits for completion before returning") { + val context = getApplicationContext() + val trigger = CountDownLatch(1) + // started signals when the first caller has entered internalInit() and reached the + // blocking SharedPreferences access -- by which point initState is IN_PROGRESS. + val started = CountDownLatch(1) + val blockingCtx = object : ContextWrapper(context) { + override fun getSharedPreferences(name: String, mode: Int): SharedPreferences { + started.countDown() + // Bounded await -- if the test logic ever fails to release `trigger`, the + // IO worker exits cleanly instead of blocking forever and deadlocking the suite. + trigger.await(30, TimeUnit.SECONDS) + return super.getSharedPreferences(name, mode) + } + } + val os = OneSignalImpV2() + + runBlocking { + // First caller: stalls inside internalInit() once it tries to read prefs. + val firstInit = async(Dispatchers.IO) { + os.initWithContextSuspend(blockingCtx, "appId") + } + + // Deterministically wait until firstInit has entered the prefs-blocking region. + // After this point, initState is guaranteed to be IN_PROGRESS. + started.await(5, TimeUnit.SECONDS) shouldBe true + os.isInitialized shouldBe false + firstInit.isCompleted shouldBe false + + // Second caller: under the OLD behavior the synchronized(initLock) block sees + // state == IN_PROGRESS and returns `true` *while* internalInit() (running on the + // first caller) has not yet reached `initState = InitState.SUCCESS`. Under the + // fix, this call must suspend until the first caller fully completes init -- so + // that os.isInitialized is guaranteed `true` at the moment we return. + // + // We capture isInitialized at exactly the moment initWithContextSuspend returns + // for the second caller. Old code: false (bug). New code: true (fix). + val secondInit = async(Dispatchers.IO) { + val result = os.initWithContextSuspend(context, "appId") + val initializedAtReturn = os.isInitialized + Pair(result, initializedAtReturn) + } + + // Sanity: the second caller has not pre-empted the test by returning before + // we unblock the first caller (timing depends on lazy ServiceProvider locks). + Thread.sleep(200) + + // Unblock the first caller so internalInit() can complete (state -> SUCCESS). + trigger.countDown() + + firstInit.await() shouldBe true + val (secondResult, initializedAtReturn) = secondInit.await() + secondResult shouldBe true + // KEY assertion: os must be fully initialized at the moment the re-entrant + // suspend init returns. Old code violated this by returning early while state + // was still IN_PROGRESS, which is what allowed SyncJobService to reach + // runBackgroundServices() before bootstrap() had run. + initializedAtReturn shouldBe true + os.isInitialized shouldBe true + } + } + + // Regression test for review-flagged Defect 1 (stale latch on retry-after-FAILED). + // + // Background: `suspendCompletion` was a single-shot `val CompletableDeferred`. Once + // any init terminates (even FAILED), the deferred stays permanently complete -- so a + // re-entrant suspend caller arriving DURING a subsequent retry would `await()` on the + // already-completed deferred, return instantly, and read transient state (likely + // IN_PROGRESS -> false), silently dropping JobService work. + // + // The fix resets `suspendCompletion` whenever the synchronized(initLock) block flips state + // into IN_PROGRESS, and await sites local-capture the deferred under the lock. + test("initWithContextSuspend resets latch on retry-after-FAILED") { + val context = getApplicationContext() + val os = OneSignalImpV2() + + runBlocking { + // 1. Force a deterministic FAILED first init via the user-locked branch + // (matches the pattern used by other tests in this file). + mockkObject(AndroidUtils) + every { AndroidUtils.isAndroidUserUnlocked(any()) } returns false + val firstInit = os.initWithContextSuspend(context, "appId") + firstInit shouldBe false + os.isInitialized shouldBe false + // At this point the (pre-fix) single-shot `suspendCompletion` is permanently complete. + unmockkObject(AndroidUtils) + + // 2. Stall a fresh retry via BlockingPrefsContext so initState sits at IN_PROGRESS. + val started = CountDownLatch(1) + val trigger = CountDownLatch(1) + val blockingCtx = object : ContextWrapper(context) { + override fun getSharedPreferences(name: String, mode: Int): SharedPreferences { + started.countDown() + // Bounded await -- if the test logic ever fails to release `trigger`, the + // IO worker exits cleanly instead of blocking forever and deadlocking the suite. + trigger.await(30, TimeUnit.SECONDS) + return super.getSharedPreferences(name, mode) + } + } + + val secondInit = async(Dispatchers.IO) { os.initWithContextSuspend(blockingCtx, "appId") } + started.await(5, TimeUnit.SECONDS) shouldBe true + os.isInitialized shouldBe false + secondInit.isCompleted shouldBe false + + // 3. Re-entrant caller. With Defect 1, it would wake immediately on the + // FAILED-generation latch (still permanently complete) and return based on + // transient state. With the fix, it waits on the *new* generation's latch. + val thirdInit = async(Dispatchers.IO) { + val r = os.initWithContextSuspend(context, "appId") + val initializedAtReturn = os.isInitialized + Pair(r, initializedAtReturn) + } + + Thread.sleep(300) + thirdInit.isCompleted shouldBe false // would be true with the stale-latch bug + + // 4. Release the second init; both new callers complete with state == SUCCESS. + trigger.countDown() + + secondInit.await() shouldBe true + val (thirdResult, thirdInitializedAtReturn) = thirdInit.await() + thirdResult shouldBe true + thirdInitializedAtReturn shouldBe true + os.isInitialized shouldBe true + } + } + + // Regression test for review-flagged Defect 2 (indefinite hang if internalInit throws). + // + // Background: `internalInit` had no try/catch wrapping its body. An unchecked throw from + // initEssentials/bootstrapServices/etc. would leave initState=IN_PROGRESS forever and + // `suspendCompletion` uncompleted -- causing every re-entrant suspend caller (e.g. + // SyncJobService) to hang on `await()` indefinitely, holding its budget slot until the + // OS killed the worker. + // + // The fix wraps internalInit's body in try/catch, ensuring a terminal state and + // `notifyInitComplete()` on any throw. + test("initWithContextSuspend reaches terminal state when internalInit throws") { + val context = getApplicationContext() + val os = OneSignalImpV2() + + // Make the very first call inside internalInit throw (mirrors the user-locked test + // pattern, but with a throw instead of a `false` return). This is the cheapest way to + // simulate "any unchecked exception during bootstrap" without coupling to specific + // bootstrap internals. + mockkObject(AndroidUtils) + every { AndroidUtils.isAndroidUserUnlocked(any()) } throws RuntimeException("simulated bootstrap failure") + + runBlocking { + // Without the fix: the throw propagates out of internalInit, escapes withContext, + // and either fails the test outright or leaves the SDK in a deadlocked + // IN_PROGRESS state with `suspendCompletion` never completed. + // With the fix: the catch block sets FAILED + notifyInitComplete + returns false. + val firstResult = withTimeoutOrNull(5_000) { + os.initWithContextSuspend(context, "appId") + } + firstResult shouldBe false + os.isInitialized shouldBe false + + unmockkObject(AndroidUtils) + + // State is FAILED so a retry is allowed and (per Defect 1 fix) gets a fresh latch. + // This also doubles as a smoke test that we didn't leak IN_PROGRESS. + val retry = withTimeoutOrNull(5_000) { + os.initWithContextSuspend(context, "appId") + } + retry shouldBe true + os.isInitialized shouldBe true + } + } +}) + +/** Gives each test its own SharedPreferences files, so other tests' writes can't leak in. */ +class IsolatedPrefsContext(context: Context) : ContextWrapper(context) { + private val prefix = java.util.UUID.randomUUID().toString() + + override fun getSharedPreferences( + name: String, + mode: Int, + ): SharedPreferences = super.getSharedPreferences("$prefix-$name", mode) +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index 7416b2910c..2d9998be66 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -81,6 +81,20 @@ class StartupServiceTests : FunSpec({ verify(exactly = 0) { mockBootstrapService2.bootstrap() } } + test("a bootstrap retried after a failure skips the services that already bootstrapped") { + val done = mockk(relaxed = true) + val flaky = mockk() + every { flaky.bootstrap() } throws Exception("first attempt") andThen Unit + val startupService = StartupService(setupServiceProvider(listOf(done, flaky), listOf())) + + shouldThrowUnit { startupService.bootstrap() } + startupService.bootstrap() + startupService.bootstrap() + + verify(exactly = 1) { done.bootstrap() } + verify(exactly = 2) { flaky.bootstrap() } + } + test("startup will call all IStartableService dependencies successfully after a short delay") { // Given val mockStartupService1 = mockk(relaxed = true) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt new file mode 100644 index 0000000000..387635295a --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt @@ -0,0 +1,92 @@ +package com.onesignal.internal + +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +class InitGateTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + // Runs isReady on its own thread; the returned latch opens when it returns. + fun isReadyOnThread( + gate: InitGate, + result: AtomicReference, + ): CountDownLatch { + val finished = CountDownLatch(1) + Thread { + result.set(gate.isReady(SdkOperation.LOGIN)) + finished.countDown() + }.apply { isDaemon = true }.start() + return finished + } + + test("isReady is true at SUCCESS and false at NOT_STARTED and FAILED") { + val gate = InitGate() + gate.isReady(SdkOperation.LOGIN) shouldBe false + + gate.tryStart() shouldBe true + gate.complete(InitState.FAILED) + gate.isReady(SdkOperation.LOGIN) shouldBe false + + gate.tryStart() shouldBe true + gate.complete(InitState.SUCCESS) + gate.isReady(SdkOperation.LOGIN) shouldBe true + } + + test("tryStart refuses while an attempt is running or after success") { + val gate = InitGate() + gate.tryStart() shouldBe true + gate.tryStart() shouldBe false + gate.complete(InitState.SUCCESS) + gate.tryStart() shouldBe false + } + + test("isReady at IN_PROGRESS waits, then reports the attempt's outcome") { + for (outcome in listOf(InitState.SUCCESS, InitState.FAILED)) { + val gate = InitGate() + gate.tryStart() + val result = AtomicReference(null) + val finished = isReadyOnThread(gate, result) + + finished.await(200, TimeUnit.MILLISECONDS) shouldBe false + gate.complete(outcome) + + finished.await(2, TimeUnit.SECONDS) shouldBe true + result.get() shouldBe (outcome == InitState.SUCCESS) + } + } + + test("a waiter follows a retry that starts before it wakes up") { + val gate = InitGate() + gate.tryStart() + val result = AtomicReference(null) + val finished = isReadyOnThread(gate, result) + finished.await(200, TimeUnit.MILLISECONDS) shouldBe false + + // Holding the gate's lock makes fail-then-retry atomic: the waiter can only see the retry. + val lock = InitGate::class.java.getDeclaredField("lock").apply { isAccessible = true }.get(gate) + synchronized(lock) { + gate.complete(InitState.FAILED) + gate.tryStart() shouldBe true + } + finished.await(200, TimeUnit.MILLISECONDS) shouldBe false + + gate.complete(InitState.SUCCESS) + finished.await(2, TimeUnit.SECONDS) shouldBe true + result.get() shouldBe true + } + + test("notReadyMessage distinguishes a failed init from one that never started") { + val gate = InitGate() + gate.notReadyMessage("login") shouldBe "'login' ignored: call initWithContext with a valid appId first." + gate.tryStart() + gate.complete(InitState.FAILED) + gate.notReadyMessage("login") shouldBe "'login' ignored: initWithContext failed." + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt new file mode 100644 index 0000000000..b52b92bfa2 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt @@ -0,0 +1,247 @@ +package com.onesignal.internal + +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class OneSignalImpV2Tests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + fun markInitialized(os: OneSignalImpV2) { + os.gate.tryStart() + os.gate.complete(InitState.SUCCESS) + } + + test("attempting login before initWithContext is ignored without throwing") { + val os = OneSignalImpV2() + // NONE: otherwise Logging.error calls the unmocked android.util.Log. + Logging.logLevel = LogLevel.NONE + + os.login("login-id") + os.login("") + + os.isInitialized shouldBe false + } + + test("login with empty externalId after init does not switch user") { + val os = OneSignalImpV2() + markInitialized(os) + // NONE: otherwise Logging.error calls the unmocked android.util.Log. + Logging.logLevel = LogLevel.NONE + + os.login("") + } + + test("login with a null byte in externalId after init does not switch user") { + val os = OneSignalImpV2() + markInitialized(os) + Logging.logLevel = LogLevel.NONE + + os.login("\u0000: 1") + } + + test("updateUserJwt with empty externalId or token after init does not store the token") { + val os = OneSignalImpV2() + markInitialized(os) + Logging.logLevel = LogLevel.NONE + + os.updateUserJwt("", "token") + os.updateUserJwt("user", "") + } + + test("attempting logout before initWithContext is ignored without throwing") { + val os = OneSignalImpV2() + Logging.logLevel = LogLevel.NONE + + os.logout() + + os.isInitialized shouldBe false + } + + test("blockingGet returns synchronously without dispatching when initState is SUCCESS") { + val os = OneSignalImpV2() + markInitialized(os) + + val blockingGet = OneSignalImpV2::class.java.getDeclaredMethod("blockingGet", Function0::class.java) + blockingGet.isAccessible = true + + val getter = { "value" } + blockingGet.invoke(os, getter) shouldBe "value" + } + + // Comprehensive tests for deprecated properties that should work before and after initialization + context("consentRequired property") { + context("before initWithContext") { + test("get returns false by default") { + // Given + val os = OneSignalImpV2() + + // When & Then + os.consentRequired shouldBe false + } + + test("set and get works correctly") { + // Given + val os = OneSignalImpV2() + + // When + os.consentRequired = true + + // Then + os.consentRequired shouldBe true + + // When + os.consentRequired = false + + // Then + os.consentRequired shouldBe false + } + + test("set should not throw") { + // Given + val os = OneSignalImpV2() + + // When & Then - should not throw + os.consentRequired = false + os.consentRequired = true + } + } + } + + context("consentGiven property") { + context("before initWithContext") { + test("get returns false by default") { + // Given + val os = OneSignalImpV2() + + // When & Then + os.consentGiven shouldBe false + } + + test("set and get works correctly") { + // Given + val os = OneSignalImpV2() + + // When + os.consentGiven = true + + // Then + os.consentGiven shouldBe true + + // When + os.consentGiven = false + + // Then + os.consentGiven shouldBe false + } + + test("set should not throw") { + // Given + val os = OneSignalImpV2() + + // When & Then - should not throw + os.consentGiven = true + os.consentGiven = false + } + } + } + + context("disableGMSMissingPrompt property") { + context("before initWithContext") { + test("get returns false by default") { + // Given + val os = OneSignalImpV2() + + // When & Then + os.disableGMSMissingPrompt shouldBe false + } + + test("set and get works correctly") { + // Given + val os = OneSignalImpV2() + + // When + os.disableGMSMissingPrompt = true + + // Then + os.disableGMSMissingPrompt shouldBe true + + // When + os.disableGMSMissingPrompt = false + + // Then + os.disableGMSMissingPrompt shouldBe false + } + + test("set should not throw") { + // Given + val os = OneSignalImpV2() + + // When & Then - should not throw + os.disableGMSMissingPrompt = true + os.disableGMSMissingPrompt = false + } + } + } + + context("property consistency tests") { + test("all properties maintain state correctly") { + // Given + val os = OneSignalImpV2() + + // When - set all properties to true + os.consentRequired = true + os.consentGiven = true + os.disableGMSMissingPrompt = true + + // Then - all should be true + os.consentRequired shouldBe true + os.consentGiven shouldBe true + os.disableGMSMissingPrompt shouldBe true + + // When - set all properties to false + os.consentRequired = false + os.consentGiven = false + os.disableGMSMissingPrompt = false + + // Then - all should be false + os.consentRequired shouldBe false + os.consentGiven shouldBe false + os.disableGMSMissingPrompt shouldBe false + } + + test("properties are independent of each other") { + // Given + val os = OneSignalImpV2() + + // When - set only consentRequired to true + os.consentRequired = true + + // Then - only consentRequired should be true + os.consentRequired shouldBe true + os.consentGiven shouldBe false + os.disableGMSMissingPrompt shouldBe false + + // When - set only consentGiven to true + os.consentRequired = false + os.consentGiven = true + + // Then - only consentGiven should be true + os.consentRequired shouldBe false + os.consentGiven shouldBe true + os.disableGMSMissingPrompt shouldBe false + + // When - set only disableGMSMissingPrompt to true + os.consentGiven = false + os.disableGMSMissingPrompt = true + + // Then - only disableGMSMissingPrompt should be true + os.consentRequired shouldBe false + os.consentGiven shouldBe false + os.disableGMSMissingPrompt shouldBe true + } + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/AppIdHelperV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/AppIdHelperV2Tests.kt new file mode 100644 index 0000000000..f54e6148a4 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/AppIdHelperV2Tests.kt @@ -0,0 +1,315 @@ +package com.onesignal.user.internal + +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.preferences.IPreferencesService +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys +import com.onesignal.core.internal.preferences.PreferenceStores +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify + +/** + * Unit tests for the resolveAppId function in AppIdResolution.kt + * + * These tests focus on the pure business logic of App ID resolution, + * complementing the integration tests in SDKInitTests.kt which test + * end-to-end SDK initialization behavior. + */ +class AppIdHelperV2Tests : FunSpec({ + // Test constants - using consistent naming with SDKInitTests + val testAppId = "appId" + val differentAppId = "different-app-id" + val legacyAppId = "legacy-app-id" + + beforeEach { + Logging.logLevel = LogLevel.NONE + } + + test("resolveAppId with new appId and no existing appId forces user creation") { + // Given - fresh config model with no appId property + val configModel = ConfigModel() + // Don't set any appId - simulates fresh install + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(testAppId, configModel, mockPreferencesService) + + // Then + result.appId shouldBe testAppId + result.forceCreateUser shouldBe true + result.failed shouldBe false + + // Should not check legacy preferences when appId is provided + verify(exactly = 0) { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } + } + + test("resolveAppId with same appId as existing does not force user creation") { + // Given - config model with existing appId + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(differentAppId, configModel, mockPreferencesService) + + // Then + result.appId shouldBe differentAppId + result.forceCreateUser shouldBe false + result.failed shouldBe false + } + + test("resolveAppId with different appId than existing forces user creation") { + // Given - config model with different existing appId + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(testAppId, configModel, mockPreferencesService) + + // Then + result.appId shouldBe testAppId + result.forceCreateUser shouldBe true + result.failed shouldBe false + } + + test("resolveAppId with null appId and existing appId in config returns existing") { + // Given - config model with existing appId + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(null, configModel, mockPreferencesService) + + // Then + result.appId shouldBe differentAppId // should return the existing appId from configModel + result.forceCreateUser shouldBe false + result.failed shouldBe false + + // Should not check legacy preferences when config already has appId + verify(exactly = 0) { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } + } + + test("resolveAppId with null appId and no existing appId finds legacy appId") { + // Given - fresh config model with no appId property + val configModel = ConfigModel() + + val mockPreferencesService = mockk() + every { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } returns legacyAppId + + // When + val result = resolveAppIdV2(null, configModel, mockPreferencesService) + + // Then + result.appId shouldBe legacyAppId + result.forceCreateUser shouldBe true // Legacy appId found forces user creation + result.failed shouldBe false + + // Should check legacy preferences + verify(exactly = 1) { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } + } + + test("resolveAppId with null appId and no existing appId and no legacy appId fails") { + // Given - fresh config model with no appId property and no legacy appId + val configModel = ConfigModel() + + val mockPreferencesService = mockk() + every { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } returns null + + // When + val result = resolveAppIdV2(null, configModel, mockPreferencesService) + + // Then + result.appId shouldBe null + result.forceCreateUser shouldBe false + result.failed shouldBe true + + // Should check legacy preferences + verify(exactly = 1) { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } + } + + test("AppIdResolution data class has correct properties") { + // Given + val appIdResolution = + AppIdResolution( + appId = "test-app-id", + forceCreateUser = true, + failed = false, + ) + + // Then + appIdResolution.appId shouldBe "test-app-id" + appIdResolution.forceCreateUser shouldBe true + appIdResolution.failed shouldBe false + } + + test("AppIdResolution handles null appId correctly") { + // Given + val appIdResolution = + AppIdResolution( + appId = null, + forceCreateUser = false, + failed = true, + ) + + // Then + appIdResolution.appId shouldBe null + appIdResolution.forceCreateUser shouldBe false + appIdResolution.failed shouldBe true + } + + test("configModel hasProperty check works correctly with appId set") { + // Given - config model with appId explicitly set + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(testAppId, configModel, mockPreferencesService) + + // Then - should detect property exists and force user creation due to different appId + result.appId shouldBe testAppId + result.forceCreateUser shouldBe true + result.failed shouldBe false + } + + test("empty appId falls back to the legacy appId") { + val configModel = ConfigModel() + + val mockPreferencesService = mockk() + every { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } returns legacyAppId + + val result = resolveAppIdV2("", configModel, mockPreferencesService) + + result.appId shouldBe legacyAppId + result.forceCreateUser shouldBe true + result.failed shouldBe false + } + + test("empty appId falls back to the cached appId without resetting the user") { + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + val result = resolveAppIdV2("", configModel, mockPreferencesService) + + result.appId shouldBe differentAppId + result.forceCreateUser shouldBe false + result.failed shouldBe false + } + + test("appId with a null byte falls back to the cached appId") { + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + val result = resolveAppIdV2("app\u0000id", configModel, mockPreferencesService) + + result.appId shouldBe differentAppId + result.forceCreateUser shouldBe false + result.failed shouldBe false + } + + test("empty appId with nothing cached fails resolution so init is skipped") { + val configModel = ConfigModel() + + val mockPreferencesService = mockk() + every { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } returns null + + val result = resolveAppIdV2("", configModel, mockPreferencesService) + + result.appId shouldBe null + result.failed shouldBe true + } + + test("resolveAppId with existing appId property but same value") { + // Given - config model with the same appId already set + val configModel = ConfigModel() + configModel.appId = differentAppId + + val mockPreferencesService = mockk(relaxed = true) + + // When + val result = resolveAppIdV2(differentAppId, configModel, mockPreferencesService) + + // Then - should not force user creation when appId is unchanged + result.appId shouldBe differentAppId + result.forceCreateUser shouldBe false + result.failed shouldBe false + } + + test("legacy appId fallback when config model exists but has no appId property") { + // Given - config model that exists but doesn't have appId set + val configModel = ConfigModel() + // Don't set appId to simulate hasProperty returning false + + val mockPreferencesService = mockk() + every { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } returns legacyAppId + + // When + val result = resolveAppIdV2(null, configModel, mockPreferencesService) + + // Then + result.appId shouldBe legacyAppId + result.forceCreateUser shouldBe true + result.failed shouldBe false + + verify(exactly = 1) { + mockPreferencesService.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) + } + } + + test("an invalid cached appId is not used") { + for (cached in listOf("", "app\u0000id")) { + val configModel = ConfigModel().apply { appId = cached } + val prefs = mockk() + every { prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) } returns cached + + val result = resolveAppIdV2("", configModel, prefs) + + result.failed shouldBe true + result.appId shouldBe null + } + } + + test("an invalid cached config appId falls through to a valid legacy appId") { + val configModel = ConfigModel().apply { appId = "" } + val prefs = mockk() + every { prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_LEGACY_APP_ID) } returns legacyAppId + + val result = resolveAppIdV2(null, configModel, prefs) + + result.appId shouldBe legacyAppId + result.failed shouldBe false + } +}) From 8e8fc053c35bbd12548407fde8559084c61b41c9 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 20 Oct 2026 14:30:12 -0500 Subject: [PATCH 2/6] fix: [SDK-5538] guard waits on the init thread and report calls dropped before init A wait for init from the thread running init (for example a listener replayed during init that calls back into OneSignal) now returns not-ready instead of waiting on itself; a failed attempt finishing late never clears a retry's claim. A call on an interrupted thread no longer throws InterruptedException: only an in-progress wait enters runBlocking, and an interrupt there returns not-ready and keeps the flag. Every init attempt now ends: a LinkageError, or a block that returns without completing, marks it FAILED instead of leaving every sync accessor blocked. Each ignored call logs ERROR once per attempt (with the failure trace once) and DEBUG after that, so a polling getter cannot flood remote logging. Calls made before any attempt are summarized in one ERROR when the attempt ends, success or failure, since remote logging only starts inside init; queued registrations are not counted. Documents that permission and canRequestPermission read false before init, which members each stand-in queues, and that a null appId with nothing cached warns while an invalid one errors. Trims oversized comments and makes the suspend login tests assert the switched user instead of passing on a timeout. Co-authored-by: Cursor --- .../src/main/java/com/onesignal/IOneSignal.kt | 15 +- .../src/main/java/com/onesignal/OneSignal.kt | 28 ++- .../java/com/onesignal/internal/InitGate.kt | 131 +++++++++--- .../java/com/onesignal/internal/InitState.kt | 4 +- .../com/onesignal/internal/OneSignalImpV2.kt | 16 +- .../notifications/INotificationsManager.kt | 6 +- .../application/SDKInitSuspendV2Tests.kt | 125 +++--------- .../internal/application/SDKInitV2Tests.kt | 165 +++++++-------- .../com/onesignal/internal/InitGateTests.kt | 191 ++++++++++++++++++ 9 files changed, 422 insertions(+), 259 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt index a7c86f2109..ef0ee8b3bc 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt @@ -20,11 +20,8 @@ interface IOneSignal { val isInitialized: Boolean /** - * The user manager for accessing user-scoped - * management. - * - * Before init succeeds this is a stand-in: listener and setting registrations are replayed once - * init succeeds, other calls are logged and ignored. + * The user manager. Waits while init is running. Before init succeeds this is a stand-in: + * observers are attached once init succeeds, other calls are logged and ignored. */ val user: IUserManager @@ -82,8 +79,8 @@ interface IOneSignal { * Initialize the OneSignal SDK. This should be called during startup of the application. * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached - * app ID is used; if none is cached, initialization is skipped and an error is logged. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. + * If none is cached, init fails (a warning for null, an error for invalid). * * @return true once initialization has started (or is already running or done), false for a * null [context] or an immediate startup failure. Check [isInitialized] for the outcome. @@ -170,8 +167,8 @@ interface IOneSignal { * Initialize the OneSignal SDK, suspend until initialization is completed * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached - * app ID is used; if none is cached, initialization is skipped and an error is logged. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. + * If none is cached, init fails (a warning for null, an error for invalid). * * @return true if the SDK could be successfully initialized, false otherwise. */ diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt index fb885b8add..295b658e15 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt @@ -37,9 +37,8 @@ object OneSignal { get() = oneSignal.sdkVersion /** - * The user manager for accessing user-scoped management, with a device-scoped user until (or - * if) [login] is called. Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are - * replayed once init succeeds, other calls are logged and ignored. + * The user manager, with a device-scoped user until (or if) [login] is called. Waits while init is running. + * Before [initWithContext] succeeds this is a stand-in: observers are attached once init succeeds, other calls are logged and ignored. */ @JvmStatic val User: IUserManager @@ -54,27 +53,24 @@ object OneSignal { get() = oneSignal.session /** - * The notification manager for accessing device-scoped notification management. - * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are - * replayed once init succeeds, other calls are logged and ignored. + * The notification manager. Waits while init is running. Before [initWithContext] succeeds this is a stand-in: + * listeners and permission observers are attached once init succeeds, other calls are logged and ignored. */ @JvmStatic val Notifications: INotificationsManager get() = oneSignal.notifications /** - * The location manager for accessing device-scoped location management. - * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are - * replayed once init succeeds, other calls are logged and ignored. + * The location manager. Waits while init is running. Before [initWithContext] succeeds this is a stand-in: + * a value set on `isShared` is applied once init succeeds, other calls are logged and ignored. */ @JvmStatic val Location: ILocationManager get() = oneSignal.location /** - * The In App Messaging manager for accessing device-scoped IAP management. - * Before [initWithContext] succeeds this is a stand-in: listener and setting registrations are - * replayed once init succeeds, other calls are logged and ignored. + * The In App Messaging manager. Waits while init is running. Before [initWithContext] succeeds this is a stand-in: + * listeners and `paused` are applied once init succeeds, other calls are logged and ignored. */ @JvmStatic val InAppMessages: IInAppMessagesManager @@ -128,8 +124,8 @@ object OneSignal { * Initialize the OneSignal SDK. This should be called during startup of the application. * * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached - * app ID is used; if none is cached, initialization is skipped and an error is logged. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. + * If none is cached, init fails (a warning for null, an error for invalid). Check [isInitialized] for the outcome. */ @JvmStatic fun initWithContext( @@ -145,8 +141,8 @@ object OneSignal { * Uses Dispatchers.IO internally to prevent ANRs and optimize for I/O operations. * * @param context Application context is recommended for SDK operations - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached - * app ID is used; if none is cached, initialization is skipped and an error is logged. + * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. + * If none is cached, init fails (a warning for null, an error for invalid). * @return Boolean indicating if initialization was successful. */ @JvmStatic diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt index ae097e9b66..043b9632ef 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitGate.kt @@ -3,6 +3,7 @@ package com.onesignal.internal import com.onesignal.debug.internal.logging.Logging import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.runBlocking +import java.util.concurrent.atomic.AtomicReference /** * Init state for [OneSignalImpV2] and the waits on it. Never throws: callers that need the SDK @@ -27,14 +28,54 @@ internal class InitGate { @Volatile private var failureTraceLogged = false + // Guarded by [lock]. Each ignored call logs ERROR once per attempt, so a polling getter can't flood + // remote logging; repeats go to DEBUG. + private val loggedTargets = HashSet() + + // Guarded by [lock]; bumped by [tryStart]. + private var attempt = 0 + + // A wait on this thread while IN_PROGRESS would wait on itself (e.g. a listener replayed during init). + private val initThread = AtomicReference(null) + + private val earlyCalls = EarlyCalls() + val isSuccess: Boolean get() = state == InitState.SUCCESS + private val isOnInitThread: Boolean get() = state == InitState.IN_PROGRESS && initThread.get() === Thread.currentThread() + + // Every attempt must end: one left IN_PROGRESS blocks every sync accessor forever. + fun runInit(block: () -> Boolean): Boolean { + val current = Thread.currentThread() + val owned = synchronized(lock) { attempt } + initThread.set(current) + try { + return block() + } catch (e: LinkageError) { + Logging.error("OneSignal: initWithContext hit a LinkageError; marking init FAILED", e) + addFailureCause(e) + return false + } finally { + failIfUnfinished(owned) + // A retry woken by this attempt's FAILED may already have claimed the slot. + initThread.compareAndSet(current, null) + } + } + + // Keyed by attempt, not thread: a retry may start before this attempt's thread leaves [runInit]. + private fun failIfUnfinished(owned: Int) { + val abandoned = synchronized(lock) { state == InitState.IN_PROGRESS && attempt == owned } + if (abandoned) complete(InitState.FAILED) + } + /** Starts a new attempt and returns true, or returns false if one is running or has succeeded. */ fun tryStart(): Boolean = synchronized(lock) { if (state.isSDKAccessible()) return false failure = IllegalStateException("OneSignal initWithContext failed.") failureTraceLogged = false + loggedTargets.clear() + attempt++ state = InitState.IN_PROGRESS completion = CompletableDeferred() true @@ -53,17 +94,16 @@ internal class InitGate { state = terminal completion.complete(Unit) } + // Also on FAILED: remote logging is usually up by then, and init may never succeed. + earlyCalls.logAndClear() } /** Waits out in-progress attempts, including a retry that starts while waiting. True only on SUCCESS. */ suspend fun await(operation: SdkOperation? = null): Boolean { while (true) { val (observed, signal) = synchronized(lock) { state to completion } - when (observed) { - InitState.SUCCESS -> return true - InitState.NOT_STARTED, InitState.FAILED -> return false - InitState.IN_PROGRESS -> awaitAttempt(signal, operation) - } + if (observed != InitState.IN_PROGRESS || isOnInitThread) return observed == InitState.SUCCESS + awaitAttempt(signal, operation) } } @@ -74,10 +114,21 @@ internal class InitGate { log: Boolean = true, ): Boolean { if (isSuccess) return true - // Dispatcher-less: resumes on the caller's own event loop, never on a busy IO worker. - return runBlocking { awaitReady(operation, member, log) } + val ready = if (state == InitState.IN_PROGRESS && !isOnInitThread) awaitBlocking(operation) else isSuccess + if (!ready && log) logNotReady(target(operation, member)) + return ready } + // runBlocking throws on an interrupted thread, so it is only entered when there is something to wait for. + private fun awaitBlocking(operation: SdkOperation): Boolean = + try { + // Dispatcher-less: resumes on the caller's own event loop, never on a busy IO worker. + runBlocking { await(operation) } + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + false + } + suspend fun awaitReady( operation: SdkOperation, member: String? = null, @@ -89,10 +140,11 @@ internal class InitGate { } fun notReadyMessage(target: String): String = - if (state == InitState.FAILED) { - "'$target' ignored: initWithContext failed." - } else { - "'$target' ignored: call initWithContext with a valid appId first." + when { + state == InitState.FAILED -> "'$target' ignored: initWithContext failed." + isOnInitThread -> "'$target' ignored: called from inside initWithContext." + state == InitState.IN_PROGRESS -> "'$target' ignored: the thread was interrupted while waiting for initWithContext." + else -> "'$target' ignored: call initWithContext with a valid appId first." } fun logQueued( @@ -104,26 +156,51 @@ internal class InitGate { private fun logNotReady(target: String) { val message = notReadyMessage(target) - val trace = failure.takeIf { state == InitState.FAILED && !failureTraceLogged } - if (trace == null) { - Logging.error(message) - return + var first = false + val trace = + synchronized(lock) { + if (state == InitState.NOT_STARTED) earlyCalls.record(target) + first = loggedTargets.add(target) + failure.takeIf { first && state == InitState.FAILED && !failureTraceLogged }?.also { failureTraceLogged = true } + } + when { + !first -> Logging.debug(message) + trace == null -> Logging.error(message) + else -> Logging.error(message, trace) } - failureTraceLogged = true - Logging.error(message, trace) } +} - private suspend fun awaitAttempt( - signal: CompletableDeferred, - operation: SdkOperation?, - ) { - Logging.debug("Waiting for init to complete...") - val startTime = System.currentTimeMillis() - signal.await() - val elapsed = System.currentTimeMillis() - startTime - val before = operation?.let { " before '$it'" } ?: "" - Logging.debug("OneSignalImpV2 initialization completed$before (took ${elapsed}ms)") +// Distinct calls dropped before any init attempt, logged once when an attempt ends, since remote logging +// starts only inside init. Later drops log themselves; queued registrations are not counted. +private class EarlyCalls { + private val calls = LinkedHashSet() + + fun record(target: String) { + synchronized(calls) { if (calls.size < MAX_EARLY_CALLS) calls.add(target) } } + + fun logAndClear() { + val logged = synchronized(calls) { calls.toList().also { calls.clear() } } + if (logged.isEmpty()) return + Logging.error("OneSignal calls made before initWithContext were ignored: ${logged.joinToString()}. Call initWithContext first.") + } + + private companion object { + const val MAX_EARLY_CALLS = 20 + } +} + +private suspend fun awaitAttempt( + signal: CompletableDeferred, + operation: SdkOperation?, +) { + Logging.debug("Waiting for init to complete...") + val startTime = System.currentTimeMillis() + signal.await() + val elapsed = System.currentTimeMillis() - startTime + val before = operation?.let { " before '$it'" } ?: "" + Logging.debug("OneSignalImpV2 initialization completed$before (took ${elapsed}ms)") } private fun target( diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt index 0481d6c4be..c76bc3aeb9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/InitState.kt @@ -14,8 +14,8 @@ internal enum class InitState { NOT_STARTED, /** - * SDK initialization is currently in progress. - * Calls that require initialization wait (block or suspend) until this completes. + * In progress. Calls that need init wait for it, except on the thread running init (or an interrupted + * one), where they return a default instead. */ IN_PROGRESS, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt index af69ce9f87..d54b8e89dd 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt @@ -286,12 +286,7 @@ internal class OneSignalImpV2 : IOneSignal, ensureApplicationServiceStarted(context) } - /** - * One concise WARN line at init with the build/runtime facts most useful for - * release triage from a raw log capture: SDK version, the shared KMP module version, - * OS/API, device, host app + version, and the crash storage dir. - * Best-effort — never lets diagnostics interfere with init. - */ + /** One WARN line of build and runtime facts for triage from a raw log. Best effort: never fails init. */ @Suppress("TooGenericExceptionCaught", "SwallowedException") internal fun logStartupDiagnostics(context: Context) { try { @@ -381,10 +376,7 @@ internal class OneSignalImpV2 : IOneSignal, return false } - // Warm OneSignalDispatchers on a dedicated daemon thread so the first production caller - // of suspendifyOnIO / launchOnSerialIO doesn't pay the ThreadPoolExecutor + dispatcher + - // scope construction cost on the main thread (observed as 5-20s main-thread blocks at the - // first foreground/background lifecycle event). prewarm() is idempotent and fire-and-forget. + // Builds the dispatchers off the main thread, so the first suspendifyOnIO caller doesn't pay for it there. OneSignalDispatchers.prewarm() if (!gate.tryStart()) { @@ -407,7 +399,7 @@ internal class OneSignalImpV2 : IOneSignal, // Dispatch init asynchronously so this method never blocks the caller. Callers that // need to wait (accessors, login, logout) block on the gate. suspendifyOnIO { - internalInit(context, appId) + gate.runInit { internalInit(context, appId) } } return true } @@ -610,7 +602,7 @@ internal class OneSignalImpV2 : IOneSignal, try { return withContext(ioDispatcher) { started = true - internalInit(context, appId) + gate.runInit { internalInit(context, appId) } } } finally { // A caller cancelled before dispatch would otherwise leave this attempt IN_PROGRESS forever. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationsManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationsManager.kt index 6490752c3b..67c456d367 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationsManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/notifications/INotificationsManager.kt @@ -5,12 +5,14 @@ package com.onesignal.notifications */ interface INotificationsManager { /** - * Whether this app has push notification permission. + * Whether this app has push notification permission. Before `initWithContext` succeeds this is + * `false` because the permission is not yet known, not because it was denied. */ val permission: Boolean /** - * Whether this app can request push notification permission. + * Whether this app can request push notification permission. Before `initWithContext` succeeds + * this is `false` because it is not yet known. */ val canRequestPermission: Boolean diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt index 3bd9d64a38..21f371ecef 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitSuspendV2Tests.kt @@ -11,7 +11,6 @@ import com.onesignal.internal.OneSignalImpV2 import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withTimeout @RobolectricTest class SDKInitSuspendV2Tests : FunSpec({ @@ -84,137 +83,63 @@ class SDKInitSuspendV2Tests : FunSpec({ // ===== LOGIN TESTS ===== - test("login suspend method works after initWithContextSuspend") { - // Given + test("login after initWithContextSuspend switches to the external ID") { val context = getApplicationContext() val os = OneSignalImpV2() - val testExternalId = "testUser123" runBlocking { - // When - val initResult = os.initWithContextSuspend(context, "testAppId") - initResult shouldBe true - - // Login with timeout - demonstrates suspend method works correctly - try { - withTimeout(2000) { // 2 second timeout - os.login(testExternalId) - } - // If we get here, login completed successfully (unlikely in test env) - os.isInitialized shouldBe true - } catch (e: kotlinx.coroutines.TimeoutCancellationException) { - // Expected timeout due to operation queue processing in test environment - // This proves the suspend method is working correctly - os.isInitialized shouldBe true - println("Login suspend method works correctly - timed out as expected due to operation queue") - } + os.initWithContextSuspend(context, "testAppId") shouldBe true + os.login("testUser123") + os.user.externalId shouldBe "testUser123" } } - // Note: Tests for null appId removed since appId is now non-nullable - - test("login suspend method with JWT token") { - // Given + test("login with a JWT after initWithContextSuspend switches to the external ID") { val context = getApplicationContext() val os = OneSignalImpV2() - val testExternalId = "testUser789" - val jwtToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." runBlocking { - // When - val initResult = os.initWithContextSuspend(context, "testAppId") - initResult shouldBe true - - try { - withTimeout(2000) { // 2 second timeout - os.login(testExternalId, jwtToken) - } - os.isInitialized shouldBe true - } catch (e: kotlinx.coroutines.TimeoutCancellationException) { - // Expected timeout due to operation queue processing - os.isInitialized shouldBe true - println("Login with JWT suspend method works correctly - timed out as expected due to operation queue") - } + os.initWithContextSuspend(context, "testAppId") shouldBe true + os.login("testUser789", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...") + os.user.externalId shouldBe "testUser789" } } - // ===== LOGOUT TESTS ===== - - test("logout suspend method works after initWithContextSuspend") { - // Given + test("logout after login returns to a device-scoped user") { val context = getApplicationContext() val os = OneSignalImpV2() runBlocking { - // When - val initResult = os.initWithContextSuspend(context, "testAppId") - initResult shouldBe true - - // Logout with timeout - demonstrates suspend method works correctly - try { - withTimeout(2000) { // 2 second timeout - os.logout() - } - // If we get here, logout completed successfully (unlikely in test env) - os.isInitialized shouldBe true - } catch (e: kotlinx.coroutines.TimeoutCancellationException) { - // Expected timeout due to operation queue processing in test environment - // This proves the suspend method is working correctly - os.isInitialized shouldBe true - println("Logout suspend method works correctly - timed out as expected due to operation queue") - } + os.initWithContextSuspend(context, "testAppId") shouldBe true + os.login("user1") + os.logout() + os.user.externalId shouldBe "" } } - // ===== INTEGRATION TESTS ===== - - test("multiple login calls work correctly") { - // Given + test("the last of several logins wins") { val context = getApplicationContext() val os = OneSignalImpV2() runBlocking { - // When - val initResult = os.initWithContextSuspend(context, "testAppId") - initResult shouldBe true - - try { - withTimeout(3000) { // 3 second timeout for multiple operations - os.login("user1") - os.login("user2") - os.login("user3") - } - os.isInitialized shouldBe true - } catch (e: kotlinx.coroutines.TimeoutCancellationException) { - // Expected timeout due to operation queue processing - os.isInitialized shouldBe true - println("Multiple login calls suspend method works correctly - timed out as expected due to operation queue") - } + os.initWithContextSuspend(context, "testAppId") shouldBe true + os.login("user1") + os.login("user2") + os.login("user3") + os.user.externalId shouldBe "user3" } } - test("login and logout sequence works correctly") { - // Given + test("login, logout, login ends on the second user") { val context = getApplicationContext() val os = OneSignalImpV2() runBlocking { - // When - val initResult = os.initWithContextSuspend(context, "testAppId") - initResult shouldBe true - - try { - withTimeout(3000) { // 3 second timeout for sequence - os.login("user1") - os.logout() - os.login("user2") - } - os.isInitialized shouldBe true - } catch (e: kotlinx.coroutines.TimeoutCancellationException) { - // Expected timeout due to operation queue processing - os.isInitialized shouldBe true - println("Login/logout sequence suspend methods work correctly - timed out as expected due to operation queue") - } + os.initWithContextSuspend(context, "testAppId") shouldBe true + os.login("user1") + os.logout() + os.login("user2") + os.user.externalId shouldBe "user2" } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt index 0003062def..47142b83e0 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/SDKInitV2Tests.kt @@ -42,12 +42,7 @@ import java.util.concurrent.atomic.AtomicReference @RobolectricTest class SDKInitV2Tests : FunSpec({ - /** - * Helper function to wait for OneSignal initialization to complete. - * @param oneSignalImp The OneSignalImpV2 instance to wait for - * @param maxAttempts Maximum number of attempts (default: 100) - * @param sleepMs Sleep duration between attempts in milliseconds (default: 20) - */ + /** Polls until [oneSignalImp] is initialized, up to [maxAttempts] sleeps of [sleepMs]. */ fun waitForInitialization(oneSignalImp: OneSignalImpV2, maxAttempts: Int = 100, sleepMs: Long = 20) { var attempts = 0 while (!oneSignalImp.isInitialized && attempts < maxAttempts) { @@ -208,6 +203,40 @@ class SDKInitV2Tests : FunSpec({ os.isInitialized shouldBe true } + test("registrations queued before a failed init are kept and applied when a retry succeeds") { + val context = IsolatedPrefsContext(getApplicationContext()) + val os = OneSignalImpV2() + os.user.addObserver(mockk(relaxed = true)) + + os.initWithContext(context, "") + os.user + os.isInitialized shouldBe false + + os.initWithContext(context, "appId") + waitForInitialization(os) + (os.user as UserManager).changeHandlersNotifier.hasSubscribers shouldBe true + } + + test("calls dropped before init are summarized in one error once init succeeds; queued registrations are not") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val os = OneSignalImpV2() + os.user.addTag("key", "value") + os.user.addObserver(mockk(relaxed = true)) + os.initWithContext(IsolatedPrefsContext(getApplicationContext()), "appId") + waitForInitialization(os) + } finally { + Logging.removeListener(listener) + } + + events.count { + it.level == LogLevel.ERROR && + it.entry.contains("calls made before initWithContext were ignored: User.addTag. Call initWithContext first.") + } shouldBe 1 + } + test("pre-init registrations are replayed before init reports success") { val os = OneSignalImpV2() var stateAtReplay: InitState? = null @@ -263,7 +292,7 @@ class SDKInitV2Tests : FunSpec({ events.count { it.level == LogLevel.WARN && it.entry.contains("using the cached app ID") } shouldBe 1 } - test("a call before init logs one error naming the call, with the failure trace once per attempt") { + test("an ignored call logs one error per attempt, with the failure trace, and repeats at debug") { val events = CopyOnWriteArrayList() val listener = ILogListener { events += it } Logging.addListener(listener) @@ -279,9 +308,9 @@ class SDKInitV2Tests : FunSpec({ os.user.addTag("key", "value") os.user.addTag("key", "value") - val errors = events.filter { it.level == LogLevel.ERROR && it.entry.contains("'User.addTag'") } - errors.size shouldBe 2 - errors.count { it.entry.contains("OneSignal initWithContext failed.") } shouldBe 1 + val logged = events.filter { it.entry.contains("'User.addTag'") } + logged.map { it.level } shouldBe listOf(LogLevel.ERROR, LogLevel.DEBUG) + logged[0].entry.contains("OneSignal initWithContext failed.") shouldBe true } finally { Logging.removeListener(listener) } @@ -692,12 +721,8 @@ class SDKInitV2Tests : FunSpec({ } test("initWithContext recovers when ensureApplicationServiceStarted throws synchronously") { - // Given: a context whose applicationContext is NOT an Application instance. - // ApplicationService.start does `context.applicationContext as Application`, which - // will throw ClassCastException — simulates restricted hosts (Robolectric custom - // application factories, instrumentation context wrappers, multi-process content - // provider init order). Without recovery, initState would be left at IN_PROGRESS - // forever and accessors would deadlock on suspendCompletion.await(). + // applicationContext is not an Application, so ApplicationService.start throws ClassCastException, + // as in some restricted hosts. Init must still reach a terminal state. val baseContext = getApplicationContext() val brokenContext = object : ContextWrapper(baseContext) { @@ -718,17 +743,8 @@ class SDKInitV2Tests : FunSpec({ waitForInitialization(os) } - test("accessor returns once init is SUCCESS even when the IO dispatcher is saturated (OneSignal-Flutter-SDK#1163)") { - // Regression test for the Flutter main-thread ANR. getServiceWithFeatureGate used to - // ALWAYS route through waitAndReturn -> waitForInit -> runBlocking(OneSignalDispatchers.IO), - // even after init had already reached SUCCESS. On Flutter's main-thread lifecycleInit that - // parks the UI thread on a cold-start-contended IO pool (OneSignal-Flutter-SDK#1163). The - // fix adds a lock-free SUCCESS fast-path, centralized in waitForInit(). - // - // We reproduce the contention faithfully: saturate OneSignalDispatchers.IO so that the - // pre-fix runBlocking(OneSignalDispatchers.IO) would park indefinitely behind the busy - // workers, then assert the accessor still returns. With the fix it answers synchronously; - // without it the accessor thread would still be parked when we check. + test("accessor returns once init is SUCCESS even when the IO dispatcher is saturated") { + // At SUCCESS an accessor must answer without an IO worker; a busy pool must not park the main thread. val context = getApplicationContext() val os = OneSignalImpV2() @@ -740,18 +756,12 @@ class SDKInitV2Tests : FunSpec({ val result = arrayOfNulls(1) runOnThreadAndWait { result[0] = os.user } - // Then: the fast-path answered synchronously and the thread finished. The pre-fix - // runBlocking(OneSignalDispatchers.IO) path would still be parked here. result[0] shouldNotBe null } } - test("login returns once init is SUCCESS even when the IO dispatcher is saturated (OneSignal-Flutter-SDK#1163)") { - // Companion regression test proving the SUCCESS fast-path is centralized in waitForInit(), - // so it covers the operation paths (login / logout / updateUserJwt / JWT listeners), not - // just the service accessors. login() used to route through - // waitForInit -> runBlocking(OneSignalDispatchers.IO) even after init reached SUCCESS, so a - // saturated IO pool would park the caller indefinitely. + test("login returns once init is SUCCESS even when the IO dispatcher is saturated") { + // Same as above for the operation paths (login, logout, updateUserJwt, JWT listeners). val context = getApplicationContext() val os = OneSignalImpV2() @@ -763,9 +773,6 @@ class SDKInitV2Tests : FunSpec({ // short-circuit (the subsequent enqueue is fire-and-forget via suspendifyOnIO and does // not block the caller). runOnThreadAndWait { os.login("externalId") } - - // Then: the fast-path answered synchronously and the call returned. The pre-fix - // runBlocking(OneSignalDispatchers.IO) path would still be parked here. } } @@ -880,17 +887,8 @@ class SDKInitV2Tests : FunSpec({ oneSignalImp.isInitialized shouldBe false } - // Regression test for the SessionService NPE during in-flight background init. - // - // Background: when init is in flight, internalInit() has not yet called bootstrapServices(), - // which means IBootstrapService implementations like SessionService have null model fields. - // SyncJobService re-enters via initWithContextSuspend(context, null) and then immediately - // calls runBackgroundServices(); if initWithContextSuspend returns true while init is still - // in progress (the old behavior), the loop hits SessionService.endSession() → NPE. - // - // The fix: initWithContextSuspend must suspend until init is *fully* completed, not just - // kicked off. This test verifies that contract by checking that os.isInitialized is true - // at the moment the re-entrant suspend call returns. + // A re-entrant caller (e.g. SyncJobService) uses services right after this returns, so it must wait + // for init to finish, not just start; SessionService would otherwise hit null models. test("initWithContextSuspend with in-flight init waits for completion before returning") { val context = getApplicationContext() val trigger = CountDownLatch(1) @@ -920,14 +918,7 @@ class SDKInitV2Tests : FunSpec({ os.isInitialized shouldBe false firstInit.isCompleted shouldBe false - // Second caller: under the OLD behavior the synchronized(initLock) block sees - // state == IN_PROGRESS and returns `true` *while* internalInit() (running on the - // first caller) has not yet reached `initState = InitState.SUCCESS`. Under the - // fix, this call must suspend until the first caller fully completes init -- so - // that os.isInitialized is guaranteed `true` at the moment we return. - // - // We capture isInitialized at exactly the moment initWithContextSuspend returns - // for the second caller. Old code: false (bug). New code: true (fix). + // The second caller must see isInitialized == true at the moment its call returns. val secondInit = async(Dispatchers.IO) { val result = os.initWithContextSuspend(context, "appId") val initializedAtReturn = os.isInitialized @@ -953,16 +944,7 @@ class SDKInitV2Tests : FunSpec({ } } - // Regression test for review-flagged Defect 1 (stale latch on retry-after-FAILED). - // - // Background: `suspendCompletion` was a single-shot `val CompletableDeferred`. Once - // any init terminates (even FAILED), the deferred stays permanently complete -- so a - // re-entrant suspend caller arriving DURING a subsequent retry would `await()` on the - // already-completed deferred, return instantly, and read transient state (likely - // IN_PROGRESS -> false), silently dropping JobService work. - // - // The fix resets `suspendCompletion` whenever the synchronized(initLock) block flips state - // into IN_PROGRESS, and await sites local-capture the deferred under the lock. + // A caller arriving during a retry after FAILED must wait on the retry's signal, not the finished one. test("initWithContextSuspend resets latch on retry-after-FAILED") { val context = getApplicationContext() val os = OneSignalImpV2() @@ -975,7 +957,6 @@ class SDKInitV2Tests : FunSpec({ val firstInit = os.initWithContextSuspend(context, "appId") firstInit shouldBe false os.isInitialized shouldBe false - // At this point the (pre-fix) single-shot `suspendCompletion` is permanently complete. unmockkObject(AndroidUtils) // 2. Stall a fresh retry via BlockingPrefsContext so initState sits at IN_PROGRESS. @@ -996,9 +977,7 @@ class SDKInitV2Tests : FunSpec({ os.isInitialized shouldBe false secondInit.isCompleted shouldBe false - // 3. Re-entrant caller. With Defect 1, it would wake immediately on the - // FAILED-generation latch (still permanently complete) and return based on - // transient state. With the fix, it waits on the *new* generation's latch. + // 3. Re-entrant caller: must wait on the retry's signal, not the finished FAILED one. val thirdInit = async(Dispatchers.IO) { val r = os.initWithContextSuspend(context, "appId") val initializedAtReturn = os.isInitialized @@ -1019,32 +998,16 @@ class SDKInitV2Tests : FunSpec({ } } - // Regression test for review-flagged Defect 2 (indefinite hang if internalInit throws). - // - // Background: `internalInit` had no try/catch wrapping its body. An unchecked throw from - // initEssentials/bootstrapServices/etc. would leave initState=IN_PROGRESS forever and - // `suspendCompletion` uncompleted -- causing every re-entrant suspend caller (e.g. - // SyncJobService) to hang on `await()` indefinitely, holding its budget slot until the - // OS killed the worker. - // - // The fix wraps internalInit's body in try/catch, ensuring a terminal state and - // `notifyInitComplete()` on any throw. + // A throw inside init must still end the attempt, or every waiter (e.g. SyncJobService) hangs. test("initWithContextSuspend reaches terminal state when internalInit throws") { val context = getApplicationContext() val os = OneSignalImpV2() - // Make the very first call inside internalInit throw (mirrors the user-locked test - // pattern, but with a throw instead of a `false` return). This is the cheapest way to - // simulate "any unchecked exception during bootstrap" without coupling to specific - // bootstrap internals. + // The first call inside internalInit, so no bootstrap internals are involved. mockkObject(AndroidUtils) every { AndroidUtils.isAndroidUserUnlocked(any()) } throws RuntimeException("simulated bootstrap failure") runBlocking { - // Without the fix: the throw propagates out of internalInit, escapes withContext, - // and either fails the test outright or leaves the SDK in a deadlocked - // IN_PROGRESS state with `suspendCompletion` never completed. - // With the fix: the catch block sets FAILED + notifyInitComplete + returns false. val firstResult = withTimeoutOrNull(5_000) { os.initWithContextSuspend(context, "appId") } @@ -1053,8 +1016,7 @@ class SDKInitV2Tests : FunSpec({ unmockkObject(AndroidUtils) - // State is FAILED so a retry is allowed and (per Defect 1 fix) gets a fresh latch. - // This also doubles as a smoke test that we didn't leak IN_PROGRESS. + // FAILED, so a retry is allowed and gets a fresh signal. val retry = withTimeoutOrNull(5_000) { os.initWithContextSuspend(context, "appId") } @@ -1062,6 +1024,27 @@ class SDKInitV2Tests : FunSpec({ os.isInitialized shouldBe true } } + + test("a LinkageError during init ends the attempt as FAILED instead of leaving it in progress") { + val context = getApplicationContext() + mockkObject(AndroidUtils) + try { + every { AndroidUtils.isAndroidUserUnlocked(any()) } throws NoClassDefFoundError("simulated missing class") + + val suspendOs = OneSignalImpV2() + runBlocking { withTimeoutOrNull(5_000) { suspendOs.initWithContextSuspend(context, "appId") } } shouldBe false + suspendOs.gate.state shouldBe InitState.FAILED + + val syncOs = OneSignalImpV2() + syncOs.initWithContext(context, "appId") shouldBe true + val deadline = System.currentTimeMillis() + 5_000 + while (syncOs.gate.state == InitState.IN_PROGRESS && System.currentTimeMillis() < deadline) Thread.sleep(10) + syncOs.gate.state shouldBe InitState.FAILED + syncOs.user.onesignalId shouldBe "" + } finally { + unmockkObject(AndroidUtils) + } + } }) /** Gives each test its own SharedPreferences files, so other tests' writes can't leak in. */ diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt index 387635295a..015656f83b 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/InitGateTests.kt @@ -1,9 +1,12 @@ package com.onesignal.internal +import com.onesignal.debug.ILogListener import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent import com.onesignal.debug.internal.logging.Logging import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe +import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicReference @@ -89,4 +92,192 @@ class InitGateTests : FunSpec({ gate.complete(InitState.FAILED) gate.notReadyMessage("login") shouldBe "'login' ignored: initWithContext failed." } + + test("a wait on the init thread while init is running returns false instead of waiting on itself") { + val gate = InitGate() + gate.tryStart() + val onInitThread = AtomicReference(null) + val otherThread = AtomicReference(null) + + val initThread = + Thread { + gate.runInit { + onInitThread.set(gate.isReady(SdkOperation.LOGIN)) + val finished = isReadyOnThread(gate, otherThread) + finished.await(200, TimeUnit.MILLISECONDS) shouldBe false + gate.complete(InitState.SUCCESS) + finished.await(2, TimeUnit.SECONDS) shouldBe true + true + } + }.apply { isDaemon = true } + initThread.start() + initThread.join(5_000) + + initThread.isAlive shouldBe false + onInitThread.get() shouldBe false + otherThread.get() shouldBe true + } + + test("a failed attempt finishing late does not clear the init thread a retry already claimed") { + val gate = InitGate() + val retryClaimed = CountDownLatch(1) + val firstDone = CountDownLatch(1) + val retryResult = AtomicReference(null) + val retry = + Thread { + gate.tryStart() + gate.runInit { + retryClaimed.countDown() + firstDone.await(2, TimeUnit.SECONDS) + retryResult.set(gate.isReady(SdkOperation.LOGIN)) + gate.complete(InitState.SUCCESS) + true + } + }.apply { isDaemon = true } + + gate.tryStart() + val first = + Thread { + gate.runInit { + gate.complete(InitState.FAILED) + retry.start() + retryClaimed.await(2, TimeUnit.SECONDS) + false + } + firstDone.countDown() + }.apply { isDaemon = true } + first.start() + first.join(2_000) + retry.join(2_000) + + retry.isAlive shouldBe false + retryResult.get() shouldBe false + gate.isSuccess shouldBe true + } + + test("calls before init are summarized in one error when init succeeds, and only once") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val gate = InitGate() + gate.isReady(SdkOperation.LOGIN) + gate.isReady(SdkOperation.LOGIN) + gate.logQueued(SdkOperation.NOTIFICATIONS, "addClickListener") + gate.isReady(SdkOperation.USER, "addTag") + events.clear() + + gate.tryStart() + gate.complete(InitState.SUCCESS) + gate.complete(InitState.SUCCESS) + + val summaries = events.filter { it.level == LogLevel.ERROR }.map { it.entry.substringAfter("] ") } + summaries shouldBe + listOf("OneSignal calls made before initWithContext were ignored: login, User.addTag. Call initWithContext first.") + } finally { + Logging.removeListener(listener) + } + } + + test("no summary is logged when nothing was called before init") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val gate = InitGate() + gate.tryStart() + gate.complete(InitState.SUCCESS) + events.filter { it.level == LogLevel.ERROR } shouldBe emptyList() + } finally { + Logging.removeListener(listener) + } + } + + test("calls before init are also summarized when the attempt fails") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val gate = InitGate() + gate.isReady(SdkOperation.USER, "addTag") + events.clear() + + gate.tryStart() + gate.complete(InitState.FAILED) + + events.filter { it.level == LogLevel.ERROR }.map { it.entry.substringAfter("] ") } shouldBe + listOf("OneSignal calls made before initWithContext were ignored: User.addTag. Call initWithContext first.") + } finally { + Logging.removeListener(listener) + } + } + + test("a repeated ignored call logs ERROR once per attempt and DEBUG after that") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val gate = InitGate() + repeat(3) { gate.isReady(SdkOperation.USER, "onesignalId") } + + val levels = events.filter { it.entry.contains("'User.onesignalId' ignored") }.map { it.level } + levels shouldBe listOf(LogLevel.ERROR, LogLevel.DEBUG, LogLevel.DEBUG) + } finally { + Logging.removeListener(listener) + } + } + + test("a new attempt logs a repeated ignored call at ERROR again") { + val events = CopyOnWriteArrayList() + val listener = ILogListener { events += it } + Logging.addListener(listener) + try { + val gate = InitGate() + gate.isReady(SdkOperation.USER, "onesignalId") + gate.tryStart() + val initThread = + Thread { + gate.runInit { + gate.isReady(SdkOperation.USER, "onesignalId") + gate.complete(InitState.SUCCESS) + true + } + }.apply { isDaemon = true } + initThread.start() + initThread.join(2_000) + + val levels = events.filter { it.entry.contains("'User.onesignalId' ignored") }.map { it.level } + levels shouldBe listOf(LogLevel.ERROR, LogLevel.ERROR) + } finally { + Logging.removeListener(listener) + } + } + + test("isReady on an interrupted thread returns false without throwing and keeps the interrupt") { + for (inProgress in listOf(false, true)) { + val gate = InitGate() + if (inProgress) gate.tryStart() + val result = AtomicReference(null) + val stillInterrupted = AtomicReference(null) + val caller = + Thread { + Thread.currentThread().interrupt() + result.set(gate.isReady(SdkOperation.USER, "addTag")) + stillInterrupted.set(Thread.currentThread().isInterrupted) + }.apply { isDaemon = true } + caller.start() + caller.join(2_000) + + caller.isAlive shouldBe false + result.get() shouldBe false + stillInterrupted.get() shouldBe true + } + } + + test("an attempt whose block returns without completing is marked FAILED") { + val gate = InitGate() + gate.tryStart() + gate.runInit { false } shouldBe false + gate.state shouldBe InitState.FAILED + } }) From 821d55df2304d92b717401e294ebad0a1d747dc3 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 2 Nov 2026 11:28:23 -0600 Subject: [PATCH 3/6] fix: [SDK-5538] keep the legacy null-argument crash and document required arguments With the kill switch on, OneSignalImp again throws the NullPointerException a Java null used to hit in the non-null facade: a null context on initWithContext or initWithContextSuspend, and a null appId on initWithContext. Same type and message as before, so legacy crash grouping is unchanged. KDocs on init, login and updateUserJwt now state that the app ID, external ID and token are required, and what happens when they are missing (ignored and logged, or INVALID_ARGUMENT for the composite login), instead of describing null as a supported value. Co-authored-by: Cursor --- .../src/main/java/com/onesignal/IOneSignal.kt | 25 ++++++++--------- .../src/main/java/com/onesignal/OneSignal.kt | 27 ++++++++++--------- .../com/onesignal/internal/OneSignalImp.kt | 11 ++++++-- .../onesignal/internal/OneSignalImpTests.kt | 15 +++++++++++ 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt index ef0ee8b3bc..3bb1ed8eef 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt @@ -78,9 +78,9 @@ interface IOneSignal { /** * Initialize the OneSignal SDK. This should be called during startup of the application. * - * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. - * If none is cached, init fails (a warning for null, an error for invalid). + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. + * @param appId Your OneSignal app ID. Always pass it: a null or invalid value falls back to the app ID + * cached by an earlier launch, and with none cached init fails and logs. * * @return true once initialization has started (or is already running or done), false for a * null [context] or an immediate startup failure. Check [isInitialized] for the outcome. @@ -113,7 +113,7 @@ interface IOneSignal { * subscriptions from the current user (if there is one) to the newly logged in user. This is * because both Push and IAM are owned by the device. * - * @param externalId The external ID of the user that is to be logged in. + * @param externalId The external ID of the user to log in. Must be non-empty: an empty value is ignored and logged. * @param jwtBearerToken The optional JWT bearer token generated by your backend to establish * trust for the login operation. Required when identity verification has been enabled. See * [Identity Verification | OneSignal](https://documentation.onesignal.com/docs/identity-verification) @@ -139,8 +139,8 @@ interface IOneSignal { * [IUserJwtInvalidatedListener.onUserJwtInvalidated] callback). Stores the JWT and * wakes the operation queue so any deferred ops can dispatch with the fresh token. * - * @param externalId The external ID the JWT belongs to. - * @param token The new JWT bearer token issued by your backend. + * @param externalId The external ID the JWT belongs to. Must be non-empty. + * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ fun updateUserJwt( externalId: String, @@ -166,9 +166,9 @@ interface IOneSignal { /** * Initialize the OneSignal SDK, suspend until initialization is completed * - * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. - * If none is cached, init fails (a warning for null, an error for invalid). + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. + * @param appId Your OneSignal app ID. Always pass it: a null or invalid value falls back to the app ID + * cached by an earlier launch, and with none cached init fails and logs. * * @return true if the SDK could be successfully initialized, false otherwise. */ @@ -243,7 +243,7 @@ interface IOneSignal { * Login a user with external ID and optional JWT token (suspend version). * Handles initialization automatically. * - * @param externalId The external ID of the user that is to be logged in. + * @param externalId The external ID of the user to log in. Must be non-empty: an empty value is ignored and logged. * @param jwtBearerToken The optional JWT bearer token generated by your backend to establish * trust for the login operation. Required when identity verification has been enabled. * See [Identity Verification | OneSignal](https://documentation.onesignal.com/docs/identity-verification) @@ -256,6 +256,7 @@ interface IOneSignal { /** * Login with [externalId] plus [profile] as one Create User upsert (anonymous is not merged). Email/SMS create or transfer; a foreign alias fails it. * Same id is a no-op once a backend ID exists. Backend failure leaves this identity local; INVALID_ARGUMENT and NOT_INITIALIZED change nothing. + * @param externalId Required: null or empty returns INVALID_ARGUMENT and changes nothing. */ suspend fun login( externalId: String?, @@ -272,8 +273,8 @@ interface IOneSignal { * Update the JWT bearer token associated with [externalId] (suspend version). Suspends while * initialization is in progress, then stores the JWT; if the SDK is not initialized it logs instead. * - * @param externalId The external ID the JWT belongs to. - * @param token The new JWT bearer token issued by your backend. + * @param externalId The external ID the JWT belongs to. Must be non-empty. + * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ suspend fun updateUserJwtSuspend( externalId: String, diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt index 295b658e15..5e3a7bca01 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt @@ -123,9 +123,9 @@ object OneSignal { /** * Initialize the OneSignal SDK. This should be called during startup of the application. * - * @param context The Android context the SDK should use. - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. - * If none is cached, init fails (a warning for null, an error for invalid). Check [isInitialized] for the outcome. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. + * @param appId Your OneSignal app ID. Always pass it: a null or invalid value falls back to the app ID + * cached by an earlier launch, and with none cached init fails and logs. Check [isInitialized] for the outcome. */ @JvmStatic fun initWithContext( @@ -140,9 +140,9 @@ object OneSignal { * This method provides a suspended version that returns a boolean indicating success. * Uses Dispatchers.IO internally to prevent ANRs and optimize for I/O operations. * - * @param context Application context is recommended for SDK operations - * @param appId The application ID the OneSignal SDK is bound to. If null or invalid, the cached app ID is used. - * If none is cached, init fails (a warning for null, an error for invalid). + * @param context Application context is recommended for SDK operations. Required: a null context is logged and init does not start. + * @param appId Your OneSignal app ID. Always pass it: a null or invalid value falls back to the app ID + * cached by an earlier launch, and with none cached init fails and logs. * @return Boolean indicating if initialization was successful. */ @JvmStatic @@ -303,7 +303,7 @@ object OneSignal { * subscriptions from the current user (if there is one) to the newly logged in user. This is * because both Push and IAM are owned by the device. * - * @param externalId The external ID of the user that is to be logged in. + * @param externalId The external ID of the user to log in. Must be non-empty: an empty value is ignored and logged. */ @JvmStatic fun login(externalId: String) = oneSignal.login(externalId) @@ -324,7 +324,7 @@ object OneSignal { * subscriptions from the current user (if there is one) to the newly logged in user. This is * because both Push and IAM are owned by the device. * - * @param externalId The external ID of the user that is to be logged in. + * @param externalId The external ID of the user to log in. Must be non-empty: an empty value is ignored and logged. * @param jwtBearerToken The optional JWT bearer token generated by your backend to establish * trust for the login operation. Required when identity verification has been enabled. See * [Identity Verification | OneSignal](https://documentation.onesignal.com/docs/identity-verification) @@ -350,8 +350,8 @@ object OneSignal { * [IUserJwtInvalidatedListener.onUserJwtInvalidated] callback). Stores the JWT and * wakes the operation queue so any deferred ops can dispatch with the fresh token. * - * @param externalId The external ID the JWT belongs to. - * @param token The new JWT bearer token issued by your backend. + * @param externalId The external ID the JWT belongs to. Must be non-empty. + * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ @JvmStatic fun updateUserJwt( @@ -413,7 +413,7 @@ object OneSignal { * subscriptions from the current user (if there is one) to the newly logged in user. This is * because both Push and IAM are owned by the device. * - * @param externalId The external ID of the user that is to be logged in. + * @param externalId The external ID of the user to log in. Must be non-empty: an empty value is ignored and logged. * @param jwtBearerToken The optional JWT bearer token generated by your backend to establish * trust for the login operation. Required when identity verification has been enabled. See * [Identity Verification | OneSignal](https://documentation.onesignal.com/docs/identity-verification) @@ -429,6 +429,7 @@ object OneSignal { /** * Login with [externalId] plus [profile] as one Create User upsert (anonymous is not merged). Email/SMS create or transfer; a foreign alias fails it. * Same id is a no-op once a backend ID exists. Backend failure leaves this identity local; INVALID_ARGUMENT and NOT_INITIALIZED change nothing. + * @param externalId Required: null or empty returns INVALID_ARGUMENT and changes nothing. */ @JvmStatic @JvmOverloads @@ -456,8 +457,8 @@ object OneSignal { * Update the JWT bearer token associated with [externalId] without blocking the calling * thread. Suspend-safe version of [updateUserJwt]. * - * @param externalId The external ID the JWT belongs to. - * @param token The new JWT bearer token issued by your backend. + * @param externalId The external ID the JWT belongs to. Must be non-empty. + * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ @JvmStatic suspend fun updateUserJwtSuspend( 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 6dcd267df1..b39b136c49 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 @@ -348,7 +348,8 @@ internal class OneSignalImp : IOneSignal, appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "Calling deprecated initWithContext(context: $context, appId: $appId)") - requireNotNull(context) { "initWithContext: context is required" } + if (context == null) throw nullParameter("initWithContext", "context") + if (appId == null) throw nullParameter("initWithContext", "appId") // Warm OneSignalDispatchers on a dedicated daemon thread so the first production caller // of suspendifyOnIO / launchOnSerialIO doesn't pay the ThreadPoolExecutor + dispatcher + @@ -758,7 +759,7 @@ internal class OneSignalImp : IOneSignal, appId: String?, ): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context, appId: $appId)") - requireNotNull(context) { "initWithContext: context is required" } + if (context == null) throw nullParameter("initWithContextSuspend", "context") // Same warm-up as the synchronous variant. Reaching this entry point on the main thread // (e.g. SyncJobService.onStartJob -> suspendifyOnIO -> initWithContext(context)) pays the @@ -919,3 +920,9 @@ internal class OneSignalImp : IOneSignal, logoutHelper.enqueueLogout(context) } } + +// The kill switch's fallback keeps the crash a Java null used to hit in the non-null facade. +private fun nullParameter( + method: String, + parameter: String, +) = NullPointerException("Parameter specified as non-null is null: method com.onesignal.OneSignal.$method, parameter $parameter") 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 051d89fbc6..fb71ff5d32 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,5 +1,6 @@ package com.onesignal.internal +import android.content.Context import com.onesignal.ErrorCode import com.onesignal.LoginData import com.onesignal.OneSignalResult @@ -9,6 +10,7 @@ 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.mockk.mockk import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineStart @@ -438,4 +440,17 @@ class OneSignalImpTests : FunSpec({ result.error!!.first.code shouldBe ErrorCode.INVALID_ARGUMENT result.error!!.first.message shouldBe "Tag keys must be non-empty and values non-null" } + + test("a null context or a null appId still crashes the legacy init the way the non-null API did") { + val os = OneSignalImp() + val context = mockk(relaxed = true) + + shouldThrowUnit { os.initWithContext(null, "appId") }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.initWithContext, parameter context" + shouldThrowUnit { os.initWithContext(context, null) }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.initWithContext, parameter appId" + shouldThrowUnit { runBlocking { os.initWithContextSuspend(null, "appId") } }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.initWithContextSuspend, parameter context" + os.isInitialized shouldBe false + } }) From 6f9ba081ac0e323baeb9863d125e5bea67a6335b Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 15:08:12 -0600 Subject: [PATCH 4/6] fix: [SDK-5538] stop the preferences write loop from spinning after cancellation The loop caught Throwable, which includes CancellationException, so once its scope was cancelled every pass logged an error, applied an empty edit and looped again without ever suspending. Only reachable through resetForTest() today, where it ran behind every Robolectric spec that initialized the SDK and kept QueuedWork.waitToFinish from ever draining. Co-authored-by: Cursor --- .../preferences/impl/PreferencesService.kt | 4 ++++ .../preferences/PreferencesServiceTests.kt | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/impl/PreferencesService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/impl/PreferencesService.kt index 0caa063c52..66caf86a45 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/impl/PreferencesService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/preferences/impl/PreferencesService.kt @@ -11,6 +11,7 @@ import com.onesignal.core.internal.startup.IStartableService import com.onesignal.core.internal.time.ITime import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.delay internal class PreferencesService( @@ -234,6 +235,9 @@ internal class PreferencesService( // wait to be woken up for the next pass waiter.waitForWake() + } catch (e: CancellationException) { + // Swallowing this turns the loop into a hot spin once the scope is gone. + throw e } catch (e: Throwable) { Logging.log(LogLevel.ERROR, "Error with Preference work loop", e) } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/preferences/PreferencesServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/preferences/PreferencesServiceTests.kt index 52b07f8423..ffdefa1220 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/preferences/PreferencesServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/preferences/PreferencesServiceTests.kt @@ -3,7 +3,9 @@ package com.onesignal.core.internal.preferences import android.content.Context import androidx.test.core.app.ApplicationProvider import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.core.internal.preferences.impl.PreferencesService +import com.onesignal.debug.ILogListener import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging import com.onesignal.mocks.AndroidMockHelper @@ -12,6 +14,7 @@ import io.kotest.assertions.throwables.shouldThrowUnit import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import kotlinx.coroutines.delay +import java.util.concurrent.atomic.AtomicInteger @RobolectricTest class PreferencesServiceTests : FunSpec({ @@ -228,4 +231,23 @@ class PreferencesServiceTests : FunSpec({ store.contains(mockStringPrefStoreKey) shouldBe false store.contains(mockStringSetPrefStoreKey) shouldBe false } + + test("the write loop stops when its scope is cancelled instead of spinning on the error path") { + val preferencesService = PreferencesService(AndroidMockHelper.applicationService(), MockHelper.time(1000)) + val loopErrors = AtomicInteger() + val listener = ILogListener { if (it.entry.contains("Error with Preference work loop")) loopErrors.incrementAndGet() } + Logging.addListener(listener) + try { + preferencesService.start() + preferencesService.saveBool(mockPrefStoreName, mockBoolPrefStoreKey, true) + delay(300) + + OneSignalDispatchers.resetForTest() + delay(500) + + loopErrors.get() shouldBe 0 + } finally { + Logging.removeListener(listener) + } + } }) From 0405606e7da1fa0bec613dda7758659c3c4ad0f9 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 15:08:20 -0600 Subject: [PATCH 5/6] fix: [SDK-5538] end init as FAILED when the IO pool rejects it; guard log listeners When the OneSignal IO pool is full, kotlinx cancels the launched job before its body runs, so the attempt stayed IN_PROGRESS and every accessor blocked forever. initWithContext now keeps the Job and marks the attempt FAILED with a cause if the body never started. A log listener that called back into the SDK before init recursed until the stack overflowed, and a listener that threw escaped through calls the SDK promises to drop. callLogListeners now skips re-entrant delivery on the same thread and isolates each listener's failure to logcat. Co-authored-by: Cursor --- .../debug/internal/logging/Logging.kt | 21 ++++++++++-- .../com/onesignal/internal/OneSignalImpV2.kt | 21 ++++++++++-- .../debug/internal/logging/LoggingTest.kt | 33 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt index 6d7a071bcb..1755100f81 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt @@ -179,19 +179,34 @@ object Logging { } } + // A listener that calls back into the SDK logs again on this thread; delivering that line to the + // listeners too would recurse until the stack overflows. + private val dispatchingToListeners = ThreadLocal() + + @Suppress("TooGenericExceptionCaught") private fun callLogListeners( level: LogLevel, message: String, throwable: Throwable?, ) { - if (logListeners.isEmpty()) return + if (logListeners.isEmpty() || dispatchingToListeners.get() == true) return var logEntry = message if (throwable != null) { logEntry += "\n" + android.util.Log.getStackTraceString(throwable) } - for (listener in logListeners) { - listener.onLogEvent(OneSignalLogEvent(level, logEntry)) + dispatchingToListeners.set(true) + try { + for (listener in logListeners) { + try { + listener.onLogEvent(OneSignalLogEvent(level, logEntry)) + } catch (e: Exception) { + // The SDK line being logged may be on a path that promises not to throw. + logToLogcat(LogLevel.ERROR, "A OneSignal log listener threw", e) + } + } + } finally { + dispatchingToListeners.set(false) } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt index d54b8e89dd..435b706f80 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImpV2.kt @@ -55,6 +55,7 @@ import com.onesignal.user.internal.resolveAppIdV2 import com.onesignal.user.internal.subscriptions.SubscriptionModelStore import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext +import java.util.concurrent.atomic.AtomicBoolean internal class OneSignalImpV2 : IOneSignal, IServiceProvider { @@ -398,12 +399,26 @@ internal class OneSignalImpV2 : IOneSignal, // Dispatch init asynchronously so this method never blocks the caller. Callers that // need to wait (accessors, login, logout) block on the gate. - suspendifyOnIO { - gate.runInit { internalInit(context, appId) } - } + val started = AtomicBoolean(false) + OneSignalDispatchers + .launchOnIO { + started.set(true) + gate.runInit { internalInit(context, appId) } + }.invokeOnCompletion { cause -> + // A full IO pool rejects the dispatch and cancels the job before it runs; left alone the + // attempt stays IN_PROGRESS and every accessor blocks forever. + if (!started.get()) failUnscheduledInit(cause) + } return true } + private fun failUnscheduledInit(cause: Throwable?) { + val failure = IllegalStateException("initWithContext could not be scheduled: the OneSignal IO pool rejected it", cause) + Logging.error("initWithContext could not be scheduled; marking init FAILED", failure) + gate.addFailureCause(failure) + gate.complete(InitState.FAILED) + } + /** * Called from internal classes only. Remain suspend until initialization is fully completed. */ diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/logging/LoggingTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/logging/LoggingTest.kt index e7f10f0e78..dc52d32f26 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/logging/LoggingTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/logging/LoggingTest.kt @@ -282,6 +282,39 @@ class LoggingTest : FunSpec({ Logging.removeListener(mockListener) } + test("a listener that logs is not re-entered for the line it caused") { + val events = mutableListOf() + val listener = + ILogListener { event -> + events.add(event.entry) + Logging.error("from inside the listener") + } + Logging.addListener(listener) + try { + Logging.error("outer") + + events.size shouldBe 1 + events.single().contains("outer") shouldBe true + } finally { + Logging.removeListener(listener) + } + } + + test("a listener that throws does not stop the other listeners or the caller") { + val throwing = ILogListener { throw IllegalStateException("listener bug") } + val second = mockk(relaxed = true) + Logging.addListener(throwing) + Logging.addListener(second) + try { + Logging.error("outer") + + verify { second.onLogEvent(any()) } + } finally { + Logging.removeListener(throwing) + Logging.removeListener(second) + } + } + // ===== All Log Levels Tests ===== test("all log levels should work correctly") { From 972133df3d03d403ec066910b59dc9c1fc9136a2 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 15:08:20 -0600 Subject: [PATCH 6/6] test: [SDK-5538] fault injection against the never-crash init Forces a full IO pool, a re-entrant and a throwing log listener, a throwing replay target, a failed init followed by a retry with held settings, and sixteen threads calling every public member through an in-flight init. Co-authored-by: Cursor --- .../internal/OneSignalImpV2FaultTests.kt | 303 ++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt new file mode 100644 index 0000000000..0980494d0f --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt @@ -0,0 +1,303 @@ +package com.onesignal.internal + +import android.content.Context +import android.content.ContextWrapper +import android.content.SharedPreferences +import androidx.test.core.app.ApplicationProvider.getApplicationContext +import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.IOneSignal +import com.onesignal.OneSignalUserProfile +import com.onesignal.common.threading.OneSignalDispatchers +import com.onesignal.debug.ILogListener +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.inAppMessages.IInAppMessagesManager +import com.onesignal.location.ILocationManager +import com.onesignal.notifications.INotificationsManager +import com.onesignal.session.ISessionManager +import com.onesignal.user.IUserManager +import com.onesignal.user.internal.jwt.JwtTokenStore +import com.onesignal.user.subscriptions.IPushSubscription +import io.kotest.assertions.withClue +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldBeEmpty +import io.kotest.matchers.ints.shouldBeLessThanOrEqual +import io.kotest.matchers.shouldBe +import io.mockk.mockkClass +import kotlinx.coroutines.runBlocking +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.ThreadPoolExecutor +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference +import kotlin.reflect.KCallable +import kotlin.reflect.KClass +import kotlin.reflect.KFunction +import kotlin.reflect.KMutableProperty +import kotlin.reflect.KProperty +import kotlin.reflect.KType +import kotlin.reflect.full.callSuspend + +/** + * Fault injection against [OneSignalImpV2]: each test forces one thing the platform or the app can do to + * the SDK (a full pool, a re-entrant or throwing listener, a throwing replay, a hammered API) and asserts no throw, no hang. + */ +@RobolectricTest +class OneSignalImpV2FaultTests : FunSpec({ + + fun quietOneSignal() = OneSignalImpV2().also { Logging.logLevel = LogLevel.NONE } + + fun awaitTerminal(os: OneSignalImpV2, timeoutMs: Long = 5_000): InitState { + val deadline = System.currentTimeMillis() + timeoutMs + while (os.gate.state == InitState.IN_PROGRESS && System.currentTimeMillis() < deadline) Thread.sleep(10) + return os.gate.state + } + + /** Runs [block] on a daemon thread and fails if it throws or is still running after [timeoutMs]. */ + fun onDaemonThread(timeoutMs: Long = 5_000, block: () -> T): T { + val result = AtomicReference() + val error = AtomicReference(null) + val thread = Thread { try { result.set(block()) } catch (t: Throwable) { error.set(t) } } + thread.isDaemon = true + thread.start() + thread.join(timeoutMs) + if (thread.isAlive) throw TimeoutException("still running after ${timeoutMs}ms") + error.get()?.let { throw it } + return result.get() + } + + beforeAny { Logging.logLevel = LogLevel.NONE } + + afterAny { + OneSignalDispatchers.resetForTest() + val context = getApplicationContext() + context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE).edit().clear().commit() + } + + test("init dispatched onto a full IO pool ends FAILED instead of blocking every accessor forever") { + OneSignalDispatchers.resetForTest() + val ioExecutor = AtomicReference(null) + OneSignalDispatchers.afterExecutorCreateForTest = { lane, executor -> if (lane == "IO") ioExecutor.set(executor) } + + val release = CountDownLatch(1) + try { + // 3 workers busy plus a 200-deep queue: the next dispatch is rejected, which cancels its job before it runs. + repeat(203) { OneSignalDispatchers.launchOnIO { release.await(30, TimeUnit.SECONDS) } } + val deadline = System.currentTimeMillis() + 5_000 + while (System.currentTimeMillis() < deadline) { + val pool = ioExecutor.get() + if (pool != null && pool.activeCount == 3 && pool.queue.remainingCapacity() == 0) break + Thread.sleep(10) + } + ioExecutor.get()!!.queue.remainingCapacity() shouldBe 0 + + val os = quietOneSignal() + os.initWithContext(getApplicationContext(), "appId") + + awaitTerminal(os) shouldBe InitState.FAILED + onDaemonThread { os.user.onesignalId } shouldBe "" + os.isInitialized shouldBe false + } finally { + release.countDown() + } + + // The pool drains, and a retry goes through. + OneSignalDispatchers.resetForTest() + val retry = quietOneSignal() + retry.initWithContext(getApplicationContext(), "appId") + awaitTerminal(retry) shouldBe InitState.SUCCESS + } + + test("a log listener that calls back into the SDK before init does not recurse until the stack overflows") { + val os = quietOneSignal() + val reentries = AtomicInteger() + val listener = + ILogListener { + reentries.incrementAndGet() + os.user.onesignalId + } + Logging.addListener(listener) + try { + onDaemonThread { os.user.externalId } shouldBe "" + // One re-entry per log line is fine; unbounded re-entry is the StackOverflowError this guards. + reentries.get() shouldBeLessThanOrEqual 4 + } finally { + Logging.removeListener(listener) + } + } + + test("a log listener that throws does not turn a dropped call into a crash") { + val os = quietOneSignal() + val listener = ILogListener { throw IllegalStateException("listener bug") } + Logging.addListener(listener) + try { + onDaemonThread { os.user.externalId } shouldBe "" + onDaemonThread { os.login("someone") } + } finally { + Logging.removeListener(listener) + } + } + + test("held settings survive a failed init and are applied to the real manager when the retry succeeds") { + // Core tests ship the Misconfigured managers, whose setters throw; that throw is the proof the + // held value reached the real setter, and the replay must survive it. + val context = getApplicationContext() + val os = quietOneSignal() + os.inAppMessages.paused = true + os.location.isShared = true + os.inAppMessages.paused shouldBe true + os.location.isShared shouldBe true + + os.initWithContext(context, "") + awaitTerminal(os) shouldBe InitState.FAILED + os.inAppMessages.paused shouldBe true + os.location.isShared shouldBe true + + val logs = ConcurrentLinkedQueue() + val listener = ILogListener { logs.add(it.entry) } + Logging.addListener(listener) + try { + os.initWithContext(context, "appId") + awaitTerminal(os) shouldBe InitState.SUCCESS + } finally { + Logging.removeListener(listener) + } + logs.count { it.contains("Replaying 'paused' threw") } shouldBe 1 + logs.count { it.contains("Replaying 'isShared' threw") } shouldBe 1 + } + + test("a replayed registration that throws does not fail init or lose the other registrations") { + val context = getApplicationContext() + val os = quietOneSignal() + // Misconfigured IAM throws on addClickListener; the JWT listener replayed after it must still land. + os.inAppMessages.addClickListener(mockkClass(com.onesignal.inAppMessages.IInAppMessageClickListener::class, relaxed = true)) + val fired = CountDownLatch(1) + os.addUserJwtInvalidatedListener { fired.countDown() } + + os.initWithContext(context, "appId") + awaitTerminal(os) shouldBe InitState.SUCCESS + val store = os.getService(JwtTokenStore::class.java) + store.putJwt("ext", "token") + store.invalidateJwt("ext") + fired.await(2, TimeUnit.SECONDS) shouldBe true + } + + test("sixteen threads calling every public member through an in-flight init all return once it finishes") { + val base = getApplicationContext() + val holdInit = CountDownLatch(1) + val initEntered = CountDownLatch(1) + val blockingContext = + object : ContextWrapper(base) { + override fun getSharedPreferences(name: String, mode: Int): SharedPreferences { + initEntered.countDown() + holdInit.await(30, TimeUnit.SECONDS) + return super.getSharedPreferences(name, mode) + } + } + val os = quietOneSignal() + os.initWithContext(blockingContext, "appId") + initEntered.await(5, TimeUnit.SECONDS) shouldBe true + os.gate.state shouldBe InitState.IN_PROGRESS + + val failures = ConcurrentLinkedQueue() + val inFlight = ConcurrentHashMap() + val pool = Executors.newFixedThreadPool(16) { r -> Thread(r).also { it.isDaemon = true } } + val go = CountDownLatch(1) + val done = CountDownLatch(16) + repeat(16) { n -> + pool.execute { + go.await() + try { + hammer(os, n, failures) { inFlight[n] = it } + } finally { + inFlight.remove(n) + done.countDown() + } + } + } + go.countDown() + Thread.sleep(200) + holdInit.countDown() + + val finished = done.await(20, TimeUnit.SECONDS) + pool.shutdownNow() + failures.toList().shouldBeEmpty() + withClue("still running: ${inFlight.values.sorted()}") { finished shouldBe true } + os.gate.state shouldBe InitState.SUCCESS + } +}) + +private fun dummy(type: KType): Any? { + if (type.isMarkedNullable) return null + val kClass = type.classifier as KClass<*> + return when { + kClass == String::class -> "sample" + kClass == Boolean::class -> false + kClass == Int::class -> 0 + kClass == Float::class -> 0f + kClass == Map::class -> emptyMap() + kClass == Collection::class -> emptyList() + kClass == OneSignalUserProfile::class -> OneSignalUserProfile.builder().build() + kClass.java.isEnum -> kClass.java.enumConstants!!.first() + else -> mockkClass(kClass, relaxed = true) + } +} + +private fun KCallable<*>.touchesInit() = parameters.any { it.type.classifier == Context::class } + +private fun invoke(member: KCallable<*>, target: Any) { + runBlocking { + when (member) { + is KProperty<*> -> { + member.getter.call(target) + if (member is KMutableProperty<*>) member.setter.call(target, dummy(member.returnType)) + } + is KFunction<*> -> member.callSuspend(target, *member.parameters.drop(1).map { dummy(it.type) }.toTypedArray()) + else -> Unit + } + Unit + } +} + +// loginSuspend and the composite login await the backend round trip, which no unit test can answer. +private fun KCallable<*>.awaitsBackend() = name == "loginSuspend" || parameters.any { it.type.classifier == OneSignalUserProfile::class } + +// Every member of every public surface, in a different order per thread, recording anything that throws. +private fun hammer( + os: OneSignalImpV2, + seed: Int, + failures: ConcurrentLinkedQueue, + onCall: (String) -> Unit, +) { + val surfaces: List, () -> Any>> = + listOf( + IOneSignal::class to { os }, + IUserManager::class to { os.user }, + IPushSubscription::class to { os.user.pushSubscription }, + INotificationsManager::class to { os.notifications }, + IInAppMessagesManager::class to { os.inAppMessages }, + ILocationManager::class to { os.location }, + ISessionManager::class to { os.session }, + ) + val calls = + surfaces.flatMap { (type, target) -> + type.members + .filter { it.name !in setOf("equals", "hashCode", "toString") && !it.touchesInit() && !it.awaitsBackend() } + .map { member -> "${type.simpleName}.${member.name}" to { invoke(member, target()) } } + }.shuffled(java.util.Random(seed.toLong())) + for ((name, call) in calls) { + onCall(name) + try { + call() + } catch (t: Throwable) { + val cause = (t as? java.lang.reflect.InvocationTargetException)?.targetException ?: t + // The Misconfigured managers throw by design once init succeeds; that is the legacy contract. + if (cause.message?.contains("Must include gradle module") == true) continue + failures.add("$name threw $cause") + } + } +}