fix(serviceConfig): caching reset on config update (RedHatInsights#879)

* config, resets response cache when props are updated
* testing, expand testing baseline for config rewrite

.eslintrc

@@ -121,6 +121,7 @@
     "no-unsafe-optional-chaining": 1,
     "prefer-exponentiation-operator": 0,
     "prefer-promise-reject-errors": 1,
+    "prefer-regex-literals": 0,
     "prettier/prettier": [
       "error",
       {

src/services/__tests__/__snapshots__/config.test.js.snap

@@ -2,9 +2,12 @@
 
 exports[`ServiceConfig should handle caching service calls: cached responses, emulated 304 1`] = `
 Array [
-  200,
-  304,
-  200,
+  "1. method=get, status=200, desc=initial call",
+  "2. method=get, status=304, desc=repeat 1st call and config",
+  "3. method=get, status=200, desc=updated config",
+  "4. method=post, status=200, desc=attempt post method",
+  "5. method=get, status=304, desc=repeat 3rd call and config",
+  "6. method=get, status=200, desc=no caching",
 ]
 `;
 
@@ -34,6 +37,7 @@ Array [
       "code": undefined,
       "config": Object {
         "adapter": [Function],
+        "cacheResponse": false,
         "cancelToken": CancelToken {
           "promise": Promise {
             "then": [Function],
@@ -68,6 +72,7 @@ Array [
       "request": Request {
         "config": Object {
           "adapter": [Function],
+          "cacheResponse": false,
           "cancelToken": CancelToken {
             "promise": Promise {
               "then": [Function],
@@ -113,3 +118,53 @@ Array [
   },
 ]
 `;
+
+exports[`ServiceConfig should handle producing a service call configuration: response configs 1`] = `
+Array [
+  "{
+  transitional: {
+    silentJSONParsing: true,
+    forcedJSONParsing: true,
+    clarifyTimeoutError: false
+  },
+  adapter: function mockAdapter(config) {\\\\n\\\\t  return new Promise(function (resolve, reject) {\\\\n\\\\t    var request = new Request(resolve, reject, config);\\\\n\\\\t    moxios.requests.track(request);\\\\n\\\\t\\\\n\\\\t    // Check for matching stub to auto respond with\\\\n\\\\t    for (var i = 0, l = moxios.stubs.count(); i < l; i++) {\\\\n\\\\t      var stub = moxios.stubs.at(i);\\\\n\\\\t      var correctURL = stub.url instanceof RegExp ? stub.url.test(request.url) : stub.url === request.url;\\\\n\\\\t      var correctMethod = true;\\\\n\\\\t\\\\n\\\\t      if (stub.method !== undefined) {\\\\n\\\\t        correctMethod = stub.method.toLowerCase() === request.config.method.toLowerCase();\\\\n\\\\t      }\\\\n\\\\t\\\\n\\\\t      if (correctURL && correctMethod) {\\\\n\\\\t        if (stub.timeout) {\\\\n\\\\t          throwTimeout(config);\\\\n\\\\t        }\\\\n\\\\t        request.respondWith(stub.response);\\\\n\\\\t        stub.resolve();\\\\n\\\\t        break;\\\\n\\\\t      }\\\\n\\\\t    }\\\\n\\\\t  });\\\\n\\\\t},
+  transformRequest: [
+    function transformRequest(data, headers) {\\\\n    normalizeHeaderName(headers, 'Accept');\\\\n    normalizeHeaderName(headers, 'Content-Type');\\\\n\\\\n    if (utils.isFormData(data) ||\\\\n      utils.isArrayBuffer(data) ||\\\\n      utils.isBuffer(data) ||\\\\n      utils.isStream(data) ||\\\\n      utils.isFile(data) ||\\\\n      utils.isBlob(data)\\\\n    ) {\\\\n      return data;\\\\n    }\\\\n    if (utils.isArrayBufferView(data)) {\\\\n      return data.buffer;\\\\n    }\\\\n    if (utils.isURLSearchParams(data)) {\\\\n      setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8');\\\\n      return data.toString();\\\\n    }\\\\n    if (utils.isObject(data) || (headers && headers['Content-Type'] === 'application/json')) {\\\\n      setContentTypeIfUnset(headers, 'application/json');\\\\n      return stringifySafely(data);\\\\n    }\\\\n    return data;\\\\n  }
+  ],
+  transformResponse: [
+    function transformResponse(data) {\\\\n    var transitional = this.transitional || defaults.transitional;\\\\n    var silentJSONParsing = transitional && transitional.silentJSONParsing;\\\\n    var forcedJSONParsing = transitional && transitional.forcedJSONParsing;\\\\n    var strictJSONParsing = !silentJSONParsing && this.responseType === 'json';\\\\n\\\\n    if (strictJSONParsing || (forcedJSONParsing && utils.isString(data) && data.length)) {\\\\n      try {\\\\n        return JSON.parse(data);\\\\n      } catch (e) {\\\\n        if (strictJSONParsing) {\\\\n          if (e.name === 'SyntaxError') {\\\\n            throw enhanceError(e, this, 'E_JSON_PARSE');\\\\n          }\\\\n          throw e;\\\\n        }\\\\n      }\\\\n    }\\\\n\\\\n    return data;\\\\n  }
+  ],
+  timeout: 60000,
+  xsrfCookieName: XSRF-TOKEN,
+  xsrfHeaderName: X-XSRF-TOKEN,
+  maxContentLength: -1,
+  maxBodyLength: -1,
+  validateStatus: function validateStatus(status) {\\\\n    return status >= 200 && status < 300;\\\\n  },
+  headers: {
+    Accept: application/json, text/plain, */*
+  },
+  exposeCacheId: true,
+  url: /test/,
+  params: {
+    lorem: ipsum,
+    dolor: sit
+  },
+  schema: [
+    successResponse => \`\${successResponse}-schema-transform\`
+  ],
+  transform: [
+    successResponse => \`\${successResponse}-transform\`
+  ],
+  cacheResponse: false,
+  method: get,
+  cacheId: null
+}",
+]
+`;
+
+exports[`ServiceConfig should handle transforming service call responses: transformed responses 1`] = `
+Array [
+  "success-schema-transform",
+  "success-transform",
+]
+`;

src/services/__tests__/config.test.js

@@ -15,6 +15,19 @@ describe('ServiceConfig', () => {
     return response;
   };
 
+  // JSON stringify, and replace functions as strings
+  const stringifyConfig = config =>
+    JSON.stringify(
+      config,
+      (key, value) => {
+        if (typeof value === 'function') {
+          return value.toString();
+        }
+        return value;
+      },
+      2
+    ).replace(new RegExp('\\"', 'g'), '');
+
   beforeAll(() => {
     moxios.install();
 
@@ -50,6 +63,22 @@ describe('ServiceConfig', () => {
     expect(configObject.timeout).toBe(3);
   });
 
+  it('should handle producing a service call configuration', async () => {
+    const config = [];
+
+    const responseOne = await service.serviceCall({
+      cache: false,
+      exposeCacheId: true,
+      url: '/test/',
+      params: { lorem: 'ipsum', dolor: 'sit' },
+      schema: [successResponse => `${successResponse}-schema-transform`],
+      transform: [successResponse => `${successResponse}-transform`]
+    });
+    config.push(stringifyConfig(responseOne.request.config));
+
+    expect(config).toMatchSnapshot('response configs');
+  });
+
   it('should handle a bundled authentication and service call', async () => {
     const response = await service.serviceCall({ url: '/test/' });
 
@@ -85,22 +114,79 @@ describe('ServiceConfig', () => {
     const responseOne = await service.serviceCall({
       cache: true,
       url: '/test/',
-      params: { lorem: 'ipsum', dolor: 'sit' }
+      params: { lorem: 'ipsum', dolor: 'sit' },
+      exposeCacheId: true
     });
-    responses.push(responseOne.status);
+    responses.push(`1. method=get, status=${responseOne.status}, desc=initial call`);
 
     // Second, call the same endpoint with same params, expect a cached response, emulated 304
     const responseTwo = await service.serviceCall({
       cache: true,
       url: '/test/',
-      params: { lorem: 'ipsum', dolor: 'sit' }
+      params: { lorem: 'ipsum', dolor: 'sit' },
+      exposeCacheId: true
     });
-    responses.push(responseTwo.status);
+    responses.push(`2. method=get, status=${responseTwo.status}, desc=repeat 1st call and config`);
 
-    // Third, updating params creates a new cache
-    const responseThree = await service.serviceCall({ cache: true, url: '/test/', params: { lorem: 'ipsum' } });
-    responses.push(responseThree.status);
+    // Third, updating config creates a new cache
+    const responseThree = await service.serviceCall({
+      cache: true,
+      url: '/test/',
+      params: { lorem: 'ipsum' },
+      exposeCacheId: true
+    });
+    responses.push(`3. method=get, status=${responseThree.status}, desc=updated config`);
+
+    // Fourth, confirm cache isn't created for other methods, i.e. post
+    const responseFour = await service.serviceCall({
+      cache: true,
+      method: 'post',
+      url: '/test/',
+      params: { lorem: 'ipsum' },
+      exposeCacheId: true
+    });
+    responses.push(`4. method=post, status=${responseFour.status}, desc=attempt post method`);
+
+    // Fifth, confirm cache exists from responseThree
+    const responseFive = await service.serviceCall({
+      cache: true,
+      url: '/test/',
+      params: { lorem: 'ipsum' },
+      exposeCacheId: true
+    });
+    responses.push(`5. method=get, status=${responseFive.status}, desc=repeat 3rd call and config`);
+
+    // Six, don't use a cache
+    const responseSix = await service.serviceCall({
+      cache: false,
+      url: '/test/',
+      params: { lorem: 'ipsum' },
+      exposeCacheId: true
+    });
+    responses.push(`6. method=get, status=${responseSix.status}, desc=no caching`);
 
     expect(responses).toMatchSnapshot('cached responses, emulated 304');
   });
+
+  it('should handle transforming service call responses', async () => {
+    const responses = [];
+
+    // First, use the schema transform - alias for transform, but relates a sequence and happens before other transformations.
+    const responseOne = await service.serviceCall({
+      cache: true,
+      url: '/test/',
+      schema: [successResponse => `${successResponse}-schema-transform`]
+    });
+    responses.push(responseOne.data);
+
+    // Second, use a transform
+    const responseTwo = await service.serviceCall({
+      cache: true,
+      url: '/test/',
+      transform: [successResponse => `${successResponse}-transform`]
+    });
+    responses.push(responseTwo.data);
+
+    expect(responses).toMatchSnapshot('transformed responses');
+  });
 });

src/services/config.js

@@ -1,5 +1,6 @@
 import axios, { CancelToken } from 'axios';
 import LruCache from 'lru-cache';
+import _isPlainObject from 'lodash/isPlainObject';
 import { platformServices } from './platform/platformServices';
 import { serviceHelpers } from './common/helpers';
 
@@ -34,6 +35,7 @@ const responseCache = new LruCache({
   updateAgeOnGet: true
 });
 
+// ToDo: consider another way of hashing cacheIDs. base64 could get a little large depending on settings, i.e. md5
 /**
  * Set Axios configuration, which includes response schema validation and caching.
  * Call platform "getUser" auth method, and apply service config. Service configuration
@@ -48,20 +50,42 @@ const responseCache = new LruCache({
  * @param {boolean} config.cache
  * @param {boolean} config.cancel
  * @param {string} config.cancelId
+ * @param {boolean} config.exposeCacheId
  * @param {Array} config.schema
  * @param {Array} config.transform
  * @returns {Promise<*>}
  */
 const serviceCall = async config => {
   await platformServices.getUser();
 
-  const updatedConfig = { ...config, cache: undefined, cacheResponse: config.cache };
+  const updatedConfig = { ...config, cache: undefined, cacheResponse: config.cache, method: config.method || 'get' };
   const responseTransformers = [];
   const cancelledMessage = 'cancelled request';
   const axiosInstance = axios.create();
-  const sortedParams =
-    (updatedConfig.params && Object.entries(updatedConfig.params).sort(([a], [b]) => a.localeCompare(b))) || [];
-  const cacheId = `${updatedConfig.url}-${JSON.stringify(sortedParams)}`;
+
+  // don't cache responses if "get" isn't used
+  updatedConfig.cacheResponse = updatedConfig.cacheResponse === true && updatedConfig.method === 'get';
+
+  // account for alterations to transforms, and other config props
+  const cacheId =
+    (updatedConfig.cacheResponse === true &&
+      `${btoa(
+        JSON.stringify(updatedConfig, (key, value) => {
+          if (value !== updatedConfig && _isPlainObject(value)) {
+            return (Object.entries(value).sort(([a], [b]) => a.localeCompare(b)) || []).toString();
+          }
+          if (typeof value === 'function') {
+            return value.toString();
+          }
+          return value;
+        })
+      )}`) ||
+    null;
+
+  // simple check to place responsibility on consumer, primarily used for testing
+  if (updatedConfig.exposeCacheId === true) {
+    updatedConfig.cacheId = cacheId;
+  }
 
   if (updatedConfig.cancel === true) {
     const cancelTokensId = `${updatedConfig.cancelId || ''}-${updatedConfig.url}`;