Add legacy db snake case mappers (#131)

https://eaflood.atlassian.net/browse/WATER-3896

Whilst working on [Add SROC Supplementary Billing Invoice Service](#119) we hit an issue. It was the first time we needed to query the `crm_v2` schema. But that threw errors due to the way [Objection.js knexSnakeCaseMappers()](https://vincit.github.io/objection.js/api/objection/#knexsnakecasemappers) works. It uses Knex's [wrapIdentifier()](https://knexjs.org/guide/#wrapidentifier) and [postProcessResponse()](https://knexjs.org/guide/#postprocessresponse) hooks to see each 'identifier' name. It can then test whether it needs converting, either from or to snake case.

For example, `knex('table').withSchema('foo').select('table.field as otherName').where('id', 1)` will call `wrapIdentifier()` for the values `'table'`, `'foo'`, `'table'`, `'field'`, `'otherName'` and `'id'`.

**knexSnakeCaseMappers()** takes some options, one of them being `underscoreBeforeDigits`. If we didn't use this, fields like `address_line_1` or `section_127_agreement` in the DB would be incorrectly converted to `address_line1` and `section127_agreement`. But this is what leads to our problem.

The previous teams' decision to name one of the schemas `crm_v2` leads to an incorrect conversion. **knexSnakeCaseMappers()** is seeing this and returning `'crm_v_2'`. It doesn't know it's a schema instead of a column name because Knex doesn't provide that context. This is the exact same issue we faced in [Make timestamps consistent at model layer](#85).

Thankfully, a solution that didn't work there will work here. We can provide our own custom Knex snake case mappers implementation which knows to ignore `'crm_v_2'`, whilst calling **Objection.js** own methods for everything else. It also gives us a solution if we face any more funnies like this when dealing with the legacy database.

app/lib/legacy-db-snake-case-mappers.lib.js

@@ -0,0 +1,69 @@
+'use strict'
+
+/**
+ * General helper methods
+ * @module LegacyDbSnakeCaseMappersLib
+ */
+
+const { camelCase, knexIdentifierMappers, snakeCase } = require('objection/lib/utils/identifierMapping')
+
+/**
+ * Return an object containing Knex postProcessResponse() and wrapIdentifier() hooks used in Db query and result parsing
+ *
+ * The standard problem with a JavaScript app talking to a DB is the convention for SQL is to use snake_case for field
+ * names and in Javascript it's camelCase. When dealing with results or sending data to the DB, in code you want to use
+ * camelCase. But the db needs to see snake_case.
+ *
+ * Both Objection.js and Knex have solutions for this; generally referred to as 'snake case mappers'.
+ * `knexfile.application.js` has more details on this.
+ *
+ * But we have had to customise the out-of-the-box solution because of naming choices made by the previous delivery team
+ * in the legacy DB. Specifically, using 'crm_v2' for a schema name. The out-of-the-box solution has an option,
+ * `underscoreBeforeDigits`, for dealing with property names like `addressLine1`. Without it the DB would be sent
+ * `address_line1`.
+ *
+ * So, we have to have this set. But that breaks the schema name parsing. `crmV2` becomes `crm_v_2`. We cannot think of
+ * a way to express it in the code which will make the out-of-the-box solution work. SO, instead we have had to create
+ * our own legacyDbSnakeCaseMappers().
+ *
+ * It simply looks for the value 'crm_v2' and when seen, returns it as is without any formatting. For everything else,
+ * it passes control to the out-of-the-box solution.
+ *
+ * @param {Object} opt Object containing options used by
+ *  {@link https://vincit.github.io/objection.js/api/objection/#knexsnakecasemappers|knexsnakecasemappers()}
+ *
+ * @returns object containing Knex postProcessResponse() and wrapIdentifier() hooks
+ */
+function legacyDbSnakeCaseMappers (opt = {}) {
+  return knexIdentifierMappers({
+    parse: (str) => _legacyCamelCase(str, opt),
+    format: (str) => _legacySnakeCase(str, opt)
+  })
+}
+
+function _legacyCamelCase (str, { upperCase = false } = {}) {
+  if (str === 'crm_v2') {
+    return str
+  }
+
+  return camelCase(str, { upperCase })
+}
+
+function _legacySnakeCase (
+  str,
+  {
+    upperCase = false,
+    underscoreBeforeDigits = false,
+    underscoreBetweenUppercaseLetters = false
+  } = {}
+) {
+  if (str === 'crm_v2') {
+    return str
+  }
+
+  return snakeCase(str, { upperCase, underscoreBeforeDigits, underscoreBetweenUppercaseLetters })
+}
+
+module.exports = {
+  legacyDbSnakeCaseMappers
+}

knexfile.application.js

@@ -1,10 +1,11 @@
 'use strict'
 
-const { knexSnakeCaseMappers } = require('objection')
+const { legacyDbSnakeCaseMappers } = require('./app/lib/legacy-db-snake-case-mappers.lib.js')
 
 /**
- * Passing in `knexSnakeCaseMappers` allows us to use camelCase everywhere and knex will convert it to snake_case on
- * the fly.
+ * Passing in our variant of `knexSnakeCaseMappers` allows us to use camelCase everywhere and knex will convert it to
+ * snake_case on the fly. (see `app/lib/legacy-db-snake-case-mappers.lib.js` for details on why we have our own
+ * variant)
  *
  * This means when we access a property on the model we can use camelCase even if the underlying database property
  * was snake_case. It also means we get camelCase object keys, handy when you need to return a db query result as is
@@ -14,18 +15,16 @@ const { knexSnakeCaseMappers } = require('objection')
  *
  * We set the `underscoreBeforeDigits` option so that properties like lineAttr1 are correctly changed to line_attr_1.
  *
- * However this causes issues with migrations as it still applies the underscore before the digit even if the rest of
- * the name is snake case. So for example, a migration to create line_attr_1 will actually create line_attr__1. We
- * therefore only add `knexSnakeCaseMappers` when running the application to ensure that it isn't applied to
- * migrations.
- *
- *
+ * However, this causes issues with migrations as it works differently. It still applies the underscore before the digit
+ * even if the rest of the name is snake case. For example, a migration to create `line_attr_1` will actually create
+ * `line_attr__1`. So, we only add `knexSnakeCaseMappers` when running the application to ensure that it isn't applied
+ * to migrations.
  */
 
 const knexfile = require('./knexfile')
 
 for (const environment in knexfile) {
-  Object.assign(knexfile[environment], knexSnakeCaseMappers({ underscoreBeforeDigits: true }))
+  Object.assign(knexfile[environment], legacyDbSnakeCaseMappers({ underscoreBeforeDigits: true }))
 }
 
 module.exports = { ...knexfile }

test/lib/legacy-db-snake-case-mappers.lib.test.js

@@ -0,0 +1,117 @@
+'use strict'
+
+// Test framework dependencies
+const Lab = require('@hapi/lab')
+const Code = require('@hapi/code')
+
+const { describe, it } = exports.lab = Lab.script()
+const { expect } = Code
+
+// Thing under test
+const LegacyDbSnakeCaseMappersLib = require('../../app/lib/legacy-db-snake-case-mappers.lib.js')
+
+// NOTE: Ideally, we would have liked to spy on the Objection snakeCase and camelCase methods to confirm they are or are
+// not being called depending on the circumstance. But all our attempts with Sinon failed (a common issue we have when
+// testing with Objection.js)
+describe('RequestLib', () => {
+  describe('#legacyDbSnakeCaseMappers', () => {
+    // We always pass in these options. See knexfile.application.js and how legacyDbSnakeCaseMappers() is called
+    const options = { underscoreBeforeDigits: true }
+
+    describe('when called', () => {
+      it('returns an object containing knex wrapIdentifier() and postProcessResponse() hooks', (options) => {
+        const result = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers()
+
+        expect(result).to.include('wrapIdentifier')
+        expect(result).to.include('postProcessResponse')
+      })
+    })
+
+    describe('when the postProcessResponse() hook it creates is used', () => {
+      const dbResult = [
+        {
+          address_line_1: '10 Downing Street',
+          purpose: 'Residence of the prime minster',
+          is_occupied: true,
+          section_127_Agreement: false,
+          crm_v2: 'I am really a schema disguised as a table column :-)'
+        }
+      ]
+
+      it('handles the knex db result object as expected', () => {
+        const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+        const result = identifierMapping.postProcessResponse(dbResult)
+
+        expect(result).to.equal([
+          {
+            addressLine1: '10 Downing Street',
+            purpose: 'Residence of the prime minster',
+            isOccupied: true,
+            section127Agreement: false,
+            crm_v2: 'I am really a schema disguised as a table column :-)'
+          }
+        ])
+      })
+    })
+
+    describe('when the wrapIdentifier() hook it creates is used', () => {
+      function origWrap (identifier) {
+        return identifier
+      }
+
+      describe('and passed the identifier `addressLine1`', () => {
+        it('returns `address_line_1`', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('addressLine1', origWrap)
+
+          expect(result).to.equal('address_line_1')
+        })
+      })
+
+      describe('and passed the identifier `purpose`', () => {
+        it('returns `purpose`', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('purpose', origWrap)
+
+          expect(result).to.equal('purpose')
+        })
+      })
+
+      describe('and passed the identifier `isOccupied`', () => {
+        it('returns `is_occupied`', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('isOccupied', origWrap)
+
+          expect(result).to.equal('is_occupied')
+        })
+      })
+
+      describe('and passed the identifier `section127Agreement`', () => {
+        it('returns `section_127_agreement`', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('section127Agreement', origWrap)
+
+          expect(result).to.equal('section_127_agreement')
+        })
+      })
+
+      describe('and passed the identifier `crm_v2`', () => {
+        it('returns `crm_v2` (it does no formatting)', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('crm_v2', origWrap)
+
+          expect(result).to.equal('crm_v2')
+        })
+      })
+
+      describe('and passed the identifier `foo_v2`', () => {
+        it('returns `foo_v_2` (confirmation on crm_v2 is special!)', () => {
+          const identifierMapping = LegacyDbSnakeCaseMappersLib.legacyDbSnakeCaseMappers(options)
+          const result = identifierMapping.wrapIdentifier('foo_v2', origWrap)
+
+          expect(result).to.equal('foo_v_2')
+        })
+      })
+    })
+  })
+})