Skip to content

Commit

Permalink
fix: copy more than just metadata (#976)
Browse files Browse the repository at this point in the history 
* fix: copy more than just metadata

* chore: address comments

* chore: address comment
  • Loading branch information
@maxakuru
maxakuru authored Jul 18, 2024
1 parent 23f327b commit 70279a2
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 17 deletions.
31 changes: 27 additions & 4 deletions packages/helix-shared-storage/src/storage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ class Bucket {
}
}

/** @type {S3Client} */
get client() {
return this._s3;
}

/** @type {string} */
get bucket() {
return this._bucket;
}
Expand Down Expand Up @@ -312,8 +314,19 @@ class Bucket {

try {
if (opts.addMetadata) {
const meta = await this.metadata(key) ?? {};
input.Metadata = { ...meta, ...opts.addMetadata };
const headers = await this.head(key);
if (!headers) {
const e = new Error('not found');
e.Code = 'NoSuchKey';
throw e;
}
['ContentType', 'ContentEncoding', 'CacheControl', 'ContentDisposition', 'Expires'].forEach((name) => {
if (headers[name]) {
input[name] = headers[name];
}
});
/* c8 ignore next */
input.Metadata = { ...(headers?.Metadata ?? {}), ...opts.addMetadata };
input.MetadataDirective = 'REPLACE';
}
// write to s3 and r2 (mirror) in parallel
Expand Down Expand Up @@ -465,8 +478,18 @@ class Bucket {
};
try {
if (opts.addMetadata) {
const meta = await this.metadata(task.src) ?? {};
input.Metadata = { ...meta, ...opts.addMetadata };
const headers = await this.head(task.src);
if (!headers) {
// this should never happen, since we just listed it
return;
}
['ContentType', 'ContentEncoding', 'CacheControl', 'ContentDisposition', 'Expires'].forEach((name) => {
if (headers[name]) {
input[name] = headers[name];
}
});
/* c8 ignore next */
input.Metadata = { ...(headers?.Metadata ?? {}), ...opts.addMetadata };
input.MetadataDirective = 'REPLACE';
}
// write to s3 and r2 (mirror) in parallel
Expand Down
74 changes: 61 additions & 13 deletions packages/helix-shared-storage/test/storage.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,20 +490,26 @@ describe('Storage test', () => {
.times(10)
.reply((uri) => {
heads.push(uri);
// reject first 2 uris
if (heads.length <= 2) {
// reject first uri, this skips the copy entirely
if (heads.length === 1) {
return [404];
}
return [200, undefined, {
return [200, [], {
expires: 'Thu, 23 Nov 2023 10:35:10 GMT',
'content-type': 'text/plain',
'content-encoding': 'gzip',
'x-amz-meta-x-dont-overwrite': 'foo',
'x-amz-meta-x-last-modified-by': 'anonymous',
}];
})
.put(/.*/)
.times(10)
.times(9)
.reply(function f(uri) {
puts.s3.push(uri);
putsHeaders.s3.push({
expires: this.req.headers.expires,
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -516,10 +522,13 @@ describe('Storage test', () => {
});
nock(`https://helix-code-bus.${CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com`)
.put(/.*/)
.times(10)
.times(9)
.reply(function f(uri) {
puts.r2.push(uri);
putsHeaders.r2.push({
expires: this.req.headers.expires,
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -538,14 +547,16 @@ describe('Storage test', () => {
puts.r2.sort();
heads.sort();

assert.strictEqual(putsHeaders.s3.length, 10);
assert.strictEqual(putsHeaders.r2.length, 10);
assert.strictEqual(putsHeaders.s3.length, 9);
assert.strictEqual(putsHeaders.r2.length, 9);

Object.values(putsHeaders).forEach((s3r2) => {
s3r2.forEach((headers, i) => {
// first 2 returned 404, so no meta existed
s3r2.forEach((headers) => {
assert.deepEqual(headers, {
'x-amz-meta-x-dont-overwrite': i <= 1 ? undefined : 'foo',
expires: 'Thu, 23 Nov 2023 10:35:10 GMT',
'content-type': 'text/plain',
'content-encoding': 'gzip',
'x-amz-meta-x-dont-overwrite': 'foo',
'x-amz-meta-x-last-modified-by': '[email protected]',
'x-amz-metadata-directive': 'REPLACE',
});
Expand All @@ -567,7 +578,7 @@ describe('Storage test', () => {
assert.deepEqual(heads, expectedHeads);

const expectedPuts = [
'/bar/.circleci/config.yml?x-id=CopyObject',
// '/bar/.circleci/config.yml?x-id=CopyObject', // 404, skipped
'/bar/.gitignore?x-id=CopyObject',
'/bar/.vscode/launch.json?x-id=CopyObject',
'/bar/.vscode/settings.json?x-id=CopyObject',
Expand Down Expand Up @@ -614,13 +625,17 @@ describe('Storage test', () => {
const putsHeaders = { s3: undefined, r2: undefined };
nock('https://helix-code-bus.s3.fake.amazonaws.com')
.head('/owner/repo/ref/foo.md')
.reply(200, undefined, {
.reply(200, [], {
'x-amz-meta-x-dont-overwrite': 'foo',
'x-amz-meta-x-last-modified-by': 'anonymous',
'content-type': 'text/plain',
'content-encoding': 'gzip',
})
.put('/owner/repo/ref/foo/bar.md?x-id=CopyObject')
.reply(function f(uri) {
putsHeaders.s3 = {
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -632,6 +647,8 @@ describe('Storage test', () => {
.put('/owner/repo/ref/foo/bar.md?x-id=CopyObject')
.reply(function f(uri) {
putsHeaders.r2 = {
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -652,11 +669,15 @@ describe('Storage test', () => {
assert.deepEqual(puts.r2, expectedPuts);

assert.deepEqual(putsHeaders.s3, {
'content-type': 'text/plain',
'content-encoding': 'gzip',
'x-amz-metadata-directive': 'REPLACE',
'x-amz-meta-x-dont-overwrite': 'foo',
'x-amz-meta-x-last-modified-by': '[email protected]',
});
assert.deepEqual(putsHeaders.r2, {
'content-type': 'text/plain',
'content-encoding': 'gzip',
'x-amz-metadata-directive': 'REPLACE',
'x-amz-meta-x-dont-overwrite': 'foo',
'x-amz-meta-x-last-modified-by': '[email protected]',
Expand All @@ -668,10 +689,13 @@ describe('Storage test', () => {
const putsHeaders = { s3: undefined, r2: undefined };
nock('https://helix-code-bus.s3.fake.amazonaws.com')
.head('/owner/repo/ref/foo.md')
.reply(404)
.reply(200, [], { 'content-type': 'text/plain' })
.put('/owner/repo/ref/foo/bar.md?x-id=CopyObject')
.reply(function f(uri) {
putsHeaders.s3 = {
expires: this.req.headers.expires,
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -683,6 +707,9 @@ describe('Storage test', () => {
.put('/owner/repo/ref/foo/bar.md?x-id=CopyObject')
.reply(function f(uri) {
putsHeaders.r2 = {
expires: this.req.headers.expires,
'content-type': this.req.headers['content-type'],
'content-encoding': this.req.headers['content-encoding'],
'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'],
Expand All @@ -703,17 +730,38 @@ describe('Storage test', () => {
assert.deepEqual(puts.r2, expectedPuts);

assert.deepEqual(putsHeaders.s3, {
expires: undefined,
'content-type': 'text/plain',
'content-encoding': undefined,
'x-amz-metadata-directive': 'REPLACE',
'x-amz-meta-x-dont-overwrite': undefined,
'x-amz-meta-x-last-modified-by': '[email protected]',
});
assert.deepEqual(putsHeaders.r2, {
expires: undefined,
'content-type': 'text/plain',
'content-encoding': undefined,
'x-amz-metadata-directive': 'REPLACE',
'x-amz-meta-x-dont-overwrite': undefined,
'x-amz-meta-x-last-modified-by': '[email protected]',
});
});

it('copy and add metadata should throw error when source not found', async () => {
nock('https://helix-code-bus.s3.fake.amazonaws.com')
.head('/owner/repo/ref/foo.md')
.reply(404);

const bus = storage.codeBus();
await assert.rejects(
bus.copy(
'/owner/repo/ref/foo.md',
'/owner/repo/ref/foo/bar.md',
{ addMetadata: { 'x-last-modified-by': '[email protected]' } },
),
);
});

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')
Expand Down

0 comments on commit 70279a2

Please sign in to comment.