refactor: concurreny fixes and refactored deferred

Also changed format settings,
fixed AxiosResponse types
and coded more tests

.prettierrc.js

@@ -3,6 +3,6 @@
 module.exports = require('@arthurfiorette/prettier-config')({
   tsdoc: true,
   jsdocSpaces: 1,
-  jsdocPrintWidth: 100,
+  jsdocPrintWidth: 70,
   jsdocSingleLineComment: false
 });

jest.config.js

@@ -1,4 +1,6 @@
-/** @type {import('ts-jest/dist/types').InitialOptionsTsJest} */
+/**
+ * @type {import('ts-jest/dist/types').InitialOptionsTsJest}
+ */
 module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node'

package.json

@@ -8,7 +8,7 @@
     "test": "jest",
     "test:watch": "jest --watch",
     "test:coverage": "jest --coverage",
-    "prettify": "prettier . --write --plugin prettier-plugin-jsdoc --plugin prettier-plugin-organize-imports",
+    "prettify": "prettier --write .",
     "lint": "tsc --noEmit && eslint . --ext .ts",
     "lint:fix": "eslint . --ext .ts --fix",
     "version": "auto-changelog -p && git add CHANGELOG.md"

src/axios/cache.ts

@@ -9,9 +9,9 @@ import CacheInstance, { AxiosCacheInstance, CacheProperties } from './types';
 /**
  * Apply the caching interceptors for a already created axios instance.
  *
- * @param axios the already created axios instance
- * @param config the config for the caching interceptors
- * @returns the same instance but with caching enabled
+ * @param axios The already created axios instance
+ * @param config The config for the caching interceptors
+ * @returns The same instance but with caching enabled
  */
 export function applyCache(
   axios: AxiosInstance,
@@ -59,9 +59,9 @@ export function applyCache(
 /**
  * Returns a new axios instance with caching enabled.
  *
- * @param config the config for the caching interceptors
- * @param axiosConfig the config for the created axios instance
- * @returns the same instance but with caching enabled
+ * @param config The config for the caching interceptors
+ * @param axiosConfig The config for the created axios instance
+ * @returns The same instance but with caching enabled
  */
 export function createCache(
   config: Partial<CacheInstance> & Partial<CacheProperties> = {},

src/axios/types.ts

@@ -37,8 +37,8 @@ export type CacheProperties = {
   ttl: number;
 
   /**
-   * If this interceptor should configure the cache from the request cache header
-   * When used, the ttl property is ignored
+   * If this interceptor should configure the cache from the request
+   * cache header When used, the ttl property is ignored
    *
    * @default false
    */
@@ -54,27 +54,35 @@ export type CacheProperties = {
   /**
    * The function to check if the response code permit being cached.
    *
-   * @default { statusCheck: [200, 399] }
+   * @default {statusCheck: [200, 399]}
    */
   cachePredicate: CachePredicate;
 
   /**
-   * Once the request is resolved, this specifies what requests should we change the cache.
-   * Can be used to update the request or delete other caches.
+   * Once the request is resolved, this specifies what requests should
+   * we change the cache. Can be used to update the request or delete
+   * other caches.
    *
    * If the function returns nothing, the entry is deleted
    *
    * This is independent if the request made was cached or not.
    *
-   * The id used is the same as the id on `CacheRequestConfig['id']`, auto-generated or not.
+   * The id used is the same as the id on `CacheRequestConfig['id']`,
+   * auto-generated or not.
    *
-   * @default {}
+   * @default
    */
   update: Record<string, CacheUpdater>;
 };
 
-export type CacheAxiosResponse = AxiosResponse & {
+export type CacheAxiosResponse<T = any> = AxiosResponse<T> & {
   config: CacheRequestConfig;
+
+  /**
+   * The id used for this request. if config specified an id, the id
+   * will be returned
+   */
+  id: string | symbol;
 };
 
 /**
@@ -82,9 +90,9 @@ export type CacheAxiosResponse = AxiosResponse & {
  */
 export type CacheRequestConfig = AxiosRequestConfig & {
   /**
-   * An id for this request, if this request is used in cache, only the last request made with this id will be returned.
+   * An id for this request, if this request is used in cache, only
+   * the last request made with this id will be returned.
    *
-   * @see cacheKey
    * @default undefined
    */
   id?: string | symbol;
@@ -107,19 +115,21 @@ export default interface CacheInstance {
 
   /**
    * The function used to create different keys for each request.
-   * Defaults to a function that priorizes the id, and if not specified,
-   * a string is generated using the method, baseUrl, params, and url
+   * Defaults to a function that priorizes the id, and if not
+   * specified, a string is generated using the method, baseUrl,
+   * params, and url
    */
   generateKey: KeyGenerator;
 
   /**
-   * A simple object that holds all deferred objects until it is resolved.
+   * A simple object that holds all deferred objects until it is
+   * resolved or rejected.
    */
-  waiting: Record<string, Deferred<CachedResponse>>;
+  waiting: Record<string, Deferred<CachedResponse, void>>;
 
   /**
-   * The function to parse and interpret response headers.
-   * Only used if cache.interpretHeader is true.
+   * The function to parse and interpret response headers. Only used
+   * if cache.interpretHeader is true.
    */
   headerInterpreter: HeaderInterpreter;
 
@@ -135,7 +145,8 @@ export default interface CacheInstance {
 }
 
 /**
- * Same as the AxiosInstance but with CacheRequestConfig as a config type.
+ * Same as the AxiosInstance but with CacheRequestConfig as a config
+ * type and CacheAxiosResponse as response type.
  *
  * @see AxiosInstance
  * @see CacheRequestConfig
@@ -154,23 +165,23 @@ export interface AxiosCacheInstance extends AxiosInstance, CacheInstance {
 
   getUri(config?: CacheRequestConfig): string;
 
-  request<T = any, R = AxiosResponse<T>>(config: CacheRequestConfig): Promise<R>;
+  request<T = any, R = CacheAxiosResponse<T>>(config: CacheRequestConfig): Promise<R>;
 
-  get<T = any, R = AxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
-  delete<T = any, R = AxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
-  head<T = any, R = AxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
-  options<T = any, R = AxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
-  post<T = any, R = AxiosResponse<T>>(
+  get<T = any, R = CacheAxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
+  delete<T = any, R = CacheAxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
+  head<T = any, R = CacheAxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
+  options<T = any, R = CacheAxiosResponse<T>>(url: string, config?: CacheRequestConfig): Promise<R>;
+  post<T = any, R = CacheAxiosResponse<T>>(
     url: string,
     data?: any,
     config?: CacheRequestConfig
   ): Promise<R>;
-  put<T = any, R = AxiosResponse<T>>(
+  put<T = any, R = CacheAxiosResponse<T>>(
     url: string,
     data?: any,
     config?: CacheRequestConfig
   ): Promise<R>;
-  patch<T = any, R = AxiosResponse<T>>(
+  patch<T = any, R = CacheAxiosResponse<T>>(
     url: string,
     data?: any,
     config?: CacheRequestConfig

src/header/types.ts

@@ -1,11 +1,10 @@
 /**
  * Interpret the cache control header, if present.
  *
- * @param header the header object to interpret.
- *
- * @returns `false` if cache should not be used. `undefined` when provided
- * headers was not enough to determine a valid value. Or a `number` containing
- * the number of **milliseconds** to cache the response.
+ * @param header The header object to interpret.
+ * @returns `false` if cache should not be used. `undefined` when
+ *   provided headers was not enough to determine a valid value. Or a
+ *   `number` containing the number of **milliseconds** to cache the response.
  */
 export type HeaderInterpreter = (
   headers?: Record<Lowercase<string>, string>

src/interceptors/request.ts

@@ -1,6 +1,6 @@
 import { AxiosCacheInstance, CacheRequestConfig } from '../axios/types';
 import { CachedResponse, CachedStorageValue, LoadingStorageValue } from '../storage/types';
-import { Deferred } from '../util/deferred';
+import { deferred } from '../util/deferred';
 import { CACHED_STATUS_CODE, CACHED_STATUS_TEXT } from '../util/status-codes';
 import { AxiosInterceptor } from './types';
 
@@ -12,7 +12,7 @@ export class CacheRequestInterceptor implements AxiosInterceptor<CacheRequestCon
   };
 
   onFulfilled = async (config: CacheRequestConfig): Promise<CacheRequestConfig> => {
-    // Ignore caching
+    // Skip cache
     if (config.cache === false) {
       return config;
     }
@@ -31,17 +31,24 @@ export class CacheRequestInterceptor implements AxiosInterceptor<CacheRequestCon
 
     // Not cached, continue the request, and mark it as fetching
     emptyState: if (cache.state == 'empty') {
-      // This if catches concurrent access to a new key.
-      // The js event loop skips in the first await statement,
-      // so the next code block will be executed both if called
-      // from two places asynchronously.
+      /**
+       * This checks for simultaneous access to a new key. The js
+       * event loop jumps on the first await statement, so the second
+       * (asynchronous call) request may have already started executing.
+       */
       if (this.axios.waiting[key]) {
         cache = (await this.axios.storage.get(key)) as CachedStorageValue | LoadingStorageValue;
         break emptyState;
       }
 
       // Create a deferred to resolve other requests for the same key when it's completed
-      this.axios.waiting[key] = new Deferred();
+      this.axios.waiting[key] = deferred();
+
+      /**
+       * Add a default reject handler to detect when the request is
+       * aborted without others waiting
+       */
+      this.axios.waiting[key]?.catch(() => {});
 
       await this.axios.storage.set(key, {
         state: 'loading',
@@ -56,14 +63,21 @@ export class CacheRequestInterceptor implements AxiosInterceptor<CacheRequestCon
     if (cache.state === 'loading') {
       const deferred = this.axios.waiting[key];
 
-      // If the deferred is undefined, means that the
-      // outside has removed that key from the waiting list
+      /**
+       * If the deferred is undefined, means that the outside has
+       * removed that key from the waiting list
+       */
       if (!deferred) {
         await this.axios.storage.remove(key);
         return config;
       }
 
-      data = await deferred;
+      try {
+        data = await deferred;
+      } catch (e) {
+        // The deferred is rejected when the request that we are waiting rejected cache.
+        return config;
+      }
     } else {
       data = cache.data;
     }

src/interceptors/response.ts

@@ -19,7 +19,7 @@ export class CacheResponseInterceptor implements AxiosInterceptor<CacheAxiosResp
     this.axios.interceptors.response.use(this.onFulfilled);
   };
 
-  testCachePredicate = (response: AxiosResponse, { cache }: CacheConfig): boolean => {
+  private testCachePredicate = (response: AxiosResponse, { cache }: CacheConfig): boolean => {
     const cachePredicate = cache?.cachePredicate || this.axios.defaults.cache.cachePredicate;
 
     return (
@@ -28,13 +28,27 @@ export class CacheResponseInterceptor implements AxiosInterceptor<CacheAxiosResp
     );
   };
 
+  /**
+   * Rejects cache for this response. Also update the waiting list for
+   * this key by rejecting it.
+   */
+  private rejectResponse = async (key: string) => {
+    // Update the cache to empty to prevent infinite loading state
+    await this.axios.storage.remove(key);
+    // Reject the deferred if present
+    this.axios.waiting[key]?.reject();
+    delete this.axios.waiting[key];
+  };
+
   onFulfilled = async (response: CacheAxiosResponse): Promise<CacheAxiosResponse> => {
-    // Ignore caching
+    const key = this.axios.generateKey(response.config);
+    response.id = key;
+
+    // Skip cache
     if (response.config.cache === false) {
       return response;
     }
 
-    const key = this.axios.generateKey(response.config);
     const cache = await this.axios.storage.get(key);
 
     // Response shouldn't be cached or was already cached
@@ -44,8 +58,7 @@ export class CacheResponseInterceptor implements AxiosInterceptor<CacheAxiosResp
 
     // Config told that this response should be cached.
     if (!this.testCachePredicate(response, response.config as CacheConfig)) {
-      // Update the cache to empty to prevent infinite loading state
-      await this.axios.storage.remove(key);
+      await this.rejectResponse(key);
       return response;
     }
 
@@ -56,8 +69,7 @@ export class CacheResponseInterceptor implements AxiosInterceptor<CacheAxiosResp
 
       // Cache should not be used
       if (expirationTime === false) {
-        // Update the cache to empty to prevent infinite loading state
-        await this.axios.storage.remove(key);
+        await this.rejectResponse(key);
         return response;
       }
 

src/storage/types.ts

@@ -1,7 +1,7 @@
 export interface CacheStorage {
   /**
-   * Returns the cached value for the given key.
-   * Must handle cache miss and staling by returning a new `StorageValue` with `empty` state.
+   * Returns the cached value for the given key. Must handle cache
+   * miss and staling by returning a new `StorageValue` with `empty` state.
    */
   get: (key: string) => Promise<StorageValue>;
 

src/util/cache-predicate.ts

@@ -4,16 +4,15 @@ export type CachePredicate = CachePredicateObject | ((response: AxiosResponse) =
 
 export type CachePredicateObject = {
   /**
-   * The status predicate, if a tuple is returned,
-   * the first and seconds value means the interval (inclusive) accepted.
-   * Can also be a function.
+   * The status predicate, if a tuple is returned, the first and
+   * seconds value means the interval (inclusive) accepted. Can also
+   * be a function.
    */
   statusCheck?: [start: number, end: number] | ((status: number) => boolean);
 
   /**
-   * Matches if the response header container all keys.
-   * A tuple also checks for values.
-   * Can also be a predicate.
+   * Matches if the response header container all keys. A tuple also
+   * checks for values. Can also be a predicate.
    */
   containsHeaders?: Record<string, true | string | ((header: string) => boolean)>;
 

src/util/deferred.ts

@@ -1,67 +1,26 @@
 export type MaybePromise<T> = T | PromiseLike<T>;
 
-/**
- * Represents the completion of an asynchronous operation that can be completed later.
- */
-export class Deferred<T = any> implements PromiseLike<T> {
-  private readonly promise: Promise<T>;
-  private _resolve: (value: MaybePromise<T>) => void = () => {};
-  private _reject: (reason?: any) => void = () => {};
-
-  constructor() {
-    this.promise = new Promise((resolve, reject) => {
-      this._resolve = resolve;
-      this._reject = reject;
-    });
-  }
-
-  /**
-   * Resolve this deferred promise with the given value.
-   * @param the value to resolve
-   */
-  public readonly resolve = (value: MaybePromise<T>): void => {
-    this._resolve(value);
-  };
-
-  /**
-   * Reject this deferred promise with the given reason.
-   * @param reason the reason to reject this deferred promise
-   */
-  public readonly reject = (reason?: any): void => {
-    this._reject(reason);
-  };
-
-  /**
-   * Attaches callbacks for the resolution and/or rejection of the Promise.
-   * @param onfulfilled The callback to execute when the Promise is resolved.
-   * @param onrejected The callback to execute when the Promise is rejected.
-   * @returns A Promise for the completion of which ever callback is executed.
-   */
-  public readonly then = <TResult1 = T, TResult2 = never>(
-    onfulfilled?: ((value: T) => MaybePromise<TResult1>) | undefined | null,
-    onrejected?: ((reason: any) => MaybePromise<TResult2>) | undefined | null
-  ): Promise<TResult1 | TResult2> => {
-    return this.promise.then(onfulfilled, onrejected);
-  };
-
-  /**
-   * Attaches a callback for only the rejection of the Promise.
-   * @param onrejected The callback to execute when the Promise is rejected.
-   * @returns A Promise for the completion of the callback.
-   */
-  public readonly catch = <TResult = never>(
-    onrejected?: ((reason: any) => MaybePromise<TResult>) | undefined | null
-  ): Promise<T | TResult> => {
-    return this.promise.catch(onrejected);
-  };
-
-  /**
-   * Attaches a callback that is invoked when the Promise is settled (fulfilled or rejected). The
-   * resolved value cannot be modified from the callback.
-   * @param onfinally The callback to execute when the Promise is settled (fulfilled or rejected).
-   * @returns A Promise for the completion of the callback.
-   */
-  public readonly finally = (onfinally?: (() => void) | undefined | null): Promise<T> => {
-    return this.promise.finally(onfinally);
-  };
+export interface Deferred<T, E> extends Promise<T> {
+  resolve(value: MaybePromise<T>): void;
+  reject(reason: E): void;
+}
+
+/**
+ * Returns a promise that can be resolved or reject later
+ *
+ * @returns The deferred promise
+ */
+export function deferred<T, E>(): Deferred<T, E> {
+  let reject: Deferred<T, E>['reject'] = () => {};
+  let resolve: Deferred<T, E>['resolve'] = () => {};
+
+  const promise = new Promise((res, rej) => {
+    resolve = res;
+    reject = rej;
+  }) as Deferred<T, E>;
+
+  promise.resolve = resolve;
+  promise.reject = reject;
+
+  return promise;
 }

test/interceptors/request.test.ts

@@ -33,9 +33,44 @@ describe('test request interceptor', () => {
 
     const [resp1, resp2] = await Promise.all([axios.get(''), axios.get('')]);
 
-    expect(resp1).toHaveProperty('status', axiosMock.statusCode);
-    expect(resp1).toHaveProperty('statusText', axiosMock.statusText);
-    expect(resp2).toHaveProperty('status', StatusCodes.CACHED_STATUS_CODE);
-    expect(resp2).toHaveProperty('statusText', StatusCodes.CACHED_STATUS_TEXT);
+    expect(resp1.status).toBe(axiosMock.statusCode);
+    expect(resp1.statusText).toBe(axiosMock.statusText);
+    expect(resp2.status).toBe(StatusCodes.CACHED_STATUS_CODE);
+    expect(resp2.statusText).toBe(StatusCodes.CACHED_STATUS_TEXT);
+  });
+
+  it('tests concurrent requests with cache: false', async () => {
+    const axios = mockAxios();
+
+    const results = await Promise.all([
+      axios.get('', { cache: false }),
+      axios.get(''),
+      axios.get('', { cache: false })
+    ]);
+    for (const result of results) {
+      expect(result.status).toBe(axiosMock.statusCode);
+      expect(result.statusText).toBe(axiosMock.statusText);
+    }
+  });
+
+  /**
+   * This is to test when two requests are made simultaneously. With
+   * that, the second response waits the deferred from the first one.
+   * Because the first request is not cached, the second should not be
+   * waiting forever for the deferred to be resolved with a cached response.
+   */
+  it('tests concurrent requests with uncached responses', async () => {
+    const axios = mockAxios();
+
+    const [, resp2] = await Promise.all([
+      axios.get('', {
+        // Simple predicate to ignore cache in the response step.
+        cache: { cachePredicate: () => false }
+      }),
+      axios.get('')
+    ]);
+
+    expect(resp2.status).toBe(axiosMock.statusCode);
+    expect(resp2.statusText).toBe(axiosMock.statusText);
   });
 });

test/interceptors/response.test.ts

@@ -1,7 +1,23 @@
-// import { mockAxios } from '../mocks/axios';
+import { axiosMock, mockAxios } from '../mocks/axios';
 
 describe('test request interceptor', () => {
-  it('tests', () => {
-    //const axios = mockAxios();
+  it('tests cache predicate integration', async () => {
+    const axios = mockAxios();
+
+    const fetch = () =>
+      axios.get('', {
+        cache: {
+          cachePredicate: {
+            responseMatch: () => false
+          }
+        }
+      });
+
+    // Make first request to cache it
+    await fetch();
+    const result = await fetch();
+
+    expect(result.status).toBe(axiosMock.statusCode);
+    expect(result.statusText).toBe(axiosMock.statusText);
   });
 });

test/util/deferred.test.ts

@@ -1,52 +1,67 @@
-import { Deferred } from '../../src/util/deferred';
+import { deferred } from '../../src/util/deferred';
 
 describe('Tests cached status code', () => {
   it('test resolve method', () => {
-    const deferred = new Deferred();
+    const d = deferred();
 
-    expect(deferred).resolves.toBe(1);
-    deferred.resolve(1);
+    expect(d).resolves.toBe(1);
+    d.resolve(1);
   });
 
   it('test reject method', () => {
-    const deferred = new Deferred();
+    const d = deferred();
 
-    expect(deferred).rejects.toBe(1);
-    deferred.reject(1);
+    expect(d).rejects.toBe(1);
+    d.reject(1);
   });
 
   it('test then method', () => {
-    const deferred = new Deferred();
+    const d = deferred();
 
-    deferred.then((data) => {
+    d.then((data) => {
       expect(data).toBe(1);
     });
 
-    deferred.resolve(1);
+    d.resolve(1);
   });
 
   it('test catch method', () => {
-    const deferred = new Deferred();
+    const d = deferred();
 
-    deferred.catch((data) => {
+    d.catch((data) => {
       expect(data).toBe(1);
     });
 
-    deferred.resolve(1);
+    d.resolve(1);
   });
 
   it('test finally method', () => {
-    const deferred = new Deferred<number>();
+    const d = deferred<number, any>();
 
     let data: number;
-    deferred.then((d) => {
+    d.then((d) => {
       data = d;
     });
 
-    deferred.finally(() => {
+    d.finally(() => {
       expect(data).toBe(1);
     });
 
-    deferred.resolve(1);
+    d.resolve(1);
+  });
+
+  it('test with try catch', async () => {
+    const d = deferred<number, any>();
+
+    process.nextTick(d.resolve, 1);
+
+    let data: number;
+    try {
+      data = await d;
+    } catch (err) {
+      data = 2;
+    }
+
+    expect(data).toBe(1);
   });
 });