Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CacheKeyResolver fromFieldArguments does not work #821

Closed
nicusorflorin opened this issue Feb 14, 2018 · 13 comments
Closed

CacheKeyResolver fromFieldArguments does not work #821

nicusorflorin opened this issue Feb 14, 2018 · 13 comments

Comments

@nicusorflorin
Copy link

nicusorflorin commented Feb 14, 2018

I have a cachekeyresolver that looks like:

class MyCacheKeyResolver : CacheKeyResolver() {
    override fun fromFieldArguments(field: ResponseField, variables: Operation.Variables): CacheKey {
        return if (field.arguments().containsKey("searchPhrase")) {
            if ((variables as? SearchQuery.Variables) != null) {
                val typeNameAndIDKey = field.fieldName() + "." + variables.filters().value?.nodeTypes() +
                        "." + variables.searchPhrase()?.value
                Log.d("ApolloCache", "cache_key: " + typeNameAndIDKey)
                CacheKey.from(typeNameAndIDKey)
            } else {
                CacheKey.NO_KEY
            }
        } else {
            CacheKey.NO_KEY
        }
    }

    override fun fromFieldRecordSet(field: ResponseField, recordSet: MutableMap<String, Any>): CacheKey {
        return if (recordSet.containsKey("id")) {
            val typeNameAndIDKey = recordSet["__typename"].toString() + "." + recordSet["id"]
            Log.d("ApolloCache", "cache_key: " + typeNameAndIDKey)
            CacheKey.from(typeNameAndIDKey)
        } else {
            CacheKey.NO_KEY
        }
    }
}

from fieldRecordSet it works perfectly, but the arguments one does not, it returns the proper key, but the response is still fetched from network and not from the cache

@sav007
Copy link
Contributor

sav007 commented Feb 14, 2018

Are you sure that the key is returned by fromFieldArguments returns correct cache key? As well could you pls set logger (com.apollographql.apollo.ApolloClient.Builder#logger) maybe you will find smth interesting there?

@nicusorflorin
Copy link
Author

nicusorflorin commented Feb 15, 2018

What do you mean correct cache key? it returns the key that I compose there every time.
And the logger only says that the key was not found in the cache:

02-15 02:08:00.835 24204-24283/com.package.name I/System.out: Failed to read cache response Cache MISS: failed to find record in cache by reference
02-15 02:09:30.120 24204-24283/com.package.name I/System.out: Cache MISS for operation com.package.name.SearchQuery@7596ae0 

@sav007
Copy link
Contributor

sav007 commented Feb 15, 2018

My concern is that the key that generated internally doesn't match with your logic in fromFieldArguments. Could you please show the SearchQuery.graphql

Have you tried to leave fromFieldArguments to return CacheKey.NO_KEY in this case it will use default internal impl to resolve cache record.

@nicusorflorin
Copy link
Author

nicusorflorin commented Feb 15, 2018

I did try with CacheKey.NO_KEY, had no luck,

This is the query

query SearchQuery($filters: Filters, $count: Int, $curs: Cursor, $searchPhrase: String) {
  nodes(filters: $filters, first: $count, after: $curs, searchPhrase: $searchPhrase) {
    totalCount
    pageInfo {
      ...PageInfoFragment
    }
    edges {
      cursor
      node {
        id
          ...EpisodeListFragment
          ...MovieListFragment
          ...ShowListFragment
      }
    }
  }
}

I thought that the cachekeyresolver's purpose was for me to set my own logic for how the key is composed and what it contains. What kind of key does it generate internally?

@sav007
Copy link
Contributor

sav007 commented Feb 15, 2018

CacheKeyResolver#fromFieldRecordSet is used mostly when you have global id or like in your example, recordSet["__typename"].toString() + "." + recordSet["id"] that's absolutely correct.

CacheKeyResolver#fromFieldArguments is used to resolve root of the query, like for instance you fetch list of nodes and you have another query to fetch one node by id. Then you can provide different root for the query, so by default the root will be node(1234) but you want to have 1234.

In your case you shouldn't use CacheKeyResolver#fromFieldArguments and just return CacheKey.NO_KEY.

When this query is cached the key for the root will be quite complicated, like nodes(filters:SMTH, first:20,....). Here is the code: https://github.com/apollographql/apollo-android/blob/master/apollo-api/src/main/java/com/apollographql/apollo/api/ResponseField.java#L240

Could you elaborate on why CacheKey.NO_KEY doesn't work for you?

@nicusorflorin
Copy link
Author

nicusorflorin commented Feb 15, 2018

it seems it returns twice when there is something in cache and once when it isn't, when it is called twice it returns once from cache and once from network.

02-15 10:39:12.191 21471-21471/com.package.name D/ApolloCache: StartCall
02-15 10:39:12.195 21471-21555/com.package.name D/ApolloCache: Failed to read cache response Missing value: nodes
02-15 10:39:12.196 21471-21555/com.package.name D/ApolloCache: Cache MISS for operation com.package.name.SearchQuery@3bdca8b 
02-15 10:39:12.454 21471-21539/com.package.name D/ApolloCache: Response from cache: false
02-15 10:39:12.464 21471-21533/com.package.name D/ApolloCache: Cache HIT for operation com.package.name.SearchQuery@3bdca8b 
02-15 10:39:12.464 21471-21533/com.package.name D/ApolloCache: Response from cache: true
02-15 10:39:12.508 21471-21471/com.package.name D/ApolloCache: onNext
02-15 10:39:12.508 21471-21471/com.package.name D/ApolloCache: onNext

My ResponseFetcher is CACHE_FIRST, it shouldn't return from network when it finds cache
It doesn't seem to be consistent though, sometimes it returns only one, sometimes it returns twice

I ran some additional tests, to make sure of the findings, and added a button that only does the query, it returns consistently from the network, so no cache results, not sure why in the normal flow sometimes it returns that it finds in cache paired with a not found in cache, something weird happens behind the scenes

@sav007
Copy link
Contributor

sav007 commented Feb 15, 2018

Are you sure that the response was cached previously?
And arguments filters: $filters, first: $count, after: $curs, searchPhrase: $searchPhras were not changed for the query that you expect to be fetched from cache?

I think one take away from this conversation is that we need have the way to dump the cache, at least the mem cache. It will be easier to handle such issues

@nicusorflorin
Copy link
Author

nicusorflorin commented Feb 16, 2018

well I don't know how to check if it was cached, but if i make the same query twice in a row, the second time should be from cache right?
And yes nothing changes in the query, I thought the same and double and triple checked it, all the arguments are constructed the same with Input.fromNullable()
Also I agree that would be a useful feature to have. Till then i'm stuck on this issue

@sav007
Copy link
Contributor

sav007 commented Feb 16, 2018

Is there any way I can try your query? Is GraphQL endpoint public exposed?

@sav007
Copy link
Contributor

sav007 commented Feb 16, 2018

I've prepared PR #825 to be able dump the content of normalized cache.

@nicusorflorin
Copy link
Author

nicusorflorin commented Feb 16, 2018

No unfortunately it is not public, so I can't give you access. I will wait till a snapshot is up with that PR, and see what it turns out in it

@sav007
Copy link
Contributor

sav007 commented May 1, 2018

Please try on new version if you still see the same issue pls reopen this issue again.

@sav007 sav007 closed this as completed May 1, 2018
@shasharm
Copy link

shasharm commented May 1, 2020

This issue is still there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants