[7주차/여니] 워크북 제출합니다#41
Conversation
- API의 구조를 커서 페이징을 적용하기 위한 구조로 변경함. - 미션의 마감기한과 멤버미션의 ID를 커서로 사용함.
- 미션 남은 기한과 미션 아이디를 커서로 사용함. - region 테이블의 지역명을 name -> regionName으로 변경함.
- MemberConverter에서 발생할 수 있는 NullPointerException을 예방하는 로직을 추가함. - Mission Entity의 missionKey에 대한 주석을 추가함.
- id/별점 기준 커서 페이징을 적용함
- MethodArgumentNotValid - HandlerMethodValidation - ConstraintViolation
YoungJJun
left a comment
YoungJJun
left a comment
There was a problem hiding this comment.
7주차 피드백
-
먼저 모든 미션에 대해 API 잘 구현된 것 같습니다!
-
Request Body가 있는 API에 검증 어노테이션 + GeneralExceptionAdvice도 잘 적용해주셨어요.
-
memberId 하드코딩
미션에서 하드코딩 없이 구현하는걸 요구하고 있지만 memberId를 토큰에서 추출한다는 개념만 이해하고 계시고
@RequestBody,@RequestParam,@PathVariable각각 언제 쓰는지만 이해하고 계시면 하드코딩 유지하셔도 괜찮습니다. -
@Validated어노테이션 누락@RequestParam,@PathVariable에@Min,@NotBlank,@Positive등의 Bean Validation 어노테이션을 사용할 때 Controller에@Validated를 붙이지 않으면 동작하지 않을 수 있습니다.반면
@RequestBodyDTO 검증은@Valid만으로도 가능합니다.현재 PR에서
MemberController,MissionController,ReviewController모두@Validated가 누락되어 있는 것 같아요. 한 번 확인해주시면 좋을 것 같습니다.Spring 버전에 따라 자동으로 연동되어 동작하기도 한다고 하는데 안정성을 위해서는 붙이는게 좋을 것 같아요.
추가로 알아두면 좋은 점으로
@RequestBody+@Valid실패 시
MethodArgumentNotValidException이 발생합니다.@RequestParam+@Validated실패 시
ConstraintViolationException이 발생합니다.따라서
@RestControllerAdvice에서 두 예외 모두 각각 처리해줘야 합니다! -
MemberService.getHome()에서 최종 리턴값 goalCount 를 10으로 하드코딩하신 것 같은데 10개 달성 시 보상 지급이라 이렇게 구현하신게 맞으실까요? 의도하신 거라면 그대로 두셔도 됩니다.그런데 이 값이 다른 곳에서도 쓰일 가능성이 있다면, 상수나 설정값으로 한 곳에서 관리하는 방법도 있다는 점만 알아두시면 좋을 것 같아요.
예를 들어
MISSION_GOAL_COUNT같은 상수로 분리하거나application.yml에 두고@Value로 주입받는 식으로요.
여니 이번주도 너무 잘 구현해주셨어요 수고하셨습니당.
chazy-d
left a comment
chazy-d
left a comment
There was a problem hiding this comment.
여니 7주차도 수고 많으셨어요!! 😊
저번주에 리뷰를 못남겨드려서, 리뷰 남기기 위해 코드 플로우를 쭉 따라가며 읽어보았는데, 역시 깔끔하십니다!.! 이번주차도 화이팅!
| @OneToMany(mappedBy = "review", cascade = CascadeType.ALL, orphanRemoval = true) | ||
| @Builder.Default | ||
| @BatchSize(size = 100) | ||
| private List<ReviewPhoto> reviewPhotoList = new ArrayList<>(); | ||
|
|
||
| @OneToMany(mappedBy = "review", cascade = CascadeType.ALL, orphanRemoval = true) | ||
| @Builder.Default | ||
| @BatchSize(size = 100) | ||
| private List<ReviewReply> reviewReplyList = new ArrayList<>(); |
There was a problem hiding this comment.
리뷰 목록에서 사진/답글 컬렉션에 @batchsize를 적용하신 점이 좋아요!! 여러 리뷰의 연관 컬렉션을 조회할 때 N+1을 의식하신건가!? 라는 생각이 들어서 화면 확장까지 고려한게 느껴졌습니다.
| Page<MemberMission> findMemberMissionsUsingOffset( | ||
| @Param("memberId") Long memberId, | ||
| @Param("statuses") List<MissionStatus> statuses, | ||
| Pageable pageable |
There was a problem hiding this comment.
Pageable을 컨트롤러에서 바로 받는 방식은 Spring Data가 공식적으로 지원하는 방식이라 구현이 간단하고, Repository까지 자연스럽게 이어지는 장점이 큰 것 같습니다.
다만 외부 클라이언트와 맞추는 API라면 page가 0부터 시작한다는 점, sort=createdAt,desc처럼 정렬 파라미터가 엔티티 필드명과 직접 연결될 수 있다는 점, size 최대값이나 허용 가능한 정렬 필드를 어디서 제한할지 정도는 팀 규칙으로 정해두면 좋을 것 같습니다. 약간 dto 안쓰고 entity를 그대로 응답하는 느낌?
특히 정렬 조건이 createdAt, deadline 같은 엔티티 필드와 1:1로 대응하지 않고 LATEST, POPULAR처럼 서비스 의미를 갖는 값으로 제공되어야 한다면, 컨트롤러에서는 별도 요청 DTO나 enum을 받고 내부에서 PageRequest로 변환하는 방식도 고려할 수 있을 것 같습니다.
예를 들면 GET /api/missions?pageNumber=1&pageSize=10&sortType=LATEST처럼 외부 API는 서비스 의미를 드러내고, 내부에서 PageRequest.of(...)로 바꾸는 방식입니다.
| public static List<MissionResDTO.MissionDto> toMissionResDTO(List<MemberMission> memberMissions){ | ||
| return memberMissions.stream() | ||
| .map(mm -> MissionResDTO.MissionDto.builder() | ||
| .memberMissionId(mm.getId()) | ||
| .missionId(mm.getMission().getId()) | ||
| .missionDescription(mm.getMission().getDescription()) | ||
| .missionPoints(mm.getMission().getPoints()) | ||
| .missionStatus(mm.getMissionStatus()) | ||
| .storeId(mm.getMission().getStore().getId()) | ||
| .storeName(mm.getMission().getStore().getName()) | ||
| .build() | ||
| ) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
MissionConverter에서 MemberMission -> Mission -> Store 값을 사용하고 있는데, offset 조회 쿼리에서는 MemberMission만 조회하고 있어서 목록 변환 과정에서 mission, store 추가 조회가 발생할 수 있을 것 같습니다.
커서 조회 쪽에서는 이미 join fetch mm.mission, join fetch m.store를 사용하고 있어서, offset 조회도 비슷하게 fetch join을 적용하고 Page 사용을 위해 countQuery를 분리하면 더 일관된 조회 전략이 될 것 같습니다!!
3 participants
✅ 실습 체크리스트
✅ 컨벤션 체크리스트
📌 주안점