-
Notifications
You must be signed in to change notification settings - Fork 1
[release] 진행상황을 main 브랜치에 반영 #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
[FIX] 환승할 때마다 walk step 추가, 공간 필터링을 이용해 OOM 문제 해결
Walkthrough지리적 경계(바운딩 박스) 기반 조회 메서드를 Node/Edge 리포지토리에 추가하고, TransportationService에서 공간 필터링·전이보행(step) 삽입·OOM 처리 등을 도입했습니다. BusInfoService는 실제 API 호출을 비활성화하고 더미 데이터 반환으로 단축 경로를 적용했습니다. 공개 API 시그니처 변경은 없습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant T as TransportationService
participant NR as NodeRepository
participant ER as EdgeRepository
C->>T: findMultipleTransportationRoutes(start, end)
T->>T: calculateBoundingBox(start, end, SPATIAL_BUFFER_KM)
T->>NR: findNodesInBoundingBox(minLat,maxLat,minLon,maxLon)
T->>ER: findEdgesInBoundingBox(minLat,maxLat,minLon,maxLon)
T->>T: findNearbyNodes / buildGraph (filtered)
T->>T: route search
alt transfer between segments
T->>T: addTransferWalkSteps(calculateTransferWalkDistance)
end
T-->>C: PathSteps (including WALK transfers)
sequenceDiagram
participant U as Caller
participant B as BusInfoService
participant R as RouteRepository
U->>B: getBusInfo(stationName, busId, x, y)
B->>R: find route by busId (for shuttle check)
R-->>B: route name
B-->>U: Dummy BusInfo (random interval/low-floor, shuttle flag)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
387-429: 시작/도착 임시 노드 중복 생성 제거 및 일관된 패널티 적용현재
nodes리스트에 -1/-2 ID 노드가 이미 추가된 상태에서 동일 ID의 노드를 다시 생성해 추가하고 있습니다. 이는 중복 엔티티(동일 ID)로 혼동을 유발하고, 불필요한 리스트/맵 갱신을 초래합니다. 또한 원/종점 도보 연결에도STEP_PENALTY를 동일하게 적용해 일관성을 유지하세요.- // 1. 임시 노드 생성 - Node startNode = Node.createNode(-1L, startTmp.getStationName(), DirectionType.WALK, - startTmp.getLatitude(), startTmp.getLongitude()); - Node endNode = Node.createNode(-2L, endTmp.getStationName(), DirectionType.WALK, - endTmp.getLatitude(), endTmp.getLongitude()); - - graph.put(startNode.getId(), new ArrayList<>()); - graph.put(endNode.getId(), new ArrayList<>()); + // 1. 기존 임시 노드 재사용 (중복 생성 방지) + Node startNode = startTmp; + Node endNode = endTmp; + graph.computeIfAbsent(startNode.getId(), k -> new ArrayList<>()); + graph.computeIfAbsent(endNode.getId(), k -> new ArrayList<>()); @@ - int weight = (int)(haversine( + int weight = (int)(haversine( startNode.getLatitude(), startNode.getLongitude(), candidate.getLatitude(), candidate.getLongitude() ) * METER_CONVERSION); - weightMap.put(Pair.of(startNode.getId(), candidate.getId()), weight); + weightMap.put(Pair.of(startNode.getId(), candidate.getId()), weight + STEP_PENALTY); @@ - int weight = (int)(haversine( + int weight = (int)(haversine( candidate.getLatitude(), candidate.getLongitude(), endNode.getLatitude(), endNode.getLongitude() ) * METER_CONVERSION); - weightMap.put(Pair.of(candidate.getId(), endNode.getId()), weight); + weightMap.put(Pair.of(candidate.getId(), endNode.getId()), weight + STEP_PENALTY); @@ - nodes.add(startNode); - nodes.add(endNode); + // 기존 nodes에 이미 존재하므로 재추가하지 않음
690-706: 지하철 분기에서 NPE 가능성 (route null 가정 누락)
currentEdge.getRoute()가 null인 경우가 발생하면getRouteId()접근에서 NPE가 발생합니다. BUS 분기처럼 null 체크를 추가하세요.- } else if (currentType == DirectionType.SUBWAY) { + } else if (currentType == DirectionType.SUBWAY) { try { - if (currentEdge.getStartNode() != null) { - TransportationResponseDto.NodeInfo nodeInfo = facilityService.getNodeInfo(currentEdge.getStartNode().getId(), currentEdge.getRoute().getRouteId()); + if (currentEdge.getStartNode() != null && currentEdge.getRoute() != null) { + TransportationResponseDto.NodeInfo nodeInfo = + facilityService.getNodeInfo( + currentEdge.getStartNode().getId(), + currentEdge.getRoute().getRouteId() + ); subwayInfo = new TransportationResponseDto.SubwayInfo( nodeInfo.wheelchair(), nodeInfo.elevator(), nodeInfo.accessibleRestroom() ); - } else { + } else { subwayInfo = new TransportationResponseDto.SubwayInfo( new ArrayList<>(), new ArrayList<>(), false ); } } catch (Exception e) { log.error("지하철 정보 조회 실패: {}", e.getMessage()); subwayInfo = new TransportationResponseDto.SubwayInfo( new ArrayList<>(), new ArrayList<>(), false ); }
860-885: 환승 횟수 계산 로직 오류(모든 비-WALK step에서 증가) → 과도한 필터링 발생현재 조건 분기가 상호배타적이지 않아 동일 노선 연장, 동일 모드 유지 등 모든 경우에 환승 카운트가 증가합니다. 동일 모드·동일 노선은 환승으로 보지 않도록 수정해야 합니다.
- private int calculateTransferCount(List<TransportationResponseDto.Step> steps) { - int transferCount = 0; - DirectionType previousMode = null; - String previousRouteName = null; - - for (TransportationResponseDto.Step step : steps) { - if (step.mode() != DirectionType.WALK && step.mode() != DirectionType.FROM_WAYPOINT && step.mode() != DirectionType.TO_WAYPOINT) { - if (previousMode != null) { - if (previousMode == step.mode() && - previousRouteName != null && step.routeName() != null && - !previousRouteName.equals(step.routeName())) { - transferCount++; - } else if (previousMode == step.mode() && - previousRouteName != null && step.routeName() != null && - previousRouteName.equals(step.routeName())) { - transferCount++; - } else if (previousMode != step.mode()) { - transferCount++; - } - } - previousMode = step.mode(); - previousRouteName = step.routeName(); - } - } - return transferCount; - } + private int calculateTransferCount(List<TransportationResponseDto.Step> steps) { + int transferCount = 0; + DirectionType previousMode = null; + String previousRouteName = null; + + for (TransportationResponseDto.Step step : steps) { + DirectionType mode = step.mode(); + if (mode == DirectionType.WALK || mode == DirectionType.FROM_WAYPOINT || mode == DirectionType.TO_WAYPOINT) { + continue; + } + + if (previousMode != null) { + boolean isTransfer = false; + if (mode != previousMode) { + // 모드 변경(BUS <-> SUBWAY 등)은 환승 + isTransfer = true; + } else { + // 동일 모드 내에서 노선이 바뀌는 경우에만 환승 + if (mode == DirectionType.BUS || mode == DirectionType.SUBWAY) { + if (!Objects.equals(previousRouteName, step.routeName())) { + isTransfer = true; + } + } + } + if (isTransfer) transferCount++; + } + + previousMode = mode; + previousRouteName = step.routeName(); + } + return transferCount; + }
🧹 Nitpick comments (14)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (5)
32-34: 더미 경로로의 즉시 반환은 프로덕션 안정성 리스크 — 환경별 플래그/프로파일로 제어 권장서비스키 이슈로 임시 우회하는 의도는 이해하지만, 현재 코드는 어떤 환경에서도 항상 랜덤 더미를 반환합니다. 운영/스테이징에서 실제 API를 복구해도 우회 코드를 제거하기 전까지 반영되지 않는 점이 리스크입니다. 환경별 플래그(@Profile 또는 application.yml 기반 boolean)로 더미/실제 경로를 토글하도록 해주세요.
예시:
- application.yml
- open-data.use-dummy-bus-info: true/false
- BusInfoService.getBusInfo:
- if (openDataProperties.useDummyBusInfo()) { return createDummyBusInfo(...); } else { return getBusInfoFromApi(...); }
- 실제 API 코드는 별도 메서드로 추출해 컴파일되도록 유지(주석 처리 금지)
원하시면 플래그 추가 PR 패치를 만들어드리겠습니다.
35-36: 대규모 주석 처리된 실제 API 코드 → 메서드 추출로 코드 부패 방지실제 경로 전체를 블록 주석 처리하면 컴파일·리팩토링 대상에서 제외되어 빠르게 부패합니다. 주석을 제거하고 private 메서드로 분리(getBusInfoFromApi)한 뒤, 상단에서 플래그로 분기하는 구조를 권장합니다. 이렇게 하면 정적 분석/리팩토링의 보호를 받으며, 복구 시 안전하게 스위칭할 수 있습니다.
Also applies to: 98-100
87-87: 배차간격 파싱 실패 시 0으로 대체는 혼동 가능 — null 반환 고려현재(주석 블록이지만) 파싱 실패 시 dispatchInterval을 0으로 대체합니다. 0은 "즉시 도착"으로 오해될 소지가 있어, "미측정/미상" 의미의 null이 더 안전합니다. 소비 측에서 0과 null을 동일하게 처리하는지 확인 부탁드립니다.
103-121: 더미 데이터의 비결정성(Math.random)으로 테스트 플래키 발생 가능 — 입력 기반 시드로 결정적 생성 권장현재 더미 값이 매 호출마다 바뀝니다. 이는 스냅샷/UI 테스트, 캐싱/로깅에서 플래키를 유발합니다. stationName/busId 등을 시드로 사용해 결정적으로 생성하면 안정적입니다. 또한 좌표(x,y) 등 잠재적 민감정보 로그는 레벨을 낮추거나 제거를 권장합니다. routeName NPE도 방지해 주세요.
적용 예시(제한된 범위 diff):
- log.info("🎭 더미 BusInfo 생성 - stationName: {}, busId: {}, x: {}, y: {}", stationName, busId, x, y); + log.debug("🎭 더미 BusInfo 생성 - stationName: {}, busId: {}", stationName, busId); @@ - if (busId != null) { - var route = routeRepository.findById(busId); - isShuttleBus = route.isPresent() && route.get().getRouteName().contains("마포"); - } + if (busId != null) { + var route = routeRepository.findById(busId); + isShuttleBus = route.isPresent() + && route.get().getRouteName() != null + && route.get().getRouteName().contains("마포"); + } @@ - List<Boolean> isLowFloor = new ArrayList<>(); - isLowFloor.add(Math.random() < 0.7); - isLowFloor.add(Math.random() < 0.5); + List<Boolean> isLowFloor = new ArrayList<>(2); + java.util.Random rng = new java.util.Random(java.util.Objects.hash(stationName, busId)); + isLowFloor.add(rng.nextDouble() < 0.7); + isLowFloor.add(rng.nextDouble() < 0.5); @@ - Integer dispatchInterval = (int) (Math.random() * 15) + 1; + Integer dispatchInterval = rng.nextInt(15) + 1; // 1~15분추가로, "마포" 키워드는 상수로 추출해 중복을 제거하면 유지보수에 유리합니다.
198-198: 정류소 목록 비어있음 경고 로그 추가 👍엣지 케이스 추적에 유용합니다. 호출 빈도가 높다면 warn 대신 info/debug로 조정해 로그 노이즈를 완화하는 것도 고려해 주세요.
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java (2)
19-24: Bounding Box 내 양 끝점 모두 포함하도록 조건 강화 제안 (불필요 로딩 감소)현재 OR 조건으로 한쪽 끝점만 박스 내에 있어도 Edge를 로드합니다. 이후
buildGraph(...)에서nodeIds.contains(...)로 다시 걸러지므로, 박스 밖 노드가 연결된 엣지는 대부분 버려져 성능/메모리 낭비가 발생합니다. 두 끝점 모두 박스 내에 있는 경우만 조회하도록 AND 조건으로 좁히는 편이 더 일관되고 효율적입니다.다음과 같이 WHERE 절을 수정해 주세요:
- "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) OR " + - "(en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)") + "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) AND " + + " (en.latitude BETWEEN :minLat AND :maxLat AND en.longitude BETWEEN :minLon AND :maxLon)")
13-17: 대량 조회 대비를 위한 인덱스/공간 인덱스 고려경계 박스 기반 조회는 데이터 볼륨이 큰 경우 병목이 될 수 있습니다. DB 레벨에서 다음을 고려해 보세요:
- latitude, longitude에 대한 복합 인덱스
- 가능하다면 공간 타입(POINT) + 공간 인덱스 및 공간 함수(ST_Within 등)로 전환
src/main/java/com/wayble/server/direction/repository/NodeRepository.java (1)
12-20: JPQL Bounding Box 쿼리 추가, 방향성 적절합니다필요한 파라미터 바인딩과 BETWEEN 조건이 명확하며, 서비스의 공간 필터링과 일관됩니다. 성능 향상을 위해서는 DB 레벨 인덱싱도 함께 검토해 주세요(위 EdgeRepository 코멘트 참조).
src/main/java/com/wayble/server/direction/service/TransportationService.java (6)
43-45: 주석 오타 수정 제안: ‘지작점’ → ‘시작점’가독성을 위해 한 글자 오타를 바로잡아 주세요.
- private static final double SPATIAL_BUFFER_KM = 15.0; // 지작점/도착점 주변 15km + private static final double SPATIAL_BUFFER_KM = 15.0; // 시작점/도착점 주변 15km
136-141: 경계 박스 경도 버퍼 계산 개선(중위 위도 사용)으로 누락·과포함 최소화경도당 km 환산에 시작점 위도만 사용하면, 시작/도착점 위도가 크게 다른 경우 박스가 왜곡될 수 있습니다. 두 지점의 중위 위도를 사용해 경도 버퍼를 계산하도록 보정하세요.
- double minLat = Math.min(start.getLatitude(), end.getLatitude()) - SPATIAL_BUFFER_KM / 111.0; - double maxLat = Math.max(start.getLatitude(), end.getLatitude()) + SPATIAL_BUFFER_KM / 111.0; - double minLon = Math.min(start.getLongitude(), end.getLongitude()) - SPATIAL_BUFFER_KM / (111.0 * Math.cos(Math.toRadians(start.getLatitude()))); - double maxLon = Math.max(start.getLongitude(), end.getLongitude()) + SPATIAL_BUFFER_KM / (111.0 * Math.cos(Math.toRadians(start.getLatitude()))); + double midLat = (start.getLatitude() + end.getLatitude()) / 2.0; + double latBuffer = SPATIAL_BUFFER_KM / 111.0; + double lonBuffer = SPATIAL_BUFFER_KM / (111.0 * Math.cos(Math.toRadians(midLat))); + + double minLat = Math.min(start.getLatitude(), end.getLatitude()) - latBuffer; + double maxLat = Math.max(start.getLatitude(), end.getLatitude()) + latBuffer; + double minLon = Math.min(start.getLongitude(), end.getLongitude()) - lonBuffer; + double maxLon = Math.max(start.getLongitude(), end.getLongitude()) + lonBuffer;
485-486: 동적 도보 간선 추가 시 불변 리스트 방지안전하게 변경 가능한 리스트를 보장하기 위해
computeIfAbsent사용을 권장합니다. 현재 코드에서도 초기화가 되어 있지만, 방어적으로 개선해 두면 유지보수성이 좋아집니다.- List<Edge> currentEdges = graph.getOrDefault(curr.getId(), List.of()); + List<Edge> currentEdges = graph.computeIfAbsent(curr.getId(), k -> new ArrayList<>());
775-808: 정류장명 문자열 매칭 대신 Node 식별자/좌표 기반 거리 계산 권장정류장명이 중복되거나 유사한 경우 잘못된 노드를 매칭할 수 있습니다. 환승 Walk Step을 추가하는 시점에 경계가 되는 정확한 Node(또는 id/좌표)를 직접 전달받아 거리 계산을 수행하면 정확도가 올라갑니다. 예:
addTransferWalkSteps에서 경계 edge의 end/start node 좌표를 바로 사용.원하시면
Step구조에 Node id(또는 좌표)를 포함하는 방안을 제안/적용 패치를 드리겠습니다.
128-131: OOM 처리 시 스택트레이스 로깅으로 원인 파악성 향상메시지만 로깅하면 원인 추적이 어렵습니다. 예외 객체 자체를 함께 로깅하세요.
- } catch (OutOfMemoryError e) { - log.error("Out of memory error in transportation route finding: {}", e.getMessage()); + } catch (OutOfMemoryError e) { + log.error("Out of memory error in transportation route finding", e); throw new ApplicationException(PATH_NOT_FOUND); }
120-122: 불필요한 파라미터 제거로 시그니처 단순화
findMultipleOptimalRoutes에 전달되는nearestToStart,nearestToEnd는 사용되지 않습니다. 시그니처 및 호출부에서 제거해 단순화하세요.- List<List<TransportationResponseDto.Step>> result = findMultipleOptimalRoutes( - graphData.graph(), startTmp, endTmp, graphData.weightMap(), nodes, nearestToStart, nearestToEnd - ); + List<List<TransportationResponseDto.Step>> result = findMultipleOptimalRoutes( + graphData.graph(), startTmp, endTmp, graphData.weightMap(), nodes + );아래 시그니처도 함께 정리합니다:
- private List<List<TransportationResponseDto.Step>> findMultipleOptimalRoutes( - Map<Long, List<Edge>> graph, - Node startTmp, - Node endTmp, - Map<Pair<Long, Long>, Integer> weightMap, - List<Node> nodes, - Node nearestToStart, - Node nearestToEnd) { + private List<List<TransportationResponseDto.Step>> findMultipleOptimalRoutes( + Map<Long, List<Edge>> graph, + Node startTmp, + Node endTmp, + Map<Pair<Long, Long>, Integer> weightMap, + List<Node> nodes) {
📜 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 sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java(2 hunks)src/main/java/com/wayble/server/direction/repository/NodeRepository.java(1 hunks)src/main/java/com/wayble/server/direction/service/BusInfoService.java(4 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (1)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
BusInfo(39-43)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
src/main/java/com/wayble/server/direction/service/WalkingService.java (1)
Slf4j(20-64)
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java (2)
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (2)
Entity(8-47)createEdge(38-46)src/main/java/com/wayble/server/direction/entity/transportation/Node.java (1)
Entity(13-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-docker-image-on-ec2
🔇 Additional comments (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
732-734: 환승 Walk Step 추가 로직 방향성 좋습니다전이 구간에 명시적 도보 단계를 삽입해 사용자 이해도와 UX를 개선합니다. 아래
calculateTransferWalkDistance개선과 함께 정확도가 더 올라갈 수 있습니다.
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit