From 5efbf08cb79d9ed14c0838c575f888d61eae9b3f Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 21 Dec 2023 08:43:15 +1300 Subject: [PATCH 1/4] Fix bug where no instructions are found for a given object Usually happens on an abstract field which may be backed by a hydration depending on the object implementations --- .../hydration/batch/NadelNewBatchHydrator.kt | 22 +++++++++++-------- ...conditional-hydration-in-abstract-type.yml | 1 - 2 files changed, 13 insertions(+), 10 deletions(-) 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 a62a5c508..ac9066b3f 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 @@ -373,7 +373,7 @@ internal class NadelNewBatchHydrator( sourceObjects: List, ): List { return sourceObjects - .map { sourceObject -> + .mapNotNull { sourceObject -> val instructions = instructionsByObjectTypeNames.getInstructionsForNode( executionBlueprint = executionBlueprint, service = sourceFieldService, @@ -381,15 +381,19 @@ internal class NadelNewBatchHydrator( parentNode = sourceObject, ) - val sourceIdsPairedWithInstructions = getInstructionParingForSourceIds( - sourceObject = sourceObject, - instructions = instructions, - ) + if (instructions.isEmpty()) { + null + } else { + val sourceIdsPairedWithInstructions = getInstructionParingForSourceIds( + sourceObject = sourceObject, + instructions = instructions, + ) - SourceObjectMetadata( - sourceObject, - sourceIdsPairedWithInstructions, - ) + SourceObjectMetadata( + sourceObject, + sourceIdsPairedWithInstructions, + ) + } } } diff --git a/test/src/test/resources/fixtures/new hydration/new batching/new-batching-conditional-hydration-in-abstract-type.yml b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-conditional-hydration-in-abstract-type.yml index 93f714610..69a9bc30e 100644 --- a/test/src/test/resources/fixtures/new hydration/new batching/new-batching-conditional-hydration-in-abstract-type.yml +++ b/test/src/test/resources/fixtures/new hydration/new batching/new-batching-conditional-hydration-in-abstract-type.yml @@ -51,7 +51,6 @@ overallSchema: } # language=GraphQL underlyingSchema: - # language=GraphQL monolith: | type Query { activity: [IActivity] From 5679486796b20a1e08892327912c1c94b5a94c73 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 21 Dec 2023 08:49:53 +1300 Subject: [PATCH 2/4] Add tests --- ...he-result-are-backed-a-batch-hydration.yml | 130 ++++++++++++ ...s-in-the-result-are-backed-a-hydration.yml | 129 ++++++++++++ ...he-result-are-backed-a-batch-hydration.yml | 176 +++++++++++++++++ ...s-in-the-result-are-backed-a-hydration.yml | 187 ++++++++++++++++++ 4 files changed, 622 insertions(+) create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-batch-hydration.yml create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-hydration.yml create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-batch-hydration.yml create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-hydration.yml diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-batch-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-batch-hydration.yml new file mode 100644 index 000000000..1bff02a48 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-batch-hydration.yml @@ -0,0 +1,130 @@ +name: "new no object fields in the result are backed a batch hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + user: User + @hydrated( + service: "users" + field: "usersByIds" + arguments: [{name: "ids" value: "$source.userId"}] + inputIdentifiedBy: [{sourceId: "userId" resultId: "id"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on Activity { + __typename__batch_hydration__user: __typename + batch_hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-hydration.yml new file mode 100644 index 000000000..be49369ac --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-no-object-fields-in-the-result-are-backed-a-hydration.yml @@ -0,0 +1,129 @@ +name: "new no object fields in the result are backed a hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + user: User + @hydrated( + service: "users" + field: "userById" + arguments: [{name: "id" value: "$source.userId"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on Activity { + __typename__hydration__user: __typename + hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-batch-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-batch-hydration.yml new file mode 100644 index 000000000..70aa50423 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-batch-hydration.yml @@ -0,0 +1,176 @@ +name: "new some object fields in the result are backed a batch hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + user: User + @hydrated( + service: "users" + field: "usersByIds" + arguments: [{name: "ids" value: "$source.userId"}] + inputIdentifiedBy: [{sourceId: "userId" resultId: "id"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on Activity { + __typename__batch_hydration__user: __typename + batch_hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "__typename__batch_hydration__user": "Activity", + "batch_hydration__user__userId": "user-100" + }, + { + "__typename__batch_hydration__user": "Activity", + "batch_hydration__user__userId": "user-20" + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + usersByIds(ids: ["user-100", "user-20"]) { + batch_hydration__user__id: id + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "usersByIds": [ + { + "batch_hydration__user__id": "user-100", + "name": "Spaces" + }, + { + "batch_hydration__user__id": "user-20", + "name": "Newmarket" + } + ] + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Spaces" + } + }, + { + "user": { + "name": "Newmarket" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-hydration.yml new file mode 100644 index 000000000..92a139730 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-object-fields-in-the-result-are-backed-a-hydration.yml @@ -0,0 +1,187 @@ +name: "new some object fields in the result are backed a hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + user: User + @hydrated( + service: "users" + field: "userById" + arguments: [{name: "id" value: "$source.userId"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on Activity { + __typename__hydration__user: __typename + hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "__typename__hydration__user": "Activity", + "hydration__user__userId": "user-100" + }, + { + "__typename__hydration__user": "Activity", + "hydration__user__userId": "user-20" + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + userById(id: "user-100") { + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "userById": { + "name": "Hello" + } + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + userById(id: "user-20") { + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "userById": { + "name": "World" + } + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Hello" + } + }, + { + "user": { + "name": "World" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } From 9ccf8a4bf5e4797907e77cafd055120fe43dba1d Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 21 Dec 2023 08:54:49 +1300 Subject: [PATCH 3/4] Add some more tests --- ...he-result-are-backed-a-batch-hydration.yml | 176 +++++++++++++++++ ...s-in-the-result-are-backed-a-hydration.yml | 187 ++++++++++++++++++ 2 files changed, 363 insertions(+) create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-batch-hydration.yml create mode 100644 test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-hydration.yml diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-batch-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-batch-hydration.yml new file mode 100644 index 000000000..39d4f9aa4 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-batch-hydration.yml @@ -0,0 +1,176 @@ +name: "new some renamed object types have fields in the result are backed a batch hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + user: User + @hydrated( + service: "users" + field: "usersByIds" + arguments: [{name: "ids" value: "$source.userId"}] + inputIdentifiedBy: [{sourceId: "userId" resultId: "id"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type Activity implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on Activity { + __typename__batch_hydration__user: __typename + batch_hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "__typename__batch_hydration__user": "Activity", + "batch_hydration__user__userId": "user-100" + }, + { + "__typename__batch_hydration__user": "Activity", + "batch_hydration__user__userId": "user-20" + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + usersByIds(ids: ["user-100", "user-20"]) { + batch_hydration__user__id: id + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "usersByIds": [ + { + "batch_hydration__user__id": "user-100", + "name": "Spaces" + }, + { + "batch_hydration__user__id": "user-20", + "name": "Newmarket" + } + ] + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Spaces" + } + }, + { + "user": { + "name": "Newmarket" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } diff --git a/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-hydration.yml b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-hydration.yml new file mode 100644 index 000000000..6caaccfb2 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/abstract types/new-some-renamed-object-types-have-fields-in-the-result-are-backed-a-hydration.yml @@ -0,0 +1,187 @@ +name: "new some renamed object types have fields in the result are backed a hydration" +enabled: true +# language=GraphQL +overallSchema: + activity: | + type Query { + activity: [IActivity] + } + interface IActivity { + user: User + } + type Activity implements IActivity @renamed(from: "SomethingLame") { + id: ID! + user: User + @hydrated( + service: "users" + field: "userById" + arguments: [{name: "id" value: "$source.userId"}] + ) + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +underlyingSchema: + activity: | + type Query { + activity: [IActivity] + } + type User { + id: ID! + name: String + } + interface IActivity { + user: User + } + type SomethingLame implements IActivity { + id: ID! + userId: ID + user: User @deprecated(reason: "Fake") + } + type SingleActivity implements IActivity { + id: ID! + user: User + } + users: | + type Query { + userById(id: ID!): User + } + type User { + id: ID! + name: String + } +# language=GraphQL +query: | + { + activity { + user { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "activity" + request: + # language=GraphQL + query: | + { + activity { + ... on SomethingLame { + __typename__hydration__user: __typename + hydration__user__userId: userId + } + ... on SingleActivity { + user { + name + } + } + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "__typename__hydration__user": "SomethingLame", + "hydration__user__userId": "user-100" + }, + { + "__typename__hydration__user": "SomethingLame", + "hydration__user__userId": "user-20" + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + userById(id: "user-100") { + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "userById": { + "name": "Hello" + } + }, + "extensions": {} + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + userById(id: "user-20") { + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "userById": { + "name": "World" + } + }, + "extensions": {} + } +# language=JSON +response: | + { + "data": { + "activity": [ + { + "user": { + "name": "John" + } + }, + { + "user": { + "name": "Hello" + } + }, + { + "user": { + "name": "World" + } + }, + { + "user": { + "name": "Mayor" + } + } + ] + }, + "errors": [] + } From 828b4813b1259a768d193158dbc7f263fc58ec1b Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 21 Dec 2023 14:48:33 +1300 Subject: [PATCH 4/4] Add extra test --- ...new-batch-hydration-null-source-object.yml | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 test/src/test/resources/fixtures/new hydration/new-batch-hydration-null-source-object.yml diff --git a/test/src/test/resources/fixtures/new hydration/new-batch-hydration-null-source-object.yml b/test/src/test/resources/fixtures/new hydration/new-batch-hydration-null-source-object.yml new file mode 100644 index 000000000..6fac51562 --- /dev/null +++ b/test/src/test/resources/fixtures/new hydration/new-batch-hydration-null-source-object.yml @@ -0,0 +1,127 @@ +name: "new batch hydration null source object" +enabled: true +# language=GraphQL +overallSchema: + issues: | + type Query { + myIssues(n: Int! = 10): [Issue] + } + type Issue { + title: String + assigneeId: ID + assignee: User + @hydrated( + service: "users" + field: "usersByIds" + arguments: [{name: "ids" value: "$source.assigneeId"}] + inputIdentifiedBy: [{sourceId: "assigneeId", resultId: "id"}] + ) + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + email: String + } +# language=GraphQL +underlyingSchema: + issues: | + type Query { + topIssue: Issue + myIssues(n: Int! = 10): [Issue] + } + type Issue { + title: String + assigneeId: ID + collaboratorIds: [ID!] + } + users: | + type Query { + usersByIds(ids: [ID!]!): [User] + } + type User { + id: ID! + name: String + email: String + } +# language=GraphQL +query: | + query { + myIssues { + title + assignee { + name + } + } + } +variables: { } +serviceCalls: + - serviceName: "issues" + request: + # language=GraphQL + query: | + { + myIssues { + __typename__batch_hydration__assignee: __typename + batch_hydration__assignee__assigneeId: assigneeId + title + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "myIssues": [ + { + "__typename__batch_hydration__assignee": "Issue", + "batch_hydration__assignee__assigneeId": "user-256", + "title": "Popular" + }, + null + ] + } + } + - serviceName: "users" + request: + # language=GraphQL + query: | + { + usersByIds(ids: ["user-256"]) { + batch_hydration__assignee__id: id + name + } + } + variables: { } + # language=JSON + response: |- + { + "data": { + "usersByIds": [ + { + "batch_hydration__assignee__id": "user-256", + "name": "2^8" + } + ] + }, + "extensions": {} + } +# language=JSON +response: |- + { + "data": { + "myIssues": [ + { + "title": "Popular", + "assignee": { + "name": "2^8" + } + }, + null + ] + }, + "extensions": {} + }