fix: strip response cache fields from incremental execution result (#…

…2309) * fix: strip response cache fields from incremental execution result * Update packages/plugins/response-cache/src/plugin.ts Co-authored-by: Valentin Cocaud <[email protected]> --------- Co-authored-by: Valentin Cocaud <[email protected]>
n1ru4l · Oct 9, 2024 · 4fd5917 · 4fd5917
1 parent 9f65fcb
commit 4fd5917
Show file tree

Hide file tree

Showing 5 changed files with 146 additions and 42 deletions.
diff --git a/.changeset/soft-coats-retire.md b/.changeset/soft-coats-retire.md
@@ -0,0 +1,5 @@
+---
+'@envelop/response-cache': patch
+---
+
+Strip `__responseCacheId` and `__responseCacheTypeName` from incremental delivery execution result.
diff --git a/packages/plugins/response-cache/package.json b/packages/plugins/response-cache/package.json
@@ -59,7 +59,7 @@
   },
   "devDependencies": {
     "@envelop/core": "workspace:^",
-    "@graphql-tools/executor": "^1.1.0",
+    "@graphql-tools/executor": "^1.3.1",
     "@graphql-tools/schema": "10.0.6",
     "graphql": "16.8.1",
     "ioredis-mock": "5.9.1",

diff --git a/packages/plugins/response-cache/src/plugin.ts b/packages/plugins/response-cache/src/plugin.ts
@@ -411,6 +411,7 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
           executed = true;
           return execute(args);
         });
