From c1ee073a82b5c7294a3f243ad86834c8c4518cb1 Mon Sep 17 00:00:00 2001 From: Andrew Thauer Date: Mon, 13 Mar 2023 16:06:08 -0400 Subject: [PATCH 1/2] feat: add lastModifiedAt to UrlReader methods Signed-off-by: Andrew Thauer --- .changeset/sharp-rings-cry.md | 6 ++ packages/backend-common/api-report.md | 2 + .../src/reading/AwsS3UrlReader.test.ts | 42 +++++++++- .../src/reading/AwsS3UrlReader.ts | 30 ++++---- .../src/reading/AzureUrlReader.ts | 1 + .../reading/BitbucketCloudUrlReader.test.ts | 77 +++++++++++++++++++ .../src/reading/BitbucketCloudUrlReader.ts | 10 ++- .../src/reading/BitbucketServerUrlReader.ts | 10 ++- .../src/reading/BitbucketUrlReader.test.ts | 77 +++++++++++++++++++ .../src/reading/BitbucketUrlReader.ts | 10 ++- .../src/reading/FetchUrlReader.test.ts | 25 +++++- .../src/reading/FetchUrlReader.ts | 7 ++ .../src/reading/GiteaUrlReader.test.ts | 48 +++++++++++- .../src/reading/GiteaUrlReader.ts | 4 + .../src/reading/GithubUrlReader.test.ts | 8 +- .../src/reading/GithubUrlReader.ts | 8 ++ .../src/reading/GitlabUrlReader.test.ts | 38 ++++++++- .../src/reading/GitlabUrlReader.ts | 10 ++- .../src/reading/ReadUrlResponseFactory.ts | 1 + .../src/reading/tree/ReadableArrayResponse.ts | 1 + .../reading/tree/TarArchiveResponse.test.ts | 5 ++ .../reading/tree/ZipArchiveResponse.test.ts | 5 ++ .../src/reading/tree/ZipArchiveResponse.ts | 3 + packages/backend-common/src/reading/types.ts | 7 ++ packages/backend-common/src/reading/util.ts | 23 ++++++ packages/backend-plugin-api/api-report.md | 4 + .../services/definitions/UrlReaderService.ts | 42 ++++++++++ 27 files changed, 477 insertions(+), 27 deletions(-) create mode 100644 .changeset/sharp-rings-cry.md create mode 100644 packages/backend-common/src/reading/util.ts diff --git a/.changeset/sharp-rings-cry.md b/.changeset/sharp-rings-cry.md new file mode 100644 index 0000000000..9e32a53526 --- /dev/null +++ b/.changeset/sharp-rings-cry.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-common': minor +'@backstage/backend-plugin-api': minor +--- + +Added `lastModifiedAt` field on `UrlReaderService` responses and a `lastModifiedAfter` option to `UrlReaderService.readUrl`. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index ebdd7315c9..f1d8a74dbe 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -301,6 +301,7 @@ export class FetchUrlReader implements UrlReader { export type FromReadableArrayOptions = Array<{ data: Readable; path: string; + lastModifiedAt?: Date; }>; // @public @@ -635,6 +636,7 @@ export class ReadUrlResponseFactory { // @public export type ReadUrlResponseFactoryFromStreamOptions = { etag?: string; + lastModifiedAt?: Date; }; // @public diff --git a/packages/backend-common/src/reading/AwsS3UrlReader.test.ts b/packages/backend-common/src/reading/AwsS3UrlReader.test.ts index 0bd4d2636f..b37a052c1d 100644 --- a/packages/backend-common/src/reading/AwsS3UrlReader.test.ts +++ b/packages/backend-common/src/reading/AwsS3UrlReader.test.ts @@ -297,23 +297,26 @@ describe('AwsS3UrlReader', () => { ), ), ETag: '123abc', + LastModified: new Date('2020-01-01T00:00:00Z'), }); }); it('returns contents of an object in a bucket via buffer', async () => { - const { buffer, etag } = await reader.readUrl( + const { buffer, etag, lastModifiedAt } = await reader.readUrl( 'https://test-bucket.s3.us-east-2.amazonaws.com/awsS3-mock-object.yaml', ); expect(etag).toBe('123abc'); + expect(lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); const response = await buffer(); expect(response.toString().trim()).toBe('site_name: Test'); }); it('returns contents of an object in a bucket via stream', async () => { - const { buffer, etag } = await reader.readUrl( + const { buffer, etag, lastModifiedAt } = await reader.readUrl( 'https://test-bucket.s3.us-east-2.amazonaws.com/awsS3-mock-object.yaml', ); expect(etag).toBe('123abc'); + expect(lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); const response = await buffer(); expect(response.toString().trim()).toBe('site_name: Test'); }); @@ -403,6 +406,41 @@ describe('AwsS3UrlReader', () => { }); }); + describe('readUrl with lastModifiedAfter', () => { + const [{ reader }] = createReader({ + integrations: { + awsS3: [ + { + host: 'amazonaws.com', + accessKeyId: 'fake-access-key', + secretAccessKey: 'fake-secret-key', + }, + ], + }, + }); + + beforeEach(() => { + s3Client.reset(); + const t = new S3ServiceException({ + name: '304', + $fault: 'client', + $metadata: { httpStatusCode: 304 }, + }); + s3Client.on(GetObjectCommand).rejects(t); + }); + + it('returns contents of an object in a bucket', async () => { + await expect( + reader.readUrl!( + 'https://test-bucket.s3.us-east-2.amazonaws.com/awsS3-mock-object.yaml', + { + lastModifiedAfter: new Date('2020-01-01T00:00:00Z'), + }, + ), + ).rejects.toThrow(NotModifiedError); + }); + }); + describe('readTree', () => { let awsS3UrlReader: AwsS3UrlReader; diff --git a/packages/backend-common/src/reading/AwsS3UrlReader.ts b/packages/backend-common/src/reading/AwsS3UrlReader.ts index d06e2d0ad6..4d4c28071f 100644 --- a/packages/backend-common/src/reading/AwsS3UrlReader.ts +++ b/packages/backend-common/src/reading/AwsS3UrlReader.ts @@ -41,6 +41,7 @@ import { ListObjectsV2Command, ListObjectsV2CommandOutput, GetObjectCommand, + GetObjectCommandInput, } from '@aws-sdk/client-s3'; import { AbortController } from '@aws-sdk/abort-controller'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; @@ -253,6 +254,8 @@ export class AwsS3UrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { + const { etag, lastModifiedAfter } = options ?? {}; + try { const { path, bucket, region } = parseUrl(url, this.integration.config); const s3Client = await this.buildS3Client( @@ -262,19 +265,15 @@ export class AwsS3UrlReader implements UrlReader { ); const abortController = new AbortController(); - let params; - if (options?.etag) { - params = { - Bucket: bucket, - Key: path, - IfNoneMatch: options.etag, - }; - } else { - params = { - Bucket: bucket, - Key: path, - }; - } + const params: GetObjectCommandInput = { + Bucket: bucket, + Key: path, + ...(etag && { IfNoneMatch: etag }), + ...(lastModifiedAfter && { + IfModifiedSince: lastModifiedAfter, + }), + }; + options?.signal?.addEventListener('abort', () => abortController.abort()); const getObjectCommand = new GetObjectCommand(params); const response = await s3Client.send(getObjectCommand, { @@ -284,10 +283,10 @@ export class AwsS3UrlReader implements UrlReader { const s3ObjectData = await this.retrieveS3ObjectData( response.Body as Readable, ); - const etag = response.ETag; return ReadUrlResponseFactory.fromReadable(s3ObjectData, { - etag: etag, + etag: response.ETag, + lastModifiedAt: response.LastModified, }); } catch (e) { if (e.$metadata && e.$metadata.httpStatusCode === 304) { @@ -347,6 +346,7 @@ export class AwsS3UrlReader implements UrlReader { responses.push({ data: s3ObjectData, path: String(allObjects[i]), + lastModifiedAt: response?.LastModified ?? undefined, }); } diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 08e3917b38..90cf8f29bd 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -192,6 +192,7 @@ export class AzureUrlReader implements UrlReader { base: url, }), content: file.content, + lastModifiedAt: file.lastModifiedAt, })), }; } diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts index ca6e15a005..c00aeba124 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.test.ts @@ -156,6 +156,83 @@ describe('BitbucketCloudUrlReader', () => { expect(buffer.toString()).toBe('foo'); expect(result.etag).toBe('new-etag-value'); }); + + it('should be able to readUrl via buffer without If-Modified-Since', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-None-Match')).toBeNull(); + return res( + ctx.status(200), + ctx.body('foo'), + ctx.set('ETag', 'etag-value'), + ctx.set( + 'Last-Modified', + new Date('2020-01-01T00:00:00Z').toUTCString(), + ), + ); + }, + ), + ); + + const result = await reader.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + ); + const buffer = await result.buffer(); + expect(result.lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); + expect(buffer.toString()).toBe('foo'); + }); + + it('should be throw not modified when If-Modified-Since returns a 304', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-Modified-Since')).toBe( + new Date('1999 12 31 23:59:59 GMT').toUTCString(), + ); + return res(ctx.status(304)); + }, + ), + ); + + await expect( + reader.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + { lastModifiedAfter: new Date('1999 12 31 23:59:59 GMT') }, + ), + ).rejects.toThrow(NotModifiedError); + }); + + it('should be able to readUrl when If-Modified-Since is before Last-Modified', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-Modified-Since')).toBe( + new Date('1999 12 31 23:59:59 GMT').toUTCString(), + ); + return res( + ctx.status(200), + ctx.set( + 'Last-Modified', + new Date('2020-01-01T00:00:00Z').toUTCString(), + ), + ctx.body('foo'), + ); + }, + ), + ); + + const result = await reader.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + { lastModifiedAfter: new Date('1999 12 31 23:59:59 GMT') }, + ); + const buffer = await result.buffer(); + expect(buffer.toString()).toBe('foo'); + expect(result.lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); + }); }); describe('read', () => { diff --git a/packages/backend-common/src/reading/BitbucketCloudUrlReader.ts b/packages/backend-common/src/reading/BitbucketCloudUrlReader.ts index b83e012423..1522b7678d 100644 --- a/packages/backend-common/src/reading/BitbucketCloudUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketCloudUrlReader.ts @@ -40,6 +40,7 @@ import { UrlReader, } from './types'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; /** * Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for files from Bitbucket Cloud. @@ -80,7 +81,7 @@ export class BitbucketCloudUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { - const { etag, signal } = options ?? {}; + const { etag, lastModifiedAfter, signal } = options ?? {}; const bitbucketUrl = getBitbucketCloudFileFetchUrl( url, this.integration.config, @@ -95,6 +96,9 @@ export class BitbucketCloudUrlReader implements UrlReader { headers: { ...requestOptions.headers, ...(etag && { 'If-None-Match': etag }), + ...(lastModifiedAfter && { + 'If-Modified-Since': lastModifiedAfter.toUTCString(), + }), }, // TODO(freben): The signal cast is there because pre-3.x versions of // node-fetch have a very slightly deviating AbortSignal type signature. @@ -115,6 +119,9 @@ export class BitbucketCloudUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } @@ -184,6 +191,7 @@ export class BitbucketCloudUrlReader implements UrlReader { base: url, }), content: file.content, + lastModifiedAt: file.lastModifiedAt, })), }; } diff --git a/packages/backend-common/src/reading/BitbucketServerUrlReader.ts b/packages/backend-common/src/reading/BitbucketServerUrlReader.ts index a97c3b39bb..2b6b5a1f8f 100644 --- a/packages/backend-common/src/reading/BitbucketServerUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketServerUrlReader.ts @@ -39,6 +39,7 @@ import { UrlReader, } from './types'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; /** * Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for files from Bitbucket Server APIs. @@ -71,7 +72,7 @@ export class BitbucketServerUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { - const { etag, signal } = options ?? {}; + const { etag, lastModifiedAfter, signal } = options ?? {}; const bitbucketUrl = getBitbucketServerFileFetchUrl( url, this.integration.config, @@ -86,6 +87,9 @@ export class BitbucketServerUrlReader implements UrlReader { headers: { ...requestOptions.headers, ...(etag && { 'If-None-Match': etag }), + ...(lastModifiedAfter && { + 'If-Modified-Since': lastModifiedAfter.toUTCString(), + }), }, // TODO(freben): The signal cast is there because pre-3.x versions of // node-fetch have a very slightly deviating AbortSignal type signature. @@ -106,6 +110,9 @@ export class BitbucketServerUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } @@ -175,6 +182,7 @@ export class BitbucketServerUrlReader implements UrlReader { base: url, }), content: file.content, + lastModifiedAt: file.lastModifiedAt, })), }; } diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 922480f8e7..15e007d516 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -204,6 +204,83 @@ describe('BitbucketUrlReader', () => { expect(buffer.toString()).toBe('foo'); expect(result.etag).toBe('new-etag-value'); }); + + it('should be able to readUrl via buffer without If-Modified-Since', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-None-Match')).toBeNull(); + return res( + ctx.status(200), + ctx.body('foo'), + ctx.set('ETag', 'etag-value'), + ctx.set( + 'Last-Modified', + new Date('2020-01-01T00:00:00Z').toUTCString(), + ), + ); + }, + ), + ); + + const result = await bitbucketProcessor.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + ); + const buffer = await result.buffer(); + expect(result.lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); + expect(buffer.toString()).toBe('foo'); + }); + + it('should be throw not modified when If-Modified-Since returns a 304', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-Modified-Since')).toBe( + new Date('1999 12 31 23:59:59 GMT').toUTCString(), + ); + return res(ctx.status(304)); + }, + ), + ); + + await expect( + bitbucketProcessor.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + { lastModifiedAfter: new Date('1999 12 31 23:59:59 GMT') }, + ), + ).rejects.toThrow(NotModifiedError); + }); + + it('should be able to readUrl when If-Modified-Since is before Last-Modified', async () => { + worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage-verification/test-template/src/master/template.yaml', + (req, res, ctx) => { + expect(req.headers.get('If-Modified-Since')).toBe( + new Date('1999 12 31 23:59:59 GMT').toUTCString(), + ); + return res( + ctx.status(200), + ctx.set( + 'Last-Modified', + new Date('2020-01-01T00:00:00Z').toUTCString(), + ), + ctx.body('foo'), + ); + }, + ), + ); + + const result = await bitbucketProcessor.readUrl( + 'https://bitbucket.org/backstage-verification/test-template/src/master/template.yaml', + { lastModifiedAfter: new Date('1999 12 31 23:59:59 GMT') }, + ); + const buffer = await result.buffer(); + expect(buffer.toString()).toBe('foo'); + expect(result.lastModifiedAt).toEqual(new Date('2020-01-01T00:00:00Z')); + }); }); describe('read', () => { diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 01539594dd..3a34c725c6 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -41,6 +41,7 @@ import { UrlReader, } from './types'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; /** * Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for files from Bitbucket v1 and v2 APIs, such @@ -96,7 +97,7 @@ export class BitbucketUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { - const { etag, signal } = options ?? {}; + const { etag, lastModifiedAfter, signal } = options ?? {}; const bitbucketUrl = getBitbucketFileFetchUrl(url, this.integration.config); const requestOptions = getBitbucketRequestOptions(this.integration.config); @@ -106,6 +107,9 @@ export class BitbucketUrlReader implements UrlReader { headers: { ...requestOptions.headers, ...(etag && { 'If-None-Match': etag }), + ...(lastModifiedAfter && { + 'If-Modified-Since': lastModifiedAfter.toUTCString(), + }), }, // TODO(freben): The signal cast is there because pre-3.x versions of // node-fetch have a very slightly deviating AbortSignal type signature. @@ -126,6 +130,9 @@ export class BitbucketUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } @@ -195,6 +202,7 @@ export class BitbucketUrlReader implements UrlReader { base: url, }), content: file.content, + lastModifiedAt: file.lastModifiedAt, })), }; } diff --git a/packages/backend-common/src/reading/FetchUrlReader.test.ts b/packages/backend-common/src/reading/FetchUrlReader.test.ts index e45117a106..ad834c64ef 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.test.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.test.ts @@ -45,6 +45,21 @@ describe('FetchUrlReader', () => { ); } + if ( + req.headers.get('if-modified-since') && + new Date(req.headers.get('if-modified-since') ?? '') < + new Date('2021-01-01T00:00:00Z') + ) { + return res( + ctx.status(304), + ctx.set('Content-Type', 'text/plain'), + ctx.set( + 'last-modified', + new Date('2021-01-01T00:00:00Z').toUTCString(), + ), + ); + } + return res( ctx.status(200), ctx.set('Content-Type', 'text/plain'), @@ -192,7 +207,7 @@ describe('FetchUrlReader', () => { }); describe('readUrl', () => { - it('should throw NotModified if server responds with 304', async () => { + it('should throw NotModified if server responds with 304 from etag', async () => { await expect( fetchUrlReader.readUrl('https://backstage.io/some-resource', { etag: 'foo', @@ -200,6 +215,14 @@ describe('FetchUrlReader', () => { ).rejects.toThrow(NotModifiedError); }); + it('should throw NotModified if server responds with 304 from lastModifiedAfter', async () => { + await expect( + fetchUrlReader.readUrl('https://backstage.io/some-resource', { + lastModifiedAfter: new Date('2020-01-01T00:00:00Z'), + }), + ).rejects.toThrow(NotModifiedError); + }); + it('should return etag from the response', async () => { const response = await fetchUrlReader.readUrl( 'https://backstage.io/some-resource', diff --git a/packages/backend-common/src/reading/FetchUrlReader.ts b/packages/backend-common/src/reading/FetchUrlReader.ts index 20b78d63c3..90c1cf6c26 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.ts @@ -26,6 +26,7 @@ import { } from './types'; import path from 'path'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; const isInRange = (num: number, [start, end]: [number, number]) => { return num >= start && num <= end; @@ -127,6 +128,9 @@ export class FetchUrlReader implements UrlReader { response = await fetch(url, { headers: { ...(options?.etag && { 'If-None-Match': options.etag }), + ...(options?.lastModifiedAfter && { + 'If-Modified-Since': options.lastModifiedAfter.toUTCString(), + }), }, // TODO(freben): The signal cast is there because pre-3.x versions of // node-fetch have a very slightly deviating AbortSignal type signature. @@ -147,6 +151,9 @@ export class FetchUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } diff --git a/packages/backend-common/src/reading/GiteaUrlReader.test.ts b/packages/backend-common/src/reading/GiteaUrlReader.test.ts index 6c8722d2e5..1860d9f461 100644 --- a/packages/backend-common/src/reading/GiteaUrlReader.test.ts +++ b/packages/backend-common/src/reading/GiteaUrlReader.test.ts @@ -25,7 +25,7 @@ import { UrlReaderPredicateTuple } from './types'; import { DefaultReadTreeResponseFactory } from './tree'; import getRawBody from 'raw-body'; import { GiteaUrlReader } from './GiteaUrlReader'; -import { NotFoundError } from '@backstage/errors'; +import { NotFoundError, NotModifiedError } from '@backstage/errors'; const treeResponseFactory = DefaultReadTreeResponseFactory.create({ config: new ConfigReader({}), @@ -200,5 +200,51 @@ describe('GiteaUrlReader', () => { 'https://gitea.com/owner/project/src/branch/branch2/LICENSE could not be read as https://gitea.com/api/v1/repos/owner/project/contents/LICENSE?ref=branch2, 500 Error!!!', ); }); + + it('should throw NotModified if server responds with 304 from etag', async () => { + worker.use( + rest.get( + 'https://gitea.com/api/v1/repos/owner/project/contents/LICENSE', + (_, res, ctx) => { + return res(ctx.set('ETag', 'foo'), ctx.status(304, 'Error!!!')); + }, + ), + ); + + await expect( + giteaProcessor.readUrl( + 'https://gitea.com/owner/project/src/branch/branch2/LICENSE', + { + etag: 'foo', + }, + ), + ).rejects.toThrow(NotModifiedError); + }); + + it('should throw NotModified if server responds with 304 from lastModifiedAfter', async () => { + worker.use( + rest.get( + 'https://gitea.com/api/v1/repos/owner/project/contents/LICENSE', + (_, res, ctx) => { + return res( + ctx.set( + 'Last-Modified', + new Date('2020-01-01T00:00:00Z').toUTCString(), + ), + ctx.status(304, 'Error!!!'), + ); + }, + ), + ); + + await expect( + giteaProcessor.readUrl( + 'https://gitea.com/owner/project/src/branch/branch2/LICENSE', + { + lastModifiedAfter: new Date('2020-01-01T00:00:00Z'), + }, + ), + ).rejects.toThrow(NotModifiedError); + }); }); }); diff --git a/packages/backend-common/src/reading/GiteaUrlReader.ts b/packages/backend-common/src/reading/GiteaUrlReader.ts index 1471bab608..34542e878e 100644 --- a/packages/backend-common/src/reading/GiteaUrlReader.ts +++ b/packages/backend-common/src/reading/GiteaUrlReader.ts @@ -34,6 +34,7 @@ import { NotModifiedError, } from '@backstage/errors'; import { Readable } from 'stream'; +import { parseLastModified } from './util'; /** * Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for the Gitea v1 api. @@ -86,6 +87,9 @@ export class GiteaUrlReader implements UrlReader { Readable.from(Buffer.from(content, 'base64')), { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }, ); } diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 2315d1f9e8..1ee25920f2 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -247,7 +247,7 @@ describe('GithubUrlReader', () => { ).rejects.toThrow(/rate limit exceeded/); }); - it('should return etag from the response', async () => { + it('should return etag and last-modified from the response', async () => { (mockCredentialsProvider.getCredentials as jest.Mock).mockResolvedValue({ headers: { Authorization: 'bearer blah', @@ -261,6 +261,11 @@ describe('GithubUrlReader', () => { return res( ctx.status(200), ctx.set('Etag', 'foo'), + ctx.set( + 'Last-Modified', + new Date('2021-01-01T00:00:00Z').toUTCString(), + ), + ctx.body('bar'), ); }, @@ -271,6 +276,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock/tree/blob/main', ); expect(response.etag).toBe('foo'); + expect(response.lastModifiedAt).toEqual(new Date('2021-01-01T00:00:00Z')); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index d91c35621d..ee781daf0b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -40,6 +40,7 @@ import { ReadUrlResponse, } from './types'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; export type GhRepoResponse = RestEndpointMethodTypes['repos']['get']['response']['data']; @@ -109,6 +110,9 @@ export class GithubUrlReader implements UrlReader { headers: { ...credentials?.headers, ...(options?.etag && { 'If-None-Match': options.etag }), + ...(options?.lastModifiedAfter && { + 'If-Modified-Since': options.lastModifiedAfter.toUTCString(), + }), Accept: 'application/vnd.github.v3.raw', }, // TODO(freben): The signal cast is there because pre-3.x versions of @@ -130,6 +134,9 @@ export class GithubUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } @@ -290,6 +297,7 @@ export class GithubUrlReader implements UrlReader { return files.map(file => ({ url: pathToUrl(file.path), content: file.content, + lastModifiedAt: file.lastModifiedAt, })); } diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 3751ee868e..d926b2be9b 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -184,7 +184,7 @@ describe('GitlabUrlReader', () => { treeResponseFactory, }); - it('should throw NotModified on HTTP 304', async () => { + it('should throw NotModified on HTTP 304 from etag', async () => { worker.use( rest.get('*/api/v4/projects/:name', (_, res, ctx) => res(ctx.status(200), ctx.json({ id: 12345 })), @@ -205,13 +205,44 @@ describe('GitlabUrlReader', () => { ).rejects.toThrow(NotModifiedError); }); - it('should return etag in response', async () => { + it('should throw NotModified on HTTP 304 from lastModifiedAt', async () => { + worker.use( + rest.get('*/api/v4/projects/:name', (_, res, ctx) => + res(ctx.status(200), ctx.json({ id: 12345 })), + ), + rest.get('*', (req, res, ctx) => { + expect(req.headers.get('If-Modified-Since')).toBe( + new Date('2019 12 31 23:59:59 GMT').toUTCString(), + ); + return res(ctx.status(304)); + }), + ); + + await expect( + reader.readUrl!( + 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml', + { + lastModifiedAfter: new Date('2019 12 31 23:59:59 GMT'), + }, + ), + ).rejects.toThrow(NotModifiedError); + }); + + it('should return etag and last-modified in response', async () => { worker.use( rest.get('*/api/v4/projects/:name', (_, res, ctx) => res(ctx.status(200), ctx.json({ id: 12345 })), ), rest.get('*', (_req, res, ctx) => { - return res(ctx.status(200), ctx.set('ETag', '999'), ctx.body('foo')); + return res( + ctx.status(200), + ctx.set('ETag', '999'), + ctx.set( + 'Last-Modified', + new Date('2020 01 01 00:0:00 GMT').toUTCString(), + ), + ctx.body('foo'), + ); }), ); @@ -219,6 +250,7 @@ describe('GitlabUrlReader', () => { 'https://gitlab.com/groupA/teams/teamA/subgroupA/repoA/-/blob/branch/my/path/to/file.yaml', ); expect(result.etag).toBe('999'); + expect(result.lastModifiedAt).toEqual(new Date('2020 01 01 00:0:00 GMT')); const content = await result.buffer(); expect(content.toString()).toBe('foo'); }); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index bebc7453d0..df8441c1f7 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -40,6 +40,7 @@ import { } from './types'; import { trimEnd, trimStart } from 'lodash'; import { ReadUrlResponseFactory } from './ReadUrlResponseFactory'; +import { parseLastModified } from './util'; /** * Implements a {@link @backstage/backend-plugin-api#UrlReaderService} for files on GitLab. @@ -72,7 +73,7 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { - const { etag, signal } = options ?? {}; + const { etag, lastModifiedAfter, signal } = options ?? {}; const builtUrl = await this.getGitlabFetchUrl(url); let response: Response; @@ -81,6 +82,9 @@ export class GitlabUrlReader implements UrlReader { headers: { ...getGitLabRequestOptions(this.integration.config).headers, ...(etag && { 'If-None-Match': etag }), + ...(lastModifiedAfter && { + 'If-Modified-Since': lastModifiedAfter.toUTCString(), + }), }, // TODO(freben): The signal cast is there because pre-3.x versions of // node-fetch have a very slightly deviating AbortSignal type signature. @@ -101,6 +105,9 @@ export class GitlabUrlReader implements UrlReader { if (response.ok) { return ReadUrlResponseFactory.fromNodeJSReadable(response.body, { etag: response.headers.get('ETag') ?? undefined, + lastModifiedAt: parseLastModified( + response.headers.get('Last-Modified'), + ), }); } @@ -247,6 +254,7 @@ export class GitlabUrlReader implements UrlReader { files: files.map(file => ({ url: this.integration.resolveUrl({ url: `/${file.path}`, base: url }), content: file.content, + lastModifiedAt: file.lastModifiedAt, })), }; } diff --git a/packages/backend-common/src/reading/ReadUrlResponseFactory.ts b/packages/backend-common/src/reading/ReadUrlResponseFactory.ts index 91d1939c24..69b9c6c16a 100644 --- a/packages/backend-common/src/reading/ReadUrlResponseFactory.ts +++ b/packages/backend-common/src/reading/ReadUrlResponseFactory.ts @@ -61,6 +61,7 @@ export class ReadUrlResponseFactory { return stream; }, etag: options?.etag, + lastModifiedAt: options?.lastModifiedAt, }; } diff --git a/packages/backend-common/src/reading/tree/ReadableArrayResponse.ts b/packages/backend-common/src/reading/tree/ReadableArrayResponse.ts index eabaa3bc56..1429931142 100644 --- a/packages/backend-common/src/reading/tree/ReadableArrayResponse.ts +++ b/packages/backend-common/src/reading/tree/ReadableArrayResponse.ts @@ -63,6 +63,7 @@ export class ReadableArrayResponse implements ReadTreeResponse { files.push({ path: this.stream[i].path, content: () => getRawBody(this.stream[i].data), + lastModifiedAt: this.stream[i]?.lastModifiedAt, }); } } diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index f0cbc6a612..1765a41156 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -45,10 +45,12 @@ describe('TarArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: undefined, }, { path: 'docs/index.md', content: expect.any(Function), + lastModifiedAt: undefined, }, ]); const contents = await Promise.all(files.map(f => f.content())); @@ -70,6 +72,7 @@ describe('TarArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: undefined, }, ]); const content = await files[0].content(); @@ -93,10 +96,12 @@ describe('TarArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: undefined, }, { path: 'docs/index.md', content: expect.any(Function), + lastModifiedAt: undefined, }, ]); const contents = await Promise.all(files.map(f => f.content())); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index e45a25db66..0b010565dd 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -59,10 +59,12 @@ describe('ZipArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: expect.any(Date), }, { path: 'docs/index.md', content: expect.any(Function), + lastModifiedAt: expect.any(Date), }, ]); @@ -85,6 +87,7 @@ describe('ZipArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: expect.any(Date), }, ]); const content = await files[0].content(); @@ -108,10 +111,12 @@ describe('ZipArchiveResponse', () => { { path: 'mkdocs.yml', content: expect.any(Function), + lastModifiedAt: expect.any(Date), }, { path: 'docs/index.md', content: expect.any(Function), + lastModifiedAt: expect.any(Date), }, ]); const contents = await Promise.all(files.map(f => f.content())); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 5a1c5f9688..1e55150310 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -146,6 +146,9 @@ export class ZipArchiveResponse implements ReadTreeResponse { files.push({ path: this.getInnerPath(entry.fileName), content: async () => await streamToBuffer(content), + lastModifiedAt: entry.lastModFileTime + ? new Date(entry.lastModFileTime) + : undefined, }); }); diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index 35380029e2..7f4d407fd0 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -65,6 +65,7 @@ export type ReaderFactory = (options: { */ export type ReadUrlResponseFactoryFromStreamOptions = { etag?: string; + lastModifiedAt?: Date; }; /** @@ -95,10 +96,16 @@ export type FromReadableArrayOptions = Array<{ * The raw data itself. */ data: Readable; + /** * The filepath of the data. */ path: string; + + /** + * Last modified date of the file contents. + */ + lastModifiedAt?: Date; }>; /** diff --git a/packages/backend-common/src/reading/util.ts b/packages/backend-common/src/reading/util.ts new file mode 100644 index 0000000000..3d65c2c527 --- /dev/null +++ b/packages/backend-common/src/reading/util.ts @@ -0,0 +1,23 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export function parseLastModified(value: string | null | undefined) { + if (!value) { + return undefined; + } + + return new Date(value); +} diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 5e64502b78..7dc6ae97b3 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -359,11 +359,13 @@ export type ReadTreeResponseDirOptions = { export type ReadTreeResponseFile = { path: string; content(): Promise; + lastModifiedAt?: Date; }; // @public export type ReadUrlOptions = { etag?: string; + lastModifiedAfter?: Date; signal?: AbortSignal; }; @@ -372,6 +374,7 @@ export type ReadUrlResponse = { buffer(): Promise; stream?(): Readable; etag?: string; + lastModifiedAt?: Date; }; // @public (undocumented) @@ -420,6 +423,7 @@ export type SearchResponse = { export type SearchResponseFile = { url: string; content(): Promise; + lastModifiedAt?: Date; }; // @public (undocumented) diff --git a/packages/backend-plugin-api/src/services/definitions/UrlReaderService.ts b/packages/backend-plugin-api/src/services/definitions/UrlReaderService.ts index ef825a7fcd..c189b4a339 100644 --- a/packages/backend-plugin-api/src/services/definitions/UrlReaderService.ts +++ b/packages/backend-plugin-api/src/services/definitions/UrlReaderService.ts @@ -64,6 +64,26 @@ export type ReadUrlOptions = { */ etag?: string; + /** + * A date which can be provided to check whether a + * {@link UrlReaderService.readUrl} response has changed since the lastModifiedAt. + * + * @remarks + * + * In the {@link UrlReaderService.readUrl} response, an lastModifiedAt is returned + * along with data. The lastModifiedAt date represents the last time the data + * was modified. + * + * When an lastModifiedAfter is given in ReadUrlOptions, {@link UrlReaderService.readUrl} + * will compare the lastModifiedAfter against the lastModifiedAt of the target. If + * the data has not been modified since this date, the {@link UrlReaderService.readUrl} + * will throw a {@link @backstage/errors#NotModifiedError} indicating that the + * response does not contain any new data. If they do not match, + * {@link UrlReaderService.readUrl} will return the rest of the response along with new + * lastModifiedAt date. + */ + lastModifiedAfter?: Date; + /** * An abort signal to pass down to the underlying request. * @@ -102,6 +122,11 @@ export type ReadUrlResponse = { * Can be used to compare and cache responses when doing subsequent calls. */ etag?: string; + + /** + * Last modified date of the file contents. + */ + lastModifiedAt?: Date; }; /** @@ -213,8 +238,20 @@ export type ReadTreeResponse = { * @public */ export type ReadTreeResponseFile = { + /** + * The filepath of the data. + */ path: string; + + /** + * The binary contents of the file. + */ content(): Promise; + + /** + * The last modified timestamp of the data. + */ + lastModifiedAt?: Date; }; /** @@ -278,4 +315,9 @@ export type SearchResponseFile = { * The binary contents of the file. */ content(): Promise; + + /** + * The last modified timestamp of the data. + */ + lastModifiedAt?: Date; }; From 34041c45c80c2a785248e369238cccd250d5454a Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Tue, 14 Mar 2023 14:57:55 +0100 Subject: [PATCH 2/2] Update sharp-rings-cry.md Signed-off-by: Ben Lambert --- .changeset/sharp-rings-cry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/sharp-rings-cry.md b/.changeset/sharp-rings-cry.md index 9e32a53526..ada106b251 100644 --- a/.changeset/sharp-rings-cry.md +++ b/.changeset/sharp-rings-cry.md @@ -1,5 +1,5 @@ --- -'@backstage/backend-common': minor +'@backstage/backend-common': patch '@backstage/backend-plugin-api': minor ---