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 all 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
40 changes: 19 additions & 21 deletions federation-integration-testsuite-js/src/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,9 @@ import * as reviewsWithUpdate from './special-cases/reviewsWithUpdate';
import * as accountsWithoutTag from './special-cases/accountsWithoutTag';
import * as reviewsWithoutTag from './special-cases/reviewsWithoutTag';

export {
accounts,
books,
documents,
inventory,
product,
reviews,
reviewsWithUpdate,
};
const fixtures = [accounts, books, documents, inventory, product, reviews];

export const fixtures = [
accounts,
books,
documents,
inventory,
product,
reviews,
];

export const fixturesWithUpdate = [
const fixturesWithUpdate = [
accounts,
books,
documents,
Expand All @@ -36,7 +19,7 @@ export const fixturesWithUpdate = [
reviewsWithUpdate,
];

export const fixturesWithoutTag = [
const fixturesWithoutTag = [
accountsWithoutTag,
books,
documents,
Expand All @@ -45,11 +28,26 @@ export const fixturesWithoutTag = [
reviewsWithoutTag,
];

export const fixtureNames = [
const fixtureNames = [
accounts.name,
product.name,
inventory.name,
reviews.name,
books.name,
documents.name,
];

export { superGraphWithInaccessible } from './special-cases/supergraphWithInaccessible';
export {
accounts,
books,
documents,
inventory,
product,
reviews,
reviewsWithUpdate,
fixtures,
fixturesWithUpdate,
fixturesWithoutTag,
fixtureNames,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { composeAndValidate } from '@apollo/federation';
import { assertCompositionSuccess } from '@apollo/federation/dist/composition/utils';
import {
DirectiveDefinitionNode,
SchemaDefinitionNode,
DocumentNode,
DirectiveNode,
parse,
visit,
} from 'graphql';
import { fixtures } from '..';

const compositionResult = composeAndValidate(fixtures);
assertCompositionSuccess(compositionResult);
const parsed = parse(compositionResult.supergraphSdl);

// We need to collect the AST for the inaccessible definition as well
// as the @core and @inaccessible usages. Parsing SDL is a fairly
// clean approach to this and easier to update than handwriting the AST.
const [inaccessibleDefinition, schemaDefinition] = parse(`#graphql
# inaccessibleDefinition
directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION
schema
# inaccessibleCoreUsage
@core(feature: "https://specs.apollo.dev/inaccessible/v0.1")
# inaccessibleUsage
@inaccessible {
query: Query
}
`).definitions as [DirectiveDefinitionNode, SchemaDefinitionNode];

const [inaccessibleCoreUsage, inaccessibleUsage] =
schemaDefinition.directives as [DirectiveNode, DirectiveNode];

// Append the AST with the inaccessible definition,
// @core inaccessible usage, and @inaccessible usage on the `ssn` field
const superGraphWithInaccessible: DocumentNode = visit(parsed, {
Document(node) {
return {
...node,
definitions: [...node.definitions, inaccessibleDefinition],
};
},
SchemaDefinition(node) {
return {
...node,
directives: [...(node.directives ?? []), inaccessibleCoreUsage],
};
},
ObjectTypeDefinition(node) {
return {
...node,
fields:
node.fields?.map((field) => {
if (field.name.value === 'ssn') {
return {
...field,
directives: [...(field.directives ?? []), inaccessibleUsage],
};
}
return field;
}) ?? [],
};
},
});

export { superGraphWithInaccessible };
190 changes: 168 additions & 22 deletions gateway-js/src/__tests__/executeQueryPlan.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
import { getIntrospectionQuery, GraphQLSchema } from 'graphql';
import {
buildClientSchema,
getIntrospectionQuery,
GraphQLObjectType,
GraphQLSchema,
print,
} 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 {
astSerializer,
queryPlanSerializer,
superGraphWithInaccessible,
} from 'apollo-federation-integration-testsuite';
import { buildComposedSchema, QueryPlanner } from '@apollo/query-planner';
import { ApolloGateway } from '..';
import { ApolloServerBase as ApolloServer } from 'apollo-server-core';
import { getFederatedTestingSchema } from './execution-utils';
import { QueryPlanner } from '@apollo/query-planner';

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

let schema: GraphQLSchema;
let queryPlanner: QueryPlanner;

beforeEach(() => {
expect(
() =>
({
serviceMap,
schema,
queryPlanner,
} = getFederatedTestingSchema()),
({ serviceMap, schema, queryPlanner } = getFederatedTestingSchema()),
).not.toThrow();
});

function buildRequestContext(): GraphQLRequestContext {
// @ts-ignore
// @ts-ignore
return {
cache: undefined as any,
context: {},
Expand Down Expand Up @@ -146,9 +153,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 +230,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 +339,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 +387,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 +1181,147 @@ describe('executeQueryPlan', () => {
}
`);
});

describe('@inaccessible', () => {
it(`should not include @inaccessible fields in introspection`, async () => {
schema = buildComposedSchema(superGraphWithInaccessible);
queryPlanner = new QueryPlanner(schema);

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);

schema = buildComposedSchema(superGraphWithInaccessible);

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

queryPlanner = new QueryPlanner(schema);
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({
supergraphSdl: print(superGraphWithInaccessible),
});

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: 1 addition & 6 deletions gateway-js/src/__tests__/execution-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,7 @@ export function getFederatedTestingSchema(services: ServiceDefinitionModule[] =
]),
);

const compositionResult = composeAndValidate(
Object.entries(serviceMap).map(([serviceName, dataSource]) => ({
name: serviceName,
typeDefs: dataSource.sdl(),
})),
);
const compositionResult = composeAndValidate(services);

if (compositionHasErrors(compositionResult)) {
throw new GraphQLSchemaValidationError(compositionResult.errors);
Expand Down
Loading