Merge pull request #2829 from apollographql/jake/fed-duplicate-enums

Handle duplicate enums across services
apollographql · Jun 14, 2019 · b635f17 · b635f17
2 parents 4e4aa81 + 67dc0ab
commit b635f17
Show file tree

Hide file tree

Showing 13 changed files with 874 additions and 205 deletions.
diff --git a/packages/apollo-federation/src/composition/__tests__/compose.test.ts b/packages/apollo-federation/src/composition/__tests__/compose.test.ts
diff --git a/packages/apollo-federation/src/composition/compose.ts b/packages/apollo-federation/src/composition/compose.ts
@@ -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],

diff --git a/packages/apollo-federation/src/composition/rules.ts b/packages/apollo-federation/src/composition/rules.ts
@@ -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]);
diff --git a/packages/apollo-federation/src/composition/types.ts b/packages/apollo-federation/src/composition/types.ts
@@ -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;
+};
diff --git a/...ederation/src/composition/validate/preComposition/__tests__/duplicateEnumOrScalar.test.ts b/...ederation/src/composition/validate/preComposition/__tests__/duplicateEnumOrScalar.test.ts
@@ -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\`",
+        },
+      ]
+    `);
+  });
+});
diff --git a/...o-federation/src/composition/validate/preComposition/__tests__/duplicateEnumValue.test.ts b/...o-federation/src/composition/validate/preComposition/__tests__/duplicateEnumValue.test.ts
@@ -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.",
+        },
+      ]
+    `);
+  });
+});
diff --git a/packages/apollo-federation/src/composition/validate/preComposition/duplicateEnumOrScalar.ts b/packages/apollo-federation/src/composition/validate/preComposition/duplicateEnumOrScalar.ts
@@ -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;
+};
diff --git a/packages/apollo-federation/src/composition/validate/preComposition/duplicateEnumValue.ts b/packages/apollo-federation/src/composition/validate/preComposition/duplicateEnumValue.ts
@@ -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;
+};
diff --git a/packages/apollo-federation/src/composition/validate/preComposition/index.ts b/packages/apollo-federation/src/composition/validate/preComposition/index.ts
@@ -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';