Skip to content

Commit 7b6b0c2

Browse files
Change id parameter to name for both bucket lifecycle and r2 bucket lock commands (#8367)
* Change id parameter to name for both bucket lifecycle and r2 bucket lock commands * Added tests for id alias * Apply suggestions from code review Co-authored-by: Edmund Hung <[email protected]> --------- Co-authored-by: Edmund Hung <[email protected]>
1 parent 5875adb commit 7b6b0c2

File tree

5 files changed

+142
-63
lines changed

5 files changed

+142
-63
lines changed

.changeset/whole-dots-smile.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": minor
3+
---
4+
5+
fix: for both the r2 bucket lifecycle and r2 bucket lock commands --id parameter deprecated in favor of --name

packages/wrangler/src/__tests__/r2.test.ts

Lines changed: 87 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2016,7 +2016,7 @@ describe("r2", () => {
20162016
await runWrangler(`r2 bucket lifecycle list ${bucketName}`);
20172017
expect(std.out).toMatchInlineSnapshot(`
20182018
"Listing lifecycle rules for bucket 'my-bucket'...
2019-
id: rule-1
2019+
name: rule-1
20202020
enabled: Yes
20212021
prefix: images/
20222022
action: Expire objects after 30 days"
@@ -2074,15 +2074,15 @@ describe("r2", () => {
20742074
)
20752075
);
20762076
await runWrangler(
2077-
`r2 bucket lifecycle add ${bucketName} --id ${ruleId} --prefix ${prefix} --expire-days ${conditionValue}`
2077+
`r2 bucket lifecycle add ${bucketName} --name ${ruleId} --prefix ${prefix} --expire-days ${conditionValue}`
20782078
);
20792079
expect(std.out).toMatchInlineSnapshot(`
20802080
"Adding lifecycle rule 'my-rule' to bucket 'my-bucket'...
20812081
✨ Added lifecycle rule 'my-rule' to bucket 'my-bucket'."
20822082
`);
20832083
});
20842084

2085-
it("it should add a date lifecycle rule using command-line arguments", async () => {
2085+
it("it should add a date lifecycle rule using command-line arguments and id alias", async () => {
20862086
const bucketName = "my-bucket";
20872087
const ruleId = "my-rule";
20882088
const prefix = "images/";
@@ -2142,6 +2142,52 @@ describe("r2", () => {
21422142
});
21432143
describe("remove", () => {
21442144
it("should remove a lifecycle rule as expected", async () => {
2145+
const bucketName = "my-bucket";
2146+
const ruleId = "my-rule";
2147+
const lifecycleRules = {
2148+
rules: [
2149+
{
2150+
id: ruleId,
2151+
enabled: true,
2152+
conditions: {},
2153+
},
2154+
],
2155+
};
2156+
msw.use(
2157+
http.get(
2158+
"*/accounts/:accountId/r2/buckets/:bucketName/lifecycle",
2159+
async ({ params }) => {
2160+
const { accountId, bucketName: bucketParam } = params;
2161+
expect(accountId).toEqual("some-account-id");
2162+
expect(bucketParam).toEqual(bucketName);
2163+
return HttpResponse.json(createFetchResult(lifecycleRules));
2164+
},
2165+
{ once: true }
2166+
),
2167+
http.put(
2168+
"*/accounts/:accountId/r2/buckets/:bucketName/lifecycle",
2169+
async ({ request, params }) => {
2170+
const { accountId, bucketName: bucketParam } = params;
2171+
expect(accountId).toEqual("some-account-id");
2172+
expect(bucketName).toEqual(bucketParam);
2173+
const requestBody = await request.json();
2174+
expect(requestBody).toEqual({
2175+
rules: [],
2176+
});
2177+
return HttpResponse.json(createFetchResult({}));
2178+
},
2179+
{ once: true }
2180+
)
2181+
);
2182+
await runWrangler(
2183+
`r2 bucket lifecycle remove ${bucketName} --name ${ruleId}`
2184+
);
2185+
expect(std.out).toMatchInlineSnapshot(`
2186+
"Removing lifecycle rule 'my-rule' from bucket 'my-bucket'...
2187+
Lifecycle rule 'my-rule' removed from bucket 'my-bucket'."
2188+
`);
2189+
});
2190+
it("should remove a lifecycle rule as expected with id alias", async () => {
21452191
const bucketName = "my-bucket";
21462192
const ruleId = "my-rule";
21472193
const lifecycleRules = {
@@ -2207,7 +2253,7 @@ describe("r2", () => {
22072253
);
22082254
await expect(() =>
22092255
runWrangler(
2210-
`r2 bucket lifecycle remove ${bucketName} --id ${ruleId}`
2256+
`r2 bucket lifecycle remove ${bucketName} --name ${ruleId}`
22112257
)
22122258
).rejects.toThrowErrorMatchingInlineSnapshot(
22132259
"[Error: Lifecycle rule with ID 'my-rule' not found in configuration for 'my-bucket'.]"
@@ -2472,17 +2518,17 @@ describe("r2", () => {
24722518
await runWrangler(`r2 bucket lock list ${bucketName}`);
24732519
expect(std.out).toMatchInlineSnapshot(`
24742520
"Listing lock rules for bucket 'my-bucket'...
2475-
id: rule-age
2521+
name: rule-age
24762522
enabled: Yes
24772523
prefix: images/age
24782524
condition: after 1 day
24792525
2480-
id: rule-date
2526+
name: rule-date
24812527
enabled: Yes
24822528
prefix: images/date
24832529
condition: on 2025-01-30
24842530
2485-
id: rule-indefinite
2531+
name: rule-indefinite
24862532
enabled: Yes
24872533
prefix: images/indefinite
24882534
condition: indefinitely"
@@ -2517,7 +2563,7 @@ describe("r2", () => {
25172563
result: true,
25182564
});
25192565
await runWrangler(
2520-
`r2 bucket lock add ${bucketName} --id "rule-no-prefix" --retention-days 1`
2566+
`r2 bucket lock add ${bucketName} --name "rule-no-prefix" --retention-days 1`
25212567
);
25222568
expect(std.out).toMatchInlineSnapshot(`
25232569
"Adding lock rule 'rule-no-prefix' to bucket 'my-bucket'...
@@ -2538,13 +2584,13 @@ describe("r2", () => {
25382584
});
25392585

25402586
await runWrangler(
2541-
`r2 bucket lock add ${bucketName} --id 'rule-not-indefinite' --prefix prefix-not-indefinite`
2587+
`r2 bucket lock add ${bucketName} --name 'rule-not-indefinite' --prefix prefix-not-indefinite`
25422588
);
25432589
expect(std.out).toMatchInlineSnapshot(`
25442590
"Add cancelled."
25452591
`);
25462592
});
2547-
it("it should add an age lock rule using command-line arguments", async () => {
2593+
it("it should add an age lock rule using command-line arguments and id alias", async () => {
25482594
setIsTTY(true);
25492595
const bucketName = "my-bucket";
25502596

@@ -2576,7 +2622,7 @@ describe("r2", () => {
25762622
// age
25772623
await expect(() =>
25782624
runWrangler(
2579-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days one`
2625+
`r2 bucket lock add ${bucketName} --name rule-age --prefix prefix-age --retention-days one`
25802626
)
25812627
).rejects.toThrowErrorMatchingInlineSnapshot(
25822628
`[Error: Days must be a number.]`
@@ -2600,7 +2646,7 @@ describe("r2", () => {
26002646
// age
26012647
await expect(() =>
26022648
runWrangler(
2603-
`r2 bucket lock add ${bucketName} --id rule-age --prefix prefix-age --retention-days -10`
2649+
`r2 bucket lock add ${bucketName} --name rule-age --prefix prefix-age --retention-days -10`
26042650
)
26052651
).rejects.toThrowErrorMatchingInlineSnapshot(
26062652
`[Error: Days must be a positive number: -10]`
@@ -2623,7 +2669,7 @@ describe("r2", () => {
26232669
]);
26242670
// date
26252671
await runWrangler(
2626-
`r2 bucket lock add ${bucketName} --id rule-date --prefix prefix-date --retention-date 2025-01-30`
2672+
`r2 bucket lock add ${bucketName} --name rule-date --prefix prefix-date --retention-date 2025-01-30`
26272673
);
26282674
expect(std.out).toMatchInlineSnapshot(`
26292675
"Adding lock rule 'rule-date' to bucket 'my-bucket'...
@@ -2638,7 +2684,7 @@ describe("r2", () => {
26382684
// date
26392685
await expect(() =>
26402686
runWrangler(
2641-
`r2 bucket lock add ${bucketName} --id "rule-date" --prefix "prefix-date" --retention-date "January 30, 2025"`
2687+
`r2 bucket lock add ${bucketName} --name "rule-date" --prefix "prefix-date" --retention-date "January 30, 2025"`
26422688
)
26432689
).rejects.toThrowErrorMatchingInlineSnapshot(
26442690
`[Error: Date must be a valid date in the YYYY-MM-DD format: January 30, 2025]`
@@ -2660,7 +2706,7 @@ describe("r2", () => {
26602706
]);
26612707

26622708
await runWrangler(
2663-
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite`
2709+
`r2 bucket lock add ${bucketName} --name rule-indefinite --prefix prefix-indefinite --retention-indefinite`
26642710
);
26652711
expect(std.out).toMatchInlineSnapshot(`
26662712
"Adding lock rule 'rule-indefinite' to bucket 'my-bucket'...
@@ -2690,7 +2736,7 @@ describe("r2", () => {
26902736
});
26912737

26922738
await runWrangler(
2693-
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite`
2739+
`r2 bucket lock add ${bucketName} --name rule-indefinite --prefix prefix-indefinite`
26942740
);
26952741
expect(std.out).toMatchInlineSnapshot(`
26962742
"? Are you sure you want to add lock rule 'rule-indefinite' to bucket 'my-bucket' without retention? The lock rule will apply to all matching objects indefinitely.
@@ -2716,7 +2762,7 @@ describe("r2", () => {
27162762

27172763
await expect(() =>
27182764
runWrangler(
2719-
`r2 bucket lock add ${bucketName} --id rule-indefinite --prefix prefix-indefinite --retention-indefinite false`
2765+
`r2 bucket lock add ${bucketName} --name rule-indefinite --prefix prefix-indefinite --retention-indefinite false`
27202766
)
27212767
).rejects.toThrowErrorMatchingInlineSnapshot(
27222768
`[Error: Retention must be specified.]`
@@ -2731,12 +2777,34 @@ describe("r2", () => {
27312777
await expect(() =>
27322778
runWrangler(`r2 bucket lock add ${bucketName}`)
27332779
).rejects.toThrowErrorMatchingInlineSnapshot(
2734-
`[Error: Must specify a rule ID.]`
2780+
`[Error: Must specify a rule name.]`
27352781
);
27362782
});
27372783
});
27382784
describe("remove", () => {
27392785
it("should remove a lock rule as expected", async () => {
2786+
const bucketName = "my-bucket";
2787+
const ruleId = "my-rule";
2788+
const lockRules: BucketLockRule[] = [
2789+
{
2790+
id: ruleId,
2791+
enabled: true,
2792+
prefix: "prefix",
2793+
condition: {
2794+
type: "Indefinite",
2795+
},
2796+
},
2797+
];
2798+
mockBucketLockPutWithExistingRules(bucketName, lockRules, []);
2799+
await runWrangler(
2800+
`r2 bucket lock remove ${bucketName} --name ${ruleId}`
2801+
);
2802+
expect(std.out).toMatchInlineSnapshot(`
2803+
"Removing lock rule 'my-rule' from bucket 'my-bucket'...
2804+
Lock rule 'my-rule' removed from bucket 'my-bucket'."
2805+
`);
2806+
});
2807+
it("should remove a lock rule as expected with id alias", async () => {
27402808
const bucketName = "my-bucket";
27412809
const ruleId = "my-rule";
27422810
const lockRules: BucketLockRule[] = [
@@ -2764,7 +2832,7 @@ describe("r2", () => {
27642832

27652833
mockBucketLockPutWithExistingRules(bucketName, [], []);
27662834
await expect(() =>
2767-
runWrangler(`r2 bucket lock remove ${bucketName} --id ${ruleId}`)
2835+
runWrangler(`r2 bucket lock remove ${bucketName} --name ${ruleId}`)
27682836
).rejects.toThrowErrorMatchingInlineSnapshot(
27692837
"[Error: Lock rule with ID 'my-rule' not found in configuration for 'my-bucket'.]"
27702838
);

packages/wrangler/src/r2/helpers.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ function formatCondition(condition: LifecycleCondition): string {
985985
}
986986

987987
export function tableFromLifecycleRulesResponse(rules: LifecycleRule[]): {
988-
id: string;
988+
name: string;
989989
enabled: string;
990990
prefix: string;
991991
action: string;
@@ -1018,7 +1018,7 @@ export function tableFromLifecycleRulesResponse(rules: LifecycleRule[]): {
10181018
}
10191019

10201020
rows.push({
1021-
id: rule.id,
1021+
name: rule.id,
10221022
enabled: rule.enabled ? "Yes" : "No",
10231023
prefix: rule.conditions.prefix || "(all prefixes)",
10241024
action: actions.join(", ") || "(none)",
@@ -1083,7 +1083,7 @@ export interface BucketLockRuleCondition {
10831083
}
10841084

10851085
export function tableFromBucketLockRulesResponse(rules: BucketLockRule[]): {
1086-
id: string;
1086+
name: string;
10871087
enabled: string;
10881088
prefix: string;
10891089
condition: string;
@@ -1092,7 +1092,7 @@ export function tableFromBucketLockRulesResponse(rules: BucketLockRule[]): {
10921092
for (const rule of rules) {
10931093
const conditionString = formatLockCondition(rule.condition);
10941094
rows.push({
1095-
id: rule.id,
1095+
name: rule.id,
10961096
enabled: rule.enabled ? "Yes" : "No",
10971097
prefix: rule.prefix || "(all prefixes)",
10981098
condition: conditionString,

0 commit comments

Comments
 (0)