Replacing classes with modules and functions Pt.2 (#48)

This follows on from [Replacing classes with modules and functions](#46) as we managed to miss some bits! This second attempt hopefully covers all the bits we missed on the first pass of refactoring things away from classes.
DEFRA · Dec 9, 2022 · f5b1d68 · f5b1d68
1 parent 06d45e8
commit f5b1d68
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 134 deletions.
diff --git a/app/services/plugins/filter-routes.service.js b/app/services/plugins/filter-routes.service.js
@@ -5,46 +5,46 @@
  * @module FilterRoutesService
  */
 
-class FilterRoutesService {
-  /**
-   * When running in a production environment ('pre' or 'prd') filter the routes we register with Hapi.
-   *
-   * Initially conceived as a very simple 'feature toggle' solution. In the main our features are related to endpoints;
-   * a new feature typically means a new endpoint. We want the ability to work on new features, but still push other
-   * changes, for example, patches and fixes into production.
-   *
-   * So, we use this service to ensure any endpoint that is still being worked on is not available when the API is
-   * running in production. We also include pre-production in our protected environments so we can test and ensure
-   * an endpoint does not get registered as part of our release testing and sign-off.
-   *
-   * We identify routes to filter because they have a custom `excludeFromProd` property which has been added to the
-   * {@link https://hapi.dev/api/?v=20.1.3#-routeoptionsapp|route options app property} the Hapi provided place apps
-   * can store route configuration
-   *
-   * This service is used by the `RouterPlugin` to check which routes need filtering before it then registers them with
-   * the Hapi server instance.
-   *
-   * @param {Object[]} routes An array of Hapi routes expected to be provided by the `RouterPlugin`
-   * @param {string} environment The current environment ('dev', 'tst', 'tra', 'pre' or 'prd')
-   *
-   * @returns {Object[]} an array of Hapi routes, filtered depending on the current environment and whether any paths
-   * have been registered as needing filtering
-   */
-  static go (routes, environment) {
-    if (this._protectedEnvironment(environment)) {
-      return this._filteredRoutes(routes)
-    }
-
-    return routes
+/**
+ * When running in a production environment ('pre' or 'prd') filter the routes we register with Hapi.
+ *
+ * Initially conceived as a very simple 'feature toggle' solution. In the main our features are related to endpoints;
+ * a new feature typically means a new endpoint. We want the ability to work on new features, but still push other
+ * changes, for example, patches and fixes into production.
+ *
+ * So, we use this service to ensure any endpoint that is still being worked on is not available when the API is
+ * running in production. We also include pre-production in our protected environments so we can test and ensure
+ * an endpoint does not get registered as part of our release testing and sign-off.
+ *
+ * We identify routes to filter because they have a custom `excludeFromProd` property which has been added to the
+ * {@link https://hapi.dev/api/?v=20.1.3#-routeoptionsapp|route options app property} the Hapi provided place apps
+ * can store route configuration
+ *
+ * This service is used by the `RouterPlugin` to check which routes need filtering before it then registers them with
+ * the Hapi server instance.
+ *
+ * @param {Object[]} routes An array of Hapi routes expected to be provided by the `RouterPlugin`
+ * @param {string} environment The current environment ('dev', 'tst', 'tra', 'pre' or 'prd')
+ *
+ * @returns {Object[]} an array of Hapi routes, filtered depending on the current environment and whether any paths
+ * have been registered as needing filtering
+ */
+function go (routes, environment) {
+  if (_protectedEnvironment(environment)) {
+    return _filteredRoutes(routes)
   }
 
-  static _protectedEnvironment (environment) {
-    return ['pre', 'prd'].includes(environment)
-  }
+  return routes
+}
 
-  static _filteredRoutes (routes) {
-    return routes.filter(route => !route?.options?.app?.excludeFromProd)
-  }
+function _protectedEnvironment (environment) {
+  return ['pre', 'prd'].includes(environment)
+}
+
+function _filteredRoutes (routes) {
+  return routes.filter(route => !route?.options?.app?.excludeFromProd)
 }
 
-module.exports = FilterRoutesService
+module.exports = {
+  go
+}
diff --git a/app/services/plugins/hapi-pino-ignore-request.service.js b/app/services/plugins/hapi-pino-ignore-request.service.js
@@ -7,54 +7,54 @@
 
 const LogConfig = require('../../../config/log.config.js')
 
-class HapiPinoIgnoreRequestService {
-  /**
-   * Returns true or false whether a request should be loged
-   *
-   * Used by `app/plugins/hapi_pino.plugin.js` to control what does and doesn't get added to our log output. We built
-   * `/status` to support AWS load balancer health checks which fire approximately every 500ms. If we logged these
-   * requests our log would be too noisy to prove useful. (`/` and `/status` go to the same place hence both are
-   * listed).
-   *
-   * When a view is requested, a number of assets will be requested along with it. So, a single request for a page will
-   * result in the following log entries
-   *
-   * ```text
-   * [09:41:06.763] INFO (17542): [response] get /service-status 200 (1138ms)
-   * [09:41:06.871] INFO (17542): [response] get /assets/stylesheets/application.css 200 (72ms)
-   * [09:41:06.873] INFO (17542): [response] get /assets/all.js 200 (64ms)
-   * [09:41:06.893] INFO (17542): [response] get /assets/images/govuk-crest.png 200 (8ms)
-   * [09:41:06.926] INFO (17542): [response] get /assets/fonts/light-94a07e06a1-v2.woff2 200 (19ms)
-   * [09:41:06.928] INFO (17542): [response] get /assets/fonts/bold-b542beb274-v2.woff2 200 (18ms)
-   * [09:41:07.032] INFO (17542): [response] get /assets/images/favicon.ico 200 (6ms)
-   * ```
-   *
-   * And these are just the first line from each entry. Because we log both the request and response details when
-   * viewed locally each entry is 41 lines long!
-   *
-   * So, we also do not log any requests to `/assets/*`.
-   *
-   * @param {Object} _options The options passed to the HapiPino plugin
-   * @param {request} request Hapi request object created internally for each incoming request
-   *
-   * @returns {boolean} true if the request should be ignored, else false
-   */
-  static go (_options, request) {
-    const staticPaths = ['/', '/status']
-
-    // If request is a known path ignore it
-    if (staticPaths.includes(request.path)) {
-      return true
-    }
+/**
+ * Returns true or false whether a request should be loged
+ *
+ * Used by `app/plugins/hapi_pino.plugin.js` to control what does and doesn't get added to our log output. We built
+ * `/status` to support AWS load balancer health checks which fire approximately every 500ms. If we logged these
+ * requests our log would be too noisy to prove useful. (`/` and `/status` go to the same place hence both are
+ * listed).
+ *
+ * When a view is requested, a number of assets will be requested along with it. So, a single request for a page will
+ * result in the following log entries
+ *
+ * ```text
+ * [09:41:06.763] INFO (17542): [response] get /service-status 200 (1138ms)
+ * [09:41:06.871] INFO (17542): [response] get /assets/stylesheets/application.css 200 (72ms)
+ * [09:41:06.873] INFO (17542): [response] get /assets/all.js 200 (64ms)
+ * [09:41:06.893] INFO (17542): [response] get /assets/images/govuk-crest.png 200 (8ms)
+ * [09:41:06.926] INFO (17542): [response] get /assets/fonts/light-94a07e06a1-v2.woff2 200 (19ms)
+ * [09:41:06.928] INFO (17542): [response] get /assets/fonts/bold-b542beb274-v2.woff2 200 (18ms)
+ * [09:41:07.032] INFO (17542): [response] get /assets/images/favicon.ico 200 (6ms)
+ * ```
+ *
+ * And these are just the first line from each entry. Because we log both the request and response details when
+ * viewed locally each entry is 41 lines long!
+ *
+ * So, we also do not log any requests to `/assets/*`.
+ *
+ * @param {Object} _options The options passed to the HapiPino plugin
+ * @param {request} request Hapi request object created internally for each incoming request
+ *
+ * @returns {boolean} true if the request should be ignored, else false
+ */
+function go (_options, request) {
+  const staticPaths = ['/', '/status']
 
-    // If logging asset requests is disabled and the request is for an asset ignore it
-    if (!LogConfig.logAssetRequests && request.path.startsWith('/assets')) {
-      return true
-    }
+  // If request is a known path ignore it
+  if (staticPaths.includes(request.path)) {
+    return true
+  }
 
-    // Do not ignore all other requests
-    return false
+  // If logging asset requests is disabled and the request is for an asset ignore it
+  if (!LogConfig.logAssetRequests && request.path.startsWith('/assets')) {
+    return true
   }
+
+  // Do not ignore all other requests
+  return false
 }
 
-module.exports = HapiPinoIgnoreRequestService
+module.exports = {
+  go
+}
diff --git a/test/services/service-status.service.test.js b/test/services/service-status.service.test.js
@@ -54,11 +54,11 @@ describe('Service Status service', () => {
       Nock(servicesConfig.serviceForeground.url).get('/health/info').reply(200, { version: '8.0.1', commit: '83d0e8c' })
 
       // Unfortunately, this convoluted test setup is the only way we've managed to stub how the promisified version of
-      // `child-process.exec()` behaves in the class under test.
+      // `child-process.exec()` behaves in the module under test.
       // We create an anonymous stub, which responds differently depending on which service is being checked. We then
       // stub the util library's `promisify()` method and tell it to calll our anonymous stub when invoked. The bit that
       // makes all this work is the fact we use Proxyquire to load our stubbed util instead of the real one when we load
-      // our class under test
+      // our module under test
       const execStub = Sinon
         .stub()
         .withArgs('clamdscan --version')

diff --git a/test/support/helpers/billing-batch.helper.js b/test/support/helpers/billing-batch.helper.js
@@ -6,56 +6,56 @@
 
 const { db } = require('../../../db/db.js')
 
-class BillingBatchHelper {
-  /**
-   * Add a new billing batch
-   *
-   * If no `data` is provided, default values will be used. These are
-   *
-   * - `region_id` - bd114474-790f-4470-8ba4-7b0cc9c225d7
-   * - `batch_type` - supplementary
-   * - `from_financial_year_ending` - 2023
-   * - `to_financial_year_ending` - 2023
-   * - `status` - processing
-   * - `scheme` - sroc
-   *
-   * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database
-   *
-   * @returns {string} The ID of the newly created record
-   */
-  static async add (data = {}) {
-    const insertData = this.defaults(data)
-
-    const result = await db.table('water.billing_batches')
-      .insert(insertData)
-      .returning('billing_batch_id')
-
-    return result
+/**
+ * Add a new billing batch
+ *
+ * If no `data` is provided, default values will be used. These are
+ *
+ * - `region_id` - bd114474-790f-4470-8ba4-7b0cc9c225d7
+ * - `batch_type` - supplementary
+ * - `from_financial_year_ending` - 2023
+ * - `to_financial_year_ending` - 2023
+ * - `status` - processing
+ * - `scheme` - sroc
+ *
+ * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database
+ *
+ * @returns {string} The ID of the newly created record
+ */
+async function add (data = {}) {
+  const insertData = defaults(data)
+
+  const result = await db.table('water.billing_batches')
+    .insert(insertData)
+    .returning('billing_batch_id')
+
+  return result
+}
+
+/**
+ * Returns the defaults used when creating a new billing batch
+ *
+ * It will override or append to them any data provided. Mainly used by the `add()` method, we make it available
+ * for use in tests to avoid having to duplicate values.
+ *
+ * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database
+ */
+function defaults (data = {}) {
+  const defaults = {
+    region_id: 'bd114474-790f-4470-8ba4-7b0cc9c225d7',
+    batch_type: 'supplementary',
+    from_financial_year_ending: 2023,
+    to_financial_year_ending: 2023,
+    status: 'processing',
+    scheme: 'sroc'
   }
 
-  /**
-   * Returns the defaults used when creating a new billing batch
-   *
-   * It will override or append to them any data provided. Mainly used by the `add()` method, we make it available
-   * for use in tests to avoid having to duplicate values.
-   *
-   * @param {Object} [data] Any data you want to use instead of the defaults used here or in the database
-   */
-  static defaults (data = {}) {
-    const defaults = {
-      region_id: 'bd114474-790f-4470-8ba4-7b0cc9c225d7',
-      batch_type: 'supplementary',
-      from_financial_year_ending: 2023,
-      to_financial_year_ending: 2023,
-      status: 'processing',
-      scheme: 'sroc'
-    }
-
-    return {
-      ...defaults,
-      ...data
-    }
+  return {
+    ...defaults,
+    ...data
   }
 }
 
-module.exports = BillingBatchHelper
+module.exports = {
+  add
+}