-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor: 점수,알림,신고 기능 추가 및 기존 로직 리팩토링 #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough여러 도메인에서 API 응답/오류 포맷 변경, 인증 리프레시 및 익명 등록 리팩터링, 미션 상속 구조 도입과 캘린더/미션 집계 API 추가, 소셜(다이어리/댓글/피드/좋아요/팔로우/방명록) 리팩터링, 이벤트·알림 리스너 확장, 보안 필터 체인 및 테스트 정비가 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AuthCtrl as AuthController
participant AuthSvc as AuthService
participant RTRepo as RefreshTokenRepository
participant UserRepo as UserRepository
Client->>AuthCtrl: POST /auth/refresh (X-Refresh-Token, X-Client-Device-Id)
AuthCtrl->>AuthSvc: refreshAccessToken(token, deviceId)
AuthSvc->>RTRepo: findByToken(token)
alt token not found or expired
AuthSvc-->>AuthCtrl: throw CustomApiException(INVALID_TOKEN)
else valid token
AuthSvc->>UserRepo: findByUuid(uuidFromToken)
UserRepo-->>AuthSvc: User
AuthSvc-->>AuthCtrl: TokenRefreshResponse(newAccessToken[, newRefreshToken])
end
AuthCtrl-->>Client: 200 ApiResponse<TokenRefreshResponse>
sequenceDiagram
autonumber
participant Client
participant GardenCtrl as GardenController
participant GardenSvc as GardenService
participant UserSvc as UserService
participant EventPub as ApplicationEventPublisher
Client->>GardenCtrl: POST /gardens/{id}/water (auth)
GardenCtrl->>GardenSvc: waterGarden(actorId, gardenId)
GardenSvc->>UserSvc: addExperience(actorId, WATERING_POINTS)
alt actor != owner
GardenSvc->>EventPub: publish WateredByFriendEvent(owner, actor)
end
GardenSvc-->>GardenCtrl: void
GardenCtrl-->>Client: 200 ApiResponse<Void>
sequenceDiagram
autonumber
participant Client
participant FeedCtrl as FeedController
participant FeedSvc as FeedService
participant FollowRepo as FollowRepository
participant DiaryRepo as DiaryRepository
participant AvatarRepo as AvatarPostRepository
Client->>FeedCtrl: GET /feed?filter=following (auth)
FeedCtrl->>FeedSvc: getFeed(currentUserUuid, "following")
FeedSvc->>FollowRepo: findByFollower(currentUser)
FeedSvc->>DiaryRepo: findByUserInAndIsPublicIsTrue(following, pageable)
FeedSvc->>AvatarRepo: findByUserIn(following, pageable)
FeedSvc-->>FeedCtrl: List<FeedItemResponse> (merged & sorted)
FeedCtrl-->>Client: 200 ApiResponse<List<FeedItemResponse>>
sequenceDiagram
autonumber
participant Client
participant CommentCtrl as CommentController
participant CommentSvc as CommentService
participant EventPub as ApplicationEventPublisher
participant Listener as NotificationEventListener
participant NotifSvc as NotificationService
Client->>CommentCtrl: POST /comments (auth, body)
CommentCtrl->>CommentSvc: createComment(writerId, request)
CommentSvc->>CommentSvc: persist Comment (Diary/AvatarPost)
CommentSvc->>EventPub: publish CommentCreatedEvent(comment)
EventPub-->>Listener: on CommentCreatedEvent
Listener->>NotifSvc: sendNotification(receiver, type, url, message)
CommentSvc-->>CommentCtrl: CommentResponse
CommentCtrl-->>Client: 200 ApiResponse<CommentResponse>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 62
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
src/main/java/com/example/cp_main_be/domain/social/avatarpost/dto/PostInfoResponse.java (1)
1-16: PostInfoResponse 리팩토링: Comment 엔티티 직접 노출 제거 및 boolean 필드명 정리PostInfoResponse에서 JPA 엔티티
Comment를 그대로 사용하고 있으며, 전용 DTO가 아직 존재하지 않습니다. 이로 인해 다음과 같은 문제가 발생할 수 있습니다.
- 직렬화 시 Jackson 순환 참조 또는
LazyInitializationException예외 위험- 댓글 컬렉션 직렬화 시 N+1/과다 페치로 인한 성능 저하
- boolean 필드명
isBookmarked사용으로 JSON 프로퍼티명 혼동따라서 아래와 같이 필수 리팩터링을 권장합니다.
수정 대상 파일
- src/main/java/com/example/cp_main_be/domain/social/avatarpost/dto/PostInfoResponse.java
제안 Diff
package com.example.cp_main_be.domain.social.avatarpost.dto; -import com.example.cp_main_be.domain.social.comment.domain.Comment; +import com.example.cp_main_be.domain.social.comment.dto.CommentSummaryResponse; import java.util.List; import lombok.Builder; import lombok.Getter; @Getter @Builder public class PostInfoResponse { private String imageUrl; private int likeCount; - private List<Comment> comments; - private boolean isBookmarked; + private List<CommentSummaryResponse> comments; + private boolean bookmarked; }추가 안내
CommentSummaryResponseDTO가 아직 없으므로 아래와 같이 새 파일을 생성하세요.package com.example.cp_main_be.domain.social.comment.dto; import java.time.Instant; import lombok.Builder; import lombok.Getter; @Getter @Builder public class CommentSummaryResponse { private Long id; private Long authorId; private String authorNickname; private String content; private Instant createdAt; }- 만약 클라이언트가
isBookmarked키를 기대하는 경우, 필드명을bookmarked로 변경한 뒤를 추가하여 역호환성을 유지하세요.@JsonProperty("isBookmarked") private boolean bookmarked;- 서비스/레포지토리 레벨에서 DTO 프로젝션 또는 전용 조회로 댓글을 불러온 후, 컨트롤러에서 엔티티가 노출되지 않도록 매핑을 완료해야 합니다.
src/test/java/com/example/cp_main_be/domain/garden/service/GardenServiceTest.java (1)
149-170: GardenService.MAX_GARDEN_COUNT(4)에 맞춰 테스트 경계 수정 필요서비스 구현에서는 MAX_GARDEN_COUNT가 4로 설정되어 있어 현재 텃밭 개수가 4개 이상일 때만 IllegalStateException을 던집니다.
테스트unlockGarden_Fail_MaxGardensReached(src/test/java/.../GardenServiceTest.java:164–170)에서는 현재 텃밭 3개를 가진 사용자에 대해 예외를 기대하고 있어 CI에서 실패하고 있습니다.수정 방향 (택1):
- 테스트 기준 변경: 사용자
gardens리스트를 4개(Garden.builder().slotNumber(1~4))로 초기화하고,
예외 메시지를 “텃밭은 최대 4개까지만 생성할 수 있습니다.”로 검증하도록 수정- 서비스 기준 변경: MAX_GARDEN_COUNT를 3으로 조정(비즈니스 규칙이 3개까지 허용이라면) 후 상수 및 주석, 메시지 동기화
위 중 선택해 주세요.
필요 시 구체적인 패치 예시를 추가로 제안드립니다.src/main/java/com/example/cp_main_be/domain/admin/presentation/AdminController.java (3)
54-61: PathVariable 이름 불일치로 400 오류 발생 가능.매핑은 {dailyMissionMasterId}인데 파라미터 이름은 id입니다. 이름을 맞추거나 name 속성을 명시하세요.
- public ResponseEntity<ApiResponse<AdminResponseDTO.DailyMissionMastersResDTO>> updateDailyMission( - @RequestBody AdminRequestDTO.UpdateMissionRequestDTO requestDTO, @PathVariable Long id) { + public ResponseEntity<ApiResponse<AdminResponseDTO.DailyMissionMastersResDTO>> updateDailyMission( + @RequestBody AdminRequestDTO.UpdateMissionRequestDTO requestDTO, + @PathVariable(name = "dailyMissionMasterId") Long id) {
100-104: 여러 POST/PUT 엔드포인트에서 @RequestBody 누락.JSON 본문 바인딩을 위해 @RequestBody를 추가해 주세요. 가능하면 @Valid도 함께 적용을 권장합니다.
- public ResponseEntity<ApiResponse<QuizOptions>> createQuizOption( - AdminRequestDTO.CreateQuizRequestDTO requestDTO) { + public ResponseEntity<ApiResponse<QuizOptions>> createQuizOption( + @RequestBody @Valid AdminRequestDTO.CreateQuizRequestDTO requestDTO) { ... - public ResponseEntity<ApiResponse<AdminResponseDTO.PlantMasterResDTO>> createNewPlant( - AdminRequestDTO.CreatePlantMasterRequestDTO requestDTO) { + public ResponseEntity<ApiResponse<AdminResponseDTO.PlantMasterResDTO>> createNewPlant( + @RequestBody @Valid AdminRequestDTO.CreatePlantMasterRequestDTO requestDTO) { ... - public ResponseEntity<ApiResponse<AdminResponseDTO.PlantMasterResDTO>> updatePlantMasters( - @PathVariable(name = "plantId") Long plantId, - AdminRequestDTO.UpdatePlantMasterRequestDTO requestDTO) { + public ResponseEntity<ApiResponse<AdminResponseDTO.PlantMasterResDTO>> updatePlantMasters( + @PathVariable(name = "plantId") Long plantId, + @RequestBody @Valid AdminRequestDTO.UpdatePlantMasterRequestDTO requestDTO) {Also applies to: 108-113, 117-123
91-96: @RequestBody 누락 및 메서드명 오타(chage→change) 긴급 수정 필요다음 위치에서 반드시 반영해주세요:
- AdminController.java
changeUserStatus메서드의 두 번째 파라미터에@RequestBody애노테이션 추가- AdminService 인터페이스 (
src/main/java/com/example/cp_main_be/domain/admin/service/AdminService.java)
- 메서드명
chageUserStatus→changeUserStatus로 수정- Service 구현체 (예:
AdminServiceImpl.java)
- 인터페이스 변경에 맞춰 구현체 메서드명도 동일하게 수정
- 테스트 클래스 (
src/test/java/com/example/cp_main_be/domain/admin/service/AdminServiceTest.java)
- 호출부 및 검증 메서드 이름(
chageUserStatus→changeUserStatus) 업데이트예시 diff:
--- a/src/main/java/com/example/cp_main_be/domain/admin/presentation/AdminController.java +++ b/src/main/java/com/example/cp_main_be/domain/admin/presentation/AdminController.java @@ -91,7 +91,8 @@ public class AdminController { @PutMapping("/users/{userId}/status") @Operation(summary = "사용자 상태 변경(ban 등) API") public ResponseEntity<ApiResponse<User>> changeUserStatus( - @PathVariable(name = "userId") Long userId, - AdminRequestDTO.ChangeUserStatusRequestDTO request) { + @PathVariable(name = "userId") Long userId, + @RequestBody AdminRequestDTO.ChangeUserStatusRequestDTO request) { // chageUserStatus → changeUserStatus (오타 수정) - User user = adminService.chageUserStatus(userId, request); + User user = adminService.changeUserStatus(userId, request); return ResponseEntity.ok(ApiResponse.success(user)); }이 변경은 API 바인딩 오류 및 코드 혼선을 방지하기 위해 필수입니다.
src/main/java/com/example/cp_main_be/domain/social/bookmark/domain/Bookmark.java (1)
14-14: 중복 @builder 사용으로 Lombok 오류 가능성클래스 레벨과 생성자 레벨에 @builder가 중복 선언되어 Lombok 처리 시 충돌(컴파일 오류) 가능성이 큽니다. 하나만 남겨 주세요. 생성자 빌더를 유지해 최소 필드만 노출하는 방식을 권장합니다.
적용 예:
@NoArgsConstructor(access = AccessLevel.PROTECTED) -@AllArgsConstructor(access = AccessLevel.PROTECTED) -@Builder +@AllArgsConstructor(access = AccessLevel.PROTECTED) public class Bookmark { ... - @Builder public Bookmark(User user, AvatarPost post) { this.user = user; this.avatarPost = post; }Also applies to: 42-46
src/main/java/com/example/cp_main_be/domain/reports/domain/Reports.java (2)
32-35: ManyToOne에 CascadeType.ALL 적용은 상위 엔터티 삭제 위험
ReportReason는 다대일 참조 대상입니다.Reports삭제가ReportReason삭제로 전파될 수 있어 치명적입니다. 다대일에는 보통 cascade를 두지 않습니다.- @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.ALL) + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "report_reason_id") private ReportReason reason;
53-55: [동일 이슈] ManyToOne(User)에 CascadeType.ALL 적용신고 레코드 삭제가 User 삭제로 전파될 위험이 있습니다. 즉시 제거 바랍니다.
- @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.ALL) + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_id") private User user;검증 스크립트(실제 삭제 호출이 있는지 확인):
#!/bin/bash # Reports 삭제 로직 탐색: cascade 위험도 파악 rg -nP --type=java -C2 '\.delete\(\s*reports?\s*\)|\.deleteById\(|@OnDelete'src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java (2)
37-42: NPE로 테스트 실패: FeedService 의존성(@mock) 누락으로 @Injectmocks 주입 실패파이프라인 로그에 따르면
diaryRepository/followRepository가 null이라getFeed호출 시 NPE가 발생합니다. 현재 테스트는UserRepository만 @mock이고,@InjectMocks로 주입되는FeedService의 나머지 의존성이 모킹되지 않아 NPE가 발생합니다. 최소한 로그에 등장한 리포지토리들을 @mock으로 추가해 주세요.적용 예시(패키지 경로는 실제 코드 기준으로 정정 필요):
@@ @Mock private UserRepository userRepository; + // NOTE: 실제 패키지 경로 확인/정정 필요 + @Mock private com.example.cp_main_be.domain.diary.domain.repository.DiaryRepository diaryRepository; + @Mock private com.example.cp_main_be.domain.social.follow.domain.repository.FollowRepository followRepository; @@ void getFeed_success_noFilter() { @@ - List<FeedItemResponse> feedItems = feedService.getFeed(userUuid, null); + List<FeedItemResponse> feedItems = feedService.getFeed(userUuid, null); // then Assertions.assertNotNull(feedItems);권장 추가: 의존 메서드들이 호출되더라도 빈 컬렉션을 반환하도록 느슨하게 스텁해 NPE를 방지하세요(정확한 메서드명은 FeedService 의존성 확인 후 보완).
// 예시: BDDMockito의 willReturn(emptyList()) 형태로 스텁 // given(diaryRepository.findAllBy...(...)).willReturn(Collections.emptyList());
54-59: following 필터 케이스에서도 동일 NPE 위험 — followRepository 모킹 필수파이프라인 에러대로
followRepository가 null입니다. 위 코멘트의 @mock 추가 및 최소 스텁을 본 테스트에도 동일하게 적용해 주세요. 가능하면 following 필터에 따른 분기 검증(예: follow 기반 조회 메서드가 1회 호출되었는지)까지 포함하면 회귀에 강합니다.적용 예시(메서드명이 다를 수 있어 의사코드 형태):
// then Assertions.assertNotNull(feedItems); // org.mockito.Mockito.verify(followRepository, times(1)) // .findFollowingIdsByUserId(user.getId());src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/presentation/UserDailyMissionController.java (2)
39-45: IDOR 취약점 가능성: 인증 주체 미사용으로 타인의 미션/퀴즈에 접근 가능여러 엔드포인트가
@AuthenticationPrincipal없이userDailyMissionId만으로 리소스에 접근하고 있습니다. 서비스 레이어에서 “해당 미션이 로그인 유저 소유인지”를 항상 검증하지 않으면, ID 추측으로 타인의 미션을 완료/조회/업로드/응답할 수 있는 IDOR가 됩니다. 컨트롤러에서 인증 주체를 받아 서비스로 넘기고, 서비스 내에서도 소유권 검증을 강제하세요.예시(컨트롤러에서 사용자 전달):
- public ResponseEntity<ApiResponse<Void>> completeDailyMission( - @PathVariable Long userDailyMissionId) { - userDailyMissionService.completeDailyMission(userDailyMissionId); + public ResponseEntity<ApiResponse<Void>> completeDailyMission( + @AuthenticationPrincipal User user, + @PathVariable Long userDailyMissionId) { + userDailyMissionService.completeDailyMission(user.getId(), userDailyMissionId); return ResponseEntity.ok(ApiResponse.success(null)); }다른 메서드도 동일하게
@AuthenticationPrincipal User user(또는Long userId)를 받아 서비스에 전달하고, 서비스는 “userId와 userDailyMissionId의 소유 관계”를 검증해야 합니다.Also applies to: 47-54, 56-63, 65-75, 77-84
82-82: 메서드명 오타 수정 필요:summitAnswer→submitAnswer서비스 시그니처와 호출부가 모두
summitAnswer로 잘못 표기되어 있습니다. 아래 위치들을 전역적으로 변경해 주세요.•
src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/service/UserDailyMissionService.java
– 메서드 선언부 (line 63)
•src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/presentation/UserDailyMissionController.java
– 호출부 (line 82)
•src/test/java/com/example/cp_main_be/domain/user_daily_missions/service/UserDailyMissionServiceTest.java
– 주석 처리된 테스트 코드 내 호출부 전부예시 diff:
--- a/src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/service/UserDailyMissionService.java +++ b/src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/service/UserDailyMissionService.java - public Boolean summitAnswer(QuizRequestDTO request, Long userDailyMissionId) { + public Boolean submitAnswer(QuizRequestDTO request, Long userDailyMissionId) {--- a/src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/presentation/UserDailyMissionController.java +++ b/src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/presentation/UserDailyMissionController.java - Boolean result = userDailyMissionService.summitAnswer(request, userDailyMissionId); + Boolean result = userDailyMissionService.submitAnswer(request, userDailyMissionId);--- a/src/test/java/com/example/cp_main_be/domain/user_daily_missions/service/UserDailyMissionServiceTest.java +++ b/src/test/java/com/example/cp_main_be/domain/user_daily_missions/service/UserDailyMissionServiceTest.java -// Boolean result = userDailyMissionService.summitAnswer(request, userDailyMissionId); +// Boolean result = userDailyMissionService.submitAnswer(request, userDailyMissionId);src/main/java/com/example/cp_main_be/domain/social/guestbook/presentation/GuestbookController.java (1)
3-5: 미사용 의존성 제거: UserService 주입 및 임포트 삭제컨트롤러에서
UserService를 사용하지 않습니다. 불필요한 의존성 주입은 테스트/빈 로딩 비용을 늘립니다. 제거를 권장합니다.-import com.example.cp_main_be.domain.member.user.service.UserService; @@ - private final UserService userService;Also applies to: 22-24
src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/service/UserDailyMissionService.java (1)
82-86: 요청 필드명과 비교 대상 불일치로 인한 오답 판정 버그현재 request.getSelectedOptionId()를 optionOrder와 비교하고 있습니다. 이름상 “Id”면 DB PK와 비교하는 것이 자연스럽습니다. 현재 구현은 옵션 순서와 ID가 다를 경우 정답을 찾지 못합니다.
다음과 같이 수정 권장:
- if (quizOptions.getOptionOrder() == request.getSelectedOptionId()) { + if (quizOptions.getId().equals(request.getSelectedOptionId())) { selectedOption = quizOptions; isCorrect = quizOptions.isCorrect(); break; }만약 요청이 “순서 번호”를 담는 구조라면 DTO 필드명을 selectedOptionOrder로 교정하고, 비교도 order 기준으로 일치시키세요.
src/main/java/com/example/cp_main_be/domain/member/notification/presentation/NotificationController.java (1)
49-51: 소유권 검증 로직 추가가 필요합니다Service-Layer에서 알림 소유자를 검증하지 않고 단순히 ID만으로
readNotification을 호출하고 있어, 인증된 사용자가 타인의 알림을 ‘읽음’ 처리할 수 있는 취약점이 확인되었습니다.• NotificationController.java (49–51행)
@AuthenticationPrincipal User user파라미터를 추가하여, 컨트롤러 레벨에서 호출 사용자 정보를 넘겨주세요.
• NotificationService.java (120행)- 기존
public void readNotification(Long notificationId)시그니처를public void readNotification(Long userId, Long notificationId)로 변경하고, 조회한 Notification의receiver.getId()와 전달된userId가 일치하는지 검증하세요.
예)Notification notification = notificationRepository.findById(notificationId) .orElseThrow(() -> new IllegalArgumentException("존재하지 않는 알림입니다.")); if (!notification.getReceiver().getId().equals(userId)) { throw new AccessDeniedException("본인의 알림만 읽음 처리할 수 있습니다."); } notification.read();권장 수정 예시:
@PatchMapping("/{id}/read") - public ResponseEntity<ApiResponse<Void>> readNotification(@PathVariable Long id) { - notificationService.readNotification(id); + public ResponseEntity<ApiResponse<Void>> readNotification( + @AuthenticationPrincipal User user, + @PathVariable Long id) { + notificationService.readNotification(user.getId(), id); return ResponseEntity.ok(ApiResponse.success(null)); }- @Transactional - public void readNotification(Long notificationId) { - Notification notification = notificationRepository.findById(notificationId) - .orElseThrow(...); - notification.read(); - } + @Transactional + public void readNotification(Long userId, Long notificationId) { + Notification notification = notificationRepository.findById(notificationId) + .orElseThrow(() -> new IllegalArgumentException("존재하지 않는 알림입니다.")); + if (!notification.getReceiver().getId().equals(userId)) { + throw new AccessDeniedException("본인의 알림만 읽음 처리할 수 있습니다."); + } + notification.read(); + }위 보완 작업을 통해 인증된 사용자만 자신의 알림을 읽음 처리할 수 있도록 보장해주세요.
src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java (1)
76-90: update 시 저장 누락 및 예외 타입 개선
setContent()후 DB 저장 없이getResponse()로 바로 가고, 내부에서 다시 save를 수행하는 구조는 응집도가 낮습니다. update 경로에서 명시적으로 저장하고,getResponse()는 순수 매핑만 담당하도록 역할을 분리합시다.RuntimeException은 도메인 컨텍스트가 불명확합니다.CommentNotFoundException,CommentAccessDeniedException같은 커스텀 예외로 교체를 권합니다.comment.setContent(request.getContent()); - - return getResponse(comment); + Comment saved = commentRepository.save(comment); + return getResponse(saved);추가로 예외 클래스를 별도 패키지에 정의하는 예시(참고):
public class CommentNotFoundException extends RuntimeException { ... } public class CommentAccessDeniedException extends RuntimeException { ... }
| if (dailyMissionMaster.getMissionType() != MissionType.QUIZ) { | ||
| throw new IllegalArgumentException("퀴즈 타입의 미션에만 선지를 추가할 수 있습니다."); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
퀴즈 타입 가드 추가로 단위 테스트 실패 발생 — 계약 변경 영향 처리 필요
새 가드가 비-QUIZ 미션에 대해 IllegalArgumentException을 던지면서 파이프라인 테스트(“퀴즈 옵션 생성 성공”)가 실패했습니다. 비즈니스 규칙 상 “QUIZ 타입에만 선지 추가”가 올바르다면, 테스트/픽스처를 QUIZ 타입으로 정정해야 하고, 아니라면 서비스 로직을 재고해야 합니다.
권장 대응 옵션:
- 테스트 수정: 테스트 픽스처가 생성하는
DailyMissionMaster.missionType을QUIZ로 세팅. - 예외 표준화: 도메인 예외(예:
InvalidMissionTypeException)로 교체해 에러 코드/메시지 일관성 확보. - 로깅 보강: 타입 불일치 시 미션 ID/타입을 warn 로그로 남겨 운영 추적성 향상.
검증 스크립트(문제 테스트와 픽스처 파악):
#!/bin/bash
# 실패 테스트와 MissionType 설정 코드 위치 찾기
fd -t f -e java | xargs rg -nP 'AdminService(Test)?|createQuizOption|MissionType\.(?!QUIZ)\w+'추가 확인 사항:
QuizOptions가 미션과의 연관을 필수로 요구한다면, 아래처럼 연관 주입을 누락하지 않았는지 점검하세요(필드명이 다를 수 있으니 엔티티 확인 필요).
return QuizOptions.builder() // QuizOptions 퀴즈의 선지
.optionText(requestDTO.getOptionText())
.optionOrder(requestDTO.getOptionOrder())
.isCorrect(requestDTO.isCorrect())
+ .dailyMissionMaster(dailyMissionMaster) // 연관 필드명 예시
.build();- 생성 후 영속화 경로(레포지토리 save 또는 연관 편의 메서드 cascade)가 명확한지 확인하세요. 현재 메서드는 build만 하고 save는 하지 않습니다. 호출부에서 save하지 않는다면 데이터가 유실됩니다.
🧰 Tools
🪛 GitHub Check: Gradle Tests
[failure] 85-85: com.example.cp_main_be.domain.admin.service.AdminServiceTest ► 퀴즈 옵션 생성 성공
Failed test found in:
build/test-results/test/TEST-com.example.cp_main_be.domain.admin.service.AdminServiceTest.xml
Error:
java.lang.IllegalArgumentException: 퀴즈 타입의 미션에만 선지를 추가할 수 있습니다.
🪛 GitHub Actions: Spring Boot CI/CD with AWS
[error] 85-85: 퀴즈 타입의 미션에만 선지를 추가할 수 있습니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/cp_main_be/domain/admin/service/AdminService.java
around lines 84-86, the new guard throws IllegalArgumentException for non-QUIZ
missions causing the "create quiz option success" unit test to fail; to fix,
either update the failing test fixture to set DailyMissionMaster.missionType to
MissionType.QUIZ so it matches the business rule, or replace the thrown
IllegalArgumentException with a domain-specific exception (e.g.,
InvalidMissionTypeException) to standardize error handling and update
callers/tests accordingly; additionally, add a warn log that includes the
mission ID and actual mission type when the guard triggers, and ensure the
created QuizOption entity is persisted (call repository.save or use the owning
entity’s convenience method/cascade) or document that persistence is handled by
the caller.
| public interface DailyQuestionAnswerRepository extends JpaRepository<DailyQuestionAnswer, Long> { | ||
| boolean existsByUserAndAnsweredDate(User user, LocalDate date); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
중복 응답 저장 시 TOCTOU 레이스 가능성에 대한 주의
existsByUserAndAnsweredDate(...) 사전 체크 후 save(...)를 수행하는 패턴은 동시 요청에서 레이스가 발생할 수 있습니다. DB에 (user_id, answered_date) unique 제약이 있다면, 서비스 계층에서 DataIntegrityViolationException을 캐치해 의미 있는 도메인 예외로 변환하는 방식을 병행하세요.
서비스 계층 예시(참고용):
@Transactional
public void saveAnswer(User user, DailyQuestionAnswerRequest req) {
try {
repository.save(new DailyQuestionAnswer(user, req.getQuestion(), req.getAnswer(), LocalDate.now()));
} catch (DataIntegrityViolationException e) {
throw new AlreadyAnsweredTodayException(user.getId(), LocalDate.now());
}
}검증 스크립트(레포 내 사용 위치와 예외 처리 확인):
🏁 Script executed:
#!/bin/bash
# existsByUserAndAnsweredDate 사용 및 예외 처리 여부 탐색
rg -nP -C3 'existsByUserAndAnsweredDate\s*\(' --type=java
rg -nP -C3 'DataIntegrityViolationException|AlreadyAnsweredTodayException' --type=javaLength of output: 5738
🏁 Script executed:
#!/bin/bash
# Inspect DailyQuestionAnswerService class definition
rg -nP -C3 'class DailyQuestionAnswerService' --type=java src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
# Check repository.save usage in DailyQuestionAnswerService
rg -nP -C3 'save\(' --type=java src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
# Inspect DailyQuestionAnswer entity for unique constraints
rg -nP -C3 '@Table' --type=java src/main/java/com/example/cp_main_be/domain/daily_question/domain/DailyQuestionAnswer.java
rg -nP -C3 'unique' --type=java src/main/java/com/example/cp_main_be/domain/daily_question/domain/DailyQuestionAnswer.javaLength of output: 1359
DailyQuestionAnswerService: TOCTOU 방지용 DB 제약 위반 예외 처리 추가 필요
현재 DailyQuestionAnswerService에서는 중복 응답을 방지하기 위해 사전 체크(existsByUserAndAnsweredDate)만 수행하고, 실제 저장 시에는 DB의 unique 제약 위반을 처리하지 않아 동시성 레이스 상황에서 500 에러가 발생할 수 있습니다. 엔티티에는 이미 (user_id, answered_date) 콤보에 대한 unique 제약이 설정되어 있으므로, 저장 시 발생하는 DataIntegrityViolationException을 잡아 의미 있는 도메인 예외로 변환하는 로직을 반드시 추가해야 합니다.
-
확인된 unique 제약:
src/main/java/com/example/cp_main_be/domain/daily_question/domain/DailyQuestionAnswer.java
@Table(... uniqueConstraints = {@UniqueConstraint(columnNames = {"user_id", "answered_date"})})citeturn1fetch3 -
변경 대상 서비스:
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
현재 save 호출 위치(라인 40)에서 예외 처리가 없습니다. citeturn1fetch2 -
적용 예시(diff):
- dailyQuestionAnswerRepository.save(answer);
+ try {
+ dailyQuestionAnswerRepository.save(answer);
+ } catch (DataIntegrityViolationException e) {
+ // 이미 오늘 답변한 경우 도메인 예외로 전환
+ throw new AlreadyAnsweredTodayException(user.getId(), today);
+ }- 추가로 필요한 import:
import org.springframework.dao.DataIntegrityViolationException;
import com.example.cp_main_be.global.exception.AlreadyAnsweredTodayException;위와 같이 수정하면 사전 체크에서도 놓치는 동시성 레이스 상황을 DB 제약 위반으로 잡아 도메인 예외로 처리할 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
around line 40, wrap the repository.save(...) call in a try-catch that catches
org.springframework.dao.DataIntegrityViolationException and throws the domain
AlreadyAnsweredTodayException (import
com.example.cp_main_be.global.exception.AlreadyAnsweredTodayException) to
convert DB unique-constraint violations into a meaningful domain error; add the
two imports and ensure any transaction boundaries still allow the
DataIntegrityViolationException to be caught and translated before propagating.
| @NotBlank(message = "Question cannot be blank") | ||
| private String question; | ||
|
|
||
| @NotNull(message = "Answer cannot be null") | ||
| private AnswerType answer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
요청 스키마 수정 필요: question 텍스트 대신 questionId 수신
서버가 관리하는 질문과 일치 보장이 없으므로, 텍스트를 입력받지 말고 식별자 기반으로 참조하세요.
- @NotBlank(message = "Question cannot be blank")
- private String question;
+ @NotNull(message = "QuestionId cannot be null")
+ private Long questionId;
@@
@NotNull(message = "Answer cannot be null")
private AnswerType answer;🏁 Script executed:
#!/bin/bash
# 컨트롤러/서비스에서 getQuestion() 사용처 확인 -> questionId로 변경 필요 위치 파악
rg -n "getQuestion\(\)" src/main/java --type=java -C2Length of output: 779
질문 입력을 식별자 기반으로 변경하세요
요청 스키마와 서비스 레이어 모두에서 question 텍스트 대신 questionId를 사용해 서버가 관리하는 질문과의 일관성을 보장해야 합니다.
• 수정 위치
- DTO:
src/main/java/com/example/cp_main_be/domain/daily_question/dto/DailyQuestionAnswerRequest.java - 서비스:
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java(35번 라인)
• DTO 수정 예시
- @NotBlank(message = "Question cannot be blank")
- private String question;
+ @NotNull(message = "QuestionId cannot be null")
+ private Long questionId;
@NotNull(message = "Answer cannot be null")
private AnswerType answer;• 서비스 레이어 변경 안내
DailyQuestionAnswerService에QuestionRepository(또는 유사한) 주입- 빌더 호출부에서
requestDto.getQuestion()대신repository.findById(requestDto.getQuestionId()).orElseThrow(...)형태로 질문 엔티티를 조회하여 설정
DailyQuestionAnswer.builder()
.user(user)
- .question(requestDto.getQuestion())
+ .question(questionRepository.findById(requestDto.getQuestionId())
+ .orElseThrow(() -> new EntityNotFoundException("Question not found: " + requestDto.getQuestionId())))
.answer(requestDto.getAnswer())
.answeredDate(today)
.build();위 변경을 통해 클라이언트가 전달한 식별자를 기반으로 서버 내 질문 데이터를 참조하도록 보장해야 합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/daily_question/dto/DailyQuestionAnswerRequest.java
around lines 13-17, replace the String question field with a questionId (e.g.,
Long questionId) and adjust validations (use @NotNull) so the DTO carries the
question identifier; in
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
around line 35, inject the QuestionRepository (or equivalent) into the service,
fetch the Question entity via
repository.findById(requestDto.getQuestionId()).orElseThrow(...) and pass that
entity into the DailyQuestionAnswer.builder() in place of
requestDto.getQuestion(), keeping answer and answeredDate assignments the same
and throwing an appropriate EntityNotFoundException when missing.
| // 사용자가 오늘 이미 답변했는지 확인 | ||
| if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { | ||
| throw new IllegalStateException("User has already answered today's question."); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
동시성 취약점: “존재 확인 후 저장” 경쟁 조건 발생 가능
두 요청이 동시에 존재여부 검사를 통과하면 중복 저장이 가능합니다. DB 수준의 유니크 제약과 예외 전환으로 방지하세요.
- 엔티티(DailyQuestionAnswer)에
(user_id, answered_date)유니크 제약 추가:
// DailyQuestionAnswer.java (예시)
@Entity
@Table(
name = "daily_question_answer",
uniqueConstraints = @UniqueConstraint(columnNames = {"user_id", "answered_date"})
)
public class DailyQuestionAnswer { ... }- 서비스는 “저장 시 충돌”을 처리:
- // 사용자가 오늘 이미 답변했는지 확인
- if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) {
- throw new IllegalStateException("User has already answered today's question.");
- }
+ // DB 유니크 제약 기반으로 충돌 처리
+ if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) {
+ throw new IllegalStateException("User has already answered today's question.");
+ }
@@
- dailyQuestionAnswerRepository.save(answer);
+ try {
+ dailyQuestionAnswerRepository.save(answer);
+ } catch (org.springframework.dao.DataIntegrityViolationException e) {
+ // 유니크 제약 위반 -> 이미 답변
+ throw new IllegalStateException("User has already answered today's question.", e);
+ }Also applies to: 40-41
🏁 Script executed:
#!/bin/bash
# 유니크 제약 존재 여부 확인
rg -n "@Table\([^)]*uniqueConstraints" src/main/java --type=java -C2 | rg -n "DailyQuestionAnswer|user_id|answered_date" -nLength of output: 125
DailyQuestionAnswer 엔티티 클래스에 @Table 어노테이션(유니크 제약) 존재 여부를 확인하기 위해 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# 1. 엔티티 클래스 위치 확인
ENTITY_FILE=$(rg -l "class DailyQuestionAnswer" -g '*.java')
echo "Found entity: $ENTITY_FILE"
# 2. @Table 어노테이션 유무 확인 (uniqueConstraints 포함 여부)
if [ -n "$ENTITY_FILE" ]; then
rg -n "@Table" "$ENTITY_FILE" -C3 || echo "No @Table annotation in $ENTITY_FILE"
else
echo "DailyQuestionAnswer.java 파일을 찾을 수 없습니다."
fiLength of output: 2514
아래 스크립트로 DailyQuestionAnswer 엔티티 클래스에 @Table 어노테이션 및 유니크 제약 설정이 있는지 재확인해 주세요.
🏁 Script executed:
#!/bin/bash
FILE="src/main/java/com/example/cp_main_be/domain/daily_question/domain/DailyQuestionAnswer.java"
echo "🔍 Inspecting $FILE"
# @Table 어노테이션 존재 여부 확인
rg -n "@Table" "$FILE" -C3 || echo "❌ @Table 어노테이션이 없습니다."
# UniqueConstraint 설정 존재 여부 확인
rg -n "UniqueConstraint" "$FILE" -C3 || echo "❌ UniqueConstraint 정의가 없습니다."Length of output: 1049
DB 경쟁 조건 방지: save 호출 시 유니크 제약 위반 예외 처리 추가
현재 DailyQuestionAnswer 엔티티에는 이미 (user_id, answered_date)에 대한 유니크 제약이 설정되어 있으므로(DB 레벨에서 중복을 방지), 서비스 레이어에서 동시성으로 인한 제약 위반 시 이를 잡아내고 적절히 처리해야 합니다.
검토 대상:
- 파일:
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java - 라인: 27–31, 40–41
적용할 변경 사항:
@@ -27,7 +27,15 @@ public void answerDailyQuestion(User user, LocalDate today, DailyQuestionAnswerRequ
- dailyQuestionAnswerRepository.save(answer);
+ try {
+ dailyQuestionAnswerRepository.save(answer);
+ } catch (org.springframework.dao.DataIntegrityViolationException e) {
+ // 유니크 제약 위반 → 이미 답변됨
+ throw new IllegalStateException(
+ "User has already answered today's question.",
+ e
+ );
+ }- 필요한 경우
import org.springframework.dao.DataIntegrityViolationException;추가
위 변경으로 동일 사용자의 동시 요청이 발생해도, DB 제약 위반을 명확히 잡아내고 IllegalStateException으로 전환하여 일관된 예외 처리를 보장할 수 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 사용자가 오늘 이미 답변했는지 확인 | |
| if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { | |
| throw new IllegalStateException("User has already answered today's question."); | |
| } | |
| // Add this import at the top of the file: | |
| import org.springframework.dao.DataIntegrityViolationException; | |
| public void answerDailyQuestion(User user, LocalDate today, DailyQuestionAnswerRequest request) { | |
| // 사용자가 오늘 이미 답변했는지 확인 | |
| if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { | |
| throw new IllegalStateException("User has already answered today's question."); | |
| } | |
| DailyQuestionAnswer answer = DailyQuestionAnswer.of(user, /* 질문 조회 후 매핑 */, request); | |
| try { | |
| // 변경: 직접 save 호출 대신 예외를 잡아 전환 | |
| dailyQuestionAnswerRepository.save(answer); | |
| } catch (DataIntegrityViolationException e) { | |
| // 유니크 제약 위반 → 이미 답변됨 | |
| throw new IllegalStateException( | |
| "User has already answered today's question.", | |
| e | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/daily_question/service/DailyQuestionAnswerService.java
around lines 27–31 and 40–41, the pre-check using existsByUserAndAnsweredDate
can race leading to a DB unique constraint violation on save; update the save
flow to catch org.springframework.dao.DataIntegrityViolationException (add the
import if missing) around the repository.save(...) call and translate that
exception into an IllegalStateException with a clear message like "User has
already answered today's question." so concurrent requests are handled
consistently; keep the initial existsBy... check but add the try/catch to handle
DB-level uniqueness failures.
| DailyQuestionAnswer answer = | ||
| DailyQuestionAnswer.builder() | ||
| .user(user) // 매개변수로 받은 user 객체를 바로 사용 | ||
| .question(requestDto.getQuestion()) | ||
| .answer(requestDto.getAnswer()) | ||
| .answeredDate(today) | ||
| .build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
데이터 무결성: 요청에서 넘어온 ‘question’ 문자열을 그대로 저장하면 안 됩니다
클라이언트가 임의의 질문 내용을 보내 저장할 수 있어 무결성/감사 추적이 깨집니다. 서버가 관리하는 오늘의 질문(또는 questionId)을 기준으로 조회/연결해야 합니다.
- DTO를
questionId로 변경하고, 서비스에서 해당 질문을 조회해 설정하세요.
- .question(requestDto.getQuestion())
- .answer(requestDto.getAnswer())
+ .question(question) // 아래에서 조회한 엔티티
+ .answer(requestDto.getAnswer())서비스 내부(예시):
// DailyQuestion question = dailyQuestionService.getTodayQuestion(); // 또는 repo로 today 조회
// 또는: DailyQuestion question = dailyQuestionRepository.findById(requestDto.getQuestionId()).orElseThrow(...);원하시면 DTO/엔티티/서비스 전반의 변경 패치까지 한 번에 제안 드리겠습니다.
| spring.security.debug=true | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
spring.security.debug 전역 활성화는 운영 환경에서 보안/성능 리스크가 큼 — 프로필로 분리 권장
spring.security.debug=true는 모든 요청에 대해 Security filter chain 동작을 상세 출력하여 토큰/헤더 메타데이터 노출 및 성능 저하 위험이 있습니다. 공용 application.properties가 아닌 개발 전용 프로필로 이동해 주세요.
권장 변경(현재 파일에서 제거):
- spring.security.debug=true추가(새 파일: src/main/resources/application-dev.properties):
spring.security.debug=true
원하시면 dev/test/prod 프로필 파일 분리와 CI/CD에서 SPRING_PROFILES_ACTIVE 세팅 가이드를 함께 드리겠습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spring.security.debug=true | |
| // (Lines 27–28 in src/main/resources/application.properties have been removed) |
🤖 Prompt for AI Agents
In src/main/resources/application.properties around lines 27 to 28, remove the
global setting "spring.security.debug=true" because enabling security debug in
the shared config exposes sensitive headers/tokens and harms performance;
instead, create a new src/main/resources/application-dev.properties file and add
"spring.security.debug=true" there so debug only runs under the dev profile, and
ensure CI/CD or runtime uses SPRING_PROFILES_ACTIVE to select the proper profile
for each environment.
| cloud.aws.region.static=ap-northeast-2 | ||
|
|
||
| spring.security.debug=true | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
org.springframework.security TRACE 전역 설정은 과도함 — 범위를 축소하고 프로필 분리
패키지 전체를 TRACE로 올리면 로그가 폭증하고, 민감 정보가 간접적으로 드러날 가능성이 높습니다. 개발 프로필에서만 제한된 서브패키지에 DEBUG 수준을 적용하는 방식을 권장합니다.
현재 파일에서 제거:
- logging.level.org.springframework.security=TRACE개발 프로필용(예: src/main/resources/application-dev.properties):
# 필요한 영역만 DEBUG로 제한
logging.level.org.springframework.security.web.FilterChainProxy=DEBUG
logging.level.org.springframework.security.authentication=DEBUG
# 필요시 문제 구간만 일시적으로 TRACE
# logging.level.org.springframework.security.web.authentication=TRACE
추가 팁: Authorization 헤더 등의 값이 로그에 찍히지 않도록 로깅 마스킹(예: Logback 터보 필터 또는 로그 마스커) 적용을 검토해 주세요. 필요하시면 샘플 설정을 제공하겠습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logging.level.org.springframework.security=TRACE |
🤖 Prompt for AI Agents
In src/main/resources/application.properties at line 29 there is a global TRACE
for org.springframework.security which is too broad and risky; remove that line
from this global file, create or update
src/main/resources/application-dev.properties to set DEBUG only for specific
security subpackages (e.g., FilterChainProxy and authentication) and reserve
TRACE only for targeted temporary troubleshooting, and consider enabling log
masking/filtering for sensitive headers like Authorization via Logback turbo
filters or a logging masker.
src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java
Show resolved
Hide resolved
| // @Test | ||
| // @DisplayName("일일 미션 목록 조회 성공") | ||
| // void getDailyMissions_Success() { | ||
| // // Given (준비) | ||
| // Long userId = 1L; | ||
| // DailyMissionMaster missionMaster1 = DailyMissionMaster.builder().id(101L).title("미션 | ||
| // 1").build(); | ||
| // DailyMissionMaster missionMaster2 = DailyMissionMaster.builder().id(102L).title("미션 | ||
| // 2").build(); | ||
| // | ||
| // UserDailyMission userMission1 = | ||
| // UserDailyMission.builder().id(1L).dailyMissionMaster(missionMaster1).build(); | ||
| // UserDailyMission userMission2 = | ||
| // UserDailyMission.builder().id(2L).dailyMissionMaster(missionMaster2).build(); | ||
| // | ||
| // List<UserDailyMission> missions = List.of(userMission1, userMission2); | ||
| // | ||
| // given(userDailyMissionRepository.findAllByUser_Id(userId)).willReturn(missions); | ||
| // | ||
| // // When (실행) | ||
| // DailyMissionResponseDTO result = userDailyMissionService.getDailyMissions(userId); | ||
| // | ||
| // // Then (검증) | ||
| // assertThat(result).isNotNull(); | ||
| // assertThat(result.getTodayMissions()).hasSize(2); | ||
| // assertThat(result.getTodayMissions().get(0).getMissionTitle()).isEqualTo("미션 1"); | ||
| // assertThat(result.getTodayMissions().get(1).getMissionTitle()).isEqualTo("미션 2"); | ||
| // | ||
| // verify(userDailyMissionRepository, times(1)).findAllByUser_Id(userId); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
주요 테스트 주석 처리 금지 — @disabled로 관리하고 TODO 사유를 명시하세요
핵심 시나리오(목록 조회, 완료 성공)를 통째로 주석 처리하면 커버리지 급락과 회귀 탐지 실패로 이어집니다. 리팩토링 중이라면 @Disabled("리팩토링: 서브타입 기반 UserDailyMission 변경 대응 예정")으로 비활성화하고, 가능한 빠르게 신규 도메인(이미지/퀴즈 미션 서브타입) 기준으로 재작성해 주세요.
다음 스크립트로 주석 처리된 테스트를 찾고, @Disabled 누락 케이스를 확인할 수 있습니다:
Also applies to: 73-87
🏁 Script executed:
#!/bin/bash
# 주석 처리된 @Test 라인 카운트 및 Disabled 미적용 테스트 찾기
echo "Commented-out @Test occurrences:"
rg -nP '^\s*//\s*@Test' -n src/test | wc -l
echo "Tests lacking @Disabled in this file:"
rg -nP 'class\s+UserDailyMissionServiceTest\b' -n src/test -l | xargs -I{} rg -nP '@Test(?!.*@Disabled)' -n {}Length of output: 442
테스트 주석 처리 금지 — 모든 비활성화는 @Disabled로 관리하고 사유를 명시하세요
다음 파일에서 총 6건의 주석 처리된 @Test 메소드가 @Disabled 없이 주석 처리되어 있습니다. 주석 대신 @Disabled("리팩토링: 서브타입 기반 UserDailyMission 변경 대응 예정")를 메소드 바로 위에 추가하고, 가능한 빠르게 image/quiz 미션 서브타입 기준으로 테스트를 재작성해 주세요.
- 대상 파일:
src/test/java/com/example/cp_main_be/domain/user_daily_missions/service/UserDailyMissionServiceTest.java - 주석 처리된
@Test위치(라인 번호): 42, 73, 109, 159, 200, 237 - 예시 diff:
- // @Test + @Disabled("리팩토링: 서브타입 기반 UserDailyMission 변경 대응 예정") + @Test
- 추가 권장 사항:
- 주석 처리된 테스트는 스크립트로 전체 파일을 한 번 더 점검하여漏れ가 없는지 확인하세요.
- 임시 비활성화가 아닌 경우, 즉시 리팩토링된 로직에 맞춰 테스트를 재작성하여 활성화 상태를 유지해야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @Test | |
| // @DisplayName("일일 미션 목록 조회 성공") | |
| // void getDailyMissions_Success() { | |
| // // Given (준비) | |
| // Long userId = 1L; | |
| // DailyMissionMaster missionMaster1 = DailyMissionMaster.builder().id(101L).title("미션 | |
| // 1").build(); | |
| // DailyMissionMaster missionMaster2 = DailyMissionMaster.builder().id(102L).title("미션 | |
| // 2").build(); | |
| // | |
| // UserDailyMission userMission1 = | |
| // UserDailyMission.builder().id(1L).dailyMissionMaster(missionMaster1).build(); | |
| // UserDailyMission userMission2 = | |
| // UserDailyMission.builder().id(2L).dailyMissionMaster(missionMaster2).build(); | |
| // | |
| // List<UserDailyMission> missions = List.of(userMission1, userMission2); | |
| // | |
| // given(userDailyMissionRepository.findAllByUser_Id(userId)).willReturn(missions); | |
| // | |
| // // When (실행) | |
| // DailyMissionResponseDTO result = userDailyMissionService.getDailyMissions(userId); | |
| // | |
| // // Then (검증) | |
| // assertThat(result).isNotNull(); | |
| // assertThat(result.getTodayMissions()).hasSize(2); | |
| // assertThat(result.getTodayMissions().get(0).getMissionTitle()).isEqualTo("미션 1"); | |
| // assertThat(result.getTodayMissions().get(1).getMissionTitle()).isEqualTo("미션 2"); | |
| // | |
| // verify(userDailyMissionRepository, times(1)).findAllByUser_Id(userId); | |
| // } | |
| @Disabled("리팩토링: 서브타입 기반 UserDailyMission 변경 대응 예정") | |
| @Test | |
| @DisplayName("일일 미션 목록 조회 성공") | |
| void getDailyMissions_Success() { | |
| // Given (준비) | |
| Long userId = 1L; | |
| DailyMissionMaster missionMaster1 = DailyMissionMaster.builder().id(101L).title("미션 1").build(); | |
| DailyMissionMaster missionMaster2 = DailyMissionMaster.builder().id(102L).title("미션 2").build(); | |
| UserDailyMission userMission1 = | |
| UserDailyMission.builder().id(1L).dailyMissionMaster(missionMaster1).build(); | |
| UserDailyMission userMission2 = | |
| UserDailyMission.builder().id(2L).dailyMissionMaster(missionMaster2).build(); | |
| List<UserDailyMission> missions = List.of(userMission1, userMission2); | |
| given(userDailyMissionRepository.findAllByUser_Id(userId)).willReturn(missions); | |
| // When (실행) | |
| DailyMissionResponseDTO result = userDailyMissionService.getDailyMissions(userId); | |
| // Then (검증) | |
| assertThat(result).isNotNull(); | |
| assertThat(result.getTodayMissions()).hasSize(2); | |
| assertThat(result.getTodayMissions().get(0).getMissionTitle()).isEqualTo("미션 1"); | |
| assertThat(result.getTodayMissions().get(1).getMissionTitle()).isEqualTo("미션 2"); | |
| verify(userDailyMissionRepository, times(1)).findAllByUser_Id(userId); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/example/cp_main_be/domain/user_daily_missions/service/UserDailyMissionServiceTest.java
around lines 42 to 71 (and also at the other commented test locations noted: 73,
109, 159, 200, 237), several @Test methods are commented out — replace each
commented-out test with the original @Test method signature left intact and add
@Disabled("리팩토링: 서브타입 기반 UserDailyMission 변경 대응 예정") immediately above the
method (preserve the method body as-is), remove the comment markers so the
method is present but disabled, and run a quick grep/script across the test file
to ensure no other test methods remain commented out instead of being annotated.
| // @Test | ||
| // @DisplayName("퀴즈 정답 제출 - 정답") | ||
| // void summitAnswer_Correct() { | ||
| // // Given | ||
| // Long userDailyMissionId = 1L; | ||
| // Long dailyMissionMasterId = 101L; | ||
| // Long quizId = 201L; | ||
| // int correctAnswerNumber = 2; | ||
| // | ||
| // QuizRequestDTO request = new QuizRequestDTO(); | ||
| // request.setAnswerNumber(correctAnswerNumber); | ||
| // | ||
| // DailyMissionMaster missionMaster = | ||
| // DailyMissionMaster.builder().id(dailyMissionMasterId).build(); | ||
| // UserDailyMission userMission = | ||
| // | ||
| // UserDailyMission.builder().id(userDailyMissionId).dailyMissionMaster(missionMaster).build(); | ||
| // Quiz quiz = Quiz.builder().id(quizId).dailyMissionMaster(missionMaster).build(); | ||
| // | ||
| // List<QuizOptions> options = | ||
| // List.of( | ||
| // QuizOptions.builder().id(301L).quiz(quiz).optionOrder(1).isCorrect(false).build(), | ||
| // QuizOptions.builder().id(302L).quiz(quiz).optionOrder(2).isCorrect(true).build(), | ||
| // QuizOptions.builder().id(303L).quiz(quiz).optionOrder(3).isCorrect(false).build()); | ||
| // | ||
| // given(userDailyMissionRepository.findById(userDailyMissionId)) | ||
| // .willReturn(Optional.of(userMission)); | ||
| // given(quizRepository.findByDailyMissionMaster_Id(dailyMissionMasterId)) | ||
| // .willReturn(Optional.of(quiz)); | ||
| // given(quizOptionsRepository.findAllByQuizId(quizId)).willReturn(options); | ||
| // | ||
| // // When | ||
| // Boolean result = userDailyMissionService.summitAnswer(request, userDailyMissionId); | ||
| // | ||
| // // Then | ||
| // assertThat(result).isTrue(); | ||
| // verify(userDailyMissionRepository, times(1)).findById(userDailyMissionId); | ||
| // verify(quizRepository, times(1)).findByDailyMissionMaster_Id(dailyMissionMasterId); | ||
| // verify(quizOptionsRepository, times(1)).findAllByQuizId(quizId); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("퀴즈 정답 제출 - 오답") | ||
| // void summitAnswer_Incorrect() { | ||
| // // Given | ||
| // Long userDailyMissionId = 1L; | ||
| // Long dailyMissionMasterId = 101L; | ||
| // Long quizId = 201L; | ||
| // int incorrectAnswerNumber = 1; | ||
| // | ||
| // QuizRequestDTO request = new QuizRequestDTO(); | ||
| // request.setAnswerNumber(incorrectAnswerNumber); | ||
| // | ||
| // DailyMissionMaster missionMaster = | ||
| // DailyMissionMaster.builder().id(dailyMissionMasterId).build(); | ||
| // UserDailyMission userMission = | ||
| // | ||
| // UserDailyMission.builder().id(userDailyMissionId).dailyMissionMaster(missionMaster).build(); | ||
| // Quiz quiz = Quiz.builder().id(quizId).dailyMissionMaster(missionMaster).build(); | ||
| // | ||
| // List<QuizOptions> options = | ||
| // List.of( | ||
| // QuizOptions.builder().id(301L).quiz(quiz).optionOrder(1).isCorrect(false).build(), | ||
| // QuizOptions.builder().id(302L).quiz(quiz).optionOrder(2).isCorrect(true).build()); | ||
| // | ||
| // given(userDailyMissionRepository.findById(userDailyMissionId)) | ||
| // .willReturn(Optional.of(userMission)); | ||
| // given(quizRepository.findByDailyMissionMaster_Id(dailyMissionMasterId)) | ||
| // .willReturn(Optional.of(quiz)); | ||
| // given(quizOptionsRepository.findAllByQuizId(quizId)).willReturn(options); | ||
| // | ||
| // // When | ||
| // Boolean result = userDailyMissionService.summitAnswer(request, userDailyMissionId); | ||
| // | ||
| // // Then | ||
| // assertThat(result).isFalse(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("퀴즈 정답 제출 실패 - 퀴즈 없음") | ||
| // void summitAnswer_Fail_QuizNotFound() { | ||
| // // Given | ||
| // Long userDailyMissionId = 1L; | ||
| // Long dailyMissionMasterId = 101L; | ||
| // QuizRequestDTO request = new QuizRequestDTO(); | ||
| // request.setAnswerNumber(1); | ||
| // | ||
| // DailyMissionMaster missionMaster = | ||
| // DailyMissionMaster.builder().id(dailyMissionMasterId).build(); | ||
| // UserDailyMission userMission = | ||
| // | ||
| // UserDailyMission.builder().id(userDailyMissionId).dailyMissionMaster(missionMaster).build(); | ||
| // | ||
| // given(userDailyMissionRepository.findById(userDailyMissionId)) | ||
| // .willReturn(Optional.of(userMission)); | ||
| // given(quizRepository.findByDailyMissionMaster_Id(dailyMissionMasterId)) | ||
| // .willReturn(Optional.empty()); | ||
| // | ||
| // // When & Then | ||
| // RuntimeException exception = | ||
| // assertThrows( | ||
| // RuntimeException.class, | ||
| // () -> { | ||
| // userDailyMissionService.summitAnswer(request, userDailyMissionId); | ||
| // }); | ||
| // | ||
| // assertThat(exception.getMessage()).isEqualTo("퀴즈가 존재하지 않습니다."); | ||
| // } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
퀴즈 제출 테스트 전부 주석 처리 — 정답/오답/퀴즈없음 분기 재구축 필요
타입드 서브타입(Quiz/Image) 도입으로 인해 기존 로직이 변경되었다면, 새로운 모델에 맞춰 파라미터와 리포지토리 호출을 재편성해 주세요. 파라미터화 테스트(정답/오답)를 권장합니다.
원하시면 신규 도메인 구조(예: UserQuizMission) 기준의 테스트 스켈레톤을 만들어 드리겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
| public TokenRefreshResponse refreshAccessToken(String refreshToken) { | ||
| // Refresh Token 유효성 검사 | ||
| if (!jwtTokenProvider.validateToken(refreshToken)) { | ||
| throw new RuntimeException("Invalid Refresh Token"); // TODO: Custom Exception | ||
| // 명확한 예외를 던져 클라이언트가 재로그인(닉네임 입력)하도록 유도 | ||
| throw new CustomApiException(ErrorCode.INVALID_TOKEN); | ||
| } | ||
|
|
||
| String uuid = jwtTokenProvider.getUuidFromToken(refreshToken); | ||
|
|
||
| // 사용자 존재 여부 확인 (선택 사항, Refresh Token이 유효하면 사용자도 유효하다고 가정할 수 있음) | ||
| userRepository | ||
| .findByUuid(java.util.UUID.fromString(uuid)) | ||
| .orElseThrow(() -> new RuntimeException("User not found")); // TODO: Custom Exception | ||
|
|
||
| // 새로운 Access Token과 Refresh Token 모두 발급 | ||
| // [수정] 리프레시 토큰은 만료 기간이 길기 때문에 보통 새로 발급하지 않거나, | ||
| // 클라이언트와 협의하여 선택적으로 재발급(Rotation)할 수 있습니다. 여기서는 Access Token만 재발급합니다. | ||
| String newAccessToken = jwtTokenProvider.generateAccessToken(uuid); | ||
| String newRefreshToken = jwtTokenProvider.generateRefreshToken(uuid); | ||
|
|
||
| return new TokenRefreshResponse(newAccessToken, newRefreshToken, false); | ||
| return new TokenRefreshResponse(newAccessToken, refreshToken, false); // 기존 리프레시 토큰 반환 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삭제/정지 사용자, 존재하지 않는 사용자에 대한 리프레시 토큰 갱신 우발 허용
현재 유효성 검증 후 곧바로 uuid로 액세스 토큰을 재발급합니다. 해당 uuid의 사용자가 삭제/정지 상태거나 존재하지 않아도 토큰이 발급될 수 있습니다. 최소한 사용자 존재 및 상태 검증을 추가하세요.
public TokenRefreshResponse refreshAccessToken(String refreshToken) {
if (!jwtTokenProvider.validateToken(refreshToken)) {
// 명확한 예외를 던져 클라이언트가 재로그인(닉네임 입력)하도록 유도
throw new CustomApiException(ErrorCode.INVALID_TOKEN);
}
String uuid = jwtTokenProvider.getUuidFromToken(refreshToken);
+ // 사용자 존재/상태 확인 (삭제/정지 계정 차단)
+ User user = userRepository
+ .findByUuid(java.util.UUID.fromString(uuid))
+ .orElseThrow(() -> new CustomApiException(ErrorCode.USER_NOT_FOUND));
+ if (user.isBlockedOrDeactivated != null && user.isBlockedOrDeactivated()) {
+ throw new CustomApiException(ErrorCode.FORBIDDEN_USER); // 프로젝트의 적절한 에러코드로 대체
+ }
// [수정] 리프레시 토큰은 만료 기간이 길기 때문에 보통 새로 발급하지 않거나,
// 클라이언트와 협의하여 선택적으로 재발급(Rotation)할 수 있습니다. 여기서는 Access Token만 재발급합니다.
- String newAccessToken = jwtTokenProvider.generateAccessToken(uuid);
+ String newAccessToken = jwtTokenProvider.generateAccessToken(uuid);
return new TokenRefreshResponse(newAccessToken, refreshToken, false); // 기존 리프레시 토큰 반환
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public TokenRefreshResponse refreshAccessToken(String refreshToken) { | |
| // Refresh Token 유효성 검사 | |
| if (!jwtTokenProvider.validateToken(refreshToken)) { | |
| throw new RuntimeException("Invalid Refresh Token"); // TODO: Custom Exception | |
| // 명확한 예외를 던져 클라이언트가 재로그인(닉네임 입력)하도록 유도 | |
| throw new CustomApiException(ErrorCode.INVALID_TOKEN); | |
| } | |
| String uuid = jwtTokenProvider.getUuidFromToken(refreshToken); | |
| // 사용자 존재 여부 확인 (선택 사항, Refresh Token이 유효하면 사용자도 유효하다고 가정할 수 있음) | |
| userRepository | |
| .findByUuid(java.util.UUID.fromString(uuid)) | |
| .orElseThrow(() -> new RuntimeException("User not found")); // TODO: Custom Exception | |
| // 새로운 Access Token과 Refresh Token 모두 발급 | |
| // [수정] 리프레시 토큰은 만료 기간이 길기 때문에 보통 새로 발급하지 않거나, | |
| // 클라이언트와 협의하여 선택적으로 재발급(Rotation)할 수 있습니다. 여기서는 Access Token만 재발급합니다. | |
| String newAccessToken = jwtTokenProvider.generateAccessToken(uuid); | |
| String newRefreshToken = jwtTokenProvider.generateRefreshToken(uuid); | |
| return new TokenRefreshResponse(newAccessToken, newRefreshToken, false); | |
| return new TokenRefreshResponse(newAccessToken, refreshToken, false); // 기존 리프레시 토큰 반환 | |
| } | |
| public TokenRefreshResponse refreshAccessToken(String refreshToken) { | |
| if (!jwtTokenProvider.validateToken(refreshToken)) { | |
| // 명확한 예외를 던져 클라이언트가 재로그인(닉네임 입력)하도록 유도 | |
| throw new CustomApiException(ErrorCode.INVALID_TOKEN); | |
| } | |
| String uuid = jwtTokenProvider.getUuidFromToken(refreshToken); | |
| // 사용자 존재/상태 확인 (삭제/정지 계정 차단) | |
| User user = userRepository | |
| .findByUuid(java.util.UUID.fromString(uuid)) | |
| .orElseThrow(() -> new CustomApiException(ErrorCode.USER_NOT_FOUND)); | |
| if (user.isBlockedOrDeactivated()) { | |
| throw new CustomApiException(ErrorCode.FORBIDDEN_USER); // 프로젝트의 적절한 에러코드로 대체 | |
| } | |
| // [수정] 리프레시 토큰은 만료 기간이 길기 때문에 보통 새로 발급하지 않거나, | |
| // 클라이언트와 협의하여 선택적으로 재발급(Rotation)할 수 있습니다. 여기서는 Access Token만 재발급합니다. | |
| String newAccessToken = jwtTokenProvider.generateAccessToken(uuid); | |
| return new TokenRefreshResponse(newAccessToken, refreshToken, false); // 기존 리프레시 토큰 반환 | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/auth/service/AuthService.java
around lines 25 to 38, the refreshAccessToken path validates the token but does
not verify that the user identified by the extracted uuid exists and is active;
add a lookup of the member (e.g., memberRepository.findByUuid(...) or via
MemberService), check for existence and that the member is not deleted or
suspended, and if the check fails throw a suitable CustomApiException/ErrorCode
(e.g., USER_NOT_FOUND or USER_INACTIVE); only after passing these checks
generate and return the new access token (keeping the refresh token behavior
unchanged).
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.security.core.context.SecurityContextHolder; | ||
| import org.springframework.security.core.annotation.AuthenticationPrincipal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
@AuthenticationPrincipal로 도메인 User를 직접 주입하는 방식, 구성 확인 필요
보안 설정에서 Authentication의 principal이 실제로 com.example.cp_main_be.domain.member.user.domain.User 인스턴스로 설정되어 있지 않다면 런타임 바인딩 실패 또는 null 주입 가능성이 있습니다. 커스텀 Principal을 사용 중이면 @AuthenticationPrincipal(expression = "user") 또는 별도의 DTO/Principal 타입을 권장합니다. 서비스 레이어에는 가급적 엔티티 자체보다는 식별자(Long id 또는 UUID)를 넘기는 편이 영속성 컨텍스트/세션 분리 문제를 줄입니다.
실제 principal 타입과 사용 현황 점검 스크립트입니다. 결과를 공유해 주시면 후속 제안 드리겠습니다.
Also applies to: 35-35, 43-43, 52-52, 59-59, 70-70
🏁 Script executed:
#!/bin/bash
# principal 타입 수집
rg -nP '@AuthenticationPrincipal\s+([A-Za-z0-9_$.]+)' -S -C1
# User 엔티티가 UserDetails 구현 여부 확인
rg -nP '(class|record)\s+User\b.*implements\s+UserDetails\b' -S
# 커스텀 Principal/Adapter 추적
rg -nP '(implements|extends)\s+UserDetails|AuthenticationPrincipal|Principal' -S -C2
# Security 설정에서 Authentication 생성 지점 추적(JWT/세션)
rg -nP 'Authentication\(|setAuthentication\(|UsernamePasswordAuthenticationToken' -S -C3Length of output: 55301
SecurityContext Principal 주입 검증 및 개선 제안
-
JwtAuthenticationFilter 검증
JwtAuthenticationFilter.java에서방식으로 JPAUsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken(user, ..., authorities); SecurityContextHolder.getContext().setAuthentication(authentication);
User엔티티를 principal로 직접 설정하고 있어,@AuthenticationPrincipal User user파라미터 주입은 정상 동작함.UserDetails미구현 여부와 무관하게, 커스텀 JWT 필터에서 직접 바인딩하므로 런타임 바인딩 실패 이슈는 없음.
-
Long 타입 주입 문제
- 아래 컨트롤러들은
@AuthenticationPrincipal Long userId를 사용 중이나, 실제 principal은User객체이므로 null 또는 주입 실패가 발생할 수 있음:
•src/main/java/com/example/cp_main_be/domain/reports/presentation/ReportController.java(23행)
•src/main/java/com/example/cp_main_be/domain/garden/garden/presentation/GardenController.java(35, 43행)
- 아래 컨트롤러들은
-
개선 방안
- 컨트롤러 레벨
@AuthenticationPrincipal(expression = "id") Long userId로 변경하여 principal 내부의id필드를 직접 주입하거나,@AuthenticationPrincipal User user를 주입한 뒤user.getId()호출 방식으로 통일
- 서비스 레이어
- 엔티티 대신 식별자만 전달하여 영속성 컨텍스트/세션 분리 문제 완화
- 컨트롤러 레벨
-
추가 권장
- 필요한 사용자 정보(id, username 등)만 담은
UserPrincipal또는PrincipalDTO클래스를 도입해 컨트롤러·서비스 사이에서 사용하면 유지보수성과 안정성을 더욱 높일 수 있습니다.
- 필요한 사용자 정보(id, username 등)만 담은
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/presentation/UserController.java
(around line 14) and in the controllers flagged:
src/main/java/com/example/cp_main_be/domain/reports/presentation/ReportController.java
(around line 22) and
src/main/java/com/example/cp_main_be/domain/garden/garden/presentation/GardenController.java
(around lines 33–43), change the incorrect principal injection of type Long to
reliably extract the id from the bound User principal: either update controller
method parameters from @AuthenticationPrincipal Long userId to
@AuthenticationPrincipal(expression = "id") Long userId, or inject the User
entity via @AuthenticationPrincipal User user and then call user.getId(); also
ensure downstream service signatures accept and use the userId (Long) instead of
the JPA entity to avoid persistence/session coupling, and consider introducing a
lightweight UserPrincipal/PrincipalDTO for controllers if you want to avoid
exposing the JPA entity.
| public void addExperience(Long actorId, int points) { | ||
| User user = userRepository.findById(actorId).get(); | ||
| user.addExperience(points); | ||
| levelService.checkLevelUp(user); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional.get() 직접 호출 지양: 사용자 미존재 시 명시적 예외 반환
findById(...).get()은 미존재 시 NoSuchElementException으로 변환되어 일관된 예외 처리가 어려워집니다. 기존 패턴과 동일하게 UserNotFoundException을 던지도록 수정하세요.
- public void addExperience(Long actorId, int points) {
- User user = userRepository.findById(actorId).get();
+ public void addExperience(Long actorId, int points) {
+ User user =
+ userRepository
+ .findById(actorId)
+ .orElseThrow(() -> new UserNotFoundException("사용자를 찾을 수 없습니다."));
user.addExperience(points);
levelService.checkLevelUp(user);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void addExperience(Long actorId, int points) { | |
| User user = userRepository.findById(actorId).get(); | |
| user.addExperience(points); | |
| levelService.checkLevelUp(user); | |
| } | |
| public void addExperience(Long actorId, int points) { | |
| User user = | |
| userRepository | |
| .findById(actorId) | |
| .orElseThrow(() -> new UserNotFoundException("사용자를 찾을 수 없습니다.")); | |
| user.addExperience(points); | |
| levelService.checkLevelUp(user); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around lines 24 to 28, avoid calling findById(...).get(); instead retrieve the
user with userRepository.findById(actorId).orElseThrow(() -> new
UserNotFoundException(actorId)) so a consistent UserNotFoundException is thrown
when the user is missing, then call user.addExperience(points) and
levelService.checkLevelUp(user) as before; follow the existing project pattern
for the exception type and message.
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | ||
| Object principal = authentication.getPrincipal(); | ||
|
|
||
| // Principal이 User 객체인 경우 (현재 JWT 필터에서 이렇게 저장함) | ||
| if (principal instanceof User) { | ||
| return (User) principal; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Null/익명 인증 가드 추가
인증 객체가 없거나 익명인 경우 NPE 또는 오동작 가능성이 있습니다. 초기에 가드를 두는 것을 권장합니다.
- Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
- Object principal = authentication.getPrincipal();
+ Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+ if (authentication == null || !authentication.isAuthenticated()) {
+ throw new IllegalArgumentException("인증 정보를 찾을 수 없습니다.");
+ }
+ Object principal = authentication.getPrincipal();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| Object principal = authentication.getPrincipal(); | |
| // Principal이 User 객체인 경우 (현재 JWT 필터에서 이렇게 저장함) | |
| if (principal instanceof User) { | |
| return (User) principal; | |
| } | |
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| if (authentication == null || !authentication.isAuthenticated()) { | |
| throw new IllegalArgumentException("인증 정보를 찾을 수 없습니다."); | |
| } | |
| Object principal = authentication.getPrincipal(); | |
| // Principal이 User 객체인 경우 (현재 JWT 필터에서 이렇게 저장함) | |
| if (principal instanceof User) { | |
| return (User) principal; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/user/service/UserService.java
around lines 85 to 92, add a guard to handle missing or anonymous Authentication
before accessing principal: check if SecurityContextHolder.getContext() or
getAuthentication() is null, and if authentication.isAuthenticated() is false or
the principal equals the typical anonymous marker (e.g. "anonymousUser"), then
return an appropriate response (either null or throw a specific Unauthenticated
exception) instead of proceeding; only cast and return (User) principal after
these null/anonymous checks.
| @Inheritance(strategy = InheritanceType.JOINED) // 1. 상속 전략 설정 (조인 전략) | ||
| @DiscriminatorColumn(name = "mission_type") // 2. 타입을 구분할 컬럼 | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Setter | ||
| @Builder | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class UserDailyMission { | ||
| public abstract class UserDailyMission { // 3. 추상 클래스로 변경 | ||
| @Id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
클래스 레벨 @Setter 제거 및 불변성 강화 필요
엔티티 전체에 @Setter를 부여하면 ID/연관관계/감사 필드까지 외부에서 임의 변경이 가능해집니다. 이는 JPA 더티체킹 오작동, 일관성 붕괴(예: isCompleted만 true로 바뀌고 completedAt은 null)로 이어질 수 있습니다. 클래스 레벨 @Setter를 제거하고, 의도된 변경만을 수행하는 도메인 메서드로 한정하세요. 또한 조회/집계 최적화를 위해 인덱스를 추가하는 것을 권장합니다.
권장 변경:
-@NoArgsConstructor(access = AccessLevel.PROTECTED)
-@Setter
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
+@Table(
+ name = "user_daily_mission",
+ indexes = {
+ @Index(name = "idx_udm_created_at", columnList = "created_at"),
+ @Index(name = "idx_udm_user", columnList = "user_id")
+ }
+)
public abstract class UserDailyMission {도메인 메서드 추가(파일 하단에 추가):
+ /**
+ * 미션 완료 처리: 점수/완료시각/완료플래그를 원자적으로 설정합니다.
+ */
+ public void markCompleted(long score) {
+ if (score < 0) throw new IllegalArgumentException("score must be non-negative");
+ this.score = score;
+ this.isCompleted = true;
+ this.completedAt = LocalDateTime.now();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Inheritance(strategy = InheritanceType.JOINED) // 1. 상속 전략 설정 (조인 전략) | |
| @DiscriminatorColumn(name = "mission_type") // 2. 타입을 구분할 컬럼 | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @Setter | |
| @Builder | |
| @AllArgsConstructor | |
| @NoArgsConstructor | |
| public class UserDailyMission { | |
| public abstract class UserDailyMission { // 3. 추상 클래스로 변경 | |
| @Id | |
| // --- around lines 11–16 in UserDailyMission.java --- | |
| @Inheritance(strategy = InheritanceType.JOINED) // 1. 상속 전략 설정 (조인 전략) | |
| @DiscriminatorColumn(name = "mission_type") // 2. 타입을 구분할 컬럼 | |
| -@NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @Table( | |
| name = "user_daily_mission", | |
| indexes = { | |
| @Index(name = "idx_udm_created_at", columnList = "created_at"), | |
| @Index(name = "idx_udm_user", columnList = "user_id") | |
| } | |
| ) | |
| public abstract class UserDailyMission { // 3. 추상 클래스로 변경 | |
| @Id | |
| // … |
| @Inheritance(strategy = InheritanceType.JOINED) // 1. 상속 전략 설정 (조인 전략) | |
| @DiscriminatorColumn(name = "mission_type") // 2. 타입을 구분할 컬럼 | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @Setter | |
| @Builder | |
| @AllArgsConstructor | |
| @NoArgsConstructor | |
| public class UserDailyMission { | |
| public abstract class UserDailyMission { // 3. 추상 클래스로 변경 | |
| @Id | |
| // --- add at the bottom of UserDailyMission.java, inside the class --- | |
| /** | |
| * 미션 완료 처리: 점수/완료시각/완료플래그를 원자적으로 설정합니다. | |
| */ | |
| public void markCompleted(long score) { | |
| if (score < 0) { | |
| throw new IllegalArgumentException("score must be non-negative"); | |
| } | |
| this.score = score; | |
| this.isCompleted = true; | |
| this.completedAt = LocalDateTime.now(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/mission/user_daily_mission/domain/UserDailyMission.java
around lines 11 to 16, remove the class-level @Setter to prevent uncontrolled
mutation of ID/relations/audit fields; instead make fields private (no public
setters) and expose only specific domain methods (e.g., markCompleted(),
reopen(), updateProgress(...)) at the bottom of the file that update related
fields together (e.g., set isCompleted and completedAt atomically); add targeted
setters only where absolutely required (protected/package-private) for JPA, and
add appropriate @Index annotations on columns used in lookups/aggregations to
optimize queries.
| UUID uuid = UUID.fromString(uuidStr); | ||
|
|
||
| User user = | ||
| userRepository | ||
| .findByUuid(uuid) | ||
| .orElseThrow( | ||
| () -> new UsernameNotFoundException("User not found with uuid: " + uuid)); | ||
|
|
||
| UsernamePasswordAuthenticationToken authentication = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
잘못된 UUID/미존재 사용자 처리 시 체인 지속 보장
UUID.fromString 예외나 미존재 사용자에서 예외를 던지면 필터 체인이 중단되어 전역 예외 처리 의도와 달라질 수 있습니다. 인증 미설정 상태로 체인을 지속하는 방식을 권장합니다.
- String uuidStr = jwtTokenProvider.getUuidFromToken(token); // uuid를 가져오는 로직이 필요합니다.
- UUID uuid = UUID.fromString(uuidStr);
-
- User user =
- userRepository
- .findByUuid(uuid)
- .orElseThrow(
- () -> new UsernameNotFoundException("User not found with uuid: " + uuid));
+ String uuidStr = jwtTokenProvider.getUuidFromToken(token);
+ UUID uuid;
+ try {
+ uuid = UUID.fromString(uuidStr);
+ } catch (IllegalArgumentException e) {
+ logger.debug("Invalid UUID format extracted from token.");
+ filterChain.doFilter(request, response);
+ return;
+ }
+
+ User user =
+ userRepository
+ .findByUuid(uuid)
+ .orElse(null);
+ if (user == null) {
+ logger.debug("User not found for uuid: {}", uuid);
+ filterChain.doFilter(request, response);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UUID uuid = UUID.fromString(uuidStr); | |
| User user = | |
| userRepository | |
| .findByUuid(uuid) | |
| .orElseThrow( | |
| () -> new UsernameNotFoundException("User not found with uuid: " + uuid)); | |
| UsernamePasswordAuthenticationToken authentication = | |
| String uuidStr = jwtTokenProvider.getUuidFromToken(token); | |
| UUID uuid; | |
| try { | |
| uuid = UUID.fromString(uuidStr); | |
| } catch (IllegalArgumentException e) { | |
| logger.debug("Invalid UUID format extracted from token."); | |
| filterChain.doFilter(request, response); | |
| return; | |
| } | |
| User user = | |
| userRepository | |
| .findByUuid(uuid) | |
| .orElse(null); | |
| if (user == null) { | |
| logger.debug("User not found for uuid: {}", uuid); | |
| filterChain.doFilter(request, response); | |
| return; | |
| } | |
| UsernamePasswordAuthenticationToken authentication = |
🤖 Prompt for AI Agents
In src/main/java/com/example/cp_main_be/global/jwt/JwtAuthenticationFilter.java
around lines 45 to 53, avoid throwing on invalid UUID or missing user which
breaks the filter chain; instead wrap UUID.fromString and
userRepository.findByUuid in a try/catch (catch IllegalArgumentException and
UsernameNotFoundException / NoSuchElement) and when either occurs log the
problem, do not set authentication, call filterChain.doFilter(request, response)
and return so the chain continues with an unauthenticated context; if both
succeed, proceed to create and set the UsernamePasswordAuthenticationToken as
before.
| import com.example.cp_main_be.domain.social.avatarpost.domain.repository.AvatarPostRepository; | ||
| import com.example.cp_main_be.domain.social.comment.domain.Comment; | ||
| import com.example.cp_main_be.domain.social.diary.domain.Diary; | ||
| import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
패키지 경로 대소문자 불일치
Repository 디렉토리가 대문자로 시작하는데, Java 패키지 명명 규칙에 따르면 소문자를 사용해야 합니다.
-import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository;
+import com.example.cp_main_be.domain.social.diary.domain.repository.DiaryRepository;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository; | |
| import com.example_cp_main_be.domain.social.diary.domain.repository.DiaryRepository; |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/global/listener/NotificationEventListener.java
around line 10, the import uses a package path with "Repository" starting with
an uppercase letter which violates Java package naming conventions; rename the
directory from "Repository" to "repository" and update the import to
com.example.cp_main_be.domain.social.diary.domain.repository.DiaryRepository,
then update the package declarations and all imports in any classes under that
directory (and any references across the project) to the new lowercase package
name and rebuild to ensure no remaining references to the old path.
| if (Objects.equals(targetType, "DIARY")) { | ||
| receiver = diaryRepository.findById(targetId).get().getUser(); | ||
| url = "/api/v1/diaries/" + targetId; | ||
| } else { | ||
| receiver = avatarPostRepository.findById(targetId).get().getUser(); | ||
| url = "/api/v1/avatar-posts/" + targetId; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional 처리 시 예외 처리 필요
findById(targetId).get()을 직접 호출하면 해당 엔티티가 없을 때 NoSuchElementException이 발생합니다. 적절한 예외 처리가 필요합니다.
if (Objects.equals(targetType, "DIARY")) {
- receiver = diaryRepository.findById(targetId).get().getUser();
+ Diary diary = diaryRepository.findById(targetId)
+ .orElseThrow(() -> new IllegalArgumentException("일기를 찾을 수 없습니다: " + targetId));
+ receiver = diary.getUser();
url = "/api/v1/diaries/" + targetId;
} else {
- receiver = avatarPostRepository.findById(targetId).get().getUser();
+ AvatarPost avatarPost = avatarPostRepository.findById(targetId)
+ .orElseThrow(() -> new IllegalArgumentException("아바타 포스트를 찾을 수 없습니다: " + targetId));
+ receiver = avatarPost.getUser();
url = "/api/v1/avatar-posts/" + targetId;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Objects.equals(targetType, "DIARY")) { | |
| receiver = diaryRepository.findById(targetId).get().getUser(); | |
| url = "/api/v1/diaries/" + targetId; | |
| } else { | |
| receiver = avatarPostRepository.findById(targetId).get().getUser(); | |
| url = "/api/v1/avatar-posts/" + targetId; | |
| } | |
| if (Objects.equals(targetType, "DIARY")) { | |
| Diary diary = diaryRepository.findById(targetId) | |
| .orElseThrow(() -> new IllegalArgumentException("일기를 찾을 수 없습니다: " + targetId)); | |
| receiver = diary.getUser(); | |
| url = "/api/v1/diaries/" + targetId; | |
| } else { | |
| AvatarPost avatarPost = avatarPostRepository.findById(targetId) | |
| .orElseThrow(() -> new IllegalArgumentException("아바타 포스트를 찾을 수 없습니다: " + targetId)); | |
| receiver = avatarPost.getUser(); | |
| url = "/api/v1/avatar-posts/" + targetId; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/global/listener/NotificationEventListener.java
around lines 91 to 97, the code calls findById(targetId).get() which will throw
NoSuchElementException when the entity is missing; change to handle the Optional
explicitly by replacing .get() with .orElseThrow(...) (e.g. throw a meaningful
NotFound/EntityNotFoundException with a clear message including targetType and
targetId) and then use the returned entity to set receiver and url; ensure to
import or define the chosen exception and keep behavior consistent with the rest
of the project (or return/skip when absent if that matches business logic).
src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java
Outdated
Show resolved
Hide resolved
| CommentResponse updatedComment = commentService.updateComment(commentId, writerId, request); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
updateComment_success NPE — 응답 구성 시 타겟 자원 접근 안전화 필요
파이프라인 로그에 따르면 CommentService.getResponse 경로에서 AvatarPost가 null일 수 있습니다. 테스트 측면에서는 Comment 엔티티에 targetType/targetId를 세팅하고 해당 리포지토리 목을 스텁해야 하며, 서비스 측면에서는 타입 분기 후 해당 타입에 필요한 연관만 안전하게 접근해야 합니다.
다음 점검을 권장합니다:
- 테스트: Comment.builder()에 targetType=DIARY, targetId 설정 + diaryRepository.findById(...) 스텁 추가.
- 서비스: getResponse 내부에서 switch(targetType) 후 필요한 연관만 접근하고, 다른 연관은 null 체크.
서비스 구현 확인용 스크립트:
🏁 Script executed:
#!/bin/bash
# CommentService의 getResponse 및 리포지토리 의존성 탐색
rg -nP 'class\s+CommentService\b' -C3
rg -nP 'getResponse\s*\(' -C5
rg -nP 'DiaryRepository|AvatarPostRepository' -C2Length of output: 17529
CommentService.getResponse NullPointerException 방지 및 테스트 보강 필요
다음 두 부분을 반드시 수정해야 합니다.
• 테스트 보강 (src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java)
– updateComment_success 테스트에서
• commentRepository.findById(...)가 반환하는 Comment 빌더에 .avatarPost(…) 또는 .diary(…)를 설정하세요.
• 예:
java Comment comment = Comment.builder() .id(commentId) .writer(writer) .avatarPost(avatarPost) // 또는 .diary(diary) .content("새 내용") .build(); given(commentRepository.findById(commentId)).willReturn(Optional.of(comment));
– 위처럼 설정하지 않으면 getResponse 내부에서 comment.getAvatarPost().getId() 호출 시 NPE가 발생합니다.
• 서비스 로직 방어 코드 추가 (src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java)
– getResponse 메서드가 현재 if (comment.getDiary() == null) 만으로 분기하여 AvatarPost에 바로 접근하고 있어, AvatarPost가 null일 경우 NPE가 발생합니다.
– 다음과 같이 순서 및 null 체크를 명시적으로 처리하세요.
private CommentResponse getResponse(Comment comment) {
Long targetId;
String targetType;
- if (comment.getDiary() == null) {
- targetId = comment.getAvatarPost().getId();
- targetType = "AVATAR_POST";
- } else {
- targetId = comment.getDiary().getId();
- targetType = "DIARY";
- }
+ if (comment.getAvatarPost() != null) {
+ targetId = comment.getAvatarPost().getId();
+ targetType = "AVATAR_POST";
+ } else if (comment.getDiary() != null) {
+ targetId = comment.getDiary().getId();
+ targetType = "DIARY";
+ } else {
+ throw new IllegalStateException("댓글의 대상 리소스가 존재하지 않습니다.");
+ }
return CommentResponse.of(comment, targetId, targetType);
}위 두 가지 변경 후, updateComment 테스트가 NPE 없이 정상 통과되고, CommentResponse 구성 시 모든 코드 경로에서 null 안전성이 보장되는지 확인 바랍니다.
🧰 Tools
🪛 GitHub Actions: Spring Boot CI/CD with AWS
[error] 98-98: CommentServiceTest.updateComment_success: NullPointerException in getResponse path; AvatarPost may be null.
🤖 Prompt for AI Agents
In
src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java
around lines 98-99 and in
src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java,
the updateComment test and getResponse logic allow a NullPointerException when
avatarPost or diary are null; update the test's Comment builder to include
either .avatarPost(avatarPost) or .diary(diary) before stubbing
commentRepository.findById(...) so the test provides the expected relation, and
modify CommentService.getResponse to explicitly check both comment.getDiary()
and comment.getAvatarPost() in a safe order (e.g., if diary != null use diary
fields; else if avatarPost != null use avatarPost fields; else handle absent
relation by returning response with null/empty IDs or defaults) so all code
paths avoid direct dereference of a possibly-null avatarPost or diary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (5)
src/main/java/com/example/cp_main_be/global/common/ErrorCode.java (1)
13-13: INVALID_TOKEN의 HTTP 상태를 401(UNAUTHORIZED)로 조정 권고이전 리뷰에서 제안되었던 내용과 동일합니다. 현재 400(BAD_REQUEST)은 인증 실패 의미를 충분히 전달하지 못합니다. 401로 변경하고 코드도 4xx대로 분리하는 편이 명확합니다. (예: E-40101)
추천 수정안:
- INVALID_TOKEN(HttpStatus.BAD_REQUEST, "E-50004", "부적절한 토큰입니다."), + INVALID_TOKEN(HttpStatus.UNAUTHORIZED, "E-40101", "부적절한 토큰입니다."),src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java (3)
28-31: 신규 의존성 주입은 적절하지만 테스트 정합성 보강 필요DiaryRepository, AvatarPostRepository, ApplicationEventPublisher 주입은 이번 리팩터링 의도와 일치합니다. 다만 이로 인해 단위테스트에서 모킹 누락 시 NPE가 발생합니다. 기존 파이프라인 실패 로그와도 일치하므로 테스트에서 해당 목을 반드시 주입/스텁하세요.
다음 스크립트로 테스트에서
CommentService모킹 누락 여부를 점검해 주세요.#!/bin/bash # CommentService 의존성 모킹 누락 탐지 rg -nP '@InjectMocks\s+CommentService|@Mock(?:Bean)?\s+(DiaryRepository|AvatarPostRepository|ApplicationEventPublisher)' -C2 src/test
32-74: 타겟 타입 문자열 비교 → Enum 전환 및 입력 검증 추가 제안
- "DIARY"/"AVATAR_POST" 하드코딩 문자열 비교는 오타에 취약합니다. Enum(TargetType)을 도입하면 컴파일 타임 보장이 가능하고 switch 사용으로 가독성이 개선됩니다.
- 요청 본문의 content가 null/blank인 경우를 서비스 단에서 방어하면 예외 흐름이 선명해집니다(또는 DTO에 @notblank).
아래는 최소 변경 예시입니다.
@@ - // 2. Comment 빌더 준비 - Comment.CommentBuilder commentBuilder = - Comment.builder().writer(writer).content(request.getContent()); + // 2. 입력 검증 + Comment 빌더 준비 + if (request.getContent() == null || request.getContent().isBlank()) { + throw new IllegalArgumentException("댓글 내용이 비어 있습니다."); + } + Comment.CommentBuilder commentBuilder = + Comment.builder().writer(writer).content(request.getContent()); @@ - String targetType = request.getTargetType(); - Long targetId = request.getTargetId(); + String targetType = request.getTargetType(); + Long targetId = request.getTargetId(); @@ - if ("DIARY".equalsIgnoreCase(targetType)) { + if ("DIARY".equalsIgnoreCase(targetType)) { Diary diary = diaryRepository .findById(targetId) .orElseThrow( () -> new IllegalArgumentException("ID에 해당하는 일기를 찾을 수 없습니다: " + targetId)); commentBuilder.diary(diary); - } else if ("AVATAR_POST".equalsIgnoreCase(targetType)) { + } else if ("AVATAR_POST".equalsIgnoreCase(targetType)) { AvatarPost avatarPost = avatarPostRepository .findById(targetId) .orElseThrow( () -> new IllegalArgumentException("ID에 해당하는 아바타 포스트를 찾을 수 없습니다: " + targetId)); commentBuilder.avatarPost(avatarPost); } else { throw new IllegalArgumentException("지원하지 않는 대상 타입입니다: " + targetType); }추가로, Enum 전환 스케치:
// 예: com.example.cp_main_be.domain.social.comment.model.TargetType public enum TargetType { DIARY, AVATAR_POST }- String targetType = request.getTargetType(); + TargetType targetType = TargetType.valueOf(request.getTargetType().toUpperCase()); @@ - if ("DIARY".equalsIgnoreCase(targetType)) { + switch (targetType) { + case DIARY -> { Diary diary = diaryRepository.findById(targetId) .orElseThrow(() -> new DiaryNotFoundException(targetId)); commentBuilder.diary(diary); - } else if ("AVATAR_POST".equalsIgnoreCase(targetType)) { - ... - } else { - throw new IllegalArgumentException("지원하지 않는 대상 타입입니다: " + targetType); - } + } + case AVATAR_POST -> { + AvatarPost avatarPost = avatarPostRepository.findById(targetId) + .orElseThrow(() -> new AvatarPostNotFoundException(targetId)); + commentBuilder.avatarPost(avatarPost); + } + }Enum/예외 클래스 생성은 별도 파일 추가가 필요합니다. 적용 의사가 있으면 후속 패치 제안드리겠습니다.
66-74: save 결과 재사용 및 이벤트 발행 대상 일관화현재
save반환값을 무시하고 동일 인스턴스를 이벤트에 사용합니다. JPA에선 동일 레퍼런스가 갱신되긴 하지만, 관례상saved변수로 명시하면 명확성이 올라가고 후속 변경 시 안전합니다. 또한 과거 코멘트와 동일 제안입니다.- Comment comment = commentBuilder.build(); - commentRepository.save(comment); + Comment comment = commentBuilder.build(); + Comment saved = commentRepository.save(comment); @@ - eventPublisher.publishEvent(new CommentCreatedEvent(comment)); + eventPublisher.publishEvent(new CommentCreatedEvent(saved)); @@ - return getResponse(comment); + return getResponse(saved);src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java (1)
1-27: 테스트 픽스처 단순화 및 외부 협력자 결합도 낮추기테스트가 SUT 외부(리포지토리) 구현에 의존하는 단언을 피하고, 응답 DTO만으로 검증하는 방향이 바람직합니다. 현재는 크게 문제 없으나, 향후
CommentResponse에writerId를 노출한다면 해당 값을 직접 단언하도록 변경해 주세요.
🧹 Nitpick comments (64)
src/main/java/com/example/cp_main_be/global/jwt/JwtTokenProvider.java (1)
72-90: JJWT 파서 사용/주석 불일치 정정 + exp null 안전성 + 타임존 주입 가능성 제안
- 코드 주석은 parserBuilder()를 언급하지만 실제 코드는 parser()를 사용하고 있어 혼동이 있습니다. 최신 JJWT 가이드에 맞춰 parserBuilder() 사용으로 정리하는 편이 명확합니다.
- 혹시라도 exp 클레임이 없는 토큰을 받으면 NPE가 발생할 수 있으므로 방어 코드 한 줄을 추가하는 것을 권장합니다. (현 흐름상 우리 쪽에서 발급한 토큰만 다루는 경우라도 안전망 차원에서 유익)
- 시스템 기본 타임존 의존 대신, 애플리케이션 설정(예: Asia/Seoul)으로 주입받아 일관성을 확보하는 것도 고려해 주세요.
적용 예시(diff):
- public LocalDateTime getExpirationLocalDateTime(String token) { - // 1. parserBuilder()로 시작합니다. - Claims claims = - Jwts.parser() - // 2. 서명 키를 설정합니다. - .setSigningKey(getSigningKey()) - // 3. 파서를 빌드합니다. - .build() - // 4. 토큰을 파싱하여 Claims(내용)를 가져옵니다. - .parseClaimsJws(token) - .getBody(); - - // Claims에서 만료 시간을 가져옵니다. - Date expiration = claims.getExpiration(); - Instant expInstant = expiration.toInstant(); - - // Instant를 시스템 기본 시간대의 LocalDateTime으로 변환합니다. - return LocalDateTime.ofInstant(expInstant, ZoneId.systemDefault()); - } + public LocalDateTime getExpirationLocalDateTime(String token) { + // 1) parserBuilder()로 파서 생성 + Claims claims = + Jwts.parserBuilder() + .setSigningKey(getSigningKey()) + .build() + .parseClaimsJws(token) + .getBody(); + + // 2) exp 방어적 검사 + Date expiration = claims.getExpiration(); + if (expiration == null) { + throw new IllegalStateException("JWT에 exp 클레임이 없습니다."); + } + + // 3) 타임존: 필요 시 설정값으로 주입 고려 (@Value 등) + return LocalDateTime.ofInstant(expiration.toInstant(), ZoneId.systemDefault()); + }추가로, 본 파일 내 다른 파싱 지점(getUuidFromToken, validateToken)도 parserBuilder()로 통일하면 가독성과 일관성이 좋아집니다. 필요하시면 공용 private 메서드(parseClaims)로 중복 제거도 가능합니다.
src/test/java/com/example/cp_main_be/domain/member/auth/service/AuthServiceTest.java (1)
1-5: 테스트 스켈레톤만 존재 — 핵심 시나리오 테스트 보강 필요PR 요지(리프레시 토큰 롤링/영속화, 익명가입 등)에 비해 테스트가 비어 있습니다. 최소한 다음 케이스를 커버해 주세요: 유효/만료/미등록 리프레시 토큰 갱신, 롤링 on/off, 익명 사용자 등록(리프레시 저장/만료 시각 일치), 토큰 무효화 흐름.
초기 뼈대를 다음처럼 추가해 두면 점진 구현에 용이합니다(일단 Disabled로 추가 후 점차 활성화):
package com.example.cp_main_be.domain.member.auth.service; import static org.junit.jupiter.api.Assertions.*; -class AuthServiceTest {} +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +class AuthServiceTest { + + @Disabled("TODO: 유효 리프레시 토큰 → 액세스 토큰 재발급 및 롤링 검증") + @Test + void refreshAccessToken_validToken_rotatesAndPersists() {} + + @Disabled("TODO: 미등록/만료/서명불일치 토큰 → INVALID_TOKEN 예외") + @Test + void refreshAccessToken_invalidToken_throws() {} + + @Disabled("TODO: 익명 사용자 등록 → 사용자/리프레시 저장 및 만료시각 일치") + @Test + void registerNewAnonymousUser_persistsRefreshTokenWithExpiry() {} +}src/main/java/com/example/cp_main_be/domain/member/auth/service/RefreshTokenCleanupService.java (2)
20-31: 스케줄러 예외 전파 방지 및 집계 오버플로우 가능성 개선
- 스케줄러 메서드에서 발생한 예외가 잡히지 않으면 실행 주기 전체에 영향 줄 수 있습니다. try/catch로 오류 로깅을 추가해 주세요.
- count()는 long을 반환합니다. int 캐스팅은 불필요하고 드물게나마 오버플로우 소지가 있습니다.
적용 예시(diff):
@Scheduled(cron = "0 0 3 * * *") public void cleanupExpiredTokens() { - int beforeCount = (int) refreshTokenRepository.count(); - refreshTokenRepository.deleteAllByExpiresAtBefore(LocalDateTime.now()); - int afterCount = (int) refreshTokenRepository.count(); - - log.info( - "✅ 만료된 Refresh Token 정리 완료: {} → {} ({}개 삭제)", - beforeCount, - afterCount, - beforeCount - afterCount); + try { + long beforeCount = refreshTokenRepository.count(); + refreshTokenRepository.deleteAllByExpiresAtBefore(LocalDateTime.now()); + long afterCount = refreshTokenRepository.count(); + + log.info( + "✅ 만료된 Refresh Token 정리 완료: {} → {} ({}개 삭제)", + beforeCount, afterCount, (beforeCount - afterCount)); + } catch (Exception e) { + log.error("❌ 만료된 Refresh Token 정리 중 오류 발생", e); + } }추가로, 운영 타임존을 고정하고 싶다면 @scheduled(zone = "Asia/Seoul") 혹은 설정값 주입을 고려해 주세요.
11-18: 중복 책임 정리 제안: 서비스 내 purge 메서드와 스케줄러 간 역할 통합AuthService.purgeExpiredTokens()와 본 스케줄러의 삭제 로직이 중복됩니다. 삭제 책임을 CleanupService로 일원화하고, 서비스 내 공개 메서드는 CleanupService를 위임 호출하도록 구조를 단순화하면 유지보수성이 좋아집니다.
원하시면 리포지토리 레벨에서 삭제 건수를 반환하도록 시그니처를 바꾸고(아래 리포지토리 코멘트 참고), 스케줄러/수동 호출 모두 동일 로직을 재사용하게 리팩토링해 드릴 수 있습니다.
src/main/java/com/example/cp_main_be/domain/member/auth/domain/repository/RefreshTokenRepository.java (1)
10-22: RefreshTokenRepository의 만료 토큰 삭제 메서드에 삭제 건수 반환 및 호출부 업데이트 적용을 권장드립니다.
변경 내용 요약
void deleteAllByExpiresAtBefore(LocalDateTime now)→long deleteByExpiresAtBefore(LocalDateTime cutoff)- 호출부에서 반환값을 받아 로깅·모니터링에 활용
호출부 위치
src/main/java/com/example/cp_main_be/domain/member/auth/service/RefreshTokenCleanupService.java(라인 23)src/main/java/com/example/cp_main_be/domain/member/auth/service/AuthService.java(라인 122)리포지토리 변경 예시 (diff)
public interface RefreshTokenRepository extends JpaRepository<RefreshToken, String> { Optional<RefreshToken> findByToken(String token); List<RefreshToken> findAllByUserUuid(UUID userUuid); List<RefreshToken> findAllByUserUuidAndDeviceId(UUID userUuid, String deviceId); void deleteByToken(String token); void deleteAllByUserUuid(UUID userUuid); - void deleteAllByExpiresAtBefore(LocalDateTime now); + long deleteByExpiresAtBefore(LocalDateTime cutoff); }
- 호출부 변경 예시
// RefreshTokenCleanupService.cleanupExpiredTokens() long deleted = refreshTokenRepository.deleteByExpiresAtBefore(LocalDateTime.now()); logger.info("cleanupExpiredTokens: {} rows deleted", deleted); // AuthService.purgeExpiredTokens() long deleted = refreshTokenRepository.deleteByExpiresAtBefore(LocalDateTime.now()); logger.info("purgeExpiredTokens: {} rows deleted", deleted);src/main/java/com/example/cp_main_be/domain/member/auth/domain/RefreshToken.java (2)
10-16: 인덱스/해시/동등성 등 엔티티 운영성·보안성 강화 제안
- 조회/청소에 자주 쓰이는 userUuid, expiresAt에 인덱스를 권장합니다.
- 동등성 기준을 PK(token)으로 고정하려면 @EqualsAndHashCode(of = "token")를 명시해 Hash 기반 컬렉션 사용 시 오동작을 방지할 수 있습니다.
- deviceId에 적정 길이 제한을 두면 스키마 안정성이 높아집니다.
적용 예시(diff):
@Entity -@Table(name = "refresh_tokens") +@Table( + name = "refresh_tokens", + indexes = { + @Index(name = "idx_refresh_tokens_user_uuid", columnList = "userUuid"), + @Index(name = "idx_refresh_tokens_expires_at", columnList = "expiresAt") + }) @Getter @Setter +@EqualsAndHashCode(of = "token") @NoArgsConstructor @AllArgsConstructor @Builder public class RefreshToken { @@ /** 선택: 동시 기기 구분용 (원치 않으면 제거) */ - private String deviceId; // X-Client-Device-Id 헤더 등으로 구분 가능 + @Column(length = 128) + private String deviceId; // X-Client-Device-Id 헤더 등으로 구분 가능Also applies to: 24-34
19-23: DB 내 리프레시 토큰 원문 저장 → 해시 저장 전환 고려(보안)토큰 원문을 PK/컬럼으로 그대로 저장하면 DB 유출 시 즉시 재사용 위험이 있습니다. 발급 시점에 토큰 원문은 클라이언트로만 전달하고, DB에는 해시(예: SHA-256)만 저장·조회하는 전략(“lookup by hashed token”)을 권장합니다. 이는 OAuth/OIDC 제공자들이 흔히 채택하는 패턴입니다. 마이그레이션은 점진적 이중 필드(원문+해시) 운용 후 원문 필드 제거로 진행할 수 있습니다.
원하시면 해시 전환을 위한 엔티티/리포지토리/서비스 변경안과 마이그레이션 스크립트 초안을 제공하겠습니다.
src/main/java/com/example/cp_main_be/global/common/ErrorCode.java (1)
15-15: 코드 네이밍/메시지 톤 정합성 제안NOT_FOUND가 404 상황인데 코드가 500대(E-50006)입니다. 내부 정책에 따르되, 가능하면 404 계열 코드로 맞추면 가독성이 좋아집니다. 또한 메시지에서 “Resource” 대신 “리소스” 등 한글 표기를 사용하면 일관성이 올라갑니다.
예시:
- NOT_FOUND(HttpStatus.NOT_FOUND, "E-50006", "Resource를 찾을 수 없습니다."); + NOT_FOUND(HttpStatus.NOT_FOUND, "E-40401", "리소스를 찾을 수 없습니다.");src/main/java/com/example/cp_main_be/domain/member/auth/service/AuthService.java (7)
31-46: 만료 토큰과 단순 무효 토큰을 로깅/구분 권장만료된 리프레시 토큰 접근을 로깅하면 운영 관측이 쉬워집니다. 또한 클라이언트 UX를 위해 만료와 무효를 구분하는 에러코드 도입도 고려해주세요(예: REFRESH_TOKEN_EXPIRED).
다음처럼 최소 로깅을 추가하는 것을 제안합니다:
- if (saved.getExpiresAt().isBefore(LocalDateTime.now())) { - refreshTokenRepository.deleteByToken(incomingRefreshToken); - throw new CustomApiException(ErrorCode.INVALID_TOKEN); - } + if (saved.getExpiresAt().isBefore(LocalDateTime.now())) { + refreshTokenRepository.deleteByToken(incomingRefreshToken); + logger.warn("만료된 리프레시 토큰 접근 차단: userUuid={}, deviceId={}", saved.getUserUuid(), deviceId); + throw new CustomApiException(ErrorCode.INVALID_TOKEN); + }
49-57: JWT 클레임 파싱 대신 DB의 userUuid 사용 권장이미 DB에서 토큰을 조회해
saved.getUserUuid()를 확보했습니다. 굳이 JWT 클레임을 파싱해 UUID로 변환할 필요가 없습니다(형식 오류/예외 가능성 제거, 단일 진실원천 확보). DB 값을 사용하도록 단순화하세요.제안 수정:
- // 4) JWT에서 uuid 추출 - String uuidStr = jwtTokenProvider.getUuidFromToken(incomingRefreshToken); - UUID uuid = UUID.fromString(uuidStr); + // 4) DB 기준으로 사용자 식별 (JWT 클레임 파싱 불필요) + UUID uuid = saved.getUserUuid();부가적으로, 보안 강화가 필요하면 “DB userUuid”와 “JWT 클레임의 uuid”가 불일치할 경우 무효 처리하는 방어 로직을 추가하는 것도 유효합니다.
61-66: 리프레시 토큰 롤링 플래그 하드코딩 해제운영 환경별로 롤링 여부가 달라질 수 있습니다. 하드코드 대신 상수/설정으로 분리하세요.
변경 제안(메서드 내부):
- boolean rotateRefreshToken = true; // 필요 시 yml로 뺄 수 있음 - if (!rotateRefreshToken) { + if (!ROTATE_REFRESH_TOKEN) { // 롤링 안 함 → 기존 리프레시 토큰 그대로 반환 return new TokenRefreshResponse(newAccessToken, incomingRefreshToken, false); }그리고 클래스 상단(메서드 외부)에 다음 필드를 추가하세요:
// 향후 @ConfigurationProperties로 교체 고려 private static final boolean ROTATE_REFRESH_TOKEN = true;옵션: application.yml로 분리하고 @ConfigurationProperties로 주입하면 런타임 토글도 수월합니다.
86-107: 익명 사용자 생성 시 입력값 검증 및 중복 방지 제안
- deviceId가 null/blank인 경우를 검증하고, 필요 시 normalize/길이 제한을 권장합니다.
- 닉네임은 UUID substring 기반으로 충분히 충돌 가능성이 낮지만, 유니크 제약이 있다면 중복 처리(재시도) 로직이 필요합니다.
예: deviceId 유효성 빠른 체크
- public TokenRefreshResponse registerNewAnonymousUser(String deviceId) { + public TokenRefreshResponse registerNewAnonymousUser(String deviceId) { + if (deviceId == null || deviceId.isBlank()) { + throw new CustomApiException(ErrorCode.INVALID_FILE); // 프로젝트 정책에 맞는 적절한 코드로 교체 + }(상응하는 전용 에러코드가 있다면 그 코드로 교체하세요.)
110-118: 대량 무효화/로그아웃 경로에 관찰 가능성 추가 권장운영 추적을 위해 무효화 건수를 로그로 남기면 분석이 용이합니다. 또한 (userUuid, deviceId) 유니크 제약을 엔티티에 부여하면 동시 갱신 시 다중 토큰 허용을 예방할 수 있습니다.
예시 로그 추가:
public void revokeAllByUser(UUID userUuid) { - refreshTokenRepository.deleteAllByUserUuid(userUuid); + refreshTokenRepository.deleteAllByUserUuid(userUuid); + logger.info("Revoked all refresh tokens for userUuid={}", userUuid); }엔티티 제약(참고, 엔티티 파일에서 적용):
- RefreshToken(token) UNIQUE
- RefreshToken(userUuid, deviceId) UNIQUE (단말별 단일 토큰 보장)
- expiresAt 인덱스 추가(아래 코멘트 참조)
120-123: 만료 토큰 청소 성능 개선(인덱스 권고)
expiresAt에 인덱스가 없으면 대량 데이터 환경에서 청소 비용이 커질 수 있습니다. DB 수준 인덱스 추가를 권장합니다. 또한 스케줄 주기/배치 사이즈 설정으로 락 경쟁을 줄일 수 있습니다.
68-75: 확인:TokenRefreshResponseDTO의 boolean은isNewAccount이며, 신규 계정 생성 여부를 나타냅니다현재
AuthService에서 리프레시 토큰 갱신 시에는 항상false, 회원가입(익명 가입) 시에는true로 내려주고 있는데, 이는 DTO 필드 이름(isNewAccount)과 의도에 부합합니다.– src/main/java/com/example/cp_main_be/domain/member/auth/dto/response/TokenRefreshResponse.java
•private boolean isNewAccount;(신규 계정 여부)추천 작업 (선택적 리팩터링)
- DTO 필드와 생성자에 JavaDoc을 추가해
isNewAccount가 정확히 무엇을 의미하는지 명시하세요.- (추가 필요 시) 토큰 롤링 여부를 별도로 전달해야 한다면
isRotated같은 별도 boolean 필드를 도입하는 방안도 고려할 수 있습니다.src/test/java/com/example/cp_main_be/domain/admin/service/AdminServiceTest.java (5)
187-188: QUIZ 타입 설정 변경 적절 + 실패 케이스 테스트 추가 제안
- 변경이 서비스의 가드(퀴즈 타입 검증)에 부합합니다. 이로써 성공 케이스가 잘 보장됩니다.
- 보강을 위해 “QUIZ가 아닌 미션 타입”일 때 예외 및 메시지를 검증하는 실패 테스트를 추가해 주세요. 또한 성공 케이스에서 optionOrder 등 추가 필드도 함께 단언하면 회귀에 더 강합니다.
아래처럼 실패 케이스 테스트를 추가하는 것을 제안합니다(비-QUIZ 타입은 실제 enum에 맞게 조정: 예시로 IMAGE 사용).
@@ @Test @DisplayName("퀴즈 옵션 생성 성공") void createQuizOption_Success() { @@ } + + @Test + @DisplayName("퀴즈 옵션 생성 실패 - QUIZ 타입이 아닌 미션") + void createQuizOption_Fail_WrongMissionType() { + // Given + Long missionMasterId = 2L; + AdminRequestDTO.CreateQuizRequestDTO requestDTO = + AdminRequestDTO.CreateQuizRequestDTO.builder() + .missionMasterId(missionMasterId) + .optionText("보기1") + .optionOrder(1) + .isCorrect(false) + .build(); + + DailyMissionMaster nonQuizMission = + DailyMissionMaster.builder().missionType(MissionType.IMAGE).id(missionMasterId).build(); + given(dailyMissionMasterRepository.findById(missionMasterId)) + .willReturn(Optional.of(nonQuizMission)); + + // When & Then + IllegalArgumentException ex = + assertThrows(IllegalArgumentException.class, () -> adminService.createQuizOption(requestDTO)); + assertThat(ex.getMessage()).isEqualTo("퀴즈 타입의 미션에만 선지를 추가할 수 있습니다."); + verify(dailyMissionMasterRepository, times(1)).findById(missionMasterId); + }
120-121: 테스트 안정성 개선: LocalDateTime.now() 대신 고정 시간 사용실행 시점에 따라 값이 달라지는
now()는 회귀 테스트에 불안정 요소가 됩니다. 고정된 시간을 사용하면 재현성과 안정성이 좋아집니다.- requestDTO.setKeywordDate(LocalDateTime.now()); + requestDTO.setKeywordDate(LocalDateTime.of(2025, 1, 1, 0, 0));
96-111: 예외 타입 일관성 확보 필요NotFound/유효성 오류 상황에서 예외 타입이
IllegalArgumentException,IllegalStateException,RuntimeException으로 혼재합니다. 서비스/컨트롤러 단에서 일관된 도메인 예외(예: NotFoundException, BadRequestException 등) 혹은 최소한 표준화된 표준 예외 하나로 정리하고, 테스트도 그에 맞춰 통일하는 것을 권장합니다. 이는 에러 핸들러/ApiResponse 일관성에도 도움이 됩니다.Also applies to: 201-216, 260-275, 310-323
49-72: 생성 필드 매핑을 ArgumentCaptor로 검증해 커버리지 강화현재는 저장 호출 여부만 검증합니다.
missionType,rewardPoints,description등 요청 필드가 엔티티에 제대로 매핑되어 저장되는지까지 확인하면 회귀 방지에 유용합니다.필요 시 아래와 같이 보강할 수 있습니다.
@@ // Then - assertThat(result.getId()).isEqualTo(1L); - assertThat(result.getTitle()).isEqualTo("새 미션"); - verify(dailyMissionMasterRepository, times(1)).save(any(DailyMissionMaster.class)); + assertThat(result.getId()).isEqualTo(1L); + assertThat(result.getTitle()).isEqualTo("새 미션"); + + ArgumentCaptor<DailyMissionMaster> captor = ArgumentCaptor.forClass(DailyMissionMaster.class); + verify(dailyMissionMasterRepository, times(1)).save(captor.capture()); + DailyMissionMaster savedArg = captor.getValue(); + assertThat(savedArg.getTitle()).isEqualTo("새 미션"); + assertThat(savedArg.getDescription()).isEqualTo("미션 설명"); + assertThat(savedArg.getMissionType()).isEqualTo(MissionType.QUIZ); + assertThat(savedArg.getRewardPoints()).isEqualTo(100L);추가로 상단에 import가 필요합니다:
+import org.mockito.ArgumentCaptor;
167-167: 메서드명 typo 수정:chageUserStatus→changeUserStatus서비스, 컨트롤러, 테스트 코드에서 메서드명 오타가 발견되었습니다. 아래 위치를 모두 수정하여 일관된 이름을 사용하세요.
• src/main/java/com/example/cp_main_be/domain/admin/service/AdminService.java (라인 72)
- 메서드 선언부를
chageUserStatus→changeUserStatus로 변경• src/main/java/com/example/cp_main_be/domain/admin/presentation/AdminController.java (라인 94)
- 호출부
adminService.chageUserStatus(...)→adminService.changeUserStatus(...)로 수정• src/test/java/com/example/cp_main_be/domain/admin/service/AdminServiceTest.java (라인 167)
- 테스트 호출부
adminService.chageUserStatus(...)→adminService.changeUserStatus(...)로 수정예시 diff:
--- a/src/main/java/com/example/cp_main_be/domain/admin/service/AdminService.java @@ -72,7 +72,7 @@ public class AdminService { - public User chageUserStatus(Long userId, AdminRequestDTO.ChangeUserStatusRequestDTO requestDTO) { + public User changeUserStatus(Long userId, AdminRequestDTO.ChangeUserStatusRequestDTO requestDTO) {--- a/src/main/java/com/example/cp_main_be/domain/admin/presentation/AdminController.java @@ -94,7 +94,7 @@ public class AdminController { - User user = adminService.chageUserStatus(userId, request); + User user = adminService.changeUserStatus(userId, request);--- a/src/test/java/com/example/cp_main_be/domain/admin/service/AdminServiceTest.java @@ -167,7 +167,7 @@ public class AdminServiceTest { - User result = adminService.chageUserStatus(userId, requestDTO); + User result = adminService.changeUserStatus(userId, requestDTO);src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java (4)
76-91: 업데이트 경로의 예외 타입 표준화 및 save 호출 생략 제안
RuntimeException은 글로벌 에러 핸들러와의 매핑이 불투명합니다.CommentNotFoundException,UnauthorizedCommentAccessException등 도메인 예외로 교체하세요.- JPA 더티체킹이 활성화되어 있어 명시적
save가 없어도 됩니다(가독성 측면에서 권장). 리포지토리 구현에 따라 차이는 없지만 불필요한 호출을 줄일 수 있습니다.- .orElseThrow(() -> new RuntimeException("댓글을 찾을 수 없습니다.")); // TODO: Custom Exception + .orElseThrow(() -> new CommentNotFoundException(commentId)); @@ - throw new RuntimeException("댓글 작성자만 수정할 수 있습니다."); // TODO: Custom Exception + throw new UnauthorizedCommentAccessException("댓글 작성자만 수정할 수 있습니다."); @@ - comment.setContent(request.getContent()); - commentRepository.save(comment); + comment.setContent(request.getContent()); + // flush는 트랜잭션 종료 시점에 수행됨또한 "수정 시 알림 발행" 요구사항이 없다면 현재 구현이 적절합니다. 만약 수정 이벤트도 알림 대상이라면
CommentUpdatedEvent발행을 검토하세요.
105-120: getResponse의 null-세이프 분기 LGTMDiary/AvatarPost 양방향에 대해 명시적 null 체크로 NPE가 제거되었습니다. 응답 생성도 중복 저장 없이 처리되어 적절합니다.
추가 제안:
- 문자열
"DIARY","AVATAR_POST"생성도 Enum 기반으로 변환하면 오타 위험을 더 줄일 수 있습니다.
71-74: 이벤트 발행 시점: 트랜잭션 커밋 이후 보장 필요 여부 확인현재 서비스 내에서 즉시
publishEvent를 호출합니다. 리스너가 DB 조회를 수행하거나 ID가 필요하다면@TransactionalEventListener(phase = AFTER_COMMIT)를 사용하는 편이 안전합니다. 리스너가 이미 AFTER_COMMIT이라면 현 구현으로 충분합니다.필요 시 다음과 같이 이벤트 리스너를 조정하세요.
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void on(CommentCreatedEvent event) { ... }
13-13: 패키지 네이밍 컨벤션(Repository 대소문자) 정리 권장
com.example...diary.domain.Repository.DiaryRepository처럼 패키지 경로에 대문자가 포함됩니다. 자바 컨벤션 상 패키지는 소문자 사용이 일반적입니다. 대규모 변경이므로 이번 PR 범위가 아니면 추후 일괄 정리 이슈로 전환을 권장합니다.src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java (6)
30-35: AvatarPost 경로 커버리지를 위한 목 추가 제안현재 Diary 경로만 커버합니다.
AVATAR_POST분기 테스트를 추가하려면AvatarPostRepository목이 필요합니다.@Mock private CommentRepository commentRepository; @Mock private UserRepository userRepository; @Mock private DiaryRepository diaryRepository; + @Mock private com.example.cp_main_be.domain.social.avatarpost.domain.repository.AvatarPostRepository avatarPostRepository; @Mock private ApplicationEventPublisher eventPublisher;원하시면 AVATAR_POST 성공/실패 케이스 테스트 메서드도 생성해 드립니다.
47-56: 이벤트 발행 검증 추가 및 불필요한 단언 제거
- create 경로에서
CommentCreatedEvent발행을 검증하면 리팩터링 의도(이벤트 기반 알림)를 테스트가 담보합니다.Assertions.assertEquals(writer.getId(), writerId);는 지역 변수끼리 비교로 의미가 없습니다. 응답 DTO 기반 단언만 유지하세요.@@ - Assertions.assertEquals(writer.getId(), writerId); - Assertions.assertEquals(writer.getUsername(), createdComment.getWriter()); + Assertions.assertEquals(writer.getUsername(), createdComment.getWriter()); @@ - verify(commentRepository).save(any(Comment.class)); + verify(commentRepository).save(any(Comment.class)); + verify(eventPublisher).publishEvent(org.mockito.ArgumentMatchers.any( + com.example.cp_main_be.global.event.CommentCreatedEvent.class));추가로 상단 import에
com.example.cp_main_be.global.event.CommentCreatedEvent를 포함해 주세요.Also applies to: 60-64
93-108: update 경로의 준비 데이터 적절 — 추가 단언 보강 제안NPE 방지를 위해
diary연결을 추가한 점 좋습니다. 여기서도 응답의 targetType/targetId 외에, writer 일치, repository 상호작용 횟수 등을 더 엄밀히 검증할 수 있습니다(예:verify(commentRepository, times(1)).save(any())). 이벤트 미발행 정책이라면 해당 호출이 없는 것도 명시적으로 검증 가능합니다.@@ CommentResponse updatedComment = commentService.updateComment(commentId, writerId, request); @@ - verify(commentRepository).findById(commentId); - verify(commentRepository).save(any(Comment.class)); + verify(commentRepository).findById(commentId); + verify(commentRepository, org.mockito.Mockito.times(1)).save(any(Comment.class)); + // 업데이트 시 이벤트를 발행하지 않는 정책을 명확히 하고자 한다면: + verify(eventPublisher, org.mockito.Mockito.never()).publishEvent(any());Also applies to: 115-125
67-83: 작성자 없음 케이스: save 미호출 검증은 적절 — Diary NotFound 케이스 추가 권장작성자 없음은 커버되고 있으나, target 자원 미존재(예: Diary/AvatarPost not found) 케이스가 없습니다. 아래와 같은 테스트를 추가해 주세요.
@DisplayName("댓글 생성 실패 - 다이어리 없음") @Test void createComment_fail_diaryNotFound() { Long writerId = 1L; CommentRequest request = new CommentRequest(); request.setContent("c"); request.setTargetId(999L); request.setTargetType("DIARY"); User writer = User.builder().id(writerId).username("writer").build(); given(userRepository.findById(writerId)).willReturn(Optional.of(writer)); given(diaryRepository.findById(999L)).willReturn(Optional.empty()); Assertions.assertThrows(IllegalArgumentException.class, () -> commentService.createComment(writerId, request)); verify(commentRepository, org.mockito.Mockito.never()).save(any(Comment.class)); }
30-35: 권한 실패 케이스의 예외 타입 명확화 제안서비스가 커스텀 예외로 전환되면, 테스트도 해당 예외 타입으로 단언을 갱신해야 합니다(현재는 RuntimeException). 이후 예외 클래스를 도입하면 아래처럼 바꾸는 것을 권장합니다.
- Assertions.assertThrows( - RuntimeException.class, - () -> commentService.updateComment(commentId, otherUserId, request)); + Assertions.assertThrows( + UnauthorizedCommentAccessException.class, + () -> commentService.updateComment(commentId, otherUserId, request));동일하게 delete 실패 테스트도 커스텀 예외로 갱신해 주세요.
Also applies to: 144-164
30-35: AVATAR_POST 분기 커버리지 추가 제안기능 리스크를 낮추기 위해 AVATAR_POST 경로의 성공/실패 테스트를 추가해 주세요. 아래는 예시입니다.
@DisplayName("댓글 생성 성공 - AVATAR_POST") @Test void createComment_success_avatarPost() { Long writerId = 1L; CommentRequest request = new CommentRequest(); request.setContent("테스트 댓글"); request.setTargetId(20L); request.setTargetType("AVATAR_POST"); User writer = User.builder().id(writerId).username("writer").build(); AvatarPost avatarPost = AvatarPost.builder().id(20L).title("아바타").build(); given(userRepository.findById(writerId)).willReturn(Optional.of(writer)); given(avatarPostRepository.findById(20L)).willReturn(Optional.of(avatarPost)); given(commentRepository.save(any(Comment.class))).willAnswer(invocation -> invocation.getArgument(0)); CommentResponse res = commentService.createComment(writerId, request); Assertions.assertEquals(20L, res.getTargetId()); Assertions.assertEquals("AVATAR_POST", res.getTargetType()); verify(eventPublisher).publishEvent(any(CommentCreatedEvent.class)); }필요 import:
- AvatarPost, AvatarPostRepository
- CommentCreatedEvent
Also applies to: 37-66, 85-125 </blockquote></details> <details> <summary>src/main/java/com/example/cp_main_be/domain/social/guestbook/dto/request/GuestbookRequest.java (1)</summary><blockquote> `13-13`: **DTO 캡슐화 약화 — public 필드 노출** 요청 DTO의 필드를 public으로 노출하면 이후 유효성·불변성 보장이 약해집니다. 동일 프로젝트의 다른 요청 DTO들(DailyQuestionAnswerRequest, CommentRequest)도 private 필드 스타일을 사용합니다. private으로 되돌려 일관성과 안전성을 높이는 것을 권장합니다. ```diff - public String content; + private String content;추가로 생성자 기반 입력을 선호한다면 @Setter 제거를 고려해도 됩니다(선택).
-@Settersrc/test/java/com/example/cp_main_be/domain/social/guestbook/service/GuestbookServiceTest.java (6)
67-82: 성공 경로에서 현재 사용자 조회 호출까지 검증해 회귀 방지경험치 지급이 getCurrentUser() 기반으로 이뤄지는 계약을 고정하려면 해당 호출 자체도 검증해 두는 것이 안전합니다.
// 2. 경험치 추가 메서드가 호출되었는지 검증 - verify(userService).addExperience(writerId, 6); + verify(userService).getCurrentUser(); + verify(userService).addExperience(writerId, 6);
84-106: 자기 방명록에도 경험치 지급이 요구사항인지 확인 필요자기 자신에게 남긴 방명록에도 경험치가 지급되도록 테스트가 고정되어 있습니다. 악용 가능성(무한 자기 작성)에 대비해 요구사항 확인이 필요합니다. 만약 지급 대상이 타인에게 남긴 경우로 제한되어야 한다면 테스트/구현을 조정해야 합니다.
현 계약 유지 시, 성공 경로와 동일하게 현재 사용자 조회 호출까지 검증하면 회귀 방지에 도움이 됩니다.
// then verify(guestbookRepository).save(any(Guestbook.class)); - verify(userService).addExperience(userId, 6); + verify(userService).getCurrentUser(); + verify(userService).addExperience(userId, 6); // 알림 서비스는 호출되지 않아야 함 verify(notificationService, never()).send(any(), any(), any(), any());
109-135: 일일 제한 실패 시 부수 효과가 전혀 발생하지 않음을 명시적으로 검증현재 save 미호출만 검증하고 있습니다. 경험치 지급과 알림 발송도 발생하지 않음을 함께 검증해 실패 경로의 부수 효과를 확실히 차단하세요.
assertThat(exception.getMessage()).isEqualTo("해당 사용자에게는 하루에 한 번만 방명록을 작성할 수 있습니다."); - verify(guestbookRepository, never()).save(any()); // save는 호출되면 안 됨 + verify(guestbookRepository, never()).save(any()); // save는 호출되면 안 됨 + verify(userService, never()).addExperience(anyLong(), anyInt()); + verify(notificationService, never()).send(any(), any(), any(), any());
138-153: 작성자 미존재 실패 시 부수 효과 차단 검증 보강작성자를 찾지 못하면 저장/알림/경험치 지급이 전혀 일어나지 않음을 함께 검증해 주세요.
assertThrows( UserNotFoundException.class, () -> { guestbookService.createGuestbook(writerId, ownerId, request); }); + verify(guestbookRepository, never()).save(any()); + verify(notificationService, never()).send(any(), any(), any(), any()); + verify(userService, never()).addExperience(anyLong(), anyInt());
156-173: 소유자 미존재 실패 시에도 저장/알림/경험치 지급 미발생 검증 추가실패 경로의 사이드 이펙트를 명시적으로 금지해 테스트 안정성을 높여 주세요.
assertThrows( UserNotFoundException.class, () -> { guestbookService.createGuestbook(writerId, ownerId, request); }); + verify(guestbookRepository, never()).save(any()); + verify(notificationService, never()).send(any(), any(), any(), any()); + verify(userService, never()).addExperience(anyLong(), anyInt());
79-82: 알림 전송 검증 적절 — 이벤트 기반 전환은 선택 사항현재
GuestbookService#createGuestbook메서드에서NotificationService.send를 직접 호출하고 있으므로, 테스트에서 해당 호출을 검증하는 방식이 구현에 부합합니다.
향후 서비스가 이벤트 기반 아키텍처로 전환될 경우,ApplicationEventPublisher를 모킹하여GuestbookCreatedEvent발행 여부를 검증하는 테스트 방식으로 변경하는 것을 선택적으로 고려할 수 있습니다.
- src/main/java/com/example/cp_main_be/domain/social/guestbook/service/GuestbookService.java:58 (NotificationService.send 호출 지점)
src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java (8)
52-55: 테스트 입력 userUuid와 mockUser 불일치 — 시나리오 가독성 개선 제안no-filter 경로에서 user를 사용하지 않더라도, 전달하는 UUID를 mockUser와 맞추면 의도 파악이 쉬워집니다.
- UUID userUuid = UUID.randomUUID(); + UUID userUuid = mockUser.getUuid();
53-71: LocalDateTime.now 다중 호출로 미세한 타이밍 오차 가능성 — 기준 시각 고정 권장동일 테스트 내에서 now()를 여러 번 호출하면 시스템 시간 변화에 따라 아주 드물게 경계값에서 순서가 흔들릴 수 있습니다. 기준 시각을 한 번만 정의해 쓰면 완전 결정적(deterministic)입니다.
- Diary oldDiary = - Diary.builder() - .user(mockUser) // 사용자 설정 - .createdAt(LocalDateTime.now().minusDays(2)) - .build(); + // 고정 기준 시각(예시) + LocalDateTime baseTime = LocalDateTime.of(2025, 1, 1, 12, 0); + + Diary oldDiary = + Diary.builder() + .user(mockUser) + .createdAt(baseTime.minusDays(2)) + .build(); @@ - AvatarPost recentAvatarPost = + AvatarPost recentAvatarPost = AvatarPost.builder() .user(mockUser) // 사용자 설정 - .createdAt(LocalDateTime.now().minusHours(1)) + .createdAt(baseTime.minusHours(1)) .build(); @@ - Diary recentDiary = + Diary recentDiary = Diary.builder() .user(mockUser) // 사용자 설정 - .createdAt(LocalDateTime.now()) + .createdAt(baseTime) .build();
83-87: 정렬 검증 간결화 제안: createdAt 추출로 단일 어설션개별 인덱스 비교 대신 추출-비교로 의도와 실패 메시지가 더 명확해집니다.
- // 최신순으로 정렬되었는지 확인 (가장 최신인 recentDiary가 첫 번째여야 함) - assertThat(feed.get(0).getCreatedAt()).isEqualTo(recentDiary.getCreatedAt()); - assertThat(feed.get(1).getCreatedAt()).isEqualTo(recentAvatarPost.getCreatedAt()); - assertThat(feed.get(2).getCreatedAt()).isEqualTo(oldDiary.getCreatedAt()); + // 최신순 정렬 검증 + assertThat(feed) + .extracting(FeedItemResponse::getCreatedAt) + .containsExactly( + recentDiary.getCreatedAt(), + recentAvatarPost.getCreatedAt(), + oldDiary.getCreatedAt());
104-109: 두 번째 테스트도 기준 시각 고정 권장테스트 간 일관성을 위해 여기에서도 고정 시각을 권장합니다.
- Diary followedUserDiary = - Diary.builder().user(followingUser1).createdAt(LocalDateTime.now()).build(); + LocalDateTime baseTime = LocalDateTime.of(2025, 1, 1, 13, 0); + Diary followedUserDiary = + Diary.builder().user(followingUser1).createdAt(baseTime).build(); @@ - Diary notFollowedUserDiary = - Diary.builder().user(notFollowingUser).createdAt(LocalDateTime.now().minusDays(1)).build(); + Diary notFollowedUserDiary = + Diary.builder().user(notFollowingUser).createdAt(baseTime.minusDays(1)).build();
144-147: 예외 메시지까지 검증하면 실패 원인 파악이 더 쉬워집니다지금 코드는 미-throw 시 메시지로만 사용됩니다. 실제 예외 메시지 내용도 확인하도록 보강을 제안합니다.
- assertThrows( - UserNotFoundException.class, - () -> feedService.getFeed(nonExistentUserUuid, "following"), - "사용자를 찾을 수 없습니다."); + UserNotFoundException ex = + assertThrows( + UserNotFoundException.class, + () -> feedService.getFeed(nonExistentUserUuid, "following")); + assertThat(ex).hasMessageContaining("사용자를 찾을 수 없습니다.");
165-168: no-posts 경로에서도 불필요한 following 로직 미호출 검증 추가 제안모든 경로에서 불필요 호출이 없는지 명시적으로 보장하면 회귀에 강합니다.
assertThat(feed).isNotNull(); assertThat(feed).isEmpty(); + verify(diaryRepository).findByIsPublicIsTrue(any(Pageable.class)); + verify(avatarPostRepository).findAll(any(Pageable.class)); + verify(followRepository, never()).findByFollower(any());
48-92: 추가 커버리지 제안: no-filter 경로의 페이징/정렬 인자 검증현재는 any(Pageable.class)로 스텁만 하고 있습니다. PageRequest의 size/sort가 서비스 요구사항대로 전달되는지 ArgumentCaptor로 검증하면 더 견고합니다.
원하시면 ArgumentCaptor 기반 예시 추가해드리겠습니다.
94-134: following + 아바타 포스트 혼합 정렬 시나리오 테스트 추가 제안팔로우 대상의 다이어리와 아바타 포스트가 동시에 존재할 때에도 전체가 최신순으로 정렬되는지 별도 테스트를 권장합니다.
아래 예시 메서드를 추가해드릴 수 있습니다(필요 시 요청 주세요).
@Test @DisplayName("피드 조회 성공 - 'following' + 아바타 포스트 포함 시 최신순 정렬") void getFeed_withFollowingFilter_includesAvatarPosts_sorted() { User current = User.builder().uuid(UUID.randomUUID()).build(); User followed = User.builder().uuid(UUID.randomUUID()).build(); given(userRepository.findByUuid(current.getUuid())).willReturn(Optional.of(current)); given(followRepository.findByFollower(current)) .willReturn(List.of(Follow.builder().follower(current).following(followed).build())); LocalDateTime base = LocalDateTime.of(2025, 1, 2, 9, 0); Diary d = Diary.builder().user(followed).createdAt(base.minusHours(2)).build(); AvatarPost p = AvatarPost.builder().user(followed).createdAt(base.minusHours(1)).build(); given(diaryRepository.findByUserInAndIsPublicIsTrue(anyList(), any(Pageable.class))) .willReturn(List.of(d)); given(avatarPostRepository.findByUserIn(anyList(), any(Pageable.class))) .willReturn(List.of(p)); List<FeedItemResponse> feed = feedService.getFeed(current.getUuid(), "following"); assertThat(feed).extracting(FeedItemResponse::getCreatedAt) .containsExactly(p.getCreatedAt(), d.getCreatedAt()); }src/test/java/com/example/cp_main_be/domain/garden/service/GardenServiceTest.java (3)
148-151: DisplayName의 최대 개수 표기가 현재 정책(4개)와 불일치합니다표시명은 여전히 “(3개)”로 남아 있어 오해의 소지가 있습니다. 메시지와 테스트 데이터는 이미 4개 기준으로 맞춰져 있으므로 DisplayName도 함께 갱신하세요.
- @DisplayName("실패 - 최대 텃밭 개수(3개)에 도달했을 때 예외가 발생한다") + @DisplayName("실패 - 최대 텃밭 개수(4개)에 도달했을 때 예외가 발생한다")
151-156: Given 주석도 실제 테스트 데이터(4개 보유)에 맞춰 갱신하세요주석에는 “이미 3개”라고 되어 있으나 아래에서 1~4 슬롯 총 4개를 생성하고 있습니다. 문서/주석과 테스트 데이터가 어긋나면 유지보수 시 혼란이 큽니다.
- // 레벨 4이지만, 텃밭을 이미 3개 가지고 있는 사용자 + // 레벨 4이지만, 텃밭을 이미 4개 가지고 있는 사용자
169-170: 예외 메시지 단정은 문구 변화에 취약합니다 — 포함 검증으로 완화 권장정확 일치 비교는 사소한 문구 변경에도 테스트가 깨집니다. 정책상 최대 개수(4)만 핵심이면 contains 기반으로 단정하세요.
- assertThat(exception.getMessage()).isEqualTo("텃밭은 최대 4개까지만 생성할 수 있습니다."); + assertThat(exception).hasMessageContaining("최대 4개");src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/AnswerType.java (2)
13-14: 점수(가중치) 매핑 메서드를 enum에 내장해 중복 로직 방지점수 체계(예: YES=+1, NEUTRAL=0, NO=-1)를 사용하는 곳이 늘어날 수 있습니다. 산발적으로 if/switch를 쓰기보다 enum이 소유하도록 두면 응집도가 올라갑니다.
아래와 같이 간단한 메서드를 추가해두면 좋습니다:
private final String description; + + public int score() { + return switch (this) { + case YES -> 1; + case NEUTRAL -> 0; + case NO -> -1; + }; + } }
8-14: API 직렬화/역직렬화 규약 확인 필요(영문 상수 vs. 한글 설명 노출)요청/응답에서 AnswerType을 문자열로 주고받을 때, 현재는 기본적으로 enum name(YES/NEUTRAL/NO)이 사용됩니다. 한글 설명을 노출하고 싶다면 @JsonValue/@JsonCreator 전략으로 계약이 바뀌므로, 프런트/모바일과 합의가 먼저 필요합니다. 합의되면 적용 방향 제안드립니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java (2)
34-38: 서버 타임존 의존 최소화 및 테스트 용이성: Clock 주입 권장LocalDate.now()는 서버 타임존에 의존합니다. 일일 질문은 날짜 경계(자정)와 강하게 결부되므로, Clock을 주입받아 LocalDate.now(clock)로 계산하면 테스트와 운영 타임존 이슈를 동시에 해결할 수 있습니다. 기본 Bean(Clock.systemDefaultZone())을 구성하거나 구성 클래스로 주입하세요.
예시(개념 코드):
@RequiredArgsConstructor @Service public class DailyQuestionService { private final Clock clock; public String getQuestionForToday() { int dayOfYear = LocalDate.now(clock).getDayOfYear(); int questionIndex = (dayOfYear - 1) % QUESTIONS.size(); return QUESTIONS.get(questionIndex); } }
10-32: 하드코딩된 문항 소스의 외부화 고려(운영/콘텐츠팀 협업 용이성)문항이 코드에 고정되어 있으면 운영 변경 주기가 빨라질 때 배포가 병목이 됩니다. 설정(예: YAML), DB, 혹은 Feature Flag/콘텐츠 관리로 외부화하면 운영 탄력성이 올라갑니다. 버전/언어(다국어) 스키마를 함께 설계하면 확장에도 유리합니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
8-10: Java record로 더 간소화 가능(선택)불변 DTO는 record로 대체하면 Lombok 의존 없이도 동일한 목적을 달성합니다. 변경 영향이 적다면 고려해볼 만합니다.
예시:
public record DailyQuestionResponse(String question) {}src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java (2)
20-41: 요청 DTO와 서비스 책임 정렬서비스가 서버측 질문을 강제한다면, DailyQuestionAnswerRequest의 question 필드는 더 이상 필수일 필요가 없습니다. DTO에서 필수 제약 제거 또는 서버 미사용 필드 삭제를 고려해 주세요. 이 변경은 컨트랙트 영향이 있으므로 API 소비자와 합의가 선행되어야 합니다.
29-30: 예외 메시지/타입 일관화(국문화 및 도메인 예외)IllegalStateException 대신 도메인 예외(예: AlreadyAnsweredTodayException)와 메시지 국제화를 사용하면 API 응답 일관성이 높아집니다. 메시지 코드는 MessageSource로 관리하세요.
원하시면 예외 클래스/ControllerAdvice 템플릿을 생성해 드리겠습니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (3)
4-4: 위 변경에 따른 import 정리
question필드를 제거하면@NotBlankimport가 사용되지 않습니다. 불필요한 의존 제거로 정리하세요.-import jakarta.validation.constraints.NotBlank;Also applies to: 13-15
16-17: 유효성 강화: enum 값 외 추가 제약 필요 여부 검토
answer는 enum 역직렬화로 기본 검증이 되지만, 비즈니스적으로 허용 범위를 더 엄격히 규정해야 한다면 커스텀 validator(예: 특정 날짜에는 특정 타입만 허용 등)를 고려하세요. 현재로서는 충분하나 정책이 있다면 DTO 레벨에서 막는 편이 좋습니다.
9-11: Jackson 역직렬화 실패 위험 확인 — DTO 바인딩 보강 권장프로젝트 내에 별도의
ObjectMapper빈 정의나@JsonAutoDetect설정이 확인되지 않으므로(Jackson 기본 가시성: private 필드에 퍼블릭 세터가 없으면 역직렬화 불가), 아래 중 하나를 적용해 주세요.• 세터 추가
@Getter -@NoArgsConstructor +@NoArgsConstructor +@Setter public class DailyQuestionAnswerRequest { // … }• 생성자 기반 바인딩 (불변 DTO 선호 시)
-@Getter -@NoArgsConstructor +@Getter +@RequiredArgsConstructor +@Builder public class DailyQuestionAnswerRequest { - @NotNull(message = "Answer cannot be null") - private AnswerType answer; + private final AnswerType answer; // … }추가로, 전역 설정으로 필드 가시성을 조정하려면
@JsonAutoDetect(fieldVisibility = Visibility.ANY)또는spring.jackson.visibility.field=ANY도 고려할 수 있습니다.— 검증 스크립트 예시
rg -nP '@Bean\s*\(\)\s*public\s+ObjectMapper' --type=java src rg -nR 'JsonAutoDetect' -C2 srcsrc/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java (2)
33-35: “오늘” 산정 기준(타임존) 명시 권장서버 로컬 타임존과 클라이언트 타임존이 다를 때 날짜 경계 이슈가 생깁니다. 서비스에서
ZoneId를 명시(예: 사용자의 선호/프로필 타임존 또는 서버 표준 KST/UTC)하고, GET/POST 모두 동일 기준으로 동작하도록 보장해 주세요. 문서에도 기준을 명시하는 것을 권장합니다.#!/bin/bash # 오늘 기준 산정 코드 점검 (LocalDate.now, ZoneId 사용 여부) rg -nP -C3 'LocalDate\.now\(|ZonedDateTime|ZoneId' --type=java
38-44: 중복 제출 예외 처리 강화 및 HTTP 상태 코드 검토 필요
- DailyQuestionAnswerService( src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java 라인 28)에서 existsByUserAndAnsweredDate 체크 후 중복 시 IllegalStateException 을 던지는데, 글로벌 핸들러의 RuntimeException 처리로 500 INTERNAL_SERVER_ERROR가 반환됩니다.
→ DuplicateSubmissionException(예: CustomApiException 하위) 등으로 전환하여 409 Conflict로 매핑하도록 수정하세요.- GlobalExceptionHandler( src/main/java/com/example/cp_main_be/global/GlobalExceptionHandler.java )에 CustomApiException 기반 핸들러 또는 @ExceptionHandler(DuplicateSubmissionException.class) 구성하여 적절한 HTTP 상태 코드를 반환하도록 추가 구현이 필요합니다.
- saveDailyQuestionAnswer 컨트롤러( src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java )에서 성공 응답이 바디 없이 null인 경우, 팀 컨벤션에 맞춰 200 OK 대신 204 No Content 사용을 고려해 보세요.
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/DailyQuestionAnswer.java (4)
23-25: JPA 레벨 불변식 강화:@ManyToOne(optional = false)명시 권장DB 컬럼
nullable = false와 JPA의optional = false를 일치시켜 영속성 컨텍스트 단계에서 조기에 오류를 드러내는 편이 좋습니다.- @ManyToOne(fetch = FetchType.LAZY) + @ManyToOne(fetch = FetchType.LAZY, optional = false) @JoinColumn(name = "user_id", nullable = false) private User user;
27-29: DTO-엔티티 길이 정책 정합성 확보엔티티는
length = 500인데, 요청 DTO에는 길이 상한 검증이 없습니다. DTO에@Size(max = 500)(또는 질문 텍스트를 서버에서 채우도록 DTO에서 제거 — 상단 코멘트 참고)를 추가해 DB 제약 위반을 사전에 방지하세요.
34-35:answeredDate는 서버에서만 설정하도록 보장클라이언트가 날짜를 조작하지 못하도록 서비스에서 서버 기준(명시적
ZoneId)으로LocalDate를 설정하세요. 또한, 이 필드를 기준으로 포인트 적립/중복 방지 로직이 동작한다면 트랜잭션 경계 내에서 일관성 있게 처리되도록 주의가 필요합니다.
37-44: 생성자/팩토리에서 불변식 검증 추가 고려빌더를 통해 생성 시
user,answer,answeredDate가 누락되면 런타임에서 나중에 실패합니다. 도메인 무결성을 위해 정적 팩토리 또는 빌더의build()훅에서 널/상태 검증을 추가하는 것을 고려하세요.예시(별도 유틸/검증 메서드 사용 가능):
@Builder public DailyQuestionAnswer(User user, String question, AnswerType answer, LocalDate answeredDate) { this.user = java.util.Objects.requireNonNull(user, "user"); this.question = question; // 서버 소스로 채움 권장 this.answer = java.util.Objects.requireNonNull(answer, "answer"); this.answeredDate = java.util.Objects.requireNonNull(answeredDate, "answeredDate"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (24)
src/main/java/com/example/cp_main_be/domain/member/auth/domain/RefreshToken.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/auth/domain/repository/RefreshTokenRepository.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/auth/presentation/AuthController.java(2 hunks)src/main/java/com/example/cp_main_be/domain/member/auth/service/AuthService.java(2 hunks)src/main/java/com/example/cp_main_be/domain/member/auth/service/RefreshTokenCleanupService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/AnswerType.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/DailyQuestionAnswer.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java(4 hunks)src/main/java/com/example/cp_main_be/domain/social/guestbook/dto/request/GuestbookRequest.java(1 hunks)src/main/java/com/example/cp_main_be/global/common/ErrorCode.java(1 hunks)src/main/java/com/example/cp_main_be/global/jwt/JwtTokenProvider.java(2 hunks)src/test/java/com/example/cp_main_be/domain/admin/service/AdminServiceTest.java(1 hunks)src/test/java/com/example/cp_main_be/domain/auth/service/AuthServiceTest.java(0 hunks)src/test/java/com/example/cp_main_be/domain/garden/service/GardenServiceTest.java(2 hunks)src/test/java/com/example/cp_main_be/domain/member/auth/service/AuthServiceTest.java(1 hunks)src/test/java/com/example/cp_main_be/domain/social/comment/service/CommentServiceTest.java(6 hunks)src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java(1 hunks)src/test/java/com/example/cp_main_be/domain/social/guestbook/service/GuestbookServiceTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/com/example/cp_main_be/domain/auth/service/AuthServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/cp_main_be/domain/member/auth/presentation/AuthController.java
🧰 Additional context used
🧬 Code graph analysis (10)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
Getter(9-18)
src/main/java/com/example/cp_main_be/domain/member/auth/service/RefreshTokenCleanupService.java (1)
src/main/java/com/example/cp_main_be/domain/member/auth/service/AuthService.java (1)
Service(19-124)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/AnswerType.java (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
Getter(9-18)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
Getter(6-10)
src/test/java/com/example/cp_main_be/domain/social/guestbook/service/GuestbookServiceTest.java (1)
src/main/java/com/example/cp_main_be/global/exception/UserNotFoundException.java (1)
UserNotFoundException(3-7)
src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java (1)
src/main/java/com/example/cp_main_be/domain/member/auth/presentation/AuthController.java (1)
RestController(12-36)
src/main/java/com/example/cp_main_be/domain/social/guestbook/dto/request/GuestbookRequest.java (2)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
Getter(9-18)src/main/java/com/example/cp_main_be/domain/social/comment/dto/request/CommentRequest.java (1)
Getter(8-19)
src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java (1)
src/main/java/com/example/cp_main_be/global/exception/UserNotFoundException.java (1)
UserNotFoundException(3-7)
src/main/java/com/example/cp_main_be/domain/social/comment/service/CommentService.java (1)
src/main/java/com/example/cp_main_be/global/exception/UserNotFoundException.java (1)
UserNotFoundException(3-7)
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/DailyQuestionAnswer.java (2)
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java (1)
Getter(9-18)src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
Getter(6-10)
🔇 Additional comments (12)
src/main/java/com/example/cp_main_be/global/common/ErrorCode.java (1)
14-14: 중복 코드 해소 LGTMFILE_SIZE_EXCEEDED의 코드가 E-50005로 분리되어 과거 중복 문제가 해소되었습니다. 👍
src/test/java/com/example/cp_main_be/domain/social/guestbook/service/GuestbookServiceTest.java (1)
69-75: LGTM — ArgumentCaptor로 저장 엔티티 검증한 점 좋습니다저장 전송 객체의 writer/owner/content를 모두 캡처 후 검증해 회귀 탐지력이 높습니다.
src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java (3)
89-92: no-filter 경로에서 following 로직 미호출 검증 좋습니다불필요한 쿼리/로직 실행을 방지하는 회귀를 잘 커버하고 있습니다.
128-134: following 필터 경로의 상호작용 검증이 명확합니다유저 조회 → 팔로잉 목록 → 대상 사용자들의 게시물 조회 순서와 범위를 잘 검증하고 있습니다.
1-170: 레포지토리 및 서비스 시그니처 일관성 확인 완료패키지 경로(
com.example.cp_main_be.domain.social.diary.domain.Repository)와 메서드 시그니처(findByIsPublicIsTrue(Pageable),findByUserInAndIsPublicIsTrue(List<User>, Pageable),findAll(Pageable)등)가 테스트 코드와 실제 구현에서 모두 일치함을 확인했습니다.
따라서 추가 수정이나 조치는 필요하지 않습니다.src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/AnswerType.java (1)
8-11: enum 정의 깔끔합니다의미 있는 세 가지 상태로 잘 한정했고, 한글 설명 필드도 가독성이 좋습니다. Lombok 조합(@Getter, @requiredargsconstructor)도 적절합니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionResponse.java (1)
8-10: 단방향 응답 DTO로 충분하며 간결합니다서버에서 생성해서 직렬화만 하므로 불변 필드+필수 생성자 조합이 적절합니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/repository/DailyQuestionAnswerRepository.java (2)
8-10: 리포지토리 메서드 네이밍과 의도 적합하루 1회 응답 제약을 점검하는 existsBy... 시그니처가 깔끔합니다.
8-10: 이미 DB 유니크 제약이 선언되어 있습니다 — 서비스 레이어 예외 매핑을 검증해주세요DailyQuestionAnswer 엔티티에
@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"user_id", "answered_date"})})가 선언되어 있어, DB 레벨에서 중복 삽입을 방지할 유니크 제약은 충족됩니다.검증 및 보완이 필요한 사항:
- DailyQuestionAnswer 엔티티
- 경로:
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/DailyQuestionAnswer.java- 16번째 라인에서
@UniqueConstraint(columnNames = {"user_id", "answered_date"})선언 확인- 서비스 레이어 예외 처리
- 예:
DailyQuestionAnswerService또는DailyQuestionService의 저장 메서드DataIntegrityViolationException을 잡아DuplicateDailyQuestionAnswerException(또는 유사한 도메인 예외)으로 변환하는 로직이 구현되어 있는지 확인예시 구현:
try { repository.save(answer); } catch (DataIntegrityViolationException e) { throw new DuplicateDailyQuestionAnswerException("이미 오늘의 질문에 답변하셨습니다.", e); }위 사항을 검토하고, 누락된 경우 서비스 레이어에 예외 매핑 코드를 추가해주세요.
src/main/java/com/example/cp_main_be/domain/member/daily_question/presentation/DailyQuestionController.java (2)
30-36: 응답 DTO 래핑과 흐름은 명확합니다 — LGTM“오늘의 질문”을 서비스에서 받아 DTO로 감싸고
ApiResponse.success로 표준화해 반환하는 구조는 일관되고 명확합니다.
41-41:@AuthenticationPrincipal User검증 완료
JwtAuthenticationFilter에서UsernamePasswordAuthenticationToken(user, …)를 통해 도메인User객체를 SecurityContext의 principal로 설정하고 있습니다.- Spring Security가 기본 제공하는
AuthenticationPrincipalArgumentResolver가 이 principal을 컨트롤러 파라미터의User타입으로 바인딩하므로, 별도의 커스텀 리졸버나 추가 매핑 없이도 정상 동작합니다.따라서 해당 패턴 사용은 적절하며, 추가 수정이 필요하지 않습니다.
src/main/java/com/example/cp_main_be/domain/member/daily_question/domain/DailyQuestionAnswer.java (1)
14-16: 유니크 제약으로 1일 1회 응답 보장 — 적절합니다
(user_id, answered_date)유니크 제약으로 데이터 정합성을 DB 레벨에서 보장하는 선택이 좋습니다. 서비스 레이어에서의 선조회와 함께 사용하면 UX 측면에서도 안정적입니다.
| @NotBlank(message = "Question cannot be blank") | ||
| private String question; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
클라이언트가 보내는 question 값 신뢰 문제 — 서버 소스로 강제하거나 요청 DTO에서 제거 권장
요청 본문에 question 문자열을 포함시키면 클라이언트 임의값으로 서버 저장값이 왜곡될 수 있습니다. “오늘의 질문”은 서버 기준(또는 서버가 신뢰하는 소스)으로 확정되어야 하며, 사용자는 오직 answer만 제출하도록 하는 편이 안전합니다. 현재 엔티티에도 question이 저장되므로 서비스 레이어가 서버 소스로 설정하고, 요청 DTO에서는 제거하는 것을 권장합니다.
다음과 같이 DTO에서 question을 제거하세요(불필요해진 @NotBlank import도 함께 삭제 필요).
- @NotBlank(message = "Question cannot be blank")
- private String question;추가로 상단 import도 정리해야 합니다(별도 코멘트에 diff 포함).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @NotBlank(message = "Question cannot be blank") | |
| private String question; |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/dto/DailyQuestionAnswerRequest.java
around lines 13 to 15, remove the client-supplied question field and its
@NotBlank annotation (and delete the unused javax/validation import), leaving
the DTO to carry only the answer; update the class imports to remove the unused
validation import and any other now-unused imports, and then ensure
service-layer code is changed to set the entity's question from the trusted
server source (today's question) instead of reading it from the DTO.
| import com.example.cp_main_be.domain.member.daily_question.domain.DailyQuestionAnswer; | ||
| import com.example.cp_main_be.domain.member.daily_question.domain.repository.DailyQuestionAnswerRepository; | ||
| import com.example.cp_main_be.domain.member.daily_question.dto.DailyQuestionAnswerRequest; | ||
| import com.example.cp_main_be.domain.member.user.domain.User; | ||
| import java.time.LocalDate; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
DB 무결성 예외 매핑을 위해 DataIntegrityViolationException import 추가
유니크 제약 기반으로 중복 방지 시 DataIntegrityViolationException을 처리해야 합니다. import 추가가 필요합니다.
import java.time.LocalDate;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
+import org.springframework.dao.DataIntegrityViolationException;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import com.example.cp_main_be.domain.member.daily_question.domain.DailyQuestionAnswer; | |
| import com.example.cp_main_be.domain.member.daily_question.domain.repository.DailyQuestionAnswerRepository; | |
| import com.example.cp_main_be.domain.member.daily_question.dto.DailyQuestionAnswerRequest; | |
| import com.example.cp_main_be.domain.member.user.domain.User; | |
| import java.time.LocalDate; | |
| import lombok.RequiredArgsConstructor; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.transaction.annotation.Transactional; | |
| import com.example.cp_main_be.domain.member.daily_question.domain.DailyQuestionAnswer; | |
| import com.example.cp_main_be.domain.member.daily_question.domain.repository.DailyQuestionAnswerRepository; | |
| import com.example.cp_main_be.domain.member.daily_question.dto.DailyQuestionAnswerRequest; | |
| import com.example.cp_main_be.domain.member.user.domain.User; | |
| import java.time.LocalDate; | |
| import lombok.RequiredArgsConstructor; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.transaction.annotation.Transactional; | |
| import org.springframework.dao.DataIntegrityViolationException; |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java
around lines 3 to 11, add the import for Spring's
DataIntegrityViolationException to enable mapping/handling of DB integrity
constraint violations: import
org.springframework.dao.DataIntegrityViolationException; Ensure this import is
present so the service can catch and handle DataIntegrityViolationException
where duplicate/unique constraint violations occur.
| private final DailyQuestionAnswerRepository dailyQuestionAnswerRepository; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
오늘의 질문을 서버에서 결정하기 위해 DailyQuestionService 의존성 주입
클라이언트가 보낸 question 문자열을 신뢰하지 말고 서버에서 오늘의 질문을 결정/저장하세요. 이를 위해 필드 의존성을 추가합니다. (@requiredargsconstructor로 자동 주입)
private final DailyQuestionAnswerRepository dailyQuestionAnswerRepository;
+ private final DailyQuestionService dailyQuestionService;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private final DailyQuestionAnswerRepository dailyQuestionAnswerRepository; | |
| private final DailyQuestionAnswerRepository dailyQuestionAnswerRepository; | |
| private final DailyQuestionService dailyQuestionService; |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java
around lines 17-18, add a new final field for DailyQuestionService (e.g. private
final DailyQuestionService dailyQuestionService;) so it gets injected via the
existing @RequiredArgsConstructor; then replace any code that trusts a
client-provided question string by calling dailyQuestionService to
determine/fetch today's question on the server and use that question when
creating/saving DailyQuestionAnswer (remove reliance on the client question
parameter and ensure persistence uses the server-derived question).
| LocalDate today = LocalDate.now(); | ||
|
|
||
| // 사용자가 오늘 이미 답변했는지 확인 | ||
| if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { | ||
| throw new IllegalStateException("User has already answered today's question."); | ||
| } | ||
|
|
||
| DailyQuestionAnswer answer = | ||
| DailyQuestionAnswer.builder() | ||
| .user(user) // 매개변수로 받은 user 객체를 바로 사용 | ||
| .question(requestDto.getQuestion()) | ||
| .answer(requestDto.getAnswer()) | ||
| .answeredDate(today) | ||
| .build(); | ||
|
|
||
| dailyQuestionAnswerRepository.save(answer); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
데이터 무결성 보강: (1) 클라이언트 입력 검증 제거, (2) 유니크 제약 기반 예외 처리, (3) 존재 체크 제거
- 현재는 requestDto.getQuestion()을 그대로 저장합니다. 클라이언트 변조 가능성이 있으므로 서버에서 오늘의 질문을 사용하세요.
- exists → save는 경쟁조건이 있습니다. 유니크 제약과 예외 매핑으로 단일화하세요.
- LocalDate today = LocalDate.now();
-
- // 사용자가 오늘 이미 답변했는지 확인
- if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) {
- throw new IllegalStateException("User has already answered today's question.");
- }
-
- DailyQuestionAnswer answer =
- DailyQuestionAnswer.builder()
- .user(user) // 매개변수로 받은 user 객체를 바로 사용
- .question(requestDto.getQuestion())
- .answer(requestDto.getAnswer())
- .answeredDate(today)
- .build();
-
- dailyQuestionAnswerRepository.save(answer);
+ LocalDate today = LocalDate.now();
+ String todayQuestion = dailyQuestionService.getQuestionForToday();
+
+ DailyQuestionAnswer answer =
+ DailyQuestionAnswer.builder()
+ .user(user)
+ .question(todayQuestion) // 클라이언트 입력 대신 서버 결정 값 사용
+ .answer(requestDto.getAnswer())
+ .answeredDate(today)
+ .build();
+
+ try {
+ dailyQuestionAnswerRepository.save(answer);
+ } catch (DataIntegrityViolationException e) {
+ // (user_id, answered_date) 유니크 제약 위배 → 이미 응답한 상태
+ throw new IllegalStateException("이미 오늘의 질문에 응답하였습니다.", e);
+ }추가로, 타임존 경계(자정) 정확도를 높이려면 Clock 주입 후 LocalDate.now(clock) 사용을 고려하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LocalDate today = LocalDate.now(); | |
| // 사용자가 오늘 이미 답변했는지 확인 | |
| if (dailyQuestionAnswerRepository.existsByUserAndAnsweredDate(user, today)) { | |
| throw new IllegalStateException("User has already answered today's question."); | |
| } | |
| DailyQuestionAnswer answer = | |
| DailyQuestionAnswer.builder() | |
| .user(user) // 매개변수로 받은 user 객체를 바로 사용 | |
| .question(requestDto.getQuestion()) | |
| .answer(requestDto.getAnswer()) | |
| .answeredDate(today) | |
| .build(); | |
| dailyQuestionAnswerRepository.save(answer); | |
| } | |
| LocalDate today = LocalDate.now(); | |
| String todayQuestion = dailyQuestionService.getQuestionForToday(); | |
| DailyQuestionAnswer answer = | |
| DailyQuestionAnswer.builder() | |
| .user(user) | |
| .question(todayQuestion) // 클라이언트 입력 대신 서버 결정 값 사용 | |
| .answer(requestDto.getAnswer()) | |
| .answeredDate(today) | |
| .build(); | |
| try { | |
| dailyQuestionAnswerRepository.save(answer); | |
| } catch (DataIntegrityViolationException e) { | |
| // (user_id, answered_date) 유니크 제약 위배 → 이미 응답한 상태 | |
| throw new IllegalStateException("이미 오늘의 질문에 응답하였습니다.", e); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/member/daily_question/service/DailyQuestionAnswerService.java
around lines 25 to 41, replace the use of requestDto.getQuestion() and the
existsByUserAndAnsweredDate check: obtain today via an injected Clock (use
LocalDate.now(clock)) and fetch the server-side "today's question" to set on the
entity rather than using client input; remove the pre-check
existsByUserAndAnsweredDate to avoid race conditions and instead call save()
inside a try/catch that maps the
unique-constraint/DataIntegrityViolationException thrown by the repository to
the existing IllegalStateException (or a custom conflict exception) so
concurrent saves are handled via DB uniqueness; ensure Clock is injected into
the service constructor and used for date resolution.
|
|
||
| @Getter | ||
| @Setter | ||
| @AllArgsConstructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Jackson 역직렬화 실패 가능성 — @NoArgsConstructor 누락
@AllArgsConstructor 추가로 기본 생성자가 사라집니다. @RequestBody(JSON) 바인딩 시 Jackson은 기본 생성자를 선호하므로 런타임 역직렬화 실패 가능성이 큽니다. 테스트에서는 new GuestbookRequest("...")로 생성해 통과하지만, 실제 컨트롤러 바인딩에서는 문제가 날 수 있습니다. @NoArgsConstructor를 함께 추가해 주세요.
권장 수정안:
-import lombok.AllArgsConstructor;
+import lombok.AllArgsConstructor;
+import lombok.NoArgsConstructor;
@@
-@AllArgsConstructor
+@AllArgsConstructor
+@NoArgsConstructor문제 재현 가능성 점검 스크립트:
🏁 Script executed:
#!/bin/bash
# GuestbookRequest가 @RequestBody로 사용되는지 확인
rg -nP -C3 '(@RequestBody\s+GuestbookRequest|GuestbookRequest)' src/main/java/com/example/cp_main_be/domain/social/guestbookLength of output: 5124
GuestbookRequest에 기본 생성자(@NoArgsConstructor) 추가 필요
현재 GuestbookRequest 클래스에 @AllArgsConstructor만 선언되어 있어 기본(no-arg) 생성자가 제거되었습니다.
GuestbookController의 아래 시그니처에서 Jackson이 기본 생성자를 통해 객체를 인스턴스화하기 때문에, 런타임에 역직렬화 실패가 발생할 수 있습니다.
- 확인 위치
src/main/java/com/example/cp_main_be/domain/social/guestbook/presentation/GuestbookController.java:30
@RequestBody @Valid GuestbookRequest request사용
권장 수정안:
-import lombok.AllArgsConstructor;
+import lombok.AllArgsConstructor;
+import lombok.NoArgsConstructor;
@@
-@AllArgsConstructor
+@AllArgsConstructor
+@NoArgsConstructor
public class GuestbookRequest {
@NotBlank(message = "방명록 내용은 필수입니다.")
public String content;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @AllArgsConstructor | |
| ++ b/src/main/java/com/example/cp_main_be/domain/social/guestbook/dto/request/GuestbookRequest.java | |
| @@ | |
| import lombok.AllArgsConstructor; | |
| import lombok.NoArgsConstructor; | |
| @@ | |
| @AllArgsConstructor | |
| @NoArgsConstructor | |
| public class GuestbookRequest { | |
| @NotBlank(message = "방명록 내용은 필수입니다.") | |
| public String content; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/social/guestbook/dto/request/GuestbookRequest.java
around line 10, the class currently declares only @AllArgsConstructor which
removes the no-arg constructor required for Jackson deserialization; add the
Lombok annotation @NoArgsConstructor to the class (or manually add a public
no-argument constructor) so Jackson can instantiate the DTO when binding
@RequestBody in GuestbookController, and ensure Lombok is imported/available for
compilation.
| // @DisplayName("텃밭 물주기 성공") | ||
| // @Test | ||
| // void waterGarden_Success() { | ||
| // // given | ||
| // Long gardenId = 1L; | ||
| // User user = User.builder().id(1L).uuid(UUID.randomUUID()).username("testuser").build(); | ||
| // Garden garden = Garden.builder().user(user).slotNumber(1).build(); | ||
| // | ||
| // given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden)); | ||
| // | ||
| // // when | ||
| // gardenService.waterGarden(gardenId); | ||
| // | ||
| // // then | ||
| // assertThat(garden.getWaterCount()).isEqualTo(1); | ||
| // } | ||
| // | ||
| // @DisplayName("텃밭 햇빛 주기 성공") | ||
| // @Test | ||
| // void sunlightGarden_Success() { | ||
| // // given | ||
| // Long gardenId = 1L; | ||
| // User user = User.builder().id(1L).uuid(UUID.randomUUID()).username("testuser").build(); | ||
| // Garden garden = Garden.builder().user(user).slotNumber(1).build(); | ||
| // given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden)); | ||
| // | ||
| // // when | ||
| // gardenService.sunlightGarden(gardenId); | ||
| // | ||
| // // then | ||
| // assertThat(garden.getSunlightCount()).isEqualTo(1); | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
주석 처리된 물주기/햇빛 주기 테스트를 신규 시그니처(행위자 포함) 기준으로 복구하세요
waterGarden, sunlightGarden의 메서드 시그니처/권한 체크/이벤트 발행으로 로직이 바뀐 것으로 보이며(행위자 ID 필요), 기존 테스트를 단순 주석 처리하면 커버리지가 크게 줄고 회귀를 잡기 어렵습니다. 최소한 “본인 행위자” 성공 경로에 대한 단위 테스트를 복구하고, 필요 시 이벤트/포인트 적립 의존성은 Mock 주입으로 검증하세요.
아래는 이 구간을 즉시 복구하는 최소 변경 예시입니다(행위자=소유자). 실제 시그니처와 반환타입에 맞게 조정해 주세요.
- // @DisplayName("텃밭 물주기 성공")
- // @Test
- // void waterGarden_Success() {
- // // given
- // Long gardenId = 1L;
- // User user = User.builder().id(1L).uuid(UUID.randomUUID()).username("testuser").build();
- // Garden garden = Garden.builder().user(user).slotNumber(1).build();
- //
- // given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
- //
- // // when
- // gardenService.waterGarden(gardenId);
- //
- // // then
- // assertThat(garden.getWaterCount()).isEqualTo(1);
- // }
- //
- // @DisplayName("텃밭 햇빛 주기 성공")
- // @Test
- // void sunlightGarden_Success() {
- // // given
- // Long gardenId = 1L;
- // User user = User.builder().id(1L).uuid(UUID.randomUUID()).username("testuser").build();
- // Garden garden = Garden.builder().user(user).slotNumber(1).build();
- // given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
- //
- // // when
- // gardenService.sunlightGarden(gardenId);
- //
- // // then
- // assertThat(garden.getSunlightCount()).isEqualTo(1);
- // }
+ @DisplayName("텃밭 물주기 성공 - 본인")
+ @Test
+ void waterGarden_Success() {
+ // given
+ Long actorId = 1L;
+ Long gardenId = 1L;
+ User user = User.builder().id(actorId).uuid(UUID.randomUUID()).username("testuser").build();
+ Garden garden = Garden.builder().user(user).slotNumber(1).build();
+ given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
+
+ // when
+ gardenService.waterGarden(actorId, gardenId);
+
+ // then
+ assertThat(garden.getWaterCount()).isEqualTo(1);
+ }
+
+ @DisplayName("텃밭 햇빛 주기 성공 - 본인")
+ @Test
+ void sunlightGarden_Success() {
+ // given
+ Long actorId = 1L;
+ Long gardenId = 1L;
+ User user = User.builder().id(actorId).uuid(UUID.randomUUID()).username("testuser").build();
+ Garden garden = Garden.builder().user(user).slotNumber(1).build();
+ given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
+
+ // when
+ gardenService.sunlightGarden(actorId, gardenId);
+
+ // then
+ assertThat(garden.getSunlightCount()).isEqualTo(1);
+ }추가로, GardenService가 이벤트 발행/포인트 적립을 수행한다면 다음 Mock을 테스트 클래스 상단에 추가해 NPE를 방지하고 상호작용을 검증하세요(패키지/타입은 실제 구현에 맞게 조정 필요).
// 파일 상단 필드로 추가 권장
@Mock private org.springframework.context.ApplicationEventPublisher eventPublisher;
// 예: 포인트/점수 서비스가 있다면
@Mock private com.example.cp_main_be.domain.point.ScoreService scoreService;메서드 시그니처와 의존성을 정확히 파악하려면 아래 스크립트로 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# GardenService 내 메서드 시그니처/의존성 확인
rg -nP -C2 --type=java '\bwaterGarden\s*\(' src/main/java
rg -nP -C2 --type=java '\bsunlightGarden\s*\(' src/main/java
rg -nP -C3 --type=java 'ApplicationEventPublisher|publishEvent|ScoreService|Point' src/main/javaLength of output: 17517
GardenServiceTest: waterGarden/sunlightGarden 테스트 복구 및 의존성 모킹
실행하신 스크립트 결과, GardenService.waterGarden(Long actorId, Long gardenId) 및 sunlightGarden(Long actorId, Long gardenId) 시그니처가 변경되었으며, 내부에서 UserService와 ApplicationEventPublisher를 사용하고 있습니다. 따라서 주석 처리된 기존 테스트를 복원할 때 아래 사항을 반영해야 NPE 방지 및 로직/이벤트/포인트 적립 검증이 가능합니다.
• GardenServiceTest 클래스 상단에 의존성 모킹 추가
@Mock private GardenRepository gardenRepository;@Mock private UserService userService;@Mock private ApplicationEventPublisher eventPublisher;@InjectMocks private GardenService gardenService;
• 테스트 메서드 복구 시 반영할 사항
userService.findUserById(actorId)호출에 대한 Stub 처리given(userService.findUserById(actorId)).willReturn(owner);
- 물주기/햇빛 주기 후 포인트 적립 및 이벤트 발행 상호작용 검증
then(userService).should().addExperience(actorId, GardenService.WATERING_POINTS); then(eventPublisher).should().publishEvent(any(WateredByFriendEvent.class));
- 햇빛 주기 테스트에도 동일한 패턴으로
SunlightByFriendEvent등을 검증
아래는 최소한의 복구 예시입니다(패키지·타입·상수명은 실제 구현에 맞게 조정).
--- a/src/test/java/com/example/cp_main_be/domain/garden/garden/service/GardenServiceTest.java
+++ b/src/test/java/com/example/cp_main_be/domain/garden/garden/service/GardenServiceTest.java
@@ 클래스 선언부 상단
@ExtendWith(MockitoExtension.class)
class GardenServiceTest {
- @Mock private GardenRepository gardenRepository;
- @Mock private UserService userService;
- @Mock private ApplicationEventPublisher eventPublisher;
- @InjectMocks private GardenService gardenService;
+ @Mock private GardenRepository gardenRepository;
+ @Mock private UserService userService;
+ @Mock private ApplicationEventPublisher eventPublisher;
+ @InjectMocks private GardenService gardenService;
@@ @DisplayName("텃밭 물주기 성공 - 본인")
- // given
- Long actorId = 1L;
- Long gardenId = 1L;
- User user = User.builder().id(actorId).uuid(UUID.randomUUID()).username("testuser").build();
- given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
+ // given
+ Long actorId = 1L;
+ Long gardenId = 1L;
+ User owner = User.builder()
+ .id(actorId)
+ .uuid(UUID.randomUUID())
+ .username("testuser")
+ .build();
+ given(userService.findUserById(actorId)).willReturn(owner);
+ Garden garden = Garden.builder()
+ .user(owner)
+ .slotNumber(1)
+ .build();
+ given(gardenRepository.findById(gardenId))
+ .willReturn(Optional.of(garden));
@@ // when
- gardenService.waterGarden(actorId, gardenId);
+ gardenService.waterGarden(actorId, gardenId);
@@ // then
- assertThat(garden.getWaterCount()).isEqualTo(1);
+ assertThat(garden.getWaterCount()).isEqualTo(1);
+ then(userService).should().addExperience(actorId, GardenService.WATERING_POINTS);
+ then(eventPublisher).should().publishEvent(any(WateredByFriendEvent.class));
@@ @DisplayName("텃밭 햇빛 주기 성공 - 본인")
- // given
- Long actorId = 1L;
- Long gardenId = 1L;
- User user = User.builder().id(actorId).uuid(UUID.randomUUID()).username("testuser").build();
- given(gardenRepository.findById(gardenId)).willReturn(Optional.of(garden));
+ // given
+ Long actorId = 1L;
+ Long gardenId = 1L;
+ User owner = User.builder()
+ .id(actorId)
+ .uuid(UUID.randomUUID())
+ .username("testuser")
+ .build();
+ given(userService.findUserById(actorId)).willReturn(owner);
+ Garden garden = Garden.builder()
+ .user(owner)
+ .slotNumber(1)
+ .build();
+ given(gardenRepository.findById(gardenId))
+ .willReturn(Optional.of(garden));
@@ // when
- gardenService.sunlightGarden(actorId, gardenId);
+ gardenService.sunlightGarden(actorId, gardenId);
@@ // then
- assertThat(garden.getSunlightCount()).isEqualTo(1);
+ assertThat(garden.getSunlightCount()).isEqualTo(1);
+ then(userService).should().addExperience(actorId, GardenService.SUNLIGHT_POINTS);
+ then(eventPublisher).should().publishEvent(any(SunlightByFriendEvent.class));🤖 Prompt for AI Agents
In
src/test/java/com/example/cp_main_be/domain/garden/service/GardenServiceTest.java
around lines 65-96, restore the commented waterGarden/sunlightGarden tests but
update them for the new method signatures and dependencies: add @Mock
GardenRepository, @Mock UserService, @Mock ApplicationEventPublisher and
@InjectMocks GardenService at the test class top; in each test stub
userService.findUserById(actorId) to return the owner user and
gardenRepository.findById(gardenId) to return the garden; call
gardenService.waterGarden(actorId, gardenId) /
gardenService.sunlightGarden(actorId, gardenId); then verify interactions:
userService.addExperience(actorId, <appropriate points constant>) was called and
eventPublisher.publishEvent(...) was called with the correct event type
(WateredByFriendEvent / SunlightByFriendEvent); adjust any constant/type names
to match the production code.
| import com.example.cp_main_be.domain.social.diary.domain.Diary; | ||
| import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository; | ||
| import com.example.cp_main_be.domain.social.follow.domain.Follow; | ||
| import com.example.cp_main_be.domain.social.follow.domain.repository.FollowRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DiaryRepository import 경로의 대소문자 오류로 컴파일 실패 가능
패키지명은 관례상 소문자여야 하며, 다른 Repository import들과도 불일치합니다. 실제 패키지가 ...domain.repository라면 아래처럼 수정하세요.
-import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository;
+import com.example.cp_main_be.domain.social.diary.domain.repository.DiaryRepository;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import com.example.cp_main_be.domain.social.diary.domain.Diary; | |
| import com.example.cp_main_be.domain.social.diary.domain.Repository.DiaryRepository; | |
| import com.example.cp_main_be.domain.social.follow.domain.Follow; | |
| import com.example.cp_main_be.domain.social.follow.domain.repository.FollowRepository; | |
| import com.example.cp_main_be.domain.social.diary.domain.Diary; | |
| import com.example.cp_main_be.domain.social.diary.domain.repository.DiaryRepository; | |
| import com.example.cp_main_be.domain.social.follow.domain.Follow; | |
| import com.example.cp_main_be.domain.social.follow.domain.repository.FollowRepository; |
🤖 Prompt for AI Agents
In
src/test/java/com/example/cp_main_be/domain/social/feed/service/FeedServiceTest.java
around lines 15 to 18, the import for DiaryRepository uses an uppercase
"Repository" segment causing a package name mismatch; change the import to the
correct lowercase package path (e.g.,
com.example.cp_main_be.domain.social.diary.domain.repository.DiaryRepository) so
it matches the actual package and other repository imports, then update any IDE
imports or rebuild to ensure compilation succeeds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (12)
src/main/java/com/example/cp_main_be/domain/delivery/dto/response/DeliveryResponse.java (1)
3-3: 빈 DTO는 유지보수성/일관성 저하 — 실제 응답 스키마를 정의하거나 미사용 시 제거 권장현재 빈 클래스로 직렬화 시
{}만 내려가며, 컨트롤러/서비스가 반환하지도 않습니다. 사용 계획이 없다면 제거하고, 사용할 예정이라면 식별자/주소/생성시각 등 클라이언트가 바로 쓰는 필드를 포함한 응답 스키마와 정적 팩토리(from)를 정의해 주세요.적용 예시(참고용):
+package com.example.cp_main_be.domain.delivery.dto.response; + +import com.example.cp_main_be.domain.delivery.domain.Delivery; +import java.time.LocalDateTime; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; + - public class DeliveryResponse {} +@Getter +@Builder +@AllArgsConstructor +public class DeliveryResponse { + private final Long id; + private final String recipientName; + private final String recipientPhone; // 마스킹 고려 권장 + private final String postalCode; + private final String address; + private final String addressDetail; + private final String message; + private final LocalDateTime createdAt; + + public static DeliveryResponse from(Delivery d) { + return DeliveryResponse.builder() + .id(d.getId()) + .recipientName(d.getRecipientName()) + .recipientPhone(d.getRecipientPhone()) + .postalCode(d.getPostalCode()) + .address(d.getAddress()) + .addressDetail(d.getAddressDetail()) + .message(d.getMessage()) + .createdAt(d.getCreatedAt()) + .build(); + } +}src/main/java/com/example/cp_main_be/domain/delivery/dto/request/DeliveryRequest.java (1)
14-16: 전화번호/우편번호 형식 검증 보강 권장@notblank 만으로는 형식 오류를 걸러내지 못합니다. 간단한 정규식 검증을 추가해 서버측 데이터 무결성을 강화하세요.
예시:
import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; @NotBlank(message = "수령인 연락처는 필수입니다.") + @Pattern( + regexp = "^[0-9\\-]{9,15}$", + message = "연락처 형식이 올바르지 않습니다." + ) private String recipientPhone; @NotBlank(message = "우편번호는 필수입니다.") + @Pattern( + regexp = "^[0-9]{5}$", + message = "우편번호는 5자리 숫자여야 합니다." + ) private String postalCode;추가로 주소/메시지에 @SiZe 상한을 두는 것도 권장합니다(예: 주소 255, 메시지 500 등).
Also applies to: 17-21
src/main/java/com/example/cp_main_be/domain/delivery/service/DeliveryService.java (1)
25-26: 생성 결과(id) 반환해 API 201/Location 설정 가능하게 하기현재 void 반환으로 컨트롤러가 200 OK + 빈 본문만 응답합니다. 생성된 배송 id를 반환하면 REST 관례(201 Created, Location 헤더)와 클라이언트 후속 요청(상세 조회/리다이렉트)이 쉬워집니다.
- public void createDeliveryRequest(Long userId, DeliveryRequest request) { + public Long createDeliveryRequest(Long userId, DeliveryRequest request) { ... - Delivery savedDelivery = deliveryRepository.save(delivery); // 👈 저장 후 객체 받기 + Delivery savedDelivery = deliveryRepository.save(delivery); // 👈 저장 후 객체 받기 ... - notificationService.send( - user, - user, - NotificationType.SEED_DELIVERY, - "/deliveries/" + savedDelivery.getId() // 배송 상세 조회 페이지 URL - ); + notificationService.send( + user, + user, + NotificationType.SEED_DELIVERY, + "/deliveries/" + savedDelivery.getId() + ); + return savedDelivery.getId(); }컨트롤러에서 반환값을 받아 201 Created + Location 설정을 할 수 있습니다(아래 컨트롤러 코멘트 참고).
Also applies to: 42-52
src/main/java/com/example/cp_main_be/domain/delivery/presentation/DeliveryController.java (1)
35-35: 201 Created + Location 헤더로 생성 응답 개선리소스 생성 API는 200 OK 대신 201 Created와 Location 헤더를 사용하는 것이 표준적입니다. 서비스가 id를 반환하도록 변경했다면 다음과 같이 개선 가능합니다.
- return ResponseEntity.ok(ApiResponse.success(null)); + Long id = deliveryService.createDeliveryRequest(userId, deliveryRequest); + return ResponseEntity + .created(java.net.URI.create("/api/v1/deliveries/" + id)) + .body(ApiResponse.success(null));src/main/java/com/example/cp_main_be/domain/delivery/domain/Delivery.java (8)
11-14: 엔티티에 Auditing 리스너와 user_id 인덱스 추가 제안
- 생성일시 관리를 위해 Spring Data JPA Auditing 리스너를 붙이면 일관성과 테스트 용이성이 올라갑니다.
user_id조회가 잦을 가능성이 높으므로 인덱스를 미리 추가해두면 좋습니다.아래처럼 주석 바로 위에 어노테이션을 추가해 주세요.
@Entity +@EntityListeners(AuditingEntityListener.class) +@Table( + name = "delivery", + indexes = { + @Index(name = "idx_delivery_user_id", columnList = "user_id") + } +) @Getter @NoArgsConstructor(access = AccessLevel.PROTECTED) public class Delivery {
3-10: Auditing 및 null 방어를 위한 import 추가Auditing과 생성자 인자 null 방어를 위한 유틸 import가 필요합니다.
import com.example.cp_main_be.domain.member.user.domain.User; import jakarta.persistence.*; import java.time.LocalDateTime; +import java.util.Objects; import lombok.AccessLevel; import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; +import org.springframework.data.annotation.CreatedDate; +import org.springframework.data.jpa.domain.support.AuditingEntityListener;
43-49: 생성일시는 @PrePersist 대신 Auditing(@CreatedDate)로 관리 + not updatable
- 서비스/테스트 일관성을 위해
@CreatedDate+ Auditing 리스너 사용을 권장합니다.- 컬럼을
updatable = false로 고정해 무결성을 강화하세요.- 타임존 이슈가 민감하면
LocalDateTime대신Instant또는OffsetDateTime도 고려해 주세요.- private LocalDateTime createdAt; + @CreatedDate + @Column(nullable = false, updatable = false) + private LocalDateTime createdAt; - @PrePersist - public void prePersist() { - this.createdAt = LocalDateTime.now(); - // this.status = DeliveryStatus.REQUESTED; - } + // Auditing으로 대체되어 @PrePersist는 제거합니다.
24-37: 컬럼 길이/제약 추가로 데이터 품질 및 저장소 보호DB 스키마 레벨에서 길이/제약을 명확히 하면 데이터 품질을 보장하고, 인덱스 효율도 좋아집니다. 운영 DB에선 마이그레이션이 필요할 수 있어 적용 전 확인 부탁드립니다.
- @Column(nullable = false) + @Column(nullable = false, length = 50) private String recipientName; - @Column(nullable = false) + @Column(nullable = false, length = 20) private String recipientPhone; - @Column(nullable = false) + @Column(nullable = false, length = 20) private String postalCode; - @Column(nullable = false) + @Column(nullable = false, length = 255) private String address; - private String addressDetail; - private String message; + @Column(length = 255) + private String addressDetail; + @Column(length = 200) + private String message;추가로, 엔티티 레벨에서도
@Size,@Pattern(전화번호 형식) 같은 Bean Validation을 병행하면 방어선이 두터워집니다. DTO에서 1차 검증이 있더라도, 내부 사용/배치 경로를 통한 유입을 막는 데 유용합니다.
27-37: 전화번호/주소 등 PII 암호화 고려전화번호·주소·메시지는 PII입니다. DB 평문 저장 대신 JPA
AttributeConverter기반 컬럼 단위 암호화를 검토해 주세요. KMS/HSM 연동 또는 앱 레벨 키 롤오버 전략도 함께 설계하면 좋습니다.예) 컨버터 적용 예시
// 컨버터가 준비되어 있다면 @Convert(converter = Encryptor.class) private String recipientPhone; @Convert(converter = Encryptor.class) private String address; // 필요 시 message도 동일하게 적용도입 시 쿼리 조건(where)로 해당 컬럼을 쓰지 않도록 주의가 필요합니다(불가피할 땐 해시 보조 컬럼 추가).
39-42: 배송 상태 Enum은 지금 활성화해도 복잡도 낮음주문/배송 도메인은 상태 전이가 필수로 따라옵니다. 초기에 최소 상태라도 도입해 두면 이후 이관 비용이 줄어듭니다.
- // @Enumerated(EnumType.STRING) - // private DeliveryStatus status; + @Enumerated(EnumType.STRING) + @Column(nullable = false, length = 20) + private DeliveryStatus status = DeliveryStatus.REQUESTED;별도 파일로
DeliveryStatus(REQUESTED, IN_PROGRESS, COMPLETED, CANCELED 등) 정의와, 상태 전이 메서드(도메인 규칙 캡슐화)를 추가하는 방식을 권장합니다. 원하시면 스텁 코드 제공해 드리겠습니다.
51-67: 빌더 진입점에서 필수 인자 null 방어DTO 검증이 있더라도 서비스 내부/배치 경로 유입을 막기 위해 필수 필드를 방어해 주세요.
@Builder public Delivery( User user, String recipientName, String recipientPhone, String postalCode, String address, String addressDetail, String message) { - this.user = user; - this.recipientName = recipientName; - this.recipientPhone = recipientPhone; - this.postalCode = postalCode; - this.address = address; + this.user = Objects.requireNonNull(user, "user must not be null"); + this.recipientName = Objects.requireNonNull(recipientName, "recipientName must not be null"); + this.recipientPhone = Objects.requireNonNull(recipientPhone, "recipientPhone must not be null"); + this.postalCode = Objects.requireNonNull(postalCode, "postalCode must not be null"); + this.address = Objects.requireNonNull(address, "address must not be null"); this.addressDetail = addressDetail; this.message = message; }빈 문자열까지 막으려면
Assert.hasText(...)(Spring) 또는 커스텀 유틸 사용을 고려해 주세요.
37-37: 필드 명확성(nit): message → deliveryMessage의미가 다소 모호해 보입니다. 도메인 명확성을 위해
deliveryMessage같은 이름을 고려해 주세요. 호환성 이슈가 있으면 API/DB 마이그레이션 계획이 필요합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/main/java/com/example/cp_main_be/domain/delivery/domain/Delivery.java(1 hunks)src/main/java/com/example/cp_main_be/domain/delivery/domain/repository/DeliveryRepository.java(1 hunks)src/main/java/com/example/cp_main_be/domain/delivery/dto/request/DeliveryRequest.java(1 hunks)src/main/java/com/example/cp_main_be/domain/delivery/dto/response/DeliveryResponse.java(1 hunks)src/main/java/com/example/cp_main_be/domain/delivery/presentation/DeliveryController.java(1 hunks)src/main/java/com/example/cp_main_be/domain/delivery/service/DeliveryService.java(1 hunks)src/main/java/com/example/cp_main_be/domain/reports/presentation/ReportController.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/cp_main_be/domain/reports/presentation/ReportController.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/example/cp_main_be/domain/delivery/service/DeliveryService.java (1)
src/main/java/com/example/cp_main_be/global/exception/UserNotFoundException.java (1)
UserNotFoundException(3-7)
src/main/java/com/example/cp_main_be/domain/delivery/presentation/DeliveryController.java (1)
src/main/java/com/example/cp_main_be/domain/reports/presentation/ReportController.java (1)
RestController(16-32)
src/main/java/com/example/cp_main_be/domain/delivery/domain/Delivery.java (1)
src/main/java/com/example/cp_main_be/domain/delivery/dto/request/DeliveryRequest.java (1)
Getter(7-25)
🔇 Additional comments (3)
src/main/java/com/example/cp_main_be/domain/delivery/domain/repository/DeliveryRepository.java (1)
6-6: LGTM — 표준 Spring Data JPA 리포지토리 정의 적절별도 @repository 애노테이션 없이도 프록시 빈이 등록됩니다. 현재 범위에서는 충분합니다.
src/main/java/com/example/cp_main_be/domain/delivery/domain/Delivery.java (2)
20-22: ManyToOne 기본 EAGER를 LAZY로 명시한 점 좋아요
@ManyToOne(fetch = FetchType.LAZY)로 과도한 로딩을 방지한 선택이 합리적입니다. 영속성 컨텍스트 밖 직렬화만 피하면 성능과 N+1 측면에서 도움이 됩니다.
20-22: LAZY 로딩 직렬화 주의(검증 요청)컨트롤러에서 엔티티를 직접 반환하면
user의 LAZY 프록시로 인해LazyInitializationException이 날 수 있습니다. 반드시 DTO 매핑 후 반환하는지 확인 부탁드립니다.
| @Getter | ||
| @NoArgsConstructor | ||
| public class DeliveryRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jackson 역직렬화 실패 가능성 — 세터/생성자/빌더 누락
현재 @Getter + @NoArgsConstructor 만으로는 기본 설정의 Jackson에서 필드 바인딩이 실패할 가능성이 큽니다(특히 private 필드, 세터 없음). 요청 바인딩 시 400(Bad Request)로 떨어질 수 있습니다. 세터를 추가하거나(간단) 빌더/@AllArgsConstructor + @JsonCreator를 도입해 주세요.
간단 수정안(@Setter 추가):
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.NoArgsConstructor;
+import lombok.Setter;
@Getter
+@Setter
@NoArgsConstructor
public class DeliveryRequest {대안(불변 DTO 선호 시): @Builder + @AllArgsConstructor(onConstructor=@__(@JsonCreator)) 를 사용하고 세터는 제거하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/delivery/dto/request/DeliveryRequest.java
around lines 7-9, Jackson deserialization may fail because the class only has
@Getter and @NoArgsConstructor (private fields without setters), so add either
standard setters (e.g., annotate with @Setter or add explicit setX methods) to
allow Jackson to bind request fields, or convert to an immutable DTO by
replacing setters with @Builder and
@AllArgsConstructor(onConstructor=@__(@JsonCreator)) so Jackson can construct
the object via the annotated constructor; pick one approach and update
imports/annotations accordingly.
| @AuthenticationPrincipal UserDetails userDetails, // 현재 로그인한 사용자 정보 | ||
| @Valid @RequestBody DeliveryRequest deliveryRequest) { | ||
|
|
||
| // userDetails에서 사용자 ID를 추출하여 서비스에 전달 | ||
| // Long userId = ((CustomUserDetails) userDetails).getId(); // UserDetails 구현체에 맞게 수정 | ||
| Long userId = 1L; // 임시 ID, 실제로는 위와 같이 인증 정보에서 가져와야 합니다. | ||
|
|
||
| deliveryService.createDeliveryRequest(userId, deliveryRequest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
하드코딩된 userId=1L 제거 — 인증 사용자 ID를 @AuthenticationPrincipal로 직접 주입
고정값 1L는 보안·권한 측면에서 치명적이며, 실제 사용자 데이터가 오염됩니다. 프로젝트 내 다른 컨트롤러(예: ReportController)와 동일하게 Long userId를 직접 주입 받도록 변경하세요.
- public ResponseEntity<ApiResponse<Void>> requestDelivery(
- @AuthenticationPrincipal UserDetails userDetails, // 현재 로그인한 사용자 정보
- @Valid @RequestBody DeliveryRequest deliveryRequest) {
-
- // userDetails에서 사용자 ID를 추출하여 서비스에 전달
- // Long userId = ((CustomUserDetails) userDetails).getId(); // UserDetails 구현체에 맞게 수정
- Long userId = 1L; // 임시 ID, 실제로는 위와 같이 인증 정보에서 가져와야 합니다.
-
- deliveryService.createDeliveryRequest(userId, deliveryRequest);
+ public ResponseEntity<ApiResponse<Void>> requestDelivery(
+ @AuthenticationPrincipal Long userId,
+ @Valid @RequestBody DeliveryRequest deliveryRequest) {
+
+ deliveryService.createDeliveryRequest(userId, deliveryRequest);검증용 스크립트:
결과에 따라 보안 설정(Principal 타입)을 일관화하세요.
🏁 Script executed:
#!/bin/bash
# 프로젝트에서 @AuthenticationPrincipal Long 사용 패턴 확인
rg -nP -C2 --type=java '@AuthenticationPrincipal\s+Long\b'
# 혹시 CustomUserDetails 패턴을 쓰는지 확인
rg -nP -C3 --type=java 'class\s+CustomUserDetails\b|implements\s+UserDetails'Length of output: 3229
DeliveryController에서 하드코딩된 userId 제거 및 @AuthenticationPrincipal Long userId 적용 필요
DeliveryController의 userId = 1L 하드코딩은 보안·권한 관리에 심각한 문제를 야기합니다. 프로젝트 내 다른 컨트롤러들(ReportController, GardenController)은 이미 @AuthenticationPrincipal Long userId를 사용하여 인증된 사용자 ID를 일관되게 주입하고 있으므로, 동일한 방식으로 수정해야 합니다.
-
대상 파일:
src/main/java/com/example/cp_main_be/domain/delivery/presentation/DeliveryController.java:26-33 -
수정 예시(diff):
- public ResponseEntity<ApiResponse<Void>> requestDelivery( - @AuthenticationPrincipal UserDetails userDetails, // 현재 로그인한 사용자 정보 - @Valid @RequestBody DeliveryRequest deliveryRequest) { - - // userDetails에서 사용자 ID를 추출하여 서비스에 전달 - // Long userId = ((CustomUserDetails) userDetails).getId(); // UserDetails 구현체에 맞게 수정 - Long userId = 1L; // 임시 ID, 실제로는 위와 같이 인증 정보에서 가져와야 합니다. - - deliveryService.createDeliveryRequest(userId, deliveryRequest); + public ResponseEntity<ApiResponse<Void>> requestDelivery( + @AuthenticationPrincipal Long userId, + @Valid @RequestBody DeliveryRequest deliveryRequest) { + + deliveryService.createDeliveryRequest(userId, deliveryRequest);
-
검증 결과:
ReportController, GardenController 등에서 이미@AuthenticationPrincipal Long userId패턴 사용 확인됨 (rg 결과)
위 수정으로 보안·권한 로직을 일관화하고, 실제 로그인된 사용자 ID가 서비스 로직에 전달되도록 보장하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @AuthenticationPrincipal UserDetails userDetails, // 현재 로그인한 사용자 정보 | |
| @Valid @RequestBody DeliveryRequest deliveryRequest) { | |
| // userDetails에서 사용자 ID를 추출하여 서비스에 전달 | |
| // Long userId = ((CustomUserDetails) userDetails).getId(); // UserDetails 구현체에 맞게 수정 | |
| Long userId = 1L; // 임시 ID, 실제로는 위와 같이 인증 정보에서 가져와야 합니다. | |
| deliveryService.createDeliveryRequest(userId, deliveryRequest); | |
| public ResponseEntity<ApiResponse<Void>> requestDelivery( | |
| @AuthenticationPrincipal Long userId, | |
| @Valid @RequestBody DeliveryRequest deliveryRequest) { | |
| deliveryService.createDeliveryRequest(userId, deliveryRequest); |
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/delivery/presentation/DeliveryController.java
around lines 26-33, remove the hardcoded Long userId = 1L and change the
controller method to accept the authenticated user id by replacing the current
@AuthenticationPrincipal UserDetails userDetails parameter with
@AuthenticationPrincipal Long userId (or add @AuthenticationPrincipal Long
userId alongside if other details are needed), then pass that userId into
deliveryService.createDeliveryRequest(userId, deliveryRequest); and remove the
temporary comment and unused casts so the real authenticated user id is used
consistently like ReportController/GardenController.
| notificationService.send( | ||
| user, | ||
| user, | ||
| NotificationType.SEED_DELIVERY, | ||
| "/deliveries/" + savedDelivery.getId() // 배송 상세 조회 페이지 URL | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
알림 전송의 트랜잭션 타이밍 — 커밋 전 발송 위험(AFTER_COMMIT 권장)
현재 send 호출이 같은 트랜잭션 내에서 실행되어 롤백 시 “없는 리소스에 대한 알림”이 발송될 수 있습니다. 이벤트 발행 + @TransactionalEventListener(phase = AFTER_COMMIT) 패턴으로 커밋 이후에 알림을 보내세요.
적용 스케치(핵심 변경만, 실제 이벤트 클래스/리스너는 프로젝트 표준에 맞춰 조정):
- notificationService.send(
- user,
- user,
- NotificationType.SEED_DELIVERY,
- "/deliveries/" + savedDelivery.getId()
- );
+ publisher.publishEvent(
+ new SeedDeliveryStartedEvent(
+ savedDelivery.getId(),
+ user.getId(),
+ "/deliveries/" + savedDelivery.getId())
+ );클래스 상단(필드 주입 추가 — Lombok @requiredargsconstructor로 생성자 자동 생성):
private final org.springframework.context.ApplicationEventPublisher publisher;검증용 스크립트(레포지토리 루트에서 실행):
예상: AFTER_COMMIT 리스너가 없으면 위 제안을 적용하는 것이 안전합니다.
🏁 Script executed:
#!/bin/bash
# NotificationService 사용 방식/시그니처 확인
rg -nP -C3 --type=java 'class\s+NotificationService\b'
rg -nP -C3 --type=java '\bNotificationService\b[^\n]*\bsend\s*\('
# AFTER_COMMIT 리스너 존재 여부 확인
rg -nP -C2 --type=java '@TransactionalEventListener|TransactionPhase\.AFTER_COMMIT'
# 이미 존재하는 SeedDelivery 관련 이벤트/리스너가 있는지 확인
rg -nP -C3 --type=java 'SeedDeliveryStartedEvent|deliver(y|ies).*(Event|Listener)'Length of output: 1861
🏁 Script executed:
#!/bin/bash
set -e
# Inspect send(...) methods in NotificationService
rg -n -nP --type=java 'send\s*\(' -C3 src/main/java/com/example/cp_main_be/domain/member/notification/service/NotificationService.java || true
# Dump the existing SeedDeliveryStartedEvent signature
echo "==== SeedDeliveryStartedEvent class ===="
sed -n '1,200p' src/main/java/com/example/cp_main_be/global/event/SeedDeliveryStartedEvent.java || trueLength of output: 1073
🏁 Script executed:
#!/bin/bash
set -e
# Locate send method signature and usages in NotificationService
rg --type=java '\bsend\s*\(' -C3 src/main/java/com/example/cp_main_be/domain/member/notification/service/NotificationService.java || true
# Print implementation context around the send method declaration
sed -n '1,200p' src/main/java/com/example/cp_main_be/domain/member/notification/service/NotificationService.java | sed -n '30,120p'Length of output: 5323
커밋 이후 알림 발송 패턴 적용 권장 (AFTER_COMMIT)
현재 DeliveryService에서 배송 생성 직후 즉시 notificationService.send(…)를 호출하고 있어, 서비스 트랜잭션이 롤백될 경우 “존재하지 않는 배송”에 대한 알림이 발송될 위험이 있습니다. Spring 이벤트 + @TransactionalEventListener(phase = AFTER_COMMIT) 패턴으로 트랜잭션 커밋 이후에 알림을 보내도록 리팩터링을 반드시 적용하세요.
검증 결과:
NotificationService.send(…)는@Transactional이 적용된 클래스 내부 메서드로, 호출 시점에 동일 트랜잭션 컨텍스트를 사용합니다.- 코드베이스에
@TransactionalEventListener(phase = AFTER_COMMIT)또는TransactionPhase.AFTER_COMMIT사용 사례가 없습니다. - 이미
SeedDeliveryStartedEvent클래스만 존재할 뿐, 해당 이벤트를 처리하는 리스너는 없습니다.
수정 포인트:
- DeliveryService.java(46–51)
- 직접 호출하던
notificationService.send(…)코드를 이벤트 발행으로 대체 ApplicationEventPublisher주입
- 직접 호출하던
- 이벤트 클래스
- 기존
SeedDeliveryStartedEvent를 배송 ID, URL, 사용자 ID를 담도록 확장하거나, 새 이벤트 클래스 정의
- 기존
- 이벤트 리스너
@Component+@TransactionalEventListener(phase = AFTER_COMMIT)로 구현- 리스너 내부에서
notificationService.send(…)호출
예시 스케치:
--- src/main/java/com/example/cp_main_be/domain/delivery/service/DeliveryService.java
@@ -46,6 +46,7 @@ public class DeliveryService {
Delivery savedDelivery = deliveryRepository.save(delivery);
+ publisher.publishEvent(new SeedDeliveryStartedEvent(
+ savedDelivery.getId(),
+ user.getId(),
+ "/deliveries/" + savedDelivery.getId()));
}// DeliveryService 클래스 상단
private final ApplicationEventPublisher publisher;// 새 이벤트 리스너 예시
@Component
public class SeedDeliveryEventListener {
private final NotificationService notificationService;
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void onSeedDeliveryStarted(SeedDeliveryStartedEvent event) {
User user = userRepository.findById(event.getUserId()).orElseThrow();
notificationService.send(
user, user, NotificationType.SEED_DELIVERY, event.getUrl());
}
}이렇게 변경하면, 트랜잭션 커밋이 보장된 이후에만 알림이 전송되어 “없는 리소스” 에 대한 알림 발송 문제를 방지할 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/cp_main_be/domain/delivery/service/DeliveryService.java
around lines 46–51, replace the direct call to notificationService.send(...)
with publishing an application event (inject ApplicationEventPublisher into
DeliveryService) carrying deliveryId, url and userId; create or extend
SeedDeliveryStartedEvent to contain these fields; implement a @Component event
listener class with @TransactionalEventListener(phase =
TransactionPhase.AFTER_COMMIT) that resolves the user (via repository) and calls
notificationService.send(...) using data from the event; ensure the publisher
publishes the event after saving the delivery so the notification is sent only
after transaction commit.
close #40
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Refactor
Documentation
Tests