Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix operation names #1568

Merged
merged 3 commits into from
Mar 8, 2022
Merged

Fix operation names #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bcb186
Sanitize subgraph names when including in QP
Mar 7, 2022
873b396
Update operation names in tests for _ to __
Mar 7, 2022
bc4a192
Remove left-over debug code
Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions gateway-js/src/__tests__/build-query-plan.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,8 @@ Scenario: supports basic, single-service mutation
"password"
],
"operationKind": "mutation",
"operation": "mutation Login_accounts_0($username:String!$password:String!){login(username:$username password:$password){id}}",
"operationName": "Login_accounts_0"
"operation": "mutation Login__accounts__0($username:String!$password:String!){login(username:$username password:$password){id}}",
"operationName": "Login__accounts__0"
}
}
"""
Expand Down Expand Up @@ -1044,8 +1044,8 @@ Scenario: supports mutations with a cross-service request
"password"
],
"operationKind": "mutation",
"operation": "mutation Login_accounts_0($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "Login_accounts_0"
"operation": "mutation Login__accounts__0($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "Login__accounts__0"
},
{
"kind": "Flatten",
Expand Down Expand Up @@ -1073,8 +1073,8 @@ Scenario: supports mutations with a cross-service request
],
"variableUsages": [],
"operationKind": "query",
"operation": "query Login_reviews_1($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "Login_reviews_1"
"operation": "query Login__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "Login__reviews__1"
}
},
{
Expand Down Expand Up @@ -1106,8 +1106,8 @@ Scenario: supports mutations with a cross-service request
],
"variableUsages": [],
"operationKind": "query",
"operation": "query Login_product_2($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "Login_product_2"
"operation": "query Login__product__2($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "Login__product__2"
}
}
]
Expand Down Expand Up @@ -1142,8 +1142,8 @@ Scenario: returning across service boundaries
"body"
],
"operationKind": "mutation",
"operation": "mutation Review_reviews_0($upc:String!$body:String!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{__typename upc}}}",
"operationName": "Review_reviews_0"
"operation": "mutation Review__reviews__0($upc:String!$body:String!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{__typename upc}}}",
"operationName": "Review__reviews__0"
},
{
"kind": "Flatten",
Expand Down Expand Up @@ -1171,8 +1171,8 @@ Scenario: returning across service boundaries
],
"variableUsages": [],
"operationKind": "query",
"operation": "query Review_product_1($representations:[_Any!]!){_entities(representations:$representations){...on Furniture{name}}}",
"operationName": "Review_product_1"
"operation": "query Review__product__1($representations:[_Any!]!){_entities(representations:$representations){...on Furniture{name}}}",
"operationName": "Review__product__1"
}
}
]
Expand Down Expand Up @@ -1219,8 +1219,8 @@ Scenario: supports multiple root mutations
"password"
],
"operationKind": "mutation",
"operation": "mutation LoginAndReview_accounts_0($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "LoginAndReview_accounts_0"
"operation": "mutation LoginAndReview__accounts__0($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "LoginAndReview__accounts__0"
},
{
"kind": "Flatten",
Expand Down Expand Up @@ -1248,8 +1248,8 @@ Scenario: supports multiple root mutations
],
"variableUsages": [],
"operationKind": "query",
"operation": "query LoginAndReview_reviews_1($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "LoginAndReview_reviews_1"
"operation": "query LoginAndReview__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "LoginAndReview__reviews__1"
}
},
{
Expand Down Expand Up @@ -1281,8 +1281,8 @@ Scenario: supports multiple root mutations
],
"variableUsages": [],
"operationKind": "query",
"operation": "query LoginAndReview_product_2($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "LoginAndReview_product_2"
"operation": "query LoginAndReview__product__2($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "LoginAndReview__product__2"
}
},
{
Expand All @@ -1293,8 +1293,8 @@ Scenario: supports multiple root mutations
"body"
],
"operationKind": "mutation",
"operation": "mutation LoginAndReview_reviews_3($upc:String!$body:String!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{__typename upc}}}",
"operationName": "LoginAndReview_reviews_3"
"operation": "mutation LoginAndReview__reviews__3($upc:String!$body:String!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{__typename upc}}}",
"operationName": "LoginAndReview__reviews__3"
},
{
"kind": "Flatten",
Expand Down Expand Up @@ -1322,8 +1322,8 @@ Scenario: supports multiple root mutations
],
"variableUsages": [],
"operationKind": "query",
"operation": "query LoginAndReview_product_4($representations:[_Any!]!){_entities(representations:$representations){...on Furniture{name}}}",
"operationName": "LoginAndReview_product_4"
"operation": "query LoginAndReview__product__4($representations:[_Any!]!){_entities(representations:$representations){...on Furniture{name}}}",
"operationName": "LoginAndReview__product__4"
}
}
]
Expand Down Expand Up @@ -1378,8 +1378,8 @@ Scenario: multiple root mutations with correct service order
"updatedReview"
],
"operationKind": "mutation",
"operation": "mutation LoginAndReview_reviews_0($upc:String!$body:String!$updatedReview:UpdateReviewInput!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{upc}}updateReview(review:$updatedReview){id body}}",
"operationName": "LoginAndReview_reviews_0"
"operation": "mutation LoginAndReview__reviews__0($upc:String!$body:String!$updatedReview:UpdateReviewInput!){reviewProduct(input:{upc:$upc body:$body}){__typename ...on Furniture{upc}}updateReview(review:$updatedReview){id body}}",
"operationName": "LoginAndReview__reviews__0"
},
{
"kind": "Fetch",
Expand All @@ -1389,8 +1389,8 @@ Scenario: multiple root mutations with correct service order
"password"
],
"operationKind": "mutation",
"operation": "mutation LoginAndReview_accounts_1($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "LoginAndReview_accounts_1"
"operation": "mutation LoginAndReview__accounts__1($username:String!$password:String!){login(username:$username password:$password){__typename id}}",
"operationName": "LoginAndReview__accounts__1"
},
{
"kind": "Flatten",
Expand Down Expand Up @@ -1418,8 +1418,8 @@ Scenario: multiple root mutations with correct service order
],
"variableUsages": [],
"operationKind": "query",
"operation": "query LoginAndReview_reviews_2($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "LoginAndReview_reviews_2"
"operation": "query LoginAndReview__reviews__2($representations:[_Any!]!){_entities(representations:$representations){...on User{reviews{product{__typename ...on Book{__typename isbn}...on Furniture{upc}}}}}}",
"operationName": "LoginAndReview__reviews__2"
}
},
{
Expand Down Expand Up @@ -1451,8 +1451,8 @@ Scenario: multiple root mutations with correct service order
],
"variableUsages": [],
"operationKind": "query",
"operation": "query LoginAndReview_product_3($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "LoginAndReview_product_3"
"operation": "query LoginAndReview__product__3($representations:[_Any!]!){_entities(representations:$representations){...on Book{upc}}}",
"operationName": "LoginAndReview__product__3"
}
},
{
Expand All @@ -1462,8 +1462,8 @@ Scenario: multiple root mutations with correct service order
"reviewId"
],
"operationKind": "mutation",
"operation": "mutation LoginAndReview_reviews_4($reviewId:ID!){deleteReview(id:$reviewId)}",
"operationName": "LoginAndReview_reviews_4"
"operation": "mutation LoginAndReview__reviews__4($reviewId:ID!){deleteReview(id:$reviewId)}",
"operationName": "LoginAndReview__reviews__4"
}
]
}
Expand Down
139 changes: 138 additions & 1 deletion query-planner-js/src/__tests__/buildPlan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { composeServices } from '@apollo/composition';
import { asFed2SubgraphDocument, assert, buildSchema, operationFromDocument, Schema, ServiceDefinition } from '@apollo/federation-internals';
import gql from 'graphql-tag';
import { MAX_COMPUTED_PLANS } from '../buildPlan';
import { FetchNode } from '../QueryPlan';
import { FetchNode, FlattenNode, SequenceNode } from '../QueryPlan';
import { FieldNode, OperationDefinitionNode, parse } from 'graphql';

expect.addSnapshotSerializer(astSerializer);
Expand Down Expand Up @@ -1064,6 +1064,143 @@ describe('@requires', () => {
})
});

describe('fetch operation names', () => {
test('handle subgraph with - in the name', () => {
const subgraph1 = {
name: 'S1',
typeDefs: gql`
type Query {
t: T
}

type T @key(fields: "id") {
id: ID!
}
`
}

const subgraph2 = {
name: 'non-graphql-name',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
x: Int
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(api, gql`
query myOp {
t {
x
}
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "S1") {
{
t {
__typename
id
}
}
},
Flatten(path: "t") {
Fetch(service: "non-graphql-name") {
{
... on T {
__typename
id
}
} =>
{
... on T {
x
}
}
},
},
},
}
`);
const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode;
expect(fetch.operation).toMatch(/^query myOp__non_graphqlname__1.*/i);
});

test('handle very non-graph subgraph name', () => {
const subgraph1 = {
name: 'S1',
typeDefs: gql`
type Query {
t: T
}

type T @key(fields: "id") {
id: ID!
}
`
}

const subgraph2 = {
name: '42!',
typeDefs: gql`
type T @key(fields: "id") {
id: ID!
x: Int
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(api, gql`
query myOp {
t {
x
}
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "S1") {
{
t {
__typename
id
}
}
},
Flatten(path: "t") {
Fetch(service: "42!") {
{
... on T {
__typename
id
}
} =>
{
... on T {
x
}
}
},
},
},
}
`);
const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode;

expect(fetch.operation).toMatch(/^query myOp___42__1.*/i);
});
});

test('Correctly handle case where there is too many plans to consider', () => {
// Creating realistic examples where there is too many plan to consider is not trivial, but creating unrealistic examples
// is thankfully trivial. Here, we just have 2 subgraphs that are _exactly the same_ with a single type having plenty of
Expand Down
Loading