Merge pull request #859 from apollographql/trevor/print-tag-inaccessi…

…ble-directives-conditionally

fix(federation): Print @tag and @inaccessible directives conditionally

federation-integration-testsuite-js/src/fixtures/index.ts

@@ -4,7 +4,9 @@ import * as documents from './documents';
 import * as inventory from './inventory';
 import * as product from './product';
 import * as reviews from './reviews';
-import * as reviewsWithUpdate from './reviewsWithUpdate'
+import * as reviewsWithUpdate from './special-cases/reviewsWithUpdate';
+import * as accountsWithoutTagOrInaccessible from './special-cases/accountsWithoutTagOrInaccessible';
+import * as reviewsWithoutTagOrInaccessible from './special-cases/reviewsWithoutTagOrInaccessible';
 
 export {
   accounts,
@@ -13,7 +15,7 @@ export {
   inventory,
   product,
   reviews,
-  reviewsWithUpdate
+  reviewsWithUpdate,
 };
 
 export const fixtures = [
@@ -34,6 +36,15 @@ export const fixturesWithUpdate = [
   reviewsWithUpdate,
 ];
 
+export const fixturesWithoutTagOrInaccessible = [
+  accountsWithoutTagOrInaccessible,
+  books,
+  documents,
+  inventory,
+  product,
+  reviewsWithoutTagOrInaccessible,
+];
+
 export const fixtureNames = [
   accounts.name,
   product.name,

federation-integration-testsuite-js/src/fixtures/special-cases/accountsWithoutTagOrInaccessible.ts

@@ -0,0 +1,58 @@
+import gql from 'graphql-tag';
+
+export { name, url, resolvers } from '../accounts';
+export const typeDefs = gql`
+  directive @stream on FIELD
+  directive @transform(from: String!) on FIELD
+
+  schema {
+    query: RootQuery
+    mutation: Mutation
+  }
+
+  extend type RootQuery {
+    user(id: ID!): User
+    me: User
+  }
+
+  type PasswordAccount @key(fields: "email") {
+    email: String!
+  }
+
+  type SMSAccount @key(fields: "number") {
+    number: String
+  }
+
+  union AccountType = PasswordAccount | SMSAccount
+
+  type UserMetadata {
+    name: String
+    address: String
+    description: String
+  }
+
+  type User @key(fields: "id") @key(fields: "username name { first last }") {
+    id: ID!
+    name: Name
+    username: String
+    birthDate(locale: String): String
+    account: AccountType
+    metadata: [UserMetadata]
+    ssn: String
+  }
+
+  type Name {
+    first: String
+    last: String
+  }
+
+  type Mutation {
+    login(username: String!, password: String!): User
+  }
+
+  extend type Library @key(fields: "id") {
+    id: ID! @external
+    name: String @external
+    userAccount(id: ID! = "1"): User @requires(fields: "name")
+  }
+`;

federation-integration-testsuite-js/src/fixtures/reviewsWithUpdate.ts → federation-integration-testsuite-js/src/fixtures/special-cases/reviewsWithUpdate.ts

@@ -1,6 +1,6 @@
 import gql from 'graphql-tag';
 
-import * as reviewsService from './reviews';
+import * as reviewsService from '../reviews';
 
 export const name = reviewsService.name;
 export const url = reviewsService.url;

federation-integration-testsuite-js/src/fixtures/special-cases/reviewsWithoutTagOrInaccessible.ts

@@ -0,0 +1,90 @@
+import gql from 'graphql-tag';
+
+export { name, url, resolvers } from '../reviews';
+export const typeDefs = gql`
+  directive @stream on FIELD
+  directive @transform(from: String!) on FIELD
+
+  extend type Query {
+    topReviews(first: Int = 5): [Review]
+  }
+
+  type Review @key(fields: "id") {
+    id: ID!
+    body(format: Boolean = false): String
+    author: User @provides(fields: "username")
+    product: Product
+    metadata: [MetadataOrError]
+  }
+
+  input UpdateReviewInput {
+    id: ID!
+    body: String
+  }
+
+  extend type UserMetadata {
+    address: String @external
+  }
+
+  extend type User @key(fields: "id") {
+    id: ID! @external
+    username: String @external
+    reviews: [Review]
+    numberOfReviews: Int!
+    metadata: [UserMetadata] @external
+    goodAddress: Boolean @requires(fields: "metadata { address }")
+  }
+
+  extend interface Product {
+    reviews: [Review]
+  }
+
+  extend type Furniture implements Product @key(fields: "upc") {
+    upc: String! @external
+    reviews: [Review]
+  }
+
+  extend type Book implements Product @key(fields: "isbn") {
+    isbn: String! @external
+    reviews: [Review]
+    similarBooks: [Book]! @external
+    relatedReviews: [Review!]! @requires(fields: "similarBooks { isbn }")
+  }
+
+  extend interface Vehicle {
+    retailPrice: String
+  }
+
+  extend type Car implements Vehicle @key(fields: "id") {
+    id: String! @external
+    price: String @external
+    retailPrice: String @requires(fields: "price")
+  }
+
+  extend type Van implements Vehicle @key(fields: "id") {
+    id: String! @external
+    price: String @external
+    retailPrice: String @requires(fields: "price")
+  }
+
+  extend type Mutation {
+    reviewProduct(upc: String!, body: String!): Product
+    updateReview(review: UpdateReviewInput!): Review
+    deleteReview(id: ID!): Boolean
+  }
+
+  # Value type
+  type KeyValue {
+    key: String!
+    value: String!
+  }
+
+  # Value type
+  type Error {
+    code: Int
+    message: String
+  }
+
+  # Value type
+  union MetadataOrError = KeyValue | Error
+`;

federation-js/src/composition/compose.ts

@@ -21,7 +21,7 @@ import {
   DirectiveNode,
 } from 'graphql';
 import { transformSchema } from 'apollo-graphql';
-import apolloTypeSystemDirectives from '../directives';
+import apolloTypeSystemDirectives, { appliedDirectives, federationDirectives } from '../directives';
 import {
   findDirectivesOnNode,
   isStringValueNode,
@@ -35,7 +35,7 @@ import {
   getFederationMetadata,
   CompositionResult,
   isDirectiveDefinitionNode,
-  isApolloTypeSystemDirective
+  isFederationDirective
 } from './utils';
 import {
   ServiceDefinition,
@@ -129,6 +129,11 @@ type FieldDirectivesMap = Map<string, DirectiveNode[]>;
 // TODO: rename?
 type TypeNameToFieldDirectivesMap = Map<string, FieldDirectivesMap>;
 
+/**
+ * A set of directive names that have been used at least once
+ */
+type AppliedDirectiveUsages = Set<string>;
+
 /**
  * Loop over each service and process its typeDefs (`definitions`)
  * - build up typeToServiceMap
@@ -143,6 +148,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
   const keyDirectivesMap: KeyDirectivesMap = Object.create(null);
   const valueTypes: ValueTypes = new Set();
   const typeNameToFieldDirectivesMap: TypeNameToFieldDirectivesMap = new Map();
+  const appliedDirectiveUsages: AppliedDirectiveUsages = new Set();
 
   for (const { typeDefs, name: serviceName } of serviceList) {
     // Build a new SDL with @external fields removed, as well as information about
@@ -190,19 +196,24 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
         for (const field of definition.fields ?? []) {
           const fieldName = field.name.value;
 
-          const tagAndInaccessibleDirectives = [
-            ...findDirectivesOnNode(field, 'tag'),
-            ...findDirectivesOnNode(field, 'inaccessible'),
-          ];
+          const tagUsages = findDirectivesOnNode(field, 'tag');
+          const inaccessibleUsages = findDirectivesOnNode(
+            field,
+            'inaccessible',
+          );
+
+          if (tagUsages.length > 0) appliedDirectiveUsages.add('tag');
+          if (inaccessibleUsages.length > 0)
+            appliedDirectiveUsages.add('inaccessible');
 
-          if (tagAndInaccessibleDirectives.length > 0) {
+          if (tagUsages.length > 0 || inaccessibleUsages.length > 0) {
             const fieldToDirectivesMap = mapGetOrSet(
               typeNameToFieldDirectivesMap,
               typeName,
               new Map(),
             );
             const directives = mapGetOrSet(fieldToDirectivesMap, fieldName, []);
-            directives.push(...tagAndInaccessibleDirectives);
+            directives.push(...[...tagUsages, ...inaccessibleUsages]);
           }
         }
       }
@@ -386,25 +397,35 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
     keyDirectivesMap,
     valueTypes,
     typeNameToFieldDirectivesMap,
+    appliedDirectiveUsages,
   };
 }
 
 export function buildSchemaFromDefinitionsAndExtensions({
   typeDefinitionsMap,
   typeExtensionsMap,
   directiveDefinitionsMap,
+  appliedDirectiveUsages,
 }: {
   typeDefinitionsMap: TypeDefinitionsMap;
   typeExtensionsMap: TypeExtensionsMap;
   directiveDefinitionsMap: DirectiveDefinitionsMap;
+  appliedDirectiveUsages: AppliedDirectiveUsages;
 }) {
   let errors: GraphQLError[] | undefined = undefined;
 
+  // We only want to include the definitions of applied directives (currently
+  // just @tag and @include) if there are usages.
+  const appliedDirectivesToInclude = appliedDirectives.filter((directive) =>
+    appliedDirectiveUsages.has(directive.name),
+  );
+
   let schema = new GraphQLSchema({
     query: undefined,
     directives: [
       ...specifiedDirectives,
-      ...apolloTypeSystemDirectives,
+      ...federationDirectives,
+      ...appliedDirectivesToInclude,
     ],
   });
 
@@ -425,23 +446,19 @@ export function buildSchemaFromDefinitionsAndExtensions({
   const definitionsDocument: DocumentNode = {
     kind: Kind.DOCUMENT,
     definitions: [
-      ...Object.values(typeDefinitionsMap).flatMap(typeDefinitions => {
+      ...Object.values(typeDefinitionsMap).flatMap((typeDefinitions) => {
         // See if any of our Objects or Interfaces implement any interfaces at all.
         // If not, we can return early.
         if (!typeDefinitions.some(nodeHasInterfaces)) return typeDefinitions;
 
-        const uniqueInterfaces: Map<
-          string,
-          NamedTypeNode
-        > = (typeDefinitions as HasInterfaces[]).reduce(
-          (map, objectTypeDef) => {
-            objectTypeDef.interfaces?.forEach((iface) =>
-              map.set(iface.name.value, iface),
-            );
-            return map;
-          },
-          new Map(),
-        );
+        const uniqueInterfaces: Map<string, NamedTypeNode> = (
+          typeDefinitions as HasInterfaces[]
+        ).reduce((map, objectTypeDef) => {
+          objectTypeDef.interfaces?.forEach((iface) =>
+            map.set(iface.name.value, iface),
+          );
+          return map;
+        }, new Map());
 
         // No interfaces, no aggregation - just return what we got.
         if (uniqueInterfaces.size === 0) return typeDefinitions;
@@ -455,10 +472,9 @@ export function buildSchemaFromDefinitionsAndExtensions({
             interfaces: Array.from(uniqueInterfaces.values()),
           },
         ];
-
       }),
       ...Object.values(directiveDefinitionsMap).map(
-        definitions => Object.values(definitions)[0],
+        (definitions) => Object.values(definitions)[0],
       ),
     ],
   };
@@ -489,7 +505,7 @@ export function buildSchemaFromDefinitionsAndExtensions({
   schema = new GraphQLSchema({
     ...schema.toConfig(),
     directives: [
-      ...schema.getDirectives().filter((x) => !isApolloTypeSystemDirective(x)),
+      ...schema.getDirectives().filter((x) => !isFederationDirective(x)),
     ],
   });
 
@@ -705,12 +721,14 @@ export function composeServices(services: ServiceDefinition[]): CompositionResul
     keyDirectivesMap,
     valueTypes,
     typeNameToFieldDirectivesMap,
+    appliedDirectiveUsages,
   } = buildMapsFromServiceList(services);
 
   let { schema, errors } = buildSchemaFromDefinitionsAndExtensions({
     typeDefinitionsMap,
     typeExtensionsMap,
     directiveDefinitionsMap,
+    appliedDirectiveUsages,
   });
 
   // TODO: We should fix this to take non-default operation root types in

federation-js/src/composition/utils.ts

@@ -44,6 +44,7 @@ import {
 } from './types';
 import apolloTypeSystemDirectives, {
   ASTNodeWithDirectives,
+  federationDirectives,
 } from '../directives';
 import { assert, isNotNullOrUndefined } from '../utilities';
 
@@ -633,6 +634,10 @@ export function isApolloTypeSystemDirective(directive: GraphQLDirective): boolea
   return apolloTypeSystemDirectives.some(({ name }) => name === directive.name);
 }
 
+export function isFederationDirective(directive: GraphQLDirective): boolean {
+  return federationDirectives.some(({ name }) => name === directive.name);
+}
+
 export const reservedRootFields = ['_service', '_entities'];
 
 // Map of OperationTypeNode to its respective default root operation type name