[Hyeonu] Week7 미션#72
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9c56cefa6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| missionList=missionRepository.findMissionByStore_IdOrderByIdDesc(storeId,pageRequest); | ||
| } | ||
| // 다음 커서 계산 | ||
| nextCursor=missionList.getContent().getLast().getId()+":"+missionList.getContent().getLast().getId(); |
There was a problem hiding this comment.
missionList가 비어 있어도 바로 getLast()를 호출해 NoSuchElementException이 발생합니다. 가게에 미션이 0건인 요청에서 500 오류가 나면 커서 페이징의 기본 동작(빈 목록도 정상 응답)이 깨집니다. missionList.getContent().isEmpty()를 먼저 확인해 빈 경우 nextCursor를 null로 두고, 데이터가 있을 때만 마지막 원소로 커서를 계산하도록 분기하세요. 다음 학습으로 Slice 페이징의 엣지 케이스(0건/마지막 페이지) 처리를 정리해보면 좋습니다.
Useful? React with 👍 / 👎.
| ){ | ||
| return Mission.builder() | ||
| .store(store) | ||
| .status(dto.status()) |
There was a problem hiding this comment.
미션 생성 시 status 기본값을 애플리케이션에서 보장하세요
미션 생성 시 status를 검증 없이 dto.status()로 그대로 저장하고 있어 null 입력이 그대로 DB에 들어갈 수 있습니다. JPA에서는 null 필드도 INSERT에 포함되므로 컬럼 DEFAULT 'READY'가 기대대로 적용되지 않을 수 있고, 이후 m.status='READY' 조건 조회에서 방금 생성한 미션이 누락될 수 있습니다. status == null이면 READY를 세팅하거나 DTO에 @NotNull을 추가해 입력 규칙을 명확히 하세요. 다음 학습으로는 JPA에서 엔티티 기본값/DB 기본값이 적용되는 시점을 확인해보면 좋습니다.
Useful? React with 👍 / 👎.
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class ReviewException extends RuntimeException { |
There was a problem hiding this comment.
ReviewException을 공통 예외 체계에 연결하세요
ReviewException이 ProjectException이 아니라 RuntimeException을 직접 상속해, query가 잘못된 경우 의도한 400 계열 코드 대신 전역 Exception 핸들러로 떨어져 500으로 응답됩니다. Spring 학습 관점에서 예외 타입-핸들러 매핑이 끊기면 API 계약을 이해하기 어려워집니다. ReviewException을 ProjectException 기반으로 바꾸고(ReviewErrorCode도 공통 에러 코드 인터페이스 구현), 전역 핸들러가 해당 코드를 사용하도록 맞추세요. 다음으로 예외 계층 설계와 @RestControllerAdvice 매핑 규칙을 같이 학습해보세요.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
Page와 Slice, Stream API, 객체 그래프 탐색, Bean Validation의 기본 차이를 정리한 점이 적절합니다. 추가로 Slice는 다음 페이지 존재 여부를 확인하지만 그 자체가 곧 커서 기반 페이지네이션은 아니라는 점, 객체 그래프 탐색은 지연 로딩과 트랜잭션 범위에 따라 N+1 문제가 발생할 수 있다는 점을 함께 정리하면 JPA 이해도가 높아집니다.
[코드 리뷰]
컨트롤러-서비스-리포지토리 계층을 나누고 DTO와 컨버터를 활용해 응답을 구성한 점이 좋습니다. 예외 코드와 성공 코드를 도메인별로 분리하려는 방향도 적절합니다. 다만 조회 API의 HTTP 메서드 선택, 커서 페이지네이션의 빈 결과 처리, 요청 DTO 검증, 서비스 계층의 경계값 방어를 보완하면 MVC 책임 분리와 안정성이 더 높아집니다. Cursor 기반 조회는 첫 페이지, 마지막 페이지, 빈 결과, 잘못된 query 값의 흐름을 각각 테스트해 보는 것을 권장합니다.
| | 전체 개수 | O | X | | ||
| | Count 쿼리 | O | X | | ||
| | 성능 | 상대적으로 느림 | 상대적으로 빠름 | | ||
| | 사용처 | 게시판, 관리자 페이지 | 무한스크롤, 커서 페이징 | |
There was a problem hiding this comment.
Slice는 COUNT 쿼리를 생략하고 다음 페이지 존재 여부를 확인하는 반환 타입이지만, 그 자체가 곧 커서 페이징을 의미하지는 않습니다. PageRequest의 page 번호를 사용하는 Slice는 여전히 offset 기반으로 동작할 수 있으므로, offset 기반 Slice와 id/rating 기반 cursor pagination의 차이를 별도로 정리하는 것을 권장합니다.
| @PathVariable Long missionId, | ||
| @RequestBody MissionReqDTO.ChallengeDTO request | ||
| // 내가 진행중이거나 완료한 미션 목록 조회(페이징 포함) | ||
| @PostMapping("/members/missions") |
There was a problem hiding this comment.
목록 조회는 리소스를 생성하지 않으므로 일반적으로 GET에 query parameter를 사용하는 편이 REST 관점에서 더 자연스럽습니다. 요청 조건이 복잡해 POST를 선택할 수도 있지만, 단순한 memberId/status/page 조회라면 컨트롤러의 역할과 HTTP 메서드 의미가 일치하도록 GET 설계를 우선 검토하는 것을 권장합니다.
| String nextCursor; | ||
|
|
||
| // 커서가 있는 경우 | ||
| if(!cursor.equals("-1")) { |
There was a problem hiding this comment.
cursor가 null로 전달되면 equals 호출에서 NullPointerException이 발생할 수 있습니다. 컨트롤러에서 기본값을 지정하거나 서비스에서 cursor != null && !cursor.equals("-1")처럼 방어하면 서비스 계층의 입력 경계 처리가 더 안정적입니다.
| missionList=missionRepository.findMissionByStore_IdOrderByIdDesc(storeId,pageRequest); | ||
| } | ||
| // 다음 커서 계산 | ||
| nextCursor=missionList.getContent().getLast().getId()+":"+missionList.getContent().getLast().getId(); |
There was a problem hiding this comment.
조회 결과가 비어 있으면 getLast() 호출에서 예외가 발생합니다. 또한 다음 페이지가 없을 때도 nextCursor를 만들 필요가 없으므로, ReviewService처럼 hasNext()와 isEmpty()를 함께 확인한 뒤 커서를 생성하는 방식으로 통일하는 것을 권장합니다.
| public class ReviewReqDTO { | ||
| @Builder | ||
| public record CreateReview( | ||
| Long memberId, |
There was a problem hiding this comment.
리뷰 생성 요청의 memberId, rating, content에도 Bean Validation 제약을 두면 컨트롤러 진입 시점에 잘못된 요청을 일관되게 걸러낼 수 있습니다. 예를 들어 memberId는 @NotNull, rating은 범위 검증, content는 @NotBlank를 검토하면 DTO가 요청 계약을 표현하는 책임을 더 잘 수행합니다.
| @PathVariable Long storeId, | ||
| @RequestBody @Valid MissionReqDTO.CreateMission dto // Valid 검증 어노테이션떄매 사용 | ||
| ){ | ||
| BaseSuccessCode code=MissionSuccessCode.CREATED; |
There was a problem hiding this comment.
해당 방식대로 변수를 선언하여 Return하는 방식도 있지만, ApiResponse의 인자로 MissionSuccessCode.CREATED를 바로 작성할 수 있기도 합니다.
🔗 연관 이슈
closes #70
🛠 작업 내용
6주차 코드 전체 수정
내가 진행중인 미션 조회
내가 생성한 리뷰를 조회
RequestBody에 있는 검증 어노테이션 체크
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
내가 진행중인 미션 조회하기 (오프셋 기반 페이지네이션으로 응답하기, 사용자 ID는 Request Body에서 받기 하드코딩 X)
이 미션을 수행하려는데 내가 지금 작성된 코드가 ~인데 사용자 ID는 Request Body에서 받기 이 부분을 어떻게하라는거야?
🧠 AI 응답
✅ 내가 최종 선택한 방법 (이유)
RequestBody에서 사용자 id를 받으려는데 스웨거에서 오류가 떠서 GetMapping으로 바꾸니까 바로됨
💡 나만의 Tip (선택)