Fix FakeImageEngine not updating the chain's request. (#1905)

* Fix FakeImageEngine not updating the chain's request. * Change implementation.
coil-kt · Oct 25, 2023 · 466a430 · 466a430
1 parent c2dd9b4
commit 466a430
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 6 deletions.
diff --git a/coil-base/api/coil-base.api b/coil-base/api/coil-base.api
@@ -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;
 }
 

diff --git a/coil-base/src/main/java/coil/intercept/Interceptor.kt b/coil-base/src/main/java/coil/intercept/Interceptor.kt
@@ -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
@@ -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.
          */

diff --git a/coil-base/src/main/java/coil/intercept/RealInterceptorChain.kt b/coil-base/src/main/java/coil/intercept/RealInterceptorChain.kt
@@ -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 {
+        if (index > 0) checkRequest(request, interceptors[index - 1])
+        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])

diff --git a/coil-base/src/test/java/coil/intercept/RealInterceptorChainTest.kt b/coil-base/src/test/java/coil/intercept/RealInterceptorChainTest.kt
@@ -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,

diff --git a/coil-test/src/main/java/coil/test/FakeImageLoaderEngine.kt b/coil-test/src/main/java/coil/test/FakeImageLoaderEngine.kt
@@ -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
@@ -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
     }
 

diff --git a/coil-test/src/test/java/coil/test/FakeImageLoaderEngineTest.kt b/coil-test/src/test/java/coil/test/FakeImageLoaderEngineTest.kt
@@ -6,9 +6,11 @@ import android.graphics.drawable.ColorDrawable
 import androidx.test.core.app.ApplicationProvider
 import coil.ImageLoader
 import coil.decode.DataSource
+import coil.request.ErrorResult
 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
@@ -124,4 +126,46 @@ 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)
+    }
+
+    @Test
+    fun `request transformer enforces invariants`() = runTest {
+        val url = "https://www.example.com/image.jpg"
+        val engine = FakeImageLoaderEngine.Builder()
+            .intercept(url, ColorDrawable(Color.RED))
+            .requestTransformer { request ->
+                request.newBuilder()
+                    .data(null)
+                    .build()
+            }
+            .build()
+        val imageLoader = ImageLoader.Builder(context)
+            .components { add(engine) }
+            .build()
+        val request = ImageRequest.Builder(context)
+            .data(url)
+            .build()
+
+        val result = imageLoader.execute(request)
+        assertIs<ErrorResult>(result)
+        assertIs<IllegalStateException>(result.throwable)
+    }
 }