-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] 대중교통 길찾기의 환승 방식 조정 #140
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
Walkthrough그래프 기반 경로 탐색으로 서비스 로직을 전면 교체하고, 동적 도보 엣지(출발/도착 1km, 정류장 간 2km), 페이징, 노드 팩토리 메서드 및 TransportationGraphDto를 추가했으며 엔티티 연관관계에 LAZY/BatchSize 적용과 페치조인 리포지토리 메서드를 도입했습니다. Node–Facility 단방향 매핑은 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as TransportationService
participant R as EdgeRepository
participant G as Graph Builder
participant D as Dijkstra
participant B as TransportInfo
C->>S: findRoutes(request, cursor, size)
S->>R: findAllWithNodesAndRoute()
R-->>S: edges with start/end/route
S->>G: build TransportationGraphDto & weightMap
S->>G: add walking connections (OD 1km, stops 2km)
G-->>S: enriched graph
S->>D: run Dijkstra (visit cap, transfer constraints)
D-->>S: path candidate
S->>S: merge segments, validate transfers
S->>B: fetch bus/subway info (best-effort)
B-->>S: transport details
S-->>C: TransportationResponseDto (steps + PageInfo)
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
✨ 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: 6
🔭 Outside diff range comments (4)
src/main/java/com/wayble/server/direction/entity/transportation/Facility.java (1)
18-20: @mapsid와 @GeneratedValue 동시 사용은 잘못된 매핑 — 공유 PK 매핑을 정정하세요현재 Facility.id에 @GeneratedValue(IDENTITY)를 사용하면서, 같은 PK를 공유하도록 node 필드에 @mapsid를 설정했습니다. @mapsid를 사용할 때는 FK 대상(Node)의 PK를 그대로 사용하므로 Facility의 PK를 별도로 자동 생성하면 충돌/무결성 오류가 발생합니다. 새 Facility를 persist할 때 예외가 나올 수 있습니다.
아래처럼 @GeneratedValue 제거 및 @OnetoOne(optional = false)로 강제 관계를 명시하는 것을 권장합니다.
- @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; @@ - @OneToOne(fetch = FetchType.LAZY) + @OneToOne(fetch = FetchType.LAZY, optional = false) @MapsId @JoinColumn(name = "id") private Node node;Also applies to: 41-44
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (1)
38-46: 엔티티 팩토리에서 수동 ID 주입 제거 및 호출부 수정 필요
JPA 엔티티는@GeneratedValue로 PK를 관리하기 때문에,createEdge(Long id, …)로 수동 ID를 주입하면 충돌 또는 혼란이 발생할 수 있습니다. 동적 그래프용 엣지는 별도 DTO/record로 분리하거나, 팩토리에서 ID 파라미터를 제거하고 호출부를 함께 리팩토링하세요.수정할 위치:
- src/main/java/com/wayble/server/direction/entity/transportation/Edge.java
- src/main/java/com/wayble/server/direction/service/TransportationService.java (호출부 총 4곳)
예시 diff:
--- a/src/main/java/com/wayble/server/direction/entity/transportation/Edge.java +++ b/src/main/java/com/wayble/server/direction/entity/transportation/Edge.java @@ public static Edge createEdge(Long id, Node startNode, Node endNode, DirectionType edgeType) { - public static Edge createEdge(Long id, Node startNode, Node endNode, DirectionType edgeType) { + public static Edge createEdge(Node startNode, Node endNode, DirectionType edgeType) { return Edge.builder() - .id(id) .edgeType(edgeType) .startNode(startNode) .endNode(endNode) .route(null) .build();호출부 변경 예:
--- a/src/main/java/com/wayble/server/direction/service/TransportationService.java @@ (라인 218 등) - Edge walkEdge = Edge.createEdge(-3L, node1, node2, DirectionType.WALK); + Edge walkEdge = Edge.createEdge(node1, node2, DirectionType.WALK);필요 시, JPA 엔티티 대신
GraphEdge같은 전용 DTO/record를 만들어 동적 그래프 로직을 분리 관리하시길 권장합니다.src/main/java/com/wayble/server/direction/entity/transportation/Node.java (1)
13-19: Node↔Facility 양방향 매핑 제거 후 직접 참조 코드 전환 필요Node 엔티티에서 Facility 연관을 제거했으므로, 아래 위치에서 여전히
getFacility()호출 또는facility_id매핑을 직접 참조하고 있습니다. 모두FacilityRepository.findByNodeId(nodeId)호출로 대체하거나, DTO/문서 생성 로직에서 별도 조회 방식으로 수정해야 합니다.
- src/main/java/com/wayble/server/wayblezone/service/WaybleZoneService.java (34-36, 68-70)
- src/main/java/com/wayble/server/wayblezone/repository/WaybleZoneRepositoryImpl.java (51-53, 98-100)
- src/main/java/com/wayble/server/direction/entity/transportation/Lift.java (27-29)
- src/main/java/com/wayble/server/direction/entity/transportation/Elevator.java (24-26)
- src/main/java/com/wayble/server/explore/entity/WaybleZoneDocument.java (50-52)
- src/main/java/com/wayble/server/explore/dto/common/WaybleZoneInfoResponseDto.java (31-33)
- src/main/java/com/wayble/server/explore/repository/recommend/WaybleZoneQueryRecommendRepository.java (148-150)
- src/main/java/com/wayble/server/admin/service/AdminWaybleZoneService.java (177-180)
- src/main/java/com/wayble/server/admin/repository/AdminWaybleZoneRepositoryImpl.java (67-72)
위치별로 아래와 같이 변경을 권장드립니다.
- var facility = zone.getFacility(); + var facility = facilityRepository.findByNodeId(zone.getId());src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
524-533: Step 생성자 인자 개수 및 타입 불일치
TransportationResponseDto.Step생성자 호출 시 8개의 인자를 전달하고 있으며,moveInfoList,moveNumber,busInfo,subwayInfo등은 실제Step레코드에 정의되지 않은 필드들입니다.
Step레코드의 정의에 맞게 수정해야 합니다:-mergedSteps.add(new TransportationResponseDto.Step( - currentType, - moveInfoList, - routeName, - moveNumber, - busInfo, - subwayInfo, - fromName, - toName -)); +// NodeInfo 생성 (busInfo 또는 subwayInfo를 NodeInfo로 변환 필요) +TransportationResponseDto.NodeInfo nodeInfo = null; +if (subwayInfo != null) { + nodeInfo = new TransportationResponseDto.NodeInfo( + subwayInfo.wheelchair(), + subwayInfo.elevator(), + subwayInfo.accessibleRestroom() + ); +} + +mergedSteps.add(new TransportationResponseDto.Step( + currentType, + routeName, + nodeInfo, + fromName, + toName +));
🧹 Nitpick comments (4)
src/main/java/com/wayble/server/direction/entity/transportation/Facility.java (1)
3-8: 사소한 정리: 불필요/오해 소지가 있는 import와 주석 오탈자
- 동일 패키지에 대한
import com.wayble.server.direction.entity.transportation.*;는 불필요합니다.org.springframework.lang.Nullable는 미사용으로 보입니다.- 주석 오탈자 “toliet” → “toilet”.
-import org.springframework.lang.Nullable; - -import com.wayble.server.direction.entity.transportation.*; +// unused imports removed @@ - //for toliet + // for toiletsrc/main/java/com/wayble/server/direction/repository/EdgeRepository.java (1)
12-16: fetch join으로 N+1 제거 👍; 대규모 그래프 풀로딩은 메모리 부담 — 범위 제한 API 병행 권장현재 메서드는 모든 Edge를 startNode/endNode/route까지 fetch join으로 로딩합니다. 데이터셋이 큰 경우 메모리 사용량과 쿼리 시간이 급격히 증가할 수 있습니다. 출발지/도착지 주변만 부분 로딩(바운딩 박스), 또는 페이징/스트리밍 메서드도 함께 제공하는 것을 권장합니다.
예시(바운딩 박스 필터 추가 메서드):
public interface EdgeRepository extends JpaRepository<Edge, Long> { @@ List<Edge> findAllWithNodesAndRoute(); + + @Query("SELECT DISTINCT e FROM Edge e " + + "JOIN FETCH e.startNode s " + + "JOIN FETCH e.endNode t " + + "LEFT JOIN FETCH e.route r " + + "WHERE (s.latitude BETWEEN :minLat AND :maxLat AND s.longitude BETWEEN :minLon AND :maxLon) " + + " OR (t.latitude BETWEEN :minLat AND :maxLat AND t.longitude BETWEEN :minLon AND :maxLon)") + List<Edge> findAllWithinBounds(double minLat, double minLon, double maxLat, double maxLon); }추가로, 읽기 전용 힌트/페치 사이즈 힌트를 통해 1차 캐시 부하를 줄일 수도 있습니다.
src/main/java/com/wayble/server/direction/service/TransportationService.java (2)
187-187: 주석의 거리 정보 오류메서드 내부 주석에 "800m 이내"라고 표시되어 있지만, 실제로는
NEARBY_STATION_WALK_DISTANCE(2000m)를 사용하고 있습니다.-// 1. 각 노드별로 인근 정류장 찾기 (800m 이내, 최대 3개) +// 1. 각 노드별로 인근 정류장 찾기 (2000m 이내, 최대 3개)
32-40: 상수 정의 개선 제안페널티와 제약 관련 상수들이 잘 정의되어 있습니다. 추후 설정 파일로 외부화하면 더욱 유연한 관리가 가능할 것입니다.
추후
@Value어노테이션을 사용하여 application.yml에서 관리하는 것을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/wayble/server/direction/dto/TransportationGraphDto.java(1 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Edge.java(1 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Facility.java(1 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Node.java(3 hunks)src/main/java/com/wayble/server/direction/repository/EdgeRepository.java(1 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/com/wayble/server/direction/repository/EdgeRepository.java (1)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (4)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)src/main/java/com/wayble/server/direction/entity/transportation/Elevator.java (1)
Entity(6-26)src/main/java/com/wayble/server/direction/entity/transportation/Lift.java (1)
Entity(6-29)src/main/java/com/wayble/server/direction/entity/Edge.java (1)
Edge(5-11)
src/main/java/com/wayble/server/direction/entity/transportation/Node.java (1)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
src/main/java/com/wayble/server/direction/dto/TransportationResponseDto.java (1)
Schema(9-37)src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java (1)
Service(16-170)src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
🔇 Additional comments (6)
src/main/java/com/wayble/server/direction/entity/transportation/Facility.java (1)
41-44: LAZY 전환은 N+1 완화에 유효합니다Facility → Node 단방향 연관의 지연 로딩 전환은 전체 조회 시 불필요한 즉시 로딩을 줄여 성능상 이점이 있습니다. 위의 @mapsid 매핑 정정만 함께 반영하면 좋겠습니다.
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (1)
24-36: ManyToOne의 LAZY 전환은 타당합니다startNode/endNode/route를 LAZY로 전환한 것은 불필요한 즉시 로딩을 줄여 전반적인 성능에 도움이 됩니다. 이 엔티티를 직렬화 계층(컨트롤러 응답 등)으로 직접 노출하지 않는 전제에서 특히 유효합니다.
src/main/java/com/wayble/server/direction/dto/TransportationGraphDto.java (1)
3-12: DTO 외부 노출 없음 — JPA 엔티티 사용 안전위 스크립트 검색 결과,
TransportationGraphDto는 컨트롤러나 외부 API 응답으로 직렬화되지 않고 서비스 내부(TransportationService)에서만 사용되고 있습니다.
따라서 지연 로딩 예외나 영속성 컨텍스트 누수 위험은 발생하지 않습니다.• 주요 사용 위치
TransportationService.buildGraph(...)→new TransportationGraphDto(graph, weightMap)TransportationService.findOptimalRoute(...)내부 연산용권장 사항(선택)
- 그래프 전용 순수 값 타입(
record Edge등)으로 변환하면 로직 분리가 명확해집니다.Map<Pair<Long, Long>, Integer>대신EdgeKey(from, to)같은 전용 키 타입을 도입하면 가독성·안정성이 향상됩니다.src/main/java/com/wayble/server/direction/entity/transportation/Node.java (2)
37-53: Edge/Route 컬렉션에 LAZY + @batchsize(100) 적용은 N+1 완화에 효과적입니다대량 그래프 탐색 시 반복 접근으로 인한 추가 쿼리를 줄이는 데 도움이 됩니다. 좋은 선택입니다.
63-65: createNode는 임시 노드 생성용으로 보이나, ID 충돌 방지 전략을 명확히 하세요임시 출발/도착 노드를 위해
createNode를 사용한다면:
- 영속화하지 않도록 보장하거나,
- DB와 충돌하지 않는 ID 범위(예: 음수/오프셋)를 일관되게 사용,
- equals/hashCode가 id만 보므로, 임시 노드 간 id 유일성 보장
중 하나를 명시하세요. 서비스에서 임시 노드를 영속화하지 않는지 재확인 부탁드립니다.
제안: 임시 노드 ID 정책(예: 음수 시작 오프셋)을 명문화하고 상수로 관리하면 안전합니다.
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
193-194: 도보 연결 제한 개수 불일치주석에는 "최대 3개"라고 되어 있지만 실제 코드는 3개에서 break하므로 2개까지만 연결됩니다.
-// 1. 각 노드별로 인근 정류장 찾기 (800m 이내, 최대 3개) +// 1. 각 노드별로 인근 정류장 찾기 (2000m 이내, 최대 3개) for (Node node1 : nodes) { int connectionCount = 0; // ... for (Node node2 : nodes) { if (node1.getId().equals(node2.getId())) continue; - if (connectionCount >= 3) break; + if (connectionCount >= 3) break; // 이미 3개 연결된 경우 중단 // ... if (distance <= NEARBY_STATION_WALK_DISTANCE) { nearbyNodes.add(node2); connectionCount++; } }Likely an incorrect or invalid review comment.
src/main/java/com/wayble/server/direction/service/TransportationService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
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: 2
🔭 Outside diff range comments (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
485-494: Step 생성자 인자 불일치 - 8개 인자 전달Step 생성자에 8개의 인자를 전달하고 있지만, Step 레코드는 5개의 필드만 정의되어 있습니다.
-mergedSteps.add(new TransportationResponseDto.Step( - currentType, - moveInfoList, - routeName, - moveNumber, - busInfo, - subwayInfo, - fromName, - toName -)); +// Step의 실제 필드에 맞게 수정 +// busInfo 또는 subwayInfo를 NodeInfo로 변환 +TransportationResponseDto.NodeInfo information = null; +if (currentType == DirectionType.SUBWAY && currentEdge.getStartNode() != null) { + information = facilityService.getNodeInfo(currentEdge.getStartNode().getId()); +} +mergedSteps.add(new TransportationResponseDto.Step( + currentType, + routeName, + information, + fromName, + toName +));
♻️ Duplicate comments (6)
src/main/java/com/wayble/server/direction/service/TransportationService.java (6)
273-308: 다익스트라 알고리즘 내에서 동적 도보 연결 생성의 부적절함다익스트라 알고리즘 실행 중에 동적으로 도보 엣지를 생성하는 것은 알고리즘의 정확성을 해칠 수 있습니다. 그래프는 알고리즘 실행 전에 완전히 구성되어야 합니다.
동적 도보 연결 생성 로직을
buildGraph메서드로 이동시키는 것이 좋습니다.
428-430: Step 생성자 인자 개수 불일치
TransportationResponseDto.Step생성자를 호출할 때 8개의 인자를 전달하고 있지만, 실제Step레코드는 5개의 필드만 가지고 있습니다 (mode,routeName,information,from,to).올바른 생성자 호출로 수정해야 합니다:
-mergedSteps.add(new TransportationResponseDto.Step( - DirectionType.WALK, null, null, 0, null, null, fromName, toName -)); +mergedSteps.add(new TransportationResponseDto.Step( + DirectionType.WALK, null, null, fromName, toName +));
435-437: 정의되지 않은 타입 사용: MoveInfo
TransportationResponseDto.MoveInfo타입을 사용하고 있지만,TransportationResponseDto에는MoveInfo가 정의되어 있지 않습니다.이 부분을 제거하거나, 필요하다면
MoveInfo타입을 먼저 정의해야 합니다.
439-441: 정의되지 않은 타입 사용: BusInfo 및 SubwayInfo
TransportationResponseDto.BusInfo와TransportationResponseDto.SubwayInfo타입을 사용하고 있지만, 실제로 정의되어 있지 않습니다.
BusInfoService가 반환하는 타입과 일치하도록 수정하거나, 해당 타입들을TransportationResponseDto에 정의해야 합니다.#!/bin/bash # BusInfo와 SubwayInfo 타입이 실제로 정의되어 있는지 확인 echo "=== TransportationResponseDto에서 BusInfo와 SubwayInfo 정의 찾기 ===" ast-grep --pattern 'record BusInfo($$$) { $$$ }' src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java echo -e "\n=== TransportationResponseDto에서 SubwayInfo 정의 찾기 ===" ast-grep --pattern 'record SubwayInfo($$$) { $$$ }' src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java echo -e "\n=== BusInfoService.getBusInfo의 반환 타입 확인 ===" ast-grep --pattern 'public $_ getBusInfo($$$) { $$$ }' src/main/java/com/wayble/server/direction/service/BusInfoService.java
559-561: NullPointerException 발생 가능
currentStep.routeName()이 null일 수 있는데.equals()메서드를 직접 호출하고 있어 NullPointerException이 발생할 수 있습니다.null 체크를 추가해야 합니다:
-if (currentStep.mode() == nextStep.mode() && - currentStep.routeName() != null && nextStep.routeName() != null && - !currentStep.routeName().equals(nextStep.routeName())) { +if (currentStep.mode() == nextStep.mode() && + currentStep.routeName() != null && nextStep.routeName() != null && + !currentStep.routeName().equals(nextStep.routeName())) {
467-471: SubwayInfo 생성자 호출 오류
TransportationResponseDto.SubwayInfo를 생성할 때 NodeInfo의 필드를 직접 전달하고 있는데, SubwayInfo 타입이 정의되어 있지 않아 컴파일 오류가 발생합니다.NodeInfo를 직접 사용하도록 수정해야 합니다:
-TransportationResponseDto.NodeInfo nodeInfo = facilityService.getNodeInfo(currentEdge.getStartNode().getId()); -subwayInfo = new TransportationResponseDto.SubwayInfo( - nodeInfo.wheelchair(), - nodeInfo.elevator(), - nodeInfo.accessibleRestroom() -); +TransportationResponseDto.NodeInfo nodeInfo = facilityService.getNodeInfo(currentEdge.getStartNode().getId()); +// nodeInfo를 Step에 직접 전달
🧹 Nitpick comments (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
137-140: 환승 횟수 제한이 너무 엄격함환승 횟수를 4회 이상 제외하는 로직이 있는데, 일반적으로 대도시 대중교통에서는 3-4회 환승이 필요한 경우가 종종 있습니다.
환승 제한을 5회로 늘리거나, 설정 가능한 파라미터로 만드는 것을 고려하세요.
+private static final int MAX_TRANSFER_COUNT = 5; // 최대 환승 횟수 -if (transferCount >= 4) { +if (transferCount >= MAX_TRANSFER_COUNT) { return new ArrayList<>(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (5)
src/main/java/com/wayble/server/direction/service/BusInfoService.java (1)
Service(22-203)src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
Service(23-109)src/main/java/com/wayble/server/direction/dto/TransportationResponseDto.java (1)
Schema(9-37)src/main/java/com/wayble/server/direction/service/WaybleDijkstraService.java (1)
Service(16-170)src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
Entity(9-36)
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
src/main/java/com/wayble/server/direction/service/TransportationService.java
Show resolved
Hide resolved
zyovn
left a comment
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.
수고하셨습니다!
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit
신기능
성능/안정성