Image Caching Improvement

[andrewdmontgomery]

The existing image caching doesn't have a way to detect changes to the remote image. If the image is fetched and cached, and then subsequently updated through some other means (e.g. the web portal), this caching mechanism won't detect that.

The source of truth is the Last-Modified header in the response from the API when we fetch an avatar. We can use that. We should be able to add an If-Modified-Since header to our requests and then process the response accordingly:

• If it has been modified, fetch and cache
• If it hasn't, use the cached response

I believe this would just mean updating the imageRequest(url:URL) -> URLRequest function to set the right header:

private extension URLRequest {
    static func imageRequest(url: URL, ifModified checkModifiedSince: Bool = true) -> URLRequest {
        var request = URLRequest(url: url)
        request.httpShouldHandleCookies = false
        request.addValue("image/*", forHTTPHeaderField: "Accept")
        if checkModifiedSince {
            let requestDate = Date().ISO8601Format() // Wrong format, but you get the idea
            request.addValue(requestDate, forHTTPHeaderField: "If-Modified-Since")
        }
        return request
    }
}

The response:

• 200 + Data: the resource has been modified, and the latest image is included
• 304, no Data: the resource has not been modified.

So we would:

• 200: use the received data to update our GravatarImageCache
• 304: use the existing GravatarImageCache

In fact, we'd want to:

1. Check if we have a cached image
   • If we do, set the If-Modified-Since header of the request
2. Check if gravatar is reachable
3. If it's reachable, send request
   • If we receive a 200 + Data, set cache
   • If we receive a 304, get cache (must only be possible if we have a cached image)

[pinarol]

You are suggesting this to avoid re-downloading the same image right?

Setting cache policy to reloadRevalidatingCacheData seems to be doing that already. https://stackoverflow.com/questions/72995149/urlsession-caching-not-working-as-documented-what-am-i-doing-wrong

So let's investigate that first.

This also made me realize, we should use "reloadIgnoringLocalCacheData" when the caller party passes "forceRefresh = true".

[andrewdmontgomery]

You are suggesting this to avoid re-downloading the same image right?

Yep. And yeah, that sounds like a good direction to try first.

This also made me realize, we should use "reloadIgnoringLocalCacheData" when the caller party passes "ignoreCache = true".

Good call

[pinarol]

In the meantime I was checking what the default cache policy does:

https://developer.apple.com/documentation/foundation/nsurlrequest/cachepolicy/useprotocolcachepolicy#discussion

It first checks the cached response to see if it is stale or requires revalidation. If yes, it makes a HEAD request to check if there's a new image on the server, only after that it fetches the image. I think it also depends on how much of these things our backend supports but the default cache policy looks also quite reasonable to me.

[pinarol]

Doing a backlog cleanup and closing this for now. We can reopen it in the future if needed.