From 39daef823edb1a021996352cedcf7cae37bfc444 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 26 Jun 2026 09:56:37 -0400 Subject: [PATCH] fix(cache): replace custom ETag strategy with OkHttp native cache Drop ETagCacheStrategy, ETagOfflineFallbackInterceptor, and per-request etagRevalidation() in favor of OkHttp built-in HTTP cache. This gives us max-age freshness, conditional If-None-Match revalidation, and offline stale-serve out of the box. Coil disk cache is disabled so OkHttp is the single disk layer, fixing the icons-flashing-on-launch bug caused by Coil DefaultCacheStrategy bypassing freshness checks. Signed-off-by: Brandon McAnsh --- .../kotlin/com/flipcash/app/FlipcashApp.kt | 24 +- .../app/core/cache/ETagRevalidation.kt | 151 ------------- .../app/core/ui/TokenImageWithName.kt | 2 - .../app/core/cache/ETagCacheStrategyTest.kt | 205 ------------------ 4 files changed, 7 insertions(+), 375 deletions(-) delete mode 100644 apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/cache/ETagRevalidation.kt delete mode 100644 apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/cache/ETagCacheStrategyTest.kt diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/FlipcashApp.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/FlipcashApp.kt index 75ad934cf..16524885c 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/FlipcashApp.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/FlipcashApp.kt @@ -7,15 +7,12 @@ import androidx.work.Configuration import coil3.ImageLoader import coil3.PlatformContext import coil3.SingletonImageLoader -import coil3.annotation.ExperimentalCoilApi -import coil3.disk.DiskCache -import coil3.disk.directory import coil3.network.okhttp.OkHttpNetworkFetcherFactory import coil3.request.CachePolicy import coil3.request.crossfade import com.flipcash.app.auth.AuthManager -import com.flipcash.app.core.cache.ETagCacheStrategy -import com.flipcash.app.core.cache.ETagOfflineFallbackInterceptor +import okhttp3.Cache +import okhttp3.OkHttpClient import com.flipcash.app.currency.PreferredCurrencyController import com.getcode.opencode.repositories.EventRepository import com.getcode.utils.trace @@ -53,23 +50,16 @@ class FlipcashApp : Application(), Configuration.Provider, SingletonImageLoader. trace("app onCreate end") } - @OptIn(ExperimentalCoilApi::class) override fun newImageLoader(context: PlatformContext): ImageLoader { + val okHttpClient = OkHttpClient.Builder() + .cache(Cache(context.cacheDir.resolve("http_image_cache"), 50L * 1024 * 1024)) + .build() return ImageLoader.Builder(context) .crossfade(true) .memoryCachePolicy(CachePolicy.ENABLED) - .diskCachePolicy(CachePolicy.ENABLED) - .diskCache { - DiskCache.Builder() - .directory(context.cacheDir.resolve("image_cache")) - .maxSizePercent(0.2) - .build() - } + .diskCachePolicy(CachePolicy.DISABLED) .components { - add(ETagOfflineFallbackInterceptor()) - add(OkHttpNetworkFetcherFactory( - cacheStrategy = { ETagCacheStrategy() }, - )) + add(OkHttpNetworkFetcherFactory(callFactory = { okHttpClient })) } .build() } diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/cache/ETagRevalidation.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/cache/ETagRevalidation.kt deleted file mode 100644 index a189d5d4d..000000000 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/cache/ETagRevalidation.kt +++ /dev/null @@ -1,151 +0,0 @@ -package com.flipcash.app.core.cache - -import coil3.Extras -import coil3.annotation.ExperimentalCoilApi -import coil3.getExtra -import coil3.intercept.Interceptor -import coil3.network.CacheStrategy -import coil3.network.NetworkRequest -import coil3.network.NetworkResponse -import coil3.request.ErrorResult -import coil3.request.ImageRequest -import coil3.request.ImageResult -import coil3.request.Options -import java.io.IOException - -/** - * [Extras.Key] that enables ETag-based cache revalidation for an individual image request. - * When `true`, [ETagCacheStrategy] will attach conditional headers (`If-None-Match` / - * `If-Modified-Since`) so Coil revalidates against the server instead of blindly returning - * the disk-cached image. - */ -internal val ETagRevalidationKey = Extras.Key(default = false) - -/** - * Opts this image request into ETag-based cache revalidation. - * - * When set, the [ETagCacheStrategy] reads `ETag` and `Last-Modified` values stored in the - * disk-cache metadata and sends conditional request headers so the server can respond with - * `304 Not Modified` when the cached image is still fresh. - * - * Pair with [ETagOfflineFallbackInterceptor] to gracefully serve stale cache when the - * network is unavailable. - */ -fun ImageRequest.Builder.etagRevalidation() = apply { - extras[ETagRevalidationKey] = true -} - -/** - * A [CacheStrategy] that respects `Cache-Control: max-age` and performs conditional - * revalidation using `ETag` / `Last-Modified` headers stored in Coil's disk-cache metadata. - * - * **Read behaviour:** - * - If [ETagRevalidationKey] is **not** set on the request, the cached response is returned - * immediately (same as [CacheStrategy.DEFAULT]). - * - If the key **is** set but the cached entry is still fresh (within `max-age`), the cached - * response is returned without a network trip. - * - If the entry is stale, cached `etag` and `last-modified` headers are copied into - * `If-None-Match` / `If-Modified-Since` on the outgoing [NetworkRequest]. The server can - * then respond with `304` to avoid re-downloading the image body. - * - * **Write behaviour:** delegates entirely to [CacheStrategy.DEFAULT], which merges headers on - * `304` responses and writes new bodies on `2xx` responses. - */ -@OptIn(ExperimentalCoilApi::class) -class ETagCacheStrategy : CacheStrategy { - - override suspend fun read( - cacheResponse: NetworkResponse, - networkRequest: NetworkRequest, - options: Options, - ): CacheStrategy.ReadResult { - if (!options.getExtra(ETagRevalidationKey)) { - return CacheStrategy.ReadResult(cacheResponse) - } - - if (!isStale(cacheResponse)) { - return CacheStrategy.ReadResult(cacheResponse) - } - - val cachedHeaders = cacheResponse.headers - val etag = cachedHeaders["etag"] - val lastModified = cachedHeaders["last-modified"] - - if (etag == null && lastModified == null) { - return CacheStrategy.ReadResult(networkRequest) - } - - val requestHeaders = networkRequest.headers.newBuilder() - if (etag != null) { - requestHeaders["if-none-match"] = etag - } - if (lastModified != null) { - requestHeaders["if-modified-since"] = lastModified - } - - return CacheStrategy.ReadResult( - networkRequest.copy(headers = requestHeaders.build()) - ) - } - - override suspend fun write( - cacheResponse: NetworkResponse?, - networkRequest: NetworkRequest, - networkResponse: NetworkResponse, - options: Options, - ): CacheStrategy.WriteResult { - return CacheStrategy.DEFAULT.write(cacheResponse, networkRequest, networkResponse, options) - } - - companion object { - private val MAX_AGE_REGEX = Regex("""max-age\s*=\s*(\d+)""") - - /** - * Returns `true` if the cached response has exceeded its `max-age`, or if - * `responseMillis` is missing (pre-existing cache entries). - */ - internal fun isStale(cacheResponse: NetworkResponse): Boolean { - val responseMillis = cacheResponse.responseMillis - if (responseMillis <= 0L) return true - - val cacheControl = cacheResponse.headers["cache-control"] ?: return true - val maxAgeSeconds = MAX_AGE_REGEX.find(cacheControl) - ?.groupValues?.get(1)?.toLongOrNull() - ?: return true - - val ageMillis = System.currentTimeMillis() - responseMillis - return ageMillis > maxAgeSeconds * 1000 - } - } -} - -/** - * A Coil [Interceptor] that provides offline fallback for requests using ETag revalidation. - * - * When [ETagRevalidationKey] is enabled, [ETagCacheStrategy] forces a network round-trip for - * conditional validation. If the device is offline (or the request otherwise fails with an - * [IOException]), this interceptor retries the request with revalidation **disabled**, allowing - * the default cache strategy to serve the stale disk-cached image instead of surfacing an error. - * - * Requests that do not opt into ETag revalidation are passed through unmodified. - */ -@OptIn(ExperimentalCoilApi::class) -class ETagOfflineFallbackInterceptor : Interceptor { - - override suspend fun intercept(chain: Interceptor.Chain): ImageResult { - val request = chain.request - if (!request.getExtra(ETagRevalidationKey)) { - return chain.proceed() - } - - val result = chain.proceed() - if (result is ErrorResult && result.throwable is IOException) { - val fallbackRequest = request.newBuilder() - .apply { extras[ETagRevalidationKey] = false } - .build() - return chain.withRequest(fallbackRequest).proceed() - } - - return result - } -} diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/ui/TokenImageWithName.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/ui/TokenImageWithName.kt index de74f9cc4..9425ab32c 100644 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/ui/TokenImageWithName.kt +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/ui/TokenImageWithName.kt @@ -20,7 +20,6 @@ import coil3.compose.LocalPlatformContext import coil3.request.ImageRequest import coil3.request.crossfade import coil3.request.error -import com.flipcash.app.core.cache.etagRevalidation import com.getcode.opencode.model.financial.Token import com.getcode.theme.CodeTheme import com.getcode.ui.components.R @@ -108,7 +107,6 @@ fun TokenIcon( .data(image) .crossfade(false) .error(R.drawable.ic_placeholder_user) - .etagRevalidation() .build(), contentDescription = null, contentScale = ContentScale.Crop, diff --git a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/cache/ETagCacheStrategyTest.kt b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/cache/ETagCacheStrategyTest.kt deleted file mode 100644 index e3fcfb565..000000000 --- a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/cache/ETagCacheStrategyTest.kt +++ /dev/null @@ -1,205 +0,0 @@ -package com.flipcash.app.core.cache - -import coil3.Extras -import coil3.annotation.ExperimentalCoilApi -import coil3.network.NetworkHeaders -import coil3.network.NetworkRequest -import coil3.network.NetworkResponse -import coil3.request.Options -import kotlinx.coroutines.test.runTest -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner -import org.robolectric.RuntimeEnvironment -import org.robolectric.annotation.Config -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertNotNull -import kotlin.test.assertNull -import kotlin.test.assertTrue - -@OptIn(ExperimentalCoilApi::class) -@RunWith(RobolectricTestRunner::class) -@Config(manifest = Config.NONE) -class ETagCacheStrategyTest { - - private val strategy = ETagCacheStrategy() - - private fun options(revalidate: Boolean): Options { - val extras = Extras.Builder() - .apply { if (revalidate) set(ETagRevalidationKey, true) } - .build() - return Options(context = RuntimeEnvironment.getApplication(), extras = extras) - } - - private fun freshCacheResponse( - maxAgeSeconds: Long = 3600, - etag: String? = null, - lastModified: String? = null, - ): NetworkResponse { - val headers = NetworkHeaders.Builder() - .set("cache-control", "public, max-age=$maxAgeSeconds") - if (etag != null) headers.set("etag", etag) - if (lastModified != null) headers.set("last-modified", lastModified) - return NetworkResponse( - code = 200, - responseMillis = System.currentTimeMillis() - 60_000, // 1 minute ago - headers = headers.build(), - ) - } - - private fun staleCacheResponse( - maxAgeSeconds: Long = 3600, - etag: String? = null, - lastModified: String? = null, - ): NetworkResponse { - val headers = NetworkHeaders.Builder() - .set("cache-control", "public, max-age=$maxAgeSeconds") - if (etag != null) headers.set("etag", etag) - if (lastModified != null) headers.set("last-modified", lastModified) - return NetworkResponse( - code = 200, - responseMillis = System.currentTimeMillis() - (maxAgeSeconds * 1000 + 60_000), - headers = headers.build(), - ) - } - - private val baseNetworkRequest = NetworkRequest(url = "https://example.com/icon.png") - - // region key disabled - - @Test - fun `without key returns cache response`() = runTest { - val result = strategy.read(staleCacheResponse(), baseNetworkRequest, options(false)) - assertNotNull(result.response) - assertNull(result.request) - } - - // endregion - - // region freshness - - @Test - fun `with key and fresh cache returns cache response`() = runTest { - val cached = freshCacheResponse(etag = "\"abc123\"") - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.response) - assertNull(result.request) - } - - @Test - fun `with key and stale cache revalidates`() = runTest { - val cached = staleCacheResponse(etag = "\"abc123\"") - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - assertNull(result.response) - } - - @Test - fun `with key and no cache-control header treats as stale`() = runTest { - val cached = NetworkResponse( - code = 200, - responseMillis = System.currentTimeMillis(), - headers = NetworkHeaders.Builder() - .set("etag", "\"abc123\"") - .build(), - ) - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - } - - @Test - fun `with key and zero responseMillis treats as stale`() = runTest { - val cached = NetworkResponse( - code = 200, - responseMillis = 0L, - headers = NetworkHeaders.Builder() - .set("cache-control", "public, max-age=3600") - .set("etag", "\"abc123\"") - .build(), - ) - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - } - - // endregion - - // region conditional headers - - @Test - fun `stale cache with etag adds If-None-Match`() = runTest { - val cached = staleCacheResponse(etag = "\"abc123\"") - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - assertEquals("\"abc123\"", result.request!!.headers["if-none-match"]) - } - - @Test - fun `stale cache with last-modified adds If-Modified-Since`() = runTest { - val cached = staleCacheResponse(lastModified = "Wed, 21 Oct 2015 07:28:00 GMT") - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - assertEquals("Wed, 21 Oct 2015 07:28:00 GMT", result.request!!.headers["if-modified-since"]) - } - - @Test - fun `stale cache with both headers adds both conditional headers`() = runTest { - val cached = staleCacheResponse( - etag = "\"abc123\"", - lastModified = "Wed, 21 Oct 2015 07:28:00 GMT", - ) - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - assertEquals("\"abc123\"", result.request!!.headers["if-none-match"]) - assertEquals("Wed, 21 Oct 2015 07:28:00 GMT", result.request!!.headers["if-modified-since"]) - } - - @Test - fun `stale cache with no etag or last-modified sends plain request`() = runTest { - val cached = staleCacheResponse() - val result = strategy.read(cached, baseNetworkRequest, options(true)) - assertNotNull(result.request) - assertNull(result.request!!.headers["if-none-match"]) - assertNull(result.request!!.headers["if-modified-since"]) - } - - // endregion - - // region isStale - - @Test - fun `isStale returns false when within max-age`() { - val response = freshCacheResponse(maxAgeSeconds = 3600) - assertFalse(ETagCacheStrategy.isStale(response)) - } - - @Test - fun `isStale returns true when past max-age`() { - val response = staleCacheResponse(maxAgeSeconds = 3600) - assertTrue(ETagCacheStrategy.isStale(response)) - } - - @Test - fun `isStale returns true when no cache-control`() { - val response = NetworkResponse( - code = 200, - responseMillis = System.currentTimeMillis(), - headers = NetworkHeaders.EMPTY, - ) - assertTrue(ETagCacheStrategy.isStale(response)) - } - - @Test - fun `isStale returns true when responseMillis is zero`() { - val response = NetworkResponse( - code = 200, - responseMillis = 0L, - headers = NetworkHeaders.Builder() - .set("cache-control", "public, max-age=3600") - .build(), - ) - assertTrue(ETagCacheStrategy.isStale(response)) - } - - // endregion -}