[refactor] 대중교통 길찾기 api 관련 리팩토링#107
Conversation
[feat] Discord Webhook 에러 알림 기능 구현
[fix] 배포 서버에서 로그인,유저 정보 조회 관련 에러 해결
Walkthrough이 변경사항은 KRIC 외부 API 연동을 위한 설정 및 WebClient 도입, FacilityService의 리팩터링, Directions API의 인증 강화, Discord 웹훅을 통한 예외 알림 추가, 엔티티 및 DTO의 필드명 및 타입 보정, 그리고 Dijkstra 알고리즘 및 거리 체크 로직 개선 등을 포함합니다. 일부 불필요한 설정 및 서비스 파일이 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant KRIC_API
participant DiscordWebhook
Client->>Controller: POST /api/v1/directions/
Controller->>Service: findDirections(request)
Service->>Service: 거리 30km 이상 여부 확인
alt 거리 초과
Service-->>Controller: 예외 발생 (DISTANCE_TOO_FAR)
Controller-->>DiscordWebhook: 예외 정보 전송 (개발 환경만)
Controller-->>Client: 400 에러 응답
else 정상 처리
Service->>KRIC_API: WebClient로 화장실 정보 조회
KRIC_API-->>Service: 응답 반환
Service-->>Controller: 결과 반환
Controller-->>Client: 응답 반환
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
.github/workflows/cd-develop.yml (1)
6-8: 주석 처리된 트리거 코드 제거 권장feature 브랜치에 대한 주석 처리된 트리거 설정은 프로덕션 워크플로우 파일에 불필요합니다. 코드 가독성을 위해 제거하는 것이 좋습니다.
-#push: -# branches: [ "feature/seungin" ]src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (1)
64-100: Discord 알림 구현이 적절함개발 환경에서만 알림을 보내도록 프로파일 체크가 잘 구현되었고, 에러 처리도 적절합니다.
웹훅 실패 시 더 자세한 에러 정보를 로깅하면 디버깅에 도움이 될 것입니다:
-} catch (Exception e){ - log.error(e.getMessage()); +} catch (Exception e){ + log.error("Discord 웹훅 전송 실패: {}", e.getMessage(), e);src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (1)
38-46: 정적 팩토리 메서드 구현이 깔끔합니다.
createEdge메서드가 Edge 생성을 잘 캡슐화하고 있습니다. 다만, JavaDoc 주석을 추가하면 더 좋을 것 같습니다.+ /** + * Edge 인스턴스를 생성하는 정적 팩토리 메서드 + * @param id Edge ID + * @param startNode 시작 노드 + * @param endNode 종료 노드 + * @param edgeType Edge 타입 + * @return 생성된 Edge 인스턴스 + */ public static Edge createEdge(Long id, Node startNode, Node endNode, DirectionType edgeType) {src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
101-103: 로그 레벨 조정 고려숫자 형식 오류는 데이터 품질 문제일 수 있으므로 debug보다는 warn 레벨이 적절할 수 있습니다.
- } catch (NumberFormatException e) { - log.debug("지하철 역에 대해 잘못된 숫자 형식. 지하철역 번호 {}: {}", stinCd, item.toltNum()); - } + } catch (NumberFormatException e) { + log.warn("지하철 역에 대해 잘못된 숫자 형식. 지하철역 번호 {}: {}", stinCd, item.toltNum()); + }src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
49-50: 노드 생성 시 띄어쓰기 오류
DirectionType.FROM_WAYPOINT뒤에 불필요한 공백이 있습니다.- Node start = new Node(-1L, origin.name(), DirectionType.FROM_WAYPOINT ,origin.latitude(), origin.longitude()); + Node start = new Node(-1L, origin.name(), DirectionType.FROM_WAYPOINT, origin.latitude(), origin.longitude());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/cd-develop.yml(2 hunks)src/main/java/com/wayble/server/ServerApplication.java(2 hunks)src/main/java/com/wayble/server/auth/entity/RefreshToken.java(1 hunks)src/main/java/com/wayble/server/common/config/RestTemplateConfig.java(0 hunks)src/main/java/com/wayble/server/common/config/SecurityConfig.java(0 hunks)src/main/java/com/wayble/server/common/config/WebClientConfig.java(3 hunks)src/main/java/com/wayble/server/common/dto/DiscordWebhookPayload.java(1 hunks)src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java(3 hunks)src/main/java/com/wayble/server/direction/controller/TransportationController.java(2 hunks)src/main/java/com/wayble/server/direction/dto/TransportationResponseDto.java(3 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Edge.java(2 hunks)src/main/java/com/wayble/server/direction/entity/transportation/Node.java(1 hunks)src/main/java/com/wayble/server/direction/exception/DirectionErrorCase.java(1 hunks)src/main/java/com/wayble/server/direction/external/kric/KricProperties.java(1 hunks)src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawBody.java(1 hunks)src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawItem.java(1 hunks)src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawResponse.java(1 hunks)src/main/java/com/wayble/server/direction/repository/FacilityRepository.java(1 hunks)src/main/java/com/wayble/server/direction/service/EdgeService.java(0 hunks)src/main/java/com/wayble/server/direction/service/FacilityService.java(3 hunks)src/main/java/com/wayble/server/direction/service/TransportationService.java(9 hunks)src/main/java/com/wayble/server/user/service/UserInfoService.java(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/com/wayble/server/common/config/SecurityConfig.java
- src/main/java/com/wayble/server/common/config/RestTemplateConfig.java
- src/main/java/com/wayble/server/direction/service/EdgeService.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
src/main/java/com/wayble/server/direction/service/TransportationService.java (1)
Slf4j(21-354)
🔇 Additional comments (26)
.github/workflows/cd-develop.yml (1)
192-192: 워크플로우 파일명과 브랜치 참조 불일치파일명은
cd-develop.yml이지만 Discord 메시지에서는 "main" 브랜치 배포를 언급하고 있습니다. 이 워크플로우가 실제로 main 브랜치용인지 develop 브랜치용인지 확인이 필요합니다.워크플로우의 의도된 용도를 명확히 하고, 필요시 파일명을
cd-main.yml로 변경하거나 메시지를 "develop" 브랜치로 수정해주세요.src/main/java/com/wayble/server/common/dto/DiscordWebhookPayload.java (1)
5-17: Discord 웹훅 페이로드 구조가 적절하게 구현됨Java record를 사용한 불변 DTO 구현이 깔끔하고 Discord 웹훅 API 구조와 잘 일치합니다.
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (1)
44-44: 예외 핸들러와 Discord 알림 통합이 깔끔함두 예외 핸들러 모두에서 일관되게 Discord 알림을 호출하고 있어 좋습니다.
Also applies to: 58-58
src/main/java/com/wayble/server/direction/exception/DirectionErrorCase.java (1)
12-12: 새로운 거리 제한 에러 케이스 추가가 적절합니다.30km 거리 제한을 위한 에러 케이스가 적절하게 정의되었습니다. HTTP 상태 코드(400), 에러 코드(4002), 그리고 한국어 메시지가 일관성 있게 구성되었습니다.
src/main/java/com/wayble/server/auth/entity/RefreshToken.java (1)
18-18: 데이터베이스 컬럼명 명시가 좋은 개선사항입니다.JPA 컬럼 매핑에서 명시적으로
"user_id"컬럼명을 지정한 것은 데이터베이스 네이밍 전략에 의존하지 않고 명확성을 높이는 좋은 방법입니다.src/main/java/com/wayble/server/direction/external/kric/KricProperties.java (1)
5-9: 현대적인 Spring Boot 설정 패턴을 잘 활용했습니다.
@ConfigurationProperties와 record 클래스를 사용한 설정 구조가 깔끔하고 불변성을 보장합니다."kric.api"접두사도 설정 범위를 명확히 식별할 수 있어 좋습니다.src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawItem.java (1)
1-1: 외부 API DTO 패키지 구조 개선이 적절합니다.KRIC 관련 DTO를
external.kric.dto패키지로 이동한 것은 외부 API와 내부 코드를 명확히 구분하고 코드 조직화를 개선하는 좋은 리팩토링입니다.src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawResponse.java (1)
1-1: 일관된 패키지 구조 개선입니다.다른 KRIC DTO들과 함께
external.kric.dto패키지로 이동한 것이 일관성 있고 적절합니다. 외부 API DTO들을 체계적으로 정리하는 좋은 접근방식입니다.src/main/java/com/wayble/server/direction/external/kric/dto/KricToiletRawBody.java (1)
1-1: 패키지 구조 개선 승인KRIC 관련 DTO를 전용 패키지로 이동시킨 것은 외부 API 통합의 모듈성을 향상시키는 좋은 구조적 개선입니다.
src/main/java/com/wayble/server/ServerApplication.java (2)
4-4: Import 추가 승인KRIC 외부 API 통합을 위한 import 추가가 적절합니다.
20-20: Configuration Properties 설정 승인기존 TMapProperties와 일관된 패턴으로 KricProperties를 추가한 것이 좋습니다. Spring Boot의 configuration binding 규칙을 잘 따르고 있습니다.
src/main/java/com/wayble/server/direction/repository/FacilityRepository.java (2)
5-6: Import 추가 승인필요한 타입들의 import가 적절히 추가되었습니다.
11-11: Repository 메소드 추가 승인
findByNodeId메소드는 Spring Data JPA 명명 규칙을 잘 따르고 있으며,Optional반환 타입을 사용하여 null safety를 보장합니다. FacilityService의 업데이트된 요구사항을 잘 지원할 것 같습니다.src/main/java/com/wayble/server/direction/dto/TransportationResponseDto.java (3)
4-4: Nullable 어노테이션 Import 개선 승인Micrometer의 어노테이션 대신 Spring의
@Nullable을 사용하는 것이 이 컨텍스트에 더 적합합니다.
16-17: Nullability 명시 개선
routeName과information필드에@Nullable어노테이션을 추가한 것이 타입 안전성을 향상시킵니다.
35-35: 오타 수정 승인"Longtitude"를 "Longitude"로 수정한 것은 중요한 오타 수정입니다. 정확한 지리적 용어를 사용하는 것이 코드의 가독성과 정확성을 높입니다.
src/main/java/com/wayble/server/common/config/WebClientConfig.java (3)
4-4: KricProperties Import 추가 승인KRIC API 설정을 위한 import가 적절히 추가되었습니다.
15-15: 의존성 주입 추가 승인기존 TMapProperties와 일관된 패턴으로 KricProperties 의존성을 추가했습니다.
30-35: KRIC WebClient Bean 추가 승인기존
tMapWebClient()메소드와 동일한 패턴을 따라kricWebClient()Bean을 추가한 것이 일관성을 유지하며 좋습니다. RestTemplate에서 WebClient로의 마이그레이션을 잘 지원할 것 같습니다.src/main/java/com/wayble/server/direction/entity/transportation/Node.java (1)
52-55: 생성자 개선이 적절합니다!
DirectionType파라미터 추가로 노드 타입을 명확히 구분할 수 있게 되었습니다. 경로 탐색 로직 개선에 도움이 될 것으로 보입니다.src/main/java/com/wayble/server/direction/entity/transportation/Edge.java (1)
10-10: 빌더 접근 제한이 적절합니다.빌더를 private으로 변경하여 정적 팩토리 메서드를 통한 생성을 강제하는 것은 좋은 캡슐화 전략입니다.
src/main/java/com/wayble/server/direction/controller/TransportationController.java (1)
54-56: HTTP 메서드 변경 적절 – 코드 내 잔여 GET 매핑 없음, 클라이언트·문서 공지 필요rg를 통해 코드베이스(.java, .md)에서
@GetMapping,getDirections,directions참조를 확인한 결과 이전 GET 엔드포인트 관련 코드는 더 이상 존재하지 않습니다.
따라서 코드상 잔여 작업은 없으며, POST 전환은 올바른 선택입니다. 다만, Breaking Change이므로 다음 사항을 반드시 진행해 주세요:
- API 문서(README, Swagger/OpenAPI 등)에 POST 엔드포인트(
/directions/transportation/)로 변경된 사항을 반영- 클라이언트 팀 또는 사용자에게 변경된 HTTP 메서드 및 경로를 사전 공지
src/main/java/com/wayble/server/direction/service/FacilityService.java (1)
31-32: findByNodeId 메서드 정의 확인됨FacilityRepository 인터페이스에
Optional<Facility> findByNodeId(Long nodeId)
가 선언되어 있어 변경이 올바르게 반영되었습니다.src/main/java/com/wayble/server/direction/service/TransportationService.java (3)
32-35: 상수 추출이 적절합니다.매직 넘버를 상수로 추출하여 코드의 가독성과 유지보수성이 향상되었습니다.
100-100: 정적 팩토리 메서드 사용이 일관성 있습니다.Edge 생성을
Edge.createEdge로 통일하여 코드 일관성이 향상되었습니다.Also applies to: 104-104
260-263: Null 체크 추가가 적절합니다.
getStationName()이 null일 수 있는 경우를 처리하여 NullPointerException을 방지합니다.
| @@ -19,11 +30,19 @@ public class GlobalExceptionHandler { | |||
|
|
|||
| private final RestTemplate restTemplate = new RestTemplate(); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
RestTemplate을 빈으로 주입받는 것을 권장
RestTemplate을 직접 인스턴스화하는 대신 빈으로 주입받으면 테스트 용이성과 설정 관리가 개선됩니다.
-private final RestTemplate restTemplate = new RestTemplate();
+private final RestTemplate restTemplate;생성자나 @Autowired를 통해 주입받거나, WebClient 사용을 고려해보세요. PR 목적에서 WebClient 도입을 언급했으므로 일관성을 위해 여기서도 WebClient 사용이 적절할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
at line 31, the RestTemplate is instantiated directly, which is not recommended.
Refactor the code to inject RestTemplate as a Spring bean via constructor
injection or @Autowired to improve testability and configuration management.
Alternatively, considering the PR mentions WebClient adoption, replace
RestTemplate with a WebClient bean injected similarly for consistency.
| try{ | ||
| KricToiletRawResponse response = kricWebClient | ||
| .get() | ||
| .uri(uri) | ||
| .retrieve() | ||
| .bodyToMono(KricToiletRawResponse.class) | ||
| .block(); | ||
|
|
||
| List<KricToiletRawItem> items = response.getBody().body().item(); | ||
| items = response.body().item(); | ||
|
|
||
| } catch(Exception e){ | ||
| log.info("역사 화장실 api 호출 중 에러 발생: {}: {}", uri, e.getCause()); | ||
| return new HashMap<>(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
WebClient 사용 개선 필요
WebClient를 사용하면서 block()을 호출하는 것은 리액티브 프로그래밍의 이점을 활용하지 못합니다. 또한 예외 처리에서 e.getCause()가 null일 수 있습니다.
- try{
- KricToiletRawResponse response = kricWebClient
- .get()
- .uri(uri)
- .retrieve()
- .bodyToMono(KricToiletRawResponse.class)
- .block();
-
- items = response.body().item();
-
- } catch(Exception e){
- log.info("역사 화장실 api 호출 중 에러 발생: {}: {}", uri, e.getCause());
- return new HashMap<>();
- }
+ try{
+ KricToiletRawResponse response = kricWebClient
+ .get()
+ .uri(uri)
+ .retrieve()
+ .bodyToMono(KricToiletRawResponse.class)
+ .block();
+
+ items = response.body().item();
+
+ } catch(Exception e){
+ log.warn("역사 화장실 api 호출 중 에러 발생: {}: {}", uri, e.getMessage(), e);
+ return new HashMap<>();
+ }📝 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.
| try{ | |
| KricToiletRawResponse response = kricWebClient | |
| .get() | |
| .uri(uri) | |
| .retrieve() | |
| .bodyToMono(KricToiletRawResponse.class) | |
| .block(); | |
| List<KricToiletRawItem> items = response.getBody().body().item(); | |
| items = response.body().item(); | |
| } catch(Exception e){ | |
| log.info("역사 화장실 api 호출 중 에러 발생: {}: {}", uri, e.getCause()); | |
| return new HashMap<>(); | |
| } | |
| try{ | |
| KricToiletRawResponse response = kricWebClient | |
| .get() | |
| .uri(uri) | |
| .retrieve() | |
| .bodyToMono(KricToiletRawResponse.class) | |
| .block(); | |
| items = response.body().item(); | |
| } catch(Exception e){ | |
| log.warn("역사 화장실 api 호출 중 에러 발생: {}: {}", uri, e.getMessage(), e); | |
| return new HashMap<>(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/FacilityService.java around
lines 79 to 92, avoid using block() on the WebClient call to preserve reactive
programming benefits; instead, refactor to return a Mono or use reactive
operators properly. Also, improve exception logging by checking if e.getCause()
is null before logging or log the exception message and stack trace directly to
avoid null pointer issues.
| // 거리 검증 (30km 제한) | ||
| double distance = haversine(origin.latitude(), origin.longitude(), | ||
| destination.latitude(), destination.longitude()); | ||
| if (distance >= METER_CONVERSION) { | ||
| throw new ApplicationException(DISTANCE_TOO_FAR); | ||
| } |
There was a problem hiding this comment.
거리 검증 로직 오류
거리 계산 결과는 km 단위인데 METER_CONVERSION(1000)과 비교하고 있습니다. 30km 제한을 확인하려면 DISTANCE_CONSTRAINT와 비교해야 합니다.
- // 거리 검증 (30km 제한)
- double distance = haversine(origin.latitude(), origin.longitude(),
- destination.latitude(), destination.longitude());
- if (distance >= METER_CONVERSION) {
- throw new ApplicationException(DISTANCE_TOO_FAR);
- }
+ // 거리 검증 (30km 제한)
+ double distance = haversine(origin.latitude(), origin.longitude(),
+ destination.latitude(), destination.longitude());
+ if (distance >= DISTANCE_CONSTRAINT) {
+ throw new ApplicationException(DISTANCE_TOO_FAR);
+ }🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/direction/service/TransportationService.java
around lines 42 to 47, the distance validation compares the distance in
kilometers with METER_CONVERSION (1000), which is incorrect. Replace the
comparison value with DISTANCE_CONSTRAINT (30) to correctly enforce the 30km
distance limit.
| user.getDisabilityType().size(); | ||
| user.getMobilityAid().size(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
NullPointerException 가능성 및 지연 로딩 초기화 방식 개선 필요
userType이 DISABLED가 아닐 경우 disabilityType, mobilityAid는 null로 저장되므로, 현재 코드에서 .size() 호출 시 NullPointerException이 발생합니다. 또한 컬렉션 초기화를 위해 사이드-이펙트 호출을 넣는 방식은 의도가 명확하지 않아 유지보수성이 떨어집니다.
아래와 같이 null 체크를 추가하고, 주석으로 의도를 명확히 하거나 Hibernate.initialize()(JPA 구현체 의존)·fetch join으로 대체하는 방안을 고려해 주세요.
- user.getDisabilityType().size();
- user.getMobilityAid().size();
+ // LAZY 컬렉션 초기화를 위한 접근
+ if (user.getDisabilityType() != null) {
+ user.getDisabilityType().size();
+ }
+ if (user.getMobilityAid() != null) {
+ user.getMobilityAid().size();
+ }또한 본 메서드는 조회 전용이므로
@Transactional(readOnly = true) 옵션을 부여하면 불필요한 쓰기 플러시를 방지해 성능을 개선할 수 있습니다.
📝 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.
| user.getDisabilityType().size(); | |
| user.getMobilityAid().size(); | |
| // LAZY 컬렉션 초기화를 위한 접근 | |
| if (user.getDisabilityType() != null) { | |
| user.getDisabilityType().size(); | |
| } | |
| if (user.getMobilityAid() != null) { | |
| user.getMobilityAid().size(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/service/UserInfoService.java around
lines 112 to 114, calling .size() on user.getDisabilityType() and
user.getMobilityAid() can cause NullPointerException if userType is not DISABLED
and these collections are null. Fix this by adding null checks before calling
.size() or use Hibernate.initialize() to explicitly initialize these
collections. Also, add a comment explaining the purpose of this initialization
to improve code clarity. Additionally, annotate the method with
@Transactional(readOnly = true) to optimize performance by preventing
unnecessary write flushes.
seung-in-Yoo
left a comment
There was a problem hiding this comment.
효인님 수고하셨습니다!! 급한게 아니라면 머지하는 브랜치 main 말고 develop으로 바꿔서 머지하는건 어떨까요!?
|
헉 수정하겠습니다! |
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
보안
기타