[HSC-416] Instant -> LocalDateTime 파싱 오류 수정#275
Merged
Merged
Conversation
tkv00
added
🐛 fix
github-actions
Bot
changed the title
🧪 Test Coverage Report (JaCoCo)
Package line spark (sample): 📦 Package coverage (worst 10)
🧨 Lowest coverage classes (worst 10)
🧩 Changed files coverage breakdown (1 files)
🔎 HTML 리포트: Actions → Artifacts → |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 HandleLogFeatureUseCase에서 Instant를 LocalDate로 변환하는 로직을 resolveBaseDate 메서드로 분리하여 개선하였습니다. 다만, request.timeStamp()가 null일 경우 resolveBaseDate 내부에서는 방어되지만 LogFeatureEvent 생성 시에는 null이 그대로 전달되어 잠재적인 NullPointerException 발생 위험이 식별되었습니다. 또한, 하드코딩된 타임존 문자열을 상수로 관리하고 불필요한 Optional 처리를 제거하여 로직을 단순화할 것을 권장합니다.
Comment on lines
36
to
41
| calculateLogChurnScoreService.calculateAndStore( | ||
| request.memberId(), | ||
| LocalDate.from(request.timeStamp()), | ||
| resolveBaseDate(request.timeStamp()), | ||
| List.of(new LogFeatureEvent( | ||
| resolveEventId(request), | ||
| request.timeStamp(), |
There was a problem hiding this comment.
🛡️ Code Review Report
1. 🔍 요약 (Summary)
Instant를 LocalDate로 변환하는 과정에서 발생하는 파싱 오류를 수정하였습니다. 다만, null 처리 로직의 불일치로 인한 잠재적 NullPointerException 위험이 발견되었습니다.
2. 🛑 Blocking Issues (Must Fix)
- [위반 규칙]: 로직 일관성 결여 및 잠재적 NPE 위험
- [위치]:
HandleLogFeatureUseCase.java(Line 36-41) - [문제 이유]:
resolveBaseDate내부에서는null발생 시Instant.now()를 사용하도록 방어 코드가 작성되어 있으나, 41라인에서LogFeatureEvent를 생성할 때는 원본request.timeStamp()를 그대로 전달합니다. 이로 인해timeStamp가null일 경우CalculateLogChurnScoreService에서event.timestamp().toString()호출 시 NPE가 발생합니다. - [해결 제안]:
execute메서드 시작 부분에서Instant값을 확정하여 일관되게 사용하세요.
Suggested change
| calculateLogChurnScoreService.calculateAndStore( | |
| request.memberId(), | |
| LocalDate.from(request.timeStamp()), | |
| resolveBaseDate(request.timeStamp()), | |
| List.of(new LogFeatureEvent( | |
| resolveEventId(request), | |
| request.timeStamp(), | |
| Instant timestamp = Optional.ofNullable(request.timeStamp()).orElseGet(Instant::now); | |
| calculateLogChurnScoreService.calculateAndStore( | |
| request.memberId(), | |
| resolveBaseDate(timestamp), | |
| List.of(new LogFeatureEvent( | |
| resolveEventId(request), | |
| timestamp, |
Comment on lines
+58
to
+63
| private LocalDate resolveBaseDate(Instant timestamp) { | ||
| return Optional.ofNullable(timestamp) | ||
| .orElseGet(Instant::now) | ||
| .atZone(ZoneId.of("Asia/Seoul")) | ||
| .toLocalDate(); | ||
| } |
There was a problem hiding this comment.
🔍 타임존 하드코딩 및 로직 단순화
- 위치:
HandleLogFeatureUseCase.java(Line 58-63) - 문제 이유:
"Asia/Seoul"타임존이 하드코딩되어 있어 유지보수성이 떨어집니다. 또한 DTO에@NotNull이 적용되어 있으므로Optional처리를 제거하여 로직을 단순화할 수 있습니다. - 해결 제안: 타임존을 상수로 분리하고 로직을 단순화하세요.
private LocalDate resolveBaseDate(Instant timestamp) {
return timestamp.atZone(ZoneId.of("Asia/Seoul"))
.toLocalDate();
}
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
📝작업 내용
👀변경 사항
🎫 Jira Ticket
#️⃣관련 이슈