Skip to content

Commit

Permalink
chore: Refactor user/group update/delete methods to return entities
Browse files Browse the repository at this point in the history
  • Loading branch information
SteveGT96 committed Nov 5, 2024
1 parent dc324ad commit a624295
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 24 deletions.
19 changes: 11 additions & 8 deletions src/main/java/org/isf/menu/manager/UserBrowsingManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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.
Expand Down Expand Up @@ -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")));
}
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -321,9 +323,10 @@ public UserGroup newUserGroup(UserGroup userGroup, List<Permission> 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<User> users = getUser(userGroup.getCode());
if (users != null && users.stream().anyMatch(User::isDeleted)) {
Expand Down
22 changes: 14 additions & 8 deletions src/main/java/org/isf/menu/service/MenuIoOperations.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -348,13 +353,14 @@ public UserGroup newUserGroup(UserGroup userGroup, List<Permission> 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());
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/isf/menu/TestUserBrowsingManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/isf/menu/Tests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit a624295

Please sign in to comment.