All cacheKey override on Request. about okhttp HOT 19 OPEN

As a kludgy workaround, you can use application and network interceptors to modify what gets evaluated for caching, and what gets sent on the wire.

But we should consider a proper cache key API. Or learn from browsers on transient params?

  data class OriginalUrl(val url: HttpUrl)

  val client = OkHttpClient.Builder().eventListener(object : EventListener() {
      override fun cacheMiss(call: Call) {
        println("cacheMiss ${call.request().url}")
      }

      override fun cacheHit(call: Call, response: Response) {
        println("cacheHit ${call.request().url}")
      }

      override fun cacheConditionalHit(call: Call, cachedResponse: Response) {
        println("cacheConditionalHit ${call.request().url}")
      }
    }).addInterceptor {
      val request = it.request()
      val url = request.url
      // only strip the key param for caching, not other params
      val newRequest = if (url.queryParameter("key") != null) {
        request.newBuilder().tag<OriginalUrl>(OriginalUrl(url)).url(url.newBuilder().removeAllQueryParameters("key").build()).build()
      } else {
        request
      }
      it.proceed(newRequest).newBuilder().request(request).build()
    }.addNetworkInterceptor {
      val request = it.request()
      val originalUrl = request.tag<OriginalUrl>()?.url
      // if we modified the url for caching, send the real one
      if (originalUrl != null) {
        val networkRequest = request.newBuilder().url(originalUrl).build()
        it.proceed(networkRequest).newBuilder().request(request).build()
      } else {
        it.proceed(request)
      }
    }.cache(Cache("/cache".toPath(), Long.MAX_VALUE, FakeFileSystem())).build()

  // cache miss - first request
  client.newCall(Request("https://httpbin.org/cache/3600?key=123".toHttpUrl())).execute().use {
    println(it.body.string())
  }
  // cache hit - cached without key
  client.newCall(Request("https://httpbin.org/cache/3600?key=345".toHttpUrl())).execute().use {
    println(it.body.string())
  }

  // cache miss - another=123
  client.newCall(Request("https://httpbin.org/cache/3600?another=123".toHttpUrl())).execute().use {
    println(it.body.string())
  }
  // cache miss - another=345
  client.newCall(Request("https://httpbin.org/cache/3600?another=345".toHttpUrl())).execute().use {
    println(it.body.string())
  }

from okhttp.

I’m glad this issue was reported before we proceeded on #8113.

I’m tempted to do a cacheKey: ByteString parameter on the Request itself: if specified, we hash that instead of the request URL. We could even cache POST requests that have a cache key.

from okhttp.

Can you link to how browsers remove the transient params?

from okhttp.

Can you link to how browsers remove the transient params?

See ignoreSearch in Cache.match():
https://developer.mozilla.org/en-US/docs/Web/API/Cache/match#ignoresearch

from okhttp.

As a kludgy workaround, you can use application and network interceptors to modify what gets evaluated for caching, and what gets sent on the wire.

Thank you for the suggestion. I'll see if I can make it work.

from okhttp.

That Cache seems like a specific web workers thing, slightly detached from fetch.

https://developer.mozilla.org/en-US/docs/Web/API/Cache

So not sure we want to make our Cache the same thing. And it doesn't seem like it applies to the fetch API calls.

But still worth working out how to support

from okhttp.

That Cache seems like a specific web workers thing, slightly detached from fetch.

https://developer.mozilla.org/en-US/docs/Web/API/Cache

So not sure we want to make our Cache the same thing. And it doesn't seem like it applies to the fetch API calls.

But still worth working out how to support

Here is an example how ignoreSearch is used to check cache while executing HTTP GET (an event is sent to the service worker, service worker looks up cached response first):

https://github.com/tinode/webapp/blob/5faedc35f3b27bff11f50fc1a388f5bef1ebd2fc/service-worker.js#L154

self.addEventListener('fetch', event => {
  if (event.request.method != 'GET') {
    return;
  }

  event.respondWith((async _ => {
    //  Try to find the response in the cache.
    const cache = await caches.open(PACKAGE_VERSION);

    const reqUrl = new URL(event.request.url);
    // Using ignoreSearch=true to read cached images and docs despite different auth signatures.
    const cachedResponse = await cache.match(event.request, {ignoreSearch: (self.location.origin == reqUrl.origin)});
    if (cachedResponse) {
      return cachedResponse;
    }
    // Not found in cache.
    const response = await fetch(event.request);
    if (!response || response.status != 200 || response.type != 'basic') {
      return response;
    }
    if (reqUrl.protocol == 'http:' || reqUrl.protocol == 'https:') {
      await cache.put(event.request, response.clone());
    }
    return response;
  })());
});

from okhttp.

Yep, understood.

It's not a HTTP cache (honouring cache headers)

It's a specific web worker cache for Request/Response.

Might be interesting to allow for a cache only request without going through with a call.

from okhttp.

I'm going to close, I don't think we can copy the WebWorker Cache, and there is an ugly workaround.

from okhttp.

I'm going to close, I don't think we can copy the WebWorker Cache, and there is an ugly workaround.

Is there a plan to address it in any ("right") way or no plan at all? I need to make the decision to keep Picasso (which relies on okhttp cache) or switch to Glide. If there is no plan then I have to byte the bullet and switch.

Thanks.

from okhttp.

The fix in okhttp could be trivially simple. Make class Cache open and make key method not static.

from okhttp.

We have a similar requirement for a cache key for DoH DNS caching. #8113

So maybe as part of that.

This might be some optional tag for a cache key if it's not just POST.

from okhttp.

We have a similar requirement for a cache key for DoH DNS caching. #8113

So maybe as part of that.

@swankjesse any thoughts?

from okhttp.

Yes, looks similar. The simplest solution would be to allow users to provide a custom implementation of the Cache.key().

from okhttp.

Similar requests #1605

from okhttp.

I'll reopen for further discussion

from okhttp.

BTW, #8113 would likely require passing the entire Request to Cache.key which I guess should not be a problem.

from okhttp.

BTW, #8113 would likely require passing the entire Request to Cache.key which I guess should not be a problem.

It's unlikely we would open up Cache for subclassing, we are very careful about new public API surface. So more likely some way to specify the cache key for a request.

from okhttp.

Yep, let's do it.

Does the override on otherwise non cacheable requests affect just the initial cache search, or also update the cache if a miss? or stays uncacheable?

from okhttp.