-
Notifications
You must be signed in to change notification settings - Fork 1
[Feat] 서울 제외 교통사고 다발지역 응답 추가 #67
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughKoroad 핫스팟 동기화 시스템을 서울 특화 구현에서 동적 지역 기반 구현으로 변경합니다. 타입 필드를 String에서 KoroadType 열거형으로 변환하고, 지역 정보를 관리하는 새로운 유틸리티 클래스를 추가하며, 불필요한 KoroadRiskService를 제거합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as Controller<br/>(Sync)
participant Region as KoroadRegion
participant Service as SyncService
participant ApiClient as ApiClient
participant Repository as Repository
rect rgba(100, 150, 255, 0.5)
Note over Client,Repository: 이전: 서울 전용 동기화
Client->>Controller: POST /sync/seoul<br/>(apiType, type)
Controller->>Service: syncHotspots<br/>(apiType, type, sido, gugun)
Service->>ApiClient: fetchHotspots<br/>(apiType, sido, gugun)
ApiClient-->>Service: hotspots
Service->>Repository: deleteByTypeAndSiDoAndGuGun<br/>(type[String], sido, gugun)
Service->>Repository: save(hotspots)
end
rect rgba(150, 200, 100, 0.5)
Note over Client,Repository: 이후: 동적 지역 기반 동기화
Client->>Controller: POST /sync(type)
Controller->>Region: SIDO_GUGUN.entrySet()
Region-->>Controller: 모든 지역 반복
loop 각 지역(sido, gugun)
Controller->>Service: syncHotspots<br/>(type, sido, gugun)
Service->>ApiClient: fetchHotspots<br/>(type, sido, gugun)
ApiClient-->>Service: hotspots
Note over Service: KoroadType.from(type)<br/>타입 변환
Service->>Repository: deleteByTypeAndSiDoAndGuGun<br/>(koroadType[Enum], sido, gugun)
Service->>Repository: save(hotspots)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. 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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/backend/knowhow/domain/alert/client/KoroadApiClient.java (1)
51-83: API 키가 로그에 노출됩니다.Line 63, 69, 82에서
url을 그대로 로깅하면ServiceKey가 외부에 노출됩니다. 민감정보이므로 마스킹/제외가 필요합니다.🔒️ 개선 예시 (키 마스킹)
- String url = baseUrl + path + String url = baseUrl + path + "?ServiceKey=" + apiKey + "&searchYearCd=" + searchYear + "&siDo=" + siDo + "&guGun=" + guGun + "&type=json" + "&numOfRows=9999" + "&pageNo=1"; + String safeUrl = url.replace(apiKey, "****"); ... - log.warn("[KoroadApiClient] Null response. url={}", url); + log.warn("[KoroadApiClient] Null response. url={}", safeUrl); ... - url, res.getResultCode(), res.getResultMsg() + safeUrl, res.getResultCode(), res.getResultMsg() ... - log.error("[KoroadApiClient] fetchHotspots error. type={}, url={}, message={}", type, url, e.getMessage(), e); + log.error("[KoroadApiClient] fetchHotspots error. type={}, url={}, message={}", type, safeUrl, e.getMessage(), e);
🤖 Fix all issues with AI agents
In
`@src/main/java/backend/knowhow/domain/alert/controller/KoroadHotspotSyncController.java`:
- Around line 27-40: Validate the incoming type before entering the nested loop
in syncSeoul: call KoroadType.from(type) once at the top of the method (e.g.,
assign to a variable like KoroadType koroadType = KoroadType.from(type)) so
invalid types immediately trigger the existing
BaseException(ErrorType.BAD_REQUEST) and prevent repeated calls to
syncService.syncHotspots; then use the validated koroadType (or simply ignore
and continue using type) inside the loop over KoroadRegion.SIDO_GUGUN when
invoking syncService.syncHotspots.
In `@src/main/java/backend/knowhow/domain/alert/domain/KoroadHotspot.java`:
- Around line 25-26: The KoroadHotspot.type field is currently using JPA's
default ORDINAL mapping which will mismatch the existing VARCHAR column; add an
explicit enum mapping by annotating the field (KoroadHotspot.type) with
`@Enumerated`(EnumType.STRING) so KoroadType values are persisted as their names;
ensure the corresponding import for javax.persistence.Enumerated and
javax.persistence.EnumType is added/updated and run a quick schema/data check to
confirm compatibility with the existing string column.
In `@src/main/java/backend/knowhow/domain/alert/domain/KoroadType.java`:
- Around line 24-28: KoroadType.from currently calls equalsIgnoreCase on the
incoming value and will NPE for null (and should reject blank inputs); update
KoroadType.from to first validate the input (e.g., if value == null or
value.trim().isEmpty()) and immediately throw new
BaseException(ErrorType.BAD_REQUEST), then proceed with
Arrays.stream(values())...filter(type ->
type.param.equalsIgnoreCase(value))...orElseThrow(...) as before so
equalsIgnoreCase is only invoked on non-null, non-blank input.
🧹 Nitpick comments (1)
src/main/java/backend/knowhow/domain/alert/client/KoroadApiClient.java (1)
28-28: searchYear 하드코딩 대신 동적 계산 권장.“최근 3년 기준”이라면 매년 값이 바뀌어야 하므로 연도 하드코딩은 데이터 최신성에 영향을 줍니다. 시스템 날짜 기반 계산 또는 설정값으로 전환하는 쪽이 안전합니다.
♻️ 개선 예시
- private static final int searchYear = 2024; // 최근 3년 기준 연도 + private static final int searchYear = java.time.Year.now().getValue() - 2; // 최근 3년 기준 연도
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/backend/knowhow/domain/alert/client/KoroadApiClient.javasrc/main/java/backend/knowhow/domain/alert/controller/KoroadHotspotSyncController.javasrc/main/java/backend/knowhow/domain/alert/domain/KoroadHotspot.javasrc/main/java/backend/knowhow/domain/alert/domain/KoroadType.javasrc/main/java/backend/knowhow/domain/alert/mapper/KoroadHotspotMapper.javasrc/main/java/backend/knowhow/domain/alert/repository/KoroadHotspotRepository.javasrc/main/java/backend/knowhow/domain/alert/service/KoroadHotspotSyncService.javasrc/main/java/backend/knowhow/domain/alert/service/KoroadRiskService.javasrc/main/java/backend/knowhow/domain/alert/util/KoroadRegion.java
💤 Files with no reviewable changes (1)
- src/main/java/backend/knowhow/domain/alert/service/KoroadRiskService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/backend/knowhow/domain/alert/controller/KoroadHotspotSyncController.java (1)
src/main/java/backend/knowhow/domain/alert/util/KoroadRegion.java (1)
KoroadRegion(8-350)
🔇 Additional comments (4)
src/main/java/backend/knowhow/domain/alert/util/KoroadRegion.java (1)
1-350: 구성 명확하고 사용 목적에 부합합니다.시도/구군 코드 매핑을 중앙화한 점이 좋습니다.
src/main/java/backend/knowhow/domain/alert/mapper/KoroadHotspotMapper.java (1)
27-32: Enum 전환과 문구 매핑이 깔끔합니다.타입 안정성이 좋아졌습니다.
src/main/java/backend/knowhow/domain/alert/repository/KoroadHotspotRepository.java (1)
11-15: KoroadType 기반 쿼리 메서드 전환 좋습니다.
타입 안정성이 높아지고 상위 레이어와 일관성이 생겼습니다.src/main/java/backend/knowhow/domain/alert/service/KoroadHotspotSyncService.java (1)
25-55: KoroadType 매핑과 저장 흐름 정리가 깔끔합니다.
enum으로 통일되면서 타입 안정성이 좋아졌습니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @PostMapping("/sync") | ||
| public String syncSeoul( | ||
| @RequestParam String apiType, // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등) | ||
| @RequestParam String type // DB KoroadHotspot.type 에 저장할 값 ("OLD_MAN", "CHILD", "SCHOOL" 등) | ||
| @RequestParam String type // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등) | ||
| ) { | ||
|
|
||
| log.info("start"); | ||
| log.info("sync start"); | ||
| int total = 0; | ||
|
|
||
| for (Integer guGun : SEOUL_GUGUN_CODES) { | ||
| syncService.syncHotspots(apiType, type, SEOUL_SIDO, guGun); | ||
| log.info("{} finish", guGun); | ||
| total++; | ||
| for (var entry : KoroadRegion.SIDO_GUGUN.entrySet()) { | ||
| int siDo = entry.getKey(); | ||
| for (int guGun : entry.getValue()) { | ||
| syncService.syncHotspots(type, siDo, guGun); | ||
| total++; | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the controller file and understand its structure
find . -type f -name "KoroadHotspotSyncController.java"Repository: Care-Ride/backend
Length of output: 149
🏁 Script executed:
# Search for KoroadType class definition
find . -type f -name "KoroadType.java"Repository: Care-Ride/backend
Length of output: 128
🏁 Script executed:
# Look for syncService.syncHotspots() method
find . -type f -name "*SyncService*.java" -o -name "*Service*.java" | grep -i syncRepository: Care-Ride/backend
Length of output: 143
🏁 Script executed:
# Read KoroadType.java
cat ./src/main/java/backend/knowhow/domain/alert/domain/KoroadType.javaRepository: Care-Ride/backend
Length of output: 772
🏁 Script executed:
# Read the full KoroadHotspotSyncController.java
cat ./src/main/java/backend/knowhow/domain/alert/controller/KoroadHotspotSyncController.javaRepository: Care-Ride/backend
Length of output: 1324
🏁 Script executed:
# Read the syncService implementation
cat ./src/main/java/backend/knowhow/domain/alert/service/KoroadHotspotSyncService.javaRepository: Care-Ride/backend
Length of output: 2846
🏁 Script executed:
# Find BaseException definition
find . -type f -name "BaseException.java"Repository: Care-Ride/backend
Length of output: 135
🏁 Script executed:
# Find ErrorType definition
find . -type f -name "ErrorType.java"Repository: Care-Ride/backend
Length of output: 130
🏁 Script executed:
# Read BaseException
cat ./src/main/java/backend/knowhow/global/common/exception/BaseException.javaRepository: Care-Ride/backend
Length of output: 561
🏁 Script executed:
# Read ErrorType
cat ./src/main/java/backend/knowhow/global/common/response/ErrorType.javaRepository: Care-Ride/backend
Length of output: 3365
요청 파라미터 type 검증을 루프 시작 전에 추가하기
요청 파라미터 type이 잘못되면 중첩 루프 내에서 syncService.syncHotspots()를 호출할 때마다 외부 API 통신을 시도하다가 결국 KoroadType.from(type)에서 BaseException(ErrorType.BAD_REQUEST)를 던지게 됩니다. 루프 시작 전에 미리 한 번 검증하여 불필요한 외부 API 호출을 방지하고 즉시 400으로 응답하세요.
✅ 제안 수정안
`@PostMapping`("/sync")
public String syncSeoul(
`@RequestParam` String type // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등)
) {
+ KoroadType.from(type); // 루프 전에 한 번만 검증
log.info("sync start");
int total = 0;참고: KoroadType.from(type)이 이미 BaseException(ErrorType.BAD_REQUEST)를 던지므로, 별도의 try-catch 처리 없이 기존 예외 처리 메커니즘이 자동으로 400을 반환합니다.
📝 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.
| @PostMapping("/sync") | |
| public String syncSeoul( | |
| @RequestParam String apiType, // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등) | |
| @RequestParam String type // DB KoroadHotspot.type 에 저장할 값 ("OLD_MAN", "CHILD", "SCHOOL" 등) | |
| @RequestParam String type // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등) | |
| ) { | |
| log.info("start"); | |
| log.info("sync start"); | |
| int total = 0; | |
| for (Integer guGun : SEOUL_GUGUN_CODES) { | |
| syncService.syncHotspots(apiType, type, SEOUL_SIDO, guGun); | |
| log.info("{} finish", guGun); | |
| total++; | |
| for (var entry : KoroadRegion.SIDO_GUGUN.entrySet()) { | |
| int siDo = entry.getKey(); | |
| for (int guGun : entry.getValue()) { | |
| syncService.syncHotspots(type, siDo, guGun); | |
| total++; | |
| } | |
| `@PostMapping`("/sync") | |
| public String syncSeoul( | |
| `@RequestParam` String type // KoroadApiClient 에서 사용하는 구분자 ("oldman", "child", "school" 등) | |
| ) { | |
| KoroadType.from(type); // 루프 전에 한 번만 검증 | |
| log.info("sync start"); | |
| int total = 0; | |
| for (var entry : KoroadRegion.SIDO_GUGUN.entrySet()) { | |
| int siDo = entry.getKey(); | |
| for (int guGun : entry.getValue()) { | |
| syncService.syncHotspots(type, siDo, guGun); | |
| total++; | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/backend/knowhow/domain/alert/controller/KoroadHotspotSyncController.java`
around lines 27 - 40, Validate the incoming type before entering the nested loop
in syncSeoul: call KoroadType.from(type) once at the top of the method (e.g.,
assign to a variable like KoroadType koroadType = KoroadType.from(type)) so
invalid types immediately trigger the existing
BaseException(ErrorType.BAD_REQUEST) and prevent repeated calls to
syncService.syncHotspots; then use the validated koroadType (or simply ignore
and continue using type) inside the loop over KoroadRegion.SIDO_GUGUN when
invoking syncService.syncHotspots.
| @Column(nullable = false, length = 20) | ||
| private String type; | ||
| private KoroadType type; |
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.
Enum 매핑 방식 지정 누락으로 데이터 불일치 위험.
KoroadType을 저장할 때 JPA 기본값이 ORDINAL이라 기존 문자열 컬럼과 호환되지 않습니다. STRING 매핑을 명시해야 합니다.
🐛 수정 제안
- `@Column`(nullable = false, length = 20)
+ `@Enumerated`(EnumType.STRING)
+ `@Column`(nullable = false, length = 20)
private KoroadType type;📝 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.
| @Column(nullable = false, length = 20) | |
| private String type; | |
| private KoroadType type; | |
| `@Enumerated`(EnumType.STRING) | |
| `@Column`(nullable = false, length = 20) | |
| private KoroadType type; |
🤖 Prompt for AI Agents
In `@src/main/java/backend/knowhow/domain/alert/domain/KoroadHotspot.java` around
lines 25 - 26, The KoroadHotspot.type field is currently using JPA's default
ORDINAL mapping which will mismatch the existing VARCHAR column; add an explicit
enum mapping by annotating the field (KoroadHotspot.type) with
`@Enumerated`(EnumType.STRING) so KoroadType values are persisted as their names;
ensure the corresponding import for javax.persistence.Enumerated and
javax.persistence.EnumType is added/updated and run a quick schema/data check to
confirm compatibility with the existing string column.
| public static KoroadType from(String value) { | ||
| return Arrays.stream(values()) | ||
| .filter(type -> type.param.equalsIgnoreCase(value)) | ||
| .findFirst() | ||
| .orElseThrow(() -> new BaseException(ErrorType.BAD_REQUEST)); |
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.
null 입력 시 NPE로 500 발생 가능.
from(null) 호출 시 equalsIgnoreCase에서 NPE가 발생합니다. 입력값이 비어 있는 경우도 BAD_REQUEST로 일관되게 처리하도록 방어가 필요합니다.
🐛 수정 제안
public static KoroadType from(String value) {
+ if (value == null || value.isBlank()) {
+ throw new BaseException(ErrorType.BAD_REQUEST);
+ }
return Arrays.stream(values())
.filter(type -> type.param.equalsIgnoreCase(value))
.findFirst()
.orElseThrow(() -> new BaseException(ErrorType.BAD_REQUEST));
}📝 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.
| public static KoroadType from(String value) { | |
| return Arrays.stream(values()) | |
| .filter(type -> type.param.equalsIgnoreCase(value)) | |
| .findFirst() | |
| .orElseThrow(() -> new BaseException(ErrorType.BAD_REQUEST)); | |
| public static KoroadType from(String value) { | |
| if (value == null || value.isBlank()) { | |
| throw new BaseException(ErrorType.BAD_REQUEST); | |
| } | |
| return Arrays.stream(values()) | |
| .filter(type -> type.param.equalsIgnoreCase(value)) | |
| .findFirst() | |
| .orElseThrow(() -> new BaseException(ErrorType.BAD_REQUEST)); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/backend/knowhow/domain/alert/domain/KoroadType.java` around
lines 24 - 28, KoroadType.from currently calls equalsIgnoreCase on the incoming
value and will NPE for null (and should reject blank inputs); update
KoroadType.from to first validate the input (e.g., if value == null or
value.trim().isEmpty()) and immediately throw new
BaseException(ErrorType.BAD_REQUEST), then proceed with
Arrays.stream(values())...filter(type ->
type.param.equalsIgnoreCase(value))...orElseThrow(...) as before so
equalsIgnoreCase is only invoked on non-null, non-blank input.
🚘 작업 내용
#️⃣ 테스트 내용
🔗 관련 이슈
Summary by CodeRabbit
릴리스 노트
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.