From a6242951c35b49ce03b771e8b54c545fe1e25f3a Mon Sep 17 00:00:00 2001 From: SteveGT96 Date: Tue, 5 Nov 2024 11:10:25 +0100 Subject: [PATCH] chore: Refactor user/group update/delete methods to return entities --- .../isf/menu/manager/UserBrowsingManager.java | 19 +++++++++------- .../isf/menu/service/MenuIoOperations.java | 22 ++++++++++++------- .../org/isf/menu/TestUserBrowsingManager.java | 8 +++---- src/test/java/org/isf/menu/Tests.java | 8 +++---- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/isf/menu/manager/UserBrowsingManager.java b/src/main/java/org/isf/menu/manager/UserBrowsingManager.java index 7cb3e3650..34098bed1 100644 --- a/src/main/java/org/isf/menu/manager/UserBrowsingManager.java +++ b/src/main/java/org/isf/menu/manager/UserBrowsingManager.java @@ -109,10 +109,10 @@ public User newUser(User user) throws OHServiceException { /** * Updates an existing {@link User} in the DB. * @param user - the {@link User} to update - * @return {@code true} if the user has been updated, {@code false} otherwise. + * @return the {@link User} that has been updated * @throws OHServiceException When failed to update user */ - public boolean updateUser(User user) throws OHServiceException { + public User updateUser(User user) throws OHServiceException { return ioOperations.updateUser(user); } @@ -129,13 +129,14 @@ public boolean updatePassword(User user) throws OHServiceException { /** * Deletes an existing {@link User}. * @param user - the {@link User} to delete + * @return the {@link User} that has been deleted * @throws OHServiceException When failed to delete user */ - public void deleteUser(User user) throws OHServiceException { + public User deleteUser(User user) throws OHServiceException { if (user.getUserName().equals("admin")) { throw new OHDataValidationException(new OHExceptionMessage(MessageBundle.getMessage("angal.userbrowser.theadminusercannotbedeleted.msg"))); } - ioOperations.deleteUser(user); + return ioOperations.deleteUser(user); } // TODO: revisit the individual methods for failed attempts, locking, last login time, etc. @@ -273,9 +274,10 @@ public String getUsrInfo(String userName) throws OHServiceException { /** * Deletes a {@link UserGroup}. * @param aGroup - the {@link UserGroup} to delete + * @return the {@link UserGroup} that has been deleted * @throws OHServiceException When failed to delete group */ - public void deleteGroup(UserGroup aGroup) throws OHServiceException { + public UserGroup deleteGroup(UserGroup aGroup) throws OHServiceException { if (aGroup.getCode().equals("admin")) { throw new OHDataValidationException(new OHExceptionMessage(MessageBundle.getMessage("angal.groupsbrowser.theadmingroupcannotbedeleted.msg"))); } @@ -284,7 +286,7 @@ public void deleteGroup(UserGroup aGroup) throws OHServiceException { throw new OHDataIntegrityViolationException( new OHExceptionMessage(MessageBundle.getMessage("angal.groupsbrowser.thisgrouphasusersandcannotbedeleted.msg"))); } - ioOperations.deleteGroup(aGroup); + return ioOperations.deleteGroup(aGroup); } /** @@ -321,9 +323,10 @@ public UserGroup newUserGroup(UserGroup userGroup, List permissions) /** * Updates an existing {@link UserGroup} in the DB. * @param userGroup - the {@link UserGroup} to update - * @return {@code true} if the group has been updated, {@code false} otherwise. + * @return the {@link UserGroup} that has been updated + * @throws OHServiceException If failed to update group */ - public boolean updateUserGroup(UserGroup userGroup) throws OHServiceException { + public UserGroup updateUserGroup(UserGroup userGroup) throws OHServiceException { if (userGroup.isDeleted()) { List users = getUser(userGroup.getCode()); if (users != null && users.stream().anyMatch(User::isDeleted)) { diff --git a/src/main/java/org/isf/menu/service/MenuIoOperations.java b/src/main/java/org/isf/menu/service/MenuIoOperations.java index defcc6364..ef96e9ff3 100644 --- a/src/main/java/org/isf/menu/service/MenuIoOperations.java +++ b/src/main/java/org/isf/menu/service/MenuIoOperations.java @@ -178,12 +178,13 @@ public User newUser(User user) throws OHServiceException { /** * Updates an existing {@link User} in the DB * @param user - the {@link User} to update - * @return new {@link User} + * @return the updated {@link User} * @throws OHServiceException When failed to update user */ - public boolean updateUser(User user) throws OHServiceException { + public User updateUser(User user) throws OHServiceException { ensureUserNotDeleted(user.getUserName()); - return repository.updateUser(user.getDesc(), user.getUserGroupName(), user.isDeleted(), user.getUserName()) > 0; + repository.updateUser(user.getDesc(), user.getUserGroupName(), user.isDeleted(), user.getUserName()); + return getUserByName(user.getUserName()); } /** @@ -200,11 +201,13 @@ public boolean updatePassword(User user) throws OHServiceException { /** * Deletes an existing {@link User} * @param user - the {@link User} to delete + * @return the {@link User} that has been deleted * @throws OHServiceException When failed to delete user */ - public void deleteUser(User user) throws OHServiceException { + public User deleteUser(User user) throws OHServiceException { ensureUserNotDeleted(user.getUserName()); repository.delete(user); + return user; } public void updateFailedAttempts(String userName, int newFailAttempts) { @@ -302,11 +305,13 @@ private GroupMenu insertGroupMenu(UserGroup aGroup, UserMenuItem item) throws OH /** * Deletes a {@link UserGroup} * @param aGroup - the {@link UserGroup} to delete + * @return the {@link UserGroup} that has been deleted * @throws OHServiceException When failed to delete group */ - public void deleteGroup(UserGroup aGroup) throws OHServiceException { + public UserGroup deleteGroup(UserGroup aGroup) throws OHServiceException { ensureUserGroupNotDeleted(aGroup.getCode()); groupRepository.delete(aGroup); + return aGroup; } /** @@ -348,13 +353,14 @@ public UserGroup newUserGroup(UserGroup userGroup, List permissions) /** * Updates an existing {@link UserGroup} in the DB * @param aGroup - the {@link UserGroup} to update - * @return {@code true} if the group has been updated, {@code false} otherwise. + * @return the {@link UserGroup} that has been updated * @throws OHServiceException When failed to update the user group */ - public boolean updateUserGroup(UserGroup aGroup) throws OHServiceException { + public UserGroup updateUserGroup(UserGroup aGroup) throws OHServiceException { ensureUserGroupNotDeleted(aGroup.getCode()); - return groupRepository.update(aGroup.getDesc(), aGroup.isDeleted(), aGroup.getCode()) > 0; + groupRepository.update(aGroup.getDesc(), aGroup.isDeleted(), aGroup.getCode()); + return findByCode(aGroup.getCode()); } /** diff --git a/src/test/java/org/isf/menu/TestUserBrowsingManager.java b/src/test/java/org/isf/menu/TestUserBrowsingManager.java index 27460aaed..73d2823a1 100644 --- a/src/test/java/org/isf/menu/TestUserBrowsingManager.java +++ b/src/test/java/org/isf/menu/TestUserBrowsingManager.java @@ -117,7 +117,7 @@ void testIncreaseFailedAttempts() throws Exception { int failedAttempts = user.getFailedAttempts(); userBrowsingManager.increaseFailedAttempts(user); - user.setFailedAttempts(failedAttempts+ 1); + user.setFailedAttempts(failedAttempts + 1); assertThat(user.getFailedAttempts()).isEqualTo(failedAttempts + 1); } @@ -163,7 +163,7 @@ void testUnlockWhenTimeExpired() throws Exception { user.setAccountLocked(true); user.setLockedTime(TimeTools.getNow().minusDays(30)); user.setAccountLocked(true); - assertThat(userBrowsingManager.updateUser(user)).isTrue(); + assertThat(userBrowsingManager.updateUser(user).getUserName()).isEqualTo(user.getUserName()); User updatedUser = userBrowsingManager.getUserByName(userName); @@ -184,7 +184,7 @@ void testUnlockWhenTimeExpiredFails() throws Exception { user.setAccountLocked(true); user.setLockedTime(TimeTools.getNow().plusDays(30)); user.setAccountLocked(true); - assertThat(userBrowsingManager.updateUser(user)).isTrue(); + assertThat(userBrowsingManager.updateUser(user).getUserName()).isEqualTo(user.getUserName()); User updatedUser = userBrowsingManager.getUserByName(userName); @@ -211,7 +211,7 @@ void testInvalidUserName() throws Exception { user.setUserName("A@!"); userBrowsingManager.newUser(user); }) - .isInstanceOf(OHDataValidationException.class); + .isInstanceOf(OHDataValidationException.class); } private String setupTestUser(boolean usingSet) throws OHException { diff --git a/src/test/java/org/isf/menu/Tests.java b/src/test/java/org/isf/menu/Tests.java index 7512e7eaa..ef6dcd3f6 100644 --- a/src/test/java/org/isf/menu/Tests.java +++ b/src/test/java/org/isf/menu/Tests.java @@ -209,7 +209,7 @@ void testIoUpdateUser() throws Exception { User foundUser = userIoOperationRepository.findById(userName).orElse(null); assertThat(foundUser).isNotNull(); foundUser.setDesc("Update"); - assertThat(menuIoOperation.updateUser(foundUser)).isTrue(); + assertThat(menuIoOperation.updateUser(foundUser).getUserName()).isEqualTo(foundUser.getUserName()); User updatedUser = userIoOperationRepository.findById(userName).orElse(null); assertThat(updatedUser).isNotNull(); assertThat(updatedUser.getDesc()).isEqualTo("Update"); @@ -323,7 +323,7 @@ void testIoUpdateUserGroup() throws Exception { UserGroup foundUserGroup = userGroupIoOperationRepository.findById(code).orElse(null); assertThat(foundUserGroup).isNotNull(); foundUserGroup.setDesc("Update"); - assertThat(menuIoOperation.updateUserGroup(foundUserGroup)).isTrue(); + assertThat(menuIoOperation.updateUserGroup(foundUserGroup).getCode()).isEqualTo(foundUserGroup.getCode()); UserGroup updatedUserGroup = userGroupIoOperationRepository.findById(code).orElse(null); assertThat(updatedUserGroup).isNotNull(); assertThat(updatedUserGroup.getDesc()).isEqualTo("Update"); @@ -437,7 +437,7 @@ void testMgrUpdateUser() throws Exception { User foundUser = userIoOperationRepository.findById(userName).orElse(null); assertThat(foundUser).isNotNull(); foundUser.setDesc("Update"); - assertThat(userBrowsingManager.updateUser(foundUser)).isTrue(); + assertThat(userBrowsingManager.updateUser(foundUser).getUserName()).isEqualTo(foundUser.getUserName()); User updatedUser = userIoOperationRepository.findById(userName).orElse(null); assertThat(updatedUser).isNotNull(); assertThat(updatedUser.getDesc()).isEqualTo("Update"); @@ -542,7 +542,7 @@ void testMgrUpdateUserGroup() throws Exception { UserGroup foundUserGroup = userGroupIoOperationRepository.findById(code).orElse(null); assertThat(foundUserGroup).isNotNull(); foundUserGroup.setDesc("Update"); - assertThat(userBrowsingManager.updateUserGroup(foundUserGroup)).isTrue(); + assertThat(userBrowsingManager.updateUserGroup(foundUserGroup).getCode()).isEqualTo(foundUserGroup.getCode()); UserGroup updatedUserGroup = userGroupIoOperationRepository.findById(code).orElse(null); assertThat(updatedUserGroup).isNotNull(); assertThat(updatedUserGroup.getDesc()).isEqualTo("Update");