Call cacheDir lazily in ImageSource. (#1761)

* Call cacheDir lazily in ImageSource. * Change exception type.
coil-kt · May 14, 2023 · 779fd54 · 779fd54
1 parent 65ff90e
commit 779fd54
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/coil-base/src/main/java/coil/decode/ImageSource.kt b/coil-base/src/main/java/coil/decode/ImageSource.kt
@@ -65,7 +65,7 @@ fun ImageSource(
 fun ImageSource(
     source: BufferedSource,
     context: Context,
-): ImageSource = SourceImageSource(source, context.safeCacheDir, null)
+): ImageSource = SourceImageSource(source, { context.safeCacheDir }, null)
 
 /**
  * Create a new [ImageSource] backed by a [BufferedSource].
@@ -80,7 +80,7 @@ fun ImageSource(
     source: BufferedSource,
     context: Context,
     metadata: ImageSource.Metadata? = null,
-): ImageSource = SourceImageSource(source, context.safeCacheDir, metadata)
+): ImageSource = SourceImageSource(source, { context.safeCacheDir }, metadata)
 
 /**
  * Create a new [ImageSource] backed by a [BufferedSource].
@@ -92,7 +92,7 @@ fun ImageSource(
 fun ImageSource(
     source: BufferedSource,
     cacheDirectory: File,
-): ImageSource = SourceImageSource(source, cacheDirectory, null)
+): ImageSource = SourceImageSource(source, { cacheDirectory }, null)
 
 /**
  * Create a new [ImageSource] backed by a [BufferedSource].
@@ -107,7 +107,7 @@ fun ImageSource(
     source: BufferedSource,
     cacheDirectory: File,
     metadata: ImageSource.Metadata? = null,
-): ImageSource = SourceImageSource(source, cacheDirectory, metadata)
+): ImageSource = SourceImageSource(source, { cacheDirectory }, metadata)
 
 /**
  * Provides access to the image data to be decoded.
@@ -246,18 +246,15 @@ internal class FileImageSource(
 
 internal class SourceImageSource(
     source: BufferedSource,
-    private val cacheDirectory: File,
+    cacheDirectoryFactory: () -> File,
     override val metadata: Metadata?
 ) : ImageSource() {
 
     private var isClosed = false
     private var source: BufferedSource? = source
+    private var cacheDirectoryFactory: (() -> File)? = cacheDirectoryFactory
     private var file: Path? = null
 
-    init {
-        require(cacheDirectory.isDirectory) { "cacheDirectory must be a directory." }
-    }
-
     override val fileSystem get() = FileSystem.SYSTEM
 
     @Synchronized
@@ -275,13 +272,14 @@ internal class SourceImageSource(
         file?.let { return it }
 
         // Copy the source to a temp file.
-        // Replace JVM call with https://github.com/square/okio/issues/1090 once it's available.
-        val tempFile = File.createTempFile("tmp", null, cacheDirectory).toOkioPath()
+        val tempFile = createTempFile()
         fileSystem.write(tempFile) {
             writeAll(source!!)
         }
         source = null
-        return tempFile.also { file = it }
+        file = tempFile
+        cacheDirectoryFactory = null
+        return tempFile
     }
 
     @Synchronized
@@ -297,6 +295,14 @@ internal class SourceImageSource(
         file?.let(fileSystem::delete)
     }
 
+    private fun createTempFile(): Path {
+        val cacheDirectory = cacheDirectoryFactory!!.invoke()
+        check(cacheDirectory.isDirectory) { "cacheDirectory must be a directory." }
+
+        // Replace JVM call with https://github.com/square/okio/issues/1090 once it's available.
+        return File.createTempFile("tmp", null, cacheDirectory).toOkioPath()
+    }
+
     private fun assertNotClosed() {
         check(!isClosed) { "closed" }
     }

diff --git a/coil-test-paparazzi/src/test/java/coil/test/PaparazziTest.kt b/coil-test-paparazzi/src/test/java/coil/test/PaparazziTest.kt
@@ -9,7 +9,10 @@ import app.cash.paparazzi.DeviceConfig.Companion.PIXEL_6
 import app.cash.paparazzi.Paparazzi
 import coil.ImageLoader
 import coil.compose.AsyncImage
+import coil.decode.ImageSource
 import coil.request.ImageRequest
+import kotlin.test.assertTrue
+import okio.Buffer
 import org.junit.Rule
 import org.junit.Test
 
@@ -72,4 +75,11 @@ class PaparazziTest {
             )
         }
     }
+
+    /** Regression test: https://github.com/coil-kt/coil/issues/1754 */
+    @Test
+    fun createSourceImageSource() {
+        val source = ImageSource(Buffer(), paparazzi.context)
+        assertTrue(source.source().exhausted())
+    }
 }