fixup! Update IAM manager & backend service with retry logic, optiona…

…l headers
OneSignal · Sep 13, 2024 · ae2ed04 · ae2ed04
1 parent d7d2948
commit ae2ed04
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 22 deletions.
diff --git a/...ssages/src/main/java/com/onesignal/inAppMessages/internal/backend/IInAppBackendService.kt b/...ssages/src/main/java/com/onesignal/inAppMessages/internal/backend/IInAppBackendService.kt
@@ -13,7 +13,8 @@ internal interface IInAppBackendService {
      *
      * @param appId The ID of the application that the IAM will be retrieved from.
      * @param subscriptionId The specific subscription within the [appId] the IAM will be delivered to.
-     * @param offset OPTIONAL - used for read your write consistency
+     * @param offset Used for read your write consistency
+     * @param sessionDurationProvider Lambda to calculate the session duration at the time of the request
      *
      * @return The list of IAMs associated to the subscription, or null if the IAMs could not be retrieved.
      */

diff --git a/...es/src/main/java/com/onesignal/inAppMessages/internal/backend/impl/InAppBackendService.kt b/...es/src/main/java/com/onesignal/inAppMessages/internal/backend/impl/InAppBackendService.kt
@@ -209,8 +209,9 @@ internal class InAppBackendService(
     ): List<InAppMessage>? {
         var attempts = 1
         var delayTime = 1 // Start with a 1-second delay for exponential backoff
+        var retryLimit = DEFAULT_RETRY_LIMIT
 
-        while (true) {
+        while (attempts <= retryLimit + 1) {
             val retryCount = if (attempts > 1) attempts - 1 else null
             val values =
                 OptionalHeaders(
@@ -224,28 +225,17 @@ internal class InAppBackendService(
                 val jsonResponse = response.payload?.let { JSONObject(it) }
                 return jsonResponse?.let { hydrateInAppMessages(it) }
             } else if (response.statusCode == 425) { // 425 Too Early
-                val retryLimit = response.retryLimit ?: DEFAULT_RETRY_LIMIT
-                val retryAfter = response.retryAfterSeconds ?: DEFAULT_RETRY_AFTER_SECONDS
-
-                if (attempts == retryLimit) {
-                    break
+                if (response.retryLimit != null) {
+                    retryLimit = response.retryLimit!!
                 }
-
+                val retryAfter = response.retryAfterSeconds ?: DEFAULT_RETRY_AFTER_SECONDS
                 delay(retryAfter * 1_000L)
             } else if (response.statusCode == 429) { // 429 Too Many Requests
                 val retryAfter = response.retryAfterSeconds ?: delayTime
 
-                if (attempts == (response.retryLimit ?: DEFAULT_RETRY_LIMIT)) {
-                    break
-                }
-
                 delay(retryAfter * 1_000L)
                 delayTime *= 2 // Exponential backoff
             } else if (response.statusCode >= 500) {
-                if (attempts == DEFAULT_RETRY_LIMIT) {
-                    break
-                }
-
                 delay(delayTime * 1_000L)
                 delayTime *= 2 // Exponential backoff
             } else {
@@ -257,20 +247,18 @@ internal class InAppBackendService(
 
         // If all retries fail, make a final attempt without the offset. This will tell the server,
         // we give up, just give me the IAMs without first ensuring data consistency
-        return fetchInAppMessagesWithoutOffset(baseUrl, sessionDurationProvider, attempts)
+        return fetchInAppMessagesWithoutOffset(baseUrl, sessionDurationProvider)
     }
 
     private suspend fun fetchInAppMessagesWithoutOffset(
         url: String,
         sessionDurationProvider: () -> Long,
-        retryCount: Int,
     ): List<InAppMessage>? {
         val response =
             _httpClient.get(
                 url,
                 OptionalHeaders(
                     sessionDuration = sessionDurationProvider(),
-                    retryCount = retryCount,
                     offset = 0,
                 ),
             )

diff --git a/...es/src/test/java/com/onesignal/inAppMessages/internal/backend/InAppBackendServiceTests.kt b/...es/src/test/java/com/onesignal/inAppMessages/internal/backend/InAppBackendServiceTests.kt
@@ -117,6 +117,7 @@ class InAppBackendServiceTests :
                     HttpResponse(425, null, retryAfterSeconds = 1, retryLimit = 3),
                     HttpResponse(425, null, retryAfterSeconds = 1, retryLimit = 3),
                     HttpResponse(425, null, retryAfterSeconds = 1, retryLimit = 3),
+                    HttpResponse(425, null, retryAfterSeconds = 1, retryLimit = 3),
                     HttpResponse(200, "{ in_app_messages: [] }"),
                 )
 
@@ -133,13 +134,13 @@ class InAppBackendServiceTests :
                 mockHttpClient.get(
                     "apps/appId/subscriptions/subscriptionId/iams",
                     match {
-                        it.offset != null && it.retryCount == null && it.sessionDuration == mockSessionDurationProvider()
+                        it.offset == 1234L && it.retryCount == null && it.sessionDuration == mockSessionDurationProvider()
                     },
                 )
             }
 
             // Verify that the get method retried twice with the offset
-            coVerify(exactly = 2) {
+            coVerify(exactly = 3) {
                 mockHttpClient.get(
                     "apps/appId/subscriptions/subscriptionId/iams",
                     match {
@@ -153,7 +154,7 @@ class InAppBackendServiceTests :
                 mockHttpClient.get(
                     "apps/appId/subscriptions/subscriptionId/iams",
                     match {
-                        it.offset == 0L && it.retryCount == 3 && it.sessionDuration == mockSessionDurationProvider()
+                        it.offset == 0L && it.sessionDuration == mockSessionDurationProvider() && it.retryCount == null
                     },
                 )
             }