Implement the etag functionality in the readUrl method of FetchUrlReader

Signed-off-by: Dominik Henneke <dominik.henneke@sda-se.com>

.changeset/friendly-tips-jam.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Implement the etag functionality in the `readUrl` method of `FetchUrlReader`.

packages/backend-common/src/reading/FetchUrlReader.test.ts

@@ -15,12 +15,16 @@
  */
 
 import { ConfigReader } from '@backstage/config';
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import { msw } from '@backstage/test-utils';
+import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import { getVoidLogger } from '../logging';
 import { FetchUrlReader } from './FetchUrlReader';
 import { DefaultReadTreeResponseFactory } from './tree';
 
+const fetchUrlReader = new FetchUrlReader();
+
 describe('FetchUrlReader', () => {
   const worker = setupServer();
 
@@ -30,6 +34,39 @@ describe('FetchUrlReader', () => {
     jest.clearAllMocks();
   });
 
+  beforeEach(() => {
+    worker.use(
+      rest.get('https://backstage.io/some-resource', (req, res, ctx) => {
+        if (req.headers.get('if-none-match') === 'foo') {
+          return res(
+            ctx.status(304),
+            ctx.set('Content-Type', 'text/plain'),
+            ctx.set('etag', 'foo'),
+          );
+        }
+
+        return res(
+          ctx.status(200),
+          ctx.set('Content-Type', 'text/plain'),
+          ctx.set('etag', 'foo'),
+          ctx.body('content foo'),
+        );
+      }),
+    );
+
+    worker.use(
+      rest.get('https://backstage.io/not-exists', (_req, res, ctx) => {
+        return res(ctx.status(404));
+      }),
+    );
+
+    worker.use(
+      rest.get('https://backstage.io/error', (_req, res, ctx) => {
+        return res(ctx.status(500), ctx.body('An internal error occured'));
+      }),
+    );
+  });
+
   it('factory should create a single entry with a predicate that matches config', async () => {
     const entries = FetchUrlReader.factory({
       config: new ConfigReader({
@@ -70,4 +107,55 @@ describe('FetchUrlReader', () => {
     expect(predicate(new URL('https://a.examples.org:700/test'))).toBe(true);
     expect(predicate(new URL('https://a.b.examples.org:700/test'))).toBe(true);
   });
+
+  describe('read', () => {
+    it('should return etag from the response', async () => {
+      const buffer = await fetchUrlReader.read(
+        'https://backstage.io/some-resource',
+      );
+      expect(buffer.toString()).toBe('content foo');
+    });
+
+    it('should throw NotFound if server responds with 404', async () => {
+      await expect(
+        fetchUrlReader.read('https://backstage.io/not-exists'),
+      ).rejects.toThrow(NotFoundError);
+    });
+
+    it('should throw Error if server responds with 500', async () => {
+      await expect(
+        fetchUrlReader.read('https://backstage.io/error'),
+      ).rejects.toThrow(Error);
+    });
+  });
+
+  describe('readUrl', () => {
+    it('should throw NotModified if server responds with 304', async () => {
+      await expect(
+        fetchUrlReader.readUrl('https://backstage.io/some-resource', {
+          etag: 'foo',
+        }),
+      ).rejects.toThrow(NotModifiedError);
+    });
+
+    it('should return etag from the response', async () => {
+      const response = await fetchUrlReader.readUrl(
+        'https://backstage.io/some-resource',
+      );
+      expect(response.etag).toBe('foo');
+      expect((await response.buffer()).toString()).toEqual('content foo');
+    });
+
+    it('should throw NotFound if server responds with 404', async () => {
+      await expect(
+        fetchUrlReader.readUrl('https://backstage.io/not-exists'),
+      ).rejects.toThrow(NotFoundError);
+    });
+
+    it('should throw Error if server responds with 500', async () => {
+      await expect(
+        fetchUrlReader.readUrl('https://backstage.io/error'),
+      ).rejects.toThrow(Error);
+    });
+  });
 });

packages/backend-common/src/reading/FetchUrlReader.ts

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
+import { NotFoundError, NotModifiedError } from '@backstage/errors';
 import fetch from 'cross-fetch';
-import { NotFoundError } from '@backstage/errors';
 import {
   ReaderFactory,
   ReadTreeResponse,
@@ -57,15 +57,34 @@ export class FetchUrlReader implements UrlReader {
   };
 
   async read(url: string): Promise<Buffer> {
+    const response = await this.readUrl(url);
+    return response.buffer();
+  }
+
+  async readUrl(
+    url: string,
+    options?: ReadUrlOptions,
+  ): Promise<ReadUrlResponse> {
     let response: Response;
     try {
-      response = await fetch(url);
+      response = await fetch(url, {
+        headers: {
+          ...(options?.etag && { 'If-None-Match': options.etag }),
+        },
+      });
     } catch (e) {
       throw new Error(`Unable to read ${url}, ${e}`);
     }
 
+    if (response.status === 304) {
+      throw new NotModifiedError();
+    }
+
     if (response.ok) {
-      return Buffer.from(await response.text());
+      return {
+        buffer: async () => Buffer.from(await response.text()),
+        etag: response.headers.get('ETag') ?? undefined,
+      };
     }
 
     const message = `could not read ${url}, ${response.status} ${response.statusText}`;
@@ -75,15 +94,6 @@ export class FetchUrlReader implements UrlReader {
     throw new Error(message);
   }
 
-  async readUrl(
-    url: string,
-    _options?: ReadUrlOptions,
-  ): Promise<ReadUrlResponse> {
-    // TODO etag is not implemented yet.
-    const buffer = await this.read(url);
-    return { buffer: async () => buffer };
-  }
-
   async readTree(): Promise<ReadTreeResponse> {
     throw new Error('FetchUrlReader does not implement readTree');
   }