[wrangler] Fix nested D1 migrations

.changeset/clever-forest-d1-recursive-migrations.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+Fix D1 migrations to discover nested SQL files
+
+`wrangler d1 migrations list` and `wrangler d1 migrations apply` now recursively discover `.sql` files under `migrations_dir`. Nested migrations are recorded by their relative path, so layouts like `migrations/20240501120000_initial/migration.sql` can be applied without colliding with other `migration.sql` files. `wrangler d1 migrations create` also accounts for nested migration prefixes when choosing the next migration number.

.changeset/fresh-d1-logs.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+Fix D1 migration logging after failed JSON queries
+
+D1 commands that temporarily suppress logs for JSON/internal queries now always restore the previous logger level after errors. This prevents a failed migration setup/query from hiding subsequent migration progress output.

packages/wrangler/src/__tests__/d1/migrate.test.ts

@@ -1,3 +1,5 @@
+import * as fs from "node:fs";
+import * as path from "node:path";
 import { writeWranglerConfig } from "@cloudflare/workers-utils/test-helpers";
 import { http, HttpResponse } from "msw";
 import { describe, it, vi } from "vitest";
@@ -50,6 +52,30 @@ describe("migrate", () => {
 			await runWrangler("d1 migrations create D1 test-migration");
 			expect(mockStd.out).toContain("Successfully created Migration");
 		});
+
+		it("should create migrations after nested migrations", async ({
+			expect,
+		}) => {
+			setIsTTY(false);
+			writeWranglerConfig({
+				d1_databases: [{ binding: "D1", database_name: "D1" }],
+			});
+			fs.mkdirSync(path.join("migrations", "20240501120000_initial"), {
+				recursive: true,
+			});
+			fs.writeFileSync(
+				path.join("migrations", "20240501120000_initial", "migration.sql"),
+				""
+			);
+
+			await runWrangler("d1 migrations create D1 test-migration");
+
+			expect(
+				fs.existsSync(
+					path.join("migrations", "20240501120001_test-migration.sql")
+				)
+			).toBe(true);
+		});
 	});
 
 	describe("apply", () => {
@@ -158,7 +184,6 @@ Your database may not be available to serve requests during the migration, conti
 			expect,
 		}) => {
 			setIsTTY(false);
-			const std = mockConsoleMethods();
 			msw.use(
 				http.post(
 					"*/accounts/:accountId/d1/database/:databaseId/query",
@@ -208,24 +233,115 @@ Your database may not be available to serve requests during the migration, conti
 						binding: "DATABASE",
 						database_name: "db",
 						database_id: "xxxx",
-						migrations_dir: "/tmp/my-migrations-go-here",
+						migrations_dir: "my-migrations-go-here",
 					},
 				],
 				account_id: "nx01",
 			});
-			mockConfirm({
-				text: `No migrations folder found.
-Ok to create /tmp/my-migrations-go-here?`,
-				result: true,
-			});
 			await runWrangler("d1 migrations create db test");
