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

Handle duplicate enums across services #2829

Merged
merged 3 commits into from
Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
353 changes: 156 additions & 197 deletions packages/apollo-federation/src/composition/__tests__/compose.test.ts

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions packages/apollo-federation/src/composition/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
GraphQLSchema,
extendSchema,
Kind,
TypeDefinitionNode,
TypeExtensionNode,
isTypeDefinitionNode,
isTypeExtensionNode,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -240,6 +240,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
kind: Kind.OBJECT_TYPE_DEFINITION,
name: { kind: Kind.NAME, value: extensionTypeName },
fields: [],
serviceName: null,
},
];

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

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

export const compositionRules = specifiedSDLRules.filter(
rule => rule.name !== 'UniqueDirectivesPerLocation',
);
import {
UniqueTypeNamesWithoutEnumsOrScalars,
MatchingEnums,
} from './validate/sdl';

const omit = [
'UniqueDirectivesPerLocation',
'UniqueTypeNames',
'UniqueEnumValueNames',
];

export const compositionRules = specifiedSDLRules
.filter(rule => !omit.includes(rule.name))
.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;

Expand Down Expand Up @@ -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
@@ -0,0 +1,97 @@
import gql from 'graphql-tag';
import { duplicateEnumOrScalar as validateDuplicateEnumOrScalar } from '../';
import { graphqlErrorSerializer } from '../../../../snapshotSerializers';

expect.addSnapshotSerializer(graphqlErrorSerializer);

