마이페이지 CRUD, 독서기록 CRUD, 모든 API 엑세스 토큰 적용, ENUM 정상화#78
Conversation
📝 WalkthroughWalkthroughThis pull request refactors authentication across multiple controller classes to use Spring Security's Changes
Poem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@booklog/src/main/java/com/example/booklog/domain/library/shelves/controller/ReadingLogsController.java`:
- Line 14: The file ReadingLogsController.java contains an invalid/unused import
"import io.swagger.v3.oas.annotations.Parameter.*;" — replace it with the
correct annotation import "io.swagger.v3.oas.annotations.Parameter" if you
intend to use the `@Parameter` annotation, or simply remove the import entirely if
`@Parameter` is not used in ReadingLogsController; update the import statement
accordingly so it is syntactically correct and only present when referenced by
the controller code.
In
`@booklog/src/main/java/com/example/booklog/domain/users/controller/MeProfileController.java`:
- Around line 58-64: Add a specific exception handler method
handleIllegalArgumentException in the GlobalExceptionHandler to catch
IllegalArgumentException and return HTTP 400 with an ApiResponse using
GeneralErrorCode.VALIDATION_ERROR; implement it to log the error (e.g.,
log.error("IllegalArgumentException: {}", e.getMessage())) and return
ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.onFailure(GeneralErrorCode.VALIDATION_ERROR,
e.getMessage())); this ensures validation errors thrown by services like
MeProfileService.updateProfile (e.g., NICKNAME_EMPTY, NICKNAME_TOO_LONG) map to
400 instead of hitting the generic Exception handler.
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/MeAvatarService.java`:
- Around line 44-51: AmazonS3Manager.uploadFile currently swallows IOExceptions
and can return a URL for a non-existent object; change uploadFile to propagate
failures by throwing an unchecked IllegalStateException (include the caught
IOException as the cause and a descriptive message) instead of returning a
fake/silent URL, and then update MeAvatarService (the code around
amazonS3Manager.uploadFile(...) and user.updateProfile(...)) to either let that
exception bubble or catch it and translate it to an application-level error
before persisting the user's profileImageUrl so you never save a broken URL to
the DB.
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/ReadingCalendarService.java`:
- Around line 42-47: The parseYearMonth(String month) method throws
IllegalArgumentException for invalid formats which is currently mapped to HTTP
500 by the generic handler; fix by either adding a specific handler in
GlobalExceptionHandler: add an `@ExceptionHandler`(IllegalArgumentException.class)
method that returns ResponseEntity with HttpStatus.BAD_REQUEST and an
appropriate message, or change parseYearMonth to throw new
ResponseStatusException(HttpStatus.BAD_REQUEST, "month 형식이 올바르지 않습니다. 예)
2026-01") instead of IllegalArgumentException; reference the parseYearMonth
method and GlobalExceptionHandler class to locate where to implement the change.
🧹 Nitpick comments (3)
booklog/src/main/java/com/example/booklog/domain/library/shelves/controller/BookshelvesController.java (1)
90-96: Consider adding@Validannotation for request body validation.The
updatemethod is missing the@Validannotation onBookshelfUpdateRequest, while thecreatemethod (line 46) includes it. This inconsistency could allow invalid data to bypass validation constraints defined inBookshelfUpdateRequest.🔧 Suggested fix
public void update( `@AuthenticationPrincipal` CustomUserDetails userDetails, `@PathVariable`(name = "shelfId") Long shelfId, - `@RequestBody` BookshelfUpdateRequest req + `@RequestBody` `@Valid` BookshelfUpdateRequest req ) {booklog/src/main/java/com/example/booklog/domain/users/service/MeProfileService.java (1)
25-26: Simplify Optional handling.
.orElseGet(() -> null)can be simplified to.orElse(null)sincenullis a constant and doesn't require lazy evaluation.♻️ Suggested simplification
- UserSettings settings = userSettingsRepository.findById(userId) - .orElseGet(() -> null); + UserSettings settings = userSettingsRepository.findById(userId) + .orElse(null);booklog/src/main/java/com/example/booklog/domain/library/shelves/repository/ReadingLogsRepository.java (1)
27-61: Consider indexing to keep the calendar query fast at scale.
The correlated NOT EXISTS per day can get expensive as logs grow; a composite index on(user_book_id, read_date, created_at, id)(and/or a supporting index onuser_book.user_id) will help the optimizer.
| import io.swagger.v3.oas.annotations.media.*; | ||
| import io.swagger.v3.oas.annotations.responses.ApiResponse; | ||
| import io.swagger.v3.oas.annotations.tags.Tag; | ||
| import io.swagger.v3.oas.annotations.Parameter.*; |
There was a problem hiding this comment.
Invalid or unused import statement.
io.swagger.v3.oas.annotations.Parameter is an annotation type, not a class with nested members. The wildcard import Parameter.* is syntactically incorrect for annotations and this import doesn't appear to be used anywhere in the file.
🔧 Suggested fix
-import io.swagger.v3.oas.annotations.Parameter.*;If you intended to use @Parameter annotation for path variables, the correct import would be:
import io.swagger.v3.oas.annotations.Parameter;🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/library/shelves/controller/ReadingLogsController.java`
at line 14, The file ReadingLogsController.java contains an invalid/unused
import "import io.swagger.v3.oas.annotations.Parameter.*;" — replace it with the
correct annotation import "io.swagger.v3.oas.annotations.Parameter" if you
intend to use the `@Parameter` annotation, or simply remove the import entirely if
`@Parameter` is not used in ReadingLogsController; update the import statement
accordingly so it is syntactically correct and only present when referenced by
the controller code.
| @PatchMapping("/profile") | ||
| public MeProfileResponse updateProfile( | ||
| @AuthenticationPrincipal CustomUserDetails userDetails, | ||
| @RequestBody MeProfileUpdateRequest req | ||
| ) { | ||
| return meProfileService.updateProfile(userDetails.getUserId(), req); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ControllerAdvice or ExceptionHandler handling IllegalArgumentException
rg -n -A5 'ControllerAdvice|ExceptionHandler.*IllegalArgumentException' --type=javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 1345
🏁 Script executed:
cat -n booklog/src/main/java/com/example/booklog/global/common/GlobalExceptionHandler.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 3661
🏁 Script executed:
find . -name "MeProfileService.java" -type f | head -20Repository: Project-BookLog/BookLog-BackEnd
Length of output: 161
🏁 Script executed:
cat -n ./booklog/src/main/java/com/example/booklog/domain/users/service/MeProfileService.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 3961
Add specific exception handler for IllegalArgumentException to return HTTP 400 for validation errors.
The service throws IllegalArgumentException for validation errors (NICKNAME_EMPTY, NICKNAME_TOO_LONG) and data errors (USER_NOT_FOUND), but the global GlobalExceptionHandler lacks a specific handler for this exception. Currently, IllegalArgumentException is caught by the generic Exception handler and returns HTTP 500, when validation errors should return HTTP 400. Add an @ExceptionHandler(IllegalArgumentException.class) to map these errors appropriately:
`@ExceptionHandler`(IllegalArgumentException.class)
public ResponseEntity<ApiResponse<String>> handleIllegalArgumentException(IllegalArgumentException e) {
log.error("IllegalArgumentException: {}", e.getMessage());
ApiResponse<String> response = ApiResponse.onFailure(
GeneralErrorCode.VALIDATION_ERROR,
e.getMessage()
);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response);
}🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/users/controller/MeProfileController.java`
around lines 58 - 64, Add a specific exception handler method
handleIllegalArgumentException in the GlobalExceptionHandler to catch
IllegalArgumentException and return HTTP 400 with an ApiResponse using
GeneralErrorCode.VALIDATION_ERROR; implement it to log the error (e.g.,
log.error("IllegalArgumentException: {}", e.getMessage())) and return
ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.onFailure(GeneralErrorCode.VALIDATION_ERROR,
e.getMessage())); this ensures validation errors thrown by services like
MeProfileService.updateProfile (e.g., NICKNAME_EMPTY, NICKNAME_TOO_LONG) map to
400 instead of hitting the generic Exception handler.
| // 2) S3 key 생성 + 업로드 | ||
| String key = amazonS3Manager.generateProfileKeyName(uuid); | ||
| String url = amazonS3Manager.uploadFile(key, file); | ||
|
|
||
| // 3) Users 갱신 (닉네임 유지, 이미지만 변경) | ||
| user.updateProfile(user.getNickname(), url); | ||
|
|
||
| return new MeAvatarUpdateResponse(user.getId(), user.getProfileImageUrl()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "MeAvatarService.java" -o -name "AmazonS3Manager.java"Repository: Project-BookLog/BookLog-BackEnd
Length of output: 232
🏁 Script executed:
git ls-files | grep -E "(MeAvatarService|AmazonS3Manager)" | head -20Repository: Project-BookLog/BookLog-BackEnd
Length of output: 228
🏁 Script executed:
cat -n booklog/src/main/java/com/example/booklog/domain/users/service/MeAvatarService.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 2876
🏁 Script executed:
cat -n booklog/src/main/java/com/example/booklog/aws/s3/AmazonS3Manager.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 2127
Fail fast on S3 upload errors before updating the user.
AmazonS3Manager.uploadFile() (line 30-40) catches IOException silently but still returns a URL to a non-existent object. This allows the service (line 46) to persist a broken profileImageUrl to the database without any error signal.
Required fix: Modify AmazonS3Manager.uploadFile() to throw IllegalStateException on IOException instead of swallowing it:
Required change in AmazonS3Manager
} catch (IOException e){
- log.error("error at AmazonS3Manager uploadFile : {}", (Object) e.getStackTrace());
+ log.error("error at AmazonS3Manager uploadFile", e);
+ throw new IllegalStateException("S3_UPLOAD_FAILED", e);
}Optionally, add defensive error handling in the service layer as well.
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/MeAvatarService.java`
around lines 44 - 51, AmazonS3Manager.uploadFile currently swallows IOExceptions
and can return a URL for a non-existent object; change uploadFile to propagate
failures by throwing an unchecked IllegalStateException (include the caught
IOException as the cause and a descriptive message) instead of returning a
fake/silent URL, and then update MeAvatarService (the code around
amazonS3Manager.uploadFile(...) and user.updateProfile(...)) to either let that
exception bubble or catch it and translate it to an application-level error
before persisting the user's profileImageUrl so you never save a broken URL to
the DB.
| private YearMonth parseYearMonth(String month) { | ||
| try { | ||
| return YearMonth.parse(month); // "YYYY-MM" | ||
| } catch (DateTimeParseException e) { | ||
| throw new IllegalArgumentException("month 형식이 올바르지 않습니다. 예) 2026-01"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for global exception handling / advice.
rg -n "@ControllerAdvice|@RestControllerAdvice|@ExceptionHandler|ResponseStatusException" -g "*.java"Repository: Project-BookLog/BookLog-BackEnd
Length of output: 587
🏁 Script executed:
#!/bin/bash
# Find where ReadingCalendarService is used
rg -n "ReadingCalendarService|parseYearMonth" -g "*.java" -A 2Repository: Project-BookLog/BookLog-BackEnd
Length of output: 2116
🏁 Script executed:
#!/bin/bash
# Look for exception configuration or mapping
rg -n "IllegalArgumentException|exception-mapping|error.handling" -g "*.java" -g "*.properties" -g "*.yml" -g "*.yaml"Repository: Project-BookLog/BookLog-BackEnd
Length of output: 10997
🏁 Script executed:
#!/bin/bash
# Read the GlobalExceptionHandler to see how it handles exceptions
cat -n booklog/src/main/java/com/example/booklog/global/common/GlobalExceptionHandler.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 3661
Add specific handler for IllegalArgumentException to return HTTP 400.
The global exception handler catches all Exception types and returns HTTP 500 (line 62-71 of GlobalExceptionHandler.java). Since IllegalArgumentException is thrown for invalid month format, add a specific @ExceptionHandler(IllegalArgumentException.class) that returns HttpStatus.BAD_REQUEST, or use ResponseStatusException(HttpStatus.BAD_REQUEST, ...) directly in the parseYearMonth method.
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/ReadingCalendarService.java`
around lines 42 - 47, The parseYearMonth(String month) method throws
IllegalArgumentException for invalid formats which is currently mapped to HTTP
500 by the generic handler; fix by either adding a specific handler in
GlobalExceptionHandler: add an `@ExceptionHandler`(IllegalArgumentException.class)
method that returns ResponseEntity with HttpStatus.BAD_REQUEST and an
appropriate message, or change parseYearMonth to throw new
ResponseStatusException(HttpStatus.BAD_REQUEST, "month 형식이 올바르지 않습니다. 예)
2026-01") instead of IllegalArgumentException; reference the parseYearMonth
method and GlobalExceptionHandler class to locate where to implement the change.
#74
#72
#71
#70
#69
#68
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.