From 7762d542002e565a69cf8a715ec38e5dfeadbfdf Mon Sep 17 00:00:00 2001
From: Eric Peterson <ericpeterson@spotify.com>
Date: Fri, 18 Mar 2022 11:23:50 +0100
Subject: [PATCH] Fix TechDocs backend cache bug on HEAD request.

Signed-off-by: Eric Peterson <ericpeterson@spotify.com>
---
 .../techdocs-head-shoulders-knees-toes.md       |  5 +++++
 .../src/cache/cacheMiddleware.test.ts           | 17 +++++++++++++++++
 .../src/cache/cacheMiddleware.ts                |  8 +++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/techdocs-head-shoulders-knees-toes.md

diff --git a/.changeset/techdocs-head-shoulders-knees-toes.md b/.changeset/techdocs-head-shoulders-knees-toes.md
new file mode 100644
index 0000000000..f96b624897
--- /dev/null
+++ b/.changeset/techdocs-head-shoulders-knees-toes.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-techdocs-backend': patch
+---
+
+Fixed a bug affecting those with cache enabled that would result in empty content being cached if the first attempt to load a static asset from storage were made via a `HEAD` request, rather than a `GET` request.
diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts
index 312674a5bf..b6a0f7db56 100644
--- a/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts
+++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.test.ts
@@ -85,6 +85,15 @@ describe('createCacheMiddleware', () => {
       expect(cache.set).not.toHaveBeenCalled();
     });
 
+    it('checks cache for head requests', async () => {
+      cache.get.mockResolvedValueOnce(getMockHttpResponseFor('xyz'));
+
+      await request(app).head('/static/docs/foo.html').expect(200);
+
+      await waitForSocketClose();
+      expect(cache.set).not.toHaveBeenCalled();
+    });
+
     it('sets cache when content is cacheable', async () => {
       const expectedPath = 'default/api/xyz/index.html';
       await request(app)
@@ -105,5 +114,13 @@ describe('createCacheMiddleware', () => {
       await waitForSocketClose();
       expect(cache.set).not.toHaveBeenCalled();
     });
+
+    it('does not set cache on head requests', async () => {
+      const expectedPath = 'default/api/xyz/index.html';
+      await request(app).head(`/static/docs/${expectedPath}`).expect(200);
+
+      await waitForSocketClose();
+      expect(cache.set).not.toHaveBeenCalled();
+    });
   });
 });
diff --git a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts
index cb59681304..51cce6d220 100644
--- a/plugins/techdocs-backend/src/cache/cacheMiddleware.ts
+++ b/plugins/techdocs-backend/src/cache/cacheMiddleware.ts
@@ -36,6 +36,7 @@ export const createCacheMiddleware = ({
   cacheMiddleware.use(async (req, res, next) => {
     const socket = res.socket;
     const isCacheable = req.path.startsWith('/static/docs/');
+    const isGetRequest = req.method === 'GET';
 
     // Continue early if this is non-cacheable, or there's no socket.
     if (!isCacheable || !socket) {
@@ -69,7 +70,12 @@ export const createCacheMiddleware = ({
     socket.on('close', async hadError => {
       const content = Buffer.concat(chunks);
       const head = content.toString('utf8', 0, 12);
-      if (writeToCache && !hadError && head.match(/HTTP\/\d\.\d 200/)) {
+      if (
+        isGetRequest &&
+        writeToCache &&
+        !hadError &&
+        head.match(/HTTP\/\d\.\d 200/)
+      ) {
         await cache.set(reqPath, content);
       }
     });