[FEAT] 지하철의 승강기 정보 출력 #166
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
WalkthroughDirectionType enum moved to a new package and imports updated. Elevator entity changed from latitude/longitude to a single location string. Added ElevatorRepository and integrated it into FacilityService to fetch elevator info for facilities. Adjusted FacilityService constructor and logic to populate elevator data. No control-flow changes elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant FacilityService
participant FacilityRepository
participant ElevatorRepository
Client->>FacilityService: getNodeInfo(routeId, facilityId, ...)
FacilityService->>FacilityRepository: findById(facilityId)
FacilityRepository-->>FacilityService: Facility
FacilityService->>ElevatorRepository: findByFacility(Facility)
ElevatorRepository-->>FacilityService: List<Elevator>
FacilityService->>FacilityService: dedupe/trim/sort locations
FacilityService-->>Client: Node info incl. elevator locations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
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: 1
🔭 Outside diff range comments (4)
src/main/java/com/wayble/server/direction/service/TransportationService.java (4)
64-71: 페이징 커서 경계 체크 누락 — IndexOutOfBoundsException 가능성요청 cursor가 전체 경로 수를 초과할 경우
subList에서 범위 오류가 날 수 있습니다. 커서를 size에 클램핑하세요.아래 수정 제안:
- int startIndex = (request.cursor() != null) ? request.cursor() : 0; - int pageSize = (request.size() != null) ? request.size() : 5; // 기본값 5로 설정 - int endIndex = Math.min(startIndex + pageSize, allRoutes.size()); + int rawCursor = (request.cursor() != null) ? request.cursor() : 0; + int pageSize = (request.size() != null) ? request.size() : 5; // 기본값 5로 설정 + int startIndex = Math.max(0, Math.min(rawCursor, allRoutes.size())); + int endIndex = Math.min(startIndex + pageSize, allRoutes.size()); boolean hasNext = endIndex < allRoutes.size(); Integer nextCursor = hasNext ? endIndex : null;
485-487: List.of()는 불변 — 동적 walk edge 추가 시 UnsupportedOperationException 위험
graph.getOrDefault(..., List.of())로 가져온 리스트가 불변일 수 있어currentEdges.add(...)에서 런타임 예외가 발생할 수 있습니다.computeIfAbsent로 가변 리스트를 확보하세요.아래 수정 제안:
- List<Edge> currentEdges = graph.getOrDefault(curr.getId(), List.of()); + List<Edge> currentEdges = graph.computeIfAbsent(curr.getId(), k -> new ArrayList<>());
690-706: 지하철 정보 조회 시 NPE 가능성 — currentEdge.getRoute() null 체크 필요
currentEdge.getRoute().getRouteId()접근 전에getRoute()null 체크가 없습니다. 실제 데이터에 따라 NPE 발생 여지가 큽니다.아래와 같이 보강해 주세요:
- } 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()); - - subwayInfo = new TransportationResponseDto.SubwayInfo( - nodeInfo.wheelchair(), - nodeInfo.elevator(), - nodeInfo.accessibleRestroom() - ); - } else { - subwayInfo = new TransportationResponseDto.SubwayInfo( - new ArrayList<>(), - new ArrayList<>(), - false - ); - } + 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 { + 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 ); } }
427-429: 임시 노드 중복 추가 가능성 — equals가 id 기반이라 List 중복 허용됨이미
nodes에 -1L/-2L ID를 가진 임시 노드를 추가한 상태에서 동일 ID의 새 노드를 다시 추가합니다. equals가 id 기준이므로contains체크 후 추가하세요.- nodes.add(startNode); - nodes.add(endNode); + if (!nodes.contains(startNode)) { + nodes.add(startNode); + } + if (!nodes.contains(endNode)) { + nodes.add(endNode); + }
🧹 Nitpick comments (5)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
678-684: 버스 정보 부재 시 경로 전체를 바로 폐기하는 정책 확인 필요저상버스 전무 + 배차간격 0 조건에서
return new ArrayList<>()로 전체 경로를 즉시 비워 반환합니다. 사용자 입장에선 “버스 구간만 이용 불가”인데 전체 경로가 사라지는 결과가 될 수 있습니다. 정책 의도인지 확인 부탁드립니다.의도대로라면 이후 필터 단계에서 제외하거나, 버스 구간만 대체/표기(“해당 시간 운행정보 없음”)하는 방안을 고려할 수 있습니다. 원하시면 정책에 맞춘 처리 분기 코드도 제안드리겠습니다.
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
21-23: 주석의 예시값이 실제 enum과 불일치합니다현재 DirectionType에는 START/FINISH가 없고 TO_WAYPOINT/FROM_WAYPOINT가 존재합니다. 주석을 enum 값과 일치하도록 정리하는 게 좋겠습니다.
적용 제안(diff):
- DirectionType mode, // 예: START, WALK, SUBWAY, BUS, FINISH + DirectionType mode, // 예: WALK, SUBWAY, BUS, TO_WAYPOINT, FROM_WAYPOINTsrc/main/java/com/wayble/server/direction/entity/transportation/Elevator.java (1)
17-18: 컬럼명/길이 명시로 일관성과 호환성 개선 제안
- 컬럼명이
location인 경우 일부 DB/툴링에서 혼동될 수 있어 구체적인elevator_location으로 정리하면 가독성과 일관성이 좋아집니다(동일 도메인인wheelchair_location과의 naming 일관성도 확보).- 길이 제한을 지정하면 스키마 의도를 명확히 할 수 있습니다(예: VARCHAR(255)).
적용 제안(diff):
- @Column(name = "location", nullable = false) + @Column(name = "elevator_location", length = 255, nullable = false) private String location;src/main/java/com/wayble/server/direction/repository/ElevatorRepository.java (1)
12-13: 파생 쿼리 메서드로 단순화 가능현재 @query는 불필요합니다. Spring Data JPA의 파생 쿼리를 사용하면 동일한 동작을 더 간결하게 표현할 수 있습니다.
적용 제안(diff):
- @Query("SELECT e FROM Elevator e WHERE e.facility = :facility") - List<Elevator> findByFacility(@Param("facility") Facility facility); + List<Elevator> findByFacility(Facility facility);src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
73-74: 불필요한 인자(routeId) 제거 및 중복 제거 로직 추가 제안
- getElevatorInfo의
routeId인자는 사용되지 않습니다. 시그니처와 호출부에서 제거하면 간결해집니다.- 엘리베이터 위치 문자열의 공백 정리는 되어 있으나, 중복 제거는 없습니다. DB/데이터 품질 상 중복이 존재할 가능성을 고려해 dedup + sort를 권장합니다.
호출부 정리(diff):
- elevator = getElevatorInfo(facility, routeId); + elevator = getElevatorInfo(facility);메서드 시그니처 및 본문 개선(diff):
- private List<String> getElevatorInfo(Facility facility, Long routeId) { + private List<String> getElevatorInfo(Facility facility) { List<String> elevatorLocations = new ArrayList<>(); try { List<Elevator> elevators = elevatorRepository.findByFacility(facility); for (Elevator elevator : elevators) { String location = elevator.getLocation(); if (location != null && !location.trim().isEmpty()) { elevatorLocations.add(location.trim()); } } - - elevatorLocations.sort(String::compareTo); - + // 중복 제거 후 정렬 + elevatorLocations = new java.util.ArrayList<>(new java.util.LinkedHashSet<>(elevatorLocations)); + elevatorLocations.sort(String::compareTo); } catch(Exception e) { log.error("엘리베이터 정보 조회 실패 - facilityId: {}, error: {}", facility.getId(), e.getMessage(), e); } return elevatorLocations; }Also applies to: 142-163
📜 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 (9)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.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/Elevator.java(1 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Node.java(1 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Route.java(1 hunks)src/main/java/com/wayble/server/direction/entity/type/DirectionType.java(1 hunks)src/main/java/com/wayble/server/direction/repository/ElevatorRepository.java(1 hunks)src/main/java/com/wayble/server/direction/service/FacilityService.java(4 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (2)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)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 (2)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)src/main/java/com/wayble/server/direction/entity/type/Type.java (1)
Getter(5-22)
src/main/java/com/wayble/server/direction/entity/transportation/Elevator.java (6)
src/main/java/com/wayble/server/direction/entity/transportation/Lift.java (1)
Entity(6-29)src/main/java/com/wayble/server/direction/entity/transportation/Facility.java (1)
Entity(11-45)src/main/java/com/wayble/server/direction/dto/request/TransportationRequestDto.java (1)
Location(12-16)src/main/java/com/wayble/server/direction/entity/transportation/Wheelchair.java (1)
Entity(6-24)src/main/java/com/wayble/server/common/entity/Address.java (1)
ToString(7-51)src/main/java/com/wayble/server/explore/dto/facility/WaybleFacilityConditionDto.java (1)
WaybleFacilityConditionDto(8-21)
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)
src/main/java/com/wayble/server/direction/entity/type/DirectionType.java (2)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)src/main/java/com/wayble/server/direction/entity/type/Type.java (2)
Getter(5-22)Type(19-21)
src/main/java/com/wayble/server/direction/repository/ElevatorRepository.java (2)
src/main/java/com/wayble/server/direction/repository/FacilityRepository.java (2)
FacilityRepository(12-17)Query(13-16)src/main/java/com/wayble/server/direction/entity/transportation/Lift.java (1)
Entity(6-29)
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)
src/main/java/com/wayble/server/direction/service/FacilityService.java (6)
src/main/java/com/wayble/server/direction/entity/transportation/Lift.java (1)
Entity(6-29)src/main/java/com/wayble/server/direction/entity/transportation/Facility.java (1)
Entity(11-45)src/main/java/com/wayble/server/direction/repository/FacilityRepository.java (1)
FacilityRepository(12-17)src/main/java/com/wayble/server/explore/entity/FacilityType.java (1)
FacilityType(3-6)src/test/java/com/wayble/server/explore/WaybleFacilityApiIntegrationTest.java (1)
Test(179-235)src/main/java/com/wayble/server/explore/dto/common/FacilityResponseDto.java (1)
Builder(6-29)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
src/main/java/com/wayble/server/direction/entity/DirectionType.java (1)
DirectionType(3-8)
🔇 Additional comments (7)
src/main/java/com/wayble/server/direction/entity/type/DirectionType.java (1)
1-1: 패키지 이동 검증 완료 — DB 마이그레이션 불필요
- 기존 import(
com.wayble.server.direction.entity.DirectionType) 사용 흔적 없음- 중복된
DirectionTypeenum 선언 없음따라서 패키지 이동으로 인한 추가 작업이나 DB 마이그레이션은 필요 없습니다. 👍
src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (1)
3-3: import 경로 업데이트 적절패키지 이동에 따른 import 변경이 정확합니다. 추가 변경 없이도 컴파일/런타임 영향은 없습니다.
src/main/java/com/wayble/server/direction/entity/transportation/Route.java (1)
4-4: import 경로 업데이트 적절 — EnumType.STRING으로 DB 영향 없음routeType이 EnumType.STRING이므로 enum 패키지 이동으로 인한 DB 마이그레이션 필요 없습니다.
src/main/java/com/wayble/server/direction/entity/transportation/Node.java (1)
11-11: import 경로 업데이트 적절패키지 이동에 따른 import 변경이 정확합니다. 다른 로직 변경 없이 안전합니다.
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
10-10: import 경로 업데이트 적절DirectionType 패키지 이동 반영 OK.
src/main/java/com/wayble/server/direction/dto/response/TransportationResponseDto.java (1)
5-5: DirectionType import 경로 변경 LGTM패키지 이동(
.entity→.entity.type) 반영이 정확합니다. 외부 공개 API 영향도 없어 보입니다.src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
36-36: ElevatorRepository 주입 추가 LGTM엘리베이터 정보 조회 책임이 서비스 외부(Repository)로 명확히 분리되어 역할이 선명해졌습니다.
src/main/java/com/wayble/server/direction/entity/transportation/Elevator.java
Show resolved
Hide resolved
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