Repository: salsita/node-pg-migrate
Pull request #580 (2020-03-30 07:15:00)

Possibility to pass custom logger
Taking an idea from #503


Patch:
diff --git a/.eslintrc.js b/.eslintrc.js
index 680c47a..9fbb66b 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -27,7 +27,6 @@ module.exports = {
     mocha: true,
   },
   rules: {
-    'no-console': 0,
     'no-underscore-dangle': 0,
     'security/detect-object-injection': 0,
     '@typescript-eslint/explicit-function-return-type': 0,
diff --git a/README.md b/README.md
index 8f461ae..ba672bc 100644
--- a/README.md
+++ b/README.md
@@ -32,7 +32,7 @@ Run `npm run migrate create my first migration`. It will create file `xxx_my-fir
 Open it and change contents to:
 
 ```js
-exports.up = pgm => {
+exports.up = (pgm) => {
   pgm.createTable('users', {
     id: 'id',
     name: { type: 'varchar(1000)', notNull: true },
@@ -73,7 +73,7 @@ If you will want to change your schema later, you can e.g. add lead paragraph to
 Run `npm run migrate create posts lead`, edit `xxx_posts_lead.js`:
 
 ```js
-exports.up = pgm => {
+exports.up = (pgm) => {
   pgm.addColumns('posts', {
     lead: { type: 'text', notNull: true },
   })
diff --git a/bin/node-pg-migrate b/bin/node-pg-migrate
index 965404c..34b6a47 100755
--- a/bin/node-pg-migrate
+++ b/bin/node-pg-migrate
@@ -1,5 +1,5 @@
 #!/usr/bin/env node
-/* eslint-disable strict */
+/* eslint-disable strict, no-console */
 
 'use strict'
 
diff --git a/docs/README.md b/docs/README.md
index 39043b2..1ffe08a 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -34,7 +34,7 @@ Run `npm run migrate create my first migration`. It will create file `xxx_my-fir
 Open it and change contents to:
 
 ```js
-exports.up = pgm => {
+exports.up = (pgm) => {
   pgm.createTable('users', {
     id: 'id',
     name: { type: 'varchar(1000)', notNull: true },
@@ -75,7 +75,7 @@ If you will want to change your schema later, you can e.g. add lead paragraph to
 Run `npm run migrate create posts lead`, edit `xxx_posts_lead.js`:
 
 ```js
-exports.up = pgm => {
+exports.up = (pgm) => {
   pgm.addColumns('posts', {
     lead: { type: 'text', notNull: true },
   })
diff --git a/docs/api.md b/docs/api.md
index 55aa957..b53f10d 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -21,5 +21,6 @@ which takes options argument with following structure (similar to [command line
 - `noLock` _[boolean]_ - Disables locking mechanism and checks
 - `fake` _[boolean]_ - Mark migrations as run without actually performing them (use with caution!)
 - `dryRun` _[boolean]_
-- `log` _[function]_ - Redirect log messages to this function, rather than `console.log`
+- `log` _[function]_ - Redirect log messages to this function, rather than `console`
+- `logger` _[object with debug/info/warn/error methods]_ - Redirect messages to this logger object, rather than `console`
 - `decamelize` _[boolean]_ - Runs [`decamelize`](https://github.com/sindresorhus/decamelize) on table/column/etc. names
diff --git a/docs/migrations.md b/docs/migrations.md
index f276b57..122c8fa 100644
--- a/docs/migrations.md
+++ b/docs/migrations.md
@@ -31,7 +31,7 @@ In some cases, you may want to perform some async operation during a migration,
 
 ```javascript
 exports.up = function up(pgm, run) {
-  doSomethingAsync(function() {
+  doSomethingAsync(function () {
     run()
   })
 }
@@ -40,8 +40,8 @@ exports.up = function up(pgm, run) {
 Another way how to perform some async operation is to return [Promise](https://promisesaplus.com/) from `up` or `down` function. Example:
 
 ```javascript
-exports.up = function(pgm) {
-  return new Promise(resolve => {
+exports.up = function (pgm) {
+  return new Promise((resolve) => {
     // doSomethingAsync
     resolve()
   })
@@ -51,7 +51,7 @@ exports.up = function(pgm) {
 or
 
 ```javascript
-exports.up = async pgm => {
+exports.up = async (pgm) => {
   // doSomethingAsync
 }
 ```
diff --git a/src/db.ts b/src/db.ts
index 6cdc9bb..3822a46 100644
--- a/src/db.ts
+++ b/src/db.ts
@@ -3,19 +3,7 @@
  */
 
 import { Client, ClientConfig, QueryArrayResult, QueryResult, QueryArrayConfig, QueryConfig } from 'pg'
-// or native libpq bindings
-// const pg = require('pg/native');
-
-// see ClientBase in @types/pg
-export interface DB {
-  query(queryConfig: QueryArrayConfig, values?: any[]): Promise<QueryArrayResult>
-  query(queryConfig: QueryConfig): Promise<QueryResult>
-  query(queryTextOrConfig: string | QueryConfig, values?: any[]): Promise<QueryResult>
-
-  select(queryConfig: QueryArrayConfig, values?: any[]): Promise<any[]>
-  select(queryConfig: QueryConfig): Promise<any[]>
-  select(queryTextOrConfig: string | QueryConfig, values?: any[]): Promise<any[]>
-}
+import { Logger, DB } from './types'
 
 export interface DBConnection extends DB {
   createConnection(): Promise<void>
@@ -28,7 +16,7 @@ export interface DBConnection extends DB {
   close(): Promise<void>
 }
 
-const db = (connection: Client | string | ClientConfig, log = console.error): DBConnection => {
+const db = (connection: Client | string | ClientConfig, logger: Logger = console): DBConnection => {
   const isExternalClient = connection instanceof Client
   let clientActive = false
 
@@ -42,7 +30,7 @@ const db = (connection: Client | string | ClientConfig, log = console.error): DB
         ? resolve()
         : client.connect((err) => {
             if (err) {
-              log('could not connect to postgres', err)
+              logger.error('could not connect to postgres', err)
               return reject(err)
             }
             clientActive = true
@@ -67,14 +55,14 @@ const db = (connection: Client | string | ClientConfig, log = console.error): DB
         const stringEnd = string.substr(endLineWrapPos)
         const startLineWrapPos = stringStart.lastIndexOf('\n') + 1
         const padding = ' '.repeat(position - startLineWrapPos - 1)
-        log(`Error executing:
+        logger.error(`Error executing:
 ${stringStart}
 ${padding}^^^^${stringEnd}
 
 ${message}
 `)
       } else {
-        log(`Error executing:
+        logger.error(`Error executing:
 ${string}
 ${err}
 `)
@@ -109,7 +97,7 @@ ${err}
 
     close: async () => {
       await beforeCloseListeners.reduce(
-        (promise, listener) => promise.then(listener).catch((err: any) => log(err.stack || err)),
+        (promise, listener) => promise.then(listener).catch((err: any) => logger.error(err.stack || err)),
         Promise.resolve(),
       )
       if (!isExternalClient) {
diff --git a/src/migration-builder.ts b/src/migration-builder.ts
index 9c94e2a..be598f4 100644
--- a/src/migration-builder.ts
+++ b/src/migration-builder.ts
@@ -9,10 +9,9 @@
  and it makes inference of down migrations possible.
  */
 
-import { DB } from './db'
 import { createSchemalize } from './utils'
 import { ColumnDefinitions } from './operations/tablesTypes'
-import { MigrationBuilder, MigrationOptions } from './types'
+import { DB, MigrationBuilder, MigrationOptions } from './types'
 
 import * as domains from './operations/domains'
 import * as extensions from './operations/extensions'
diff --git a/src/migration.ts b/src/migration.ts
index eb66d4f..8b746aa 100644
--- a/src/migration.ts
+++ b/src/migration.ts
@@ -12,7 +12,7 @@ import path from 'path'
 import { promisify } from 'util'
 import { DBConnection } from './db'
 import MigrationBuilder from './migration-builder'
-import { MigrationAction, MigrationBuilderActions, MigrationDirection, RunnerOption } from './types'
+import { MigrationAction, MigrationBuilderActions, MigrationDirection, RunnerOption, Logger } from './types'
 import { getMigrationTableSchema } from './utils'
 import { ColumnDefinitions } from './operations/tablesTypes'
 
@@ -91,7 +91,7 @@ export class Migration implements RunMigration {
 
   public readonly typeShorthands?: ColumnDefinitions
 
-  public readonly log: typeof console.log
+  public readonly logger: Logger
 
   constructor(
     db: DBConnection,
@@ -99,7 +99,7 @@ export class Migration implements RunMigration {
     { up, down }: MigrationBuilderActions,
     options: RunnerOption,
     typeShorthands?: ColumnDefinitions,
-    log = console.log,
+    logger: Logger = console,
   ) {
     this.db = db
     this.path = migrationPath
@@ -109,7 +109,7 @@ export class Migration implements RunMigration {
     this.down = down
     this.options = options
     this.typeShorthands = typeShorthands
-    this.log = log
+    this.logger = logger
   }
 
   _getMarkAsRun(action: MigrationAction) {
@@ -118,10 +118,10 @@ export class Migration implements RunMigration {
     const { name } = this
     switch (action) {
       case this.down:
-        this.log(`### MIGRATION ${this.name} (DOWN) ###`)
+        this.logger.info(`### MIGRATION ${this.name} (DOWN) ###`)
         return `DELETE FROM "${schema}"."${migrationsTable}" WHERE name='${name}';`
       case this.up:
-        this.log(`### MIGRATION ${this.name} (UP) ###`)
+        this.logger.info(`### MIGRATION ${this.name} (UP) ###`)
         return `INSERT INTO "${schema}"."${migrationsTable}" (name, run_on) VALUES ('${name}', NOW());`
       default:
         throw new Error('Unknown direction')
@@ -145,14 +145,14 @@ export class Migration implements RunMigration {
       sqlSteps.push('COMMIT;')
     } else if (this.options.singleTransaction && !pgm.isUsingTransaction()) {
       // in singleTransaction mode we are already wrapped in a global transaction
-      this.log('#> WARNING: Need to break single transaction! <')
+      this.logger.warn('#> WARNING: Need to break single transaction! <')
       sqlSteps.unshift('COMMIT;')
       sqlSteps.push('BEGIN;')
     } else if (!this.options.singleTransaction || !pgm.isUsingTransaction()) {
-      this.log('#> WARNING: This migration is not wrapped in a transaction! <')
+      this.logger.warn('#> WARNING: This migration is not wrapped in a transaction! <')
     }
 
-    this.log(`${sqlSteps.join('\n')}\n\n`)
+    this.logger.debug(`${sqlSteps.join('\n')}\n\n`)
 
     return sqlSteps.reduce(
       (promise: Promise<unknown>, sql) => promise.then((): unknown => this.options.dryRun || this.db.query(sql)),
diff --git a/src/runner.ts b/src/runner.ts
index 3902c37..f0b74ab 100644
--- a/src/runner.ts
+++ b/src/runner.ts
@@ -2,7 +2,14 @@ import path from 'path'
 import Db, { DBConnection } from './db'
 import { ColumnDefinitions } from './operations/tablesTypes'
 import { Migration, loadMigrationFiles, RunMigration } from './migration'
-import { MigrationBuilderActions, MigrationDirection, RunnerOptionClient, RunnerOptionUrl, RunnerOption } from './types'
+import {
+  MigrationBuilderActions,
+  MigrationDirection,
+  RunnerOptionClient,
+  RunnerOptionUrl,
+  RunnerOption,
+  Logger,
+} from './types'
 import { createSchemalize, getMigrationTableSchema, getSchemas } from './utils'
 import migrateSqlFile from './sqlMigration'
 
@@ -13,7 +20,7 @@ const idColumn = 'id'
 const nameColumn = 'name'
 const runOnColumn = 'run_on'
 
-const loadMigrations = async (db: DBConnection, options: RunnerOption, log: typeof console.log) => {
+const loadMigrations = async (db: DBConnection, options: RunnerOption, logger: Logger) => {
   try {
     let shorthands: ColumnDefinitions = {}
     const files = await loadMigrationFiles(options.dir, options.ignorePattern)
@@ -34,7 +41,7 @@ const loadMigrations = async (db: DBConnection, options: RunnerOption, log: type
           {
             ...shorthands,
           },
-          log,
+          logger,
         )
       }),
     )
@@ -137,9 +144,15 @@ const runMigrations = (toRun: Migration[], method: 'markAsRun' | 'apply', direct
     Promise.resolve(),
   )
 
+const getLogger = ({ log, logger }: RunnerOption): Logger => {
+  if (typeof logger === 'object') return logger
+  if (typeof log === 'function') return { debug: log, info: log, warn: log, error: log }
+  return console
+}
+
 export default async (options: RunnerOption): Promise<RunMigration[]> => {
-  const log = options.log || console.log
-  const db = Db((options as RunnerOptionClient).dbClient || (options as RunnerOptionUrl).databaseUrl, log)
+  const logger = getLogger(options)
+  const db = Db((options as RunnerOptionClient).dbClient || (options as RunnerOptionUrl).databaseUrl, logger)
   try {
     await db.createConnection()
     if (options.schema) {
@@ -159,7 +172,10 @@ export default async (options: RunnerOption): Promise<RunMigration[]> => {
       await lock(db)
     }
 
-    const [migrations, runNames] = await Promise.all([loadMigrations(db, options, log), getRunMigrations(db, options)])
+    const [migrations, runNames] = await Promise.all([
+      loadMigrations(db, options, logger),
+      getRunMigrations(db, options),
+    ])
 
     if (options.checkOrder) {
       checkOrder(runNames, migrations)
@@ -168,14 +184,14 @@ export default async (options: RunnerOption): Promise<RunMigration[]> => {
     const toRun: Migration[] = getMigrationsToRun(options, runNames, migrations)
 
     if (!toRun.length) {
-      log('No migrations to run!')
+      logger.info('No migrations to run!')
       return []
     }
 
     // TODO: add some fancy colors to logging
-    log('> Migrating files:')
+    logger.info('> Migrating files:')
     toRun.forEach((m) => {
-      log(`> - ${m.name}`)
+      logger.info(`> - ${m.name}`)
     })
 
     if (options.fake) {
@@ -186,7 +202,7 @@ export default async (options: RunnerOption): Promise<RunMigration[]> => {
         await runMigrations(toRun, 'apply', options.direction)
         await db.query('COMMIT')
       } catch (err) {
-        log('> Rolling back attempted migration ...')
+        logger.warn('> Rolling back attempted migration ...')
         await db.query('ROLLBACK')
         throw err
       }
diff --git a/src/types.ts b/src/types.ts
index 42c25f4..ef7f1bf 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -1,6 +1,5 @@
-import { Client } from 'pg'
+import { Client, QueryArrayResult, QueryResult, QueryArrayConfig, QueryConfig } from 'pg'
 import { TlsOptions } from 'tls'
-import { DB } from './db'
 import { Name } from './operations/generalTypes'
 
 import * as domains from './operations/domainsTypes'
@@ -20,6 +19,17 @@ import * as views from './operations/viewsTypes'
 import * as mViews from './operations/viewsMaterializedTypes'
 import PgLiteral from './operations/PgLiteral'
 
+// see ClientBase in @types/pg
+export interface DB {
+  query(queryConfig: QueryArrayConfig, values?: any[]): Promise<QueryArrayResult>
+  query(queryConfig: QueryConfig): Promise<QueryResult>
+  query(queryTextOrConfig: string | QueryConfig, values?: any[]): Promise<QueryResult>
+
+  select(queryConfig: QueryArrayConfig, values?: any[]): Promise<any[]>
+  select(queryConfig: QueryConfig): Promise<any[]>
+  select(queryTextOrConfig: string | QueryConfig, values?: any[]): Promise<any[]>
+}
+
 export interface MigrationBuilder {
   createExtension: (...args: Parameters<extensions.CreateExtension>) => void
   dropExtension: (...args: Parameters<extensions.DropExtension>) => void
@@ -182,6 +192,9 @@ export enum PgType { // eslint-disable-line import/prefer-default-export
 
 export type MigrationDirection = 'up' | 'down'
 
+export type Logger = Pick<typeof console, 'debug' | 'info' | 'warn' | 'error'>
+export type LogFn = (msg: string) => void
+
 export interface RunnerOptionConfig {
   migrationsTable: string
   migrationsSchema?: string
@@ -200,7 +213,8 @@ export interface RunnerOptionConfig {
   noLock?: boolean
   fake?: boolean
   decamelize?: boolean
-  log?: (msg: string) => void
+  log?: LogFn
+  logger?: Logger
 }
 
 export interface ConnectionConfig {


Test patch:
diff --git a/test/migration-test.ts b/test/migration-test.ts
index 7d23ac9..8bc97b8 100644
--- a/test/migration-test.ts
+++ b/test/migration-test.ts
@@ -1,7 +1,7 @@
 /* eslint-disable no-unused-expressions */
 import sinon, { SinonSpy } from 'sinon'
 import { expect } from 'chai'
-import { RunnerOption } from '../src/types'
+import { RunnerOption, Logger } from '../src/types'
 import { Migration } from '../src/migration'
 import { DBConnection } from '../src/db'
 
@@ -14,7 +14,7 @@ const actionsPromise = require(`./${promiseMigration}`) // eslint-disable-line i
 
 describe('lib/migration', () => {
   const dbMock = {} as DBConnection
-  const log: typeof console.log = () => null
+  const logger: Logger = { debug: () => null, info: () => null, warn: () => null, error: () => null }
   const options = { migrationsTable } as RunnerOption
   let migration
   let queryMock: SinonSpy
@@ -26,28 +26,28 @@ describe('lib/migration', () => {
 
   describe('self.applyUp', () => {
     it('normal operations: db.query should be called', () => {
-      migration = new Migration(dbMock, callbackMigration, actionsCallback, options, {}, log)
+      migration = new Migration(dbMock, callbackMigration, actionsCallback, options, {}, logger)
       return migration.apply('up').then(() => {
         expect(queryMock).to.be.called
       })
     })
 
     it('normal operations: db.query should be called when returning promise', () => {
-      migration = new Migration(dbMock, promiseMigration, actionsPromise, options, {}, log)
+      migration = new Migration(dbMock, promiseMigration, actionsPromise, options, {}, logger)
       return migration.apply('up').then(() => {
         expect(queryMock).to.be.called
       })
     })
 
     it('--dry-run option: db.query should not be called', () => {
-      migration = new Migration(dbMock, callbackMigration, actionsCallback, { ...options, dryRun: true }, {}, log)
+      migration = new Migration(dbMock, callbackMigration, actionsCallback, { ...options, dryRun: true }, {}, logger)
       return migration.apply('up').then(() => {
         expect(queryMock).to.not.be.called
       })
     })
 
     it('should make proper SQL calls', () => {
-      migration = new Migration(dbMock, promiseMigration, actionsCallback, options, {}, log)
+      migration = new Migration(dbMock, promiseMigration, actionsCallback, options, {}, logger)
       return migration.apply('up').then(() => {
         expect(queryMock).to.have.callCount(4)
         expect(queryMock.getCall(0).args[0]).to.equal('BEGIN;')
@@ -59,7 +59,7 @@ describe('lib/migration', () => {
 
     it('should fail with an error message if the migration is invalid', () => {
       const invalidMigrationName = 'invalid-migration'
-      migration = new Migration(dbMock, invalidMigrationName, {}, options, {}, log)
+      migration = new Migration(dbMock, invalidMigrationName, {}, options, {}, logger)
       const direction = 'up'
       let error
       try {
@@ -75,21 +75,21 @@ describe('lib/migration', () => {
 
   describe('self.applyDown', () => {
     it('normal operations: db.query should be called', () => {
-      migration = new Migration(dbMock, callbackMigration, actionsCallback, options, {}, log)
+      migration = new Migration(dbMock, callbackMigration, actionsCallback, options, {}, logger)
       return migration.apply('down').then(() => {
         expect(queryMock).to.be.called
       })
     })
 
     it('--dry-run option: db.query should not be called', () => {
-      migration = new Migration(dbMock, callbackMigration, actionsCallback, { ...options, dryRun: true }, {}, log)
+      migration = new Migration(dbMock, callbackMigration, actionsCallback, { ...options, dryRun: true }, {}, logger)
       return migration.apply('down').then(() => {
         expect(queryMock).to.not.be.called
       })
     })
 
     it('should make proper SQL calls', () => {
-      migration = new Migration(dbMock, promiseMigration, actionsCallback, options, {}, log)
+      migration = new Migration(dbMock, promiseMigration, actionsCallback, options, {}, logger)
       return migration.apply('down').then(() => {
         expect(queryMock).to.have.callCount(4)
         expect(queryMock.getCall(0).args[0]).to.equal('BEGIN;')
diff --git a/test/ts/customRunner.ts b/test/ts/customRunner.ts
index b507846..1f21623 100644
--- a/test/ts/customRunner.ts
+++ b/test/ts/customRunner.ts
@@ -1,6 +1,7 @@
 import { resolve } from 'path'
 import runner, { RunnerOption } from '../../dist'
 
+/* eslint-disable no-console */
 const run = async () => {
   try {
     const options: Omit<RunnerOption, 'direction'> & { databaseUrl: string } = {