Skip to content

Commit b635f17

Browse files
author
James Baxley
authored
Merge pull request #2829 from apollographql/jake/fed-duplicate-enums
Handle duplicate enums across services
2 parents 4e4aa81 + 67dc0ab commit b635f17

13 files changed

Lines changed: 874 additions & 205 deletions

File tree

packages/apollo-federation/src/composition/__tests__/compose.test.ts

Lines changed: 156 additions & 197 deletions
Large diffs are not rendered by default.

packages/apollo-federation/src/composition/compose.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
GraphQLSchema,
55
extendSchema,
66
Kind,
7-
TypeDefinitionNode,
87
TypeExtensionNode,
98
isTypeDefinitionNode,
109
isTypeExtensionNode,
@@ -33,13 +32,14 @@ import {
3332
ServiceName,
3433
ExternalFieldDefinition,
3534
ServiceNameToKeyDirectivesMap,
35+
FederatedTypeDefinitionNode,
3636
} from './types';
3737
import { validateSDL } from 'graphql/validation/validate';
3838
import { compositionRules } from './rules';
3939

4040
// Map of all definitions to eventually be passed to extendSchema
4141
interface DefinitionsMap {
42-
[name: string]: TypeDefinitionNode[];
42+
[name: string]: FederatedTypeDefinitionNode[];
4343
}
4444
// Map of all extensions to eventually be passed to extendSchema
4545
interface ExtensionsMap {
@@ -157,9 +157,9 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
157157
* take precedence). If not, create the definitions array and add it to the definitionsMap.
158158
*/
159159
if (definitionsMap[typeName]) {
160-
definitionsMap[typeName].push(definition);
160+
definitionsMap[typeName].push({ ...definition, serviceName });
161161
} else {
162-
definitionsMap[typeName] = [definition];
162+
definitionsMap[typeName] = [{ ...definition, serviceName }];
163163
}
164164
} else if (isTypeExtensionNode(definition)) {
165165
const typeName = definition.name.value;
@@ -240,6 +240,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
240240
kind: Kind.OBJECT_TYPE_DEFINITION,
241241
name: { kind: Kind.NAME, value: extensionTypeName },
242242
fields: [],
243+
serviceName: null,
243244
},
244245
];
245246

