[Suhhee] Week7 미션#73
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acb83055db
ℹ️ 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".
| String[] cursorSplit = cursor.split(":"); | ||
| Double scoreCursor = Double.parseDouble(cursorSplit[0]); | ||
| Long idCursor = Long.parseLong(cursorSplit[1]); |
There was a problem hiding this comment.
query=rate일 때 cursor를 무조건 score:id 형식으로 가정하고 바로 인덱싱/파싱해서, 형식이 조금만 어긋나도 ArrayIndexOutOfBoundsException/NumberFormatException이 발생하고 현재 전역 처리 흐름상 500으로 떨어집니다. Spring 관점에서 이는 비즈니스 오류가 아니라 요청값 검증 실패이므로 400으로 명확히 내려주는 것이 맞고, cursor 전용 요청 DTO(또는 파서)에서 형식/범위를 먼저 검증한 뒤 GeneralException으로 변환하세요. 다음 학습 키워드는 Validation boundary와 예외 계층(입력 예외 vs 시스템 예외)입니다.
Useful? React with 👍 / 👎.
| .storeName(review.getStore().getStoreName()) | ||
| .nickname(review.getUser().getNickname()) |
There was a problem hiding this comment.
리뷰를 조회한 뒤 DTO 매핑에서 store, user, ownerComment의 지연 로딩 연관을 순회 접근하고 있어 페이지 크기만큼 추가 쿼리가 반복되는 N+1이 발생합니다. Spring/JPA 학습 관점에서도 객체 그래프 탐색을 서비스 매핑에서 무심코 수행하면 읽기 성능과 트랜잭션 의존성이 강해지는 전형적인 케이스이므로, 조회 쿼리에서 fetch join/@EntityGraph를 사용하거나 처음부터 DTO 프로젝션으로 필요한 필드만 조회하도록 분리하는 편이 좋습니다. 다음 학습 키워드는 지연 로딩과 N+1의 근본 원인 및 읽기 전용 쿼리 설계입니다.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
Page와 Slice, Stream API, 객체 그래프 탐색, @Valid/@validated의 핵심 차이를 코드 예시와 함께 정리한 점이 좋습니다. 특히 Slice가 COUNT 쿼리를 생략한다는 관점과 검증 예외 타입 차이를 함께 정리하여 이번 미션 코드와 연결하기에 적절합니다. 추가로 객체 그래프 탐색은 지연 로딩, fetch join, @EntityGraph, batch size가 각각 어떤 상황에서 사용되는지까지 비교하면 JPA 조회 성능을 이해하는 데 도움이 됩니다.
[코드 리뷰]
전체적으로 하드코딩된 사용자 ID를 제거하고 페이지네이션 응답 DTO를 도입하려는 방향이 적절합니다. 리뷰 조회도 id 기반과 별점 기반 커서를 분리하여 정렬 기준을 명확히 하려는 시도가 좋습니다. 다만 현재 변경에는 컴파일 가능성, 검증 경계, JPA 연관관계 조회 책임과 관련하여 반드시 점검해야 할 부분이 있습니다. 특히 존재하지 않는 연관 엔티티 참조와 Lazy 연관 탐색에 따른 N+1 가능성은 서비스 계층과 Repository 계층의 책임을 나누어 보완할 필요가 있습니다.
| private List<ReviewPhoto> reviewPhotoList = new ArrayList<>(); | ||
|
|
||
| @OneToOne(mappedBy = "review", fetch = FetchType.LAZY, cascade = CascadeType.ALL) | ||
| private OwnerComment ownerComment; |
There was a problem hiding this comment.
현재 브랜치에서 OwnerComment 타입 정의가 확인되지 않아 컴파일 실패 가능성이 높습니다. 연관관계를 추가할 때는 대상 엔티티, 패키지 위치, import, mappedBy 필드명이 함께 일치해야 합니다. 리뷰와 사장님 답글의 생명주기 소유자가 누구인지 먼저 정리한 뒤 @OneToOne의 owning side와 cascade 범위를 결정하는 방식을 권장합니다.
| List<ReviewResponseDto.MyReviewDto> dtoList = reviewSlice.getContent().stream() | ||
| .map(review -> ReviewResponseDto.MyReviewDto.builder() | ||
| .reviewId(review.getId()) | ||
| .storeName(review.getStore().getStoreName()) |
There was a problem hiding this comment.
Review 목록을 조회한 뒤 store, user, ownerComment를 DTO 변환 중에 순차 접근하고 있어 Lazy 로딩에 의한 N+1 쿼리가 발생할 수 있습니다. 조회 전용 API에서는 Repository가 필요한 연관 데이터를 fetch join, @EntityGraph, 또는 전용 projection으로 한 번에 가져오도록 설계하면 Service는 변환 책임에 집중하고 조회 책임은 Repository에 모일 수 있습니다.
| if (cursor.equals("-1")) { | ||
| reviewSlice = reviewRepository.findMyReviewsOrderByScoreDesc(userId, pageRequest); | ||
| } else { | ||
| String[] cursorSplit = cursor.split(":"); |
There was a problem hiding this comment.
커서를 문자열로 직접 split/parse하면 rate 정렬 커서 형식이 잘못 들어온 경우 ArrayIndexOutOfBoundsException이나 NumberFormatException이 일반 예외 처리로 흘러갈 수 있습니다. Controller 요청 DTO 또는 별도 Cursor 값 객체를 두고 형식 검증과 파싱 책임을 분리하면 Service의 분기 로직이 단순해지고 예외 응답도 일관되게 관리할 수 있습니다.
| @Schema(description = "진행 중인 미션 조회 요청") | ||
| public record GetMyMissionsDto( | ||
| @Schema(description = "조회할 유저 ID", example = "1") | ||
| Long userId |
There was a problem hiding this comment.
요청 본문으로 userId를 받는 구조라면 @NotNull 같은 Bean Validation 제약과 Controller의 @Valid 적용이 함께 필요합니다. 현재 상태에서는 null이 Service까지 전달되어 Repository 조회 조건으로 사용될 수 있으므로, MVC 흐름에서 Controller가 입력 검증 경계 역할을 하도록 보완하는 것을 권장합니다.
| * `comment`: 댓글 객체 | ||
| * `comment.getUser()`: 댓글을 작성한 사용자 (User) | ||
| * `comment.getUser().getName()`: 사용자의 이름 (Name) | ||
|
|
There was a problem hiding this comment.
객체 그래프 탐색과 N+1 문제를 함께 연결한 점이 좋습니다. 추가로 fetch join만 해결책으로 고정하기보다 @EntityGraph, batch size, DTO projection이 각각 어떤 조회 목적에 적합한지도 비교하면 JPA 조회 전략을 더 입체적으로 이해할 수 있습니다.
| @@ -41,12 +40,17 @@ public MissionResponseDto.MissionListDto getMissions(String status, Integer page | |||
| .build()) | |||
| .collect(Collectors.toList()); | |||
There was a problem hiding this comment.
큰 차이는 아니지만,
.collect(Collectors.toList())의 경우 가변 리스트를 반환하며,
.toList()는 불변 리스트를 반환합니다.
해당 Stream이 반환하는 값은 DTO로 보입니다. 설계 측면에서 불변으로 전달하는 것도 괜찮은 방법이지 싶네요.
| */ | ||
| Long dummyUserId = 1L; | ||
| User user = userRepository.findById(dummyUserId) | ||
| .orElseThrow(() -> new RuntimeException("해당 유저를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
RuntimeException의 경우, Unchked Exception의 최상위 클래스로서, 전역 예외 핸들러에서 5xx 상태를 반환할 가능성이 높습니다. 인지하고 있으시다면 괜찮지만, 커스텀 예외 객체를 던지는 것을 권장합니다.
3 participants
🔗 연관 이슈
🛠 작업 내용
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
내가 작성한 코드에서 잘못 작성된 부분, 로직이 잘못된 부분이 있으면 해당 부분에 대해서 알려줘
🧠 AI 응답
잘못 작성된 로직에 대한 수정사항 + 오타 수정
✅ 내가 최종 선택한 방법 (이유)
💡 나만의 Tip (선택)