Skip to content

Commit 5a099c0

Browse files
committed
avoid unnecessary fetch users
This commit avoids to unnecessary fetch user scenarios: 1. App open, first time. 2. Login, when user is currently anonymous Case 1 UserRefreshService would always add a RefreshUserOperation to the OperationRepo on cold start. However this is not needed for the first time the app is opened, as there is no need to fetch a User we just created. Case 2 If OneSignal.login() is called and the user is currently anonymous then we simply identify the User. In this case there is no need to fetch the user, as it is the same user, we just updated it.
1 parent 88b60a3 commit 5a099c0

File tree

4 files changed

+54
-13
lines changed

4 files changed

+54
-13
lines changed

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import com.onesignal.user.internal.identity.IdentityModel
4444
import com.onesignal.user.internal.identity.IdentityModelStore
4545
import com.onesignal.user.internal.operations.LoginUserFromSubscriptionOperation
4646
import com.onesignal.user.internal.operations.LoginUserOperation
47-
import com.onesignal.user.internal.operations.RefreshUserOperation
4847
import com.onesignal.user.internal.operations.TransferSubscriptionOperation
4948
import com.onesignal.user.internal.properties.PropertiesModel
5049
import com.onesignal.user.internal.properties.PropertiesModelStore
@@ -384,17 +383,6 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
384383

385384
if (!result) {
386385
Logging.log(LogLevel.ERROR, "Could not login user")
387-
} else {
388-
// enqueue a RefreshUserOperation to pull the user from the backend and refresh the models.
389-
// This is a separate enqueue operation to ensure any outstanding operations that happened
390-
// after the createAndSwitchToNewUser have been executed, and the retrieval will be the
391-
// most up to date reflection of the user.
392-
operationRepo!!.enqueueAndWait(
393-
RefreshUserOperation(
394-
configModel!!.appId,
395-
identityModelStore!!.model.onesignalId,
396-
),
397-
)
398386
}
399387
}
400388
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.onesignal.user.internal.identity.IdentityModelStore
2626
import com.onesignal.user.internal.operations.CreateSubscriptionOperation
2727
import com.onesignal.user.internal.operations.DeleteSubscriptionOperation
2828
import com.onesignal.user.internal.operations.LoginUserOperation
29+
import com.onesignal.user.internal.operations.RefreshUserOperation
2930
import com.onesignal.user.internal.operations.SetAliasOperation
3031
import com.onesignal.user.internal.operations.TransferSubscriptionOperation
3132
import com.onesignal.user.internal.operations.UpdateSubscriptionOperation
@@ -196,7 +197,15 @@ internal class LoginUserOperationExecutor(
196197
subscriptionModel?.setStringProperty(SubscriptionModel::id.name, backendSubscription.id, ModelChangeTags.HYDRATE)
197198
}
198199

199-
return ExecutionResponse(ExecutionResult.SUCCESS, idTranslations)
200+
val wasPossiblyAnUpsert = identities.isNotEmpty()
201+
val followUpOperations =
202+
if (wasPossiblyAnUpsert) {
203+
listOf(RefreshUserOperation(createUserOperation.appId, backendOneSignalId))
204+
} else {
205+
null
206+
}
207+
208+
return ExecutionResponse(ExecutionResult.SUCCESS, idTranslations, followUpOperations)
200209
} catch (ex: BackendException) {
201210
val responseType = NetworkUtils.getResponseStatusType(ex.statusCode)
202211

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.onesignal.user.internal.service
22

3+
import com.onesignal.common.IDManager
34
import com.onesignal.core.internal.application.IApplicationLifecycleHandler
45
import com.onesignal.core.internal.application.IApplicationService
56
import com.onesignal.core.internal.config.ConfigModelStore
@@ -21,6 +22,8 @@ class UserRefreshService(
2122
) : IStartableService,
2223
IApplicationLifecycleHandler {
2324
private fun refreshUser() {
25+
if (IDManager.isLocalId(_identityModelStore.model.onesignalId)) return
26+
2427
_operationRepo.enqueue(
2528
RefreshUserOperation(
2629
_configModelStore.model.appId,

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,47 @@ class LoginUserOperationExecutorTests : FunSpec({
156156
) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any()) }
157157
}
158158

159+
// If the User is identified then the backend may have found an existing User, if so
160+
// we need to refresh it so we get all it's full subscription list
161+
test("login identified user returns result with RefreshUser") {
162+
// Given
163+
val mockUserBackendService = mockk<IUserBackendService>()
164+
coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
165+
CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
166+
167+
val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
168+
169+
val mockIdentityModelStore = MockHelper.identityModelStore()
170+
val mockPropertiesModelStore = MockHelper.propertiesModelStore()
171+
val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
172+
173+
val loginUserOperationExecutor =
174+
LoginUserOperationExecutor(
175+
mockIdentityOperationExecutor,
176+
MockHelper.applicationService(),
177+
MockHelper.deviceService(),
178+
mockUserBackendService,
179+
mockIdentityModelStore,
180+
mockPropertiesModelStore,
181+
mockSubscriptionsModelStore,
182+
MockHelper.configModelStore(),
183+
MockHelper.languageContext(),
184+
)
185+
val operations = listOf<Operation>(LoginUserOperation(appId, localOneSignalId, "externalId", null))
186+
187+
// When
188+
val response = loginUserOperationExecutor.execute(operations)
189+
190+
// Then
191+
response.result shouldBe ExecutionResult.SUCCESS
192+
response.operations!!.size shouldBe 1
193+
(response.operations!![0] as RefreshUserOperation).let {
194+
it.shouldBeInstanceOf<RefreshUserOperation>()
195+
it.appId shouldBe appId
196+
it.onesignalId shouldBe remoteOneSignalId
197+
}
198+
}
199+
159200
test("login identified user with association succeeds when association is successful") {
160201
// Given
161202
val mockUserBackendService = mockk<IUserBackendService>()

0 commit comments

Comments
 (0)