Refactor HTTP client to support passing header values + add support for Retry-Limit

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt

@@ -13,6 +13,7 @@ import com.onesignal.core.internal.backend.InfluenceParamsObject
 import com.onesignal.core.internal.backend.ParamsObject
 import com.onesignal.core.internal.http.CacheKeys
 import com.onesignal.core.internal.http.IHttpClient
+import com.onesignal.core.internal.http.impl.OptionalHeaders
 import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
 import org.json.JSONObject
@@ -31,7 +32,7 @@ internal class ParamsBackendService(
             paramsUrl += "?player_id=$subscriptionId"
         }
 
-        val response = _http.get(paramsUrl, CacheKeys.REMOTE_PARAMS)
+        val response = _http.get(paramsUrl, OptionalHeaders(cacheKey = CacheKeys.REMOTE_PARAMS))
 
         if (!response.isSuccess) {
             throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/HttpResponse.kt

@@ -23,6 +23,11 @@ class HttpResponse(
      * The module handing this should delay any future requests by this time.
      */
     val retryAfterSeconds: Int? = null,
+    /**
+     * Optional Integer value may be returned from the backend.
+     * The module handling this should not retry more than this number.
+     */
+    val retryLimit: Int? = null,
 ) {
     /**
      * Whether the response is a successful one.

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/IHttpClient.kt

@@ -1,5 +1,6 @@
 package com.onesignal.core.internal.http
 
+import com.onesignal.core.internal.http.impl.OptionalHeaders
 import org.json.JSONObject
 
 /**
@@ -18,6 +19,7 @@ interface IHttpClient {
     suspend fun post(
         url: String,
         body: JSONObject,
+        headers: OptionalHeaders? = null,
     ): HttpResponse
 
     /**
@@ -33,7 +35,7 @@ interface IHttpClient {
      */
     suspend fun get(
         url: String,
-        cacheKey: String? = null,
+        headers: OptionalHeaders? = null,
     ): HttpResponse
 
     /**
@@ -47,6 +49,7 @@ interface IHttpClient {
     suspend fun put(
         url: String,
         body: JSONObject,
+        headers: OptionalHeaders? = null,
     ): HttpResponse
 
     /**
@@ -60,6 +63,7 @@ interface IHttpClient {
     suspend fun patch(
         url: String,
         body: JSONObject,
+        headers: OptionalHeaders? = null,
     ): HttpResponse
 
     /**
@@ -69,7 +73,10 @@ interface IHttpClient {
      *
      * @return The response returned.
      */
-    suspend fun delete(url: String): HttpResponse
+    suspend fun delete(
+        url: String,
+        headers: OptionalHeaders? = null,
+    ): HttpResponse
 }
 
 /**

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/HttpClient.kt

@@ -45,41 +45,37 @@ internal class HttpClient(
     override suspend fun post(
         url: String,
         body: JSONObject,
-    ): HttpResponse {
-        return makeRequest(url, "POST", body, _configModelStore.model.httpTimeout, null)
-    }
+        headers: OptionalHeaders?,
+    ): HttpResponse = makeRequest(url, "POST", body, _configModelStore.model.httpTimeout, headers)
 
     override suspend fun get(
         url: String,
-        cacheKey: String?,
-    ): HttpResponse {
-        return makeRequest(url, null, null, _configModelStore.model.httpGetTimeout, cacheKey)
-    }
+        headers: OptionalHeaders?,
+    ): HttpResponse = makeRequest(url, null, null, _configModelStore.model.httpGetTimeout, headers)
 
     override suspend fun put(
         url: String,
         body: JSONObject,
-    ): HttpResponse {
-        return makeRequest(url, "PUT", body, _configModelStore.model.httpTimeout, null)
-    }
+        headers: OptionalHeaders?,
+    ): HttpResponse = makeRequest(url, "PUT", body, _configModelStore.model.httpTimeout, headers)
 
     override suspend fun patch(
         url: String,
         body: JSONObject,
-    ): HttpResponse {
-        return makeRequest(url, "PATCH", body, _configModelStore.model.httpTimeout, null)
-    }
+        headers: OptionalHeaders?,
+    ): HttpResponse = makeRequest(url, "PATCH", body, _configModelStore.model.httpTimeout, headers)
 
-    override suspend fun delete(url: String): HttpResponse {
-        return makeRequest(url, "DELETE", null, _configModelStore.model.httpTimeout, null)
-    }
+    override suspend fun delete(
+        url: String,
+        headers: OptionalHeaders?,
+    ): HttpResponse = makeRequest(url, "DELETE", null, _configModelStore.model.httpTimeout, headers)
 
     private suspend fun makeRequest(
         url: String,
         method: String?,
         jsonBody: JSONObject?,
         timeout: Int,
-        cacheKey: String?,
+        headers: OptionalHeaders?,
     ): HttpResponse {
         // If privacy consent is required but not yet given, any non-GET request should be blocked.
         if (method != null && _configModelStore.model.consentRequired == true && _configModelStore.model.consentGiven != true) {
@@ -94,7 +90,7 @@ internal class HttpClient(
 
         try {
             return withTimeout(getThreadTimeout(timeout).toLong()) {
-                return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, cacheKey)
+                return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, headers)
             }
         } catch (e: TimeoutCancellationException) {
             Logging.error("HttpClient: Request timed out: $url", e)
@@ -110,7 +106,7 @@ internal class HttpClient(
         method: String?,
         jsonBody: JSONObject?,
         timeout: Int,
-        cacheKey: String?,
+        headers: OptionalHeaders?,
     ): HttpResponse {
         var retVal: HttpResponse? = null
 
@@ -174,11 +170,16 @@ internal class HttpClient(
                         outputStream.write(sendBytes)
                     }
 
-                    if (cacheKey != null) {
-                        val eTag = _prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + cacheKey)
+                    // H E A D E R S
 
+                    if (headers?.cacheKey != null) {
+                        val eTag =
+                            _prefs.getString(
+                                PreferenceStores.ONESIGNAL,
+                                PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + headers.cacheKey,
+                            )
                         if (eTag != null) {
-                            con.setRequestProperty("if-none-match", eTag)
+                            con.setRequestProperty("If-None-Match", eTag)
                             Logging.debug("HttpClient: Adding header if-none-match: $eTag")
                         }
                     }
@@ -187,6 +188,7 @@ internal class HttpClient(
                     httpResponse = con.responseCode
 
                     val retryAfter = retryAfterFromResponse(con)
+                    val retryLimit = retryLimitFromResponse(con)
                     val newDelayUntil = _time.currentTimeMillis + (retryAfter ?: 0) * 1_000
                     if (newDelayUntil > delayNewRequestsUntil) delayNewRequestsUntil = newDelayUntil
 
@@ -195,39 +197,44 @@ internal class HttpClient(
                             val cachedResponse =
                                 _prefs.getString(
                                     PreferenceStores.ONESIGNAL,
-                                    PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + cacheKey,
+                                    PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + headers?.cacheKey,
                                 )
-                            Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - Using Cached response due to 304: " + cachedResponse)
+                            Logging.debug(
+                                "HttpClient: Got Response = ${method ?: "GET"} ${con.url} - Using Cached response due to 304: " +
+                                    cachedResponse,
+                            )
 
                             // TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?
-                            retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter)
+                            retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
                         }
                         HttpURLConnection.HTTP_ACCEPTED, HttpURLConnection.HTTP_CREATED, HttpURLConnection.HTTP_OK -> {
                             val inputStream = con.inputStream
                             val scanner = Scanner(inputStream, "UTF-8")
                             val json = if (scanner.useDelimiter("\\A").hasNext()) scanner.next() else ""
                             scanner.close()
-                            Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - STATUS: $httpResponse - Body: " + json)
+                            Logging.debug(
+                                "HttpClient: Got Response = ${method ?: "GET"} ${con.url} - STATUS: $httpResponse - Body: " + json,
+                            )
 
-                            if (cacheKey != null) {
+                            if (headers?.cacheKey != null) {
                                 val eTag = con.getHeaderField("etag")
                                 if (eTag != null) {
                                     Logging.debug("HttpClient: Got Response = Response has etag of $eTag so caching the response.")
 
                                     _prefs.saveString(
                                         PreferenceStores.ONESIGNAL,
-                                        PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + cacheKey,
+                                        PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + headers.cacheKey,
                                         eTag,
                                     )
                                     _prefs.saveString(
                                         PreferenceStores.ONESIGNAL,
-                                        PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + cacheKey,
+                                        PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + headers.cacheKey,
                                         json,
                                     )
                                 }
                             }
 
-                            retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter)
+                            retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
                         }
                         else -> {
                             Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - FAILED STATUS: $httpResponse")
@@ -248,7 +255,7 @@ internal class HttpClient(
                                 Logging.warn("HttpClient: Got Response = $method - STATUS: $httpResponse - No response body!")
                             }
 
-                            retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter)
+                            retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
                         }
                     }
                 } catch (t: Throwable) {
@@ -288,6 +295,19 @@ internal class HttpClient(
         }
     }
 
+    /**
+     * Reads the HTTP Retry-Limit from the response.
+     */
+    private fun retryLimitFromResponse(con: HttpURLConnection): Int? {
+        val retryLimitStr = con.getHeaderField("Retry-Limit")
+        return if (retryLimitStr != null) {
+            Logging.debug("HttpClient: Response Retry-After: $retryLimitStr")
+            retryLimitStr.toIntOrNull()
+        } else {
+            null
+        }
+    }
+
     private fun logHTTPSent(
         method: String?,
         url: URL,

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/http/impl/OptionalHeaders.kt

@@ -0,0 +1,5 @@
+package com.onesignal.core.internal.http.impl
+
+data class OptionalHeaders(
+    val cacheKey: String? = null,
+)

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/http/HttpClientTests.kt

@@ -3,6 +3,7 @@ package com.onesignal.core.internal.http
 import com.onesignal.common.OneSignalUtils
 import com.onesignal.core.internal.device.impl.InstallIdService
 import com.onesignal.core.internal.http.impl.HttpClient
+import com.onesignal.core.internal.http.impl.OptionalHeaders
 import com.onesignal.core.internal.time.impl.Time
 import com.onesignal.debug.LogLevel
 import com.onesignal.debug.internal.logging.Logging
@@ -88,9 +89,10 @@ class HttpClientTests : FunSpec({
         val httpClient = mocks.httpClient
 
         // When
-        val response1 = httpClient.get("URL", "CACHE_KEY")
+        val headers = OptionalHeaders(cacheKey = "CACHE_KEY")
+        val response1 = httpClient.get("URL", headers)
         factory.mockResponse = mockResponse2
-        val response2 = httpClient.get("URL", "CACHE_KEY")
+        val response2 = httpClient.get("URL", headers)
 
         // Then
         response1.statusCode shouldBe 200
@@ -123,13 +125,14 @@ class HttpClientTests : FunSpec({
         val httpClient = mocks.httpClient
 
         // When
-        val response1 = httpClient.get("URL", "CACHE_KEY")
+        val headers = OptionalHeaders(cacheKey = "CACHE_KEY")
+        val response1 = httpClient.get("URL", headers)
 
         factory.mockResponse = mockResponse2
-        val response2 = httpClient.get("URL", "CACHE_KEY")
+        val response2 = httpClient.get("URL", headers)
 
         factory.mockResponse = mockResponse3
-        val response3 = httpClient.get("URL", "CACHE_KEY")
+        val response3 = httpClient.get("URL", headers)
 
         // Then
         response1.statusCode shouldBe 200