Implement ConcurrentRequestStrategy to support awaiting in-flight requests.#3326
Implement ConcurrentRequestStrategy to support awaiting in-flight requests.#3326colinrtwhite merged 22 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a ConcurrentRequestStrategy abstraction and wires it into NetworkFetcher (and the OkHttp/Ktor factories) to coordinate concurrent network requests for the same resource, including a de-duping strategy that allows waiters to resume after the leader finishes.
Changes:
- Add
ConcurrentRequestStrategy+DeDupeConcurrentRequestStrategyand comprehensive unit tests for coordination behavior. - Update
NetworkFetcherto apply a configurable concurrency strategy keyed by the disk cache key, and adjust constructor/factory wiring (including binary-compat shims). - Update OkHttp/Ktor factory APIs and network fetcher tests to pass through
ConcurrentRequestStrategy.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/test-utils/src/commonMain/kotlin/coil3/test/utils/AbstractNetworkFetcherTest.kt | Adds an integration-style test verifying de-duping results in 1 network fetch + remaining disk reads. |
| coil-network-okhttp/src/commonTest/kotlin/coil3/network/okhttp/OkHttpNetworkFetcherTest.kt | Updates test factory creation to inject a ConcurrentRequestStrategy. |
| coil-network-okhttp/src/commonMain/kotlin/coil3/network/okhttp/OkHttpNetworkFetcher.kt | Adds a new OkHttpNetworkFetcherFactory overload that accepts concurrentRequestStrategy while keeping hidden binary-compat overload. |
| coil-network-okhttp/api/jvm/coil-network-okhttp.api | Updates JVM API surface to reflect the new factory overload(s). |
| coil-network-okhttp/api/android/coil-network-okhttp.api | Updates Android API surface to reflect the new factory overload(s). |
| coil-network-ktor3/src/commonTest/kotlin/coil3/network/ktor3/KtorNetworkFetcherTest.kt | Updates test factory creation to pass a ConcurrentRequestStrategy. |
| coil-network-ktor3/src/commonMain/kotlin/coil3/network/ktor3/KtorNetworkFetcher.kt | Adds overloads to accept concurrentRequestStrategy and keeps hidden binary-compat overloads. |
| coil-network-ktor3/api/jvm/coil-network-ktor3.api | Updates JVM API surface for new Ktor factory overload(s). |
| coil-network-ktor3/api/coil-network-ktor3.klib.api | Updates KLIB API surface for new Ktor factory overload(s). |
| coil-network-ktor3/api/android/coil-network-ktor3.api | Updates Android API surface for new Ktor factory overload(s). |
| coil-network-ktor2/src/commonTest/kotlin/coil3/network/ktor2/KtorNetworkFetcherTest.kt | Updates test factory creation to pass a ConcurrentRequestStrategy. |
| coil-network-ktor2/src/commonMain/kotlin/coil3/network/ktor2/KtorNetworkFetcher.kt | Adds overloads to accept concurrentRequestStrategy and keeps hidden binary-compat overloads. |
| coil-network-ktor2/api/jvm/coil-network-ktor2.api | Updates JVM API surface for new Ktor factory overload(s). |
| coil-network-ktor2/api/coil-network-ktor2.klib.api | Updates KLIB API surface for new Ktor factory overload(s). |
| coil-network-ktor2/api/android/coil-network-ktor2.api | Updates Android API surface for new Ktor factory overload(s). |
| coil-network-core/src/commonTest/kotlin/coil3/network/NetworkFetcherTest.kt | Updates NetworkFetcher tests for the new constructor signature (lazy connectivity + strategy). |
| coil-network-core/src/commonTest/kotlin/coil3/network/ConcurrentRequestStrategyTest.kt | Adds new tests covering uncoordinated vs de-duped concurrency semantics and failure/cancellation behavior. |
| coil-network-core/src/commonMain/kotlin/coil3/network/NetworkFetcher.kt | Wires ConcurrentRequestStrategy into fetch execution and updates factory/constructor (with hidden binary-compat constructor). |
| coil-network-core/src/commonMain/kotlin/coil3/network/ConcurrentRequestStrategy.kt | Introduces ConcurrentRequestStrategy and a de-dupe implementation backed by keyed coordination. |
| coil-network-core/api/jvm/coil-network-core.api | Updates JVM API surface for new strategy + updated NetworkFetcher/Factory signatures. |
| coil-network-core/api/android/coil-network-core.api | Updates Android API surface (currently appears inconsistent with source). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # coil-network-core/api/coil-network-core.klib.api
a338a6c to
e48fcc8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
luciofm
left a comment
There was a problem hiding this comment.
Thanks @colinrtwhite 😄 , I was planning to get back to it but you beat me...
Did some testing and all look good to me
Minor tweaks (naming, param order, binary compatibility fixes, extra tests) to @luciofm's work to implement concurrent request handling (thank you!). Full credit to him.