Skip to content

Commit 78264aa

Browse files
author
Rodrigo Gomez Palacio
authored
Merge pull request #2182 from OneSignal/http-client-headers
Refactor HTTP client to support passing header values + add support for `Retry-Limit`
2 parents 54ab98e + 8e97336 commit 78264aa

File tree

6 files changed

+80
-39
lines changed

6 files changed

+80
-39
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import com.onesignal.core.internal.backend.InfluenceParamsObject
1313
import com.onesignal.core.internal.backend.ParamsObject
1414
import com.onesignal.core.internal.http.CacheKeys
1515
import com.onesignal.core.internal.http.IHttpClient
16+
import com.onesignal.core.internal.http.impl.OptionalHeaders
1617
import com.onesignal.debug.LogLevel
1718
import com.onesignal.debug.internal.logging.Logging
1819
import org.json.JSONObject
@@ -31,7 +32,7 @@ internal class ParamsBackendService(
3132
paramsUrl += "?player_id=$subscriptionId"
3233
}
3334

34-
val response = _http.get(paramsUrl, CacheKeys.REMOTE_PARAMS)
35+
val response = _http.get(paramsUrl, OptionalHeaders(cacheKey = CacheKeys.REMOTE_PARAMS))
3536

3637
if (!response.isSuccess) {
3738
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ class HttpResponse(
2323
* The module handing this should delay any future requests by this time.
2424
*/
2525
val retryAfterSeconds: Int? = null,
26+
/**
27+
* Optional Integer value may be returned from the backend.
28+
* The module handling this should not retry more than this number.
29+
*/
30+
val retryLimit: Int? = null,
2631
) {
2732
/**
2833
* Whether the response is a successful one.

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.onesignal.core.internal.http
22

3+
import com.onesignal.core.internal.http.impl.OptionalHeaders
34
import org.json.JSONObject
45

56
/**
@@ -18,6 +19,7 @@ interface IHttpClient {
1819
suspend fun post(
1920
url: String,
2021
body: JSONObject,
22+
headers: OptionalHeaders? = null,
2123
): HttpResponse
2224

2325
/**
@@ -33,7 +35,7 @@ interface IHttpClient {
3335
*/
3436
suspend fun get(
3537
url: String,
36-
cacheKey: String? = null,
38+
headers: OptionalHeaders? = null,
3739
): HttpResponse
3840

3941
/**
@@ -47,6 +49,7 @@ interface IHttpClient {
4749
suspend fun put(
4850
url: String,
4951
body: JSONObject,
52+
headers: OptionalHeaders? = null,
5053
): HttpResponse
5154

5255
/**
@@ -60,6 +63,7 @@ interface IHttpClient {
6063
suspend fun patch(
6164
url: String,
6265
body: JSONObject,
66+
headers: OptionalHeaders? = null,
6367
): HttpResponse
6468

6569
/**
@@ -69,7 +73,10 @@ interface IHttpClient {
6973
*
7074
* @return The response returned.
7175
*/
72-
suspend fun delete(url: String): HttpResponse
76+
suspend fun delete(
77+
url: String,
78+
headers: OptionalHeaders? = null,
79+
): HttpResponse
7380
}
7481

7582
/**

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

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,41 +45,37 @@ internal class HttpClient(
4545
override suspend fun post(
4646
url: String,
4747
body: JSONObject,
48-
): HttpResponse {
49-
return makeRequest(url, "POST", body, _configModelStore.model.httpTimeout, null)
50-
}
48+
headers: OptionalHeaders?,
49+
): HttpResponse = makeRequest(url, "POST", body, _configModelStore.model.httpTimeout, headers)
5150

5251
override suspend fun get(
5352
url: String,
54-
cacheKey: String?,
55-
): HttpResponse {
56-
return makeRequest(url, null, null, _configModelStore.model.httpGetTimeout, cacheKey)
57-
}
53+
headers: OptionalHeaders?,
54+
): HttpResponse = makeRequest(url, null, null, _configModelStore.model.httpGetTimeout, headers)
5855

5956
override suspend fun put(
6057
url: String,
6158
body: JSONObject,
62-
): HttpResponse {
63-
return makeRequest(url, "PUT", body, _configModelStore.model.httpTimeout, null)
64-
}
59+
headers: OptionalHeaders?,
60+
): HttpResponse = makeRequest(url, "PUT", body, _configModelStore.model.httpTimeout, headers)
6561

6662
override suspend fun patch(
6763
url: String,
6864
body: JSONObject,
69-
): HttpResponse {
70-
return makeRequest(url, "PATCH", body, _configModelStore.model.httpTimeout, null)
71-
}
65+
headers: OptionalHeaders?,
66+
): HttpResponse = makeRequest(url, "PATCH", body, _configModelStore.model.httpTimeout, headers)
7267

73-
override suspend fun delete(url: String): HttpResponse {
74-
return makeRequest(url, "DELETE", null, _configModelStore.model.httpTimeout, null)
75-
}
68+
override suspend fun delete(
69+
url: String,
70+
headers: OptionalHeaders?,
71+
): HttpResponse = makeRequest(url, "DELETE", null, _configModelStore.model.httpTimeout, headers)
7672

7773
private suspend fun makeRequest(
7874
url: String,
7975
method: String?,
8076
jsonBody: JSONObject?,
8177
timeout: Int,
82-
cacheKey: String?,
78+
headers: OptionalHeaders?,
8379
): HttpResponse {
8480
// If privacy consent is required but not yet given, any non-GET request should be blocked.
8581
if (method != null && _configModelStore.model.consentRequired == true && _configModelStore.model.consentGiven != true) {
@@ -94,7 +90,7 @@ internal class HttpClient(
9490

9591
try {
9692
return withTimeout(getThreadTimeout(timeout).toLong()) {
97-
return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, cacheKey)
93+
return@withTimeout makeRequestIODispatcher(url, method, jsonBody, timeout, headers)
9894
}
9995
} catch (e: TimeoutCancellationException) {
10096
Logging.error("HttpClient: Request timed out: $url", e)
@@ -110,7 +106,7 @@ internal class HttpClient(
110106
method: String?,
111107
jsonBody: JSONObject?,
112108
timeout: Int,
113-
cacheKey: String?,
109+
headers: OptionalHeaders?,
114110
): HttpResponse {
115111
var retVal: HttpResponse? = null
116112

@@ -174,11 +170,16 @@ internal class HttpClient(
174170
outputStream.write(sendBytes)
175171
}
176172

177-
if (cacheKey != null) {
178-
val eTag = _prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + cacheKey)
173+
// H E A D E R S
179174

175+
if (headers?.cacheKey != null) {
176+
val eTag =
177+
_prefs.getString(
178+
PreferenceStores.ONESIGNAL,
179+
PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + headers.cacheKey,
180+
)
180181
if (eTag != null) {
181-
con.setRequestProperty("if-none-match", eTag)
182+
con.setRequestProperty("If-None-Match", eTag)
182183
Logging.debug("HttpClient: Adding header if-none-match: $eTag")
183184
}
184185
}
@@ -187,6 +188,7 @@ internal class HttpClient(
187188
httpResponse = con.responseCode
188189

189190
val retryAfter = retryAfterFromResponse(con)
191+
val retryLimit = retryLimitFromResponse(con)
190192
val newDelayUntil = _time.currentTimeMillis + (retryAfter ?: 0) * 1_000
191193
if (newDelayUntil > delayNewRequestsUntil) delayNewRequestsUntil = newDelayUntil
192194

@@ -195,39 +197,44 @@ internal class HttpClient(
195197
val cachedResponse =
196198
_prefs.getString(
197199
PreferenceStores.ONESIGNAL,
198-
PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + cacheKey,
200+
PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + headers?.cacheKey,
199201
)
200-
Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - Using Cached response due to 304: " + cachedResponse)
202+
Logging.debug(
203+
"HttpClient: Got Response = ${method ?: "GET"} ${con.url} - Using Cached response due to 304: " +
204+
cachedResponse,
205+
)
201206

202207
// TODO: SHOULD RETURN OK INSTEAD OF NOT_MODIFIED TO MAKE TRANSPARENT?
203-
retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter)
208+
retVal = HttpResponse(httpResponse, cachedResponse, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
204209
}
205210
HttpURLConnection.HTTP_ACCEPTED, HttpURLConnection.HTTP_CREATED, HttpURLConnection.HTTP_OK -> {
206211
val inputStream = con.inputStream
207212
val scanner = Scanner(inputStream, "UTF-8")
208213
val json = if (scanner.useDelimiter("\\A").hasNext()) scanner.next() else ""
209214
scanner.close()
210-
Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - STATUS: $httpResponse - Body: " + json)
215+
Logging.debug(
216+
"HttpClient: Got Response = ${method ?: "GET"} ${con.url} - STATUS: $httpResponse - Body: " + json,
217+
)
211218

212-
if (cacheKey != null) {
219+
if (headers?.cacheKey != null) {
213220
val eTag = con.getHeaderField("etag")
214221
if (eTag != null) {
215222
Logging.debug("HttpClient: Got Response = Response has etag of $eTag so caching the response.")
216223

217224
_prefs.saveString(
218225
PreferenceStores.ONESIGNAL,
219-
PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + cacheKey,
226+
PreferenceOneSignalKeys.PREFS_OS_ETAG_PREFIX + headers.cacheKey,
220227
eTag,
221228
)
222229
_prefs.saveString(
223230
PreferenceStores.ONESIGNAL,
224-
PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + cacheKey,
231+
PreferenceOneSignalKeys.PREFS_OS_HTTP_CACHE_PREFIX + headers.cacheKey,
225232
json,
226233
)
227234
}
228235
}
229236

230-
retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter)
237+
retVal = HttpResponse(httpResponse, json, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
231238
}
232239
else -> {
233240
Logging.debug("HttpClient: Got Response = ${method ?: "GET"} ${con.url} - FAILED STATUS: $httpResponse")
@@ -248,7 +255,7 @@ internal class HttpClient(
248255
Logging.warn("HttpClient: Got Response = $method - STATUS: $httpResponse - No response body!")
249256
}
250257

251-
retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter)
258+
retVal = HttpResponse(httpResponse, jsonResponse, retryAfterSeconds = retryAfter, retryLimit = retryLimit)
252259
}
253260
}
254261
} catch (t: Throwable) {
@@ -288,6 +295,19 @@ internal class HttpClient(
288295
}
289296
}
290297

298+
/**
299+
* Reads the HTTP Retry-Limit from the response.
300+
*/
301+
private fun retryLimitFromResponse(con: HttpURLConnection): Int? {
302+
val retryLimitStr = con.getHeaderField("Retry-Limit")
303+
return if (retryLimitStr != null) {
304+
Logging.debug("HttpClient: Response Retry-After: $retryLimitStr")
305+
retryLimitStr.toIntOrNull()
306+
} else {
307+
null
308+
}
309+
}
310+
291311
private fun logHTTPSent(
292312
method: String?,
293313
url: URL,

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package com.onesignal.core.internal.http.impl
2+
3+
data class OptionalHeaders(
4+
val cacheKey: String? = null,
5+
)

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.onesignal.core.internal.http
33
import com.onesignal.common.OneSignalUtils
44
import com.onesignal.core.internal.device.impl.InstallIdService
55
import com.onesignal.core.internal.http.impl.HttpClient
6+
import com.onesignal.core.internal.http.impl.OptionalHeaders
67
import com.onesignal.core.internal.time.impl.Time
78
import com.onesignal.debug.LogLevel
89
import com.onesignal.debug.internal.logging.Logging
@@ -88,9 +89,10 @@ class HttpClientTests : FunSpec({
8889
val httpClient = mocks.httpClient
8990

9091
// When
91-
val response1 = httpClient.get("URL", "CACHE_KEY")
92+
val headers = OptionalHeaders(cacheKey = "CACHE_KEY")
93+
val response1 = httpClient.get("URL", headers)
9294
factory.mockResponse = mockResponse2
93-
val response2 = httpClient.get("URL", "CACHE_KEY")
95+
val response2 = httpClient.get("URL", headers)
9496

9597
// Then
9698
response1.statusCode shouldBe 200
@@ -123,13 +125,14 @@ class HttpClientTests : FunSpec({
123125
val httpClient = mocks.httpClient
124126

125127
// When
126-
val response1 = httpClient.get("URL", "CACHE_KEY")
128+
val headers = OptionalHeaders(cacheKey = "CACHE_KEY")
129+
val response1 = httpClient.get("URL", headers)
127130

128131
factory.mockResponse = mockResponse2
129-
val response2 = httpClient.get("URL", "CACHE_KEY")
132+
val response2 = httpClient.get("URL", headers)
130133

131134
factory.mockResponse = mockResponse3
132-
val response3 = httpClient.get("URL", "CACHE_KEY")
135+
val response3 = httpClient.get("URL", headers)
133136

134137
// Then
135138
response1.statusCode shouldBe 200

0 commit comments

Comments
 (0)