From a9828ad83cb879932b0ec58772d48f5fa356d141 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Sat, 16 Dec 2023 12:55:35 -0500 Subject: [PATCH 01/32] issue-163: added ability to include external files in current.sql. --- README.md | 41 +++++++++++++++++++++++++++++ __tests__/compile.test.ts | 55 ++++++++++++++++++++++++++++++++++++++- src/commands/compile.ts | 6 +++-- src/commands/run.ts | 6 +++-- src/migration.ts | 12 +++++++++ 5 files changed, 115 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 530e669..ecbcedd 100644 --- a/README.md +++ b/README.md @@ -724,6 +724,47 @@ by `graphile-migrate watch` is defined. By default this is in the `migrations/current.sql` file, but it might be `migrations/current/*.sql` if you're using folder mode. +#### Including external files in the current migration +You can include external files in your `current.sql` to better assist in source +control. These includes are always relative to the root of where graphile-migrate +is running. + +For example. Given the following directory structure: +``` +/- migrate + - migrations + | + - current.sql + - fixtures + | + - functions + | + - myfunction.sql +``` + +and the contents of `myfunction.sql`: +``` +select 'not really a function'; +``` + +If you want to make changes to `myFunction.sql` without having to copy the contents +into `current.sql` (the old way), simply use the `--! include ` annotation +in `current.sql`: + +``` +--!include fixtures/functions/myfunction.sql +select * from tables; +select 'more migration stuff'; + +``` +and the resulting file that is compiled would look like: +``` +select 'not really a function'; +select * from tables; +select 'more migration stuff'; +``` +with the contents of `myfunction.sql` replacing the include line. + ### Committed migration(s) The files for migrations that you've committed with `graphile-migrate commit` diff --git a/__tests__/compile.test.ts b/__tests__/compile.test.ts index 522d07c..2f96baa 100644 --- a/__tests__/compile.test.ts +++ b/__tests__/compile.test.ts @@ -1,7 +1,8 @@ import "./helpers"; -import { compile } from "../src"; +import * as mockFs from "mock-fs"; +import { compile } from "../src"; let old: string | undefined; beforeAll(() => { old = process.env.DATABASE_AUTHENTICATOR; @@ -11,6 +12,10 @@ afterAll(() => { process.env.DATABASE_AUTHENTICATOR = old; }); +afterEach(() => { + mockFs.restore(); +}); + it("compiles SQL with settings", async () => { expect( await compile( @@ -48,3 +53,51 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; COMMIT; `); }); + +it("compiles an included file", async () => { + mockFs({ + "foo.sql": "select * from foo;", + }); + expect( + await compile( + { + connectionString: "postgres://dbowner:dbpassword@dbhost:1221/dbname", + placeholders: { + ":DATABASE_AUTHENTICATOR": "!ENV", + }, + }, + `\ +--!include foo.sql +`, + ), + ).toEqual(`\ +select * from foo; +`); +}); + +it("compiles multiple included files", async () => { + mockFs({ + "foo.sql": "select * from foo;", + "bar.sql": "select * from bar;", + "baz.sql": "select * from baz;", + }); + expect( + await compile( + { + connectionString: "postgres://dbowner:dbpassword@dbhost:1221/dbname", + placeholders: { + ":DATABASE_AUTHENTICATOR": "!ENV", + }, + }, + `\ +--!include foo.sql +--!include bar.sql +--!include baz.sql +`, + ), + ).toEqual(`\ +select * from foo; +select * from bar; +select * from baz; +`); +}); diff --git a/src/commands/compile.ts b/src/commands/compile.ts index 1bf62bf..f768f8a 100644 --- a/src/commands/compile.ts +++ b/src/commands/compile.ts @@ -1,7 +1,7 @@ import { promises as fsp } from "fs"; import { CommandModule } from "yargs"; -import { compilePlaceholders } from "../migration"; +import { compileIncludes, compilePlaceholders } from "../migration"; import { parseSettings, Settings } from "../settings"; import { CommonArgv, getSettings, readStdin } from "./_common"; @@ -15,7 +15,9 @@ export async function compile( shadow = false, ): Promise { const parsedSettings = await parseSettings(settings, shadow); - return compilePlaceholders(parsedSettings, content, shadow); + return await compileIncludes( + compilePlaceholders(parsedSettings, content, shadow), + ); } export const compileCommand: CommandModule< diff --git a/src/commands/run.ts b/src/commands/run.ts index f4a065f..4c9ff3e 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -4,7 +4,7 @@ import { CommandModule } from "yargs"; import { DO_NOT_USE_DATABASE_URL } from "../actions"; import { runQueryWithErrorInstrumentation } from "../instrumentation"; -import { compilePlaceholders } from "../migration"; +import { compileIncludes, compilePlaceholders } from "../migration"; import { withClient } from "../pgReal"; import { makeRootDatabaseConnectionString, @@ -35,7 +35,9 @@ export async function run( } = {}, ): Promise { const parsedSettings = await parseSettings(settings, shadow); - const sql = compilePlaceholders(parsedSettings, content, shadow); + const sql = await compileIncludes( + compilePlaceholders(parsedSettings, content, shadow), + ); const baseConnectionString = rootDatabase ? parsedSettings.rootConnectionString : shadow diff --git a/src/migration.ts b/src/migration.ts index 5177fd2..4ede4e4 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -118,6 +118,18 @@ export function compilePlaceholders( )(content); } +export async function compileIncludes(content: string): Promise { + const regex = /--!include (.*.sql)/g; + let compiledContent = content; + let match = regex.exec(content); + while (match != null) { + const fileContents = await fsp.readFile(match[1], "utf8"); + compiledContent = compiledContent.replace(match[0], fileContents); + match = regex.exec(content); + } + return compiledContent; +} + const TABLE_CHECKS = { migrations: { columnCount: 4, From c818a0393b5c420fb14534f455d98b4059636473 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:46:53 -0500 Subject: [PATCH 02/32] Update README.md Co-authored-by: Benjie --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ecbcedd..64a0cd8 100644 --- a/README.md +++ b/README.md @@ -726,8 +726,8 @@ you're using folder mode. #### Including external files in the current migration You can include external files in your `current.sql` to better assist in source -control. These includes are always relative to the root of where graphile-migrate -is running. +control. These includes are identified by paths within the `migrations/fixtures` +folder. For example. Given the following directory structure: ``` From 62982cadac265d12982a455bb865bcf1a2689cfa Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:47:04 -0500 Subject: [PATCH 03/32] Update README.md Co-authored-by: Benjie --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 64a0cd8..c888d18 100644 --- a/README.md +++ b/README.md @@ -735,11 +735,11 @@ For example. Given the following directory structure: - migrations | - current.sql - - fixtures - | - - functions + - fixtures | - - myfunction.sql + - functions + | + - myfunction.sql ``` and the contents of `myfunction.sql`: From 8abc06fc75409bc558ddb60228f8a6627de4cf94 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:47:22 -0500 Subject: [PATCH 04/32] Update README.md Co-authored-by: Benjie --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c888d18..6f1939e 100644 --- a/README.md +++ b/README.md @@ -743,8 +743,11 @@ For example. Given the following directory structure: ``` and the contents of `myfunction.sql`: -``` -select 'not really a function'; +```sql +create or replace function myfunction(a int, b int) +returns int as $$ + select a + b; +$$ language sql stable; ``` If you want to make changes to `myFunction.sql` without having to copy the contents From 1f2013ffa7e96d164b6fe42c158affcaf87403dd Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:47:42 -0500 Subject: [PATCH 05/32] Update README.md Co-authored-by: Benjie --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6f1939e..3a29278 100644 --- a/README.md +++ b/README.md @@ -750,9 +750,11 @@ returns int as $$ $$ language sql stable; ``` -If you want to make changes to `myFunction.sql` without having to copy the contents -into `current.sql` (the old way), simply use the `--! include ` annotation -in `current.sql`: +When you make changes to `myfunction.sql`, include it in your current migration +by adding `--! include functions/myfunction.sql` to your `current.sql` (or any +`current/*.sql`). This statement doesn't need to be at the top of the file, +wherever it is will be replaced by the content of +`migrations/fixtures/functions/myfunction.sql` when the migration is committed. ``` --!include fixtures/functions/myfunction.sql From e676a1eb980801a4552bb384abcf68b78afd2ae8 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:48:19 -0500 Subject: [PATCH 06/32] Update README.md Co-authored-by: Benjie --- README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3a29278..83426de 100644 --- a/README.md +++ b/README.md @@ -756,11 +756,10 @@ by adding `--! include functions/myfunction.sql` to your `current.sql` (or any wherever it is will be replaced by the content of `migrations/fixtures/functions/myfunction.sql` when the migration is committed. -``` +```sql --!include fixtures/functions/myfunction.sql -select * from tables; -select 'more migration stuff'; - +drop policy if exists access_by_numbers on mytable; +create policy access_by_numbers on mytable for update using (myfunction(4, 2) < 42); ``` and the resulting file that is compiled would look like: ``` From d4d15ac93d9585314131c2e0e34b57fb3766d34d Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:48:44 -0500 Subject: [PATCH 07/32] Update README.md Co-authored-by: Benjie --- README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 83426de..64cae90 100644 --- a/README.md +++ b/README.md @@ -761,13 +761,17 @@ wherever it is will be replaced by the content of drop policy if exists access_by_numbers on mytable; create policy access_by_numbers on mytable for update using (myfunction(4, 2) < 42); ``` -and the resulting file that is compiled would look like: -``` -select 'not really a function'; -select * from tables; -select 'more migration stuff'; +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 +create or replace function myfunction(a int, b int) +returns int as $$ + select a + b; +$$ language sql stable; +drop policy if exists access_by_numbers on mytable; +create policy access_by_numbers on mytable for update using (myfunction(4, 2) < 42); ``` -with the contents of `myfunction.sql` replacing the include line. ### Committed migration(s) From 90da00f8ed6ac7fc4e48895b419ad2d43a775e73 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:49:05 -0500 Subject: [PATCH 08/32] Update __tests__/compile.test.ts Co-authored-by: Benjie --- __tests__/compile.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/__tests__/compile.test.ts b/__tests__/compile.test.ts index 2f96baa..5326910 100644 --- a/__tests__/compile.test.ts +++ b/__tests__/compile.test.ts @@ -77,9 +77,10 @@ select * from foo; it("compiles multiple included files", async () => { mockFs({ - "foo.sql": "select * from foo;", - "bar.sql": "select * from bar;", - "baz.sql": "select * from baz;", + "migrations/fixtures/dir1/foo.sql": "select * from foo;", + "migrations/fixtures/dir2/bar.sql": "select * from bar;", + "migrations/fixtures/dir3/baz.sql": "--! include dir4/qux.sql", + "migrations/fixtures/dir4/qux.sql": "select * from qux;", }); expect( await compile( From f676d3aafdf30f73beccd1191207398662265955 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:49:21 -0500 Subject: [PATCH 09/32] Update __tests__/compile.test.ts Co-authored-by: Benjie --- __tests__/compile.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/compile.test.ts b/__tests__/compile.test.ts index 5326910..db46192 100644 --- a/__tests__/compile.test.ts +++ b/__tests__/compile.test.ts @@ -91,9 +91,9 @@ it("compiles multiple included files", async () => { }, }, `\ ---!include foo.sql ---!include bar.sql ---!include baz.sql +--!include dir1/foo.sql +--!include dir2/bar.sql +--!include dir3/baz.sql `, ), ).toEqual(`\ From b47053c34c55029dc72d416c4295993725834854 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Thu, 21 Dec 2023 22:49:42 -0500 Subject: [PATCH 10/32] Update __tests__/compile.test.ts Co-authored-by: Benjie --- __tests__/compile.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/compile.test.ts b/__tests__/compile.test.ts index db46192..ec0f33c 100644 --- a/__tests__/compile.test.ts +++ b/__tests__/compile.test.ts @@ -99,6 +99,6 @@ it("compiles multiple included files", async () => { ).toEqual(`\ select * from foo; select * from bar; -select * from baz; +select * from qux; `); }); From c41cc221cb3503070318fc1e8e829bcf1ee10c3d Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 22 Dec 2023 00:18:08 -0500 Subject: [PATCH 11/32] Fixed up include code to use a fixed directory, and recursively include. --- README.md | 2 +- __tests__/compile.test.ts | 49 ----------------------- __tests__/include.test.ts | 83 +++++++++++++++++++++++++++++++++++++++ src/commands/compile.ts | 6 +-- src/commands/run.ts | 6 +-- src/migration.ts | 26 +++++++++--- 6 files changed, 108 insertions(+), 64 deletions(-) create mode 100644 __tests__/include.test.ts diff --git a/README.md b/README.md index 64cae90..3953165 100644 --- a/README.md +++ b/README.md @@ -751,7 +751,7 @@ $$ language sql stable; ``` When you make changes to `myfunction.sql`, include it in your current migration -by adding `--! include functions/myfunction.sql` to your `current.sql` (or any +by adding `--!include functions/myfunction.sql` to your `current.sql` (or any `current/*.sql`). This statement doesn't need to be at the top of the file, wherever it is will be replaced by the content of `migrations/fixtures/functions/myfunction.sql` when the migration is committed. diff --git a/__tests__/compile.test.ts b/__tests__/compile.test.ts index ec0f33c..4747483 100644 --- a/__tests__/compile.test.ts +++ b/__tests__/compile.test.ts @@ -53,52 +53,3 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; COMMIT; `); }); - -it("compiles an included file", async () => { - mockFs({ - "foo.sql": "select * from foo;", - }); - expect( - await compile( - { - connectionString: "postgres://dbowner:dbpassword@dbhost:1221/dbname", - placeholders: { - ":DATABASE_AUTHENTICATOR": "!ENV", - }, - }, - `\ ---!include foo.sql -`, - ), - ).toEqual(`\ -select * from foo; -`); -}); - -it("compiles multiple included files", async () => { - mockFs({ - "migrations/fixtures/dir1/foo.sql": "select * from foo;", - "migrations/fixtures/dir2/bar.sql": "select * from bar;", - "migrations/fixtures/dir3/baz.sql": "--! include dir4/qux.sql", - "migrations/fixtures/dir4/qux.sql": "select * from qux;", - }); - expect( - await compile( - { - connectionString: "postgres://dbowner:dbpassword@dbhost:1221/dbname", - placeholders: { - ":DATABASE_AUTHENTICATOR": "!ENV", - }, - }, - `\ ---!include dir1/foo.sql ---!include dir2/bar.sql ---!include dir3/baz.sql -`, - ), - ).toEqual(`\ -select * from foo; -select * from bar; -select * from qux; -`); -}); diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts new file mode 100644 index 0000000..817a80a --- /dev/null +++ b/__tests__/include.test.ts @@ -0,0 +1,83 @@ +import "./helpers"; + +import * as mockFs from "mock-fs"; + +import { compileIncludes } from "../src/migration"; +import { ParsedSettings, parseSettings } from "../src/settings"; + +let old: string | undefined; +let settings: ParsedSettings; +beforeAll(async () => { + old = process.env.DATABASE_AUTHENTICATOR; + process.env.DATABASE_AUTHENTICATOR = "dbauth"; + settings = await parseSettings({ + connectionString: "postgres://dbowner:dbpassword@dbhost:1221/dbname", + placeholders: { + ":DATABASE_AUTHENTICATOR": "!ENV", + }, + migrationsFolder: 'migrations' + }); +}); +afterAll(() => { + process.env.DATABASE_AUTHENTICATOR = old; +}); + +afterEach(() => { + mockFs.restore(); +}); + + +it("compiles an included file", async () => { + mockFs({ + "migrations/fixtures/foo.sql": "select * from foo;", + }); + expect( + await compileIncludes( + settings, + `\ +--!include foo.sql +`, + ), + ).toEqual(`\ +select * from foo; +`); +}); + +it("compiles multiple included files", async () => { + mockFs({ + "migrations/fixtures/dir1/foo.sql": "select * from foo;", + "migrations/fixtures/dir2/bar.sql": "select * from bar;", + "migrations/fixtures/dir3/baz.sql": "--!include dir4/qux.sql", + "migrations/fixtures/dir4/qux.sql": "select * from qux;", + }); + expect( + await compileIncludes( + settings, + `\ +--!include dir1/foo.sql +--!include dir2/bar.sql +--!include dir3/baz.sql +`, + ), + ).toEqual(`\ +select * from foo; +select * from bar; +select * from qux; +`); +}); + +it("compiles an included file, and won't get stuck in an infinite include loop", async () => { + mockFs({ + "migrations/fixtures/foo.sql": "select * from foo;--!include foo.sql", + }); + expect( + await compileIncludes( + settings, + `\ +--!include foo.sql +`, + ), + ).toEqual(`\ +select * from foo; +`); +}); \ No newline at end of file diff --git a/src/commands/compile.ts b/src/commands/compile.ts index f768f8a..1bf62bf 100644 --- a/src/commands/compile.ts +++ b/src/commands/compile.ts @@ -1,7 +1,7 @@ import { promises as fsp } from "fs"; import { CommandModule } from "yargs"; -import { compileIncludes, compilePlaceholders } from "../migration"; +import { compilePlaceholders } from "../migration"; import { parseSettings, Settings } from "../settings"; import { CommonArgv, getSettings, readStdin } from "./_common"; @@ -15,9 +15,7 @@ export async function compile( shadow = false, ): Promise { const parsedSettings = await parseSettings(settings, shadow); - return await compileIncludes( - compilePlaceholders(parsedSettings, content, shadow), - ); + return compilePlaceholders(parsedSettings, content, shadow); } export const compileCommand: CommandModule< diff --git a/src/commands/run.ts b/src/commands/run.ts index 4c9ff3e..f4a065f 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -4,7 +4,7 @@ import { CommandModule } from "yargs"; import { DO_NOT_USE_DATABASE_URL } from "../actions"; import { runQueryWithErrorInstrumentation } from "../instrumentation"; -import { compileIncludes, compilePlaceholders } from "../migration"; +import { compilePlaceholders } from "../migration"; import { withClient } from "../pgReal"; import { makeRootDatabaseConnectionString, @@ -35,9 +35,7 @@ export async function run( } = {}, ): Promise { const parsedSettings = await parseSettings(settings, shadow); - const sql = await compileIncludes( - compilePlaceholders(parsedSettings, content, shadow), - ); + const sql = compilePlaceholders(parsedSettings, content, shadow); const baseConnectionString = rootDatabase ? parsedSettings.rootConnectionString : shadow diff --git a/src/migration.ts b/src/migration.ts index 4ede4e4..fd1066f 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -118,16 +118,30 @@ export function compilePlaceholders( )(content); } -export async function compileIncludes(content: string): Promise { +export async function compileIncludes(parsedSettings: ParsedSettings, content: string, processedFiles: Array = []): Promise { const regex = /--!include (.*.sql)/g; let compiledContent = content; let match = regex.exec(content); - while (match != null) { - const fileContents = await fsp.readFile(match[1], "utf8"); - compiledContent = compiledContent.replace(match[0], fileContents); - match = regex.exec(content); + const includePath = `${parsedSettings.migrationsFolder}/fixtures/` + if(match) { + while (match != null) { + //If we've already processed this file, skip it (prevents infinite chains) + if(!processedFiles.includes(`${includePath}${match[1]}`)){ + processedFiles.push(`${includePath}${match[1]}`); + const fileContents = await fsp.readFile(`${includePath}${match[1]}`, "utf8"); + compiledContent = compiledContent.replace(match[0], fileContents) + match = regex.exec(content); + } else { + //remove recursive include and continue + compiledContent = compiledContent.replace(match[0], ''); + match = regex.exec(content); + } + } + //recursively call compileIncludes to catch includes in the included files. + return await compileIncludes(parsedSettings, compiledContent, processedFiles); + } else { + return compiledContent; } - return compiledContent; } const TABLE_CHECKS = { From 75e0203cc476f1d1b2d6f9c734b00b68ec2ced3c Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 22 Dec 2023 15:08:33 -0500 Subject: [PATCH 12/32] Added fsrealpath support, still working on tests. --- __tests__/include.test.ts | 17 ++++++++++++++++- src/migration.ts | 15 ++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index 817a80a..3eec0cc 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -80,4 +80,19 @@ it("compiles an included file, and won't get stuck in an infinite include loop", ).toEqual(`\ select * from foo; `); -}); \ No newline at end of file +}); + +/*it("disallows calling files outside of the migrations/fixtures folder", async () => { + mockFs({ + "migrations/fixtures/bar.sql": "", + "outsideFolder/foo.sql": "select * from foo;", + }); + expect(() => { + compileIncludes( + settings, + `\ +--!include ../../outsideFolder/foo.sql +`, + )}, + ).toThrow(); +});*/ \ No newline at end of file diff --git a/src/migration.ts b/src/migration.ts index fd1066f..ae4e396 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -123,12 +123,20 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s let compiledContent = content; let match = regex.exec(content); const includePath = `${parsedSettings.migrationsFolder}/fixtures/` + const realPath = await fsp.realpath(includePath); if(match) { while (match != null) { + //make sure the include path starts with the real path of the fixtures folder. + const includeRegex = new RegExp(`^${realPath}`); + const includeRealPath = await fsp.realpath(`${includePath}${match[1]}`); + if(includeRegex.exec(includeRealPath) === null) { + throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); + } + //If we've already processed this file, skip it (prevents infinite chains) - if(!processedFiles.includes(`${includePath}${match[1]}`)){ - processedFiles.push(`${includePath}${match[1]}`); - const fileContents = await fsp.readFile(`${includePath}${match[1]}`, "utf8"); + if(!processedFiles.includes(includeRealPath)){ + processedFiles.push(includeRealPath); + const fileContents = await fsp.readFile(includeRealPath, "utf8"); compiledContent = compiledContent.replace(match[0], fileContents) match = regex.exec(content); } else { @@ -137,6 +145,7 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s match = regex.exec(content); } } + //recursively call compileIncludes to catch includes in the included files. return await compileIncludes(parsedSettings, compiledContent, processedFiles); } else { From cbaa1b572830480225bfc2667402607cfcd904ab Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 29 Dec 2023 18:47:53 -0500 Subject: [PATCH 13/32] issue-163: got throwing test fixed --- __tests__/include.test.ts | 12 ++++++------ src/migration.ts | 13 +++++++++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index 3eec0cc..9a722e7 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -82,17 +82,17 @@ select * from foo; `); }); -/*it("disallows calling files outside of the migrations/fixtures folder", async () => { +it("disallows calling files outside of the migrations/fixtures folder", async () => { mockFs({ "migrations/fixtures/bar.sql": "", "outsideFolder/foo.sql": "select * from foo;", }); - expect(() => { - compileIncludes( + + await expect(compileIncludes( settings, `\ --!include ../../outsideFolder/foo.sql `, - )}, - ).toThrow(); -});*/ \ No newline at end of file + ) + ).rejects.toThrow(); +}); \ No newline at end of file diff --git a/src/migration.ts b/src/migration.ts index ae4e396..bbc72dd 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -127,8 +127,17 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s if(match) { while (match != null) { //make sure the include path starts with the real path of the fixtures folder. - const includeRegex = new RegExp(`^${realPath}`); - const includeRealPath = await fsp.realpath(`${includePath}${match[1]}`); + let includeRegex; + let includeRealPath; + + try { + includeRegex = new RegExp(`^${realPath}`); + includeRealPath = await fsp.realpath(`${includePath}${match[1]}`); + } catch (e) { + throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); + } + + if(includeRegex.exec(includeRealPath) === null) { throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); } From 710ff4fbab471e4f17fa2d4304e5508ab16792c9 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 29 Dec 2023 20:01:00 -0500 Subject: [PATCH 14/32] Issue-163: fixed broken tests after adding includes to readCurrentMigration --- __tests__/readCurrentMigration.test.ts | 12 ++++++++++++ src/__mocks__/migration.ts | 2 ++ src/current.ts | 11 ++++++++--- src/migration.ts | 14 ++++++++++++-- 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/__tests__/readCurrentMigration.test.ts b/__tests__/readCurrentMigration.test.ts index ba75bfa..0d54c3c 100644 --- a/__tests__/readCurrentMigration.test.ts +++ b/__tests__/readCurrentMigration.test.ts @@ -102,3 +102,15 @@ With multiple lines const content = await readCurrentMigration(parsedSettings, currentLocation); expect(content).toEqual(contentWithSplits); }); + +it("reads from current.sql, and processes included files", async () => { + mockFs({ + "migrations/current.sql": "--!include foo_current.sql", + "migrations/fixtures/foo_current.sql": "-- TEST from foo" + }); + + const currentLocation = await getCurrentMigrationLocation(parsedSettings); + const content = await readCurrentMigration(parsedSettings, currentLocation); + expect(content).toEqual("-- TEST from foo"); + +}); \ No newline at end of file diff --git a/src/__mocks__/migration.ts b/src/__mocks__/migration.ts index 074281b..8ff8109 100644 --- a/src/__mocks__/migration.ts +++ b/src/__mocks__/migration.ts @@ -36,3 +36,5 @@ export const runStringMigration = jest.fn( export const runCommittedMigration = jest.fn( (_client, _settings, _context, _committedMigration, _logSuffix) => {}, ); + +export const compileIncludes = jest.fn((parsedSettings, content) => {return content}); diff --git a/src/current.ts b/src/current.ts index e0c324f..f4ff607 100644 --- a/src/current.ts +++ b/src/current.ts @@ -3,7 +3,11 @@ import { promises as fsp, Stats } from "fs"; import { isNoTransactionDefined } from "./header"; import { errorCode } from "./lib"; -import { parseMigrationText, serializeHeader } from "./migration"; +import { + compileIncludes, + parseMigrationText, + serializeHeader, +} from "./migration"; import { ParsedSettings } from "./settings"; export const VALID_FILE_REGEX = /^([0-9]+)(-[-_a-zA-Z0-9]*)?\.sql$/; @@ -106,7 +110,7 @@ export async function readCurrentMigration( const content = await readFileOrNull(location.path); // If file doesn't exist, treat it as if it were empty. - return content || ""; + return await compileIncludes(_parsedSettings, content || ""); } else { const files = await fsp.readdir(location.path); const parts = new Map< @@ -181,7 +185,8 @@ export async function readCurrentMigration( if (headerLines.length) { wholeBody = headerLines.join("\n") + "\n\n" + wholeBody; } - return wholeBody; + + return await compileIncludes(_parsedSettings, wholeBody); } } diff --git a/src/migration.ts b/src/migration.ts index bbc72dd..98fed55 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -123,7 +123,18 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s let compiledContent = content; let match = regex.exec(content); const includePath = `${parsedSettings.migrationsFolder}/fixtures/` - const realPath = await fsp.realpath(includePath); + let realPath; + + //if the fixtures folder isn't defined, catch the error and return the original content. + try { + realPath = await fsp.realpath(includePath); + } catch (e) { + if (!realPath) { + parsedSettings.logger.warn(`Warning: ${includePath} is not defined.`) + return content; + } + } + if(match) { while (match != null) { //make sure the include path starts with the real path of the fixtures folder. @@ -137,7 +148,6 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); } - if(includeRegex.exec(includeRealPath) === null) { throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); } From e9483cd105be6c5ec7496d4d39b3b788f4ca295b Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 29 Dec 2023 20:07:07 -0500 Subject: [PATCH 15/32] issue-163: added fixtures directory to watch --- src/commands/watch.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 6f0c502..79a76cc 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -217,7 +217,11 @@ export async function _watch( } }); }; - const watcher = chokidar.watch(currentLocation.path, { + const watcher = chokidar.watch( + [ + currentLocation.path, + `${parsedSettings.migrationsFolder}/fixtures` + ], { /* * Without `usePolling`, on Linux, you can prevent the watching from * working by issuing `git stash && sleep 2 && git stash pop`. This is From 40d90f83c420c82374c1a1c0554bf70991a90b0e Mon Sep 17 00:00:00 2001 From: John Barlow Date: Fri, 29 Dec 2023 20:08:26 -0500 Subject: [PATCH 16/32] issue-163: lint fix --- README.md | 4 +++ __tests__/include.test.ts | 10 +++--- __tests__/readCurrentMigration.test.ts | 5 ++- src/__mocks__/migration.ts | 4 ++- src/commands/watch.ts | 47 +++++++++++++------------- src/migration.ts | 34 +++++++++++++------ 6 files changed, 60 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 3953165..d3ed601 100644 --- a/README.md +++ b/README.md @@ -725,11 +725,13 @@ by `graphile-migrate watch` is defined. By default this is in the you're using folder mode. #### Including external files in the current migration + You can include external files in your `current.sql` to better assist in source control. These includes are identified by paths within the `migrations/fixtures` folder. For example. Given the following directory structure: + ``` /- migrate - migrations @@ -743,6 +745,7 @@ For example. Given the following directory structure: ``` and the contents of `myfunction.sql`: + ```sql create or replace function myfunction(a int, b int) returns int as $$ @@ -761,6 +764,7 @@ wherever it is will be replaced by the content of drop policy if exists access_by_numbers on mytable; create policy access_by_numbers on mytable for update using (myfunction(4, 2) < 42); ``` + 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: diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index 9a722e7..a72bf29 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -15,7 +15,7 @@ beforeAll(async () => { placeholders: { ":DATABASE_AUTHENTICATOR": "!ENV", }, - migrationsFolder: 'migrations' + migrationsFolder: "migrations", }); }); afterAll(() => { @@ -26,7 +26,6 @@ afterEach(() => { mockFs.restore(); }); - it("compiles an included file", async () => { mockFs({ "migrations/fixtures/foo.sql": "select * from foo;", @@ -88,11 +87,12 @@ it("disallows calling files outside of the migrations/fixtures folder", async () "outsideFolder/foo.sql": "select * from foo;", }); - await expect(compileIncludes( + await expect( + compileIncludes( settings, `\ --!include ../../outsideFolder/foo.sql `, - ) + ), ).rejects.toThrow(); -}); \ No newline at end of file +}); diff --git a/__tests__/readCurrentMigration.test.ts b/__tests__/readCurrentMigration.test.ts index 0d54c3c..6e0efb5 100644 --- a/__tests__/readCurrentMigration.test.ts +++ b/__tests__/readCurrentMigration.test.ts @@ -106,11 +106,10 @@ With multiple lines it("reads from current.sql, and processes included files", async () => { mockFs({ "migrations/current.sql": "--!include foo_current.sql", - "migrations/fixtures/foo_current.sql": "-- TEST from foo" + "migrations/fixtures/foo_current.sql": "-- TEST from foo", }); const currentLocation = await getCurrentMigrationLocation(parsedSettings); const content = await readCurrentMigration(parsedSettings, currentLocation); expect(content).toEqual("-- TEST from foo"); - -}); \ No newline at end of file +}); diff --git a/src/__mocks__/migration.ts b/src/__mocks__/migration.ts index 8ff8109..c5cced4 100644 --- a/src/__mocks__/migration.ts +++ b/src/__mocks__/migration.ts @@ -37,4 +37,6 @@ export const runCommittedMigration = jest.fn( (_client, _settings, _context, _committedMigration, _logSuffix) => {}, ); -export const compileIncludes = jest.fn((parsedSettings, content) => {return content}); +export const compileIncludes = jest.fn((parsedSettings, content) => { + return content; +}); diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 79a76cc..0ce3277 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -218,32 +218,31 @@ export async function _watch( }); }; const watcher = chokidar.watch( - [ - currentLocation.path, - `${parsedSettings.migrationsFolder}/fixtures` - ], { - /* - * Without `usePolling`, on Linux, you can prevent the watching from - * working by issuing `git stash && sleep 2 && git stash pop`. This is - * annoying. - */ - usePolling: true, + [currentLocation.path, `${parsedSettings.migrationsFolder}/fixtures`], + { + /* + * Without `usePolling`, on Linux, you can prevent the watching from + * working by issuing `git stash && sleep 2 && git stash pop`. This is + * annoying. + */ + usePolling: true, - /* - * Some editors stream the writes out a little at a time, we want to wait - * for the write to finish before triggering. - */ - awaitWriteFinish: { - stabilityThreshold: 200, - pollInterval: 100, - }, + /* + * Some editors stream the writes out a little at a time, we want to wait + * for the write to finish before triggering. + */ + awaitWriteFinish: { + stabilityThreshold: 200, + pollInterval: 100, + }, - /* - * We don't want to run the queue too many times during startup; so we - * call it once on the 'ready' event. - */ - ignoreInitial: true, - }); + /* + * We don't want to run the queue too many times during startup; so we + * call it once on the 'ready' event. + */ + ignoreInitial: true, + }, + ); watcher.on("add", queue); watcher.on("change", queue); watcher.on("unlink", queue); diff --git a/src/migration.ts b/src/migration.ts index 98fed55..4b0f775 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -118,11 +118,15 @@ export function compilePlaceholders( )(content); } -export async function compileIncludes(parsedSettings: ParsedSettings, content: string, processedFiles: Array = []): Promise { +export async function compileIncludes( + parsedSettings: ParsedSettings, + content: string, + processedFiles: Array = [], +): Promise { const regex = /--!include (.*.sql)/g; let compiledContent = content; let match = regex.exec(content); - const includePath = `${parsedSettings.migrationsFolder}/fixtures/` + const includePath = `${parsedSettings.migrationsFolder}/fixtures/`; let realPath; //if the fixtures folder isn't defined, catch the error and return the original content. @@ -130,12 +134,12 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s realPath = await fsp.realpath(includePath); } catch (e) { if (!realPath) { - parsedSettings.logger.warn(`Warning: ${includePath} is not defined.`) + parsedSettings.logger.warn(`Warning: ${includePath} is not defined.`); return content; } } - if(match) { + if (match) { while (match != null) { //make sure the include path starts with the real path of the fixtures folder. let includeRegex; @@ -145,28 +149,36 @@ export async function compileIncludes(parsedSettings: ParsedSettings, content: s includeRegex = new RegExp(`^${realPath}`); includeRealPath = await fsp.realpath(`${includePath}${match[1]}`); } catch (e) { - throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); + throw new Error( + `include path not in ${parsedSettings.migrationsFolder}/fixtures/`, + ); } - if(includeRegex.exec(includeRealPath) === null) { - throw new Error(`include path not in ${parsedSettings.migrationsFolder}/fixtures/`); + if (includeRegex.exec(includeRealPath) === null) { + throw new Error( + `include path not in ${parsedSettings.migrationsFolder}/fixtures/`, + ); } //If we've already processed this file, skip it (prevents infinite chains) - if(!processedFiles.includes(includeRealPath)){ + if (!processedFiles.includes(includeRealPath)) { processedFiles.push(includeRealPath); const fileContents = await fsp.readFile(includeRealPath, "utf8"); - compiledContent = compiledContent.replace(match[0], fileContents) + compiledContent = compiledContent.replace(match[0], fileContents); match = regex.exec(content); } else { //remove recursive include and continue - compiledContent = compiledContent.replace(match[0], ''); + compiledContent = compiledContent.replace(match[0], ""); match = regex.exec(content); } } //recursively call compileIncludes to catch includes in the included files. - return await compileIncludes(parsedSettings, compiledContent, processedFiles); + return await compileIncludes( + parsedSettings, + compiledContent, + processedFiles, + ); } else { return compiledContent; } From 83c4f85d50f38e613c5f7828235a5d0ac0fee168 Mon Sep 17 00:00:00 2001 From: John Barlow Date: Wed, 3 Jan 2024 17:09:54 -0500 Subject: [PATCH 17/32] Update src/current.ts Co-authored-by: Benjie --- src/current.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/current.ts b/src/current.ts index f4ff607..ede4c23 100644 --- a/src/current.ts +++ b/src/current.ts @@ -186,7 +186,7 @@ export async function readCurrentMigration( wholeBody = headerLines.join("\n") + "\n\n" + wholeBody; } - return await compileIncludes(_parsedSettings, wholeBody); + return compileIncludes(_parsedSettings, wholeBody); } } From b7fed69fbd9a37e5ffb194c32bf4fcda1b81401a Mon Sep 17 00:00:00 2001 From: John Barlow Date: Wed, 3 Jan 2024 17:10:17 -0500 Subject: [PATCH 18/32] Update src/current.ts Co-authored-by: Benjie --- src/current.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/current.ts b/src/current.ts index ede4c23..085032a 100644 --- a/src/current.ts +++ b/src/current.ts @@ -110,7 +110,7 @@ export async function readCurrentMigration( const content = await readFileOrNull(location.path); // If file doesn't exist, treat it as if it were empty. - return await compileIncludes(_parsedSettings, content || ""); + return compileIncludes(_parsedSettings, content || ""); } else { const files = await fsp.readdir(location.path); const parts = new Map< From ce63096be86882403d07d35b774f706231e574ff Mon Sep 17 00:00:00 2001 From: John Barlow Date: Wed, 3 Jan 2024 17:17:37 -0500 Subject: [PATCH 19/32] issue-163: added globstar to fixtures watch. --- src/commands/watch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 0ce3277..3ab9bfc 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -218,7 +218,7 @@ export async function _watch( }); }; const watcher = chokidar.watch( - [currentLocation.path, `${parsedSettings.migrationsFolder}/fixtures`], + [currentLocation.path, `${parsedSettings.migrationsFolder}/fixtures/**`], { /* * Without `usePolling`, on Linux, you can prevent the watching from From b2f2b6eb86cfb8ac71314e3c249bb3c5939d616d Mon Sep 17 00:00:00 2001 From: John Barlow Date: Wed, 3 Jan 2024 19:19:57 -0500 Subject: [PATCH 20/32] issue-163: fixed watch glob, fixed premature regex escaping of content. --- __tests__/include.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/commands/watch.ts | 5 ++++- src/migration.ts | 5 ++++- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index a72bf29..80bec5b 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -96,3 +96,42 @@ it("disallows calling files outside of the migrations/fixtures folder", async () ), ).rejects.toThrow(); }); + +it("compiles an included file that contains escapable things", async () => { + mockFs({ + "migrations/fixtures/foo.sql": `\ +begin; + +create or replace function current_user_id() returns uuid as $$ + select nullif(current_setting('user.id', true)::text, '')::uuid; +$$ language sql stable; + +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; +`, + }); + expect( + await compileIncludes( + settings, + `\ +--!include foo.sql +`, + ), + ).toEqual(`\ +begin; + +create or replace function current_user_id() returns uuid as $$ + select nullif(current_setting('user.id', true)::text, '')::uuid; +$$ language sql stable; + +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; + +`); +}); diff --git a/src/commands/watch.ts b/src/commands/watch.ts index 3ab9bfc..b52e945 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -218,7 +218,10 @@ export async function _watch( }); }; const watcher = chokidar.watch( - [currentLocation.path, `${parsedSettings.migrationsFolder}/fixtures/**`], + [ + currentLocation.path, + `${parsedSettings.migrationsFolder}/fixtures/**/*`, + ], { /* * Without `usePolling`, on Linux, you can prevent the watching from diff --git a/src/migration.ts b/src/migration.ts index 4b0f775..4012408 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -164,7 +164,10 @@ export async function compileIncludes( if (!processedFiles.includes(includeRealPath)) { processedFiles.push(includeRealPath); const fileContents = await fsp.readFile(includeRealPath, "utf8"); - compiledContent = compiledContent.replace(match[0], fileContents); + compiledContent = compiledContent.replace( + match[0], + fileContents.replace(/\$/g, "$$$$"), + ); match = regex.exec(content); } else { //remove recursive include and continue From bddfcce8c32d0de2f9002f818fb76384ce340a6d Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 13:59:17 +0000 Subject: [PATCH 21/32] Wider 'pg' range --- package.json | 6 ++--- yarn.lock | 73 +++++++++++----------------------------------------- 2 files changed, 18 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index ec4a026..5dfd46b 100644 --- a/package.json +++ b/package.json @@ -42,12 +42,12 @@ "dependencies": { "@graphile/logger": "^0.2.0", "@types/json5": "^2.2.0", - "@types/node": "^20.11.5", - "@types/pg": "^8.10.9", + "@types/node": "^18", + "@types/pg": ">=6 <9", "chalk": "^4", "chokidar": "^3.5.3", "json5": "^2.2.3", - "pg": "^8.11.3", + "pg": ">=6.5 <9", "pg-connection-string": "^2.6.2", "pg-minify": "^1.6.3", "tslib": "^2.6.2", diff --git a/yarn.lock b/yarn.lock index 1039154..47b3c25 100644 --- a/yarn.lock +++ b/yarn.lock @@ -744,26 +744,33 @@ dependencies: "@types/node" "*" -"@types/node@*", "@types/node@^20.11.5": +"@types/node@*": version "20.11.5" resolved "https://registry.yarnpkg.com/@types/node/-/node-20.11.5.tgz#be10c622ca7fcaa3cf226cf80166abc31389d86e" integrity sha512-g557vgQjUUfN76MZAN/dt1z3dzcUsimuysco0KeluHgrPdJXkP/XdAURgyO2W9fZWHRtRBiVKzKn8vyOAwlG+w== dependencies: undici-types "~5.26.4" +"@types/node@^18": + version "18.19.8" + resolved "https://registry.yarnpkg.com/@types/node/-/node-18.19.8.tgz#c1e42b165e5a526caf1f010747e0522cb2c9c36a" + integrity sha512-g1pZtPhsvGVTwmeVoexWZLTQaOvXwoSq//pTL0DHeNzUDrFnir4fgETdhjhIxjVnN+hKOuh98+E1eMLnUXstFg== + dependencies: + undici-types "~5.26.4" + "@types/parse-json@^4.0.0": version "4.0.2" resolved "https://registry.yarnpkg.com/@types/parse-json/-/parse-json-4.0.2.tgz#5950e50960793055845e956c427fc2b0d70c5239" integrity sha512-dISoDXWWQwUquiKsyZ4Ng+HX2KsPL7LyHKHQwgGFEA3IaKac4Obd+h2a/a6waisAoepJlBcx9paWqjA8/HVjCw== -"@types/pg@^8.10.9": - version "8.10.9" - resolved "https://registry.yarnpkg.com/@types/pg/-/pg-8.10.9.tgz#d20bb948c6268c5bd847e2bf968f1194c5a2355a" - integrity sha512-UksbANNE/f8w0wOMxVKKIrLCbEMV+oM1uKejmwXr39olg4xqcfBDbXxObJAt6XxHbDa4XTKOlUEcEltXDX+XLQ== +"@types/pg@>=6 <9": + version "8.6.0" + resolved "https://registry.yarnpkg.com/@types/pg/-/pg-8.6.0.tgz#34233b891a127d6caaad28e177b1baec1a2958d4" + integrity sha512-3JXFrsl8COoqVB1+2Pqelx6soaiFVXzkT3fkuSNe7GB40ysfT0FHphZFPiqIXpMyTHSFRdLTyZzrFBrJRPAArA== dependencies: "@types/node" "*" pg-protocol "*" - pg-types "^4.0.1" + pg-types "^2.2.0" "@types/semver@^7.3.12", "@types/semver@^7.5.0": version "7.5.6" @@ -3155,11 +3162,6 @@ object.values@^1.1.7: define-properties "^1.2.0" es-abstract "^1.22.1" -obuf@~1.1.2: - version "1.1.2" - resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e" - integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg== - once@^1.3.0: version "1.4.0" resolved "https://registry.yarnpkg.com/once/-/once-1.4.0.tgz#583b1aa775961d4b113ac17d9c50baef9dd76bd1" @@ -3291,17 +3293,12 @@ pg-minify@^1.6.3: resolved "https://registry.yarnpkg.com/pg-minify/-/pg-minify-1.6.3.tgz#3def4c876a2d258da20cfdb0e387373d41c7a4dc" integrity sha512-NoSsPqXxbkD8RIe+peQCqiea4QzXgosdTKY8p7PsbbGsh2F8TifDj/vJxfuR8qJwNYrijdSs7uf0tAe6WOyCsQ== -pg-numeric@1.0.2: - version "1.0.2" - resolved "https://registry.yarnpkg.com/pg-numeric/-/pg-numeric-1.0.2.tgz#816d9a44026086ae8ae74839acd6a09b0636aa3a" - integrity sha512-BM/Thnrw5jm2kKLE5uJkXqqExRUY/toLHda65XgFTBTFYZyopbKjBe29Ii3RbkvlsMoFwD+tHeGaCjjv0gHlyw== - pg-pool@^3.6.1: version "3.6.1" resolved "https://registry.yarnpkg.com/pg-pool/-/pg-pool-3.6.1.tgz#5a902eda79a8d7e3c928b77abf776b3cb7d351f7" integrity sha512-jizsIzhkIitxCGfPRzJn1ZdcosIt3pz9Sh3V01fm1vZnbnCMgmGl5wvGGdNN2EL9Rmb0EcFoCkixH4Pu+sP9Og== -pg-protocol@*, pg-protocol@^1.6.0: +pg-protocol@^1.6.0: version "1.6.0" resolved "https://registry.yarnpkg.com/pg-protocol/-/pg-protocol-1.6.0.tgz#4c91613c0315349363af2084608db843502f8833" integrity sha512-M+PDm637OY5WM307051+bsDia5Xej6d9IR4GwJse1qA1DIhiKlksvrneZOYQq42OM+spubpcNYEo2FcKQrDk+Q== @@ -3317,20 +3314,7 @@ pg-types@^2.1.0: postgres-date "~1.0.4" postgres-interval "^1.1.0" -pg-types@^4.0.1: - version "4.0.1" - resolved "https://registry.yarnpkg.com/pg-types/-/pg-types-4.0.1.tgz#31857e89d00a6c66b06a14e907c3deec03889542" - integrity sha512-hRCSDuLII9/LE3smys1hRHcu5QGcLs9ggT7I/TCs0IE+2Eesxi9+9RWAAwZ0yaGjxoWICF/YHLOEjydGujoJ+g== - dependencies: - pg-int8 "1.0.1" - pg-numeric "1.0.2" - postgres-array "~3.0.1" - postgres-bytea "~3.0.0" - postgres-date "~2.0.1" - postgres-interval "^3.0.0" - postgres-range "^1.1.1" - -pg@^8.11.3: +"pg@>=6.5 <9": version "8.11.3" resolved "https://registry.yarnpkg.com/pg/-/pg-8.11.3.tgz#d7db6e3fe268fcedd65b8e4599cda0b8b4bf76cb" integrity sha512-+9iuvG8QfaaUrrph+kpF24cXkH1YOOUeArRNYIxq1viYHZagBxrTno7cecY1Fa44tJeZvaoG+Djpkc3JwehN5g== @@ -3395,33 +3379,16 @@ postgres-array@~2.0.0: resolved "https://registry.yarnpkg.com/postgres-array/-/postgres-array-2.0.0.tgz#48f8fce054fbc69671999329b8834b772652d82e" integrity sha512-VpZrUqU5A69eQyW2c5CA1jtLecCsN2U/bD6VilrFDWq5+5UIEVO7nazS3TEcHf1zuPYO/sqGvUvW62g86RXZuA== -postgres-array@~3.0.1: - version "3.0.2" - resolved "https://registry.yarnpkg.com/postgres-array/-/postgres-array-3.0.2.tgz#68d6182cb0f7f152a7e60dc6a6889ed74b0a5f98" - integrity sha512-6faShkdFugNQCLwucjPcY5ARoW1SlbnrZjmGl0IrrqewpvxvhSLHimCVzqeuULCbG0fQv7Dtk1yDbG3xv7Veog== - postgres-bytea@~1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/postgres-bytea/-/postgres-bytea-1.0.0.tgz#027b533c0aa890e26d172d47cf9ccecc521acd35" integrity sha512-xy3pmLuQqRBZBXDULy7KbaitYqLcmxigw14Q5sj8QBVLqEwXfeybIKVWiqAXTlcvdvb0+xkOtDbfQMOf4lST1w== -postgres-bytea@~3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/postgres-bytea/-/postgres-bytea-3.0.0.tgz#9048dc461ac7ba70a6a42d109221619ecd1cb089" - integrity sha512-CNd4jim9RFPkObHSjVHlVrxoVQXz7quwNFpz7RY1okNNme49+sVyiTvTRobiLV548Hx/hb1BG+iE7h9493WzFw== - dependencies: - obuf "~1.1.2" - postgres-date@~1.0.4: version "1.0.7" resolved "https://registry.yarnpkg.com/postgres-date/-/postgres-date-1.0.7.tgz#51bc086006005e5061c591cee727f2531bf641a8" integrity sha512-suDmjLVQg78nMK2UZ454hAG+OAW+HQPZ6n++TNDUX+L0+uUlLywnoxJKDou51Zm+zTCjrCl0Nq6J9C5hP9vK/Q== -postgres-date@~2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/postgres-date/-/postgres-date-2.0.1.tgz#638b62e5c33764c292d37b08f5257ecb09231457" - integrity sha512-YtMKdsDt5Ojv1wQRvUhnyDJNSr2dGIC96mQVKz7xufp07nfuFONzdaowrMHjlAzY6GDLd4f+LUHHAAM1h4MdUw== - postgres-interval@^1.1.0: version "1.2.0" resolved "https://registry.yarnpkg.com/postgres-interval/-/postgres-interval-1.2.0.tgz#b460c82cb1587507788819a06aa0fffdb3544695" @@ -3429,16 +3396,6 @@ postgres-interval@^1.1.0: dependencies: xtend "^4.0.0" -postgres-interval@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/postgres-interval/-/postgres-interval-3.0.0.tgz#baf7a8b3ebab19b7f38f07566c7aab0962f0c86a" - integrity sha512-BSNDnbyZCXSxgA+1f5UU2GmwhoI0aU5yMxRGO8CdFEcY2BQF9xm/7MqKnYoM1nJDk8nONNWDk9WeSmePFhQdlw== - -postgres-range@^1.1.1: - version "1.1.3" - resolved "https://registry.yarnpkg.com/postgres-range/-/postgres-range-1.1.3.tgz#9ccd7b01ca2789eb3c2e0888b3184225fa859f76" - integrity sha512-VdlZoocy5lCP0c/t66xAfclglEapXPCIVhqqJRncYpvbCgImF0w67aPKfbqUMr72tO2k5q0TdTZwCLjPTI6C9g== - prelude-ls@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.2.1.tgz#debc6489d7a6e6b0e7611888cec880337d316396" From 82a2315cdb5666deac99daf4650d4784f5f72c7d Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 14:01:11 +0000 Subject: [PATCH 22/32] Using variable now --- src/current.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/current.ts b/src/current.ts index 085032a..339ea7e 100644 --- a/src/current.ts +++ b/src/current.ts @@ -103,14 +103,14 @@ function idFromFilename(file: string): number { } export async function readCurrentMigration( - _parsedSettings: ParsedSettings, + parsedSettings: ParsedSettings, location: CurrentMigrationLocation, ): Promise { if (location.isFile) { const content = await readFileOrNull(location.path); // If file doesn't exist, treat it as if it were empty. - return compileIncludes(_parsedSettings, content || ""); + return compileIncludes(parsedSettings, content || ""); } else { const files = await fsp.readdir(location.path); const parts = new Map< @@ -186,7 +186,7 @@ export async function readCurrentMigration( wholeBody = headerLines.join("\n") + "\n\n" + wholeBody; } - return compileIncludes(_parsedSettings, wholeBody); + return compileIncludes(parsedSettings, wholeBody); } } From 68d4fbd7e3a010ae5f7dc7405c5642727d4e39bb Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 15:09:08 +0000 Subject: [PATCH 23/32] Refactor the include code --- src/migration.ts | 124 ++++++++++++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/src/migration.ts b/src/migration.ts index 4012408..cb97ba5 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -1,4 +1,5 @@ import { promises as fsp } from "fs"; +import { relative } from "path"; import { VALID_FILE_REGEX } from "./current"; import { calculateHash } from "./hash"; @@ -118,73 +119,86 @@ export function compilePlaceholders( )(content); } +async function realpathOrNull(path: string): Promise { + try { + return await fsp.realpath(path); + } catch (e) { + return null; + } +} + export async function compileIncludes( parsedSettings: ParsedSettings, content: string, - processedFiles: Array = [], + processedFiles: ReadonlySet = new Set(), ): Promise { - const regex = /--!include (.*.sql)/g; - let compiledContent = content; - let match = regex.exec(content); - const includePath = `${parsedSettings.migrationsFolder}/fixtures/`; - let realPath; + const regex = /^--!include\s+(.*\.sql)\s*$/gm; - //if the fixtures folder isn't defined, catch the error and return the original content. - try { - realPath = await fsp.realpath(includePath); - } catch (e) { - if (!realPath) { - parsedSettings.logger.warn(`Warning: ${includePath} is not defined.`); - return content; - } - } + // Don't need to validate this unless an include happens. MUST end in a `/` + const fixturesPath = `${parsedSettings.migrationsFolder}/fixtures/`; - if (match) { - while (match != null) { - //make sure the include path starts with the real path of the fixtures folder. - let includeRegex; - let includeRealPath; + // Find all includes in this `content` + const matches = content.matchAll(regex); - try { - includeRegex = new RegExp(`^${realPath}`); - includeRealPath = await fsp.realpath(`${includePath}${match[1]}`); - } catch (e) { - throw new Error( - `include path not in ${parsedSettings.migrationsFolder}/fixtures/`, - ); - } + // Go through these matches and resolve their full paths, checking they are allowed + const sqlPathByRawSqlPath: Record = Object.create(null); + for (const match of matches) { + const [, rawSqlPath] = match; + const sqlPath = await realpathOrNull(`${fixturesPath}${rawSqlPath}`); - if (includeRegex.exec(includeRealPath) === null) { - throw new Error( - `include path not in ${parsedSettings.migrationsFolder}/fixtures/`, - ); - } + if (!sqlPath) { + throw new Error( + `Include of '${rawSqlPath}' failed because '${fixturesPath}${rawSqlPath}' doesn't seem to exist?`, + ); + } - //If we've already processed this file, skip it (prevents infinite chains) - if (!processedFiles.includes(includeRealPath)) { - processedFiles.push(includeRealPath); - const fileContents = await fsp.readFile(includeRealPath, "utf8"); - compiledContent = compiledContent.replace( - match[0], - fileContents.replace(/\$/g, "$$$$"), - ); - match = regex.exec(content); - } else { - //remove recursive include and continue - compiledContent = compiledContent.replace(match[0], ""); - match = regex.exec(content); - } + if (processedFiles.has(sqlPath)) { + throw new Error( + `Circular include detected - '${sqlPath}' is included again! Trace:\n ${[...processedFiles].reverse().join("\n ")}`, + ); } - //recursively call compileIncludes to catch includes in the included files. - return await compileIncludes( - parsedSettings, - compiledContent, - processedFiles, - ); - } else { - return compiledContent; + const relativePath = relative(fixturesPath, sqlPath); + if (relativePath.startsWith("..")) { + throw new Error( + `Forbidden: cannot include path '${sqlPath}' because it's not inside '${fixturesPath}'`, + ); + } + + // Looks good to me + sqlPathByRawSqlPath[rawSqlPath] = sqlPath; } + + // For the unique set of paths, load the file and then recursively do its own includes + const distinctSqlPaths = [...new Set(Object.values(sqlPathByRawSqlPath))]; + const contentsForDistinctSqlPaths = await Promise.all( + distinctSqlPaths.map(async (sqlPath) => { + const fileContents = await fsp.readFile(sqlPath, "utf8"); + const processed = await compileIncludes( + parsedSettings, + fileContents, + new Set([...processedFiles, sqlPath]), + ); + return processed; + }), + ); + + // Turn the results into a map for ease of lookup + const contentBySqlPath: Record = Object.create(null); + for (let i = 0, l = distinctSqlPaths.length; i < l; i++) { + const sqlPath = distinctSqlPaths[i]; + const content = contentsForDistinctSqlPaths[i]; + contentBySqlPath[sqlPath] = content; + } + + // Simple string replacement for each path matched + const compiledContent = content.replace(regex, (_match, rawSqlPath) => { + const sqlPath = sqlPathByRawSqlPath[rawSqlPath]; + const content = contentBySqlPath[sqlPath]; + return content; + }); + + return compiledContent; } const TABLE_CHECKS = { From da4bed9b9a14da029d528359ab75ea097b52d7dd Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 15:29:29 +0000 Subject: [PATCH 24/32] Fix lint issues --- src/migration.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/migration.ts b/src/migration.ts index cb97ba5..7b2926f 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -130,7 +130,7 @@ async function realpathOrNull(path: string): Promise { export async function compileIncludes( parsedSettings: ParsedSettings, content: string, - processedFiles: ReadonlySet = new Set(), + processedFiles: ReadonlySet = new Set(), ): Promise { const regex = /^--!include\s+(.*\.sql)\s*$/gm; @@ -141,7 +141,7 @@ export async function compileIncludes( const matches = content.matchAll(regex); // Go through these matches and resolve their full paths, checking they are allowed - const sqlPathByRawSqlPath: Record = Object.create(null); + const sqlPathByRawSqlPath = Object.create(null) as Record; for (const match of matches) { const [, rawSqlPath] = match; const sqlPath = await realpathOrNull(`${fixturesPath}${rawSqlPath}`); @@ -184,7 +184,7 @@ export async function compileIncludes( ); // Turn the results into a map for ease of lookup - const contentBySqlPath: Record = Object.create(null); + const contentBySqlPath = Object.create(null) as Record; for (let i = 0, l = distinctSqlPaths.length; i < l; i++) { const sqlPath = distinctSqlPaths[i]; const content = contentsForDistinctSqlPaths[i]; @@ -192,11 +192,14 @@ export async function compileIncludes( } // Simple string replacement for each path matched - const compiledContent = content.replace(regex, (_match, rawSqlPath) => { - const sqlPath = sqlPathByRawSqlPath[rawSqlPath]; - const content = contentBySqlPath[sqlPath]; - return content; - }); + const compiledContent = content.replace( + regex, + (_match, rawSqlPath: string) => { + const sqlPath = sqlPathByRawSqlPath[rawSqlPath]; + const content = contentBySqlPath[sqlPath]; + return content; + }, + ); return compiledContent; } From 642be0d0ec80ec4e4435b9a891f44beef6edd71b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 15:46:45 +0000 Subject: [PATCH 25/32] Fix whitespace bug --- src/migration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migration.ts b/src/migration.ts index 7b2926f..10e08e1 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -132,7 +132,7 @@ export async function compileIncludes( content: string, processedFiles: ReadonlySet = new Set(), ): Promise { - const regex = /^--!include\s+(.*\.sql)\s*$/gm; + const regex = /^--!include[ \t]+(.*\.sql)[ \t]*$/gm; // Don't need to validate this unless an include happens. MUST end in a `/` const fixturesPath = `${parsedSettings.migrationsFolder}/fixtures/`; From e5598459e492a19839a93042529c52cf6e752be1 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 15:56:59 +0000 Subject: [PATCH 26/32] Fix test and enforce full stack trace --- __tests__/include.test.ts | 17 ++++++++++------- src/current.ts | 15 ++++++++++++--- src/migration.ts | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index 80bec5b..19c7efd 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -1,6 +1,6 @@ import "./helpers"; -import * as mockFs from "mock-fs"; +import mockFs from "mock-fs"; import { compileIncludes } from "../src/migration"; import { ParsedSettings, parseSettings } from "../src/settings"; @@ -36,6 +36,7 @@ it("compiles an included file", async () => { `\ --!include foo.sql `, + new Set(["current.sql"]), ), ).toEqual(`\ select * from foo; @@ -57,6 +58,7 @@ it("compiles multiple included files", async () => { --!include dir2/bar.sql --!include dir3/baz.sql `, + new Set(["current.sql"]), ), ).toEqual(`\ select * from foo; @@ -67,18 +69,17 @@ select * from qux; it("compiles an included file, and won't get stuck in an infinite include loop", async () => { mockFs({ - "migrations/fixtures/foo.sql": "select * from foo;--!include foo.sql", + "migrations/fixtures/foo.sql": "select * from foo;\n--!include foo.sql", }); - expect( - await compileIncludes( + await expect( + compileIncludes( settings, `\ --!include foo.sql `, + new Set(["current.sql"]), ), - ).toEqual(`\ -select * from foo; -`); + ).rejects.toThrowError(/Circular include/); }); it("disallows calling files outside of the migrations/fixtures folder", async () => { @@ -93,6 +94,7 @@ it("disallows calling files outside of the migrations/fixtures folder", async () `\ --!include ../../outsideFolder/foo.sql `, + new Set(["current.sql"]), ), ).rejects.toThrow(); }); @@ -119,6 +121,7 @@ commit; `\ --!include foo.sql `, + new Set(["current.sql"]), ), ).toEqual(`\ begin; diff --git a/src/current.ts b/src/current.ts index 339ea7e..e9f7817 100644 --- a/src/current.ts +++ b/src/current.ts @@ -110,7 +110,11 @@ export async function readCurrentMigration( const content = await readFileOrNull(location.path); // If file doesn't exist, treat it as if it were empty. - return compileIncludes(parsedSettings, content || ""); + return compileIncludes( + parsedSettings, + content || "", + new Set(location.path), + ); } else { const files = await fsp.readdir(location.path); const parts = new Map< @@ -160,7 +164,12 @@ export async function readCurrentMigration( for (const id of ids) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const { file, filePath, bodyPromise } = parts.get(id)!; - const contents = await bodyPromise; + const rawContents = await bodyPromise; + const contents = await compileIncludes( + parsedSettings, + rawContents, + new Set([filePath]), + ); const { body, headers } = parseMigrationText(filePath, contents, false); headerses.push(headers); if (isNoTransactionDefined(body)) { @@ -186,7 +195,7 @@ export async function readCurrentMigration( wholeBody = headerLines.join("\n") + "\n\n" + wholeBody; } - return compileIncludes(parsedSettings, wholeBody); + return wholeBody; } } diff --git a/src/migration.ts b/src/migration.ts index 10e08e1..8060cf7 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -130,7 +130,7 @@ async function realpathOrNull(path: string): Promise { export async function compileIncludes( parsedSettings: ParsedSettings, content: string, - processedFiles: ReadonlySet = new Set(), + processedFiles: ReadonlySet, ): Promise { const regex = /^--!include[ \t]+(.*\.sql)[ \t]*$/gm; From 47d0b33b475f3b6e62020f45d7e2ee6afae2ddce Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:08:22 +0000 Subject: [PATCH 27/32] Better error tests --- __tests__/__snapshots__/include.test.ts.snap | 9 +++++ __tests__/include.test.ts | 39 +++++++++++--------- src/migration.ts | 2 +- 3 files changed, 32 insertions(+), 18 deletions(-) create mode 100644 __tests__/__snapshots__/include.test.ts.snap diff --git a/__tests__/__snapshots__/include.test.ts.snap b/__tests__/__snapshots__/include.test.ts.snap new file mode 100644 index 0000000..d19c402 --- /dev/null +++ b/__tests__/__snapshots__/include.test.ts.snap @@ -0,0 +1,9 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`compiles an included file, and won't get stuck in an infinite include loop 1`] = ` +"Circular include detected - '~/migrations/fixtures/foo.sql' is included again! Trace: + ~/migrations/fixtures/foo.sql + ~/migrations/current.sql" +`; + +exports[`disallows calling files outside of the migrations/fixtures folder 1`] = `"Forbidden: cannot include path '~/outsideFolder/foo.sql' because it's not inside 'migrations/fixtures/'"`; diff --git a/__tests__/include.test.ts b/__tests__/include.test.ts index 19c7efd..adafe55 100644 --- a/__tests__/include.test.ts +++ b/__tests__/include.test.ts @@ -26,6 +26,9 @@ afterEach(() => { mockFs.restore(); }); +/** Pretents that our compiled files are 'current.sql' */ +const FAKE_VISITED = new Set([`${process.cwd()}/migrations/current.sql`]); + it("compiles an included file", async () => { mockFs({ "migrations/fixtures/foo.sql": "select * from foo;", @@ -36,7 +39,7 @@ it("compiles an included file", async () => { `\ --!include foo.sql `, - new Set(["current.sql"]), + FAKE_VISITED, ), ).toEqual(`\ select * from foo; @@ -58,7 +61,7 @@ it("compiles multiple included files", async () => { --!include dir2/bar.sql --!include dir3/baz.sql `, - new Set(["current.sql"]), + FAKE_VISITED, ), ).toEqual(`\ select * from foo; @@ -71,15 +74,16 @@ it("compiles an included file, and won't get stuck in an infinite include loop", mockFs({ "migrations/fixtures/foo.sql": "select * from foo;\n--!include foo.sql", }); - await expect( - compileIncludes( - settings, - `\ + const promise = compileIncludes( + settings, + `\ --!include foo.sql `, - new Set(["current.sql"]), - ), - ).rejects.toThrowError(/Circular include/); + FAKE_VISITED, + ); + await expect(promise).rejects.toThrowError(/Circular include/); + const message = await promise.catch((e) => e.message); + expect(message.replaceAll(process.cwd(), "~")).toMatchSnapshot(); }); it("disallows calling files outside of the migrations/fixtures folder", async () => { @@ -88,15 +92,16 @@ it("disallows calling files outside of the migrations/fixtures folder", async () "outsideFolder/foo.sql": "select * from foo;", }); - await expect( - compileIncludes( - settings, - `\ + const promise = compileIncludes( + settings, + `\ --!include ../../outsideFolder/foo.sql `, - new Set(["current.sql"]), - ), - ).rejects.toThrow(); + FAKE_VISITED, + ); + await expect(promise).rejects.toThrowError(/Forbidden: cannot include/); + const message = await promise.catch((e) => e.message); + expect(message.replaceAll(process.cwd(), "~")).toMatchSnapshot(); }); it("compiles an included file that contains escapable things", async () => { @@ -121,7 +126,7 @@ commit; `\ --!include foo.sql `, - new Set(["current.sql"]), + FAKE_VISITED, ), ).toEqual(`\ begin; diff --git a/src/migration.ts b/src/migration.ts index 8060cf7..2c0c511 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -154,7 +154,7 @@ export async function compileIncludes( if (processedFiles.has(sqlPath)) { throw new Error( - `Circular include detected - '${sqlPath}' is included again! Trace:\n ${[...processedFiles].reverse().join("\n ")}`, + `Circular include detected - '${sqlPath}' is included again! Trace:\n ${[...processedFiles].reverse().join("\n ")}`, ); } From b28b4fd7720ccd958463bfc8ece94853d06f6c25 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:11:27 +0000 Subject: [PATCH 28/32] realPath the fixtures --- __tests__/__snapshots__/include.test.ts.snap | 2 +- src/migration.ts | 23 +++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/__tests__/__snapshots__/include.test.ts.snap b/__tests__/__snapshots__/include.test.ts.snap index d19c402..bb00715 100644 --- a/__tests__/__snapshots__/include.test.ts.snap +++ b/__tests__/__snapshots__/include.test.ts.snap @@ -6,4 +6,4 @@ exports[`compiles an included file, and won't get stuck in an infinite include l ~/migrations/current.sql" `; -exports[`disallows calling files outside of the migrations/fixtures folder 1`] = `"Forbidden: cannot include path '~/outsideFolder/foo.sql' because it's not inside 'migrations/fixtures/'"`; +exports[`disallows calling files outside of the migrations/fixtures folder 1`] = `"Forbidden: cannot include path '~/outsideFolder/foo.sql' because it's not inside '~/migrations/fixtures'"`; diff --git a/src/migration.ts b/src/migration.ts index 2c0c511..948317c 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -134,21 +134,32 @@ export async function compileIncludes( ): Promise { const regex = /^--!include[ \t]+(.*\.sql)[ \t]*$/gm; - // Don't need to validate this unless an include happens. MUST end in a `/` - const fixturesPath = `${parsedSettings.migrationsFolder}/fixtures/`; - // Find all includes in this `content` - const matches = content.matchAll(regex); + const matches = [...content.matchAll(regex)]; + + // There's no includes + if (matches.length === 0) { + return content; + } + + // Since there's at least one include, we need the fixtures path: + const rawFixturesPath = `${parsedSettings.migrationsFolder}/fixtures`; + const fixturesPath = await realpathOrNull(rawFixturesPath); + if (!fixturesPath) { + throw new Error( + `File contains '--!include' but fixtures folder '${rawFixturesPath}' doesn't exist?`, + ); + } // Go through these matches and resolve their full paths, checking they are allowed const sqlPathByRawSqlPath = Object.create(null) as Record; for (const match of matches) { const [, rawSqlPath] = match; - const sqlPath = await realpathOrNull(`${fixturesPath}${rawSqlPath}`); + const sqlPath = await realpathOrNull(`${fixturesPath}/${rawSqlPath}`); if (!sqlPath) { throw new Error( - `Include of '${rawSqlPath}' failed because '${fixturesPath}${rawSqlPath}' doesn't seem to exist?`, + `Include of '${rawSqlPath}' failed because '${fixturesPath}/${rawSqlPath}' doesn't seem to exist?`, ); } From a1d9a0203181365ede42a45b4465020081153ee4 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:16:31 +0000 Subject: [PATCH 29/32] No need to glob --- src/commands/watch.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/commands/watch.ts b/src/commands/watch.ts index b52e945..0ce3277 100644 --- a/src/commands/watch.ts +++ b/src/commands/watch.ts @@ -218,10 +218,7 @@ export async function _watch( }); }; const watcher = chokidar.watch( - [ - currentLocation.path, - `${parsedSettings.migrationsFolder}/fixtures/**/*`, - ], + [currentLocation.path, `${parsedSettings.migrationsFolder}/fixtures`], { /* * Without `usePolling`, on Linux, you can prevent the watching from From 8c0ad1b27899eb275f3ad01fe6f676fdb1d438fd Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:17:38 +0000 Subject: [PATCH 30/32] Oops, missed array --- src/current.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/current.ts b/src/current.ts index e9f7817..748598b 100644 --- a/src/current.ts +++ b/src/current.ts @@ -113,7 +113,7 @@ export async function readCurrentMigration( return compileIncludes( parsedSettings, content || "", - new Set(location.path), + new Set([location.path]), ); } else { const files = await fsp.readdir(location.path); From f7768ab17fe10043adde18e965e02c7075a42461 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:20:27 +0000 Subject: [PATCH 31/32] Improve error further --- __tests__/__snapshots__/include.test.ts.snap | 2 +- src/migration.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/__snapshots__/include.test.ts.snap b/__tests__/__snapshots__/include.test.ts.snap index bb00715..13a4422 100644 --- a/__tests__/__snapshots__/include.test.ts.snap +++ b/__tests__/__snapshots__/include.test.ts.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`compiles an included file, and won't get stuck in an infinite include loop 1`] = ` -"Circular include detected - '~/migrations/fixtures/foo.sql' is included again! Trace: +"Circular include detected - '~/migrations/fixtures/foo.sql' is included again! Import statement: \`--!include foo.sql\`; trace: ~/migrations/fixtures/foo.sql ~/migrations/current.sql" `; diff --git a/src/migration.ts b/src/migration.ts index 948317c..11365f3 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -154,7 +154,7 @@ export async function compileIncludes( // Go through these matches and resolve their full paths, checking they are allowed const sqlPathByRawSqlPath = Object.create(null) as Record; for (const match of matches) { - const [, rawSqlPath] = match; + const [line, rawSqlPath] = match; const sqlPath = await realpathOrNull(`${fixturesPath}/${rawSqlPath}`); if (!sqlPath) { @@ -165,7 +165,7 @@ export async function compileIncludes( if (processedFiles.has(sqlPath)) { throw new Error( - `Circular include detected - '${sqlPath}' is included again! Trace:\n ${[...processedFiles].reverse().join("\n ")}`, + `Circular include detected - '${sqlPath}' is included again! Import statement: \`${line}\`; trace:\n ${[...processedFiles].reverse().join("\n ")}`, ); } From 062003f007b6556b29ea4120c5e3f01038893f44 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 18 Jan 2024 16:25:12 +0000 Subject: [PATCH 32/32] Neater imports --- __tests__/commit.test.ts | 2 +- __tests__/uncommit.test.ts | 2 +- __tests__/writeCurrentMigration.test.ts | 2 +- scripts/update-docs.js | 2 +- src/actions.ts | 2 +- src/commands/_common.ts | 3 ++- src/commands/commit.ts | 2 +- src/commands/compile.ts | 2 +- src/commands/init.ts | 2 +- src/commands/run.ts | 2 +- src/commands/uncommit.ts | 2 +- src/migration.ts | 2 +- 12 files changed, 13 insertions(+), 12 deletions(-) diff --git a/__tests__/commit.test.ts b/__tests__/commit.test.ts index 97a3e56..5bf2321 100644 --- a/__tests__/commit.test.ts +++ b/__tests__/commit.test.ts @@ -1,6 +1,6 @@ import "./helpers"; // Has side-effects; must come first -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import mockFs from "mock-fs"; import { commit } from "../src"; diff --git a/__tests__/uncommit.test.ts b/__tests__/uncommit.test.ts index 2087382..a57031c 100644 --- a/__tests__/uncommit.test.ts +++ b/__tests__/uncommit.test.ts @@ -1,6 +1,6 @@ import "./helpers"; // Has side-effects; must come first -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import mockFs from "mock-fs"; import { commit, migrate, uncommit } from "../src"; diff --git a/__tests__/writeCurrentMigration.test.ts b/__tests__/writeCurrentMigration.test.ts index 816205b..b13422a 100644 --- a/__tests__/writeCurrentMigration.test.ts +++ b/__tests__/writeCurrentMigration.test.ts @@ -1,6 +1,6 @@ import "./helpers"; // Has side-effects; must come first -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import mockFs from "mock-fs"; import { diff --git a/scripts/update-docs.js b/scripts/update-docs.js index 5fd4536..7645dfb 100755 --- a/scripts/update-docs.js +++ b/scripts/update-docs.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -const { promises: fsp } = require("fs"); +const fsp = require("fs/promises"); const { spawnSync } = require("child_process"); async function main() { diff --git a/src/actions.ts b/src/actions.ts index 34048c8..bbb1351 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -1,6 +1,6 @@ import { Logger } from "@graphile/logger"; import { exec as rawExec } from "child_process"; -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { parse } from "pg-connection-string"; import { inspect, promisify } from "util"; diff --git a/src/commands/_common.ts b/src/commands/_common.ts index ed9a5af..bfd7f41 100644 --- a/src/commands/_common.ts +++ b/src/commands/_common.ts @@ -1,4 +1,5 @@ -import { constants, promises as fsp } from "fs"; +import { constants } from "fs"; +import * as fsp from "fs/promises"; import * as JSON5 from "json5"; import { resolve } from "path"; import { parse } from "pg-connection-string"; diff --git a/src/commands/commit.ts b/src/commands/commit.ts index 1911624..41bede9 100644 --- a/src/commands/commit.ts +++ b/src/commands/commit.ts @@ -1,5 +1,5 @@ import pgMinify = require("pg-minify"); -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { CommandModule } from "yargs"; import { diff --git a/src/commands/compile.ts b/src/commands/compile.ts index 1bf62bf..2516cf5 100644 --- a/src/commands/compile.ts +++ b/src/commands/compile.ts @@ -1,4 +1,4 @@ -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { CommandModule } from "yargs"; import { compilePlaceholders } from "../migration"; diff --git a/src/commands/init.ts b/src/commands/init.ts index f346e57..b617c30 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -1,4 +1,4 @@ -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { CommandModule } from "yargs"; import { getCurrentMigrationLocation, writeCurrentMigration } from "../current"; diff --git a/src/commands/run.ts b/src/commands/run.ts index f4a065f..7d963c2 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -1,4 +1,4 @@ -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { QueryResultRow } from "pg"; import { CommandModule } from "yargs"; diff --git a/src/commands/uncommit.ts b/src/commands/uncommit.ts index 771e4e5..01fd188 100644 --- a/src/commands/uncommit.ts +++ b/src/commands/uncommit.ts @@ -1,5 +1,5 @@ import pgMinify = require("pg-minify"); -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { CommandModule } from "yargs"; import { diff --git a/src/migration.ts b/src/migration.ts index 11365f3..6d5a26d 100644 --- a/src/migration.ts +++ b/src/migration.ts @@ -1,4 +1,4 @@ -import { promises as fsp } from "fs"; +import * as fsp from "fs/promises"; import { relative } from "path"; import { VALID_FILE_REGEX } from "./current";