feat: 병천 날씨 조회 API 추가#2276
Open
dh2906 wants to merge 10 commits into
Open
Conversation
- 기상청 단기예보 API를 병천면 고정 좌표로 호출해 화면에 필요한 날씨 정보를 제공한다. - 응답을 기온과 날씨 상태로 제한해 클라이언트가 디자인에 필요한 값만 의존하도록 했다. - 발표시각 보정과 SKY/PTY 코드 매핑을 분리해 잘못된 예보시각이나 상태값 노출을 막는다. - 컨트롤러 인수 테스트와 날씨 도메인 단위 테스트로 기본 조회 흐름을 검증했다.
- 서버 인스턴스가 여러 대여도 같은 예보시각에는 Redis 캐시를 공유하도록 했다. - 캐시 키를 병천 고정 위치로 제한하고 1시간 TTL을 적용해 기상청 API 중복 호출을 줄인다. - 캐시된 예보시각이 현재 계산된 예보시각과 다를 때만 새 데이터를 가져오도록 했다. - Redis 캐시 적중과 미적중 흐름을 단위 테스트로 검증했다.
- 병천 날씨 조회 API가 Swagger UI의 Campus API 그룹에 노출되도록 패키지 스캔 범위에 포함했다. - API 구현과 캐시 동작은 유지하고 문서 탐색 가능성만 보완했다.
- 기상청 응답을 문자열로 먼저 받아 XML 에러나 비정상 JSON 응답의 원인을 메시지에 남기도록 했다. - 최신 발표시각 예보가 아직 없거나 예보 항목이 누락된 경우에만 직전 발표시각으로 한 번 재조회한다. - 인증키 오류 같은 재시도로 해결되지 않는 응답은 그대로 드러내 불필요한 중복 호출을 막는다. - 직전 발표시각 계산 테스트를 추가해 자정 경계 fallback을 검증했다.
- 기상청 API가 401/403 같은 HTTP 오류를 반환해도 응답 본문을 파싱해 실제 인증 오류 메시지를 남긴다. - 인증키 오류와 서비스 접근 권한 오류를 구분할 수 있게 해 운영 설정 확인 시간을 줄인다. - 기존 날씨 도메인 테스트를 다시 실행해 응답 처리 변경의 회귀를 확인했다.
- 사용자 조회 요청에서 기상청 API를 호출하지 않고 Redis 캐시만 읽도록 책임을 분리했다. - 매시 정각 스케줄러가 병천 날씨를 갱신해 캐시 miss가 사용자 요청 경로에서 외부 호출로 이어지지 않게 했다. - 스케줄 경계와 일시적인 갱신 실패를 버티도록 캐시 TTL을 조회 주기보다 길게 설정했다. - API 조회, 캐시 miss, 스케줄러 갱신 흐름을 테스트로 검증했다.
📝 WalkthroughWalkthroughAdds a weather feature: KMA short-term forecast client with retry and XML/JSON error handling, domain DTOs and enums, Redis caching plus hourly scheduler, a GET /weather REST endpoint with OpenAPI docs, and unit/acceptance tests. ChangesWeather Forecast Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/in/koreatech/koin/domain/weather/client/WeatherClient.java`:
- Line 157: The exception currently throws KoinIllegalStateException with the
full urlBuilder (which contains the serviceKey) causing potential API key
leakage; update the throw in WeatherClient to avoid including the full request
URI—either remove urlBuilder from the message or include only non-sensitive
parts (e.g., the path or a redacted URI that strips or masks the
serviceKey/query param), and ensure the thrown message still provides context
like "Failed to build short-term forecast API URL" while not logging the
sensitive serviceKey.
- Around line 191-193: The null-unsafe comparison of resultCode in WeatherClient
can throw NPE; update the conditional that currently uses resultCode.equals(...)
so it handles nulls (e.g., replace with a null-safe check such as if (resultCode
== null || (!"00".equals(resultCode) && !"0".equals(resultCode))) or use
Objects.equals/Objects.nonNull accordingly) so that missing resultCode still
triggers WeatherOpenApiException.withDetail; reference the resultCode variable
and the WeatherOpenApiException.withDetail call when making the change.
In `@src/main/java/in/koreatech/koin/domain/weather/model/WeatherCache.java`:
- Around line 9-21: The WeatherCache Redis entity is using the wrong Id
annotation: replace the jakarta.persistence.Id import with
org.springframework.data.annotation.Id so the `@RedisHash`("Weather") entity maps
correctly; update the import statement and ensure the private String id field in
class WeatherCache is annotated with org.springframework.data.annotation.Id
instead of jakarta.persistence.Id.
In
`@src/main/java/in/koreatech/koin/domain/weather/model/WeatherForecastRequestTime.java`:
- Around line 30-40: The calculation in
WeatherForecastRequestTime.from(LocalDateTime now) uses
now.toLocalTime().minusMinutes(10) which can wrap across midnight and detach
date context; instead compute a single date-aware timestamp (e.g.,
availableDateTime = now.minusMinutes(10)), then derive baseDate =
availableDateTime.toLocalDate() and availableTime =
availableDateTime.toLocalTime() before iterating BASE_TIMES to find baseTime;
keep the existing fallback that if baseTime == null you decrement baseDate and
pick the last BASE_TIMES entry so the date and time remain consistent across
midnight boundary.
In
`@src/main/java/in/koreatech/koin/domain/weather/scheduler/WeatherScheduler.java`:
- Around line 17-24: Add a startup invocation of the existing refresh logic so
the cache is populated immediately on app start: in WeatherScheduler, keep the
existing `@Scheduled`(cron = "0 0 * * * *") public void refreshWeather() as-is but
also call weatherService.refreshWeather() once during startup (for example from
a method annotated with `@EventListener`(ApplicationReadyEvent.class) or
`@PostConstruct`) and wrap it with the same try/catch/logging used in
refreshWeather() to surface errors; reference the WeatherScheduler class, the
refreshWeather() method, and the weatherService.refreshWeather() call when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73d11a6e-7730-4f67-ad1c-0fb9f8152068
📒 Files selected for processing (18)
src/main/java/in/koreatech/koin/domain/weather/client/WeatherClient.javasrc/main/java/in/koreatech/koin/domain/weather/controller/WeatherApi.javasrc/main/java/in/koreatech/koin/domain/weather/controller/WeatherController.javasrc/main/java/in/koreatech/koin/domain/weather/dto/WeatherApiResponse.javasrc/main/java/in/koreatech/koin/domain/weather/dto/WeatherResponse.javasrc/main/java/in/koreatech/koin/domain/weather/exception/WeatherOpenApiException.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherCache.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherCondition.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherForecast.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherForecastRequestTime.javasrc/main/java/in/koreatech/koin/domain/weather/repository/WeatherCacheRepository.javasrc/main/java/in/koreatech/koin/domain/weather/scheduler/WeatherScheduler.javasrc/main/java/in/koreatech/koin/domain/weather/service/WeatherService.javasrc/main/java/in/koreatech/koin/global/config/SwaggerGroupConfig.javasrc/test/java/in/koreatech/koin/acceptance/domain/WeatherApiTest.javasrc/test/java/in/koreatech/koin/unit/domain/weather/WeatherConditionTest.javasrc/test/java/in/koreatech/koin/unit/domain/weather/WeatherForecastRequestTimeTest.javasrc/test/java/in/koreatech/koin/unit/domain/weather/WeatherServiceTest.java
- 기상청 요청 URL 생성 실패 시 serviceKey가 포함된 URI를 남기지 않도록 민감 정보를 제거 - Redis 캐시 식별자와 기상청 resultCode 검증을 Spring Data 및 null-safe 방식으로 보정 - 자정 직후 발표시각 계산과 애플리케이션 기동 시 날씨 캐시 초기화를 보강
- 기상청 응답에서 TMP, SKY, PTY 중 일부가 누락되면 기본 날씨로 오인하지 않고 비정상 응답으로 처리 - 예보시각 데이터가 아예 없는 경우에만 직전 발표시각 재조회가 동작하도록 실패 원인을 분리 - WeatherClient 단위 테스트로 정상 매핑, 직전 발표시각 fallback, 필수 항목 누락 경로를 고정
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/in/koreatech/koin/unit/domain/weather/WeatherClientTest.java (1)
89-92: ⚡ Quick winAssert the missing-category detail, not only the default exception message.
Right now this can pass even if a different failure path throws the same default message. Please also assert the detail (e.g.,
"missing category: SKY") to pin this test to the intended contract on Line 89.Proposed test tightening
- assertThatThrownBy(() -> weatherClient.getWeatherForecast(REQUEST_TIME)) - .isInstanceOf(WeatherOpenApiException.class) - .hasMessage("기상청 단기예보 API 응답이 정상적이지 않습니다."); + assertThatThrownBy(() -> weatherClient.getWeatherForecast(REQUEST_TIME)) + .isInstanceOf(WeatherOpenApiException.class) + .hasMessage("기상청 단기예보 API 응답이 정상적이지 않습니다.") + .satisfies(ex -> assertThat(((WeatherOpenApiException) ex).getFullMessage()) + .contains("missing category: SKY"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/in/koreatech/koin/unit/domain/weather/WeatherClientTest.java` around lines 89 - 92, The test currently only asserts the default exception message for weatherClient.getWeatherForecast but not the specific error detail; update the assertion for WeatherOpenApiException (thrown by weatherClient.getWeatherForecast(REQUEST_TIME)) to also assert the exception's detail field (e.g., call getDetail() on the caught WeatherOpenApiException) equals the expected string "missing category: SKY", so the test pins the failure path to the intended contract rather than any exception with the same default message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/test/java/in/koreatech/koin/unit/domain/weather/WeatherClientTest.java`:
- Around line 89-92: The test currently only asserts the default exception
message for weatherClient.getWeatherForecast but not the specific error detail;
update the assertion for WeatherOpenApiException (thrown by
weatherClient.getWeatherForecast(REQUEST_TIME)) to also assert the exception's
detail field (e.g., call getDetail() on the caught WeatherOpenApiException)
equals the expected string "missing category: SKY", so the test pins the failure
path to the intended contract rather than any exception with the same default
message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3846ec1-fac7-4aaa-9aa8-659ba2b583f8
📒 Files selected for processing (6)
src/main/java/in/koreatech/koin/domain/weather/client/WeatherClient.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherCache.javasrc/main/java/in/koreatech/koin/domain/weather/model/WeatherForecastRequestTime.javasrc/main/java/in/koreatech/koin/domain/weather/scheduler/WeatherScheduler.javasrc/test/java/in/koreatech/koin/unit/domain/weather/WeatherClientTest.javasrc/test/java/in/koreatech/koin/unit/domain/weather/WeatherForecastRequestTimeTest.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/in/koreatech/koin/domain/weather/scheduler/WeatherScheduler.java
- src/main/java/in/koreatech/koin/domain/weather/model/WeatherCache.java
- src/main/java/in/koreatech/koin/domain/weather/model/WeatherForecastRequestTime.java
- src/test/java/in/koreatech/koin/unit/domain/weather/WeatherForecastRequestTimeTest.java
- src/main/java/in/koreatech/koin/domain/weather/client/WeatherClient.java
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.
🔍 개요
/weather에서 제공할 수 있게 했습니다.🚀 주요 변경 내용
GET /weather응답을 기온과 날씨 상태 두 값으로 구성했습니다.nx=66,ny=109)로 호출하고,TMP,SKY,PTY값을 화면용 응답으로 매핑했습니다.02/05/08/11/14/17/20/23시)과 제공 지연을 고려해 조회 기준 시간을 계산하고, 데이터 미제공 시 직전 발표시각으로 한 번 보정합니다.💬 참고 사항
./gradlew test --tests 'in.koreatech.koin.unit.domain.weather.*' --tests 'in.koreatech.koin.acceptance.domain.WeatherApiTest' --no-daemon✅ Checklist (완료 조건)
Summary by CodeRabbit
New Features
Documentation
Tests