From a9ef47fe7a52a378936ff4be9ffd3fb724f00ee9 Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Fri, 17 Nov 2023 16:54:31 +0100 Subject: [PATCH] SONAR-21051 remove un-necessarry re-fetch of UserDao --- .../src/main/js/api/mocks/UsersServiceMock.ts | 4 ++-- server/sonar-web/src/main/js/api/users.ts | 4 ++-- .../apps/users/components/DeactivateForm.tsx | 6 ++--- .../js/apps/users/components/UserForm.tsx | 2 +- .../common/user/service/UserServiceIT.java | 3 ++- .../server/common/user/UserDeactivator.java | 23 +++++++++---------- .../common/user/service/UserService.java | 15 ++++++------ 7 files changed, 29 insertions(+), 28 deletions(-) diff --git a/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts b/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts index 878ad25b9b2..0136165ef39 100644 --- a/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts +++ b/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts @@ -287,7 +287,7 @@ export default class UsersServiceMock { handleUpdateUser: typeof updateUser = (id, data) => { const { email, name, scmAccounts } = data; - const user = this.users.find((u) => u.login === id); + const user = this.users.find((u) => u.id === id); if (!user) { return Promise.reject('No such user'); } @@ -381,7 +381,7 @@ export default class UsersServiceMock { }; handleDeactivateUser: typeof deleteUser = (data) => { - const index = this.users.findIndex((u) => u.login === data.login); + const index = this.users.findIndex((u) => u.id === data.id); const user = this.users.splice(index, 1)[0]; user.active = false; return this.reply(undefined); diff --git a/server/sonar-web/src/main/js/api/users.ts b/server/sonar-web/src/main/js/api/users.ts index f4bbe883ca0..c5f15c59441 100644 --- a/server/sonar-web/src/main/js/api/users.ts +++ b/server/sonar-web/src/main/js/api/users.ts @@ -110,8 +110,8 @@ export function updateUser( return axiosToCatch.patch(`${USERS_ENDPOINT}/${id}`, data); } -export function deleteUser({ login, anonymize }: { login: string; anonymize?: boolean }) { - return axios.delete(`${USERS_ENDPOINT}/${login}`, { params: { anonymize } }); +export function deleteUser({ id, anonymize }: { id: string; anonymize?: boolean }) { + return axios.delete(`${USERS_ENDPOINT}/${id}`, { params: { anonymize } }); } export function setHomePage(homepage: HomePage): Promise { diff --git a/server/sonar-web/src/main/js/apps/users/components/DeactivateForm.tsx b/server/sonar-web/src/main/js/apps/users/components/DeactivateForm.tsx index daaee9d7f35..188a55bd319 100644 --- a/server/sonar-web/src/main/js/apps/users/components/DeactivateForm.tsx +++ b/server/sonar-web/src/main/js/apps/users/components/DeactivateForm.tsx @@ -26,11 +26,11 @@ import { ResetButtonLink, SubmitButton } from '../../../components/controls/butt import { Alert } from '../../../components/ui/Alert'; import { translate, translateWithParameters } from '../../../helpers/l10n'; import { useDeactivateUserMutation } from '../../../queries/users'; -import { UserActive } from '../../../types/users'; +import { RestUserDetailed } from '../../../types/users'; export interface Props { onClose: () => void; - user: UserActive; + user: RestUserDetailed; } export default function DeactivateForm(props: Props) { @@ -42,7 +42,7 @@ export default function DeactivateForm(props: Props) { const handleDeactivate = (event: React.SyntheticEvent) => { event.preventDefault(); deactivateUser( - { login: user.login, anonymize }, + { id: user.id, anonymize }, { onSuccess: props.onClose, }, diff --git a/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx b/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx index c722d017cfc..ecb6aa69b9a 100644 --- a/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx +++ b/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx @@ -82,7 +82,7 @@ export default function UserForm(props: Props) { updateUser( { - id: login, + id: user?.id!, data: isInstanceManaged || !user?.local ? { scmAccounts } diff --git a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java index 0ad92fa02f4..96aaa999c3c 100644 --- a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java +++ b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/user/service/UserServiceIT.java @@ -675,8 +675,9 @@ public class UserServiceIT { UserDto admin = db.users().insertUser(); db.users().insertGlobalPermissionOnUser(admin, GlobalPermission.ADMINISTER); + String adminUuid = admin.getUuid(); assertThatThrownBy(() -> { - userService.deactivate(admin.getUuid(), false); + userService.deactivate(adminUuid, false); }) .isInstanceOf(BadRequestException.class) .hasMessage("User is last administrator, and cannot be deactivated"); diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/UserDeactivator.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/UserDeactivator.java index 92e2d51cdb1..eb379072517 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/UserDeactivator.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/UserDeactivator.java @@ -38,22 +38,21 @@ public class UserDeactivator { this.userAnonymizer = userAnonymizer; } - public UserDto deactivateUser(DbSession dbSession, String login) { - UserDto user = doBeforeDeactivation(dbSession, login); - return deactivateUser(dbSession, user); + public UserDto deactivateUser(DbSession dbSession, UserDto userDto) { + UserDto user = doBeforeDeactivation(dbSession, userDto); + return deactivateUserInternal(dbSession, user); } - public UserDto deactivateUserWithAnonymization(DbSession dbSession, String login) { - UserDto user = doBeforeDeactivation(dbSession, login); + public UserDto deactivateUserWithAnonymization(DbSession dbSession, UserDto userDto) { + UserDto user = doBeforeDeactivation(dbSession, userDto); anonymizeUser(dbSession, user); - return deactivateUser(dbSession, user); + return deactivateUserInternal(dbSession, user); } - private UserDto doBeforeDeactivation(DbSession dbSession, String login) { - UserDto user = getUserOrThrow(dbSession, login); - ensureNotLastAdministrator(dbSession, user); - deleteRelatedData(dbSession, user); - return user; + private UserDto doBeforeDeactivation(DbSession dbSession, UserDto userDto) { + ensureNotLastAdministrator(dbSession, userDto); + deleteRelatedData(dbSession, userDto); + return userDto; } private void ensureNotLastAdministrator(DbSession dbSession, UserDto user) { @@ -82,7 +81,7 @@ public class UserDeactivator { dbClient.scimUserDao().deleteByUserUuid(dbSession, user.getUuid()); } - private UserDto deactivateUser(DbSession dbSession, UserDto user) { + private UserDto deactivateUserInternal(DbSession dbSession, UserDto user) { dbClient.userDao().deactivateUser(dbSession, user); dbSession.commit(); return getUserOrThrow(dbSession, user.getLogin()); diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java index 5b60eb8b150..32e091e401c 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/user/service/UserService.java @@ -46,6 +46,7 @@ import org.sonar.server.user.UserUpdater; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.emptyToNull; import static java.util.Comparator.comparing; +import static java.util.Objects.requireNonNull; import static org.sonar.server.exceptions.NotFoundException.checkFound; import static org.sonar.server.user.ExternalIdentity.SQ_AUTHORITY; @@ -145,9 +146,9 @@ public class UserService { managedInstanceChecker.throwIfUserIsManaged(dbSession, uuid); UserDto deactivatedUser; if (Boolean.TRUE.equals(anonymize)) { - deactivatedUser = userDeactivator.deactivateUserWithAnonymization(dbSession, userDto.getLogin()); + deactivatedUser = userDeactivator.deactivateUserWithAnonymization(dbSession, userDto); } else { - deactivatedUser = userDeactivator.deactivateUser(dbSession, userDto.getLogin()); + deactivatedUser = userDeactivator.deactivateUser(dbSession, userDto); } dbSession.commit(); return deactivatedUser; @@ -178,18 +179,18 @@ public class UserService { if (Boolean.FALSE.equals(userCreateRequest.isLocal())) { newUserBuilder.setExternalIdentity(new ExternalIdentity(SQ_AUTHORITY, login, login)); } - return registerUser(dbSession, login, newUserBuilder.build()); + return registerUser(dbSession, newUserBuilder.build()); } } - private UserInformation registerUser(DbSession dbSession, String uuid, NewUser newUserBuilder) { - UserDto user = dbClient.userDao().selectByLogin(dbSession, newUserBuilder.login()); + private UserInformation registerUser(DbSession dbSession, NewUser newUser) { + UserDto user = dbClient.userDao().selectByLogin(dbSession, requireNonNull(newUser.login())); if (user == null) { - user = userUpdater.createAndCommit(dbSession, newUserBuilder, u -> { + user = userUpdater.createAndCommit(dbSession, newUser, u -> { }); } else { checkArgument(!user.isActive(), "An active user with login '%s' already exists", user.getLogin()); - user = userUpdater.reactivateAndCommit(dbSession, user, newUserBuilder, u -> { + user = userUpdater.reactivateAndCommit(dbSession, user, newUser, u -> { }); } return fetchUser(user.getUuid()); -- 2.39.5