+			mockStd.getAndClearOut();
+
+			await runWrangler("d1 migrations apply db --remote");
+			expect(mockStd.out).toContain("Migrations to be applied:");
+			expect(mockStd.out).toContain("0001_test.sql");
+		});
+
+		it("should apply nested migrations with relative path names", async ({
+			expect,
+		}) => {
+			setIsTTY(false);
+			const sqlBodies: string[] = [];
+			msw.use(
+				http.post(
+					"*/accounts/:accountId/d1/database/:databaseId/query",
+					async ({ request }) => {
+						const body = (await request.json()) as { sql?: string };
+						if (body.sql) {
+							sqlBodies.push(body.sql);
+						}
+
+						return HttpResponse.json(
+							{
+								result: [
+									{
+										results: [],
+										success: true,
+										meta: {},
+									},
+								],
+								success: true,
+								errors: [],
+								messages: [],
+							},
+							{ status: 200 }
+						);
+					}
+				),
+				http.get("*/accounts/:accountId/d1/database/:databaseId", async () => {
+					return HttpResponse.json(
+						{
+							result: {
+								file_size: 7421952,
+								name: "db",
+								num_tables: 2,
+								uuid: "xxxx",
+								version: "production",
+							},
+							success: true,
+							errors: [],
+							messages: [],
+						},
+						{ status: 200 }
+					);
+				})
+			);
+			writeWranglerConfig({
+				d1_databases: [
+					{
+						binding: "DATABASE",
+						database_name: "db",
+						database_id: "xxxx",
+					},
+				],
+				account_id: "account-id",
+			});
+			fs.mkdirSync(path.join("migrations", "20240501120000_initial"), {
+				recursive: true,
+			});
+			fs.mkdirSync(path.join("migrations", "20240501130000_quo'ted"), {
+				recursive: true,
+			});
+			fs.writeFileSync(
+				path.join("migrations", "20240501120000_initial", "migration.sql"),
+				"CREATE TABLE users (id INTEGER PRIMARY KEY);"
+			);
+			fs.writeFileSync(
+				path.join("migrations", "20240501130000_quo'ted", "migration.sql"),
+				"CREATE INDEX quoted_test ON users (id);"
+			);
 			mockConfirm({
-				text: `About to apply 1 migration(s)
+				text: `About to apply 2 migration(s)
 Your database may not be available to serve requests during the migration, continue?`,
 				result: true,
 			});
+
 			await runWrangler("d1 migrations apply db --remote");
-			expect(std.out).toBe("");
+
+			const initialMigrationSql = sqlBodies.find((sql) => {
+				return sql.includes("CREATE TABLE users");
+			});
+			const quotedMigrationSql = sqlBodies.find((sql) => {
+				return sql.includes("CREATE INDEX quoted_test");
+			});
+			expect(initialMigrationSql).toContain(
+				"values ('20240501120000_initial/migration.sql');"
+			);
+			expect(initialMigrationSql).not.toContain("values ('migration.sql');");
+			expect(quotedMigrationSql).toContain(
+				"values ('20240501130000_quo''ted/migration.sql');"
+			);
+			expect(mockStd.out).toContain("20240501120000_initial/migration.sql");
+			expect(mockStd.out).toContain("20240501130000_quo'ted/migration.sql");
 		});
 	});
 

packages/wrangler/src/__tests__/d1/migrations/helpers.test.ts

@@ -1,7 +1,10 @@
 import * as fs from "node:fs";
 import * as path from "node:path";
 import { describe, it } from "vitest";
-import { getMigrationNames } from "../../../d1/migrations/helpers";
+import {
+	getMigrationNames,
+	getNextMigrationNumber,
+} from "../../../d1/migrations/helpers";
 import { runInTempDir } from "../../helpers/run-in-tmp";
 
 describe("getMigrationNames", () => {
@@ -73,4 +76,93 @@ describe("getMigrationNames", () => {
 		const result = getMigrationNames(migrationsDir);
 		expect(result).toEqual([]);
 	});
+
+	it("should return sorted nested SQL files as normalized relative paths", ({
+		expect,
+	}) => {
+		const migrationsDir = "./migrations";
+		fs.mkdirSync(path.join(migrationsDir, "20240501120000_initial"), {
+			recursive: true,
+		});
+		fs.mkdirSync(path.join(migrationsDir, "20240501130000_add_users"), {
+			recursive: true,
+		});
+
+		fs.writeFileSync(path.join(migrationsDir, "0001_top_level.sql"), "-- test");
+		fs.writeFileSync(
+			path.join(migrationsDir, "20240501130000_add_users", "migration.sql"),
+			"-- test"
+		);
+		fs.writeFileSync(
+			path.join(migrationsDir, "20240501120000_initial", "migration.sql"),
+			"-- test"
+		);
+		fs.writeFileSync(
+			path.join(migrationsDir, "20240501120000_initial", "README.md"),
+			"# readme"
+		);
+
+		const result = getMigrationNames(migrationsDir);
+
+		expect(result).toEqual([
+			"0001_top_level.sql",
+			"20240501120000_initial/migration.sql",
+			"20240501130000_add_users/migration.sql",
+		]);
+	});
+});
+
+describe("getNextMigrationNumber", () => {
+	runInTempDir();
+
+	it("should return the next number after existing top-level migrations", ({
+		expect,
+	}) => {
+		const migrationsDir = "./migrations";
+		fs.mkdirSync(migrationsDir, { recursive: true });
+
+		fs.writeFileSync(path.join(migrationsDir, "0001_create_tables.sql"), "");
+		fs.writeFileSync(path.join(migrationsDir, "0003_add_columns.sql"), "");
+
+		expect(getNextMigrationNumber(migrationsDir)).toBe(4);
+	});
+
+	it("should include nested migrations when calculating the next number", ({
+		expect,
+	}) => {
+		const migrationsDir = "./migrations";
+		fs.mkdirSync(path.join(migrationsDir, "9999_nested"), { recursive: true });
+
+		fs.writeFileSync(path.join(migrationsDir, "0002_top_level.sql"), "");
+		fs.writeFileSync(
+			path.join(migrationsDir, "9999_nested", "migration.sql"),
+			""
+		);
+
+		expect(getNextMigrationNumber(migrationsDir)).toBe(10000);
+	});
+
+	it("should ignore nonnumeric top-level SQL files", ({ expect }) => {
+		const migrationsDir = "./migrations";
+		fs.mkdirSync(migrationsDir, { recursive: true });
+
+		fs.writeFileSync(path.join(migrationsDir, "README.sql"), "");
+		fs.writeFileSync(path.join(migrationsDir, "schema.sql"), "");
+
+		expect(getNextMigrationNumber(migrationsDir)).toBe(1);
+	});
+
+	it("should continue after nested timestamp migrations", ({ expect }) => {
+		const migrationsDir = "./migrations";
+		fs.mkdirSync(path.join(migrationsDir, "20240501120000_initial"), {
+			recursive: true,
+		});
+
+		fs.writeFileSync(
+			path.join(migrationsDir, "20240501120000_initial", "migration.sql"),
+			""
+		);
+
+		expect(getNextMigrationNumber(migrationsDir)).toBe(20240501120001);
+	});
 });