fix: ensures dbname is always the last arg provided to pg_dump

eslint.config.js

@@ -19,6 +19,7 @@ export default [
       'unicorn/no-array-for-each': 'warn',
       'unicorn/no-array-reduce': 'warn',
       'unicorn/no-empty-file': 'warn',
+      'unicorn/no-useless-undefined': 'warn',
       'unicorn/prefer-event-target': 'warn',
       'unicorn/prefer-spread': 'warn',
     },

src/commands/pg/pull.ts

@@ -60,7 +60,7 @@ export default class Pull extends Command {
     const target = await maybeTunnel(targetIn)
     const exclude = exclusions.map(e => ('--exclude-table-data=' + e)).join(' ')
 
-    const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', ...connArgs(source, true)]
+    const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku']
 
     if (exclude !== '') dumpFlags.push(exclude)
 
@@ -74,7 +74,7 @@ export default class Pull extends Command {
     }
     if (source.password) dumpOptions.env.PGPASSWORD = source.password
 
-    const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', ...connArgs(target)]
+    const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner']
 
     const restoreOptions: childProcess.SpawnOptions & {env: NodeJS.ProcessEnv} = {
       env: {...env},
@@ -83,8 +83,13 @@ export default class Pull extends Command {
     }
     if (target.password) restoreOptions.env.PGPASSWORD = target.password
 
-    const pgDump = childProcess.spawn('pg_dump', dumpFlags, dumpOptions)
-    const pgRestore = childProcess.spawn('pg_restore', restoreFlags, restoreOptions)
+    // Ensures the database name is always the last value in the args.
+    // This is necessary for the windows pg_dump and pg_restore binaries.
+    const dumpArgs = dumpFlags.concat(connArgs(source, true))
+    const restoreArgs = restoreFlags.concat(connArgs(target))
+
+    const pgDump = childProcess.spawn('pg_dump', dumpArgs, dumpOptions)
+    const pgRestore = childProcess.spawn('pg_restore', restoreArgs, restoreOptions)
 
     await spawnPipe(pgDump, pgRestore)
 

src/commands/pg/push.ts

@@ -56,7 +56,7 @@ export default class Push extends Command {
     const target = await maybeTunnel(targetIn)
     const exclude = exclusions.map(e => ('--exclude-table-data=' + e)).join(' ')
 
-    const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', ...connArgs(source, true)]
+    const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku']
 
     if (exclude !== '') dumpFlags.push(exclude)
 
@@ -70,7 +70,7 @@ export default class Push extends Command {
     }
     if (source.password) dumpOptions.env.PGPASSWORD = source.password
 
-    const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', ...connArgs(target)]
+    const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner']
 
     const restoreOptions: childProcess.SpawnOptions & {env: NodeJS.ProcessEnv} = {
       env: {...env},
@@ -79,8 +79,13 @@ export default class Push extends Command {
     }
     if (target.password) restoreOptions.env.PGPASSWORD = target.password
 
-    const pgDump = childProcess.spawn('pg_dump', dumpFlags, dumpOptions)
-    const pgRestore = childProcess.spawn('pg_restore', restoreFlags, restoreOptions)
+    // Ensures the database name is always the last value in the args.
+    // This is necessary for the windows pg_dump and pg_restore binaries.
+    const dumpArgs = dumpFlags.concat(connArgs(source, true))
+    const restoreArgs = restoreFlags.concat(connArgs(target))
+
+    const pgDump = childProcess.spawn('pg_dump', dumpArgs, dumpOptions)
+    const pgRestore = childProcess.spawn('pg_restore', restoreArgs, restoreOptions)
 
     await spawnPipe(pgDump, pgRestore)
 

test/unit/commands/pg/pull.unit.test.ts

@@ -88,6 +88,58 @@ describe('pg:pull', function () {
     expect(stderr).to.eq('')
   })
 
+  describe('with --exclude-table-data', function () {
+    beforeEach(function () {
+      spawnStub.withArgs('pg_dump', match.array, match.any).returns({
+        on: exitHandler,
+        stdout: {
+          pipe() {},
+        },
+      })
+      spawnStub.withArgs('pg_restore', match.array, match.any).returns({
+        on: exitHandler,
+        stdin: {
+          end() {},
+        },
+      })
+    })
+
+    const getDumpArgs = (): string[] => {
+      const dumpCall = spawnStub.getCalls().find(call => call.args[0] === 'pg_dump')
+      expect(dumpCall, 'pg_dump should have been spawned').to.not.eq(undefined)
+      return dumpCall!.args[1] as string[]
+    }
+
+    it('passes the exclusion and keeps the database name as the last pg_dump argument', async () => {
+      await runCommand(Cmd, [
+        'postgres-1',
+        'localdb',
+        '-a',
+        'myapp',
+        '--exclude-table-data=logs',
+      ])
+
+      const args = getDumpArgs()
+      expect(args).to.include('--exclude-table-data=logs')
+      expect(args.at(-1)).to.eq('mydb')
+    })
+
+    it('keeps the database name last when excluding multiple tables', async () => {
+      await runCommand(Cmd, [
+        'postgres-1',
+        'localdb',
+        '-a',
+        'myapp',
+        '--exclude-table-data=logs;events',
+      ])
+
+      const args = getDumpArgs()
+      expect(args.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true)
+      expect(args.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true)
+      expect(args.at(-1)).to.eq('mydb')
+    })
+  })
+
   skipOnWindows('opens an SSH tunnel and runs pg_dump for bastion databases', async () => {
     db.bastionHost = 'bastion-host'
     db.bastionKey = 'super-private-key'

test/unit/commands/pg/push.unit.test.ts

@@ -186,6 +186,64 @@ describe('pg:push', function () {
     expect(stderr).to.eq('')
   })
 
+  describe('with --exclude-table-data', function () {
+    beforeEach(function () {
+      spawnStub.withArgs('pg_dump', match.array, match.any).returns({
+        on: exitHandler,
+        stdout: {
+          pipe() {},
+        },
+      })
+      spawnStub.withArgs('pg_restore', match.array, match.any).returns({
+        on: exitHandler,
+        stdin: {
+          end() {},
+        },
+      })
+    })
+
+    const getSpawnArgs = (command: string): string[] => {
+      const call = spawnStub.getCalls().find(call => call.args[0] === command)
+      expect(call, `${command} should have been spawned`).to.not.eq(undefined)
+      return call!.args[1] as string[]
+    }
+
+    it('passes the exclusion and keeps the database name as the last pg_dump argument', async () => {
+      await runCommand(Cmd, [
+        'localdb',
+        'postgres-1',
+        '-a',
+        'myapp',
+        '--exclude-table-data=logs',
+      ])
+
+      const dumpArgs = getSpawnArgs('pg_dump')
+      expect(dumpArgs).to.include('--exclude-table-data=logs')
+      expect(dumpArgs.at(-1)).to.eq('localdb')
+
+      const restoreArgs = getSpawnArgs('pg_restore')
+      expect(restoreArgs.at(-1)).to.eq('mydb')
+    })
+
+    it('keeps the database name last when excluding multiple tables', async () => {
+      await runCommand(Cmd, [
+        'localdb',
+        'postgres-1',
+        '-a',
+        'myapp',
+        '--exclude-table-data=logs;events',
+      ])
+
+      const dumpArgs = getSpawnArgs('pg_dump')
+      expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=logs'))).to.eq(true)
+      expect(dumpArgs.some(arg => arg.includes('--exclude-table-data=events'))).to.eq(true)
+      expect(dumpArgs.at(-1)).to.eq('localdb')
+
+      const restoreArgs = getSpawnArgs('pg_restore')
+      expect(restoreArgs.at(-1)).to.eq('mydb')
+    })
+  })
+
   skipOnWindows('exits non-zero when there is an error', async () => {
     const dumpFlags = ['--verbose', '-F', 'c', '-Z', '0', '-N', '_heroku', 'localdb']
     const restoreFlags = ['--verbose', '-F', 'c', '--no-acl', '--no-owner', '-U', 'jeff', '-h', 'herokai.com', '-p', '5432', '-d', 'mydb']