Skip to content

Commit

Permalink
fix: improve validation message for kv:namespace create
Browse files Browse the repository at this point in the history 
Previously, if the user passed multiple positional arguments (which is invalid)
the error message would suggest that these should be grouped in quotes.
But this is also wrong, since a namespace binding name must not contain spaces.

Also this commit adds additional tests for `kv:namespace create`.
  • Loading branch information
@petebacondarwin
petebacondarwin committed Jan 2, 2022
1 parent ceadab1 commit 5856807
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 92 deletions.
9 changes: 9 additions & 0 deletions .changeset/tricky-baboons-build.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

Improve validation message for `kv:namespace create`

Previously, if the user passed multiple positional arguments (which is invalid)
the error message would suggest that these should be grouped in quotes.
But this is also wrong, since a namespace binding name must not contain spaces.
2 changes: 1 addition & 1 deletion .vscode/settings.json
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"editor.defaultFormatter": "esbenp.prettier-vscode",
"cSpell.words": ["cfetch", "execa", "iarna", "Miniflare"]
"cSpell.words": ["cfetch", "execa", "iarna", "Miniflare", "Positionals"]
}
246 changes: 190 additions & 56 deletions packages/wrangler/src/__tests__/kv.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,76 +1,210 @@
import { setMock, unsetAllMocks } from "./mock-cfetch";
import { runWrangler } from "./run-wrangler";
import { runInTempDir } from "./run-in-tmp";
import { writeFileSync } from "fs";

