Repository: mercurius-js/mercurius
Pull request #645 (2021-11-10 17:45:26)

compatibility: schema validation in gateway mode 
Added a filter on the query extensions in the schema create. 
This filter removes all the _service fields in the extend queries. 
This will help to be compatible with multiple services in gateway mode that do not remove the _service field in the SDL while still supporting federation.


see:
https://github.com/mercurius-js/mercurius/issues/643


Patch:
diff --git a/docs/hooks.md b/docs/hooks.md
index 943a1b1..f92c097 100644
--- a/docs/hooks.md
+++ b/docs/hooks.md
@@ -61,15 +61,17 @@ fastify.graphql.addHook('preValidation', async (schema, document, context) => {
 
 In the `preExecution` hook, you can modify the following items by returning them in the hook definition:
   - `document`
+  - `schema`
   - `errors`
 
-Note that if you modify the `document` object, the jit compilation will be disabled for the request.
+Note that if you modify the `schema` or the `document` object, the [jit](./api/options.md#plugin-options) compilation will be disabled for the request.
 
 ```js
 fastify.graphql.addHook('preExecution', async (schema, document, context) => {
-  const { modifiedDocument, errors } = await asyncMethod(document)
+  const { modifiedSchema, modifiedDocument, errors } = await asyncMethod(document)
 
   return {
+    schema: modifiedSchema, // ⚠️ changing the schema may break the query execution. Use it carefully.
     document: modifiedDocument,
     errors
   }
diff --git a/index.d.ts b/index.d.ts
index 958b6a0..64629d2 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -786,6 +786,7 @@ type ValidationRules =
     }) => ValidationRule[]);
 
 export interface PreExecutionHookResponse<TError extends Error> {
+  schema?: GraphQLSchema
   document?: DocumentNode
   errors?: TError[]
 }
diff --git a/index.js b/index.js
index 69be7d8..17b3db3 100644
--- a/index.js
+++ b/index.js
@@ -517,7 +517,6 @@ const plugin = fp(async function (app, opts) {
     }
 
     const shouldCompileJit = cached && cached.count++ === minJit
-
     // Validate variables
     if (variables !== undefined && !shouldCompileJit) {
       const executionContext = buildExecutionContext(fastifyGraphQl.schema, document, root, context, variables, operationName)
@@ -529,24 +528,30 @@ const plugin = fp(async function (app, opts) {
     }
 
     // Trigger preExecution hook
+    let modifiedSchema
     let modifiedDocument
     if (context.preExecution !== null) {
-      ({ modifiedDocument } = await preExecutionHandler({ schema: fastifyGraphQl.schema, document, context }))
+      ({ modifiedSchema, modifiedDocument } = await preExecutionHandler({ schema: fastifyGraphQl.schema, document, context }))
     }
 
     // minJit is 0 by default
-    if (shouldCompileJit && !modifiedDocument) {
-      cached.jit = compileQuery(fastifyGraphQl.schema, document, operationName)
+    if (shouldCompileJit) {
+      if (!modifiedSchema && !modifiedDocument) {
+        // can compile only when the schema and document are not modified
+        cached.jit = compileQuery(fastifyGraphQl.schema, document, operationName)
+      } else {
+        // the counter must decrease to ignore the query
+        cached && cached.count--
+      }
     }
 
-    if (cached && cached.jit !== null && !modifiedDocument) {
+    if (cached && cached.jit !== null && !modifiedSchema && !modifiedDocument) {
       const execution = await cached.jit.query(root, context, variables || {})
-
       return maybeFormatErrors(execution, context)
     }
 
     const execution = await execute(
-      fastifyGraphQl.schema,
+      modifiedSchema || fastifyGraphQl.schema,
       modifiedDocument || document,
       root,
       context,
diff --git a/lib/federation.js b/lib/federation.js
index 4c8736f..b5bcf53 100644
--- a/lib/federation.js
+++ b/lib/federation.js
@@ -276,6 +276,16 @@ function buildFederationSchema (schema, isGateway) {
   const parsedOriginalSchema = parse(schema)
   const { typeStubs, extensions, definitions } = getStubTypes(parsedOriginalSchema.definitions, isGateway)
 
+  // before we validate the federationSchema, we want to remove the _service field from the extended query,
+  // if there is one, as this field should be excluded from the validation to provide broader support
+  // for different federation implementations => https://github.com/mercurius-js/mercurius/issues/643
+  const filteredExtensions = extensions.map(extension => {
+    if (extension.name.value === 'Query') {
+      extension.fields = extension.fields.filter(field => field.name.value !== '_service')
+    }
+    return extension
+  })
+
   // Add type stubs - only needed for federation
   federationSchema = extendSchema(federationSchema, {
     kind: Kind.DOCUMENT,
@@ -291,7 +301,7 @@ function buildFederationSchema (schema, isGateway) {
   // Add all extensions
   const extensionsDocument = {
     kind: Kind.DOCUMENT,
-    definitions: extensions
+    definitions: filteredExtensions
   }
 
   // instead of relying on extendSchema internal validations
diff --git a/lib/handlers.js b/lib/handlers.js
index 1c8d2b4..bdc29a0 100644
--- a/lib/handlers.js
+++ b/lib/handlers.js
@@ -29,16 +29,20 @@ async function preValidationHandler (request) {
 }
 
 async function preExecutionHandler (request) {
-  const { errors, modifiedDocument } = await preExecutionHooksRunner(
+  const { errors, modifiedDocument, modifiedSchema } = await preExecutionHooksRunner(
     request.context.preExecution,
     request
   )
   if (errors.length > 0) {
     addErrorsToContext(request.context, errors)
   }
-  if (typeof modifiedDocument !== 'undefined') {
-    return { modifiedDocument, modifiedQuery: print(request.document) }
+  if (typeof modifiedDocument !== 'undefined' || typeof modifiedSchema !== 'undefined') {
+    return Object.create(null, {
+      modifiedDocument: { value: modifiedDocument },
+      modifiedSchema: { value: modifiedSchema }
+    })
   }
+
   return {}
 }
 
@@ -51,7 +55,10 @@ async function preGatewayExecutionHandler (request) {
     addErrorsToContext(request.context, errors)
   }
   if (typeof modifiedDocument !== 'undefined') {
-    return { modifiedDocument, modifiedQuery: print(modifiedDocument) }
+    return Object.create(null, {
+      modifiedDocument: { value: modifiedDocument },
+      modifiedQuery: { get: () => print(modifiedDocument) }
+    })
   }
   return {}
 }
diff --git a/lib/hooks.js b/lib/hooks.js
index f059e6b..394f9b5 100644
--- a/lib/hooks.js
+++ b/lib/hooks.js
@@ -87,12 +87,19 @@ async function hooksRunner (functions, runner, request) {
 
 async function preExecutionHooksRunner (functions, request) {
   let errors = []
+  let modifiedSchema
   let modifiedDocument
 
   for (const fn of functions) {
-    const result = await fn(request.schema, modifiedDocument || request.document, request.context)
+    const result = await fn(modifiedSchema || request.schema,
+      modifiedDocument || request.document,
+      request.context
+    )
 
     if (result) {
+      if (typeof result.schema !== 'undefined') {
+        modifiedSchema = result.schema
+      }
       if (typeof result.document !== 'undefined') {
         modifiedDocument = result.document
       }
@@ -102,7 +109,7 @@ async function preExecutionHooksRunner (functions, request) {
     }
   }
 
-  return { errors, modifiedDocument }
+  return { errors, modifiedDocument, modifiedSchema }
 }
 
 async function preGatewayExecutionHooksRunner (functions, request) {
diff --git a/package.json b/package.json
index 1436416..2aa2ee7 100644
--- a/package.json
+++ b/package.json
@@ -9,6 +9,7 @@
     "unit": "tap test/*.js test/gateway/*.js test/internals/*.js",
     "cov": "tap  --coverage-report=html -J test/*.js test/gateway/*.js",
     "lint": "npm run lint:standard && npm run lint:typescript",
+    "lint:fix": "standard --fix",
     "lint:standard": "standard | snazzy",
     "lint:typescript": "standard --parser @typescript-eslint/parser --plugin @typescript-eslint/eslint-plugin test/types/*.ts",
     "typescript": "tsd",


Test patch:
diff --git a/test/gateway/schema.js b/test/gateway/schema.js
index 29f21b9..f465ed5 100644
--- a/test/gateway/schema.js
+++ b/test/gateway/schema.js
@@ -2458,3 +2458,299 @@ test('Update the schema without any changes', async (t) => {
 
   t.equal(newSchema, null)
 })
+
+test('It builds the gateway schema correctly with two services query extension having the _service fields', async (t) => {
+  const users = {
+    u1: {
+      id: 'u1',
+      name: 'John'
+    },
+    u2: {
+      id: 'u2',
+      name: 'Jane'
+    },
+    u3: {
+      id: 'u3',
+      name: 'Jack'
+    }
+  }
+
+  const posts = {
+    p1: {
+      pid: 'p1',
+      title: 'Post 1',
+      content: 'Content 1',
+      authorId: 'u1'
+    },
+    p2: {
+      pid: 'p2',
+      title: 'Post 2',
+      content: 'Content 2',
+      authorId: 'u2'
+    },
+    p3: {
+      pid: 'p3',
+      title: 'Post 3',
+      content: 'Content 3',
+      authorId: 'u1'
+    },
+    p4: {
+      pid: 'p4',
+      title: 'Post 4',
+      content: 'Content 4',
+      authorId: 'u2'
+    }
+  }
+
+  const [userService, userServicePort] = await createService(t, `
+    directive @customDirective on FIELD_DEFINITION
+
+    type _Service {
+      sdl: String
+    }
+
+    extend type Query {
+      me: User
+      hello: String
+      _service: _Service!
+    }
+
+    type User @key(fields: "id") {
+      id: ID!
+      name: String!
+      avatar(size: AvatarSize): String
+      friends: [User]
+    }
+
+    enum AvatarSize {
+      small
+      medium
+      large
+    }
+  `, {
+    Query: {
+      me: (root, args, context, info) => {
+        return users.u1
+      },
+      hello: () => 'World'
+    },
+    User: {
+      __resolveReference: (user, args, context, info) => {
+        return users[user.id]
+      },
+      avatar: (user, { size }) => `avatar-${size}.jpg`,
+      friends: (user) => Object.values(users).filter(u => u.id !== user.id)
+    }
+  })
+
+  const [postService, postServicePort] = await createService(t, `
+    type Post @key(fields: "pid") {
+      pid: ID!
+      title: String
+      content: String
+      author: User @requires(fields: "title")
+    }
+
+    type _Service {
+      sdl: String
+    }
+
+    extend type Query {
+      topPosts(count: Int): [Post]
+      _service: _Service!
+    }
+
+    type User @key(fields: "id") @extends {
+      id: ID! @external
+      name: String @external
+      posts: [Post]
+      numberOfPosts: Int @requires(fields: "id")
+    }
+  `, {
+    Post: {
+      __resolveReference: (post, args, context, info) => {
+        return posts[post.pid]
+      },
+      author: (post, args, context, info) => {
+        return {
+          __typename: 'User',
+          id: post.authorId
+        }
+      }
+    },
+    User: {
+      posts: (user, args, context, info) => {
+        return Object.values(posts).filter(p => p.authorId === user.id)
+      },
+      numberOfPosts: (user) => {
+        return Object.values(posts).filter(p => p.authorId === user.id).length
+      }
+    },
+    Query: {
+      topPosts: (root, { count = 2 }) => Object.values(posts).slice(0, count)
+    }
+  })
+
+  const gateway = Fastify()
+  t.teardown(async () => {
+    await gateway.close()
+    await postService.close()
+    await userService.close()
+  })
+  gateway.register(GQL, {
+    gateway: {
+      services: [{
+        name: 'user',
+        url: `http://localhost:${userServicePort}/graphql`,
+        rewriteHeaders: (headers) => {
+          if (headers.authorization) {
+            return {
+              authorization: headers.authorization
+            }
+          }
+        }
+      }, {
+        name: 'post',
+        url: `http://localhost:${postServicePort}/graphql`
+      }]
+    }
+  })
+
+  const query = `
+  query MainQuery(
+    $size: AvatarSize
+    $count: Int
+  ) {
+    me {
+      id
+      name
+      avatar(size: $size)
+      friends {
+        ...UserFragment
+        friends {
+          ...UserFragment
+        }
+      }
+      posts {
+        ...PostFragment
+      }
+      numberOfPosts
+    }
+    topPosts(count: $count) {
+      ...PostFragment
+    }
+    hello
+  }
+
+  fragment UserFragment on User {
+    id
+    name
+    avatar(size: medium)
+    numberOfPosts
+  }
+
+  fragment PostFragment on Post {
+    pid
+    title
+    content
+    ...AuthorFragment
+  }
+  
+  fragment AuthorFragment on Post {
+    author {
+      ...UserFragment
+    }
+  }`
+  const res = await gateway.inject({
+    method: 'POST',
+    headers: {
+      'content-type': 'application/json',
+      authorization: 'bearer supersecret'
+    },
+    url: '/graphql',
+    body: JSON.stringify({
+      query,
+      variables: {
+        size: 'small',
+        count: 1
+      }
+    })
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: {
+      me: {
+        id: 'u1',
+        name: 'John',
+        avatar: 'avatar-small.jpg',
+        friends: [{
+          id: 'u2',
+          name: 'Jane',
+          avatar: 'avatar-medium.jpg',
+          numberOfPosts: 2,
+          friends: [{
+            id: 'u1',
+            name: 'John',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 2
+          }, {
+            id: 'u3',
+            name: 'Jack',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 0
+          }]
+        }, {
+          id: 'u3',
+          name: 'Jack',
+          avatar: 'avatar-medium.jpg',
+          numberOfPosts: 0,
+          friends: [{
+            id: 'u1',
+            name: 'John',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 2
+          }, {
+            id: 'u2',
+            name: 'Jane',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 2
+          }]
+        }],
+        posts: [{
+          pid: 'p1',
+          title: 'Post 1',
+          content: 'Content 1',
+          author: {
+            id: 'u1',
+            name: 'John',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 2
+          }
+        }, {
+          pid: 'p3',
+          title: 'Post 3',
+          content: 'Content 3',
+          author: {
+            id: 'u1',
+            name: 'John',
+            avatar: 'avatar-medium.jpg',
+            numberOfPosts: 2
+          }
+        }],
+        numberOfPosts: 2
+      },
+      topPosts: [{
+        pid: 'p1',
+        title: 'Post 1',
+        content: 'Content 1',
+        author: {
+          id: 'u1',
+          name: 'John',
+          avatar: 'avatar-medium.jpg',
+          numberOfPosts: 2
+        }
+      }],
+      hello: 'World'
+    }
+  })
+})
diff --git a/test/hooks.js b/test/hooks.js
index 941a2e0..7b674d1 100644
--- a/test/hooks.js
+++ b/test/hooks.js
@@ -2,7 +2,9 @@
 
 const { test } = require('tap')
 const Fastify = require('fastify')
