Refactor to use ChargingModuleRequestLib (#130)

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

We [previously](#129) introduced `ChargingModuleRequestLib` to handle requests to the Charging Module -- obtaining a Cognito token, formatting the response, etc. This PR updates our existing code to use it.

While doing this we realise that we need to make changes to `ChargingModuleRequestLib` to ensure it plays nicely with calling services. We therefore no longer manipulate the response -- for example, we previously overwrite it with the contents of the response body, meaning we lost things like http headers, status code etc.  We now leave the body where it is.

We also amend how `ChargingModuleCreateBillRunService` returns the created bill run to again remove manipulation of the response. The CM returns bill run data in a `billRun` object within its response; we previously moved the contents out so they weren't nested within this object but we now leave the `billRun` object alone. This means we don't need to make special allowances for things like error responses (ie. "if the response has no body then it is a network error so don't manipulate it, otherwise if it has a body and a bill run then it is a valid response so manipulate it, otherwise if it has a body but no bill run then it is an http error so don't manipulate it").

app/lib/charging-module-request.lib.js

@@ -15,7 +15,7 @@ const servicesConfig = require('../../config/services.config.js')
  *
  * @returns {Object} result An object representing the result of the request
  * @returns {boolean} result.succeeded Whether the request was successful
- * @returns {Object} result.response The Charging Module response if successful; or the error response if not
+ * @returns {Object} result.response The Charging Module response if successful or the error response if not.
  */
 async function get (route) {
   const result = await _sendRequest(route, RequestLib.get)
@@ -31,7 +31,7 @@ async function get (route) {
  *
  * @returns {Object} result An object representing the result of the request
  * @returns {boolean} result.succeeded Whether the request was successful
- * @returns {Object} result.response The Charging Module response if successful; or the error response if not
+ * @returns {Object} result.response The Charging Module response if successful or the error response if not.
  */
 async function post (route, body = {}) {
   const result = await _sendRequest(route, RequestLib.post, body)
@@ -48,7 +48,7 @@ async function post (route, body = {}) {
  *
  * @returns {Object} The result of the request passed back from RequestLib
  */
-async function _sendRequest (route, method, body = {}) {
+async function _sendRequest (route, method, body) {
   const url = new URL(route, servicesConfig.chargingModule.url)
   const authentication = await global.HapiServerMethods.getChargingModuleToken()
   const options = _requestOptions(authentication.accessToken, body)
@@ -63,20 +63,20 @@ function _requestOptions (accessToken, body) {
     headers: {
       authorization: `Bearer ${accessToken}`
     },
-    json: body
+    body: JSON.stringify(body)
   }
 }
 
 /**
  * Parses the response from RequestLib. If the response contains a body then we convert it from JSON to an object.
  */
 function _parseResult (result) {
-  let response = result.response
+  const { response } = result
 
-  // If the request got a response from the Charging Module we will have a response body. If the request errored, for
-  // example a timeout because the Charging Module is down, response will be the instance of the error thrown by Got.
+  // If the request got a response from the Charging Module we will have a response body in JSON format. In this
+  // scenario, we overwrite it with a parsed object.
   if (response.body) {
-    response = JSON.parse(response.body)
+    response.body = JSON.parse(response.body)
   }
 
   return {

app/services/charging-module/create-bill-run.service.js

@@ -5,11 +5,8 @@
  * @module ChargingModuleCreateBillRunService
  */
 
-const ChargingModuleTokenService = require('./token.service.js')
 const RegionModel = require('../../models/water/region.model.js')
-const RequestLib = require('../../lib/request.lib.js')
-
-const servicesConfig = require('../../../config/services.config.js')
+const ChargingModuleRequestLib = require('../../lib/charging-module-request.lib.js')
 
 /**
  * Sends a request to the Charging Module to create a new bill run and returns the result.
@@ -22,31 +19,14 @@ const servicesConfig = require('../../../config/services.config.js')
  * @returns {Object} result.response Details of the created bill run if successful; or the error response if not
  */
 async function go (regionId, ruleset) {
-  const url = new URL('/v3/wrls/bill-runs', servicesConfig.chargingModule.url)
-
-  const authentication = await ChargingModuleTokenService.go()
-
-  const options = await _options(regionId, ruleset, authentication)
-  const result = await RequestLib.post(url.href, options)
-
-  return _parseResult(result)
-}
-
-async function _options (regionId, ruleset, authentication) {
   const region = await _getChargeRegionId(regionId)
 
-  return {
-    headers: {
-      authorization: `Bearer ${authentication.accessToken}`
-    },
-    json: {
-      region,
-      ruleset
-    }
-  }
+  const result = await ChargingModuleRequestLib.post('/v3/wrls/bill-runs', { region, ruleset })
+
+  return result
 }
 
-// Gets the single-letter charge region id for the provided region id UUID
+// Gets the single-letter charge region code for the provided regionId UUID
 async function _getChargeRegionId (regionId) {
   const result = await RegionModel.query()
     .select('chargeRegionId')
@@ -55,22 +35,6 @@ async function _getChargeRegionId (regionId) {
   return result.chargeRegionId
 }
 
-function _parseResult (result) {
-  let response = result.response
-
-  // If the request got a response from the Charging Module we will have a response body. If the request errored, for
-  // example a timeout because the Charging Module is down, response will be the instance of the error thrown by Got.
-  if (response.body) {
-    const parsedBody = JSON.parse(response.body)
-    response = result.succeeded ? parsedBody.billRun : parsedBody
-  }
-
-  return {
-    succeeded: result.succeeded,
-    response
-  }
-}
-
 module.exports = {
   go
 }

app/services/health/info.service.js

@@ -10,6 +10,7 @@ const ChildProcess = require('child_process')
 const util = require('util')
 const exec = util.promisify(ChildProcess.exec)
 
+const ChargingModuleRequestLib = require('../../lib/charging-module-request.lib.js')
 const RequestLib = require('../../lib/request.lib.js')
 
 const servicesConfig = require('../../../config/services.config.js')
@@ -79,8 +80,7 @@ async function _getAddressFacadeData () {
 }
 
 async function _getChargingModuleData () {
-  const statusUrl = new URL('/status', servicesConfig.chargingModule.url)
-  const result = await RequestLib.get(statusUrl.href)
+  const result = await ChargingModuleRequestLib.get('/status')
 
   if (result.succeeded) {
     return result.response.headers['x-cma-docker-tag']

test/lib/charging-module-request.lib.test.js

@@ -64,10 +64,8 @@ describe('ChargingModuleRequestLib', () => {
         expect(result.succeeded).to.be.true()
       })
 
-      it('returns the response as an object', async () => {
-        const { response } = result
-
-        expect(response.testObject.test).to.equal('yes')
+      it('returns the response body as an object', async () => {
+        expect(result.response.body.testObject.test).to.equal('yes')
       })
     })
 
@@ -76,8 +74,9 @@ describe('ChargingModuleRequestLib', () => {
         Sinon.stub(RequestLib, 'get').resolves({
           succeeded: false,
           response: {
-            statusCode: 400,
-            testError: 'TEST_ERROR'
+            statusCode: 404,
+            statusMessage: 'Not Found',
+            body: '{"statusCode":404,"error":"Not Found","message":"Not Found"}'
           }
         })
 
@@ -89,9 +88,7 @@ describe('ChargingModuleRequestLib', () => {
       })
 
       it('returns the error response', async () => {
-        const { response } = result
-
-        expect(response.testError).to.equal('TEST_ERROR')
+        expect(result.response.statusMessage).to.equal('Not Found')
       })
     })
   })
@@ -105,29 +102,28 @@ describe('ChargingModuleRequestLib', () => {
           succeeded: true,
           response: {
             statusCode: 200,
-            body: '{"testObject": {"test": "yes"}}'
+            statusMessage: 'OK',
+            body: '{"testObject": {"test":"yes"}}'
           }
         })
 
-        result = await ChargingModuleRequestLib.post(testRoute, { test: true })
+        result = await ChargingModuleRequestLib.post(testRoute, { test: 'yes' })
       })
 
       it('calls the Charging Module with the required options', async () => {
         const requestArgs = RequestLib.post.firstCall.args
 
         expect(requestArgs[0]).to.endWith('/TEST_ROUTE')
         expect(requestArgs[1].headers).to.include({ authorization: 'Bearer ACCESS_TOKEN' })
-        expect(requestArgs[1].json).to.include({ test: true })
+        expect(requestArgs[1].body).to.equal('{"test":"yes"}')
       })
 
       it('returns a `true` success status', async () => {
         expect(result.succeeded).to.be.true()
       })
 
-      it('returns the response as an object', async () => {
-        const { response } = result
-
-        expect(response.testObject.test).to.equal('yes')
+      it('returns the response body as an object', async () => {
+        expect(result.response.body.testObject.test).to.equal('yes')
       })
     })
 
@@ -136,8 +132,9 @@ describe('ChargingModuleRequestLib', () => {
         Sinon.stub(RequestLib, 'post').resolves({
           succeeded: false,
           response: {
-            statusCode: 400,
-            testError: 'TEST_ERROR'
+            statusCode: 404,
+            statusMessage: 'Not Found',
+            body: '{"statusCode":404,"error":"Not Found","message":"Not Found"}'
           }
         })
 
@@ -149,9 +146,7 @@ describe('ChargingModuleRequestLib', () => {
       })
 
       it('returns the error response', async () => {
-        const { response } = result
-
-        expect(response.testError).to.equal('TEST_ERROR')
+        expect(result.response.statusMessage).to.equal('Not Found')
       })
     })
   })

test/services/charging-module/create-bill-run.service.test.js

@@ -13,8 +13,7 @@ const DatabaseHelper = require('../../support/helpers/database.helper.js')
 const RegionHelper = require('../../support/helpers/water/region.helper.js')
 
 // Things we need to stub
-const ChargingModuleTokenService = require('../../../app/services/charging-module/token.service.js')
-const RequestLib = require('../../../app/lib/request.lib.js')
+const ChargingModuleRequestLib = require('../../../app/lib/charging-module-request.lib.js')
 
 // Thing under test
 const ChargingModuleCreateBillRunService = require('../../../app/services/charging-module/create-bill-run.service.js')
@@ -25,11 +24,6 @@ describe('Charge module create bill run service', () => {
   beforeEach(async () => {
     await DatabaseHelper.clean()
     testRegion = await RegionHelper.add()
-
-    Sinon.stub(ChargingModuleTokenService, 'go').resolves({
-      accessToken: 'ACCESS_TOKEN',
-      expiresIn: 3600
-    })
   })
 
   afterEach(() => {
@@ -40,46 +34,45 @@ describe('Charge module create bill run service', () => {
     let result
 
     beforeEach(async () => {
-      Sinon.stub(RequestLib, 'post').resolves({
+      Sinon.stub(ChargingModuleRequestLib, 'post').resolves({
         succeeded: true,
         response: {
-          statusCode: 200,
-          body: '{"billRun": {"id": "2bbbe459-966e-4026-b5d2-2f10867bdddd", "billRunNumber": 10004}}'
+          body: {
+            billRun: {
+              id: '2bbbe459-966e-4026-b5d2-2f10867bdddd',
+              billRunNumber: 10004
+            }
+          }
         }
       })
 
       result = await ChargingModuleCreateBillRunService.go(testRegion.regionId, 'sroc')
     })
 
-    it('calls the Charging Module with the required options', async () => {
-      const requestArgs = RequestLib.post.firstCall.args
-
-      expect(requestArgs[0]).to.endWith('/wrls/bill-runs')
-      expect(requestArgs[1].headers).to.include({ authorization: 'Bearer ACCESS_TOKEN' })
-      expect(requestArgs[1].json).to.include({ region: testRegion.chargeRegionId, ruleset: 'sroc' })
-    })
-
     it('returns a `true` success status', async () => {
       expect(result.succeeded).to.be.true()
     })
 
     it('returns the bill run id and number in the `response`', async () => {
       const { response } = result
 
-      expect(response.id).to.equal('2bbbe459-966e-4026-b5d2-2f10867bdddd')
-      expect(response.billRunNumber).to.equal(10004)
+      expect(response.body.billRun.id).to.equal('2bbbe459-966e-4026-b5d2-2f10867bdddd')
+      expect(response.body.billRun.billRunNumber).to.equal(10004)
     })
   })
 
   describe('when the service cannot create a bill run', () => {
     describe('because the request did not return a 2xx/3xx response', () => {
       beforeEach(async () => {
-        Sinon.stub(RequestLib, 'post').resolves({
+        Sinon.stub(ChargingModuleRequestLib, 'post').resolves({
           succeeded: false,
           response: {
-            // For consistency we include the status code in the response but note that it is also returned in the body
-            statusCode: 401,
-            body: '{"statusCode":401,"error":"Unauthorized","message":"Invalid JWT: Token format not valid","attributes":{"error":"Invalid JWT: Token format not valid"}}'
+            body: {
+              statusCode: 401,
+              error: 'Unauthorized',
+              message: 'Invalid JWT: Token format not valid',
+              attributes: { error: 'Invalid JWT: Token format not valid' }
+            }
           }
         })
       })
@@ -93,15 +86,15 @@ describe('Charge module create bill run service', () => {
       it('returns the error in the `response`', async () => {
         const result = await ChargingModuleCreateBillRunService.go(testRegion.regionId, 'sroc')
 
-        expect(result.response.statusCode).to.equal(401)
-        expect(result.response.error).to.equal('Unauthorized')
-        expect(result.response.message).to.equal('Invalid JWT: Token format not valid')
+        expect(result.response.body.statusCode).to.equal(401)
+        expect(result.response.body.error).to.equal('Unauthorized')
+        expect(result.response.body.message).to.equal('Invalid JWT: Token format not valid')
       })
     })
 
     describe('because the request attempt returned an error, for example, TimeoutError', () => {
       beforeEach(async () => {
-        Sinon.stub(RequestLib, 'post').resolves({
+        Sinon.stub(ChargingModuleRequestLib, 'post').resolves({
           succeeded: false,
           response: new Error("Timeout awaiting 'request' for 5000ms")
         })

test/services/health/info.service.test.js

@@ -13,6 +13,7 @@ const { expect } = Code
 const servicesConfig = require('../../../config/services.config.js')
 
 // Things we need to stub
+const ChargingModuleRequestLib = require('../../../app/lib/charging-module-request.lib.js')
 const RequestLib = require('../../../app/lib/request.lib.js')
 
 // Thing under test
@@ -35,15 +36,13 @@ describe('Info service', () => {
     },
     app: { succeeded: true, response: { statusCode: 200, body: '{ "version": "9.0.99", "commit": "99d0e8c" }' } }
   }
+  let chargingModuleRequestLibStub
   let requestLibStub
 
   beforeEach(() => {
     requestLibStub = Sinon.stub(RequestLib, 'get')
     // These requests will remain unchanged throughout the tests. We do alter the ones to the AddressFacade and the
     // water-api (foreground-service) though, which is why they are defined separately in each test.
-    requestLibStub
-      .withArgs(`${servicesConfig.chargingModule.url}/status`)
-      .resolves(goodRequestResults.chargingModule)
     requestLibStub
       .withArgs(`${servicesConfig.serviceBackground.url}/health/info`)
       .resolves(goodRequestResults.app)
@@ -71,6 +70,11 @@ describe('Info service', () => {
     requestLibStub
       .withArgs(`${servicesConfig.returns.url}/health/info`)
       .resolves(goodRequestResults.app)
+
+    chargingModuleRequestLibStub = Sinon.stub(ChargingModuleRequestLib, 'get')
+    chargingModuleRequestLibStub
+      .withArgs('/status')
+      .resolves(goodRequestResults.chargingModule)
   })
 
   afterEach(() => {