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.

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

Remove @inaccessible elements when converting to API schema #807

Merged
merged 24 commits into from
Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
dbc62a7
Remove `@inaccessible` elements when converting to API schema
martijnwalraven Jun 11, 2021
4bb1eef
Add test for removing `@inaccessible` union types
martijnwalraven Jun 14, 2021
37c3b01
Update Rust snapshot to account for `@tag` and `@inaccessible` defini…
martijnwalraven May 13, 2021
c78453f
Use API schema for validation and response execution
martijnwalraven Jun 24, 2021
2a43cbf
Cache API schema in `toAPISchema()`
martijnwalraven Jun 24, 2021
dc63dc2
Remove `@inaccessible` types from `implements` clauses and unions
martijnwalraven Jun 24, 2021
998f12e
Throw when a field returning an @inaccessible type isn't marked @inac…
martijnwalraven Jul 8, 2021
547ded6
Fix removing root types
martijnwalraven Jul 8, 2021
2a927e3
Validate input schema, and validate API schema, before caching it
martijnwalraven Jul 9, 2021
58c3763
Fix tests
trevor-scheer Jul 14, 2021
a404067
Bypassing federation testing utility fixes tests for some reason.
trevor-scheer Jul 14, 2021
6bab098
Update harmonizer snapshot test
trevor-scheer Jul 14, 2021
e5ab303
Merge branch 'release-gateway-0.34' into inaccessible-removal-in-api-…
trevor-scheer Jul 15, 2021
1a05bde
Progress on adding in @inaccessible for our test fixtures
trevor-scheer Jul 15, 2021
9cbe3bb
Use the correct schema for the test
trevor-scheer Jul 18, 2021
8c4a882
unfocus test
trevor-scheer Jul 18, 2021
f950a4b
Incorporate testing suggestions from @timbotnik
trevor-scheer Jul 18, 2021
9cb04ac
Revert back to getFederatedTestingSchema, use fixtures directly
trevor-scheer Jul 18, 2021
232a5be
Merge branch 'release-gateway-0.34' into inaccessible-removal-in-api-…
trevor-scheer Jul 18, 2021
f8afba1
Fixture cleanup
trevor-scheer Jul 18, 2021
0afe6ff
Update supergraph test fixture inaccessible locations
trevor-scheer Jul 18, 2021
c1ff284
Retry package-lock updates from base
trevor-scheer Jul 19, 2021
5d4ceb5
Update query-planner-js/src/composedSchema/toAPISchema.ts
trevor-scheer Jul 19, 2021
29eb30b
Use node's native url package
trevor-scheer Jul 19, 2021
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
209 changes: 183 additions & 26 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
import { getIntrospectionQuery, GraphQLSchema } from 'graphql';
import {
buildClientSchema,
getIntrospectionQuery,
GraphQLObjectType,
GraphQLSchema,
parse,
} from 'graphql';
import { addResolversToSchema, GraphQLResolverMap } from 'apollo-graphql';
import gql from 'graphql-tag';
import { GraphQLRequestContext } from 'apollo-server-types';
import { AuthenticationError } from 'apollo-server-core';
import { buildOperationContext } from '../operationContext';
import { executeQueryPlan } from '../executeQueryPlan';
import { LocalGraphQLDataSource } from '../datasources/LocalGraphQLDataSource';
import { astSerializer, queryPlanSerializer } from 'apollo-federation-integration-testsuite';
import { getFederatedTestingSchema } from './execution-utils';
import { QueryPlanner } from '@apollo/query-planner';
import {
astSerializer,
queryPlanSerializer,
} from 'apollo-federation-integration-testsuite';
import { buildComposedSchema, QueryPlanner } from '@apollo/query-planner';
import { buildFederatedSchema, composeAndValidate } from '@apollo/federation';
import { ApolloGateway } from '..';
import { ApolloServerBase as ApolloServer } from 'apollo-server-core';
import { fixtures } from 'apollo-federation-integration-testsuite';
import { buildLocalService } from './execution-utils';

