feat: [SDK-5170] register push before FCM credentials upload

.gitignore

@@ -7,6 +7,7 @@ build/
 local.properties
 OneSignalSDK/local.properties
 examples/demo/local.properties
+google-services.json
 
 # macOS
 .DS_Store

OneSignalSDK/build.gradle

@@ -10,7 +10,7 @@ buildscript {
         ]
         androidGradlePluginVersion = '8.8.2'
         detektVersion = '1.21.0'
-        googleServicesGradlePluginVersion = '4.3.10'
+        googleServicesGradlePluginVersion = '4.4.2'
         huaweiAgconnectVersion = '1.9.1.304'
         huaweiHMSPushVersion = '6.3.0.304'
         huaweiHMSLocationVersion = '4.0.0.300'

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt

@@ -16,6 +16,8 @@ import com.onesignal.user.internal.subscriptions.ISubscriptionManager
 import com.onesignal.user.internal.subscriptions.SubscriptionModel
 import com.onesignal.user.internal.subscriptions.SubscriptionStatus
 import com.onesignal.user.subscriptions.ISubscription
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 
 /**
  * The device registration listener will subscribe to events and at the appropriate time will
@@ -33,6 +35,8 @@ internal class DeviceRegistrationListener(
     ISingletonModelStoreChangeHandler<ConfigModel>,
     IPermissionObserver,
     ISubscriptionChangedHandler {
+    private val pushTokenMutex = Mutex()
+
     override fun start() {
         _configModelStore.subscribe(this)
         _notificationsManager.addPermissionObserver(this)
@@ -94,12 +98,14 @@ internal class DeviceRegistrationListener(
         val pushSubscription = _subscriptionManager.subscriptions.push
 
         suspendifyOnIO {
-            val pushTokenAndStatus = _pushTokenManager.retrievePushToken()
-            val permission = _notificationsManager.permission
-            _subscriptionManager.addOrUpdatePushSubscriptionToken(
-                pushTokenAndStatus.token,
-                if (permission) pushTokenAndStatus.status else SubscriptionStatus.NO_PERMISSION,
-            )
+            pushTokenMutex.withLock {
+                val pushTokenAndStatus = _pushTokenManager.retrievePushToken()
+                val permission = _notificationsManager.permission
+                _subscriptionManager.addOrUpdatePushSubscriptionToken(
+                    pushTokenAndStatus.token,
+                    if (permission) pushTokenAndStatus.status else SubscriptionStatus.NO_PERMISSION,
+                )
+            }
         }
     }
 

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/pushtoken/PushTokenManager.kt

@@ -33,19 +33,10 @@ internal class PushTokenManager(
                 if (registerResult.status.value == SubscriptionStatus.SUBSCRIBED.value) {
                     pushTokenStatus = registerResult.status
                 } else if (registerResult.status.value < SubscriptionStatus.SUBSCRIBED.value) {
-                    // Only allow errored statuses if we have never gotten a token. This ensures the
-                    // device will not later be marked unsubscribed due to any inconsistencies returned
-                    // by Google Play services. Also do not override a config error status if we got a
-                    // runtime error
-                    if (pushToken == null &&
-                        (
-                            pushTokenStatus == SubscriptionStatus.NO_PERMISSION ||
-                                pushStatusRuntimeError(pushTokenStatus)
-                            )
-                    ) {
+                    if (shouldUpdateErrorStatus(registerResult.status)) {
                         pushTokenStatus = registerResult.status
                     }
-                } else if (pushStatusRuntimeError(pushTokenStatus)) {
+                } else if (pushTokenStatus.isRetryableTokenError) {
                     pushTokenStatus = registerResult.status
                 }
 
@@ -56,7 +47,10 @@ internal class PushTokenManager(
         return PushTokenResponse(pushToken, pushTokenStatus)
     }
 
-    private fun pushStatusRuntimeError(status: SubscriptionStatus): Boolean {
-        return status.value < -6
-    }
+    private fun shouldUpdateErrorStatus(newStatus: SubscriptionStatus): Boolean =
+        when {
+            !newStatus.isRetryableTokenError -> true
+            pushToken != null -> false
+            else -> pushTokenStatus == SubscriptionStatus.NO_PERMISSION || pushTokenStatus.isRetryableTokenError
+        }
 }

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/registration/impl/PushRegistratorAbstractGoogle.kt

@@ -51,6 +51,8 @@ internal abstract class PushRegistratorAbstractGoogle(
     @Throws(ExecutionException::class, InterruptedException::class, IOException::class)
     abstract suspend fun getToken(senderId: String): String
 
+    protected open fun resolveSenderId(configuredSenderId: String?): String? = configuredSenderId
+
     override suspend fun registerForPush(): IPushRegistrator.RegisterResult {
         if (!_configModelStore.model.isInitializedWithRemote) {
             return IPushRegistrator.RegisterResult(null, SubscriptionStatus.FIREBASE_FCM_INIT_ERROR)
@@ -61,7 +63,8 @@ internal abstract class PushRegistratorAbstractGoogle(
             return IPushRegistrator.RegisterResult(null, SubscriptionStatus.MISSING_FIREBASE_FCM_LIBRARY)
         }
 
-        return if (!isValidProjectNumber(_configModelStore.model.googleProjectNumber)) {
+        val senderId = resolveSenderId(_configModelStore.model.googleProjectNumber)
+        return if (senderId == null || !isValidProjectNumber(senderId)) {
             Logging.warn(
                 "Missing Google Project number!\nPlease enter a Google Project number / Sender ID on under App Settings > Android > Configuration on the OneSignal dashboard.",
             )
@@ -70,7 +73,7 @@ internal abstract class PushRegistratorAbstractGoogle(
                 SubscriptionStatus.INVALID_FCM_SENDER_ID,
             )
         } else {
-            internalRegisterForPush(_configModelStore.model.googleProjectNumber!!)
+            internalRegisterForPush(senderId)
         }
     }
 
@@ -131,6 +134,8 @@ internal abstract class PushRegistratorAbstractGoogle(
                 registrationId,
                 SubscriptionStatus.SUBSCRIBED,
             )
+        } catch (e: FCMSenderIdMismatchException) {
+            return invalidSenderIdResult(e)
         } catch (e: IOException) {
             val pushStatus: SubscriptionStatus = pushStatusFromThrowable(e)
             val exceptionMessage: String? = AndroidUtils.getRootCauseMessage(e)
@@ -167,6 +172,14 @@ internal abstract class PushRegistratorAbstractGoogle(
         return null
     }
 
+    private fun invalidSenderIdResult(exception: FCMSenderIdMismatchException): IPushRegistrator.RegisterResult {
+        Logging.warn("FCM sender ID mismatch", exception)
+        return IPushRegistrator.RegisterResult(
+            null,
+            SubscriptionStatus.INVALID_FCM_SENDER_ID,
+        )
+    }
+
     private fun pushStatusFromThrowable(throwable: Throwable): SubscriptionStatus {
         val exceptionMessage: String? = AndroidUtils.getRootCauseMessage(throwable)
         return if (throwable is IOException) {
@@ -180,21 +193,7 @@ internal abstract class PushRegistratorAbstractGoogle(
         }
     }
 
-    private fun isValidProjectNumber(senderId: String?): Boolean {
-        val isProjectNumberValidFormat: Boolean =
-            try {
-                senderId!!.toFloat()
-                true
-            } catch (t: Throwable) {
-                false
-            }
-
-        if (!isProjectNumberValidFormat) {
-            return false
-        }
-
-        return true
-    }
+    private fun isValidProjectNumber(senderId: String): Boolean = senderId.toFloatOrNull() != null
 
     companion object {
         private const val REGISTRATION_RETRY_COUNT = 5

OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/registration/impl/PushRegistratorFCM.kt

@@ -40,6 +40,7 @@ internal class PushRegistratorFCM(
     private val apiKey: String
 
     private var firebaseApp: FirebaseApp? = null
+    private var firebaseAppSenderId: String? = null
     override val providerName: String
         get() = "FCM"
 
@@ -54,15 +55,19 @@ internal class PushRegistratorFCM(
 
     @Throws(ExecutionException::class, InterruptedException::class)
     override suspend fun getToken(senderId: String): String {
+        val hostApp = hostFirebaseApp()
         return FCMTokenProvider.getToken(
             senderId = senderId,
             installationIdEnabled = ::installationIdEnabled,
             legacyToken = { getLegacyToken(senderId) },
             installationIdApiAvailable = { FCMTokenProvider.hasRegisterMethod(FirebaseMessaging::class.java) },
-            installationIdRegistration = ::defaultAppRegistration,
+            installationIdRegistration = { hostApp?.let(::installationIdRegistration) },
         )
     }
 
+    override fun resolveSenderId(configuredSenderId: String?): String? =
+        configuredSenderId ?: hostFirebaseApp()?.let(::hostFirebaseSenderId)
+
     private fun getLegacyToken(senderId: String): Task<String> {
         val app = initFirebaseApp(senderId)
         // We use the named app's FirebaseMessaging instance instead of FirebaseMessaging.getInstance()
@@ -82,41 +87,56 @@ internal class PushRegistratorFCM(
     // Installation ID registration is rejected unless the sender id, app id, and api key all belong
     //   to the same Firebase project. Our own FirebaseApp pairs the app's sender id with OneSignal's
     //   shared project credentials, so only the host app's default FirebaseApp can be used for it.
-    private fun defaultAppRegistration(): FCMTokenProvider.InstallationIdRegistration? {
-        val defaultApp =
-            FirebaseApp
-                .getApps(_applicationService.appContext)
-                .firstOrNull { it.name == FirebaseApp.DEFAULT_APP_NAME } ?: return null
-        val defaultSenderId =
-            FCMTokenProvider.defaultSenderId(
-                defaultApp.options.gcmSenderId,
-                defaultApp.options.applicationId,
-            )
-
+    private fun installationIdRegistration(hostApp: FirebaseApp): FCMTokenProvider.InstallationIdRegistration {
         return FCMTokenProvider.InstallationIdRegistration(
-            senderId = defaultSenderId,
-            register = { FCMTokenProvider.invokeRegister(defaultApp.get(FirebaseMessaging::class.java)) },
-            installationId = { FirebaseInstallations.getInstance(defaultApp).id },
+            senderId = hostFirebaseSenderId(hostApp),
+            register = { FCMTokenProvider.invokeRegister(hostApp.get(FirebaseMessaging::class.java)) },
+            installationId = { FirebaseInstallations.getInstance(hostApp).id },
         )
     }
 
-    private fun initFirebaseApp(senderId: String): FirebaseApp {
-        firebaseApp?.let { return it }
+    private fun hostFirebaseApp(): FirebaseApp? =
+        FirebaseApp
+            .getApps(_applicationService.appContext)
+            .firstOrNull { it.name == FirebaseApp.DEFAULT_APP_NAME }
+
+    private fun hostFirebaseSenderId(hostApp: FirebaseApp): String? =
+        FCMTokenProvider.firebaseAppSenderId(
+            hostApp.options.gcmSenderId,
+            hostApp.options.applicationId,
+        )
+
+    /**
+     * @param resolvedSenderId sender ID from the dashboard configuration, or the host Firebase app
+     * sender ID when the dashboard has not provided one.
+     */
+    private fun initFirebaseApp(resolvedSenderId: String): FirebaseApp {
+        firebaseApp?.let {
+            if (firebaseAppSenderId == resolvedSenderId) return it
+            it.delete()
+            firebaseApp = null
+            firebaseAppSenderId = null
+        }
         val firebaseOptions =
             FirebaseOptions
                 .Builder()
-                .setGcmSenderId(senderId)
+                .setGcmSenderId(resolvedSenderId)
                 .setApplicationId(appId)
                 .setApiKey(apiKey)
                 .setProjectId(projectId)
                 .build()
         return FirebaseApp.initializeApp(_applicationService.appContext, firebaseOptions, FCM_APP_NAME)
-            .also { firebaseApp = it }
+            .also {
+                firebaseApp = it
+                firebaseAppSenderId = resolvedSenderId
+            }
     }
 }
 
