Uniquify and sanitize enum values in the join__Graph enum

federation-js/src/__tests__/joinSpec.test.ts

@@ -0,0 +1,45 @@
+import { fixtures } from 'apollo-federation-integration-testsuite';
+import { getJoins } from "../joinSpec";
+
+const questionableNamesRemap = {
+  accounts: 'ServiceA',
+  books: 'serviceA',
+  documents: 'servicea_2',
+  inventory: 'servicea_2_',
+  product: '9product*!',
+  reviews: 'reviews_9',
+};
+
+const fixturesWithQuestionableServiceNames = fixtures.map((service) => ({
+  ...service,
+  name: questionableNamesRemap[service.name],
+}));
+
+describe('join__Graph enum', () => {
+  it('correctly uniquifies and sanitizes service names', () => {
+    const { sanitizedServiceNames } = getJoins(
+      fixturesWithQuestionableServiceNames,
+    );
+
+    /**
+     * Expectations
+     * 1. Non-Alphanumeric characters are replaced with _ (9product*!)
+     * 2. Numeric first characters are prefixed with _ (9product*!)
+     * 3. Names ending in an underscore followed by numbers `_\d+` are suffixed with _ (reviews_9, servicea_2)
+     * 4. Names are uppercased (all)
+     * 5. After transformations 1-4, duplicates are suffixed with _{n} where {n} is number of times we've seen the dupe (ServiceA + serviceA, servicea_2 + servicea_2_)
+     *
+     * Miscellany
+     * (serviceA) tests the edge case of colliding with a name we generated
+     * (servicea_2_) tests a collision against (documents) post-transformation
+     */
+    expect(sanitizedServiceNames).toMatchObject({
+      '9product*!': '_9PRODUCT__',
+      ServiceA: 'SERVICEA',
+      reviews_9: 'REVIEWS_9_',
+      serviceA: 'SERVICEA_2',
+      servicea_2: 'SERVICEA_2_',
+      servicea_2_: 'SERVICEA_2__2',
+    });
+  })
+})

federation-js/src/joinSpec.ts

@@ -25,16 +25,64 @@ const JoinGraphDirective = new GraphQLDirective({
   }
 });
 
+/**
+ * Expectations
+ * 1. Non-Alphanumeric characters are replaced with _ (alphaNumericUnderscoreOnly)
+ * 2. Numeric first characters are prefixed with _ (noNumericFirstChar)
+ * 3. Names ending in an underscore followed by numbers `_\d+` are suffixed with _ (noUnderscoreNumericEnding)
+ * 4. Names are uppercased (toUpper)
+ * 5. After transformations 1-4, duplicates are suffixed with _{n} where {n} is number of times we've seen the dupe
+ *
+ * Note: Collisions with name's we've generated are also accounted for
+ */
 function getJoinGraphEnum(serviceList: ServiceDefinition[]) {
-  return new GraphQLEnumType({
-    name: 'join__Graph',
-    values: Object.fromEntries(
-      serviceList.map((service) => [
-        service.name.toUpperCase(),
-        { value: service },
-      ]),
-    ),
-  });
+  // Track whether we've seen a name and how many times
+  const nameMap: Map<string, number> = new Map();
+  // Build a map of original service name to generated name
+  const sanitizedServiceNames: Record<string, string> = Object.create(null);
+
+  function uniquifyAndSanitizeGraphQLName(name: string) {
+    // Transforms to ensure valid graphql `Name`
+    const alphaNumericUnderscoreOnly = name.replace(/[^_a-zA-Z0-9]/g, '_');
+    const noNumericFirstChar = alphaNumericUnderscoreOnly.match(/^[0-9]/)
+      ? '_' + alphaNumericUnderscoreOnly
+      : alphaNumericUnderscoreOnly;
+    const noUnderscoreNumericEnding = noNumericFirstChar.match(/_[0-9]+$/)
+      ? noNumericFirstChar + '_'
+      : noNumericFirstChar;
+
+    // toUpper not really necessary but follows convention of enum values
+    const toUpper = noUnderscoreNumericEnding.toLocaleUpperCase();
+
+    // Uniquifying post-transform
+    const nameCount = nameMap.get(toUpper);
+    if (nameCount) {
+      // Collision - bump counter by one
+      nameMap.set(toUpper, nameCount + 1);
+      const uniquified = `${toUpper}_${nameCount + 1}`;
+      // We also now need another entry for the name we just generated
+      nameMap.set(uniquified, 1);
+      sanitizedServiceNames[name] = uniquified;
+      return uniquified;
+    } else {
+      nameMap.set(toUpper, 1);
+      sanitizedServiceNames[name] = toUpper;
+      return toUpper;
+    }
+  }
+
+  return {
+    sanitizedServiceNames,
+    JoinGraphEnum: new GraphQLEnumType({
+      name: 'join__Graph',
+      values: Object.fromEntries(
+        serviceList.map((service) => [
+          uniquifyAndSanitizeGraphQLName(service.name),
+          { value: service },
+        ]),
+      ),
+    }),
+  };
 }
 
 function getJoinFieldDirective(JoinGraphEnum: GraphQLEnumType) {
@@ -68,7 +116,7 @@ function getJoinOwnerDirective(JoinGraphEnum: GraphQLEnumType) {
 }
 
 export function getJoins(serviceList: ServiceDefinition[]) {
-  const JoinGraphEnum = getJoinGraphEnum(serviceList);
+  const { sanitizedServiceNames, JoinGraphEnum } = getJoinGraphEnum(serviceList);
   const JoinFieldDirective = getJoinFieldDirective(JoinGraphEnum);
   const JoinOwnerDirective = getJoinOwnerDirective(JoinGraphEnum);
 
@@ -87,11 +135,12 @@ export function getJoins(serviceList: ServiceDefinition[]) {
   });
 
   return {
+    sanitizedServiceNames,
     FieldSetScalar,
     JoinTypeDirective,
     JoinFieldDirective,
     JoinOwnerDirective,
     JoinGraphEnum,
     JoinGraphDirective,
-  }
+  };
 }

