Merge pull request #88 from fabscale/fn/cache-promises

Cache pending queries

packages/client/README.md

@@ -172,6 +172,9 @@ it will immediately return the same response as the last time, and _not_ hit the
 
 The `cacheEntity` and `cacheId` are required to be able to clear the cache. You can find more information on that later.
 
+Note that when using the cache, it will also avoid making parallel API requests.
+This means that if a query is currently pending, it will _not_ make an additional network request if the same query is made, but re-use the same promise.
+
 Finally, you can also decide to only use a cached record unless it is stale:
 
 ```js

packages/client/addon/services/graphql.ts

@@ -1,7 +1,9 @@
 import { getOwner } from '@ember/application';
 import { assert } from '@ember/debug';
 import Service from '@ember/service';
-import GraphQLCache from '@ember-graphql-client/client/utils/graphql-cache';
+import GraphQLCache, {
+  GraphQLEntityCache,
+} from '@ember-graphql-client/client/utils/graphql-cache';
 import GraphQLRequestClient, {
   GraphQLRequestClientInterface,
   MutationOptions,
@@ -119,18 +121,29 @@ export default class GraphQLService extends Service {
       ? this._maybeUseCachedResponse(options, cacheOptions)
       : undefined;
 
-    if (cachedResponse) {
-      return cachedResponse;
+    let promise = cachedResponse || client.query(options);
+
+    // We cache the promise to avoid parallel network requests
+    if (!cachedResponse) {
+      this._maybeStoreCachedResponse(options, cacheOptions, promise);
     }
 
     let token = waiter.beginAsync();
     try {
-      let response = await client.query(options);
-      this._maybeStoreCachedResponse(options, cacheOptions, response);
+      let response = await promise;
+
+      // If the response is not just the cachedResponse, we try to cache it
+      if (response !== cachedResponse) {
+        this._maybeStoreCachedResponse(options, cacheOptions, response);
+      }
       waiter.endAsync(token);
       return response;
     } catch (error) {
       waiter.endAsync(token);
+
+      // Make sure to clear the cached response, to avoid having a rejected promise cached
+      this._maybeClearCachedResponse(options, cacheOptions);
+
       throw this._handleError(error, { source: 'query' });
     }
   }
@@ -182,17 +195,14 @@ export default class GraphQLService extends Service {
     options: QueryOptions,
     cacheOptions: QueryCacheOptions | undefined
   ): undefined | GraphqlResponse {
-    if (!cacheOptions) {
-      return;
-    }
+    let cache = this._getCache(cacheOptions);
 
-    let { cacheEntity, cacheSeconds, cacheId } = cacheOptions;
-
-    if (!cacheEntity) {
+    if (!cache) {
       return;
     }
 
-    let cache = this.cache.getCache(cacheEntity, cacheId);
+    let { cacheSeconds } = cacheOptions!;
+
     let cachedResponse = cache.get(options);
 
     if (typeof cachedResponse === 'undefined') {
@@ -215,8 +225,33 @@ export default class GraphQLService extends Service {
   _maybeStoreCachedResponse(
     options: QueryOptions,
     cacheOptions: QueryCacheOptions | undefined,
-    response: GraphqlResponse
+    response: GraphqlResponse | Promise<GraphqlResponse>
   ): void {
+    let cache = this._getCache(cacheOptions);
+
+    if (!cache) {
+      return;
+    }
+
+    cache.set(options, response);
+  }
+
+  _maybeClearCachedResponse(
+    options: QueryOptions,
+    cacheOptions: QueryCacheOptions | undefined
+  ): void {
+    let cache = this._getCache(cacheOptions);
+
+    if (!cache) {
+      return;
+    }
+
+    cache.remove(options);
+  }
+
+  _getCache(
+    cacheOptions: QueryCacheOptions | undefined
+  ): GraphQLEntityCache | undefined {
     if (!cacheOptions) {
       return;
     }
@@ -227,8 +262,7 @@ export default class GraphQLService extends Service {
       return;
     }
 
-    let cache = this.cache.getCache(cacheEntity, cacheId);
-    cache.set(options, response);
+    return this.cache.getCache(cacheEntity, cacheId);
   }
 
   _handleError(

packages/client/tests/unit/services/graphql-test.ts

@@ -469,6 +469,43 @@ module('Unit | Service | graphql', function (hooks) {
       assert.verifySteps(['query is called for namespace=post & variables={}']);
     });
 
+    test('it works with cache & pending queries', async function (this: Context, assert) {
+      let { graphql } = this;
+
+      let promise1 = graphql.query(
+        {
+          query: queryStaticPost,
+          namespace: 'post',
+        },
+        {
+          cacheEntity: 'Post',
+        }
+      );
+
+      let promise2 = graphql.query(
+        {
+          query: queryStaticPost,
+          namespace: 'post',
+        },
+        {
+          cacheEntity: 'Post',
+        }
+      );
+
+      let response1 = await promise1;
+      let response2 = await promise2;
+
+      assert.strictEqual(response1, response2, 'responses are the same');
+
+      assert.deepEqual(response1, {
+        id: '1',
+        title:
+          'sunt aut facere repellat provident occaecati excepturi optio reprehenderit',
+      });
+
+      assert.verifySteps(['query is called for namespace=post & variables={}']);
+    });
+
     test('it caches per variable set', async function (this: Context, assert) {
       let { graphql } = this;
 
@@ -628,6 +665,120 @@ module('Unit | Service | graphql', function (hooks) {
       ]);
     });
 
+    test('it does not cache errors', async function (this: Context, assert) {
+      let shouldError = true;
+
+      class TestGraphQLRequestClient extends GraphQLRequestClient {
+        async query(options: QueryOptions): Promise<any> {
+          assert.step(
+            `query is called for namespace=${
+              options.namespace
+            } & variables=${JSON.stringify(options.variables || {})}`
+          );
+
+          await new Promise((resolve) => setTimeout(resolve, 100));
+
+          if (shouldError) {
+            throw new GraphQLNetworkError();
+          }
+
+          return super.query(options);
+        }
+      }
+
+      let graphql = this.owner.lookup('service:graphql') as GraphQLService;
+
+      let graphQLClient = new GraphQLClient(graphql.apiURL!, {});
+      let client = new TestGraphQLRequestClient(graphQLClient);
+
+      graphql.client = client;
+
+      let promise1 = graphql.query(
+        {
+          query: queryStaticPost,
+          namespace: 'post',
+        },
+        {
+          cacheEntity: 'Post',
+        }
+      );
+
+      let promise2 = graphql.query(
+        {
+          query: queryStaticPost,
+          namespace: 'post',
+        },
+        {
+          cacheEntity: 'Post',
+        }
+      );
+
+      try {
+        await promise1;
+      } catch (error) {
+        assert.true(
+          error instanceof GraphQLNetworkError,
+          'error is network error'
+        );
+        assert.step('promise1 error');
+      }
+
+      try {
+        await promise2;
+      } catch (error) {
+        assert.true(
+          error instanceof GraphQLNetworkError,
+          'error is network error'
+        );
+        assert.step('promise2 error');
+      }
+
+      try {
+        await graphql.query(
+          {
+            query: queryStaticPost,
+            namespace: 'post',
+          },
+          {
+            cacheEntity: 'Post',
+          }
+        );
+      } catch (error) {
+        assert.true(
+          error instanceof GraphQLNetworkError,
+          'error is network error'
+        );
+        assert.step('promise3 error');
+      }
+
+      shouldError = false;
+
+      let response = await graphql.query(
+        {
+          query: queryStaticPost,
+          namespace: 'post',
+        },
+        {
+          cacheEntity: 'Post',
+        }
+      );
+
+      assert.deepEqual(response, {
+        id: '1',
+        title:
+          'sunt aut facere repellat provident occaecati excepturi optio reprehenderit',
+      });
+
+      assert.verifySteps([
+        'query is called for namespace=post & variables={}',
+        'promise1 error',
+        'promise2 error',
+        'query is called for namespace=post & variables={}',
+        'promise3 error',
+        'query is called for namespace=post & variables={}',
+      ]);
+    });
+
     test('it allows to invalidate caches with a mutation', async function (this: Context, assert) {
       let { graphql } = this;