Skip to content

Commit b140a49

Browse files
[1084] add getOrCreate function to UserService (#1294)
* feat: add getorcreate function in UserService * test: add tests for get or create user * refactor: create or update user service * test: update tests with new create or update service * fix: file paths create or update user * refactor: delete unused old usecase for users * fix: update user test broken * refactor: add update chech + code improvement * test: add test case for not updating user * refactor: implement comments on PR * refactor: move one time used code in create user
1 parent 96f7ca3 commit b140a49

9 files changed

Lines changed: 229 additions & 83 deletions

File tree

lib/server/WebUiServer.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,9 @@ const { OpenIdConfig, JwtConfig } = require('../config');
1616
const { HttpServer } = require('@aliceo2/web-ui');
1717
const path = require('path');
1818
const buildEndpoints = require('./routers');
19-
const {
20-
user: {
21-
CreateOrUpdateUser,
22-
},
23-
} = require('../usecases');
2419
const { BkpRoles } = require('../domain/enums/BkpRoles.js');
2520
const { isInTestMode } = require('../utilities/env-utils');
21+
const { createOrUpdateUser } = require('./services/user/createOrUpdateUser.js');
2622

2723
/**
2824
* WebUI implementation of the Server.
@@ -61,7 +57,14 @@ class WebUiServer {
6157
return;
6258
}
6359

64-
const user = await new CreateOrUpdateUser().execute(request.session);
60+
const user = await createOrUpdateUser(request.session);
61+
if (request.session.access) {
62+
user.access = request.session.access;
63+
}
64+
if (request.session.username) {
65+
user.username = request.session.username;
66+
}
67+
6568
if (isInTestMode() && request.query?.token) {
6669
user.access = user.access ?? request.query.token === 'admin' ? [BkpRoles.ADMIN] : [BkpRoles.GUEST];
6770
}

lib/server/services/user/UserService.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,41 @@
1111
* or submit itself to any jurisdiction.
1212
*/
1313

14+
const { userAdapter } = require('../../../database/adapters/index.js');
15+
const { createOrUpdateUser } = require('./createOrUpdateUser.js');
16+
const { getUser } = require('./getUser.js');
17+
1418
/**
1519
* @typedef UserIdentifier object to uniquely identify a user
1620
* @property {number} [userId] the id of the user in the application's context
1721
* @property {number} [externalUserId] the CERN user's id
1822
*/
23+
24+
/**
25+
* Global service to handle user instances
26+
*/
27+
class UserService {
28+
/**
29+
* Find a user from the database or create it as a new user if it does not exist yet.
30+
*
31+
* @param {UserIdentifier} identifier the identifier of the user to find
32+
* @param {string|null} name - username of the user
33+
* @returns {User} - found user | newly created user
34+
*/
35+
async getOrCreate(identifier, name) {
36+
const foundUser = await getUser(identifier);
37+
38+
// If the user does not exist, create it as a new user
39+
if (!foundUser) {
40+
const { externalUserId } = identifier;
41+
const user = await createOrUpdateUser({ personid: externalUserId, name });
42+
return user ? userAdapter.toEntity(user) : null;
43+
}
44+
45+
return userAdapter.toEntity(foundUser);
46+
}
47+
}
48+
49+
exports.UserService = UserService;
50+
51+
exports.userService = new UserService();
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @license
3+
* Copyright CERN and copyright holders of ALICE O2. This software is
4+
* distributed under the terms of the GNU General Public License v3 (GPL
5+
* Version 3), copied verbatim in the file "COPYING".
6+
*
7+
* See http://alice-o2.web.cern.ch/license for full licensing information.
8+
*
9+
* In applying this license CERN does not waive the privileges and immunities
10+
* granted to it by virtue of its status as an Intergovernmental Organization
11+
* or submit itself to any jusdiction.
12+
*/
13+
14+
const { getUser } = require('./getUser.js');
15+
const { BadParameterError } = require('../../errors/BadParameterError.js');
16+
const UserRepository = require('../../../database/repositories/UserRepository.js');
17+
18+
/**
19+
* Creates or updates a user in the database and returns the user object
20+
*
21+
* @param {Object} newUser the user to create
22+
* @param {number} [newUser.personid] the CERN user's id, used as externalUserId
23+
* @param {string|null} [newUser.name] username for in bookkeeping db
24+
* @return {Promise<SequelizeUser>} resolve once the creation is done providing the user that has been (or will be) created
25+
*/
26+
exports.createOrUpdateUser = async (newUser) => {
27+
const { personid, name } = newUser;
28+
29+
// Check null OR undefined because !personid returns true with personid == 0
30+
if (personid === null || personid === undefined) {
31+
throw new BadParameterError('Cannot create without personid');
32+
}
33+
34+
// GetUser expects an identifier with two options, so surround it with {}
35+
let user = await getUser({ externalUserId: personid });
36+
if (user) {
37+
if (user.name !== name) {
38+
user = await UserRepository.update(user, { name });
39+
}
40+
} else {
41+
user = await UserRepository.insert({ externalId: personid, name });
42+
}
43+
44+
return user;
45+
};

lib/usecases/index.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const server = require('./server');
2222
const status = require('./status');
2323
const subsystem = require('./subsystem');
2424
const tag = require('./tag');
25-
const user = require('./user');
2625

2726
module.exports = {
2827
attachment,
@@ -36,5 +35,4 @@ module.exports = {
3635
status,
3736
subsystem,
3837
tag,
39-
user,
4038
};

lib/usecases/user/CreateOrUpdateUser.js

Lines changed: 0 additions & 72 deletions
This file was deleted.

test/lib/server/services/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const RunTypeSuite = require('./runType/index.js');
2424
const ShiftSuite = require('./shift/index.js');
2525
const StatisticsSuite = require('./statistics/index.js');
2626
const LhcPeriodSuite = require('./lhcPeriod');
27+
const UserSuite = require('./user/index.js');
2728
const { resetDatabaseContent } = require('../../../utilities/resetDatabaseContent.js');
2829

2930
module.exports = () => {
@@ -44,4 +45,5 @@ module.exports = () => {
4445
describe('Shift', ShiftSuite);
4546
describe('Statistics', StatisticsSuite);
4647
describe('LhcPeriod', LhcPeriodSuite);
48+
describe('User', UserSuite);
4749
};
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @license
3+
* Copyright CERN and copyright holders of ALICE O2. This software is
4+
* distributed under the terms of the GNU General Public License v3 (GPL
5+
* Version 3), copied verbatim in the file "COPYING".
6+
*
7+
* See http://alice-o2.web.cern.ch/license for full licensing information.
8+
*
9+
* In applying this license CERN does not waive the privileges and immunities
10+
* granted to it by virtue of its status as an Intergovernmental Organization
11+
* or submit itself to any jurisdiction.
12+
*/
13+
14+
const { expect } = require('chai');
15+
const { userService } = require('../../../../../lib/server/services/user/UserService.js');
16+
const { getUser } = require('../../../../../lib/server/services/user/getUser.js');
17+
const { createOrUpdateUser } = require('../../../../../lib/server/services/user/createOrUpdateUser.js');
18+
19+
module.exports = () => {
20+
it('should successfully get the existing user with getOrCreate function', async () => {
21+
const externalId = 1001;
22+
const name = 'testUser';
23+
24+
// Create user in db
25+
await createOrUpdateUser({ personid: externalId, name });
26+
27+
const user = await userService.getOrCreate({ externalUserId: externalId }, name);
28+
29+
// User should exist now
30+
expect(user).not.to.be.null;
31+
// Id should be filled automatically
32+
expect(user.id).not.to.be.null;
33+
// ExternalId and name should be equal
34+
expect(user.externalId).to.equal(externalId);
35+
expect(user.name).to.equal(name);
36+
});
37+
38+
it('should successfully create the non-existing user with getOrCreate function', async () => {
39+
const externalId = 1002;
40+
const name = 'testUser';
41+
42+
// Before creation it should be null
43+
const nullUser = await getUser({ externalUserId: externalId });
44+
expect(nullUser).to.be.null;
45+
46+
// Create the user using the getOrCreate function
47+
await userService.getOrCreate({ externalUserId: externalId }, name);
48+
const user = await getUser({ externalUserId: externalId });
49+
50+
// User should exist now
51+
expect(user).not.to.be.null;
52+
// Id should be filled automatically
53+
expect(user.id).not.to.be.null;
54+
// ExternalId and name should be equal
55+
expect(user.externalId).to.equal(externalId);
56+
expect(user.name).to.equal(name);
57+
});
58+
};
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @license
3+
* Copyright CERN and copyright holders of ALICE O2. This software is
4+
* distributed under the terms of the GNU General Public License v3 (GPL
5+
* Version 3), copied verbatim in the file "COPYING".
6+
*
7+
* See http://alice-o2.web.cern.ch/license for full licensing information.
8+
*
9+
* In applying this license CERN does not waive the privileges and immunities
10+
* granted to it by virtue of its status as an Intergovernmental Organization
11+
* or submit itself to any jurisdiction.
12+
*/
13+
14+
const { expect } = require('chai');
15+
const { createOrUpdateUser } = require('../../../../../lib/server/services/user/createOrUpdateUser');
16+
17+
module.exports = () => {
18+
it('should successfully create the user', async () => {
19+
const externalId = 1000;
20+
const name = 'testUser';
21+
22+
const user = await createOrUpdateUser({ personid: externalId, name });
23+
24+
// User should exist now
25+
expect(user).not.to.be.null;
26+
// Id should be filled automatically
27+
expect(user.id).not.to.be.null;
28+
// ExternalId and name should be equal
29+
expect(user.externalId).to.equal(externalId);
30+
expect(user.name).to.equal(name);
31+
});
32+
33+
it('should successfully update the user', async () => {
34+
const externalId = 1001;
35+
const name = 'testUser';
36+
const updatedName = 'testUserUpdated';
37+
38+
// Create user
39+
const user = await createOrUpdateUser({ personid: externalId, name });
40+
41+
// Update user
42+
const updatedUser = await createOrUpdateUser({ personid: externalId, name: updatedName });
43+
44+
// Users should exist now
45+
expect(user).not.to.be.null;
46+
expect(updatedUser).not.to.be.null;
47+
// Id should be filled automatically
48+
expect(user.id).not.to.be.null;
49+
// Id of user and updatedUser should be the same because it is updated
50+
expect(user.id).to.equal(updatedUser.id);
51+
// ExternalId should be equal
52+
expect(user.externalId).to.equal(updatedUser.externalId);
53+
// Name should be updated
54+
expect(updatedUser.name).to.equal(updatedName);
55+
});
56+
57+
it('should not update the user but still return the same user', async () => {
58+
const externalId = 1001;
59+
const name = 'testUser';
60+
61+
// Create user
62+
const user = await createOrUpdateUser({ personid: externalId, name });
63+
64+
// Update user with the same variables (should stay the same)
65+
const sameUser = await createOrUpdateUser({ personid: externalId, name });
66+
67+
// Users should exist now
68+
expect(user).not.to.be.null;
69+
expect(sameUser).not.to.be.null;
70+
// Id should be filled automatically
71+
expect(sameUser.id).not.to.be.null;
72+
// Id, externalId and name should be the same
73+
expect(user.id).to.equal(sameUser.id);
74+
expect(user.externalId).to.equal(sameUser.externalId);
75+
expect(user.name).to.equal(sameUser.name);
76+
});
77+
};
Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
* or submit itself to any jurisdiction.
1212
*/
1313

14-
const CreateOrUpdateUser = require('./CreateOrUpdateUser');
14+
const UserServiceTest = require('./UserService.test.js');
15+
const createOrUpdateUserTest = require('./createOrUpdateUser.test.js');
1516

16-
module.exports = {
17-
CreateOrUpdateUser,
17+
module.exports = () => {
18+
describe('createOrUpdateUser', createOrUpdateUserTest);
19+
describe('UserService', UserServiceTest);
1820
};

0 commit comments

Comments
 (0)