feat: add support for conditional get requests (apache-superset#119)

* feat: add support for conditional requests * feat: add unit tests for conditional requests * feat: use invariant * feat: add type guard * feat: fix lint * feat: add more unit tests
preset-io · Apr 2, 2019 · 2ca55ed · 2ca55ed
1 parent 5aa383e
commit 2ca55ed
Show file tree

Hide file tree

Showing 6 changed files with 173 additions and 1 deletion.
diff --git a/package.json b/package.json
@@ -66,6 +66,12 @@
       "typescript"
     ],
     "jest": {
+      "globals": {
+        "caches": true
+      },
+      "setupFiles": [
+        "<rootDir>/test/setupTests.ts"
+      ],
       "testPathIgnorePatterns": [
         "<rootDir>/packages/generator-superset"
       ],

diff --git a/packages/superset-ui-connection/README.md b/packages/superset-ui-connection/README.md
@@ -23,6 +23,7 @@ a high-level it supports:
 - supports `GET` and `POST` requests (no `PUT` or `DELETE`)
 - timeouts
 - query aborts through the `AbortController` API
+- conditional `GET` requests using `If-None-Match` and `ETag` headers
 
 #### Example usage
 

diff --git a/packages/superset-ui-connection/src/callApi/callApi.ts b/packages/superset-ui-connection/src/callApi/callApi.ts
@@ -1,5 +1,7 @@
+/* eslint compat/compat: 'off' */
 import 'whatwg-fetch';
 import { CallApi } from '../types';
+import { CACHE_AVAILABLE, CACHE_KEY, HTTP_STATUS_NOT_MODIFIED, HTTP_STATUS_OK } from '../constants';
 
 // This function fetches an API response and returns the corresponding json
 export default function callApi({
@@ -26,6 +28,38 @@ export default function callApi({
     signal,
   };
 
+  if (method === 'GET' && CACHE_AVAILABLE) {
+    return caches.open(CACHE_KEY).then(supersetCache =>
+      supersetCache
+        .match(url)
+        .then(cachedResponse => {
+          if (cachedResponse) {
+            // if we have a cached response, send its ETag in the
+            // `If-None-Match` header in a conditional request
+            const etag = cachedResponse.headers.get('Etag') as string;
+            request.headers = { ...request.headers, 'If-None-Match': etag };
+          }
+
+          return fetch(url, request);
+        })
+        .then(response => {
+          if (response.status === HTTP_STATUS_NOT_MODIFIED) {
+            return supersetCache.match(url).then(cachedResponse => {
+              if (cachedResponse) {
+                return cachedResponse.clone();
+              }
+              throw new Error('Received 304 but no content is cached!');
+            });
+          } else if (response.status === HTTP_STATUS_OK && response.headers.get('Etag')) {
+            supersetCache.delete(url);
+            supersetCache.put(url, response.clone());
+          }
+
+          return response;
+        }),
+    );
+  }
+
   if (
     (method === 'POST' || method === 'PATCH' || method === 'PUT') &&
     typeof postPayload === 'object'
@@ -44,5 +78,5 @@ export default function callApi({
     request.body = formData;
   }
 
-  return fetch(url, request); // eslint-disable-line compat/compat
+  return fetch(url, request);
 }
diff --git a/packages/superset-ui-connection/src/constants.ts b/packages/superset-ui-connection/src/constants.ts
@@ -0,0 +1,7 @@
+// HTTP status codes
+export const HTTP_STATUS_OK = 200;
+export const HTTP_STATUS_NOT_MODIFIED = 304;
+
+// Namespace for Cache API
+export const CACHE_AVAILABLE = 'caches' in self;
+export const CACHE_KEY = '@SUPERSET-UI/CONNECTION';
diff --git a/packages/superset-ui-connection/test/callApi/callApi.test.ts b/packages/superset-ui-connection/test/callApi/callApi.test.ts
@@ -1,6 +1,7 @@
 /* eslint promise/no-callback-in-promise: 'off' */
 import fetchMock from 'fetch-mock';
 import callApi from '../../src/callApi/callApi';
+import * as constants from '../../src/constants';
 
 import { LOGIN_GLOB } from '../fixtures/constants';
 import throwIfCalled from '../utils/throwIfCalled';
@@ -17,16 +18,23 @@ describe('callApi()', () => {
   const mockPostUrl = '/mock/post/url';
   const mockPutUrl = '/mock/put/url';
   const mockPatchUrl = '/mock/patch/url';
+  const mockCacheUrl = '/mock/cache/url';
 
   const mockGetPayload = { get: 'payload' };
   const mockPostPayload = { post: 'payload' };
   const mockPutPayload = { post: 'payload' };
   const mockPatchPayload = { post: 'payload' };
+  const mockCachePayload = {
+    status: 200,
+    body: 'BODY',
+    headers: { Etag: 'etag' },
+  };
 
   fetchMock.get(mockGetUrl, mockGetPayload);
   fetchMock.post(mockPostUrl, mockPostPayload);
   fetchMock.put(mockPutUrl, mockPutPayload);
   fetchMock.patch(mockPatchUrl, mockPatchPayload);
+  fetchMock.get(mockCacheUrl, mockCachePayload);
 
   afterEach(fetchMock.reset);
 
@@ -292,6 +300,92 @@ describe('callApi()', () => {
     });
   });
 
+  it('caches requests with ETags', () =>
+    callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+      const calls = fetchMock.calls(mockCacheUrl);
+      expect(calls).toHaveLength(1);
+
+      return caches.open(constants.CACHE_KEY).then(supersetCache =>
+        supersetCache.match(mockCacheUrl).then(cachedResponse => {
+          expect(cachedResponse).toBeDefined();
+
+          return Promise.resolve();
+        }),
+      );
+    }));
+
+  it('works when the Cache API is disabled', () => {
+    Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false });
+
+    return callApi({ url: mockCacheUrl, method: 'GET' }).then(firstResponse => {
+      const calls = fetchMock.calls(mockCacheUrl);
+      expect(calls).toHaveLength(1);
+      expect(firstResponse.body).toEqual('BODY');
+
+      return callApi({ url: mockCacheUrl, method: 'GET' }).then(secondResponse => {
+        const fetchParams = calls[1][1];
+        expect(calls).toHaveLength(2);
+
+        // second call should not have If-None-Match header
+        expect(fetchParams.headers).toBeUndefined();
+        expect(secondResponse.body).toEqual('BODY');
+
+        Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true });
+
+        return Promise.resolve();
+      });
+    });
+  });
+
+  it('sends known ETags in the If-None-Match header', () =>
+    // first call sets the cache
+    callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+      const calls = fetchMock.calls(mockCacheUrl);
+      expect(calls).toHaveLength(1);
+
+      // second call sends the Etag in the If-None-Match header
+      return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+        const fetchParams = calls[1][1];
+        const headers = { 'If-None-Match': 'etag' };
+        expect(calls).toHaveLength(2);
+        expect(fetchParams.headers).toEqual(expect.objectContaining(headers));
+
+        return Promise.resolve();
+      });
+    }));
+
+  it('reuses cached responses on 304 status', () =>
+    // first call sets the cache
+    callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+      const calls = fetchMock.calls(mockCacheUrl);
+      expect(calls).toHaveLength(1);
+
+      // second call reuses the cached payload on a 304
+      const mockCachedPayload = { status: 304 };
+      fetchMock.get(mockCacheUrl, mockCachedPayload, { overwriteRoutes: true });
+
+      return callApi({ url: mockCacheUrl, method: 'GET' }).then(response => {
+        expect(calls).toHaveLength(2);
+        expect(response.body).toEqual('BODY');
+
+        return Promise.resolve();
+      });
+    }));
+
+  it('throws error when cache fails on 304', () => {
+    // this should never happen, since a 304 is only returned if we have
+    // the cached response and sent the If-None-Match header
+    const mockUncachedUrl = '/mock/uncached/url';
+    const mockCachedPayload = { status: 304 };
+    fetchMock.get(mockUncachedUrl, mockCachedPayload);
+
+    return callApi({ url: mockUncachedUrl, method: 'GET' }).catch(error => {
+      const calls = fetchMock.calls(mockUncachedUrl);
+      expect(calls).toHaveLength(1);
+      expect(error.message).toEqual('Received 304 but no content is cached!');
+    });
+  });
+
   it('rejects if the request throws', () => {
     const mockErrorUrl = '/mock/error/url';
     const mockErrorPayload = { status: 500, statusText: 'Internal error' };

diff --git a/test/setupTests.ts b/test/setupTests.ts
@@ -0,0 +1,30 @@
+const caches = {};
+
+class Cache {
+  cache: object;
+  constructor(key: string) {
+    caches[key] = caches[key] || {};
+    this.cache = caches[key];
+  }
+  match(url: string): Promise<Response | undefined> {
+    return new Promise((resolve, reject) => resolve(this.cache[url]));
+  }
+  delete(url: string): Promise<boolean> {
+    delete this.cache[url];
+    return new Promise((resolve, reject) => resolve(true));
+  }
+  put(url: string, response: Response): Promise<void> {
+    this.cache[url] = response;
+    return Promise.resolve();
+  }
+};
+
+class CacheStorage {
+  open(key: string): Promise<Cache> {
+    return new Promise((resolve, reject) => {
+      resolve(new Cache(key));
+    });
+  }
+};
+
+global.caches = new CacheStorage();