Skip to content

Commit

Permalink
Add matchingEnums to sdl validation rules
Browse files Browse the repository at this point in the history
JakeDawkins committed Jun 14, 2019
1 parent bab5b34 commit 8754e50
Showing 10 changed files with 524 additions and 259 deletions.
353 changes: 156 additions & 197 deletions packages/apollo-federation/src/composition/__tests__/compose.test.ts

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -206,46 +206,3 @@ it('errors on invalid usages of default operation names', () => {
});

it.todo('errors on duplicate types where there is a mismatch of field types');

fit('does not error with duplicate enums', () => {
const serviceA = {
typeDefs: gql`
type Query {
products: [Product]!
}
type Product @key(fields: "sku") {
sku: String!
upc: String!
type: ProductType
}
enum ProductType {
FURNITURE
BOOK
DIGITAL
}
`,
name: 'serviceA',
};

const serviceB = {
typeDefs: gql`
type Product @extends @key(fields: "sku") {
sku: String! @external
price: Int! @requires(fields: "sku")
productType: ProductType
}
enum ProductType {
FURNITURE
BOOK
DIGITAL
}
`,
name: 'serviceB',
};

const { schema, errors } = composeAndValidate([serviceA, serviceB]);
expect(errors).toHaveLength(0);
});
10 changes: 6 additions & 4 deletions packages/apollo-federation/src/composition/compose.ts
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@ import {
GraphQLSchema,
extendSchema,
Kind,
TypeDefinitionNode,
TypeExtensionNode,
isTypeDefinitionNode,
isTypeExtensionNode,
@@ -33,13 +32,14 @@ import {
ServiceName,
ExternalFieldDefinition,
ServiceNameToKeyDirectivesMap,
FederatedTypeDefinitionNode,
} from './types';
import { validateSDL } from 'graphql/validation/validate';
import { compositionRules } from './rules';

// Map of all definitions to eventually be passed to extendSchema
interface DefinitionsMap {
[name: string]: TypeDefinitionNode[];
[name: string]: FederatedTypeDefinitionNode[];
}
// Map of all extensions to eventually be passed to extendSchema
interface ExtensionsMap {
@@ -157,9 +157,9 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
* take precedence). If not, create the definitions array and add it to the definitionsMap.
*/
if (definitionsMap[typeName]) {
definitionsMap[typeName].push(definition);
definitionsMap[typeName].push({ ...definition, serviceName });
} else {
definitionsMap[typeName] = [definition];
definitionsMap[typeName] = [{ ...definition, serviceName }];
}
} else if (isTypeExtensionNode(definition)) {
const typeName = definition.name.value;
@@ -240,6 +240,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
kind: Kind.OBJECT_TYPE_DEFINITION,
name: { kind: Kind.NAME, value: extensionTypeName },
fields: [],
serviceName: null,
},
];

