Skip to content

Commit

Permalink
Remove @inaccessible elements when converting to API schema (#807)
Browse files Browse the repository at this point in the history
This is an initial implementation of support for @inaccessible when converting
a composed schema to an API schema. For now, it removes any fields, object
types, or interface types with an @inaccessible directive.

As part of this change, buildComposedSchema no longer returns an API
schema. Instead, a separate toAPISchema function is used to convert the
composed schema. The reason for this is that the gateway needs the complete
schema for query planning, including elements that will be removed by
@inaccessible.

Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Co-authored-by: Jesse Rosenberger <git@jro.cc>
  • Loading branch information
3 people authored Jul 19, 2021
1 parent c3bd6dd commit 623658b
Show file tree
Hide file tree
Showing 18 changed files with 802 additions and 102 deletions.
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

0 comments on commit 623658b

Please sign in to comment.