[Gibeom] Week7 미션#68
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e26be52d07
ℹ️ 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".
| Slice<Review> findReviewsByMember_IdOrderByScoreDescIdDesc(Long memberId, Pageable pageable); | ||
|
|
||
| @Query("SELECT r FROM Review r WHERE r.member.id = :memberId " + | ||
| "AND (r.score < :scoreCursor OR (r.score = :scoreCursor AND r.id < :idCursor)) " + |
There was a problem hiding this comment.
JPQL 커서 파라미터를 메서드 시그니처와 일치시키세요
JPQL에서 :idCursor를 사용하지만 findReviewsByScoreCursor 시그니처에는 해당 @Param 인자가 없어 named parameter 바인딩이 깨집니다. 이 경우 Spring Data JPA가 쿼리를 해석할 때 예외가 발생해 리포지토리 사용 시점에 실패하므로, @Param("idCursor") Long idCursor를 메서드에 추가하고 서비스에서 커서의 score/id를 함께 전달하도록 맞춰야 합니다. 다음 학습 주제로는 Spring Data JPA의 @Query named parameter 규칙과 동점 정렬용 tie-breaker 커서 설계를 추천합니다.
Useful? React with 👍 / 👎.
| Long prevCursor = Long.parseLong(cursorSplit[0]); //이거 워크북 코드에 왜 있는지? | ||
| idCursor = Long.parseLong(cursorSplit[1]); |
There was a problem hiding this comment.
nextCursor 형식과 파싱 로직을 동일하게 맞추세요
첫 페이지 응답에서 nextCursor를 id:123 형식으로 만들었는데, 다음 요청 파싱에서는 cursorSplit[0]을 Long.parseLong(...)으로 변환하고 있어 두 번째 호출부터 "id"를 숫자로 파싱하다 NumberFormatException이 납니다. 커서 계약은 생성/파싱이 완전히 대칭이어야 하므로 id 접두어를 문자열로 검증한 뒤 두 번째 토큰만 숫자로 읽거나, 아예 숫자 기반 고정 포맷으로 통일하세요. 다음으로 커서 기반 페이지네이션에서 포맷 계약과 입력 검증 전략을 함께 학습하면 좋습니다.
Useful? React with 👍 / 👎.
| List <MemberMission> memberMissions = memberMissionRepository | ||
| .findAllByMember_IdAndStatus(memberId, status); |
There was a problem hiding this comment.
getMissionsByStatus에서 PageRequest를 만들지만 실제 조회는 findAllByMember_IdAndStatus(...)(List 반환)로 호출해서 pageSize/pageNum/sort가 전혀 적용되지 않습니다. 이 상태면 API 시그니처와 동작이 불일치하고, Spring Data 페이징 학습 관점에서도 잘못된 습관이 생기므로 리포지토리 메서드를 Page<MemberMission> + Pageable 형태로 바꿔 pageRequest를 전달하도록 수정하는 것이 좋습니다. 다음 학습 주제는 Page/Slice/List의 차이와 Pageable 적용 흐름입니다.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
JPA, N+1, 로딩 전략, JPQL, Fetch Join, EntityGraph, flush/commit 흐름을 전반적으로 정리한 점은 좋습니다. 다만 지연 로딩 장단점, JPQL 표현, 커밋과 플러시의 트랜잭션 경계는 더 정확히 구분하면 학습 효과가 높아집니다. EntityGraph와 Fetch Join은 어떤 상황에서 선택할지까지 함께 비교해 보기를 권장합니다.
[코드 리뷰]
도메인별 controller/service/repository 구조를 분리하고 DTO 변환 계층을 둔 점은 적절합니다. 이번 범위에서는 페이징 요청값이 실제 repository 쿼리에 반영되는지, 커서 포맷과 JPQL 파라미터가 일치하는지, JPA 연관관계의 소유자와 cascade 방향이 도메인 책임에 맞는지 점검이 필요합니다. 특히 인증 주체, 페이징, 예외 처리는 컨트롤러와 서비스의 책임을 명확히 나누면 MVC 구조가 더 안정적으로 유지됩니다.
| show-sql: true # ??? SQL ??? ??? ???? ?? ?? | ||
| hibernate: | ||
| ddl-auto: update # ?????? ?? ? ?????? ???? ??? ?? | ||
| ddl-auto: create # ?????? ?? ? ?????? ???? ??? ?? |
There was a problem hiding this comment.
ddl-auto: create는 애플리케이션 시작 때 기존 테이블을 다시 만들 수 있어 학습 중 저장한 데이터가 사라질 수 있습니다. 스키마 자동 생성 옵션의 차이를 비교하고, 반복 실행 환경에서는 update 또는 validate처럼 목적에 맞는 값을 선택하는 것을 권장합니다.
| sortInfo = Sort.by("id").descending(); | ||
| } | ||
|
|
||
| PageRequest pageRequest |
There was a problem hiding this comment.
pageSize, pageNum, sort로 PageRequest를 만들었지만 실제 조회는 findAllByMember_IdAndStatus(...)를 호출하여 페이징과 정렬이 적용되지 않습니다. 서비스 메서드의 책임이 API 요청 조건을 repository 쿼리로 전달하는 것이므로 Pageable을 받는 repository 메서드로 연결하는 것을 권장합니다.
| Slice<Review> findReviewsByMember_IdOrderByScoreDescIdDesc(Long memberId, Pageable pageable); | ||
|
|
||
| @Query("SELECT r FROM Review r WHERE r.member.id = :memberId " + | ||
| "AND (r.score < :scoreCursor OR (r.score = :scoreCursor AND r.id < :idCursor)) " + |
There was a problem hiding this comment.
JPQL에서 :idCursor를 사용하지만 메서드 파라미터에는 idCursor가 없습니다. 애플리케이션 시작 시점에 쿼리 생성 오류가 발생할 수 있으므로 @Param("idCursor")를 추가하고, score 기반 커서라면 score와 id를 함께 넘기도록 맞추는 것이 필요합니다.
| } | ||
|
|
||
| //다음 커서 계산 (마지막 요소의 id) | ||
| nextCursor = reviewList.getContent().getLast().getId() + ":" + reviewList.getContent().getLast().getId(); |
There was a problem hiding this comment.
조회 결과가 비어 있으면 getLast() 호출에서 예외가 발생합니다. 커서 페이징에서는 마지막 요소가 없을 때 nextCursor를 null로 내려주고, hasNext와 함께 응답을 구성하는 방식으로 방어 로직을 두는 것을 권장합니다.
| reviewList = reviewRepository.findReviewsByMember_IdOrderByScoreDescIdDesc(memberId, pageRequest); | ||
| } | ||
| //다음 커서 계산 | ||
| nextCursor = reviewList.getContent().getLast().getId() + ":" + reviewList.getContent().getLast().getId(); |
There was a problem hiding this comment.
score 기준 페이징 응답인데 nextCursor가 id:id 형태로 만들어지고 있습니다. 다음 요청에서 scoreCursor를 파싱하는 로직과 커서 생성 형식이 일치해야 하므로 score:id처럼 정렬 기준과 보조 기준을 함께 담는 구조로 통일하는 것이 필요합니다.
| case "score": | ||
| // 커서 타입 변환 | ||
| int scoreCursor = Integer.parseInt(cursorSplit[0]); | ||
| int idCursor = Integer.parseInt(cursorSplit[1]); |
There was a problem hiding this comment.
Review 식별자는 Repository와 엔티티 기준으로 Long 타입입니다. 커서의 id 부분을 int로 파싱하면 타입 일관성이 깨지고, id 값이 커질 때 오버플로우 위험이 있습니다. Long.parseLong(cursorSplit[1])로 파싱하고 Repository 파라미터도 Long으로 맞추는 방식을 권장합니다.
There was a problem hiding this comment.
해당 설정의 경우, Spring Security에 대한 내용으로, 클라이언트가 요청에 동봉한 JWT와 같은 Authorization 필드를 사용해 이 사용자가 본 서버에서 발행된 유저인지 인증하고, 매핑된 요청 path에 대해 접근할 수 있는 권한을 갖고 있는지 인가하는 설정과 관련되어 있습니다.
따라서, 이번 주차에서는 사용하지 않는 내용입니다. 검토 부탁드립니다.
@AuthenticationPrincipal, SecuritiConfig
There was a problem hiding this comment.
스프링의 로그인 기능을 구현을 확인하고 싶어서 임의로 설정해봤었습니다.
피드백 주셔서 감사합니다.
| @AuthenticationPrincipal Long memberId | ||
| ){ | ||
| BaseSuccessCode code = MemberSuccessCode.OK; | ||
| return ApiResponse.onSuccess(code, memberService.getInfo(memberId)); |
There was a problem hiding this comment.
Controller의 응답을 ResponseEntity로 래핑하지 않는 경우, HTTP 상태코드를 커스텀하기 위해서는 항상 ApiResponse에서 정적 팩토리 메소드를 활용하여야 합니다.
ResponseEntity의 경우, 명시적으로 Http 프로토콜 레이어를 제어함을 의미하며, Header, Cookie 등을 제어하는 등 http 프로토콜에 대한 모든 것을 다룰 수 있지만, ApiResponse만 반환하는 형태는 이러한 기능을 지원하지 않으므로, 이미 작성된 패키지를 활용하는 것이 바람직합니다.
코드 예시
@GetMapping("/{userId}/me")
public ResponseEntity<ApiResponse<?>> getInfo(
@PathVariable Long memberId
) {
// ProjectException을 상속한 커스텀 예외 객체를 throw
if(memberId < 1)
throw new IllegalMemberIdException();
// 생략
return ResponseEntity
.status(code.getStatus)
.body(ApiResponse.onSuccess(code, memberService.getInfo(memberId));
}
``There was a problem hiding this comment.
기존 응답인 ApiResponse는 기본 응답(200,500)만 반환하는 단순 소통방식이었음을 확인했습니다.
부가적으로 ResponseEntity로 래핑해 반환시 클라이언트와 어떤 상호작용이 있는지, 사용이유과 장점에 대해 공부하고 리팩토링 해보았습니다. 감사합니다.
| @RequestParam String query | ||
| ) { | ||
| BaseSuccessCode code = ReviewSuccessCode.OK; | ||
| ReviewResDTO.Pagination<ReviewResDTO.getReview> result = reviewService.getMemberReviewsOrderByScore(memberId, pageSize, cursor, query); |
There was a problem hiding this comment.
현재 컨트롤러가 query 값과 관계없이 항상 getMemberReviewsOrderByScore(...)를 호출합니다. 이 상태에서는 query=id 요청이 기존 id 커서 페이지네이션 로직으로 전달되지 않고 QUERY_NOT_VALID 예외가 발생할 수 있습니다. 컨트롤러에서 query 값에 따라 id/score 전용 서비스 메서드를 분기하거나, 서비스 계층의 단일 메서드 내부에서 정렬 전략을 선택하도록 책임을 정리하는 방식을 권장합니다. MVC 관점에서 컨트롤러는 요청 파라미터를 해석해 올바른 유스케이스로 위임하고, 서비스는 해당 정렬 정책을 명확히 수행하도록 역할을 나누면 유지보수성이 높아집니다.
🔗 연관 이슈
🛠 작업 내용
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
커서기반 페이지네이션과 커서 문자열, 파싱원리에 대해 설명해줘.
reviewList에 getId()가 안되는데 해결해줘.
🧠 AI 응답
커서 기반 페이지네이션 로직: 733:733과 같은 커서 문자열의 파싱 원리와 nextCursor를 계산하는 코드의 의미를 상세히 분석해 드렸습니다.
해당 기능은 자바 21버전부터 추가된 기능입니다. 자바 버전을 올리시길 바랍니다.
List
✅ 내가 최종 선택한 방법 (이유)
💡 나만의 Tip (선택)