fix(federation): Create new @key validations (#4498)

A few cases of invalid @key directive arrangements are currently possible
with our existing validation suite. This PR adds 3 new validations
to ensure that users are informed of these invalid states and block
composition appropriately.

* Owning type must specify a @key
* Extending types can't specify multiple @keys
* Extending types must use a @key specified by the owning type

Additionally:
These invalid arrangements can result in runtime errors when printing CSDL,
nor is it really valid to return an "attempted" print of CSDL, so the result
of `composedSdl` is now `undefined` in the case that there are composition
errors.

docs/source/federation/errors.md

@@ -24,6 +24,9 @@ If Apollo Gateway encounters an error, composition fails. This document lists co
 | `KEY_FIELDS_SELECT_INVALID_TYPE`  | The `fields` argument of an entity's `@key` includes at least one root field that results in a list, interface, or union type. Root fields of these types cannot be part of a `@key`. |
 | `KEY_FIELDS_MISSING_ON_BASE` | The `fields` argument of an entity's `@key` includes at least one field that's also defined in another service. Each field of an entity should be defined in exactly one service. |
 | `KEY_FIELDS_MISSING_EXTERNAL` | An implementing service is attempting to `extend` another service's entity, but its `@key` includes at least one field that is not marked as `@external`. |
+| `KEY_MISSING_ON_BASE` | An implementing service is attempting to `extend` another service's entity, but the originating service hasn't defined a `@key` for the entity. |
+| `MULTIPLE_KEYS_ON_EXTENSION` | An implementing service is attempting to `extend` another service's entity, but it's specified multiple `@key` directives. Extending services can only use one of the `@key`s specified by the originating service. |
+| `KEY_NOT_SPECIFIED` | An implementing service is attempting to `extend` another service's entity, but it is using a `@key` that is not specified in the originating service. Valid `@key`s are specified by the owning service. |
 
 ## `@external`
 

packages/apollo-federation/CHANGELOG.md

@@ -7,6 +7,7 @@
 - __FIX__: CSDL complex `@key`s shouldn't result in an unparseable document [PR #4490](https://github.com/apollographql/apollo-server/pull/4490)
 - __FIX__: Value type validations - restrict unions, scalars, enums [PR #4496](https://github.com/apollographql/apollo-server/pull/4496)
 - __FIX__: Composition - aggregate interfaces for types and interfaces in composed schema [PR #4497](https://github.com/apollographql/apollo-server/pull/4497)
+- __FIX__: Create new `@key` validations to prevent invalid compositions [PR #4498](https://github.com/apollographql/apollo-server/pull/4498)
 
 ## v0.19.1
 

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

@@ -31,7 +31,11 @@ export function composeAndValidate(serviceList: ServiceDefinition[]) {
     }),
   );
 
-  const composedSdl = printComposedSdl(compositionResult.schema, serviceList);
+  // We shouldn't try to print the SDL if there were errors during composition
+  const composedSdl =
+    errors.length === 0
+      ? printComposedSdl(compositionResult.schema, serviceList)
+      : undefined;
 
   // TODO remove the warnings array once no longer used by clients
   return {

packages/apollo-federation/src/composition/validate/postComposition/__tests__/keysMatchBaseService.test.ts

@@ -0,0 +1,117 @@
+import gql from 'graphql-tag';
+import { composeServices } from '../../../compose';
+import { keysMatchBaseService as validateKeysMatchBaseService } from '../';
+import { graphqlErrorSerializer } from '../../../../snapshotSerializers';
+
+expect.addSnapshotSerializer(graphqlErrorSerializer);
+
+describe('keysMatchBaseService', () => {
+  it('returns no errors with proper @key usage', () => {
+    const serviceA = {
+      typeDefs: gql`
+        type Product @key(fields: "sku") {
+          sku: String!
+          upc: String!
+        }
+      `,
+      name: 'serviceA',
+    };
+
+    const serviceB = {
+      typeDefs: gql`
+        extend type Product @key(fields: "sku") {
+          sku: String! @external
+          price: Int!
+        }
+      `,
+      name: 'serviceB',
+    };
+
+    const serviceList = [serviceA, serviceB];
+    const { schema, errors } = composeServices(serviceList);
+    expect(errors).toHaveLength(0);
+
+    const validationErrors = validateKeysMatchBaseService({
+      schema,
+      serviceList,
+    });
+    expect(validationErrors).toHaveLength(0);
+  });
+
+  it('requires a @key to be specified on the originating type', () => {
+    const serviceA = {
+      typeDefs: gql`
+        type Product {
+          sku: String!
+          upc: String!
+        }
+      `,
+      name: 'serviceA',
+    };
+
+    const serviceB = {
+      typeDefs: gql`
+        extend type Product @key(fields: "sku") {
+          sku: String! @external
+          price: Int!
+        }
+      `,
+      name: 'serviceB',
+    };
+
+    const serviceList = [serviceA, serviceB];
+    const { schema, errors } = composeServices(serviceList);
+    expect(errors).toHaveLength(0);
+
+    const validationErrors = validateKeysMatchBaseService({
+      schema,
+      serviceList,
+    });
+    expect(validationErrors).toHaveLength(1);
+    expect(validationErrors[0]).toMatchInlineSnapshot(`
+      Object {
+        "code": "KEY_MISSING_ON_BASE",
+        "message": "[serviceA] Product -> appears to be an entity but no @key directives are specified on the originating type.",
+      }
+    `);
+  });
+
+  it('requires extending services to use a @key specified by the originating type', () => {
+    const serviceA = {
+      typeDefs: gql`
+        type Product @key(fields: "sku upc") {
+          sku: String!
+          upc: String!
+        }
+      `,
+      name: 'serviceA',
+    };
+
+    const serviceB = {
+      typeDefs: gql`
+        extend type Product @key(fields: "sku") {
+          sku: String! @external
+          price: Int!
+        }
+      `,
+      name: 'serviceB',
+    };
+
+    const serviceList = [serviceA, serviceB];
+    const { schema, errors } = composeServices(serviceList);
+    expect(errors).toHaveLength(0);
+
+    const validationErrors = validateKeysMatchBaseService({
+      schema,
+      serviceList,
+    });
+    expect(validationErrors).toHaveLength(1);
+    expect(validationErrors[0]).toMatchInlineSnapshot(`
+      Object {
+        "code": "KEY_NOT_SPECIFIED",
+        "message": "[serviceB] Product -> extends from serviceA but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
+      	@key(fields: \\"sku upc\\")",
+      }
+    `);
+  });
+});

packages/apollo-federation/src/composition/validate/postComposition/externalUnused.ts

@@ -25,6 +25,14 @@ export const externalUnused: PostCompositionValidator = ({ schema }) => {
     // If externals is populated, we need to look at each one and confirm
     // it is used
     const typeFederationMetadata = getFederationMetadata(parentType);
+
+    // Escape a validation case that's falling through incorrectly. This case
+    // is handled by `keysMatchBaseService`.
+    if (typeFederationMetadata) {
+      const {serviceName, keys} = typeFederationMetadata;
+      if (serviceName && keys && !keys[serviceName]) continue;
+    }
+
     if (typeFederationMetadata?.externals) {
       // loop over every service that has extensions with @external
       for (const [serviceName, externalFieldsForService] of Object.entries(

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

@@ -17,6 +17,7 @@ export {
   executableDirectivesInAllServices,
 } from './executableDirectivesInAllServices';
 export { executableDirectivesIdentical } from './executableDirectivesIdentical';
+export { keysMatchBaseService } from './keysMatchBaseService';
 
 export type PostCompositionValidator = ({
   schema,

packages/apollo-federation/src/composition/validate/postComposition/keysMatchBaseService.ts

@@ -0,0 +1,86 @@
+import { isObjectType, GraphQLError, SelectionNode } from 'graphql';
+import {
+  logServiceAndType,
+  errorWithCode,
+  getFederationMetadata,
+} from '../../utils';
+import { PostCompositionValidator } from '.';
+import { printWithReducedWhitespace } from '../../../service';
+
+/**
+ *  1. KEY_MISSING_ON_BASE - Originating types must specify at least 1 @key directive
+ *  2. MULTIPLE_KEYS_ON_EXTENSION - Extending services may not use more than 1 @key directive
+ *  3. KEY_NOT_SPECIFIED - Extending services must use a valid @key specified by the originating type
+ */
+export const keysMatchBaseService: PostCompositionValidator = function ({
+  schema,
+}) {
+  const errors: GraphQLError[] = [];
+  const types = schema.getTypeMap();
+  for (const [parentTypeName, parentType] of Object.entries(types)) {
+    // Only object types have fields
+    if (!isObjectType(parentType)) continue;
+
+    const typeFederationMetadata = getFederationMetadata(parentType);
+
+    if (typeFederationMetadata) {
+      const { serviceName, keys } = typeFederationMetadata;
+
+      if (serviceName && keys) {
+        if (!keys[serviceName]) {
+          errors.push(
+            errorWithCode(
+              'KEY_MISSING_ON_BASE',
+              logServiceAndType(serviceName, parentTypeName) +
+                `appears to be an entity but no @key directives are specified on the originating type.`,
+            ),
+          );
+          continue;
+        }
+
+        const availableKeys = keys[serviceName].map(printFieldSet);
+        Object.entries(keys)
+          // No need to validate that the owning service matches its specified keys
+          .filter(([service]) => service !== serviceName)
+          .forEach(([extendingService, keyFields]) => {
+            // Extensions can't specify more than one key
+            if (keyFields.length > 1) {
+              errors.push(
+                errorWithCode(
+                  'MULTIPLE_KEYS_ON_EXTENSION',
+                  logServiceAndType(extendingService, parentTypeName) +
+                    `is extended from service ${serviceName} but specifies multiple @key directives. Extensions may only specify one @key.`,
+                ),
+              );
+              return;
+            }
+
+            // This isn't representative of an invalid graph, but it is an existing
+            // limitation of the query planner that we want to validate against for now.
+            // In the future, `@key`s just need to be "reachable" through a number of
+            // services which can link one key to another via "joins".
+            const extensionKey = printFieldSet(keyFields[0]);
+            if (!availableKeys.includes(extensionKey)) {
+              errors.push(
+                errorWithCode(
+                  'KEY_NOT_SPECIFIED',
+                  logServiceAndType(extendingService, parentTypeName) +
+                    `extends from ${serviceName} but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:\n` +
+                    `\t${availableKeys
+                      .map((fieldSet) => `@key(fields: "${fieldSet}")`)
+                      .join('\n\t')}`,
+                ),
+              );
+              return;
+            }
+          });
+      }
+    }
+  }
+
+  return errors;
+};
+
+function printFieldSet(selections: readonly SelectionNode[]): string {
+  return selections.map(printWithReducedWhitespace).join(' ');
+}