Apollo-android: classes generated with `sealedClassesForEnumsMatching` break sqlite based caching

[mateuszkwiecinski]

Summary
When using sealedClassesForEnumsMatching generated classes as query parameters, apollo-android library uses their hashcode when generating a cache keys which gets problematic when using cache provided by com.apollographql.apollo:apollo-normalized-cache-sqlite

After app restart all generated classes have different hashcodes which result in cache misses.

Version
2.4.5

Description
When implementing sqlite based cache I noticed some of the queries are always fetched from the network reporting 100% cache misses. I dug into the problem and noticed that apollo includes class hashcode when generating cache keys.
This make app to properly cache responses in its memory cache (as long as generated objects are the same instance), but it breaks after app restart.
I guess generated classes should have hashcode method overriden, or if that's a result of toString() maybe it would be worth to have it constant and also generated?

tl;dr

1. Have a graphql query that takes graphql enum
2. Use cache from com.apollographql.apollo:apollo-normalized-cache-sqlite on Android
3. When fetched library will report 100% cache misses

[martinbonnin]

Oh wow, good catch! That's definitely not what we want. The toString() is coming from there: https://github.com/apollographql/apollo-android/blob/1fd45e073960ee12b73a4967e26b261ffe14443d/apollo-api/src/commonMain/kotlin/com/apollographql/apollo/api/internal/json/Utils.kt#L35

Overriding toString() might do the job, I wonder if there's another way to fix it though.

[mateuszkwiecinski]

The plugin generates unique override fun operationId(): String = OPERATION_ID for each query. Maybe it could similary generate identifiers for each enum option?

btw. I just realized including a class name might also be problematic if generated class is obfuscated using R8 🤔 So even if I (as a workaround) disable the sealedClassesForEnumsMatching and use enum instead of an object it would include the obfuscated name as a part of a cache key - which might also cause cache misses across different app versions. So id doesn't matter if I use sealedClassesForEnumsMatching or not 🤔

[martinbonnin]

The plugin generates unique override fun operationId(): String = OPERATION_ID for each query. Maybe it could similary generate identifiers for each enum option?

Yes, either that or we could alse make all sealed classes inherit from a generic GraphQLEnum {val rawValue} interface.

I just realized including a class name might also be problematic if generated class is obfuscated using R8

I would expect the toString() method of enums to not include a class name and be robust to minification so using enums should be safe.

[mateuszkwiecinski]

I would expect the toString() method of enums to not include a class name and be robust to minification so using enums should be safe.

Yeah, sorry, you're right. Regular enums are indeed ok 👍

[lwasyl]

@martinbonnin I took a stab at this with https://github.com/apollographql/apollo-android/pull/2776. I don't know if this is what you meant, also I'm not sure how (and where) to test the SQL cache behavior, though.

[martinbonnin]

I just bumped into https://github.com/apollographql/apollo-android/issues/2577 which is the exact same issue with scalar values. Linking it here in case someone else bumps into it.