ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1314 from ing-bank/fix/ajax

Browse source 
fix(ajax): bug fixes and readability improvements
This commit is contained in:
Joren Broekema 2021-04-07 17:27:55 +02:00 committed by GitHub
commit a4e99cc91e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 116 additions and 100 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/eight-pumpkins-clean.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/ajax': patch
---
Fix(ajax) options expansion, fix removing request interceptors, use 1 hour as default time to live, check for null when serializing the search params

49
packages/ajax/src/AjaxClient.js
View file

@ -27,11 +27,11 @@ export class AjaxClient {
xsrfCookieName: 'XSRF-TOKEN',
xsrfHeaderName: 'X-XSRF-TOKEN',
jsonPrefix: '',
...config,
cacheOptions: {
getCacheIdentifier: () => '_default',
...config.cacheOptions,
},
...config,
};
/** @type {Array.<RequestInterceptor|CachedRequestInterceptor>} */
@ -43,24 +43,18 @@ export class AjaxClient {
this.addRequestInterceptor(acceptLanguageRequestInterceptor);
}
if (this.__config.xsrfCookieName && this.__config.xsrfHeaderName) {
this.addRequestInterceptor(
createXSRFRequestInterceptor(this.__config.xsrfCookieName, this.__config.xsrfHeaderName),
);
const { xsrfCookieName, xsrfHeaderName } = this.__config;
if (xsrfCookieName && xsrfHeaderName) {
this.addRequestInterceptor(createXSRFRequestInterceptor(xsrfCookieName, xsrfHeaderName));
}
if (this.__config.cacheOptions && this.__config.cacheOptions.useCache) {
const { cacheOptions } = this.__config;
if (cacheOptions?.useCache) {
this.addRequestInterceptor(
cacheRequestInterceptorFactory(
this.__config.cacheOptions.getCacheIdentifier,
this.__config.cacheOptions,
),
cacheRequestInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
);
this.addResponseInterceptor(
cacheResponseInterceptorFactory(
this.__config.cacheOptions.getCacheIdentifier,
this.__config.cacheOptions,
),
cacheResponseInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
);
}
}
@ -84,10 +78,9 @@ export class AjaxClient {
/** @param {RequestInterceptor} requestInterceptor */
removeRequestInterceptor(requestInterceptor) {
const indexOf = this._requestInterceptors.indexOf(requestInterceptor);
if (indexOf !== -1) {
this._requestInterceptors.splice(indexOf);
}
this._requestInterceptors = this._requestInterceptors.filter(
interceptor => interceptor !== requestInterceptor,
);
}
/** @param {ResponseInterceptor} responseInterceptor */
@ -97,10 +90,9 @@ export class AjaxClient {
/** @param {ResponseInterceptor} responseInterceptor */
removeResponseInterceptor(responseInterceptor) {
const indexOf = this._responseInterceptors.indexOf(responseInterceptor);
if (indexOf !== -1) {
this._responseInterceptors.splice(indexOf, 1);
}
this._responseInterceptors = this._responseInterceptors.filter(
interceptor => interceptor !== responseInterceptor,
);
}
/**
@ -117,9 +109,9 @@ export class AjaxClient {
request.params = init?.params;
// run request interceptors, returning directly and skipping the network
// if a interceptor returns a Response
const interceptedRequestOrResponse = await this.__interceptRequest(request);
if (interceptedRequestOrResponse instanceof Response) {
// prevent network request, return cached response
return interceptedRequestOrResponse;
}
@ -147,12 +139,12 @@ export class AjaxClient {
const lionInit = {
...init,
headers: {
...(init && init.headers),
...init?.headers,
accept: 'application/json',
},
};
if (lionInit && lionInit.body) {
if (lionInit?.body) {
// eslint-disable-next-line no-param-reassign
lionInit.headers['content-type'] = 'application/json';
lionInit.body = JSON.stringify(lionInit.body);
@ -163,10 +155,9 @@ export class AjaxClient {
const response = await this.request(info, jsonInit);
let responseText = await response.text();
if (typeof this.__config.jsonPrefix === 'string') {
if (responseText.startsWith(this.__config.jsonPrefix)) {
responseText = responseText.substring(this.__config.jsonPrefix.length);
}
const { jsonPrefix } = this.__config;
if (typeof jsonPrefix === 'string' && responseText.startsWith(jsonPrefix)) {
responseText = responseText.substring(jsonPrefix.length);
}
try {

85
packages/ajax/src/interceptors-cache.js
View file

@ -6,15 +6,11 @@ import './typedef.js';
const SECOND = 1000;
const MINUTE = SECOND * 60;
const HOUR = MINUTE * 60;
const DEFAULT_TIME_TO_LIVE = HOUR;
class Cache {
constructor() {
this.expiration = new Date().getTime() + HOUR;
/**
* @type {{[url: string]: CacheConfig }}
*/
this.cacheConfig = {};
this.expiration = new Date().getTime() + DEFAULT_TIME_TO_LIVE;
/**
* @type {{[url: string]: {expires: number, response: CacheResponse} }}
* @private
@ -97,7 +93,7 @@ class Cache {
this._validateCache();
Object.keys(this._cacheObject).forEach(key => {
if (key.indexOf(url) > -1) {
if (key.includes(url)) {
delete this._cacheObject[key];
this.resolvePendingRequest(key);
}
@ -113,15 +109,9 @@ class Cache {
Object.keys(this._cacheObject).forEach(key => {
const notMatch = !new RegExp(regex).test(key);
if (notMatch) return;
const isDataDeleted = delete this._cacheObject[key];
delete this._cacheObject[key];
this.resolvePendingRequest(key);
if (!isDataDeleted) {
throw new Error(`Failed to delete cache for a request '${key}'`);
}
});
}
@ -135,6 +125,7 @@ class Cache {
if (new Date().getTime() > this.expiration) {
// @ts-ignore
this._cacheObject = {};
return false;
}
return true;
}
@ -150,7 +141,7 @@ let caches = {};
*/
export const searchParamSerializer = (params = {}) =>
// @ts-ignore
typeof params === 'object' ? new URLSearchParams(params).toString() : '';
typeof params === 'object' && params !== null ? new URLSearchParams(params).toString() : '';
/**
* Returns the active cache instance for the current session
@ -159,10 +150,9 @@ export const searchParamSerializer = (params = {}) =>
* @param {string} cacheIdentifier usually the refreshToken of the owner of the cache
*/
const getCache = cacheIdentifier => {
if (caches[cacheIdentifier] && caches[cacheIdentifier]._validateCache()) {
if (caches[cacheIdentifier]?._validateCache()) {
return caches[cacheIdentifier];
}
// invalidate old caches
caches = {};
// create new cache
@ -193,7 +183,7 @@ export const validateOptions = ({
// validate 'timeToLive', default 1 hour
if (timeToLive === undefined) {
timeToLive = 0;
timeToLive = DEFAULT_TIME_TO_LIVE;
}
if (Number.isNaN(parseInt(String(timeToLive), 10))) {
throw new Error('Property `timeToLive` must be of type `number`');
@ -213,7 +203,7 @@ export const validateOptions = ({
// validate 'requestIdentificationFn', default is url + searchParams
if (requestIdentificationFn) {
if (typeof requestIdentificationFn !== 'function') {
throw new Error('Property `requestIdentificationFn` must be of type `function` or `falsy`');
throw new Error('Property `requestIdentificationFn` must be of type `function`');
}
} else {
requestIdentificationFn = /** @param {any} data */ (
@ -235,25 +225,6 @@ export const validateOptions = ({
};
};
/**
* Request interceptor to return relevant cached requests
* @param {ValidatedCacheOptions} validatedInitialCacheOptions
* @param {CacheOptions=} configCacheOptions
* @returns {ValidatedCacheOptions}
*/
function composeCacheOptions(validatedInitialCacheOptions, configCacheOptions) {
let actionCacheOptions = validatedInitialCacheOptions;
if (configCacheOptions) {
actionCacheOptions = validateOptions({
...validatedInitialCacheOptions,
...configCacheOptions,
});
}
return actionCacheOptions;
}
/**
* Request interceptor to return relevant cached requests
* @param {function(): string} getCacheIdentifier used to invalidate cache if identifier is changed
@ -264,12 +235,11 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
const validatedInitialCacheOptions = validateOptions(globalCacheOptions);
return /** @param {CacheRequest} cacheRequest */ async cacheRequest => {
const { method } = cacheRequest;
const cacheOptions = validateOptions({
...validatedInitialCacheOptions,
...cacheRequest.cacheOptions,
});
const cacheOptions = composeCacheOptions(
validatedInitialCacheOptions,
cacheRequest.cacheOptions,
);
cacheRequest.cacheOptions = cacheOptions;
// don't use cache if 'useCache' === false
@ -278,12 +248,12 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
}
const cacheId = cacheOptions.requestIdentificationFn(cacheRequest, searchParamSerializer);
// cacheIdentifier is used to bind the cache to the current session
const currentCache = getCache(getCacheIdentifier());
const { method } = cacheRequest;
// don't use cache if the request method is not part of the configs methods
if (cacheOptions.methods.indexOf(method.toLowerCase()) === -1) {
if (!cacheOptions.methods.includes(method.toLowerCase())) {
// If it's NOT one of the config.methods, invalidate caches
currentCache.delete(cacheId);
// also invalidate caches matching to cacheOptions
@ -310,11 +280,7 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive);
if (cacheResponse) {
// eslint-disable-next-line no-param-reassign
if (!cacheRequest.cacheOptions) {
cacheRequest.cacheOptions = { useCache: false };
}
cacheRequest.cacheOptions = cacheRequest.cacheOptions ?? { useCache: false };
const response = /** @type {CacheResponse} */ cacheResponse.clone();
response.request = cacheRequest;
response.fromCache = true;
@ -350,10 +316,11 @@ export const cacheResponseInterceptorFactory = (getCacheIdentifier, globalCacheO
throw new Error('Missing request in response.');
}
const cacheOptions = composeCacheOptions(
validatedInitialCacheOptions,
cacheResponse.request?.cacheOptions,
);
const cacheOptions = validateOptions({
...validatedInitialCacheOptions,
...cacheResponse.request?.cacheOptions,
});
// string that identifies cache entry
const cacheId = cacheOptions.requestIdentificationFn(
cacheResponse.request,
@ -363,13 +330,11 @@ export const cacheResponseInterceptorFactory = (getCacheIdentifier, globalCacheO
const isAlreadyFromCache = !!cacheResponse.fromCache;
// caching all responses with not default `timeToLive`
const isCacheActive = cacheOptions.timeToLive > 0;
const isMethodSupported = cacheOptions.methods.includes(
cacheResponse.request.method.toLowerCase(),
);
// if the request is one of the options.methods; store response in cache
if (
!isAlreadyFromCache &&
isCacheActive &&
cacheOptions.methods.indexOf(cacheResponse.request.method.toLowerCase()) > -1
) {
if (!isAlreadyFromCache && isCacheActive && isMethodSupported) {
// store the response data in the cache and mark request as resolved
currentCache.set(cacheId, cacheResponse.clone());
}

7
packages/ajax/src/interceptors.js
View file

@ -17,10 +17,9 @@ export function getCookie(name, _document = document) {
*/
export async function acceptLanguageRequestInterceptor(request) {
if (!request.headers.has('accept-language')) {
let locale = document.documentElement.lang || 'en';
if (document.documentElement.getAttribute('data-localize-lang')) {
locale = document.documentElement.getAttribute('data-localize-lang') || 'en';
}
const documentLocale = document.documentElement.lang;
const localizeLang = document.documentElement.getAttribute('data-localize-lang');
const locale = localizeLang || documentLocale || 'en';
request.headers.set('accept-language', locale);
}
return request;

68
packages/ajax/test/AjaxClient.test.js
View file

@ -18,6 +18,54 @@ describe('AjaxClient', () => {
fetchStub.restore();
});
describe('options', () => {
it('creates options object by expanding cacheOptions', async () => {
// Given
const getCacheIdentifier = () => '_DEFAULT_CACHE_ID';
const config = {
jsonPrefix: ")]}',",
cacheOptions: {
useCache: true,
timeToLive: 1000 * 60 * 5, // 5 minutes
getCacheIdentifier,
},
};
const expected = {
addAcceptLanguage: true,
xsrfCookieName: 'XSRF-TOKEN',
xsrfHeaderName: 'X-XSRF-TOKEN',
jsonPrefix: ")]}',",
cacheOptions: {
useCache: true,
timeToLive: 300000,
getCacheIdentifier,
},
};
// When
const ajax1 = new AjaxClient(config);
const result = ajax1.options;
// Then
expect(result).to.deep.equal(expected);
});
it('has default getCacheIdentifier function when cacheOptions does not provide one', async () => {
// Given
const config = {
cacheOptions: {
useCache: true,
timeToLive: 1000 * 60 * 5, // 5 minutes
},
};
// When
// @ts-expect-error
const ajax1 = new AjaxClient(config);
const result = ajax1.options?.cacheOptions?.getCacheIdentifier;
// Then
expect(result).not.to.be.undefined;
expect(result).to.be.a('function');
});
});
describe('request()', () => {
it('calls fetch with the given args, returning the result', async () => {
const response = await (await ajax.request('/foo', { method: 'POST' })).text();
@ -130,15 +178,22 @@ describe('AjaxClient', () => {
});
it('removeRequestInterceptor() removes a request interceptor', async () => {
const interceptor = /** @param {Request} r */ async r =>
const interceptor1 = /** @param {Request} r */ async r =>
new Request(`${r.url}/intercepted-1`);
ajax.addRequestInterceptor(interceptor);
ajax.removeRequestInterceptor(interceptor);
const interceptor2 = /** @param {Request} r */ async r =>
new Request(`${r.url}/intercepted-2`);
const interceptor3 = /** @param {Request} r */ async r =>
new Request(`${r.url}/intercepted-3`);
ajax.addRequestInterceptor(interceptor1);
ajax.addRequestInterceptor(interceptor2);
ajax.addRequestInterceptor(interceptor3);
ajax.removeRequestInterceptor(interceptor1);
await ajax.request('/foo', { method: 'POST' });
const request = fetchStub.getCall(0).args[0];
expect(request.url).to.equal(`${window.location.origin}/foo`);
expect(request.url).to.equal(`${window.location.origin}/foo/intercepted-2/intercepted-3`);
});
it('removeResponseInterceptor() removes a request interceptor', async () => {
@ -148,8 +203,9 @@ describe('AjaxClient', () => {
// @ts-expect-error we're mocking the response as a simple promise which returns a string
ajax.removeResponseInterceptor(interceptor);
const response = await (await ajax.request('/foo', { method: 'POST' })).text();
expect(response).to.equal('mock response');
const response = await ajax.request('/foo', { method: 'POST' });
const text = await response.text();
expect(text).to.equal('mock response');
});
});

2
packages/ajax/test/interceptors-cache.test.js
View file

@ -135,7 +135,7 @@ describe('ajax cache', () => {
requestIdentificationFn: 'not a function',
});
removeCacheInterceptors(ajax, indexes);
}).to.throw(/Property `requestIdentificationFn` must be of type `function` or `falsy`/);
}).to.throw(/Property `requestIdentificationFn` must be of type `function`/);
});
});