Repository: mercurius-js/mercurius
Pull request #377 (2021-01-11 22:10:45)

fix: Don't use extendSchema's validations in buildFederationSchema.
### Summary
Currently a gateway will throw a validation error if more than one federated service has the same directive defined - excluding directives added by `federationMetadata`. (ie. `Error: There can be only one directive named ...`)

While `compositionRules` does remove the `UniqueDirectivesPerLocationRule`, it is being circumvented by not passing `assumeValidSDL: true` to the `extendSchema` calls prior to the `validateSDL` call.

[This](https://github.com/mercurius-js/mercurius/blob/1ae98ad2d577e1a4d83a098544a21b358374d97c/lib/federation.js#L248) call of `extendSchema` is the direct cause of the issue, but based on [this](https://github.com/mercurius-js/mercurius/blob/1ae98ad2d577e1a4d83a098544a21b358374d97c/lib/federation.js#L259) comment I assume the intention is to not have `extendSchema` do any validation - so I've added `assumeValidSDL: true` to the other `extendSchema` calls in `buildFederationSchema` as well.

### Test plan
The existence of this issue can be confirmed by running the added test without the changes to `buildFederationSchema`.

HW13: Thanks! 😄  

Patch:
diff --git a/lib/federation.js b/lib/federation.js
index 1e781ea..c7c080a 100644
--- a/lib/federation.js
+++ b/lib/federation.js
@@ -233,7 +233,11 @@ function buildFederationSchema (schema, isGateway) {
     query: undefined
   })
 
-  federationSchema = extendSchema(federationSchema, parse(isGateway ? BASE_FEDERATION_TYPES : FEDERATION_SCHEMA))
+  federationSchema = extendSchema(
+    federationSchema,
+    parse(isGateway ? BASE_FEDERATION_TYPES : FEDERATION_SCHEMA),
+    { assumeValidSDL: true }
+  )
 
   const parsedOriginalSchema = parse(schema)
   const { typeStubs, extensions, definitions } = getStubTypes(parsedOriginalSchema.definitions, isGateway)
@@ -242,13 +246,13 @@ function buildFederationSchema (schema, isGateway) {
   federationSchema = extendSchema(federationSchema, {
     kind: Kind.DOCUMENT,
     definitions: typeStubs
-  })
+  }, { assumeValidSDL: true })
 
   // Add default type definitions
   federationSchema = extendSchema(federationSchema, {
     kind: Kind.DOCUMENT,
     definitions
-  })
+  }, { assumeValidSDL: true })
 
   // Add all extensions
   const extensionsDocument = {


Test patch:
diff --git a/test/federation.js b/test/federation.js
index ad609f1..b8bd71b 100644
--- a/test/federation.js
+++ b/test/federation.js
@@ -72,7 +72,7 @@ test('federation support using schema from buildFederationSchema', async (t) =>
     extend type Query {
       me: User
     }
-    
+
     extend type Mutation {
       add(a: Int, b: Int): Int
     }
@@ -537,6 +537,40 @@ test('buildFederationSchema works correctly with multiple type extensions', asyn
   }
 })
 
+test('buildFederationSchema ignores UniqueDirectivesPerLocationRule when validating', async (t) => {
+  const schema = `
+    directive @upper on FIELD_DEFINITION
+
+    extend type Query {
+      topPosts: [Post]
+    }
+
+    type Post @key(fields: "id") {
+      id: ID!
+      title: String @upper
+      content: String
+      author: User
+    }
+
+    directive @upper on FIELD_DEFINITION
+
+    extend type User @key(fields: "id") {
+      id: ID! @external
+      posts: [Post]
+    }
+
+    extend type User @key(fields: "id") {
+      other: String @upper
+    }
+  `
+  try {
+    buildFederationSchema(schema)
+    t.pass('schema built without errors')
+  } catch (err) {
+    t.fail('it should not throw errors', err)
+  }
+})
+
 test('buildFederationSchema still validate schema for errors (1 error)', async (t) => {
   const schema = `
     extend type Query {