Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(serviceConfig): sw-630 hash for cache, cancel #996

Merged
merged 2 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"@redhat-cloud-services/frontend-components-utilities": "3.3.5",
"axios": "^0.27.2",
"classnames": "^2.3.2",
"crypto-js": "^4.1.1",
"i18next": "^22.0.4",
"i18next-http-backend": "^2.0.0",
"iso-639-1": "^2.1.15",
Expand Down
7 changes: 7 additions & 0 deletions src/common/__tests__/__snapshots__/helpers.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ exports[`Helpers should expose a window object: limited window object 1`] = `
"UI_WINDOW_ID": "curiosity",
"aggregatedError": [Function],
"browserExpose": [Function],
"generateHash": [Function],
"generateId": [Function],
"isDate": [Function],
"isPromise": [Function],
Expand Down Expand Up @@ -79,6 +80,7 @@ exports[`Helpers should expose a window object: window object 1`] = `
"UI_WINDOW_ID": "curiosity",
"aggregatedError": [Function],
"browserExpose": [Function],
"generateHash": [Function],
"generateId": [Function],
"isDate": [Function],
"isPromise": [Function],
Expand Down Expand Up @@ -133,6 +135,7 @@ exports[`Helpers should have specific functions: helpers 1`] = `
"UI_WINDOW_ID": "curiosity",
"aggregatedError": [Function],
"browserExpose": [Function],
"generateHash": [Function],
"generateId": [Function],
"isDate": [Function],
"isPromise": [Function],
Expand All @@ -142,3 +145,7 @@ exports[`Helpers should have specific functions: helpers 1`] = `
"numberDisplay": [Function],
}
`;

exports[`Helpers should support generated consistent hashes from objects: hash 1`] = `"216252ce5b9858560d4a8134f2106fc8b1141b70"`;

exports[`Helpers should support generated consistent hashes from objects: method md5 1`] = `"a105a8ae3437698acca4ca9d5b2b4dde"`;
13 changes: 13 additions & 0 deletions src/common/__tests__/helpers.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cryptoMd5 from 'crypto-js/md5';
import { helpers } from '../helpers';

describe('Helpers', () => {
Expand Down Expand Up @@ -29,6 +30,18 @@ describe('Helpers', () => {
expect(helpers.generateId('lorem')).toBe('lorem-');
});

it('should support generated consistent hashes from objects', () => {
expect(
helpers.generateHash({ lorem: 'ipsum', dolor: ['sit', null, undefined, 1, () => 'hello world'] })
).toMatchSnapshot('hash');
expect(
helpers.generateHash(
{ lorem: 'ipsum', dolor: ['sit', null, undefined, 1, () => 'hello world'] },
{ method: cryptoMd5 }
)
).toMatchSnapshot('method md5');
});

it('should determine a date', () => {
expect(helpers.isDate(new Date('broken'))).toBe(true);
expect(helpers.isDate(Date)).toBe(false);
Expand Down
27 changes: 27 additions & 0 deletions src/common/helpers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import numbro from 'numbro';
import cryptoSha1 from 'crypto-js/sha1';
import _isPlainObject from 'lodash/isPlainObject';

/**
* Fill for AggregatedError
Expand Down Expand Up @@ -49,6 +51,30 @@ const isDate = date => Object.prototype.toString.call(date) === '[object Date]';
*/
const isPromise = obj => /^\[object (Promise|Async|AsyncFunction)]/.test(Object.prototype.toString.call(obj));

/**
* Generate a consistent hash
*
* @param {object} obj
* @param {object} options
* @param {Function} options.method
* @returns {*|string}
*/
const generateHash = (obj, { method = cryptoSha1 } = {}) =>
method(
JSON.stringify(
Object.entries(obj).sort(([a], [b]) => a.localeCompare(b)),
(key, value) => {
if (value !== obj && _isPlainObject(value)) {
return JSON.stringify(Object.entries(value).sort(([a], [b]) => a.localeCompare(b)) || []);
}
if (typeof value === 'function') {
return value.toString();
}
return value;
}
)
).toString();

/**
* An empty function.
* Typically used as a default prop.
Expand Down Expand Up @@ -331,6 +357,7 @@ const browserExpose = (obj = {}, options) => {
const helpers = {
aggregatedError,
browserExpose,
generateHash,
generateId,
isDate,
isPromise,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ exports[`Service Helpers should attempt to apply a Joi schema to a response: sch
exports[`Service Helpers should have specific functions: serviceHelpers 1`] = `
{
"camelCase": [Function],
"generateHash": [Function],
"passDataToCallback": [Function],
"schemaResponse": [Function],
"timeoutFunctionCancel": [Function],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ exports[`ServiceConfig should allow passing a function and emulating a service c

