Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single Bucket - Delete object #67

Merged
merged 6 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ module.exports = {
extends: '@adobe/helix',
rules: {
'no-await-in-loop': 0,
'max-len': ["error", { "code": 200 }],
},
};
2 changes: 0 additions & 2 deletions src/routes/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import putObject from '../storage/object/put.js';
import deleteObjects from '../storage/object/delete.js';

import putHelper from '../helpers/source.js';
import { postObjectVersion } from '../storage/version/put.js';

async function invalidateCollab(api, url, env) {
const invPath = `/api/v1/${api}?doc=${url}`;
Expand All @@ -25,7 +24,6 @@ async function invalidateCollab(api, url, env) {
}

export async function deleteSource({ req, env, daCtx }) {
await postObjectVersion(req, env, daCtx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this line here is to save a version of the file before deletion, so that it could be restored if the delete was accidental. I think it would be good to keep that.

/cc @karlpauls

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, there was a discussion about this (OOB). The problem was it didn't consider directories correctly. IMO it would make sense to keep it unless it causes big performance concerns but it would need to be only done for files and you'd have to evaluate the impact for large trees I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was discussed on Slack w/ @auniverseaway , and the decision was to remove it.

const resp = await deleteObjects(env, daCtx);

if (resp.status === 204) {
Expand Down
67 changes: 10 additions & 57 deletions src/storage/object/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,68 +9,21 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import {
S3Client,
DeleteObjectCommand,
ListObjectsV2Command,
} from '@aws-sdk/client-s3';
import { getSignedUrl } from '@aws-sdk/s3-request-presigner';

import getS3Config from '../utils/config.js';

function buildInput(org, key) {
return {
Bucket: `${org}-content`,
Prefix: `${key}/`,
};
}

export async function deleteObject(client, org, Key) {
try {
const delCommand = new DeleteObjectCommand({ Bucket: `${org}-content`, Key });
const url = await getSignedUrl(client, delCommand, { expiresIn: 3600 });
return fetch(url, { method: 'DELETE' });
} catch (e) {
// eslint-disable-next-line no-console
console.log(`There was an error deleting ${Key}.`);
return e;
}
}

export default async function deleteObjects(env, daCtx) {
const config = getS3Config(env);
const client = new S3Client(config);
const input = buildInput(daCtx.org, daCtx.key);

let ContinuationToken;

const fullKey = `${daCtx.org}/${daCtx.key}`;
const prefix = `${fullKey}/`;
// The input prefix has a forward slash to prevent (drafts + drafts-new, etc.).
// Which means the list will only pickup children. This adds to the initial list.
const sourceKeys = [daCtx.key, `${daCtx.key}.props`];
const sourceKeys = [fullKey, `${fullKey}.props`];

let truncated = false;
do {
try {
const command = new ListObjectsV2Command({ ...input, ContinuationToken });
const resp = await client.send(command);

const { Contents = [], NextContinuationToken } = resp;
sourceKeys.push(...Contents.map(({ Key }) => Key));

await Promise.all(
new Array(1).fill(null).map(async () => {
while (sourceKeys.length) {
await deleteObject(client, daCtx.org, sourceKeys.pop());
}
}),
);

ContinuationToken = NextContinuationToken;
} catch (e) {
// eslint-disable-next-line no-console
console.log(e);
return { body: '', status: 404 };
}
} while (ContinuationToken);

const r2objects = await env.DA_CONTENT.list({ prefix, limit: 500 });
const { objects } = r2objects;
truncated = r2objects.truncated;
sourceKeys.push(...objects.map(({ key }) => key));
await env.DA_CONTENT.delete(sourceKeys);
} while (truncated);
return { body: null, status: 204 };
}
4 changes: 2 additions & 2 deletions src/storage/object/move.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
} from '@aws-sdk/client-s3';

import getS3Config from '../utils/config.js';
import { deleteObject } from './delete.js';
import deleteObjects from './delete.js';

Check warning on line 19 in src/storage/object/move.js

View check run for this annotation

Codecov / codecov/patch

src/storage/object/move.js#L19

Added line #L19 was not covered by tests

function buildInput(org, key) {
return {
Expand Down Expand Up @@ -71,7 +71,7 @@
const copied = await copyFile(client, daCtx.org, key, details);
// Only delete the source if the file was successfully copied
if (copied.$metadata.httpStatusCode === 200) {
const deleted = await deleteObject(client, daCtx.org, key);
const deleted = await deleteObjects(client, daCtx.org, key);

Check warning on line 74 in src/storage/object/move.js

View check run for this annotation

Codecov / codecov/patch

src/storage/object/move.js#L74

Added line #L74 was not covered by tests
result.status = deleted.status === 204 ? 204 : deleted.status;
} else {
result.status = copied.$metadata.httpStatusCode;
Expand Down
4 changes: 2 additions & 2 deletions src/storage/version/put.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export async function putObjectWithVersion(env, daCtx, update, body) {
const Version = current.metadata?.version || crypto.randomUUID();
const Users = JSON.stringify(daCtx.users);
const input = buildInput(update);
const Timestamp = `${Date.now()}`;
const Timestamp = Date.now();
const Path = update.key;

if (current.status === 404) {
Expand All @@ -100,7 +100,7 @@ export async function putObjectWithVersion(env, daCtx, update, body) {

// Store the body if preparsingstore is not defined, so a once-off store
const storeBody = !body && pps === '0';
const Preparsingstore = storeBody ? Timestamp : pps;
const Preparsingstore = storeBody ? `${Timestamp}` : pps;
const Label = storeBody ? 'Collab Parse' : update.label;

const versionResp = await putVersion(config, {
Expand Down
42 changes: 42 additions & 0 deletions test/it/delete.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2024 Adobe. All rights reserved.
* This file is licensed to you 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 REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import assert from 'node:assert'
import { destroyMiniflare, getMiniflare } from '../mocks/miniflare.js';
import worker from '../../src/index.js';

describe('DELETE HTTP Requests', () => {
let mf;
let env;
beforeEach(async () => {
mf = await getMiniflare();
env = await mf.getBindings();
});
afterEach(async () => {
await destroyMiniflare(mf);
});

describe ('/source', async () => {
it('handles non-existing file', async () => {
const req = new Request('https://admin.da.live/source/wknd/does-not-exist', { method: 'DELETE' });
const resp = await worker.fetch(req, env);
assert.strictEqual(resp.status, 204);
});

it('handles existing file', async () => {
const req = new Request('https://admin.da.live/source/wknd/index.html', { method: 'DELETE' });
const resp = await worker.fetch(req, env);
assert.strictEqual(resp.status, 204);
const obj = await env.DA_CONTENT.get('wknd/index.html');
assert.ifError(obj);
});
});
})
2 changes: 1 addition & 1 deletion test/it/get.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { destroyMiniflare, getMiniflare } from '../mocks/miniflare.js';
import worker from '../../src/index.js';
import { SignJWT } from 'jose';

describe('GET HTTP Requests', async () => {
describe('GET HTTP Requests', () => {
let mf;
let env;
beforeEach(async () => {
Expand Down
2 changes: 0 additions & 2 deletions test/it/head.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import assert from 'node:assert';
import { destroyMiniflare, getMiniflare } from '../mocks/miniflare.js';
import worker from '../../src/index.js';
import { SignJWT } from 'jose';


describe('HEAD HTTP Requests', async () => {
let mf;
Expand Down
18 changes: 10 additions & 8 deletions test/mocks/miniflare.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,27 @@ export async function getMiniflare() {
}
}
`,
serviceBindings: {
dacollab() {
return {
fetch: () => { /* no-op fetch */ },
};
},
},
kvNamespaces: { DA_AUTH: 'DA_AUTH', DA_CONFIG: 'DA_CONFIG' },
r2Buckets: { DA_CONTENT: 'DA_CONTENT' },
bindings: { DA_BUCKET_NAME: 'da-content' },
});
const env = await mf.getBindings();
for (let name of orgs) {
const auth = config[name];
const content = `Hello ${name}!`;
await env.DA_CONTENT.put(
`${name}/index.html`,
`Hello ${name}!`,
content,
{
httpMetadata: { contentType: 'text/html' },
customMetadata: {
id: '123',
version: '123',
users: `[{"email":"user@${name}.com"}]`,
timestamp: '1720723249932',
path: `${name}/index.html`
}
customMetadata: { id: '123', version: '123', users: `[{"email":"user@${name}.com"}]`, timestamp: '1720723249932', path: `${name}/index.html` }
}
);
if (auth) await env.DA_CONFIG.put(name, JSON.stringify(auth));
Expand Down
14 changes: 1 addition & 13 deletions test/routes/source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,6 @@ describe('Source Route', () => {
const env = { dacollab };
const daCtx = {};

const postObjVerCalled = [];
const postObjVerResp = async (r, e, c) => {
if (r === req && e === env && c === daCtx) {
postObjVerCalled.push('postObjectVersion');
return {status: 201};
}
};

const deleteCalled = [];
const deleteResp = async (e, c) => {
if (e === env && c === daCtx) {
Expand All @@ -178,16 +170,12 @@ describe('Source Route', () => {
'../../src/storage/object/delete.js': {
default: deleteResp
},
'../../src/storage/version/put.js': {
postObjectVersion: postObjVerResp
}
}
);
const resp = await deleteSource({req, env, daCtx});
assert.equal(204, resp.status);
assert.deepStrictEqual(['postObjectVersion'], postObjVerCalled);
assert.deepStrictEqual(deleteCalled, ['deleteObject']);
assert.deepStrictEqual(daCalled,
['https://localhost/api/v1/deleteadmin?doc=http://somehost.com/somedoc.html']);
});
});
});
75 changes: 75 additions & 0 deletions test/storage/object/delete.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright 2024 Adobe. All rights reserved.
* This file is licensed to you 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 REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import assert from 'node:assert';
import { destroyMiniflare, getMiniflare } from '../../mocks/miniflare.js';
import listObjects from '../../../src/storage/object/list.js';
import deleteObjects from '../../../src/storage/object/delete.js';

describe('delete object(s)', () => {
let mf;
let env;
beforeEach(async () => {
mf = await getMiniflare();
env = await mf.getBindings();
});
afterEach(async () => {
await destroyMiniflare(mf);
});

it('handles no object', async () => {
const daCtx = { users: [{email: '[email protected]'}], org: 'geometrixx', key: 'does-not-exist.html' };
const resp = await deleteObjects(env, daCtx);
assert.strictEqual(resp.status, 204);
});

it('deletes a single object', async () => {
const daCtx = { users: [{email: '[email protected]'}], org: 'geometrixx', key: 'outdoors/index.html' };

await env.DA_CONTENT.put('geometrixx/shapes.props', '{"key":"value"}');
await env.DA_CONTENT.put('geometrixx/we-retail.props', '{"key":"value"}');
await env.DA_CONTENT.put('geometrixx/outdoors/index.html', 'Hello');

const resp = await deleteObjects(env, daCtx);
assert.strictEqual(resp.status, 204);
const head = await env.DA_CONTENT.head('geometrixx/outdoors/index.html');
assert.ifError(head);
});

it('deletes a folder', async () => {
await env.DA_CONTENT.put('geometrixx/outdoors/index.html', 'Hello!');
await env.DA_CONTENT.put('geometrixx/outdoors/logo.jpg', '1234');
await env.DA_CONTENT.put('geometrixx/outdoors/hero.jpg', '1234');
await env.DA_CONTENT.put('geometrixx/outdoors/coats/coats.props', '{"key": "value"}');
await env.DA_CONTENT.put('geometrixx/outdoors/pants/pants.props', '{"key": "value"}');
await env.DA_CONTENT.put('geometrixx/outdoors/hats/hats.props', '{"key": "value"}');

const daCtx = { users: [{email: '[email protected]'}], org: 'geometrixx', key: 'outdoors' };
const resp = await deleteObjects(env, daCtx);
assert.strictEqual(resp.status, 204);
const list = await env.DA_CONTENT.list({ prefix: 'geometrixx/outdoors' });
assert.strictEqual(list.objects.length, 0);
});

it('deletes a folder (truncated list === true)', async function() {
this.timeout(10000);
await env.DA_CONTENT.put('geometrixx/outdoors/index.html', 'Hello!');
for (let i = 0; i < 1000; i++) {
await env.DA_CONTENT.put(`geometrixx/outdoors/${i}/${i}.html`, 'Content');
}

const daCtx = { users: [{email: '[email protected]'}], org: 'geometrixx', key: 'outdoors' };
const resp = await deleteObjects(env, daCtx);
assert.strictEqual(resp.status, 204);
const list = await env.DA_CONTENT.list({ prefix: 'geometrixx/outdoors' });
assert.strictEqual(list.objects.length, 0);
});
});