Hi, folks! I was doing some investigation for a po...

# apollo-kotlin

[Eduard Boloș]

Hi, folks! I was doing some investigation for a possibly clever way to query data from the network if the cached data is older than X, with X defined on a per-query basis. That's how I stumbled upon the storeReceiveDate extension function. However, looking at the implementation, it seem like the receive date is not actually stored anywhere, isn't it? 🙃
Is there a not-extremely-hard way to achieve what I want? The only way I could think of so far is by having a custom RecordDatabase implementation that actually stores the dates too in a new column, and rewriting SqlNormalizedCache to make use of that column and filter the fields based on a value from the CacheHeaders argument, but that seems a significant amount of error-prone work.

[bod]

An expiration feature is being worked on in the incubating version of the cache, and you can see a sneak peek at how it works here. I think it should do what you need, but feedback is very welcome!

storeReceivedDate has no effect in the stable library, as you noticed.

The incubating cache is not ready for production though, and in the meantime we don’t have a way to do that in the stable lib.

Instead of rewriting the whole cache, if what you need is a per-query expiration date, maybe you can maintain a separate mapping query→date somewhere?

[Eduard Boloș]

Oh, yes, the query.maxStale(1.hours) is exactly what I would like to have ❤️ I will probably give it a try on Monday, and give feedback if needed 😄

The incubating cache is not ready for production though, and in the meantime we don’t have a way to do that in the stable lib.

I will commit the cardinal sin now, but I gotta ask: is there a timeline, even if it's very rough, when this might get promoted into the stable lib?

Instead of rewriting the whole cache, if what you need is a per-query expiration date, maybe you can maintain a separate mapping query→date somewhere?

Yeah, this would be another possible idea, although not perfect, as some queries overlap, so it might be that query A might've been executed at time X, but it's data to have been last refreshed at time Y due to query B that contains a superset of A's fields being executed meanwhile. But I will keep this option in mind as a fallback 👍

[bod]

Thanks a lot! Actually the expiration work is not in the latest release, but I'll publish one now.

Sorry no timeline at the moment 😅

And yes you're totally right about the overlaps - this is far from perfect but may be a good compromise as it's simple to reason about and probably not too hard to implement (well... famous last words! 😄).

[Eduard Boloș]

Haha! 😄
I am happy to use a snapshot version as well, as long as it's there. Thanks!

[bod]

thanks! 🙏 Yes the snapshots are good to check it out. 0.0.4-SNAPSHOT is the one.

[Eduard Boloș]

Cool, I'll get back to this soon. Have a great weekend!

[Eduard Boloș]

Hello! So I've been trying out the incubating cache lib today, and I have several questions and thoughts 😄

First of all, I see that the sql normalized cache uses a totally different schema than the non-incubating one, with a table called blobs. Will this be the final design of the schema? The reason why I am asking is because I think it might be crucial for existing apps that use Apollo for the existing cache to be reused (well, at least for us it would be), either by extending the existing records table, or offering some other migration path without losing the cache. Related, if the blobs will be kept, would the IDE plugin support it?

Then the second most important thing I have doubts about is how is ExpirationCacheResolver supposed to work more exactly. If I leave in the default MaxAgeProvider with an infinite time duration, and I set call.maxStale(1.minutes), then in resolveField the variable fieldMaxAge will be a humongous number, making val stale = fieldAge - fieldMaxAge to be a humongous negative number, thus the condition if (stale >= maxStale) will be false, as the negative number will never be greater than whatever maxStale we set on the call. Was my expectation for the maxStale on the call to take precedence over the global max age wrong? Or is there something fishy in the current implementation?

A small thing I noticed is that NormalizedCache is no longer an abstract class that contains a nextCache property, but it's an interface, so you now need to define a reference yourself, that you pass from a chain method in the factory that you also need to implement yourself. Is this change here to stay? We have a couple of custom NormalizedCache implementations, one for tracing purposes, and one that is now basically deprecated by doNotStore flag, so it was a bit annoying having to adjust their implementation, but not an end of the world.

Next is a papercut: ApolloCall cache headers are not additive, so by doing apolloCall.maxStale(period).cacheHeaders(myCacheHeaders) you overwrite the ApolloCacheHeaders.MAX_STALE header 🙃 I had to resort to an ugly hack to get access to the internal CacheHeadersContext so that I can add my own cache headers that I use in a custom interceptor next to the one added by maxStale. Maybe there is an API to do this already, but I couldn't find it.

Sadly, because of the current ExpirationCacheResolver I couldn't do extensive testing, but I could reimplement my own CacheResolver according to my own expectations, and finish testing that way. But on a first look, apart from the other thing I mentioned about the schema, this looks great 🙂

[Eduard Boloș]

Oh, another thing, to clarify: if I do client.query(MyQuery()).fetchPolicy(FetchPolicy.CacheOnly).maxStale(1.hours).execute(), like in the example in the docs, there wouldn't be any automatic fallback to the network, but if I'd use the CacheFirst fetch policy, then it would hit the network if the cache is stale, right?

[bod]

Hey thanks a lot for the feedback 🙏 This is great!

Will this be the final design of the schema?

This is probably not the final design. In fact we also want to explore other storages outside of SQLite 🙂. But maybe a first iteration will use the blobs or a version of it.
In terms of the migration path - I think it will be a best effort: we will want to encourage devs to use the new cache, and therefore we’ll want to make it easy for them. On the other end, I don’t want to make promises, this will probably be tricky, or even “to complex to be worth it”.
I guess my question on that front is: for your app, is it a blocker to lose the cached data? Is it “just” that the next time the user opens the app, loading will be slower, or would that actually break functionality?


