diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsController.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsController.kt index 20e4802c74..91d87ee396 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsController.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsController.kt @@ -1,6 +1,7 @@ package com.onesignal.session.internal.outcomes.impl import android.os.Process +import com.onesignal.common.NetworkUtils import com.onesignal.common.exceptions.BackendException import com.onesignal.common.threading.suspendifyOnThread import com.onesignal.core.internal.config.ConfigModelStore @@ -75,10 +76,15 @@ internal class OutcomeEventsController( _outcomeEventsCache.deleteOldOutcomeEvent(event) } catch (ex: BackendException) { - Logging.warn( - """OutcomeEventsController.sendSavedOutcomeEvent: Sending outcome with name: ${event.outcomeId} failed with status code: ${ex.statusCode} and response: ${ex.response} -Outcome event was cached and will be reattempted on app cold start""", - ) + val responseType = NetworkUtils.getResponseStatusType(ex.statusCode) + val err = "OutcomeEventsController.sendSavedOutcomeEvent: Sending outcome with name: ${event.outcomeId} failed with status code: ${ex.statusCode} and response: ${ex.response}" + + if (responseType == NetworkUtils.ResponseStatusType.RETRYABLE) { + Logging.warn("$err Outcome event was cached and will be reattempted on app cold start") + } else { + Logging.error("$err Outcome event will be omitted!") + _outcomeEventsCache.deleteOldOutcomeEvent(event) + } } } @@ -220,14 +226,19 @@ Outcome event was cached and will be reattempted on app cold start""", // The only case where an actual success has occurred and the OutcomeEvent should be sent back return OutcomeEvent.fromOutcomeEventParamstoOutcomeEvent(eventParams) } catch (ex: BackendException) { - Logging.warn( - """OutcomeEventsController.sendAndCreateOutcomeEvent: Sending outcome with name: $name failed with status code: ${ex.statusCode} and response: ${ex.response} -Outcome event was cached and will be reattempted on app cold start""", - ) - - // Only if we need to save and retry the outcome, then we will save the timestamp for future sending - eventParams.timestamp = timestampSeconds - _outcomeEventsCache.saveOutcomeEvent(eventParams) + val responseType = NetworkUtils.getResponseStatusType(ex.statusCode) + val err = "OutcomeEventsController.sendAndCreateOutcomeEvent: Sending outcome with name: $name failed with status code: ${ex.statusCode} and response: ${ex.response}" + + if (responseType == NetworkUtils.ResponseStatusType.RETRYABLE) { + Logging.warn("$err Outcome event was cached and will be reattempted on app cold start") + + // Only if we need to save and retry the outcome, then we will save the timestamp for future sending + eventParams.timestamp = timestampSeconds + _outcomeEventsCache.saveOutcomeEvent(eventParams) + } else { + Logging.error("$err Outcome event will be omitted!") + _outcomeEventsCache.deleteOldOutcomeEvent(eventParams) + } // Return null to determine not a failure, but not a success in terms of the request made return null diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsRepository.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsRepository.kt index 310824167b..8a9b7ecb66 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsRepository.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/impl/OutcomeEventsRepository.kt @@ -8,7 +8,7 @@ import com.onesignal.session.internal.influence.Influence import com.onesignal.session.internal.influence.InfluenceChannel import com.onesignal.session.internal.influence.InfluenceType import com.onesignal.session.internal.influence.InfluenceType.Companion.fromString -import com.onesignal.session.internal.outcomes.migrations.RemoveZeroSessionTimeRecords +import com.onesignal.session.internal.outcomes.migrations.RemoveInvalidSessionTimeRecords import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.json.JSONArray @@ -102,7 +102,7 @@ internal class OutcomeEventsRepository( override suspend fun getAllEventsToSend(): List { val events: MutableList = ArrayList() withContext(Dispatchers.IO) { - RemoveZeroSessionTimeRecords.run(_databaseProvider) + RemoveInvalidSessionTimeRecords.run(_databaseProvider) _databaseProvider.os.query(OutcomeEventsTable.TABLE_NAME) { cursor -> if (cursor.moveToFirst()) { do { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveZeroSessionTimeRecords.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveInvalidSessionTimeRecords.kt similarity index 59% rename from OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveZeroSessionTimeRecords.kt rename to OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveInvalidSessionTimeRecords.kt index 93bab4d4a3..e9464af7a2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveZeroSessionTimeRecords.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/outcomes/migrations/RemoveInvalidSessionTimeRecords.kt @@ -4,20 +4,22 @@ import com.onesignal.core.internal.database.IDatabaseProvider import com.onesignal.session.internal.outcomes.impl.OutcomeEventsTable /** - * Purpose: Clean up invalid cached os__session_duration outcome records - * with zero session_time produced in SDK versions 5.1.15 to 5.1.20 so we stop - * sending these requests to the backend. + * Purpose: Clean up invalid cached os__session_duration outcome records with + * 1. zero session_time produced in SDK versions 5.1.15 to 5.1.20 + * 2. missing session_time produced in SDK + * so we stop sending these requests to the backend. * * Issue: SessionService.backgroundRun() didn't account for it being run more * than one time in the background, when this happened it would create a - * outcome record with zero time which is invalid. + * outcome record with zero time or null which is invalid. */ -object RemoveZeroSessionTimeRecords { +object RemoveInvalidSessionTimeRecords { fun run(databaseProvider: IDatabaseProvider) { databaseProvider.os.delete( OutcomeEventsTable.TABLE_NAME, OutcomeEventsTable.COLUMN_NAME_NAME + " = \"os__session_duration\"" + - " AND " + OutcomeEventsTable.COLUMN_NAME_SESSION_TIME + " = 0", + " AND (" + OutcomeEventsTable.COLUMN_NAME_SESSION_TIME + " = 0" + + " OR " + OutcomeEventsTable.COLUMN_NAME_SESSION_TIME + " IS NULL)", null, ) } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsControllerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsControllerTests.kt index a4d9d14a83..b4559464ac 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsControllerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsControllerTests.kt @@ -723,4 +723,105 @@ class OutcomeEventsControllerTests : FunSpec({ } coVerify(exactly = 0) { mockOutcomeEventsRepository.deleteOldOutcomeEvent(any()) } } + + test("send saved outcome event with 400 error deletes event and does not retry") { + // Given + val now = 111111L + val mockSessionService = mockk() + every { mockSessionService.subscribe(any()) } just Runs + + val subscriptionModel = createTestSubscriptionModel() + + val mockSubscriptionManager = mockk() + every { mockSubscriptionManager.subscriptions.push } returns PushSubscription(subscriptionModel) + + val mockInfluenceManager = mockk() + val mockOutcomeEventsRepository = mockk() + coEvery { mockOutcomeEventsRepository.cleanCachedUniqueOutcomeEventNotifications() } just runs + coEvery { mockOutcomeEventsRepository.deleteOldOutcomeEvent(any()) } just runs + coEvery { mockOutcomeEventsRepository.getAllEventsToSend() } returns + listOf( + OutcomeEventParams("outcomeId1", OutcomeSource(OutcomeSourceBody(JSONArray().put("notificationId1")), null), .4f, 0, 1111), + ) + val mockOutcomeEventsPreferences = spyk() + val mockOutcomeEventsBackend = mockk() + coEvery { mockOutcomeEventsBackend.sendOutcomeEvent(any(), any(), any(), any(), any(), any()) } throws BackendException(400, "Bad Request") + + val outcomeEventsController = + OutcomeEventsController( + mockSessionService, + mockInfluenceManager, + mockOutcomeEventsRepository, + mockOutcomeEventsPreferences, + mockOutcomeEventsBackend, + MockHelper.configModelStore(), + MockHelper.identityModelStore { it.onesignalId = "onesignalId" }, + mockSubscriptionManager, + MockHelper.deviceService(), + MockHelper.time(now), + ) + + // When + outcomeEventsController.start() + delay(1000) + + // Then + coVerify(exactly = 1) { + mockOutcomeEventsBackend.sendOutcomeEvent(any(), any(), any(), any(), any(), any()) + } + coVerify(exactly = 1) { + mockOutcomeEventsRepository.deleteOldOutcomeEvent(any()) + } + } + + test("send outcome event with 400 error deletes event and returns null") { + // Given + val now = 111L + val mockSessionService = mockk() + every { mockSessionService.subscribe(any()) } just Runs + + val mockInfluenceManager = mockk() + every { mockInfluenceManager.influences } returns listOf(Influence(InfluenceChannel.NOTIFICATION, InfluenceType.UNATTRIBUTED, null)) + + val subscriptionModel = createTestSubscriptionModel() + + val mockSubscriptionManager = mockk() + every { mockSubscriptionManager.subscriptions.push } returns PushSubscription(subscriptionModel) + + val mockOutcomeEventsRepository = spyk() + val mockOutcomeEventsPreferences = spyk() + val mockOutcomeEventsBackend = mockk() + coEvery { mockOutcomeEventsBackend.sendOutcomeEvent(any(), any(), any(), any(), any(), any()) } throws BackendException(400, "Bad Request") + + val outcomeEventsController = + OutcomeEventsController( + mockSessionService, + mockInfluenceManager, + mockOutcomeEventsRepository, + mockOutcomeEventsPreferences, + mockOutcomeEventsBackend, + MockHelper.configModelStore(), + MockHelper.identityModelStore(), + mockSubscriptionManager, + MockHelper.deviceService(), + MockHelper.time(now), + ) + + // When + val evnt = outcomeEventsController.sendOutcomeEvent("OUTCOME_1") + + // Then + evnt shouldBe null + + coVerify(exactly = 1) { + mockOutcomeEventsBackend.sendOutcomeEvent(any(), any(), any(), any(), any(), any()) + } + coVerify(exactly = 1) { + mockOutcomeEventsRepository.deleteOldOutcomeEvent(any()) + } + // Verify event is NOT saved for retry (unlike other error codes) + coVerify(exactly = 0) { + mockOutcomeEventsRepository.saveOutcomeEvent(any()) + } + } })