Remove unnecessary suspend from ApolloStore functions

docs/source/migration/4.0.mdx

@@ -373,7 +373,7 @@ import com.example.type.__Schema
 import data.builders.schema.__Schema
 ```
 
-### KotlinLabs directive are now version 0.2
+### KotlinLabs directives are now version 0.2
 
 The embedded [kotlinlabs/0.2](https://specs.apollo.dev/kotlin_labs/v0.2/) directives are now version 0.2.
 
@@ -392,6 +392,15 @@ This is a backward compatible change.
 
 ## Cache
 
+### ApolloStore
+
+In Apollo Kotlin 3.x, most `ApolloStore` functions were marked as `suspend` even though they were not actually suspending and perform the work in the thread they are called from.
+In particular, they can be blocking when the underlying cache is doing IO, so calling them from the main thread can lead to ANRs.
+
+In Apollo Kotlin 4.0 this is still the case but the functions are no longer marked as `suspend` to avoid any confusion.
+
+### Configuration order
+
 The normalized cache must be configured before the auto persisted queries, configuring it after will now fail (see https://github.com/apollographql/apollo-kotlin/pull/4709).
 
 ## apollo-ast

libraries/apollo-debug-server/src/commonMain/kotlin/com/apollographql/apollo3/debugserver/internal/graphql/GraphQL.kt

@@ -2,8 +2,8 @@ package com.apollographql.apollo3.debugserver.internal.graphql
 
 import com.apollographql.apollo3.ApolloClient
 import com.apollographql.apollo3.annotations.GraphQLAdapter
-import com.apollographql.apollo3.annotations.GraphQLObject
 import com.apollographql.apollo3.annotations.GraphQLName
+import com.apollographql.apollo3.annotations.GraphQLObject
 import com.apollographql.apollo3.api.Adapter
 import com.apollographql.apollo3.api.CustomScalarAdapters
 import com.apollographql.apollo3.api.ExecutionContext
@@ -20,7 +20,6 @@ import com.apollographql.apollo3.execution.ExecutableSchema
 import com.apollographql.apollo3.execution.GraphQLRequest
 import com.apollographql.apollo3.execution.GraphQLRequestError
 import com.apollographql.apollo3.execution.parsePostGraphQLRequest
-import kotlinx.coroutines.runBlocking
 import okio.Buffer
 import java.util.concurrent.atomic.AtomicReference
 import kotlin.reflect.KClass
@@ -86,7 +85,7 @@ internal class GraphQLApolloClient(
 
   fun normalizedCaches(): List<NormalizedCache> {
     val apolloStore = runCatching {  apolloClient.apolloStore }.getOrNull() ?: return emptyList()
-    return runBlocking { apolloStore.dump() }.map {
+    return apolloStore.dump().map {
       NormalizedCache(id, it.key, it.value)
     }
   }

libraries/apollo-normalized-cache-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/ApolloStore.kt

@@ -25,10 +25,13 @@ import kotlin.reflect.KClass
 
 /**
  * ApolloStore exposes a thread-safe api to access a [com.apollographql.apollo3.cache.normalized.api.NormalizedCache].
+ *
+ * Note that most operations are synchronous and might block if the underlying cache is doing IO - calling them from the main thread
+ * should be avoided.
  */
 interface ApolloStore {
   /**
-   * [changedKeys] will emit changes as they are written
+   * Exposes the keys of records that have changed.
    */
   val changedKeys: SharedFlow<Set<String>>
 
@@ -43,7 +46,7 @@ interface ApolloStore {
    *
    * @return the operation data
    */
-  suspend fun <D : Operation.Data> readOperation(
+  fun <D : Operation.Data> readOperation(
       operation: Operation<D>,
       customScalarAdapters: CustomScalarAdapters = CustomScalarAdapters.Empty,
       cacheHeaders: CacheHeaders = CacheHeaders.NONE,
@@ -61,7 +64,7 @@ interface ApolloStore {
    *
    * @return the fragment data
    */
-  suspend fun <D : Fragment.Data> readFragment(
+  fun <D : Fragment.Data> readFragment(
       fragment: Fragment<D>,
       cacheKey: CacheKey,
       customScalarAdapters: CustomScalarAdapters = CustomScalarAdapters.Empty,
@@ -78,7 +81,7 @@ interface ApolloStore {
    * @param publish       whether to publish the changed keys to listeners
    * @return the changed keys
    */
-  suspend fun <D : Operation.Data> writeOperation(
+  fun <D : Operation.Data> writeOperation(
       operation: Operation<D>,
       operationData: D,
       customScalarAdapters: CustomScalarAdapters = CustomScalarAdapters.Empty,
@@ -97,7 +100,7 @@ interface ApolloStore {
    * @param publish whether to publish the changed keys to listeners
    * @return the changed keys
    */
-  suspend fun <D : Fragment.Data> writeFragment(
+  fun <D : Fragment.Data> writeFragment(
       fragment: Fragment<D>,
       cacheKey: CacheKey,
       fragmentData: D,
@@ -108,13 +111,14 @@ interface ApolloStore {
 
   /**
    * Write operation data to the optimistic store.
+   * This is a synchronous operation that might block if the underlying cache is doing IO.
    *
    * @param operation     [Operation] response data of which should be written to the store
    * @param operationData [Operation.Data] operation response data to be written to the store
    * @param mutationId    mutation unique identifier
    * @return the changed keys
    */
-  suspend fun <D : Operation.Data> writeOptimisticUpdates(
+  fun <D : Operation.Data> writeOptimisticUpdates(
       operation: Operation<D>,
       operationData: D,
       mutationId: Uuid,
@@ -124,11 +128,12 @@ interface ApolloStore {
 
   /**
    * Rollback operation data optimistic updates.
+   * This is a synchronous operation that might block if the underlying cache is doing IO.
    *
    * @param mutationId mutation unique identifier
    * @return the changed keys
    */
-  suspend fun rollbackOptimisticUpdates(
+  fun rollbackOptimisticUpdates(
       mutationId: Uuid,
       publish: Boolean = true,
   ): Set<String>
@@ -149,7 +154,7 @@ interface ApolloStore {
    * @param cascade defines if remove operation is propagated to the referenced entities
    * @return `true` if the record was successfully removed, `false` otherwise
    */
-  suspend fun remove(cacheKey: CacheKey, cascade: Boolean = true): Boolean
+  fun remove(cacheKey: CacheKey, cascade: Boolean = true): Boolean
 
   /**
    * Remove a list of cache records
@@ -159,8 +164,11 @@ interface ApolloStore {
    * @param cacheKeys keys of records to be removed
    * @return the number of records that have been removed
    */
-  suspend fun remove(cacheKeys: List<CacheKey>, cascade: Boolean = true): Int
+  fun remove(cacheKeys: List<CacheKey>, cascade: Boolean = true): Int
 
+  /**
+   * Normalize [data] to a map of [Record] keyed by [Record.key].
+   */
   fun <D : Operation.Data> normalize(
       operation: Operation<D>,
       data: D,
@@ -170,19 +178,24 @@ interface ApolloStore {
   /**
    * @param keys A set of keys of [Record] which have changed.
    */
-  suspend fun publish(keys: Set<String>)
+  fun publish(keys: Set<String>)
 
   /**
    * Direct access to the cache.
+   * This is a synchronous operation that might block if the underlying cache is doing IO.
    *
-   * @param block a function that can access the cache. The function will be called from a background thread
+   * @param block a function that can access the cache.
    */
-  suspend fun <R> accessCache(block: (NormalizedCache) -> R): R
+  fun <R> accessCache(block: (NormalizedCache) -> R): R
 
-  suspend fun dump(): Map<KClass<*>, Map<String, Record>>
+  /**
+   * Dump the content of the store for debugging purposes.
+   * This is a synchronous operation that might block if the underlying cache is doing IO.
+   */
+  fun dump(): Map<KClass<*>, Map<String, Record>>
 
   /**
-   * releases resources associated with this store.
+   * Release resources associated with this store.
    */
   fun dispose()
 }

libraries/apollo-normalized-cache-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/internal/ApolloCacheInterceptor.kt

@@ -38,7 +38,7 @@ import kotlinx.coroutines.launch
 internal class ApolloCacheInterceptor(
     val store: ApolloStore,
 ) : ApolloInterceptor {
-  private suspend fun <D : Operation.Data> maybeAsync(request: ApolloRequest<D>, block: suspend () -> Unit) {
+  private fun <D : Operation.Data> maybeAsync(request: ApolloRequest<D>, block: () -> Unit) {
     if (request.writeToCacheAsynchronously) {
       val scope = request.executionContext[ConcurrencyInfo]!!.coroutineScope
       scope.launch {
@@ -56,7 +56,7 @@ internal class ApolloCacheInterceptor(
   /**
    * @param extraKeys extra keys to publish in case there is optimistic data
    */
-  private suspend fun <D : Operation.Data> maybeWriteToCache(
+  private fun <D : Operation.Data> maybeWriteToCache(
       request: ApolloRequest<D>,
       response: ApolloResponse<D>,
       customScalarAdapters: CustomScalarAdapters,
@@ -198,7 +198,7 @@ internal class ApolloCacheInterceptor(
     }
   }
 
-  private suspend fun <D : Query.Data> readFromCache(
+  private fun <D : Query.Data> readFromCache(
       request: ApolloRequest<D>,
       customScalarAdapters: CustomScalarAdapters,
   ): ApolloResponse<D> {
@@ -248,7 +248,7 @@ internal class ApolloCacheInterceptor(
   }
 
 
-  private suspend fun <D : Operation.Data> readFromNetwork(
+  private fun <D : Operation.Data> readFromNetwork(
       request: ApolloRequest<D>,
       chain: ApolloInterceptorChain,
       customScalarAdapters: CustomScalarAdapters,

libraries/apollo-normalized-cache-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/internal/DefaultApolloStore.kt

@@ -53,12 +53,12 @@ internal class DefaultApolloStore(
 
   private val lock = Lock()
 
-  override suspend fun publish(keys: Set<String>) {
+  override fun publish(keys: Set<String>) {
     if (keys.isEmpty()) {
       return
     }
 
-    changedKeysEvents.emit(keys)
+    changedKeysEvents.tryEmit(keys)
   }
 
   override fun clearAll(): Boolean {
@@ -68,7 +68,7 @@ internal class DefaultApolloStore(
     return true
   }
 
-  override suspend fun remove(
+  override fun remove(
       cacheKey: CacheKey,
       cascade: Boolean,
   ): Boolean {
@@ -77,7 +77,7 @@ internal class DefaultApolloStore(
     }
   }
 
-  override suspend fun remove(
+  override fun remove(
       cacheKeys: List<CacheKey>,
       cascade: Boolean,
   ): Int {
@@ -105,7 +105,7 @@ internal class DefaultApolloStore(
     )
   }
 
-  override suspend fun <D : Operation.Data> readOperation(
+  override fun <D : Operation.Data> readOperation(
       operation: Operation<D>,
       customScalarAdapters: CustomScalarAdapters,
       cacheHeaders: CacheHeaders,
@@ -122,7 +122,7 @@ internal class DefaultApolloStore(
     }.toData(operation.adapter(), customScalarAdapters, variables)
   }
 
-  override suspend fun <D : Fragment.Data> readFragment(
+  override fun <D : Fragment.Data> readFragment(
       fragment: Fragment<D>,
       cacheKey: CacheKey,
       customScalarAdapters: CustomScalarAdapters,
@@ -141,15 +141,14 @@ internal class DefaultApolloStore(
     }.toData(fragment.adapter(), customScalarAdapters, variables)
   }
 
-
-  override suspend fun <R> accessCache(block: (NormalizedCache) -> R): R {
+  override fun <R> accessCache(block: (NormalizedCache) -> R): R {
     /**
      * We don't know how the cache is going to be used, assume write access
      */
     return lock.write { block(cache) }
   }
 
-  override suspend fun <D : Operation.Data> writeOperation(
+  override fun <D : Operation.Data> writeOperation(
       operation: Operation<D>,
       operationData: D,
       customScalarAdapters: CustomScalarAdapters,
@@ -165,7 +164,7 @@ internal class DefaultApolloStore(
     ).second
   }
 
-  override suspend fun <D : Fragment.Data> writeFragment(
+  override fun <D : Fragment.Data> writeFragment(
       fragment: Fragment<D>,
       cacheKey: CacheKey,
       fragmentData: D,
@@ -192,7 +191,7 @@ internal class DefaultApolloStore(
     return changedKeys
   }
 
-  suspend fun <D : Operation.Data> writeOperationWithRecords(
+  private fun <D : Operation.Data> writeOperationWithRecords(
       operation: Operation<D>,
       operationData: D,
       cacheHeaders: CacheHeaders,
@@ -218,7 +217,7 @@ internal class DefaultApolloStore(
   }
 
 
-  override suspend fun <D : Operation.Data> writeOptimisticUpdates(
+  override fun <D : Operation.Data> writeOptimisticUpdates(
       operation: Operation<D>,
       operationData: D,
       mutationId: Uuid,
@@ -252,7 +251,7 @@ internal class DefaultApolloStore(
     return changedKeys
   }
 
-  override suspend fun rollbackOptimisticUpdates(
+  override fun rollbackOptimisticUpdates(
       mutationId: Uuid,
       publish: Boolean,
   ): Set<String> {
@@ -273,7 +272,7 @@ internal class DefaultApolloStore(
     }
   }
 
-  override suspend fun dump(): Map<KClass<*>, Map<String, Record>> {
+  override fun dump(): Map<KClass<*>, Map<String, Record>> {
     return lock.read {
       cache.dump()
     }