Skip to content

Commit

Permalink
Fix FakeImageEngine not updating the chain's request.
Browse files Browse the repository at this point in the history
  • Loading branch information
colinrtwhite committed Oct 25, 2023
1 parent c2dd9b4 commit 17e1259
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 12 deletions.
1 change: 1 addition & 0 deletions coil-base/api/coil-base.api
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ public abstract interface class coil/intercept/Interceptor$Chain {
public abstract fun getRequest ()Lcoil/request/ImageRequest;
public abstract fun getSize ()Lcoil/size/Size;
public abstract fun proceed (Lcoil/request/ImageRequest;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun withRequest (Lcoil/request/ImageRequest;)Lcoil/intercept/Interceptor$Chain;
public abstract fun withSize (Lcoil/size/Size;)Lcoil/intercept/Interceptor$Chain;
}

Expand Down
16 changes: 15 additions & 1 deletion coil-base/src/main/java/coil/intercept/Interceptor.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package coil.intercept

import coil.ImageLoader
import coil.annotation.ExperimentalCoilApi
import coil.request.ImageRequest
import coil.request.ImageResult
import coil.size.Size
Expand All @@ -19,7 +20,20 @@ fun interface Interceptor {
val size: Size

/**
* Set the requested [Size] to load the image at.
* Copy the current [Chain] and replace [request].
*
* This method is similar to [proceed] except it doesn't advance the chain to the
* next interceptor.
*
* @param request The current image request.
*/
@ExperimentalCoilApi
fun withRequest(request: ImageRequest): Chain

/**
* Copy the current [Chain] and replace [size].
*
* Use this method to replace the resolved size for this image request.
*
* @param size The requested size for the image.
*/
Expand Down
29 changes: 22 additions & 7 deletions coil-base/src/main/java/coil/intercept/RealInterceptorChain.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ internal class RealInterceptorChain(
val isPlaceholderCached: Boolean,
) : Interceptor.Chain {

override fun withSize(size: Size) = copy(size = size)
override fun withRequest(request: ImageRequest): Interceptor.Chain {
checkRequest(request, null)
return copy(request = request)
}

override fun withSize(size: Size): Interceptor.Chain {
return copy(size = size)
}

override suspend fun proceed(request: ImageRequest): ImageResult {
if (index > 0) checkRequest(request, interceptors[index - 1])
Expand All @@ -27,25 +34,33 @@ internal class RealInterceptorChain(
return result
}

private fun checkRequest(request: ImageRequest, interceptor: Interceptor) {
private fun checkRequest(request: ImageRequest, interceptor: Interceptor?) {
check(request.context === initialRequest.context) {
"Interceptor '$interceptor' cannot modify the request's context."
"${errorPrefix(interceptor)} cannot modify the request's context."
}
check(request.data !== NullRequestData) {
"Interceptor '$interceptor' cannot set the request's data to null."
"${errorPrefix(interceptor)} cannot set the request's data to null."
}
check(request.target === initialRequest.target) {
"Interceptor '$interceptor' cannot modify the request's target."
"${errorPrefix(interceptor)} cannot modify the request's target."
}
check(request.lifecycle === initialRequest.lifecycle) {
"Interceptor '$interceptor' cannot modify the request's lifecycle."
"${errorPrefix(interceptor)} cannot modify the request's lifecycle."
}
check(request.sizeResolver === initialRequest.sizeResolver) {
"Interceptor '$interceptor' cannot modify the request's size resolver. " +
"${errorPrefix(interceptor)} cannot modify the request's size resolver. " +
"Use `Interceptor.Chain.withSize` instead."
}
}

private fun errorPrefix(interceptor: Interceptor?): String {
if (interceptor != null) {
return "Interceptor '$interceptor'"
} else {
return "'withRequest'"
}
}

private fun copy(
index: Int = this.index,
request: ImageRequest = this.request,
Expand Down
14 changes: 14 additions & 0 deletions coil-base/src/test/java/coil/intercept/RealInterceptorChainTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ class RealInterceptorChainTest {
assertSame(request, result.request)
}

@Test
fun `withRequest modifies the chain's request`() = runTest {
val initialRequest = createRequest(context)
val interceptor1 = Interceptor { chain ->
val request = chain.request.newBuilder()
.memoryCacheKey(MemoryCache.Key("test"))
.build()
assertEquals(chain.withRequest(request).request, request)
chain.proceed(request)
}

testChain(initialRequest, listOf(interceptor1))
}

private suspend fun testChain(request: ImageRequest, interceptors: List<Interceptor>): ImageResult {
val chain = RealInterceptorChain(
initialRequest = request,
Expand Down
11 changes: 7 additions & 4 deletions coil-test/src/main/java/coil/test/FakeImageLoaderEngine.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import coil.intercept.Interceptor
import coil.request.ImageRequest
import coil.request.ImageResult
import coil.size.Size
import coil.test.FakeImageLoaderEngine.RequestTransformer
import coil.transition.Transition
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
Expand Down Expand Up @@ -47,19 +48,21 @@ class FakeImageLoaderEngine private constructor(
val results: Flow<ResultValue> get() = _results.asSharedFlow()

override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
val request = RequestValue(requestTransformer.transform(chain.request), chain.size)
_requests.emit(request)
val request = requestTransformer.transform(chain.request)
val requestValue = RequestValue(request, chain.size)
_requests.emit(requestValue)

var result: ImageResult? = null
for (interceptor in interceptors) {
result = interceptor.intercept(chain)
val newChain = chain.withRequest(request)
result = interceptor.intercept(newChain)
if (result != null) break
}
if (result == null) {
result = defaultInterceptor.intercept(chain)
}

_results.emit(ResultValue(request, result))
_results.emit(ResultValue(requestValue, result))
return result
}

Expand Down
20 changes: 20 additions & 0 deletions coil-test/src/test/java/coil/test/FakeImageLoaderEngineTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import coil.decode.DataSource
import coil.request.ImageRequest
import coil.request.SuccessResult
import coil.test.FakeImageLoaderEngine.OptionalInterceptor
import coil.transition.Transition
import kotlin.test.assertIs
import kotlin.test.assertSame
import kotlinx.coroutines.test.runTest
Expand Down Expand Up @@ -124,4 +125,23 @@ class FakeImageLoaderEngineTest {
assertSame(drawable, result.drawable)
}
}

@Test
fun `removes transition factory`() = runTest {
val url = "https://www.example.com/image.jpg"
val engine = FakeImageLoaderEngine.Builder()
.intercept(url, ColorDrawable(Color.RED))
.build()
val imageLoader = ImageLoader.Builder(context)
.components { add(engine) }
.build()
val request = ImageRequest.Builder(context)
.data(url)
.crossfade(true)
.build()

val result = imageLoader.execute(request)
assertIs<SuccessResult>(result)
assertSame(Transition.Factory.NONE, result.request.transitionFactory)
}
}

0 comments on commit 17e1259

Please sign in to comment.