apollographql / apollo-kotlin

:rocket:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.75k stars 651 forks source link

Http cache does not work after migration from 2.0.3 to 2.5.4 #2957

Closed beibuttukibayev closed 3 years ago

beibuttukibayev commented 3 years ago

Summary After we upgraded Apollo version from 2.0.3 to 2.5.4 http cache stopped working for some reason. Requests work fine, but without caching mechanism.

Version 2.5.4

Description Working code that we used in 2.0.3 to cache:

val queryCall = query(query)
        .httpCachePolicy(HttpCachePolicy.CACHE_FIRST)
        .toDeferred()
        .await()

This piece of code works in 2.5.4 but without caching. I also tried:

val queryCall = query(query)
        .toBuilder()
        .httpCachePolicy(HttpCachePolicy.CACHE_FIRST)
        .build()
        .await()

Apollo Client for both cases is created by:

fun createApollo(okHttp: OkHttpClient, context: Context): ApolloClient {
    val cacheFile = File(context.applicationContext.cacheDir, APOLLO_CACHE_DIRECTORY_NAME)
    val cacheStore = DiskLruHttpCacheStore(cacheFile, CACHE_SIZE_IN_BYTES)

    return ApolloClient
        .builder()
        .serverUrl(BASE_APOLLO_URL)
        .httpCache(ApolloHttpCache(cacheStore))
        .okHttpClient(okHttp)
        .build()
}
martinbonnin commented 3 years ago

Thanks for reaching out! We have tests for basic cases that are still passing like https://github.com/apollographql/apollo-android/blob/dd10f00cf3dc43b63213685e0b926dffd5c34162/apollo-integration/src/test/java/com/apollographql/apollo/HttpCacheTest.java#L585 So I'm assuming there's more to the story. Is there any chance you can put a small reproducer somewhere? Also, does your query use variables? Can you share it?

bekabot commented 3 years ago

Thank you for your quick reply. @beibuttukibayev is my second account.

So I created this project where error occurs. Steps to reproduce the issue:

1) Run project and click text view to make request 2) Request is successful (and data is saved in cache) 3) Turn off internet 3) Make request again 5) Data should be fetched from cache (current version is 2.0.3)

6) Go to build.gradle & Change apolloVersion variable to 2.5.4 (already commented) 7) Change apollo-gradle-plugin version to 2.5.4 (already commented) 9) Delete app (to ensure cache is deleted) & Run app & Request data 10) Request is successful 11) Turn off internet & Request data again 12) No data is returned

martinbonnin commented 3 years ago

Nice! Thanks you so much, will look into it.

martinbonnin commented 3 years ago

It looks like it's coming from the HTTPS configuration. I can make it work by commenting the custom SSLSocketFactory and HostNameVerifier: https://github.com/martinbonnin/ApolloCacheErrorReproducer/commit/6ddaf214bcacf60c715bb262505be2caa85d423d#diff-263cd378c282b15abc5bc3316427664a4a9c94f7d5b20d79bdfacc68cfa95d1fR40

I'm not super familiar with this code, HTTPS can get pretty complex, but I would assume that the cache is still working in production versions of the app that don't include the "accept-all" SSL configuration?

martinbonnin commented 3 years ago

For the record, this is the exception I get when trying to write into the cache:

javax.net.ssl.SSLPeerUnverifiedException: Failed to find a trusted cert that signed Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            02:ac:5c:26:6a:0b:40:9b:8f:0b:79:f2:ae:46:25:77
    Signature Algorithm: sha1WithRSAEncryption
        Issuer: C=US, O=DigiCert Inc, OU=www.digicert.com, CN=DigiCert High Assurance EV Root CA
        Validity
            Not Before: Nov 10 00:00:00 2006 GMT
            Not After : Nov 10 00:00:00 2031 GMT
        Subject: C=US, O=DigiCert Inc, OU=www.digicert.com, CN=DigiCert High Assurance EV Root CA
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    [...]
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Key Usage: critical
                Digital Signature, Certificate Sign, CRL Sign
            X509v3 Basic Constraints: critical
                CA:TRUE
            X509v3 Subject Key Identifier: 
                B1:3E:C3:69:03:F8:BF:47:01:D4:98:26:1A:08:02:EF:63:64:2B:C3
            X509v3 Authority Key Identifier: 
                keyid:B1:3E:C3:69:03:F8:BF:47:01:D4:98:26:1A:08:02:EF:63:64:2B:C3
    Signature Algorithm: sha1WithRSAEncryption
                    [...]
    at okhttp3.internal.tls.BasicCertificateChainCleaner.clean(BasicCertificateChainCleaner.kt:91)
    at okhttp3.internal.connection.RealConnection$connectTls$1.invoke(RealConnection.kt:393)
    at okhttp3.internal.connection.RealConnection$connectTls$1.invoke(RealConnection.kt:73)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at okhttp3.Handshake.peerCertificates(Unknown Source:7)
    at com.apollographql.apollo.cache.http.ResponseHeaderRecord.writeTo(ResponseHeaderRecord.java:223)
    at com.apollographql.apollo.cache.http.ApolloHttpCache.cacheProxy(ApolloHttpCache.java:111)
    at com.apollographql.apollo.cache.http.HttpCacheInterceptor.cacheFirst(HttpCacheInterceptor.java:133)
    at com.apollographql.apollo.cache.http.HttpCacheInterceptor.intercept(HttpCacheInterceptor.java:54)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
    at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.kt:219)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:100)
    at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:197)
    at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:502)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.lang.Thread.run(Thread.java:923)
martinbonnin commented 3 years ago

Alright, it's an OkHttp thing. Apollo 2.0.3 has OkHttp 4.5.0. We had to downgrade that to keep supporting older Android devices but OkHttp actually has a few fixes in 4.4.1 that seems to be the fix for your issue. From the Changelog:

Fix: Don't reuse a connection on redirect if certs match but DNS does not. For better locality and performance OkHttp 
attempts to use the same pooled connection across redirects and follow-ups. It independently shares connections when
 the IP addresses and certificates match, even if the host names do not. In 4.4.0 we introduced a regression where we 
shared a connection when certificates matched but the DNS addresses did not. This would only occur when following a
 redirect from one hostname to another, and where both hosts had common certificates.

Fix: Don't fail on a redirect when a client has configured a 'trust everything' trust manager. Typically this would cause 
certain redirects to fail in debug and development configurations.

If you force OkHttp >= 4.4.1 in your Gradle configuration it should work again (edit, corrected from '>' to '>=')

bekabot commented 3 years ago

After forcing okHttp version to 4.5.0 caching works as expected. Thank you @martinbonnin !

martinbonnin commented 3 years ago

Nice! As I just edited above, 4.4.1 should work too but there's no harm in using 4.5.0! Thanks for investigating this and the nice reproducer 👍