Use --! Included blocks on commit so uncommit can undo includes (#211)

Author: benjie

README.md

@@ -796,10 +796,12 @@ and when the migration is committed, watched, run, or compiled, the contents of
 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);
 ```

__tests__/compile.test.ts

@@ -71,7 +71,9 @@ select 2;
     ),
   ).toEqual(`\
 select 1;
+--! Included foo.sql
 select * from foo;
+--! EndIncluded foo.sql
 select 2;
 `);
 });

__tests__/helpers.ts

@@ -266,6 +266,17 @@ export const makeMigrations = (commitMessage?: string) => {
     commitMessage ? `\n--! Message: ${commitMessage}` : ``
   }\n\n${MIGRATION_NOTRX_TEXT.trim()}\n`;
 
+  const MIGRATION_INCLUDED_FIXTURE = "select 42;\n";
+
+  const MIGRATION_INCLUDE_TEXT = `--!include foo.sql`;
+  const MIGRATION_INCLUDE_COMPILED = `--! Included foo.sql\n${MIGRATION_INCLUDED_FIXTURE.trim()}\n--! EndIncluded foo.sql\n`;
+  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.trim()}\n`;
+
   const MIGRATION_MULTIFILE_FILES = {
     "migrations/links/two.sql": "select 2;",
     "migrations/current": {
@@ -308,6 +319,10 @@ select 3;
     MIGRATION_NOTRX_TEXT,
     MIGRATION_NOTRX_HASH,
     MIGRATION_NOTRX_COMMITTED,
+    MIGRATION_INCLUDE_TEXT,
+    MIGRATION_INCLUDE_HASH,
+    MIGRATION_INCLUDE_COMMITTED,
+    MIGRATION_INCLUDED_FIXTURE,
     MIGRATION_MULTIFILE_TEXT,
     MIGRATION_MULTIFILE_HASH,
     MIGRATION_MULTIFILE_COMMITTED,

__tests__/include.test.ts

@@ -42,7 +42,9 @@ it("compiles an included file", async () => {
       FAKE_VISITED,
     ),
   ).toEqual(`\
+--! Included foo.sql
 select * from foo;
+--! EndIncluded foo.sql
 `);
 });
 
@@ -64,9 +66,17 @@ it("compiles multiple included files", async () => {
       FAKE_VISITED,
     ),
   ).toEqual(`\
+--! Included dir1/foo.sql
 select * from foo;
+--! EndIncluded dir1/foo.sql
+--! Included dir2/bar.sql
 select * from bar;
+--! EndIncluded dir2/bar.sql
+--! Included dir3/baz.sql
+--! Included dir4/qux.sql
 select * from qux;
+--! EndIncluded dir4/qux.sql
+--! EndIncluded dir3/baz.sql
 `);
 });
 
@@ -129,6 +139,7 @@ commit;
       FAKE_VISITED,
     ),
   ).toEqual(`\
+--! Included 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;
-
+--! EndIncluded foo.sql
 `);
 });

__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`);
 });

__tests__/uncommit.test.ts

@@ -55,8 +55,11 @@ 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,
+      MIGRATION_INCLUDED_FIXTURE,
     } = makeMigrations(commitMessage);
 
     it("rolls back migration", async () => {
@@ -88,6 +91,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_INCLUDED_FIXTURE,
+      });
+      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`]:
@@ -139,5 +172,54 @@ describe.each([[undefined], ["My Commit Message"]])(
         ),
       ).toEqual(MIGRATION_MULTIFILE_COMMITTED);
     });
+
+    it("supports the same fixture twice", async () => {
+      const current = `\
+--!include fixture2.sql
+select 22;
+--!include fixture2.sql
+`;
+      mockFs({
+        "migrations/fixtures/fixture1.sql": "select 'fixture1';",
+        "migrations/fixtures/fixture2.sql":
+          "select 1;\n--!include fixture1.sql\nselect 2;",
+        [`migrations/committed/000001${commitMessageSlug}.sql`]:
+          MIGRATION_1_COMMITTED,
+        [`migrations/committed/000002${commitMessageSlug}.sql`]:
+          MIGRATION_MULTIFILE_COMMITTED,
+        "migrations/current/1.sql": current,
+      });
+      await migrate(settings);
+      await commit(settings, commitMessage);
+      expect(
+        await fsp.readFile(
+          `migrations/committed/000003${commitMessageSlug}.sql`,
+          "utf8",
+        ),
+      ).toContain(
+        `\
+--! Included fixture2.sql
+select 1;
+--! Included fixture1.sql
+select 'fixture1';
+--! EndIncluded fixture1.sql
+select 2;
+--! EndIncluded fixture2.sql
+select 22;
+--! Included fixture2.sql
+select 1;
+--! Included fixture1.sql
+select 'fixture1';
+--! EndIncluded fixture1.sql
+select 2;
+--! EndIncluded fixture2.sql
+`,
+      );
+      await uncommit(settings);
+
+      expect(await fsp.readFile(`migrations/current/1.sql`, "utf8")).toEqual(
+        (commitMessage ? `--! Message: ${commitMessage}\n\n` : "") + current,
+      );
+    });
   },
 );

src/commands/uncommit.ts

@@ -40,7 +40,16 @@ export async function _uncommit(parsedSettings: ParsedSettings): Promise<void> {
   // Restore current.sql from migration
   const lastMigrationFilepath = lastMigration.fullPath;
   const contents = await fsp.readFile(lastMigrationFilepath, "utf8");
-  const { headers, body } = parseMigrationText(lastMigrationFilepath, contents);
+  const { headers, body: committedBody } = parseMigrationText(
+    lastMigrationFilepath,
+    contents,
+  );
+
+  // Replace included migrations with their `--!include` equivalent
+  const body = committedBody.replace(
+    /^--! Included (?<filename>\S+)$[\s\S]*?^--! EndIncluded \k<filename>$/gm,
+    (_, filename) => `--!include ${filename}`,
+  );
 
   // Drop Hash, Previous and AllowInvalidHash from headers; then write out
   const { Hash, Previous, AllowInvalidHash, ...otherHeaders } = headers;

src/migration.ts

@@ -132,6 +132,11 @@ export async function compileIncludes(
   content: string,
   processedFiles: ReadonlySet<string>,
 ): Promise<string> {
+  if (/--!\s*(End)?Included?/.test(content)) {
+    throw new Error(
+      "`--! Included` / `--! EndIncluded` comments not allowed in user migrations. Use `--!include` instead.",
+    );
+  }
   const regex = /^--![ \t]*include[ \t]+(.*\.sql)[ \t]*$/gm;
 
   // Find all includes in this `content`
@@ -208,7 +213,7 @@ export async function compileIncludes(
     (_match, rawSqlPath: string) => {
       const sqlPath = sqlPathByRawSqlPath[rawSqlPath];
       const content = contentBySqlPath[sqlPath];
-      return content;
+      return `--! Included ${rawSqlPath}\n${content.trim()}\n--! EndIncluded ${rawSqlPath}`;
     },
   );