diff --git a/CODE_REFACTOR_PLAN.md b/CODE_REFACTOR_PLAN.md index 7dbc40a..a1bc3c0 100644 --- a/CODE_REFACTOR_PLAN.md +++ b/CODE_REFACTOR_PLAN.md @@ -162,7 +162,7 @@ all identified issues with an aggressive approach to modernization. ## Phase 4: API Layer - Modern REST Design (Days 8-9) -### 4.1 Custom Annotations & Argument Resolvers ⬜ +### 4.1 Custom Annotations & Argument Resolvers ✅ **New Files**: `CurrentUser.java`, `CurrentUserArgumentResolver.java`, `WebConfig.java` @@ -175,7 +175,7 @@ all identified issues with an aggressive approach to modernization. **Breaking Changes**: Controller method signatures change (BREAKING but cleaner) -### 4.2 Controller Refactoring ⬜ +### 4.2 Controller Refactoring ✅ **Files**: `AuthController.java`, `UserController.java` @@ -190,7 +190,7 @@ all identified issues with an aggressive approach to modernization. **Breaking Changes**: None (internal improvements) -### 4.3 Global Exception Handler Enhancement ⬜ +### 4.3 Global Exception Handler Enhancement ✅ **File**: `GlobalExceptionHandler.java` diff --git a/src/main/java/org/nkcoder/annotation/CurrentUser.java b/src/main/java/org/nkcoder/annotation/CurrentUser.java new file mode 100644 index 0000000..1cf34b7 --- /dev/null +++ b/src/main/java/org/nkcoder/annotation/CurrentUser.java @@ -0,0 +1,21 @@ +package org.nkcoder.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation to inject the current authenticated user's ID into controller method parameters. + * + *

Usage: {@code public ResponseEntity getMe(@CurrentUser UUID userId)} + * + *

