From 931704c539434b94f89ede709a6328514daf28a7 Mon Sep 17 00:00:00 2001 From: Evans Djangbah Date: Fri, 9 Aug 2024 06:02:00 +0000 Subject: [PATCH] chore: Remove unnecessary error logging in PortfolioServiceImpl --- .../controller/UserController.java | 14 +++------- .../services/impl/PortfolioServiceImpl.java | 1 - .../services/impl/UserServiceImpl.java | 10 +++---- .../exceptions/GlobalExceptionHandler.java | 28 +++++++++---------- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/UserManagement/src/main/java/com/joe/trading/user_management/controller/UserController.java b/UserManagement/src/main/java/com/joe/trading/user_management/controller/UserController.java index 5cc01e9..a8f8478 100644 --- a/UserManagement/src/main/java/com/joe/trading/user_management/controller/UserController.java +++ b/UserManagement/src/main/java/com/joe/trading/user_management/controller/UserController.java @@ -73,17 +73,11 @@ public ResponseEntity> getAllUsers(UserFil public ResponseEntity updateUser(@PathVariable("id") Long userId, @Valid @RequestBody UpdateUserDto updatedUser) throws ResourceNotFoundException, IllegalArgumentException { var auth = SecurityContextHolder.getContext().getAuthentication(); + var principal = (User) auth.getPrincipal(); - var principal = auth.getPrincipal(); - if (principal instanceof User) { - var user = (User) principal; - if (user.getAccountType().equals(AccountType.USER) - && updatedUser.getAccountType().equals(AccountType.ADMIN)) { - throw new IllegalArgumentException("You are not authorized to update user to admin"); - } - } else { - // TODO: Throw a custom exception, if the principal is not an instance of User - throw new IllegalArgumentException("You are not authorized to update user"); + if (principal.getAccountType().equals(AccountType.USER) + && updatedUser.getAccountType().equals(AccountType.ADMIN)) { + throw new IllegalArgumentException("You are not authorized to update user to admin"); } User userDto = userService.updateUser(userId, updatedUser); diff --git a/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/PortfolioServiceImpl.java b/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/PortfolioServiceImpl.java index 5a4ec98..4f8fb13 100644 --- a/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/PortfolioServiceImpl.java +++ b/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/PortfolioServiceImpl.java @@ -36,7 +36,6 @@ private void deletePortfolio(PortfolioEventDto portfolio) { try { natsService.publish(Event.USER_DELETED, userMapper.toUserEventDto(u)); } catch (JsonProcessingException e) { - System.err.println("Error deleting user"); e.printStackTrace(); } } diff --git a/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/UserServiceImpl.java b/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/UserServiceImpl.java index ea996de..913748f 100644 --- a/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/UserServiceImpl.java +++ b/UserManagement/src/main/java/com/joe/trading/user_management/services/impl/UserServiceImpl.java @@ -2,8 +2,6 @@ import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -40,11 +38,11 @@ public class UserServiceImpl implements UserService { private NatsService natsService; private UserMapper userMapper; - private static final Logger logger = LoggerFactory.getLogger(UserServiceImpl.class); + private static final String USER_NOT_FOUND = "User does not exist"; public User getUserById(Long userId) throws ResourceNotFoundException { return userRepository.findById(userId).orElseThrow( - () -> new ResourceNotFoundException("User does not exist")); + () -> new ResourceNotFoundException(USER_NOT_FOUND)); } @Override @@ -108,7 +106,7 @@ public Page getUsers(UserFilterRequestDto filterRequestDto) { @Override public User updateUser(Long userId, UpdateUserDto updatedUser) throws ResourceNotFoundException { User existingUser = userRepository.findById(userId).orElseThrow( - () -> new ResourceNotFoundException("User does not exist")); + () -> new ResourceNotFoundException(USER_NOT_FOUND)); existingUser.setName(updatedUser.getName()); existingUser.setEmail(updatedUser.getEmail()); @@ -130,7 +128,7 @@ public User updateUser(Long userId, UpdateUserDto updatedUser) throws ResourceNo @Override public void deleteUser(Long userId) throws RuntimeException, ResourceNotFoundException { User user = userRepository.findById(userId).orElseThrow( - () -> new ResourceNotFoundException("User does not exist")); + () -> new ResourceNotFoundException(USER_NOT_FOUND)); List portfolios = portfolioRepository.findByUserId(userId); if (portfolios.isEmpty()) { diff --git a/shared/src/main/java/com/joe/trading/shared/exceptions/GlobalExceptionHandler.java b/shared/src/main/java/com/joe/trading/shared/exceptions/GlobalExceptionHandler.java index f02ef8b..3b2cecb 100644 --- a/shared/src/main/java/com/joe/trading/shared/exceptions/GlobalExceptionHandler.java +++ b/shared/src/main/java/com/joe/trading/shared/exceptions/GlobalExceptionHandler.java @@ -24,34 +24,34 @@ public class GlobalExceptionHandler { @ExceptionHandler(Exception.class) public ProblemDetail handleSecurityException(Exception exception) { ProblemDetail errorDetail = null; + var descField = "description"; - // TODO send this stack trace to an observability tool exception.printStackTrace(); if (exception instanceof BadCredentialsException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(401), exception.getMessage()); - errorDetail.setProperty("description", "The email or password is incorrect"); + errorDetail.setProperty(descField, "The email or password is incorrect"); errorDetail.setTitle("Authentication Error"); return errorDetail; } if (exception instanceof AccountStatusException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(403), exception.getMessage()); - errorDetail.setProperty("description", "The account is locked"); + errorDetail.setProperty(descField, "The account is locked"); errorDetail.setTitle("Account Error"); return errorDetail; } if (exception instanceof AccessDeniedException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(403), exception.getMessage()); - errorDetail.setProperty("description", "You are not authorized to access this resource"); + errorDetail.setProperty(descField, "You are not authorized to access this resource"); errorDetail.setTitle("Authorization Error"); return errorDetail; } if (exception instanceof SignatureException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(403), exception.getMessage()); - errorDetail.setProperty("description", "The JWT signature is invalid"); + errorDetail.setProperty(descField, "The JWT signature is invalid"); errorDetail.setTitle("JWT Error"); return errorDetail; } @@ -59,7 +59,7 @@ public ProblemDetail handleSecurityException(Exception exception) { if (exception instanceof MethodArgumentNotValidException) { MethodArgumentNotValidException ex = (MethodArgumentNotValidException) exception; errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(400), "Validation error"); - errorDetail.setProperty("description", "The request body is invalid"); + errorDetail.setProperty(descField, "The request body is invalid"); errorDetail.setTitle("Validation Error"); Map fieldErrors = new HashMap<>(); @@ -74,56 +74,56 @@ public ProblemDetail handleSecurityException(Exception exception) { if (exception instanceof HttpMessageNotReadableException) { HttpMessageNotReadableException ex = (HttpMessageNotReadableException) exception; errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(400), "Malformed JSON request"); - errorDetail.setProperty("description", ex.getMessage()); + errorDetail.setProperty(descField, ex.getMessage()); errorDetail.setTitle("Malformed JSON Error"); return errorDetail; } if (exception instanceof ExpiredJwtException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(403), exception.getMessage()); - errorDetail.setProperty("description", "The JWT token has expired"); + errorDetail.setProperty(descField, "The JWT token has expired"); errorDetail.setTitle("JWT Error"); return errorDetail; } if (exception instanceof HttpRequestMethodNotSupportedException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(405), exception.getMessage()); - errorDetail.setProperty("description", "The HTTP method is not supported"); + errorDetail.setProperty(descField, "The HTTP method is not supported"); errorDetail.setTitle("Method Not Allowed"); return errorDetail; } if (exception instanceof ResourceNotFoundException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(404), exception.getMessage()); - errorDetail.setProperty("description", "The resource was not found"); + errorDetail.setProperty(descField, "The resource was not found"); errorDetail.setTitle("Resource Not Found"); return errorDetail; } if (exception instanceof EmailAlreadyExistsException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(409), exception.getMessage()); - errorDetail.setProperty("description", "The email already exists"); + errorDetail.setProperty(descField, "The email already exists"); errorDetail.setTitle("Conflict"); return errorDetail; } if (exception instanceof UserDeletionException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(500), exception.getMessage()); - errorDetail.setProperty("description", "An error occurred while deleting the user"); + errorDetail.setProperty(descField, "An error occurred while deleting the user"); errorDetail.setTitle("User Deletion Error"); return errorDetail; } if (exception instanceof NoResourceFoundException) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(404), exception.getMessage()); - errorDetail.setProperty("description", "The resource was not found"); + errorDetail.setProperty(descField, "The resource was not found"); errorDetail.setTitle("Resource Not Found"); return errorDetail; } if (errorDetail == null) { errorDetail = ProblemDetail.forStatusAndDetail(HttpStatusCode.valueOf(500), exception.getMessage()); - errorDetail.setProperty("description", "Unknown internal server error."); + errorDetail.setProperty(descField, "Unknown internal server error."); errorDetail.setTitle("Internal Server Error"); }