From f2631ea5b9771265178fa654f6971078e5b94173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Wed, 4 Jun 2025 20:19:11 +0200 Subject: [PATCH 01/10] feat: add configuration endpoint --- Control/lib/api.js | 7 ++ .../controllers/Configuration.controller.js | 95 +++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 Control/lib/controllers/Configuration.controller.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 667936635..9ede775fc 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -33,6 +33,7 @@ const { } = require('./middleware/getDetectorsLockOwnershipMiddlewareFactory.js'); // controllers +const {ConfigurationController} = require('./controllers/Configuration.controller.js'); const {ConsulController} = require('./controllers/Consul.controller.js'); const {EnvironmentController} = require('./controllers/Environment.controller.js'); const {LockController} = require('./controllers/Lock.controller.js'); @@ -90,6 +91,9 @@ module.exports.setup = (http, ws) => { const broadcastService = new BroadcastService(ws); const cacheService = new CacheService(broadcastService); const environmentCacheService = new EnvironmentCacheService(broadcastService, eventEmitter); + + const configurationController = new ConfigurationController(consulService, config.consul); + configurationController.testConsulStatus(); const consulController = new ConsulController(consulService, config.consul); consulController.testConsulStatus(); @@ -230,6 +234,9 @@ module.exports.setup = (http, ws) => { statusController.getAliECSIntegratedServicesStatus.bind(statusController), ); + // Configuration + http.get('/configurations', configurationController.getConfigurations.bind(configurationController), {public: true}); + // Consul const validateService = consulController.validateService.bind(consulController); http.get('/consul/flps', validateService, consulController.getFLPs.bind(consulController)); diff --git a/Control/lib/controllers/Configuration.controller.js b/Control/lib/controllers/Configuration.controller.js new file mode 100644 index 000000000..109d1ff03 --- /dev/null +++ b/Control/lib/controllers/Configuration.controller.js @@ -0,0 +1,95 @@ +/** + * @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 { LogManager, LogLevel } = require("@aliceo2/web-ui"); +const { errorHandler, errorLogger } = require("../utils.js"); +const { getConsulConfig } = require("../config/publicConfigProvider.js"); + +/** + * Gateway for all Consul Consumer calls + */ +class ConfigurationController { + /** + * Setup ConfigurationController + * @param {ConsulService} consulService + * @param {JSON} config + */ + constructor(consulService, config) { + this.consulService = consulService; + this.config = getConsulConfig({ consul: config }); + this.configurationsPath = `${this.config.qcPath}/ANY/any`; + + this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cog"}/consul`); + } + + /** + * Check if consulService is present: + * * If yes, allow request to continue + * * If not, send response accordingly + * @param {Request} req + * @param {Response} res + * @param {Next} next + */ + validateService(req, res, next) { + if (this.consulService) { + next(); + } else { + errorHandler("Unable to retrieve configuration of consul service", res, 502); + } + } + + /** + * Method to check if consul service can be used + */ + async testConsulStatus() { + this.consulService + .getConsulLeaderStatus() + .then((data) => this._logger.info(`Service is up and running on: ${data}`)) + .catch((error) => this._logger.error(`Connection failed due to ${error}`)); + } + + /** + * Get configurations from Consul + * @param {Request} req + * @param {Response} res + */ + async getConfigurations(req, res) { + const prefix = req.query.prefix; + const recurse = req.query.recurse === "true"; + const prefixPath = prefix ? `${this.configurationsPath}/${prefix}` : this.configurationsPath; + try { + const data = await this.consulService.getOnlyRawValuesByKeyPrefix(prefixPath); + const parsedData = Object.entries(data || {}) + .map(([key, value]) => { + try { + return { + key, + value: JSON.parse(value), + }; + } catch (e) { + return undefined; + } + }) + .filter((item) => item !== undefined) + .filter((item) => recurse || !item.key.replace(`${prefixPath}/`, "").includes("/")); + + res.status(200).json(parsedData); + } catch (error) { + errorLogger(error, this._logger); + errorHandler("Error retrieving configurations", res, 500); + } + } +} + +exports.ConfigurationController = ConfigurationController; From 82407cd1d2a27ea2df801abea44872c9cd0850e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Tue, 10 Jun 2025 21:17:26 +0200 Subject: [PATCH 02/10] feat: implement getConfigurationByKey endpoint, refactor QCConfigurationController and create QCConfiguration.service --- Control/lib/api.js | 16 ++- .../controllers/Configuration.controller.js | 95 ------------- .../controllers/QCConfiguration.controller.js | 81 +++++++++++ .../lib/services/QCConfiguration.service.js | 129 ++++++++++++++++++ 4 files changed, 221 insertions(+), 100 deletions(-) delete mode 100644 Control/lib/controllers/Configuration.controller.js create mode 100644 Control/lib/controllers/QCConfiguration.controller.js create mode 100644 Control/lib/services/QCConfiguration.service.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 9ede775fc..8beec400e 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -33,7 +33,7 @@ const { } = require('./middleware/getDetectorsLockOwnershipMiddlewareFactory.js'); // controllers -const {ConfigurationController} = require('./controllers/Configuration.controller.js'); +const {QCConfigurationController} = require('./controllers/QCConfiguration.controller.js'); const {ConsulController} = require('./controllers/Consul.controller.js'); const {EnvironmentController} = require('./controllers/Environment.controller.js'); const {LockController} = require('./controllers/Lock.controller.js'); @@ -54,6 +54,7 @@ const {LockService} = require('./services/Lock.service.js'); const {RunService} = require('./services/Run.service.js'); const {StatusService} = require('./services/Status.service.js'); const {WorkflowTemplateService} = require('./services/WorkflowTemplate.service.js'); +const {QCConfigurationService} = require('./services/QCConfiguration.service.js'); // web-ui services const {NotificationService, ConsulService} = require('@aliceo2/web-ui'); @@ -91,9 +92,9 @@ module.exports.setup = (http, ws) => { const broadcastService = new BroadcastService(ws); const cacheService = new CacheService(broadcastService); const environmentCacheService = new EnvironmentCacheService(broadcastService, eventEmitter); - - const configurationController = new ConfigurationController(consulService, config.consul); - configurationController.testConsulStatus(); + const qcConfigurationService = new QCConfigurationService(consulService); + + const qcConfigurationController = new QCConfigurationController(qcConfigurationService, config.consul); const consulController = new ConsulController(consulService, config.consul); consulController.testConsulStatus(); @@ -235,7 +236,12 @@ module.exports.setup = (http, ws) => { ); // Configuration - http.get('/configurations', configurationController.getConfigurations.bind(configurationController), {public: true}); + http.get('/configurations', qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController), { + public: true + }); + http.get('/configuration', qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController), { + public: true + }); // Consul const validateService = consulController.validateService.bind(consulController); diff --git a/Control/lib/controllers/Configuration.controller.js b/Control/lib/controllers/Configuration.controller.js deleted file mode 100644 index 109d1ff03..000000000 --- a/Control/lib/controllers/Configuration.controller.js +++ /dev/null @@ -1,95 +0,0 @@ -/** - * @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 { LogManager, LogLevel } = require("@aliceo2/web-ui"); -const { errorHandler, errorLogger } = require("../utils.js"); -const { getConsulConfig } = require("../config/publicConfigProvider.js"); - -/** - * Gateway for all Consul Consumer calls - */ -class ConfigurationController { - /** - * Setup ConfigurationController - * @param {ConsulService} consulService - * @param {JSON} config - */ - constructor(consulService, config) { - this.consulService = consulService; - this.config = getConsulConfig({ consul: config }); - this.configurationsPath = `${this.config.qcPath}/ANY/any`; - - this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cog"}/consul`); - } - - /** - * Check if consulService is present: - * * If yes, allow request to continue - * * If not, send response accordingly - * @param {Request} req - * @param {Response} res - * @param {Next} next - */ - validateService(req, res, next) { - if (this.consulService) { - next(); - } else { - errorHandler("Unable to retrieve configuration of consul service", res, 502); - } - } - - /** - * Method to check if consul service can be used - */ - async testConsulStatus() { - this.consulService - .getConsulLeaderStatus() - .then((data) => this._logger.info(`Service is up and running on: ${data}`)) - .catch((error) => this._logger.error(`Connection failed due to ${error}`)); - } - - /** - * Get configurations from Consul - * @param {Request} req - * @param {Response} res - */ - async getConfigurations(req, res) { - const prefix = req.query.prefix; - const recurse = req.query.recurse === "true"; - const prefixPath = prefix ? `${this.configurationsPath}/${prefix}` : this.configurationsPath; - try { - const data = await this.consulService.getOnlyRawValuesByKeyPrefix(prefixPath); - const parsedData = Object.entries(data || {}) - .map(([key, value]) => { - try { - return { - key, - value: JSON.parse(value), - }; - } catch (e) { - return undefined; - } - }) - .filter((item) => item !== undefined) - .filter((item) => recurse || !item.key.replace(`${prefixPath}/`, "").includes("/")); - - res.status(200).json(parsedData); - } catch (error) { - errorLogger(error, this._logger); - errorHandler("Error retrieving configurations", res, 500); - } - } -} - -exports.ConfigurationController = ConfigurationController; diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js new file mode 100644 index 000000000..ce3296d0b --- /dev/null +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -0,0 +1,81 @@ +/** + * @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 { LogManager } = require("@aliceo2/web-ui"); +const { errorHandler, errorLogger } = require("../utils.js"); +const { getConsulConfig } = require("../config/publicConfigProvider.js"); + +/** + * Gateway for all Consul Consumer calls + */ +class QCConfigurationController { + /** + * Setup ConfigurationController + * @param {QCConfigurationService} qcConfigurationService + * @param {JSON} config + */ + constructor(qcConfigurationService, config) { + this.qcConfigurationService = qcConfigurationService; + this.config = getConsulConfig({ consul: config }); + this.configurationsPath = `${this.config.qcPath}/ANY/any`; + + this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cog"}/consul`); + } + + /** + * Get configurations from Consul + * @param {Request} req + * @param {Response} res + */ + async getConfigurationsKeys(req, res) { + const { prefix = "", recurse = false } = req.query; + const prefixPath = prefix ? `${this.configurationsPath}/${prefix}` : this.configurationsPath; + + try { + const parsedData = await this.qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); + if (!parsedData || parsedData.length === 0) { + return errorHandler("No configurations found", res, 404); + } + + res.status(200).json(parsedData); + } catch (error) { + errorLogger(error, this._logger); + errorHandler("Error retrieving configurations", res, 500); + } + } + + /** + * Get configurations from Consul + * @param {Request} req + * @param {Response} res + */ + async getConfigurationByKey(req, res) { + const { key } = req.query; + + try { + const value = await this.qcConfigurationService.getConfigurationByKey(key); + console.log("Retrieved configuration:", value); + if (!value) { + return errorHandler("Configuration not found", res, 404); + } + + res.status(200).json(value); + } catch (error) { + errorLogger(error, this._logger); + errorHandler("Error retrieving configuration", res, 500); + } + } +} + +exports.QCConfigurationController = QCConfigurationController; diff --git a/Control/lib/services/QCConfiguration.service.js b/Control/lib/services/QCConfiguration.service.js new file mode 100644 index 000000000..8997eab2b --- /dev/null +++ b/Control/lib/services/QCConfiguration.service.js @@ -0,0 +1,129 @@ +/** + * @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 { NotFoundError, ServiceUnavailableError } = require("@aliceo2/web-ui"); +const { errorHandler } = require("../utils.js"); + +/** + * @class + * QCConfigurationService class to be user for communicating with the Consul service + */ +class QCConfigurationService { + /** + * @constructor + * Constructor for configuring the initial state of stored information + * @param {ConsulService} consulService - service to communicate with Consul + */ + constructor(consulService) { + /** + * @type {ConsulService} + */ + this._consulService = consulService; + } + + /** + * Initialize Lock service based on the provided list of detectors + * @param {Array} detectors = [] - list of detectors to be used for the lock mechanism + * @return {void} + */ + // setLockStatesForDetectors(detectors = []) { + // for (const detectorName of detectors) { + // this._locksByDetector[detectorName] = new DetectorLock(detectorName); + // } + // } + + /** + * Check if consulService is present: + * * If yes, allow request to continue + * * If not, send response accordingly + * @param {Request} req + * @param {Response} res + * @param {Next} next + */ + validateService(req, res, next) { + if (this.consulService) { + next(); + } else { + errorHandler("Unable to retrieve configuration of consul service", res, 502); + } + } + + /** + * Method to check if consul service can be used + */ + async testConsulStatus() { + this.consulService + .getConsulLeaderStatus() + .then((data) => this._logger.info(`Service is up and running on: ${data}`)) + .catch((error) => this._logger.error(`Connection failed due to ${error}`)); + } + + /** + * Get keys of configurations stored in Consul + * @param {String} prefix - prefix to filter the keys + * @param {boolean} [recurse=false] - whether to recurse into subdirectories + */ + async getKeysOfValidConfigurations(prefix, recurse = false) { + if (!this._consulService) { + throw new ServiceUnavailableError("Consul service is not available"); + } + try { + const data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); + const parsedData = []; + + Object.entries(data || {}).forEach(([key, value]) => { + try { + if (!JSON.parse(value)) { + return; + } + if (!recurse && key.replace(`${prefix}/`, "").includes("/")) { + return; + } + + parsedData.push(key); + } catch (e) { + // skip + } + }); + + return parsedData; + } catch (error) { + if (error instanceof NotFoundError) { + throw new NotFoundError(`No configurations found for prefix: ${prefix}`); + } + throw error; + } + } + + /** + * Get configuration by key from Consul + * @param {String} key - the key of the configuration + */ + async getConfigurationByKey(key) { + if (!this._consulService) { + throw new ServiceUnavailableError("Consul service is not available"); + } + try { + const value = await this._consulService.getOnlyRawValueByKey(key); + if (!value) { + throw new NotFoundError(`Configuration not found for key: ${key}`); + } + return value; + } catch (error) { + throw new ServiceUnavailableError(`Error retrieving configuration for key: ${key}`); + } + } +} + +exports.QCConfigurationService = QCConfigurationService; From 1e340a8c5a66018f339c8d325cdc863190488ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Wed, 11 Jun 2025 19:09:37 +0200 Subject: [PATCH 03/10] small code refactor --- Control/lib/api.js | 18 ++++++---- .../controllers/QCConfiguration.controller.js | 36 ++++++++++--------- .../lib/services/QCConfiguration.service.js | 14 +++----- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/Control/lib/api.js b/Control/lib/api.js index 8beec400e..040900bee 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -93,6 +93,7 @@ module.exports.setup = (http, ws) => { const cacheService = new CacheService(broadcastService); const environmentCacheService = new EnvironmentCacheService(broadcastService, eventEmitter); const qcConfigurationService = new QCConfigurationService(consulService); + qcConfigurationService.testConsulStatus(); const qcConfigurationController = new QCConfigurationController(qcConfigurationService, config.consul); @@ -236,12 +237,17 @@ module.exports.setup = (http, ws) => { ); // Configuration - http.get('/configurations', qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController), { - public: true - }); - http.get('/configuration', qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController), { - public: true - }); + const qcValidateService = qcConfigurationService.validateService.bind(qcConfigurationService); + http.get( + "/configurations", qcValidateService, + qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController), + { public: true } + ); + http.get( + '/configuration', qcValidateService, + qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController), + { public: true } + ); // Consul const validateService = consulController.validateService.bind(consulController); diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index ce3296d0b..44e4fba5c 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -21,29 +21,29 @@ const { getConsulConfig } = require("../config/publicConfigProvider.js"); */ class QCConfigurationController { /** - * Setup ConfigurationController + * Setup QCConfigurationController * @param {QCConfigurationService} qcConfigurationService * @param {JSON} config */ constructor(qcConfigurationService, config) { - this.qcConfigurationService = qcConfigurationService; - this.config = getConsulConfig({ consul: config }); - this.configurationsPath = `${this.config.qcPath}/ANY/any`; + this._qcConfigurationService = qcConfigurationService; + this._config = getConsulConfig({ consul: config }); + this._qcConfigurationsPath = `${this._config.qcPath}/ANY/any`; - this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cog"}/consul`); + this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cnf"}/qc-configuration-controller`); } /** - * Get configurations from Consul - * @param {Request} req - * @param {Response} res - */ + * Method to get configurations names + * @param {Request} req + * @param {Response} res + */ async getConfigurationsKeys(req, res) { const { prefix = "", recurse = false } = req.query; - const prefixPath = prefix ? `${this.configurationsPath}/${prefix}` : this.configurationsPath; + const prefixPath = prefix ? `${this._qcConfigurationsPath}/${prefix}` : this._qcConfigurationsPath; try { - const parsedData = await this.qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); + const parsedData = await this._qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); if (!parsedData || parsedData.length === 0) { return errorHandler("No configurations found", res, 404); } @@ -56,16 +56,18 @@ class QCConfigurationController { } /** - * Get configurations from Consul - * @param {Request} req - * @param {Response} res - */ + * Method to get configuration value by key + * @param {Request} req + * @param {Response} res + */ async getConfigurationByKey(req, res) { const { key } = req.query; + if (!key) { + return errorHandler("Missing configuration key", res, 400); + } try { - const value = await this.qcConfigurationService.getConfigurationByKey(key); - console.log("Retrieved configuration:", value); + const value = await this._qcConfigurationService.getConfigurationByKey(key); if (!value) { return errorHandler("Configuration not found", res, 404); } diff --git a/Control/lib/services/QCConfiguration.service.js b/Control/lib/services/QCConfiguration.service.js index 8997eab2b..455c017d0 100644 --- a/Control/lib/services/QCConfiguration.service.js +++ b/Control/lib/services/QCConfiguration.service.js @@ -12,7 +12,7 @@ * or submit itself to any jurisdiction. */ -const { NotFoundError, ServiceUnavailableError } = require("@aliceo2/web-ui"); +const { NotFoundError, ServiceUnavailableError, LogManager } = require("@aliceo2/web-ui"); const { errorHandler } = require("../utils.js"); /** @@ -30,6 +30,8 @@ class QCConfigurationService { * @type {ConsulService} */ this._consulService = consulService; + + this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cnf"}/qc-configuration-service`); } /** @@ -52,7 +54,7 @@ class QCConfigurationService { * @param {Next} next */ validateService(req, res, next) { - if (this.consulService) { + if (this._consulService) { next(); } else { errorHandler("Unable to retrieve configuration of consul service", res, 502); @@ -63,7 +65,7 @@ class QCConfigurationService { * Method to check if consul service can be used */ async testConsulStatus() { - this.consulService + this._consulService .getConsulLeaderStatus() .then((data) => this._logger.info(`Service is up and running on: ${data}`)) .catch((error) => this._logger.error(`Connection failed due to ${error}`)); @@ -75,9 +77,6 @@ class QCConfigurationService { * @param {boolean} [recurse=false] - whether to recurse into subdirectories */ async getKeysOfValidConfigurations(prefix, recurse = false) { - if (!this._consulService) { - throw new ServiceUnavailableError("Consul service is not available"); - } try { const data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); const parsedData = []; @@ -111,9 +110,6 @@ class QCConfigurationService { * @param {String} key - the key of the configuration */ async getConfigurationByKey(key) { - if (!this._consulService) { - throw new ServiceUnavailableError("Consul service is not available"); - } try { const value = await this._consulService.getOnlyRawValueByKey(key); if (!value) { From 09ad06f635a2ee87aae54ddeceb84be1908b4d34 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 11 Jun 2025 23:09:37 +0200 Subject: [PATCH 04/10] change error handling in qcConfigurationController and add mocha tests for qcConfigurationController and qcConfigurationService --- .../controllers/QCConfiguration.controller.js | 17 ++-- .../mocha-qcConfiguration.controller.test.js | 93 +++++++++++++++++++ .../mocha-qcConfiguration.service.test.js | 69 ++++++++++++++ 3 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js create mode 100644 Control/test/lib/services/mocha-qcConfiguration.service.test.js diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index 44e4fba5c..9410ac170 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -12,8 +12,8 @@ * or submit itself to any jurisdiction. */ -const { LogManager } = require("@aliceo2/web-ui"); -const { errorHandler, errorLogger } = require("../utils.js"); +const { LogManager, updateAndSendExpressResponseFromNativeError, InvalidInputError, NotFoundError } = require("@aliceo2/web-ui"); +const { errorLogger } = require("../utils.js"); const { getConsulConfig } = require("../config/publicConfigProvider.js"); /** @@ -45,13 +45,13 @@ class QCConfigurationController { try { const parsedData = await this._qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); if (!parsedData || parsedData.length === 0) { - return errorHandler("No configurations found", res, 404); + updateAndSendExpressResponseFromNativeError(res, new NotFoundError("No configurations found")); } res.status(200).json(parsedData); } catch (error) { errorLogger(error, this._logger); - errorHandler("Error retrieving configurations", res, 500); + updateAndSendExpressResponseFromNativeError(res, error); } } @@ -62,20 +62,23 @@ class QCConfigurationController { */ async getConfigurationByKey(req, res) { const { key } = req.query; + console.log(`getConfigurationByKey: ${key}`); if (!key) { - return errorHandler("Missing configuration key", res, 400); + updateAndSendExpressResponseFromNativeError(res, new InvalidInputError("Missing configuration key")); } try { const value = await this._qcConfigurationService.getConfigurationByKey(key); + console.log("value") + console.log(value) if (!value) { - return errorHandler("Configuration not found", res, 404); + updateAndSendExpressResponseFromNativeError(res, new NotFoundError("Configuration not found")); } res.status(200).json(value); } catch (error) { errorLogger(error, this._logger); - errorHandler("Error retrieving configuration", res, 500); + updateAndSendExpressResponseFromNativeError(res, error); } } } diff --git a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js new file mode 100644 index 000000000..26a8cf91e --- /dev/null +++ b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js @@ -0,0 +1,93 @@ +/** + * @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. +*/ +/* eslint-disable max-len */ + +const assert = require('assert'); +const sinon = require('sinon'); + +const { QCConfigurationController } = require('../../../lib/controllers/QCConfiguration.controller.js'); +const { QCConfigurationService } = require('../../../lib/services/QCConfiguration.service.js'); + +describe(`'QCConfigurationController' test suite`, () => { + describe(`'getConfigurationsKeys' test suite`, () => { + let qcConfigurationService, qcConfigurationController; + before(() => { + qcConfigurationService = new QCConfigurationService({ + getOnlyRawValuesByKeyPrefix: sinon.stub().resolves({ + "o2/components/qc/ANY/any/dir1": undefined, + "o2/components/qc/ANY/any/dir1/prefix1": '{"key1": "value1", "key2": "value2"}', + "o2/components/qc/ANY/any/prefix1": '{"key1": "value1", "key2": "value2"}', + "o2/components/qc/ANY/any/prefix2": '"key1": "value1"', + }), + }); + + qcConfigurationController = new QCConfigurationController(qcConfigurationService, {consul: {qcPath: 'o2/components/qc'}}); + }); + + it('should return keys of all valid configurations in main directory', async () => { + const req = { query: { } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationsKeys(req, res); + assert.ok(res.status.calledWith(200)); + assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/prefix1']); + }); + + it('should return keys of all valid configurations in prefix directory when prefix is set', async () => { + const req = { query: { prefix: 'dir1' } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationsKeys(req, res); + assert.ok(res.status.calledWith(200)); + assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/dir1/prefix1']); + }); + + it('should return keys of all valid configurations when recurse is true', async () => { + const req = { query: { recurse: true } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationsKeys(req, res); + assert.ok(res.status.calledWith(200)); + assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/dir1/prefix1', 'o2/components/qc/ANY/any/prefix1']); + }); + }) + + describe(`'getConfigurationByKey' test suite`, () => { + let qcConfigurationService, qcConfigurationController; + before(() => { + qcConfigurationService = new QCConfigurationService({ + getOnlyRawValueByKey: sinon.stub().resolves({"key1": "value1", "key2": "value2"}), + }); + + qcConfigurationController = new QCConfigurationController(qcConfigurationService, {consul: {qcPath: 'o2/components/qc'}}); + }); + + it('should return configuration for a valid key', async () => { + const req = { query: { key: 'o2/components/qc/ANY/any/prefix1' } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationByKey(req, res); + assert.ok(res.status.calledWith(200)); + assert.deepStrictEqual(res.json.firstCall.args[0], {key1: 'value1', key2: 'value2'}); + }); + + it('should return 400 for missing configuration key', async () => { + const req = { query: {} }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationByKey(req, res); + assert.ok(res.status.calledWith(400)); + assert.deepStrictEqual(res.json.firstCall.args[0], { + message: 'Missing configuration key', + status: 400, + title: 'Invalid Input', + }); + }); + }); +}); diff --git a/Control/test/lib/services/mocha-qcConfiguration.service.test.js b/Control/test/lib/services/mocha-qcConfiguration.service.test.js new file mode 100644 index 000000000..fd86a20df --- /dev/null +++ b/Control/test/lib/services/mocha-qcConfiguration.service.test.js @@ -0,0 +1,69 @@ +/** + * @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. + */ +/* eslint-disable max-len */ + +const assert = require("assert"); +const sinon = require("sinon"); + +const { QCConfigurationService } = require("../../../lib/services/QCConfiguration.service.js"); + +describe(`'QCConfigurationService' test suite`, () => { + + describe(`'getKeysOfValidConfigurations' test suite`, () => { + let qcConfigurationService; + before(() => { + qcConfigurationService = new QCConfigurationService({ + getOnlyRawValuesByKeyPrefix: sinon.stub().resolves({ + "any/dir1": undefined, + "any/dir1/prefix1": '{"key1": "value1", "key2": "value2"}', + "any/prefix1": '{"key1": "value1", "key2": "value2"}', + "any/prefix2": '"key1": "value1"', + }), + }); + }); + + it("should return keys of all valid configurations in main directory", async () => { + const prefix = "any"; + const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); + assert.deepStrictEqual(configurations, ["any/prefix1"]); + }); + + it("should return keys of all valid configurations in prefix directory when prefix is set", async () => { + const prefix = "any/dir1"; + const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); + assert.deepStrictEqual(configurations, ["any/dir1/prefix1"]); + }); + + it("should return keys of all valid configurations when recurse is true", async () => { + const prefix = "any"; + const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix, true); + assert.deepStrictEqual(configurations, ["any/dir1/prefix1", "any/prefix1"]); + }); + }); + + describe(`'getConfigurationByKey' test suite`, () => { + let qcConfigurationService; + before(() => { + qcConfigurationService = new QCConfigurationService({ + getOnlyRawValueByKey: sinon.stub().resolves({"key1": "value1", "key2": "value2"}), + }); + }); + + it("should return configuration for a valid key", async () => { + const key = "any/prefix1"; + const configuration = await qcConfigurationService.getConfigurationByKey(key); + assert.deepStrictEqual(configuration, {"key1": "value1", "key2": "value2"}); + }); + }); +}); From bf8c19b9be85977a1f264a176858bb3380133a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Tue, 17 Jun 2025 16:58:00 +0200 Subject: [PATCH 05/10] implement api end-test --- Control/lib/api.js | 8 ++-- .../controllers/QCConfiguration.controller.js | 18 ++++---- .../api-get-configurations.test.js | 43 +++++++++++++++++++ Control/test/mocha-index.js | 2 + 4 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 Control/test/api/configuration/api-get-configurations.test.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 040900bee..afc4db72a 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -240,13 +240,11 @@ module.exports.setup = (http, ws) => { const qcValidateService = qcConfigurationService.validateService.bind(qcConfigurationService); http.get( "/configurations", qcValidateService, - qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController), - { public: true } + qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController) ); http.get( - '/configuration', qcValidateService, - qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController), - { public: true } + '/configuration', qcValidateService, + qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController) ); // Consul diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index 9410ac170..a88cd6d97 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -12,7 +12,12 @@ * or submit itself to any jurisdiction. */ -const { LogManager, updateAndSendExpressResponseFromNativeError, InvalidInputError, NotFoundError } = require("@aliceo2/web-ui"); +const { + LogManager, + updateAndSendExpressResponseFromNativeError, + InvalidInputError, + NotFoundError, +} = require("@aliceo2/web-ui"); const { errorLogger } = require("../utils.js"); const { getConsulConfig } = require("../config/publicConfigProvider.js"); @@ -21,10 +26,10 @@ const { getConsulConfig } = require("../config/publicConfigProvider.js"); */ class QCConfigurationController { /** - * Setup QCConfigurationController - * @param {QCConfigurationService} qcConfigurationService - * @param {JSON} config - */ + * Setup QCConfigurationController + * @param {QCConfigurationService} qcConfigurationService + * @param {JSON} config + */ constructor(qcConfigurationService, config) { this._qcConfigurationService = qcConfigurationService; this._config = getConsulConfig({ consul: config }); @@ -62,15 +67,12 @@ class QCConfigurationController { */ async getConfigurationByKey(req, res) { const { key } = req.query; - console.log(`getConfigurationByKey: ${key}`); if (!key) { updateAndSendExpressResponseFromNativeError(res, new InvalidInputError("Missing configuration key")); } try { const value = await this._qcConfigurationService.getConfigurationByKey(key); - console.log("value") - console.log(value) if (!value) { updateAndSendExpressResponseFromNativeError(res, new NotFoundError("Configuration not found")); } diff --git a/Control/test/api/configuration/api-get-configurations.test.js b/Control/test/api/configuration/api-get-configurations.test.js new file mode 100644 index 000000000..354dd1bda --- /dev/null +++ b/Control/test/api/configuration/api-get-configurations.test.js @@ -0,0 +1,43 @@ + +/** + * @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 request = require('supertest'); +const { ADMIN_TEST_TOKEN, TEST_URL } = require('../generateToken.js'); + +describe(`'API - GET - /configurations' test suite`, () => { + it('should successfully get all configurations', async () => { + await request(`${TEST_URL}/api/configurations`) + .get(`/?token=${ADMIN_TEST_TOKEN}`) + .expect(200); + }); + + it('should return unauthorized error for missing token requests', async () => { + await request(`${TEST_URL}/api/configurations`) + .get('/') + .expect(403, { + error: '403 - Json Web Token Error', + message: 'You must provide a JWT token' + }); + }); + + it('should return unauthorized error for invalid token requests', async () => { + await request(`${TEST_URL}/api/configurations`) + .get('/?token=invalid-token') + .expect(403, { + error: '403 - Json Web Token Error', + message: 'Invalid JWT token provided' + }); + }); +}); diff --git a/Control/test/mocha-index.js b/Control/test/mocha-index.js index 239827c4f..5fc18e3dd 100644 --- a/Control/test/mocha-index.js +++ b/Control/test/mocha-index.js @@ -173,6 +173,8 @@ describe('Control', function() { require('./api/lock/api-get-locks.test'); require('./api/lock/api-put-locks.test'); + require('./api/configuration/api-get-configurations.test'); + beforeEach(() => this.ok = true); afterEach(() => { From fdae06ea937b38912678ff1602e6a5bc17d80004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Sun, 29 Jun 2025 16:46:10 +0200 Subject: [PATCH 06/10] Fix /configurations api-tests by adding nock --- Control/index.js | 7 +++ .../api-get-configurations.test.js | 2 +- Control/test/config/testConfigForConsul.js | 46 +++++++++++++++++++ Control/test/mocha-index.js | 3 +- 4 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 Control/test/config/testConfigForConsul.js diff --git a/Control/index.js b/Control/index.js index 05c0d798e..baba603db 100644 --- a/Control/index.js +++ b/Control/index.js @@ -19,6 +19,13 @@ const config = require('./lib/config/configProvider.js'); const {buildPublicConfig} = require('./lib/config/publicConfigProvider.js'); const api = require('./lib/api.js'); + +// Initialize nock for Consul only if we are in test environment +if (process.env.NODE_ENV === "test") { + const { initializeNockForConsul } = require("./test/config/testConfigForConsul.js"); + initializeNockForConsul(); +} + // ------------------------------------------------------- buildPublicConfig(config); diff --git a/Control/test/api/configuration/api-get-configurations.test.js b/Control/test/api/configuration/api-get-configurations.test.js index 354dd1bda..9742e4531 100644 --- a/Control/test/api/configuration/api-get-configurations.test.js +++ b/Control/test/api/configuration/api-get-configurations.test.js @@ -20,7 +20,7 @@ describe(`'API - GET - /configurations' test suite`, () => { it('should successfully get all configurations', async () => { await request(`${TEST_URL}/api/configurations`) .get(`/?token=${ADMIN_TEST_TOKEN}`) - .expect(200); + .expect(200, ['key1']); }); it('should return unauthorized error for missing token requests', async () => { diff --git a/Control/test/config/testConfigForConsul.js b/Control/test/config/testConfigForConsul.js new file mode 100644 index 000000000..bf702a4cb --- /dev/null +++ b/Control/test/config/testConfigForConsul.js @@ -0,0 +1,46 @@ +/** + * @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 nock = require('nock'); +const config = require('../test-config'); + +const CONSUL_URL = `http://${config.consul.hostname}:${config.consul.port}`; +const KV_PATH = '/v1/kv/'; +const QC_CONFIG_PATH = `${KV_PATH}${config.consul.qcPath}/ANY/any`; +const QC_CONFIG_QUERY = '?recurse=true'; + +/** + * Setup nock environment to intercept requests to the Consul API. + */ +const initializeNockForConsul = () => { + console.log('Initializing nock for Consul...'); + console.log(`Mocking Consul at ${CONSUL_URL}`); + nock(CONSUL_URL) + .persist() + .get(`${QC_CONFIG_PATH}${QC_CONFIG_QUERY}`) + .reply(200, JSON.stringify([ + { + LockIndex: 0, + Key: "key1", + Flags: 0, + Value: Buffer.from(JSON.stringify({key1: "value1"})).toString('base64'), + CreateIndex: 1, + ModifyIndex: 1 + } + ])) +} + +module.exports = { + initializeNockForConsul +}; \ No newline at end of file diff --git a/Control/test/mocha-index.js b/Control/test/mocha-index.js index 5fc18e3dd..5d37ad011 100644 --- a/Control/test/mocha-index.js +++ b/Control/test/mocha-index.js @@ -48,7 +48,8 @@ describe('Control', function() { const {calls: apricotCalls} = apricotGRPCServer(config); // Start web-server in background - subprocess = spawn('node', ['index.js', 'test/test-config.js'], {stdio: 'pipe'}); + subprocess = spawn('node', ['index.js', 'test/test-config.js'], + {stdio: 'pipe', env: {...process.env, NODE_ENV: 'test'}}); subprocess.stdout.on('data', (chunk) => { subprocessOutput += chunk.toString(); }); From 540993c2bd98135f7b2a786c56eee6ea792d90db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Sun, 29 Jun 2025 17:25:33 +0200 Subject: [PATCH 07/10] Add api tests for /configuration endpoint --- .../api-get-configuration.test.js | 43 +++++++++++++++++++ Control/test/config/testConfigForConsul.js | 16 ++++--- .../mocha-qcConfiguration.controller.test.js | 2 +- .../mocha-qcConfiguration.service.test.js | 4 +- Control/test/mocha-index.js | 1 + 5 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 Control/test/api/configuration/api-get-configuration.test.js diff --git a/Control/test/api/configuration/api-get-configuration.test.js b/Control/test/api/configuration/api-get-configuration.test.js new file mode 100644 index 000000000..98ce12f14 --- /dev/null +++ b/Control/test/api/configuration/api-get-configuration.test.js @@ -0,0 +1,43 @@ + +/** + * @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 request = require('supertest'); +const { ADMIN_TEST_TOKEN, TEST_URL } = require('../generateToken.js'); + +describe(`'API - GET - /configuration' test suite`, () => { + it('should successfully get specific configuration', async () => { + await request(`${TEST_URL}/api/configuration`) + .get(`/?key=key1&token=${ADMIN_TEST_TOKEN}`) + .expect(200, JSON.stringify({key: "value"})); + }); + + it('should return unauthorized error for missing token requests', async () => { + await request(`${TEST_URL}/api/configuration`) + .get('/key=key1') + .expect(403, { + error: '403 - Json Web Token Error', + message: 'You must provide a JWT token' + }); + }); + + it('should return unauthorized error for invalid token requests', async () => { + await request(`${TEST_URL}/api/configuration`) + .get('/?key=key1&token=invalid-token') + .expect(403, { + error: '403 - Json Web Token Error', + message: 'Invalid JWT token provided' + }); + }); +}); diff --git a/Control/test/config/testConfigForConsul.js b/Control/test/config/testConfigForConsul.js index bf702a4cb..6adfdf127 100644 --- a/Control/test/config/testConfigForConsul.js +++ b/Control/test/config/testConfigForConsul.js @@ -17,18 +17,19 @@ const config = require('../test-config'); const CONSUL_URL = `http://${config.consul.hostname}:${config.consul.port}`; const KV_PATH = '/v1/kv/'; -const QC_CONFIG_PATH = `${KV_PATH}${config.consul.qcPath}/ANY/any`; -const QC_CONFIG_QUERY = '?recurse=true'; /** * Setup nock environment to intercept requests to the Consul API. */ const initializeNockForConsul = () => { - console.log('Initializing nock for Consul...'); - console.log(`Mocking Consul at ${CONSUL_URL}`); nock(CONSUL_URL) .persist() - .get(`${QC_CONFIG_PATH}${QC_CONFIG_QUERY}`) + .get('/v1/status/leader') + .reply(200, 'http://localhost:8550'); + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}${config.consul.qcPath}/ANY/any?recurse=true`) .reply(200, JSON.stringify([ { LockIndex: 0, @@ -39,6 +40,11 @@ const initializeNockForConsul = () => { ModifyIndex: 1 } ])) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}key1?raw=true`) + .reply(200, JSON.stringify({key: "value"})) } module.exports = { diff --git a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js index 26a8cf91e..1f70256d7 100644 --- a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js +++ b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js @@ -64,7 +64,7 @@ describe(`'QCConfigurationController' test suite`, () => { let qcConfigurationService, qcConfigurationController; before(() => { qcConfigurationService = new QCConfigurationService({ - getOnlyRawValueByKey: sinon.stub().resolves({"key1": "value1", "key2": "value2"}), + getOnlyRawValueByKey: sinon.stub().resolves({key1: "value1", key2: "value2"}), }); qcConfigurationController = new QCConfigurationController(qcConfigurationService, {consul: {qcPath: 'o2/components/qc'}}); diff --git a/Control/test/lib/services/mocha-qcConfiguration.service.test.js b/Control/test/lib/services/mocha-qcConfiguration.service.test.js index fd86a20df..659f3de1e 100644 --- a/Control/test/lib/services/mocha-qcConfiguration.service.test.js +++ b/Control/test/lib/services/mocha-qcConfiguration.service.test.js @@ -56,14 +56,14 @@ describe(`'QCConfigurationService' test suite`, () => { let qcConfigurationService; before(() => { qcConfigurationService = new QCConfigurationService({ - getOnlyRawValueByKey: sinon.stub().resolves({"key1": "value1", "key2": "value2"}), + getOnlyRawValueByKey: sinon.stub().resolves({key1: "value1", key2: "value2"}), }); }); it("should return configuration for a valid key", async () => { const key = "any/prefix1"; const configuration = await qcConfigurationService.getConfigurationByKey(key); - assert.deepStrictEqual(configuration, {"key1": "value1", "key2": "value2"}); + assert.deepStrictEqual(configuration, {key1: "value1", key2: "value2"}); }); }); }); diff --git a/Control/test/mocha-index.js b/Control/test/mocha-index.js index 5d37ad011..ca8488e89 100644 --- a/Control/test/mocha-index.js +++ b/Control/test/mocha-index.js @@ -175,6 +175,7 @@ describe('Control', function() { require('./api/lock/api-put-locks.test'); require('./api/configuration/api-get-configurations.test'); + require('./api/configuration/api-get-configuration.test'); beforeEach(() => this.ok = true); From 41da43680b5cba9cce8edf0429d4fb658bfb91b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Tue, 1 Jul 2025 19:48:35 +0200 Subject: [PATCH 08/10] Change /configuration endpoint with query parameter to /configurations with route parameter, update tests and fix things after code review --- Control/lib/api.js | 2 +- .../lib/controllers/QCConfiguration.controller.js | 2 +- Control/lib/services/QCConfiguration.service.js | 15 ++------------- .../configuration/api-get-configuration.test.js | 12 ++++++------ .../mocha-qcConfiguration.controller.test.js | 4 ++-- 5 files changed, 12 insertions(+), 23 deletions(-) diff --git a/Control/lib/api.js b/Control/lib/api.js index afc4db72a..53682458a 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -243,7 +243,7 @@ module.exports.setup = (http, ws) => { qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController) ); http.get( - '/configuration', qcValidateService, + '/configurations/:key(*)', qcValidateService, qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController) ); diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index a88cd6d97..fd71e375b 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -66,7 +66,7 @@ class QCConfigurationController { * @param {Response} res */ async getConfigurationByKey(req, res) { - const { key } = req.query; + const { key } = req.params; if (!key) { updateAndSendExpressResponseFromNativeError(res, new InvalidInputError("Missing configuration key")); } diff --git a/Control/lib/services/QCConfiguration.service.js b/Control/lib/services/QCConfiguration.service.js index 455c017d0..8901e6166 100644 --- a/Control/lib/services/QCConfiguration.service.js +++ b/Control/lib/services/QCConfiguration.service.js @@ -34,17 +34,6 @@ class QCConfigurationService { this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? "cnf"}/qc-configuration-service`); } - /** - * Initialize Lock service based on the provided list of detectors - * @param {Array} detectors = [] - list of detectors to be used for the lock mechanism - * @return {void} - */ - // setLockStatesForDetectors(detectors = []) { - // for (const detectorName of detectors) { - // this._locksByDetector[detectorName] = new DetectorLock(detectorName); - // } - // } - /** * Check if consulService is present: * * If yes, allow request to continue @@ -83,10 +72,10 @@ class QCConfigurationService { Object.entries(data || {}).forEach(([key, value]) => { try { - if (!JSON.parse(value)) { + if (!recurse && key.replace(`${prefix}/`, "").includes("/")) { return; } - if (!recurse && key.replace(`${prefix}/`, "").includes("/")) { + if (!JSON.parse(value)) { return; } diff --git a/Control/test/api/configuration/api-get-configuration.test.js b/Control/test/api/configuration/api-get-configuration.test.js index 98ce12f14..548036a97 100644 --- a/Control/test/api/configuration/api-get-configuration.test.js +++ b/Control/test/api/configuration/api-get-configuration.test.js @@ -18,14 +18,14 @@ const { ADMIN_TEST_TOKEN, TEST_URL } = require('../generateToken.js'); describe(`'API - GET - /configuration' test suite`, () => { it('should successfully get specific configuration', async () => { - await request(`${TEST_URL}/api/configuration`) - .get(`/?key=key1&token=${ADMIN_TEST_TOKEN}`) + await request(`${TEST_URL}/api/configurations/key1`) + .get(`/?token=${ADMIN_TEST_TOKEN}`) .expect(200, JSON.stringify({key: "value"})); }); it('should return unauthorized error for missing token requests', async () => { - await request(`${TEST_URL}/api/configuration`) - .get('/key=key1') + await request(`${TEST_URL}/api/configurations/key1`) + .get('/') .expect(403, { error: '403 - Json Web Token Error', message: 'You must provide a JWT token' @@ -33,8 +33,8 @@ describe(`'API - GET - /configuration' test suite`, () => { }); it('should return unauthorized error for invalid token requests', async () => { - await request(`${TEST_URL}/api/configuration`) - .get('/?key=key1&token=invalid-token') + await request(`${TEST_URL}/api/configurations/key1`) + .get('/?token=invalid-token') .expect(403, { error: '403 - Json Web Token Error', message: 'Invalid JWT token provided' diff --git a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js index 1f70256d7..476d14aef 100644 --- a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js +++ b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js @@ -71,7 +71,7 @@ describe(`'QCConfigurationController' test suite`, () => { }); it('should return configuration for a valid key', async () => { - const req = { query: { key: 'o2/components/qc/ANY/any/prefix1' } }; + const req = { params: { key: 'o2/components/qc/ANY/any/prefix1' } }; const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; await qcConfigurationController.getConfigurationByKey(req, res); assert.ok(res.status.calledWith(200)); @@ -79,7 +79,7 @@ describe(`'QCConfigurationController' test suite`, () => { }); it('should return 400 for missing configuration key', async () => { - const req = { query: {} }; + const req = { params: {} }; const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; await qcConfigurationController.getConfigurationByKey(req, res); assert.ok(res.status.calledWith(400)); From bc937072f747057532c4553f4d7f236f0197a61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Sun, 17 Aug 2025 23:15:08 +0200 Subject: [PATCH 09/10] chore: Apply requested changes from PR review --- Control/lib/api.js | 48 ++++++--- Control/lib/controllers/Consul.controller.js | 25 ----- .../controllers/QCConfiguration.controller.js | 22 ++-- .../validateConsulServiceMiddlewareFactory.js | 37 +++++++ .../lib/services/QCConfiguration.service.js | 101 +++++++----------- .../controllers/mocha-consul-controller.js | 31 ------ .../mocha-qcConfiguration.controller.test.js | 26 +++++ ...dateConsulServiceMiddlewareFactory.test.js | 57 ++++++++++ .../mocha-qcConfiguration.service.test.js | 79 ++++++++++++++ 9 files changed, 285 insertions(+), 141 deletions(-) create mode 100644 Control/lib/middleware/validateConsulServiceMiddlewareFactory.js create mode 100644 Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js diff --git a/Control/lib/api.js b/Control/lib/api.js index 53682458a..c3ee61641 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -24,6 +24,7 @@ const { DetectorId } = require('./common/detectorId.enum.js'); const {minimumRoleMiddleware} = require('./middleware/minimumRole.middleware.js'); const {addDetectorIdMiddleware} = require('./middleware/addDetectorId.middleware.js'); const {requireDetectorOrGlobalRoleMiddleware} = require('./middleware/requireDetectorOrGlobalRole.middleware.js'); +const {validateConsulServiceMiddlewareFactory} = require('./middleware/validateConsulServiceMiddlewareFactory.js'); const { setDetectorsFromEnvironmentMiddlewareFactory @@ -92,13 +93,11 @@ module.exports.setup = (http, ws) => { const broadcastService = new BroadcastService(ws); const cacheService = new CacheService(broadcastService); const environmentCacheService = new EnvironmentCacheService(broadcastService, eventEmitter); - const qcConfigurationService = new QCConfigurationService(consulService); - qcConfigurationService.testConsulStatus(); + const qcConfigurationService = new QCConfigurationService(consulService); const qcConfigurationController = new QCConfigurationController(qcConfigurationService, config.consul); const consulController = new ConsulController(consulService, config.consul); - consulController.testConsulStatus(); const ctrlProxy = new GrpcServiceClient(config.grpc, O2_CONTROL_PROTO_PATH); const ctrlService = new ControlService(ctrlProxy, consulController, config.grpc, O2_CONTROL_PROTO_PATH); @@ -156,7 +155,7 @@ module.exports.setup = (http, ws) => { const intervals = new Intervals(); - initializeData(apricotService, lockService); + initializeData(apricotService, lockService, consulService); initializeIntervals(intervals, statusService, runService, bkpService, environmentService); const coreMiddleware = [ @@ -164,6 +163,7 @@ module.exports.setup = (http, ws) => { ]; const setDetectorsFromEnvironmentMiddleware = setDetectorsFromEnvironmentMiddlewareFactory(environmentService); const verifyLockOwnershipMiddleware = getDetectorsLockOwnershipMiddlewareFactory(lockService); + const validateConsulServiceMiddleware = validateConsulServiceMiddlewareFactory(consulService); ctrlProxy.methods.forEach( (method) => http.post(`/${method}`, coreMiddleware, (req, res) => ctrlService.executeCommand(req, res)), @@ -237,23 +237,30 @@ module.exports.setup = (http, ws) => { ); // Configuration - const qcValidateService = qcConfigurationService.validateService.bind(qcConfigurationService); http.get( - "/configurations", qcValidateService, + '/configurations', validateConsulServiceMiddleware, qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController) ); http.get( - '/configurations/:key(*)', qcValidateService, + '/configurations/:key(*)', validateConsulServiceMiddleware, qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController) ); // Consul - const validateService = consulController.validateService.bind(consulController); - http.get('/consul/flps', validateService, consulController.getFLPs.bind(consulController)); - http.get('/consul/crus', validateService, consulController.getCRUs.bind(consulController)); - http.get('/consul/crus/config', validateService, consulController.getCRUsWithConfiguration.bind(consulController)); - http.get('/consul/crus/aliases', validateService, consulController.getCRUsAlias.bind(consulController)); - http.post('/consul/crus/config/save', validateService, consulController.saveCRUsConfiguration.bind(consulController)); + http.get('/consul/flps', validateConsulServiceMiddleware, consulController.getFLPs.bind(consulController)); + http.get('/consul/crus', validateConsulServiceMiddleware, consulController.getCRUs.bind(consulController)); + http.get( + '/consul/crus/config', validateConsulServiceMiddleware, + consulController.getCRUsWithConfiguration.bind(consulController) + ); + http.get( + '/consul/crus/aliases', validateConsulServiceMiddleware, + consulController.getCRUsAlias.bind(consulController) + ); + http.post( + '/consul/crus/config/save', validateConsulServiceMiddleware, + consulController.saveCRUsConfiguration.bind(consulController) + ); }; /** @@ -297,8 +304,21 @@ 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 {ConsulService} consulService - service for communicating with Consul */ -async function initializeData(apricotService, lockService) { +async function initializeData(apricotService, lockService, consulService) { await apricotService.init(); lockService.setLockStatesForDetectors(apricotService.detectors); + await testConsulStatus(consulService); } + +/** + * Method to check if consul service can be used + * @param {ConsulService} consulService + */ +async function testConsulStatus(consulService) { + consulService + .getConsulLeaderStatus() + .then((data) => logger.info(`Service is up and running on: ${data}`)) + .catch((error) => logger.error(`Connection failed due to ${error}`)); +} \ No newline at end of file diff --git a/Control/lib/controllers/Consul.controller.js b/Control/lib/controllers/Consul.controller.js index 70cc80ade..285f5e7ae 100644 --- a/Control/lib/controllers/Consul.controller.js +++ b/Control/lib/controllers/Consul.controller.js @@ -39,31 +39,6 @@ class ConsulController { this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? 'cog'}/consul`); } - /** - * Check if consulService is present: - * * If yes, allow request to continue - * * If not, send response accordingly - * @param {Request} req - * @param {Response} res - * @param {Next} next - */ - validateService(req, res, next) { - if (this.consulService) { - next(); - } else { - errorHandler('Unable to retrieve configuration of consul service', res, 502); - } - } - - /** - * Method to check if consul service can be used - */ - async testConsulStatus() { - this.consulService.getConsulLeaderStatus() - .then((data) => this._logger.info(`Service is up and running on: ${data}`)) - .catch((error) => this._logger.error(`Connection failed due to ${error}`)); - } - /** * Method to request all CRUs available in consul KV store under the * hardware key diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index fd71e375b..90594599f 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -27,8 +27,8 @@ const { getConsulConfig } = require("../config/publicConfigProvider.js"); class QCConfigurationController { /** * Setup QCConfigurationController - * @param {QCConfigurationService} qcConfigurationService - * @param {JSON} config + * @param {QCConfigurationService} qcConfigurationService - service for managing QC configurations + * @param {JSON} config - consul configuration */ constructor(qcConfigurationService, config) { this._qcConfigurationService = qcConfigurationService; @@ -40,8 +40,8 @@ class QCConfigurationController { /** * Method to get configurations names - * @param {Request} req - * @param {Response} res + * @param {Request} req - HTTP Request object + * @param {Response} res - HTTP Response object */ async getConfigurationsKeys(req, res) { const { prefix = "", recurse = false } = req.query; @@ -49,8 +49,10 @@ class QCConfigurationController { try { const parsedData = await this._qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); + if (!parsedData || parsedData.length === 0) { updateAndSendExpressResponseFromNativeError(res, new NotFoundError("No configurations found")); + return; } res.status(200).json(parsedData); @@ -62,21 +64,19 @@ class QCConfigurationController { /** * Method to get configuration value by key - * @param {Request} req - * @param {Response} res + * @param {Request} req - HTTP Request object + * @param {Response} res - HTTP Response object */ async getConfigurationByKey(req, res) { const { key } = req.params; - if (!key) { + + if (!key || key.trim() === "") { updateAndSendExpressResponseFromNativeError(res, new InvalidInputError("Missing configuration key")); + return; } try { const value = await this._qcConfigurationService.getConfigurationByKey(key); - if (!value) { - updateAndSendExpressResponseFromNativeError(res, new NotFoundError("Configuration not found")); - } - res.status(200).json(value); } catch (error) { errorLogger(error, this._logger); diff --git a/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js b/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js new file mode 100644 index 000000000..f2f709b83 --- /dev/null +++ b/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js @@ -0,0 +1,37 @@ +/** + * @license + * Copyright CERN and copyright holders of ALICE O2. This software is + * distributed under the terms of the GNU General Public License v3 (GPL + * Version 3), copied verbatim in the file "COPYING". + * + * See http://alice-o2.web.cern.ch/license for full licensing information. + * + * 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. + */ + +/** + * Factory function to check if consul service is available + * + * @param {ConsulService} consulService - service for which availability is checked + * @returns {function(req, res, next): void} - middleware function + */ +const validateConsulServiceMiddlewareFactory = (consulService) => { + /** + * Middleware function to check if consul service is available + * @param {Request} req - HTTP Request object + * @param {Response} res - HTTP Response object + * @param {Next} next - HTTP Next object to use if checks pass + * @returns {void} continue if checks pass, uses response object to respond with error if checks fail + */ + return async (req, res, next) => { + if (consulService) { + next(); + } else { + res.status(502).json({ message: "Unable to retrieve configuration of consul service" }); + } + }; +}; + +exports.validateConsulServiceMiddlewareFactory = validateConsulServiceMiddlewareFactory; diff --git a/Control/lib/services/QCConfiguration.service.js b/Control/lib/services/QCConfiguration.service.js index 8901e6166..f14a6bd15 100644 --- a/Control/lib/services/QCConfiguration.service.js +++ b/Control/lib/services/QCConfiguration.service.js @@ -12,8 +12,7 @@ * or submit itself to any jurisdiction. */ -const { NotFoundError, ServiceUnavailableError, LogManager } = require("@aliceo2/web-ui"); -const { errorHandler } = require("../utils.js"); +const { NotFoundError, LogManager } = require("@aliceo2/web-ui"); /** * @class @@ -35,79 +34,61 @@ class QCConfigurationService { } /** - * Check if consulService is present: - * * If yes, allow request to continue - * * If not, send response accordingly - * @param {Request} req - * @param {Response} res - * @param {Next} next + * Get keys of configurations stored in Consul + * @param {String} prefix - prefix to filter the keys + * @param {boolean} [recurse=false] - whether to recurse into subdirectories */ - validateService(req, res, next) { - if (this._consulService) { - next(); - } else { - errorHandler("Unable to retrieve configuration of consul service", res, 502); + async getKeysOfValidConfigurations(prefix, recurse = false) { + let data; + + try { + data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); + } catch (e) { + return []; } + + return this.filterConfigurations(data, recurse, prefix); } /** - * Method to check if consul service can be used + * Get configuration by key from Consul + * @param {string} key - the key of the configuration */ - async testConsulStatus() { - this._consulService - .getConsulLeaderStatus() - .then((data) => this._logger.info(`Service is up and running on: ${data}`)) - .catch((error) => this._logger.error(`Connection failed due to ${error}`)); + async getConfigurationByKey(key) { + try{ + return await this._consulService.getOnlyRawValueByKey(key); + }catch (error) { + this._logger.error(`Error getting configuration by key: ${key}`, error); + throw new NotFoundError(`Configuration not found for key: ${key}`); + } } + /** - * Get keys of configurations stored in Consul - * @param {String} prefix - prefix to filter the keys - * @param {boolean} [recurse=false] - whether to recurse into subdirectories + * Filters a configuration object and returns keys of entries with valid JSON values. + * @param {object} configs - an object with string values to be checked. + * @param {boolean} recurse - whether to recurse into subdirectories + * @param {string} prefix - the prefix to filter keys */ - async getKeysOfValidConfigurations(prefix, recurse = false) { - try { - const data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); - const parsedData = []; - - Object.entries(data || {}).forEach(([key, value]) => { - try { - if (!recurse && key.replace(`${prefix}/`, "").includes("/")) { - return; - } - if (!JSON.parse(value)) { - return; - } + filterConfigurations(configs, recurse, prefix) { + const parsedData = []; + Object.entries(configs || {}).forEach(([key, value]) => { + try { + if (!recurse && key.replace(`${prefix}/`, "").includes("/")) { + return; + } + const parsedValue = JSON.parse(value); + + if (typeof parsedValue === 'object' && parsedValue !== null && !Array.isArray(parsedValue)) { parsedData.push(key); - } catch (e) { - // skip } - }); - - return parsedData; - } catch (error) { - if (error instanceof NotFoundError) { - throw new NotFoundError(`No configurations found for prefix: ${prefix}`); + } catch (e) { + // skip } - throw error; - } - } + }); - /** - * Get configuration by key from Consul - * @param {String} key - the key of the configuration - */ - async getConfigurationByKey(key) { - try { - const value = await this._consulService.getOnlyRawValueByKey(key); - if (!value) { - throw new NotFoundError(`Configuration not found for key: ${key}`); - } - return value; - } catch (error) { - throw new ServiceUnavailableError(`Error retrieving configuration for key: ${key}`); - } + return parsedData; } } diff --git a/Control/test/lib/controllers/mocha-consul-controller.js b/Control/test/lib/controllers/mocha-consul-controller.js index 762bb1324..4f3c8bfb3 100644 --- a/Control/test/lib/controllers/mocha-consul-controller.js +++ b/Control/test/lib/controllers/mocha-consul-controller.js @@ -63,37 +63,6 @@ describe('ConsulController test suite', () => { }); }); - describe('Test Consul Connection', async () => { - let consulService; - beforeEach(() => consulService = {}); - it('should successfully query host of ConsulLeader', async () => { - consulService.getConsulLeaderStatus = sinon.stub().resolves('localhost:8550'); - const connector = new ConsulController(consulService, config); - await connector.testConsulStatus(); - }); - it('should successfully query host of ConsulLeader and fail gracefully', async () => { - consulService.getConsulLeaderStatus = sinon.stub().rejects('Unable to query Consul'); - const connector = new ConsulController(consulService, config); - await connector.testConsulStatus(); - }); - it('should successfully validate connector if consulService is present', () => { - const connector = new ConsulController(consulService, config); - const next = sinon.stub(); - connector.validateService({}, {}, next); - assert.ok(next.calledWith()); - }); - it('should successfully respond with error on connector validate if consulService is missing', () => { - const connector = new ConsulController(undefined, config); - const res = { - status: sinon.stub(), - send: sinon.stub(), - }; - connector.validateService({}, res, {}); - assert.ok(res.status.calledWith(502)); - assert.ok(res.send.calledWith({message: 'Unable to retrieve configuration of consul service'})); - }); - }); - describe('Request CRUs tests', async () => { let consulService; beforeEach(() => { diff --git a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js index 476d14aef..458a97480 100644 --- a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js +++ b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js @@ -58,6 +58,19 @@ describe(`'QCConfigurationController' test suite`, () => { assert.ok(res.status.calledWith(200)); assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/dir1/prefix1', 'o2/components/qc/ANY/any/prefix1']); }); + + it('should return 404 when no configurations are found', async () => { + qcConfigurationController._qcConfigurationService._consulService.getOnlyRawValuesByKeyPrefix.resolves({}); + const req = { query: { } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationsKeys(req, res); + assert.ok(res.status.calledWith(404)); + assert.deepStrictEqual(res.json.firstCall.args[0], { + message: 'No configurations found', + status: 404, + title: 'Not Found', + }); + }); }) describe(`'getConfigurationByKey' test suite`, () => { @@ -89,5 +102,18 @@ describe(`'QCConfigurationController' test suite`, () => { title: 'Invalid Input', }); }); + + it('should return 404 for non-existing configuration key', async () => { + qcConfigurationController._qcConfigurationService._consulService.getOnlyRawValueByKey.rejects(new Error("Non-2xx status code")); + const req = { params: { key: 'o2/components/qc/ANY/any/non-existing-key' } }; + const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; + await qcConfigurationController.getConfigurationByKey(req, res); + assert.ok(res.status.calledWith(404)); + assert.deepStrictEqual(res.json.firstCall.args[0], { + message: 'Configuration not found for key: o2/components/qc/ANY/any/non-existing-key', + status: 404, + title: 'Not Found', + }); + }); }); }); diff --git a/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js b/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js new file mode 100644 index 000000000..de4eaf051 --- /dev/null +++ b/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js @@ -0,0 +1,57 @@ +/** + * @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 sinon = require('sinon'); +const { validateConsulServiceMiddlewareFactory } = require('../../../lib/middleware/validateConsulServiceMiddlewareFactory.js'); + +describe('`validateConsulServiceMiddlewareFactory` test suite', () => { + let consulService, reqMock, resMock, nextMock; + + beforeEach(() => { + consulService = {}; + + reqMock = {}; + + resMock = { + status: sinon.stub().returnsThis(), + json: sinon.stub(), + }; + + nextMock = sinon.stub(); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should call next() consulService is ok', async () => { + await validateConsulServiceMiddlewareFactory(consulService)(reqMock, resMock, nextMock); + + assert.ok(nextMock.calledOnce); + assert.ok(resMock.status.notCalled); + assert.ok(resMock.json.notCalled); + }); + + it('should return 502 if there consulService is not available', async () => { + consulService = null; + await validateConsulServiceMiddlewareFactory(consulService)(reqMock, resMock, nextMock); + + assert.ok(resMock.status.calledOnceWith(502)); + assert.ok(resMock.json.calledOnceWith({ + message: 'Unable to retrieve configuration of consul service', + })); + assert.ok(nextMock.notCalled); + }); +}); diff --git a/Control/test/lib/services/mocha-qcConfiguration.service.test.js b/Control/test/lib/services/mocha-qcConfiguration.service.test.js index 659f3de1e..507d0a48f 100644 --- a/Control/test/lib/services/mocha-qcConfiguration.service.test.js +++ b/Control/test/lib/services/mocha-qcConfiguration.service.test.js @@ -50,6 +50,13 @@ describe(`'QCConfigurationService' test suite`, () => { const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix, true); assert.deepStrictEqual(configurations, ["any/dir1/prefix1", "any/prefix1"]); }); + + it("should return an empty array when no valid configurations are found", async () => { + qcConfigurationService._consulService.getOnlyRawValuesByKeyPrefix.resolves({}); + const prefix = "nonexistent"; + const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); + assert.deepStrictEqual(configurations, []); + }); }); describe(`'getConfigurationByKey' test suite`, () => { @@ -65,5 +72,77 @@ describe(`'QCConfigurationService' test suite`, () => { const configuration = await qcConfigurationService.getConfigurationByKey(key); assert.deepStrictEqual(configuration, {key1: "value1", key2: "value2"}); }); + + it("should throw NotFoundError for an invalid key", async () => { + qcConfigurationService._consulService.getOnlyRawValueByKey.rejects(new Error("Not found")); + const key = "invalid/key"; + try { + await qcConfigurationService.getConfigurationByKey(key); + assert.fail("Expected NotFoundError to be thrown"); + } catch (error) { + assert.strictEqual(error.message, `Configuration not found for key: ${key}`); + } + }); + }); + + describe('`filterConfigurations` test suite', () => { + let qcConfigurationService; + + before(() => { + // This setup runs once before all tests in this suite + qcConfigurationService = new QCConfigurationService({}); + }); + + it('should return keys of valid JSON objects and ignore others when recurse is false', () => { + const configs = { + 'any/valid_object': '{"key": "value"}', + 'any/empty_object': '{}', + 'any/nested/key': '{"a": 1}', + 'any/not_a_json': 'just a plain string', + 'any/malformed_json': '{"key":', + 'any/json_string': '"a valid json string"', + 'any/json_array': '[1, 2, 3]', + 'any/json_null': 'null', + }; + + const expectedKeys = ['any/valid_object', 'any/empty_object']; + const result = qcConfigurationService.filterConfigurations(configs, false, 'any'); + + assert.deepStrictEqual(result, expectedKeys); + }); + + it('should include nested keys when recurse is true', () => { + const configs = { + 'any/valid_object': '{"key": "value"}', + 'any/nested/key': '{"a": 1}', + 'any/nested/invalid': 'not json', + }; + const expectedKeys = ['any/valid_object', 'any/nested/key']; + const result = qcConfigurationService.filterConfigurations(configs, true, 'any'); + + assert.deepStrictEqual(result, expectedKeys); + }); + + it('should return an empty array when no valid JSON objects are found', () => { + const configs = { + 'any/invalid1': 'not a json', + 'any/invalid2': '{"a":1,', + 'any/valid_but_string': '"hello"', + }; + + const result = qcConfigurationService.filterConfigurations(configs, false, 'any'); + + assert.deepStrictEqual(result, []); + }); + + it('should return an empty array for null, undefined and empty object input', () => { + const resultForNull = qcConfigurationService.filterConfigurations(null, false, 'any'); + const resultForUndefined = qcConfigurationService.filterConfigurations(undefined, false, 'any'); + const resultForEmpty = qcConfigurationService.filterConfigurations({}, false, 'any'); + + assert.deepStrictEqual(resultForNull, []); + assert.deepStrictEqual(resultForUndefined, []); + assert.deepStrictEqual(resultForEmpty, []); + }); }); }); From f1c09a0b10b6b5d32bd3e486fa9224f9180586b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Piechnik?= Date: Wed, 20 Aug 2025 23:14:54 +0200 Subject: [PATCH 10/10] chore: Apply requested changes from PR review --- Control/lib/api.js | 10 +- .../controllers/QCConfiguration.controller.js | 28 ++- .../validateConsulServiceMiddlewareFactory.js | 4 +- .../lib/services/QCConfiguration.service.js | 22 +- .../api-get-configuration.test.js | 58 ++++- .../api-get-configurations.test.js | 55 ++++- Control/test/config/testConfigForConsul.js | 38 +++- .../mocha-qcConfiguration.controller.test.js | 210 +++++++++++------- ...dateConsulServiceMiddlewareFactory.test.js | 10 +- .../mocha-qcConfiguration.service.test.js | 146 ++++++------ 10 files changed, 370 insertions(+), 211 deletions(-) diff --git a/Control/lib/api.js b/Control/lib/api.js index 3276d2e02..2ca5b17c0 100644 --- a/Control/lib/api.js +++ b/Control/lib/api.js @@ -260,11 +260,11 @@ module.exports.setup = (http, ws) => { // Configuration http.get( '/configurations', validateConsulServiceMiddleware, - qcConfigurationController.getConfigurationsKeys.bind(qcConfigurationController) + qcConfigurationController.getConfigurationsKeysHandler.bind(qcConfigurationController) ); http.get( '/configurations/:key(*)', validateConsulServiceMiddleware, - qcConfigurationController.getConfigurationByKey.bind(qcConfigurationController) + qcConfigurationController.getConfigurationByKeyHandler.bind(qcConfigurationController) ); // Consul @@ -328,18 +328,18 @@ function initializeIntervals(intervalsService, statusService, runService, bkpSer * @param {ConsulService} consulService - service for communicating with Consul */ async function initializeData(apricotService, lockService, consulService) { + testConsulStatus(consulService); await apricotService.init(); lockService.setLockStatesForDetectors(apricotService.detectors); - await testConsulStatus(consulService); } /** * Method to check if consul service can be used * @param {ConsulService} consulService */ -async function testConsulStatus(consulService) { +function testConsulStatus(consulService) { consulService .getConsulLeaderStatus() .then((data) => logger.info(`Service is up and running on: ${data}`)) .catch((error) => logger.error(`Connection failed due to ${error}`)); -} \ No newline at end of file +} diff --git a/Control/lib/controllers/QCConfiguration.controller.js b/Control/lib/controllers/QCConfiguration.controller.js index 90594599f..8e359ed51 100644 --- a/Control/lib/controllers/QCConfiguration.controller.js +++ b/Control/lib/controllers/QCConfiguration.controller.js @@ -17,6 +17,7 @@ const { updateAndSendExpressResponseFromNativeError, InvalidInputError, NotFoundError, + ServiceUnavailableError, } = require("@aliceo2/web-ui"); const { errorLogger } = require("../utils.js"); const { getConsulConfig } = require("../config/publicConfigProvider.js"); @@ -43,22 +44,27 @@ class QCConfigurationController { * @param {Request} req - HTTP Request object * @param {Response} res - HTTP Response object */ - async getConfigurationsKeys(req, res) { + async getConfigurationsKeysHandler(req, res) { const { prefix = "", recurse = false } = req.query; const prefixPath = prefix ? `${this._qcConfigurationsPath}/${prefix}` : this._qcConfigurationsPath; try { - const parsedData = await this._qcConfigurationService.getKeysOfValidConfigurations(prefixPath, recurse); + const validKeys = await this._qcConfigurationService.retrieveKeysOfValidConfigurations(prefixPath, recurse); - if (!parsedData || parsedData.length === 0) { - updateAndSendExpressResponseFromNativeError(res, new NotFoundError("No configurations found")); + if (!validKeys || validKeys.length === 0) { + updateAndSendExpressResponseFromNativeError(res, new NotFoundError("No valid configurations found")); return; } - res.status(200).json(parsedData); + res.status(200).json(validKeys); } catch (error) { errorLogger(error, this._logger); - updateAndSendExpressResponseFromNativeError(res, error); + if (error.message?.includes('Non-2xx status code: 404')) { + updateAndSendExpressResponseFromNativeError(res, + new NotFoundError(`Configurations prefix not found: "${prefixPath}"`)); + } else { + updateAndSendExpressResponseFromNativeError(res, new ServiceUnavailableError("Consul service unavailable")); + } } } @@ -67,7 +73,7 @@ class QCConfigurationController { * @param {Request} req - HTTP Request object * @param {Response} res - HTTP Response object */ - async getConfigurationByKey(req, res) { + async getConfigurationByKeyHandler(req, res) { const { key } = req.params; if (!key || key.trim() === "") { @@ -76,11 +82,15 @@ class QCConfigurationController { } try { - const value = await this._qcConfigurationService.getConfigurationByKey(key); + const value = await this._qcConfigurationService.retrieveConfigurationByKey(key); res.status(200).json(value); } catch (error) { errorLogger(error, this._logger); - updateAndSendExpressResponseFromNativeError(res, error); + if (error.message?.includes('Non-2xx status code: 404')) { + updateAndSendExpressResponseFromNativeError(res, new NotFoundError(`Configuration not found for key: ${key}`)); + } else { + updateAndSendExpressResponseFromNativeError(res, new ServiceUnavailableError("Consul service unavailable")); + } } } } diff --git a/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js b/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js index f2f709b83..1b12f0c90 100644 --- a/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js +++ b/Control/lib/middleware/validateConsulServiceMiddlewareFactory.js @@ -11,6 +11,8 @@ * or submit itself to any jurisdiction. */ +const { updateAndSendExpressResponseFromNativeError, ServiceUnavailableError } = require("@aliceo2/web-ui"); + /** * Factory function to check if consul service is available * @@ -29,7 +31,7 @@ const validateConsulServiceMiddlewareFactory = (consulService) => { if (consulService) { next(); } else { - res.status(502).json({ message: "Unable to retrieve configuration of consul service" }); + updateAndSendExpressResponseFromNativeError(res, new ServiceUnavailableError("Consul service is not available")); } }; }; diff --git a/Control/lib/services/QCConfiguration.service.js b/Control/lib/services/QCConfiguration.service.js index f14a6bd15..3c860a962 100644 --- a/Control/lib/services/QCConfiguration.service.js +++ b/Control/lib/services/QCConfiguration.service.js @@ -12,7 +12,7 @@ * or submit itself to any jurisdiction. */ -const { NotFoundError, LogManager } = require("@aliceo2/web-ui"); +const { LogManager } = require("@aliceo2/web-ui"); /** * @class @@ -38,15 +38,8 @@ class QCConfigurationService { * @param {String} prefix - prefix to filter the keys * @param {boolean} [recurse=false] - whether to recurse into subdirectories */ - async getKeysOfValidConfigurations(prefix, recurse = false) { - let data; - - try { - data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); - } catch (e) { - return []; - } - + async retrieveKeysOfValidConfigurations(prefix, recurse = false) { + const data = await this._consulService.getOnlyRawValuesByKeyPrefix(prefix); return this.filterConfigurations(data, recurse, prefix); } @@ -54,13 +47,8 @@ class QCConfigurationService { * Get configuration by key from Consul * @param {string} key - the key of the configuration */ - async getConfigurationByKey(key) { - try{ - return await this._consulService.getOnlyRawValueByKey(key); - }catch (error) { - this._logger.error(`Error getting configuration by key: ${key}`, error); - throw new NotFoundError(`Configuration not found for key: ${key}`); - } + async retrieveConfigurationByKey(key) { + return await this._consulService.getOnlyRawValueByKey(key); } diff --git a/Control/test/api/configuration/api-get-configuration.test.js b/Control/test/api/configuration/api-get-configuration.test.js index 548036a97..599dcbbd7 100644 --- a/Control/test/api/configuration/api-get-configuration.test.js +++ b/Control/test/api/configuration/api-get-configuration.test.js @@ -16,25 +16,59 @@ const request = require('supertest'); const { ADMIN_TEST_TOKEN, TEST_URL } = require('../generateToken.js'); -describe(`'API - GET - /configuration' test suite`, () => { - it('should successfully get specific configuration', async () => { - await request(`${TEST_URL}/api/configurations/key1`) - .get(`/?token=${ADMIN_TEST_TOKEN}`) - .expect(200, JSON.stringify({key: "value"})); +describe(`'API - GET - /configurations/:key(*)' test suite`, () => { + it('should return 200 with the configuration object for an existing key', async () => { + const expectedBody = { key: "value" }; + await request(`${TEST_URL}/api/configurations`) + .get(`/key1?token=${ADMIN_TEST_TOKEN}`) + .expect(200, expectedBody); }); - it('should return unauthorized error for missing token requests', async () => { - await request(`${TEST_URL}/api/configurations/key1`) - .get('/') + it('should return 404 when the configuration key does not exist', async () => { + const expectedError = { + message: "Configuration not found for key: nonexistent", + status: 404, + title: "Not Found" + }; + await request(`${TEST_URL}/api/configurations`) + .get(`/nonexistent?token=${ADMIN_TEST_TOKEN}`) + .expect(404, expectedError); + }); + + it('should return 400 when the key parameter is empty', async () => { + const expectedError = { + message: "Missing configuration key", + status: 400, + title: "Invalid Input" + }; + await request(`${TEST_URL}/api/configurations`) + .get(`/%20?token=${ADMIN_TEST_TOKEN}`) + .expect(400, expectedError); + }); + + it('should return 503 when Consul fails to respond', async () => { + const expectedError = { + message: "Consul service unavailable", + status: 503, + title: "Service Unavailable" + }; + await request(`${TEST_URL}/api/configurations`) + .get(`/consul-failure?token=${ADMIN_TEST_TOKEN}`) + .expect(503, expectedError); + }); + + it('should return 403 unauthorized error for missing token requests', async () => { + await request(`${TEST_URL}/api/configurations`) + .get('/key1') .expect(403, { error: '403 - Json Web Token Error', - message: 'You must provide a JWT token' + message: 'You must provide a JWT token' }); }); - it('should return unauthorized error for invalid token requests', async () => { - await request(`${TEST_URL}/api/configurations/key1`) - .get('/?token=invalid-token') + it('should return 403 unauthorized error for invalid token requests', async () => { + await request(`${TEST_URL}/api/configurations`) + .get('/key1?token=invalid-token') .expect(403, { error: '403 - Json Web Token Error', message: 'Invalid JWT token provided' diff --git a/Control/test/api/configuration/api-get-configurations.test.js b/Control/test/api/configuration/api-get-configurations.test.js index 9742e4531..afea531e9 100644 --- a/Control/test/api/configuration/api-get-configurations.test.js +++ b/Control/test/api/configuration/api-get-configurations.test.js @@ -15,26 +15,63 @@ const request = require('supertest'); const { ADMIN_TEST_TOKEN, TEST_URL } = require('../generateToken.js'); +const { consul: { qcPath } } = require('../../test-config'); describe(`'API - GET - /configurations' test suite`, () => { - it('should successfully get all configurations', async () => { - await request(`${TEST_URL}/api/configurations`) - .get(`/?token=${ADMIN_TEST_TOKEN}`) + it('should return 200 with valid configuration keys', async () => { + await request(`${TEST_URL}/api`) + .get(`/configurations?token=${ADMIN_TEST_TOKEN}`) .expect(200, ['key1']); }); + + it('should return 404 when the prefix is valid but contains no keys', async () => { + const prefix = 'empty-prefix'; + const expectedError = { + message: "No valid configurations found", + status: 404, + title: "Not Found" + }; + await request(`${TEST_URL}/api`) + .get(`/configurations?prefix=${prefix}&token=${ADMIN_TEST_TOKEN}`) + .expect(404, expectedError); + }); + + it('should return 404 when the specified prefix does not exist', async () => { + const prefix = 'nonexistent-prefix'; + const expectedError = { + message: `Configurations prefix not found: "${qcPath}/ANY/any/${prefix}"`, + status: 404, + title: "Not Found" + }; + await request(`${TEST_URL}/api`) + .get(`/configurations?prefix=${prefix}&token=${ADMIN_TEST_TOKEN}`) + .expect(404, expectedError); + }); + + it('should return 503 when Consul returns an internal error', async () => { + const prefix = 'server-error-prefix'; + const expectedError = { + message: "Consul service unavailable", + status: 503, + title: "Service Unavailable" + }; + await request(`${TEST_URL}/api`) + .get(`/configurations?prefix=${prefix}&token=${ADMIN_TEST_TOKEN}`) + .expect(503, expectedError); + }); - it('should return unauthorized error for missing token requests', async () => { - await request(`${TEST_URL}/api/configurations`) - .get('/') + it('should return 403 unauthorized error for missing token requests', async () => { + await request(`${TEST_URL}/api`) + .get('/configurations') .expect(403, { error: '403 - Json Web Token Error', message: 'You must provide a JWT token' }); }); - it('should return unauthorized error for invalid token requests', async () => { - await request(`${TEST_URL}/api/configurations`) - .get('/?token=invalid-token') + it('should return 403 unauthorized error for invalid token requests', async () => { + await request(`${TEST_URL}/api`) + .get('/configurations?token=invalid-token') .expect(403, { error: '403 - Json Web Token Error', message: 'Invalid JWT token provided' diff --git a/Control/test/config/testConfigForConsul.js b/Control/test/config/testConfigForConsul.js index 6adfdf127..44572fe04 100644 --- a/Control/test/config/testConfigForConsul.js +++ b/Control/test/config/testConfigForConsul.js @@ -27,6 +27,7 @@ const initializeNockForConsul = () => { .get('/v1/status/leader') .reply(200, 'http://localhost:8550'); + // /configurations nock(CONSUL_URL) .persist() .get(`${KV_PATH}${config.consul.qcPath}/ANY/any?recurse=true`) @@ -40,13 +41,48 @@ const initializeNockForConsul = () => { ModifyIndex: 1 } ])) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}${config.consul.qcPath}/ANY/any/empty-prefix?recurse=true`) + .reply(200, JSON.stringify([ + { + LockIndex: 0, + Key: "empty-prefix", + Flags: 0, + Value: null, + CreateIndex: 1, + ModifyIndex: 1 + } + ])) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}${config.consul.qcPath}/ANY/any/nonexistent-prefix?recurse=true`) + .reply(404) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}${config.consul.qcPath}/ANY/any/server-error-prefix?recurse=true`) + .reply(503) + // /configurations/:key(*) nock(CONSUL_URL) .persist() .get(`${KV_PATH}key1?raw=true`) .reply(200, JSON.stringify({key: "value"})) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}nonexistent?raw=true`) + .reply(404) + + nock(CONSUL_URL) + .persist() + .get(`${KV_PATH}consul-failure?raw=true`) + .reply(503) } module.exports = { initializeNockForConsul -}; \ No newline at end of file +}; diff --git a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js index 458a97480..9d66ace7b 100644 --- a/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js +++ b/Control/test/lib/controllers/mocha-qcConfiguration.controller.test.js @@ -11,7 +11,6 @@ * granted to it by virtue of its status as an Intergovernmental Organization * or submit itself to any jurisdiction. */ -/* eslint-disable max-len */ const assert = require('assert'); const sinon = require('sinon'); @@ -20,100 +19,153 @@ const { QCConfigurationController } = require('../../../lib/controllers/QCConfig const { QCConfigurationService } = require('../../../lib/services/QCConfiguration.service.js'); describe(`'QCConfigurationController' test suite`, () => { - describe(`'getConfigurationsKeys' test suite`, () => { - let qcConfigurationService, qcConfigurationController; - before(() => { - qcConfigurationService = new QCConfigurationService({ - getOnlyRawValuesByKeyPrefix: sinon.stub().resolves({ - "o2/components/qc/ANY/any/dir1": undefined, - "o2/components/qc/ANY/any/dir1/prefix1": '{"key1": "value1", "key2": "value2"}', - "o2/components/qc/ANY/any/prefix1": '{"key1": "value1", "key2": "value2"}', - "o2/components/qc/ANY/any/prefix2": '"key1": "value1"', - }), - }); - - qcConfigurationController = new QCConfigurationController(qcConfigurationService, {consul: {qcPath: 'o2/components/qc'}}); + let qcConfigurationService, qcConfigurationController, req, res, statusStub, jsonStub; + + beforeEach(() => { + qcConfigurationService = new QCConfigurationService({}); + qcConfigurationService.retrieveKeysOfValidConfigurations = sinon.stub(); + qcConfigurationService.retrieveConfigurationByKey = sinon.stub(); + + qcConfigurationController = new QCConfigurationController(qcConfigurationService, { consul: { qcPath: 'o2/components/qc' } }); + + jsonStub = sinon.stub(); + statusStub = sinon.stub().returns({ json: jsonStub }); + res = { status: statusStub }; + req = { query: {}, params: {} }; + }); + + afterEach(() => { + sinon.restore(); + }); + + describe(`'getConfigurationsKeysHandler' test suite`, () => { + it('should return 200 with keys when valid configurations are found (no prefix)', async () => { + const keys = ['o2/components/qc/ANY/any/prefix1', 'o2/components/qc/ANY/any/prefix2']; + qcConfigurationService.retrieveKeysOfValidConfigurations.resolves(keys); + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(200)); + assert.deepStrictEqual(jsonStub.firstCall.args[0], keys); + assert.ok(qcConfigurationService.retrieveKeysOfValidConfigurations.calledWith('o2/components/qc/ANY/any', false)); + }); + + it('should return 200 with keys when a prefix is provided', async () => { + const keys = ['o2/components/qc/ANY/any/dir1/prefix1']; + qcConfigurationService.retrieveKeysOfValidConfigurations.resolves(keys); + req.query.prefix = 'dir1'; + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(200)); + assert.deepStrictEqual(jsonStub.firstCall.args[0], keys); + assert.ok(qcConfigurationService.retrieveKeysOfValidConfigurations.calledWith('o2/components/qc/ANY/any/dir1', false)); + }); + + it('should return 200 with keys when recurse is true', async () => { + const keys = ['o2/components/qc/ANY/any/dir1/prefix1', 'o2/components/qc/ANY/any/prefix2']; + qcConfigurationService.retrieveKeysOfValidConfigurations.resolves(keys); + req.query.recurse = true; + req.query.prefix = 'dir1'; + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(200)); + assert.deepStrictEqual(jsonStub.firstCall.args[0], keys); + assert.ok(qcConfigurationService.retrieveKeysOfValidConfigurations.calledWith('o2/components/qc/ANY/any/dir1', true)); }); - it('should return keys of all valid configurations in main directory', async () => { - const req = { query: { } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationsKeys(req, res); - assert.ok(res.status.calledWith(200)); - assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/prefix1']); + it('should return 404 when service returns an empty array', async () => { + qcConfigurationService.retrieveKeysOfValidConfigurations.resolves([]); + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(404)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'No valid configurations found'); }); - it('should return keys of all valid configurations in prefix directory when prefix is set', async () => { - const req = { query: { prefix: 'dir1' } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationsKeys(req, res); - assert.ok(res.status.calledWith(200)); - assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/dir1/prefix1']); + it('should return 404 when service returns null', async () => { + qcConfigurationService.retrieveKeysOfValidConfigurations.resolves(null); + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(404)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'No valid configurations found'); }); - it('should return keys of all valid configurations when recurse is true', async () => { - const req = { query: { recurse: true } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationsKeys(req, res); - assert.ok(res.status.calledWith(200)); - assert.deepStrictEqual(res.json.firstCall.args[0], ['o2/components/qc/ANY/any/dir1/prefix1', 'o2/components/qc/ANY/any/prefix1']); + it('should return 404 when service throws a "404" error for a non-existent prefix', async () => { + const prefix = 'nonexistent'; + req.query.prefix = prefix; + const expectedPath = `o2/components/qc/ANY/any/${prefix}`; + qcConfigurationService.retrieveKeysOfValidConfigurations.rejects(new Error('Non-2xx status code: 404')); + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(404)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, `Configurations prefix not found: "${expectedPath}"`); }); - it('should return 404 when no configurations are found', async () => { - qcConfigurationController._qcConfigurationService._consulService.getOnlyRawValuesByKeyPrefix.resolves({}); - const req = { query: { } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationsKeys(req, res); - assert.ok(res.status.calledWith(404)); - assert.deepStrictEqual(res.json.firstCall.args[0], { - message: 'No configurations found', - status: 404, - title: 'Not Found', - }); + it('should return 503 when service throws a service unavailable error', async () => { + qcConfigurationService.retrieveKeysOfValidConfigurations.rejects(new Error('Consul not working')); + + await qcConfigurationController.getConfigurationsKeysHandler(req, res); + + assert.ok(statusStub.calledWith(503)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'Consul service unavailable'); }); - }) + }); + + describe(`'getConfigurationByKeyHandler' test suite`, () => { + it('should return 200 with configuration for a valid key', async () => { + const config = { key1: 'value1' }; + const configKey = 'o2/qc/path/config1'; + req.params.key = configKey; + qcConfigurationService.retrieveConfigurationByKey.resolves(config); - describe(`'getConfigurationByKey' test suite`, () => { - let qcConfigurationService, qcConfigurationController; - before(() => { - qcConfigurationService = new QCConfigurationService({ - getOnlyRawValueByKey: sinon.stub().resolves({key1: "value1", key2: "value2"}), - }); + await qcConfigurationController.getConfigurationByKeyHandler(req, res); - qcConfigurationController = new QCConfigurationController(qcConfigurationService, {consul: {qcPath: 'o2/components/qc'}}); + assert.ok(qcConfigurationService.retrieveConfigurationByKey.calledWith(configKey)); + assert.ok(statusStub.calledWith(200)); + assert.deepStrictEqual(jsonStub.firstCall.args[0], config); }); - - it('should return configuration for a valid key', async () => { - const req = { params: { key: 'o2/components/qc/ANY/any/prefix1' } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationByKey(req, res); - assert.ok(res.status.calledWith(200)); - assert.deepStrictEqual(res.json.firstCall.args[0], {key1: 'value1', key2: 'value2'}); + + it('should return 400 if key is missing', async () => { + req.params.key = undefined; + + await qcConfigurationController.getConfigurationByKeyHandler(req, res); + + assert.ok(statusStub.calledWith(400)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'Missing configuration key'); }); - it('should return 400 for missing configuration key', async () => { - const req = { params: {} }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationByKey(req, res); - assert.ok(res.status.calledWith(400)); - assert.deepStrictEqual(res.json.firstCall.args[0], { - message: 'Missing configuration key', - status: 400, - title: 'Invalid Input', - }); + it('should return 400 if key is an empty string', async () => { + req.params.key = ' '; + + await qcConfigurationController.getConfigurationByKeyHandler(req, res); + + assert.ok(statusStub.calledWith(400)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'Missing configuration key'); }); - it('should return 404 for non-existing configuration key', async () => { - qcConfigurationController._qcConfigurationService._consulService.getOnlyRawValueByKey.rejects(new Error("Non-2xx status code")); - const req = { params: { key: 'o2/components/qc/ANY/any/non-existing-key' } }; - const res = { status: sinon.stub().returnsThis(), json: sinon.stub() }; - await qcConfigurationController.getConfigurationByKey(req, res); - assert.ok(res.status.calledWith(404)); - assert.deepStrictEqual(res.json.firstCall.args[0], { - message: 'Configuration not found for key: o2/components/qc/ANY/any/non-existing-key', - status: 404, - title: 'Not Found', - }); + it('should return 404 when service throws a "404" error for a non-existent key', async () => { + const nonExistentKey = 'non-existent-key'; + req.params.key = nonExistentKey; + qcConfigurationService.retrieveConfigurationByKey.rejects(new Error('Non-2xx status code: 404')); + + await qcConfigurationController.getConfigurationByKeyHandler(req, res); + + assert.ok(statusStub.calledWith(404)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, `Configuration not found for key: ${nonExistentKey}`); + }); + + it('should return 503 when service throws a service unavailable error', async () => { + req.params.key = 'some-key'; + qcConfigurationService.retrieveConfigurationByKey.rejects(new Error('Consul not working')); + + await qcConfigurationController.getConfigurationByKeyHandler(req, res); + + assert.ok(statusStub.calledWith(503)); + assert.deepStrictEqual(jsonStub.firstCall.args[0].message, 'Consul service unavailable'); }); }); }); diff --git a/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js b/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js index de4eaf051..1b2ddc542 100644 --- a/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js +++ b/Control/test/lib/middleware/mocha-validateConsulServiceMiddlewareFactory.test.js @@ -44,14 +44,14 @@ describe('`validateConsulServiceMiddlewareFactory` test suite', () => { assert.ok(resMock.json.notCalled); }); - it('should return 502 if there consulService is not available', async () => { + it('should return 503 if there consulService is not available', async () => { consulService = null; await validateConsulServiceMiddlewareFactory(consulService)(reqMock, resMock, nextMock); - assert.ok(resMock.status.calledOnceWith(502)); - assert.ok(resMock.json.calledOnceWith({ - message: 'Unable to retrieve configuration of consul service', - })); + assert.ok(resMock.status.calledOnceWith(503)); + assert.ok(resMock.json.calledOnceWith( + sinon.match({ message: "Consul service is not available" }) + )); assert.ok(nextMock.notCalled); }); }); diff --git a/Control/test/lib/services/mocha-qcConfiguration.service.test.js b/Control/test/lib/services/mocha-qcConfiguration.service.test.js index 507d0a48f..a1191376c 100644 --- a/Control/test/lib/services/mocha-qcConfiguration.service.test.js +++ b/Control/test/lib/services/mocha-qcConfiguration.service.test.js @@ -19,80 +19,97 @@ const sinon = require("sinon"); const { QCConfigurationService } = require("../../../lib/services/QCConfiguration.service.js"); describe(`'QCConfigurationService' test suite`, () => { + let consulServiceStub, qcConfigurationService; + + beforeEach(() => { + consulServiceStub = { + getOnlyRawValuesByKeyPrefix: sinon.stub(), + getOnlyRawValueByKey: sinon.stub(), + }; + qcConfigurationService = new QCConfigurationService(consulServiceStub); + }); - describe(`'getKeysOfValidConfigurations' test suite`, () => { - let qcConfigurationService; - before(() => { - qcConfigurationService = new QCConfigurationService({ - getOnlyRawValuesByKeyPrefix: sinon.stub().resolves({ - "any/dir1": undefined, - "any/dir1/prefix1": '{"key1": "value1", "key2": "value2"}', - "any/prefix1": '{"key1": "value1", "key2": "value2"}', - "any/prefix2": '"key1": "value1"', - }), - }); - }); + afterEach(() => { + sinon.restore(); + }); - it("should return keys of all valid configurations in main directory", async () => { + describe(`'retrieveKeysOfValidConfigurations' test suite`, () => { + it("should return keys of valid configurations only in the root of the prefix (recurse=false)", async () => { const prefix = "any"; - const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); - assert.deepStrictEqual(configurations, ["any/prefix1"]); - }); + const rawData = { + "any/dir1/nested": '{"key": "value"}', + "any/valid1": '{"key1": "value1"}', + "any/invalid_json": '"key1": "value1"', + "any/valid2": '{}', + }; + consulServiceStub.getOnlyRawValuesByKeyPrefix.resolves(rawData); + + const configurations = await qcConfigurationService.retrieveKeysOfValidConfigurations(prefix, false); - it("should return keys of all valid configurations in prefix directory when prefix is set", async () => { - const prefix = "any/dir1"; - const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); - assert.deepStrictEqual(configurations, ["any/dir1/prefix1"]); + assert.ok(consulServiceStub.getOnlyRawValuesByKeyPrefix.calledOnceWith(prefix)); + assert.deepStrictEqual(configurations, ["any/valid1", "any/valid2"]); }); it("should return keys of all valid configurations when recurse is true", async () => { const prefix = "any"; - const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix, true); - assert.deepStrictEqual(configurations, ["any/dir1/prefix1", "any/prefix1"]); + const rawData = { + "any/dir1/nested": '{"key": "value"}', + "any/valid1": '{"key1": "value1"}', + "any/dir1/invalid": 'just string', + }; + consulServiceStub.getOnlyRawValuesByKeyPrefix.resolves(rawData); + + const configurations = await qcConfigurationService.retrieveKeysOfValidConfigurations(prefix, true); + + assert.ok(consulServiceStub.getOnlyRawValuesByKeyPrefix.calledOnceWith(prefix)); + assert.deepStrictEqual(configurations, ["any/dir1/nested", "any/valid1"]); }); - it("should return an empty array when no valid configurations are found", async () => { - qcConfigurationService._consulService.getOnlyRawValuesByKeyPrefix.resolves({}); + it("should return an empty array when consul service returns no data", async () => { const prefix = "nonexistent"; - const configurations = await qcConfigurationService.getKeysOfValidConfigurations(prefix); + consulServiceStub.getOnlyRawValuesByKeyPrefix.resolves({}); + + const configurations = await qcConfigurationService.retrieveKeysOfValidConfigurations(prefix); + + assert.ok(consulServiceStub.getOnlyRawValuesByKeyPrefix.calledOnceWith(prefix)); assert.deepStrictEqual(configurations, []); }); - }); - describe(`'getConfigurationByKey' test suite`, () => { - let qcConfigurationService; - before(() => { - qcConfigurationService = new QCConfigurationService({ - getOnlyRawValueByKey: sinon.stub().resolves({key1: "value1", key2: "value2"}), - }); + it("should propagate errors from the consul service", async () => { + const testError = new Error("Consul not working"); + consulServiceStub.getOnlyRawValuesByKeyPrefix.rejects(testError); + + await assert.rejects( + async () => await qcConfigurationService.retrieveKeysOfValidConfigurations("any"), + testError + ); }); + }); + describe(`'retrieveConfigurationByKey' test suite`, () => { it("should return configuration for a valid key", async () => { const key = "any/prefix1"; - const configuration = await qcConfigurationService.getConfigurationByKey(key); - assert.deepStrictEqual(configuration, {key1: "value1", key2: "value2"}); - }); + const expectedConfig = { key1: "value1", key2: "value2" }; + consulServiceStub.getOnlyRawValueByKey.resolves(expectedConfig); + + const configuration = await qcConfigurationService.retrieveConfigurationByKey(key); - it("should throw NotFoundError for an invalid key", async () => { - qcConfigurationService._consulService.getOnlyRawValueByKey.rejects(new Error("Not found")); - const key = "invalid/key"; - try { - await qcConfigurationService.getConfigurationByKey(key); - assert.fail("Expected NotFoundError to be thrown"); - } catch (error) { - assert.strictEqual(error.message, `Configuration not found for key: ${key}`); - } + assert.ok(consulServiceStub.getOnlyRawValueByKey.calledOnceWith(key)); + assert.deepStrictEqual(configuration, expectedConfig); + }); + + it("should propagate errors from the consul service", async () => { + const testError = new Error("Consul not working"); + consulServiceStub.getOnlyRawValueByKey.rejects(testError); + + await assert.rejects( + async () => await qcConfigurationService.retrieveConfigurationByKey("any"), + testError + ); }); }); describe('`filterConfigurations` test suite', () => { - let qcConfigurationService; - - before(() => { - // This setup runs once before all tests in this suite - qcConfigurationService = new QCConfigurationService({}); - }); - it('should return keys of valid JSON objects and ignore others when recurse is false', () => { const configs = { 'any/valid_object': '{"key": "value"}', @@ -104,10 +121,9 @@ describe(`'QCConfigurationService' test suite`, () => { 'any/json_array': '[1, 2, 3]', 'any/json_null': 'null', }; - const expectedKeys = ['any/valid_object', 'any/empty_object']; + const result = qcConfigurationService.filterConfigurations(configs, false, 'any'); - assert.deepStrictEqual(result, expectedKeys); }); @@ -118,31 +134,15 @@ describe(`'QCConfigurationService' test suite`, () => { 'any/nested/invalid': 'not json', }; const expectedKeys = ['any/valid_object', 'any/nested/key']; - const result = qcConfigurationService.filterConfigurations(configs, true, 'any'); + const result = qcConfigurationService.filterConfigurations(configs, true, 'any'); assert.deepStrictEqual(result, expectedKeys); }); - it('should return an empty array when no valid JSON objects are found', () => { - const configs = { - 'any/invalid1': 'not a json', - 'any/invalid2': '{"a":1,', - 'any/valid_but_string': '"hello"', - }; - - const result = qcConfigurationService.filterConfigurations(configs, false, 'any'); - - assert.deepStrictEqual(result, []); - }); - it('should return an empty array for null, undefined and empty object input', () => { - const resultForNull = qcConfigurationService.filterConfigurations(null, false, 'any'); - const resultForUndefined = qcConfigurationService.filterConfigurations(undefined, false, 'any'); - const resultForEmpty = qcConfigurationService.filterConfigurations({}, false, 'any'); - - assert.deepStrictEqual(resultForNull, []); - assert.deepStrictEqual(resultForUndefined, []); - assert.deepStrictEqual(resultForEmpty, []); + assert.deepStrictEqual(qcConfigurationService.filterConfigurations(null, false, 'any'), []); + assert.deepStrictEqual(qcConfigurationService.filterConfigurations(undefined, false, 'any'), []); + assert.deepStrictEqual(qcConfigurationService.filterConfigurations({}, false, 'any'), []); }); }); });