From e7d70c5af064b57eee3b383bef9849270edc4dff Mon Sep 17 00:00:00 2001 From: Benoit 'BoD' Lubek Date: Thu, 2 May 2024 11:00:29 +0200 Subject: [PATCH] Remove existing interceptors from ApolloClient.Builder before adding new ones (#5858) * Remove existing ApolloInterceptors when calling ApolloClient.Builder.httpCache() * Remove existing ApolloInterceptors when calling ApolloClient.Builder.autoPersistedQueries() * Remove existing HttpInterceptors when calling ApolloClient.Builder.httpCache() * Remove existing HttpInterceptors when calling ApolloClient.Builder.httpBatching() * Add a test for httpCache * Don't crash if no `X-APOLLO-SERVED-DATE` header is present * Revert CachingHttpInterceptor change --- .../apollo3/cache/http/HttpCacheExtensions.kt | 101 +++--------------- .../internal/CacheHeadersHttpInterceptor.kt | 24 +++++ .../internal/HttpCacheApolloInterceptor.kt | 76 +++++++++++++ .../api/android/apollo-runtime.api | 1 + .../apollo-runtime/api/jvm/apollo-runtime.api | 1 + .../com/apollographql/apollo3/ApolloClient.kt | 9 ++ .../src/test/kotlin/HttpCacheTest.kt | 53 ++++++++- 7 files changed, 177 insertions(+), 88 deletions(-) create mode 100644 libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/CacheHeadersHttpInterceptor.kt create mode 100644 libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/HttpCacheApolloInterceptor.kt diff --git a/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/HttpCacheExtensions.kt b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/HttpCacheExtensions.kt index a68130ed84a..ec6753f0749 100644 --- a/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/HttpCacheExtensions.kt +++ b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/HttpCacheExtensions.kt @@ -3,30 +3,16 @@ package com.apollographql.apollo3.cache.http import com.apollographql.apollo3.ApolloClient -import com.apollographql.apollo3.api.ApolloRequest import com.apollographql.apollo3.api.ApolloResponse import com.apollographql.apollo3.api.ExecutionContext import com.apollographql.apollo3.api.MutableExecutionOptions -import com.apollographql.apollo3.api.Mutation import com.apollographql.apollo3.api.Operation -import com.apollographql.apollo3.api.Query -import com.apollographql.apollo3.api.Subscription -import com.apollographql.apollo3.api.http.HttpRequest -import com.apollographql.apollo3.api.http.HttpResponse -import com.apollographql.apollo3.api.http.valueOf -import com.apollographql.apollo3.cache.http.CachingHttpInterceptor.Companion.OPERATION_NAME_HEADER -import com.apollographql.apollo3.interceptor.ApolloInterceptor -import com.apollographql.apollo3.interceptor.ApolloInterceptorChain +import com.apollographql.apollo3.cache.http.internal.CacheHeadersHttpInterceptor +import com.apollographql.apollo3.cache.http.internal.HttpCacheApolloInterceptor import com.apollographql.apollo3.network.http.HttpInfo -import com.apollographql.apollo3.network.http.HttpInterceptor -import com.apollographql.apollo3.network.http.HttpInterceptorChain import com.apollographql.apollo3.network.http.HttpNetworkTransport -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.onCompletion -import kotlinx.coroutines.flow.onEach import okio.FileSystem import java.io.File -import java.io.IOException enum class HttpFetchPolicy { /** @@ -80,81 +66,26 @@ fun ApolloClient.Builder.httpCache( apolloHttpCache: ApolloHttpCache, ): ApolloClient.Builder { val cachingHttpInterceptor = CachingHttpInterceptor(apolloHttpCache) - val apolloRequestToCacheKey = mutableMapOf() - return addHttpInterceptor(object : HttpInterceptor { - override suspend fun intercept(request: HttpRequest, chain: HttpInterceptorChain): HttpResponse { - val cacheKey = CachingHttpInterceptor.cacheKey(request) - val requestUuid = request.headers.valueOf(CachingHttpInterceptor.REQUEST_UUID_HEADER)!! - synchronized(apolloRequestToCacheKey) { - apolloRequestToCacheKey[requestUuid] = cacheKey - } - return chain.proceed( - request.newBuilder() - .headers(request.headers.filterNot { it.name == CachingHttpInterceptor.REQUEST_UUID_HEADER }) - .addHeader(CachingHttpInterceptor.CACHE_KEY_HEADER, cacheKey) - .build() - ) + return apply { + httpInterceptors.firstOrNull { it is CacheHeadersHttpInterceptor }?.let { + removeHttpInterceptor(it) } - }).addHttpInterceptor( - cachingHttpInterceptor - ).addInterceptor(object : ApolloInterceptor { - override fun intercept(request: ApolloRequest, chain: ApolloInterceptorChain): Flow> { - val policy = getPolicy(request) - val policyStr = when (policy) { - HttpFetchPolicy.CacheFirst -> CachingHttpInterceptor.CACHE_FIRST - HttpFetchPolicy.CacheOnly -> CachingHttpInterceptor.CACHE_ONLY - HttpFetchPolicy.NetworkFirst -> CachingHttpInterceptor.NETWORK_FIRST - HttpFetchPolicy.NetworkOnly -> CachingHttpInterceptor.NETWORK_ONLY - } - - return chain.proceed( - request.newBuilder() - .addHttpHeader( - CachingHttpInterceptor.CACHE_OPERATION_TYPE_HEADER, - when (request.operation) { - is Query<*> -> "query" - is Mutation<*> -> "mutation" - is Subscription<*> -> "subscription" - else -> error("Unknown operation type") - } - ) - .addHttpHeader(CachingHttpInterceptor.CACHE_FETCH_POLICY_HEADER, policyStr) - .addHttpHeader(CachingHttpInterceptor.REQUEST_UUID_HEADER, request.requestUuid.toString()) - .addHttpHeader(OPERATION_NAME_HEADER, request.operation.name()) - .build() - ) - .run { - if (request.operation is Query<*>) { - onEach { response -> - // Revert caching of responses with errors - val cacheKey = synchronized(apolloRequestToCacheKey) { apolloRequestToCacheKey[request.requestUuid.toString()] } - if (response.hasErrors() || response.exception != null) { - try { - cacheKey?.let { cachingHttpInterceptor.cache.remove(it) } - } catch (_: IOException) { - } - } - }.onCompletion { - synchronized(apolloRequestToCacheKey) { apolloRequestToCacheKey.remove(request.requestUuid.toString()) } - } - } else { - this - } - } + httpInterceptors.firstOrNull { it is CachingHttpInterceptor }?.let { + removeHttpInterceptor(it) } - }) -} - -private fun getPolicy(request: ApolloRequest<*>): HttpFetchPolicy { - return if (request.operation is Mutation<*>) { - // Don't cache mutations - HttpFetchPolicy.NetworkOnly - } else { - request.executionContext[HttpFetchPolicyContext]?.httpFetchPolicy ?: HttpFetchPolicy.CacheFirst } + .addHttpInterceptor(CacheHeadersHttpInterceptor(apolloRequestToCacheKey)) + .addHttpInterceptor(cachingHttpInterceptor) + .apply { + interceptors.firstOrNull { it is HttpCacheApolloInterceptor }?.let { + removeInterceptor(it) + } + } + .addInterceptor(HttpCacheApolloInterceptor(apolloRequestToCacheKey, cachingHttpInterceptor)) } + val ApolloResponse.isFromHttpCache get() = executionContext[HttpInfo]?.headers?.any { // This will return true whatever the value in the header. We might want to fine tune this diff --git a/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/CacheHeadersHttpInterceptor.kt b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/CacheHeadersHttpInterceptor.kt new file mode 100644 index 00000000000..81797b4cb16 --- /dev/null +++ b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/CacheHeadersHttpInterceptor.kt @@ -0,0 +1,24 @@ +package com.apollographql.apollo3.cache.http.internal + +import com.apollographql.apollo3.api.http.HttpRequest +import com.apollographql.apollo3.api.http.HttpResponse +import com.apollographql.apollo3.api.http.valueOf +import com.apollographql.apollo3.cache.http.CachingHttpInterceptor +import com.apollographql.apollo3.network.http.HttpInterceptor +import com.apollographql.apollo3.network.http.HttpInterceptorChain + +internal class CacheHeadersHttpInterceptor(private val apolloRequestToCacheKey: MutableMap) : HttpInterceptor { + override suspend fun intercept(request: HttpRequest, chain: HttpInterceptorChain): HttpResponse { + val cacheKey = CachingHttpInterceptor.cacheKey(request) + val requestUuid = request.headers.valueOf(CachingHttpInterceptor.REQUEST_UUID_HEADER)!! + synchronized(apolloRequestToCacheKey) { + apolloRequestToCacheKey[requestUuid] = cacheKey + } + return chain.proceed( + request.newBuilder() + .headers(request.headers.filterNot { it.name == CachingHttpInterceptor.REQUEST_UUID_HEADER }) + .addHeader(CachingHttpInterceptor.CACHE_KEY_HEADER, cacheKey) + .build() + ) + } +} diff --git a/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/HttpCacheApolloInterceptor.kt b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/HttpCacheApolloInterceptor.kt new file mode 100644 index 00000000000..3c2d0bcb7d3 --- /dev/null +++ b/libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/internal/HttpCacheApolloInterceptor.kt @@ -0,0 +1,76 @@ +package com.apollographql.apollo3.cache.http.internal + +import com.apollographql.apollo3.api.ApolloRequest +import com.apollographql.apollo3.api.ApolloResponse +import com.apollographql.apollo3.api.Mutation +import com.apollographql.apollo3.api.Operation +import com.apollographql.apollo3.api.Query +import com.apollographql.apollo3.api.Subscription +import com.apollographql.apollo3.cache.http.CachingHttpInterceptor +import com.apollographql.apollo3.cache.http.HttpFetchPolicy +import com.apollographql.apollo3.cache.http.HttpFetchPolicyContext +import com.apollographql.apollo3.interceptor.ApolloInterceptor +import com.apollographql.apollo3.interceptor.ApolloInterceptorChain +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.flow.onEach +import java.io.IOException + +internal class HttpCacheApolloInterceptor( + private val apolloRequestToCacheKey: MutableMap, + private val cachingHttpInterceptor: CachingHttpInterceptor, +) : ApolloInterceptor { + override fun intercept(request: ApolloRequest, chain: ApolloInterceptorChain): Flow> { + val policy = getPolicy(request) + val policyStr = when (policy) { + HttpFetchPolicy.CacheFirst -> CachingHttpInterceptor.CACHE_FIRST + HttpFetchPolicy.CacheOnly -> CachingHttpInterceptor.CACHE_ONLY + HttpFetchPolicy.NetworkFirst -> CachingHttpInterceptor.NETWORK_FIRST + HttpFetchPolicy.NetworkOnly -> CachingHttpInterceptor.NETWORK_ONLY + } + + return chain.proceed( + request.newBuilder() + .addHttpHeader( + CachingHttpInterceptor.CACHE_OPERATION_TYPE_HEADER, + when (request.operation) { + is Query<*> -> "query" + is Mutation<*> -> "mutation" + is Subscription<*> -> "subscription" + else -> error("Unknown operation type") + } + ) + .addHttpHeader(CachingHttpInterceptor.CACHE_FETCH_POLICY_HEADER, policyStr) + .addHttpHeader(CachingHttpInterceptor.REQUEST_UUID_HEADER, request.requestUuid.toString()) + .addHttpHeader(CachingHttpInterceptor.OPERATION_NAME_HEADER, request.operation.name()) + .build() + ) + .run { + if (request.operation is Query<*>) { + onEach { response -> + // Revert caching of responses with errors + val cacheKey = synchronized(apolloRequestToCacheKey) { apolloRequestToCacheKey[request.requestUuid.toString()] } + if (response.hasErrors() || response.exception != null) { + try { + cacheKey?.let { cachingHttpInterceptor.cache.remove(it) } + } catch (_: IOException) { + } + } + }.onCompletion { + synchronized(apolloRequestToCacheKey) { apolloRequestToCacheKey.remove(request.requestUuid.toString()) } + } + } else { + this + } + } + } + + private fun getPolicy(request: ApolloRequest<*>): HttpFetchPolicy { + return if (request.operation is Mutation<*>) { + // Don't cache mutations + HttpFetchPolicy.NetworkOnly + } else { + request.executionContext[HttpFetchPolicyContext]?.httpFetchPolicy ?: HttpFetchPolicy.CacheFirst + } + } +} diff --git a/libraries/apollo-runtime/api/android/apollo-runtime.api b/libraries/apollo-runtime/api/android/apollo-runtime.api index 0ca32d97877..46a5867971c 100644 --- a/libraries/apollo-runtime/api/android/apollo-runtime.api +++ b/libraries/apollo-runtime/api/android/apollo-runtime.api @@ -118,6 +118,7 @@ public final class com/apollographql/apollo3/ApolloClient$Builder : com/apollogr public final fun httpServerUrl (Ljava/lang/String;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun interceptors (Ljava/util/List;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun networkTransport (Lcom/apollographql/apollo3/network/NetworkTransport;)Lcom/apollographql/apollo3/ApolloClient$Builder; + public final fun removeHttpInterceptor (Lcom/apollographql/apollo3/network/http/HttpInterceptor;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun removeInterceptor (Lcom/apollographql/apollo3/interceptor/ApolloInterceptor;)Lcom/apollographql/apollo3/ApolloClient$Builder; public fun sendApqExtensions (Ljava/lang/Boolean;)Lcom/apollographql/apollo3/ApolloClient$Builder; public synthetic fun sendApqExtensions (Ljava/lang/Boolean;)Ljava/lang/Object; diff --git a/libraries/apollo-runtime/api/jvm/apollo-runtime.api b/libraries/apollo-runtime/api/jvm/apollo-runtime.api index 0ca32d97877..46a5867971c 100644 --- a/libraries/apollo-runtime/api/jvm/apollo-runtime.api +++ b/libraries/apollo-runtime/api/jvm/apollo-runtime.api @@ -118,6 +118,7 @@ public final class com/apollographql/apollo3/ApolloClient$Builder : com/apollogr public final fun httpServerUrl (Ljava/lang/String;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun interceptors (Ljava/util/List;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun networkTransport (Lcom/apollographql/apollo3/network/NetworkTransport;)Lcom/apollographql/apollo3/ApolloClient$Builder; + public final fun removeHttpInterceptor (Lcom/apollographql/apollo3/network/http/HttpInterceptor;)Lcom/apollographql/apollo3/ApolloClient$Builder; public final fun removeInterceptor (Lcom/apollographql/apollo3/interceptor/ApolloInterceptor;)Lcom/apollographql/apollo3/ApolloClient$Builder; public fun sendApqExtensions (Ljava/lang/Boolean;)Lcom/apollographql/apollo3/ApolloClient$Builder; public synthetic fun sendApqExtensions (Ljava/lang/Boolean;)Ljava/lang/Object; diff --git a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt index dc4f60e55ba..21b34cf8c32 100644 --- a/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt +++ b/libraries/apollo-runtime/src/commonMain/kotlin/com/apollographql/apollo3/ApolloClient.kt @@ -616,6 +616,13 @@ private constructor( _httpInterceptors += httpInterceptor } + /** + * Removes [httpInterceptor] from the list of HTTP interceptors. + */ + fun removeHttpInterceptor(httpInterceptor: HttpInterceptor) = apply { + _httpInterceptors -= httpInterceptor + } + /** * The url of the GraphQL server used for WebSockets * Use this function or webSocketServerUrl((suspend () -> String)) but not both. @@ -856,6 +863,7 @@ private constructor( httpMethodForDocumentQueries: HttpMethod = HttpMethod.Post, enableByDefault: Boolean = true, ) = apply { + _interceptors.removeAll { it is AutoPersistedQueryInterceptor } addInterceptor( AutoPersistedQueryInterceptor( httpMethodForHashedQueries, @@ -883,6 +891,7 @@ private constructor( maxBatchSize: Int = 10, enableByDefault: Boolean = true, ) = apply { + _httpInterceptors.removeAll { it is BatchingHttpInterceptor } addHttpInterceptor(BatchingHttpInterceptor(batchIntervalMillis, maxBatchSize)) canBeBatched(enableByDefault) } diff --git a/tests/http-cache/src/test/kotlin/HttpCacheTest.kt b/tests/http-cache/src/test/kotlin/HttpCacheTest.kt index cf2f272f1ee..b1b6ae35463 100644 --- a/tests/http-cache/src/test/kotlin/HttpCacheTest.kt +++ b/tests/http-cache/src/test/kotlin/HttpCacheTest.kt @@ -1,5 +1,7 @@ - import com.apollographql.apollo3.ApolloClient +import com.apollographql.apollo3.api.http.HttpResponse +import com.apollographql.apollo3.cache.http.ApolloHttpCache +import com.apollographql.apollo3.cache.http.DiskLruHttpCache import com.apollographql.apollo3.cache.http.HttpFetchPolicy import com.apollographql.apollo3.cache.http.httpCache import com.apollographql.apollo3.cache.http.httpExpireTimeout @@ -22,6 +24,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import okhttp3.Interceptor import okhttp3.OkHttpClient +import okio.FileSystem import java.io.File import kotlin.test.Test import kotlin.test.assertEquals @@ -200,7 +203,8 @@ class HttpCacheTest { "setRandom": "42" } } - """.trimIndent()) + """.trimIndent() + ) apolloClient.mutation(mutation) .httpFetchPolicy(HttpFetchPolicy.CacheOnly) .execute() @@ -231,7 +235,8 @@ class HttpCacheTest { }, "errors": [ { "message": "GraphQL error" } ] } - """) + """ + ) apolloClient.query(GetRandomQuery()).execute() // Should not have been cached assertIs( @@ -275,4 +280,46 @@ class HttpCacheTest { } } + @Test + fun httpCacheCleansPreviousInterceptor() = runTest { + mockServer = MockServer() + val httpCache1 = CountingApolloHttpCache() + mockServer.enqueueData(data) + val apolloClient = ApolloClient.Builder() + .serverUrl(mockServer.url()) + .httpCache(httpCache1) + .build() + apolloClient.query(GetRandomQuery()).execute() + assertEquals(1, httpCache1.writes) + + val httpCache2 = CountingApolloHttpCache() + val apolloClient2 = apolloClient.newBuilder() + .httpCache(httpCache2) + .build() + mockServer.enqueueData(data) + apolloClient2.query(GetRandomQuery()).execute() + assertEquals(1, httpCache1.writes) + assertEquals(1, httpCache2.writes) + } +} + +private class CountingApolloHttpCache : ApolloHttpCache { + private val wrapped = run { + val dir = File("build/httpCache") + dir.deleteRecursively() + DiskLruHttpCache(FileSystem.SYSTEM, dir, Long.MAX_VALUE) + } + var writes = 0 + override fun write(response: HttpResponse, cacheKey: String): HttpResponse { + writes++ + return wrapped.write(response, cacheKey) + } + + override fun read(cacheKey: String): HttpResponse { + return wrapped.read(cacheKey) + } + + override fun clearAll() {} + + override fun remove(cacheKey: String) {} }