feat: deduplicate persisted document resolution requests (#5401)

graphql-hive · Aug 19, 2024 · 3ffdb6e · 3ffdb6e
1 parent b5ca177
commit 3ffdb6e
Show file tree

Hide file tree

Showing 5 changed files with 185 additions and 59 deletions.
diff --git a/.changeset/fresh-walls-push.md b/.changeset/fresh-walls-push.md
@@ -0,0 +1,8 @@
+---
+'@graphql-hive/apollo': minor
+'@graphql-hive/core': minor
+'@graphql-hive/yoga': minor
+---
+
+Deduplicate persisted document lookups from the registry for reducing the amount of concurrent HTTP
+requests.
diff --git a/packages/libraries/apollo/tests/persisted-documents.spec.ts b/packages/libraries/apollo/tests/persisted-documents.spec.ts
@@ -13,8 +13,13 @@ type ApolloServerContext = {
   req: IncomingMessage;
 };
 
+const logger = {
+  info: () => {},
+  error: () => {},
+};
+
 test('use persisted documents (GraphQL over HTTP "documentId")', async () => {
-  const httpScope = nock('http://artifatcs-cdn.localhost', {
+  const httpScope = nock('http://artifacts-cdn.localhost', {
     reqheaders: {
       'X-Hive-CDN-Key': value => {
         expect(value).toBe('foo');
@@ -36,10 +41,13 @@ test('use persisted documents (GraphQL over HTTP "documentId")', async () => {
         token: 'token',
         experimental__persistedDocuments: {
           cdn: {
-            endpoint: 'http://artifatcs-cdn.localhost',
+            endpoint: 'http://artifacts-cdn.localhost',
             accessToken: 'foo',
           },
         },
+        agent: {
+          logger,
+        },
       }),
     ],
   });
@@ -68,7 +76,7 @@ test('use persisted documents (GraphQL over HTTP "documentId")', async () => {
 });
 
 test('persisted document not found (GraphQL over HTTP "documentId")', async () => {
-  const httpScope = nock('http://artifatcs-cdn.localhost', {
+  const httpScope = nock('http://artifacts-cdn.localhost', {
     reqheaders: {
       'X-Hive-CDN-Key': value => {
         expect(value).toBe('foo');
@@ -90,10 +98,13 @@ test('persisted document not found (GraphQL over HTTP "documentId")', async () =
         token: 'token',
         experimental__persistedDocuments: {
           cdn: {
-            endpoint: 'http://artifatcs-cdn.localhost',
+            endpoint: 'http://artifacts-cdn.localhost',
             accessToken: 'foo',
           },
         },
+        agent: {
+          logger,
+        },
       }),
     ],
   });
@@ -143,11 +154,14 @@ test('arbitrary options are rejected with allowArbitraryDocuments=false (GraphQL
         token: 'token',
         experimental__persistedDocuments: {
           cdn: {
-            endpoint: 'http://artifatcs-cdn.localhost',
+            endpoint: 'http://artifacts-cdn.localhost',
             accessToken: 'foo',
           },
           allowArbitraryDocuments: false,
         },
+        agent: {
+          logger,
+        },
       }),
     ],
   });
@@ -193,11 +207,14 @@ test('arbitrary options are allowed with allowArbitraryDocuments=true (GraphQL o
         token: 'token',
         experimental__persistedDocuments: {
           cdn: {
-            endpoint: 'http://artifatcs-cdn.localhost',
+            endpoint: 'http://artifacts-cdn.localhost',
             accessToken: 'foo',
           },
           allowArbitraryDocuments: true,
         },
+        agent: {
+          logger,
+        },
       }),
     ],
   });
@@ -229,7 +246,7 @@ test('arbitrary options are allowed with allowArbitraryDocuments=true (GraphQL o
 });
 
 test('usage reporting for persisted document', async () => {
-  const httpScope = nock('http://artifatcs-cdn.localhost', {
+  const httpScope = nock('http://artifacts-cdn.localhost', {
     reqheaders: {
       'X-Hive-CDN-Key': value => {
         expect(value).toBe('foo');
@@ -286,7 +303,7 @@ test('usage reporting for persisted document', async () => {
         token: 'brrrt',
         experimental__persistedDocuments: {
           cdn: {
-            endpoint: 'http://artifatcs-cdn.localhost',
+            endpoint: 'http://artifacts-cdn.localhost',
             accessToken: 'foo',
           },
         },
@@ -300,6 +317,7 @@ test('usage reporting for persisted document', async () => {
         },
         agent: {
           maxSize: 1,
+          logger,
         },
       }),
     ],

diff --git a/packages/libraries/core/src/client/client.ts b/packages/libraries/core/src/client/client.ts
@@ -220,7 +220,10 @@ export function createHive(options: HivePluginOptions): HiveClient {
     createInstrumentedSubscribe,
     createInstrumentedExecute,
     experimental__persistedDocuments: options.experimental__persistedDocuments
-      ? createPersistedDocuments(options.experimental__persistedDocuments)
+      ? createPersistedDocuments({
+          ...options.experimental__persistedDocuments,
+          logger,
+        })
       : null,
   };
 }

diff --git a/packages/libraries/core/src/client/persisted-documents.ts b/packages/libraries/core/src/client/persisted-documents.ts
@@ -1,12 +1,17 @@
 import type { PromiseOrValue } from 'graphql/jsutils/PromiseOrValue.js';
 import LRU from 'tiny-lru';
-import type { PersistedDocumentsConfiguration } from './types';
+import { http } from './http-client.js';
+import type { Logger, PersistedDocumentsConfiguration } from './types';
 
 type HeadersObject = {
   get(name: string): string | null;
 };
 
-export function createPersistedDocuments(config: PersistedDocumentsConfiguration): null | {
+export function createPersistedDocuments(
+  config: PersistedDocumentsConfiguration & {
+    logger: Logger;
+  },
+): null | {
   resolve(documentId: string): Promise<string | null>;
   allowArbitraryDocuments(context: { headers?: HeadersObject }): PromiseOrValue<boolean>;
 } {
@@ -23,31 +28,50 @@ export function createPersistedDocuments(config: PersistedDocumentsConfiguration
     allowArbitraryDocuments = () => false;
   }
 
+  /** if there is already a in-flight request for a document, we re-use it. */
+  const fetchCache = new Map<string, Promise<string | null>>();
+
+  /** Batch load a persisted documents */
+  async function loadPersistedDocument(documentId: string) {
+    const document = persistedDocumentsCache.get(documentId);
+    if (document) {
+      return document;
+    }
+
+    const cdnDocumentId = documentId.replaceAll('~', '/');
+
+    const url = config.cdn.endpoint + '/apps/' + cdnDocumentId;
+    let promise = fetchCache.get(url);
+
+    if (!promise) {
+      promise = http
+        .get(url, {
+          headers: {
+            'X-Hive-CDN-Key': config.cdn.accessToken,
+          },
+          logger: config.logger,
+          isRequestOk: response => response.status === 200 || response.status === 404,
+        })
+        .then(async response => {
+          if (response.status !== 200) {
+            return null;
+          }
+          const text = await response.text();
+          persistedDocumentsCache.set(documentId, text);
+          return text;
+        })
+        .finally(() => {
+          fetchCache.delete(url);
+        });
+
+      fetchCache.set(url, promise);
+    }
+
+    return promise;
+  }
+
   return {
     allowArbitraryDocuments,
-    async resolve(documentId: string) {
-      const document = persistedDocumentsCache.get(documentId);
-
-      if (document) {
-        return document;
-      }
-
-      const cdnHttpId = documentId.replaceAll('~', '/');
-
-      const response = await fetch(config.cdn.endpoint + '/apps/' + cdnHttpId, {
-        method: 'GET',
-        headers: {
-          'X-Hive-CDN-Key': config.cdn.accessToken,
-        },
-      });
-
-      if (response.status !== 200) {
-        return null;
-      }
-      const txt = await response.text();
-      persistedDocumentsCache.set(documentId, txt);
-
-      return txt;
-    },
+    resolve: loadPersistedDocument,
   };
 }