-const { parse, GraphQLSchema } = require('graphql')
+const proxyquire = require('proxyquire')
+const { mapSchema } = require('@graphql-tools/utils')
+const { parse, buildSchema, GraphQLSchema } = require('graphql')
 const { promisify } = require('util')
 const GQL = require('..')
 
@@ -525,6 +527,139 @@ test('preExecution hooks should be able to modify the query document AST', async
   })
 })
 
+test('preExecution hooks should be able to modify the schema document AST', async t => {
+  t.plan(8)
+  const app = await createTestServer(t)
+
+  const query = `{ 
+    __type(name:"Query") { 
+      name
+      fields {
+        name
+      }  
+    }
+  }`
+
+  app.graphql.addHook('preExecution', async (schema, document, context) => {
+    t.type(schema, GraphQLSchema)
+    t.same(document, parse(query))
+    t.type(context, 'object')
+
+    if (context.reply.request.headers.role === 'super-user') {
+      const modifiedSchema = `
+        type Query {
+          add(x: Int, y: Int): Int
+          subtract(x: Int, y: Int): Int
+        }
+      `
+
+      return {
+        schema: buildSchema(modifiedSchema)
+      }
+    }
+  })
+
+  const reqSuper = app.inject({
+    method: 'POST',
+    headers: { 'content-type': 'application/json', role: 'super-user' },
+    url: '/graphql',
+    body: JSON.stringify({ query })
+  }).then(res => {
+    t.same(JSON.parse(res.body), {
+      data: {
+        __type: {
+          name: 'Query',
+          fields: [
+            { name: 'add' },
+            { name: 'subtract' }
+          ]
+        }
+      }
+    })
+  })
+
+  const reqNotSuper = app.inject({
+    method: 'POST',
+    headers: { 'content-type': 'application/json', role: 'not-a-super-user' },
+    url: '/graphql',
+    body: JSON.stringify({ query })
+  }).then(res => {
+    t.same(JSON.parse(res.body), {
+      data: {
+        __type: {
+          name: 'Query',
+          fields: [
+            { name: 'add' }
+          ]
+        }
+      }
+    })
+  })
+
+  await Promise.all([reqSuper, reqNotSuper])
+})
+
+test('cache skipped when the GQL Schema has been changed', async t => {
+  t.plan(4)
+
+  const app = Fastify()
+  t.teardown(() => app.close())
+
+  const plugin = proxyquire('../index', {
+    'graphql-jit': {
+      compileQuery () {
+        t.pass('the jit is called once')
+        return null
+      }
+    }
+  })
+
+  await app.register(plugin, { schema, resolvers, jit: 1 })
+  await app
+
+  app.graphql.addHook('preExecution', async (schema, document, context) => {
+    if (context.reply.request.headers.original === 'ok') {
+      return
+    }
+
+    return {
+      schema: mapSchema(schema)
+    }
+  })
+
+  const query = '{ add(x:1, y:2) }'
+
+  {
+    const res = await app.inject({
+      method: 'POST',
+      headers: { 'content-type': 'application/json', original: 'ok' },
+      url: '/graphql',
+      body: JSON.stringify({ query })
+    })
+    t.same(res.json(), { data: { add: 3 } }, 'this call warm up the jit counter')
+  }
+
+  {
+    const res = await app.inject({
+      method: 'POST',
+      headers: { 'content-type': 'application/json', original: 'NO' },
+      url: '/graphql',
+      body: JSON.stringify({ query })
+    })
+    t.same(res.json(), { data: { add: 3 } }, 'this call MUST not trigger the jit')
+  }
+
+  {
+    const res = await app.inject({
+      method: 'POST',
+      headers: { 'content-type': 'application/json', original: 'ok' },
+      url: '/graphql',
+      body: JSON.stringify({ query })
+    })
+    t.same(res.json(), { data: { add: 3 } }, 'this call triggers the jit cache')
+  }
+})
+
 test('preExecution hooks should be able to add to the errors array', async t => {
   t.plan(7)
   const app = await createTestServer(t)
diff --git a/test/types/index.ts b/test/types/index.ts
index ce6050e..d93bf42 100644
--- a/test/types/index.ts
+++ b/test/types/index.ts
@@ -537,6 +537,7 @@ app.graphql.addHook('preExecution', async function (schema, document, context) {
   expectAssignable<DocumentNode>(document)
   expectAssignable<MercuriusContext>(context)
   return {
+    schema,
     document,
     errors: [
       new Error('foo')