exports[`ServiceConfig should handle caching service calls: cached responses, emulated 304 1`] = `
[
"1. method=get, status=200, cacheId=eyJ0aW1lb3V0Ijo2MDAwMCwidXJsIjoiL3Rlc3QvIiwicGFyYW1zIjoiW1tcImRvbG9yXCIsXCJzaXRcIl0sW1wibG9yZW1cIixcImlwc3VtXCJdXSIsImV4cG9zZUNhY2hlSWQiOnRydWUsImNhY2hlUmVzcG9uc2UiOnRydWUsIm1ldGhvZCI6ImdldCJ9, desc=initial call",
"2. method=get, status=304, cacheId=eyJ0aW1lb3V0Ijo2MDAwMCwidXJsIjoiL3Rlc3QvIiwicGFyYW1zIjoiW1tcImRvbG9yXCIsXCJzaXRcIl0sW1wibG9yZW1cIixcImlwc3VtXCJdXSIsImV4cG9zZUNhY2hlSWQiOnRydWUsImNhY2hlUmVzcG9uc2UiOnRydWUsIm1ldGhvZCI6ImdldCJ9, desc=repeat 1st call and config",
"3. method=get, status=200, cacheId=eyJ0aW1lb3V0Ijo2MDAwMCwidXJsIjoiL3Rlc3QvIiwicGFyYW1zIjoiW1tcImxvcmVtXCIsXCJpcHN1bVwiXV0iLCJleHBvc2VDYWNoZUlkIjp0cnVlLCJjYWNoZVJlc3BvbnNlIjp0cnVlLCJtZXRob2QiOiJnZXQifQ==, desc=updated config",
"1. method=get, status=200, cacheId=0ec09bb76453e6a9c190cade66c6127fc0a423bf, desc=initial call",
"2. method=get, status=304, cacheId=0ec09bb76453e6a9c190cade66c6127fc0a423bf, desc=repeat 1st call and config",
"3. method=get, status=200, cacheId=27842fbee2f81542b8d1c7513810a502746f6408, desc=updated config",
"4. method=post, status=200, cacheId=null, desc=attempt post method",
"5. method=get, status=304, cacheId=eyJ0aW1lb3V0Ijo2MDAwMCwidXJsIjoiL3Rlc3QvIiwicGFyYW1zIjoiW1tcImxvcmVtXCIsXCJpcHN1bVwiXV0iLCJleHBvc2VDYWNoZUlkIjp0cnVlLCJjYWNoZVJlc3BvbnNlIjp0cnVlLCJtZXRob2QiOiJnZXQifQ==, desc=repeat 3rd call and config",
"5. method=get, status=304, cacheId=27842fbee2f81542b8d1c7513810a502746f6408, desc=repeat 3rd call and config",
"6. method=get, status=200, cacheId=null, desc=no caching",
]
`;
Expand Down
7 changes: 7 additions & 0 deletions src/services/common/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import _camelCase from 'lodash/camelCase';
import _isPlainObject from 'lodash/isPlainObject';
import { helpers } from '../../common';

/**
* Pass through generate hash
*/
const { generateHash } = helpers;

