From fc638151929dba80214f5377edf4bfb65c76ba19 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 14:17:00 +0200 Subject: [PATCH 01/18] add findOne method to Repository --- app/lib/database/repositories/Repository.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index e442c2aa6..2e08789db 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -31,12 +31,22 @@ class Repository { /** * Returns all entities. * - * @param {Object} queryClauses the find query (see sequelize findAll options) or a find query builder + * @param {Object} queryClauses the find query (see sequelize findAll options) * @returns {Promise} Promise object representing the full mock data */ async findAll(queryClauses = {}) { return this.model.findAll(queryClauses); } + + /** + * Returns one entity. + * + * @param {Object} queryClauses the find query (see sequelize findOne options) + * @returns {Promise} Promise object representing the full mock data + */ + async findOne(queryClauses = {}) { + return this.model.findOne(queryClauses); + } } module.exports = Repository; From b38332c442dad061abf59d82c941fd74a9f6d5c9 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 14:26:15 +0200 Subject: [PATCH 02/18] add method to RunService updating quality --- app/lib/services/runs/RunService.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index 3e98a8b80..07bfc321e 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -15,6 +15,7 @@ const { databaseManager: { repositories: { RunRepository, + RunDetectorsRepository, }, models: { DataPass, @@ -107,6 +108,17 @@ class RunService { }); return runs.map((run) => runAdapter.toEntity(run)); } + + async updateRunDetectorQuality(runNumber, detectorId, newQuality) { + const runDetector = RunDetectorsRepository.findOne({ + where: { + run_number: runNumber, + detector_id: detectorId, + }, + }); + await runDetector.update({ quality: newQuality }); + await runDetector.save(); + } } module.exports = { From 675bca2204fb8599c1fa2cf3e13b9765a64db363 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 14:37:54 +0200 Subject: [PATCH 03/18] add controller method --- app/lib/server/controllers/run.controller.js | 26 ++++++++++++++++++++ app/lib/services/runs/RunService.js | 2 ++ 2 files changed, 28 insertions(+) diff --git a/app/lib/server/controllers/run.controller.js b/app/lib/server/controllers/run.controller.js index 7b2f7f6e9..35b03b025 100644 --- a/app/lib/server/controllers/run.controller.js +++ b/app/lib/server/controllers/run.controller.js @@ -87,9 +87,35 @@ const listRunsPerSimulationPassHandler = async (req, res, next) => { } }; +const updateRunDetectorQualityHandler = async (req, res) => { + const customDTO = stdDataRequestDTO.keys({ + params: { + runNumber: Joi.number(), + detectorSubsystemId: Joi.number(), + }, + query: { + quality: Joi.string(), + }, + }); + + const validatedDTO = await validateDtoOrRepondOnFailure(customDTO, req, res); + if (validatedDTO) { + const runDetector = await runService.updateRunDetectorQuality( + validatedDTO.params.runNumber, + validatedDTO.params.detectorId, + validatedDTO.query.quality, + ); + + res.json({ + data: runDetector, + }); + } +}; + module.exports = { listRunsHandler, listRunsPerPeriodHandler, listRunsPerDataPass, listRunsPerSimulationPassHandler, + updateRunDetectorQualityHandler, }; diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index 07bfc321e..8647c7ce4 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -118,6 +118,8 @@ class RunService { }); await runDetector.update({ quality: newQuality }); await runDetector.save(); + + return runDetector; } } From d0983816a2568c1311a4667498f8c0a4a589b4e6 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 15:51:25 +0200 Subject: [PATCH 04/18] add orking endpoint for updating run/detectorSubsystem quality --- app/lib/server/controllers/run.controller.js | 11 ++++------- app/lib/server/routers/run.router.js | 6 ++++++ app/lib/services/runs/RunService.js | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/lib/server/controllers/run.controller.js b/app/lib/server/controllers/run.controller.js index 35b03b025..6f9f540cc 100644 --- a/app/lib/server/controllers/run.controller.js +++ b/app/lib/server/controllers/run.controller.js @@ -94,21 +94,18 @@ const updateRunDetectorQualityHandler = async (req, res) => { detectorSubsystemId: Joi.number(), }, query: { - quality: Joi.string(), + quality: Joi.string().required(), }, }); const validatedDTO = await validateDtoOrRepondOnFailure(customDTO, req, res); if (validatedDTO) { - const runDetector = await runService.updateRunDetectorQuality( + await runService.updateRunDetectorQuality( validatedDTO.params.runNumber, - validatedDTO.params.detectorId, + validatedDTO.params.detectorSubsystemId, validatedDTO.query.quality, ); - - res.json({ - data: runDetector, - }); + res.end(); } }; diff --git a/app/lib/server/routers/run.router.js b/app/lib/server/routers/run.router.js index 6ebac9672..ff672be87 100644 --- a/app/lib/server/routers/run.router.js +++ b/app/lib/server/routers/run.router.js @@ -22,5 +22,11 @@ module.exports = { controller: RunController.listRunsHandler, description: 'List all runs which are present in DB', }, + { + method: 'patch', + path: '/:runNumber/detector-subsystems/:detectorSubsystemId', + controller: RunController.updateRunDetectorQualityHandler, + description: 'Update run/detectorSubsystem based quality', + }, ], }; diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index 8647c7ce4..1e9c57153 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -110,7 +110,7 @@ class RunService { } async updateRunDetectorQuality(runNumber, detectorId, newQuality) { - const runDetector = RunDetectorsRepository.findOne({ + const runDetector = await RunDetectorsRepository.findOne({ where: { run_number: runNumber, detector_id: detectorId, From 5fd30bc5f7e76d7669dbd97e4da5f254ea0faa84 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 15:53:28 +0200 Subject: [PATCH 05/18] update endpoint tests --- test/lib/server/routes.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/server/routes.test.js b/test/lib/server/routes.test.js index b6b3427f9..0f542f974 100644 --- a/test/lib/server/routes.test.js +++ b/test/lib/server/routes.test.js @@ -32,7 +32,7 @@ module.exports = () => { }); }); describe('Endpoints', () => { - routes.map(async ({ path }) => { + routes.filter(({ method }) => method === 'get').map(async ({ path }) => { const url = `${config.http.tls ? 'https' : 'http'}://localhost:${config.http.port}/api${replaceAll(path, /:[^/]+/, '0')}`; it(`should fetch from ${path} <${url}> without errors`, async () => { await assert.doesNotReject(makeHttpRequestForJSON(url)); From ce2824666223ba005d806961bac0a7c690a8b4f2 Mon Sep 17 00:00:00 2001 From: xsalonx Date: Wed, 9 Aug 2023 17:35:10 +0200 Subject: [PATCH 06/18] refactor, add error definition --- app/lib/server/errors.js | 19 +++++++++++++++++++ app/lib/services/runs/RunService.js | 2 -- 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 app/lib/server/errors.js diff --git a/app/lib/server/errors.js b/app/lib/server/errors.js new file mode 100644 index 000000000..271e6a7ad --- /dev/null +++ b/app/lib/server/errors.js @@ -0,0 +1,19 @@ +/** + * @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. + */ + +class NotFoundError extends Error {} + +module.exports = { + NotFoundError, +}; diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index 1e9c57153..bb0e130ac 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -118,8 +118,6 @@ class RunService { }); await runDetector.update({ quality: newQuality }); await runDetector.save(); - - return runDetector; } } From b495ab7b84401eacf9f6024fce6575e8974a0daa Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 10:00:49 +0200 Subject: [PATCH 07/18] cleanup --- app/lib/services/runs/RunService.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index bb0e130ac..ca9cd4d52 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -117,7 +117,6 @@ class RunService { }, }); await runDetector.update({ quality: newQuality }); - await runDetector.save(); } } From b6d1b6321ea25c4faa8b4b73ab6f48219d206636 Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 11:13:33 +0200 Subject: [PATCH 08/18] add support for transaction to Repository --- app/lib/database/repositories/Repository.js | 31 +++++++++++++++++++++ app/lib/database/repositories/index.js | 1 + 2 files changed, 32 insertions(+) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 2e08789db..612410370 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -11,6 +11,20 @@ * or submit itself to any jurisdiction. */ +const getTransactionalMethods = (classObj) => { + const classesStack = []; + while (typeof Object.getPrototypeOf(classObj) !== 'object') { + classesStack.push(classObj); + classObj = Object.getPrototypeOf(classObj); + } + + const nonSequelizeFunctions = new Set(['constructor', 'asT', '_asT']) + return classesStack + .map(cl => Object.getOwnPropertyNames(cl.prototype)) + .flat() + .filter(n => ! nonSequelizeFunctions.has(n)); +} + /** * Sequelize implementation of the Repository. */ @@ -47,6 +61,23 @@ class Repository { async findOne(queryClauses = {}) { return this.model.findOne(queryClauses); } + + _asT() { + const { sequelize } = this.model; + getTransactionalMethods(this.constructor).forEach(transactionalMethod => { + const nonTransationBoundMethod = this[transactionalMethod].bind(this); + this[transactionalMethod] = async (...args) => + sequelize.transaction(async (t) => { + return await nonTransationBoundMethod(...args); + }); + }); + } + + asT() { + const instanceWithTransactions = new this.constructor(this.model); + instanceWithTransactions._asT(); + return instanceWithTransactions; + } } module.exports = Repository; diff --git a/app/lib/database/repositories/index.js b/app/lib/database/repositories/index.js index 86d2ff252..76eb68dea 100644 --- a/app/lib/database/repositories/index.js +++ b/app/lib/database/repositories/index.js @@ -51,6 +51,7 @@ const repositoriesFactory = (models) => { [modelName + 'Repository', new (specificallyDefinedRepositories[modelName] ?? Repository) (model), ]); + modelNameToRepository.forEach(([_, repository]) => { repository.T = repository.asT() }); return Object.fromEntries(modelNameToRepository); }; From 5abefe959e8d5393f16be0e83c82df33e6322020 Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 11:21:08 +0200 Subject: [PATCH 09/18] refactor --- app/lib/database/repositories/Repository.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 612410370..cb8151aa2 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -11,18 +11,19 @@ * or submit itself to any jurisdiction. */ -const getTransactionalMethods = (classObj) => { +const nonTransactionalFunctions = new Set(['constructor', 'asT', '_asT']) + +const getTransactionalMethodsNames = (classObj, ) => { const classesStack = []; while (typeof Object.getPrototypeOf(classObj) !== 'object') { classesStack.push(classObj); classObj = Object.getPrototypeOf(classObj); } - const nonSequelizeFunctions = new Set(['constructor', 'asT', '_asT']) return classesStack .map(cl => Object.getOwnPropertyNames(cl.prototype)) .flat() - .filter(n => ! nonSequelizeFunctions.has(n)); + .filter(name => ! nonTransactionalFunctions.has(name)); } /** @@ -64,15 +65,16 @@ class Repository { _asT() { const { sequelize } = this.model; - getTransactionalMethods(this.constructor).forEach(transactionalMethod => { - const nonTransationBoundMethod = this[transactionalMethod].bind(this); - this[transactionalMethod] = async (...args) => - sequelize.transaction(async (t) => { - return await nonTransationBoundMethod(...args); + getTransactionalMethodsNames(this.constructor).forEach(transactionalMethodName => { + const boundMethodWithoutTransaction = this[transactionalMethodName].bind(this); + this[transactionalMethodName] = async (...args) => + sequelize.transaction(async () => { + return await boundMethodWithoutTransaction(...args); }); }); } + asT() { const instanceWithTransactions = new this.constructor(this.model); instanceWithTransactions._asT(); From a1d31298ed4741668c2519a0052be2caa776e1eb Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 11:57:44 +0200 Subject: [PATCH 10/18] docs --- app/lib/database/repositories/Repository.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index cb8151aa2..85b339bcc 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -74,7 +74,11 @@ class Repository { }); } - + /** + * Create repository copy which all business related methods are wrapped with sequelize.transcation(), e.g. + * Repository.asT().findAll() is equal to sequelize.transaction(() => Repository.findAll()) + * @returns {Repository} + */ asT() { const instanceWithTransactions = new this.constructor(this.model); instanceWithTransactions._asT(); From 1c69dc723ec0bdfa2ef2813f0e7de75cba18ab05 Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 12:22:26 +0200 Subject: [PATCH 11/18] add module cls-hooked for sequelize transactions handling --- app/lib/database/DatabaseManager.js | 3 ++ app/lib/database/repositories/Repository.js | 2 +- package-lock.json | 58 +++++++++++++++++++++ package.json | 2 + 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/app/lib/database/DatabaseManager.js b/app/lib/database/DatabaseManager.js index 34dcae6fa..7d62db52f 100644 --- a/app/lib/database/DatabaseManager.js +++ b/app/lib/database/DatabaseManager.js @@ -18,6 +18,7 @@ const path = require('path') const config = require('../../config'); const modelsFactory = require('./models'); const repositoriesFactory = require('./repositories'); +const cls = require('cls-hooked'); /** * Sequelize implementation of the Database. @@ -26,6 +27,8 @@ class DatabaseManager { constructor() { this.logger = new Log(DatabaseManager.name); this.schema = 'public'; + const o2rct_namespace = cls.createNamespace('o2rct-namespace'); + Sequelize.useCLS(o2rct_namespace); this.sequelize = new Sequelize({ ...config.database, diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 85b339bcc..d9a6b469d 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -68,7 +68,7 @@ class Repository { getTransactionalMethodsNames(this.constructor).forEach(transactionalMethodName => { const boundMethodWithoutTransaction = this[transactionalMethodName].bind(this); this[transactionalMethodName] = async (...args) => - sequelize.transaction(async () => { + sequelize.transaction(async (t) => { return await boundMethodWithoutTransaction(...args); }); }); diff --git a/package-lock.json b/package-lock.json index 6dedd6f76..3c7210cfb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "0.1.9", "bundleDependencies": [ "@aliceo2/web-ui", + "cls-hooked", "deepmerge", "esm", "joi", @@ -20,6 +21,7 @@ "license": "GPL-3.0", "dependencies": { "@aliceo2/web-ui": "^2.0.0", + "cls-hooked": "^4.2.2", "csvtojson": "^2.0.10", "deepmerge": "^4.3.1", "esm": "^3.2.25", @@ -1184,6 +1186,18 @@ "integrity": "sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ==", "inBundle": true }, + "node_modules/async-hook-jl": { + "version": "1.7.6", + "resolved": "https://registry.npmjs.org/async-hook-jl/-/async-hook-jl-1.7.6.tgz", + "integrity": "sha512-gFaHkFfSxTjvoxDMYqDuGHlcRyUuamF8s+ZTtJdDzqjws4mCt7v0vuV79/E2Wr2/riMQgtG4/yUtXWs1gZ7JMg==", + "inBundle": true, + "dependencies": { + "stack-chain": "^1.3.7" + }, + "engines": { + "node": "^4.7 || >=6.9 || >=7.3" + } + }, "node_modules/asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -1561,6 +1575,29 @@ "node": ">=12" } }, + "node_modules/cls-hooked": { + "version": "4.2.2", + "resolved": "https://registry.npmjs.org/cls-hooked/-/cls-hooked-4.2.2.tgz", + "integrity": "sha512-J4Xj5f5wq/4jAvcdgoGsL3G103BtWpZrMo8NEinRltN+xpTZdI+M38pyQqhuFU/P792xkMFvnKSf+Lm81U1bxw==", + "inBundle": true, + "dependencies": { + "async-hook-jl": "^1.7.6", + "emitter-listener": "^1.0.1", + "semver": "^5.4.1" + }, + "engines": { + "node": "^4.7 || >=6.9 || >=7.3 || >=8.2.1" + } + }, + "node_modules/cls-hooked/node_modules/semver": { + "version": "5.7.2", + "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.2.tgz", + "integrity": "sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g==", + "inBundle": true, + "bin": { + "semver": "bin/semver" + } + }, "node_modules/color": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/color/-/color-3.2.1.tgz", @@ -2013,6 +2050,15 @@ "integrity": "sha512-XbCRs/34l31np/p33m+5tdBrdXu9jJkZxSbNxj5I0H1KtV2ZMSB+i/HYqDiRzHaFx2T5EdytjoBRe8QRJE2vQg==", "dev": true }, + "node_modules/emitter-listener": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/emitter-listener/-/emitter-listener-1.1.2.tgz", + "integrity": "sha512-Bt1sBAGFHY9DKY+4/2cV6izcKJUf5T7/gkdmkxzX/qv9CcGH8xSwVRW5mtX03SWJtRTWSOpzCuWN9rBFYZepZQ==", + "inBundle": true, + "dependencies": { + "shimmer": "^1.2.0" + } + }, "node_modules/emittery": { "version": "0.13.1", "resolved": "https://registry.npmjs.org/emittery/-/emittery-0.13.1.tgz", @@ -5528,6 +5574,12 @@ "node": ">=8" } }, + "node_modules/shimmer": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/shimmer/-/shimmer-1.2.1.tgz", + "integrity": "sha512-sQTKC1Re/rM6XyFM6fIAGHRPVGvyXfgzIDvzoq608vM+jeyVD0Tu1E6Np0Kc2zAIFWIj963V2800iF/9LPieQw==", + "inBundle": true + }, "node_modules/side-channel": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/side-channel/-/side-channel-1.0.4.tgz", @@ -5715,6 +5767,12 @@ "node": ">= 0.6" } }, + "node_modules/stack-chain": { + "version": "1.3.7", + "resolved": "https://registry.npmjs.org/stack-chain/-/stack-chain-1.3.7.tgz", + "integrity": "sha512-D8cWtWVdIe/jBA7v5p5Hwl5yOSOrmZPWDPe2KxQ5UAGD+nxbxU0lKXA4h85Ta6+qgdKVL3vUxsbIZjc1kBG7ug==", + "inBundle": true + }, "node_modules/stack-trace": { "version": "0.0.10", "resolved": "https://registry.npmjs.org/stack-trace/-/stack-trace-0.0.10.tgz", diff --git a/package.json b/package.json index e6c02860c..babe89909 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "homepage": "https://github.com/AliceO2Group/RunConditionTable#readme", "dependencies": { "@aliceo2/web-ui": "^2.0.0", + "cls-hooked": "^4.2.2", "csvtojson": "^2.0.10", "deepmerge": "^4.3.1", "esm": "^3.2.25", @@ -88,6 +89,7 @@ ], "bundleDependencies": [ "@aliceo2/web-ui", + "cls-hooked", "deepmerge", "esm", "joi", From 049ccaa29e67770c32fd5813292947da03d81404 Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 12:31:28 +0200 Subject: [PATCH 12/18] docs --- app/lib/database/repositories/Repository.js | 5 +++-- app/lib/database/repositories/index.js | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index d9a6b469d..6aebc1faf 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -75,8 +75,9 @@ class Repository { } /** - * Create repository copy which all business related methods are wrapped with sequelize.transcation(), e.g. - * Repository.asT().findAll() is equal to sequelize.transaction(() => Repository.findAll()) + * Create copy of repository object which all business related methods are wrapped with sequelize.transcation(), + * e.g: Repository.asT().findAll() is equal to sequelize.transaction((t) => Repository.findAll()) + * Module cls-hooked handles passing transaction object to sequelize queries automatically. * @returns {Repository} */ asT() { diff --git a/app/lib/database/repositories/index.js b/app/lib/database/repositories/index.js index 76eb68dea..0aafe5807 100644 --- a/app/lib/database/repositories/index.js +++ b/app/lib/database/repositories/index.js @@ -41,6 +41,7 @@ const validateSpecificRepositoriesConfiguration = (models) => { /** * Instantiate sequelize models repositories according to repositiry pattern. + * Each Repository Object has transactional version of itself under field 'T' @see {Repository.asT}. Those versions use global sequelize options for transactions. * @param {Object} models dict: modelName -> sequelize model, @see specificallyDefinedRepositories * @returns {Object} dict: repositoryName -> repository instance per one model, (repositoryName = modelName + 'Repository') */ From 22634e39b9a98cdb4714b6ad9d6dc4d6ff459e5c Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:07:41 +0200 Subject: [PATCH 13/18] add checked error, docs refactor --- app/lib/database/repositories/Repository.js | 28 +++++++++++++++++++++ app/lib/services/runs/RunService.js | 7 +++--- app/lib/{server => utils}/errors.js | 4 +-- 3 files changed, 34 insertions(+), 5 deletions(-) rename app/lib/{server => utils}/errors.js (89%) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 6aebc1faf..2546c1cbe 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -12,6 +12,8 @@ */ const nonTransactionalFunctions = new Set(['constructor', 'asT', '_asT']) +const { throwWrapper } = require('../../utils'); +const { NotFoundEntityError } = require('../../server/errors'); const getTransactionalMethodsNames = (classObj, ) => { const classesStack = []; @@ -63,6 +65,32 @@ class Repository { return this.model.findOne(queryClauses); } + /** + * Apply a patch on a given dbObject and save the dbObject to the database + * + * @param {Object} dbOject the database object on which to apply the patch + * @param {Object} patch the patch to apply + * @return {Promise} promise that resolves when the patch has been applied + */ + async updateOne(dbOject, patch) { + return dbOject.update(patch); + } + + /** + * Find a dbObject using query clause, apply given patch to it and save the dbObject to the database + * + * @param {Object} dbOject the database object on which to apply the patch + * @param {Object} patch the patch to apply + * @throws {NotFoundEntityError} if cannot find dbObject with given query clause + * @return {Promise} promise that resolves when the patch has been applied + */ + async findOneAndUpdate(query, patch) { + ( + await this.model.findOne(query) ?? + throwWrapper(new NotFoundEntityError(`No entity of model ${this.model.name} for clause ${JSON.stringify(query)}`)) + ).update(patch); + } + _asT() { const { sequelize } = this.model; getTransactionalMethodsNames(this.constructor).forEach(transactionalMethodName => { diff --git a/app/lib/services/runs/RunService.js b/app/lib/services/runs/RunService.js index ca9cd4d52..5eae3ead3 100644 --- a/app/lib/services/runs/RunService.js +++ b/app/lib/services/runs/RunService.js @@ -110,13 +110,14 @@ class RunService { } async updateRunDetectorQuality(runNumber, detectorId, newQuality) { - const runDetector = await RunDetectorsRepository.findOne({ + const queryClause = { where: { run_number: runNumber, detector_id: detectorId, }, - }); - await runDetector.update({ quality: newQuality }); + }; + const patch = { quality: newQuality }; + await RunDetectorsRepository.T.findOneAndUpdate(queryClause, patch); } } diff --git a/app/lib/server/errors.js b/app/lib/utils/errors.js similarity index 89% rename from app/lib/server/errors.js rename to app/lib/utils/errors.js index 271e6a7ad..571d74f8f 100644 --- a/app/lib/server/errors.js +++ b/app/lib/utils/errors.js @@ -12,8 +12,8 @@ * or submit itself to any jurisdiction. */ -class NotFoundError extends Error {} +class NotFoundEntityError extends Error {} module.exports = { - NotFoundError, + NotFoundEntityError, }; From c1768f8bc34f7ce7bd270d9cf97eb70e412e5e92 Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:12:55 +0200 Subject: [PATCH 14/18] refactor --- app/lib/database/repositories/Repository.js | 4 ++-- app/lib/utils/index.js | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 2546c1cbe..2a3c34157 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -11,9 +11,9 @@ * or submit itself to any jurisdiction. */ +const { throwWrapper, NotFoundEntityError } = require('../../utils'); + const nonTransactionalFunctions = new Set(['constructor', 'asT', '_asT']) -const { throwWrapper } = require('../../utils'); -const { NotFoundEntityError } = require('../../server/errors'); const getTransactionalMethodsNames = (classObj, ) => { const classesStack = []; diff --git a/app/lib/utils/index.js b/app/lib/utils/index.js index af25b184d..9d34bbb03 100644 --- a/app/lib/utils/index.js +++ b/app/lib/utils/index.js @@ -16,6 +16,7 @@ const LogsStacker = require('./LogsStacker.js'); const objUtils = require('./obj-utils.js'); const ResProvider = require('./ResProvider.js'); const sqlUtils = require('./sql-utils.js'); +const errors = require('./errors.js'); module.exports = { ResProvider, @@ -23,4 +24,5 @@ module.exports = { ...sqlUtils, ...httpUtils, ...objUtils, + ...errors, }; From 33e71d72515c376d14675669e18520bba5d2e3be Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:21:45 +0200 Subject: [PATCH 15/18] refactor due to strange behaviour of cls-hooked --- app/lib/database/repositories/Repository.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index 2a3c34157..b2c4c36a8 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -85,10 +85,9 @@ class Repository { * @return {Promise} promise that resolves when the patch has been applied */ async findOneAndUpdate(query, patch) { - ( - await this.model.findOne(query) ?? - throwWrapper(new NotFoundEntityError(`No entity of model ${this.model.name} for clause ${JSON.stringify(query)}`)) - ).update(patch); + const entity = await this.model.findOne(query) ?? + throwWrapper(new NotFoundEntityError(`No entity of model ${this.model.name} for clause ${JSON.stringify(query)}`)); + await entity.update(patch); } _asT() { From 87868c6f3f4f96d31b7e31580e6b972983e1104b Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:24:15 +0200 Subject: [PATCH 16/18] add customOptions --- app/lib/database/repositories/Repository.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/lib/database/repositories/Repository.js b/app/lib/database/repositories/Repository.js index b2c4c36a8..2ab7433e6 100644 --- a/app/lib/database/repositories/Repository.js +++ b/app/lib/database/repositories/Repository.js @@ -90,12 +90,12 @@ class Repository { await entity.update(patch); } - _asT() { + _asT(customOptions) { const { sequelize } = this.model; getTransactionalMethodsNames(this.constructor).forEach(transactionalMethodName => { const boundMethodWithoutTransaction = this[transactionalMethodName].bind(this); this[transactionalMethodName] = async (...args) => - sequelize.transaction(async (t) => { + sequelize.transaction(customOptions, async (t) => { return await boundMethodWithoutTransaction(...args); }); }); @@ -105,11 +105,12 @@ class Repository { * Create copy of repository object which all business related methods are wrapped with sequelize.transcation(), * e.g: Repository.asT().findAll() is equal to sequelize.transaction((t) => Repository.findAll()) * Module cls-hooked handles passing transaction object to sequelize queries automatically. + * @property {Object} customOptions - options passed to sequelize.transaction(options, callback) * @returns {Repository} */ - asT() { + asT(customOptions) { const instanceWithTransactions = new this.constructor(this.model); - instanceWithTransactions._asT(); + instanceWithTransactions._asT(customOptions); return instanceWithTransactions; } } From 80251ee04e299ec318a997b94bdad00d5b07068c Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:47:33 +0200 Subject: [PATCH 17/18] add transactional repositories tests --- test/database/Repository.test.js | 34 ++++++++++++++++++++++++++++++++ test/database/index.js | 4 +++- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/database/Repository.test.js diff --git a/test/database/Repository.test.js b/test/database/Repository.test.js new file mode 100644 index 000000000..5d9bf3402 --- /dev/null +++ b/test/database/Repository.test.js @@ -0,0 +1,34 @@ +/** + * @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. + */ +const { databaseManager: { repositories } } = require('../../app/lib/database/DatabaseManager.js'); +const assert = require('assert'); + +module.exports = () => { + describe('RepositoriesSuite', () => { + describe('testing if transaction methods give the same result as non-transactional ones', () => { + const testableMethodNames = new Set(['count', 'findAll', 'findOne']); + Object.values(repositories) + .map((repo) => Object.getOwnPropertyNames(repo.T) + .filter(n => testableMethodNames.has(n)) + .map((methodName) => [repo, methodName])) + .flat() + .map(([repo, methodName]) => { + it(`should acquire the same result from transaction method and non-transactional one for repository ${repo.model.name}' and method '${methodName}'`, async () => { + const nonTransactionResult = await repo[methodName](); + const transationResult = await repo.T[methodName](); + assert.deepStrictEqual(nonTransactionResult, transationResult); + }) + }); + }); + }); +}; diff --git a/test/database/index.js b/test/database/index.js index fe9925b21..223615557 100644 --- a/test/database/index.js +++ b/test/database/index.js @@ -13,8 +13,10 @@ const UtilitiesSuite = require('./utilities'); const DatabaseManagerSuite = require('./DatabaseManger.test'); +const RepositoriesSuite = require('./Repository.test'); module.exports = () => { - describe('DatabaseManager', DatabaseManagerSuite); + DatabaseManagerSuite(); describe('Utilities', UtilitiesSuite); + RepositoriesSuite(); }; From 1a286a5e7af1c089a8427967199802d42542ce5b Mon Sep 17 00:00:00 2001 From: xsalonx <65893715+xsalonx@users.noreply.github.com> Date: Thu, 10 Aug 2023 14:40:15 +0200 Subject: [PATCH 18/18] test refactor --- test/database/Repository.test.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/database/Repository.test.js b/test/database/Repository.test.js index 5d9bf3402..7231dbae5 100644 --- a/test/database/Repository.test.js +++ b/test/database/Repository.test.js @@ -17,18 +17,17 @@ module.exports = () => { describe('RepositoriesSuite', () => { describe('testing if transaction methods give the same result as non-transactional ones', () => { const testableMethodNames = new Set(['count', 'findAll', 'findOne']); - Object.values(repositories) - .map((repo) => Object.getOwnPropertyNames(repo.T) - .filter(n => testableMethodNames.has(n)) - .map((methodName) => [repo, methodName])) - .flat() - .map(([repo, methodName]) => { - it(`should acquire the same result from transaction method and non-transactional one for repository ${repo.model.name}' and method '${methodName}'`, async () => { - const nonTransactionResult = await repo[methodName](); - const transationResult = await repo.T[methodName](); - assert.deepStrictEqual(nonTransactionResult, transationResult); - }) - }); + Object.values(repositories).map((repo) => + describe(`${repo.model.name}Repository`, () => Object.getOwnPropertyNames(repo.T) + .filter(n => testableMethodNames.has(n)) + .map((methodName) => { + it(`should acquire the same result from transaction and non-transactional method #${methodName}`, async () => { + const nonTransactionResult = await repo[methodName](); + const transationResult = await repo.T[methodName](); + assert.deepStrictEqual(nonTransactionResult, transationResult); + }) + })) + ); }); }); };