describe('duplicateEnumOrScalar', () => {
it('does not error with proper enum and scalar usage', () => {
const serviceA = {
typeDefs: gql`
type Product @key(fields: "color { id value }") {
sku: String!
upc: String!
shippingDate: Date
type: ProductType
}
enum ProductType {
BOOK
FURNITURE
}
extend enum ProductType {
DIGITAL
}
scalar Date
`,
name: 'serviceA',
};

const warnings = validateDuplicateEnumOrScalar(serviceA);
expect(warnings).toEqual([]);
});
it('errors when there are multiple definitions of the same enum', () => {
const serviceA = {
typeDefs: gql`
type Product @key(fields: "color { id value }") {
sku: String!
upc: String!
color: Color!
}
type Color {
id: ID!
value: String!
}
enum ProductType {
BOOK
FURNITURE
}
enum ProductType {
DIGITAL
}
`,
name: 'serviceA',
};

const warnings = validateDuplicateEnumOrScalar(serviceA);
expect(warnings).toMatchInlineSnapshot(`
Array [
Object {
"code": "DUPLICATE_ENUM_DEFINITION",
"message": "[serviceA] ProductType -> The enum, \`ProductType\` was defined multiple times in this service. Remove one of the definitions for \`ProductType\`",
},
]
`);
});

it('errors when there are multiple definitions of the same scalar', () => {
const serviceA = {
typeDefs: gql`
scalar Date
type Product @key(fields: "color { id value }") {
sku: String!
upc: String!
deliveryDate: Date
}
scalar Date
`,
name: 'serviceA',
};

const warnings = validateDuplicateEnumOrScalar(serviceA);
expect(warnings).toMatchInlineSnapshot(`
Array [
Object {
"code": "DUPLICATE_SCALAR_DEFINITION",
"message": "[serviceA] Date -> The scalar, \`Date\` was defined multiple times in this service. Remove one of the definitions for \`Date\`",
},
]
`);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import gql from 'graphql-tag';
import { duplicateEnumValue as validateDuplicateEnumValue } from '../';
import { graphqlErrorSerializer } from '../../../../snapshotSerializers';

expect.addSnapshotSerializer(graphqlErrorSerializer);

describe('duplicateEnumValue', () => {
it('does not error with proper enum usage', () => {
const serviceA = {
typeDefs: gql`
type Product @key(fields: "color { id value }") {
sku: String!
upc: String!
color: Color!
}
type Color {
id: ID!
value: String!
}
enum ProductType {
BOOK
FURNITURE
}
extend enum ProductType {
DIGITAL
}
`,
name: 'serviceA',
};

const warnings = validateDuplicateEnumValue(serviceA);
expect(warnings).toEqual([]);
});
it('errors when there are duplicate enum values in a single service', () => {
const serviceA = {
typeDefs: gql`
type Product @key(fields: "color { id value }") {
sku: String!
upc: String!
color: Color!
}
type Color {
id: ID!
value: String!
}
enum ProductType {
BOOK
FURNITURE
}
extend enum ProductType {
DIGITAL
BOOK
}
`,
name: 'serviceA',
};

const warnings = validateDuplicateEnumValue(serviceA);
expect(warnings).toMatchInlineSnapshot(`
Array [
Object {
"code": "DUPLICATE_ENUM_VALUE",
"message": "[serviceA] ProductType.BOOK -> The enum, \`ProductType\` has multiple definitions of the \`BOOK\` value.",
},
]
`);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { visit, GraphQLError } from 'graphql';
import { ServiceDefinition } from '../../types';

import { logServiceAndType, errorWithCode } from '../../utils';

export const duplicateEnumOrScalar = ({
name: serviceName,
typeDefs,
}: ServiceDefinition) => {
const errors: GraphQLError[] = [];

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

visit(typeDefs, {
EnumTypeDefinition(definition) {
const name = definition.name.value;
if (enums.includes(name)) {
errors.push(
errorWithCode(
'DUPLICATE_ENUM_DEFINITION',
logServiceAndType(serviceName, name) +
`The enum, \`${name}\` was defined multiple times in this service. Remove one of the definitions for \`${name}\``,
),
);
return definition;
}
enums.push(name);
return definition;
},
ScalarTypeDefinition(definition) {
const name = definition.name.value;
if (scalars.includes(name)) {
errors.push(
errorWithCode(
'DUPLICATE_SCALAR_DEFINITION',
logServiceAndType(serviceName, name) +
`The scalar, \`${name}\` was defined multiple times in this service. Remove one of the definitions for \`${name}\``,
),
);
return definition;
}
scalars.push(name);
return definition;
},
});

return errors;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { visit, GraphQLError } from 'graphql';
import { ServiceDefinition } from '../../types';

import { logServiceAndType, errorWithCode } from '../../utils';

export const duplicateEnumValue = ({
name: serviceName,
typeDefs,
}: ServiceDefinition) => {
const errors: GraphQLError[] = [];

const enums: { [name: string]: string[] } = {};

visit(typeDefs, {
EnumTypeDefinition(definition) {
const name = definition.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 => {
if (enums[name].includes(valueName)) {
errors.push(
errorWithCode(
'DUPLICATE_ENUM_VALUE',
logServiceAndType(serviceName, name, valueName) +
`The enum, \`${name}\` has multiple definitions of the \`${valueName}\` value.`,
),
);
return;
}
enums[name].push(valueName);
});
} else {
enums[name] = enumValues;
}

return definition;
},
EnumTypeExtension(definition) {
const name = definition.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 => {
if (enums[name].includes(valueName)) {
errors.push(
errorWithCode(
'DUPLICATE_ENUM_VALUE',
logServiceAndType(serviceName, name, valueName) +
`The enum, \`${name}\` has multiple definitions of the \`${valueName}\` value.`,
),
);
return;
}
enums[name].push(valueName);
});
} else {
enums[name] = enumValues;
}

return definition;
},
});

return errors;
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export { externalUsedOnBase } from './externalUsedOnBase';
export { requiresUsedOnBase } from './requiresUsedOnBase';
export { keyFieldsMissingExternal } from './keyFieldsMissingExternal';
export { reservedFieldUsed } from './reservedFieldUsed';
export { duplicateEnumOrScalar } from './duplicateEnumOrScalar';
export { duplicateEnumValue } from './duplicateEnumValue';
Loading