/**
* A timeout cancel for function calls.
*
Expand Down Expand Up @@ -111,6 +116,7 @@ const schemaResponse = ({ casing, convert = true, id = null, response, schema }

const serviceHelpers = {
camelCase,
generateHash,
passDataToCallback,
schemaResponse,
timeoutFunctionCancel
Expand All @@ -120,6 +126,7 @@ export {
serviceHelpers as default,
serviceHelpers,
camelCase,
generateHash,
passDataToCallback,
schemaResponse,
timeoutFunctionCancel
Expand Down
20 changes: 2 additions & 18 deletions src/services/common/serviceConfig.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import axios, { CancelToken } from 'axios';
import LruCache from 'lru-cache';
import _isPlainObject from 'lodash/isPlainObject';
import { serviceHelpers } from './helpers';

/**
Expand Down Expand Up @@ -68,30 +67,15 @@ const axiosServiceCall = async (
updatedConfig.cacheResponse = updatedConfig.cacheResponse === true && updatedConfig.method === 'get';

// account for alterations to transforms, and other config props
const cacheId =
(updatedConfig.cacheResponse === true &&
`${window.btoa(
JSON.stringify(updatedConfig, (key, value) => {
if (value !== updatedConfig && _isPlainObject(value)) {
return JSON.stringify(Object.entries(value).sort(([a], [b]) => a.localeCompare(b)) || []);
}
if (typeof value === 'function') {
return value.toString();
}
return value;
})
)}`) ||
null;
const cacheId = (updatedConfig.cacheResponse === true && serviceHelpers.generateHash(updatedConfig)) || 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.method}-${
(typeof updatedConfig.url === 'function' && updatedConfig.url.toString()) || updatedConfig.url
}`;
const cancelTokensId = serviceHelpers.generateHash(updatedConfig);

if (globalCancelTokens[cancelTokensId]) {
globalCancelTokens[cancelTokensId].cancel(cancelledMessage);
Expand Down
2 changes: 1 addition & 1 deletion tests/__snapshots__/code.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ exports[`General code checks should only have specific console.[warn|log|info|er
"index.js:12: console.log(\`Emulated appNavClick: \${JSON.stringify({ id, ...rest })}\`);",
"redux/common/reduxHelpers.js:282: console.error(\`Error: Property \${prop} does not exist within the passed state.\`, state);",
"redux/common/reduxHelpers.js:286: console.warn(\`Warning: Property \${prop} does not exist within the passed initialState.\`, initialState);",
"services/common/helpers.js:95: console.error(",
"services/common/helpers.js:100: console.error(",
]
`;
10 changes: 5 additions & 5 deletions tests/__snapshots__/dist.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/js/1824*js",
"./dist/js/1858*js",
"./dist/js/2195*js",
"./dist/js/2211*js",
"./dist/js/2211*txt",
"./dist/js/2217*js",
"./dist/js/2227*js",
"./dist/js/2243*js",
Expand All @@ -40,6 +42,7 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/js/3128*js",
"./dist/js/3267*js",
"./dist/js/3280*js",
"./dist/js/3396*js",
"./dist/js/3557*js",
"./dist/js/3577*js",
"./dist/js/3722*js",
Expand Down Expand Up @@ -69,7 +72,6 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/js/5876*js",
"./dist/js/5993*js",
"./dist/js/608*js",
"./dist/js/6395*js",
"./dist/js/6402*js",
"./dist/js/6706*js",
"./dist/js/6706*txt",
Expand Down Expand Up @@ -102,8 +104,6 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/js/9517*js",
"./dist/js/9669*js",
"./dist/js/9669*txt",
"./dist/js/9706*js",
"./dist/js/9706*txt",
"./dist/js/9844*js",
"./dist/js/9928*js",
"./dist/js/9942*js",
Expand All @@ -122,6 +122,7 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/sourcemaps/1824*map",
"./dist/sourcemaps/1858*map",
"./dist/sourcemaps/2195*map",
"./dist/sourcemaps/2211*map",
"./dist/sourcemaps/2217*map",
"./dist/sourcemaps/2227*map",
"./dist/sourcemaps/2243*map",
Expand All @@ -134,6 +135,7 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/sourcemaps/3128*map",
"./dist/sourcemaps/3267*map",
"./dist/sourcemaps/3280*map",
"./dist/sourcemaps/3396*map",
"./dist/sourcemaps/3557*map",
"./dist/sourcemaps/3577*map",
"./dist/sourcemaps/3722*map",
Expand All @@ -159,7 +161,6 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/sourcemaps/5876*map",
"./dist/sourcemaps/5993*map",
"./dist/sourcemaps/608*map",
"./dist/sourcemaps/6395*map",
"./dist/sourcemaps/6402*map",
"./dist/sourcemaps/6706*map",
"./dist/sourcemaps/6816*map",
Expand Down Expand Up @@ -187,7 +188,6 @@ exports[`Build distribution should match a specific file output 1`] = `
"./dist/sourcemaps/9407*map",
"./dist/sourcemaps/9517*map",
"./dist/sourcemaps/9669*map",
"./dist/sourcemaps/9706*map",
"./dist/sourcemaps/9844*map",
"./dist/sourcemaps/9928*map",
"./dist/sourcemaps/9942*map",
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5188,6 +5188,11 @@ cross-spawn@^7.0.2, cross-spawn@^7.0.3:
shebang-command "^2.0.0"
which "^2.0.1"

crypto-js@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/crypto-js/-/crypto-js-4.1.1.tgz#9e485bcf03521041bd85844786b83fb7619736cf"
integrity sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw==

crypto-random-string@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/crypto-random-string/-/crypto-random-string-2.0.0.tgz#ef2a7a966ec11083388369baa02ebead229b30d5"
Expand Down