fix: defaultKeyGenerator keeps response unchanged (#161)

docs/pages/request-id.md

@@ -42,6 +42,9 @@ id generation can clarify this idea.
 Everything that is used to treat two requests as same or not, is done by the `generateKey`
 property.
 
+The value is hashed into a signed integer when the returned value from the provided
+generator is not a `string` or a `number`.
+
 By default, it uses the `method`, `baseURL`, `params`, `data` and `url` properties from
 the request object into an hash code generated by the
 [`object-code`](https://www.npmjs.com/package/object-code) library.

src/util/key-generator.ts

@@ -7,63 +7,57 @@ import type { KeyGenerator } from './types';
 const SLASHES_REGEX = /^\/|\/$/g;
 
 /**
- * Builds an generator that received the {@link CacheRequestConfig} and should return a
- * string id for it.
- */
-export function buildKeyGenerator<R = unknown, D = unknown>(
-  shouldHash: false,
-  generator: KeyGenerator
-): KeyGenerator<R, D>;
-
-/**
- * Builds an generator that received the {@link CacheRequestConfig} and has it's return
- * value hashed by {@link code}.
+ * Builds an generator that receives a {@link CacheRequestConfig} and returns a value
+ * hashed by {@link hash}.
  *
- * ### You can return an object that is hashed into an unique number, example:
+ * The value is hashed into a signed integer when the returned value from the provided
+ * generator is not a `string` or a `number`.
+ *
+ * You can return any type of data structure.
+ *
+ * @example
  *
  * ```js
  * // This generator will return a hash code.
  * // The code will only be the same if url, method and data are the same.
- * const generator = buildKeyGenerator(true, ({ url, method, data }) => ({
+ * const generator = buildKeyGenerator(({ url, method, data }) => ({
  *   url,
  *   method,
  *   data
  * }));
  * ```
  */
 export function buildKeyGenerator<R = unknown, D = unknown>(
-  shouldHash: true,
-  generator: (options: CacheRequestConfig<R, D>) => unknown
-): KeyGenerator<R, D>;
-
-export function buildKeyGenerator<R = unknown, D = unknown>(
-  shouldHash: boolean,
-  generator: (options: CacheRequestConfig<R, D>) => unknown
+  generator: (request: CacheRequestConfig<R, D>) => unknown
 ): KeyGenerator<R, D> {
   return (request) => {
     if (request.id) {
       return request.id;
     }
 
-    // Remove trailing slashes
-    request.baseURL && (request.baseURL = request.baseURL.replace(SLASHES_REGEX, ''));
-    request.url && (request.url = request.url.replace(SLASHES_REGEX, ''));
+    const key = generator(request);
 
-    // lowercase method
-    request.method && (request.method = request.method.toLowerCase() as Method);
+    if (typeof key === 'string' || typeof key === 'number') {
+      return `${key}`;
+    }
 
-    const result = generator(request) as string;
-    return shouldHash ? `${hash(result)}` : result;
+    return `${hash(key)}`;
   };
 }
 
 export const defaultKeyGenerator = buildKeyGenerator(
-  true,
   ({ baseURL = '', url = '', method = 'get', params, data }) => {
+    // Remove trailing slashes to avoid generating different keys for the "same" final url.
+    baseURL && (baseURL = baseURL.replace(SLASHES_REGEX, ''));
+    url && (url = url.replace(SLASHES_REGEX, ''));
+
+    // lowercase method
+    method && (method = method.toLowerCase() as Method);
+
     return {
       url: baseURL + (baseURL && url ? '/' : '') + url,
-      method,
       params: params as unknown,
+      method,
       data
     };
   }

test/util/key-generator.test.ts

@@ -1,3 +1,4 @@
+import type { CacheRequestConfig } from '../../src';
 import { buildKeyGenerator, defaultKeyGenerator } from '../../src/util/key-generator';
 import { mockAxios } from '../mocks/axios';
 
@@ -137,7 +138,7 @@ describe('tests key generation', () => {
   });
 
   it('tests buildKeyGenerator & hash: false', async () => {
-    const keyGenerator = buildKeyGenerator(false, ({ headers }) =>
+    const keyGenerator = buildKeyGenerator(({ headers }) =>
       String(headers?.['x-req-header'] || 'not-set')
     );
 
@@ -166,36 +167,56 @@ describe('tests key generation', () => {
     expect(id3).toBe('not-set');
   });
 
-  it('tests buildKeyGenerator & hash: true', async () => {
-    const keyGenerator = buildKeyGenerator(true, ({ headers }) => {
-      return headers?.['x-req-header'] || 'not-set';
-    });
+  it('expects that the response remains unchanged', () => {
+    const originalResponse: CacheRequestConfig = {
+      baseURL: 'http://example.com/',
+      url: '/test/path/',
+      method: 'get',
+      params: {
+        a: 1
+      },
+      data: {
+        object: true
+      }
+    };
 
-    const axios = mockAxios({ generateKey: keyGenerator });
+    const response = Object.assign({}, originalResponse);
 
-    const { id } = await axios.get('random-url', {
-      data: Math.random(),
-      headers: {
-        'x-req-header': 'my-custom-id'
-      }
-    });
+    const key = defaultKeyGenerator(response);
+    expect(key).toBeDefined();
 
-    const { id: id2 } = await axios.get('other-url', {
-      data: Math.random() * 2,
-      headers: {
-        'x-req-header': 'my-custom-id'
-      }
-    });
+    expect(response).toEqual(originalResponse);
 
-    const { id: id3 } = await axios.get('other-url', {
-      data: Math.random() * 2
-    });
+    const key2 = defaultKeyGenerator(response);
+    expect(key2).toBeDefined();
 
-    expect(id).toBe(id2);
-    expect(id).not.toBe('my-custom-id'); // hashed value
+    expect(key).toBe(key2);
+
+    expect(response).toEqual(originalResponse);
+  });
+
+  it('tests when hash() is used in the response', () => {
+    const keyGenerator = buildKeyGenerator(({ data }) => data);
+
+    expect(keyGenerator({ data: 'test' })).toBe('test');
+    expect(keyGenerator({ data: 123123 })).toBe('123123');
+
+    let data: unknown = { a: 1 };
+
+    expect(keyGenerator({ data })).not.toBe(data);
+    expect(typeof keyGenerator({ data })).toBe('string');
+
+    data = true;
+
+    expect(keyGenerator({ data })).not.toBe(data);
+    expect(typeof keyGenerator({ data })).toBe('string');
+
+    data = {
+      fn: () => expect(false).toBeTruthy(),
+      test: new (class Asd {})()
+    };
 
-    expect(id3).not.toBe(id);
-    expect(id3).not.toBe(id2);
-    expect(id3).not.toBe('not-set');
+    expect(keyGenerator({ data })).not.toBe(data);
+    expect(typeof keyGenerator({ data })).toBe('string');
   });
 });