@@ -266,6 +267,7 @@ export function buildSchemaFromDefinitionsAndExtensions({
266267
extensionsMap: ExtensionsMap;
267268
}) {
268269
let errors: GraphQLError[] | undefined = undefined;
270+
269271
let schema = new GraphQLSchema({
270272
query: undefined,
271273
directives: [...specifiedDirectives, ...federationDirectives],
Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
import { specifiedSDLRules } from 'graphql/validation/specifiedRules';
22

3-
export const compositionRules = specifiedSDLRules.filter(
4-
rule => rule.name !== 'UniqueDirectivesPerLocation',
5-
);
3+
import {
4+
UniqueTypeNamesWithoutEnumsOrScalars,
5+
MatchingEnums,
6+
} from './validate/sdl';
7+
8+
const omit = [
9+
'UniqueDirectivesPerLocation',
10+
'UniqueTypeNames',
11+
'UniqueEnumValueNames',
12+
];
13+
14+
export const compositionRules = specifiedSDLRules
15+
.filter(rule => !omit.includes(rule.name))
16+
.concat([UniqueTypeNamesWithoutEnumsOrScalars, MatchingEnums]);

packages/apollo-federation/src/composition/types.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { SelectionNode, DocumentNode, FieldDefinitionNode } from 'graphql';
1+
import {
2+
SelectionNode,
3+
DocumentNode,
4+
FieldDefinitionNode,
5+
TypeDefinitionNode,
6+
} from 'graphql';
27

38
export type ServiceName = string | null;
49

@@ -74,3 +79,7 @@ declare module 'graphql/type/definition' {
7479
federation?: FederationField;
7580
}
7681
}
82+
83+
export type FederatedTypeDefinitionNode = TypeDefinitionNode & {
84+
serviceName: string | null;
85+
};
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import gql from 'graphql-tag';
2+
import { duplicateEnumOrScalar as validateDuplicateEnumOrScalar } from '../';
3+
import { graphqlErrorSerializer } from '../../../../snapshotSerializers';
4+
5+
expect.addSnapshotSerializer(graphqlErrorSerializer);
6+
7+
describe('duplicateEnumOrScalar', () => {
8+
it('does not error with proper enum and scalar usage', () => {
9+
const serviceA = {
10+
typeDefs: gql`
11+
type Product @key(fields: "color { id value }") {
12+
sku: String!
13+
upc: String!
14+
shippingDate: Date
15+
type: ProductType
16+
}
17+
18+
enum ProductType {
19+
BOOK
20+
FURNITURE
21+
}
22+
23+
extend enum ProductType {
24+
DIGITAL
25+
}
26+
27+
scalar Date
28+
`,
29+
name: 'serviceA',
30+
};
31+
32+
const warnings = validateDuplicateEnumOrScalar(serviceA);
33+
expect(warnings).toEqual([]);
34+
});
35+
it('errors when there are multiple definitions of the same enum', () => {
36+
const serviceA = {
37+
typeDefs: gql`
38+
type Product @key(fields: "color { id value }") {
39+
sku: String!
40+
upc: String!
41+
color: Color!
42+
}
43+
44+
type Color {
45+
id: ID!
46+
value: String!
47+
}
48+
49+
enum ProductType {
50+
BOOK
51+
FURNITURE
52+
}
53+
54+
enum ProductType {
55+
DIGITAL
56+
}
57+
`,
58+
name: 'serviceA',
59+
};
60+
61+
const warnings = validateDuplicateEnumOrScalar(serviceA);
62+
expect(warnings).toMatchInlineSnapshot(`
63+
Array [
64+
Object {
65+
"code": "DUPLICATE_ENUM_DEFINITION",
66+
"message": "[serviceA] ProductType -> The enum, \`ProductType\` was defined multiple times in this service. Remove one of the definitions for \`ProductType\`",
67+
},
68+
]
69+
`);
70+
});
71+
72+
it('errors when there are multiple definitions of the same scalar', () => {
73+
const serviceA = {
74+
typeDefs: gql`
75+
scalar Date
76+
type Product @key(fields: "color { id value }") {
77+
sku: String!
78+
upc: String!
79+
deliveryDate: Date
80+
}
81+
82+
scalar Date
83+
`,
84+
name: 'serviceA',
85+
};
86+
87+
const warnings = validateDuplicateEnumOrScalar(serviceA);
88+
expect(warnings).toMatchInlineSnapshot(`
89+
Array [
90+
Object {
91+
"code": "DUPLICATE_SCALAR_DEFINITION",
92+
"message": "[serviceA] Date -> The scalar, \`Date\` was defined multiple times in this service. Remove one of the definitions for \`Date\`",
93+
},
94+
]
95+
`);
96+
});
97+
});
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import gql from 'graphql-tag';
2+
import { duplicateEnumValue as validateDuplicateEnumValue } from '../';
3+
import { graphqlErrorSerializer } from '../../../../snapshotSerializers';
4+
5+
expect.addSnapshotSerializer(graphqlErrorSerializer);
6+
7+
describe('duplicateEnumValue', () => {
8+
it('does not error with proper enum usage', () => {
9+
const serviceA = {
10+
typeDefs: gql`
11+
type Product @key(fields: "color { id value }") {
12+
sku: String!
13+
upc: String!
14+
color: Color!
15+
}
16+
17+
type Color {
18+
id: ID!
19+
value: String!
20+
}
21+
22+
enum ProductType {
23+
BOOK
24+
FURNITURE
25+
}
26+
27+
extend enum ProductType {
28+
DIGITAL
29+
}
30+
`,
31+
name: 'serviceA',
32+
};
33+
34+
const warnings = validateDuplicateEnumValue(serviceA);
35+
expect(warnings).toEqual([]);
36+
});
37+
it('errors when there are duplicate enum values in a single service', () => {
38+
const serviceA = {
39+
typeDefs: gql`
40+
type Product @key(fields: "color { id value }") {
41+
sku: String!
42+
upc: String!
43+
color: Color!
44+
}
45+
46+
type Color {
47+
id: ID!
48+
value: String!
49+
}
50+
51+
enum ProductType {
52+
BOOK
53+
FURNITURE
54+
}
55+
56+
extend enum ProductType {
57+
DIGITAL
58+
BOOK
59+
}
60+
`,
61+
name: 'serviceA',
62+
};
63+
64+
const warnings = validateDuplicateEnumValue(serviceA);
65+
expect(warnings).toMatchInlineSnapshot(`
66+
Array [
67+
Object {
68+
"code": "DUPLICATE_ENUM_VALUE",
69+
"message": "[serviceA] ProductType.BOOK -> The enum, \`ProductType\` has multiple definitions of the \`BOOK\` value.",
70+
},
71+
]
72+
`);
73+
});
74+
});
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { visit, GraphQLError } from 'graphql';
2+
import { ServiceDefinition } from '../../types';
3+
4+
import { logServiceAndType, errorWithCode } from '../../utils';
5+
6+
export const duplicateEnumOrScalar = ({
7+
name: serviceName,
8+
typeDefs,
9+
}: ServiceDefinition) => {
10+
const errors: GraphQLError[] = [];
11+
12+
// keep track of every enum and scalar and error if there are ever duplicates
13+
const enums: string[] = [];
14+
const scalars: string[] = [];
15+
16+
visit(typeDefs, {
17+
EnumTypeDefinition(definition) {
18+
const name = definition.name.value;
19+
if (enums.includes(name)) {
20+
errors.push(
21+
errorWithCode(
22+
'DUPLICATE_ENUM_DEFINITION',
23+
logServiceAndType(serviceName, name) +
24+
`The enum, \`${name}\` was defined multiple times in this service. Remove one of the definitions for \`${name}\``,
25+
),
26+
);
27+
return definition;
28+
}
29+
enums.push(name);
30+
return definition;
31+
},
32+
ScalarTypeDefinition(definition) {
33+
const name = definition.name.value;
34+
if (scalars.includes(name)) {
35+
errors.push(
36+
errorWithCode(
37+
'DUPLICATE_SCALAR_DEFINITION',
38+
logServiceAndType(serviceName, name) +
39+
`The scalar, \`${name}\` was defined multiple times in this service. Remove one of the definitions for \`${name}\``,
40+
),
41+
);
42+
return definition;
43+
}
44+
scalars.push(name);
45+
return definition;
46+
},
47+
});
48+
49+
return errors;
50+
};
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { visit, GraphQLError } from 'graphql';
2+
import { ServiceDefinition } from '../../types';
3+
4+
import { logServiceAndType, errorWithCode } from '../../utils';
5+
6+
export const duplicateEnumValue = ({
7+
name: serviceName,
8+
typeDefs,
9+
}: ServiceDefinition) => {
10+
const errors: GraphQLError[] = [];
11+
12+
const enums: { [name: string]: string[] } = {};
13+
14+
visit(typeDefs, {
15+
EnumTypeDefinition(definition) {
16+
const name = definition.name.value;
17+
const enumValues =
18+
definition.values && definition.values.map(value => value.name.value);
19+
20+
if (!enumValues) return definition;
21+
22+
if (enums[name] && enums[name].length) {
23+
enumValues.map(valueName => {
24+
if (enums[name].includes(valueName)) {
25+
errors.push(
26+
errorWithCode(
27+
'DUPLICATE_ENUM_VALUE',
28+
logServiceAndType(serviceName, name, valueName) +
29+
`The enum, \`${name}\` has multiple definitions of the \`${valueName}\` value.`,
30+
),
31+
);
32+
return;
33+
}
34+
enums[name].push(valueName);
35+
});
36+
} else {
37+
enums[name] = enumValues;
38+
}
39+
40+
return definition;
41+
},
42+
EnumTypeExtension(definition) {
43+
const name = definition.name.value;
44+
const enumValues =
45+
definition.values && definition.values.map(value => value.name.value);
46+
47+
if (!enumValues) return definition;
48+
49+
if (enums[name] && enums[name].length) {
50+
enumValues.map(valueName => {
51+
if (enums[name].includes(valueName)) {
52+
errors.push(
53+
errorWithCode(
54+
'DUPLICATE_ENUM_VALUE',
55+
logServiceAndType(serviceName, name, valueName) +
56+
`The enum, \`${name}\` has multiple definitions of the \`${valueName}\` value.`,
57+
),
58+
);
59+
return;
60+
}
61+
enums[name].push(valueName);
62+
});
63+
} else {
64+
enums[name] = enumValues;
65+
}
66+
67+
return definition;
68+
},
69+
});
70+
71+
return errors;
72+
};

packages/apollo-federation/src/composition/validate/preComposition/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@ export { externalUsedOnBase } from './externalUsedOnBase';
22
export { requiresUsedOnBase } from './requiresUsedOnBase';
33
export { keyFieldsMissingExternal } from './keyFieldsMissingExternal';
44
export { reservedFieldUsed } from './reservedFieldUsed';
5+
export { duplicateEnumOrScalar } from './duplicateEnumOrScalar';
6+
export { duplicateEnumValue } from './duplicateEnumValue';

0 commit comments

Comments
 (0)