From bdfaa4d6624a374fb036dd6826a549d968c51731 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Tue, 19 May 2026 15:55:32 +0200 Subject: [PATCH 1/3] Extract detector fetching method from ApricotService --- Control/lib/control-core/ApricotService.js | 31 +++++------- .../mocha-apricot.service.test.js | 47 ------------------- 2 files changed, 11 insertions(+), 67 deletions(-) diff --git a/Control/lib/control-core/ApricotService.js b/Control/lib/control-core/ApricotService.js index 511b1ffce..72f8075a3 100644 --- a/Control/lib/control-core/ApricotService.js +++ b/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>} + */ + 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) => { 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 d5bba161c..a8bd5194b 100644 --- a/Control/test/lib/control-core/mocha-apricot.service.test.js +++ b/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; From 81d0cd5302ae6392754f3e45f544db6903ea73d3 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Tue, 19 May 2026 15:56:33 +0200 Subject: [PATCH 2/3] App detector service functionality and tests --- Control/lib/services/Detector.service.js | 52 ++++++++++++++++- .../services/mocha-detector.service.test.js | 57 +++++++++++++++++-- 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/Control/lib/services/Detector.service.js b/Control/lib/services/Detector.service.js index 71ba47ba5..c2a396188 100644 --- a/Control/lib/services/Detector.service.js +++ b/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} + */ + 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>} - 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} + */ + get detectors() { + return this._detectors; + } + + /** + * Setter for the list of detectors cached in memory + * @param {Array} detectors - list of strings with detector names to be cached in memory + */ + set detectors(detectors) { + this._detectors = detectors; + } } module.exports = {DetectorService}; diff --git a/Control/test/lib/services/mocha-detector.service.test.js b/Control/test/lib/services/mocha-detector.service.test.js index 189802aba..02bb876d5 100644 --- a/Control/test/lib/services/mocha-detector.service.test.js +++ b/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' + ); + }); + }); }); From 507fced405d91305d57d22ad48bbc513049a0898 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Tue, 19 May 2026 15:56:46 +0200 Subject: [PATCH 3/3] Make use of extracted code in API --- Control/lib/api.js | 25 ++++++--- .../api/detectors/api-get-detectors.test.js | 51 +++++++++++++++++++ Control/test/mocha-index.js | 1 + 3 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 Control/test/api/detectors/api-get-detectors.test.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 2e0960a5c..aca56fc92 100644 --- a/Control/lib/api.js +++ b/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); } /** diff --git a/Control/test/api/detectors/api-get-detectors.test.js b/Control/test/api/detectors/api-get-detectors.test.js new file mode 100644 index 000000000..dccd9936e --- /dev/null +++ b/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); + }); +}); diff --git a/Control/test/mocha-index.js b/Control/test/mocha-index.js index abe6dadf8..5ad2bc06e 100644 --- a/Control/test/mocha-index.js +++ b/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);