test: [SDK-5538] guard stand-ins and the public API against drift

OneSignalSDK/detekt/detekt-baseline-core.xml

@@ -537,7 +537,7 @@
     <ID>UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun isInBadState(): Boolean</ID>
     <ID>UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun recover()</ID>
     <ID>UndocumentedPublicFunction:IMigrationRecovery.kt$IMigrationRecovery$fun recoveryMessage(): String</ID>
-    <ID>UndocumentedPublicFunction:IOneSignal.kt$IOneSignal$fun login(externalId: String)</ID>
+    <ID>UndocumentedPublicFunction:IOneSignal.kt$IOneSignal$fun login(externalId: String?)</ID>
     <ID>UndocumentedPublicFunction:IOperationRepo.kt$IOperationRepo$fun forceExecuteOperations()</ID>
     <ID>UndocumentedPublicFunction:IOperationRepo.kt$IOperationRepo$suspend fun awaitInitialized()</ID>
     <ID>UndocumentedPublicFunction:IOperationRepo.kt$inline fun &lt;reified T : Operation> IOperationRepo.containsInstanceOf(): Boolean</ID>

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,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. The log says the SDK is not initialized.
      */
-    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. 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)
      */
     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. 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(
-        externalId: String,
+        externalId: String?,
         token: String,
     )
 
@@ -166,9 +166,9 @@ interface IOneSignal {
     /**
      * Initialize the OneSignal SDK, suspend until initialization is completed
      *
-     * @param context The Android context the SDK should use. 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,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. 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)
      */
     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. 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(
-        externalId: String,
+        externalId: String?,
         token: String,
     )
 }

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,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. A null value's log says the SDK is not initialized when it is not.
      */
     @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. 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)
      */
     @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. 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
     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. 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)
      */
     @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. 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
     suspend fun updateUserJwtSuspend(
-        externalId: String,
+        externalId: String?,
         token: String,
     ) {
         oneSignal.updateUserJwtSuspend(externalId, token)

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].
  */

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/DeferredManagers.kt

@@ -41,6 +41,11 @@ internal open class DeferredManager<T : Any>(
     private val pending = mutableListOf<Pending<T>>()
     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,15 +60,22 @@ internal open class DeferredManager<T : Any>(
     ) = 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,
         target: Any,
         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<T : Any>(
         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<T : Any>(
         }
     }
 
-    @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<Pending<T>> =
+        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<T>, 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
         }
     }
 

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)
         }