Repository: RasCarlito/axios-cache-adapter (js)

Issue:
2.6.0 Adding ability to cache requests made with methods beyond GET is a breaking change
Hello 👋  

Today we had the below unit test fail, due to the change introduced in `2.6.0` that now _automatically_ caches requests made with HTTP verbs outside of GET (i.e. POST, PUT etc.). I think the behaviour should be for consumers to opt-in to this enhancement through a config option, and the default behaviour should remain. Otherwise it is a breaking change and should result in a major version bump.

```
it('should not cache the request', async () => {
      nock(baseUrl, { reqheaders }).post(path, body).twice().reply(200, validResponse);
      const firstResponse = await service.post(`${baseUrl}${path}`, body, reqheaders, operation);
      const secondResponse = await service.post(`${baseUrl}${path}`, body, reqheaders, operation);
      expect(firstResponse.data).to.deep.equal(validResponse);
      expect(secondResponse.data).to.deep.equal(validResponse);
      expect(firstResponse.request.fromCache).to.be.undefined;
      expect(secondResponse.request.fromCache).to.be.undefined; // Test fails on this line with true != undefined
    });
```

Pull request description:
fix(exclude): only cache get requests by default, add config.exclude.methods option
Fixes #230 

- [x] Fix code
- [x] Fix tests
- [x] Fix documentation

Patch:
diff --git a/README.md b/README.md
index aefe2ec..d65508a 100644
--- a/README.md
+++ b/README.md
@@ -27,6 +27,8 @@ Or from a CDN like unpkg.com
 
 ## Usage
 
+**Important note:** Only `GET` request results are cached by default. Executing a request using any method listed in `exclude.methods` will invalidate the cache for the given URL.
+
 ### Instantiate adapter on its own
 
 You can instantiate the `axios-cache-adapter` on its own using the `setupCache()` method and then attach the adapter manually to an instance of `axios`.