expect.addSnapshotSerializer(astSerializer);
expect.addSnapshotSerializer(queryPlanSerializer);
Expand All @@ -34,20 +47,26 @@ describe('executeQueryPlan', () => {

let schema: GraphQLSchema;
let queryPlanner: QueryPlanner;

beforeEach(() => {
expect(
() =>
({
serviceMap,
schema,
queryPlanner,
} = getFederatedTestingSchema()),
).not.toThrow();
// expect(
// () =>
// ({
// serviceMap,
// schema,
// queryPlanner,
// } = getFederatedTestingSchema()),
// ).not.toThrow();
let compositionResult = composeAndValidate(fixtures);

schema = buildComposedSchema(parse(compositionResult.supergraphSdl!));
queryPlanner = new QueryPlanner(schema);
serviceMap = Object.fromEntries(
fixtures.map((f) => [f.name, buildLocalService([f])]),
);
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
});

function buildRequestContext(): GraphQLRequestContext {
// @ts-ignore
// @ts-ignore
return {
cache: undefined as any,
context: {},
Expand Down Expand Up @@ -146,9 +165,8 @@ describe('executeQueryPlan', () => {
});

it(`should not send request to downstream services when all entities are undefined`, async () => {
const accountsEntitiesResolverSpy = spyOnEntitiesResolverInService(
'accounts',
);
const accountsEntitiesResolverSpy =
spyOnEntitiesResolverInService('accounts');

const operationString = `#graphql
query {
Expand Down Expand Up @@ -224,9 +242,8 @@ describe('executeQueryPlan', () => {
});

it(`should send a request to downstream services for the remaining entities when some entities are undefined`, async () => {
const accountsEntitiesResolverSpy = spyOnEntitiesResolverInService(
'accounts',
);
const accountsEntitiesResolverSpy =
spyOnEntitiesResolverInService('accounts');

const operationString = `#graphql
query {
Expand Down Expand Up @@ -334,9 +351,8 @@ describe('executeQueryPlan', () => {
});

it(`should not send request to downstream service when entities don't match type conditions`, async () => {
const reviewsEntitiesResolverSpy = spyOnEntitiesResolverInService(
'reviews',
);
const reviewsEntitiesResolverSpy =
spyOnEntitiesResolverInService('reviews');

const operationString = `#graphql
query {
Expand Down Expand Up @@ -383,9 +399,8 @@ describe('executeQueryPlan', () => {
});

it(`should send a request to downstream services for the remaining entities when some entities don't match type conditions`, async () => {
const reviewsEntitiesResolverSpy = spyOnEntitiesResolverInService(
'reviews',
);
const reviewsEntitiesResolverSpy =
spyOnEntitiesResolverInService('reviews');

const operationString = `#graphql
query {
Expand Down Expand Up @@ -1178,4 +1193,146 @@ describe('executeQueryPlan', () => {
}
`);
});

describe('@inaccessible', () => {
it(`should not include @inaccessible fields in introspection`, async () => {
const operationContext = buildOperationContext({
schema,
operationDocument: gql`
${getIntrospectionQuery()}
`,
});
const queryPlan = queryPlanner.buildQueryPlan(operationContext);

const response = await executeQueryPlan(
queryPlan,
serviceMap,
buildRequestContext(),
operationContext,
);

expect(response.data).toHaveProperty('__schema');
expect(response.errors).toBeUndefined();

const introspectedSchema = buildClientSchema(response.data as any);

const userType = introspectedSchema.getType('User') as GraphQLObjectType;

expect(userType.getFields()['username']).toBeDefined();
expect(userType.getFields()['ssn']).toBeUndefined();
});

it(`should not return @inaccessible fields`, async () => {
const operationString = `#graphql
query {
topReviews {
body
author {
username
ssn
}
}
}
`;

const operationDocument = gql(operationString);

const operationContext = buildOperationContext({
schema,
operationDocument,
});

const queryPlan = queryPlanner.buildQueryPlan(operationContext);

const response = await executeQueryPlan(
queryPlan,
serviceMap,
buildRequestContext(),
operationContext,
);

expect(response.data).toMatchInlineSnapshot(`
Object {
"topReviews": Array [
Object {
"author": Object {
"username": "@ada",
},
"body": "Love it!",
},
Object {
"author": Object {
"username": "@ada",
},
"body": "Too expensive.",
},
Object {
"author": Object {
"username": "@complete",
},
"body": "Could be better.",
},
Object {
"author": Object {
"username": "@complete",
},
"body": "Prefer something else.",
},
Object {
"author": Object {
"username": "@complete",
},
"body": "Wish I had read this before.",
},
],
}
`);
});

it(`should return a validation error when an @inaccessible field is requested`, async () => {
// Because validation is part of the Apollo Server request pipeline,
// we have to construct an instance of ApolloServer and execute the
// the operation against it.
// This test uses the same `gateway.load()` pattern as existing tests that
// execute operations against Apollo Server (like queryPlanCache.test.ts).
// But note that this is only one possible initialization path for the
// gateway, and with the current duplication of logic we'd actually need
// to test other scenarios (like loading from supergraph SDL) separately.

const gateway = new ApolloGateway({
localServiceList: fixtures,
buildService: (service) => {
// @ts-ignore
return new LocalGraphQLDataSource(buildFederatedSchema([service]));
},
});

const { schema, executor } = await gateway.load();

const server = new ApolloServer({ schema, executor });

const query = `#graphql
query {
topReviews {
body
author {
username
ssn
}
}
}
`;

const response = await server.executeOperation({
query,
});

expect(response.data).toBeUndefined();
expect(response.errors).toMatchInlineSnapshot(`
Array [
[ValidationError: Cannot query field "ssn" on type "User".],
]
`);
});
});
});
7 changes: 3 additions & 4 deletions gateway-js/src/__tests__/gateway/composedSdl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@ describe('Using supergraphSdl configuration', () => {
const server = await getSupergraphSdlGatewayServer();

fetch.mockJSONResponseOnce({
data: { me: { id: 1, username: '@jbaxleyiii' } },
data: { me: { username: '@jbaxleyiii' } },
});

const result = await server.executeOperation({
query: '{ me { id username } }',
query: '{ me { username } }',
});

expect(result.data).toMatchInlineSnapshot(`
Object {
"me": Object {
"id": "1",
"username": "@jbaxleyiii",
},
}
Expand All @@ -38,7 +37,7 @@ describe('Using supergraphSdl configuration', () => {
const [url, request] = fetch.mock.calls[0];
expect(url).toEqual('https://accounts.api.com');
expect(request?.body).toEqual(
JSON.stringify({ query: '{me{id username}}', variables: {} }),
JSON.stringify({ query: '{me{username}}', variables: {} }),
);
await server.stop();
});
Expand Down
5 changes: 2 additions & 3 deletions gateway-js/src/__tests__/gateway/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ describe('ApolloGateway executor', () => {
);
});

it('should not crash if no variables are not provided', async () => {
const me = { id: '1', birthDate: '1988-10-21'};
it('should not crash if variables are not provided', async () => {
const me = { birthDate: '1988-10-21'};
fetch.mockJSONResponseOnce({ data: { me } });
const gateway = new ApolloGateway({
localServiceList: fixtures,
Expand All @@ -68,7 +68,6 @@ describe('ApolloGateway executor', () => {
const source = `#graphql
query Me($locale: String) {
me {
id
birthDate(locale: $locale)
}
}
Expand Down
5 changes: 3 additions & 2 deletions gateway-js/src/executeQueryPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import {
ResponsePath,
QueryPlanSelectionNode,
QueryPlanFieldNode,
getResponseName
getResponseName,
toAPISchema
} from '@apollo/query-planner';
import { deepMerge } from './utilities/deepMerge';
import { isNotNullOrUndefined } from './utilities/array';
Expand Down Expand Up @@ -90,7 +91,7 @@ export async function executeQueryPlan<TContext>(
// It is also used to allow execution of introspection queries though.
try {
const executionResult = await execute({
schema: operationContext.schema,
schema: toAPISchema(operationContext.schema),
document: {
kind: Kind.DOCUMENT,
definitions: [
Expand Down
8 changes: 4 additions & 4 deletions gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { getVariableValues } from 'graphql/execution/values';
import fetcher from 'make-fetch-happen';
import { HttpRequestCache } from './cache';
import { fetch } from 'apollo-server-env';
import { QueryPlanner, QueryPlan, prettyFormatQueryPlan } from '@apollo/query-planner';
import { QueryPlanner, QueryPlan, prettyFormatQueryPlan, toAPISchema } from '@apollo/query-planner';
import {
ServiceEndpointDefinition,
Experimental_DidFailCompositionCallback,
Expand Down Expand Up @@ -428,7 +428,7 @@ export class ApolloGateway implements GraphQLService {
throw e;
}

this.schema = schema;
this.schema = toAPISchema(schema);
trevor-scheer marked this conversation as resolved.
Show resolved Hide resolved
// TODO(trevor): #580 redundant parse
this.parsedSupergraphSdl = parse(supergraphSdl);
this.queryPlanner = new QueryPlanner(schema);
Expand Down Expand Up @@ -508,7 +508,7 @@ export class ApolloGateway implements GraphQLService {
"A valid schema couldn't be composed. Falling back to previous schema.",
);
} else {
this.schema = schema;
this.schema = toAPISchema(schema);
this.queryPlanner = new QueryPlanner(schema);

// Notify the schema listeners of the updated schema
Expand Down Expand Up @@ -581,7 +581,7 @@ export class ApolloGateway implements GraphQLService {
"A valid schema couldn't be composed. Falling back to previous schema.",
);
} else {
this.schema = schema;
this.schema = toAPISchema(schema);
this.queryPlanner = new QueryPlanner(schema);

// Notify the schema listeners of the updated schema
Expand Down
4 changes: 4 additions & 0 deletions harmonizer/src/snapshots/harmonizer__tests__it_works.snap
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE

directive @join__graph(name: String!, url: String!) on ENUM_VALUE

directive @tag(name: String!) repeatable on FIELD_DEFINITION

directive @inaccessible on FIELD_DEFINITION

scalar join__FieldSet

enum join__Graph {
Expand Down
Loading