Skip to content

Commit

Permalink
[D1] Don't require login when running wrangler d1 migrations list (#…
Browse files Browse the repository at this point in the history
…2657)

* fix: dont require auth for `d1 migrations list`

* fix: make d1 migrations commands consistent in behavior

* this is not an async fn

* add test for d1 migrations create
  • Loading branch information
rozenmd committed Jan 31, 2023
1 parent 2efd453 commit 8d21b2e
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-cups-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: remove the need to login when running `d1 migrations list --local`
71 changes: 71 additions & 0 deletions packages/wrangler/src/__tests__/d1/migrate.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { cwd } from "process";
import { reinitialiseAuthTokens } from "../../user";
import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
import { useMockIsTTY } from "../helpers/mock-istty";
import { runInTempDir } from "../helpers/run-in-tmp";
import { runWrangler } from "../helpers/run-wrangler";
Expand All @@ -8,6 +10,21 @@ describe("migrate", () => {
runInTempDir();
const { setIsTTY } = useMockIsTTY();

describe("create", () => {
it("should reject the --local flag for create", async () => {
setIsTTY(false);
writeWranglerToml({
d1_databases: [
{ binding: "DATABASE", database_name: "db", database_id: "xxxx" },
],
});

await expect(
runWrangler("d1 migrations create test some-message --local DATABASE")
).rejects.toThrowError(`Unknown argument: local`);
});
});

describe("apply", () => {
it("should not attempt to login in local mode", async () => {
setIsTTY(false);
Expand Down Expand Up @@ -45,4 +62,58 @@ describe("migrate", () => {
);
});
});

describe("list", () => {
mockAccountId();
mockApiToken({ apiToken: null });

it("should not attempt to login in local mode", async () => {
setIsTTY(false);
writeWranglerToml({
d1_databases: [
{ binding: "DATABASE", database_name: "db", database_id: "xxxx" },
],
});
// If we get to the point where we are checking for migrations then we have not been asked to log in.
await expect(
runWrangler("d1 migrations list --local DATABASE")
).rejects.toThrowError(
`No migrations present at ${cwd().replaceAll("\\", "/")}/migrations.`
);
});

it("should try to read D1 config from wrangler.toml when logged in", async () => {
// no need to clear this env var as it's implicitly cleared by mockApiToken in afterEach
process.env.CLOUDFLARE_API_TOKEN = "api-token";
reinitialiseAuthTokens();
setIsTTY(false);
writeWranglerToml();
await expect(
runWrangler("d1 migrations list DATABASE")
).rejects.toThrowError(
"Can't find a DB with name/binding 'DATABASE' in local config. Check info in wrangler.toml..."
);
});

it("should throw if user is not authenticated and not using --local", async () => {
setIsTTY(false);

await expect(
runWrangler("d1 migrations list DATABASE")
).rejects.toThrowError(
"In a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work"
);
});

it("should not try to read wrangler.toml in local mode", async () => {
setIsTTY(false);
writeWranglerToml();
// If we get to the point where we are checking for migrations then we have not checked wrangler.toml.
await expect(
runWrangler("d1 migrations list --local DATABASE")
).rejects.toThrowError(
`No migrations present at ${cwd().replaceAll("\\", "/")}/migrations.`
);
});
});
});
12 changes: 7 additions & 5 deletions packages/wrangler/src/d1/migrations/apply.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ import type {
export function ApplyOptions(yargs: CommonYargsArgv) {
return DatabaseWithLocal(yargs);
}

type ApplyHandlerOptions = StrictYargsOptionsToInterface<typeof ApplyOptions>;

export const ApplyHandler = withConfig<ApplyHandlerOptions>(
async ({ config, database, local, persistTo }): Promise<void> => {
logger.log(d1BetaWarning);

const databaseInfo = await getDatabaseInfoFromConfig(config, database);
const databaseInfo = getDatabaseInfoFromConfig(config, database);
if (!databaseInfo && !local) {
throw new Error(
`Can't find a DB with name/binding '${database}' in local config. Check info in wrangler.toml...`
Expand All @@ -51,10 +53,10 @@ export const ApplyHandler = withConfig<ApplyHandlerOptions>(
false
);

const migrationTableName =
const migrationsTableName =
databaseInfo?.migrationsTableName ?? DEFAULT_MIGRATION_TABLE;
await initMigrationsTable(
migrationTableName,
migrationsTableName,
local,
config,
database,
Expand All @@ -63,7 +65,7 @@ export const ApplyHandler = withConfig<ApplyHandlerOptions>(

const unappliedMigrations = (
await getUnappliedMigrations(
migrationTableName,
migrationsTableName,
migrationsPath,
local,
config,
Expand Down Expand Up @@ -124,7 +126,7 @@ Your database may not be available to serve requests during the migration, conti
"utf8"
);
query += `
INSERT INTO ${migrationTableName} (name)
INSERT INTO ${migrationsTableName} (name)
values ('${migration.Name}');
`;

Expand Down
4 changes: 3 additions & 1 deletion packages/wrangler/src/d1/migrations/create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ export function CreateOptions(yargs: CommonYargsArgv) {
demandOption: true,
});
}

type CreateHandlerOptions = StrictYargsOptionsToInterface<typeof CreateOptions>;

export const CreateHandler = withConfig<CreateHandlerOptions>(
async ({ config, database, message }): Promise<void> => {
await requireAuth({});
logger.log(d1BetaWarning);

const databaseInfo = await getDatabaseInfoFromConfig(config, database);
const databaseInfo = getDatabaseInfoFromConfig(config, database);
if (!databaseInfo) {
throw new Error(
`Can't find a DB with name/binding '${database}' in local config. Check info in wrangler.toml...`
Expand Down
18 changes: 13 additions & 5 deletions packages/wrangler/src/d1/migrations/list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import React from "react";
import { withConfig } from "../../config";
import { logger } from "../../logger";
import { requireAuth } from "../../user";
import { DEFAULT_MIGRATION_PATH, DEFAULT_MIGRATION_TABLE } from "../constants";
import { d1BetaWarning, getDatabaseInfoFromConfig } from "../utils";
import {
getMigrationsPath,
Expand All @@ -20,14 +21,18 @@ import type {
export function ListOptions(yargs: CommonYargsArgv) {
return DatabaseWithLocal(yargs);
}

type ListHandlerOptions = StrictYargsOptionsToInterface<typeof ListOptions>;

export const ListHandler = withConfig<ListHandlerOptions>(
async ({ config, database, local, persistTo }): Promise<void> => {
await requireAuth({});
if (!local) {
await requireAuth({});
}
logger.log(d1BetaWarning);

const databaseInfo = await getDatabaseInfoFromConfig(config, database);
if (!databaseInfo) {
const databaseInfo = getDatabaseInfoFromConfig(config, database);
if (!databaseInfo && !local) {
throw new Error(
`Can't find a DB with name/binding '${database}' in local config. Check info in wrangler.toml...`
);
Expand All @@ -36,13 +41,16 @@ export const ListHandler = withConfig<ListHandlerOptions>(
if (!config.configPath) {
return;
}
const { migrationsTableName, migrationsFolderPath } = databaseInfo;

const migrationsPath = await getMigrationsPath(
path.dirname(config.configPath),
migrationsFolderPath,
databaseInfo?.migrationsFolderPath ?? DEFAULT_MIGRATION_PATH,
false
);

const migrationsTableName =
databaseInfo?.migrationsTableName ?? DEFAULT_MIGRATION_TABLE;

await initMigrationsTable(
migrationsTableName,
local,
Expand Down

0 comments on commit 8d21b2e

Please sign in to comment.