[OGUI-1832] Filter out empty detectors reported by apricot

Control/lib/api.js

@@ -117,7 +117,7 @@ module.exports.setup = (http, ws) => {
   const lockService = new LockService(broadcastService);
   const lockController = new LockController(lockService);
 
-  const detectorService = new DetectorService(ctrlProxy);
+  const detectorService = new DetectorService(ctrlProxy, apricotProxy);
   const environmentService = new EnvironmentService(
     ctrlProxy, apricotService, cacheService, broadcastService, environmentCacheService
   );
@@ -167,7 +167,7 @@ module.exports.setup = (http, ws) => {
 
   const intervals = new Intervals();
 
-  initializeData(apricotService, lockService, consulService);
+  initializeData(detectorService, apricotService, lockService, consulService);
   initializeIntervals(intervals, statusService, runService, bkpService, environmentService);
 
   const coreMiddleware = [
@@ -241,7 +241,14 @@ module.exports.setup = (http, ws) => {
   apricotProxy.methods.forEach(
     (method) => http.post(`/${method}`, (req, res) => apricotService.executeCommand(req, res)),
   );
-  http.get('/core/detectors', (req, res) => apricotService.getDetectorList(req, res));
+  http.get('/core/detectors', async (_, res) => {
+    try {
+      const detectors = await detectorService.getDetectorList();
+      res.status(200).json({detectors});
+    } catch (error) {
+      res.status(503).send({message: error.message});
+    }
+  });
   http.get('/core/hostsByDetectors', (req, res) => apricotService.getHostsByDetectorList(req, res));
 
   http.post('/execute/o2-roc-config', coreMiddleware, (req, res) => ctrlService.createAutoEnvironment(req, res));
@@ -361,14 +368,18 @@ function initializeIntervals(intervalsService, statusService, runService, bkpSer
 
 /**
  * Function to initialize in order dependent services
- * @param {ApricotService} apricotService - request initial set of data from AliECS/Apricot
- * @param {LockService} lockService - initialize service with data from Apricot
+ * @param {DetectorService} detectorService - request initial detectors list and cache it in memory
+ * @param {ApricotService} apricotService - request initial hosts inventory from AliECS/Apricot
+ * @param {LockService} lockService - initialize service with data from DetectorService
  * @param {ConsulService} consulService - service for communicating with Consul
  */
-async function initializeData(apricotService, lockService, consulService) {
+async function initializeData(detectorService, apricotService, lockService, consulService) {
   testConsulStatus(consulService);
+
+  const detectors = await detectorService.getDetectorList();
+  lockService.setLockStatesForDetectors(detectors);
+
   await apricotService.init();
-  lockService.setLockStatesForDetectors(apricotService.detectors);
 }
 
 /**

Control/lib/control-core/ApricotService.js

@@ -44,7 +44,7 @@ class ApricotService {
    */
   async init() {
     try {
-      this.detectors = (await this._grpcClient['ListDetectors']()).detectors;
+      this.detectors = await this._loadDetectors();
       this._logger.infoMessage(`Initial data retrieved from AliECS/Apricot: ${this.detectors} detectors`, {
         level: 99,
         system: 'GUI',
@@ -65,6 +65,15 @@ class ApricotService {
     }
   }
 
+  /**
+   * Retrieve detectors from ECS via Apricot
+   * @returns {Promise<Array<string>>}
+   */
+  async _loadDetectors() {
+    const {detectors = []} = await this._grpcClient['ListDetectors']();
+    return detectors;
+  }
+
   /**
    * Use Apricot defined `o2apricot.proto` `GetRuntimeEntry` to retrieve the value stored in a specified key
    *
@@ -89,24 +98,6 @@ class ApricotService {
     }
   }
 
-  /**
-   * Retrieve an in-memory detectors list
-   * If list does not exist, make a request to Apricot
-   * @param {Request} req
-   * @param {Response} res
-   */
-  async getDetectorList(_, res) {
-    if (this.detectors.length === 0) {
-      try {
-        this.detectors = (await this._grpcClient['ListDetectors']()).detectors;
-      } catch (error) {
-        errorHandler(error, res, 503, 'apricotservice');
-        return;
-      }
-    }
-    res.status(200).json({detectors: this.detectors});
-  }
-
   /**
    * Return an in-memory map of hosts grouped by their detector
    * If map is empty, make a request to Apricot
@@ -116,7 +107,7 @@ class ApricotService {
   async getHostsByDetectorList(_, res) {
     if (this.hostsByDetector.size === 0) {
       try {
-        this.detectors = (await this._grpcClient['ListDetectors']()).detectors;
+        this.detectors = await this._loadDetectors();
 
         await Promise.allSettled(
           this.detectors.map(async (detector) => {

Control/lib/services/Detector.service.js

@@ -20,14 +20,25 @@ const { grpcErrorToNativeError } = require('@aliceo2/web-ui');
  */
 class DetectorService {
   /**
-   * Constructor for initializing the service with ECS gRPC service client
+   * Constructor for initializing the service with ECS and Apricot gRPC service clients
    * @param {GrpcServiceClient} ecsGrpcClient - service to interact via gRPC client with AliECS core
+   * @param {GrpcServiceClient} apricotGrpcClient - service to interact via gRPC client with AliECS Apricot
    */
-  constructor(ecsGrpcClient) {
+  constructor(ecsGrpcClient, apricotGrpcClient) {
     /**
      * @type {GrpcServiceClient}
      */
     this._ecsGrpcClient = ecsGrpcClient;
+
+    /**
+     * @type {GrpcServiceClient}
+     */
+    this._apricotGrpcClient = apricotGrpcClient;
+
+    /**
+     * @type {Array<string>}
+     */
+    this._detectors = [];
   }
 
   /**
@@ -45,6 +56,43 @@ class DetectorService {
       throw grpcErrorToNativeError(error);
     }
   }
+
+  /**
+   * Method to retrieve detectors list from ECS via Apricot gRPC service
+   * In the received list, remove empty or whitespace-only values from response.
+   * Keep the list of detectors cached in memory for future calls.
+   * @returns {Promise<Array<string>>} - list of non-empty detectors
+   * @throws {Error} - throws JS native error converted from gRPC error in case of failure
+   */
+  async getDetectorList() {
+    if (this._detectors.length > 0) {
+      return this._detectors;
+    }
+
+    try {
+      const { detectors = [] } = await this._apricotGrpcClient.ListDetectors();
+      this._detectors = detectors.filter((detector) => typeof detector === 'string' && detector.trim().length > 0);
+      return this._detectors;
+    } catch (grpcError) {
+      throw grpcErrorToNativeError(grpcError);
+    }
+  }
+
+  /**
+   * Getter for the list of detectors cached in memory
+   * @returns {Array<string>}
+   */
+  get detectors() {
+    return this._detectors;
+  }
+
+  /**
+   * Setter for the list of detectors cached in memory
+   * @param {Array<string>} detectors - list of strings with detector names to be cached in memory
+   */
+  set detectors(detectors) {
+    this._detectors = detectors;
+  }
 }
 
 module.exports = {DetectorService};

Control/test/api/detectors/api-get-detectors.test.js

@@ -0,0 +1,51 @@
+/**
+ * @license
+ * Copyright 2019-2020 CERN and copyright holders of ALICE O2.
+ * See http://alice-o2.web.cern.ch/copyright for details of the copyright holders.
+ * All rights not expressly granted are reserved.
+ *
+ * This software is distributed under the terms of the GNU General Public
+ * License v3 (GPL Version 3), copied verbatim in the file "COPYING".
+ *
+ * In applying this license CERN does not waive the privileges and immunities
+ * granted to it by virtue of its status as an Intergovernmental Organization
+ * or submit itself to any jurisdiction.
+*/
+
+const assert = require('assert');
+const request = require('supertest');
+const test = require('../../mocha-index');
+const {ADMIN_TEST_TOKEN, TEST_URL} = require('../generateToken.js');
+
+describe("'API - GET - /core/detectors' test suite", () => {
+  let apricotCalls;
+
+  before(() => {
+    apricotCalls = test.helpers.apricotCalls;
+  });
+
+  beforeEach(() => {
+    apricotCalls['listDetectors'] = undefined;
+  });
+
+  it('should successfully retrieve detectors', async () => {
+    await request(`${TEST_URL}/api`)
+      .get(`/core/detectors?token=${ADMIN_TEST_TOKEN}`)
+      .expect(200, {detectors: ['MID', 'DCS', 'ODC']}); // @link{test/config/apricot-grpc.js}
+  });
+
+  it('should serve detectors from in-memory cache without calling apricot again', async () => {
+    // First request can either warm the cache or use an already warm cache.
+    await request(`${TEST_URL}/api`)
+      .get(`/core/detectors?token=${ADMIN_TEST_TOKEN}`)
+      .expect(200, {detectors: ['MID', 'DCS', 'ODC']});
+
+    apricotCalls['listDetectors'] = undefined;
+
+    await request(`${TEST_URL}/api`)
+      .get(`/core/detectors?token=${ADMIN_TEST_TOKEN}`)
+      .expect(200, {detectors: ['MID', 'DCS', 'ODC']});
+
+    assert.strictEqual(apricotCalls['listDetectors'], undefined);
+  });
+});

Control/test/lib/control-core/mocha-apricot.service.test.js

@@ -59,53 +59,6 @@ describe('ApricotService test suite', () => {
     });
   });
 
-  describe('Check in-memory detectorlist', () => {
-    let req, res;
-    beforeEach(() => {
-      req = {};
-      res = {
-        status: sinon.stub().returnsThis(),
-        json: sinon.spy(),
-        send: sinon.spy(),
-      };
-    });
-    it('should successfully request a list of detectors from AliECS core if none are present', async () => {
-      const apricotProxy = {
-        isConnectionReady: true,
-        ListDetectors: sinon.stub().resolves({detectors: ['TST']})
-      };
-      const apricotService = new ApricotService(apricotProxy);
-      await apricotService.getDetectorList(req, res);
-      assert.ok(res.status.calledOnce);
-      assert.ok(res.status.calledWith(200));
-      assert.ok(res.json.calledOnce);
-      assert.ok(res.json.calledWith({detectors: ['TST']}));
-    });
-
-    it('should successfully return a list of detectors if already present', async () => {
-      const apricotService = new ApricotService({});
-      apricotService.detectors = ['TST'];
-      await apricotService.getDetectorList(req, res);
-      assert.ok(res.status.calledOnce);
-      assert.ok(res.status.calledWith(200));
-      assert.ok(res.json.calledOnce);
-      assert.ok(res.json.calledWith({detectors: ['TST']}));
-    });
-
-    it('should return error response if detectors are not present and AliECS replies with error', async () => {
-      const apricotProxy = {
-        isConnectionReady: true,
-        ListDetectors: sinon.stub().rejects(new Error('Unable to retrieve list'))
-      };
-      const apricotService = new ApricotService(apricotProxy);
-      await apricotService.getDetectorList(req, res);
-      assert.ok(res.status.calledOnce);
-      assert.ok(res.status.calledWith(503));
-      assert.ok(res.send.calledOnce);
-      assert.ok(res.send.calledWith({message: 'Unable to retrieve list'}));
-    });
-  });
-
   describe('Check detectors caching', () => {
     let req, res;
 

Control/test/lib/services/mocha-detector.service.test.js

@@ -24,7 +24,7 @@ describe(`'DetectorService' test suite`, () => {
     it('should successfully respond with positive boolean for empty list to check', async () => {
       const detectorService = new DetectorService({
         GetActiveDetectors: sinon.stub().resolves({detectors: ['ABC']})
-      });
+      }, {});
 
       const areDetectorsAvailable = await detectorService.areDetectorsAvailable([]);
       assert.ok(areDetectorsAvailable);
@@ -33,7 +33,7 @@ describe(`'DetectorService' test suite`, () => {
     it('should successfully respond with positive boolean for given detectors list', async () => {
       const detectorService = new DetectorService({
         GetActiveDetectors: sinon.stub().resolves({detectors: ['ABC']})
-      });
+      }, {});
 
       const areDetectorsAvailable = await detectorService.areDetectorsAvailable(['TPC']);
       assert.ok(areDetectorsAvailable);
@@ -42,7 +42,7 @@ describe(`'DetectorService' test suite`, () => {
     it('should successfully respond with negative boolean for given detectors list', async () => {
       const detectorService = new DetectorService({
         GetActiveDetectors: sinon.stub().resolves({detectors: ['ABC']})
-      });
+      }, {});
 
       const areDetectorsAvailable = await detectorService.areDetectorsAvailable(['ABC']);
       assert.ok(!areDetectorsAvailable);
@@ -51,11 +51,60 @@ describe(`'DetectorService' test suite`, () => {
     it('should reject with JS native error from grpc core proxy service', async () => {
       const detectorService = new DetectorService({
         GetActiveDetectors: sinon.stub().rejects({code: 4, details: 'Timeout'})
-      });
+      }, {});
       await assert.rejects(
         () => detectorService.areDetectorsAvailable(['TPC']),
         (err) => err instanceof TimeoutError && err.message === 'Timeout'
       );
     });
   });
+
+  describe(`'getDetectorList' test suite`, async () => {
+    it('should successfully retrieve list of detectors from apricot', async () => {
+      const detectorService = new DetectorService({}, {
+        ListDetectors: sinon.stub().resolves({detectors: ['TPC', 'TRD']})
+      });
+
+      const detectors = await detectorService.getDetectorList();
+      assert.deepStrictEqual(detectors, ['TPC', 'TRD']);
+      assert.deepStrictEqual(detectorService.detectors, ['TPC', 'TRD']);
+    });
+
+    it('should remove empty and whitespace-only detectors from returned list', async () => {
+      const detectorService = new DetectorService({}, {
+        ListDetectors: sinon.stub().resolves({detectors: ['TPC', '', '   ', '\t', 'TRD']})
+      });
+
+      const detectors = await detectorService.getDetectorList();
+      assert.deepStrictEqual(detectors, ['TPC', 'TRD']);
+      assert.deepStrictEqual(detectorService.detectors, ['TPC', 'TRD']);
+    });
+
+    it('should initialize with empty in-memory detectors list', async () => {
+      const detectorService = new DetectorService({}, {});
+      assert.deepStrictEqual(detectorService.detectors, []);
+    });
+
+    it('should return in-memory detectors list without requesting apricot again', async () => {
+      const apricotListDetectorsStub = sinon.stub().resolves({detectors: ['SHOULD_NOT_BE_USED']});
+      const detectorService = new DetectorService({}, {
+        ListDetectors: apricotListDetectorsStub
+      });
+      detectorService.detectors = ['TPC', 'TRD'];
+
+      const detectors = await detectorService.getDetectorList();
+      assert.deepStrictEqual(detectors, ['TPC', 'TRD']);
+      assert.ok(apricotListDetectorsStub.notCalled);
+    });
+
+    it('should reject with JS native error from grpc apricot proxy service', async () => {
+      const detectorService = new DetectorService({}, {
+        ListDetectors: sinon.stub().rejects({code: 4, details: 'Timeout'})
+      });
+      await assert.rejects(
+        () => detectorService.getDetectorList(),
+        (error) => error instanceof TimeoutError && error.message === 'Timeout'
+      );
+    });
+  });
 });

Control/test/mocha-index.js

@@ -182,6 +182,7 @@ describe('Control', function() {
   require('./api/configuration/api-get-configuration.test');
   require('./api/configuration/api-get-configuration-restrictions.test');
   require('./api/configuration/api-put-configuration.test');
+  require('./api/detectors/api-get-detectors.test');
 
   beforeEach(() => this.ok = true);