Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import java.net.URL;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.ResponseEntity;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.stereotype.Component;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.client.RestTemplate;
Expand All @@ -21,6 +26,7 @@
import in.koreatech.koin.domain.weather.client.dto.WeatherApiResponse.WeatherForecastItem;
import in.koreatech.koin.domain.weather.client.dto.WeatherForecastRequestTime;
import in.koreatech.koin.domain.weather.exception.WeatherOpenApiException;
import in.koreatech.koin.domain.weather.exception.WeatherOpenApiRateLimitException;
import in.koreatech.koin.domain.weather.model.WeatherForecast;
import in.koreatech.koin.global.exception.custom.KoinIllegalStateException;

Expand All @@ -33,49 +39,75 @@ public class WeatherClient {
private static final int BYEONGCHEON_NX = 66;
private static final int BYEONGCHEON_NY = 109;
private static final int ROW_COUNT = 1000;
private static final int FORECAST_RANGE_HOURS = 24;
private static final DateTimeFormatter FORECAST_DATE_TIME_FORMATTER =
DateTimeFormatter.ofPattern("yyyyMMddHHmm");
Comment on lines +42 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Cover the TTL’s final partial hour in the cached forecast window.

With [start, start + 24h) filtering and a 24h Redis TTL, a cache saved at e.g. 03:35 contains slots through 02:00 next day, but remains alive until 03:35; WeatherService.getWeather() then throws for 03:00 because that key is missing. Include the boundary hour or expire the cache at the forecast window boundary.

Proposed fix
-    private static final int FORECAST_RANGE_HOURS = 24;
+    private static final int FORECAST_RANGE_HOURS = 25;
-            // 달력 날짜가 아닌 갱신 시각을 기준으로 향후 24시간의 예보만 캐시에 저장한다.
+            // 달력 날짜가 아닌 갱신 시각을 기준으로 캐시 TTL 마지막 시간대까지 예보를 저장한다.

Also applies to: 82-89

🤖 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/main/java/in/koreatech/koin/domain/weather/client/WeatherClient.java`
around lines 42 - 44, The cached forecast window in WeatherClient is ending too
early relative to the Redis TTL, so WeatherService.getWeather() can request an
hour that is still within the cache lifetime but missing from the stored data.
Update the forecast-window logic in WeatherClient to include the final boundary
hour or, alternatively, make the cache expiration align with the exact forecast
boundary instead of a fixed 24h TTL. Check the filtering/range construction
around FORECAST_RANGE_HOURS and the cache save/expiration path so the last
partial hour is always covered.


private final String openApiKey;
private final RestTemplate restTemplate;
private final ObjectMapper objectMapper;
private final RetryTemplate retryTemplate;

public WeatherClient(
@Value("${OPEN_API_KEY_PUBLIC}") String openApiKey,
RestTemplate restTemplate,
ObjectMapper objectMapper
ObjectMapper objectMapper,
@Qualifier("weatherRetryTemplate") RetryTemplate retryTemplate
) {
this.openApiKey = openApiKey;
this.restTemplate = restTemplate;
this.objectMapper = objectMapper;
this.retryTemplate = retryTemplate;
}

public WeatherForecast getWeatherForecast(WeatherForecastRequestTime requestTime) {
public Map<String, WeatherForecast> getWeatherForecasts(WeatherForecastRequestTime requestTime) {
return retryTemplate.execute(context -> getWeatherForecastsWithFallback(requestTime));
}

private Map<String, WeatherForecast> getWeatherForecastsWithFallback(WeatherForecastRequestTime requestTime) {
try {
return getWeatherForecastWithoutFallback(requestTime);
return getWeatherForecastsWithoutFallback(requestTime);
} catch (WeatherOpenApiException e) {
if (!canRetryWithPreviousBaseTime(e)) {
throw e;
}
return getWeatherForecastWithoutFallback(requestTime.previousBaseTime());
return getWeatherForecastsWithoutFallback(requestTime.previousBaseTime());
}
}

private WeatherForecast getWeatherForecastWithoutFallback(WeatherForecastRequestTime requestTime) {
private Map<String, WeatherForecast> getWeatherForecastsWithoutFallback(WeatherForecastRequestTime requestTime) {
WeatherApiResponse response = getOpenApiResponse(requestTime);
List<WeatherForecastItem> items = extractForecastItems(response);
Map<String, String> forecasts = items.stream()
.filter(item -> requestTime.forecastDate().equals(item.fcstDate()))
.filter(item -> requestTime.forecastTime().equals(item.fcstTime()))
.collect(Collectors.toMap(
WeatherForecastItem::category,
WeatherForecastItem::fcstValue,
(previous, current) -> current
String forecastStartDateTime = forecastDateTime(requestTime);
String forecastEndDateTime = LocalDateTime.parse(forecastStartDateTime, FORECAST_DATE_TIME_FORMATTER)
.plusHours(FORECAST_RANGE_HOURS)
.format(FORECAST_DATE_TIME_FORMATTER);
// 한 번의 API 응답에 포함된 예보를 시간대별로 묶어 Redis에서 현재 시각의 예보를 선택할 수 있게 한다.
Map<String, Map<String, String>> forecastsByDateTime = items.stream()
// 달력 날짜가 아닌 갱신 시각을 기준으로 향후 24시간의 예보만 캐시에 저장한다.
.filter(item -> isWithinForecastRange(item, forecastStartDateTime, forecastEndDateTime))
.collect(Collectors.groupingBy(
item -> item.fcstDate() + item.fcstTime(),
Collectors.toMap(
WeatherForecastItem::category,
WeatherForecastItem::fcstValue,
(previous, current) -> current
)
));

if (forecasts.isEmpty()) {
if (!forecastsByDateTime.containsKey(forecastDateTime(requestTime))) {
throw WeatherOpenApiException.withDetail("forecastDateTime: "
+ requestTime.forecastDate() + requestTime.forecastTime());
}

return forecastsByDateTime.entrySet().stream()
.collect(Collectors.toMap(
Map.Entry::getKey,
entry -> toWeatherForecast(entry.getValue())
));
}

private WeatherForecast toWeatherForecast(Map<String, String> forecasts) {
String temperature = requireForecastValue(forecasts, "TMP");
String sky = requireForecastValue(forecasts, "SKY");
String precipitationType = requireForecastValue(forecasts, "PTY");
Expand All @@ -93,6 +125,20 @@ private WeatherForecast getWeatherForecastWithoutFallback(WeatherForecastRequest
}
}

private String forecastDateTime(WeatherForecastRequestTime requestTime) {
return requestTime.forecastDate() + requestTime.forecastTime();
}

private boolean isWithinForecastRange(
WeatherForecastItem item,
String forecastStartDateTime,
String forecastEndDateTime
) {
String itemForecastDateTime = item.fcstDate() + item.fcstTime();
return itemForecastDateTime.compareTo(forecastStartDateTime) >= 0
&& itemForecastDateTime.compareTo(forecastEndDateTime) < 0;
}

private String requireForecastValue(Map<String, String> forecasts, String category) {
String forecastValue = forecasts.get(category);
if (forecastValue == null || forecastValue.isBlank()) {
Expand Down Expand Up @@ -127,6 +173,9 @@ private WeatherApiResponse getOpenApiResponse(WeatherForecastRequestTime request
} catch (WeatherOpenApiException e) {
throw e;
} catch (HttpStatusCodeException e) {
if (e.getStatusCode().value() == 429) {
throw rateLimitException(requestTime, "httpStatus: " + e.getStatusCode());
}
String responseBody = e.getResponseBodyAsString();
if (responseBody != null && !responseBody.isBlank()) {
return parseResponse(responseBody, requestTime);
Expand All @@ -145,6 +194,10 @@ private WeatherApiResponse parseResponse(String responseBody, WeatherForecastReq
+ requestTime.baseDate() + requestTime.baseTime() + ", response body is empty");
}

if (isRateLimitResponse(responseBody)) {
throw rateLimitException(requestTime, responseBody.trim());
}

if (!responseBody.trim().startsWith("{")) {
throw WeatherOpenApiException.withDetail("baseDateTime: "
+ requestTime.baseDate() + requestTime.baseTime() + ", " + extractXmlErrorMessage(responseBody));
Expand All @@ -158,6 +211,20 @@ private WeatherApiResponse parseResponse(String responseBody, WeatherForecastReq
}
}

private boolean isRateLimitResponse(String responseBody) {
String normalizedResponse = responseBody.toUpperCase(Locale.ROOT);
return normalizedResponse.contains("API RATE LIMIT EXCEEDED")
|| normalizedResponse.contains("LIMITED_NUMBER_OF_SERVICE_REQUESTS_PER_SECOND_EXCEEDS_ERROR");
}

private WeatherOpenApiRateLimitException rateLimitException(
WeatherForecastRequestTime requestTime,
String cause
) {
return WeatherOpenApiRateLimitException.withDetail("baseDateTime: "
+ requestTime.baseDate() + requestTime.baseTime() + ", cause: " + cause);
}

private String getRequestURL(WeatherForecastRequestTime requestTime) {
StringBuilder urlBuilder = new StringBuilder(OPEN_API_URL);
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package in.koreatech.koin.domain.weather.config;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.retry.support.RetryTemplate;

import in.koreatech.koin.domain.weather.exception.WeatherOpenApiRateLimitException;

@Configuration
public class WeatherRetryConfig {

private static final int MAX_ATTEMPTS = 6;
private static final long RETRY_INTERVAL_MILLIS = 5_000L;

@Bean
public RetryTemplate weatherRetryTemplate() {
// 최초 호출 이후 5회 재시도해 기상청 제공 서버의 순간적인 동시 호출 제한을 흡수한다.
return RetryTemplate.builder()
.maxAttempts(MAX_ATTEMPTS)
.fixedBackoff(RETRY_INTERVAL_MILLIS)
.retryOn(WeatherOpenApiRateLimitException.class)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

public class WeatherOpenApiException extends ExternalServiceException {

private static final String DEFAULT_MESSAGE = "기상청 단기예보 API 응답이 정상적이지 않습니다.";
protected static final String DEFAULT_MESSAGE = "기상청 단기예보 API 응답이 정상적이지 않습니다.";

public WeatherOpenApiException(String message) {
super(message);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package in.koreatech.koin.domain.weather.exception;

public class WeatherOpenApiRateLimitException extends WeatherOpenApiException {

private WeatherOpenApiRateLimitException(String detail) {
super(DEFAULT_MESSAGE, detail);
}

public static WeatherOpenApiRateLimitException withDetail(String detail) {
return new WeatherOpenApiRateLimitException(detail);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package in.koreatech.koin.domain.weather.model;

import java.util.Map;
import java.util.concurrent.TimeUnit;

import org.springframework.data.annotation.Id;
Expand All @@ -15,31 +16,31 @@
public class WeatherCache {

public static final String BYEONGCHEON_ID = "byeongcheon";
private static final long CACHE_EXPIRE_HOUR = 2L;
private static final long CACHE_EXPIRE_HOUR = 24L;

@Id
private String id;

private WeatherResponse weather;
private Map<String, WeatherResponse> hourlyWeathers;

@TimeToLive(unit = TimeUnit.HOURS)
private final Long expiration;

@Builder
private WeatherCache(
String id,
WeatherResponse weather,
Map<String, WeatherResponse> hourlyWeathers,
Long expiration
) {
this.id = id;
this.weather = weather;
this.hourlyWeathers = hourlyWeathers;
this.expiration = expiration == null ? CACHE_EXPIRE_HOUR : expiration;
}

public static WeatherCache of(WeatherResponse weather) {
public static WeatherCache of(Map<String, WeatherResponse> hourlyWeathers) {
return WeatherCache.builder()
.id(BYEONGCHEON_ID)
.weather(weather)
.hourlyWeathers(hourlyWeathers)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import java.time.Clock;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import org.springframework.stereotype.Service;

Expand All @@ -17,20 +21,37 @@
@RequiredArgsConstructor
public class WeatherService {

private static final DateTimeFormatter FORECAST_DATE_TIME_FORMATTER =
DateTimeFormatter.ofPattern("yyyyMMddHHmm");

private final Clock clock;
private final WeatherClient weatherClient;
private final WeatherCacheRepository weatherCacheRepository;

public WeatherResponse getWeather() {
String forecastDateTime = LocalDateTime.now(clock)
.withMinute(0)
.withSecond(0)
.withNano(0)
.format(FORECAST_DATE_TIME_FORMATTER);

return weatherCacheRepository.findById(WeatherCache.BYEONGCHEON_ID)
.map(WeatherCache::getWeather)
.orElseThrow(() -> WeatherOpenApiException.withDetail("weather cache is empty"));
.flatMap(cache -> Optional.ofNullable(cache.getHourlyWeathers()))
.map(hourlyWeathers -> hourlyWeathers.get(forecastDateTime))
.orElseThrow(() -> WeatherOpenApiException.withDetail(
"weather cache is empty, forecastDateTime: " + forecastDateTime
));
}

public synchronized void refreshWeather() {
LocalDateTime now = LocalDateTime.now(clock);
WeatherForecastRequestTime requestTime = WeatherForecastRequestTime.from(now);
WeatherResponse response = weatherClient.getWeatherForecast(requestTime).toResponse();
weatherCacheRepository.save(WeatherCache.of(response));
// 외부 API의 시간대별 도메인 예보를 조회 API가 바로 사용할 수 있는 응답 Map으로 한 번에 변환한다.
Map<String, WeatherResponse> hourlyWeathers = weatherClient.getWeatherForecasts(requestTime).entrySet().stream()
.collect(Collectors.toMap(
Map.Entry::getKey,
entry -> entry.getValue().toResponse()
));
weatherCacheRepository.save(WeatherCache.of(hourlyWeathers));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import java.util.Map;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.MediaType;
Expand All @@ -21,9 +23,9 @@ class WeatherApiTest extends AcceptanceTest {
@Test
void 병천_날씨를_조회한다() throws Exception {
clear();
weatherCacheRepository.save(WeatherCache.of(
new WeatherResponse(21, "맑음")
));
weatherCacheRepository.save(WeatherCache.of(Map.of(
"202401151200", new WeatherResponse(21, "맑음")
)));

mockMvc.perform(
get("/weather")
Expand Down
Loading
Loading