[FIX] HOME API 연습 날짜 조회 타입 변환 오류 수정 - #150
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesHOME 날짜 처리
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
p1001q
left a comment
There was a problem hiding this comment.
💡 참고
java.sql.Date.toLocalDate()를 쓰신 선택은 좋습니다.toInstant()를 거치는 방식과 달리 타임존 변환 없이 내부에 저장된 연/월/일을 그대로 읽어오는 방식이라, DB 서버와 JVM의 타임존이 다를 때 흔히 생기는 "하루 밀림" 버그를 피할 수 있습니다.Collectors.toSet()으로 바꾼 것도 기존new HashSet<>(...)래핑과 동작은 동일하니 문제없습니다.
| @Query(""" | ||
| select distinct function('date', p.endedAt) from Playing p | ||
| where p.user.userId = :userId | ||
| and p.status = :status | ||
| and p.deletedAt is null | ||
| and p.endedAt is not null | ||
| """) | ||
| List<LocalDate> findDistinctEndedDatesByUserAndStatus( | ||
| List<Date> findDistinctEndedDatesByUserAndStatus( | ||
| @Param("userId") Long userId, | ||
| @Param("status") PlayingStatus status | ||
| ); |
There was a problem hiding this comment.
🔴 P1 — src/main/java/com/mr/domain/playing/repository/PlayingRepository.java:81-91 (그리고 CI 설정)
문제상황
이번 PR의 핵심 수정(function('date', p.endedAt)의 반환 타입을 LocalDate에서 java.sql.Date로 바꾼 것)은 PostgreSQL 런타임에서만 실제로 검증 가능합니다. 그런데 리포지토리를 다시 훑어보다가 .github/workflows/ci.yml을 열어봤는데:
# TODO: 테스트 코드 안정화 후 활성화
# - name: Run tests
# run: ./gradlew clean test
- name: Build BootJar
run: ./gradlew clean bootJar -x testCI가 테스트를 아예 실행하지 않습니다 (-x test로 스킵). 게다가 build.gradle엔 H2나 Testcontainers 같은 테스트용 DB 의존성이 전혀 없고(org.postgresql:postgresql만 runtimeOnly), @DataJpaTest/Testcontainers를 쓰는 파일도 프로젝트 전체에 하나도 없습니다.
문제가 되는 이유
이번 PR이 추가한 회귀 테스트(HomeServiceTest)는 리포지토리를 Mockito로 목킹해서 "만약 리포지토리가 java.sql.Date를 반환한다면 서비스가 잘 변환하는지"만 검증합니다. 정작 이슈 #147의 진짜 원인이었던 "PostgreSQL에서 function('date', ...)가 실제로 java.sql.Date를 반환하는가"라는 전제 자체는 어떤 자동화된 테스트로도 검증되지 않습니다. PR 설명에도 본인이 "배포 후 PostgreSQL 환경에서 GET /api/home 확인 필요"라고 적어두셨는데, 이게 정확히 이 공백을 스스로 인지하고 계신 거고요. 게다가 CI가 테스트 자체를 안 돌리니, 로컬에서 수동으로 확인 안 하면 이 PR이 머지된 후에도 아무도 자동으로 검증 못 하는 상태로 배포됩니다. 애초에 이슈 #147이 "테스트에선 안 걸리고 실제 Postgres에서만 터지는 버그"였다는 걸 생각하면, 같은 카테고리의 문제가 또 반복될 위험이 있습니다.
수정 방향
이번 PR 하나로 해결할 문제는 아니고 팀 차원의 인프라 문제라, 이 PR 자체를 블로킹할 사안은 아닙니다. 다만:
- 이 PR은 머지 전에 반드시 로컬 Postgres(
docker-compose.local.yml)로GET /api/home을 실제로 호출해서 확인하고 넘어가는 걸 강하게 권합니다(본인이 이미 계획하신 대로). - 팀 전체적으로 CI의
Run tests주석 처리를 언제 풀 계획인지, Postgres 전용 버그를 잡을 최소한의 통합 테스트(@DataJpaTest+ Testcontainers 등)를 도입할지는 별도 이슈로 논의해볼 만합니다.
Tip
"로컬(H2/모킹)에선 되는데 배포 환경(Postgres)에서만 깨진다"는 이번 버그 자체가, 테스트 인프라가 실제 DB 방언(dialect) 차이를 못 잡아내고 있다는 신호예요. 지금 당장 안 고쳐도, 이런 패턴이 반복되면 팀 회고에서 한 번 짚어볼 가치가 있어 보입니다.
| return new HashSet<>( | ||
| playingRepository.findDistinctEndedDatesByUserAndStatus(userId, PlayingStatus.COMPLETED)); | ||
| return playingRepository.findDistinctEndedDatesByUserAndStatus(userId, PlayingStatus.COMPLETED).stream() | ||
| .map(java.sql.Date::toLocalDate) |
There was a problem hiding this comment.
🟡 P3 — src/main/java/com/mr/domain/home/service/HomeService.java:89
문제상황
.map(java.sql.Date::toLocalDate)문제가 되는 이유
이 파일은 이미 import java.time.LocalDate;를 쓰고 있고 java.sql.Date와 이름이 겹치지 않아서(단순히 Date로 줄여쓰지만 않으면) 상단에 import java.sql.Date;를 추가해도 충돌이 없습니다. 굳이 전체 경로로 인라인 참조한 이유가 안 보이고, 팀 컨벤션(docs/컨벤션.md 와일드카드 import 지양 등)의 취지인 "import는 상단에 명시적으로"라는 스타일과도 살짝 어긋납니다.
수정 방향
import java.sql.Date;
...
.map(Date::toLocalDate)Tip
PlayingRepository.java에서는 이미 import java.sql.Date;로 깔끔하게 쓰고 계셔서, HomeService.java도 똑같이 맞추면 일관성이 좋아질 것 같아요.
rkdehdrbs7885-oss
left a comment
There was a problem hiding this comment.
날짜 타입 불일치가 나지 않도록 java.sql.Date 타입으로 안전하게 먼저 받은 뒤, 비즈니스 로직에서 명시적으로 LocalDate로 잘 변환하는 것 같습니다. 수고하셨습니다!
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
🔥 리뷰 요청 사항
✅ 체크리스트
📎 참고 사항
Summary by CodeRabbit