+internal class FCMSenderIdMismatchException(message: String) : IllegalStateException(message)
+
 internal object FCMTokenProvider {
-    fun defaultSenderId(
+    fun firebaseAppSenderId(
         senderId: String?,
         applicationId: String,
     ): String? =
@@ -178,20 +198,24 @@ internal object FCMTokenProvider {
             )
         }
 
-        if (registration.senderId != senderId) {
-            throw IllegalStateException(
-                "Firebase Installation ID registration is enabled ($optedIn) but the default " +
-                    "FirebaseApp uses sender id ${registration.senderId}, while OneSignal is " +
-                    "configured with sender id $senderId. Point both at the same Firebase project, " +
-                    "or set firebase_messaging_installation_id_enabled to false in your manifest " +
-                    "to keep using the legacy FCM token API.",
-            )
-        }
+        validateSenderId(senderId, registration.senderId)
 
         await(registration.register())
         return await(registration.installationId())
     }
 
+    fun validateSenderId(
+        senderId: String,
+        firebaseAppSenderId: String?,
+    ) {
+        if (firebaseAppSenderId == senderId) return
+
+        throw FCMSenderIdMismatchException(
+            "The default FirebaseApp uses sender id $firebaseAppSenderId, while OneSignal is " +
+                "configured with sender id $senderId. Point both at the same Firebase project.",
+        )
+    }
+
     /**
      * Calls register() reflectively. FirebaseMessaging.register was added in firebase-messaging
      * 25.1.0. This module compiles against the preferred 24.0.0, but the non-strict Gradle

OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListenerTests.kt

@@ -1,5 +1,8 @@
 package com.onesignal.notifications.internal.listeners
 
+import com.onesignal.common.modeling.ModelChangeTags
+import com.onesignal.common.threading.suspendifyOnIO
+import com.onesignal.core.internal.config.ConfigModel
 import com.onesignal.core.internal.config.ConfigModelStore
 import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
@@ -14,11 +17,16 @@ import com.onesignal.user.internal.subscriptions.SubscriptionModel
 import com.onesignal.user.internal.subscriptions.SubscriptionStatus
 import com.onesignal.user.internal.subscriptions.SubscriptionType
 import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.shouldBe
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CoroutineStart
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
 
 private const val NEW_TOKEN = "new-token"
 
@@ -270,4 +278,49 @@ class DeviceRegistrationListenerTests : FunSpec({
             )
         }
     }
+
+    test("serializes overlapping startup and hydration token retrievals") {
+        val harness =
+            Harness(
+                permission = true,
+                pushModel = uninitializedPushModel(),
+            )
+        val blocks = mutableListOf<suspend () -> Unit>()
+        every { suspendifyOnIO(any<suspend () -> Unit>()) } answers {
+            blocks += firstArg<suspend () -> Unit>()
+        }
+        val firstStarted = CompletableDeferred<Unit>()
+        val finishFirst = CompletableDeferred<Unit>()
+        var requestCount = 0
+        coEvery { harness.pushTokenManager.retrievePushToken() } coAnswers {
+            if (requestCount++ == 0) {
+                firstStarted.complete(Unit)
+                finishFirst.await()
+                PushTokenResponse("startup-token", SubscriptionStatus.SUBSCRIBED)
+            } else {
+                PushTokenResponse("hydrated-token", SubscriptionStatus.SUBSCRIBED)
+            }
+        }
+        val updates = mutableListOf<String?>()
+        every {
+            harness.subscriptionManager.addOrUpdatePushSubscriptionToken(any(), any())
+        } answers {
+            updates += firstArg<String?>()
+        }
+
+        harness.listener.start()
+        harness.listener.onModelReplaced(mockk<ConfigModel>(relaxed = true), ModelChangeTags.HYDRATE)
+
+        val startup = async(Dispatchers.Default) { blocks[0]() }
+        firstStarted.await()
+        val hydration =
+            async(Dispatchers.Default, start = CoroutineStart.UNDISPATCHED) {
+                blocks[1]()
+            }
+        finishFirst.complete(Unit)
+        startup.await()
+        hydration.await()
+
+        updates shouldBe listOf("startup-token", "hydrated-token")
+    }
 })