fail to start server if there are duplicate service names (#9099)

* fail to start server if there are duplicate service names

* update class names in loader test fixtures

---------

Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>

core/base-service/loader-test-fixtures/invalid-mixed.fixture.js

@@ -1,10 +1,10 @@
 import BaseJsonService from '../base-json.js'
 
 class BadBaseService {}
-class GoodService extends BaseJsonService {
+class GoodMixedService extends BaseJsonService {
   static category = 'build'
   static route = { base: 'it/is', pattern: 'good' }
 }
-class BadService extends BadBaseService {}
+class BadMixedService extends BadBaseService {}
 
-export default [GoodService, BadService]
+export default [GoodMixedService, BadMixedService]

core/base-service/loader-test-fixtures/invalid-no-base.fixture.js

@@ -1,3 +1,3 @@
-class BadService {}
+class BadNoBaseService {}
 
-export default BadService
+export default BadNoBaseService

core/base-service/loader-test-fixtures/invalid-wrong-base.fixture.js

@@ -1,4 +1,4 @@
 class BadBaseService {}
-class BadService extends BadBaseService {}
+class BadChildService extends BadBaseService {}
 
-export default BadService
+export default BadChildService

core/base-service/loader-test-fixtures/valid-array.fixture.js

@@ -1,12 +1,12 @@
 import BaseJsonService from '../base-json.js'
 
-class GoodServiceOne extends BaseJsonService {
+class GoodServiceArrayOne extends BaseJsonService {
   static category = 'build'
   static route = { base: 'good', pattern: 'one' }
 }
-class GoodServiceTwo extends BaseJsonService {
+class GoodServiceArrayTwo extends BaseJsonService {
   static category = 'build'
   static route = { base: 'good', pattern: 'two' }
 }
 
-export default [GoodServiceOne, GoodServiceTwo]
+export default [GoodServiceArrayOne, GoodServiceArrayTwo]

core/base-service/loader-test-fixtures/valid-object.fixture.js

@@ -1,12 +1,12 @@
 import BaseJsonService from '../base-json.js'
 
-class GoodServiceOne extends BaseJsonService {
+class GoodServiceObjectOne extends BaseJsonService {
   static category = 'build'
   static route = { base: 'good', pattern: 'one' }
 }
-class GoodServiceTwo extends BaseJsonService {
+class GoodServiceObjectTwo extends BaseJsonService {
   static category = 'build'
   static route = { base: 'good', pattern: 'two' }
 }
 
-export { GoodServiceOne, GoodServiceTwo }
+export { GoodServiceObjectOne, GoodServiceObjectTwo }

core/base-service/loader.js

@@ -31,6 +31,18 @@ function getServicePaths(pattern) {
   return globSync(toUnixPath(path.join(serviceDir, '**', pattern))).sort()
 }
 
+function assertNamesUnique(names, { message }) {
+  const duplicates = {}
+  Object.entries(countBy(names))
+    .filter(([name, count]) => count > 1)
+    .forEach(([name, count]) => {
+      duplicates[name] = count
+    })
+  if (Object.keys(duplicates).length) {
+    throw new Error(`${message}: ${JSON.stringify(duplicates, undefined, 2)}`)
+  }
+}
+
 async function loadServiceClasses(servicePaths) {
   if (!servicePaths) {
     servicePaths = getServicePaths('*.service.js')
@@ -64,29 +76,14 @@ async function loadServiceClasses(servicePaths) {
     })
   }
 
-  return serviceClasses
-}
-
-function assertNamesUnique(names, { message }) {
-  const duplicates = {}
-  Object.entries(countBy(names))
-    .filter(([name, count]) => count > 1)
-    .forEach(([name, count]) => {
-      duplicates[name] = count
-    })
-  if (Object.keys(duplicates).length) {
-    throw new Error(`${message}: ${JSON.stringify(duplicates, undefined, 2)}`)
-  }
-}
-
-async function checkNames() {
-  const services = await loadServiceClasses()
   assertNamesUnique(
-    services.map(({ name }) => name),
+    serviceClasses.map(({ name }) => name),
     {
       message: 'Duplicate service names found',
     }
   )
+
+  return serviceClasses
 }
 
 async function collectDefinitions() {
@@ -114,7 +111,6 @@ export {
   InvalidService,
   loadServiceClasses,
   getServicePaths,
-  checkNames,
   collectDefinitions,
   loadTesters,
 }

services/check-services.spec.js

@@ -1,11 +1,14 @@
-import { checkNames, collectDefinitions } from '../core/base-service/loader.js'
+import {
+  loadServiceClasses,
+  collectDefinitions,
+} from '../core/base-service/loader.js'
 
 // When these tests fail, they will throw AssertionErrors. Wrapping them in an
 // `expect().not.to.throw()` makes the error output unreadable.
 
 it('Services have unique names', async function () {
   this.timeout(30000)
-  await checkNames()
+  await loadServiceClasses()
 })
 
 it('Can collect the service definitions', async function () {