+
         return {
           onExecuteDone(params) {
             if (!executed) {
@@ -448,32 +449,34 @@ export function useResponseCache<PluginContext extends Record<string, any> = {}>
           processResult(data[fieldName]);
         }
 
-        if (!skip) {
-          if (ignoredTypesMap.has(typename) || (!sessionId && isPrivate(typename, data))) {
-            skip = true;
-            return;
-          }
+        if (skip) {
+          return;
+        }
 
-          types.add(typename);
-          if (typename in ttlPerType) {
-            const maybeTtl = ttlPerType[typename] as unknown;
-            currentTtl = calculateTtl(maybeTtl, currentTtl);
-          }
-          if (entityId != null) {
-            identifier.set(`${typename}:${entityId}`, { typename, id: entityId });
-          }
-          for (const fieldName in data) {
-            const fieldData = data[fieldName];
-            if (fieldData == null || (Array.isArray(fieldData) && fieldData.length === 0)) {
-              const inferredTypes = typePerSchemaCoordinateMap.get(`${typename}.${fieldName}`);
-              inferredTypes?.forEach(inferredType => {
-                if (inferredType in ttlPerType) {
-                  const maybeTtl = ttlPerType[inferredType] as unknown;
-                  currentTtl = calculateTtl(maybeTtl, currentTtl);
-                }
-                identifier.set(inferredType, { typename: inferredType });
-              });
-            }
+        if (ignoredTypesMap.has(typename) || (!sessionId && isPrivate(typename, data))) {
+          skip = true;
+          return;
+        }
+
+        types.add(typename);
+        if (typename in ttlPerType) {
+          const maybeTtl = ttlPerType[typename] as unknown;
+          currentTtl = calculateTtl(maybeTtl, currentTtl);
+        }
+        if (entityId != null) {
+          identifier.set(`${typename}:${entityId}`, { typename, id: entityId });
+        }
+        for (const fieldName in data) {
+          const fieldData = data[fieldName];
+          if (fieldData == null || (Array.isArray(fieldData) && fieldData.length === 0)) {
+            const inferredTypes = typePerSchemaCoordinateMap.get(`${typename}.${fieldName}`);
+            inferredTypes?.forEach(inferredType => {
+              if (inferredType in ttlPerType) {
+                const maybeTtl = ttlPerType[inferredType] as unknown;
+                currentTtl = calculateTtl(maybeTtl, currentTtl);
+              }
+              identifier.set(inferredType, { typename: inferredType });
+            });
           }
         }
       }
@@ -609,22 +612,19 @@ function handleAsyncIterableResult<PluginContext extends Record<string, any> = {
   const result: ExecutionResult = {};
   return {
     onNext(payload) {
-      const { data, errors, extensions } = payload.result;
-
       // This is the first result with the initial data payload sent to the client. We use it as the base result
-      if (data) {
-        result.data = data;
+      if (payload.result.data) {
+        result.data = payload.result.data;
       }
-      if (errors) {
-        result.errors = errors;
+      if (payload.result.errors) {
+        result.errors = payload.result.errors;
       }
-      if (extensions) {
-        result.extensions = extensions;
+      if (payload.result.extensions) {
+        result.extensions = payload.result.extensions;
       }
 
       if ('hasNext' in payload.result) {
         const { incremental, hasNext } = payload.result;
-
         if (incremental) {
           for (const patch of incremental) {
             mergeIncrementalResult({ executionResult: result, incrementalResult: patch });
@@ -636,6 +636,12 @@ function handleAsyncIterableResult<PluginContext extends Record<string, any> = {
           handler(result, payload.setResult);
         }
       }
+
+      const newResult = { ...payload.result };
+      if (newResult.data) {
+        newResult.data = removeMetadataFieldsFromResult(newResult.data);
+      }
+      payload.setResult(newResult);
     },
   };
 }
@@ -684,3 +690,22 @@ export const cacheControlDirective = /* GraphQL */ `
 
   directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT
 `;
+
+function removeMetadataFieldsFromResult(data: Record<string, unknown>): any {
+  if (Array.isArray(data)) {
+    return data.map(removeMetadataFieldsFromResult);
+  }
+  // clone the data to avoid mutation
+  data = { ...data };
+
+  delete data.__responseCacheTypeName;
+  delete data.__responseCacheId;
+
+  for (const key in data) {
+    if (typeof data[key] === 'object') {
+      data[key] = removeMetadataFieldsFromResult(data[key] as Record<string, unknown>);
+    }
+  }
+
+  return data;
+}
diff --git a/packages/plugins/response-cache/test/response-cache.spec.ts b/packages/plugins/response-cache/test/response-cache.spec.ts
@@ -4099,3 +4099,77 @@ it('calls enabled fn after context building', async () => {
     },
   });
 });
+
+it('correctly remove cache keys from incremental delivery result', async () => {
+  const schema = makeExecutableSchema({
+    typeDefs: /* GraphQL */ `
+      type Query {
+        users: [User!]!
+      }
+
+      type User {
+        id: ID!
+        name: String!
+      }
+
+      directive @stream on FIELD
+    `,
+    resolvers: {
+      Query: {
+        async *users() {
+          yield { id: '1', name: 'Alice' };
+          await new Promise(resolve => setTimeout(resolve, 10));
+          yield { id: '2', name: 'Bob' };
+        },
+      },
+    },
+  });
+
+  const testInstance = createTestkit(
+    [
+      useEngine({ ...GraphQLJS, execute: normalizedExecutor, subscribe: normalizedExecutor }),
+      useResponseCache({ session: () => null }),
+    ],
+    schema,
+  );
+
+  const query = /* GraphQL */ `
+    query test {
+      users @stream {
+        id
+        name
+      }
+    }
+  `;
+  const result = await testInstance.execute(query);
+  assertStreamExecutionValue(result);
+  const result1 = (await result.next()).value;
+  const result2 = (await result.next()).value;
+  const result3 = (await result.next()).value;
+  const result4 = (await result.next()).value;
+
+  expect(result1).toEqual({ data: { users: [] }, hasNext: true });
+  expect(result2).toEqual({
+    incremental: [{ items: [{ id: '1', name: 'Alice' }], path: ['users', 0] }],
+    hasNext: true,
+  });
+  expect(result3).toEqual({
+    incremental: [{ items: [{ id: '2', name: 'Bob' }], path: ['users', 1] }],
+    hasNext: true,
+  });
+  expect(result4).toEqual({ hasNext: false });
+
+  const secondResult = await testInstance.execute(query);
+  assertSingleExecutionValue(secondResult);
+  expect(secondResult).toEqual({
+    data: {
+      users: [
+        { id: '1', name: 'Alice' },
+        {
+          id: '2',
+          name: 'Bob',
+        },
+      ],
+    },
+  });
+});
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml