From e82234dc1fd25df2f7be395956725cd435a893eb Mon Sep 17 00:00:00 2001 From: doyaaaaaken Date: Wed, 14 Aug 2019 23:55:58 +0900 Subject: [PATCH 01/11] handle Retry-After header --- .../kohttp/interceptors/RetryInterceptor.kt | 29 ++++++++++++++----- .../interceptors/RetryInterceptorTest.kt | 4 +-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index ce8d533f..21f6c185 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -3,6 +3,7 @@ package io.github.rybalkinsd.kohttp.interceptors import okhttp3.Interceptor import okhttp3.Response import java.net.SocketTimeoutException +import kotlin.math.max /** * Retry Interceptor @@ -18,10 +19,10 @@ import java.net.SocketTimeoutException * @author UDarya * */ class RetryInterceptor( - private val failureThreshold: Int = 3, - private val invocationTimeout: Long = 0, - private val ratio: Int = 1, - private val errorStatuses: List = listOf(503, 504) + private val failureThreshold: Int = 3, + private val invocationTimeout: Long = 0, + private val ratio: Int = 1, + private val errorStatuses: List = listOf(429, 503, 504) ) : Interceptor { override fun intercept(chain: Interceptor.Chain): Response { @@ -34,7 +35,13 @@ class RetryInterceptor( delay = performAndReturnDelay(delay) } val response = chain.proceed(request) - if (!isRetry(response, attemptsCount)) return response + + val retryAfter = calculateRetryAfter(response, attemptsCount, delay, chain.readTimeoutMillis()) + if (retryAfter != null) { + delay = retryAfter + } else { + return response + } } catch (e: SocketTimeoutException) { if (attemptsCount >= failureThreshold) throw e } @@ -52,6 +59,14 @@ class RetryInterceptor( private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 - internal fun isRetry(response: Response, attemptsCount: Int): Boolean = - attemptsCount < failureThreshold && response.code() in errorStatuses + internal fun calculateRetryAfter(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { + val retryAfter = response.header("Retry-After")?.toLongOrNull() + if (retryAfter != null && retryAfter > maxDelayTime) return null + + return if (attemptsCount < failureThreshold && response.code() in errorStatuses) { + max(retryAfter ?: 0, delay) + } else { + null + } + } } diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index 3820c01e..c26da071 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -137,14 +137,14 @@ class RetryInterceptorTest { fun `not need retry if status code is 200`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(200).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertFalse(RetryInterceptor().isRetry(response, 1)) + Assert.assertNull(RetryInterceptor().calculateRetryAfter(response, 1, 2, 30)) } @Test fun `need retry if status code in error codes list`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(503).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertTrue(RetryInterceptor().isRetry(response, 1)) + Assert.assertNotNull(RetryInterceptor().calculateRetryAfter(response, 1, 2, 30)) } private fun getCall(client: OkHttpClient) { From 29905f23f0b256b3fa63bfb0d5dc9fe53ee1b8e6 Mon Sep 17 00:00:00 2001 From: doyaaaaaken Date: Thu, 15 Aug 2019 08:55:02 +0900 Subject: [PATCH 02/11] rename variable and method --- .../rybalkinsd/kohttp/interceptors/RetryInterceptor.kt | 8 ++++---- .../kohttp/interceptors/RetryInterceptorTest.kt | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 21f6c185..db1a4ee0 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -36,9 +36,9 @@ class RetryInterceptor( } val response = chain.proceed(request) - val retryAfter = calculateRetryAfter(response, attemptsCount, delay, chain.readTimeoutMillis()) - if (retryAfter != null) { - delay = retryAfter + val nextTime = calculateNextRetry(response, attemptsCount, delay, chain.readTimeoutMillis()) + if (nextTime != null) { + delay = nextTime } else { return response } @@ -59,7 +59,7 @@ class RetryInterceptor( private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 - internal fun calculateRetryAfter(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { + internal fun calculateNextRetry(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { val retryAfter = response.header("Retry-After")?.toLongOrNull() if (retryAfter != null && retryAfter > maxDelayTime) return null diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index c26da071..c8040f4a 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -137,14 +137,14 @@ class RetryInterceptorTest { fun `not need retry if status code is 200`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(200).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertNull(RetryInterceptor().calculateRetryAfter(response, 1, 2, 30)) + Assert.assertNull(RetryInterceptor().calculateNextRetry(response, 1, 2, 30)) } @Test fun `need retry if status code in error codes list`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(503).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertNotNull(RetryInterceptor().calculateRetryAfter(response, 1, 2, 30)) + Assert.assertNotNull(RetryInterceptor().calculateNextRetry(response, 1, 2, 30)) } private fun getCall(client: OkHttpClient) { From ab0ed64516b0c94e699df586a736296fdf5519fc Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Fri, 30 Aug 2019 18:54:13 +0900 Subject: [PATCH 03/11] Revert format change --- .../rybalkinsd/kohttp/interceptors/RetryInterceptor.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index db1a4ee0..6dac7d2f 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -19,10 +19,10 @@ import kotlin.math.max * @author UDarya * */ class RetryInterceptor( - private val failureThreshold: Int = 3, - private val invocationTimeout: Long = 0, - private val ratio: Int = 1, - private val errorStatuses: List = listOf(429, 503, 504) + private val failureThreshold: Int = 3, + private val invocationTimeout: Long = 0, + private val ratio: Int = 1, + private val errorStatuses: List = listOf(429, 503, 504) ) : Interceptor { override fun intercept(chain: Interceptor.Chain): Response { From 16a757b28250a9b0a67d56214edf0c140bd48718 Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Fri, 30 Aug 2019 19:02:13 +0900 Subject: [PATCH 04/11] fix bug: regard the unit of Retry-After as second (not millisec) --- .../github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 6dac7d2f..c32c5842 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -60,7 +60,7 @@ class RetryInterceptor( private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 internal fun calculateNextRetry(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { - val retryAfter = response.header("Retry-After")?.toLongOrNull() + val retryAfter = response.header("Retry-After")?.toLongOrNull()?.let { it * 1000 } if (retryAfter != null && retryAfter > maxDelayTime) return null return if (attemptsCount < failureThreshold && response.code() in errorStatuses) { From 642a48893eaf9716f7a302e2ee01663a2e72fe21 Mon Sep 17 00:00:00 2001 From: doyaaaaaken Date: Sun, 1 Sep 2019 15:03:29 +0900 Subject: [PATCH 05/11] refactor: simplify logic --- .../kohttp/interceptors/RetryInterceptor.kt | 22 ++++++------------- .../interceptors/RetryInterceptorTest.kt | 17 +++++++------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index c32c5842..c38ca31f 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -27,21 +27,14 @@ class RetryInterceptor( override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() + val maxDelayTime = chain.readTimeoutMillis() var attemptsCount = 0 var delay = invocationTimeout while (true) { try { - if (shouldDelay(attemptsCount)) { - delay = performAndReturnDelay(delay) - } + if (shouldDelay(attemptsCount)) performDelay(delay) val response = chain.proceed(request) - - val nextTime = calculateNextRetry(response, attemptsCount, delay, chain.readTimeoutMillis()) - if (nextTime != null) { - delay = nextTime - } else { - return response - } + delay = calculateNextRetry(response, attemptsCount, delay, maxDelayTime) ?: return response } catch (e: SocketTimeoutException) { if (attemptsCount >= failureThreshold) throw e } @@ -49,22 +42,21 @@ class RetryInterceptor( } } - internal fun performAndReturnDelay(delay: Long): Long { + private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 + + private fun performDelay(delay: Long) { try { Thread.sleep(delay) } catch (ignored: InterruptedException) { } - return delay * ratio } - private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 - internal fun calculateNextRetry(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { val retryAfter = response.header("Retry-After")?.toLongOrNull()?.let { it * 1000 } if (retryAfter != null && retryAfter > maxDelayTime) return null return if (attemptsCount < failureThreshold && response.code() in errorStatuses) { - max(retryAfter ?: 0, delay) + max(retryAfter ?: 0, delay * ratio) } else { null } diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index c8040f4a..861d337d 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -90,14 +90,15 @@ class RetryInterceptorTest { verifyGetRequest(1) } - @Test - fun `delay increase with step`() { - val retryInterceptor = RetryInterceptor(ratio = 2) - val invocationTimeout: Long = 1000 - assert(retryInterceptor.performAndReturnDelay(invocationTimeout) == invocationTimeout * 2) - assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 2) == invocationTimeout * 4) - assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 4) == invocationTimeout * 8) - } + //TODO: rewrite test +// @Test +// fun `delay increase with step`() { +// val retryInterceptor = RetryInterceptor(ratio = 2) +// val invocationTimeout: Long = 1000 +// assert(retryInterceptor.performAndReturnDelay(invocationTimeout) == invocationTimeout * 2) +// assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 2) == invocationTimeout * 4) +// assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 4) == invocationTimeout * 8) +// } @Test fun `retry just next interceptors`() { From 41714abe08079066af66ecea4881d6a696958d33 Mon Sep 17 00:00:00 2001 From: doyaaaaaken Date: Sun, 1 Sep 2019 15:43:45 +0900 Subject: [PATCH 06/11] rewrite test `delay increase with step` --- .../kohttp/interceptors/RetryInterceptor.kt | 10 ++++---- .../interceptors/RetryInterceptorTest.kt | 23 ++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index c38ca31f..6640a020 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -1,5 +1,6 @@ package io.github.rybalkinsd.kohttp.interceptors +import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Response import java.net.SocketTimeoutException @@ -34,7 +35,8 @@ class RetryInterceptor( try { if (shouldDelay(attemptsCount)) performDelay(delay) val response = chain.proceed(request) - delay = calculateNextRetry(response, attemptsCount, delay, maxDelayTime) ?: return response + delay = calculateNextRetry(response.headers(), response.code(), attemptsCount, delay, maxDelayTime) + ?: return response } catch (e: SocketTimeoutException) { if (attemptsCount >= failureThreshold) throw e } @@ -51,11 +53,11 @@ class RetryInterceptor( } } - internal fun calculateNextRetry(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { - val retryAfter = response.header("Retry-After")?.toLongOrNull()?.let { it * 1000 } + internal fun calculateNextRetry(responseHeaders: Headers, responseCode: Int, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { + val retryAfter = responseHeaders["Retry-After"]?.toLongOrNull()?.let { it * 1000 } if (retryAfter != null && retryAfter > maxDelayTime) return null - return if (attemptsCount < failureThreshold && response.code() in errorStatuses) { + return if (attemptsCount < failureThreshold && responseCode in errorStatuses) { max(retryAfter ?: 0, delay * ratio) } else { null diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index 861d337d..102ce197 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -20,6 +20,7 @@ import java.net.SocketTimeoutException import java.security.MessageDigest import java.util.* import java.util.concurrent.TimeUnit +import kotlin.test.assertEquals class RetryInterceptorTest { @@ -90,15 +91,15 @@ class RetryInterceptorTest { verifyGetRequest(1) } - //TODO: rewrite test -// @Test -// fun `delay increase with step`() { -// val retryInterceptor = RetryInterceptor(ratio = 2) -// val invocationTimeout: Long = 1000 -// assert(retryInterceptor.performAndReturnDelay(invocationTimeout) == invocationTimeout * 2) -// assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 2) == invocationTimeout * 4) -// assert(retryInterceptor.performAndReturnDelay(invocationTimeout * 4) == invocationTimeout * 8) -// } + @Test + fun `delay increase by ratio`() { + val ratio = 2 + val retryInterceptor = RetryInterceptor(ratio = ratio) + val invocationTimeout: Long = 1000 + val responseHeaders = Headers.of() + val responseCode = 429 + assertEquals(invocationTimeout * ratio, retryInterceptor.calculateNextRetry(responseHeaders, responseCode, 0, invocationTimeout, 10000)) + } @Test fun `retry just next interceptors`() { @@ -138,14 +139,14 @@ class RetryInterceptorTest { fun `not need retry if status code is 200`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(200).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertNull(RetryInterceptor().calculateNextRetry(response, 1, 2, 30)) + Assert.assertNull(RetryInterceptor().calculateNextRetry(response.headers(), response.code(), 1, 2, 30)) } @Test fun `need retry if status code in error codes list`() { val request = Request.Builder().url(HttpUrl.Builder().host(localhost).scheme("http").build()).build() val response = Response.Builder().code(503).protocol(Protocol.HTTP_1_1).message("").request(request).build() - Assert.assertNotNull(RetryInterceptor().calculateNextRetry(response, 1, 2, 30)) + Assert.assertNotNull(RetryInterceptor().calculateNextRetry(response.headers(), response.code(), 1, 2, 30)) } private fun getCall(client: OkHttpClient) { From c4c3397135cc288103cfa3dd0c6bd6f8bfd5e8f7 Mon Sep 17 00:00:00 2001 From: doyaaaaaken Date: Sun, 1 Sep 2019 16:39:37 +0900 Subject: [PATCH 07/11] enable to parse httpDate format's Retry-After header --- .../kohttp/interceptors/RetryInterceptor.kt | 19 ++++++++++++++- .../interceptors/RetryInterceptorTest.kt | 23 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 6640a020..5444ec3a 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -4,6 +4,9 @@ import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Response import java.net.SocketTimeoutException +import java.text.ParseException +import java.text.SimpleDateFormat +import java.time.Instant import kotlin.math.max /** @@ -26,6 +29,9 @@ class RetryInterceptor( private val errorStatuses: List = listOf(429, 503, 504) ) : Interceptor { + //https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Date#Syntax + private val httpDateFormat = SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz") + override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val maxDelayTime = chain.readTimeoutMillis() @@ -54,7 +60,7 @@ class RetryInterceptor( } internal fun calculateNextRetry(responseHeaders: Headers, responseCode: Int, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { - val retryAfter = responseHeaders["Retry-After"]?.toLongOrNull()?.let { it * 1000 } + val retryAfter = parseRetryAfter(responseHeaders) if (retryAfter != null && retryAfter > maxDelayTime) return null return if (attemptsCount < failureThreshold && responseCode in errorStatuses) { @@ -63,4 +69,15 @@ class RetryInterceptor( null } } + + internal fun parseRetryAfter(headers: Headers): Long? { + val retryAfter = headers["Retry-After"] ?: return null + return retryAfter.toLongOrNull()?.let { it * 1000 } ?: try { + val date = httpDateFormat.parse(retryAfter).toInstant().epochSecond + val now = Instant.now().epochSecond + (date - now).takeIf { it > 0 } + } catch (ex: ParseException) { + null + } + } } diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index 102ce197..1d1193bf 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -21,6 +21,8 @@ import java.security.MessageDigest import java.util.* import java.util.concurrent.TimeUnit import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull class RetryInterceptorTest { @@ -149,6 +151,27 @@ class RetryInterceptorTest { Assert.assertNotNull(RetryInterceptor().calculateNextRetry(response.headers(), response.code(), 1, 2, 30)) } + @Test + fun `parse RetryAfter header whose unit is second`() { + val interceptor = RetryInterceptor() + val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "2"))) + assertEquals(2 * 1000, retryAfter) + } + + @Test + fun `parse RetryAfter header whose unit is date`() { + val interceptor = RetryInterceptor() + val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "Wed, 21 Oct 2115 07:28:00 GMT"))) + assertNotNull(retryAfter) + } + + @Test + fun `return null if cannot parse RetryAfter header`() { + val interceptor = RetryInterceptor() + val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "Wed,, 21 Oct 2115 07:28:00 GMT"))) + assertNull(retryAfter) + } + private fun getCall(client: OkHttpClient) { httpGet(client = client) { host = localhost From ac1042f30fc7171e36beb9fbd5fbe726482da237 Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Wed, 4 Sep 2019 19:27:02 +0900 Subject: [PATCH 08/11] separate parse logic by strategy --- .../kohttp/interceptors/RetryInterceptor.kt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 5444ec3a..9e432e37 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -72,8 +72,16 @@ class RetryInterceptor( internal fun parseRetryAfter(headers: Headers): Long? { val retryAfter = headers["Retry-After"] ?: return null - return retryAfter.toLongOrNull()?.let { it * 1000 } ?: try { - val date = httpDateFormat.parse(retryAfter).toInstant().epochSecond + return parseRetryAfterAsNumber(retryAfter) ?: parseRetryAfterAsDate(retryAfter) + } + + private fun parseRetryAfterAsNumber(headerValue: String): Long? { + return headerValue.toLongOrNull()?.let { it * 1000 } + } + + private fun parseRetryAfterAsDate(headerValue: String): Long? { + return try { + val date = httpDateFormat.parse(headerValue).toInstant().epochSecond val now = Instant.now().epochSecond (date - now).takeIf { it > 0 } } catch (ex: ParseException) { From 173270d62ebdb435ab0271f3ea5d057ba049eedb Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Wed, 4 Sep 2019 19:42:23 +0900 Subject: [PATCH 09/11] Use ZonedDateTime in parse logic --- .../kohttp/interceptors/RetryInterceptor.kt | 15 +++++++++------ .../kohttp/interceptors/RetryInterceptorTest.kt | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 9e432e37..5ac4d2aa 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -4,9 +4,11 @@ import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Response import java.net.SocketTimeoutException -import java.text.ParseException import java.text.SimpleDateFormat -import java.time.Instant +import java.time.Duration +import java.time.ZonedDateTime +import java.time.format.DateTimeFormatter +import java.time.format.DateTimeParseException import kotlin.math.max /** @@ -81,10 +83,11 @@ class RetryInterceptor( private fun parseRetryAfterAsDate(headerValue: String): Long? { return try { - val date = httpDateFormat.parse(headerValue).toInstant().epochSecond - val now = Instant.now().epochSecond - (date - now).takeIf { it > 0 } - } catch (ex: ParseException) { + val responseDate = ZonedDateTime.parse(headerValue, DateTimeFormatter.RFC_1123_DATE_TIME) + val now = ZonedDateTime.now() + val duration = Duration.between(now, responseDate) + duration.toMillis().takeIf { it > 0 } + } catch (ex: DateTimeParseException) { null } } diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index 1d1193bf..2689c157 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -161,12 +161,12 @@ class RetryInterceptorTest { @Test fun `parse RetryAfter header whose unit is date`() { val interceptor = RetryInterceptor() - val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "Wed, 21 Oct 2115 07:28:00 GMT"))) + val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "Mon, 21 Oct 2115 07:28:00 GMT"))) assertNotNull(retryAfter) } @Test - fun `return null if cannot parse RetryAfter header`() { + fun `return null if RetryAfter header's date value is invalid`() { val interceptor = RetryInterceptor() val retryAfter = interceptor.parseRetryAfter(Headers.of(mapOf("Retry-After" to "Wed,, 21 Oct 2115 07:28:00 GMT"))) assertNull(retryAfter) From 74e2e842342a69cd1539aa2dfb24e464c8bac26e Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Wed, 4 Sep 2019 19:45:00 +0900 Subject: [PATCH 10/11] remove unused code --- .../github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt index 5ac4d2aa..30da082f 100644 --- a/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt +++ b/kohttp/src/main/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptor.kt @@ -4,7 +4,6 @@ import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Response import java.net.SocketTimeoutException -import java.text.SimpleDateFormat import java.time.Duration import java.time.ZonedDateTime import java.time.format.DateTimeFormatter @@ -31,9 +30,6 @@ class RetryInterceptor( private val errorStatuses: List = listOf(429, 503, 504) ) : Interceptor { - //https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Date#Syntax - private val httpDateFormat = SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz") - override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val maxDelayTime = chain.readTimeoutMillis() From 831264915b2d9a7f93261d9ecd8f50093e6350c2 Mon Sep 17 00:00:00 2001 From: Koyama Kenta Date: Wed, 4 Sep 2019 20:01:01 +0900 Subject: [PATCH 11/11] add test code --- .../kohttp/interceptors/RetryInterceptorTest.kt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt index 2689c157..2bc6ed29 100644 --- a/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt +++ b/kohttp/src/test/kotlin/io/github/rybalkinsd/kohttp/interceptors/RetryInterceptorTest.kt @@ -151,6 +151,22 @@ class RetryInterceptorTest { Assert.assertNotNull(RetryInterceptor().calculateNextRetry(response.headers(), response.code(), 1, 2, 30)) } + @Test + fun `get Retry-After Header's value as next retry`() { + val maxDelayTime = 2001 + val retryAfterSeconds = 2L + val retryAfter = RetryInterceptor().calculateNextRetry(Headers.of(mapOf("Retry-After" to retryAfterSeconds.toString())), 429, 1, 2, maxDelayTime) + assertEquals(retryAfterSeconds * 1000, retryAfter) + } + + @Test + fun `get null as next retry if Retry-After Header's value exceeds maxDelayTime`() { + val maxDelayTime = 1999 + val retryAfterSeconds = 2L + val retryAfter = RetryInterceptor().calculateNextRetry(Headers.of(mapOf("Retry-After" to retryAfterSeconds.toString())), 429, 1, 2, maxDelayTime) + assertNull(retryAfter) + } + @Test fun `parse RetryAfter header whose unit is second`() { val interceptor = RetryInterceptor()