Skip to content

Commit 6eb8e0f

Browse files
feat(client): expose sleeper option
fix(client): ensure single timer is created per client
1 parent ac75f13 commit 6eb8e0f

File tree

8 files changed

+158
-49
lines changed

8 files changed

+158
-49
lines changed

finch-java-client-okhttp/src/main/kotlin/com/tryfinch/api/client/okhttp/FinchOkHttpClient.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper
66
import com.tryfinch.api.client.FinchClient
77
import com.tryfinch.api.client.FinchClientImpl
88
import com.tryfinch.api.core.ClientOptions
9+
import com.tryfinch.api.core.Sleeper
910
import com.tryfinch.api.core.Timeout
1011
import com.tryfinch.api.core.http.AsyncStreamResponse
1112
import com.tryfinch.api.core.http.Headers
@@ -133,6 +134,17 @@ class FinchOkHttpClient private constructor() {
133134
clientOptions.streamHandlerExecutor(streamHandlerExecutor)
134135
}
135136

137+
/**
138+
* The interface to use for delaying execution, like during retries.
139+
*
140+
* This is primarily useful for using fake delays in tests.
141+
*
142+
* Defaults to real execution delays.
143+
*
144+
* This class takes ownership of the sleeper and closes it when closed.
145+
*/
146+
fun sleeper(sleeper: Sleeper) = apply { clientOptions.sleeper(sleeper) }
147+
136148
/**
137149
* The clock to use for operations that require timing, like retries.
138150
*

finch-java-client-okhttp/src/main/kotlin/com/tryfinch/api/client/okhttp/FinchOkHttpClientAsync.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.json.JsonMapper
66
import com.tryfinch.api.client.FinchClientAsync
77
import com.tryfinch.api.client.FinchClientAsyncImpl
88
import com.tryfinch.api.core.ClientOptions
9+
import com.tryfinch.api.core.Sleeper
910
import com.tryfinch.api.core.Timeout
1011
import com.tryfinch.api.core.http.AsyncStreamResponse
1112
import com.tryfinch.api.core.http.Headers
@@ -133,6 +134,17 @@ class FinchOkHttpClientAsync private constructor() {
133134
clientOptions.streamHandlerExecutor(streamHandlerExecutor)
134135
}
135136

137+
/**
138+
* The interface to use for delaying execution, like during retries.
139+
*
140+
* This is primarily useful for using fake delays in tests.
141+
*
142+
* Defaults to real execution delays.
143+
*
144+
* This class takes ownership of the sleeper and closes it when closed.
145+
*/
146+
fun sleeper(sleeper: Sleeper) = apply { clientOptions.sleeper(sleeper) }
147+
136148
/**
137149
* The clock to use for operations that require timing, like retries.
138150
*

finch-java-core/src/main/kotlin/com/tryfinch/api/core/ClientOptions.kt

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ private constructor(
5555
* This class takes ownership of the executor and shuts it down, if possible, when closed.
5656
*/
5757
@get:JvmName("streamHandlerExecutor") val streamHandlerExecutor: Executor,
58+
/**
59+
* The interface to use for delaying execution, like during retries.
60+
*
61+
* This is primarily useful for using fake delays in tests.
62+
*
63+
* Defaults to real execution delays.
64+
*
65+
* This class takes ownership of the sleeper and closes it when closed.
66+
*/
67+
@get:JvmName("sleeper") val sleeper: Sleeper,
5868
/**
5969
* The clock to use for operations that require timing, like retries.
6070
*
@@ -156,6 +166,7 @@ private constructor(
156166
private var checkJacksonVersionCompatibility: Boolean = true
157167
private var jsonMapper: JsonMapper = jsonMapper()
158168
private var streamHandlerExecutor: Executor? = null
169+
private var sleeper: Sleeper? = null
159170
private var clock: Clock = Clock.systemUTC()
160171
private var baseUrl: String? = null
161172
private var headers: Headers.Builder = Headers.builder()
@@ -174,6 +185,7 @@ private constructor(
174185
checkJacksonVersionCompatibility = clientOptions.checkJacksonVersionCompatibility
175186
jsonMapper = clientOptions.jsonMapper
176187
streamHandlerExecutor = clientOptions.streamHandlerExecutor
188+
sleeper = clientOptions.sleeper
177189
clock = clientOptions.clock
178190
baseUrl = clientOptions.baseUrl
179191
headers = clientOptions.headers.toBuilder()
@@ -231,6 +243,17 @@ private constructor(
231243
else streamHandlerExecutor
232244
}
233245

246+
/**
247+
* The interface to use for delaying execution, like during retries.
248+
*
249+
* This is primarily useful for using fake delays in tests.
250+
*
251+
* Defaults to real execution delays.
252+
*
253+
* This class takes ownership of the sleeper and closes it when closed.
254+
*/
255+
fun sleeper(sleeper: Sleeper) = apply { this.sleeper = PhantomReachableSleeper(sleeper) }
256+
234257
/**
235258
* The clock to use for operations that require timing, like retries.
236259
*
@@ -438,6 +461,25 @@ private constructor(
438461
*/
439462
fun build(): ClientOptions {
440463
val httpClient = checkRequired("httpClient", httpClient)
464+
val streamHandlerExecutor =
465+
streamHandlerExecutor
466+
?: PhantomReachableExecutorService(
467+
Executors.newCachedThreadPool(
468+
object : ThreadFactory {
469+
470+
private val threadFactory: ThreadFactory =
471+
Executors.defaultThreadFactory()
472+
private val count = AtomicLong(0)
473+
474+
override fun newThread(runnable: Runnable): Thread =
475+
threadFactory.newThread(runnable).also {
476+
it.name =
477+
"finch-stream-handler-thread-${count.getAndIncrement()}"
478+
}
479+
}
480+
)
481+
)
482+
val sleeper = sleeper ?: PhantomReachableSleeper(DefaultSleeper())
441483

442484
val headers = Headers.builder()
443485
val queryParams = QueryParams.builder()
@@ -471,26 +513,14 @@ private constructor(
471513
httpClient,
472514
RetryingHttpClient.builder()
473515
.httpClient(httpClient)
516+
.sleeper(sleeper)
474517
.clock(clock)
475518
.maxRetries(maxRetries)
476519
.build(),
477520
checkJacksonVersionCompatibility,
478521
jsonMapper,
479-
streamHandlerExecutor
480-
?: Executors.newCachedThreadPool(
481-
object : ThreadFactory {
482-
483-
private val threadFactory: ThreadFactory =
484-
Executors.defaultThreadFactory()
485-
private val count = AtomicLong(0)
486-
487-
override fun newThread(runnable: Runnable): Thread =
488-
threadFactory.newThread(runnable).also {
489-
it.name =
490-
"finch-stream-handler-thread-${count.getAndIncrement()}"
491-
}
492-
}
493-
),
522+
streamHandlerExecutor,
523+
sleeper,
494524
clock,
495525
baseUrl,
496526
headers.build(),
@@ -519,5 +549,6 @@ private constructor(
519549
fun close() {
520550
httpClient.close()
521551
(streamHandlerExecutor as? ExecutorService)?.shutdown()
552+
sleeper.close()
522553
}
523554
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.tryfinch.api.core
2+
3+
import java.time.Duration
4+
import java.util.Timer
5+
import java.util.TimerTask
6+
import java.util.concurrent.CompletableFuture
7+
8+
class DefaultSleeper : Sleeper {
9+
10+
private val timer = Timer("DefaultSleeper", true)
11+
12+
override fun sleep(duration: Duration) = Thread.sleep(duration.toMillis())
13+
14+
override fun sleepAsync(duration: Duration): CompletableFuture<Void> {
15+
val future = CompletableFuture<Void>()
16+
timer.schedule(
17+
object : TimerTask() {
18+
override fun run() {
19+
future.complete(null)
20+
}
21+
},
22+
duration.toMillis(),
23+
)
24+
return future
25+
}
26+
27+
override fun close() = timer.cancel()
28+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package com.tryfinch.api.core
2+
3+
import java.time.Duration
4+
import java.util.concurrent.CompletableFuture
5+
6+
/**
7+
* A delegating wrapper around a [Sleeper] that closes it once it's only phantom reachable.
8+
*
9+
* This class ensures the [Sleeper] is closed even if the user forgets to do it.
10+
*/
11+
internal class PhantomReachableSleeper(private val sleeper: Sleeper) : Sleeper {
12+
13+
init {
14+
closeWhenPhantomReachable(this, sleeper)
15+
}
16+
17+
override fun sleep(duration: Duration) = sleeper.sleep(duration)
18+
19+
override fun sleepAsync(duration: Duration): CompletableFuture<Void> =
20+
sleeper.sleepAsync(duration)
21+
22+
override fun close() = sleeper.close()
23+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package com.tryfinch.api.core
2+
3+
import java.time.Duration
4+
import java.util.concurrent.CompletableFuture
5+
6+
/**
7+
* An interface for delaying execution for a specified amount of time.
8+
*
9+
* Useful for testing and cleaning up resources.
10+
*/
11+
interface Sleeper : AutoCloseable {
12+
13+
/** Synchronously pauses execution for the given [duration]. */
14+
fun sleep(duration: Duration)
15+
16+
/** Asynchronously pauses execution for the given [duration]. */
17+
fun sleepAsync(duration: Duration): CompletableFuture<Void>
18+
19+
/** Overridden from [AutoCloseable] to not have a checked exception in its signature. */
20+
override fun close()
21+
}

finch-java-core/src/main/kotlin/com/tryfinch/api/core/http/RetryingHttpClient.kt

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.tryfinch.api.core.http
22

3+
import com.tryfinch.api.core.DefaultSleeper
34
import com.tryfinch.api.core.RequestOptions
5+
import com.tryfinch.api.core.Sleeper
46
import com.tryfinch.api.core.checkRequired
57
import com.tryfinch.api.errors.FinchIoException
68
import com.tryfinch.api.errors.FinchRetryableException
@@ -11,8 +13,6 @@ import java.time.OffsetDateTime
1113
import java.time.format.DateTimeFormatter
1214
import java.time.format.DateTimeParseException
1315
import java.time.temporal.ChronoUnit
14-
import java.util.Timer
15-
import java.util.TimerTask
1616
import java.util.UUID
1717
import java.util.concurrent.CompletableFuture
1818
import java.util.concurrent.ThreadLocalRandom
@@ -130,7 +130,10 @@ private constructor(
130130
return executeWithRetries(modifiedRequest, requestOptions)
131131
}
132132

133-
override fun close() = httpClient.close()
133+
override fun close() {
134+
httpClient.close()
135+
sleeper.close()
136+
}
134137

135138
private fun isRetryable(request: HttpRequest): Boolean =
136139
// Some requests, such as when a request body is being streamed, cannot be retried because
@@ -235,33 +238,14 @@ private constructor(
235238
class Builder internal constructor() {
236239

237240
private var httpClient: HttpClient? = null
238-
private var sleeper: Sleeper =
239-
object : Sleeper {
240-
241-
private val timer = Timer("RetryingHttpClient", true)
242-
243-
override fun sleep(duration: Duration) = Thread.sleep(duration.toMillis())
244-
245-
override fun sleepAsync(duration: Duration): CompletableFuture<Void> {
246-
val future = CompletableFuture<Void>()
247-
timer.schedule(
248-
object : TimerTask() {
249-
override fun run() {
250-
future.complete(null)
251-
}
252-
},
253-
duration.toMillis(),
254-
)
255-
return future
256-
}
257-
}
241+
private var sleeper: Sleeper? = null
258242
private var clock: Clock = Clock.systemUTC()
259243
private var maxRetries: Int = 2
260244
private var idempotencyHeader: String? = null
261245

262246
fun httpClient(httpClient: HttpClient) = apply { this.httpClient = httpClient }
263247

264-
@JvmSynthetic internal fun sleeper(sleeper: Sleeper) = apply { this.sleeper = sleeper }
248+
fun sleeper(sleeper: Sleeper) = apply { this.sleeper = sleeper }
265249

266250
fun clock(clock: Clock) = apply { this.clock = clock }
267251

@@ -272,17 +256,10 @@ private constructor(
272256
fun build(): HttpClient =
273257
RetryingHttpClient(
274258
checkRequired("httpClient", httpClient),
275-
sleeper,
259+
sleeper ?: DefaultSleeper(),
276260
clock,
277261
maxRetries,
278262
idempotencyHeader,
279263
)
280264
}
281-
282-
internal interface Sleeper {
283-
284-
fun sleep(duration: Duration)
285-
286-
fun sleepAsync(duration: Duration): CompletableFuture<Void>
287-
}
288265
}

finch-java-core/src/test/kotlin/com/tryfinch/api/core/http/RetryingHttpClientTest.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.github.tomakehurst.wiremock.junit5.WireMockTest
66
import com.github.tomakehurst.wiremock.stubbing.Scenario
77
import com.tryfinch.api.client.okhttp.OkHttpClient
88
import com.tryfinch.api.core.RequestOptions
9+
import com.tryfinch.api.core.Sleeper
910
import com.tryfinch.api.errors.FinchRetryableException
1011
import java.io.InputStream
1112
import java.time.Duration
@@ -294,12 +295,14 @@ internal class RetryingHttpClientTest {
294295
.httpClient(failingHttpClient)
295296
.maxRetries(2)
296297
.sleeper(
297-
object : RetryingHttpClient.Sleeper {
298+
object : Sleeper {
298299

299300
override fun sleep(duration: Duration) {}
300301

301302
override fun sleepAsync(duration: Duration): CompletableFuture<Void> =
302303
CompletableFuture.completedFuture(null)
304+
305+
override fun close() {}
303306
}
304307
)
305308
.build()
@@ -333,12 +336,14 @@ internal class RetryingHttpClientTest {
333336
.httpClient(httpClient)
334337
// Use a no-op `Sleeper` to make the test fast.
335338
.sleeper(
336-
object : RetryingHttpClient.Sleeper {
339+
object : Sleeper {
337340

338341
override fun sleep(duration: Duration) {}
339342

340343
override fun sleepAsync(duration: Duration): CompletableFuture<Void> =
341344
CompletableFuture.completedFuture(null)
345+
346+
override fun close() {}
342347
}
343348
)
344349

0 commit comments

Comments
 (0)