diff --git a/lib/server/kafka/AliEcsSynchronizer.js b/lib/server/kafka/AliEcsSynchronizer.js index 402e374f8a..f0b78303c0 100644 --- a/lib/server/kafka/AliEcsSynchronizer.js +++ b/lib/server/kafka/AliEcsSynchronizer.js @@ -74,17 +74,27 @@ class AliEcsSynchronizer { this.ecsRunConsumer = new AliEcsEventMessagesConsumer(kafkaClient, RUN_CONSUMER_GROUP, RUN_TOPICS); this.ecsRunConsumer.onMessageReceived(async (eventMessage) => { const { timestamp, runEvent: { environmentId, runNumber, state, transition, lastRequestUser } } = eventMessage; - const { externalId: externalUserId } = lastRequestUser ?? {}; + const { externalId: externalUserId, name: userName } = lastRequestUser ?? {}; if (state === 'CONFIGURED' && transition === 'START_ACTIVITY') { runService - .createOrUpdate(runNumber, environmentId, { timeO2Start: timestamp.toNumber() }, { userO2Start: { externalUserId } }) + .createOrUpdate( + runNumber, + environmentId, + { timeO2Start: timestamp.toNumber() }, + { userO2Start: { userIdentifier: { externalUserId }, name: userName } }, + ) .catch((error) => this._logger.errorMessage(`Failed to save run start for ${runNumber}: ${error.message}`)); } if (state === 'RUNNING' && transition === 'STOP_ACTIVITY') { runService - .createOrUpdate(runNumber, environmentId, { timeO2End: timestamp.toNumber() }, { userO2Stop: { externalUserId } }) + .createOrUpdate( + runNumber, + environmentId, + { timeO2End: timestamp.toNumber() }, + { userO2Stop: { userIdentifier: { externalUserId }, name: userName } }, + ) .catch((error) => this._logger.errorMessage(`Failed to save run end for ${runNumber}: ${error.message}`)); } }); diff --git a/lib/server/services/run/RunService.js b/lib/server/services/run/RunService.js index 231082be36..118cefcb27 100644 --- a/lib/server/services/run/RunService.js +++ b/lib/server/services/run/RunService.js @@ -45,6 +45,12 @@ const { flpRoleService } = require('../flp/FlpRoleService.js'); * @property {number} [runId] the id of the run, ignored if runNumber is present */ +/** + * @typedef GetOrCreateUserPayload object used to uniquely identify a user or create one if it do not exists yet + * @property {UserIdentifier} userIdentifier the unique user identifier + * @property {string|null} name the name of the user to create + */ + /** * @typedef RunRelationsToInclude object specifying which run relations should be fetched alongside the run * @property {boolean} [tags] if true, related tags will be fetched alongside the run @@ -207,8 +213,8 @@ class RunService { * @param {string} environmentId the id of the environment containing the run * @param {Partial} [partialRun] optional runs properties to define * @param {Object} [relations] run relations to create/update - * @param {UserIdentifier} [relations.userO2Start] if not null, the identifier of the user that started the run - * @param {UserIdentifier} [relations.userO2Stop] if not null, the identifier of the user that stopped the run + * @param {GetOrCreateUserPayload} [relations.userO2Start] if not null, the identifier of the user that started the run + * @param {GetOrCreateUserPayload} [relations.userO2Stop] if not null, the identifier of the user that stopped the run * @return {Promise} resolves with the run */ createOrUpdate(runNumber, environmentId, partialRun, relations) { @@ -261,8 +267,8 @@ class RunService { * @param {Object} [relations={}] the run's relations * @param {string|null} [relations.runTypeName=null] if not null, the name of the created run's type * @param {string|null} [relations.lhcPeriodName=null] if not null, the name of lhc period for which the run should be assigned - * @param {UserIdentifier|null} [relations.userO2Start=null] if not null, the identifier of the user that started the run - * @param {UserIdentifier|null} [relations.userO2Stop=null] if not null, the identifier of the user that stopped the run + * @param {GetOrCreateUserPayload|null} [relations.userO2Start=null] if not null, the identifier of the user that started the run + * @param {GetOrCreateUserPayload|null} [relations.userO2Stop=null] if not null, the identifier of the user that stopped the run * @return {Promise} resolve with the created run instance */ async create(newRun, relations) { @@ -285,10 +291,10 @@ class RunService { // Update the users for the run if (userO2Start) { - newRun.userIdO2Start = (await getOrCreateUser(userO2Start))?.id; + newRun.userIdO2Start = (await userService.getOrCreate(userO2Start.userIdentifier, userO2Start.name)).id; } if (userO2Stop) { - newRun.userIdO2Stop = (await getOrCreateUser(userO2Stop))?.id; + newRun.userIdO2Stop = (await userService.getOrCreate(userO2Stop.userIdentifier, userO2Stop.name)).id; } const runId = await createRun(runAdapter.toDatabase(newRun), { detectors, runType }); @@ -322,8 +328,8 @@ class RunService { * @param {UserIdentifier} [payload.relations.userIdentifier] if not null, the identifier of the user requesting the run update * @param {{detectorId: number, quality: string}[]} [payload.relations.detectorsQualities] an optional list representing the new quality of * the run's detector (the run must be related to the given detector, the detectors not in this list keep their original quality) - * @param {UserIdentifier|null} [payload.relations.userO2Start=null] if not null, the identifier of the user that started the run - * @param {UserIdentifier|null} [payload.relations.userO2Stop=null] if not null, the identifier of the user that stopped the run + * @param {GetOrCreateUserPayload|null} [payload.relations.userO2Start=null] if not null, the identifier of the user that started the run + * @param {GetOrCreateUserPayload|null} [payload.relations.userO2Stop=null] if not null, the identifier of the user that stopped the run * @param {string} [payload.relations.lhcPeriodName] if not null, the name of lhc period for which the run should be assigned * @return {Promise} resolve with the resulting run */ @@ -354,10 +360,10 @@ class RunService { // Update the users for the run if (userO2Start) { - runPatch.userIdO2Start = (await getOrCreateUser(userO2Start))?.id; + runPatch.userIdO2Start = (await userService.getOrCreate(userO2Start.userIdentifier, userO2Start.name)).id; } if (userO2Stop) { - runPatch.userIdO2Stop = (await getOrCreateUser(userO2Stop))?.id; + runPatch.userIdO2Stop = (await userService.getOrCreate(userO2Stop.userIdentifier, userO2Stop.name)).id; } await dataSource.transaction(async (transaction) => { @@ -625,22 +631,6 @@ const updateEorReasonsOnRun = async (runId, runNumber, userName, eorReasonsPatch } }; -/** - * Method to process a user for the run - * - * @param {object|null} userData - user object, either userO2Start or userO2Stop - * @param {number} [userData.externalId] - external identifier of the user - * @param {string|null} [userData.name] - name of the user or empty - * @returns {Promise} - returns the id of the user or null if the userData is null - */ -const getOrCreateUser = (userData) => { - if (userData?.externalId) { - // Create the user if it does not yet exist - return userService.getOrCreate({ externalUserId: userData.externalId }, userData.name); - } - return null; -}; - exports.RunService = RunService; exports.runService = new RunService(); diff --git a/lib/server/services/user/UserService.js b/lib/server/services/user/UserService.js index 18b983d045..58d6775abc 100644 --- a/lib/server/services/user/UserService.js +++ b/lib/server/services/user/UserService.js @@ -30,7 +30,7 @@ class UserService { * * @param {UserIdentifier} identifier the identifier of the user to find * @param {string|null} name - username of the user - * @returns {User} - found user | newly created user + * @returns {Promise} - found user | newly created user */ async getOrCreate(identifier, name) { const foundUser = await getUser(identifier); @@ -38,8 +38,7 @@ class UserService { // If the user does not exist, create it as a new user if (!foundUser) { const { externalUserId } = identifier; - const user = await createOrUpdateUser({ personid: externalUserId, name }); - return user ? userAdapter.toEntity(user) : null; + return userAdapter.toEntity(await createOrUpdateUser({ personid: externalUserId, name })); } return userAdapter.toEntity(foundUser); diff --git a/test/lib/server/services/run/RunService.test.js b/test/lib/server/services/run/RunService.test.js index 9e5411bccc..6f661f8bb5 100644 --- a/test/lib/server/services/run/RunService.test.js +++ b/test/lib/server/services/run/RunService.test.js @@ -502,7 +502,7 @@ module.exports = () => { it('should successfully create run with only userO2Start', async () => { const runNumber = ++lastRunNumber; const userO2Start = { - externalId: 1000, + userIdentifier: { externalUserId: 1000 }, name: 'test', }; @@ -525,7 +525,7 @@ module.exports = () => { it('should successfully create run with only userO2Stop', async () => { const runNumber = ++lastRunNumber; const userO2Stop = { - externalId: 1001, + userIdentifier: { externalUserId: 1001 }, name: 'test', }; @@ -548,11 +548,11 @@ module.exports = () => { it('should successfully create run with userO2Start and userO2Stop', async () => { const runNumber = ++lastRunNumber; const userO2Start = { - externalId: 1002, + userIdentifier: { externalUserId: 1002 }, name: 'test', }; const userO2Stop = { - externalId: 1003, + userIdentifier: { externalUserId: 1003 }, name: 'test', }; @@ -577,7 +577,7 @@ module.exports = () => { it('should successfully update run with only userO2Start', async () => { const runNumber = 1; const userO2Start = { - externalId: 1000, + userIdentifier: { externalUserId: 1000 }, name: 'test', }; @@ -598,7 +598,7 @@ module.exports = () => { it('should successfully update run with only userO2Stop', async () => { const runNumber = 1; const userO2Stop = { - externalId: 1000, + userIdentifier: { externalUserId: 1000 }, name: 'test', }; @@ -619,11 +619,11 @@ module.exports = () => { it('should successfully update run with userO2Start and userO2Stop', async () => { const runNumber = 1; const userO2Start = { - externalId: 1002, + userIdentifier: { externalUserId: 1002 }, name: 'test', }; const userO2Stop = { - externalId: 1003, + userIdentifier: { externalUserId: 1003 }, name: 'test', }; @@ -684,7 +684,7 @@ module.exports = () => { expect(levelsCombinations).have.all.deep.members([{ l3Level: 20003, dipoleLevel: 0 }, { l3Level: 30003, dipoleLevel: 0 }]); }); - it('should successfully extract informations from environment configuration when creating a run', async () => { + it('should successfully extract information from environment configuration when creating a run', async () => { const timeO2Start = new Date('2019-08-09 20:01:00'); const timeTrgStart = new Date('2019-08-09 20:02:00'); const timeTrgEnd = new Date('2019-08-09 20:03:00'); @@ -693,7 +693,7 @@ module.exports = () => { 1234, 'CmCvjNbg', { timeO2Start, timeTrgStart, timeTrgEnd, timeO2End }, - { externalUserId: 1 }, + { userO2Start: { userIdentifier: { externalUserId: 1 } }, userO2Stop: { userIdentifier: { externalUserId: 456 } } }, ); expect(run.triggerValue).to.equal('CTP'); @@ -716,5 +716,7 @@ module.exports = () => { expect(run.nFlps).to.equal(3); expect(run.nEpns).to.equal(2); expect(run.readoutCfgUri).to.equal('file:///local/replay/2024-04-17-pp-650khz-synt-4tf/readout-replay-24g-dd40.cfg'); + expect(run.userIdO2Start).to.equal(1); + expect(run.userIdO2Stop).to.equal(2); }); };