federation-js/src/service/printCoreSchema.ts

@@ -44,6 +44,12 @@ type Options = {
   commentDescriptions?: boolean;
 };
 
+interface PrintingContext {
+  // Core addition: we need access to a map from serviceName to its corresponding
+  // sanitized / uniquified enum value `Name` from the `join__Graph` enum
+  sanitizedServiceNames: Record<string, string>;
+}
+
 /**
  * Accepts options as an optional third argument:
  *
@@ -65,7 +71,8 @@ export function printCoreSchema(
     JoinTypeDirective,
     JoinOwnerDirective,
     JoinGraphEnum,
-    JoinGraphDirective
+    JoinGraphDirective,
+    sanitizedServiceNames,
   } = getJoins(serviceList);
 
   schema = new GraphQLSchema({
@@ -81,11 +88,16 @@ export function printCoreSchema(
     types: [FieldSetScalar, JoinGraphEnum, ...config.types],
   });
 
+  const context: PrintingContext = {
+    sanitizedServiceNames,
+  }
+
   return printFilteredSchema(
     schema,
     (n) => !isSpecifiedDirective(n),
     isDefinedType,
     options,
+    context,
   );
 }
 
@@ -107,10 +119,11 @@ function isDefinedType(type: GraphQLNamedType): boolean {
 
 function printFilteredSchema(
   schema: GraphQLSchema,
-  // serviceList: ServiceDefinition[],
   directiveFilter: (type: GraphQLDirective) => boolean,
   typeFilter: (type: GraphQLNamedType) => boolean,
   options?: Options,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
 ): string {
   const directives = schema.getDirectives().filter(directiveFilter);
   const types = Object.values(schema.getTypeMap())
@@ -120,8 +133,8 @@ function printFilteredSchema(
   return (
     [printSchemaDefinition(schema)]
       .concat(
-        directives.map(directive => printDirective(directive, options)),
-        types.map(type => printType(type, options)),
+        directives.map((directive) => printDirective(directive, options)),
+        types.map((type) => printType(type, options, context)),
       )
       .filter(Boolean)
       .join('\n\n') + '\n'
@@ -161,11 +174,16 @@ function printCoreDirectives() {
   ].map((feature) => `\n  @core(feature: "${feature}")`);
 }
 
-export function printType(type: GraphQLNamedType, options?: Options): string {
+export function printType(
+  type: GraphQLNamedType,
+  options?: Options,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
+): string {
   if (isScalarType(type)) {
     return printScalar(type, options);
   } else if (isObjectType(type)) {
-    return printObject(type, options);
+    return printObject(type, options, context);
   } else if (isInterfaceType(type)) {
     return printInterface(type, options);
   } else if (isUnionType(type)) {
@@ -183,7 +201,12 @@ function printScalar(type: GraphQLScalarType, options?: Options): string {
   return printDescription(options, type) + `scalar ${type.name}`;
 }
 
-function printObject(type: GraphQLObjectType, options?: Options): string {
+function printObject(
+  type: GraphQLObjectType,
+  options?: Options,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
+): string {
   const interfaces = type.getInterfaces();
   const implementedInterfaces = interfaces.length
     ? ' implements ' + interfaces.map((i) => i.name).join(' & ')
@@ -194,13 +217,17 @@ function printObject(type: GraphQLObjectType, options?: Options): string {
     `type ${type.name}` +
     implementedInterfaces +
     // Core addition for printing @join__owner and @join__type usages
-    printTypeJoinDirectives(type) +
-    printFields(options, type)
+    printTypeJoinDirectives(type, context) +
+    printFields(options, type, context)
   );
 }
 
 // Core change: print @join__owner and @join__type usages
-function printTypeJoinDirectives(type: GraphQLObjectType): string {
+function printTypeJoinDirectives(
+  type: GraphQLObjectType,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
+): string {
   const metadata: FederationType = type.extensions?.federation;
   if (!metadata) return '';
 
@@ -209,32 +236,42 @@ function printTypeJoinDirectives(type: GraphQLObjectType): string {
 
   // Separate owner @keys from the rest of the @keys so we can print them
   // adjacent to the @owner directive.
-  const { [ownerService]: ownerKeys = [], ...restKeys } = keys
-  const ownerEntry: [string, (readonly SelectionNode[])[]] = [ownerService, ownerKeys];
+  const { [ownerService]: ownerKeys = [], ...restKeys } = keys;
+  const ownerEntry: [string, (readonly SelectionNode[])[]] = [
+    ownerService,
+    ownerKeys,
+  ];
   const restEntries = Object.entries(restKeys);
 
   return (
-    `\n  @join__owner(graph: ${ownerService.toUpperCase()})` +
+    `\n  @join__owner(graph: ${
+      context?.sanitizedServiceNames[ownerService] ?? ownerService
+    })` +
     [ownerEntry, ...restEntries]
       .map(([service, keys = []]) =>
         keys
           .map(
             (selections) =>
-              `\n  @join__type(graph: ${service.toUpperCase()}, key: "${printFieldSet(
-                selections,
-              )}")`,
+              `\n  @join__type(graph: ${
+                context?.sanitizedServiceNames[service] ?? service
+              }, key: "${printFieldSet(selections)}")`,
           )
           .join(''),
       )
       .join('')
   );
 }
 
-function printInterface(type: GraphQLInterfaceType, options?: Options): string {
+function printInterface(
+  type: GraphQLInterfaceType,
+  options?: Options,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
+): string {
   return (
     printDescription(options, type) +
     `interface ${type.name}` +
-    printFields(options, type)
+    printFields(options, type, context)
   );
 }
 
@@ -284,8 +321,9 @@ function printInputObject(
 function printFields(
   options: Options | undefined,
   type: GraphQLObjectType | GraphQLInterfaceType,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
 ) {
-
   const fields = Object.values(type.getFields()).map(
     (f, i) =>
       printDescription(options, f, '  ', !i) +
@@ -295,7 +333,7 @@ function printFields(
       ': ' +
       String(f.type) +
       printDeprecated(f) +
-      printJoinFieldDirectives(f, type),
+      printJoinFieldDirectives(f, type, context),
   );
 
   // Core change: for entities, we want to print the block on a new line.
@@ -329,6 +367,8 @@ function printFieldSet(selections: readonly SelectionNode[]): string {
 function printJoinFieldDirectives(
   field: GraphQLField<any, any>,
   parentType: GraphQLObjectType | GraphQLInterfaceType,
+  // Core addition - see `PrintingContext` type for details
+  context?: PrintingContext,
 ): string {
   let printed = ' @join__field(';
   // Fields on the owning service do not have any federation metadata applied
@@ -341,8 +381,18 @@ function printJoinFieldDirectives(
     // Because we print `@join__type` directives based on the keys, but only used to
     // look at the owning service here, that meant we would print `@join__field`
     // without a corresponding `@join__type`, which is invalid according to the spec.
-    if (parentType.extensions?.federation?.serviceName && parentType.extensions?.federation?.keys) {
-      return printed + `graph: ${parentType.extensions?.federation.serviceName.toUpperCase()})`;
+    if (
+      parentType.extensions?.federation?.serviceName &&
+      parentType.extensions?.federation?.keys
+    ) {
+      return (
+        printed +
+        `graph: ${
+          context?.sanitizedServiceNames[
+            parentType.extensions?.federation.serviceName
+          ] ?? parentType.extensions?.federation.serviceName
+        })`
+      );
     }
     return '';
   }
@@ -356,7 +406,9 @@ function printJoinFieldDirectives(
   let directiveArgs: string[] = [];
 
   if (serviceName && serviceName.length > 0) {
-    directiveArgs.push(`graph: ${serviceName.toUpperCase()}`);
+    directiveArgs.push(
+      `graph: ${context?.sanitizedServiceNames[serviceName] ?? serviceName}`,
+    );
   }
 
   if (requires.length > 0) {