From 01e6ca56143400196f2faf7482e475c45aa1abdb Mon Sep 17 00:00:00 2001 From: Benoit 'BoD' Lubek Date: Mon, 25 Mar 2024 11:36:50 +0100 Subject: [PATCH] [Pagination] Support nodes in Connection types (#5754) * Support nodes in Connection types * Add a reference to the Relay spec issue about nodes --- design-docs/Normalized cache pagination.md | 6 +- .../apollo3/compiler/ir/IrSchema.kt | 2 +- .../cache/normalized/api/MetadataGenerator.kt | 11 +- .../cache/normalized/api/RecordMerger.kt | 106 +++++-- tests/pagination/build.gradle.kts | 6 + .../pagination/connection/schema.graphqls | 6 +- .../connectionWithNodes/extra.graphqls | 5 + .../connectionWithNodes/operations.graphql | 13 + .../connectionWithNodes/schema.graphqls | 31 ++ ...ldsTest.kt => ConnectionPaginationTest.kt} | 2 +- .../ConnectionWithNodesPaginationTest.kt | 269 ++++++++++++++++++ 11 files changed, 427 insertions(+), 30 deletions(-) create mode 100644 tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/extra.graphqls create mode 100644 tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/operations.graphql create mode 100644 tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/schema.graphqls rename tests/pagination/src/commonTest/kotlin/{TypePolicyConnectionFieldsTest.kt => ConnectionPaginationTest.kt} (99%) create mode 100644 tests/pagination/src/commonTest/kotlin/ConnectionWithNodesPaginationTest.kt diff --git a/design-docs/Normalized cache pagination.md b/design-docs/Normalized cache pagination.md index 7dd7fd57c12..6fa93fadffc 100644 --- a/design-docs/Normalized cache pagination.md +++ b/design-docs/Normalized cache pagination.md @@ -150,8 +150,10 @@ type UserConnection { } type PageInfo { - startCursor: String! - endCursor: String! + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String } type UserEdge { diff --git a/libraries/apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/ir/IrSchema.kt b/libraries/apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/ir/IrSchema.kt index d76d36ebf95..22f2b516a41 100644 --- a/libraries/apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/ir/IrSchema.kt +++ b/libraries/apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/ir/IrSchema.kt @@ -232,7 +232,7 @@ internal fun GQLInterfaceTypeDefinition.toIr(schema: Schema, usedFields: Map { return if (typeName in schema.connectionTypes) { - setOf("edges") + setOf("edges", "pageInfo") } else { emptySet() } diff --git a/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/MetadataGenerator.kt b/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/MetadataGenerator.kt index d8d3d8c834e..d71564c705f 100644 --- a/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/MetadataGenerator.kt +++ b/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/MetadataGenerator.kt @@ -34,9 +34,14 @@ class ConnectionMetadataGenerator(private val connectionTypes: Set) : Me override fun metadataForObject(obj: Any?, context: MetadataGeneratorContext): Map { if (context.field.type.rawType().name in connectionTypes) { obj as Map - val edges = obj["edges"] as List> - val startCursor = edges.firstOrNull()?.get("cursor") as String? - val endCursor = edges.lastOrNull()?.get("cursor") as String? + val pageInfo = obj["pageInfo"] as? Map + val edges = obj["edges"] as? List> + if (edges == null && pageInfo == null) { + return emptyMap() + } + // Get start and end cursors from the PageInfo, if present in the selection. Else, get it from the first and last edges. + val startCursor = pageInfo?.get("startCursor") as String? ?: edges?.firstOrNull()?.get("cursor") as String? + val endCursor = pageInfo?.get("endCursor") as String? ?: edges?.lastOrNull()?.get("cursor") as String? return mapOf( "startCursor" to startCursor, "endCursor" to endCursor, diff --git a/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/RecordMerger.kt b/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/RecordMerger.kt index 6bfead0c18a..70ff550d216 100644 --- a/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/RecordMerger.kt +++ b/libraries/apollo-normalized-cache-api-incubating/src/commonMain/kotlin/com/apollographql/apollo3/cache/normalized/api/RecordMerger.kt @@ -95,6 +95,16 @@ class FieldRecordMerger(private val fieldMerger: FieldMerger) : RecordMerger { } } +/** + * A [RecordMerger] that merges lists in Relay style Connection types. + * + * It will merge the `edges` and `nodes` lists and update the `pageInfo` field. + * + * If the incoming data can't be merged with the existing data, it will replace the existing data. + * + * Note: although `nodes` is not a standard field in Relay, it is often used - see + * [this issue on the Relay spec](https://github.com/facebook/relay/issues/3850) that discusses this pattern. + */ @ApolloExperimental val ConnectionRecordMerger = FieldRecordMerger(ConnectionFieldMerger) @@ -111,41 +121,95 @@ private object ConnectionFieldMerger : FieldRecordMerger.FieldMerger { return if (incomingBeforeArgument == null && incomingAfterArgument == null) { // Not a pagination query incoming - } else if (existingStartCursor == null || existingEndCursor == null) { + } else if (existingStartCursor == null && existingEndCursor == null) { // Existing is empty incoming - } else if (incomingStartCursor == null || incomingEndCursor == null) { + } else if (incomingStartCursor == null && incomingEndCursor == null) { // Incoming is empty existing } else { val existingValue = existing.value as Map - val existingList = existingValue["edges"] as List<*> - val incomingList = (incoming.value as Map)["edges"] as List<*> - - val mergedList: List<*> - val newStartCursor: String - val newEndCursor: String + val existingEdges = existingValue["edges"] as? List<*> + val existingNodes = existingValue["nodes"] as? List<*> + val existingPageInfo = existingValue["pageInfo"] as? Map + val existingHasPreviousPage = existingPageInfo?.get("hasPreviousPage") as? Boolean + val existingHasNextPage = existingPageInfo?.get("hasNextPage") as? Boolean + + val incomingValue = incoming.value as Map + val incomingEdges = incomingValue["edges"] as? List<*> + val incomingNodes = incomingValue["nodes"] as? List<*> + val incomingPageInfo = incomingValue["pageInfo"] as? Map + val incomingHasPreviousPage = incomingPageInfo?.get("hasPreviousPage") as? Boolean + val incomingHasNextPage = incomingPageInfo?.get("hasNextPage") as? Boolean + + val mergedEdges: List<*>? + val mergedNodes: List<*>? + val mergedStartCursor: String? + val mergedEndCursor: String? + val mergedHasPreviousPage: Boolean? + val mergedHasNextPage: Boolean? if (incomingAfterArgument == existingEndCursor) { - mergedList = existingList + incomingList - newStartCursor = existingStartCursor - newEndCursor = incomingEndCursor + // Append to the end + mergedStartCursor = existingStartCursor + mergedEndCursor = incomingEndCursor + mergedEdges = if (existingEdges == null || incomingEdges == null) { + null + } else { + existingEdges + incomingEdges + } + mergedNodes = if (existingNodes == null || incomingNodes == null) { + null + } else { + existingNodes + incomingNodes + } + mergedHasPreviousPage = existingHasPreviousPage + mergedHasNextPage = incomingHasNextPage } else if (incomingBeforeArgument == existingStartCursor) { - mergedList = incomingList + existingList - newStartCursor = incomingStartCursor - newEndCursor = existingEndCursor + // Prepend to the start + mergedStartCursor = incomingStartCursor + mergedEndCursor = existingEndCursor + mergedEdges = if (existingEdges == null || incomingEdges == null) { + null + } else { + incomingEdges + existingEdges + } + mergedNodes = if (existingNodes == null || incomingNodes == null) { + null + } else { + incomingNodes + existingNodes + } + mergedHasPreviousPage = incomingHasPreviousPage + mergedHasNextPage = existingHasNextPage } else { // We received a list which is neither the previous nor the next page. // Handle this case by resetting the cache with this page - mergedList = incomingList - newStartCursor = incomingStartCursor - newEndCursor = incomingEndCursor + mergedStartCursor = incomingStartCursor + mergedEndCursor = incomingEndCursor + mergedEdges = incomingEdges + mergedNodes = incomingNodes + mergedHasPreviousPage = incomingHasPreviousPage + mergedHasNextPage = incomingHasNextPage } - val mergedFieldValue = existingValue.toMutableMap() - mergedFieldValue["edges"] = mergedList + val mergedPageInfo: Map? = if (existingPageInfo == null && incomingPageInfo == null) { + null + } else { + (existingPageInfo.orEmpty() + incomingPageInfo.orEmpty()).toMutableMap().also { mergedPageInfo -> + if (mergedHasNextPage != null) mergedPageInfo["hasNextPage"] = mergedHasNextPage + if (mergedHasPreviousPage != null) mergedPageInfo["hasPreviousPage"] = mergedHasPreviousPage + if (mergedStartCursor != null) mergedPageInfo["startCursor"] = mergedStartCursor + if (mergedEndCursor != null) mergedPageInfo["endCursor"] = mergedEndCursor + } + } + + val mergedValue = (existingValue + incomingValue).toMutableMap() + if (mergedEdges != null) mergedValue["edges"] = mergedEdges + if (mergedNodes != null) mergedValue["nodes"] = mergedNodes + if (mergedPageInfo != null) mergedValue["pageInfo"] = mergedPageInfo + FieldRecordMerger.FieldInfo( - value = mergedFieldValue, - metadata = mapOf("startCursor" to newStartCursor, "endCursor" to newEndCursor) + value = mergedValue, + metadata = mapOf("startCursor" to mergedStartCursor, "endCursor" to mergedEndCursor) ) } } diff --git a/tests/pagination/build.gradle.kts b/tests/pagination/build.gradle.kts index 163edf04097..221647c5cca 100644 --- a/tests/pagination/build.gradle.kts +++ b/tests/pagination/build.gradle.kts @@ -58,4 +58,10 @@ apollo { @OptIn(ApolloExperimental::class) generateDataBuilders.set(true) } + service("pagination.connectionWithNodes") { + packageName.set("pagination.connectionWithNodes") + srcDir("src/commonMain/graphql/pagination/connectionWithNodes") + @OptIn(ApolloExperimental::class) + generateDataBuilders.set(true) + } } diff --git a/tests/pagination/src/commonMain/graphql/pagination/connection/schema.graphqls b/tests/pagination/src/commonMain/graphql/pagination/connection/schema.graphqls index 5189b57b6b8..e66078a5b53 100644 --- a/tests/pagination/src/commonMain/graphql/pagination/connection/schema.graphqls +++ b/tests/pagination/src/commonMain/graphql/pagination/connection/schema.graphqls @@ -8,8 +8,10 @@ type UserConnection { } type PageInfo { - startCursor: String! - endCursor: String! + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String } type UserEdge { diff --git a/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/extra.graphqls b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/extra.graphqls new file mode 100644 index 00000000000..322124bf13e --- /dev/null +++ b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/extra.graphqls @@ -0,0 +1,5 @@ +extend schema @link(url: "https://specs.apollo.dev/kotlin_labs/v0.2", import: ["@typePolicy", "@fieldPolicy"]) + +extend type Query @typePolicy(connectionFields: "users") + +extend type User @typePolicy(keyFields: "id") diff --git a/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/operations.graphql b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/operations.graphql new file mode 100644 index 00000000000..be8856b56aa --- /dev/null +++ b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/operations.graphql @@ -0,0 +1,13 @@ +query Users($first: Int, $after: String, $last: Int, $before: String) { + users(first: $first, after: $after, last: $last, before: $before) { + pageInfo { + startCursor + endCursor + } + nodes { + id + name + email + } + } +} diff --git a/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/schema.graphqls b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/schema.graphqls new file mode 100644 index 00000000000..4863babbed4 --- /dev/null +++ b/tests/pagination/src/commonMain/graphql/pagination/connectionWithNodes/schema.graphqls @@ -0,0 +1,31 @@ +type Query { + users(first: Int = 10, after: String = null, last: Int = null, before: String = null): UserConnection! +} + +# In this schema, Connection types have a `nodes` field in addition to the `edges` field. +# This can simplify accessing the data. The GitHub API uses this pattern for example. +# See [this issue on the Relay spec](https://github.com/facebook/relay/issues/3850) that discusses this. +type UserConnection { + pageInfo: PageInfo! + edges: [UserEdge!]! + nodes: [User!]! +} + +type PageInfo { + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String +} + +type UserEdge { + cursor: String! + node: User! +} + +type User { + id: ID! + name: String! + email: String! + admin: Boolean +} diff --git a/tests/pagination/src/commonTest/kotlin/TypePolicyConnectionFieldsTest.kt b/tests/pagination/src/commonTest/kotlin/ConnectionPaginationTest.kt similarity index 99% rename from tests/pagination/src/commonTest/kotlin/TypePolicyConnectionFieldsTest.kt rename to tests/pagination/src/commonTest/kotlin/ConnectionPaginationTest.kt index 7c3a55b580f..d523574946e 100644 --- a/tests/pagination/src/commonTest/kotlin/TypePolicyConnectionFieldsTest.kt +++ b/tests/pagination/src/commonTest/kotlin/ConnectionPaginationTest.kt @@ -18,7 +18,7 @@ import pagination.connection.type.buildUserEdge import kotlin.test.Test import kotlin.test.assertEquals -class TypePolicyConnectionFieldsTest { +class ConnectionPaginationTest { @Test fun typePolicyConnectionFieldsMemoryCache() { typePolicyConnectionFields(MemoryCacheFactory()) diff --git a/tests/pagination/src/commonTest/kotlin/ConnectionWithNodesPaginationTest.kt b/tests/pagination/src/commonTest/kotlin/ConnectionWithNodesPaginationTest.kt new file mode 100644 index 00000000000..11e5e017368 --- /dev/null +++ b/tests/pagination/src/commonTest/kotlin/ConnectionWithNodesPaginationTest.kt @@ -0,0 +1,269 @@ +package pagination + +import com.apollographql.apollo3.api.Optional +import com.apollographql.apollo3.cache.normalized.ApolloStore +import com.apollographql.apollo3.cache.normalized.api.ConnectionMetadataGenerator +import com.apollographql.apollo3.cache.normalized.api.ConnectionRecordMerger +import com.apollographql.apollo3.cache.normalized.api.FieldPolicyApolloResolver +import com.apollographql.apollo3.cache.normalized.api.MemoryCacheFactory +import com.apollographql.apollo3.cache.normalized.api.NormalizedCacheFactory +import com.apollographql.apollo3.cache.normalized.api.TypePolicyCacheKeyGenerator +import com.apollographql.apollo3.cache.normalized.sql.SqlNormalizedCacheFactory +import com.apollographql.apollo3.testing.internal.runTest +import pagination.connectionWithNodes.UsersQuery +import pagination.connectionWithNodes.pagination.Pagination +import pagination.connectionWithNodes.type.buildPageInfo +import pagination.connectionWithNodes.type.buildUser +import pagination.connectionWithNodes.type.buildUserConnection +import kotlin.test.Test +import kotlin.test.assertEquals + +class ConnectionWithNodesPaginationTest { + @Test + fun typePolicyConnectionFieldsMemoryCache() { + typePolicyConnectionFields(MemoryCacheFactory()) + } + + @Test + fun typePolicyConnectionFieldsBlobSqlCache() { + typePolicyConnectionFields(SqlNormalizedCacheFactory(name = "blob", withDates = true)) + } + + @Test + fun typePolicyConnectionFieldsJsonSqlCache() { + typePolicyConnectionFields(SqlNormalizedCacheFactory(name = "json", withDates = false)) + } + + @Test + fun typePolicyConnectionFieldsChainedCache() { + typePolicyConnectionFields(MemoryCacheFactory().chain(SqlNormalizedCacheFactory(name = "json", withDates = false))) + } + + private fun typePolicyConnectionFields(cacheFactory: NormalizedCacheFactory) = runTest { + val apolloStore = ApolloStore( + normalizedCacheFactory = cacheFactory, + cacheKeyGenerator = TypePolicyCacheKeyGenerator, + metadataGenerator = ConnectionMetadataGenerator(Pagination.connectionTypes), + apolloResolver = FieldPolicyApolloResolver, + recordMerger = ConnectionRecordMerger + ) + apolloStore.clearAll() + + // First page + val query1 = UsersQuery(first = Optional.Present(2)) + val data1 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx42" + endCursor = "xx43" + } + nodes = listOf( + buildUser { + id = "42" + }, + buildUser { + id = "43" + }, + ) + } + } + apolloStore.writeOperation(query1, data1) + var dataFromStore = apolloStore.readOperation(query1) + assertEquals(data1, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + + // Page after + val query2 = UsersQuery(first = Optional.Present(2), after = Optional.Present("xx43")) + val data2 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx44" + endCursor = "xx45" + } + nodes = listOf( + buildUser { + id = "44" + }, + buildUser { + id = "45" + }, + ) + } + } + apolloStore.writeOperation(query2, data2) + dataFromStore = apolloStore.readOperation(query1) + var expectedData = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx42" + endCursor = "xx45" + } + nodes = listOf( + buildUser { + id = "42" + }, + buildUser { + id = "43" + }, + buildUser { + id = "44" + }, + buildUser { + id = "45" + }, + ) + } + } + assertEquals(expectedData, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + + // Page after + val query3 = UsersQuery(first = Optional.Present(2), after = Optional.Present("xx45")) + val data3 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx46" + endCursor = "xx47" + } + nodes = listOf( + buildUser { + id = "46" + }, + buildUser { + id = "47" + }, + ) + } + } + apolloStore.writeOperation(query3, data3) + dataFromStore = apolloStore.readOperation(query1) + expectedData = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx42" + endCursor = "xx47" + } + nodes = listOf( + buildUser { + id = "42" + }, + buildUser { + id = "43" + }, + buildUser { + id = "44" + }, + buildUser { + id = "45" + }, + buildUser { + id = "46" + }, + buildUser { + id = "47" + }, + ) + } + } + assertEquals(expectedData, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + + // Page before + val query4 = UsersQuery(last = Optional.Present(2), before = Optional.Present("xx42")) + val data4 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx40" + endCursor = "xx41" + } + nodes = listOf( + buildUser { + id = "40" + }, + buildUser { + id = "41" + }, + ) + } + } + apolloStore.writeOperation(query4, data4) + dataFromStore = apolloStore.readOperation(query1) + expectedData = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx40" + endCursor = "xx47" + } + nodes = listOf( + buildUser { + id = "40" + }, + buildUser { + id = "41" + }, + buildUser { + id = "42" + }, + buildUser { + id = "43" + }, + buildUser { + id = "44" + }, + buildUser { + id = "45" + }, + buildUser { + id = "46" + }, + buildUser { + id = "47" + }, + ) + } + } + assertEquals(expectedData, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + + // Non-contiguous page (should reset) + val query5 = UsersQuery(first = Optional.Present(2), after = Optional.Present("xx50")) + val data5 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = "xx50" + endCursor = "xx51" + } + nodes = listOf( + buildUser { + id = "50" + }, + buildUser { + id = "51" + }, + ) + } + } + apolloStore.writeOperation(query5, data5) + dataFromStore = apolloStore.readOperation(query1) + assertEquals(data5, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + + // Empty page (should keep previous result) + val query6 = UsersQuery(first = Optional.Present(2), after = Optional.Present("xx51")) + val data6 = UsersQuery.Data { + users = buildUserConnection { + pageInfo = buildPageInfo { + startCursor = null + endCursor = null + } + nodes = emptyList() + + } + } + apolloStore.writeOperation(query6, data6) + dataFromStore = apolloStore.readOperation(query1) + assertEquals(data5, dataFromStore) + assertChainedCachesAreEqual(apolloStore) + } +} +