diff --git a/Control/lib/api.js b/Control/lib/api.js index aca56fc92..fa6795a6c 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -249,7 +249,14 @@ module.exports.setup = (http, ws) => { res.status(503).send({message: error.message}); } }); - http.get('/core/hostsByDetectors', (req, res) => apricotService.getHostsByDetectorList(req, res)); + http.get('/core/hostsByDetectors', async (_, res) => { + try { + const hostsByDetector = await detectorService.getHostsByDetector(); + res.status(200).json({hosts: Object.fromEntries(hostsByDetector)}); + } catch (error) { + res.status(503).send({message: error.message}); + } + }); http.post('/execute/o2-roc-config', coreMiddleware, (req, res) => ctrlService.createAutoEnvironment(req, res)); @@ -378,8 +385,7 @@ async function initializeData(detectorService, apricotService, lockService, cons const detectors = await detectorService.getDetectorList(); lockService.setLockStatesForDetectors(detectors); - - await apricotService.init(); + await detectorService.getHostsByDetector(); } /** diff --git a/Control/lib/control-core/ApricotService.js b/Control/lib/control-core/ApricotService.js index 72f8075a3..72f882de3 100644 --- a/Control/lib/control-core/ApricotService.js +++ b/Control/lib/control-core/ApricotService.js @@ -34,35 +34,6 @@ class ApricotService { assert(grpcClient, 'Missing GrpcServiceClient dependency for Apricot'); this._grpcClient = grpcClient; this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? 'cog'}/apricotservice`); - - this.detectors = []; - this.hostsByDetector = new Map(); - } - - /** - * Initialize service with static data from AliECS - */ - async init() { - try { - this.detectors = await this._loadDetectors(); - this._logger.infoMessage(`Initial data retrieved from AliECS/Apricot: ${this.detectors} detectors`, { - level: 99, - system: 'GUI', - facility: 'cog/api' - }); - await Promise.allSettled( - this.detectors.map(async (detector) => { - try { - const {hosts} = await this._grpcClient['GetHostInventory']({detector}); - this.hostsByDetector.set(detector, hosts); - } catch (error) { - this._logger.error(`Unable to retrieve list of hosts for detector: ${detector}`); - } - }) - ); - } catch (error) { - this._logger.error('Unable to list detectors'); - } } /** @@ -98,36 +69,6 @@ class ApricotService { } } - /** - * Return an in-memory map of hosts grouped by their detector - * If map is empty, make a request to Apricot - * @param {Request} req - * @param {Response} res - */ - async getHostsByDetectorList(_, res) { - if (this.hostsByDetector.size === 0) { - try { - this.detectors = await this._loadDetectors(); - - await Promise.allSettled( - this.detectors.map(async (detector) => { - try { - const {hosts} = await this._grpcClient['GetHostInventory']({detector}); - this.hostsByDetector.set(detector, hosts); - } catch (error) { - this._logger.error(`Unable to retrieve list of hosts for detector: ${detector}`); - this._logger.error(error); - } - }) - ); - } catch (error) { - errorHandler(error, res, 503, 'apricotservice'); - return; - } - } - res.status(200).json({hosts: Object.fromEntries(this.hostsByDetector)}); - } - /** * Request a list of detectors from Apricot to confirm * connection and O2Apricot are up diff --git a/Control/lib/services/Detector.service.js b/Control/lib/services/Detector.service.js index c2a396188..b3602849a 100644 --- a/Control/lib/services/Detector.service.js +++ b/Control/lib/services/Detector.service.js @@ -12,7 +12,7 @@ * or submit itself to any jurisdiction. */ -const { grpcErrorToNativeError } = require('@aliceo2/web-ui'); +const { grpcErrorToNativeError, LogManager } = require('@aliceo2/web-ui'); /** * @class @@ -39,6 +39,13 @@ class DetectorService { * @type {Array} */ this._detectors = []; + + /** + * @type {Map>} + */ + this._hostsByDetector = new Map(); + + this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? 'cog'}/detectorservice`); } /** @@ -78,6 +85,33 @@ class DetectorService { } } + /** + * Method to retrieve a map of hosts grouped by their detector via Apricot gRPC service. + * If the in-memory cache is non-empty, it is returned directly without querying Apricot. + * Individual detector failures are non-fatal; the partial result is cached and returned. + * @returns {Promise>>} - map of detector name to list of hosts + * @throws {Error} - throws JS native error converted from gRPC error if detector list fetch fails + */ + async getHostsByDetector() { + if (this._hostsByDetector.size > 0) { + return this._hostsByDetector; + } + + const detectors = await this.getDetectorList(); + await Promise.allSettled( + detectors.map(async (detector) => { + try { + const { hosts } = await this._apricotGrpcClient.GetHostInventory({ detector }); + const filteredHosts = hosts.filter((host) => typeof host === 'string' && host.trim().length > 0); + this._hostsByDetector.set(detector, filteredHosts); + } catch (error) { + this._logger.errorMessage(`Failed to retrieve hosts for detector ${detector}: ${error.message}`); + } + }) + ); + return this._hostsByDetector; + } + /** * Getter for the list of detectors cached in memory * @returns {Array} @@ -93,6 +127,14 @@ class DetectorService { set detectors(detectors) { this._detectors = detectors; } + + /** + * Getter for the hosts-by-detector map cached in memory + * @returns {Map>} + */ + get hostsByDetector() { + return this._hostsByDetector; + } } module.exports = {DetectorService}; diff --git a/Control/test/api/detectors/api-get-hosts-by-detector.test.js b/Control/test/api/detectors/api-get-hosts-by-detector.test.js new file mode 100644 index 000000000..5528a76fc --- /dev/null +++ b/Control/test/api/detectors/api-get-hosts-by-detector.test.js @@ -0,0 +1,60 @@ +/** + * @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/hostsByDetectors' test suite", () => { + let apricotCalls; + + // @link{test/config/apricot-grpc.js} — getHostInventory returns ['ali-flp-22', 'ali-flp-23'] for every detector + const EXPECTED_HOSTS = { + hosts: { + MID: ['ali-flp-22', 'ali-flp-23'], + DCS: ['ali-flp-22', 'ali-flp-23'], + ODC: ['ali-flp-22', 'ali-flp-23'], + }, + }; + + before(() => { + apricotCalls = test.helpers.apricotCalls; + }); + + beforeEach(() => { + apricotCalls['getHostInventory'] = undefined; + }); + + it('should successfully retrieve hosts grouped by detector', async () => { + await request(`${TEST_URL}/api`) + .get(`/core/hostsByDetectors?token=${ADMIN_TEST_TOKEN}`) + .expect(200, EXPECTED_HOSTS); + }); + + it('should serve hosts 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/hostsByDetectors?token=${ADMIN_TEST_TOKEN}`) + .expect(200, EXPECTED_HOSTS); + + apricotCalls['getHostInventory'] = undefined; + + await request(`${TEST_URL}/api`) + .get(`/core/hostsByDetectors?token=${ADMIN_TEST_TOKEN}`) + .expect(200, EXPECTED_HOSTS); + + assert.strictEqual(apricotCalls['getHostInventory'], undefined); + }); +}); diff --git a/Control/test/lib/control-core/mocha-apricot.service.test.js b/Control/test/lib/control-core/mocha-apricot.service.test.js index a8bd5194b..fa6ceb6e9 100644 --- a/Control/test/lib/control-core/mocha-apricot.service.test.js +++ b/Control/test/lib/control-core/mocha-apricot.service.test.js @@ -58,57 +58,7 @@ describe('ApricotService test suite', () => { assert.rejects(() => apricotService.getStatus(), new Error('Apricot is not working')); }); }); - - describe('Check detectors caching', () => { - let req, res; - - beforeEach(() => { - req = {}; - res = { - status: sinon.stub().returnsThis(), - json: sinon.spy(), - send: sinon.spy(), - }; - }); - - it('should successfully request hosts for each detector from AliECS core if none are present', async () => { - const apricotProxy = { - isConnectionReady: true, - ListDetectors: sinon.stub().resolves({detectors: ['TST']}), - GetHostInventory: sinon.stub().resolves({hosts: ['flp001']}), - }; - const apricotService = new ApricotService(apricotProxy); - await apricotService.getHostsByDetectorList(req, res); - assert.ok(res.status.calledOnce); - assert.ok(res.status.calledWith(200)); - assert.ok(res.json.calledOnce); - assert.ok(res.json.calledWith({hosts: {TST: ['flp001']}})); - }); - - it('should successfully return a map of hosts grouped by detectors if already present', async () => { - const apricotService = new ApricotService({}); - apricotService.hostsByDetector = new Map([['TST', ['flp001']]]); - await apricotService.getHostsByDetectorList(req, res); - assert.ok(res.status.calledOnce); - assert.ok(res.status.calledWith(200)); - assert.ok(res.json.calledOnce); - assert.ok(res.json.calledWith({hosts: {TST: ['flp001']}})); - }); - - it('should return error response if hostsByDetectors are not present and AliECS replies with error for initial detector request', async () => { - const apricotProxy = { - isConnectionReady: true, - ListDetectors: sinon.stub().rejects(new Error('unable to load detector list')), - }; - const apricotService = new ApricotService(apricotProxy); - await apricotService.getHostsByDetectorList(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 load detector list'})); - }); - }); - + describe('Check executing commands through `GrpcServiceClient`', () => { let apricotService; let req, res; diff --git a/Control/test/lib/services/mocha-detector.service.test.js b/Control/test/lib/services/mocha-detector.service.test.js index 02bb876d5..a5a3df5e0 100644 --- a/Control/test/lib/services/mocha-detector.service.test.js +++ b/Control/test/lib/services/mocha-detector.service.test.js @@ -107,4 +107,66 @@ describe(`'DetectorService' test suite`, () => { ); }); }); + + describe(`'getHostsByDetector' test suite`, async () => { + it('should initialize with empty in-memory hosts map', () => { + const detectorService = new DetectorService({}, {}); + assert.deepStrictEqual(detectorService.hostsByDetector.size, 0); + }); + + it('should successfully retrieve a map of hosts grouped by detector from apricot', async () => { + const detectorService = new DetectorService({}, { + ListDetectors: sinon.stub().resolves({detectors: ['TPC', 'TRD']}), + GetHostInventory: sinon.stub().resolves({hosts: ['flp001', 'flp002']}) + }); + + const hostsByDetector = await detectorService.getHostsByDetector(); + assert.deepStrictEqual(hostsByDetector, new Map([ + ['TPC', ['flp001', 'flp002']], + ['TRD', ['flp001', 'flp002']], + ])); + }); + + it('should remove empty and whitespace-only hosts from each detector entry', async () => { + const detectorService = new DetectorService({}, { + ListDetectors: sinon.stub().resolves({detectors: ['TPC']}), + GetHostInventory: sinon.stub().resolves({hosts: ['flp001', '', ' ', '\t', 'flp002']}) + }); + + const hostsByDetector = await detectorService.getHostsByDetector(); + assert.deepStrictEqual(hostsByDetector, new Map([['TPC', ['flp001', 'flp002']]])); + }); + + it('should cache result and return it without calling apricot again', async () => { + const listDetectorsStub = sinon.stub().resolves({detectors: ['SHOULD_NOT_BE_USED']}); + const detectorService = new DetectorService({}, {ListDetectors: listDetectorsStub}); + detectorService._hostsByDetector = new Map([['TPC', ['flp001']]]); + + const hostsByDetector = await detectorService.getHostsByDetector(); + assert.deepStrictEqual(hostsByDetector, new Map([['TPC', ['flp001']]])); + assert.ok(listDetectorsStub.notCalled); + }); + + it('should return partial result when individual GetHostInventory calls fail', async () => { + const detectorService = new DetectorService({}, { + ListDetectors: sinon.stub().resolves({detectors: ['TPC', 'TRD']}), + GetHostInventory: sinon.stub() + .onFirstCall().resolves({hosts: ['flp001']}) + .onSecondCall().rejects(new Error('host fetch failed')) + }); + + const hostsByDetector = await detectorService.getHostsByDetector(); + assert.deepStrictEqual(hostsByDetector, new Map([['TPC', ['flp001']]])); + }); + + it('should reject with JS native error if ListDetectors fails', async () => { + const detectorService = new DetectorService({}, { + ListDetectors: sinon.stub().rejects({code: 4, details: 'Timeout'}) + }); + await assert.rejects( + () => detectorService.getHostsByDetector(), + (error) => error instanceof TimeoutError && error.message === 'Timeout' + ); + }); + }); }); diff --git a/Control/test/mocha-index.js b/Control/test/mocha-index.js index 5ad2bc06e..37910a3f8 100644 --- a/Control/test/mocha-index.js +++ b/Control/test/mocha-index.js @@ -183,6 +183,7 @@ describe('Control', function() { require('./api/configuration/api-get-configuration-restrictions.test'); require('./api/configuration/api-put-configuration.test'); require('./api/detectors/api-get-detectors.test'); + require('./api/detectors/api-get-hosts-by-detector.test'); beforeEach(() => this.ok = true);