From dd794df0fb8a007d8e92abab9707bb54071e592b Mon Sep 17 00:00:00 2001 From: Saket Narayan Date: Fri, 13 Sep 2024 02:01:49 -0400 Subject: [PATCH] Avoid a crash if a content uri no longer exists on the disk Possibly addresses https://github.com/saket/telephoto/issues/99 --- gradle/libs.versions.toml | 3 +- zoomable-image/coil/build.gradle | 1 + .../zoomable/coil/CoilImageSourceTest.kt | 52 ++++++++++++++++++- ...mage_cached_in_memory_no_longer_exists.png | 3 ++ .../zoomable/coil/CoilImageSource.kt | 11 ++-- ...bSamplingEligibility.kt => imageChecks.kt} | 21 +++++++- 6 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 zoomable-image/coil/src/androidTest/screenshots/show_error_drawable_if_a_local_image_cached_in_memory_no_longer_exists.png rename zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/{subSamplingEligibility.kt => imageChecks.kt} (72%) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c1097a7f..e4b9dd67 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -47,6 +47,7 @@ benchmark-macro-junit4 = "1.2.4" metalava = "0.3.5" # https://github.com/tylerbwong/metalava-gradle/releases poko = "0.15.2" # https://github.com/drewhamilton/Poko/blob/main/CHANGELOG.md composeLintChecks = "1.2.0" # https://slackhq.github.io/compose-lints/changelog/ +modernstorage = "1.0.0-alpha08" # https://github.com/saket/modernstorage/releases [plugins] android-application = { id = "com.android.application", version.ref = "agp" } @@ -63,7 +64,6 @@ metalava = { id = "me.tylerbwong.gradle.metalava", version.ref = "metalava" } poko = { id = "dev.drewhamilton.poko", version.ref = "poko" } [libraries] -# Build logic dependencies plugin-agp = { module = "com.android.tools.build:gradle", version.ref = "agp" } plugin-kotlin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" } plugin-jetbrains-compose = { module = "org.jetbrains.compose:compose-gradle-plugin", version.ref = "compose-multiplatform" } @@ -128,6 +128,7 @@ testParamInjector = { module = "com.google.testparameterinjector:test-parameter- dropshots = { module = "com.dropbox.dropshots:dropshots", version.ref = "dropshots" } dropboxDiffer = { module = "com.dropbox.differ:differ", version.ref = "dropboxDiffer" } assertk = { module = "com.willowtreeapps.assertk:assertk", version.ref = "assertk" } +modernstorage = { module = "me.saket.modernstorage:modernstorage-storage", version.ref = "modernstorage" } circuit-runtime = { module = "com.slack.circuit:circuit-foundation", version.ref = "circuit" } circuit-backstack = { module = "com.slack.circuit:circuit-backstack", version.ref = "circuit" } diff --git a/zoomable-image/coil/build.gradle b/zoomable-image/coil/build.gradle index b3326c35..bd425ac8 100644 --- a/zoomable-image/coil/build.gradle +++ b/zoomable-image/coil/build.gradle @@ -25,4 +25,5 @@ dependencies { androidTestImplementation(libs.coil.svg) androidTestImplementation(libs.coil.test) androidTestImplementation(libs.okio.fakeFileSystem) + androidTestImplementation(libs.modernstorage) } diff --git a/zoomable-image/coil/src/androidTest/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSourceTest.kt b/zoomable-image/coil/src/androidTest/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSourceTest.kt index b931b116..811d2363 100644 --- a/zoomable-image/coil/src/androidTest/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSourceTest.kt +++ b/zoomable-image/coil/src/androidTest/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSourceTest.kt @@ -6,6 +6,8 @@ import android.content.Context import android.graphics.Bitmap import android.graphics.drawable.ColorDrawable import android.net.Uri +import android.os.Environment +import android.provider.MediaStore import androidx.activity.ComponentActivity import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.size @@ -46,6 +48,8 @@ import coil.request.SuccessResult import coil.size.Dimension import coil.test.FakeImageLoaderEngine import com.dropbox.dropshots.Dropshots +import com.google.modernstorage.storage.AndroidFileSystem +import com.google.modernstorage.storage.toOkioPath import com.google.testing.junit.testparameterinjector.TestParameter import com.google.testing.junit.testparameterinjector.TestParameterInjector import kotlinx.coroutines.Dispatchers @@ -383,8 +387,36 @@ class CoilImageSourceTest { } } - // todo - @Test fun show_error_drawable_if_request_fails() { + // Regression test for https://github.com/saket/telephoto/issues/99. + @Test fun show_error_drawable_if_a_local_image_cached_in_memory_no_longer_exists() = runTest { + val imageInExternalStorage: Uri = context.copyImageToExternalStorage( + context.createFileFromAsset("full_image.png") + ) + + lateinit var imageState: ZoomableImageState + val stateRestorer = StateRestorationTester(rule) + stateRestorer.setContent { + ZoomableAsyncImage( + state = rememberZoomableImageState().also { imageState = it }, + modifier = Modifier.fillMaxSize(), + model = ImageRequest.Builder(LocalContext.current) + .data(imageInExternalStorage) + .error(R.drawable.error_image) + .allowHardware(false) // Unsupported by Screenshot.capture() + .build(), + contentDescription = null + ) + } + + rule.waitUntil { imageState.isImageDisplayed } + AndroidFileSystem(context).delete(imageInExternalStorage.toOkioPath()) + + stateRestorer.emulateSavedInstanceStateRestore() + + rule.waitUntil { imageState.isImageDisplayed } + rule.runOnIdle { + dropshots.assertSnapshot(rule.activity) + } } @Test fun gifs_should_not_be_sub_sampled( @@ -609,3 +641,19 @@ private fun Context.createFileFromAsset(assetName: String): Path { } } } + +private suspend fun Context.copyImageToExternalStorage(imageFile: Path): Uri { + val fs = AndroidFileSystem(this) + val uri = fs.createMediaStoreUri( + filename = imageFile.name, + collection = MediaStore.Downloads.getContentUri(MediaStore.VOLUME_EXTERNAL_PRIMARY), + directory = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).absolutePath, + )!! + fs.write(uri.toOkioPath()) { + fs.read(imageFile) { + writeAll(this) + } + } + fs.scanUri(uri, mimeType = "image/png") + return uri +} diff --git a/zoomable-image/coil/src/androidTest/screenshots/show_error_drawable_if_a_local_image_cached_in_memory_no_longer_exists.png b/zoomable-image/coil/src/androidTest/screenshots/show_error_drawable_if_a_local_image_cached_in_memory_no_longer_exists.png new file mode 100644 index 00000000..833370e6 --- /dev/null +++ b/zoomable-image/coil/src/androidTest/screenshots/show_error_drawable_if_a_local_image_cached_in_memory_no_longer_exists.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0837eaa7240c31a7ee3b59e2181798ebd09a27c6e0143db3e469dcfe120cb6c7 +size 38555 diff --git a/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSource.kt b/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSource.kt index b3031430..f9f5cdf0 100644 --- a/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSource.kt +++ b/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/CoilImageSource.kt @@ -189,13 +189,18 @@ internal class Resolver( } SubSamplingImageSource.file(snapshot.data, preview, onClose = snapshot::close) } + result.dataSource.let { it == DataSource.DISK || it == DataSource.MEMORY_CACHE } -> { // Possible reasons for reaching this code path: // - Locally stored images such as assets, resource, etc. // - Remote image that wasn't saved to disk because of a "no-store" HTTP header. - result.request.mapRequestDataToUriOrNull()?.let { uri -> - SubSamplingImageSource.contentUriOrNull(uri, preview) - } + result.request.mapRequestDataToUriOrNull() + ?.let { uri -> SubSamplingImageSource.contentUriOrNull(uri, preview) } + ?.also { + if (result.dataSource == DataSource.MEMORY_CACHE && !it.exists()) { + return ImageDeletedOnlyFromDiskCache + } + } } else -> { diff --git a/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/subSamplingEligibility.kt b/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/imageChecks.kt similarity index 72% rename from zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/subSamplingEligibility.kt rename to zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/imageChecks.kt index dc290ab7..c9058377 100644 --- a/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/subSamplingEligibility.kt +++ b/zoomable-image/coil/src/main/kotlin/me/saket/telephoto/zoomable/coil/imageChecks.kt @@ -13,6 +13,7 @@ import me.saket.telephoto.subsamplingimage.RawImageSource import me.saket.telephoto.subsamplingimage.ResourceImageSource import me.saket.telephoto.subsamplingimage.SubSamplingImageSource import me.saket.telephoto.subsamplingimage.UriImageSource +import okio.Buffer import okio.FileSystem import okio.Source import okio.buffer @@ -26,7 +27,7 @@ internal suspend fun SubSamplingImageSource.canBeSubSampled(): Boolean { is AssetImageSource -> canBeSubSampled() is UriImageSource -> canBeSubSampled() is FileImageSource -> canBeSubSampled(FileSystem.SYSTEM.source(path)) - is RawImageSource -> canBeSubSampled(source.invoke()) + is RawImageSource -> canBeSubSampled(peek()) } } } @@ -52,3 +53,21 @@ private fun canBeSubSampled(source: Source): Boolean { !DecodeUtils.isSvg(it) && !DecodeUtils.isGif(it) } } + +context(Resolver) +internal suspend fun SubSamplingImageSource.exists(): Boolean { + return withContext(Dispatchers.IO) { + try { + val bufferedSource = when (this@exists) { + is ResourceImageSource -> return@withContext true + is RawImageSource -> peek() + is AssetImageSource -> peek(request.context).source().buffer() + is UriImageSource -> peek(request.context).source().buffer() + is FileImageSource -> FileSystem.SYSTEM.source(path).buffer() + } + bufferedSource.read(Buffer(), byteCount = 1) != -1L + } catch (e: okio.FileNotFoundException) { + false + } + } +}