From ba88ec2af10b67c7a7affb2bc4ee9832dd20b469 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 26 Jul 2024 18:07:53 -0700 Subject: [PATCH 1/3] recover from invalid Operations missing onesignalId * When uncaching an Operation that is not login-related, check for the existence of onesignalId. * Note: The login-related Operations should still have onesignalId but they can be sent without it. It is also less likely for onesignalId to be null based on when these Operations are created. * This is to remedy a rare case that an Operation could be missing the onesignalId, which would continuously cause crashes when the Operation is processed. The particular reported Operation is an UpdateSubscriptionOperation. --- .../operations/impl/OperationModelStore.kt | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt index 26eaf3ac32..1fa25fe631 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt @@ -38,8 +38,7 @@ internal class OperationModelStore(prefs: IPreferencesService) : ModelStore Date: Tue, 30 Jul 2024 20:20:33 -0700 Subject: [PATCH 2/3] [tests] fix current test after changes * Switch Operation type in an existing test now that non-login Operations are expected to have OSID * The type of Operation used in the test is irrelevant --- .../src/test/java/com/onesignal/common/ModelingTests.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt index 46ffc10452..1ee72ddb3a 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt @@ -9,8 +9,8 @@ import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys import com.onesignal.core.internal.preferences.PreferenceStores import com.onesignal.mocks.MockHelper import com.onesignal.mocks.MockPreferencesService -import com.onesignal.user.internal.operations.SetPropertyOperation -import com.onesignal.user.internal.operations.SetTagOperation +import com.onesignal.user.internal.operations.LoginUserFromSubscriptionOperation +import com.onesignal.user.internal.operations.LoginUserOperation import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore import io.kotest.core.spec.style.FunSpec @@ -150,7 +150,7 @@ class ModelingTests : FunSpec({ val operationModelStore = OperationModelStore(prefs) val jsonArray = JSONArray() - val cachedOperation = SetTagOperation() + val cachedOperation = LoginUserFromSubscriptionOperation() cachedOperation.id = UUID.randomUUID().toString() // Add duplicate operations to the cache jsonArray.put(cachedOperation.toJSON()) @@ -158,7 +158,7 @@ class ModelingTests : FunSpec({ prefs.saveString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.MODEL_STORE_PREFIX + "operations", jsonArray.toString()) // When - adding an operation first and then loading from cache - val newOperation = SetPropertyOperation() + val newOperation = LoginUserOperation() newOperation.id = UUID.randomUUID().toString() operationModelStore.add(newOperation) operationModelStore.loadOperations() From f7b0e5f049b6f4a5846c6b788b0275b6c2569d2a Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 31 Jul 2024 13:04:14 -0700 Subject: [PATCH 3/3] [tests] add test for new OperationModelStore check * Add new test file `OperationModelStoreTests` * Add test called "does not load invalid cached operations" that uncaches valid and invalid operations based on the new and existing checks in the OperationModelStore uncaching logic. --- .../operations/OperationModelStoreTests.kt | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationModelStoreTests.kt diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationModelStoreTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationModelStoreTests.kt new file mode 100644 index 0000000000..d4fcee869d --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationModelStoreTests.kt @@ -0,0 +1,66 @@ +package com.onesignal.core.internal.operations + +import com.onesignal.core.internal.operations.impl.OperationModelStore +import com.onesignal.core.internal.preferences.PreferenceOneSignalKeys +import com.onesignal.core.internal.preferences.PreferenceStores +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.MockPreferencesService +import com.onesignal.user.internal.operations.LoginUserOperation +import com.onesignal.user.internal.operations.SetPropertyOperation +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import org.json.JSONArray +import org.json.JSONObject +import java.util.UUID + +class OperationModelStoreTests : FunSpec({ + + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + test("does not load invalid cached operations") { + // Given + val prefs = MockPreferencesService() + val operationModelStore = OperationModelStore(prefs) + val jsonArray = JSONArray() + + // 1. Create a VALID Operation with onesignalId + val validOperation = SetPropertyOperation(UUID.randomUUID().toString(), UUID.randomUUID().toString(), "property", "value") + validOperation.id = UUID.randomUUID().toString() + + // 2. Create a VALID operation missing onesignalId + val validOperationMissingOnesignalId = LoginUserOperation() + validOperationMissingOnesignalId.id = UUID.randomUUID().toString() + + // 3. Create an INVALID Operation missing onesignalId + val invalidOperationMissingOnesignalId = SetPropertyOperation() + invalidOperationMissingOnesignalId.id = UUID.randomUUID().toString() + + // 4. Create an INVALID Operation missing operation name + val invalidOperationMissingName = + JSONObject() + .put("app_id", UUID.randomUUID().toString()) + .put("onesignalId", UUID.randomUUID().toString()) + .put("id", UUID.randomUUID().toString()) + + // Add the Operations to the cache + jsonArray.put(validOperation.toJSON()) + jsonArray.put(validOperationMissingOnesignalId.toJSON()) + jsonArray.put(invalidOperationMissingOnesignalId.toJSON()) + jsonArray.put(invalidOperationMissingName) + prefs.saveString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.MODEL_STORE_PREFIX + "operations", jsonArray.toString()) + + // When + operationModelStore.loadOperations() + + // Then + operationModelStore.list().count() shouldBe 2 + operationModelStore.get(validOperation.id) shouldNotBe null + operationModelStore.get(validOperationMissingOnesignalId.id) shouldNotBe null + operationModelStore.get(invalidOperationMissingOnesignalId.id) shouldBe null + operationModelStore.get(invalidOperationMissingName["id"] as String) shouldBe null + } +})