Repository: mercurius-js/mercurius
Pull request #644 (2021-11-10 15:42:07)

fix cache usage
From this PR https://github.com/mercurius-js/mercurius/pull/641#discussion_r746538702 we spotted an issue with the cache usage.

The logic here is that it is not possible to use the jit'ed function.
Within this fix, if the developer changes the `document` on a `preExectution` hook, the caching is skipped:

- the modified document does not trigger the function compiling
- the HTTP request does not use any cached query

To avoid cache skipping, we should `print()` the `modifiedDocument` and rely on the `lru` logic as done here:

https://github.com/mercurius-js/mercurius/blob/aa8eaaa0cb84ce662b84cd3b9fb4a6cf29abddb9/index.js#L445



Patch:
diff --git a/docs/hooks.md b/docs/hooks.md
index c92119a..943a1b1 100644
--- a/docs/hooks.md
+++ b/docs/hooks.md
@@ -63,6 +63,8 @@ In the `preExecution` hook, you can modify the following items by returning them
   - `document`
   - `errors`
 
+Note that if you modify the `document` object, the jit compilation will be disabled for the request.
+
 ```js
 fastify.graphql.addHook('preExecution', async (schema, document, context) => {
   const { modifiedDocument, errors } = await asyncMethod(document)
@@ -143,7 +145,7 @@ Note, the original query will still execute. Adding the above will result in the
 ```json
 {
   "data": {
-    foo: "bar"
+    "foo": "bar"
   },
   "errors": [
     {
diff --git a/index.js b/index.js
index 340add2..69be7d8 100644
--- a/index.js
+++ b/index.js
@@ -535,11 +535,11 @@ const plugin = fp(async function (app, opts) {
     }
 
     // minJit is 0 by default
-    if (shouldCompileJit) {
-      cached.jit = compileQuery(fastifyGraphQl.schema, modifiedDocument || document, operationName)
+    if (shouldCompileJit && !modifiedDocument) {
+      cached.jit = compileQuery(fastifyGraphQl.schema, document, operationName)
     }
 
-    if (cached && cached.jit !== null) {
+    if (cached && cached.jit !== null && !modifiedDocument) {
       const execution = await cached.jit.query(root, context, variables || {})
 
       return maybeFormatErrors(execution, context)


Test patch:
diff --git a/test/cache.js b/test/cache.js
new file mode 100644
index 0000000..109d82d
--- /dev/null
+++ b/test/cache.js
@@ -0,0 +1,101 @@
+'use strict'
+
+const { test } = require('tap')
+const Fastify = require('fastify')
+const GQL = require('..')
+
+const schema = `
+type User {
+  name: String!
+  password: String!
+}
+
+type Query {
+  read: [User]
+}
+`
+
+const resolvers = {
+  Query: {
+    read: async (_, obj) => {
+      return [
+        {
+          name: 'foo',
+          password: 'bar'
+        }
+      ]
+    }
+  }
+}
+
+test('cache skipped when the GQL Schema has been changed', async t => {
+  t.plan(4)
+
+  const app = Fastify()
+  t.teardown(() => app.close())
+
+  await app.register(GQL, { schema, resolvers, jit: 1 })
+
+  app.graphql.addHook('preExecution', async (schema, document, context) => {
+    if (context.reply.request.headers.super === 'true') {
+      return
+    }
+
+    const documentClone = JSON.parse(JSON.stringify(document))
+    documentClone.definitions[0].selectionSet.selections[0].selectionSet.selections =
+    document.definitions[0].selectionSet.selections[0].selectionSet.selections.filter(sel => sel.name.value !== 'password')
+
+    return {
+      document: documentClone
+    }
+  })
+
+  const query = `{
+    read {
+      name
+      password
+    }
+  }`
+
+  await superUserCall('this call warm up the jit counter')
+  await superUserCall('this call triggers the jit cache')
+
+  {
+    const res = await app.inject({
+      method: 'POST',
+      headers: { 'content-type': 'application/json', super: 'false' },
+      url: '/graphql',
+      body: JSON.stringify({ query })
+    })
+    t.same(res.json(), {
+      data: {
+        read: [
+          {
+            name: 'foo'
+          }
+        ]
+      }
+    }, 'this query should not use the cached query')
+  }
+
+  await superUserCall('this call must use the cache')
+
+  async function superUserCall (msg) {
+    const res = await app.inject({
+      method: 'POST',
+      headers: { 'content-type': 'application/json', super: 'true' },
+      url: '/graphql',
+      body: JSON.stringify({ query })
+    })
+    t.same(res.json(), {
+      data: {
+        read: [
+          {
+            name: 'foo',
+            password: 'bar'
+          }
+        ]
+      }
+    }, msg)
+  }
+})