Single transaction as default (#205)

* Single transaction as default

* Fixed tests

README.md

@@ -94,7 +94,7 @@ You can adjust defaults by passing arguments to `node-pg-migrate`:
 
 * `check-order` - Check order of migrations before running them (defaults to `true`, to switch it off supply `--no-check-order` on command line).
                   (There should be no migration with timestamp lesser than last run migration.)
-* `single-transaction` - When true, combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `false`)
+* `single-transaction` - Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`, to switch it off supply `--no-single-transaction` on command line).
 * `no-lock` - Disables locking mechanism and checks (useful for DBs which does not support SQL commands used for [locking](#locking))
 
 See all by running `node-pg-migrate --help`.
@@ -156,7 +156,7 @@ which takes options argument with following structure (similar to [command line
 * `ignorePattern` _[string]_ - Regex pattern for file names to ignore
 * `file` _[string]_ - Run only migration with this name
 * `typeShorthands` _[object]_ - Object with column type shorthands
-* `single_transaction` _[boolean]_ - When true, combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `false`)
+* `singleTransaction` _[boolean]_ - Combines all pending migrations into a single transaction so that if any migration fails, all will be rolled back (defaults to `true`)
 * `createSchema` _[boolean]_ - Creates the configured schema if it doesn't exist 
 * `createMigrationsSchema` _[boolean]_ - Creates the configured migration schema if it doesn't exist
 * `noLock` _[boolean]_ - Disables locking mechanism and checks

bin/node-pg-migrate

@@ -140,7 +140,7 @@ const argv = yargs
   })
 
   .option(singleTransactionArg, {
-    default: false,
+    default: true,
     describe: 'Combines all pending migrations into a single database transaction so that if any migration fails, all will be rolled back',
     type: 'boolean',
   })
@@ -307,7 +307,7 @@ if (action === 'create') {
     createSchema: CREATE_SCHEMA,
     createMigrationsSchema: CREATE_MIGRATIONS_SCHEMA,
     direction,
-    single_transaction: singleTransaction,
+    singleTransaction,
     noLock,
   });
   const promise = action === 'redo'

index.d.ts

@@ -407,6 +407,7 @@ export interface RunnerOption {
     dryRun?: boolean
     createSchema?: boolean
     createMigrationsSchema?: boolean
+    singleTransaction?: boolean
     noLock?: boolean
 }
 

lib/migration.js

@@ -74,16 +74,15 @@ class Migration {
           throw new Error('Unknown direction');
       }
 
-      if (pgm.isUsingTransaction()) {
-        // in single_transaction mode we are already wrapped in a
-        // global transaction
-        if (!this.options.single_transaction) {
-          // otherwise we need to create our own transaction
+      // in singleTransaction mode we are already wrapped in a global transaction
+      if (this.options.singleTransaction === false) {
+        // otherwise we need to create our own transaction
+        if (pgm.isUsingTransaction()) {
           sqlSteps.unshift('BEGIN;');
           sqlSteps.push('COMMIT;');
+        } else {
+          this.log('#> WARNING: This migration is not wrapped in a transaction! <');
         }
-      } else {
-        this.log('#> WARNING: This migration is not wrapped in a transaction! <');
       }
 
       this.log(`${sqlSteps.join('\n')}\n\n`);

lib/runner.js

@@ -143,12 +143,11 @@ const getMigrationsToRun = (options, runNames, migrations) => {
   );
 };
 
-const ifSingleTransaction = (operation, options, db) => {
-  if (options.single_transaction) {
-    return db.query(operation);
-  }
-  return Promise.resolve();
-};
+const ifSingleTransaction = (operation, options, db) => (
+  options.singleTransaction === false
+    ? Promise.resolve()
+    : db.query(operation)
+);
 
 
 export default (options) => {

test/migration-test.js

@@ -59,11 +59,9 @@ describe('lib/migration', () => {
       return migration
         .applyUp()
         .then(() => {
-          expect(dbMock.query).to.have.callCount(4);
-          expect(dbMock.query.getCall(0).args[0]).to.equal('BEGIN;');
-          expect(dbMock.query.getCall(1).args[0]).to.include('CREATE TABLE');
-          expect(dbMock.query.getCall(2).args[0]).to.include(`INSERT INTO "public"."${migrationsTable}"`);
-          expect(dbMock.query.getCall(3).args[0]).to.equal('COMMIT;');
+          expect(dbMock.query).to.have.callCount(2);
+          expect(dbMock.query.getCall(0).args[0]).to.include('CREATE TABLE');
+          expect(dbMock.query.getCall(1).args[0]).to.include(`INSERT INTO "public"."${migrationsTable}"`);
         });
     });
   });
@@ -98,11 +96,9 @@ describe('lib/migration', () => {
       return migration
         .applyDown()
         .then(() => {
-          expect(dbMock.query).to.have.callCount(4);
-          expect(dbMock.query.getCall(0).args[0]).to.equal('BEGIN;');
-          expect(dbMock.query.getCall(1).args[0]).to.include('DROP TABLE');
-          expect(dbMock.query.getCall(2).args[0]).to.include(`DELETE FROM "public"."${migrationsTable}"`);
-          expect(dbMock.query.getCall(3).args[0]).to.equal('COMMIT;');
+          expect(dbMock.query).to.have.callCount(2);
+          expect(dbMock.query.getCall(0).args[0]).to.include('DROP TABLE');
+          expect(dbMock.query.getCall(1).args[0]).to.include(`DELETE FROM "public"."${migrationsTable}"`);
         });
     });
   });