[release] 진행상황 main 브랜치에 반영 #144
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[feat] 가게가 웨이블존인지 여부를 판단하는 기능 구현
[feat] 다중 경로 추천
Walkthrough대중교통 경로 API가 단일 경로에서 다중 경로 반환으로 변경되었고, DTO 구조가 Route 단위로 확장되었습니다. 탐색 도메인에는 유효성 검증용 /validate 엔드포인트와 유사 존 검색(30m 이내) 로직이 추가되었으며, 관련 DTO 유효성 제약이 완화되고 통합 테스트가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as Transportation API
participant Service as TransportationService
participant Graph as RoutingEngine
Client->>Controller: GET /transportation/routes
Controller->>Service: findRoutes(...)
Service->>Graph: runDijkstra(start, end)
Graph-->>Service: firstRoute
Service->>Graph: penalizeUsedEdges(firstRoute)
Service->>Graph: findAlternativeRoutes(MAX_ROUTES-1)
Graph-->>Service: alternativeRoutes[]
Service->>Service: filterAndSortRoutes(routes)
Service-->>Controller: List<Route> (paged)
Controller-->>Client: 200 OK (routes)
sequenceDiagram
actor Client
participant C as WaybleZoneSearchController
participant S as WaybleZoneSearchService
participant R as WaybleZoneQuerySearchRepository
participant ES as Elasticsearch
Client->>C: GET /api/v1/wayble-zones/search/validate?...
C->>S: isValidWaybleZone(cond)
S->>R: findSimilarWaybleZone(cond)
R->>ES: query(name similarity + geo_distance≤30m)
ES-->>R: top hit or none
R-->>S: WaybleZoneSearchResponseDto or null
S-->>C: dto
C-->>Client: 200 OK (CommonResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
19-28: 조치 필요: Step 레코드의 nullable 필드(null) 생성 및 검증 부족검색 결과, 서버 코드에서 Step 생성 시 moveInfo/routeName/busInfo/subwayInfo가 null로 전달되는 호출을 확인했으며, busInfo()/subwayInfo()에 대한 명시적 null 검사 패턴은 코드베이스에서 발견되지 않았습니다.
문제가 있을 가능성이 있는 위치:
- src/main/java/com/wayble/server/direction/service/TransportationService.java
- mergedSteps.add(new TransportationResponseDto.Step(DirectionType.WALK, null, null, 0, null, null, fromName, toName))
- mergedSteps.add(new TransportationResponseDto.Step(currentType, moveInfoList, routeName, moveNumber, busInfo, ...))
권장 조치:
- 서버에서 nullable 컬렉션/문자열은 null 대신 빈 컬렉션/빈 문자열으로 전달하거나 DTO 직렬화 설정(@JsonInclude 등)으로 null을 생략하도록 통일하세요.
- busInfo/subwayInfo를 사용하는 모든 경로에 대해 null 체크를 추가하거나, 클라이언트에서 null을 안전하게 처리하도록 API 계약과 문서를 명확히 하세요.
- 클라이언트(UI)가 null 값 표시/대체 로직을 가지고 있는지 검증(또는 단위/통합 테스트 추가)하세요.
🧹 Nitpick comments (7)
src/main/java/com/wayble/server/explore/controller/WaybleZoneSearchController.java (1)
70-76: 핸들러 메서드 명명 개선 제안.
findIsValidWaybleZone는 이중 동사 + boolean 뉘앙스로 가독성이 떨어집니다.validateWaybleZone또는findSimilarWaybleZone처럼 역할 중심으로 단순화하는 것을 권장합니다.src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java (2)
580-588: 부분 문자열 추출의 안전성 보완.
zoneName길이가 2 미만인 경우substring(0, 2)에서 예외가 발생합니다. 현재 데이터셋에서는 대부분 2글자 이상이지만, 회귀 방지를 위해 안전 처리하는 것이 좋습니다.- String requestedName = zoneName.substring(0, 2); + String requestedName = zoneName.length() >= 2 ? zoneName.substring(0, 2) : zoneName;
553-555: 테스트 로그 과다 출력 줄이기.테스트에서 매번 pretty print를 출력하면 파이프라인 로그가 과도하게 길어질 수 있습니다. 실패 시에만 진단 정보가 출력되도록 조건부 로깅 또는 assertJ의 withFailMessage 활용으로 노이즈를 줄이는 것을 권장합니다.
src/main/java/com/wayble/server/explore/repository/search/WaybleZoneQuerySearchRepository.java (2)
141-155: 이름 유사도 쿼리 구성 보완 제안.현재
match+fuzzy+wildcard조합은 간단하지만, 불필요한 리소스 사용 또는 과/소매칭이 발생할 수 있습니다. 다음을 고려해보세요.
dis_max쿼리로 should 절 묶기(가장 잘 맞는 쿼리의 스코어를 채택하며 tie-breaker로 다른 쿼리 기여 조정).- prefix 검색은
match_phrase_prefix가 더 효율적/정확합니다(특히 zoneName 앞부분 검색 시).- 30m 필터가 있으므로 점수 정렬 대신 거리 우선 정렬만으로도 충분할 가능성이 큽니다. 점수는
track_scores(true)로만 조회하고 정렬은 거리만 사용하는 방식을 고려.구현은 선택 사항이지만, 정확도/성능 균형을 위해 재검토를 권장합니다.
132-136: 입력 검증 책임의 일관성.
zoneName이 null/blank일 때null을 반환하는 대신, 레이어 경계를 명확히 하기 위해 컨트롤러/서비스에서 400/404로 처리하거나 예외를 던지도록 일관화하는 것이 좋습니다(현재 컨트롤러는 @Valid로 DTO 필드 유효성 검증을 수행). 레포지토리 레이어는 검색 책임에 집중하도록 위로 끌어올리는 것을 권장합니다.src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
14-17: Route 레코드의 필드명을 더 명확하게 변경하는 것을 고려해보세요
routeIndex라는 필드명이 다소 모호합니다. 이것이 페이징된 결과에서의 순서를 나타내는 것인지, 아니면 전체 경로 목록에서의 인덱스인지 명확하지 않습니다.routeNumber또는routeOrder와 같은 더 명확한 이름을 사용하는 것이 좋겠습니다.public record Route( - Integer routeIndex, // 경로 인덱스 + Integer routeNumber, // 경로 번호 (1부터 시작) List<Step> steps // 해당 경로의 단계들 ) {}src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
86-88: createRoute 메서드가 단순한 래퍼 역할만 수행이 메서드는 단순히
TransportationResponseDto.Route생성자를 호출하는 것 외에 다른 로직이 없습니다. 이런 단순한 래퍼 메서드는 불필요한 추상화일 수 있습니다.메서드를 제거하고 직접 생성자를 호출하는 것을 고려해보세요:
- TransportationResponseDto.Route routeObj = createRoute(route, startIndex + i + 1); + TransportationResponseDto.Route routeObj = new TransportationResponseDto.Route(startIndex + i + 1, route); routeList.add(routeObj); -private TransportationResponseDto.Route createRoute(List<TransportationResponseDto.Step> steps, int routeIndex) { - return new TransportationResponseDto.Route(routeIndex, steps); -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java(1 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(5 hunks)src/main/java/com/wayble/server/explore/controller/WaybleZoneSearchController.java(1 hunks)src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java(0 hunks)src/main/java/com/wayble/server/explore/repository/search/WaybleZoneQuerySearchRepository.java(1 hunks)src/main/java/com/wayble/server/explore/service/WaybleZoneSearchService.java(1 hunks)src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/main/java/com/wayble/server/explore/service/WaybleZoneSearchService.java (2)
src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-30)src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (2)
zone(33-59)Service(20-126)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
src/main/java/com/wayble/server/explore/controller/WaybleZoneSearchController.java (2)
src/main/java/com/wayble/server/explore/controller/WaybleFacilitySearchController.java (2)
RestController(17-30)GetMapping(24-29)src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java (1)
RestController(15-37)
src/main/java/com/wayble/server/explore/repository/search/WaybleZoneQuerySearchRepository.java (2)
src/main/java/com/wayble/server/explore/repository/recommend/WaybleZoneQueryRecommendRepository.java (2)
Repository(30-163)searchPersonalWaybleZones(48-162)src/main/java/com/wayble/server/explore/repository/facility/WaybleFacilityQuerySearchRepository.java (1)
Repository(20-101)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
src/main/java/com/wayble/server/direction/controller/TransportationController.java (2)
Operation(25-60)RestController(18-62)
src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java (1)
src/test/java/com/wayble/server/explore/WaybleFacilityApiIntegrationTest.java (1)
Test(179-235)
🔇 Additional comments (2)
src/main/java/com/wayble/server/direction/service/TransportationService.java (2)
41-41: MAX_ROUTES 상수와 페이지 크기 설정의 일관성 확인 필요
MAX_ROUTES가 5로 설정되어 있는데, Line 63에서 페이지 크기 기본값도 5입니다. 이 두 값이 동일한 것이 의도된 것인지, 아니면 서로 다른 용도인지 명확히 할 필요가 있습니다.
625-627: catch 블록의 잘못된 중괄호 위치try-catch 블록의 중괄호 구조가 잘못되어 있습니다. Line 625에 불필요한 닫는 중괄호가 있고, catch 블록이 제대로 닫히지 않았습니다.
} - } catch (Exception e) { + } catch (Exception e) { log.info("버스 정보 조회 실패: {}", e.getMessage()); - } + }Likely an incorrect or invalid review comment.
| Set<Pair<Long, Long>> usedEdges = extractActualEdgesFromRoute(firstRoute, graph); | ||
|
|
||
| // 최대 4개의 추가 경로 찾기 | ||
| for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
대체 경로 생성 루프의 종료 조건 개선 필요
현재 루프가 4회 고정 반복인데, MAX_ROUTES - 1개의 대체 경로를 찾으면 조기 종료하도록 되어 있습니다. 이는 불필요한 반복을 유발할 수 있습니다.
- for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) {
+ for (int i = 0; alternativeRoutes.size() < MAX_ROUTES - 1 && i < MAX_ROUTES - 1; i++) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (int i = 0; i < 4 && alternativeRoutes.size() < MAX_ROUTES - 1; i++) { | |
| for (int i = 0; alternativeRoutes.size() < MAX_ROUTES - 1 && i < MAX_ROUTES - 1; i++) { |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around line 183, replace the fixed-count for-loop with a loop that stops
immediately when the desired number of alternative routes is reached: introduce
an attempts counter (e.g., attempts = 0) and use while (alternativeRoutes.size()
< MAX_ROUTES - 1 && attempts < 4) { ...; attempts++; } so the loop checks the
current alternativeRoutes.size() each iteration and exits as soon as MAX_ROUTES
- 1 is met, avoiding unnecessary extra iterations.
Comment on lines
+213
to
+238
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | ||
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | ||
|
|
||
| for (TransportationResponseDto.Step step : route) { | ||
| String fromName = step.from(); | ||
| String toName = step.to(); | ||
|
|
||
| for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) { | ||
| Long nodeId = entry.getKey(); | ||
| List<Edge> edges = entry.getValue(); | ||
|
|
||
| for (Edge edge : edges) { | ||
| Node fromNode = edge.getStartNode(); | ||
| Node toNode = edge.getEndNode(); | ||
|
|
||
| if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) || | ||
| (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) { | ||
| usedEdges.add(Pair.of(fromNode.getId(), toNode.getId())); | ||
| usedEdges.add(Pair.of(toNode.getId(), fromNode.getId())); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return usedEdges; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
extractActualEdgesFromRoute 메서드의 성능 최적화 필요
이 메서드는 O(n * m * k) 복잡도를 가지고 있습니다 (n: route 크기, m: graph 엔트리 수, k: 각 노드의 엣지 수). 대규모 그래프에서는 성능 문제가 발생할 수 있습니다.
노드 이름을 키로 하는 맵을 미리 구성하여 검색 성능을 개선할 수 있습니다:
private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) {
Set<Pair<Long, Long>> usedEdges = new HashSet<>();
+
+ // 노드 이름으로 엣지를 빠르게 찾기 위한 맵 구성
+ Map<String, List<Edge>> edgesByNodeName = new HashMap<>();
+ for (List<Edge> edges : graph.values()) {
+ for (Edge edge : edges) {
+ if (edge.getStartNode() != null) {
+ edgesByNodeName.computeIfAbsent(edge.getStartNode().getStationName(), k -> new ArrayList<>()).add(edge);
+ }
+ }
+ }
for (TransportationResponseDto.Step step : route) {
String fromName = step.from();
String toName = step.to();
- for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) {
- Long nodeId = entry.getKey();
- List<Edge> edges = entry.getValue();
-
- for (Edge edge : edges) {
- Node fromNode = edge.getStartNode();
- Node toNode = edge.getEndNode();
-
- if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) ||
- (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) {
- usedEdges.add(Pair.of(fromNode.getId(), toNode.getId()));
- usedEdges.add(Pair.of(toNode.getId(), fromNode.getId()));
- }
+ List<Edge> candidateEdges = edgesByNodeName.getOrDefault(fromName, new ArrayList<>());
+ for (Edge edge : candidateEdges) {
+ Node toNode = edge.getEndNode();
+ if (toNode != null && toNode.getStationName().equals(toName)) {
+ usedEdges.add(Pair.of(edge.getStartNode().getId(), toNode.getId()));
+ usedEdges.add(Pair.of(toNode.getId(), edge.getStartNode().getId()));
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | |
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | |
| for (TransportationResponseDto.Step step : route) { | |
| String fromName = step.from(); | |
| String toName = step.to(); | |
| for (Map.Entry<Long, List<Edge>> entry : graph.entrySet()) { | |
| Long nodeId = entry.getKey(); | |
| List<Edge> edges = entry.getValue(); | |
| for (Edge edge : edges) { | |
| Node fromNode = edge.getStartNode(); | |
| Node toNode = edge.getEndNode(); | |
| if ((fromNode.getStationName().equals(fromName) && toNode.getStationName().equals(toName)) || | |
| (fromNode.getStationName().equals(toName) && toNode.getStationName().equals(fromName))) { | |
| usedEdges.add(Pair.of(fromNode.getId(), toNode.getId())); | |
| usedEdges.add(Pair.of(toNode.getId(), fromNode.getId())); | |
| } | |
| } | |
| } | |
| } | |
| return usedEdges; | |
| } | |
| private Set<Pair<Long, Long>> extractActualEdgesFromRoute(List<TransportationResponseDto.Step> route, Map<Long, List<Edge>> graph) { | |
| Set<Pair<Long, Long>> usedEdges = new HashSet<>(); | |
| // 노드 이름으로 엣지를 빠르게 찾기 위한 맵 구성 | |
| Map<String, List<Edge>> edgesByNodeName = new HashMap<>(); | |
| for (List<Edge> edges : graph.values()) { | |
| for (Edge edge : edges) { | |
| if (edge.getStartNode() != null) { | |
| edgesByNodeName.computeIfAbsent(edge.getStartNode().getStationName(), k -> new ArrayList<>()).add(edge); | |
| } | |
| } | |
| } | |
| for (TransportationResponseDto.Step step : route) { | |
| String fromName = step.from(); | |
| String toName = step.to(); | |
| List<Edge> candidateEdges = edgesByNodeName.getOrDefault(fromName, new ArrayList<>()); | |
| for (Edge edge : candidateEdges) { | |
| Node toNode = edge.getEndNode(); | |
| if (toNode != null && toNode.getStationName().equals(toName)) { | |
| usedEdges.add(Pair.of(edge.getStartNode().getId(), toNode.getId())); | |
| usedEdges.add(Pair.of(toNode.getId(), edge.getStartNode().getId())); | |
| } | |
| } | |
| } | |
| return usedEdges; | |
| } |
| int weight = entry.getValue(); | ||
|
|
||
| if (usedEdges.contains(edge)) { | ||
| int penalty = routeIndex * 100000; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
패널티 계산에 매직 넘버 사용
routeIndex * 100000에서 100000이라는 매직 넘버가 사용되고 있습니다. 이 값을 상수로 정의하여 코드의 가독성과 유지보수성을 향상시키는 것이 좋습니다.
+ private static final int ROUTE_PENALTY_MULTIPLIER = 100000; // 경로별 패널티 가중치
private Map<Pair<Long, Long>, Integer> createActualEdgePenalizedWeightMap(Map<Pair<Long, Long>, Integer> originalWeightMap, Set<Pair<Long, Long>> usedEdges, int routeIndex) {
Map<Pair<Long, Long>, Integer> penalizedWeightMap = new HashMap<>();
for (Map.Entry<Pair<Long, Long>, Integer> entry : originalWeightMap.entrySet()) {
Pair<Long, Long> edge = entry.getKey();
int weight = entry.getValue();
if (usedEdges.contains(edge)) {
- int penalty = routeIndex * 100000;
+ int penalty = routeIndex * ROUTE_PENALTY_MULTIPLIER;
penalizedWeightMap.put(edge, weight + penalty);
} else {
penalizedWeightMap.put(edge, weight);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| int penalty = routeIndex * 100000; | |
| private static final int ROUTE_PENALTY_MULTIPLIER = 100000; // 경로별 패널티 가중치 | |
| private Map<Pair<Long, Long>, Integer> createActualEdgePenalizedWeightMap(Map<Pair<Long, Long>, Integer> originalWeightMap, Set<Pair<Long, Long>> usedEdges, int routeIndex) { | |
| Map<Pair<Long, Long>, Integer> penalizedWeightMap = new HashMap<>(); | |
| for (Map.Entry<Pair<Long, Long>, Integer> entry : originalWeightMap.entrySet()) { | |
| Pair<Long, Long> edge = entry.getKey(); | |
| int weight = entry.getValue(); | |
| if (usedEdges.contains(edge)) { | |
| int penalty = routeIndex * ROUTE_PENALTY_MULTIPLIER; | |
| penalizedWeightMap.put(edge, weight + penalty); | |
| } else { | |
| penalizedWeightMap.put(edge, weight); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around line 248, the penalty calculation uses a magic number ("routeIndex *
100000"); replace the literal with a clearly named constant (e.g.,
PENALTY_PER_ROUTE or ROUTE_PENALTY_MULTIPLIER) declared at the top of the class
(private static final int) and use that constant in the multiplication to
improve readability and maintainability.
Comment on lines
+290
to
+292
| // 환승 횟수 검증 (4회 이상 제외) | ||
| int transferCount = calculateTransferCount(route); | ||
| return transferCount < 4; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
환승 횟수 제한이 하드코딩됨
환승 횟수 제한이 4회로 하드코딩되어 있습니다. 이를 상수로 정의하여 설정 가능하도록 하는 것이 좋습니다.
+ private static final int MAX_TRANSFER_COUNT = 4; // 최대 환승 횟수
// 환승 횟수 검증 (4회 이상 제외)
int transferCount = calculateTransferCount(route);
- return transferCount < 4;
+ return transferCount < MAX_TRANSFER_COUNT;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around lines 290 to 292 the transfer limit is hardcoded as 4; replace the magic
number with a named constant (e.g., MAX_TRANSFER_COUNT) declared at the top of
the class (or as a configurable property if preferred) and use that constant in
the comparison; ensure the constant is documented and, if made configurable,
loaded from application properties with a sensible default and validated before
use.
Comment on lines
+301
to
309
| private int calculateWalkDistance(List<TransportationResponseDto.Step> route) { | ||
| return route.stream() | ||
| .filter(step -> step.mode() == DirectionType.WALK) | ||
| .mapToInt(step -> { | ||
| // 간단한 도보 거리 추정 (실제로는 정확한 거리 계산 필요) | ||
| return 500; // 기본값 | ||
| }) | ||
| .sum(); | ||
| } |
There was a problem hiding this comment.
도보 거리 계산이 하드코딩된 값 사용
calculateWalkDistance 메서드가 실제 거리를 계산하지 않고 모든 WALK 단계에 대해 500이라는 고정값을 반환하고 있습니다. 이는 경로 정렬 시 부정확한 결과를 초래할 수 있습니다.
실제 거리 계산 로직을 구현해야 합니다. 각 Step의 from과 to 좌표를 사용하여 haversine 함수로 실제 거리를 계산하는 구현이 필요합니다. 이 기능을 구현한 코드를 제공해드릴까요?
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around lines 301 to 309, replace the hardcoded 500-per-WALK-step with a real
distance calculation: for each Step with mode == DirectionType.WALK, extract the
from and to coordinates (lat/lon), compute the great-circle distance using a
haversine helper (use Math.sin/Math.cos/Math.asin/Math.sqrt) and return the
distance in meters, sum those distances and return the total as an int; include
null/empty-coordinate guards and a small helper method signature like
haversine(lat1, lon1, lat2, lon2) -> double (meters) to keep code clean.
Comment on lines
+70
to
+76
| @GetMapping("/validate") | ||
| public CommonResponse<WaybleZoneSearchResponseDto> findIsValidWaybleZone( | ||
| @Valid @ModelAttribute WaybleZoneSearchConditionDto conditionDto | ||
| ) | ||
| { | ||
| return CommonResponse.success(waybleZoneSearchService.isValidWaybleZone(conditionDto)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
유사 존 미발견 시의 HTTP 응답을 명확히 처리하세요.
현재 서비스가 null을 반환할 수 있으나 컨트롤러는 그대로 200 OK + null data를 반환합니다. API 계약을 명확히 하기 위해 미발견 시 404(Not Found) 또는 204(No Content)로 응답하는 것을 권장합니다. 공통 응답 규약(CommonResponse)에 맞춰 에러/빈 응답 처리가 있으면 그 방식을 사용하세요.
간단한 404 처리 예시:
@GetMapping("/validate")
public CommonResponse<WaybleZoneSearchResponseDto> findIsValidWaybleZone(
@Valid @ModelAttribute WaybleZoneSearchConditionDto conditionDto
)
{
- return CommonResponse.success(waybleZoneSearchService.isValidWaybleZone(conditionDto));
+ WaybleZoneSearchResponseDto result = waybleZoneSearchService.isValidWaybleZone(conditionDto);
+ if (result == null) {
+ throw new ResponseStatusException(HttpStatus.NOT_FOUND, "30m 이내에서 유사한 웨이블존을 찾지 못했습니다.");
+ }
+ return CommonResponse.success(result);
}추가 import:
import org.springframework.http.HttpStatus;
import org.springframework.web.server.ResponseStatusException;🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/explore/controller/WaybleZoneSearchController.java
around lines 70 to 76, the controller currently returns 200 OK with null data
when the service finds no matching zone; change it to handle "not found"
explicitly by checking the service result before wrapping it in CommonResponse
and either (a) throw a ResponseStatusException(HttpStatus.NOT_FOUND, "Wayble
zone not found") when the service returns null, or (b) if your CommonResponse
has a standard empty/404 helper, use that helper to return the correct response;
also add the necessary imports for org.springframework.http.HttpStatus and
org.springframework.web.server.ResponseStatusException if you choose the
exception approach.
Comment on lines
+156
to
+164
| // 부분 매치도 포함 (공백 제거 후 검색) | ||
| String cleanedName = cond.zoneName().replaceAll("\\s+", ""); | ||
| b.should(s -> s | ||
| .wildcard(w -> w | ||
| .field("zoneName") | ||
| .value("*" + cleanedName + "*") | ||
| .boost(1.0f) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
wildcard 쿼리를 분석 필드(text)에 적용하면 매칭 정확도가 떨어집니다.
wildcard는 보통 keyword(비분석) 서브필드에 적용해야 기대한 부분매치가 동작합니다. 현재 zoneName이 text 필드라면 토큰 단위로 매칭되어 의도와 다르게 동작하거나 누락/오탐이 발생할 수 있습니다. 매핑에 zoneName.keyword가 있다면 해당 필드로 바꾸세요.
- b.should(s -> s
- .wildcard(w -> w
- .field("zoneName")
- .value("*" + cleanedName + "*")
- .boost(1.0f)
- )
- );
+ b.should(s -> s
+ .wildcard(w -> w
+ .field("zoneName.keyword") // keyword 서브필드로 변경
+ .value("*" + cleanedName + "*")
+ .boost(1.0f)
+ )
+ );추가 개선(선택):
- 한국어/공백/오탈자까지 고려한 부분 매칭은 edge-ngram 필드 도입이 더 적합합니다(예:
zoneName.ngram에match/match_phrase_prefix).
아래 스크립트로 WaybleZoneDocument의 zoneName 매핑/필드 정의를 확인해 주세요(예: @Field 애노테이션, 서브필드 존재 여부).
🏁 Script executed:
#!/bin/bash
fd -a "WaybleZoneDocument.java"
rg -n "class WaybleZoneDocument|zoneName|@Field" -A 3 -B 3Length of output: 63167
조치 필요 — wildcard를 text(zoneName) 필드에 적용하지 마세요 (매핑에 .keyword 없음)
검증 결과: zoneName은 text 필드로 korean_edge_ngram_analyzer / korean_search_analyzer로 색인되어 있습니다 (src/main/java/com/wayble/server/explore/entity/WaybleZoneDocument.java, src/main/resources/elasticsearch/settings/wayble_zone_mappings.json). 따라서 text 필드에 wildcard를 쓰면 토큰화와 충돌해 누락/오탐이 발생할 수 있고, 현재 매핑에 .keyword 서브필드가 없어 zoneName.keyword로 바로 바꿀 수 없습니다.
수정 권장 위치:
- src/main/java/com/wayble/server/explore/repository/search/WaybleZoneQuerySearchRepository.java (현재 wildcard 블록: 약 156–164행) — wildcard 블록 제거 권장.
권장 변경(diff):
- // 부분 매치도 포함 (공백 제거 후 검색)
- String cleanedName = cond.zoneName().replaceAll("\\s+", "");
- b.should(s -> s
- .wildcard(w -> w
- .field("zoneName")
- .value("*" + cleanedName + "*")
- .boost(1.0f)
- )
- );
+ // wildcard 제거 — zoneName은 korean_edge_ngram_analyzer로 색인되어 있으므로 match / fuzzy 로 부분 매칭을 처리합니다대안(필요 시):
- wildcard가 꼭 필요하면 매핑에 keyword 서브필드 또는 별도 ngram/keyword 필드를 추가하고(예: zoneName.fields.keyword) 해당 필드로 조회하도록 변경한 뒤 재색인하세요. 예시 매핑 변경:
"zoneName": {
- "type": "text",
- "analyzer": "korean_edge_ngram_analyzer",
- "search_analyzer":"korean_search_analyzer"
+ "type": "text",
+ "analyzer": "korean_edge_ngram_analyzer",
+ "search_analyzer":"korean_search_analyzer",
+ "fields": {
+ "keyword": { "type": "keyword", "ignore_above": 256 }
+ }
}- 하지만 권장 접근은 현재 설정(edge-ngram)으로 match / match_phrase_prefix / fuzzy 조합을 사용해 부분매칭을 처리하는 것입니다(성능/정확도 측면에서 더 적합).
요약: wildcard 블록을 제거하거나, 정말 wildcard가 필요하면 매핑(및 재색인)을 먼저 추가한 후 zoneName.keyword로 쿼리하도록 변경하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 부분 매치도 포함 (공백 제거 후 검색) | |
| String cleanedName = cond.zoneName().replaceAll("\\s+", ""); | |
| b.should(s -> s | |
| .wildcard(w -> w | |
| .field("zoneName") | |
| .value("*" + cleanedName + "*") | |
| .boost(1.0f) | |
| ) | |
| ); | |
| // wildcard 제거 — zoneName은 korean_edge_ngram_analyzer로 색인되어 있으므로 match / fuzzy 로 부분 매칭을 처리합니다 |
Comment on lines
+186
to
+205
| // 정렬: 점수 + 거리 조합 | ||
| SortOptions scoreSort = SortOptions.of(s -> s.score(sc -> sc.order(SortOrder.Desc))); | ||
| SortOptions geoSort = SortOptions.of(s -> s | ||
| .geoDistance(gds -> gds | ||
| .field("address.location") | ||
| .location(GeoLocation.of(gl -> gl | ||
| .latlon(ll -> ll | ||
| .lat(cond.latitude()) | ||
| .lon(cond.longitude()) | ||
| ) | ||
| )) | ||
| .order(SortOrder.Asc) | ||
| ) | ||
| ); | ||
|
|
||
| NativeQuery nativeQuery = NativeQuery.builder() | ||
| .withQuery(query) | ||
| .withSort(scoreSort) | ||
| .withSort(geoSort) | ||
| .withPageable(PageRequest.of(0, 1)) // 첫 번째 결과만 |
There was a problem hiding this comment.
정렬값 인덱스 가정으로 인한 런타임 예외 가능성(IndexOutOfBounds/잘못된 캐스팅).
_score와 geo_distance를 함께 정렬할 때, getSortValues()에 _score가 포함되지 않는 클라이언트/버전 조합이 있습니다. 이 경우 정렬값 리스트가 1개만 반환되어 현재의 “두 번째 값이 거리” 가정이 깨질 수 있습니다. 또한 _score 값이 포함되는 경우에도 Double 캐스팅이 안전하지 않을 수 있습니다.
정렬값의 “마지막 요소를 거리로 간주”하고, Number로 캐스팅 후 doubleValue()를 사용하는 방식으로 견고하게 처리하세요.
- WaybleZoneDocument doc = hits.getSearchHit(0).getContent();
- Double distanceInMeters = (Double) hits.getSearchHit(0).getSortValues().get(1); // 거리는 두 번째 정렬값
- Double distanceInKm = distanceInMeters / 1000.0;
-
- return WaybleZoneSearchResponseDto.from(doc, distanceInKm);
+ WaybleZoneDocument doc = hits.getSearchHit(0).getContent();
+ var sortValues = hits.getSearchHit(0).getSortValues();
+ if (sortValues == null || sortValues.isEmpty()) {
+ // 방어적 처리: 거리 계산 불가 시 0으로 간주하거나(권장 X) 하버사인 계산으로 대체 고려
+ return WaybleZoneSearchResponseDto.from(doc, 0.0);
+ }
+ Number distanceInMeters = (Number) sortValues.get(sortValues.size() - 1); // 마지막 값을 거리로 간주
+ double distanceInKm = distanceInMeters.doubleValue() / 1000.0;
+ return WaybleZoneSearchResponseDto.from(doc, distanceInKm);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/explore/repository/search/WaybleZoneQuerySearchRepository.java
around lines 186 to 205, the code assumes sortValues[1] is the geo distance and
casts directly to Double which can throw IndexOutOfBounds or ClassCastException
for some client/version combos; change the logic to treat the last element of
getSortValues() as the distance, first check that getSortValues() is non-empty,
then obtain the last element, convert it safely by checking if it's a Number and
calling number.doubleValue(), otherwise parse a Double from its toString(), and
handle the empty/malformed case with a sensible default or fallback so no
unchecked casting or index assumptions remain.
Comment on lines
+46
to
+48
| public WaybleZoneSearchResponseDto isValidWaybleZone(WaybleZoneSearchConditionDto condition) { | ||
| return waybleZoneQuerySearchRepository.findSimilarWaybleZone(condition); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드 명과 반환 계약이 모호합니다 (boolean처럼 보이지만 DTO를 반환하고, 실패 시 null).
isValidWaybleZone라는 이름은 boolean 반환을 연상시키지만 실제로는 DTO를 반환합니다. 또한 유사 항목이 없을 때null을 반환해 상위 레이어에서의 처리가 모호합니다(200 OK + null data인지, 404/204인지 등).- 서비스 계층에서 실패 계약을 명확히 하거나 Optional로 래핑해 의도를 드러내는 것을 권장합니다.
다음과 같이 개선을 제안합니다.
- 메서드명을
findSimilarWaybleZone(또는validateAndFindSimilarWaybleZone)로 변경 - 반환 타입을
Optional<WaybleZoneSearchResponseDto>로 변경하여 null 회피 - 실패 정책(예: 미발견 시 예외)을 컨트롤러와 합의
예시(서비스 메서드 시그니처만):
- public WaybleZoneSearchResponseDto isValidWaybleZone(WaybleZoneSearchConditionDto condition) {
- return waybleZoneQuerySearchRepository.findSimilarWaybleZone(condition);
- }
+ public Optional<WaybleZoneSearchResponseDto> findSimilarWaybleZone(WaybleZoneSearchConditionDto condition) {
+ return Optional.ofNullable(waybleZoneQuerySearchRepository.findSimilarWaybleZone(condition));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public WaybleZoneSearchResponseDto isValidWaybleZone(WaybleZoneSearchConditionDto condition) { | |
| return waybleZoneQuerySearchRepository.findSimilarWaybleZone(condition); | |
| } | |
| public Optional<WaybleZoneSearchResponseDto> findSimilarWaybleZone(WaybleZoneSearchConditionDto condition) { | |
| return Optional.ofNullable(waybleZoneQuerySearchRepository.findSimilarWaybleZone(condition)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/explore/service/WaybleZoneSearchService.java
around lines 46 to 48, the method name isValidWaybleZone is misleading (suggests
boolean) and it returns a DTO or null; change the method to a clearly named
finder like findSimilarWaybleZone (or validateAndFindSimilarWaybleZone) and
change the signature to return Optional<WaybleZoneSearchResponseDto> instead of
returning null; update the implementation to wrap the repository result in
Optional.ofNullable(...), update all callers (controller/service tests) to
handle Optional or decide on a failure policy (map Optional.empty() to a
specific exception/HTTP status) so null is never propagated.
Comment on lines
+536
to
+541
| List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll(); | ||
| WaybleZone waybleZone = waybleZoneList.get(0); | ||
| String zoneName = waybleZone.getZoneName(); | ||
| MvcResult result = mockMvc.perform(get(baseUrl + "/validate") | ||
| .header("Authorization", "Bearer " + token) | ||
| .param("latitude", String.valueOf(waybleZone.getAddress().getLatitude())) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
테스트 안전성: 데이터 비어있을 때의 방어 로직 추가.
샘플 데이터가 의도치 않게 비어있는 경우 waybleZoneList.get(0)에서 IndexOutOfBoundsException이 발생할 수 있습니다. 방어적 체크를 추가해 테스트 안정성을 높이세요.
- List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll();
- WaybleZone waybleZone = waybleZoneList.get(0);
+ List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll();
+ assertThat(waybleZoneList).isNotEmpty();
+ WaybleZone waybleZone = waybleZoneList.get(0);🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java"
if [ ! -f "$file" ]; then
echo "FILE_NOT_FOUND: $file"
rg -n "WaybleZoneSearchApiIntegrationTest" || true
exit 0
fi
echo "== $file: lines 520-545 =="
sed -n '520,545p' "$file"Length of output: 1834
테스트 안정성: waybleZoneList가 비어있을 때 방어 로직 추가
테스트에서 waybleZoneRepository.findAll() 반환값이 비어있을 경우 waybleZoneList.get(0)에서 IndexOutOfBoundsException이 발생할 수 있으므로 조기 실패(assert)로 안정성을 높이세요.
- 파일: src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java
- 위치: findIsValidWaybleZoneTest() 내부, waybleZoneList.get(0) 호출 직전
제안하는 변경:
- List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll();
- WaybleZone waybleZone = waybleZoneList.get(0);
+ List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll();
+ assertThat(waybleZoneList).isNotEmpty();
+ WaybleZone waybleZone = waybleZoneList.get(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll(); | |
| WaybleZone waybleZone = waybleZoneList.get(0); | |
| String zoneName = waybleZone.getZoneName(); | |
| MvcResult result = mockMvc.perform(get(baseUrl + "/validate") | |
| .header("Authorization", "Bearer " + token) | |
| .param("latitude", String.valueOf(waybleZone.getAddress().getLatitude())) | |
| List<WaybleZone> waybleZoneList = waybleZoneRepository.findAll(); | |
| assertThat(waybleZoneList).isNotEmpty(); | |
| WaybleZone waybleZone = waybleZoneList.get(0); | |
| String zoneName = waybleZone.getZoneName(); | |
| MvcResult result = mockMvc.perform(get(baseUrl + "/validate") | |
| .header("Authorization", "Bearer " + token) | |
| .param("latitude", String.valueOf(waybleZone.getAddress().getLatitude())) |
🤖 Prompt for AI Agents
In
src/test/java/com/wayble/server/explore/WaybleZoneSearchApiIntegrationTest.java
around lines 536 to 541, the test calls waybleZoneList.get(0) without checking
that waybleZoneRepository.findAll() returned any elements; add a defensive
assertion right after obtaining waybleZoneList to fail fast if the list is empty
(e.g., assertFalse(waybleZoneList.isEmpty(), "Expected at least one WaybleZone
in repository") or assertThat/Assertions.assertTrue with a clear message), or
alternatively arrange/create a WaybleZone fixture before calling get(0) so the
test never depends on an empty repository.
This was referenced Aug 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit