Skip to content

Commit

Permalink
Avoid a crash if a content uri no longer exists on the disk
Browse files Browse the repository at this point in the history
Possibly addresses #99
  • Loading branch information
saket committed Sep 13, 2024
1 parent 5699a6c commit dd794df
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 7 deletions.
3 changes: 2 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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" }
Expand Down Expand Up @@ -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" }
Expand Down
1 change: 1 addition & 0 deletions zoomable-image/coil/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ dependencies {
androidTestImplementation(libs.coil.svg)
androidTestImplementation(libs.coil.test)
androidTestImplementation(libs.okio.fakeFileSystem)
androidTestImplementation(libs.modernstorage)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
}
}
}
Expand All @@ -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
}
}
}

0 comments on commit dd794df

Please sign in to comment.