From cf3730b635b388f3482a06310503b28c50bafbd3 Mon Sep 17 00:00:00 2001 From: Max Edell Date: Mon, 15 Jul 2024 07:58:45 -0700 Subject: [PATCH 1/4] fix(storage): add copy options for addMetadata --- .../helix-shared-storage/src/storage.d.ts | 52 +++++++++++++------ packages/helix-shared-storage/src/storage.js | 38 ++++++++------ 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/packages/helix-shared-storage/src/storage.d.ts b/packages/helix-shared-storage/src/storage.d.ts index e28aa2f6..a2e17cef 100644 --- a/packages/helix-shared-storage/src/storage.d.ts +++ b/packages/helix-shared-storage/src/storage.d.ts @@ -12,12 +12,32 @@ import { S3Client } from "@aws-sdk/client-s3"; + +export interface ObjectInfo { + key: string; + /** the path to the object, w/o the prefix */ + path: string; + lastModified: string; + contentLength: number; + contentType: string; +} + +/** + * @returns {boolean} {@code true} if the object is accepted + */ +export type ObjectFilter = (info: ObjectInfo) => boolean; + +export interface CopyOptions { + /** metadata to merge with existing metadata */ + addMetadata?: Record; +} + export declare interface Bucket { - get client():S3Client; + get client(): S3Client; - get bucket():string; + get bucket(): string; - get log():Console; + get log(): Console; get(key: string, meta?: object): Promise; @@ -51,7 +71,7 @@ export declare interface Bucket { * @param {boolean} [compress = true] * @returns result obtained from S3 */ - put(path: string, body: Buffer, contentType?: string, meta?: object, compress?: bool): Promise; + put(path: string, body: Buffer, contentType?: string, meta?: object, compress?: boolean): Promise; /** * Updates the metadata @@ -67,9 +87,10 @@ export declare interface Bucket { * * @param {string} src source key * @param {string} dst destination key + * @param {CopyOptions} [opts] * @returns result obtained from S3 */ - copy(src: string, dst: string): Promise; + copy(src: string, dst: string, opts?: CopyOptions): Promise; /** * Remove object(s) @@ -93,9 +114,10 @@ export declare interface Bucket { * @param {string} src Source prefix * @param {string} dst Destination prefix * @param {ObjectFilter} filter Filter function + * @param {CopyOptions} [opts] * @returns {Promise<*[]>} */ - copyDeep(src: string, dst: string, filter?: function): Promise; + copyDeep(src: string, dst: string, filter?: ObjectFilter, opts?: CopyOptions): Promise; rmdir(src: string): Promise; } @@ -103,40 +125,40 @@ export declare interface Bucket { /** * The Helix Storage provides a factory for simplified bucket operations to S3 and R2 */ -export class HelixStorage { - static fromContext(context:AdminContext):HelixStorage; +export declare class HelixStorage { + static fromContext(context: AdminContext): HelixStorage; - s3():S3Client; + s3(): S3Client; /** * creates a bucket instance that allows to perform storage related operations. * @param bucketId * @returns {Bucket} */ - bucket(bucketId:string):Bucket;; + bucket(bucketId: string): Bucket;; /** * @returns {Bucket} */ - contentBus():Bucket; + contentBus(): Bucket; /** * @returns {Bucket} */ - codeBus():Bucket; + codeBus(): Bucket; /** * @returns {Bucket} */ - mediaBus():Bucket; + mediaBus(): Bucket; /** * @returns {Bucket} */ - configBus():Bucket; + configBus(): Bucket; /** * Close this storage. Destroys the S3 client used. */ - close() + close(): void; } diff --git a/packages/helix-shared-storage/src/storage.js b/packages/helix-shared-storage/src/storage.js index 127f220e..f4754c6e 100644 --- a/packages/helix-shared-storage/src/storage.js +++ b/packages/helix-shared-storage/src/storage.js @@ -35,21 +35,11 @@ const gunzip = promisify(zlib.gunzip); /** * @typedef {import('@aws-sdk/client-s3').CommandInput} CommandInput - */ - -/** - * @typedef ObjectInfo - * @property {string} key - * @property {string} path the path to the object, w/o the prefix - * @property {string} lastModified - * @property {number} contentLength - * @property {string} contentType - */ - -/** - * @callback ObjectFilter - * @param {ObjectInfo} info of the object to filter - * @returns {boolean} {@code true} if the object is accepted + * @typedef {import('./storage.d').Bucket} BucketType + * @typedef {import('./storage.d').HelixStorage} HelixStorageType + * @typedef {import('./storage.d').ObjectInfo} ObjectInfo + * @typedef {import('./storage.d').ObjectFilter} ObjectFilter + * @typedef {import('./storage.d').CopyOptions} CopyOptions */ /** @@ -92,6 +82,7 @@ function sanitizeKey(keyOrPath) { /** * Bucket class + * @implements {BucketType} */ class Bucket { constructor(opts) { @@ -308,9 +299,10 @@ class Bucket { * * @param {string} src source key * @param {string} dst destination key + * @param {CopyOptions} [opts] * @returns result obtained from S3 */ - async copy(src, dst) { + async copy(src, dst, opts = {}) { const input = { Bucket: this.bucket, CopySource: `${this.bucket}/${sanitizeKey(src)}`, @@ -318,6 +310,11 @@ class Bucket { }; try { + if (opts.addMetadata) { + const meta = await this.metadata(src); + input.Metadata = { ...meta, ...opts.addMetadata }; + input.MetadataDirective = 'REPLACE'; + } // write to s3 and r2 (mirror) in parallel await this.sendToS3andR2(CopyObjectCommand, input); this.log.info(`object copied from ${input.CopySource} to: ${input.Bucket}/${input.Key}`); @@ -432,9 +429,10 @@ class Bucket { * @param {string} src Source prefix * @param {string} dst Destination prefix * @param {ObjectFilter} filter Filter function + * @param {CopyOptions} [opts={}] * @returns {Promise<*[]>} */ - async copyDeep(src, dst, filter = () => true) { + async copyDeep(src, dst, filter = () => true, opts = {}) { const { log } = this; const tasks = []; const Prefix = sanitizeKey(src); @@ -465,6 +463,11 @@ class Bucket { Key: task.dst, }; try { + if (opts.addMetadata) { + const meta = await this.metadata(src); + input.Metadata = { ...meta, ...opts.addMetadata }; + input.MetadataDirective = 'REPLACE'; + } // write to s3 and r2 (mirror) in parallel await this.sendToS3andR2(CopyObjectCommand, input); changes.push(task); @@ -510,6 +513,7 @@ class Bucket { /** * The Helix Storage provides a factory for simplified bucket operations to S3 and R2 + * @implements {HelixStorageType} */ export class HelixStorage { static fromContext(context) { From 6f4afe478a789801759f4227704542677f2cf2a3 Mon Sep 17 00:00:00 2001 From: Max Edell Date: Mon, 15 Jul 2024 08:43:16 -0700 Subject: [PATCH 2/4] chore: fix tests --- packages/helix-shared-storage/src/storage.js | 7 +- .../helix-shared-storage/test/storage.test.js | 159 ++++++++++++++++++ 2 files changed, 163 insertions(+), 3 deletions(-) diff --git a/packages/helix-shared-storage/src/storage.js b/packages/helix-shared-storage/src/storage.js index f4754c6e..1ded38e6 100644 --- a/packages/helix-shared-storage/src/storage.js +++ b/packages/helix-shared-storage/src/storage.js @@ -303,15 +303,16 @@ class Bucket { * @returns result obtained from S3 */ async copy(src, dst, opts = {}) { + const key = sanitizeKey(src); const input = { Bucket: this.bucket, - CopySource: `${this.bucket}/${sanitizeKey(src)}`, + CopySource: `${this.bucket}/${key}`, Key: sanitizeKey(dst), }; try { if (opts.addMetadata) { - const meta = await this.metadata(src); + const meta = await this.metadata(key); input.Metadata = { ...meta, ...opts.addMetadata }; input.MetadataDirective = 'REPLACE'; } @@ -464,7 +465,7 @@ class Bucket { }; try { if (opts.addMetadata) { - const meta = await this.metadata(src); + const meta = await this.metadata(task.src); input.Metadata = { ...meta, ...opts.addMetadata }; input.MetadataDirective = 'REPLACE'; } diff --git a/packages/helix-shared-storage/test/storage.test.js b/packages/helix-shared-storage/test/storage.test.js index 548c7caa..1f9c745e 100644 --- a/packages/helix-shared-storage/test/storage.test.js +++ b/packages/helix-shared-storage/test/storage.test.js @@ -476,6 +476,111 @@ describe('Storage test', () => { assert.deepEqual(puts.r2, expectedPuts); }); + it('can copy objects and add metadata', async () => { + const listReply = JSON.parse(await fs.readFile(path.resolve(__testdir, 'fixtures', 'list-reply-copy.json'), 'utf-8')); + const puts = { s3: [], r2: [] }; + const putsHeaders = { s3: [], r2: [] }; + const heads = []; + nock('https://helix-code-bus.s3.fake.amazonaws.com') + .get('/?list-type=2&prefix=owner%2Frepo%2Fref%2F') + .reply(200, listReply[0]) + .get('/?continuation-token=1%2Fs4dr7BSKNScrN4njX9%2BCpBNimYkuEzMWg3niTSAPMdculBmycyUPM6kv0xi46j4hdc1lFPkE%2FICI8TxG%2BVNV9Hh91Ou0hqeBYzqTRzSBSs%3D&list-type=2&prefix=owner%2Frepo%2Fref%2F') + .reply(200, listReply[1]) + .head(/.*/) + .times(10) + .reply((uri) => { + heads.push(uri); + // reject first uri + if (puts.s3.length === 1) { + return [404]; + } + return [200, undefined, { + 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-last-modified-by': 'anonymous', + }]; + }) + .put(/.*/) + .times(10) + .reply(function f(uri) { + puts.s3.push(uri); + putsHeaders.s3.push({ + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }); + // reject first uri + if (puts.s3.length === 1) { + return [404]; + } + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + nock(`https://helix-code-bus.${CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com`) + .put(/.*/) + .times(10) + .reply(function f(uri) { + puts.r2.push(uri); + putsHeaders.r2.push({ + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }); + // reject first uri + if (puts.s3.length === 1) { + return [404]; + } + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + + const bus = storage.codeBus(); + await bus.copyDeep('/owner/repo/ref/', '/bar/', undefined, { addMetadata: { 'x-last-modified-by': 'foo@example.com' } }); + + puts.s3.sort(); + puts.r2.sort(); + heads.sort(); + + assert.strictEqual(putsHeaders.s3.length, 10); + assert.strictEqual(putsHeaders.r2.length, 10); + + Object.values(putsHeaders).forEach((s3r2) => { + s3r2.forEach((headers) => { + assert.deepEqual(headers, { + 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-last-modified-by': 'foo@example.com', + 'x-amz-metadata-directive': 'REPLACE', + }); + }); + }); + + const expectedHeads = [ + '/owner/repo/ref/.circleci/config.yml', + '/owner/repo/ref/.gitignore', + '/owner/repo/ref/.vscode/launch.json', + '/owner/repo/ref/.vscode/settings.json', + '/owner/repo/ref/README.md', + '/owner/repo/ref/helix_logo.png', + '/owner/repo/ref/htdocs/favicon.ico', + '/owner/repo/ref/htdocs/style.css', + '/owner/repo/ref/index.md', + '/owner/repo/ref/src/html.pre.js', + ]; + assert.deepEqual(heads, expectedHeads); + + const expectedPuts = [ + '/bar/.circleci/config.yml?x-id=CopyObject', + '/bar/.gitignore?x-id=CopyObject', + '/bar/.vscode/launch.json?x-id=CopyObject', + '/bar/.vscode/settings.json?x-id=CopyObject', + '/bar/README.md?x-id=CopyObject', + '/bar/helix_logo.png?x-id=CopyObject', + '/bar/htdocs/favicon.ico?x-id=CopyObject', + '/bar/htdocs/style.css?x-id=CopyObject', + '/bar/index.md?x-id=CopyObject', + '/bar/src/html.pre.js?x-id=CopyObject', + ]; + assert.deepEqual(puts.s3, expectedPuts); + assert.deepEqual(puts.r2, expectedPuts); + }); + it('can copy object (non deep)', async () => { const puts = { s3: [], r2: [] }; nock('https://helix-code-bus.s3.fake.amazonaws.com') @@ -503,6 +608,60 @@ describe('Storage test', () => { assert.deepEqual(puts.r2, expectedPuts); }); + it('can copy object, and add metadata (non deep)', async () => { + const puts = { s3: [], r2: [] }; + const putsHeaders = { s3: undefined, r2: undefined }; + nock('https://helix-code-bus.s3.fake.amazonaws.com') + .head('/owner/repo/ref/foo.md') + .reply(200, undefined, { + 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-last-modified-by': 'anonymous', + }) + .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject') + .reply(function f(uri) { + putsHeaders.s3 = { + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }; + puts.s3.push(uri); + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + nock(`https://helix-code-bus.${CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com`) + .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject') + .reply(function f(uri) { + putsHeaders.r2 = { + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }; + puts.r2.push(uri); + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + + const bus = storage.codeBus(); + await bus.copy('/owner/repo/ref/foo.md', '/owner/repo/ref/foo/bar.md', { addMetadata: { 'x-last-modified-by': 'foo@example.com' } }); + + puts.s3.sort(); + puts.r2.sort(); + const expectedPuts = [ + '/owner/repo/ref/foo/bar.md?x-id=CopyObject', + ]; + assert.deepEqual(puts.s3, expectedPuts); + assert.deepEqual(puts.r2, expectedPuts); + + assert.deepEqual(putsHeaders.s3, { + 'x-amz-metadata-directive': 'REPLACE', + 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-last-modified-by': 'foo@example.com', + }); + assert.deepEqual(putsHeaders.r2, { + 'x-amz-metadata-directive': 'REPLACE', + 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-last-modified-by': 'foo@example.com', + }); + }); + it('can copy object can fail (non deep)', async () => { nock('https://helix-code-bus.s3.fake.amazonaws.com') .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject') From 5fb2264a9b3fd66e2d2a51fa09f38003814234f6 Mon Sep 17 00:00:00 2001 From: Max Edell Date: Tue, 16 Jul 2024 07:46:21 -0700 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Tobias Bocanegra --- packages/helix-shared-storage/src/storage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/helix-shared-storage/src/storage.js b/packages/helix-shared-storage/src/storage.js index 1ded38e6..d98da716 100644 --- a/packages/helix-shared-storage/src/storage.js +++ b/packages/helix-shared-storage/src/storage.js @@ -312,7 +312,7 @@ class Bucket { try { if (opts.addMetadata) { - const meta = await this.metadata(key); + const meta = await this.metadata(key) ?? {}; input.Metadata = { ...meta, ...opts.addMetadata }; input.MetadataDirective = 'REPLACE'; } @@ -465,7 +465,7 @@ class Bucket { }; try { if (opts.addMetadata) { - const meta = await this.metadata(task.src); + const meta = await this.metadata(task.src) || {}; input.Metadata = { ...meta, ...opts.addMetadata }; input.MetadataDirective = 'REPLACE'; } From 9916c3a609ca97e767580f1501e8dfbf12a19af0 Mon Sep 17 00:00:00 2001 From: Max Edell Date: Tue, 16 Jul 2024 08:10:46 -0700 Subject: [PATCH 4/4] chore: tests --- packages/helix-shared-storage/src/storage.js | 2 +- .../helix-shared-storage/test/storage.test.js | 62 +++++++++++++++++-- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/helix-shared-storage/src/storage.js b/packages/helix-shared-storage/src/storage.js index d98da716..510dc1fd 100644 --- a/packages/helix-shared-storage/src/storage.js +++ b/packages/helix-shared-storage/src/storage.js @@ -465,7 +465,7 @@ class Bucket { }; try { if (opts.addMetadata) { - const meta = await this.metadata(task.src) || {}; + const meta = await this.metadata(task.src) ?? {}; input.Metadata = { ...meta, ...opts.addMetadata }; input.MetadataDirective = 'REPLACE'; } diff --git a/packages/helix-shared-storage/test/storage.test.js b/packages/helix-shared-storage/test/storage.test.js index 1f9c745e..25059f8e 100644 --- a/packages/helix-shared-storage/test/storage.test.js +++ b/packages/helix-shared-storage/test/storage.test.js @@ -490,8 +490,8 @@ describe('Storage test', () => { .times(10) .reply((uri) => { heads.push(uri); - // reject first uri - if (puts.s3.length === 1) { + // reject first 2 uris + if (heads.length <= 2) { return [404]; } return [200, undefined, { @@ -525,7 +525,7 @@ describe('Storage test', () => { 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], }); // reject first uri - if (puts.s3.length === 1) { + if (puts.r2.length === 1) { return [404]; } return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; @@ -542,9 +542,10 @@ describe('Storage test', () => { assert.strictEqual(putsHeaders.r2.length, 10); Object.values(putsHeaders).forEach((s3r2) => { - s3r2.forEach((headers) => { + s3r2.forEach((headers, i) => { + // first 2 returned 404, so no meta existed assert.deepEqual(headers, { - 'x-amz-meta-x-dont-overwrite': 'foo', + 'x-amz-meta-x-dont-overwrite': i <= 1 ? undefined : 'foo', 'x-amz-meta-x-last-modified-by': 'foo@example.com', 'x-amz-metadata-directive': 'REPLACE', }); @@ -662,6 +663,57 @@ describe('Storage test', () => { }); }); + it('can copy object, and add metadata (non deep, no metadata already existed)', async () => { + const puts = { s3: [], r2: [] }; + const putsHeaders = { s3: undefined, r2: undefined }; + nock('https://helix-code-bus.s3.fake.amazonaws.com') + .head('/owner/repo/ref/foo.md') + .reply(404) + .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject') + .reply(function f(uri) { + putsHeaders.s3 = { + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }; + puts.s3.push(uri); + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + nock(`https://helix-code-bus.${CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com`) + .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject') + .reply(function f(uri) { + putsHeaders.r2 = { + 'x-amz-metadata-directive': this.req.headers['x-amz-metadata-directive'], + 'x-amz-meta-x-dont-overwrite': this.req.headers['x-amz-meta-x-dont-overwrite'], + 'x-amz-meta-x-last-modified-by': this.req.headers['x-amz-meta-x-last-modified-by'], + }; + puts.r2.push(uri); + return [200, '\n2021-05-05T08:37:23.000Z"f278c0035a9b4398629613a33abe6451"']; + }); + + const bus = storage.codeBus(); + await bus.copy('/owner/repo/ref/foo.md', '/owner/repo/ref/foo/bar.md', { addMetadata: { 'x-last-modified-by': 'foo@example.com' } }); + + puts.s3.sort(); + puts.r2.sort(); + const expectedPuts = [ + '/owner/repo/ref/foo/bar.md?x-id=CopyObject', + ]; + assert.deepEqual(puts.s3, expectedPuts); + assert.deepEqual(puts.r2, expectedPuts); + + assert.deepEqual(putsHeaders.s3, { + 'x-amz-metadata-directive': 'REPLACE', + 'x-amz-meta-x-dont-overwrite': undefined, + 'x-amz-meta-x-last-modified-by': 'foo@example.com', + }); + assert.deepEqual(putsHeaders.r2, { + 'x-amz-metadata-directive': 'REPLACE', + 'x-amz-meta-x-dont-overwrite': undefined, + 'x-amz-meta-x-last-modified-by': 'foo@example.com', + }); + }); + it('can copy object can fail (non deep)', async () => { nock('https://helix-code-bus.s3.fake.amazonaws.com') .put('/owner/repo/ref/foo/bar.md?x-id=CopyObject')