@@ -126,6 +128,31 @@ api.get('/get?with=query', {
 
 _Note: Not all instance options can be overridden per request, see the API documentation at the end of this readme_
 
+### Cache POST request results
+
+You can allow `axios-cache-adapter` to cache the results of a request using (almost) any HTTP method by modifying the `exclude.methods` list.
+
+```js
+import { setup } from 'axios-cache-adapter
+
+const api = setup({
+  baseURL: 'https://httpbin.org',
+
+  cache: {
+    exclude: {
+      // Only exclude PUT, PATCH and DELETE methods from cache
+      methods: ['put', 'patch', 'delete']
+    }
+  }
+})
+
+api.post('/post').then((response) => {
+  // POST request has been cached \o/
+})
+```
+
+**Note:** the request method is not used in the cache store key by default, therefore with the above setup, making a `GET` or `POST` request will respond with the same cache.
+
 ### Use localforage as cache store
 
 You can give a `localforage` instance to `axios-cache-adapter` which will be used to store cache data instead of the default [in memory](https://github.com/RasCarlito/axios-cache-adapter/blob/master/src/memory.js) store.
@@ -204,7 +231,7 @@ const response = await api.get('/url')
 ```
 
 
-#### Use Redis Default Store as Cache Store 
+#### Use Redis Default Store as Cache Store
 
 You can give a `RedisDefaultStore` instance to `axios-cache-adapter` which will be used to store cache data in Redis using the default commands instead of hash commands.
 
@@ -313,13 +340,13 @@ _Note: Passing a function to `readOnError` is a smarter thing to do as you get t
 
 ### Invalidate cache entries
 
-By default, a cache entry will be invalidated when the request method is not `GET`, `POST`, `PUT`, `PATCH` or `DELETE` if it is made to the same URL using the following method:
+Using the default `invalidation` method, a cache entry will be invalidated if a request is made using one of the methods listed in `exclude.methods`.
 
 ```js
-async defaultInvalidate (config, request) {
+async function defaultInvalidate (config, request) {
   const method = request.method.toLowerCase()
 
-  if (method !== 'get') {
+  if (config.exclude.methods.includes(method)) {
     await config.store.removeItem(config.uuid)
   }
 }
@@ -430,7 +457,14 @@ where they will be stored, etc.
     query: true,
     // {Function} Method which returns a `Boolean` to determine if request
     // should be excluded from cache.
-    filter: null
+    filter: null,
+    // {Array} HTTP methods which will be excluded from cache.
+    // Defaults to `['post', 'patch', 'put', 'delete']`
+    // Any methods listed will also trigger cache invalidation while using the default `config.invalidate` method.
+    //
+    // Note: the HEAD method is always excluded (hard coded).
+    // the OPTIONS method is ignored by this library as it is automatically handled by browsers/clients to resolve cross-site request permissions
+    methods: ['post', 'patch', 'put', 'delete']
   },
   // {Boolean} Clear cached item when it is stale.
   clearOnStale: true,
diff --git a/axios-cache-adapter.d.ts b/axios-cache-adapter.d.ts
index 4fe1336..3161fc0 100644
--- a/axios-cache-adapter.d.ts
+++ b/axios-cache-adapter.d.ts
@@ -67,6 +67,14 @@ export interface IAxiosCacheAdapterOptions
 		 * should be excluded from cache.
 		 */
 		filter?: Function;
+    /**
+     * {Array} HTTP methods which will be excluded from cache.
+     * Defaults to `['post', 'patch', 'put', 'delete']`
+     * Any methods listed will also trigger cache invalidation while using the default `config.invalidate` method.
+     *
+     * Note: the HEAD method is always excluded (hard coded).
+     */
+    methods: Array;
 	};
 	/**
 	 * {Boolean} Clear cached item when it is stale.
@@ -95,7 +103,7 @@ export interface IAxiosCacheAdapterOptions
 	 * {Function|Boolean} Print out debug log to console.
 	 */
 	debug?: Function | boolean;
-  
+
   excludeFromCache?: boolean;
 }
 
diff --git a/src/cache.js b/src/cache.js
index 72ae49f..c6ff429 100644
--- a/src/cache.js
+++ b/src/cache.js
@@ -84,10 +84,10 @@ function key (config) {
   return cacheKey
 }
 
-async function defaultInvalidate (cfg, req) {
+async function defaultInvalidate (config, req) {
   const method = req.method.toLowerCase()
-  if (!['get', 'post', 'patch', 'put', 'delete'].includes(method)) {
-    await cfg.store.removeItem(cfg.uuid)
+  if (config.exclude.methods.includes(method)) {
+    await config.store.removeItem(config.uuid)
   }
 }
 
diff --git a/src/config.js b/src/config.js
index 44a9bc1..525f892 100644
--- a/src/config.js
+++ b/src/config.js
@@ -17,7 +17,8 @@ const defaults = {
     exclude: {
       paths: [],
       query: true,
-      filter: null
+      filter: null,
+      methods: ['post', 'patch', 'put', 'delete']
     },
     adapter: axios.defaults.adapter,
     clearOnStale: true,
@@ -46,7 +47,14 @@ const disallowedPerRequestKeys = ['limit', 'store', 'adapter', 'uuid', 'acceptSt
  * @return {Object}
  */
 const makeConfig = function (override = {}) {
-  const config = { ...defaults.cache, ...override }
+  const config = {
+    ...defaults.cache,
+    ...override,
+    exclude: {
+      ...defaults.cache.exclude,
+      ...override.exclude
+    }
+  }
 
   // Create a cache key method
   config.key = key(config)
diff --git a/src/exclude.js b/src/exclude.js
index d17c62f..886553b 100644
--- a/src/exclude.js
+++ b/src/exclude.js
@@ -2,6 +2,13 @@ import { isObject } from './utilities'
 
 function exclude (config = {}, req) {
   const { exclude = {}, debug } = config
+  const method = req.method.toLowerCase()
+
+  if (method === 'head' || exclude.methods.includes(method)) {
+    debug(`Excluding request by HTTP method ${req.url}`)
+
+    return true
+  }
 
   if ((typeof exclude.filter === 'function') && exclude.filter(req)) {
     debug(`Excluding request by filter ${req.url}`)
diff --git a/src/request.js b/src/request.js
index 69932fd..bb00b4e 100644
--- a/src/request.js
+++ b/src/request.js
@@ -14,12 +14,6 @@ async function request (config, req) {
     return excludeFromCache()
   }
 
-  const method = req.method.toLowerCase()
-
-  if (method === 'head' || !['get', 'post', 'patch', 'put', 'delete'].includes(method)) {
-    return excludeFromCache()
-  }
-
   try {
     const res = await read(config, req)
 


Test patch:
diff --git a/test/spec/exclude.spec.js b/test/spec/exclude.spec.js
index 7a27982..655a265 100644
--- a/test/spec/exclude.spec.js
+++ b/test/spec/exclude.spec.js
@@ -7,23 +7,32 @@ import exclude from 'src/exclude'
 describe('Cache exclusion', () => {
   const url = 'https://some-rest.api/users'
   const debug = () => {}
+  const methods = ['post', 'patch', 'put', 'delete']
+  const method = 'get'
   // const debug = (...args) => { console.log(...args) }
 
-  it('Should not exclude if not configured', () => {
-    const config = { debug }
+  it('Should exclude requests which are listed by config.exclude.methods parameter', () => {
+    const config = {
+      exclude: { methods },
+      debug
+    }
 
-    assert.strictEqual(exclude(config, { url }), false)
+    assert.ok(exclude(config, { url, method: 'post' }))
+    assert.ok(exclude(config, { url, method: 'patch' }))
+    assert.ok(exclude(config, { url, method: 'put' }))
+    assert.ok(exclude(config, { url, method: 'delete' }))
+    assert.ok(exclude(config, { url, method: 'head' }))
   })
 
   it('Should exclude requests with query parameters with config.exclude.query=true', () => {
     const config = {
-      exclude: { query: true },
+      exclude: { query: true, methods },
       debug
     }
 
-    const reqWithQuery = { url: `${url}?with=query&params=42` }
-    const reqWithParams = { url, params: { with: 'query', params: 42 } }
-    const reqWithSearchParams = { url, params: new URLSearchParams('with=query&params=42') }
+    const reqWithQuery = { url: `${url}?with=query&params=42`, method }
+    const reqWithParams = { url, method, params: { with: 'query', params: 42 } }
+    const reqWithSearchParams = { url, method, params: new URLSearchParams('with=query&params=42') }
 
     assert.ok(exclude(config, reqWithQuery))
     assert.ok(exclude(config, reqWithParams))
@@ -32,13 +41,13 @@ describe('Cache exclusion', () => {
 
   it('Should not exclude requests with query parameters with config.exclude.query=false', () => {
     const config = {
-      exclude: { query: false },
+      exclude: { query: false, methods },
       debug
     }
 
-    const reqWithQuery = { url: `${url}?with=query&params=42` }
-    const reqWithParams = { url, params: { with: 'query', params: 42 } }
-    const reqWithSearchParams = { url, params: new URLSearchParams('with=query&params=42') }
+    const reqWithQuery = { url: `${url}?with=query&params=42`, method }
+    const reqWithParams = { url, method, params: { with: 'query', params: 42 } }
+    const reqWithSearchParams = { url, method, params: new URLSearchParams('with=query&params=42') }
 
     assert.ok(!exclude(config, reqWithQuery))
     assert.ok(!exclude(config, reqWithParams))
@@ -47,23 +56,24 @@ describe('Cache exclusion', () => {
 
   it('Should exclude requests that match paths', () => {
     const config = {
-      exclude: { paths: [/\/users/] },
+      exclude: { paths: [/\/users/], methods },
       debug
     }
 
-    assert.ok(exclude(config, { url }))
-    assert.strictEqual(exclude(config, { url: 'https://some-rest.api/invoices' }), false)
+    assert.ok(exclude(config, { url, method }))
+    assert.strictEqual(exclude(config, { url: 'https://some-rest.api/invoices', method }), false)
   })
 
   it('Should exclude filtered requests', () => {
     const config = {
       exclude: {
-        filter: req => req.params && req.params.shouldExclude
+        filter: req => req.params && req.params.shouldExclude,
+        methods
       },
       debug
     }
 
-    assert.ok(exclude(config, { url, params: { shouldExclude: true } }))
-    assert.strictEqual(exclude(config, { url }), false)
+    assert.ok(exclude(config, { url, method, params: { shouldExclude: true } }))
+    assert.strictEqual(exclude(config, { url, method }), false)
   })
 })
diff --git a/test/spec/index.spec.js b/test/spec/index.spec.js
index 5e5005e..9671dc7 100644
--- a/test/spec/index.spec.js
+++ b/test/spec/index.spec.js
@@ -68,7 +68,7 @@ describe('Integration', function () {
     }
   })
 
-  it('Should bust cache when sending something else than a get, post, patch, put or delete request', async function () {
+  it('Should bust cache when sending any of the configured exclude.methods while using default invalidate method', async function () {
     this.timeout(REQUEST_TIMEOUT)
 
     await api.cache.clear()
@@ -83,7 +83,7 @@ describe('Integration', function () {
 
     assert.strictEqual(length, 1)
 
-    res = await api({ url, method: 'OPTIONS' })
+    res = await api({ url, method: 'POST' })
     length = await api.cache.length()
 
     assert.strictEqual(length, 0)
diff --git a/test/spec/request.spec.js b/test/spec/request.spec.js
index d067aa2..018f0b1 100644
--- a/test/spec/request.spec.js
+++ b/test/spec/request.spec.js
@@ -14,6 +14,7 @@ describe('Request', () => {
   let res
   // let expires
   let store
+  const methods = ['post', 'patch', 'put', 'delete']
 
   beforeEach(() => {
     // expires = Date.now()
@@ -23,7 +24,8 @@ describe('Request', () => {
       key: key('test'),
       store,
       debug,
-      invalidate: invalidate()
+      invalidate: invalidate(),
+      exclude: { methods }
     }
 
     req = {
@@ -42,9 +44,7 @@ describe('Request', () => {
 
   it('Should notify an exclusion if url matches exclude params', async () => {
     // Exclude everything
-    config.exclude = {
-      paths: [/.+/]
-    }
+    config.exclude.paths = [/.+/]
 
     const result = await request(config, req)
 
@@ -59,8 +59,8 @@ describe('Request', () => {
     testExclusion(result)
   })
 
-  it('Should notify an exclusion and clear cache for http methods not one of get, post, patch, put or delete', async () => {
-    req.method = 'OPTIONS'
+  it('Should notify an exclusion and clear cache for http methods present in config.exclude.methods list', async () => {
+    req.method = 'POST'
 
     await store.setItem('https://httpbin.org/', res)