From 62a57f6c20750e63c656db2b4bdb322b18490d46 Mon Sep 17 00:00:00 2001 From: Alan Stephensen Date: Fri, 15 Mar 2024 18:26:20 +1100 Subject: [PATCH 1/3] Wrap included file with include comment --- README.md | 2 ++ __tests__/helpers.ts | 12 ++++++++++ __tests__/include.test.ts | 13 ++++++++++- __tests__/readCurrentMigration.test.ts | 5 +++- __tests__/uncommit.test.ts | 32 ++++++++++++++++++++++++++ src/actions.ts | 4 +++- src/commands/_common.ts | 4 +++- src/commands/migrate.ts | 4 ++-- src/commands/reset.ts | 4 +++- src/commands/run.ts | 4 ++-- src/commands/uncommit.ts | 9 +++++++- src/commands/watch.ts | 4 +++- src/current.ts | 4 +++- src/migration.ts | 14 +++++++---- tsconfig.json | 4 ++-- 15 files changed, 101 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index a1f5dec..40022f8 100644 --- a/README.md +++ b/README.md @@ -769,10 +769,12 @@ and when the migration is committed or watched, the contents of `myfunction.sql` will be included in the result, such that the following SQL is executed: ```sql +--! Included functions/myfunction.sql create or replace function myfunction(a int, b int) returns int as $$ select a + b; $$ language sql stable; +--! EndIncluded functions/myfunction.sql drop policy if exists access_by_numbers on mytable; create policy access_by_numbers on mytable for update using (myfunction(4, 2) < 42); ``` diff --git a/__tests__/helpers.ts b/__tests__/helpers.ts index 1a5af27..31e3c90 100644 --- a/__tests__/helpers.ts +++ b/__tests__/helpers.ts @@ -266,6 +266,15 @@ export const makeMigrations = (commitMessage?: string) => { commitMessage ? `\n--! Message: ${commitMessage}` : `` }\n\n${MIGRATION_NOTRX_TEXT.trim()}\n`; + const MIGRATION_INCLUDE_TEXT = `--!include foo.sql`; + const MIGRATION_INCLUDE_COMPILED = `${MIGRATION_INCLUDE_TEXT}\n${MIGRATION_1_TEXT}\n${MIGRATION_INCLUDE_TEXT}`; + const MIGRATION_INCLUDE_HASH = createHash("sha1") + .update(`${MIGRATION_INCLUDE_COMPILED.trim()}` + "\n") + .digest("hex"); + const MIGRATION_INCLUDE_COMMITTED = `--! Previous: -\n--! Hash: sha1:${MIGRATION_INCLUDE_HASH}${ + commitMessage ? `\n--! Message: ${commitMessage}` : `` + }\n\n${MIGRATION_INCLUDE_COMPILED}\n`; + const MIGRATION_MULTIFILE_FILES = { "migrations/links/two.sql": "select 2;", "migrations/current": { @@ -308,6 +317,9 @@ select 3; MIGRATION_NOTRX_TEXT, MIGRATION_NOTRX_HASH, MIGRATION_NOTRX_COMMITTED, + MIGRATION_INCLUDE_TEXT, + MIGRATION_INCLUDE_HASH, + MIGRATION_INCLUDE_COMMITTED, MIGRATION_MULTIFILE_TEXT, MIGRATION_MULTIFILE_HASH, MIGRATION_MULTIFILE_COMMITTED, diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index adafe55..60c7c8f 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -42,7 +42,9 @@ it("compiles an included file", async () => { FAKE_VISITED, ), ).toEqual(`\ +--! Include foo.sql select * from foo; +--! EndInclude foo.sql `); }); @@ -64,9 +66,17 @@ it("compiles multiple included files", async () => { FAKE_VISITED, ), ).toEqual(`\ +--! Include dir1/foo.sql select * from foo; +--! EndInclude dir1/foo.sql +--! Include dir2/bar.sql select * from bar; +--! EndInclude dir2/bar.sql +--! Include dir3/baz.sql +--! Include dir4/qux.sql select * from qux; +--! EndInclude dir4/qux.sql +--! EndInclude dir3/baz.sql `); }); @@ -129,6 +139,7 @@ commit; FAKE_VISITED, ), ).toEqual(`\ +--! Include foo.sql begin; create or replace function current_user_id() returns uuid as $$ @@ -140,6 +151,6 @@ comment on function current_user_id is E'The ID of the current user.'; grant all on function current_user_id to :DATABASE_USER; commit; - +--! EndInclude foo.sql `); }); diff --git a/__tests__/readCurrentMigration.test.ts b/__tests__/readCurrentMigration.test.ts index 6e0efb5..e6736a5 100644 --- a/__tests__/readCurrentMigration.test.ts +++ b/__tests__/readCurrentMigration.test.ts @@ -111,5 +111,8 @@ it("reads from current.sql, and processes included files", async () => { const currentLocation = await getCurrentMigrationLocation(parsedSettings); const content = await readCurrentMigration(parsedSettings, currentLocation); - expect(content).toEqual("-- TEST from foo"); + expect(content).toEqual(`\ +--! Included foo_current.sql +-- TEST from foo +--! EndIncluded foo_current.sql`); }); diff --git a/__tests__/uncommit.test.ts b/__tests__/uncommit.test.ts index a57031c..10dbe33 100644 --- a/__tests__/uncommit.test.ts +++ b/__tests__/uncommit.test.ts @@ -55,6 +55,8 @@ describe.each([[undefined], ["My Commit Message"]])( const { MIGRATION_1_TEXT, MIGRATION_1_COMMITTED, + MIGRATION_INCLUDE_TEXT, + MIGRATION_INCLUDE_COMMITTED, MIGRATION_MULTIFILE_COMMITTED, MIGRATION_MULTIFILE_FILES, } = makeMigrations(commitMessage); @@ -88,6 +90,36 @@ describe.each([[undefined], ["My Commit Message"]])( ).toEqual(MIGRATION_1_COMMITTED); }); + it("rolls back a migration that has included another file", async () => { + mockFs({ + [`migrations/committed/000001${commitMessageSlug}.sql`]: + MIGRATION_INCLUDE_COMMITTED, + "migrations/current.sql": "-- JUST A COMMENT\n", + "migrations/fixtures/foo.sql": MIGRATION_1_TEXT, + }); + await migrate(settings); + await uncommit(settings); + + await expect( + fsp.stat("migrations/committed/000001.sql"), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await fsp.readFile("migrations/current.sql", "utf8")).toEqual( + (commitMessage ? `--! Message: ${commitMessage}\n\n` : "") + + MIGRATION_INCLUDE_TEXT.trim() + + "\n", + ); + + await commit(settings); + expect( + await fsp.readFile( + `migrations/committed/000001${commitMessageSlug}.sql`, + "utf8", + ), + ).toEqual(MIGRATION_INCLUDE_COMMITTED); + }); + it("rolls back multifile migration", async () => { mockFs({ [`migrations/committed/000001${commitMessageSlug}.sql`]: diff --git a/src/actions.ts b/src/actions.ts index bbb1351..a011757 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -183,7 +183,9 @@ export function makeValidateActionCallback(logger: Logger, allowRoot = false) { specs.push(rawSpec); } else { throw new Error( - `Action spec '${inspect(rawSpec)}' not supported; perhaps you need to upgrade?`, + `Action spec '${inspect( + rawSpec, + )}' not supported; perhaps you need to upgrade?`, ); } } else { diff --git a/src/commands/_common.ts b/src/commands/_common.ts index 958a9ce..3a6fd1f 100644 --- a/src/commands/_common.ts +++ b/src/commands/_common.ts @@ -44,7 +44,9 @@ export async function getSettingsFromJSON(path: string): Promise { return JSON5.parse(data); } catch (e) { throw new Error( - `Failed to parse '${path}': ${e instanceof Error ? e.message : String(e)}`, + `Failed to parse '${path}': ${ + e instanceof Error ? e.message : String(e) + }`, ); } } diff --git a/src/commands/migrate.ts b/src/commands/migrate.ts index 4ca540f..e349da5 100644 --- a/src/commands/migrate.ts +++ b/src/commands/migrate.ts @@ -69,8 +69,8 @@ export async function _migrate( remainingMigrations.length > 0 ? `${remainingMigrations.length} committed migrations executed` : lastMigration - ? "Already up to date" - : `Up to date — no committed migrations to run` + ? "Already up to date" + : `Up to date — no committed migrations to run` }`, ); }); diff --git a/src/commands/reset.ts b/src/commands/reset.ts index e70d69d..90494f1 100644 --- a/src/commands/reset.ts +++ b/src/commands/reset.ts @@ -48,7 +48,9 @@ export async function _reset( ); } catch (e) { throw new Error( - `Failed to create database '${databaseName}' with owner '${databaseOwner}': ${e instanceof Error ? e.message : String(e)}`, + `Failed to create database '${databaseName}' with owner '${databaseOwner}': ${ + e instanceof Error ? e.message : String(e) + }`, ); } await pgClient.query( diff --git a/src/commands/run.ts b/src/commands/run.ts index 7d963c2..bb7ccbc 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -39,8 +39,8 @@ export async function run( const baseConnectionString = rootDatabase ? parsedSettings.rootConnectionString : shadow - ? parsedSettings.shadowConnectionString - : parsedSettings.connectionString; + ? parsedSettings.shadowConnectionString + : parsedSettings.connectionString; if (!baseConnectionString) { throw new Error("Could not determine connection string to use."); } diff --git a/src/commands/uncommit.ts b/src/commands/uncommit.ts index 01fd188..18b4658 100644 --- a/src/commands/uncommit.ts +++ b/src/commands/uncommit.ts @@ -42,9 +42,16 @@ export async function _uncommit(parsedSettings: ParsedSettings): Promise { const contents = await fsp.readFile(lastMigrationFilepath, "utf8"); const { headers, body } = parseMigrationText(lastMigrationFilepath, contents); + // Remove included migrations + const includeRegex = + /^--![ \t]*Included[ \t]+(?.*?\.sql)[ \t]*$.*?^--![ \t]*EndIncluded[ \t]*\k[ \t]*$/gms; + const decompiledBody = body.replace(includeRegex, (match) => { + return match.split("\n")[0].replace(" Included", "include"); + }); + // Drop Hash, Previous and AllowInvalidHash from headers; then write out const { Hash, Previous, AllowInvalidHash, ...otherHeaders } = headers; - const completeBody = serializeMigration(body, otherHeaders); + const completeBody = serializeMigration(decompiledBody, otherHeaders); await writeCurrentMigration(parsedSettings, currentLocation, completeBody); // Delete the migration from committed and from the DB diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 0ce3277..7963232 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -204,7 +204,9 @@ export async function _watch( .catch((error: unknown) => { if (!isLoggedError(error)) { parsedSettings.logger.error( - `Error occurred whilst processing migration: ${error instanceof Error ? error.message : String(error)}`, + `Error occurred whilst processing migration: ${ + error instanceof Error ? error.message : String(error) + }`, { error }, ); } diff --git a/src/current.ts b/src/current.ts index 748598b..284a148 100644 --- a/src/current.ts +++ b/src/current.ts @@ -38,7 +38,9 @@ async function readFileOrError(path: string): Promise { return await fsp.readFile(path, "utf8"); } catch (e) { throw new Error( - `Failed to read file at '${path}': ${e instanceof Error ? e.message : String(e)}`, + `Failed to read file at '${path}': ${ + e instanceof Error ? e.message : String(e) + }`, ); } } diff --git a/src/migration.ts b/src/migration.ts index 86ad7e6..4f5429d 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -132,7 +132,7 @@ export async function compileIncludes( content: string, processedFiles: ReadonlySet, ): Promise { - const regex = /^--![ \t]*include[ \t]+(.*\.sql)[ \t]*$/gm; + const regex = /^--![ \t]*[iI]nclude[ \t]+(.*\.sql)[ \t]*$/gm; // Find all includes in this `content` const matches = [...content.matchAll(regex)]; @@ -165,7 +165,11 @@ export async function compileIncludes( if (processedFiles.has(sqlPath)) { throw new Error( - `Circular include detected - '${sqlPath}' is included again! Import statement: \`${line}\`; trace:\n ${[...processedFiles].reverse().join("\n ")}`, + `Circular include detected - '${sqlPath}' is included again! Import statement: \`${line}\`; trace:\n ${[ + ...processedFiles, + ] + .reverse() + .join("\n ")}`, ); } @@ -205,10 +209,12 @@ export async function compileIncludes( // Simple string replacement for each path matched const compiledContent = content.replace( regex, - (_match, rawSqlPath: string) => { + (match, rawSqlPath: string) => { const sqlPath = sqlPathByRawSqlPath[rawSqlPath]; const content = contentBySqlPath[sqlPath]; - return content; + const included = match.replace(/^--![ \t]*include/, "--! Included"); + const endIncluded = included.replace("Included", "EndIncluded"); + return `${included}\n${content.trim()}\n${endIncluded}`; }, ); diff --git a/tsconfig.json b/tsconfig.json index c880e7f..5b9a941 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,6 +3,6 @@ "include": ["src/**/*", "__tests__/**/*", "*.js", "./.*.js"], "exclude": [], "compilerOptions": { - "noEmit": true, - }, + "noEmit": true + } } From 82b2dda9101f079286ca5d0b566685dd48bc3336 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 14 May 2024 10:20:37 +0100 Subject: [PATCH 2/3] yarn lint:fix --- src/commands/migrate.ts | 4 ++-- src/commands/run.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/commands/migrate.ts b/src/commands/migrate.ts index e349da5..4ca540f 100644 --- a/src/commands/migrate.ts +++ b/src/commands/migrate.ts @@ -69,8 +69,8 @@ export async function _migrate( remainingMigrations.length > 0 ? `${remainingMigrations.length} committed migrations executed` : lastMigration - ? "Already up to date" - : `Up to date — no committed migrations to run` + ? "Already up to date" + : `Up to date — no committed migrations to run` }`, ); }); diff --git a/src/commands/run.ts b/src/commands/run.ts index bb7ccbc..7d963c2 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -39,8 +39,8 @@ export async function run( const baseConnectionString = rootDatabase ? parsedSettings.rootConnectionString : shadow - ? parsedSettings.shadowConnectionString - : parsedSettings.connectionString; + ? parsedSettings.shadowConnectionString + : parsedSettings.connectionString; if (!baseConnectionString) { throw new Error("Could not determine connection string to use."); } From df0135951dd1289a76d251da1d69fb2a697a9abe Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 14 May 2024 10:29:03 +0100 Subject: [PATCH 3/3] Revert formatting changes --- src/actions.ts | 4 +--- src/commands/_common.ts | 4 +--- src/commands/reset.ts | 4 +--- src/commands/watch.ts | 4 +--- src/current.ts | 4 +--- src/migration.ts | 6 +----- 6 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/actions.ts b/src/actions.ts index a011757..bbb1351 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -183,9 +183,7 @@ export function makeValidateActionCallback(logger: Logger, allowRoot = false) { specs.push(rawSpec); } else { throw new Error( - `Action spec '${inspect( - rawSpec, - )}' not supported; perhaps you need to upgrade?`, + `Action spec '${inspect(rawSpec)}' not supported; perhaps you need to upgrade?`, ); } } else { diff --git a/src/commands/_common.ts b/src/commands/_common.ts index 3a6fd1f..958a9ce 100644 --- a/src/commands/_common.ts +++ b/src/commands/_common.ts @@ -44,9 +44,7 @@ export async function getSettingsFromJSON(path: string): Promise { return JSON5.parse(data); } catch (e) { throw new Error( - `Failed to parse '${path}': ${ - e instanceof Error ? e.message : String(e) - }`, + `Failed to parse '${path}': ${e instanceof Error ? e.message : String(e)}`, ); } } diff --git a/src/commands/reset.ts b/src/commands/reset.ts index 90494f1..e70d69d 100644 --- a/src/commands/reset.ts +++ b/src/commands/reset.ts @@ -48,9 +48,7 @@ export async function _reset( ); } catch (e) { throw new Error( - `Failed to create database '${databaseName}' with owner '${databaseOwner}': ${ - e instanceof Error ? e.message : String(e) - }`, + `Failed to create database '${databaseName}' with owner '${databaseOwner}': ${e instanceof Error ? e.message : String(e)}`, ); } await pgClient.query( diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 7963232..0ce3277 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -204,9 +204,7 @@ export async function _watch( .catch((error: unknown) => { if (!isLoggedError(error)) { parsedSettings.logger.error( - `Error occurred whilst processing migration: ${ - error instanceof Error ? error.message : String(error) - }`, + `Error occurred whilst processing migration: ${error instanceof Error ? error.message : String(error)}`, { error }, ); } diff --git a/src/current.ts b/src/current.ts index 284a148..748598b 100644 --- a/src/current.ts +++ b/src/current.ts @@ -38,9 +38,7 @@ async function readFileOrError(path: string): Promise { return await fsp.readFile(path, "utf8"); } catch (e) { throw new Error( - `Failed to read file at '${path}': ${ - e instanceof Error ? e.message : String(e) - }`, + `Failed to read file at '${path}': ${e instanceof Error ? e.message : String(e)}`, ); } } diff --git a/src/migration.ts b/src/migration.ts index 4f5429d..95fdb78 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -165,11 +165,7 @@ export async function compileIncludes( if (processedFiles.has(sqlPath)) { throw new Error( - `Circular include detected - '${sqlPath}' is included again! Import statement: \`${line}\`; trace:\n ${[ - ...processedFiles, - ] - .reverse() - .join("\n ")}`, + `Circular include detected - '${sqlPath}' is included again! Import statement: \`${line}\`; trace:\n ${[...processedFiles].reverse().join("\n ")}`, ); }