From 5d9d4562ef4e49b0075700f6b472e1efbf2900ec Mon Sep 17 00:00:00 2001 From: CD Cabrera Date: Tue, 1 Mar 2022 10:05:10 -0500 Subject: [PATCH] fix(serviceConfig): caching reset on config update (#879) * config, resets response cache when props are updated * testing, expand testing baseline for config rewrite --- .eslintrc | 1 + .../__snapshots__/config.test.js.snap | 160 +++++++++++++++++- src/services/__tests__/config.test.js | 151 ++++++++++++++++- src/services/config.js | 34 +++- 4 files changed, 331 insertions(+), 15 deletions(-) diff --git a/.eslintrc b/.eslintrc index 7970c6848..58767d6d5 100644 --- a/.eslintrc +++ b/.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", { diff --git a/src/services/__tests__/__snapshots__/config.test.js.snap b/src/services/__tests__/__snapshots__/config.test.js.snap index 9e8b05aa3..0522e9cee 100644 --- a/src/services/__tests__/__snapshots__/config.test.js.snap +++ b/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,101 @@ Array [ "code": undefined, "config": Object { "adapter": [Function], + "cacheResponse": false, + "cancelToken": CancelToken { + "promise": Promise { + "then": [Function], + }, + }, + "data": undefined, + "headers": Object { + "Accept": "application/json, text/plain, */*", + "Content-Type": "application/x-www-form-urlencoded", + }, + "maxBodyLength": -1, + "maxContentLength": -1, + "method": "post", + "timeout": "60000", + "transformRequest": Array [ + [Function], + ], + "transformResponse": Array [ + [Function], + ], + "transitional": Object { + "clarifyTimeoutError": false, + "forcedJSONParsing": true, + "silentJSONParsing": true, + }, + "url": "/test/", + "validateStatus": [Function], + "xsrfCookieName": "XSRF-TOKEN", + "xsrfHeaderName": "X-XSRF-TOKEN", + }, + "data": "success", + "headers": undefined, + "request": Request { + "config": Object { + "adapter": [Function], + "cacheResponse": false, + "cancelToken": CancelToken { + "promise": Promise { + "then": [Function], + }, + }, + "data": undefined, + "headers": Object { + "Accept": "application/json, text/plain, */*", + "Content-Type": "application/x-www-form-urlencoded", + }, + "maxBodyLength": -1, + "maxContentLength": -1, + "method": "post", + "timeout": "60000", + "transformRequest": Array [ + [Function], + ], + "transformResponse": Array [ + [Function], + ], + "transitional": Object { + "clarifyTimeoutError": false, + "forcedJSONParsing": true, + "silentJSONParsing": true, + }, + "url": "/test/", + "validateStatus": [Function], + "xsrfCookieName": "XSRF-TOKEN", + "xsrfHeaderName": "X-XSRF-TOKEN", + }, + "headers": Object { + "Accept": "application/json, text/plain, */*", + "Content-Type": "application/x-www-form-urlencoded", + }, + "reject": [Function], + "resolve": [Function], + "responseType": undefined, + "timeout": "60000", + "url": "/test/", + "withCredentials": false, + }, + "status": 200, + "statusText": undefined, + }, + }, + Object { + "reason": Cancel { + "message": "cancelled request", + }, + "status": "rejected", + }, + Object { + "status": "fulfilled", + "value": Response { + "code": undefined, + "config": Object { + "adapter": [Function], + "cacheResponse": false, "cancelToken": CancelToken { "promise": Promise { "then": [Function], @@ -68,6 +166,7 @@ Array [ "request": Request { "config": Object { "adapter": [Function], + "cacheResponse": false, "cancelToken": CancelToken { "promise": Promise { "then": [Function], @@ -113,3 +212,58 @@ 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", + "error-error-transform", + Array [ + "cancelled request", + undefined, + ], +] +`; diff --git a/src/services/__tests__/config.test.js b/src/services/__tests__/config.test.js index 8b7909686..1804f2c87 100644 --- a/src/services/__tests__/config.test.js +++ b/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(); @@ -23,6 +36,12 @@ describe('ServiceConfig', () => { responseText: 'success', timeout: 1 }); + + moxios.stubRequest(/\/error.*?/, { + status: 404, + responseText: 'error', + timeout: 1 + }); }); afterAll(() => { @@ -50,6 +69,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/' }); @@ -57,9 +92,12 @@ describe('ServiceConfig', () => { }); it('should handle cancelling service calls', async () => { + // Highlight cancel takes into account url and method const responseAll = await returnPromiseAsync(() => Promise.all([ service.serviceCall({ url: '/test/', cancel: true }), + service.serviceCall({ url: '/test/', method: 'post', cancel: true }), + service.serviceCall({ url: '/test/', method: 'post', cancel: true }), service.serviceCall({ url: '/test/', cancel: true }), service.serviceCall({ url: '/test/', cancel: true }) ]) @@ -67,9 +105,12 @@ describe('ServiceConfig', () => { expect(responseAll).toMatchSnapshot('cancelled request, Promise.all'); + // Highlight cancel takes into account url and method const responseAllSettled = await returnPromiseAsync(() => Promise.allSettled([ service.serviceCall({ url: '/test/', cancel: true }), + service.serviceCall({ url: '/test/', method: 'post', cancel: true }), + service.serviceCall({ url: '/test/', method: 'post', cancel: true }), service.serviceCall({ url: '/test/', cancel: true }), service.serviceCall({ url: '/test/', cancel: true }) ]) @@ -85,22 +126,118 @@ 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(`2. method=get, status=${responseTwo.status}, desc=repeat 1st call and config`); + + // 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(responseTwo.status); + responses.push(`5. method=get, status=${responseFive.status}, desc=repeat 3rd 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); + // 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); + + // Third, use error transform + let responseThree; + try { + responseThree = await service.serviceCall({ + cache: true, + url: '/error/', + transform: [ + successResponse => `${successResponse}-transform`, + errorResponse => `${errorResponse}-error-transform` + ] + }); + } catch (e) { + responseThree = e.response || e; + } + + responses.push(responseThree.data); + + // Fourth, use error transform with cancel + const responseFourConfig = { + cache: true, + cancel: true, + url: '/error/', + transform: [ + successResponse => `${successResponse}-transform`, + errorResponse => `${errorResponse}-cancel-transform` + ] + }; + + const responseFour = await Promise.allSettled([ + service.serviceCall({ + ...responseFourConfig + }), + service.serviceCall({ + ...responseFourConfig + }) + ]); + + responses.push(responseFour.map(({ reason }) => reason.message)); + + expect(responses).toMatchSnapshot('transformed responses'); + }); }); diff --git a/src/services/config.js b/src/services/config.js index 5322d9d7f..01dae75d5 100644 --- a/src/services/config.js +++ b/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,6 +50,7 @@ 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<*>} @@ -55,16 +58,37 @@ const responseCache = new LruCache({ 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}`; + const cancelTokensId = `${updatedConfig.cancelId || ''}-${updatedConfig.method}-${updatedConfig.url}`; if (cancelTokens[cancelTokensId]) { cancelTokens[cancelTokensId].cancel(cancelledMessage);