Pagination: use "field key" instead of "field name" (apollographql#5898)

* Use "field key" instead of "field name"

* Add a few cache related terms to the glossary

* Apply suggestions from code review

Co-authored-by: Martin Bonnin <[email protected]>

---------

Co-authored-by: Martin Bonnin <[email protected]>

Author: BoD

design-docs/Glossary.md

@@ -1,9 +1,6 @@
-# Codegen Glossary
-
-## Glossary
-
-A small Glossary of the terms used during codegen. The [GraphQL Spec](https://spec.graphql.org/draft/) does a nice job of defining the common terms like `Field`, `SelectionSet`, etc... so I'm not adding these terms here. But it misses some concepts that we bumped into during codegen and that I'm trying to clarify here.
+# Glossary
 
+The [GraphQL Spec](https://spec.graphql.org/draft/) does a nice job of defining common terms like `Field`, `SelectionSet`, etc. but here are a few other concepts that the library deals with, and their definition.
 
 ### Response shape
 
@@ -105,3 +102,26 @@ Example:
 ### Polymorphic field
 
 A field that can take several shapes
+
+### Record
+
+A shallow map of a response object. Nested objects in the map values are replaced by a cache reference to another Record.  
+
+### Cache key
+
+A unique identifier for a Record.
+By default it is the path formed by all the field keys from the root of the query to the field referencing the Record.
+To avoid duplication the Cache key can also be computed from the Record contents, usually using its key fields.
+
+### Field key
+
+A key that uniquely identifies a field within a Record. By default composed of the field name and the arguments passed to it.
+
+### Key fields
+
+Fields that are used to compute a Cache key for an object.
+
+### Pagination arguments
+
+Field arguments that control pagination, e.g. `first`, `after`, etc. They should be omitted when computing a field key so different pages can be merged into the same field.
+

design-docs/Normalized cache overview.md

@@ -93,7 +93,7 @@ An example:
 
 <table>
 <tr>
-<th>Key</th>
+<th>Cache key</th>
 <th>Record</th>
 </tr>
 
@@ -217,7 +217,7 @@ An instance is created when building the `ApolloClient` and held by the `ApolloC
 
 ## Record merging
 
-When a `Record` is stored in the cache, it is _merged_ with the existing one at the same key (if any):
+When a `Record` is stored in the cache, it is _merged_ with the existing one at the same cache key (if any):
 - existing fields are replaced with the new value
 - new fields are added
 

design-docs/Normalized cache pagination.md

@@ -210,7 +210,7 @@ If your schema uses a different pagination style, you can still use the paginati
 
 #### Pagination arguments
 
-The `@fieldPolicy` directive has a `paginationArgs` argument that can be used to specify the arguments that should be omitted from the field name.
+The `@fieldPolicy` directive has a `paginationArgs` argument that can be used to specify the arguments that should be omitted from the field key.
 
 Going back to the example above with `usersPage`:
 
@@ -221,7 +221,7 @@ extend type Query
 ```
 
 > [!NOTE]
-> This can also be done programmatically by configuring the `ApolloStore` with a `FieldNameGenerator` implementation.
+> This can also be done programmatically by configuring the `ApolloStore` with a `FieldKeyGenerator` implementation.
 
 With that in place, after fetching the first page, the cache will look like this:
 
@@ -231,7 +231,7 @@ With that in place, after fetching the first page, the cache will look like this
 | user:1     | id: 1, name: John Smith                                 |
 | user:2     | id: 2, name: Jane Doe                                   |
 
-The field name no longer includes the `page` argument, which means watching `UsersPage(page = 1)` or any page will observe the same list.
+The field key no longer includes the `page` argument, which means watching `UsersPage(page = 1)` or any page will observe the same list.
 
 Here's what happens when fetching the second page:
 
@@ -245,7 +245,7 @@ Here's what happens when fetching the second page:
 
 The field containing the first page was overwritten by the second page.
 
-This is because the field name is now the same for all pages and the default merging strategy is to overwrite existing fields with the new value.
+This is because the field key is now the same for all pages and the default merging strategy is to overwrite existing fields with the new value.
 
 #### Record merging
 

intellij-plugin/src/main/kotlin/com/apollographql/ijplugin/normalizedcache/NormalizedCache.kt

@@ -10,7 +10,7 @@ data class NormalizedCache(
   )
 
   data class Field(
-      val name: String,
+      val key: String,
       val value: FieldValue,
   )
 

intellij-plugin/src/main/kotlin/com/apollographql/ijplugin/normalizedcache/provider/DatabaseNormalizedCacheProvider.kt

@@ -39,8 +39,8 @@ class DatabaseNormalizedCacheProvider : NormalizedCacheProvider<File> {
           apolloRecords.map { (key, apolloRecord) ->
             NormalizedCache.Record(
                 key = key,
-                fields = apolloRecord.map { (fieldName, fieldValue) ->
-                  Field(fieldName, fieldValue.toFieldValue())
+                fields = apolloRecord.map { (fieldKey, fieldValue) ->
+                  Field(fieldKey, fieldValue.toFieldValue())
                 },
                 sizeInBytes = apolloRecord.sizeInBytes
             )

intellij-plugin/src/main/kotlin/com/apollographql/ijplugin/normalizedcache/ui/FieldTreeTable.kt

@@ -40,11 +40,14 @@ class FieldTreeTable(selectRecord: (String) -> Unit) : JBTreeTable(FieldTreeTabl
               is NormalizedCache.FieldValue.StringValue -> append("\"${v.value}\"")
               is NormalizedCache.FieldValue.NumberValue -> append(v.value.toString())
               is NormalizedCache.FieldValue.BooleanValue -> append(v.value.toString())
-              is NormalizedCache.FieldValue.ListValue -> append(when (val size = v.value.size) {
-                0 -> ApolloBundle.message("normalizedCacheViewer.fields.list.empty")
-                1 -> ApolloBundle.message("normalizedCacheViewer.fields.list.single")
-                else -> ApolloBundle.message("normalizedCacheViewer.fields.list.multiple", size)
-              }, SimpleTextAttributes.GRAY_ITALIC_ATTRIBUTES)
+              is NormalizedCache.FieldValue.ListValue -> append(
+                  when (val size = v.value.size) {
+                    0 -> ApolloBundle.message("normalizedCacheViewer.fields.list.empty")
+                    1 -> ApolloBundle.message("normalizedCacheViewer.fields.list.single")
+                    else -> ApolloBundle.message("normalizedCacheViewer.fields.list.multiple", size)
+                  },
+                  SimpleTextAttributes.GRAY_ITALIC_ATTRIBUTES
+              )
 
               is NormalizedCache.FieldValue.CompositeValue -> append("{...}", SimpleTextAttributes.GRAY_ITALIC_ATTRIBUTES)
               NormalizedCache.FieldValue.Null -> append("null")

intellij-plugin/src/main/kotlin/com/apollographql/ijplugin/normalizedcache/ui/FieldTreeTableModel.kt

@@ -14,7 +14,8 @@ class FieldTreeTableModel : ListTreeTableModel(
           override fun getColumnClass() = TreeTableModel::class.java
           override fun valueOf(item: Unit) = Unit
         },
-        object : ColumnInfo<NormalizedCacheFieldTreeNode, NormalizedCache.Field>(ApolloBundle.message("normalizedCacheViewer.fields.column.value")) {
+        object :
+          ColumnInfo<NormalizedCacheFieldTreeNode, NormalizedCache.Field>(ApolloBundle.message("normalizedCacheViewer.fields.column.value")) {
           override fun getColumnClass() = NormalizedCache.Field::class.java
           override fun valueOf(item: NormalizedCacheFieldTreeNode) = item.field
         },
@@ -42,7 +43,7 @@ class FieldTreeTableModel : ListTreeTableModel(
 
   class NormalizedCacheFieldTreeNode(val field: NormalizedCache.Field) : DefaultMutableTreeNode() {
     init {
-      userObject = field.name
+      userObject = field.key
     }
   }
 }

intellij-plugin/src/main/resources/messages/ApolloBundle.properties

@@ -191,7 +191,7 @@ errorReport.actionText=Open GitHub Issue
 toolwindow.stripe.NormalizedCacheViewer=Apollo Normalized Cache
 normalizedCacheViewer.newTab=New Tab
 normalizedCacheViewer.tabName.empty=Empty
-normalizedCacheViewer.fields.column.key=Key
+normalizedCacheViewer.fields.column.key=Field Key
 normalizedCacheViewer.fields.column.value=Value
 normalizedCacheViewer.toolbar.expandAll=Expand all keys
 normalizedCacheViewer.toolbar.collapseAll=Collapse all keys
@@ -201,7 +201,7 @@ normalizedCacheViewer.toolbar.refresh=Refresh
 normalizedCacheViewer.empty.message=Open or drag and drop a normalized cache .db file.
 normalizedCacheViewer.empty.openFile=Open file...
 normalizedCacheViewer.empty.pullFromDevice=Pull from device
-normalizedCacheViewer.records.table.key=Key
+normalizedCacheViewer.records.table.key=Cache Key
 normalizedCacheViewer.records.table.size=Size
 normalizedCacheViewer.fields.list.empty=[empty]
 normalizedCacheViewer.fields.list.single=[1 item]

libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/api/CompiledGraphQL.kt

@@ -68,7 +68,7 @@ class CompiledField internal constructor(
 
     val value = argument.value.getOrThrow()
     return if (value is CompiledVariable) {
-       if (variables.valueMap.containsKey(value.name)) {
+      if (variables.valueMap.containsKey(value.name)) {
         Optional.present(variables.valueMap[value.name])
       } else {
         // this argument has a variable value that is absent
@@ -100,7 +100,7 @@ class CompiledField internal constructor(
   /**
    * Returns a String containing the name of this field as well as encoded arguments. For an example:
    * `hero({"episode": "Jedi"})`
-   * This is mostly used internally to compute records.
+   * This is mostly used internally to compute field keys / cache keys.
    *
    * ## Note1:
    * The argument defaultValues are not added to the name. If the schema changes from:

libraries/apollo-api/src/commonMain/kotlin/com/apollographql/apollo3/exception/Exceptions.kt

@@ -17,12 +17,12 @@ sealed class ApolloException(message: String? = null, cause: Throwable? = null)
 /**
  * A generic exception used when there is no additional context besides the message.
  */
-class DefaultApolloException(message: String? = null, cause: Throwable? = null): ApolloException(message, cause)
+class DefaultApolloException(message: String? = null, cause: Throwable? = null) : ApolloException(message, cause)
 
 /**
  * No data was found
  */
-class NoDataException(cause: Throwable?): ApolloException("No data was found", cause)
+class NoDataException(cause: Throwable?) : ApolloException("No data was found", cause)
 
 /**
  * An I/O error happened: socket closed, DNS issue, TLS problem, file not found, etc...
@@ -118,7 +118,7 @@ class JsonDataException(message: String) : ApolloException(message)
  *
  * Due to the way the parsers work, it is not possible to distinguish between both cases.
  */
-class NullOrMissingField(message: String): ApolloException(message)
+class NullOrMissingField(message: String) : ApolloException(message)
 
 /**
  * The response could not be parsed because of an I/O exception.
@@ -132,8 +132,8 @@ class NullOrMissingField(message: String): ApolloException(message)
 @Deprecated("ApolloParseException was only used for I/O exceptions and is now mapped to ApolloNetworkException.")
 class ApolloParseException(message: String? = null, cause: Throwable? = null) : ApolloException(message = message, cause = cause)
 
-class ApolloGraphQLException(val error: Error): ApolloException("GraphQL error: '${error.message}'") {
-  constructor(errors: List<Error>): this(errors.first())
+class ApolloGraphQLException(val error: Error) : ApolloException("GraphQL error: '${error.message}'") {
+  constructor(errors: List<Error>) : this(errors.first())
 
   @Deprecated("Use error instead", level = DeprecationLevel.ERROR)
   val errors: List<Error> = listOf(error)
@@ -143,10 +143,17 @@ class ApolloGraphQLException(val error: Error): ApolloException("GraphQL error:
  * An object/field was missing in the cache
  * If [fieldName] is null, it means a reference to an object could not be resolved
  */
-
 class CacheMissException @ApolloInternal constructor(
+    /**
+     * The cache key to the missing object, or to the parent of the missing field if [fieldName] is not null.
+     */
     val key: String,
+
+    /**
+     * The field key that was missing. If null, it means the object referenced by [key] was missing.
+     */
     val fieldName: String? = null,
+
     stale: Boolean = false,
 ) : ApolloException(message = message(key, fieldName, stale)) {
 
@@ -156,14 +163,14 @@ class CacheMissException @ApolloInternal constructor(
   constructor(key: String, fieldName: String?) : this(key, fieldName, false)
 
   companion object {
-    internal fun message(key: String?, fieldName: String?, stale: Boolean): String {
-      return if (fieldName == null) {
-        "Object '$key' not found"
+    internal fun message(cacheKey: String?, fieldKey: String?, stale: Boolean): String {
+      return if (fieldKey == null) {
+        "Object '$cacheKey' not found"
       } else {
         if (stale) {
-          "Field '$fieldName' on object '$key' is stale"
+          "Field '$fieldKey' on object '$cacheKey' is stale"
         } else {
-          "Object '$key' has no field named '$fieldName'"
+          "Object '$cacheKey' has no field named '$fieldKey'"
         }
       }
     }

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

@@ -65,7 +65,7 @@ interface CacheResolver {
    * @param variables the variables of the current operation
    * @param parent the parent object as a map. It can contain the same values as [Record]. Especially, nested objects will be represented
    * by [CacheKey]
-   * @param parentId the id of the parent. Mainly used for debugging
+   * @param parentId the key of the parent. Mainly used for debugging
    *
    * @return a value that can go in a [Record]. No type checking is done. It is the responsibility of implementations to return the correct
    * type
@@ -83,10 +83,10 @@ class ResolverContext(
     val field: CompiledField,
     val variables: Executable.Variables,
     val parent: Map<String, @JvmSuppressWildcards Any?>,
-    val parentId: String,
+    val parentKey: String,
     val parentType: String,
     val cacheHeaders: CacheHeaders,
-    val fieldNameGenerator: FieldNameGenerator,
+    val fieldKeyGenerator: FieldKeyGenerator,
 )
 
 /**
@@ -112,12 +112,12 @@ object DefaultCacheResolver : CacheResolver {
       parent: Map<String, @JvmSuppressWildcards Any?>,
       parentId: String,
   ): Any? {
-    val name = field.nameWithArguments(variables)
-    if (!parent.containsKey(name)) {
-      throw CacheMissException(parentId, name)
+    val fieldKey = field.nameWithArguments(variables)
+    if (!parent.containsKey(fieldKey)) {
+      throw CacheMissException(parentId, fieldKey)
     }
 
-    return parent[name]
+    return parent[fieldKey]
   }
 }
 
@@ -126,12 +126,12 @@ object DefaultCacheResolver : CacheResolver {
  */
 object DefaultApolloResolver : ApolloResolver {
   override fun resolveField(context: ResolverContext): Any? {
-    val name = context.fieldNameGenerator.getFieldName(FieldNameContext(context.parentType, context.field, context.variables))
-    if (!context.parent.containsKey(name)) {
-      throw CacheMissException(context.parentId, name)
+    val fieldKey = context.fieldKeyGenerator.getFieldKey(FieldKeyContext(context.parentType, context.field, context.variables))
+    if (!context.parent.containsKey(fieldKey)) {
+      throw CacheMissException(context.parentKey, fieldKey)
     }
 
-    return context.parent[name]
+    return context.parent[fieldKey]
   }
 }
 
@@ -142,31 +142,29 @@ object DefaultApolloResolver : ApolloResolver {
 class ReceiveDateApolloResolver(private val maxAge: Int) : ApolloResolver {
 
   override fun resolveField(context: ResolverContext): Any? {
-    val field = context.field
     val parent = context.parent
-    val variables = context.variables
-    val parentId = context.parentId
+    val parentKey = context.parentKey
 
-    val name = field.nameWithArguments(variables)
-    if (!parent.containsKey(name)) {
-      throw CacheMissException(parentId, name)
+    val fieldKey = context.fieldKeyGenerator.getFieldKey(FieldKeyContext(context.parentType, context.field, context.variables))
+    if (!parent.containsKey(fieldKey)) {
+      throw CacheMissException(parentKey, fieldKey)
     }
 
     if (parent is Record) {
-      val lastUpdated = parent.dates?.get(name)
+      val lastUpdated = parent.dates?.get(fieldKey)
       if (lastUpdated != null) {
         val maxStale = context.cacheHeaders.headerValue(ApolloCacheHeaders.MAX_STALE)?.toLongOrNull() ?: 0L
         if (maxStale < Long.MAX_VALUE) {
           val age = currentTimeMillis() / 1000 - lastUpdated
           if (maxAge + maxStale - age < 0) {
-            throw CacheMissException(parentId, name, true)
+            throw CacheMissException(parentKey, fieldKey, true)
           }
 
         }
       }
     }
 
-    return parent[name]
+    return parent[fieldKey]
   }
 }
 
@@ -184,21 +182,21 @@ class ExpireDateCacheResolver : CacheResolver {
       parent: Map<String, @JvmSuppressWildcards Any?>,
       parentId: String,
   ): Any? {
-    val name = field.nameWithArguments(variables)
-    if (!parent.containsKey(name)) {
-      throw CacheMissException(parentId, name)
+    val fieldKey = field.nameWithArguments(variables)
+    if (!parent.containsKey(fieldKey)) {
+      throw CacheMissException(parentId, fieldKey)
     }
 
     if (parent is Record) {
-      val expires = parent.dates?.get(name)
+      val expires = parent.dates?.get(fieldKey)
       if (expires != null) {
         if (currentTimeMillis() / 1000 - expires >= 0) {
-          throw CacheMissException(parentId, name, true)
+          throw CacheMissException(parentId, fieldKey, true)
         }
       }
     }
 
-    return parent[name]
+    return parent[fieldKey]
   }
 }