Conversation
Walkthrough지정한 순위 범위(startRank, endRank)에 따른 장소 랭킹 조회 기능이 추가되었습니다. 저장소 쿼리, 서비스 메서드, 응답 레코드, 컨트롤러 엔드포인트, 컨트롤러 테스트 및 관련 예외 상태가 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as RankController
participant Service as RankService
participant Repo as PlaceRankRepository
participant Response as GetPlaceRankResponse
Note over Client,Controller: 클라이언트가 범위 기반 랭킹 요청 발송
Client->>Controller: GET /api/v1/places/ranks/{placeId}?startRank=X&endRank=Y
Controller->>Service: getPlaceRanks(userDetails, placeId, X, Y)
Service->>Service: validateRankRange(X, Y)
alt 유효한 범위
Service->>Repo: findRankByRange(placeId, X, Y)
Repo-->>Service: List<PlaceRankWithRankingProjection>
Service->>Service: TreeMap으로 랭킹별 그룹화
loop 각 랭킹 그룹 처리
Service->>Response: GetPlaceRankResponse.from(group, user)
Response-->>Service: GetPlaceRankResponse
end
Service-->>Controller: List<GetPlaceRankResponse>
Controller-->>Client: BaseResponse.ok(...)
else 범위 오류
Service-->>Controller: throw INVALID_RANK_RANGE
Controller-->>Client: Error 400
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/main/java/ku_rum/backend/domain/rank/application/RankService.java (1)
126-128: 사용되지 않는 파라미터를 제거하세요.
customUserDetails파라미터가 전달되지만 실제로는userService.getUser()를 통해 사용자를 조회하고 있습니다. 불필요한 파라미터를 제거하거나, 다른 메서드들과의 일관성을 위해 해당 파라미터를 활용하도록 수정하세요.-public List<GetPlaceRankResponse> getPlaceRanks(CustomUserDetails customUserDetails, Long placeId, int startRank, - int endRank) { +public List<GetPlaceRankResponse> getPlaceRanks(Long placeId, int startRank, int endRank) { User user = userService.getUser();또는 일관성을 위해 다른 메서드처럼 파라미터를 활용:
public List<GetPlaceRankResponse> getPlaceRanks(CustomUserDetails customUserDetails, Long placeId, int startRank, int endRank) { - User user = userService.getUser(); + User user = userService.findById(customUserDetails.getUserPK());src/test/java/ku_rum/backend/domain/rank/presentation/RankControllerTest.java (1)
111-162: 엣지 케이스 테스트 추가를 고려하세요.기본 시나리오에 대한 테스트는 잘 작성되었습니다. 다음과 같은 추가 테스트 케이스를 고려해보세요:
startRank > endRank인 경우startRank또는endRank가 음수인 경우- 범위에 해당하는 랭킹 데이터가 없는 경우
- 매우 큰 범위를 요청하는 경우
예시:
@Test @WithMockUser(username = "testUser", roles = {"USER"}) void getPlaceRank_InvalidRange_ThrowsException() throws Exception { Long placeId = 75L; mockMvc.perform(get("/api/v1/places/ranks/{placeId}", placeId) .param("startRank", "10") .param("endRank", "1") .header("Authorization", "Bearer ...")) .andExpect(status().isBadRequest()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/ku_rum/backend/domain/rank/application/RankService.java(2 hunks)src/main/java/ku_rum/backend/domain/rank/application/response/GetPlaceRankResponse.java(1 hunks)src/main/java/ku_rum/backend/domain/rank/domain/repository/PlaceRankRepository.java(1 hunks)src/main/java/ku_rum/backend/domain/rank/presentation/RankController.java(3 hunks)src/test/java/ku_rum/backend/domain/rank/presentation/RankControllerTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/ku_rum/backend/domain/rank/application/response/GetPlaceRankResponse.java (1)
src/main/java/ku_rum/backend/domain/rank/application/response/PlaceUserRankResponse.java (1)
PlaceUserRankResponse(10-35)
🔇 Additional comments (1)
src/test/java/ku_rum/backend/domain/rank/presentation/RankControllerTest.java (1)
31-31: 테스트 범위 변경이 적절합니다.
@SpringBootTest에서@WebMvcTest(RankController.class)로 변경하여 테스트 범위를 좁힌 것은 좋은 개선입니다. 컨트롤러 레이어만 테스트하므로 실행 속도가 빠르고 의존성이 명확해집니다.
| public record GetPlaceRankResponse(int ranking, List<String> nickname, int sharingCount, boolean isSelf) { | ||
|
|
||
| public static GetPlaceRankResponse from(List<PlaceRankWithRankingProjection> placeRanks, User user) { | ||
| if (placeRanks.isEmpty()) { | ||
| throw new GlobalException(BaseExceptionResponseStatus.RANK_NOT_FOUND); | ||
| } | ||
|
|
||
| List<String> names = placeRanks.stream() | ||
| .sorted(Comparator.comparing(PlaceRankWithRankingProjection::getModifiedAt)) | ||
| .map(placeRankWithRankingProjection -> placeRankWithRankingProjection.getNickname()) | ||
| .toList(); | ||
|
|
||
| int count = placeRanks.stream() | ||
| .findFirst() | ||
| .get() | ||
| .getCount(); | ||
|
|
||
| boolean isSelf = placeRanks.stream() | ||
| .findFirst() | ||
| .map(placeRankWithRankingProjection -> placeRankWithRankingProjection.getNickname() | ||
| .equals(user.getNickname())) | ||
| .orElse(false); | ||
|
|
||
| return new GetPlaceRankResponse(placeRanks.get(0).getRanking(), names, count, isSelf); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
코드 중복을 제거하세요.
GetPlaceRankResponse와 PlaceUserRankResponse (src/main/java/ku_rum/backend/domain/rank/application/response/PlaceUserRankResponse.java)가 거의 동일한 구조와 로직을 가지고 있습니다. 이는 DRY(Don't Repeat Yourself) 원칙을 위반합니다.
다음 중 하나의 방법을 고려하세요:
방법 1: 공통 로직을 유틸리티 클래스로 추출
public class RankResponseMapper {
public static <T extends RankResponse> T from(
List<PlaceRankWithRankingProjection> placeRanks,
User user,
BiFunction<Integer, List<String>, Integer, Boolean, T> constructor) {
// 공통 로직
}
}방법 2: 두 클래스가 실제로 동일한 목적이라면 하나로 통합
사용처를 확인하여 GetPlaceRankResponse와 PlaceUserRankResponse의 차이가 명명뿐이라면, 하나의 클래스로 통합하고 의미있는 이름을 사용하세요.
두 응답 클래스의 사용처를 확인하여 통합 가능한지 검증하세요:
🏁 Script executed:
#!/bin/bash
# Description: Check usage of both response classes
echo "=== GetPlaceRankResponse 사용처 ==="
rg -n "GetPlaceRankResponse" --type=java -C3
echo -e "\n=== PlaceUserRankResponse 사용처 ==="
rg -n "PlaceUserRankResponse" --type=java -C3Length of output: 30003
GetPlaceRankResponse와 PlaceUserRankResponse 통합 필요
두 클래스가 완전히 동일한 구조와 로직을 가지고 있습니다. PlaceUserRankResponse (src/main/java/ku_rum/backend/domain/rank/application/response/PlaceUserRankResponse.java)와 비교하면 record 정의, 필드, factory 메서드 구현이 모두 같습니다.
이 중복을 제거하세요:
- 단일 클래스로 통합하거나
- 공통 로직을 추출하여 재사용
현재 두 클래스가 서로 다른 service 메서드에서 사용되고 있으나, 응답 구조가 동일하므로 하나의 클래스를 여러 곳에서 재사용하는 것이 적절합니다.
| @Query(value = """ | ||
| SELECT | ||
| ranked.rank_id AS rankId, | ||
| ranked.count AS count, | ||
| u.nickname AS nickname, | ||
| ranked.place_place_id AS placePlaceId, | ||
| ranked.created_at AS createdAt, | ||
| ranked.modified_at AS modifiedAt, | ||
| ranked.ranking AS ranking | ||
| FROM ( | ||
| SELECT | ||
| pr.*, | ||
| DENSE_RANK() OVER (ORDER BY pr.count DESC) AS ranking | ||
| FROM place_rank pr | ||
| WHERE place_place_id =:placeId | ||
| ) ranked | ||
| JOIN users u | ||
| ON u.id = ranked.user_id | ||
| WHERE ranking >= :startRank AND ranking <= :endRank | ||
| ORDER BY ranking | ||
| """, nativeQuery = true) | ||
| List<PlaceRankWithRankingProjection> findRankByRange(@Param("placeId") Long placeId, | ||
| @Param("startRank") int startRank, | ||
| @Param("endRank") int endRank); |
There was a problem hiding this comment.
🧩 Analysis chain
입력값 검증이 필요합니다.
새로운 쿼리 메서드가 올바르게 구현되었지만, startRank와 endRank 파라미터에 대한 검증이 필요합니다. 컨트롤러 또는 서비스 레이어에서 다음 사항을 확인해야 합니다:
startRank와endRank가 양수인지startRank <= endRank인지- 적절한 범위 제한 (예: 최대 100개 조회 제한 등)
다음 스크립트로 컨트롤러와 서비스에서 검증 로직이 있는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Check if there's input validation for startRank and endRank
# Search for validation logic in controller and service
rg -n -A5 -B5 'startRank.*endRank|endRank.*startRank' --type=javaLength of output: 4090
🏁 Script executed:
#!/bin/bash
# Description: Find RankService implementation and check for validation logic in getPlaceRanks method
# Find the RankService interface/class
fd -e java RankService --type f
# Then search for the getPlaceRanks method implementation and surrounding validation
rg -n -A10 -B2 'getPlaceRanks' --type=java -iLength of output: 7325
🏁 Script executed:
#!/bin/bash
# Description: Read the complete RankService.getPlaceRanks method with range parameters
cat git/src/main/java/ku_rum/backend/domain/rank/application/RankService.java | sed -n '126,155p'Length of output: 910
입력값 검증 로직이 누락되었습니다. (중요)
RankService.getPlaceRanks() 메서드(126-145줄)에서 startRank와 endRank 파라미터가 검증 없이 직접 저장소에 전달되고 있습니다. 다음 사항을 반드시 확인하세요:
startRank와endRank가 양수인지 검증startRank <= endRank조건 검증- 조회 범위 제한 (예: 최대 100개 등) 설정
- 유효하지 않은 입력에 대한 예외 처리
컨트롤러(RankController.java 38-44줄)에서 유효성 검사를 추가하거나, 서비스 메서드 시작 부분에서 파라미터 검증 로직을 구현해야 합니다.
| @GetMapping("/ranks/{placeId}") | ||
| public BaseResponse<List<GetPlaceRankResponse>> getPlaceRank( | ||
| @AuthenticationPrincipal final CustomUserDetails userDetails, | ||
| @PathVariable("placeId") Long placeId, | ||
| @RequestParam("startRank") int startRank, | ||
| @RequestParam("endRank") int endRank) { | ||
| return BaseResponse.ok(rankService.getPlaceRanks(userDetails, placeId, startRank, endRank)); | ||
| } |
There was a problem hiding this comment.
요청 파라미터 검증을 추가하세요.
컨트롤러에서 startRank와 endRank 파라미터에 대한 검증이 누락되었습니다. 잘못된 입력값으로 인한 문제를 방지하기 위해 검증 로직을 추가해야 합니다.
다음과 같이 수정하는 것을 권장합니다:
+import jakarta.validation.constraints.Min;
+import jakarta.validation.constraints.Max;
+
@GetMapping("/ranks/{placeId}")
public BaseResponse<List<GetPlaceRankResponse>> getPlaceRank(
@AuthenticationPrincipal final CustomUserDetails userDetails,
@PathVariable("placeId") Long placeId,
- @RequestParam("startRank") int startRank,
- @RequestParam("endRank") int endRank) {
+ @RequestParam("startRank") @Min(1) int startRank,
+ @RequestParam("endRank") @Min(1) @Max(100) int endRank) {
+ if (startRank > endRank) {
+ throw new GlobalException(BaseExceptionResponseStatus.INVALID_RANK_RANGE);
+ }
return BaseResponse.ok(rankService.getPlaceRanks(userDetails, placeId, startRank, endRank));
}또한 @Validated 어노테이션을 클래스에 추가해야 검증이 동작합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/ku_rum/backend/domain/rank/presentation/RankController.java
around lines 37 to 44, the request parameters startRank and endRank lack
validation; add parameter-level constraints (e.g., @Min(1) and/or @Positive for
both, and if there is a max allowed rank use @Max) and annotate the controller
class with @Validated so Spring enforces them, then add a simple guard in the
method to ensure startRank <= endRank and throw a meaningful 400-level exception
(or return a BaseResponse error) when the range is invalid; this combination
ensures invalid inputs are rejected by framework validation and you still
enforce logical ordering.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ku_rum/backend/domain/rank/application/RankService.java(2 hunks)src/main/java/ku_rum/backend/global/support/status/BaseExceptionResponseStatus.java(1 hunks)
🔇 Additional comments (3)
src/main/java/ku_rum/backend/global/support/status/BaseExceptionResponseStatus.java (1)
127-129: LGTM!새로운 에러 상태 코드 추가가 적절합니다. 코드 번호가 순차적이며, BAD_REQUEST 상태와 메시지가 랭킹 유효성 검증 목적에 부합합니다.
src/main/java/ku_rum/backend/domain/rank/application/RankService.java (2)
13-13: LGTM!새로운 응답 타입 임포트가 적절합니다.
146-150: 검증 로직은 올바르게 구현되었습니다.검증 조건들이 적절합니다:
- startRank와 endRank가 1 이상인지 확인
- startRank가 endRank보다 크지 않은지 확인
다만, 위에서 언급한 것처럼 이 메서드가
getPlaceRanks메서드에서 호출되지 않고 있어 현재는 실제로 동작하지 않습니다.
ba3f823 to
a249489
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ku_rum/backend/domain/rank/application/RankService.java(2 hunks)src/main/java/ku_rum/backend/global/support/status/BaseExceptionResponseStatus.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ku_rum/backend/global/support/status/BaseExceptionResponseStatus.java
🔇 Additional comments (1)
src/main/java/ku_rum/backend/domain/rank/application/RankService.java (1)
147-151: 검증 로직이 올바르게 구현되었습니다.입력값 검증 메서드가 잘 구현되어 있습니다. 1부터 시작하는 랭킹 범위를 올바르게 검증하고, 잘못된 입력에 대해 적절한 예외를 발생시킵니다.
| public List<GetPlaceRankResponse> getPlaceRanks(CustomUserDetails customUserDetails, Long placeId, int startRank, | ||
| int endRank) { | ||
| validateRankRange(startRank,endRank) | ||
| User user = userService.getUser(); | ||
| List<PlaceRankWithRankingProjection> placeRankWithRankings = placeRankRepository.findRankByRange(placeId, | ||
| startRank, | ||
| endRank); | ||
|
|
||
| Map<Integer, List<PlaceRankWithRankingProjection>> placeRanksGroupedByCount = placeRankWithRankings.stream() | ||
| .collect(Collectors.groupingBy( | ||
| PlaceRankWithRankingProjection::getRanking, | ||
| TreeMap::new, | ||
| toList()) | ||
| ); | ||
|
|
||
| return placeRanksGroupedByCount.values() | ||
| .stream() | ||
| .map(placeRanks -> GetPlaceRankResponse.from(placeRanks, user)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
구문 오류: 세미콜론 누락
Line 128에서 validateRankRange(startRank,endRank) 호출 뒤에 세미콜론이 누락되어 컴파일 오류가 발생합니다.
추가 이슈:
customUserDetails파라미터가 선언되었으나 사용되지 않고 있습니다. Line 129에서userService.getUser()를 호출하여 사용자를 가져오므로, 이 파라미터는 불필요하거나 일관되게 사용되어야 합니다.- public 메서드에 대한 JavaDoc 주석이 누락되었습니다.
다음 diff를 적용하여 구문 오류를 수정하고 JavaDoc을 추가하세요:
+ /**
+ * 장소 랭킹 페이징 조회
+ *
+ * @param customUserDetails 사용자 인증 정보
+ * @param placeId 장소 ID
+ * @param startRank 시작 랭킹 (1부터 시작)
+ * @param endRank 종료 랭킹
+ * @return 랭킹 범위에 해당하는 장소 랭킹 목록
+ */
public List<GetPlaceRankResponse> getPlaceRanks(CustomUserDetails customUserDetails, Long placeId, int startRank,
int endRank) {
- validateRankRange(startRank,endRank)
+ validateRankRange(startRank, endRank);
User user = userService.getUser();customUserDetails 파라미터의 경우, 사용하지 않는다면 제거하거나, userService.getUser() 대신 이 파라미터로부터 사용자 정보를 추출하여 일관성을 유지하세요.
🤖 Prompt for AI Agents
In src/main/java/ku_rum/backend/domain/rank/application/RankService.java around
lines 126 to 145, there is a missing semicolon after the call to
validateRankRange(startRank, endRank) which causes a compile error; add the
semicolon, then address the unused customUserDetails parameter by either
removing it from the method signature (and updating all call sites) if it is
unnecessary, or replace the call to userService.getUser() with code that
extracts the User from customUserDetails so the parameter is used consistently;
finally add a brief JavaDoc for the public method describing its purpose,
parameters, and return value.
a249489 to
bb90d5c
Compare
src/main/java/ku_rum/backend/domain/rank/application/RankService.java
Outdated
Show resolved
Hide resolved
src/main/java/ku_rum/backend/domain/rank/application/response/GetPlaceRankResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/ku_rum/backend/domain/rank/application/response/GetPlaceRankResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/ku_rum/backend/domain/rank/presentation/RankController.java
Outdated
Show resolved
Hide resolved
88fa6de to
589ec76
Compare
#️⃣ 연관된 이슈
closes #317
📝작업 내용
작업 상세 내용
💬리뷰 요구사항
Summary by CodeRabbit
새로운 기능
검증 / 오류 처리
테스트