Skip to content

Commit 32ba482

Browse files
bstoppbstopp
authored
bug: Single Bucket Bulk Op Bugs & Tests (#120)
* Add e2e tests and fix bugs related to single bucket. * Fix ref to API token. * Revert previous commit. * Reference correct account * Fix e2e test reference. * Uncomment test case. * Bind to da-collab-stage * Have every test clean itself up. * Turn off timeouts on tests. * Put back the worker url. * Put back the worker url. * Make sure bucket is passed where needed for copy/move operations. * Provide a default response for root request. * Fix issues where bulk copy fails. * Reduce limit
1 parent 41df2d1 commit 32ba482

File tree

18 files changed

+377
-64
lines changed

18 files changed

+377
-64
lines changed

‎.github/workflows/deploy-main.yaml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,18 @@ jobs:
4242
id: deploy-worker
4343
uses: cloudflare/wrangler-action@v3
4444
with:
45+
quiet: true
4546
preCommands: node prepare-deploy.js
4647
command: deploy -e stage -c wrangler-versioned.toml
4748
apiToken: ${{ secrets.CLOUDFLARE_AUTH }}
49+
accountId: ${{secrets.CLOUDFLARE_ACCOUNT}}
4850
- name: Post-Deployment Integration Test
49-
run: WORKER_URL="${{ steps.deploy-worker.outputs.deployment-url }}" npm run test:it
51+
run: WORKER_URL="${{ steps.deploy-worker.outputs.deployment-url }}" npm run test:e2e
52+
env:
53+
WORKER_URL: ${{ secrets.STAGE_WORKER_URL }}
5054
- name: Semantic Release
5155
run: npm run semantic-release
5256
env:
5357
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_AUTH }}
58+
CLOUDFLARE_ACCOUNT_ID: ${{secrets.CLOUDFLARE_ACCOUNT}}
5459
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

‎.github/workflows/install-lint-test.yaml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,15 @@ jobs:
4343
id: deploy-worker
4444
uses: cloudflare/wrangler-action@v3
4545
with:
46+
quiet: true
4647
preCommands: node prepare-deploy.js
4748
command: deploy -e stage -c wrangler-versioned.toml
4849
apiToken: ${{ secrets.CLOUDFLARE_AUTH }}
50+
accountId: ${{secrets.CLOUDFLARE_ACCOUNT}}
4951
- name: Post-Deployment Integration Test
50-
run: WORKER_URL="${{ steps.deploy-worker.outputs.deployment-url }}" npm run test:it
52+
run: WORKER_URL="${{ steps.deploy-worker.outputs.deployment-url }}" npm run test:e2e
53+
env:
54+
WORKER_URL: ${{ secrets.STAGE_WORKER_URL }}
5155
- name: Semantic Release (Dry Run)
5256
run: npm run semantic-release-dry
5357
env:

‎src/handlers/get.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ function getRobots() {
2626

2727
export default async function getHandler({ env, daCtx }) {
2828
const { path } = daCtx;
29+
if (path === '/') return get404();
2930

3031
if (path.startsWith('/favicon.ico')) return get404();
3132
if (path.startsWith('/robots.txt')) return getRobots();

‎src/storage/object/copy.js‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ import { putObjectWithVersion } from '../version/put.js';
2121
import { listCommand } from '../utils/list.js';
2222
import { hasPermission } from '../../utils/auth.js';
2323

24-
const MAX_KEYS = 900;
24+
const MAX_KEYS = 35;
2525

2626
export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) => {
2727
const Key = `${sourceKey.replace(details.source, details.destination)}`;
28-
2928
if (!hasPermission(daCtx, sourceKey, 'read') || !hasPermission(daCtx, Key, 'write')) {
3029
return {
3130
$metadata: {
@@ -69,19 +68,24 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename)
6968
tags: ['METADATA', 'IF-NONE-MATCH'],
7069
},
7170
);
71+
7272
const resp = await client.send(new CopyObjectCommand(input));
7373
return resp;
7474
} catch (e) {
7575
if (e.$metadata.httpStatusCode === 412) {
7676
// Not the happy path - something is at the destination already.
7777
if (!isRename) {
7878
// This is a copy so just put the source into the target to keep the history.
79-
80-
const original = await getObject(env, { org: daCtx.org, key: sourceKey });
79+
const original = await getObject(
80+
env,
81+
{ bucket: daCtx.bucket, org: daCtx.org, key: sourceKey },
82+
);
83+
const body = await original.body.transformToString();
8184
return /* await */ putObjectWithVersion(env, daCtx, {
85+
bucket: daCtx.bucket,
8286
org: daCtx.org,
8387
key: Key,
84-
body: original.body,
88+
body,
8589
contentLength: original.contentLength,
8690
type: original.contentType,
8791
});
@@ -101,14 +105,13 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename)
101105
} finally {
102106
if (Key.endsWith('.html')) {
103107
// Reset the collab cached state for the copied object
104-
await invalidateCollab('syncAdmin', `${daCtx.origin}/source/${daCtx.org}/${Key}`, env);
108+
await invalidateCollab('syncadmin', `${daCtx.origin}/source/${daCtx.org}/${Key}`, env);
105109
}
106110
}
107111
};
108112

109113
export default async function copyObject(env, daCtx, details, isRename) {
110114
if (details.source === details.destination) return { body: '', status: 409 };
111-
112115
const config = getS3Config(env);
113116
const client = new S3Client(config);
114117

@@ -125,7 +128,7 @@ export default async function copyObject(env, daCtx, details, isRename) {
125128
let resp = await listCommand(daCtx, details, client);
126129
sourceKeys = resp.sourceKeys;
127130
if (resp.continuationToken) {
128-
continuationToken = `copy-${details.source}-${details.destination}-${crypto.randomUUID()}`;
131+
continuationToken = `copy-${daCtx.org}-${details.source}-${details.destination}-${crypto.randomUUID()}`;
129132
while (resp.continuationToken) {
130133
resp = await listCommand(daCtx, { continuationToken: resp.continuationToken }, client);
131134
remainingKeys.push(...resp.sourceKeys);

‎src/storage/object/list.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ export default async function listObjects(env, daCtx, maxKeys) {
3737
const command = new ListObjectsV2Command(input);
3838
try {
3939
const resp = await client.send(command);
40-
// console.log(resp);
4140
const body = formatList(resp);
4241
return {
4342
body: JSON.stringify(body),

‎src/storage/object/move.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export default async function moveObject(env, daCtx, details) {
4848
const resp = await client.send(command);
4949

5050
const { Contents = [], NextContinuationToken } = resp;
51-
sourceKeys.push(...Contents.map(({ Key }) => Key));
51+
sourceKeys.push(...Contents.map(({ Key }) => Key.replace(`${daCtx.org}/`, '')));
5252

5353
const movedLoad = sourceKeys
5454
.filter((key) => hasPermission(daCtx, key, 'write'))

‎src/storage/object/put.js‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,6 @@ function buildInput({
3737
};
3838
}
3939

40-
function createBucketIfMissing(client) {
41-
client.middlewareStack.add(
42-
(next) => async (args) => {
43-
// eslint-disable-next-line no-param-reassign
44-
args.request.headers['cf-create-bucket-if-missing'] = 'true';
45-
return next(args);
46-
},
47-
{
48-
step: 'build',
49-
name: 'createIfMissingMiddleware',
50-
tags: ['METADATA', 'CREATE-BUCKET-IF-MISSING'],
51-
},
52-
);
53-
}
54-
5540
/**
5641
* Check to see if the org is in the existing list of orgs
5742
*
@@ -77,9 +62,6 @@ export default async function putObject(env, daCtx, obj) {
7762
// Only allow creating a new bucket for orgs and repos
7863
if (key.split('/').length <= 1) {
7964
await checkOrgIndex(env, org);
80-
81-
// R2 ONLY FEATURE
82-
createBucketIfMissing(client);
8365
}
8466

8567
const inputs = [];

‎src/storage/utils/config.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ export default function getS3Config(env) {
1717
accessKeyId: env.S3_ACCESS_KEY_ID,
1818
secretAccessKey: env.S3_SECRET_ACCESS_KEY,
1919
},
20+
forcePathStyle: true,
2021
};
2122
}

‎src/storage/utils/list.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function buildInput(bucket, org, key) {
7878
return {
7979
Bucket: bucket,
8080
Prefix: `${org}/${key}/`,
81-
MaxKeys: 300,
81+
MaxKeys: 35,
8282
};
8383
}
8484

@@ -106,7 +106,7 @@ export async function listCommand(daCtx, details, s3client) {
106106
const resp = await s3client.send(command);
107107

108108
const { Contents = [], NextContinuationToken } = resp;
109-
sourceKeys.push(...Contents.map(({ Key }) => Key));
109+
sourceKeys.push(...Contents.map(({ Key }) => Key.replace(`${daCtx.org}/`, '')));
110110

111111
return { sourceKeys, continuationToken: NextContinuationToken };
112112
}

‎src/storage/utils/version.js‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,8 @@ import {
1313
S3Client,
1414
} from '@aws-sdk/client-s3';
1515

16-
export function createBucketIfMissing(client) {
17-
client.middlewareStack.add(
18-
(next) => async (args) => {
19-
// eslint-disable-next-line no-param-reassign
20-
args.request.headers['cf-create-bucket-if-missing'] = 'true';
21-
return next(args);
22-
},
23-
{
24-
step: 'build',
25-
name: 'createIfMissingMiddleware',
26-
tags: ['METADATA', 'CREATE-BUCKET-IF-MISSING'],
27-
},
28-
);
29-
return client;
30-
}
31-
3216
export function ifNoneMatch(config) {
3317
const client = new S3Client(config);
34-
createBucketIfMissing(client);
3518
client.middlewareStack.add(
3619
(next) => async (args) => {
3720
// eslint-disable-next-line no-param-reassign
@@ -49,7 +32,6 @@ export function ifNoneMatch(config) {
4932

5033
export function ifMatch(config, match) {
5134
const client = new S3Client(config);
52-
createBucketIfMissing(client);
5335
client.middlewareStack.add(
5436
(next) => async (args) => {
5537
// eslint-disable-next-line no-param-reassign

0 commit comments

Comments
 (0)