describe("wrangler", () => {
runInTempDir();

afterEach(() => {
unsetAllMocks();
});

describe("kv:namespace", () => {
it("can create a namespace", async () => {
const KVNamespaces: { title: string; id: string }[] = [];
setMock("/accounts/:accountId/storage/kv/namespaces", (uri, init) => {
expect(init.method === "POST");
expect(uri[0]).toEqual(
"/accounts/some-account-id/storage/kv/namespaces"
describe("create", () => {
function mockCreateRequest(expectedTitle: string) {
setMock(
"/accounts/:accountId/storage/kv/namespaces",
"POST",
([_url, accountId], { body }) => {
expect(accountId).toEqual("some-account-id");
const title = JSON.parse(body).title;
expect(title).toEqual(expectedTitle);
return { id: "some-namespace-id" };
}
);
}

it("should error if no namespace is given", async () => {
const { stdout, stderr } = await runWrangler("kv:namespace create");
expect(stdout).toMatchInlineSnapshot(`""`);
expect(stderr).toMatchInlineSnapshot(`
"wrangler kv:namespace create <namespace>
Create a new namespace
Positionals:
namespace The name of the new namespace [string] [required]
Flags:
-c, --config Path to .toml configuration file [string]
-h, --help Show help [boolean]
-v, --version Show version number [boolean]
Options:
-l, --local Run on my machine [boolean] [default: false]
--env Perform on a specific environment [string]
--preview Interact with a preview namespace [boolean]
Not enough non-option arguments: got 0, need at least 1"
`);
});

it("should error if the namespace to create contains spaces", async () => {
const { stdout, stderr } = await runWrangler(
"kv:namespace create abc def ghi"
);
const { title } = JSON.parse(init.body);
expect(title).toEqual("worker-UnitTestNamespace");
KVNamespaces.push({ title, id: "some-namespace-id" });
return { id: "some-namespace-id" };
expect(stdout).toMatchInlineSnapshot(`""`);
expect(stderr).toMatchInlineSnapshot(`
"wrangler kv:namespace create <namespace>
Create a new namespace
Positionals:
namespace The name of the new namespace [string] [required]
Flags:
-c, --config Path to .toml configuration file [string]
-h, --help Show help [boolean]
-v, --version Show version number [boolean]
Options:
-l, --local Run on my machine [boolean] [default: false]
--env Perform on a specific environment [string]
--preview Interact with a preview namespace [boolean]
Unexpected additional positional arguments \\"def ghi\\"."
`);
});

await runWrangler("kv:namespace create UnitTestNamespace");
it("should error if the namespace to create is not valid", async () => {
const { stdout, stderr } = await runWrangler(
"kv:namespace create abc-def"
);
expect(stdout).toMatchInlineSnapshot(`""`);
expect(stderr).toMatchInlineSnapshot(`
"wrangler kv:namespace create <namespace>
expect(KVNamespaces).toEqual([
{
title: "worker-UnitTestNamespace",
id: "some-namespace-id",
},
]);
Create a new namespace
Positionals:
namespace The name of the new namespace [string] [required]
Flags:
-c, --config Path to .toml configuration file [string]
-h, --help Show help [boolean]
-v, --version Show version number [boolean]
Options:
-l, --local Run on my machine [boolean] [default: false]
--env Perform on a specific environment [string]
--preview Interact with a preview namespace [boolean]
The namespace binding name \\"abc-def\\" is invalid. It can only have alphanumeric and _ characters, and cannot begin with a number."
`);
});

it("should create a namespace", async () => {
mockCreateRequest("worker-UnitTestNamespace");
const { stdout } = await runWrangler(
"kv:namespace create UnitTestNamespace"
);
expect(stdout).toMatchInlineSnapshot(`
"🌀 Creating namespace with title \\"worker-UnitTestNamespace\\"
✨ Success!
Add the following to your configuration file in your kv_namespaces array:
{ binding = \\"UnitTestNamespace\\", id = \\"some-namespace-id\\" }"
`);
});

it("should create a preview namespace if configured to do so", async () => {
mockCreateRequest("worker-UnitTestNamespace_preview");
const { stdout } = await runWrangler(
"kv:namespace create UnitTestNamespace --preview"
);
expect(stdout).toMatchInlineSnapshot(`
"🌀 Creating namespace with title \\"worker-UnitTestNamespace_preview\\"
✨ Success!
Add the following to your configuration file in your kv_namespaces array:
{ binding = \\"UnitTestNamespace\\", preview_id = \\"some-namespace-id\\" }"
`);
});

it("should create a namespace using configured worker name", async () => {
writeFileSync("./wrangler.toml", 'name = "otherWorker"', "utf-8");
mockCreateRequest("otherWorker-UnitTestNamespace");
const { stdout } = await runWrangler(
"kv:namespace create UnitTestNamespace"
);
expect(stdout).toMatchInlineSnapshot(`
"🌀 Creating namespace with title \\"otherWorker-UnitTestNamespace\\"
✨ Success!
Add the following to your configuration file in your kv_namespaces array:
{ binding = \\"UnitTestNamespace\\", id = \\"some-namespace-id\\" }"
`);
});

it("should create a namespace in an environment if configured to do so", async () => {
mockCreateRequest("worker-customEnv-UnitTestNamespace");
const { stdout } = await runWrangler(
"kv:namespace create UnitTestNamespace --env customEnv"
);
expect(stdout).toMatchInlineSnapshot(`
"🌀 Creating namespace with title \\"worker-customEnv-UnitTestNamespace\\"
✨ Success!
Add the following to your configuration file in your kv_namespaces array under [env.customEnv]:
{ binding = \\"UnitTestNamespace\\", id = \\"some-namespace-id\\" }"
`);
});
});

it("can list namespaces", async () => {
const KVNamespaces: { title: string; id: string }[] = [
{ title: "title-1", id: "id-1" },
{ title: "title-2", id: "id-2" },
];
setMock(
"/accounts/:accountId/storage/kv/namespaces\\?:qs",
(uri, init) => {
expect(uri[0]).toContain(
"/accounts/some-account-id/storage/kv/namespaces"
);
expect(uri[2]).toContain("per_page=100");
expect(uri[2]).toContain("order=title");
expect(uri[2]).toContain("direction=asc");
expect(uri[2]).toContain("page=1");
expect(init).toBe(undefined);
return KVNamespaces;
}
);
const { stdout } = await runWrangler("kv:namespace list");
const namespaces = JSON.parse(stdout) as { id: string; title: string }[];
expect(namespaces).toEqual(KVNamespaces);
describe("list", () => {
it("should list namespaces", async () => {
const KVNamespaces: { title: string; id: string }[] = [
{ title: "title-1", id: "id-1" },
{ title: "title-2", id: "id-2" },
];
setMock(
"/accounts/:accountId/storage/kv/namespaces\\?:qs",
(uri, init) => {
expect(uri[0]).toContain(
"/accounts/some-account-id/storage/kv/namespaces"
);
expect(uri[2]).toContain("per_page=100");
expect(uri[2]).toContain("order=title");
expect(uri[2]).toContain("direction=asc");
expect(uri[2]).toContain("page=1");
expect(init).toBe(undefined);
return KVNamespaces;
}
);
const { stdout } = await runWrangler("kv:namespace list");
const namespaces = JSON.parse(stdout) as {
id: string;
title: string;
}[];
expect(namespaces).toEqual(KVNamespaces);
});
});

it("can delete a namespace", async () => {
let accountId = "";
let namespaceId = "";
setMock(
"/accounts/:accountId/storage/kv/namespaces/:namespaceId",
(uri, init) => {
accountId = uri[1];
namespaceId = uri[2];
expect(uri[0]).toEqual(
"/accounts/some-account-id/storage/kv/namespaces/some-namespace-id"
);
expect(init.method).toBe("DELETE");
}
);
await runWrangler(`kv:namespace delete --namespace-id some-namespace-id`);
expect(accountId).toEqual("some-account-id");
expect(namespaceId).toEqual("some-namespace-id");
describe("delete", () => {
it("should delete a namespace", async () => {
let accountId = "";
let namespaceId = "";
setMock(
"/accounts/:accountId/storage/kv/namespaces/:namespaceId",
(uri, init) => {
accountId = uri[1];
namespaceId = uri[2];
expect(uri[0]).toEqual(
"/accounts/some-account-id/storage/kv/namespaces/some-namespace-id"
);
expect(init.method).toBe("DELETE");
}
);
await runWrangler(
`kv:namespace delete --namespace-id some-namespace-id`
);
expect(accountId).toEqual("some-account-id");
expect(namespaceId).toEqual("some-namespace-id");
});
});
});
});
11 changes: 7 additions & 4 deletions packages/wrangler/src/__tests__/mock-cfetch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,24 @@ const { pathToRegexp } = require("path-to-regexp");
let mocks = [];

export function mockCfetch(resource, init) {
for (const { regexp, handler } of mocks) {
for (const { regexp, method, handler } of mocks) {
// The `resource` regular expression will extract the labelled groups from the URL.
// Let's pass these through to the handler, to allow it to do additional checks or behaviour.
const uri = regexp.exec(resource);
if (uri !== null) {
// Does the resource path match and (if specified) the HTTP method?
if (uri !== null && (!method || method === init.method)) {
return handler(uri, init); // TODO: should we have some kind of fallthrough system? we'll see.
}
}
throw new Error(`no mocks found for ${resource}`);
}

export function setMock(resource, handler) {
export function setMock(resource, methodOrHandler, handler = methodOrHandler) {
const hasMethod = typeof methodOrHandler === "string";
const mock = {
resource,
handler,
method: hasMethod ? methodOrHandler : undefined,
handler: hasMethod ? handler : methodOrHandler,
regexp: pathToRegexp(resource),
};
mocks.push(mock);
Expand Down
51 changes: 20 additions & 31 deletions packages/wrangler/src/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
putKeyValue,
putBulkKeyValue,
deleteBulkKeyValue,
createNamespace,
isValidNamespaceBinding,
} from "./kv";

import { pages } from "./pages";
Expand Down Expand Up @@ -1140,27 +1142,28 @@ export async function main(argv: string[]): Promise<void> {
});
},
async (args) => {
if (args._.length !== 2) {
throw new Error(
`Did you forget to add quotes around "${
args.namespace
} ${args._.slice(2).join(" ")}"?`
if (args._.length > 2) {
const extraArgs = args._.slice(2).join(" ");
throw `Unexpected additional positional arguments "${extraArgs}".`;
}

if (!isValidNamespaceBinding(args.namespace)) {
throw new CommandLineArgsError(
`The namespace binding name "${args.namespace}" is invalid. It can only have alphanumeric and _ characters, and cannot begin with a number.`
);
}

const config = args.config as Config;
if (!config.name) {
console.warn(
"No configured name present, using `worker` as a prefix for the title"
);
}

const title = `${config.name || "worker"}${
args.env ? `-${args.env}` : ""
}-${args.namespace}${args.preview ? "_preview" : ""}`;

if (/[\W]+/.test(args.namespace)) {
throw new Error("invalid binding name, needs to be js friendly");
}
const name = config.name || "worker";
const environment = args.env ? `-${args.env}` : "";
const preview = args.preview ? "_preview" : "";
const title = `${name}${environment}-${args.namespace}${preview}`;

if (args.local) {
const { Miniflare } = await import("miniflare");
Expand Down Expand Up @@ -1197,30 +1200,16 @@ export async function main(argv: string[]): Promise<void> {
// TODO: generate a binding name stripping non alphanumeric chars

console.log(`🌀 Creating namespace with title "${title}"`);

const response = await cfetch<{ id: string }>(
`/accounts/${config.account_id}/storage/kv/namespaces`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
title,
}),
}
);
const namespaceId = await createNamespace(config.account_id, title);

console.log("✨ Success!");
const envString = args.env ? ` under [env.${args.env}]` : "";
const previewString = args.preview ? "preview_" : "";
console.log(
`Add the following to your configuration file in your kv_namespaces array${
args.env ? ` under [env.${args.env}]` : ""
}:`
`Add the following to your configuration file in your kv_namespaces array${envString}:`
);
console.log(
`{ binding = "${args.namespace}", ${
args.preview ? "preview_" : ""
}id = "${response.id}" }`
`{ binding = "${args.namespace}", ${previewString}id = "${namespaceId}" }`
);
}
)
Expand Down
Loading

0 comments on commit 5856807

Please sign in to comment.