From d22d48aa3a5d51d94752fa7b84ab64ad1d99677b Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 3 Jan 2024 16:45:20 +1300 Subject: [PATCH] Fix source input edge case --- .../hydration/batch/NadelNewBatchHydrator.kt | 16 +- .../hooks/new-batching-absent-source-input.kt | 57 ++++ .../hooks/new-batching-no-source-inputs.kt | 37 +++ .../new-batching-absent-source-input.yml | 215 +++++++++++++++ .../new-batching-no-source-inputs.yml | 248 ++++++++++++++++++ .../new-batching-null-source-input-object.yml | 104 ++++++++ 6 files changed, 673 insertions(+), 4 deletions(-) create mode 100644 test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-absent-source-input.kt create mode 100644 test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-no-source-inputs.kt create mode 100644 test/src/test/resources/fixtures/new hydration/new batching/new-batching-absent-source-input.yml create mode 100644 test/src/test/resources/fixtures/new hydration/new batching/new-batching-no-source-inputs.yml create mode 100644 test/src/test/resources/fixtures/new hydration/new batching/new-batching-null-source-input-object.yml diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelNewBatchHydrator.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelNewBatchHydrator.kt index ac9066b3f..9c736139f 100644 --- a/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelNewBatchHydrator.kt +++ b/lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelNewBatchHydrator.kt @@ -258,8 +258,12 @@ internal class NadelNewBatchHydrator( extractNode(sourceId, instruction) } } else { - val (sourceId, instruction) = sourceIdsPairedWithInstruction.single() - extractNode(sourceId, instruction) + if (sourceIdsPairedWithInstruction.isEmpty()) { + JsonNode.Null + } else { + val (sourceId, instruction) = sourceIdsPairedWithInstruction.single() + extractNode(sourceId, instruction) + } } } else { if (sourceIdsPairedWithInstruction == null) { @@ -272,8 +276,12 @@ internal class NadelNewBatchHydrator( }, ) } else { - val (sourceId, instruction) = sourceIdsPairedWithInstruction.single() - extractNode(sourceId, instruction) + if (sourceIdsPairedWithInstruction.isEmpty()) { + JsonNode.Null + } else { + val (sourceId, instruction) = sourceIdsPairedWithInstruction.single() + extractNode(sourceId, instruction) + } } } } diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-absent-source-input.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-absent-source-input.kt new file mode 100644 index 000000000..0a8efbce5 --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-absent-source-input.kt @@ -0,0 +1,57 @@ +package graphql.nadel.tests.hooks + +import graphql.nadel.Nadel +import graphql.nadel.NadelExecutionHints +import graphql.nadel.engine.blueprint.NadelGenericHydrationInstruction +import graphql.nadel.engine.transform.artificial.NadelAliasHelper +import graphql.nadel.engine.transform.result.json.JsonNode +import graphql.nadel.hooks.NadelExecutionHooks +import graphql.nadel.tests.EngineTestHook +import graphql.nadel.tests.UseHook + +@UseHook +class `new-batching-absent-source-input` : EngineTestHook { + override fun makeExecutionHints(builder: NadelExecutionHints.Builder): NadelExecutionHints.Builder { + return super.makeExecutionHints(builder) + .newBatchHydrationGrouping { true } + } + + override fun makeNadel(builder: Nadel.Builder): Nadel.Builder { + return super.makeNadel(builder) + .executionHooks( + object : NadelExecutionHooks { + override fun getHydrationInstruction( + instructions: List, + sourceId: JsonNode, + userContext: Any?, + ): T { + val type = (sourceId.value as String).substringBefore("/") + + return instructions + .first { + it.actorService.name.startsWith(type, ignoreCase = true) + } + } + + override fun getHydrationInstruction( + instructions: List, + parentNode: JsonNode, + aliasHelper: NadelAliasHelper, + userContext: Any?, + ): T? { + val nodeString = parentNode.value.toString() + + return when { + nodeString.contains("comment/") -> instructions.single { + it.actorFieldDef.name.contains("comment") + } + nodeString.contains("issue/") -> instructions.single { + it.actorFieldDef.name.contains("issue") + } + else -> null + } + } + }, + ) + } +} diff --git a/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-no-source-inputs.kt b/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-no-source-inputs.kt new file mode 100644 index 000000000..cf1115a67 --- /dev/null +++ b/test/src/test/kotlin/graphql/nadel/tests/hooks/new-batching-no-source-inputs.kt @@ -0,0 +1,37 @@ +package graphql.nadel.tests.hooks + +import graphql.nadel.Nadel +import graphql.nadel.NadelExecutionHints +import graphql.nadel.engine.blueprint.NadelGenericHydrationInstruction +import graphql.nadel.engine.transform.result.json.JsonNode +import graphql.nadel.hooks.NadelExecutionHooks +import graphql.nadel.tests.EngineTestHook +import graphql.nadel.tests.UseHook + +@UseHook +class `new-batching-no-source-inputs` : EngineTestHook { + override fun makeExecutionHints(builder: NadelExecutionHints.Builder): NadelExecutionHints.Builder { + return super.makeExecutionHints(builder) + .newBatchHydrationGrouping { true } + } + + override fun makeNadel(builder: Nadel.Builder): Nadel.Builder { + return super.makeNadel(builder) + .executionHooks( + object : NadelExecutionHooks { + override fun getHydrationInstruction( + instructions: List, + sourceId: JsonNode, + userContext: Any?, + ): T { + val type = (sourceId.value as String).substringBefore("/") + + return instructions + .first { + it.actorService.name.startsWith(type, ignoreCase = true) + } + } + }, + ) + } +} diff --git a/test/src/test/resources/fixtures/new hydration/new batching/new-batching-absent-source-input.yml b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-absent-source-input.yml new file mode 100644 index 000000000..c4f817b83 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-absent-source-input.yml @@ -0,0 +1,215 @@ +name: "new batching absent source input" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [Activity] + } + union ActivityContent = Issue | Comment + type Activity { + id: ID! + contentId: ID + content: ActivityContent + @hydrated( + service: "comments" + field: "commentsByIds" + arguments: [ + {name: "ids" value: "$source.contentId"} + ] + ) + @hydrated( + service: "issues" + field: "issuesByIds" + arguments: [ + {name: "ids" value: "$source.contentId"} + ] + ) + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } + # language=GraphQL + comments: | + type Query { + commentsByIds(ids: [ID!]!): [Comment!] + } + type Comment { + id: ID! + content: String + } +# language=GraphQL +underlyingSchema: + # language=GraphQL + activity: | + type Query { + activity: [Activity] + } + type Activity { + id: ID! + contentId: ID + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } + # language=GraphQL + comments: | + type Query { + commentsByIds(ids: [ID!]!): [Comment!] + } + type Comment { + id: ID! + content: String + } +# language=GraphQL +query: | + { + activity { + content { + __typename + ... on Issue { + id + title + } + ... on Comment { + id + content + } + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + __typename__batch_hydration__content: __typename + batch_hydration__content__contentId: contentId + batch_hydration__content__contentId: contentId + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "__typename__batch_hydration__content": "Activity" + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentId": "" + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentId": "comment/9001" + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentId": "issue/1234" + } + ] + }, + "extensions": {} + } + - serviceName: "issues" + request: + # language=GraphQL + query: | + { + issuesByIds(ids: ["issue/1234"]) { + __typename + id + batch_hydration__content__id: id + title + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "issuesByIds": [ + { + "__typename": "Issue", + "id": "issue/1234", + "batch_hydration__content__id": "issue/1234", + "title": "One Two Three Four" + } + ] + }, + "extensions": {} + } + - serviceName: "comments" + request: + # language=GraphQL + query: | + { + commentsByIds(ids: ["comment/9001"]) { + __typename + content + id + batch_hydration__content__id: id + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "commentsByIds": [ + { + "__typename": "Comment", + "id": "comment/9001", + "batch_hydration__content__id": "comment/9001", + "content": "It's over 9000" + } + ] + }, + "extensions": {} + } +# language=JSON +response: |- + { + "data": { + "activity": [ + { + "content": null + }, + { + "content": null + }, + { + "content": { + "__typename": "Comment", + "id": "comment/9001", + "content": "It's over 9000" + } + }, + { + "content": { + "__typename": "Issue", + "id": "issue/1234", + "title": "One Two Three Four" + } + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/new batching/new-batching-no-source-inputs.yml b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-no-source-inputs.yml new file mode 100644 index 000000000..03a004eb0 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-no-source-inputs.yml @@ -0,0 +1,248 @@ +name: "new batching no source inputs" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [Activity] + } + union ActivityContent = Issue | Comment + type Activity { + id: ID! + contentIds: [ID!]! + content: [ActivityContent] + @hydrated( + service: "comments" + field: "commentsByIds" + arguments: [ + {name: "ids" value: "$source.contentIds"} + ] + ) + @hydrated( + service: "issues" + field: "issuesByIds" + arguments: [ + {name: "ids" value: "$source.contentIds"} + ] + ) + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } + # language=GraphQL + comments: | + type Query { + commentsByIds(ids: [ID!]!): [Comment!] + } + type Comment { + id: ID! + content: String + } +# language=GraphQL +underlyingSchema: + # language=GraphQL + activity: | + type Query { + activity: [Activity] + } + type Activity { + id: ID! + contentIds: [ID!]! + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } + # language=GraphQL + comments: | + type Query { + commentsByIds(ids: [ID!]!): [Comment!] + } + type Comment { + id: ID! + content: String + } +# language=GraphQL +query: | + { + activity { + content { + __typename + ... on Issue { + id + title + } + ... on Comment { + id + content + } + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + __typename__batch_hydration__content: __typename + batch_hydration__content__contentIds: contentIds + batch_hydration__content__contentIds: contentIds + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentIds": [] + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentIds": [] + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentIds": [ + "issue/7496", + "comment/9001" + ] + }, + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__contentIds": [ + "issue/1234", + "comment/1234" + ] + } + ] + }, + "extensions": {} + } + - serviceName: "issues" + request: + # language=GraphQL + query: | + { + issuesByIds(ids: ["issue/7496", "issue/1234"]) { + __typename + id + batch_hydration__content__id: id + title + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "issuesByIds": [ + { + "__typename": "Issue", + "id": "issue/7496", + "batch_hydration__content__id": "issue/7496", + "title": "Seven Four Nine Six" + }, + { + "__typename": "Issue", + "id": "issue/1234", + "batch_hydration__content__id": "issue/1234", + "title": "One Two Three Four" + } + ] + }, + "extensions": {} + } + - serviceName: "comments" + request: + # language=GraphQL + query: | + { + commentsByIds(ids: ["comment/9001", "comment/1234"]) { + __typename + content + id + batch_hydration__content__id: id + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "commentsByIds": [ + { + "__typename": "Comment", + "id": "comment/1234", + "batch_hydration__content__id": "comment/1234", + "content": "One Two Three Four" + }, + { + "__typename": "Comment", + "id": "comment/9001", + "batch_hydration__content__id": "comment/9001", + "content": "It's over 9000" + } + ] + }, + "extensions": {} + } +# language=JSON +response: |- + { + "data": { + "activity": [ + { + "content": [] + }, + { + "content": [] + }, + { + "content": [ + { + "__typename": "Issue", + "id": "issue/7496", + "title": "Seven Four Nine Six" + }, + { + "__typename": "Comment", + "id": "comment/9001", + "content": "It's over 9000" + } + ] + }, + { + "content": [ + { + "__typename": "Issue", + "id": "issue/1234", + "title": "One Two Three Four" + }, + { + "__typename": "Comment", + "id": "comment/1234", + "content": "One Two Three Four" + } + ] + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/new batching/new-batching-null-source-input-object.yml b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-null-source-input-object.yml new file mode 100644 index 000000000..b447e3e28 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-null-source-input-object.yml @@ -0,0 +1,104 @@ +name: "new batching null source input object" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [Activity] + } + type Activity { + id: ID! + content: Issue + @hydrated( + service: "issues" + field: "issuesByIds" + arguments: [ + {name: "ids" value: "$source.reference.issueId"} + ] + ) + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } +# language=GraphQL +underlyingSchema: + # language=GraphQL + activity: | + type Query { + activity: [Activity] + } + type Activity { + id: ID! + reference: ActivityReference + } + type ActivityReference { + issueId: ID + } + # language=GraphQL + issues: | + type Query { + issuesByIds(ids: [ID!]!): [Issue!] + } + type Issue { + id: ID! + title: String + } +# language=GraphQL +query: | + { + activity { + content { + __typename + ... on Issue { + id + title + } + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + __typename__batch_hydration__content: __typename + batch_hydration__content__reference: reference { + issueId + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "__typename__batch_hydration__content": "Activity", + "batch_hydration__content__reference": null + } + ] + }, + "extensions": {} + } +# language=JSON +response: |- + { + "data": { + "activity": [ + { + "content": null + } + ] + }, + "errors": [] + }