refactor: 💡 ServicesManager factory functions vs instances (#1272)

* refactor: 💡 ServicesManager factory functions vs instances

Refactor servicesManager to take factory functions instead of instances

Closes: #1265

* Remove testing code

platform/core/src/index.js

@@ -20,11 +20,11 @@ import user from './user.js';
 import utils from './utils/';
 
 import {
-  createUINotificationService,
-  createUIModalService,
-  createUIDialogService,
-  createUIContextMenuService,
-  createUILabellingFlowService,
+  UINotificationService,
+  UIModalService,
+  UIDialogService,
+  UIContextMenuService,
+  UILabellingFlowService,
 } from './services';
 
 const OHIF = {
@@ -52,11 +52,11 @@ const OHIF = {
   measurements,
   hangingProtocols,
   //
-  createUINotificationService,
-  createUIModalService,
-  createUIDialogService,
-  createUIContextMenuService,
-  createUILabellingFlowService,
+  UINotificationService,
+  UIModalService,
+  UIDialogService,
+  UIContextMenuService,
+  UILabellingFlowService,
 };
 
 export {
@@ -83,11 +83,11 @@ export {
   measurements,
   hangingProtocols,
   //
-  createUINotificationService,
-  createUIModalService,
-  createUIDialogService,
-  createUIContextMenuService,
-  createUILabellingFlowService,
+  UINotificationService,
+  UIModalService,
+  UIDialogService,
+  UIContextMenuService,
+  UILabellingFlowService,
 };
 
 export { OHIF };

platform/core/src/index.test.js

@@ -10,11 +10,11 @@ describe('Top level exports', () => {
       'HotkeysManager',
       'ServicesManager',
       //
-      'createUINotificationService',
-      'createUIModalService',
-      'createUIDialogService',
-      'createUIContextMenuService',
-      'createUILabellingFlowService',
+      'UINotificationService',
+      'UIModalService',
+      'UIDialogService',
+      'UIContextMenuService',
+      'UILabellingFlowService',
       //
       'utils',
       'studies',

platform/core/src/services/ServicesManager.js

@@ -7,10 +7,12 @@ export default class ServicesManager {
   }
 
   /**
+   * Registers a new service.
    *
    * @param {Object} service
+   * @param {Object} configuration
    */
-  registerService(service) {
+  registerService(service, configuration = {}) {
     if (!service) {
       log.warn(
         'Attempting to register a null/undefined service. Exiting early.'
@@ -18,32 +20,45 @@ export default class ServicesManager {
       return;
     }
 
-    let serviceName = service.name;
-
-    if (!serviceName) {
+    if (!service.name) {
       log.warn(`Service name not set. Exiting early.`);
       return;
     }
 
-    if (this.registeredServiceNames.includes(serviceName)) {
+    if (this.registeredServiceNames.includes(service.name)) {
       log.warn(
-        `Extension name ${serviceName} has already been registered. Exiting before duplicating services.`
+        `Service name ${service.name} has already been registered. Exiting before duplicating services.`
       );
       return;
     }
 
-    this.services[service.name] = service;
+    if (service.create) {
+      this.services[service.name] = service.create({ configuration });
+    } else {
+      log.warn(`Service create factory function not defined. Exiting early.`);
+      return;
+    }
 
-    // Track service registration
-    this.registeredServiceNames.push(serviceName);
+    /* Track service registration */
+    this.registeredServiceNames.push(service.name);
   }
 
   /**
-   * An array of services.
+   * An array of services, or an array of arrays that contains service
+   * configuration pairs.
    *
    * @param {Object[]} services - Array of services
    */
   registerServices(services) {
-    services.forEach(service => this.registerService(service));
+    services.forEach(service => {
+      const hasConfiguration = Array.isArray(service);
+
+      if (hasConfiguration) {
+        const [ohifService, configuration] = service;
+        this.registerService(ohifService, configuration);
+      } else {
+        this.registerService(service);
+      }
+    });
   }
 }

platform/core/src/services/ServicesManager.test.js

@@ -16,18 +16,32 @@ describe('ServicesManager.js', () => {
     it('calls registerService() for each service', () => {
       servicesManager.registerService = jest.fn();
 
-      const fakeServices = [
-        { name: 'UINotificationTestService', hide: jest.fn() },
-        { name: 'UIModalTestService', hide: jest.fn() },
-      ];
-
-      servicesManager.registerServices(fakeServices);
+      servicesManager.registerServices([
+        { name: 'UINotificationTestService', create: jest.fn() },
+        { name: 'UIModalTestService', create: jest.fn() },
+      ]);
 
       expect(servicesManager.registerService.mock.calls.length).toBe(2);
     });
+
+    it('calls registerService() for each service passing its configuration if tuple', () => {
+      servicesManager.registerService = jest.fn();
+      const fakeConfiguration = { testing: true };
+
+      servicesManager.registerServices([
+        { name: 'UINotificationTestService', create: jest.fn() },
+        [{ name: 'UIModalTestService', create: jest.fn() }, fakeConfiguration],
+      ]);
+
+      expect(servicesManager.registerService.mock.calls[1]).toContain(
+        fakeConfiguration
+      );
+    });
   });
 
   describe('registerService()', () => {
+    const fakeService = { name: 'UINotificationService', create: jest.fn() };
+
     it('logs a warning if the service is null or undefined', () => {
       const undefinedService = undefined;
       const nullService = null;
@@ -39,8 +53,8 @@ describe('ServicesManager.js', () => {
     });
 
     it('logs a warning if the service does not have a name', () => {
-      const serviceWithEmptyName = { name: '', hide: jest.fn() };
-      const serviceWithoutName = { hide: jest.fn() };
+      const serviceWithEmptyName = { name: '', create: jest.fn() };
+      const serviceWithoutName = { create: jest.fn() };
 
       servicesManager.registerService(serviceWithEmptyName);
       servicesManager.registerService(serviceWithoutName);
@@ -48,23 +62,37 @@ describe('ServicesManager.js', () => {
       expect(log.warn.mock.calls.length).toBe(2);
     });
 
-    it('tracks which services have been registered', () => {
-      const service = {
-        name: 'UINotificationService',
-      };
+    it('logs a warning if the service does not have a create factory function', () => {
+      const serviceWithoutCreate = { name: 'UINotificationService' };
 
-      servicesManager.registerService(service);
-
-      expect(servicesManager.registeredServiceNames).toContain(service.name);
-    });
-
-    it('logs a warning if the service has an name that has already been registered', () => {
-      const service = { name: 'UINotificationService' };
-
-      servicesManager.registerService(service);
-      servicesManager.registerService(service);
+      servicesManager.registerService(serviceWithoutCreate);
 
       expect(log.warn.mock.calls.length).toBe(1);
     });
+
+    it('tracks which services have been registered', () => {
+      servicesManager.registerService(fakeService);
+
+      expect(servicesManager.registeredServiceNames).toContain(
+        fakeService.name
+      );
+    });
+
+    it('logs a warning if the service has an name that has already been registered', () => {
+      servicesManager.registerService(fakeService);
+      servicesManager.registerService(fakeService);
+
+      expect(log.warn.mock.calls.length).toBe(1);
+    });
+
+    it('pass configuration to service create factory function', () => {
+      const configuration = { config: 'Some configuration' };
+
+      servicesManager.registerService(fakeService, configuration);
+
+      expect(fakeService.create.mock.calls[0][0]).toEqual({
+        configuration,
+      });
+    });
   });
 });

platform/core/src/services/UIContextMenuService/index.js

@@ -5,29 +5,27 @@
  * @property {Event} event The event with tool information.
  */
 
-const uiContextMenuServicePublicAPI = {
-  name: 'UIContextMenuService',
-  hide,
-  show,
+const name = 'UIContextMenuService';
+
+const publicAPI = {
+  name,
+  hide: _hide,
+  show: _show,
   setServiceImplementation,
 };
 
-const uiContextMenuServiceImplementation = {
+const serviceImplementation = {
   _show: () => console.warn('show() NOT IMPLEMENTED'),
   _hide: () => console.warn('hide() NOT IMPLEMENTED'),
 };
 
-function createUIContextMenuService() {
-  return uiContextMenuServicePublicAPI;
-}
-
 /**
  * Show a new UI ContextMenu dialog;
  *
  * @param {ContextMenuProps} props { event }
  */
-function show({ event }) {
-  return uiContextMenuServiceImplementation._show({
+function _show({ event }) {
+  return serviceImplementation._show({
     event,
   });
 }
@@ -36,8 +34,8 @@ function show({ event }) {
  * Hide a UI ContextMenu dialog;
  *
  */
-function hide() {
-  return uiContextMenuServiceImplementation._hide();
+function _hide() {
+  return serviceImplementation._hide();
 }
 
 /**
@@ -53,11 +51,16 @@ function setServiceImplementation({
   hide: hideImplementation,
 }) {
   if (showImplementation) {
-    uiContextMenuServiceImplementation._show = showImplementation;
+    serviceImplementation._show = showImplementation;
   }
   if (hideImplementation) {
-    uiContextMenuServiceImplementation._hide = hideImplementation;
+    serviceImplementation._hide = hideImplementation;
   }
 }
 
-export default createUIContextMenuService;
+export default {
+  name,
+  create: ({ configuration = {} }) => {
+    return publicAPI;
+  },
+};

platform/core/src/services/UIDialogService/index.js

@@ -25,30 +25,28 @@
  * @property {Function} onDrag Called while dragging.
  */
 
-const uiDialogServicePublicAPI = {
-  name: 'UIDialogService',
-  dismiss,
-  dismissAll,
-  create,
+const name = 'UIDialogService';
+
+const publicAPI = {
+  name,
+  dismiss: _dismiss,
+  dismissAll: _dismissAll,
+  create: _create,
   setServiceImplementation,
 };
 
-const uiDialogServiceImplementation = {
+const serviceImplementation = {
   _dismiss: () => console.warn('dismiss() NOT IMPLEMENTED'),
   _dismissAll: () => console.warn('dismissAll() NOT IMPLEMENTED'),
   _create: () => console.warn('create() NOT IMPLEMENTED'),
 };
 
-function createUIDialogService() {
-  return uiDialogServicePublicAPI;
-}
-
 /**
  * Show a new UI dialog;
  *
  * @param {DialogProps} props { id, content, contentProps, onStart, onDrag, onStop, centralize, isDraggable, showOverlay, preservePosition, defaultPosition }
  */
-function create({
+function _create({
   id,
   content,
   contentProps,
@@ -61,7 +59,7 @@ function create({
   showOverlay = false,
   defaultPosition,
 }) {
-  return uiDialogServiceImplementation._create({
+  return serviceImplementation._create({
     id,
     content,
     contentProps,
@@ -81,8 +79,8 @@ function create({
  *
  * @returns void
  */
-function dismissAll() {
-  return uiDialogServiceImplementation._dismissAll();
+function _dismissAll() {
+  return serviceImplementation._dismissAll();
 }
 
 /**
@@ -90,8 +88,8 @@ function dismissAll() {
  *
  * @returns void
  */
-function dismiss({ id }) {
-  return uiDialogServiceImplementation._dismiss({ id });
+function _dismiss({ id }) {
+  return serviceImplementation._dismiss({ id });
 }
 
 /**
@@ -109,14 +107,19 @@ function setServiceImplementation({
   create: createImplementation,
 }) {
   if (dismissImplementation) {
-    uiDialogServiceImplementation._dismiss = dismissImplementation;
+    serviceImplementation._dismiss = dismissImplementation;
   }
   if (dismissAllImplementation) {
-    uiDialogServiceImplementation._dismissAll = dismissAllImplementation;
+    serviceImplementation._dismissAll = dismissAllImplementation;
   }
   if (createImplementation) {
-    uiDialogServiceImplementation._create = createImplementation;
+    serviceImplementation._create = createImplementation;
   }
 }
 
-export default createUIDialogService;
+export default {
+  name,
+  create: ({ configuration = {} }) => {
+    return publicAPI;
+  },
+};

platform/core/src/services/UILabellingFlowService/index.js

@@ -8,28 +8,26 @@
  *
  */
 
-const uiLabellingFlowServicePublicAPI = {
-  name: 'UILabellingFlowService',
-  show,
-  hide,
+const name = 'UILabellingFlowService';
+
+const publicAPI = {
+  name,
+  show: _show,
+  hide: _hide,
   setServiceImplementation,
 };
 
-const uiLabellingFlowServiceImplementation = {
+const serviceImplementation = {
   _show: () => console.warn('show() NOT IMPLEMENTED'),
   _hide: () => console.warn('hide() NOT IMPLEMENTED'),
 };
 
-function createUILabellingFlowService() {
-  return uiLabellingFlowServicePublicAPI;
-}
-
 /**
  * Hide a UI LabellingFlow dialog;
  *
  */
-function hide() {
-  return uiLabellingFlowServiceImplementation._hide();
+function _hide() {
+  return serviceImplementation._hide();
 }
 
 /**
@@ -37,8 +35,8 @@ function hide() {
  *
  * @param {LabellingFlowProps} props { defaultPosition, centralize, props }
  */
-function show({ defaultPosition, centralize, props }) {
-  return uiLabellingFlowServiceImplementation._show({
+function _show({ defaultPosition, centralize, props }) {
+  return serviceImplementation._show({
     defaultPosition,
     centralize,
     props,
@@ -58,11 +56,16 @@ function setServiceImplementation({
   hide: hideImplementation,
 }) {
   if (showImplementation) {
-    uiLabellingFlowServiceImplementation._show = showImplementation;
+    serviceImplementation._show = showImplementation;
   }
   if (hideImplementation) {
-    uiLabellingFlowServiceImplementation._hide = hideImplementation;
+    serviceImplementation._hide = hideImplementation;
   }
 }
 
-export default createUILabellingFlowService;
+export default {
+  name,
+  create: ({ configuration = {} }) => {
+    return publicAPI;
+  },
+};

platform/core/src/services/UIModalService/index.js

@@ -11,28 +11,26 @@
  * @property {string} [customClassName=null] The custom class to style the modal.
  */
 
-const uiModalServicePublicAPI = {
-  name: 'UIModalService',
-  hide,
-  show,
+const name = 'UIModalService';
+
+const publicAPI = {
+  name,
+  hide: _hide,
+  show: _show,
   setServiceImplementation,
 };
 
-const uiModalServiceImplementation = {
+const serviceImplementation = {
   _hide: () => console.warn('hide() NOT IMPLEMENTED'),
   _show: () => console.warn('show() NOT IMPLEMENTED'),
 };
 
-function createUIModalService() {
-  return uiModalServicePublicAPI;
-}
-
 /**
  * Show a new UI modal;
  *
  * @param {ModalProps} props { content, contentProps, shouldCloseOnEsc, isOpen, closeButton, title, customClassName }
  */
-function show({
+function _show({
   content = null,
   contentProps = null,
   shouldCloseOnEsc = false,
@@ -41,7 +39,7 @@ function show({
   title = null,
   customClassName = null,
 }) {
-  return uiModalServiceImplementation._show({
+  return serviceImplementation._show({
     content,
     contentProps,
     shouldCloseOnEsc,
@@ -57,8 +55,8 @@ function show({
  *
  * @returns void
  */
-function hide() {
-  return uiModalServiceImplementation._hide();
+function _hide() {
+  return serviceImplementation._hide();
 }
 
 /**
@@ -74,11 +72,16 @@ function setServiceImplementation({
   show: showImplementation,
 }) {
   if (hideImplementation) {
-    uiModalServiceImplementation._hide = hideImplementation;
+    serviceImplementation._hide = hideImplementation;
   }
   if (showImplementation) {
-    uiModalServiceImplementation._show = showImplementation;
+    serviceImplementation._show = showImplementation;
   }
 }
 
-export default createUIModalService;
+export default {
+  name,
+  create: ({ configuration = {} }) => {
+    return publicAPI;
+  },
+};

platform/core/src/services/UINotificationService/index.js

@@ -10,22 +10,20 @@
  * @property {boolean} [autoClose=true]
  */
 
-const uiNotificationServicePublicAPI = {
-  name: 'UINotificationService',
-  hide,
-  show,
+const name = 'UINotificationService';
+
+const publicAPI = {
+  name,
+  hide: _hide,
+  show: _show,
   setServiceImplementation,
 };
 
-const uiNotificationServiceImplementation = {
+const serviceImplementation = {
   _hide: () => console.warn('hide() NOT IMPLEMENTED'),
   _show: () => console.warn('show() NOT IMPLEMENTED'),
 };
 
-function createUINotificationService() {
-  return uiNotificationServicePublicAPI;
-}
-
 /**
  * Create and show a new UI notification; returns the
  * ID of the created notification.
@@ -33,7 +31,7 @@ function createUINotificationService() {
  * @param {Notification} notification { title, message, duration, position, type, autoClose}
  * @returns {number} id
  */
-function show({
+function _show({
   title,
   message,
   duration = 5000,
@@ -41,7 +39,7 @@ function show({
   type = 'info',
   autoClose = true,
 }) {
-  return uiNotificationServiceImplementation._show({
+  return serviceImplementation._show({
     title,
     message,
     duration,
@@ -57,8 +55,8 @@ function show({
  * @param {number} id - id of the notification to hide/dismiss
  * @returns undefined
  */
-function hide(id) {
-  return uiNotificationServiceImplementation._hide({ id });
+function _hide(id) {
+  return serviceImplementation._hide({ id });
 }
 
 /**
@@ -74,11 +72,16 @@ function setServiceImplementation({
   show: showImplementation,
 }) {
   if (hideImplementation) {
-    uiNotificationServiceImplementation._hide = hideImplementation;
+    serviceImplementation._hide = hideImplementation;
   }
   if (showImplementation) {
-    uiNotificationServiceImplementation._show = showImplementation;
+    serviceImplementation._show = showImplementation;
   }
 }
 
-export default createUINotificationService;
+export default {
+  name,
+  create: ({ configuration = {} }) => {
+    return publicAPI;
+  },
+};

platform/core/src/services/index.js

@@ -1,15 +1,15 @@
 import ServicesManager from './ServicesManager.js';
-import createUINotificationService from './UINotificationService';
-import createUIModalService from './UIModalService';
-import createUIDialogService from './UIDialogService';
-import createUIContextMenuService from './UIContextMenuService';
-import createUILabellingFlowService from './UILabellingFlowService';
+import UINotificationService from './UINotificationService';
+import UIModalService from './UIModalService';
+import UIDialogService from './UIDialogService';
+import UIContextMenuService from './UIContextMenuService';
+import UILabellingFlowService from './UILabellingFlowService';
 
 export {
-  createUINotificationService,
-  createUIModalService,
-  createUIDialogService,
-  createUIContextMenuService,
-  createUILabellingFlowService,
+  UINotificationService,
+  UIModalService,
+  UIDialogService,
+  UIContextMenuService,
+  UILabellingFlowService,
   ServicesManager,
 };

platform/viewer/src/App.js

@@ -28,11 +28,11 @@ import {
   ExtensionManager,
   ServicesManager,
   HotkeysManager,
-  createUINotificationService,
-  createUIModalService,
-  createUIDialogService,
-  createUIContextMenuService,
-  createUILabellingFlowService,
+  UINotificationService,
+  UIModalService,
+  UIDialogService,
+  UIContextMenuService,
+  UILabellingFlowService,
   utils,
   redux as reduxOHIF,
 } from '@ohif/core';
@@ -70,13 +70,6 @@ const commandsManagerConfig = {
   getActiveContexts: () => getActiveContexts(store.getState()),
 };
 
-/** Services */
-const UINotificationService = createUINotificationService();
-const UIModalService = createUIModalService();
-const UIDialogService = createUIDialogService();
-const UIContextMenuService = createUIContextMenuService();
-const UILabellingFlowService = createUILabellingFlowService();
-
 /** Managers */
 const commandsManager = new CommandsManager(commandsManagerConfig);
 const hotkeysManager = new HotkeysManager(commandsManager);
@@ -165,6 +158,14 @@ class App extends Component {
 
   render() {
     const { whiteLabelling, routerBasename } = this._appConfig;
+    const {
+      UINotificationService,
+      UIDialogService,
+      UILabellingFlowService,
+      UIModalService,
+      UIContextMenuService,
+    } = servicesManager.services;
+
     if (this._userManager) {
       return (
         <AppContext.Provider value={{ appConfig: this._appConfig }}>