Add some error messages for the developer when .service.js is malform…

…ed (#1894)

Add some error messages for the developer when .service.js is malformed

When loading `.service.js` files which don’t contain services, such as when the export is forgotten, print helpful error messages. This will only occur during development; the unit tests will catch these problems well before code reaches the server.

services/index.js

@@ -1,21 +1,48 @@
 'use strict'
 
 const glob = require('glob')
+const BaseService = require('./base')
 
-function loadServiceClasses() {
-  // New-style services
-  const services = glob
-    .sync(`${__dirname}/**/*.service.js`)
-    .map(path => require(path))
+class InvalidService extends Error {
+  constructor(message) {
+    super(message)
+    this.name = 'InvalidService'
+  }
+}
+
+function loadServiceClasses(servicePaths) {
+  if (!servicePaths) {
+    servicePaths = glob.sync(`${__dirname}/**/*.service.js`)
+  }
 
   const serviceClasses = []
-  services.forEach(service => {
-    if (typeof service === 'function') {
-      serviceClasses.push(service)
-    } else {
-      for (const serviceClass in service) {
-        serviceClasses.push(service[serviceClass])
+  servicePaths.forEach(path => {
+    const module = require(path)
+    if (
+      !module ||
+      (module.constructor === Array && module.length === 0) ||
+      (module.constructor === Object && Object.keys(module).length === 0)
+    ) {
+      throw new InvalidService(
+        `Expected ${path} to export a service or a collection of services`
+      )
+    } else if (module.prototype instanceof BaseService) {
+      serviceClasses.push(module)
+    } else if (module.constructor === Array || module.constructor === Object) {
+      for (const key in module) {
+        const serviceClass = module[key]
+        if (serviceClass.prototype instanceof BaseService) {
+          serviceClasses.push(serviceClass)
+        } else {
+          throw new InvalidService(
+            `Expected ${path} to export a service or a collection of services; one of them was ${serviceClass}`
+          )
+        }
       }
+    } else {
+      throw new InvalidService(
+        `Expected ${path} to export a service or a collection of services; got ${module}`
+      )
     }
   })
 
@@ -27,6 +54,7 @@ function loadTesters() {
 }
 
 module.exports = {
+  InvalidService,
   loadServiceClasses,
   loadTesters,
 }

services/index.spec.js

@@ -0,0 +1,57 @@
+'use strict'
+
+const { expect } = require('chai')
+const { loadServiceClasses, InvalidService } = require('./index')
+
+describe('loadServiceClasses function', function() {
+  it('throws if module exports empty', function() {
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/empty-undefined.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/empty-array.fixture.js'])
+    ).to.throw()
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/empty-object.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/empty-no-export.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses([
+        '../test-fixtures/valid-array.fixture.js',
+        '../test-fixtures/valid-class.fixture.js',
+        '../test-fixtures/empty-array.fixture.js',
+      ])
+    ).to.throw(InvalidService)
+  })
+
+  it('throws if module exports invalid', function() {
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/invalid-no-base.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/invalid-wrong-base.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses(['../test-fixtures/invalid-mixed.fixture.js'])
+    ).to.throw(InvalidService)
+    expect(() =>
+      loadServiceClasses([
+        '../test-fixtures/valid-array.fixture.js',
+        '../test-fixtures/valid-class.fixture.js',
+        '../test-fixtures/invalid-no-base.fixture.js',
+      ])
+    ).to.throw(InvalidService)
+  })
+
+  it('registers services if module exports valid service classes', function() {
+    expect(
+      loadServiceClasses([
+        '../test-fixtures/valid-array.fixture.js',
+        '../test-fixtures/valid-object.fixture.js',
+        '../test-fixtures/valid-class.fixture.js',
+      ])
+    ).to.have.length(5)
+  })
+})

test-fixtures/empty-array.fixture.js

@@ -0,0 +1,3 @@
+'use strict'
+
+module.exports = []

test-fixtures/empty-no-export.fixture.js

@@ -0,0 +1,4 @@
+/* eslint-disable */
+'use strict'
+
+class BadService {}

test-fixtures/empty-object.fixture.js

@@ -0,0 +1,3 @@
+'use strict'
+
+module.exports = {}

test-fixtures/empty-undefined.fixture.js

@@ -0,0 +1,3 @@
+'use strict'
+
+module.exports = undefined

test-fixtures/invalid-mixed.fixture.js

@@ -0,0 +1,9 @@
+'use strict'
+
+const BaseJsonService = require('../services/base-json')
+
+class BadBaseService {}
+class GoodService extends BaseJsonService {}
+class BadService extends BadBaseService {}
+
+module.exports = [GoodService, BadService]

test-fixtures/invalid-no-base.fixture.js

@@ -0,0 +1,5 @@
+'use strict'
+
+class BadService {}
+
+module.exports = BadService

test-fixtures/invalid-wrong-base.fixture.js

@@ -0,0 +1,6 @@
+'use strict'
+
+class BadBaseService {}
+class BadService extends BadBaseService {}
+
+module.exports = BadService

test-fixtures/valid-array.fixture.js

@@ -0,0 +1,9 @@
+'use strict'
+
+const BaseJsonService = require('../services/base-json')
+const LegacyService = require('../services/legacy-service')
+
+class GoodServiceOne extends BaseJsonService {}
+class GoodServiceTwo extends LegacyService {}
+
+module.exports = [GoodServiceOne, GoodServiceTwo]

test-fixtures/valid-class.fixture.js

@@ -0,0 +1,7 @@
+'use strict'
+
+const BaseJsonService = require('../services/base-json')
+
+class GoodService extends BaseJsonService {}
+
+module.exports = GoodService

test-fixtures/valid-object.fixture.js

@@ -0,0 +1,9 @@
+'use strict'
+
+const BaseJsonService = require('../services/base-json')
+const LegacyService = require('../services/legacy-service')
+
+class GoodServiceOne extends BaseJsonService {}
+class GoodServiceTwo extends LegacyService {}
+
+module.exports = { GoodServiceOne, GoodServiceTwo }