Merge pull request #2061 from OneSignal/improve/avoid_unnecessary_fetch_user_calls

[Improvement] avoid unnecessary user fetchs; on first app open and first login

Author: jkasten2

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

@@ -44,7 +44,6 @@ import com.onesignal.user.internal.identity.IdentityModel
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.LoginUserFromSubscriptionOperation
 import com.onesignal.user.internal.operations.LoginUserOperation
-import com.onesignal.user.internal.operations.RefreshUserOperation
 import com.onesignal.user.internal.operations.TransferSubscriptionOperation
 import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.properties.PropertiesModelStore
@@ -384,17 +383,6 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
 
             if (!result) {
                 Logging.log(LogLevel.ERROR, "Could not login user")
-            } else {
-                // enqueue a RefreshUserOperation to pull the user from the backend and refresh the models.
-                // This is a separate enqueue operation to ensure any outstanding operations that happened
-                // after the createAndSwitchToNewUser have been executed, and the retrieval will be the
-                // most up to date reflection of the user.
-                operationRepo!!.enqueueAndWait(
-                    RefreshUserOperation(
-                        configModel!!.appId,
-                        identityModelStore!!.model.onesignalId,
-                    ),
-                )
             }
         }
     }

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt

@@ -26,6 +26,7 @@ import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.CreateSubscriptionOperation
 import com.onesignal.user.internal.operations.DeleteSubscriptionOperation
 import com.onesignal.user.internal.operations.LoginUserOperation
+import com.onesignal.user.internal.operations.RefreshUserOperation
 import com.onesignal.user.internal.operations.SetAliasOperation
 import com.onesignal.user.internal.operations.TransferSubscriptionOperation
 import com.onesignal.user.internal.operations.UpdateSubscriptionOperation
@@ -196,7 +197,15 @@ internal class LoginUserOperationExecutor(
                 subscriptionModel?.setStringProperty(SubscriptionModel::id.name, backendSubscription.id, ModelChangeTags.HYDRATE)
             }
 
-            return ExecutionResponse(ExecutionResult.SUCCESS, idTranslations)
+            val wasPossiblyAnUpsert = identities.isNotEmpty()
+            val followUpOperations =
+                if (wasPossiblyAnUpsert) {
+                    listOf(RefreshUserOperation(createUserOperation.appId, backendOneSignalId))
+                } else {
+                    null
+                }
+
+            return ExecutionResponse(ExecutionResult.SUCCESS, idTranslations, followUpOperations)
         } catch (ex: BackendException) {
             val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
 

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/service/UserRefreshService.kt

@@ -1,5 +1,6 @@
 package com.onesignal.user.internal.service
 
+import com.onesignal.common.IDManager
 import com.onesignal.core.internal.application.IApplicationLifecycleHandler
 import com.onesignal.core.internal.application.IApplicationService
 import com.onesignal.core.internal.config.ConfigModelStore
@@ -21,6 +22,8 @@ class UserRefreshService(
 ) : IStartableService,
     IApplicationLifecycleHandler {
     private fun refreshUser() {
+        if (IDManager.isLocalId(_identityModelStore.model.onesignalId)) return
+
         _operationRepo.enqueue(
             RefreshUserOperation(
                 _configModelStore.model.appId,

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt

@@ -156,6 +156,47 @@ class LoginUserOperationExecutorTests : FunSpec({
         ) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any()) }
     }
 
+    // If the User is identified then the backend may have found an existing User, if so
+    // we need to refresh it so we get all it's full subscription list
+    test("login identified user returns result with RefreshUser") {
+        // Given
+        val mockUserBackendService = mockk<IUserBackendService>()
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
+            CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
+
+        val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
+
+        val mockIdentityModelStore = MockHelper.identityModelStore()
+        val mockPropertiesModelStore = MockHelper.propertiesModelStore()
+        val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
+
+        val loginUserOperationExecutor =
+            LoginUserOperationExecutor(
+                mockIdentityOperationExecutor,
+                MockHelper.applicationService(),
+                MockHelper.deviceService(),
+                mockUserBackendService,
+                mockIdentityModelStore,
+                mockPropertiesModelStore,
+                mockSubscriptionsModelStore,
+                MockHelper.configModelStore(),
+                MockHelper.languageContext(),
+            )
+        val operations = listOf<Operation>(LoginUserOperation(appId, localOneSignalId, "externalId", null))
+
+        // When
+        val response = loginUserOperationExecutor.execute(operations)
+
+        // Then
+        response.result shouldBe ExecutionResult.SUCCESS
+        response.operations!!.size shouldBe 1
+        (response.operations!![0] as RefreshUserOperation).let {
+            it.shouldBeInstanceOf<RefreshUserOperation>()
+            it.appId shouldBe appId
+            it.onesignalId shouldBe remoteOneSignalId
+        }
+    }
+
     test("login identified user with association succeeds when association is successful") {
         // Given
         val mockUserBackendService = mockk<IUserBackendService>()