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

GraphQL Input types toString() as cache key instead of resolving input values #103

Closed
donaldarmstrong opened this issue Jan 22, 2019 · 9 comments · Fixed by #143
Closed
Assignees
Labels
bug Something isn't working

Comments

@donaldarmstrong
Copy link

donaldarmstrong commented Jan 22, 2019

Describe the bug
A query that uses an input object results in a cache key that instead resolves the id of the object as the key instead of decomposing the map. This looks to be the same issue as apollographql/apollo-kotlin#997 resolved.

ResponseNormalizer.willResolve produces a cache key like
getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@c17c127)

whereas the 1.0.1-SNAPSHOT of apollo-android the same method produces the following cache key
getSalesOrder({"input":{"id":"583b44c5-0fa3-4af7-8079-405b92958756"}})

To Reproduce
Schema:
query GetSalesOrder($getInput: GetOrderInput!) {
getSalesOrder(input: $getInput) {
salesOrder {
id
}
}
}
and specify a response fetcher that expects to fetch from cache.
this.appSyncClient.query(query).responseFetcher(AppSyncResponseFetchers.CACHE_AND_NETWORK

Expected behavior
We'd expect the first execution to miss, but future executions with the same arguments to be a cache hit.

Screenshots
The resulting record cache has a number of unique objects which all represent the same query, and thus always a CACHE_MISS. With a cache key generated like the above, we'd expect to see only 1 cache object, and that to always have a CACHE_HIT.
Cache with the bad generated key

record = {Record@6240} 
 fields = {LinkedHashMap@6245}  size = 7
  0 = {LinkedHashMap$LinkedHashMapEntry@6250} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@f0b2ff7)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@f0b2ff7)"
  1 = {LinkedHashMap$LinkedHashMapEntry@6251} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@77dff0d)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@77dff0d)"
  2 = {LinkedHashMap$LinkedHashMapEntry@6252} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@ed7ecfb)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@ed7ecfb)"
  3 = {LinkedHashMap$LinkedHashMapEntry@6253} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@da3a165)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@da3a165)"
  4 = {LinkedHashMap$LinkedHashMapEntry@6254} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@3ec6ac7)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@3ec6ac7)"
  5 = {LinkedHashMap$LinkedHashMapEntry@6255} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@665ebbf)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@665ebbf)"
  6 = {LinkedHashMap$LinkedHashMapEntry@6256} "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@6607b78)" -> "getSalesOrder(input:com.regiscorp.athena.sdk.order.graphql.type.GetOrderInput@6607b78)"
 key = "QUERY_ROOT"
 mutationId = null
 sizeInBytes = -1
 shadow$_klass_ = {Class@5199} "class com.apollographql.apollo.cache.normalized.Record"
 shadow$_monitor_ = -2076128168

The cache using the apollo-android 1.0.1-snapshot with a generated cache key that can result is hits

