Remove existing interceptors from ApolloClient.Builder before adding new ones

libraries/apollo-http-cache/src/main/kotlin/com/apollographql/apollo3/cache/http/CachingHttpInterceptor.kt

@@ -19,14 +19,14 @@ import java.time.Instant
 import java.time.format.DateTimeParseException
 
 class CachingHttpInterceptor internal constructor(
-    private val lruHttpCache: ApolloHttpCache
+    private val lruHttpCache: ApolloHttpCache,
 ) : HttpInterceptor {
 
   constructor(
       directory: File,
       maxSize: Long,
       fileSystem: FileSystem = FileSystem.SYSTEM,
-  ): this(DiskLruHttpCache(fileSystem, directory, maxSize))
+  ) : this(DiskLruHttpCache(fileSystem, directory, maxSize))
 
   val cache: ApolloHttpCache = lruHttpCache
 
@@ -122,7 +122,8 @@ class CachingHttpInterceptor internal constructor(
                   )
               )
               .build(),
-          cacheKey)
+          cacheKey
+      )
     }
     return response
   }
@@ -152,11 +153,13 @@ class CachingHttpInterceptor internal constructor(
     }
 
     val timeoutMillis = request.headers.valueOf(CACHE_EXPIRE_TIMEOUT_HEADER)?.toLongOrNull() ?: 0
-    val servedDateMillis = try {
-      Instant.parse(response.headers.valueOf(CACHE_SERVED_DATE_HEADER)).toEpochMilli()
-    } catch (e: DateTimeParseException) {
-      0L
-    }
+    val servedDateMillis = response.headers.valueOf(CACHE_SERVED_DATE_HEADER)?.let {
+      try {
+        Instant.parse(it).toEpochMilli()
+      } catch (e: DateTimeParseException) {
+        0
+      }
+    } ?: 0
     val nowMillis = Instant.now().toEpochMilli()
 
     if (timeoutMillis > 0 && servedDateMillis > 0 && nowMillis - servedDateMillis > timeoutMillis) {

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<String, String>()
-  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 <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
-      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 <D : Operation.Data> ApolloResponse<D>.isFromHttpCache
   get() = executionContext[HttpInfo]?.headers?.any {
     // This will return true whatever the value in the header. We might want to fine tune this

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<String, String>) : 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()
+    )
+  }
+}

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<String, String>,
+    private val cachingHttpInterceptor: CachingHttpInterceptor,
+) : ApolloInterceptor {
+  override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
+    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
+    }
+  }
+}

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;

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;

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)
     }

tests/http-cache/src/test/kotlin/HttpCacheTest.kt

@@ -1,5 +1,6 @@
-
 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.HttpFetchPolicy
 import com.apollographql.apollo3.cache.http.httpCache
 import com.apollographql.apollo3.cache.http.httpExpireTimeout
@@ -200,7 +201,8 @@ class HttpCacheTest {
             "setRandom": "42"
           }
         }
-      """.trimIndent())
+      """.trimIndent()
+      )
       apolloClient.mutation(mutation)
           .httpFetchPolicy(HttpFetchPolicy.CacheOnly)
           .execute()
@@ -231,7 +233,8 @@ class HttpCacheTest {
           },
           "errors": [ { "message": "GraphQL error" } ]
         }
-      """)
+      """
+    )
     apolloClient.query(GetRandomQuery()).execute()
     // Should not have been cached
     assertIs<HttpCacheMissException>(
@@ -275,4 +278,43 @@ 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 {
+  var writes = 0
+  var response: HttpResponse? = null
+  override fun write(response: HttpResponse, cacheKey: String): HttpResponse {
+    writes++
+    this.response = response
+    return response
+  }
+
+  override fun read(cacheKey: String): HttpResponse {
+    return response!!
+  }
+
+  override fun clearAll() {}
+
+  override fun remove(cacheKey: String) {}
 }