From b701148700679c1afddb4fd8d400919a0bc5b4d8 Mon Sep 17 00:00:00 2001
From: Dirk Weber <65392306+dwrBad@users.noreply.github.com>
Date: Sat, 19 Mar 2022 21:21:36 +0100
Subject: [PATCH] feat: make POST requests cacheable (#24)

---
 src/http-data-source.ts       | 22 ++++++++---
 test/http-data-source.test.ts | 72 ++++++++++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/src/http-data-source.ts b/src/http-data-source.ts
index 1acf8d2..e9d6c14 100644
--- a/src/http-data-source.ts
+++ b/src/http-data-source.ts
@@ -140,7 +140,13 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
     request: Request,
     response: Response<TResult>,
   ): boolean {
-    return statusCodeCacheableByDefault.has(response.statusCode) && request.method === 'GET'
+    return statusCodeCacheableByDefault.has(response.statusCode) && this.isRequestCacheable(request)
+  }
+
+  protected isRequestCacheable(request: Request): boolean {
+    // default behaviour is to cache only get requests
+    // If extending to non GET requests take care to provide an adequate onCacheKeyCalculation and isResponseCacheable
+    return request.method === 'GET'
   }
 
   /**
@@ -201,7 +207,7 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
    * Execute a HTTP GET request.
    * Note that the **memoizedResults** and **cache** will be checked before request is made.
    * By default the received response will be memoized.
-   * 
+   *
    * @param path the path to the resource
    * @param requestOptions
    */
@@ -380,9 +386,11 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
 
     const cacheKey = this.onCacheKeyCalculation(request)
 
-    // check if we have any GET call in the cache to respond immediately
-    if (request.method === 'GET') {
-      // Memoize GET calls for the same data source instance
+    const isRequestMemoizable = this.isRequestMemoizable(request)
+
+    // check if we have a memoizable call in the cache to respond immediately
+    if (isRequestMemoizable) {
+      // Memoize calls for the same data source instance
       // a single instance of the data sources is scoped to one graphql request
       if (this.memoizedResults.has(cacheKey)) {
         const response = await this.memoizedResults.get(cacheKey)!
@@ -403,7 +411,9 @@ export abstract class HTTPDataSource<TContext = any> extends DataSource {
       headers,
     }
 
-    if (options.method === 'GET') {
+    const requestIsCacheable = this.isRequestCacheable(request)
+
+    if (requestIsCacheable) {
       // try to fetch from shared cache
       if (request.requestCache) {
         try {
diff --git a/test/http-data-source.test.ts b/test/http-data-source.test.ts
index 3ca3bfb..6430958 100644
--- a/test/http-data-source.test.ts
+++ b/test/http-data-source.test.ts
@@ -1310,7 +1310,7 @@ test('Should respond with stale-if-error cache on origin error', async (t) => {
   t.is(cacheMap.size, 1)
 })
 
-test('Should not cache POST requests', async (t) => {
+test('Should not cache POST requests by default', async (t) => {
   t.plan(6)
 
   const path = '/'
@@ -1481,6 +1481,76 @@ test('Should only cache GET successful responses with the correct status code',
   t.is(cacheMap.size, 0)
 })
 
+test('Should cache POST successful responses if isRequestCacheable allows to do so', async (t) => {
+  t.plan(7)
+
+  const path = '/custom/cacheable/post/route'
+
+  const wanted = { name: 'foo' }
+  const server = http.createServer((req, res) => {
+    t.is(req.method, 'POST')
+    res.writeHead(200, {
+      'content-type': 'application/json',
+    })
+    res.write(JSON.stringify(wanted))
+    res.end()
+    res.socket?.unref()
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen()
+
+  const baseURL = `http://localhost:${(server.address() as AddressInfo)?.port}`
+
+  const dataSource = new (class extends HTTPDataSource {
+    constructor() {
+      super(baseURL)
+    }
+    protected isRequestCacheable(request: Request): boolean {
+      return request.method === 'GET' || (request.method === 'POST' && request.path === path)
+    }
+    postFoo() {
+      return this.post(path, {
+        body: wanted,
+        requestCache: {
+          maxTtl: 10,
+          maxTtlIfError: 20,
+        },
+      })
+    }
+  })()
+
+  const cacheMap = new Map<string, string>()
+
+  dataSource.initialize({
+    context: {
+      a: 1,
+    },
+    cache: {
+      async delete(key: string) {
+        return cacheMap.delete(key)
+      },
+      async get(key: string) {
+        return cacheMap.get(key)
+      },
+      async set(key: string, value: string) {
+        cacheMap.set(key, value)
+      },
+    },
+  })
+
+  let response = await dataSource.postFoo()
+  t.deepEqual(response.body, wanted)
+  t.false(response.memoized)
+  t.false(response.isFromCache)
+
+  response = await dataSource.postFoo()
+  t.deepEqual(response.body, wanted)
+  t.false(response.memoized)
+  t.true(response.isFromCache)
+})
+
 test.serial('Global maxAge should be used when no maxAge was set or similar.', async (t) => {
   const path = '/'