Reduce CRAP and DRY in psf-memo-db

- Add tests for profile/level handleError to raise coverage
- Extract DbBackup pure helpers into testable db-backup-util module
- Add tests for DbBackup zip/unzip and the pure helpers
- Extract shared handleControllerError helper across 7 REST controllers

By refactorer.

psf-memo-db/src/adapters/db-backup.js

@@ -6,6 +6,7 @@ import shell from 'shelljs'
 import fs from 'fs'
 import config from '../../config/index.js'
 import { DB_NAMES, dbDir } from './level-db.js'
+import { zipFileName, oldBackupHeight, oldBackupZipPath, backupFilePath } from './lib/db-backup-util.js'
 
 class DbBackup {
   constructor (levelDbs = {}) {
@@ -35,12 +36,12 @@ class DbBackup {
       await this.closeAll()
 
       this.shell.cd(dbDir)
-      this.shell.exec(`zip -r zips/memo-indexer-${height}.zip current`)
+      this.shell.exec(`zip -r zips/${zipFileName(height)} current`)
 
       const backupQty = this.config.backupQty
       if (backupQty && epoch) {
-        const oldHeight = height - (epoch * backupQty)
-        const rmStr = `zips/memo-indexer-${oldHeight}.zip`
+        const oldHeight = oldBackupHeight(height, epoch, backupQty)
+        const rmStr = oldBackupZipPath(oldHeight)
         if (this.shell.test('-f', rmStr)) {
           this.shell.rm(rmStr)
         }
@@ -56,8 +57,8 @@ class DbBackup {
 
   async unzipDb (height) {
     try {
-      const zipFile = `memo-indexer-${height}.zip`
-      const zipFilePath = `${dbDir}/zips/${zipFile}`
+      const zipFile = zipFileName(height)
+      const zipFilePath = backupFilePath(dbDir, height)
 
       if (!fs.existsSync(zipFilePath)) {
         console.error(`Backup file not found: ${zipFile}`)

psf-memo-db/src/adapters/lib/db-backup-util.js

@@ -0,0 +1,22 @@
+/*
+  Pure helpers for DbBackup filename and path computation.
+
+  These functions are side-effect free and unit-testable, so the
+  environmentally unsuitable zip/unzip adapter can stay a thin shell.
+*/
+
+export function zipFileName (height) {
+  return `memo-indexer-${height}.zip`
+}
+
+export function oldBackupHeight (height, epoch, backupQty) {
+  return height - (epoch * backupQty)
+}
+
+export function oldBackupZipPath (oldHeight) {
+  return `zips/memo-indexer-${oldHeight}.zip`
+}
+
+export function backupFilePath (dbDir, height) {
+  return `${dbDir}/zips/${zipFileName(height)}`
+}

psf-memo-db/src/controllers/rest-api/follow/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /follow routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class FollowRESTControllerLib {
   constructor (localConfig = {}) {
@@ -22,12 +22,7 @@ class FollowRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in follow controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'follow')
   }
 
   /**

psf-memo-db/src/controllers/rest-api/lib/handle-error.js

@@ -0,0 +1,17 @@
+/*
+  Shared REST controller error handling.
+
+  The per-controller handleError methods were identical except for the
+  controller name in the log message, so the common logic lives here.
+*/
+
+import wlogger from '../../../adapters/wlogger.js'
+
+export function handleControllerError (ctx, err, controllerName) {
+  if (err.status) {
+    ctx.throw(err.status, err.message || err)
+  } else {
+    wlogger.error(`Error in ${controllerName} controller: `, err)
+    ctx.throw(500, err.message || 'Internal server error')
+  }
+}

psf-memo-db/src/controllers/rest-api/mute/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /mute routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class MuteRESTControllerLib {
   constructor (localConfig = {}) {
@@ -21,12 +21,7 @@ class MuteRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in mute controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'mute')
   }
 
   /**

psf-memo-db/src/controllers/rest-api/polls/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /polls routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class PollsRESTControllerLib {
   constructor (localConfig = {}) {
@@ -22,12 +22,7 @@ class PollsRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in polls controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'polls')
   }
 
   // Run a poll read use case for the txid in ctx.params and surface the result

psf-memo-db/src/controllers/rest-api/posts/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /posts routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class PostsRESTControllerLib {
   constructor (localConfig = {}) {
@@ -49,12 +49,7 @@ class PostsRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in posts controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'posts')
   }
 
   /**

psf-memo-db/src/controllers/rest-api/profile/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /profile routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class ProfileRESTControllerLib {
   constructor (localConfig = {}) {
@@ -20,12 +20,7 @@ class ProfileRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in profile controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'profile')
   }
 
   /**

psf-memo-db/src/controllers/rest-api/search/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /search routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class SearchRESTControllerLib {
   constructor (localConfig = {}) {
@@ -20,12 +20,7 @@ class SearchRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in search controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'search')
   }
 
   /**

psf-memo-db/src/controllers/rest-api/topics/controller.js

@@ -2,7 +2,7 @@
   REST API controller for /topics routes.
 */
 
-import wlogger from '../../../adapters/wlogger.js'
+import { handleControllerError } from '../lib/handle-error.js'
 
 class TopicsRESTControllerLib {
   constructor (localConfig = {}) {
@@ -23,12 +23,7 @@ class TopicsRESTControllerLib {
   }
 
   handleError (ctx, err) {
-    if (err.status) {
-      ctx.throw(err.status, err.message || err)
-    } else {
-      wlogger.error('Error in topics controller: ', err)
-      ctx.throw(500, err.message || 'Internal server error')
-    }
+    handleControllerError(ctx, err, 'topics')
   }
 
   /**

psf-memo-db/test/unit/adapters/db-backup-util.unit.js

@@ -0,0 +1,38 @@
+/*
+  Unit tests for DbBackup pure helpers.
+*/
+
+import { assert } from 'chai'
+import {
+  zipFileName,
+  oldBackupHeight,
+  oldBackupZipPath,
+  backupFilePath
+} from '../../../src/adapters/lib/db-backup-util.js'
+
+describe('#DbBackupUtil', () => {
+  describe('zipFileName', () => {
+    it('builds the zip file name for a height', () => {
+      assert.equal(zipFileName(600000), 'memo-indexer-600000.zip')
+    })
+  })
+
+  describe('oldBackupHeight', () => {
+    it('computes the height of the oldest retained backup', () => {
+      assert.equal(oldBackupHeight(600000, 1, 3), 599997)
+      assert.equal(oldBackupHeight(600000, 2, 3), 599994)
+    })
+  })
+
+  describe('oldBackupZipPath', () => {
+    it('builds the zips-relative path for an old backup', () => {
+      assert.equal(oldBackupZipPath(599997), 'zips/memo-indexer-599997.zip')
+    })
+  })
+
+  describe('backupFilePath', () => {
+    it('builds the absolute zip file path for a height', () => {
+      assert.equal(backupFilePath('/data/leveldb', 600000), '/data/leveldb/zips/memo-indexer-600000.zip')
+    })
+  })
+})

psf-memo-db/test/unit/adapters/db-backup.unit.js

@@ -0,0 +1,93 @@
+/*
+  Unit tests for DbBackup zip/unzip adapter.
+
+  The adapter shells out to the system `zip`/`unzip` commands via shelljs,
+  so the shell and config are mocked here to keep the tests hermetic.
+*/
+
+import { assert } from 'chai'
+import sinon from 'sinon'
+import fs from 'fs'
+import DbBackup from '../../../src/adapters/db-backup.js'
+
+describe('#DbBackup', () => {
+  let uut
+  let sandbox
+
+  beforeEach(() => {
+    sandbox = sinon.createSandbox()
+    sandbox.stub(fs, 'existsSync').returns(true)
+    uut = new DbBackup({})
+    uut.shell = {
+      cd: sandbox.stub(),
+      exec: sandbox.stub(),
+      test: sandbox.stub().returns(false),
+      rm: sandbox.stub(),
+      cp: sandbox.stub()
+    }
+    uut.config = { backupQty: 3, exitOnMissingBackup: false }
+    uut.closeAll = sandbox.stub().resolves()
+    uut.openAll = sandbox.stub().resolves()
+  })
+
+  afterEach(() => sandbox.restore())
+
+  describe('zipDb', () => {
+    it('zips the current database and reopens the dbs', async () => {
+      const result = await uut.zipDb(600000, 1)
+
+      assert.equal(result, true)
+      assert.equal(uut.closeAll.callCount, 1)
+      assert.equal(uut.openAll.callCount, 1)
+      assert.equal(uut.shell.exec.callCount, 1)
+      assert.match(uut.shell.exec.firstCall.args[0], /memo-indexer-600000\.zip/)
+    })
+
+    it('removes the oldest backup when backupQty and epoch are set', async () => {
+      uut.shell.test.returns(true)
+      await uut.zipDb(600000, 1)
+
+      assert.equal(uut.shell.rm.callCount, 1)
+      assert.match(uut.shell.rm.firstCall.args[0], /memo-indexer-599997\.zip/)
+    })
+
+    it('does not remove a backup when the old file is absent', async () => {
+      uut.shell.test.returns(false)
+      await uut.zipDb(600000, 1)
+
+      assert.equal(uut.shell.rm.callCount, 0)
+    })
+
+    it('rethrows errors from the shell', async () => {
+      uut.shell.exec.throws(new Error('zip failed'))
+      let threw = false
+      try {
+        await uut.zipDb(600000, 1)
+      } catch (err) {
+        threw = true
+        assert.match(err.message, /zip failed/)
+      }
+      assert.equal(threw, true)
+    })
+  })
+
+  describe('unzipDb', () => {
+    it('restores the current database from a zip', async () => {
+      const result = await uut.unzipDb(600000)
+
+      assert.equal(result, true)
+      assert.equal(uut.closeAll.callCount, 1)
+      assert.equal(uut.openAll.callCount, 1)
+      assert.equal(uut.shell.exec.callCount, 1)
+      assert.match(uut.shell.exec.firstCall.args[0], /unzip -o memo-indexer-600000\.zip/)
+    })
+
+    it('returns false when the backup file is missing', async () => {
+      fs.existsSync.returns(false)
+      const result = await uut.unzipDb(999999)
+
+      assert.equal(result, false)
+      assert.equal(uut.closeAll.callCount, 0)
+    })
+  })
+})

psf-memo-db/test/unit/controllers/level.controller.unit.js

@@ -55,4 +55,23 @@ describe('#LevelRESTController', () => {
     assert.equal(ctx.body.success, true)
     assert.equal(ctx.body.key, '600000:abc')
   })
+
+  it('should throw the error status when err has a status', () => {
+    const ctx = { throw: sandbox.stub() }
+    const err = { status: 400, message: 'Bad request' }
+    uut.handleError(ctx, err)
+
+    assert.equal(ctx.throw.callCount, 1)
+    assert.equal(ctx.throw.firstCall.args[0], 400)
+    assert.equal(ctx.throw.firstCall.args[1], 'Bad request')
+  })
+
+  it('should throw 422 when err has no status', () => {
+    const ctx = { throw: sandbox.stub() }
+    const err = new Error('boom')
+    uut.handleError(ctx, err)
+
+    assert.equal(ctx.throw.callCount, 1)
+    assert.equal(ctx.throw.firstCall.args[0], 422)
+  })
 })

psf-memo-db/test/unit/controllers/profile.controller.unit.js

@@ -35,4 +35,23 @@ describe('#ProfileRESTController', () => {
     assert.equal(ctx.body.profiles.length, 1)
     assert.equal(ctx.body.pagination.total, 1)
   })
+
+  it('should throw the error status when err has a status', () => {
+    const ctx = { throw: sandbox.stub() }
+    const err = { status: 404, message: 'Not found' }
+    uut.handleError(ctx, err)
+
+    assert.equal(ctx.throw.callCount, 1)
+    assert.equal(ctx.throw.firstCall.args[0], 404)
+    assert.equal(ctx.throw.firstCall.args[1], 'Not found')
+  })
+
+  it('should throw 500 when err has no status', () => {
+    const ctx = { throw: sandbox.stub() }
+    const err = new Error('boom')
+    uut.handleError(ctx, err)
+
+    assert.equal(ctx.throw.callCount, 1)
+    assert.equal(ctx.throw.firstCall.args[0], 500)
+  })
 })