@@ -266,6 +267,7 @@ export function buildSchemaFromDefinitionsAndExtensions({
extensionsMap: ExtensionsMap;
}) {
let errors: GraphQLError[] | undefined = undefined;

let schema = new GraphQLSchema({
query: undefined,
directives: [...specifiedDirectives, ...federationDirectives],
7 changes: 5 additions & 2 deletions packages/apollo-federation/src/composition/rules.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { specifiedSDLRules } from 'graphql/validation/specifiedRules';

import { UniqueTypeNamesWithoutEnumsOrScalars } from './validate/sdl';
import {
UniqueTypeNamesWithoutEnumsOrScalars,
matchingEnums,
} from './validate/sdl';

const omit = [
'UniqueDirectivesPerLocation',
@@ -10,4 +13,4 @@ const omit = [

export const compositionRules = specifiedSDLRules
.filter(rule => !omit.includes(rule.name))
.concat([UniqueTypeNamesWithoutEnumsOrScalars]);
.concat([UniqueTypeNamesWithoutEnumsOrScalars, matchingEnums]);
11 changes: 10 additions & 1 deletion packages/apollo-federation/src/composition/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { SelectionNode, DocumentNode, FieldDefinitionNode } from 'graphql';
import {
SelectionNode,
DocumentNode,
FieldDefinitionNode,
TypeDefinitionNode,
} from 'graphql';

export type ServiceName = string | null;

@@ -74,3 +79,7 @@ declare module 'graphql/type/definition' {
federation?: FederationField;
}
}

export type FederatedTypeDefinitionNode = TypeDefinitionNode & {
serviceName: string | null;
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
visit,
GraphQLError,
EnumTypeDefinitionNode,
ScalarTypeDefinitionNode,
} from 'graphql';
import { visit, GraphQLError } from 'graphql';
import { ServiceDefinition } from '../../types';

import { logServiceAndType, errorWithCode } from '../../utils';
@@ -15,8 +10,8 @@ export const duplicateEnumOrScalar = ({
const errors: GraphQLError[] = [];

// keep track of every enum and scalar and error if there are ever duplicates
const enums: EnumTypeDefinitionNode[] = [];
const scalars: ScalarTypeDefinitionNode[] = [];
const enums: string[] = [];
const scalars: string[] = [];

visit(typeDefs, {
EnumTypeDefinition(definition) {
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { visit, GraphQLError, EnumTypeDefinitionNode } from 'graphql';
import { visit, GraphQLError } from 'graphql';
import { ServiceDefinition } from '../../types';

import { logServiceAndType, errorWithCode } from '../../utils';
@@ -9,12 +9,15 @@ export const duplicateEnumValue = ({
}: ServiceDefinition) => {
const errors: GraphQLError[] = [];

const enums: Map<String, String[]> = {};
const enums: { [name: string]: string[] } = {};

visit(typeDefs, {
EnumTypeDefinition(definition) {
const name = definition.name.value;
const enumValues = definition.values.map(value => value.name.value);
const enumValues =
definition.values && definition.values.map(value => value.name.value);

if (!enumValues) return definition;

if (enums[name] && enums[name].length) {
enumValues.map(valueName => {
@@ -38,7 +41,10 @@ export const duplicateEnumValue = ({
},
EnumTypeExtension(definition) {
const name = definition.name.value;
const enumValues = definition.values.map(value => value.name.value);
const enumValues =
definition.values && definition.values.map(value => value.name.value);

if (!enumValues) return definition;

if (enums[name] && enums[name].length) {
enumValues.map(valueName => {
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
import {
GraphQLEnumType,
Kind,
DocumentNode,
validate,
GraphQLSchema,
specifiedDirectives,
} from 'graphql';
import { validateSDL } from 'graphql/validation/validate';
import gql from 'graphql-tag';

import { composeServices, buildMapsFromServiceList } from '../../../compose';
import {
astSerializer,
typeSerializer,
selectionSetSerializer,
} from '../../../../snapshotSerializers';
import { normalizeTypeDefs } from '../../../normalize';
import federationDirectives from '../../../../directives';
import { ServiceDefinition } from '../../../types';
import { matchingEnums } from '../matchingEnums';

expect.addSnapshotSerializer(astSerializer);
expect.addSnapshotSerializer(typeSerializer);
expect.addSnapshotSerializer(selectionSetSerializer);

// simulate the first half of the composition process
const createDefinitionsDocumentForServices = (
serviceList: ServiceDefinition[],
): DocumentNode => {
const { definitionsMap } = buildMapsFromServiceList(serviceList);
return {
kind: Kind.DOCUMENT,
definitions: Object.values(definitionsMap).flat(),
};
};

describe('matchingEnums', () => {
let schema: GraphQLSchema;

// create a blank schema for each test
beforeEach(() => {
schema = new GraphQLSchema({
query: undefined,
directives: [...specifiedDirectives, ...federationDirectives],
});
});

it('does not error with matching enums across services', () => {
const serviceList = [
{
typeDefs: gql`
enum ProductCategory {
BED
BATH
}
`,
name: 'serviceA',
},

{
typeDefs: gql`
enum ProductCategory {
BED
BATH
}
`,
name: 'serviceB',
},
];

const definitionsDocument = createDefinitionsDocumentForServices(
serviceList,
);
const errors = validateSDL(definitionsDocument, schema, [matchingEnums]);
expect(errors).toMatchInlineSnapshot(`Array []`);
});

it('errors when enums in separate services dont match', () => {
const serviceList = [
{
typeDefs: gql`
enum ProductCategory {
BED
BATH
}
`,
name: 'serviceA',
},
{
typeDefs: gql`
enum ProductCategory {
BEYOND
}
`,
name: 'serviceB',
},
];

const definitionsDocument = createDefinitionsDocumentForServices(
serviceList,
);
const errors = validateSDL(definitionsDocument, schema, [matchingEnums]);
expect(errors).toMatchInlineSnapshot(`
Array [
[GraphQLError: The \`ProductCategory\` enum does not have identical values in all services. Groups of services with identical values are: [serviceA], [serviceB]],
]
`);
});

it('errors when enums in separate services dont match', () => {
const serviceList = [
{
typeDefs: gql`
type Query {
products: [Product]!
}
type Product @key(fields: "sku") {
sku: String!
upc: String!
type: ProductType
}
enum ProductType {
BOOK
FURNITURE
}
`,
name: 'serviceA',
},
{
typeDefs: gql`
enum ProductType {
FURNITURE
BOOK
DIGITAL
}
`,
name: 'serviceB',
},
{
typeDefs: gql`
enum ProductType {
FURNITURE
BOOK
DIGITAL
}
`,
name: 'serviceC',
},
];

const definitionsDocument = createDefinitionsDocumentForServices(
serviceList,
);
const errors = validateSDL(definitionsDocument, schema, [matchingEnums]);
expect(errors).toMatchInlineSnapshot(`
Array [
[GraphQLError: The \`ProductType\` enum does not have identical values in all services. Groups of services with identical values are: [serviceA], [serviceB, serviceC]],
]
`);
});

it('errors when an enum name is defined as another type in a service', () => {
const serviceList = [
{
typeDefs: gql`
enum ProductType {
BOOK
FURNITURE
}
`,
name: 'serviceA',
},
{
typeDefs: gql`
type ProductType {
id: String
}
`,
name: 'serviceB',
},
{
typeDefs: gql`
enum ProductType {
FURNITURE
BOOK
DIGITAL
}
`,
name: 'serviceC',
},
];

const definitionsDocument = createDefinitionsDocumentForServices(
serviceList,
);
const errors = validateSDL(definitionsDocument, schema, [matchingEnums]);
expect(errors).toMatchInlineSnapshot(`
Array [
[GraphQLError: [serviceA] ProductType -> ProductType is an enum in [serviceA, serviceC], but not in [serviceB]],
]
`);
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export {
UniqueTypeNamesWithoutEnumsOrScalars,
} from './uniqueTypeNamesWithoutEnumsOrScalars';
export { matchingEnums } from './matchingEnums';
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { SDLValidationContext } from 'graphql/validation/ValidationContext';
import {
ASTVisitor,
Kind,
EnumTypeDefinitionNode,
EnumValueDefinitionNode,
} from 'graphql';
import { FederatedTypeDefinitionNode } from '../../types';
import { errorWithCode, logServiceAndType } from '../../utils';
import { isString } from 'util';

function isEnumDefinition(node: FederatedTypeDefinitionNode) {
return node.kind === Kind.ENUM_TYPE_DEFINITION;
}

type TypeToDefinitionsMap = {
[typeNems: string]: FederatedTypeDefinitionNode[];
};

type FederatedEnumDefinition = EnumTypeDefinitionNode & {
serviceName: string;
};

export function matchingEnums(context: SDLValidationContext): ASTVisitor {
const { definitions } = context.getDocument();

// group all definitions by name
// { MyTypeName: [{ serviceName: "A", name: {...}}]}
let definitionsByName: {
[typeName: string]: FederatedTypeDefinitionNode[];
} = (definitions as FederatedTypeDefinitionNode[]).reduce(
(typeToDefinitionsMap: TypeToDefinitionsMap, node) => {
const name = node.name.value;
if (typeToDefinitionsMap[name]) {
typeToDefinitionsMap[name].push(node);
} else {
typeToDefinitionsMap[name] = [node];
}
return typeToDefinitionsMap;
},
{},
);

// map over each group of definitions.
for (const [name, definitions] of Object.entries(definitionsByName)) {
// if every definition in the list is an enum, we don't need to error about type,
// but we do need to check to make sure every service has the same enum values
if (definitions.every(isEnumDefinition)) {
// a simple list of services to enum values for a given enum
// [{ serviceName: "serviceA", values: ["FURNITURE", "BOOK"] }]
let simpleEnumDefs: Array<{ serviceName: string; values: string[] }> = [];

// build the simpleEnumDefs list
for (const {
values,
serviceName,
} of definitions as FederatedEnumDefinition[]) {
if (serviceName && values)
simpleEnumDefs.push({
serviceName,
values: values.map(
(enumValue: EnumValueDefinitionNode) => enumValue.name.value,
),
});
}

// values need to be in order to build the matchingEnumGroups
for (const definition of simpleEnumDefs) {
definition.values = definition.values.sort();
}

// groups of services with matching values, keyed by enum values
// like {"FURNITURE,BOOK": ["ServiceA", "ServiceB"], "FURNITURE,DIGITAL": ["serviceC"]}
let matchingEnumGroups: { [values: string]: string[] } = {};

// build matchingEnumDefs
for (const definition of simpleEnumDefs) {
const key = definition.values.join();
if (matchingEnumGroups[key]) {
matchingEnumGroups[key].push(definition.serviceName);
} else {
matchingEnumGroups[key] = [definition.serviceName];
}
}

if (Object.keys(matchingEnumGroups).length > 1) {
context.reportError(
errorWithCode(
'ENUM_MISMATCH',
`The \`${name}\` enum does not have identical values in all services. Groups of services with identical values are: ${Object.values(
matchingEnumGroups,
)
.map(serviceNames => `[${serviceNames.join(', ')}]`)
.join(', ')}`,
),
);
}
} else if (definitions.some(isEnumDefinition)) {
// if only SOME definitions in the list are enums, we need to error

// first, find the services, where the defs ARE enums
const servicesWithEnum = definitions
.filter(isEnumDefinition)
.map(definition => definition.serviceName)
.filter(isString);

// find the services where the def isn't an enum
const servicesWithoutEnum = definitions
.filter(d => !isEnumDefinition(d))
.map(d => d.serviceName)
.filter(isString);

context.reportError(
errorWithCode(
'ENUM_MISMATCH_TYPE',
logServiceAndType(servicesWithEnum[0], name) +
`${name} is an enum in [${servicesWithEnum.join(
', ',
)}], but not in [${servicesWithoutEnum.join(', ')}]`,
),
);
}
}

// we don't need any visitors for this validation rule
return {};
}

0 comments on commit 8754e50

Please sign in to comment.