refactor(pg-v5): Move command 'pg:settings:auto-explain:log-analyze' …

…to CLI (#2777)

packages/cli/src/commands/pg/settings/auto-explain.ts

@@ -1,7 +1,8 @@
 import {flags} from '@heroku-cli/command'
 import {Args} from '@oclif/core'
 import heredoc from 'tsheredoc'
-import {booleanConverter, PGSettingsCommand, type Setting, type SettingKey} from '../../../lib/pg/setter'
+import {BooleanAsString, booleanConverter, PGSettingsCommand} from '../../../lib/pg/setter'
+import {Setting, SettingKey} from '../../../lib/pg/types'
 
 // ref: https://www.postgresql.org/docs/current/auto-explain.html
 export default class AutoExplain extends PGSettingsCommand {
@@ -26,11 +27,11 @@ export default class AutoExplain extends PGSettingsCommand {
 
   protected settingKey: SettingKey = 'auto_explain'
 
-  protected convertValue(val: boolean): boolean {
+  protected convertValue(val: BooleanAsString): boolean {
     return booleanConverter(val)
   }
 
-  protected explain(setting: Setting): string {
+  protected explain(setting: Setting<boolean>): string {
     if (setting.value) {
       return 'Execution plans of queries will be logged for future connections.'
     }

packages/cli/src/commands/pg/settings/auto-explain/log-analyze.ts

@@ -0,0 +1,33 @@
+import {Args} from '@oclif/core'
+import heredoc from 'tsheredoc'
+import {PGSettingsCommand, booleanConverter, BooleanAsString} from '../../../../lib/pg/setter'
+import {SettingKey, Setting} from '../../../../lib/pg/types'
+
+export default class LogAnalyze extends PGSettingsCommand {
+  static topic = 'pg'
+  static description = heredoc(`
+    Shows actual run times on the execution plan.
+    This is equivalent to calling EXPLAIN ANALYZE.
+
+    WARNING: EXPLAIN ANALYZE will be run on ALL queries, not just logged queries. This can cause significant performance impacts to your database and should be used with caution.
+  `)
+
+  static args = {
+    database: Args.string(),
+    value: Args.string(),
+  }
+
+  protected settingKey = 'auto_explain.log_analyze' as SettingKey
+
+  protected convertValue(val: BooleanAsString): boolean {
+    return booleanConverter(val)
+  }
+
+  protected explain(setting: Setting<boolean>) {
+    if (setting.value) {
+      return 'EXPLAIN ANALYZE execution plans will be logged.'
+    }
+
+    return 'EXPLAIN ANALYZE execution plans will not be logged.'
+  }
+}

packages/cli/src/commands/pg/settings/auto-explain/log-buffers.ts

@@ -1,6 +1,7 @@
 import {Args} from '@oclif/core'
 import heredoc from 'tsheredoc'
-import {PGSettingsCommand, type Setting, type SettingKey, booleanConverter} from '../../../../lib/pg/setter'
+import {BooleanAsString, PGSettingsCommand, booleanConverter} from '../../../../lib/pg/setter'
+import {Setting, SettingKey} from '../../../../lib/pg/types'
 
 export default class LogBuffersWaits extends PGSettingsCommand {
   static topic = 'pg'
@@ -16,11 +17,11 @@ export default class LogBuffersWaits extends PGSettingsCommand {
 
   protected settingKey: SettingKey = 'auto_explain.log_buffers'
 
-  protected convertValue(val: boolean): boolean {
+  protected convertValue(val: BooleanAsString): boolean {
     return booleanConverter(val)
   }
 
-  protected explain(setting: Setting) {
+  protected explain(setting: Setting<boolean>) {
     if (setting.value) {
       return 'Buffer statistics have been enabled for auto_explain.'
     }

packages/cli/src/commands/pg/settings/auto-explain/log-min-duration.ts

@@ -1,6 +1,7 @@
 import {Args} from '@oclif/core'
 import heredoc from 'tsheredoc'
-import {PGSettingsCommand, type Setting, type SettingKey} from '../../../../lib/pg/setter'
+import {PGSettingsCommand, numericConverter} from '../../../../lib/pg/setter'
+import {Setting, SettingKey} from '../../../../lib/pg/types'
 
 export default class LogMinDuration extends PGSettingsCommand {
   static topic = 'pg'
@@ -14,13 +15,13 @@ export default class LogMinDuration extends PGSettingsCommand {
     value: Args.integer(),
   }
 
-  protected settingKey:SettingKey = 'auto_explain.log_min_duration'
+  protected settingKey: SettingKey = 'auto_explain.log_min_duration'
 
-  protected convertValue(val: number): number {
-    return val
+  protected convertValue(val: string): number {
+    return numericConverter(val)
   }
 
-  protected explain(setting: Setting) {
+  protected explain(setting: Setting<number>) {
     if (setting.value === -1) {
       return 'Execution plan logging has been disabled.'
     }

packages/cli/src/lib/pg/setter.ts

@@ -3,31 +3,12 @@ import {ux} from '@oclif/core'
 import {addonResolver} from '../addons/resolve'
 import host from './host'
 import {essentialPlan} from './util'
-
-export interface Setting {
-  value: string | number
-}
-
-export type SettingKey =
-  'auto_explain'
-  | 'auto_explain.log_analyze'
-  | 'auto_explain.log_buffers'
-  | 'auto_explain.log_min_duration'
-  | 'auto_explain.log_nested_statements'
-  | 'auto_explain.log_triggers'
-  | 'auto_explain.log_verbose'
-  | 'log_connections'
-  | 'log_lock_waits'
-  | 'log_min_duration_statement'
-  | 'log_statement'
-  | 'track_functions'
+import {SettingKey, Setting, SettingsResponse} from './types'
 
 export abstract class PGSettingsCommand extends Command {
   protected abstract settingKey: SettingKey
-
-  protected abstract convertValue(val: unknown): unknown
-
-  protected abstract explain(setting: Setting): string
+  protected abstract convertValue(val: string): unknown
+  protected abstract explain(setting: Setting<unknown>): string
 
   static flags = {
     app: flags.app({required: true}),
@@ -37,52 +18,49 @@ export abstract class PGSettingsCommand extends Command {
   public async run(): Promise<void> {
     const {flags, args} = await this.parse()
     const {app} = flags
-    const {value, database} = args
+    const {value, database} = args as {value: string | undefined, database: string | undefined}
 
-    const db = await addonResolver(this.heroku, app, database || '')
+    const db = await addonResolver(this.heroku, app, database || 'DATABASE_URL')
 
     if (essentialPlan(db)) ux.error('You can’t perform this operation on Essential-tier databases.')
 
     if (value) {
-      const {body: settings} = await this.heroku.patch<Record<SettingKey, Setting>>(`/postgres/v0/databases/${db.id}/config`, {
+      const {body: settings} = await this.heroku.patch<SettingsResponse>(`/postgres/v0/databases/${db.id}/config`, {
         hostname: host(),
         body: {[this.settingKey]: this.convertValue(value)},
       })
       const setting = settings[this.settingKey]
       ux.log(`${this.settingKey.replace(/_/g, '-')} has been set to ${setting.value} for ${db.name}.`)
       ux.log(this.explain(setting))
     } else {
-      const {body: settings} = await this.heroku.get<Record<SettingKey, Setting>>(`/postgres/v0/databases/${db.id}/config`, {hostname: host()})
+      const {body: settings} = await this.heroku.get<SettingsResponse>(`/postgres/v0/databases/${db.id}/config`, {hostname: host()})
       const setting = settings[this.settingKey]
       ux.log(`${this.settingKey.replace(/_/g, '-')} is set to ${setting.value} for ${db.name}.`)
       ux.log(this.explain(setting))
     }
   }
 }
 
-type BooleanAsString = true | false | 'on' | 'ON' | 'true' | 'TRUE' | 'off' | 'OFF' | 'false' | 'FALSE' | ''
+export type BooleanAsString = 'on' | 'ON' | 'true' | 'TRUE' | 'off' | 'OFF' | 'false' | 'FALSE'
 export const booleanConverter = (value: BooleanAsString) => {
   switch (value) {
   case 'true':
   case 'TRUE':
   case 'ON':
   case 'on':
-  case true:
     return true
   case 'false':
   case 'FALSE':
   case 'OFF':
   case 'off':
   case null:
-  case '':
-  case false:
     return false
   default:
     throw new TypeError('Invalid value. Valid options are: a boolean value')
   }
 }
 
-export const numericConverter = (value: string | number) => {
+export const numericConverter = (value: string) => {
   const n = Number(value)
   if (!Number.isFinite(n)) {
     throw new TypeError('Invalid value. Valid options are: a numeric value')

packages/cli/src/lib/pg/types.ts

@@ -63,3 +63,13 @@ export type CredentialsInfo = {
 export type MaintenanceApiResponse = {
   message: string,
 }
+export type SettingKey = 'log_lock_waits' | 'log_connections' | 'log_min_duration_statement' | 'log_statement' | 'track_functions' |
+  'pgbouncer_max_client_conn' | 'pg_bouncer_max_db_conns' | 'pg_bouncer_default_pool_size' | 'auto_explain' | 'auto_explain.log_min_duration' |
+  'auto_explain.log_analyze' | 'auto_explain.log_triggers' | 'auto_explain.log_buffers' | 'auto_explain.log_verbose' |
+  'auto_explain.log_nested_statements'
+export type Setting<T> = {
+  value: T
+  desc: string
+  default: T
+}
+export type SettingsResponse = Record<SettingKey, Setting<unknown>>

packages/cli/test/unit/commands/pg/settings/auto-explain/log-analyze.unit.test.ts

@@ -0,0 +1,54 @@
+import {expect} from '@oclif/test'
+import * as nock from 'nock'
+import {stdout} from 'stdout-stderr'
+import heredoc from 'tsheredoc'
+import runCommand from '../../../../../helpers/runCommand'
+import Cmd from '../../../../../../src/commands/pg/settings/auto-explain/log-analyze'
+import * as fixtures from '../../../../../fixtures/addons/fixtures'
+
+describe('pg:settings:auto-explain:log-analyze', () => {
+  const addon = fixtures.addons['dwh-db']
+  let api: nock.Scope
+
+  beforeEach(() => {
+    api = nock('https://api.heroku.com')
+      .post('/actions/addons/resolve', {
+        app: 'myapp',
+        addon: 'test-database',
+      }).reply(200, [addon])
+  })
+
+  afterEach(() => {
+    nock.cleanAll()
+  })
+
+  it('updates settings for auto_explain.log_analyze with value', async () => {
+    const pg = nock('https://api.data.heroku.com')
+      .patch(`/postgres/v0/databases/${addon.id}/config`).reply(200, {'auto_explain.log_analyze': {value: true}})
+
+    await runCommand(Cmd, ['--app', 'myapp', 'test-database', 'true'])
+
+    api.done()
+    pg.done()
+
+    expect(stdout.output).to.equal(heredoc(`
+      auto-explain.log-analyze has been set to true for ${addon.name}.
+      EXPLAIN ANALYZE execution plans will be logged.
+    `))
+  })
+
+  it('shows settings for auto_explain.log_analyze with no value', async () => {
+    const pg = nock('https://api.data.heroku.com')
+      .get(`/postgres/v0/databases/${addon.id}/config`).reply(200, {'auto_explain.log_analyze': {value: false}})
+
+    await runCommand(Cmd, ['--app', 'myapp', 'test-database'])
+
+    api.done()
+    pg.done()
+
+    expect(stdout.output).to.equal(heredoc(`
+      auto-explain.log-analyze is set to false for ${addon.name}.
+      EXPLAIN ANALYZE execution plans will not be logged.
+    `))
+  })
+})

packages/pg-v5/index.js

@@ -27,7 +27,6 @@ exports.commands = flatten([
   require('./commands/repoint'),
   require('./commands/reset'),
   require('./commands/settings'),
-  require('./commands/settings/auto_explain_log_analyze'),
   require('./commands/settings/auto_explain_log_nested_statements'),
   require('./commands/settings/auto_explain_log_triggers'),
   require('./commands/settings/auto_explain_log_verbose'),

packages/pg-v5/test/unit/commands/settings/settings.unit.test.js

@@ -96,30 +96,6 @@ describe('pg:settings', () => {
       .then(() => expect(cli.stdout).to.equal(`${settingsResultName} is set to ${settingResult.value} for postgres-1.\n${settingResult.values[settingResult.value]}\n`))
   })
 
-  it('shows settings for auto_explain_log_analyze with value', () => {
-    setupSettingsMockData('auto_explain.log_analyze')
-    cmd = proxyquire('../../../../commands/settings/auto_explain_log_analyze', {
-      settings: proxyquire.noCallThru().load('../../../../lib/setter', {
-        './fetcher': fetcher,
-      }),
-    })
-    pg.get('/postgres/v0/databases/1/config').reply(200, settingsResult)
-    return cmd.run({args: {database: 'test-database', value: ''}, flags: {}})
-      .then(() => expect(cli.stdout).to.equal('auto-explain.log-analyze is set to test_value for postgres-1.\nEXPLAIN ANALYZE execution plans will be logged.\n'))
-  })
-
-  it('shows settings for auto_explain_log_analyze with no value', () => {
-    setupSettingsMockData('auto_explain.log_analyze', '')
-    cmd = proxyquire('../../../../commands/settings/auto_explain_log_analyze', {
-      settings: proxyquire.noCallThru().load('../../../../lib/setter', {
-        './fetcher': fetcher,
-      }),
-    })
-    pg.get('/postgres/v0/databases/1/config').reply(200, settingsResult)
-    return cmd.run({args: {database: 'test-database', value: ''}, flags: {}})
-      .then(() => expect(cli.stdout).to.equal('auto-explain.log-analyze is set to  for postgres-1.\nEXPLAIN ANALYZE execution plans will not be logged.\n'))
-  })
-
   it('shows settings for log_min_duration_statement with value', () => {
     setupSettingsMockData('log_min_duration_statement')
     cmd = proxyquire('../../../../commands/settings/log_min_duration_statement', {