About `maxStale`: this is supposed to give you a way to get data even if it is considered expired.
If you use the default maxAge, the data is never considered expired - so maxStale has no usefulness in that case.
I think what you probably want is an actual value for maxAge and maybe no maxStale?
I think maxStale can be useful for scenarios like:
• if the data in the cache is not expired, no need for network
• if it’s expired for at most a day it’s ok to show it to the user but fetch fresh data from the network
• if it’s expired than more than a day, don’t show it until we get fresh from network
I hope this is a bit clearer, and would that API be adapted to your use case?


About the changes in NormalizedCache. We made the change to simplify the code (and API) related to chaining the memory and SQLite caches. With the old API technically the SQLite could have a next cache too, but in practice this wasn’t useful (I think?) and was not correctly implemented IIRC. Removing the chaining from the contract and moving that responsibility to individual cache implementations makes a simpler API. I think we probably want to keep that change, but might reconsider if that’s too much of a pain!


About the cache headers: good call! We had the same issue with Http headers IIRC - I need to refresh my memory on what the API currently looks like, but we should probably have a similar API for those so it’s possible/easy to add headers.


About this one:

if I'd use the CacheFirst fetch policy, then it would hit the network if the cache is stale, right?

exactly - the expirations are handled as cache misses, so the fetch policies work as you’d expect.


Again thanks for the feedback and keep them coming 😄

[Eduard Boloș]

Hello!

I guess my question on that front is: for your app, is it a blocker to lose the cached data? Is it “just” that the next time the user opens the app, loading will be slower, or would that actually break functionality?

Yes, it is, sadly 😞 We rely on some parts of the data to be always there in the cache after the user logs in once, so if we'd lose that, the user won't be able to open the app anymore while offline (e.g. after updating the app to the version where this change would happen), which is quite bad for us, and for our users: we have many users in rural Africa, with poor Internet connectivity, and they rely on the offline functionality of the app in order to be able to access their money.

About `maxStale`: this is supposed to give you a way to get data even if it is considered expired. [...]

Ah, now I see, I understood this wrong, my bad. In our case, data never expires per se, we just we want it to be fresh-enough, let's say. Some queries are quite expensive to execute on the backend, so we want to avoid refreshing too often. So there's data that we refresh any time the user opens the screen or they pull to refresh, but some queries we basically want to refresh only if the data is older than X minutes. This is what I was trying to use this for 😄

So I guess one hacky way to achieve this would be to set maxAge = Duration.ZERO, and then by default set .maxStale(Duration.INFINITE) on all operations by default, with a specific duration where we actually want to force the refresh if the age is greater than our threshold.

Or we could also use our own custom header and CacheResolver that uses the received field. I already prototyped something like that (but still using the MAX_STALE header instead of a custom one 😄)

@OptIn(ApolloInternal::class)
    override fun resolveField(context: ResolverContext): Any? {
        val resolvedField = FieldPolicyCacheResolver.resolveField(context)
        val parent = context.parent
        if (parent is Record) {
            val field = context.field
            val maxStale = context.cacheHeaders.headerValue(ApolloCacheHeaders.MAX_STALE)?.toLongOrNull() ?: Long.MAX_VALUE
            val currentDate = currentTimeMillis() / 1000

            val fieldReceivedDate = parent.receivedDate(field.name)
            if (fieldReceivedDate != null) {
                val fieldAge = currentDate - fieldReceivedDate
                if (fieldAge > maxStale) {
                    throw CacheMissException(
                        context.parentKey,
                        context.fieldKeyGenerator.getFieldKey(
                            FieldKeyContext(
                                context.parentType,
                                context.field,
                                context.variables
                            )
                        ),
                        stale = true
                    )
                }
            }
        }
        return resolvedField
    }

About the changes in NormalizedCache. [...] I think we probably want to keep that change, but might reconsider if that’s too much of a pain!

Definitely not too much of a pain, it was just surprising, that's all 😄

About the cache headers: good call! We had the same issue with Http headers IIRC - I need to refresh my memory on what the API currently looks like, but we should probably have a similar API for those so it’s possible/easy to add headers.

It looks like for HTTP headers there's addHttpHeader, so probably addCacheHeader would be the equivalent 🙂

Thanks for your extended explanations!

[bod]

We rely on some parts of the data to be always there in the cache after the user logs in once, so if we'd lose that, the user won't be able to open the app anymore

In general I’d recommend to store these somewhere else (shared preferences? room db?), and consider the cache to be “only a cache” that can be invalidated and needs to be refreshed sometimes. Of course that’s just a general advice and will depend on your project.


About maxAge / maxStale I think your idea should work.
Would it also work if you use a dedicated maxAge for the expensive types/fields (+ a default maxAge of infinite), and use the CacheFirst policy (no need for maxStale)?
• If a field is present and recent enough (= not expired), use the cache, no network
• otherwise, network

[Eduard Boloș]

In general I’d recommend to store these somewhere else (shared preferences? room db?), and consider the cache to be “only a cache” that can be invalidated and needs to be refreshed sometimes. Of course that’s just a general advice and will depend on your project.

Yeah, unfortunately the app I am working on has been built by making extremely strong assumptions about Apollo and GraphQL, so now this has to be our unique source of truth 🙃

Would it also work if you use a dedicated maxAge for the expensive types/fields (+ a default maxAge of infinite), and use the CacheFirst policy (no need for maxStale)?

Yes, I believe so. If there would be a call.maxAge(Duration) function, it would be even better. But like I was saying above, this looks like something very easy to implement by end-clients as well, as long as storeReceiveDate actually works, and there's a way to read the field's received date in the cache resolver 😄

[bod]

all right so fun Record.receivedDate(field: String) needs to stay public 👍

[bod]

a call.maxAge(Duration) could be a good idea 👀