The userId is extracted from the request attributes set by JwtAuthenticationFilter. If no + * authenticated user is found, the resolver returns null (let Spring Security handle unauthorized + * access via security configuration). + */ +@Target(ElementType.PARAMETER) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface CurrentUser {} diff --git a/src/main/java/org/nkcoder/config/WebConfig.java b/src/main/java/org/nkcoder/config/WebConfig.java new file mode 100644 index 0000000..1b52982 --- /dev/null +++ b/src/main/java/org/nkcoder/config/WebConfig.java @@ -0,0 +1,21 @@ +package org.nkcoder.config; + +import java.util.List; +import org.nkcoder.resolver.CurrentUserArgumentResolver; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; + +@Configuration +public class WebConfig implements WebMvcConfigurer { + private final CurrentUserArgumentResolver currentUserArgumentResolver; + + public WebConfig(CurrentUserArgumentResolver currentUserArgumentResolver) { + this.currentUserArgumentResolver = currentUserArgumentResolver; + } + + @Override + public void addArgumentResolvers(List resolvers) { + resolvers.add(currentUserArgumentResolver); + } +} diff --git a/src/main/java/org/nkcoder/controller/AuthController.java b/src/main/java/org/nkcoder/controller/AuthController.java index b882516..40ae9c6 100644 --- a/src/main/java/org/nkcoder/controller/AuthController.java +++ b/src/main/java/org/nkcoder/controller/AuthController.java @@ -33,7 +33,7 @@ public AuthController(AuthService authService) { @PostMapping("/register") public ResponseEntity> register( @Valid @RequestBody RegisterRequest request) { - logger.info("Registration request for email: {}", request.email()); + logger.debug("Registration request received."); AuthResponse authResponse = authService.register(request); @@ -43,7 +43,7 @@ public ResponseEntity> register( @PostMapping("/login") public ResponseEntity> login(@Valid @RequestBody LoginRequest request) { - logger.info("Login request for email: {}", request.email()); + logger.debug("Login request received."); AuthResponse authResponse = authService.login(request); diff --git a/src/main/java/org/nkcoder/controller/UserController.java b/src/main/java/org/nkcoder/controller/UserController.java index 4f13ffb..96a2a6e 100644 --- a/src/main/java/org/nkcoder/controller/UserController.java +++ b/src/main/java/org/nkcoder/controller/UserController.java @@ -1,8 +1,8 @@ package org.nkcoder.controller; -import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import java.util.UUID; +import org.nkcoder.annotation.CurrentUser; import org.nkcoder.dto.common.ApiResponse; import org.nkcoder.dto.user.ChangePasswordRequest; import org.nkcoder.dto.user.UpdateProfileRequest; @@ -27,11 +27,8 @@ public UserController(UserService userService) { } @GetMapping("/me") - public ResponseEntity> getMe(HttpServletRequest request) { - UUID userId = (UUID) request.getAttribute("userId"); - String email = (String) request.getAttribute("email"); - - logger.info("Get profile request for user: {}", email); + public ResponseEntity> getMe(@CurrentUser UUID userId) { + logger.info("Get profile request for userId: {}", userId); UserResponse userResponse = userService.findById(userId); @@ -41,12 +38,9 @@ public ResponseEntity> getMe(HttpServletRequest reques @PatchMapping("/me") public ResponseEntity> updateMe( - @Valid @RequestBody UpdateProfileRequest request, HttpServletRequest httpRequest) { - - UUID userId = (UUID) httpRequest.getAttribute("userId"); - String email = (String) httpRequest.getAttribute("email"); + @CurrentUser UUID userId, @Valid @RequestBody UpdateProfileRequest request) { - logger.info("Update profile request for user: {}", email); + logger.info("Update profile request for userId: {}", userId); UserResponse userResponse = userService.updateProfile(userId, request); @@ -55,12 +49,9 @@ public ResponseEntity> updateMe( @PatchMapping("/me/password") public ResponseEntity> changeMyPassword( - @Valid @RequestBody ChangePasswordRequest request, HttpServletRequest httpRequest) { - - UUID userId = (UUID) httpRequest.getAttribute("userId"); - String email = (String) httpRequest.getAttribute("email"); + @CurrentUser UUID userId, @Valid @RequestBody ChangePasswordRequest request) { - logger.info("Change password request for user: {}", email); + logger.info("Change password request for userId: {}", userId); userService.changePassword(userId, request); @@ -71,7 +62,7 @@ public ResponseEntity> changeMyPassword( @GetMapping("/{userId}") @PreAuthorize("hasRole('ADMIN')") public ResponseEntity> getUser(@PathVariable UUID userId) { - logger.info("Admin get user request for user: {}", userId); + logger.info("Admin get user request for userId: {}", userId); UserResponse userResponse = userService.findById(userId); @@ -83,7 +74,7 @@ public ResponseEntity> getUser(@PathVariable UUID user public ResponseEntity> updateUser( @PathVariable UUID userId, @Valid @RequestBody UpdateProfileRequest request) { - logger.info("Admin update user request for user: {}", userId); + logger.info("Admin update user request for userId: {}", userId); UserResponse userResponse = userService.updateProfile(userId, request); @@ -93,9 +84,9 @@ public ResponseEntity> updateUser( @PatchMapping("/{userId}/password") @PreAuthorize("hasRole('ADMIN')") public ResponseEntity> changeUserPassword( - @PathVariable UUID userId, @RequestBody ChangePasswordRequest request) { + @PathVariable UUID userId, @Valid @RequestBody ChangePasswordRequest request) { - logger.info("Admin change password request for user: {}", userId); + logger.info("Admin change password request for userId: {}", userId); // For admin, we only use the newPassword field userService.changeUserPassword(userId, request.newPassword()); diff --git a/src/main/java/org/nkcoder/exception/GlobalExceptionHandler.java b/src/main/java/org/nkcoder/exception/GlobalExceptionHandler.java index 8abd242..ecd6f91 100644 --- a/src/main/java/org/nkcoder/exception/GlobalExceptionHandler.java +++ b/src/main/java/org/nkcoder/exception/GlobalExceptionHandler.java @@ -1,8 +1,9 @@ package org.nkcoder.exception; import com.fasterxml.jackson.core.JsonParseException; -import java.util.HashMap; +import java.time.LocalDateTime; import java.util.Map; +import java.util.stream.Collectors; import org.nkcoder.dto.common.ApiResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,20 +52,21 @@ public ResponseEntity> handleAccessDeniedException(AccessDen @ExceptionHandler(MethodArgumentNotValidException.class) public ResponseEntity> handleValidationExceptions( MethodArgumentNotValidException e) { - logger.error("Validation (invalid method argument) error: {}", e.getMessage()); - - Map errors = new HashMap<>(); - e.getBindingResult() - .getAllErrors() - .forEach( - (error) -> { - String fieldName = ((FieldError) error).getField(); - String errorMessage = error.getDefaultMessage(); - errors.put(fieldName, errorMessage); - }); + logger.debug("Validation error: {} field(s) failed", e.getBindingResult().getErrorCount()); + + Map errors = + e.getBindingResult().getFieldErrors().stream() + .collect( + Collectors.toMap( + FieldError::getField, + fieldError -> + fieldError.getDefaultMessage() != null + ? fieldError.getDefaultMessage() + : "Invalid value", + (existing, replacement) -> existing)); return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(ApiResponse.error("Validation failed")); + .body(new ApiResponse<>("Validation failed", errors, LocalDateTime.now())); } @ExceptionHandler(HttpRequestMethodNotSupportedException.class) @@ -75,26 +77,37 @@ public ResponseEntity> handleRequestMethodNotSupportedExcept .body(ApiResponse.error("Method not allowed: " + e.getMessage())); } + @ExceptionHandler(HttpMessageNotReadableException.class) + public ResponseEntity> handleHttpMessageNotReadableException( + HttpMessageNotReadableException e) { + logger.debug("Message not readable: {}", e.getMostSpecificCause().getMessage()); + + String message = "Malformed JSON request"; + Throwable cause = e.getCause(); + if (cause instanceof JsonParseException) { + message = "Invalid JSON format: " + cause.getMessage(); + } + + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.error(message)); + } + + @ExceptionHandler(HttpMediaTypeNotSupportedException.class) + public ResponseEntity> handleHttpMediaTypeNotSupportedException( + HttpMediaTypeNotSupportedException e) { + logger.debug("Unsupported media type: {}", e.getContentType()); + + String message = + String.format( + "Content type '%s' is not supported. Use 'application/json'", e.getContentType()); + return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE) + .body(ApiResponse.error(message)); + } + @ExceptionHandler(Exception.class) public ResponseEntity> handleGenericException(Exception e) { logger.error("Unexpected error: {}", e.getMessage(), e); - if (e instanceof JsonParseException) { - logger.error("JSON parsing error: {}", e.getMessage()); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(ApiResponse.error("Invalid JSON format")); - } - if (e instanceof HttpMediaTypeNotSupportedException) { - logger.error("Unsupported media type: {}", e.getMessage()); - return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE) - .body(ApiResponse.error("Unsupported media type")); - } - if (e instanceof HttpMessageNotReadableException) { - logger.error("Message not readable: {}", e.getMessage()); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(ApiResponse.error("Message not readable")); - } return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(ApiResponse.error("Internal server error")); + .body(ApiResponse.error("An unexpected error occurred. Please try again later.")); } } diff --git a/src/main/java/org/nkcoder/resolver/CurrentUserArgumentResolver.java b/src/main/java/org/nkcoder/resolver/CurrentUserArgumentResolver.java new file mode 100644 index 0000000..b40560a --- /dev/null +++ b/src/main/java/org/nkcoder/resolver/CurrentUserArgumentResolver.java @@ -0,0 +1,47 @@ +package org.nkcoder.resolver; + +import jakarta.servlet.http.HttpServletRequest; +import java.util.UUID; +import org.jetbrains.annotations.NotNull; +import org.nkcoder.annotation.CurrentUser; +import org.springframework.core.MethodParameter; +import org.springframework.stereotype.Component; +import org.springframework.web.bind.support.WebDataBinderFactory; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.method.support.ModelAndViewContainer; + +/** + * Resolves method parameters annotated with {@link CurrentUser} by extracting the user ID from + * request attributes set by {@link org.nkcoder.security.JwtAuthenticationFilter} + */ +@Component +public class CurrentUserArgumentResolver implements HandlerMethodArgumentResolver { + + private static final String USER_ID_ATTRIBUTE = "userId"; + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return parameter.hasParameterAnnotation(CurrentUser.class) + && parameter.getParameterType().equals(UUID.class); + } + + @Override + public Object resolveArgument( + @NotNull MethodParameter parameter, + ModelAndViewContainer mavContainer, + NativeWebRequest webRequest, + WebDataBinderFactory binderFactory) { + HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class); + if (request == null) { + return null; + } + + Object userId = request.getAttribute(USER_ID_ATTRIBUTE); + if (userId instanceof UUID) { + return userId; + } + + return null; + } +} diff --git a/src/test/java/org/nkcoder/controller/UserControllerTest.java b/src/test/java/org/nkcoder/controller/UserControllerTest.java index 622ad2e..3e7f54b 100644 --- a/src/test/java/org/nkcoder/controller/UserControllerTest.java +++ b/src/test/java/org/nkcoder/controller/UserControllerTest.java @@ -235,7 +235,9 @@ void shouldChangeUserPasswordAsAdmin() throws Exception { // Given ChangePasswordRequest request = new ChangePasswordRequest( - "", "NewAdminPassword123!", ""); // Only newPassword is used for admin + "OldPassword123!", + "NewAdminPassword123!", + "NewAdminPassword123!"); // Only newPassword is used for admin doNothing().when(userService).changeUserPassword(testUserId, "NewAdminPassword123!"); // When & Then