From bc5c01b1c0075d2f114a9d2c16453cd53c1e2a01 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Mon, 2 Nov 2026 11:04:57 -0600 Subject: [PATCH 1/8] test: [SDK-5538] guard stand-ins and the public API against drift DeferredManagersCoverageTests: every member of the six manager interfaces must be declared on its stand-in, not inherited from a default body. Every add...Listener/Observer and every var must have a replay case, and each case must queue before init and reach the real manager on replay. OneSignalImpV2ApiSweepTests: every IOneSignal member, and every member of the managers handed out before init, is called reflectively with dummy args and must not throw or hang past 5s. Members that need a non-null Context are skipped, since they would start a real init. Co-authored-by: Cursor --- .../internal/DeferredManagersCoverageTests.kt | 117 ++++++++++++++++++ .../internal/OneSignalImpV2ApiSweepTests.kt | 98 +++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt new file mode 100644 index 000000000..271979513 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt @@ -0,0 +1,117 @@ +package com.onesignal.internal + +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.subscriptions.IPushSubscription +import io.kotest.assertions.withClue +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.mockk +import io.mockk.mockkClass +import io.mockk.verify +import kotlin.reflect.KCallable +import kotlin.reflect.KClass +import kotlin.reflect.KFunction2 +import kotlin.reflect.KMutableProperty +import kotlin.reflect.KMutableProperty1 +import kotlin.reflect.full.declaredMembers + +private class NotReadySource : ManagerSource { + override fun get(member: String): T? = null + + override fun peek(): T? = null + + override suspend fun await(member: String): T? = null + + override fun logQueued(member: String) = Unit +} + +private val standIns: Map, KClass<*>> = + mapOf( + IUserManager::class to DeferredUserManager::class, + IPushSubscription::class to DeferredPushSubscription::class, + INotificationsManager::class to DeferredNotificationsManager::class, + IInAppMessagesManager::class to DeferredInAppMessagesManager::class, + ILocationManager::class to DeferredLocationManager::class, + ISessionManager::class to DeferredSessionManager::class, + ) + +private class ReplayCase( + val type: KClass, + val member: String, + val standIn: (ManagerSource) -> DeferredManager, + val call: T.() -> Unit, +) { + fun run() { + val real = mockkClass(type, relaxed = true) + val deferred = standIn(NotReadySource()) + @Suppress("UNCHECKED_CAST") + (deferred as T).call() + verify(exactly = 0) { real.call() } + deferred.replay(real) + verify(exactly = 1) { real.call() } + } +} + +// One listener per case, so the replayed call can be verified against the same instance. +private inline fun registration( + noinline standIn: (ManagerSource) -> DeferredManager, + add: KFunction2, +): ReplayCase { + val listener = mockk(relaxed = true) + return ReplayCase(T::class, add.name, standIn) { add(this, listener) } +} + +private inline fun setting( + noinline standIn: (ManagerSource) -> DeferredManager, + property: KMutableProperty1, +) = ReplayCase(T::class, property.name, standIn) { property.set(this, true) } + +private val replayCases: List> = + listOf( + registration(::DeferredUserManager, IUserManager::addObserver), + registration(::DeferredPushSubscription, IPushSubscription::addObserver), + registration(::DeferredNotificationsManager, INotificationsManager::addPermissionObserver), + registration(::DeferredNotificationsManager, INotificationsManager::addForegroundLifecycleListener), + registration(::DeferredNotificationsManager, INotificationsManager::addClickListener), + registration(::DeferredInAppMessagesManager, IInAppMessagesManager::addLifecycleListener), + registration(::DeferredInAppMessagesManager, IInAppMessagesManager::addClickListener), + setting(::DeferredInAppMessagesManager, IInAppMessagesManager::paused), + setting(::DeferredLocationManager, ILocationManager::isShared), + ) + +private val replayedMember = Regex("^add\\w*(Listener|Observer)$") + +private fun KCallable<*>.signature() = "$name(${parameters.drop(1).joinToString { it.type.toString() }})" + +private fun KClass<*>.apiMembers() = members.filter { it.name !in setOf("equals", "hashCode", "toString") } + +class DeferredManagersCoverageTests : FunSpec({ + test("every interface member is implemented on its stand-in, not inherited from a default body") { + val missing = + standIns.flatMap { (iface, standIn) -> + val declared = standIn.declaredMembers.map { it.signature() }.toSet() + iface.apiMembers().map { it.signature() }.filter { it !in declared } + .map { "${iface.simpleName}.$it: implement it in ${standIn.simpleName}" } + } + missing shouldBe emptyList() + } + + test("every listener, observer and setting has a replay case") { + val covered = replayCases.map { "${it.type.simpleName}.${it.member}" }.toSet() + val uncovered = + standIns.keys.flatMap { iface -> + iface.apiMembers().filter { it is KMutableProperty<*> || replayedMember.matches(it.name) } + .map { "${iface.simpleName}.${it.name}" } + .filter { it !in covered } + } + withClue("decide whether each queues before init, then add it to replayCases") { uncovered shouldBe emptyList() } + } + + test("each queued registration or setting reaches the real manager on replay") { + replayCases.forEach { case -> withClue("${case.type.simpleName}.${case.member}") { case.run() } } + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt new file mode 100644 index 000000000..58cb029a3 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt @@ -0,0 +1,98 @@ +package com.onesignal.internal + +import android.content.Context +import com.onesignal.IOneSignal +import com.onesignal.OneSignalUserProfile +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.subscriptions.IPushSubscription +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.mockkClass +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import java.lang.reflect.InvocationTargetException +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 + +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<*>.label() = "$name(${parameters.drop(1).joinToString { it.type.toString() }})" + +// Initializing with a real context is SDKInitV2Tests' job; here only the null-context variants run. +private fun KCallable<*>.startsInit() = parameters.any { it.type.classifier == Context::class && !it.type.isMarkedNullable } + +// Calls every member of [type] on [target()] with dummy args; returns the ones that threw or hung. +private fun sweep( + type: KClass<*>, + target: () -> Any, +): List = + type.members + .filter { it.name !in setOf("equals", "hashCode", "toString") && !it.startsInit() } + .mapNotNull { member -> + try { + runBlocking { + withTimeout(5_000) { + 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()) + } + Unit + } + } + null + } catch (e: TimeoutCancellationException) { + "${type.simpleName}.${member.label()} hung for 5s" + } catch (e: Throwable) { + "${type.simpleName}.${member.label()} threw ${(e as? InvocationTargetException)?.targetException ?: e}" + } + } + +// DebugManager's init resets the level to WARN, and WARN reaches the unmocked android.util.Log. +private fun quietOneSignal() = OneSignalImpV2().also { Logging.logLevel = LogLevel.NONE } + +class OneSignalImpV2ApiSweepTests : FunSpec({ + test("no IOneSignal member throws or hangs before init") { + sweep(IOneSignal::class) { quietOneSignal() } shouldBe emptyList() + } + + test("no member of a manager handed out before init throws or hangs") { + val os = quietOneSignal() + listOf, () -> Any>>( + 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 }, + ).flatMap { (type, target) -> sweep(type, target) } shouldBe emptyList() + } +}) From 135f3a2f93a95ce12407707d553236d694a8ea14 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 15:08:39 -0600 Subject: [PATCH 2/8] test: [SDK-5538] sweep the FAILED state and null arguments; compare legacy and V2 behavior The sweep now runs after a failed init as well, enforces its deadline from a separate thread so a blocking wait cannot escape withTimeout, and passes null for every required parameter to pin the Java boundary: nothing hangs and nothing but the Kotlin NullPointerException surfaces. OneSignalImpParityTests runs the same scenarios on OneSignalImp and OneSignalImpV2 and asserts the app-visible results match. Co-authored-by: Cursor --- .../application/OneSignalImpParityTests.kt | 157 ++++++++++++++++++ .../internal/OneSignalImpV2ApiSweepTests.kt | 133 +++++++++++---- 2 files changed, 259 insertions(+), 31 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt new file mode 100644 index 000000000..f93648090 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt @@ -0,0 +1,157 @@ +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.IOneSignal +import com.onesignal.common.services.IServiceProvider +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.internal.OneSignalImp +import com.onesignal.internal.OneSignalImpV2 +import io.kotest.assertions.withClue +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +/** + * Runs the same scenario on the legacy and the never-crash implementation and compares what the app + * can observe. The only intended differences are the calls that used to throw, which are covered by + * the V2 suites; everything here must stay identical. + */ +@RobolectricTest +class OneSignalImpParityTests : FunSpec({ + + fun IOneSignal.awaitInit(timeoutMs: Long = 5_000): Boolean { + val deadline = System.currentTimeMillis() + timeoutMs + while (!isInitialized && System.currentTimeMillis() < deadline) Thread.sleep(10) + return isInitialized + } + + fun IOneSignal.awaitExternalId(expected: String, timeoutMs: Long = 5_000): String { + val deadline = System.currentTimeMillis() + timeoutMs + while (user.externalId != expected && System.currentTimeMillis() < deadline) Thread.sleep(10) + return user.externalId + } + + fun IOneSignal.appIdInConfig() = (this as IServiceProvider).getService(ConfigModelStore::class.java).model.appId + + // PreferencesService flushes on a 200ms buffer, so an instance from the previous scenario can still + // write after a clear; let it land first. + fun clearPrefs() { + Thread.sleep(400) + val context = getApplicationContext() + for (name in listOf("OneSignal", "com.onesignal")) { + context.getSharedPreferences(name, Context.MODE_PRIVATE).edit().clear().commit() + } + } + + /** Inits a throwaway instance of the same implementation and waits until its app ID is on disk. */ + fun IOneSignal.cacheAppId(context: Context, appId: String) { + val warm = if (this is OneSignalImpV2) OneSignalImpV2() else OneSignalImp() + Logging.logLevel = LogLevel.NONE + warm.initWithContext(context, appId) + warm.awaitInit() + val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) + val deadline = System.currentTimeMillis() + 5_000 + while (prefs.all.values.none { it.toString().contains(appId) } && System.currentTimeMillis() < deadline) Thread.sleep(10) + prefs.all.values.any { it.toString().contains(appId) } shouldBe true + } + + fun implementations(): List IOneSignal>> = + listOf( + "legacy" to { OneSignalImp().also { Logging.logLevel = LogLevel.NONE } }, + "v2" to { OneSignalImpV2().also { Logging.logLevel = LogLevel.NONE } }, + ) + + /** Runs [scenario] on a fresh instance of each implementation, with clean prefs, and compares the results. */ + fun parity(scenario: (IOneSignal, Context) -> Any?) { + val results = + implementations().map { (name, create) -> + clearPrefs() + name to scenario(create(), getApplicationContext()) + } + withClue("legacy=${results[0].second} v2=${results[1].second}") { + results[1].second shouldBe results[0].second + } + } + + beforeAny { Logging.logLevel = LogLevel.NONE } + afterAny { clearPrefs() } + + test("init then reading every top-level value") { + parity { os, context -> + os.initWithContext(context, "appId") + os.awaitInit() + listOf( + os.isInitialized, + os.user.onesignalId.isNotEmpty(), + os.user.externalId, + os.user.pushSubscription.optedIn, + os.user.pushSubscription.id, + os.user.getTags(), + os.consentRequired, + os.consentGiven, + os.disableGMSMissingPrompt, + os.appIdInConfig(), + ) + } + } + + test("settings changed before init reach the config model") { + parity { os, context -> + os.consentRequired = true + os.disableGMSMissingPrompt = true + val beforeInit = listOf(os.consentRequired, os.consentGiven, os.disableGMSMissingPrompt) + os.initWithContext(context, "appId") + os.awaitInit() + val model = (os as IServiceProvider).getService(ConfigModelStore::class.java).model + beforeInit + listOf(os.consentRequired, os.consentGiven, os.disableGMSMissingPrompt, model.consentRequired, model.consentGiven, model.disableGMSMissingPrompt) + } + } + + test("a second initWithContext after success keeps the first app ID") { + parity { os, context -> + os.initWithContext(context, "appId") + os.awaitInit() + val second = os.initWithContext(context, "otherAppId") + Thread.sleep(100) + listOf(second, os.isInitialized, os.appIdInConfig()) + } + } + + test("login, tags and logout after init") { + parity { os, context -> + os.initWithContext(context, "appId") + os.awaitInit() + os.login("ext-1") + val afterLogin = os.awaitExternalId("ext-1") + os.user.addTags(mapOf("a" to "1", "b" to "2")) + os.user.removeTag("b") + val tags = os.user.getTags() + os.logout() + val afterLogout = os.awaitExternalId("") + listOf(afterLogin, tags, afterLogout, os.user.onesignalId.isNotEmpty()) + } + } + + test("a cached app ID is reused when init is called with an empty app ID") { + parity { os, context -> + os.cacheAppId(context, "cachedAppId") + + os.initWithContext(context, "") + os.awaitInit() + listOf(os.isInitialized, os.appIdInConfig()) + } + } + + test("init with a null app ID resolves from the cache written by an earlier init") { + parity { os, context -> + os.cacheAppId(context, "cachedAppId") + + val result = + kotlinx.coroutines.runBlocking { os.initWithContextSuspend(context, null) } + listOf(result, os.isInitialized, os.appIdInConfig()) + } + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt index 58cb029a3..7d1892e6b 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt @@ -14,14 +14,16 @@ import com.onesignal.user.subscriptions.IPushSubscription import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.mockkClass -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withTimeout import java.lang.reflect.InvocationTargetException +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KFunction import kotlin.reflect.KMutableProperty +import kotlin.reflect.KParameter import kotlin.reflect.KProperty import kotlin.reflect.KType import kotlin.reflect.full.callSuspend @@ -47,38 +49,96 @@ private fun KCallable<*>.label() = "$name(${parameters.drop(1).joinToString { it // Initializing with a real context is SDKInitV2Tests' job; here only the null-context variants run. private fun KCallable<*>.startsInit() = parameters.any { it.type.classifier == Context::class && !it.type.isMarkedNullable } +private fun KCallable<*>.isApi() = name !in setOf("equals", "hashCode", "toString") && !startsInit() + +private val sweepThread = Executors.newCachedThreadPool { r -> Thread(r).also { it.isDaemon = true } } + +// A blocking wait inside the member cannot be interrupted by withTimeout, so the deadline is enforced from outside. +private fun callWithDeadline(block: () -> Unit) { + val future = sweepThread.submit(block) + try { + future.get(5, TimeUnit.SECONDS) + } catch (e: java.util.concurrent.ExecutionException) { + throw e.cause ?: e + } +} + +private fun invoke( + member: KCallable<*>, + target: Any, + args: (KParameter) -> Any? = { dummy(it.type) }, +) { + 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(args).toTypedArray()) + else -> Unit + } + Unit + } +} + // Calls every member of [type] on [target()] with dummy args; returns the ones that threw or hung. private fun sweep( type: KClass<*>, target: () -> Any, ): List = - type.members - .filter { it.name !in setOf("equals", "hashCode", "toString") && !it.startsInit() } - .mapNotNull { member -> - try { - runBlocking { - withTimeout(5_000) { - 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()) - } - Unit - } + type.members.filter { it.isApi() }.mapNotNull { member -> + try { + callWithDeadline { invoke(member, target()) } + null + } catch (e: TimeoutException) { + "${type.simpleName}.${member.label()} hung for 5s" + } catch (e: Throwable) { + "${type.simpleName}.${member.label()} threw ${(e as? InvocationTargetException)?.targetException ?: e}" + } + } + +private fun KParameter.isRequiredReference() = !type.isMarkedNullable && !(type.classifier as KClass<*>).java.isPrimitive && type.classifier != Boolean::class && type.classifier != Int::class && type.classifier != Float::class + +// Passes null for each required parameter in turn; returns the calls that hung or threw anything but a NullPointerException. +private fun nullSweep( + type: KClass<*>, + target: () -> Any, +): List = + type.members.filterIsInstance>().filter { it.isApi() }.flatMap { member -> + member.parameters.drop(1).filter { it.isRequiredReference() }.mapNotNull { nulled -> + val outcome = + try { + callWithDeadline { invoke(member, target()) { if (it == nulled) null else dummy(it.type) } } + null + } catch (e: TimeoutException) { + "hung for 5s" + } catch (e: Throwable) { + val cause = (e as? InvocationTargetException)?.targetException ?: e + if (cause is NullPointerException) null else "threw $cause" } - null - } catch (e: TimeoutCancellationException) { - "${type.simpleName}.${member.label()} hung for 5s" - } catch (e: Throwable) { - "${type.simpleName}.${member.label()} threw ${(e as? InvocationTargetException)?.targetException ?: e}" - } + outcome?.let { "${type.simpleName}.${member.label()} with null ${nulled.name} $it" } } + } // DebugManager's init resets the level to WARN, and WARN reaches the unmocked android.util.Log. private fun quietOneSignal() = OneSignalImpV2().also { Logging.logLevel = LogLevel.NONE } +private fun failedOneSignal() = + quietOneSignal().also { + it.gate.tryStart() + it.gate.complete(InitState.FAILED) + } + +private fun managers(os: OneSignalImpV2): List, () -> Any>> = + listOf( + 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 }, + ) + class OneSignalImpV2ApiSweepTests : FunSpec({ test("no IOneSignal member throws or hangs before init") { sweep(IOneSignal::class) { quietOneSignal() } shouldBe emptyList() @@ -86,13 +146,24 @@ class OneSignalImpV2ApiSweepTests : FunSpec({ test("no member of a manager handed out before init throws or hangs") { val os = quietOneSignal() - listOf, () -> Any>>( - 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 }, - ).flatMap { (type, target) -> sweep(type, target) } shouldBe emptyList() + managers(os).flatMap { (type, target) -> sweep(type, target) } shouldBe emptyList() + } + + test("no IOneSignal member throws or hangs after a failed init") { + sweep(IOneSignal::class) { failedOneSignal() } shouldBe emptyList() + } + + test("no member of a manager handed out after a failed init throws or hangs") { + val os = failedOneSignal() + managers(os).flatMap { (type, target) -> sweep(type, target) } shouldBe emptyList() + } + + // Java callers passing null hit Kotlin's parameter check, the same as legacy; the suspend entry points + // return before touching the value. Either way the call must not hang or surface anything else. + test("a null argument for a required parameter never hangs and throws nothing but a NullPointerException") { + val before = quietOneSignal() + val failed = failedOneSignal() + (listOf(IOneSignal::class to { before }) + managers(before)).flatMap { (type, target) -> nullSweep(type, target) } shouldBe emptyList() + (listOf(IOneSignal::class to { failed }) + managers(failed)).flatMap { (type, target) -> nullSweep(type, target) } shouldBe emptyList() } }) From 58221a445a873b588b529326cc4ee185705905ab Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 16:20:23 -0600 Subject: [PATCH 3/8] fix: [SDK-5538] accept null externalId and context on the public entry points V2 logs and returns. Legacy still throws the same NullPointerException. Drop the isMissing overload that only built the log string. Co-authored-by: Cursor --- OneSignalSDK/detekt/detekt-baseline-core.xml | 2 +- .../src/main/java/com/onesignal/IOneSignal.kt | 22 +++++++------- .../src/main/java/com/onesignal/OneSignal.kt | 22 +++++++------- .../java/com/onesignal/common/InputGuard.kt | 7 ----- .../com/onesignal/internal/OneSignalImp.kt | 15 ++++++---- .../com/onesignal/internal/OneSignalImpV2.kt | 30 +++++++++---------- .../com/onesignal/common/InputGuardTests.kt | 8 ----- .../onesignal/internal/OneSignalImpTests.kt | 6 ++++ .../internal/OneSignalImpV2ApiSweepTests.kt | 4 +-- .../onesignal/internal/OneSignalImpV2Tests.kt | 6 +++- 10 files changed, 60 insertions(+), 62 deletions(-) diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index a46e9d40e..3b38acb97 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -537,7 +537,7 @@ UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun isInBadState(): Boolean UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun recover() UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun recoveryMessage(): String - UndocumentedPublicFunction:IOneSignal.kt$IOneSignal$fun login(externalId: String) + UndocumentedPublicFunction:IOneSignal.kt$IOneSignal$fun login(externalId: String?) UndocumentedPublicFunction:IOperationRepo.kt$IOperationRepo$fun forceExecuteOperations() UndocumentedPublicFunction:IOperationRepo.kt$IOperationRepo$suspend fun awaitInitialized() UndocumentedPublicFunction:IOperationRepo.kt$inline fun <reified T : Operation> IOperationRepo.containsInstanceOf(): Boolean 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 3bb1ed8ee..b258a5d2e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt @@ -93,9 +93,9 @@ interface IOneSignal { /** * Initialize the OneSignal SDK, suspend until initialization is completed * - * @param context The Android context the SDK should use. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. */ - suspend fun initWithContext(context: Context): Boolean + suspend fun initWithContext(context: Context?): Boolean /** * Login to OneSignal under the user identified by the [externalId] provided. The act of @@ -113,17 +113,17 @@ 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 to log in. Must be non-empty: an empty value is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty 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) */ fun login( - externalId: String, + externalId: String?, jwtBearerToken: String? = null, ) - fun login(externalId: String) = login(externalId, null) + fun login(externalId: String?) = login(externalId, null) /** * Logout the user previously logged in via [login]. The [user] property now references @@ -139,11 +139,11 @@ 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. Must be non-empty. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. * @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, + externalId: String?, token: String, ) @@ -243,13 +243,13 @@ 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 to log in. Must be non-empty: an empty value is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty 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) */ suspend fun loginSuspend( - externalId: String, + externalId: String?, jwtBearerToken: String? = null, ) @@ -273,11 +273,11 @@ 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. Must be non-empty. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. * @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, + externalId: String?, token: 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 5e3a7bca0..6f3d890eb 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt @@ -303,10 +303,10 @@ 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 to log in. Must be non-empty: an empty value is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. */ @JvmStatic - fun login(externalId: String) = oneSignal.login(externalId) + fun login(externalId: String?) = oneSignal.login(externalId) /** * Login to OneSignal under the user identified by the [externalId] provided. The act of @@ -324,14 +324,14 @@ 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 to log in. Must be non-empty: an empty value is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty 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) */ @JvmStatic fun login( - externalId: String, + externalId: String?, jwtBearerToken: String? = null, ) = oneSignal.login(externalId, jwtBearerToken) @@ -350,12 +350,12 @@ 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. Must be non-empty. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ @JvmStatic fun updateUserJwt( - externalId: String, + externalId: String?, token: String, ) = oneSignal.updateUserJwt(externalId, token) @@ -390,7 +390,7 @@ object OneSignal { * THIS IS AN INTERNAL INTERFACE AND SHOULD NOT BE USED DIRECTLY. */ @JvmStatic - suspend fun initWithContext(context: Context): Boolean { + suspend fun initWithContext(context: Context?): Boolean { return oneSignal.initWithContext(context) } @@ -413,14 +413,14 @@ 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 to log in. Must be non-empty: an empty value is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty 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) */ @JvmStatic suspend fun loginSuspend( - externalId: String, + externalId: String?, jwtBearerToken: String? = null, ) { oneSignal.loginSuspend(externalId, jwtBearerToken) @@ -457,12 +457,12 @@ 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. Must be non-empty. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. * @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( - externalId: String, + externalId: String?, token: String, ) { oneSignal.updateUserJwtSuspend(externalId, token) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/InputGuard.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/InputGuard.kt index 593a1eca8..527a9facd 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/InputGuard.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/InputGuard.kt @@ -1,7 +1,6 @@ package com.onesignal.common import com.onesignal.debug.internal.logging.Logging -import com.onesignal.internal.SdkOperation internal fun hasNullByte(value: String?): Boolean = value != null && '\u0000' in value @@ -23,12 +22,6 @@ fun isMissing( return error != null } -internal fun isMissing( - value: String?, - operation: SdkOperation, - parameter: String, -): Boolean = isMissing(value, "$operation: $parameter") - /** * Returns true when any of [values] is missing, per [isMissing]. */ 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 b39b136c4..36109912c 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 @@ -399,8 +399,9 @@ internal class OneSignalImp : IOneSignal, /** * Called from internal classes only. Remain suspend until initialization is fully completed. */ - override suspend fun initWithContext(context: Context): Boolean { + override suspend fun initWithContext(context: Context?): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context)") + if (context == null) throw nullParameter("initWithContext", "context") return initWithContextSuspend(context, null) } @@ -479,10 +480,11 @@ internal class OneSignalImp : IOneSignal, } override fun login( - externalId: String, + externalId: String?, jwtBearerToken: String?, ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") + if (externalId == null) throw nullParameter("login", "externalId") waitForInit(operationName = "login") @@ -504,10 +506,11 @@ internal class OneSignalImp : IOneSignal, } override fun updateUserJwt( - externalId: String, + externalId: String?, token: String, ) { Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})") + if (externalId == null) throw nullParameter("updateUserJwt", "externalId") waitForInit(operationName = "updateUserJwt") @@ -813,10 +816,11 @@ internal class OneSignalImp : IOneSignal, } override suspend fun loginSuspend( - externalId: String, + externalId: String?, jwtBearerToken: String?, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") + if (externalId == null) throw nullParameter("loginSuspend", "externalId") // suspendUntilInit throws on NOT_STARTED / FAILED (preserving initFailureException as the // cause), and only returns once initState == SUCCESS — so no post-check is needed here. @@ -891,10 +895,11 @@ internal class OneSignalImp : IOneSignal, wait.httpStatusCode?.let { "Login did not complete (HTTP $it)." } ?: "Login did not complete." override suspend fun updateUserJwtSuspend( - externalId: String, + externalId: String?, token: String, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})") + if (externalId == null) throw nullParameter("updateUserJwtSuspend", "externalId") suspendUntilInit(operationName = "updateUserJwt") 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 435b706f8..038cdbac6 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 @@ -422,7 +422,7 @@ internal class OneSignalImpV2 : IOneSignal, /** * Called from internal classes only. Remain suspend until initialization is fully completed. */ - override suspend fun initWithContext(context: Context): Boolean { + override suspend fun initWithContext(context: Context?): Boolean { Logging.log(LogLevel.DEBUG, "initWithContext(context: $context)") return initWithContextSuspend(context, null) } @@ -478,14 +478,14 @@ internal class OneSignalImpV2 : IOneSignal, } override fun login( - externalId: String, + externalId: String?, jwtBearerToken: String?, ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - if (!gate.isReady(SdkOperation.LOGIN) || isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM)) return + if (!gate.isReady(SdkOperation.LOGIN) || isMissing(externalId, "login: externalId")) return - val context = loginHelper.switchUser(externalId, jwtBearerToken).context ?: return + val context = loginHelper.switchUser(externalId!!, jwtBearerToken).context ?: return suspendifyOnIO { loginHelper.enqueueLogin(context) } } @@ -501,16 +501,16 @@ internal class OneSignalImpV2 : IOneSignal, } override fun updateUserJwt( - externalId: String, + externalId: String?, token: String, ) { Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})") if (!gate.isReady(SdkOperation.UPDATE_USER_JWT)) return - if (isMissing(externalId, SdkOperation.UPDATE_USER_JWT, EXTERNAL_ID_PARAM) || isMissing(token, SdkOperation.UPDATE_USER_JWT, TOKEN_PARAM)) return + if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return - jwtTokenStore.putJwt(externalId, token) + jwtTokenStore.putJwt(externalId!!, token) // Wake the queue so any deferred ops can dispatch with the fresh token. operationRepo.forceExecuteOperations() } @@ -629,16 +629,16 @@ internal class OneSignalImpV2 : IOneSignal, } override suspend fun loginSuspend( - externalId: String, + externalId: String?, jwtBearerToken: String?, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") if (!gate.awaitReady(SdkOperation.LOGIN)) return@withContext - if (isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM)) return@withContext + if (isMissing(externalId, "login: externalId")) return@withContext - val context = loginHelper.switchUser(externalId, jwtBearerToken).context ?: return@withContext + val context = loginHelper.switchUser(externalId!!, jwtBearerToken).context ?: return@withContext loginHelper.enqueueLogin(context) } @@ -659,7 +659,7 @@ internal class OneSignalImpV2 : IOneSignal, return@withContext OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, gate.notReadyMessage("login")) } - if (isMissing(externalId, SdkOperation.LOGIN, EXTERNAL_ID_PARAM) || externalId == null) { + if (isMissing(externalId, "login: externalId") || externalId == null) { return@withContext OneSignalResult.failure( ErrorCode.INVALID_ARGUMENT, "login: externalId is required", @@ -693,18 +693,18 @@ internal class OneSignalImpV2 : IOneSignal, wait.httpStatusCode?.let { "Login did not complete (HTTP $it)." } ?: "Login did not complete." override suspend fun updateUserJwtSuspend( - externalId: String, + externalId: String?, token: String, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})") if (!gate.awaitReady(SdkOperation.UPDATE_USER_JWT)) return@withContext - if (isMissing(externalId, SdkOperation.UPDATE_USER_JWT, EXTERNAL_ID_PARAM) || isMissing(token, SdkOperation.UPDATE_USER_JWT, TOKEN_PARAM)) { + if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) { return@withContext } - jwtTokenStore.putJwt(externalId, token) + jwtTokenStore.putJwt(externalId!!, token) operationRepo.forceExecuteOperations() } @@ -720,7 +720,5 @@ internal class OneSignalImpV2 : IOneSignal, 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/test/java/com/onesignal/common/InputGuardTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/InputGuardTests.kt index 40f8630d3..ae2957658 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/InputGuardTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/InputGuardTests.kt @@ -2,7 +2,6 @@ package com.onesignal.common import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging -import com.onesignal.internal.SdkOperation import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe @@ -25,11 +24,4 @@ class InputGuardTests : FunSpec({ isMissing("abc\u0000", "addAlias: id") shouldBe true hasMissingEntries(mapOf("external_id" to "\u0000: 1"), "addAliases", allowEmptyValue = false) shouldBe true } - - test("the operation overload applies the same checks and logs the public API name") { - isMissing(null, SdkOperation.LOGIN, "externalId") shouldBe true - isMissing("\u0000", SdkOperation.LOGIN, "externalId") shouldBe true - isMissing("user-1", SdkOperation.LOGIN, "externalId") shouldBe false - "${SdkOperation.LOGIN}: externalId" shouldBe "login: externalId" - } }) 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 fb71ff5d3..365759be4 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 @@ -451,6 +451,12 @@ class OneSignalImpTests : FunSpec({ "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" + shouldThrowUnit { runBlocking { os.initWithContext(null) } }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.initWithContext, parameter context" + shouldThrowUnit { os.login(null) }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.login, parameter externalId" + shouldThrowUnit { os.updateUserJwt(null, "token") }.message shouldBe + "Parameter specified as non-null is null: method com.onesignal.OneSignal.updateUserJwt, parameter externalId" os.isInitialized shouldBe false } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt index 7d1892e6b..e12ad51cc 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt @@ -158,8 +158,8 @@ class OneSignalImpV2ApiSweepTests : FunSpec({ managers(os).flatMap { (type, target) -> sweep(type, target) } shouldBe emptyList() } - // Java callers passing null hit Kotlin's parameter check, the same as legacy; the suspend entry points - // return before touching the value. Either way the call must not hang or surface anything else. + // Java callers passing null for a still-required argument hit Kotlin's parameter check. externalId, + // context, and appId are nullable and log instead. Either way the call must not hang. test("a null argument for a required parameter never hangs and throws nothing but a NullPointerException") { val before = quietOneSignal() val failed = failedOneSignal() 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 index b52b92bfa..fbbb0d78e 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt @@ -22,17 +22,20 @@ class OneSignalImpV2Tests : FunSpec({ os.login("login-id") os.login("") + os.login(null) + os.updateUserJwt(null, "token") os.isInitialized shouldBe false } - test("login with empty externalId after init does not switch user") { + test("login with a null or 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("") + os.login(null) } test("login with a null byte in externalId after init does not switch user") { @@ -49,6 +52,7 @@ class OneSignalImpV2Tests : FunSpec({ Logging.logLevel = LogLevel.NONE os.updateUserJwt("", "token") + os.updateUserJwt(null, "token") os.updateUserJwt("user", "") } From 9ef18e1de49b5e3b9e7d80e035a24535f55b2c21 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Thu, 5 Nov 2026 16:32:13 -0600 Subject: [PATCH 4/8] fix: [SDK-5538] log a null context, appId, or externalId by name V2 returns after the log. When the SDK is not initialized, the log says so. After a successful init, a null externalId is still rejected without that sentence. Co-authored-by: Cursor --- .../src/main/java/com/onesignal/IOneSignal.kt | 18 +-- .../src/main/java/com/onesignal/OneSignal.kt | 18 +-- .../com/onesignal/internal/OneSignalImpV2.kt | 104 +++++++++++------- .../onesignal/internal/OneSignalImpV2Tests.kt | 26 +++++ 4 files changed, 111 insertions(+), 55 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 b258a5d2e..265b2bcb3 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. Required: a null context is logged and init does not start. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. The log says the SDK is not initialized. * @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. + * cached by an earlier launch, and with none cached init fails and logs that the SDK is not initialized. * * @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. @@ -93,7 +93,7 @@ interface IOneSignal { /** * Initialize the OneSignal SDK, suspend until initialization is completed * - * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. The log says the SDK is not initialized. */ suspend fun initWithContext(context: Context?): Boolean @@ -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 to log in. Required: null or empty is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @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,7 +139,7 @@ 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. Required: null or empty is ignored and logged. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ fun updateUserJwt( @@ -166,9 +166,9 @@ interface IOneSignal { /** * Initialize the OneSignal SDK, suspend until initialization is completed * - * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. The log says the SDK is not initialized. * @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. + * cached by an earlier launch, and with none cached init fails and logs that the SDK is not initialized. * * @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 to log in. Required: null or empty is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @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) @@ -273,7 +273,7 @@ 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. Required: null or empty is ignored and logged. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ suspend fun updateUserJwtSuspend( 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 6f3d890eb..8a672631e 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. Required: a null context is logged and init does not start. + * @param context The Android context the SDK should use. Required: a null context is logged and init does not start. The log says the SDK is not initialized. * @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. + * cached by an earlier launch, and with none cached init fails and logs that the SDK is not initialized. 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. Required: a null context is logged and init does not start. + * @param context Application context is recommended for SDK operations. Required: a null context is logged and init does not start. The log says the SDK is not initialized. * @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. + * cached by an earlier launch, and with none cached init fails and logs that the SDK is not initialized. * @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 to log in. Required: null or empty is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. */ @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 to log in. Required: null or empty is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @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,7 +350,7 @@ 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. Required: null or empty is ignored and logged. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ @JvmStatic @@ -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 to log in. Required: null or empty is ignored and logged. + * @param externalId The external ID of the user to log in. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @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) @@ -457,7 +457,7 @@ 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. Required: null or empty is ignored and logged. + * @param externalId The external ID the JWT belongs to. Required: null or empty is ignored and logged. A null value's log says the SDK is not initialized when it is not. * @param token The new JWT bearer token issued by your backend. Must be non-empty; empty values are ignored and logged. */ @JvmStatic 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 038cdbac6..bd84ed95a 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 @@ -373,7 +373,7 @@ internal class OneSignalImpV2 : IOneSignal, Logging.log(LogLevel.DEBUG, "Calling deprecated initWithContext(context: $context, appId: $appId)") if (context == null) { - Logging.error(MISSING_CONTEXT_MESSAGE) + logNullArgument("initWithContext", "context") return false } @@ -446,15 +446,16 @@ internal class OneSignalImpV2 : IOneSignal, val result = resolveAppIdV2(appId, configModel, preferencesService) if (result.failed) { - val message = "initWithContext skipped: no valid appId was passed and none is cached. Pass your OneSignal app ID to initWithContext()." + val message = rejectedAppIdMessage(appId) 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) + 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.") + if (appId == null) { + Logging.warn("initWithContext: appId is null. Using the cached app ID.") + } else if (!isUsableAppId(appId)) { + Logging.warn("initWithContext: appId is empty or contains a null byte. Using the cached app ID.") } configModel.appId = result.appId!! // safe because failed is false val forceCreateUser = result.forceCreateUser @@ -483,9 +484,10 @@ internal class OneSignalImpV2 : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - if (!gate.isReady(SdkOperation.LOGIN) || isMissing(externalId, "login: externalId")) return + val id = requireExternalId(externalId, SdkOperation.LOGIN) + if (id == null || !gate.isReady(SdkOperation.LOGIN) || isMissing(id, "login: externalId")) return - val context = loginHelper.switchUser(externalId!!, jwtBearerToken).context ?: return + val context = loginHelper.switchUser(id, jwtBearerToken).context ?: return suspendifyOnIO { loginHelper.enqueueLogin(context) } } @@ -506,11 +508,11 @@ internal class OneSignalImpV2 : IOneSignal, ) { Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})") - if (!gate.isReady(SdkOperation.UPDATE_USER_JWT)) return + val id = requireExternalId(externalId, SdkOperation.UPDATE_USER_JWT) + if (id == null || !gate.isReady(SdkOperation.UPDATE_USER_JWT)) return + if (isMissing(id, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return - if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return - - jwtTokenStore.putJwt(externalId!!, token) + jwtTokenStore.putJwt(id, token) // Wake the queue so any deferred ops can dispatch with the fresh token. operationRepo.forceExecuteOperations() } @@ -588,7 +590,7 @@ internal class OneSignalImpV2 : IOneSignal, Logging.log(LogLevel.DEBUG, "initWithContext(context: $context, appId: $appId)") if (context == null) { - Logging.error(MISSING_CONTEXT_MESSAGE) + logNullArgument("initWithContext", "context") return false } @@ -634,11 +636,10 @@ internal class OneSignalImpV2 : IOneSignal, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})") - if (!gate.awaitReady(SdkOperation.LOGIN)) return@withContext - - if (isMissing(externalId, "login: externalId")) return@withContext + val id = requireExternalId(externalId, SdkOperation.LOGIN) + if (id == null || !gate.awaitReady(SdkOperation.LOGIN) || isMissing(id, "login: externalId")) return@withContext - val context = loginHelper.switchUser(externalId!!, jwtBearerToken).context ?: return@withContext + val context = loginHelper.switchUser(id, jwtBearerToken).context ?: return@withContext loginHelper.enqueueLogin(context) } @@ -655,18 +656,18 @@ internal class OneSignalImpV2 : IOneSignal, return@withContext OneSignalResult.failure(ErrorCode.INVALID_ARGUMENT, it) } - if (!gate.awaitReady(SdkOperation.LOGIN)) { - return@withContext OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, gate.notReadyMessage("login")) - } - - if (isMissing(externalId, "login: externalId") || externalId == null) { - return@withContext OneSignalResult.failure( - ErrorCode.INVALID_ARGUMENT, - "login: externalId is required", - ) + val id = requireExternalId(externalId, SdkOperation.LOGIN) + if (id == null || !gate.awaitReady(SdkOperation.LOGIN) || isMissing(id, "login: externalId")) { + if (id == null) { + return@withContext OneSignalResult.failure(ErrorCode.INVALID_ARGUMENT, nullArgumentMessage(SdkOperation.LOGIN.label, "externalId")) + } + if (!gate.isSuccess) { + return@withContext OneSignalResult.failure(ErrorCode.NOT_INITIALIZED, gate.notReadyMessage("login")) + } + return@withContext OneSignalResult.failure(ErrorCode.INVALID_ARGUMENT, "login: externalId is required") } - val switched = loginHelper.switchUser(externalId, jwtBearerToken, profile) + val switched = loginHelper.switchUser(id, jwtBearerToken, profile) if (switched.context != null) { val completed = loginHelper.enqueueLogin(switched.context, profile) if (!completed.success) { @@ -683,10 +684,10 @@ internal class OneSignalImpV2 : IOneSignal, return@withContext OneSignalResult.failure(error) } return@withContext OneSignalResult.success( - loginHelper.loginData(externalId, profile, completed, switched.onesignalId), + loginHelper.loginData(id, profile, completed, switched.onesignalId), ) } - OneSignalResult.success(loginHelper.loginData(externalId, profile, fallbackOnesignalId = switched.onesignalId)) + OneSignalResult.success(loginHelper.loginData(id, profile, fallbackOnesignalId = switched.onesignalId)) } private fun loginFailureMessage(wait: OperationWaitResult): String = @@ -698,13 +699,11 @@ internal class OneSignalImpV2 : IOneSignal, ) = withContext(ioDispatcher) { Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})") - if (!gate.awaitReady(SdkOperation.UPDATE_USER_JWT)) return@withContext + val id = requireExternalId(externalId, SdkOperation.UPDATE_USER_JWT) + if (id == null || !gate.awaitReady(SdkOperation.UPDATE_USER_JWT)) return@withContext + if (isMissing(id, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) return@withContext - if (isMissing(externalId, "updateUserJwt: externalId") || isMissing(token, "updateUserJwt: token")) { - return@withContext - } - - jwtTokenStore.putJwt(externalId!!, token) + jwtTokenStore.putJwt(id, token) operationRepo.forceExecuteOperations() } @@ -718,7 +717,38 @@ internal class OneSignalImpV2 : IOneSignal, logoutHelper.enqueueLogout(context) } - private companion object { - const val MISSING_CONTEXT_MESSAGE = "initWithContext skipped: context is required." + private fun requireExternalId( + externalId: String?, + operation: SdkOperation, + ): String? { + if (externalId != null) return externalId + logNullArgument(operation.label, "externalId") + return null + } + + private fun logNullArgument( + method: String, + parameter: String, + ) { + Logging.error(nullArgumentMessage(method, parameter)) + } + + // isInitialized is false while init is in progress, which is the state a rejected init leaves behind. + private fun nullArgumentMessage( + method: String, + parameter: String, + ): String { + val problem = "$method: $parameter is null, so this call did not run." + return if (isInitialized) problem else "$problem The SDK is not initialized." + } + + private fun rejectedAppIdMessage(appId: String?): String { + val problem = + when { + appId == null -> "initWithContext: appId is null and no app ID is cached" + appId.isEmpty() -> "initWithContext: appId is empty and no app ID is cached" + else -> "initWithContext: appId contains a null byte and no app ID is cached" + } + return "$problem, so initialization did not finish. The SDK is not initialized." } } 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 index fbbb0d78e..f0186a9b6 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2Tests.kt @@ -1,9 +1,12 @@ package com.onesignal.internal +import com.onesignal.debug.ILogListener 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.kotest.matchers.string.shouldContain +import io.kotest.matchers.string.shouldNotContain class OneSignalImpV2Tests : FunSpec({ beforeAny { @@ -28,6 +31,29 @@ class OneSignalImpV2Tests : FunSpec({ os.isInitialized shouldBe false } + test("a null context or externalId is logged, and the log says the SDK is not initialized") { + val os = OneSignalImpV2() + val entries = mutableListOf() + val listener = ILogListener { entries.add(it.entry) } + Logging.addListener(listener) + Logging.logLevel = LogLevel.NONE + try { + os.initWithContext(null, "app-id") shouldBe false + os.login(null) + entries.single { "context is null" in it } shouldContain "The SDK is not initialized." + entries.single { "login: externalId is null" in it } shouldContain "The SDK is not initialized." + + entries.clear() + markInitialized(os) + os.login(null) + val afterInit = entries.single { "login: externalId is null" in it } + afterInit shouldContain "so this call did not run." + afterInit shouldNotContain "The SDK is not initialized." + } finally { + Logging.removeListener(listener) + } + } + test("login with a null or empty externalId after init does not switch user") { val os = OneSignalImpV2() markInitialized(os) From 1624e1b5c6157a30c6b3d288a5f08eb19d626d67 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 6 Nov 2026 11:05:21 -0600 Subject: [PATCH 5/8] test: [SDK-5538] match the cached appId warning assertion to the log The warning now starts with "Using the cached app ID". The test was still looking for the old lowercase phrase, so it counted zero warnings. Co-authored-by: Cursor --- .../com/onesignal/core/internal/application/SDKInitV2Tests.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 47142b83e..c1a3dd310 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 @@ -289,7 +289,7 @@ class SDKInitV2Tests : FunSpec({ 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 + events.count { it.level == LogLevel.WARN && it.entry.contains("Using the cached app ID") } shouldBe 1 } test("an ignored call logs one error per attempt, with the failure trace, and repeats at debug") { From a05ffa900eea7aabb2ddf885463997a3a5473b28 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 6 Nov 2026 11:37:40 -0600 Subject: [PATCH 6/8] feat: [SDK-5538] roll out never-crash behind sdk_never_crash_enabled The default stays on the legacy implementation. The cached flag selects the new path on the next cold start. A missing flag, an unreadable cache, or a locked device stays on the old path. Co-authored-by: Cursor --- .../internal/NeverCrashKillSwitch.kt | 30 ++++++++--------- .../internal/NeverCrashKillSwitchTests.kt | 32 +++++++++---------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt index 4478383dd..061b0ee78 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt @@ -12,17 +12,17 @@ import org.json.JSONArray /** * Chooses the [IOneSignal] implementation once per process, from the feature flags cached by the - * previous run, so a remote change applies on the next cold start. Any failure keeps the new behavior. + * previous run, so a remote change applies on the next cold start. Any failure keeps the legacy behavior. */ internal object NeverCrashKillSwitch { - // Present means off, so installs whose init never succeeds (and never fetch flags) keep the safe default. - const val FLAG_KEY = "sdk_never_crash_disabled" + // Absent means the legacy implementation, so the rollout stays off until Turbine sends the flag. + const val FLAG_KEY = "sdk_never_crash_enabled" // Local testing only: true forces the legacy implementation, false the new one, null reads the flag. private val localOverride: Boolean? = null @Volatile - var useLegacy: Boolean = false + var useLegacy: Boolean = true private set @Volatile @@ -33,10 +33,10 @@ internal object NeverCrashKillSwitch { @Suppress("TooGenericExceptionCaught") fun evaluate(context: Context) { useLegacy = localOverride ?: try { - isDisabledByCachedFlags(context) + !isEnabledByCachedFlags(context) } catch (t: Throwable) { - Logging.warn("OneSignal: could not read cached feature flags, keeping the default init behavior", t) - false + Logging.warn("OneSignal: could not read cached feature flags, keeping the legacy init behavior", t) + true } evaluated = true } @@ -44,17 +44,17 @@ internal object NeverCrashKillSwitch { fun create(): IOneSignal = when { !evaluated -> { - Logging.debug("OneSignal: kill switch not evaluated before first use, keeping the default init behavior") - OneSignalImpV2() - } - useLegacy -> { - Logging.warn("OneSignal: $FLAG_KEY is on, using the legacy init behavior") + Logging.debug("OneSignal: rollout flag not evaluated before first use, keeping the legacy init behavior") OneSignalImp() } - else -> OneSignalImpV2() + !useLegacy -> { + Logging.warn("OneSignal: $FLAG_KEY is on, using the never-crash init behavior") + OneSignalImpV2() + } + else -> OneSignalImp() } - internal fun isDisabledByCachedFlags(context: Context): Boolean { + internal fun isEnabledByCachedFlags(context: Context): Boolean { // Credential-encrypted prefs throw on access before the user unlocks the device. val keys = if (AndroidUtils.isAndroidUserUnlocked(context)) cachedFeatureKeys(context) else null return keys?.any { it.equals(FLAG_KEY, ignoreCase = true) } == true @@ -70,7 +70,7 @@ internal object NeverCrashKillSwitch { } internal fun resetForTest() { - useLegacy = false + useLegacy = true evaluated = false } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/NeverCrashKillSwitchTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/NeverCrashKillSwitchTests.kt index a94209cc7..7fa17eccb 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/NeverCrashKillSwitchTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/NeverCrashKillSwitchTests.kt @@ -38,48 +38,48 @@ class NeverCrashKillSwitchTests : FunSpec({ afterEach { NeverCrashKillSwitch.resetForTest() } - test("the cached kill switch selects the legacy implementation") { + test("the cached flag selects the never-crash implementation") { cacheFlags("other_flag", NeverCrashKillSwitch.FLAG_KEY) NeverCrashKillSwitch.evaluate(getApplicationContext()) - NeverCrashKillSwitch.useLegacy shouldBe true - NeverCrashKillSwitch.create().shouldBeInstanceOf() + NeverCrashKillSwitch.useLegacy shouldBe false + NeverCrashKillSwitch.create().shouldBeInstanceOf() } - test("the cached kill switch matches regardless of wire casing") { + test("the cached flag matches regardless of wire casing") { cacheFlags(NeverCrashKillSwitch.FLAG_KEY.uppercase()) NeverCrashKillSwitch.evaluate(getApplicationContext()) - NeverCrashKillSwitch.useLegacy shouldBe true + NeverCrashKillSwitch.useLegacy shouldBe false } - test("without the kill switch the never-crash implementation is used") { + test("without the flag the legacy implementation is used") { cacheFlags("other_flag") NeverCrashKillSwitch.evaluate(getApplicationContext()) - NeverCrashKillSwitch.useLegacy shouldBe false - NeverCrashKillSwitch.create().shouldBeInstanceOf() + NeverCrashKillSwitch.useLegacy shouldBe true + NeverCrashKillSwitch.create().shouldBeInstanceOf() } - test("a fresh install with nothing cached uses the never-crash implementation") { + test("a fresh install with nothing cached uses the legacy implementation") { NeverCrashKillSwitch.evaluate(getApplicationContext()) - NeverCrashKillSwitch.useLegacy shouldBe false + NeverCrashKillSwitch.useLegacy shouldBe true } - test("an unreadable cached config keeps the never-crash implementation without throwing") { + test("an unreadable cached config keeps the legacy implementation without throwing") { prefs().edit().putString(configKey, "{not json").commit() NeverCrashKillSwitch.evaluate(getApplicationContext()) NeverCrashKillSwitch.evaluated shouldBe true - NeverCrashKillSwitch.useLegacy shouldBe false + NeverCrashKillSwitch.useLegacy shouldBe true } - test("a locked device keeps the never-crash implementation") { + test("a locked device keeps the legacy implementation") { cacheFlags(NeverCrashKillSwitch.FLAG_KEY) mockkObject(AndroidUtils) try { @@ -87,13 +87,13 @@ class NeverCrashKillSwitchTests : FunSpec({ NeverCrashKillSwitch.evaluate(getApplicationContext()) - NeverCrashKillSwitch.useLegacy shouldBe false + NeverCrashKillSwitch.useLegacy shouldBe true } finally { unmockkObject(AndroidUtils) } } - test("first use before the switch is evaluated uses the never-crash implementation") { - NeverCrashKillSwitch.create().shouldBeInstanceOf() + test("first use before the flag is evaluated uses the legacy implementation") { + NeverCrashKillSwitch.create().shouldBeInstanceOf() } }) From b7f98782b7bb730a396ae5cc372a18c3dec65903 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 6 Nov 2026 11:50:34 -0600 Subject: [PATCH 7/8] feat: [SDK-5538] read the never-crash flag from FeatureFlag The rollout key is FeatureFlag.SDK_NEVER_CRASH_ENABLED. The submodule pin on this stack includes that catalog entry. Co-authored-by: Cursor --- OneSignal-KMP-SDK | 2 +- .../main/java/com/onesignal/internal/NeverCrashKillSwitch.kt | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index ee61d4673..89e5f2776 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit ee61d4673265876e5cab6328b7a0745f79b7a5aa +Subproject commit 89e5f27766225f42f1c45d5c8918eb11ee5e5bc0 diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt index 061b0ee78..c822e24de 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/NeverCrashKillSwitch.kt @@ -8,6 +8,7 @@ import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys import com.onesignal.core.internal.preferences.PreferenceStores import com.onesignal.debug.internal.logging.Logging +import com.onesignal.features.FeatureFlag import org.json.JSONArray /** @@ -16,7 +17,7 @@ import org.json.JSONArray */ internal object NeverCrashKillSwitch { // Absent means the legacy implementation, so the rollout stays off until Turbine sends the flag. - const val FLAG_KEY = "sdk_never_crash_enabled" + val FLAG_KEY: String = FeatureFlag.SDK_NEVER_CRASH_ENABLED.key // Local testing only: true forces the legacy implementation, false the new one, null reads the flag. private val localOverride: Boolean? = null From 5b2ad36f3c8c59de206f0f89b17f07e3da44a623 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Fri, 6 Nov 2026 12:06:03 -0600 Subject: [PATCH 8/8] fix: [SDK-5538] keep a replayed add ahead of a concurrent remove A remove during replay was applied before the add already copied into the batch, and init's own log listener could block the thread that still had to dispatch the job. Co-authored-by: Cursor --- .../onesignal/internal/DeferredManagers.kt | 80 ++++++++++++++----- .../java/com/onesignal/internal/InitGate.kt | 5 +- .../com/onesignal/internal/OneSignalImpV2.kt | 24 +++--- .../application/OneSignalImpParityTests.kt | 24 ++++-- .../internal/DeferredManagersCoverageTests.kt | 55 +++++++++++++ .../internal/DeferredManagersTests.kt | 33 ++++++++ .../internal/OneSignalImpV2ApiSweepTests.kt | 3 +- .../internal/OneSignalImpV2FaultTests.kt | 39 ++++++++- 8 files changed, 223 insertions(+), 40 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/DeferredManagers.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/DeferredManagers.kt index e954ca767..f909d8e12 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/DeferredManagers.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/DeferredManagers.kt @@ -41,6 +41,11 @@ internal open class DeferredManager( private val pending = mutableListOf>() private var replayed = false + // Set for the whole drain. Other threads enqueue instead of touching the real manager, so a remove + // cannot land before the add that was already copied into the batch being applied. + private var replaying = false + private var replayThread: Thread? = null + // Kept after replay: if init then fails, or a replayed listener calls back in, the gate is not ready // but this manager already holds the replayed registrations, so later ones must reach it too. private var replayedTo: T? = null @@ -55,6 +60,7 @@ internal open class DeferredManager( ) = forwardOrQueue(member, call) { pending.size < MAX_PENDING && pending.add(Pending(member, target, call)) } // Before replay the real manager holds nothing, so a removal only has to cancel the queued add. + // Once that add has been copied into the batch in flight, the removal itself is queued behind it. protected fun unregisterOrCancel( member: String, addMember: String, @@ -62,8 +68,14 @@ internal open class DeferredManager( call: (T) -> Unit, ) = forwardOrQueue(member, call) { val index = pending.indexOfLast { it.member == addMember && it.target === target } - if (index >= 0) pending.removeAt(index) - true + if (index >= 0) { + pending.removeAt(index) + true + } else if (replaying) { + pending.size < MAX_PENDING && pending.add(Pending(member, target, call)) + } else { + false + } } // [onQueued] runs under the queue lock, so a getter cache it sets always matches what replays. @@ -85,13 +97,11 @@ internal open class DeferredManager( call: (T) -> Unit, enqueue: () -> Boolean, ) { - source.peek()?.let { - call(it) - return - } val (queued, target) = synchronized(pending) { - if (replayed) false to replayedTo else enqueue() to null + val applying = replaying && Thread.currentThread() === replayThread + val finished = replayed && !replaying + if (applying || finished) false to (replayedTo ?: source.peek()) else enqueue() to null } when { queued -> source.logQueued(member) @@ -100,20 +110,52 @@ internal open class DeferredManager( } } - @Suppress("TooGenericExceptionCaught") open fun replay(real: T) { - val calls = - synchronized(pending) { - replayed = true - replayedTo = real - pending.toList().also { pending.clear() } - } - for (entry in calls) { - try { - entry.call(real) - } catch (e: Exception) { - Logging.error("Replaying '${entry.member}' threw; that queued call is lost", e) + if (!beginReplay(real)) return + try { + while (true) { + val batch = takeBatch() + if (batch.isEmpty()) return + batch.forEach { deliver(it, real) } } + } finally { + abandonReplay() + } + } + + private fun beginReplay(real: T): Boolean = + synchronized(pending) { + if (replayed || replaying) return@synchronized false + replaying = true + replayThread = Thread.currentThread() + replayedTo = real + true + } + + private fun takeBatch(): List> = + synchronized(pending) { + if (pending.isNotEmpty()) return@synchronized pending.toList().also { pending.clear() } + replaying = false + replayThread = null + replayed = true + emptyList() + } + + @Suppress("TooGenericExceptionCaught") + private fun deliver(entry: Pending, real: T) { + try { + entry.call(real) + } catch (e: Exception) { + Logging.error("Replaying '${entry.member}' threw; that queued call is lost", e) + } + } + + private fun abandonReplay() { + synchronized(pending) { + if (!replaying) return@synchronized + replaying = false + replayThread = null + replayed = true } } 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 043b9632e..3039f8f26 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 @@ -45,7 +45,8 @@ internal class InitGate { 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 { + // [finishIfUnfinished] is false only for the caller's pre-dispatch work, which completes itself. + fun runInit(finishIfUnfinished: Boolean = true, block: () -> Boolean): Boolean { val current = Thread.currentThread() val owned = synchronized(lock) { attempt } initThread.set(current) @@ -56,7 +57,7 @@ internal class InitGate { addFailureCause(e) return false } finally { - failIfUnfinished(owned) + if (finishIfUnfinished) failIfUnfinished(owned) // A retry woken by this attempt's FAILED may already have claimed the slot. initThread.compareAndSet(current, null) } 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 bd84ed95a..3220641d5 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 @@ -385,15 +385,21 @@ internal class OneSignalImpV2 : IOneSignal, return true } - // 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) { - gate.addFailureCause(e) + // Claim this thread for the pre-dispatch work. A log listener that touches the SDK would + // otherwise wait for a job this thread has not launched. A throw must end FAILED. + val startedOk = + gate.runInit(finishIfUnfinished = false) { + try { + ensureApplicationServiceStarted(context) + true + } catch (e: Exception) { + gate.addFailureCause(e) + false + } + } + if (!startedOk) { gate.complete(InitState.FAILED) - Logging.error("initWithContext failed to start the application service", e) + Logging.error("initWithContext failed to start the application service", gate.failure) return false } @@ -414,9 +420,9 @@ internal class OneSignalImpV2 : IOneSignal, 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) + Logging.error("initWithContext could not be scheduled; marking init FAILED", failure) } /** diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt index f93648090..18129d729 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/application/OneSignalImpParityTests.kt @@ -51,7 +51,7 @@ class OneSignalImpParityTests : FunSpec({ val warm = if (this is OneSignalImpV2) OneSignalImpV2() else OneSignalImp() Logging.logLevel = LogLevel.NONE warm.initWithContext(context, appId) - warm.awaitInit() + warm.awaitInit() shouldBe true val prefs = context.getSharedPreferences("OneSignal", Context.MODE_PRIVATE) val deadline = System.currentTimeMillis() + 5_000 while (prefs.all.values.none { it.toString().contains(appId) } && System.currentTimeMillis() < deadline) Thread.sleep(10) @@ -82,7 +82,9 @@ class OneSignalImpParityTests : FunSpec({ test("init then reading every top-level value") { parity { os, context -> os.initWithContext(context, "appId") - os.awaitInit() + os.awaitInit() shouldBe true + os.isInitialized shouldBe true + os.appIdInConfig() shouldBe "appId" listOf( os.isInitialized, os.user.onesignalId.isNotEmpty(), @@ -104,7 +106,8 @@ class OneSignalImpParityTests : FunSpec({ os.disableGMSMissingPrompt = true val beforeInit = listOf(os.consentRequired, os.consentGiven, os.disableGMSMissingPrompt) os.initWithContext(context, "appId") - os.awaitInit() + os.awaitInit() shouldBe true + os.isInitialized shouldBe true val model = (os as IServiceProvider).getService(ConfigModelStore::class.java).model beforeInit + listOf(os.consentRequired, os.consentGiven, os.disableGMSMissingPrompt, model.consentRequired, model.consentGiven, model.disableGMSMissingPrompt) } @@ -113,7 +116,9 @@ class OneSignalImpParityTests : FunSpec({ test("a second initWithContext after success keeps the first app ID") { parity { os, context -> os.initWithContext(context, "appId") - os.awaitInit() + os.awaitInit() shouldBe true + os.isInitialized shouldBe true + os.appIdInConfig() shouldBe "appId" val second = os.initWithContext(context, "otherAppId") Thread.sleep(100) listOf(second, os.isInitialized, os.appIdInConfig()) @@ -123,9 +128,11 @@ class OneSignalImpParityTests : FunSpec({ test("login, tags and logout after init") { parity { os, context -> os.initWithContext(context, "appId") - os.awaitInit() + os.awaitInit() shouldBe true + os.isInitialized shouldBe true os.login("ext-1") val afterLogin = os.awaitExternalId("ext-1") + afterLogin shouldBe "ext-1" os.user.addTags(mapOf("a" to "1", "b" to "2")) os.user.removeTag("b") val tags = os.user.getTags() @@ -140,7 +147,9 @@ class OneSignalImpParityTests : FunSpec({ os.cacheAppId(context, "cachedAppId") os.initWithContext(context, "") - os.awaitInit() + os.awaitInit() shouldBe true + os.isInitialized shouldBe true + os.appIdInConfig() shouldBe "cachedAppId" listOf(os.isInitialized, os.appIdInConfig()) } } @@ -151,6 +160,9 @@ class OneSignalImpParityTests : FunSpec({ val result = kotlinx.coroutines.runBlocking { os.initWithContextSuspend(context, null) } + result shouldBe true + os.isInitialized shouldBe true + os.appIdInConfig() shouldBe "cachedAppId" listOf(result, os.isInitialized, os.appIdInConfig()) } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt index 271979513..3bb859af2 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersCoverageTests.kt @@ -84,6 +84,46 @@ private val replayCases: List> = ) private val replayedMember = Regex("^add\\w*(Listener|Observer)$") +private val removedMember = Regex("^remove\\w*(Listener|Observer)$") + +private class RemoveCase( + val type: KClass, + val member: String, + val standIn: (ManagerSource) -> DeferredManager, + val add: T.() -> Unit, + val remove: T.() -> Unit, +) { + fun run() { + val real = mockkClass(type, relaxed = true) + val deferred = standIn(NotReadySource()) + @Suppress("UNCHECKED_CAST") + (deferred as T).add() + @Suppress("UNCHECKED_CAST") + (deferred as T).remove() + deferred.replay(real) + verify(exactly = 0) { real.add() } + } +} + +private inline fun removal( + noinline standIn: (ManagerSource) -> DeferredManager, + add: KFunction2, + remove: KFunction2, +): RemoveCase { + val listener = mockk(relaxed = true) + return RemoveCase(T::class, remove.name, standIn, { add(this, listener) }, { remove(this, listener) }) +} + +private val removeCases: List> = + listOf( + removal(::DeferredUserManager, IUserManager::addObserver, IUserManager::removeObserver), + removal(::DeferredPushSubscription, IPushSubscription::addObserver, IPushSubscription::removeObserver), + removal(::DeferredNotificationsManager, INotificationsManager::addPermissionObserver, INotificationsManager::removePermissionObserver), + removal(::DeferredNotificationsManager, INotificationsManager::addForegroundLifecycleListener, INotificationsManager::removeForegroundLifecycleListener), + removal(::DeferredNotificationsManager, INotificationsManager::addClickListener, INotificationsManager::removeClickListener), + removal(::DeferredInAppMessagesManager, IInAppMessagesManager::addLifecycleListener, IInAppMessagesManager::removeLifecycleListener), + removal(::DeferredInAppMessagesManager, IInAppMessagesManager::addClickListener, IInAppMessagesManager::removeClickListener), + ) private fun KCallable<*>.signature() = "$name(${parameters.drop(1).joinToString { it.type.toString() }})" @@ -114,4 +154,19 @@ class DeferredManagersCoverageTests : FunSpec({ test("each queued registration or setting reaches the real manager on replay") { replayCases.forEach { case -> withClue("${case.type.simpleName}.${case.member}") { case.run() } } } + + test("every remove has an add-then-remove case") { + val covered = removeCases.map { "${it.type.simpleName}.${it.member}" }.toSet() + val uncovered = + standIns.keys.flatMap { iface -> + iface.apiMembers().filter { removedMember.matches(it.name) } + .map { "${iface.simpleName}.${it.name}" } + .filter { it !in covered } + } + withClue("pair each add with an add-then-remove that expects the real manager to see neither") { uncovered shouldBe emptyList() } + } + + test("a remove before replay cancels the queued add") { + removeCases.forEach { case -> withClue("${case.type.simpleName}.${case.member}") { case.run() } } + } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersTests.kt index c4d451273..06d53272d 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/DeferredManagersTests.kt @@ -26,6 +26,9 @@ import io.mockk.mockk import io.mockk.verify import io.mockk.verifyOrder import kotlinx.coroutines.runBlocking +import java.util.Collections +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit private class FakeSource(private val real: T) : ManagerSource { var ready = false @@ -330,6 +333,36 @@ class DeferredManagersTests : FunSpec({ deferred.paused shouldBe true } + test("a remove that arrives while its add is being replayed runs after that add") { + val real = mockk(relaxed = true) + val deferred = DeferredNotificationsManager(FakeSource(real)) + val listener = mockk() + val addEntered = CountDownLatch(1) + val removeFinished = CountDownLatch(1) + val order = Collections.synchronizedList(mutableListOf()) + every { real.addClickListener(listener) } answers { + addEntered.countDown() + check(removeFinished.await(5, TimeUnit.SECONDS)) + order.add("add-done") + } + every { real.removeClickListener(listener) } answers { order.add("remove") } + deferred.addClickListener(listener) + + val remover = + Thread { + check(addEntered.await(5, TimeUnit.SECONDS)) + deferred.removeClickListener(listener) + removeFinished.countDown() + } + remover.isDaemon = true + remover.start() + deferred.replay(real) + remover.join(5_000) + + remover.isAlive shouldBe false + order shouldBe listOf("add-done", "remove") + } + test("a listener that registers another while it is replayed reaches the real manager without waiting") { val real = mockk(relaxed = true) every { real.pushSubscription } returns mockk(relaxed = true) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt index e12ad51cc..982c2fca8 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2ApiSweepTests.kt @@ -29,8 +29,9 @@ import kotlin.reflect.KType import kotlin.reflect.full.callSuspend private fun dummy(type: KType): Any? { - if (type.isMarkedNullable) return null val kClass = type.classifier as KClass<*> + // A nullable string still has to reach the gate. A nullable Context would start a real init. + if (type.isMarkedNullable && kClass != String::class) return null return when { kClass == String::class -> "sample" kClass == Boolean::class -> false 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 index 0980494d0..7494fe196 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpV2FaultTests.kt @@ -96,9 +96,14 @@ class OneSignalImpV2FaultTests : FunSpec({ ioExecutor.get()!!.queue.remainingCapacity() shouldBe 0 val os = quietOneSignal() - os.initWithContext(getApplicationContext(), "appId") - - awaitTerminal(os) shouldBe InitState.FAILED + val listener = ILogListener { if (it.entry.contains("could not be scheduled")) os.user.onesignalId } + Logging.addListener(listener) + try { + os.initWithContext(getApplicationContext(), "appId") + awaitTerminal(os) shouldBe InitState.FAILED + } finally { + Logging.removeListener(listener) + } onDaemonThread { os.user.onesignalId } shouldBe "" os.isInitialized shouldBe false } finally { @@ -112,6 +117,34 @@ class OneSignalImpV2FaultTests : FunSpec({ awaitTerminal(retry) shouldBe InitState.SUCCESS } + test("a log listener that touches the SDK while init is starting does not block the dispatch") { + val os = quietOneSignal() + val listener = ILogListener { if (it.entry.contains("ApplicationService.init")) os.user.onesignalId } + Logging.addListener(listener) + try { + onDaemonThread { os.initWithContext(getApplicationContext(), "appId") } + awaitTerminal(os) shouldBe InitState.SUCCESS + } finally { + Logging.removeListener(listener) + } + } + + test("a LinkageError while starting the application service marks init FAILED") { + val os = quietOneSignal() + val listener = + ILogListener { + if (it.entry.contains("ApplicationService.init")) throw NoClassDefFoundError("missing") + } + Logging.addListener(listener) + try { + onDaemonThread { os.initWithContext(getApplicationContext(), "appId") } shouldBe false + os.gate.state shouldBe InitState.FAILED + onDaemonThread { os.user.onesignalId } shouldBe "" + } finally { + Logging.removeListener(listener) + } + } + 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()