Skip to content

Commit

Permalink
Use queue name for event notification config (#5369)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdeboard committed Mar 27, 2024
1 parent 0401db9 commit 7115568
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 12 deletions.
9 changes: 9 additions & 0 deletions .changeset/rich-oranges-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": minor
---

fix: Use queue name, not ID, for `r2 bucket event-notification` subcommands

Since the original command was not yet operational, this update does not constitute a breaking change.

Instead of providing the queue ID as the parameter to `--queue`, users must provide the queue _name_. Under the hood, we will query the Queues API for the queue ID given the queue name.
48 changes: 43 additions & 5 deletions packages/wrangler/src/__tests__/r2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ describe("r2", () => {
const eventTypes: R2EventType[] = ["object_create", "object_delete"];
const actions: R2EventableOperation[] = [];
const bucketName = "my-bucket";
const queue = "deadbeef-0123-4567-8910-abcdefabcdef";
const queue = "my-queue";

const config: EWCRequestBody = {
rules: [
Expand Down Expand Up @@ -580,6 +580,25 @@ describe("r2", () => {
);
return response.once(context.json(createFetchResult({})));
}
),
rest.get(
"*/accounts/:accountId/workers/queues/:queueName",
async (request, response, context) => {
const { accountId, queueName } = request.params;
expect(accountId).toEqual("some-account-id");
expect(queueName).toEqual(queue);
expect(request.headers.get("authorization")).toEqual(
"Bearer some-api-token"
);
return response.once(
context.json({
success: true,
errors: [],
messages: [],
result: {},
})
);
}
)
);
await expect(
Expand Down Expand Up @@ -624,15 +643,15 @@ describe("r2", () => {
--event-types, --event-type Specify the kinds of object events to emit notifications for. ex. '--event-types object_create object_delete' [array] [required] [choices: \\"object_create\\", \\"object_delete\\"]
--prefix only actions on objects with this prefix will emit notifications [string]
--suffix only actions on objects with this suffix will emit notifications [string]
--queue The ID of the queue to which event notifications will be sent. ex '--queue deadbeef-0123-4567-8910-abcdefgabcde' [string] [required]"
--queue The name of the queue to which event notifications will be sent. ex '--queue my-queue' [string] [required]"
`);
});
});

describe("delete", () => {
it("follows happy path as expected", async () => {
const bucketName = "my-bucket";
const queue = "deadbeef-0123-4567-8910-abcdefabcdef";
const queue = "my-queue";
msw.use(
rest.delete(
"*/accounts/:accountId/event_notifications/r2/:bucketName/configuration/queues/:queueUUID",
Expand All @@ -644,6 +663,25 @@ describe("r2", () => {
);
return response.once(context.json(createFetchResult({})));
}
),
rest.get(
"*/accounts/:accountId/workers/queues/:queueName",
async (request, response, context) => {
const { accountId, queueName } = request.params;
expect(accountId).toEqual("some-account-id");
expect(queueName).toEqual(queue);
expect(request.headers.get("authorization")).toEqual(
"Bearer some-api-token"
);
return response.once(
context.json({
success: true,
errors: [],
messages: [],
result: {},
})
);
}
)
);
await expect(
Expand All @@ -652,7 +690,7 @@ describe("r2", () => {
)
).resolves.toBe(undefined);
expect(std.out).toMatchInlineSnapshot(`
"Disabling event notifications for \\"my-bucket\\" to queue deadbeef-0123-4567-8910-abcdefabcdef...
"Disabling event notifications for \\"my-bucket\\" to queue my-queue...
Configuration deleted successfully!"
`);
});
Expand Down Expand Up @@ -682,7 +720,7 @@ describe("r2", () => {
-v, --version Show version number [boolean]
Options:
--queue The ID of the queue that is configured to receive notifications. ex '--queue deadbeef-0123-4567-8910-abcdefgabcde' [string] [required]"
--queue The name of the queue that is configured to receive notifications. ex '--queue my-queue' [string] [required]"
`);
});
});
Expand Down
16 changes: 11 additions & 5 deletions packages/wrangler/src/r2/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { getLocalPersistencePath } from "../dev/get-local-persistence-path";
import { buildPersistOptions } from "../dev/miniflare";
import { UserError } from "../errors";
import { logger } from "../logger";
import { getQueue } from "../queues/client";
import type { Config } from "../config";
import type { ApiCredentials } from "../user";
import type { R2Bucket } from "@cloudflare/workers-types/experimental";
import type { ReplaceWorkersTypes } from "miniflare";
Expand Down Expand Up @@ -378,14 +380,16 @@ export function eventNotificationHeaders(
* - 409 Conflict - A configuration between the bucket and queue already exists
* */
export async function putEventNotificationConfig(
config: Config,
apiCredentials: ApiCredentials,
accountId: string,
bucketName: string,
queueUUID: string,
queueName: string,
eventTypes: R2EventType[],
prefix?: string,
suffix?: string
): Promise<{ event_notification_detail_id: string }> {
const queue = await getQueue(config, queueName);
const headers = eventNotificationHeaders(apiCredentials);
let actions: R2EventableOperation[] = [];

Expand All @@ -400,23 +404,25 @@ export async function putEventNotificationConfig(
`Sending this configuration to "${bucketName}":\n${JSON.stringify(body)}`
);
return await fetchResult<{ event_notification_detail_id: string }>(
`/accounts/${accountId}/event_notifications/r2/${bucketName}/configuration/queues/${queueUUID}`,
`/accounts/${accountId}/event_notifications/r2/${bucketName}/configuration/queues/${queue.queue_id}`,
{ method: "PUT", body: JSON.stringify(body), headers }
);
}

export async function deleteEventNotificationConfig(
config: Config,
apiCredentials: ApiCredentials,
accountId: string,
bucketName: string,
queueUUID: string
queueName: string
): Promise<null> {
const queue = await getQueue(config, queueName);
const headers = eventNotificationHeaders(apiCredentials);
logger.log(
`Disabling event notifications for "${bucketName}" to queue ${queueUUID}...`
`Disabling event notifications for "${bucketName}" to queue ${queueName}...`
);
return await fetchResult<null>(
`/accounts/${accountId}/event_notifications/r2/${bucketName}/configuration/queues/${queueUUID}`,
`/accounts/${accountId}/event_notifications/r2/${bucketName}/configuration/queues/${queue.queue_id}`,
{ method: "DELETE", headers }
);
}
6 changes: 4 additions & 2 deletions packages/wrangler/src/r2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ export function r2(r2Yargs: CommonYargsArgv) {
})
.option("queue", {
describe:
"The ID of the queue to which event notifications will be sent. ex '--queue deadbeef-0123-4567-8910-abcdefgabcde'",
"The name of the queue to which event notifications will be sent. ex '--queue my-queue'",
demandOption: true,
requiresArg: true,
type: "string",
Expand All @@ -601,6 +601,7 @@ export function r2(r2Yargs: CommonYargsArgv) {
suffix = "",
} = args;
await putEventNotificationConfig(
config,
apiCreds,
accountId,
`${bucket}`,
Expand All @@ -625,7 +626,7 @@ export function r2(r2Yargs: CommonYargsArgv) {
})
.option("queue", {
describe:
"The ID of the queue that is configured to receive notifications. ex '--queue deadbeef-0123-4567-8910-abcdefgabcde'",
"The name of the queue that is configured to receive notifications. ex '--queue my-queue'",
demandOption: true,
requiresArg: true,
type: "string",
Expand All @@ -638,6 +639,7 @@ export function r2(r2Yargs: CommonYargsArgv) {
const apiCreds = requireApiToken();
const { bucket, queue } = args;
await deleteEventNotificationConfig(
config,
apiCreds,
accountId,
`${bucket}`,
Expand Down

0 comments on commit 7115568

Please sign in to comment.