record = {Record@6075} "Record{key='QUERY_ROOT', fields={getSalesOrder({"input":{"id":"583b44c5-0fa3-4af7-8079-405b92958756"}})=getSalesOrder({"input":{"id":"583b44c5-0fa3-4af7-8079-405b92958756"}})}}"
 fields = {LinkedHashMap@6106}  size = 1
  0 = {LinkedHashMap$LinkedHashMapEntry@6111} "getSalesOrder({"input":{"id":"583b44c5-0fa3-4af7-8079-405b92958756"}})" -> "getSalesOrder({"input":{"id":"583b44c5-0fa3-4af7-8079-405b92958756"}})"
 key = "QUERY_ROOT"
 mutationId = null
 sizeInBytes = -1
 shadow$_klass_ = {Class@5125} "class com.apollographql.apollo.cache.normalized.Record"
 shadow$_monitor_ = -1973104355```

Environment(please complete the following information):

  • AppSync SDK Version: 2.7.5

Device Information (please complete the following information):

  • Device: Pixel C Emulator
  • Android Version: API 24

Additional context
Unfortunately, the latest apollo-android code generates a POST body that results in a 400 from AppSync. This change in the apollo code apollographql/apollo-kotlin#1137 is the offending change.

@scb01 scb01 self-assigned this Jan 22, 2019
@scb01 scb01 added bug Something isn't working AppSync labels Jan 22, 2019
@scb01
Copy link
Contributor

scb01 commented Jan 22, 2019

@donaldarmstrong
Thanks for providing the detailed information on how to reproduce. I will look into this and update this thread when I have more information.

@donaldjarmstrong
Copy link

@cbommas need more information?

@frankmuellr frankmuellr assigned desokroshan and unassigned scb01 Mar 18, 2019
@desokroshan
Copy link
Contributor

@donaldarmstrong We have all the information required to start investigating the issue. I will try to reproduce the issue and let you know if I need any further information. Thanks!

@desokroshan
Copy link
Contributor

@donaldarmstrong
Update: I have drafted a PR with changes from the apollo issue referenced above. It's a work in progress. I am working on completing validations on my end and adding some tests. In the meantime feel free to work off of the branch and check if it forms the cache key appropriately.

@donaldarmstrong
Copy link
Author

donaldarmstrong commented Apr 8, 2019

Hello @desokroshan and thank you for your help. I've pulled your branch and I still see the problem.

@donaldarmstrong
Copy link
Author

donaldarmstrong commented Apr 9, 2019

Here is what the cache key looks like with your changes...
getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@690a95d"})

and the cache...
rootRecord = {Record@7694} fields = {LinkedHashMap@7700} size = 3 0 = {LinkedHashMap$LinkedHashMapEntry@7705} "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@a5e7655"})" -> "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@a5e7655"})" 1 = {LinkedHashMap$LinkedHashMapEntry@7706} "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@27d61d8"})" -> "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@27d61d8"})" 2 = {LinkedHashMap$LinkedHashMapEntry@7707} "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@690a95d"})" -> "getSalesOrder({"input":"com.regiscorp.athena.graphql.type.GetOrderInput@690a95d"})" key = "QUERY_ROOT"

I think the problem stems from com.apollographql.apollo.internal.json.Utils#writeToJson where given the argument value of value = {TreeMap@7760} size = 1 0 = {TreeMap$TreeMapEntry@7764} "input" -> key = "input" value = {GetOrderInput@7765} id = "365bfc5f-0dab-4fad-a8de-a7ee85ba27e8" salon = {GetSalonInput@7766} shadow$_klass_ = {Class@6217} "class com.regiscorp.athena.graphql.type.GetOrderInput" shadow$_monitor_ = -1986479156
the code will fall to the else on line 65.

Hope this helps. I can provide more data / assistance if needed.

@desokroshan
Copy link
Contributor

Thanks for the detailed response. I see that with the changes from PR cache keys are formed as you would expect. I get cache key as : getEvent2({"id2":{"id":"05f98509-5cf0-44fd-a484-cffb715af30f"}}) for following input type in schema

--Other types--
input EventId2 {
	id: String
}

type Query {
	getEvent2(id2: EventId2): Event
}
--rest of the schema--

query :

query getEv2($input2: EventId2) {
  getEvent2(id2: $input2){
    name
    where
  }
}

How are you consuming these changes? Also can you confirm that the input types in generated code are subclasses of InputType that was added in the PR?

@donaldarmstrong
Copy link
Author

Thanks for the InputType hint. I could not get the gradle plugin working, so I manually added the InputType like public final class GetOrderInput implements InputType.

Here is the cache after adding the interface
rootRecord = {Record@7175} fields = {LinkedHashMap@7177} size = 2 0 = {LinkedHashMap$LinkedHashMapEntry@7182} "listShifts({"filter":{"fromShiftDate":null,"salon":{"id":"3"},"toShiftDate":null},"limit":null,"nextToken":null})" -> "listShifts({"filter":{"fromShiftDate":null,"salon":{"id":"3"},"toShiftDate":null},"limit":null,"nextToken":null})" 1 = {LinkedHashMap$LinkedHashMapEntry@7183} "getSalesOrder({"input":{"id":"365bfc5f-0dab-4fad-a8de-a7ee85ba27e8","salon":{"id":"3"}}})" -> "getSalesOrder({"input":{"id":"365bfc5f-0dab-4fad-a8de-a7ee85ba27e8","salon":{"id":"3"}}})" key = "QUERY_ROOT"

The cache key looks correct, and I can also confirm that we are getting cached responses from com.apollographql.apollo.internal.interceptor.ApolloCacheInterceptor#resolveFromCache.

Thank you for the fix. I'll mark the bug as resolved and await a release!

@desokroshan
Copy link
Contributor

@donaldarmstrong Fix has been released in the version 2.8.0. Thanks for your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants