Repository navigation
[feat] #225 - 공휴일 공공데이터 Open API 연동 - #230
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Walkthrough공휴일 API 클라이언트와 API 키 설정을 추가했습니다. HolidayChecker는 연도별 공휴일 데이터를 비동기로 조회하고 캐시합니다. 일요일 판정은 유지하며, 한국어 날짜는 API 조회 결과로 공휴일 여부를 판정합니다. Changes공휴일 API 연동
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant HolidayChecker
participant holidayExecutor
participant HolidayApiClient
participant API as 공휴일 API
HolidayChecker->>holidayExecutor: 연도별 조회 작업 제출
holidayExecutor->>HolidayApiClient: fetchHolidays(year)
HolidayApiClient->>API: 연도와 API 키로 요청
API-->>HolidayApiClient: 공휴일 응답
HolidayApiClient-->>holidayExecutor: 공휴일 날짜 집합
holidayExecutor-->>HolidayChecker: 조회 결과 저장
Suggested reviewers: Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
#225의 API 키 설정, 공공데이터 클라이언트, 연도별 캐시, 한국어/영어 분기, API 호출 및 홈 표시 연동은 구현되어 있습니다. 그러나 API 키가 없거나 조회가 실패하거나 빈 결과를 받으면 HolidayChecker.isHoliday는 일요일 외 날짜를 공휴일로 판정하지 않습니다. 현재 코드에는 기존 양력 고정 공휴일 폴백이 없습니다. 이는 … |
API를 사용할 수 없는 경우, 조회 실패·빈 결과·지원 범위 밖 연도에서 기존 양력 고정 공휴일을 사용하도록 HolidayChecker의 폴백을 복원하십시오. API 성공 결과와 폴백 결과를 검증하는 자동 테스트도 추가하십시오. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 3 files. (2 skipped: 2… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Out of Scope Changes check HolidayApiClient,HolidayChecker,holidayExecutor, 환경 변수 설정은#225의API 연동, 캐시, 폴백, 언어별 판정 목표를 지원합니다. 확인 가능한 변경에 unrelated 기능은 없습니다.Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check 제목은 공휴일 공공데이터 Open API 연동이라는 PR의 핵심 변경을 정확히 설명하며, 간결하고 구체적입니다.
Full details: Linked Issues check
-
Explanation
#225의API 키 설정, 공공데이터 클라이언트, 연도별 캐시, 한국어/영어 분기, API 호출 및 홈 표시 연동은 구현되어 있습니다. 그러나 API 키가 없거나 조회가 실패하거나 빈 결과를 받으면HolidayChecker.isHoliday는 일요일 외 날짜를 공휴일로 판정하지 않습니다. 현재 코드에는 기존 양력 고정 공휴일 폴백이 없습니다. 이는 PR 설명과#225의외부 API 장애 폴백 요구를 충족하지 않습니다.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 3 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Jy000n
left a comment
Jy000n
left a comment
There was a problem hiding this comment.
고생하셨습니다! API 키를 가져와서 활용하는 건 처음이라 코드 이해하는데 시간이 좀 걸렸네요,,
로그 레벨을 디버그로 올리는 것에 대한 부분에서 단점으로 인해 현재처럼 운영 가이드로 두는 것 좋은 것 같습니다! pr에서는 반영하지 않고 실제로 debug를 켜야할 상황이 생길 때 노션을 참고하면 될 것 같네용 :)
There was a problem hiding this comment.
아 그게 GlobalExceptionHandler, MdcLoggingFilter, AI 캐시나 Gemini 쪽 로그가 대부분 영어로 남고 있어서 같은 형식으로 맞추려고 영어로 바꿨어용
| private static final ZoneId KOREA_ZONE = ZoneId.of("Asia/Seoul"); | ||
| private static final int MIN_SUPPORTED_YEAR = 2004; | ||
| private static final int MAX_FUTURE_YEARS = 2; | ||
| private static final Duration LOAD_WAIT = Duration.ofSeconds(2); |
There was a problem hiding this comment.
정상 응답이 0.2초라면 1초여도 첫 조회 대부분이 반영될 것 같은데 2초로 정하신 기준이 있으실까요?? 선로딩 덕분에 사용자가 해당 대기를 만날 일은 드물어서 문제는 아니라고 생각하기는 합니다..!! 그냥 궁금해서 ..
There was a problem hiding this comment.
2초는 항상 기다리는 시간이 아니라 최대로 기다리는 시간이라서 정상 응답이면 0.2초 정도 만에 바로 반환돼요! 2초로 정한 건 공휴일 API에 문제가 생겨도 홈 화면이 보이기까지 2초 넘게 걸리면 안 된다고 생각해서 그 안에서 끊도록 잡았어용 2초가 지나서 처음에는 공휴일이 안 보이더라도 불러오기는 백그라운드에서 계속 진행되고 있어 API 응답이 오면 캐싱되어 새로고침하면 바로 보이게 됩니다
| private static final int MIN_SUPPORTED_YEAR = 2004; | ||
| private static final int MAX_FUTURE_YEARS = 2; | ||
| private static final Duration LOAD_WAIT = Duration.ofSeconds(2); | ||
| private static final Duration RETRY_DELAY = Duration.ofMinutes(10); |
There was a problem hiding this comment.
차단 구간에서는 요청이 대기 없이 바로 응답하고 간격이 지난 뒤 첫 요청 하나만 최대 2초를 기다리는 구조라, 간격을 줄여도 영향은 작아보였는데 이것도 위 리뷰와 마찬가지로 10분으로 정하신 기준이 궁금해서 여쭤봅니다!!
There was a problem hiding this comment.
이것도 정확한 기준이 있다기보다는 API 장애 중에 외부 API로 요청이 계속 가지 않을 정도의 간격이면서 장애가 풀렸을 때 너무 늦지 않게 다시 반영될 정도로 잡은 값이에용 일단 아직 적정한 시간을 추정하기가 어려워 10분으로 두었습니다! 말씀해주신 것처럼 간격이 지난 뒤 첫 요청 하나만 기다리는 구조라 간격을 줄여도 영향은 크지 않아서 더 짧게 가져가도 괜찮을 것 같긴해용
| if (holidays.isEmpty()) { | ||
| failedAtByYear.put(year, Instant.now()); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
P5) 정상 응답이면서 빈 목록인 경우일 때 해당 상황이 발생하는 것으로 이해했습니다. 이 경우 null로만 반환하면 왜 공휴일이 보이지 않는지 확인할 수가 없는데 null로만 반환하신 이유가 따로 있으실까요?? 미발표 연도는 정상적인 상황이라 log.info()정도로 year를 포함해서 같이 남겨줘도 좋을 것 같습니다..!
There was a problem hiding this comment.
넹 그러네용 왜 공휴일이 안 보이는지 확인하기 어렵겠네용 말씀해주신 대로 year를 포함해서 log.info로 남기도록 반영했습니다 감사합니당!
laura-jung
left a comment
laura-jung
left a comment
There was a problem hiding this comment.
자윤이가 코멘트 잘 남겨줘서 코멘트 할게 별로 없네용
리뷰가 늦어져서 죄송합니다.
전체적으로 깔끔하게 잘 연결된 것 같아용 수고하셨습니다. 다만 api url을 환경변수가 아니라 그냥 코드에 노출시킨 이유가 따로 있을까요? 물론 지금 보안상 문제를 없어보이는데 궁금해서 질문드립니다!
| return false; | ||
| } | ||
|
|
||
| Set<LocalDate> koreanHolidays = findKoreanHolidays(date.getYear()); |
There was a problem hiding this comment.
P4) 실행기 큐가 가득 차면 홈 API가 500을 반환할 수 있을 것 같네요!
supplyAsync가 RejectedExecutionException을 던지면 computeIfAbsent를 거쳐 isHoliday, 이어서 홈 API까지 그대로 올라가게 되는데 그렇다면 "공휴일을 불러오지 못해도 홈 조회는 실패하지 않는다"는 원칙이 꺠질 수 있을 것 같아요.
지금은 동시에 불러오는 연도가 최대 약 25개(2004년~올해+2년)이고 실행기가 스레드 2개 + 큐 30이라 실제로 생기지는 않을 것 같지만 연도가 매년 늘어나니 startLoading 호출을 try/catch로 감싸고 실패하면 null을 반환하게 해두면 더 안전할 것 같습니다!
There was a problem hiding this comment.
오 넵! 이야기 해주신대로 holidayExecutor는 기본 거절 정책이라 큐가 가득 차면 RejectedExecutionException이 홈 API까지 올라갈 수 있어서 startLoading에서 예외를 잡아 경고 로그를 남기고 null을 반환하도록 하고 findKoreanHolidays에서는 null이면 바로 공휴일 없음으로 처리하도록 반영했습니다. 282798c
| private JsonNode readTree(String response) { | ||
| try { | ||
| return objectMapper.readTree(response); | ||
| } catch (JsonProcessingException e) { | ||
| throw new IllegalStateException("Failed to parse holiday API response."); | ||
| } | ||
| } |
There was a problem hiding this comment.
P5) 현재 응답 본문이 비어서 response가 null이면 readTree(null)이 JsonProcessingException이 아니라 IllegalArgumentException을 던지는 것 같아요. loadHolidays의 catch (Exception)에서 잡히고 키도 노출되지 않아서 동작에는 문제가 없지만 다른 실패와 같은 고정 메시지로 맞추기 위해 null일 때를 따로 확인하는 것을 추가하는게 어떨까요?
There was a problem hiding this comment.
말씀해주신 대로 응답 본문이 비어 있으면 readTree(null)에서 IllegalArgumentException이 나서 extractHolidays 앞에서 null이면 다른 실패와 같은 형식의 고정 메시지로 IllegalStateException을 던지도록 반영했습니다. 3955758
|
@laura-jung 공공데이터포털 URL은 누구나 볼 수 있는 고정 주소이고 환경(local/prod)마다 바뀔 일도 없어서 서비스 키만 나중에 테스트용 가짜 서버가 필요해지면 |
관련 이슈 🛠
작업 내용 요약 ✏️
HolidayChecker는 양력 고정 공휴일 8개만 하드코딩되어 있어 설날·추석 같은 음력 공휴일과 대체공휴일·임시공휴일이 표시되지 않았습니다.GET /api/v1/home)의isHoliday로만 내려가며 필드와 요청·응답 형식은 그대로이고true로 판단되는 날짜만 달라집니다. 그래서 클라이언트 변경은 없고 API 명세서도 따로 수정하지 않았습니다.KOEN주요 변경 사항 🛠️
HolidayApiClient추가getRestDeInfo를 연도 단위로 호출해isHoliday = Y인 날짜만 반환item이 배열/단일 객체/없음으로 오는 경우를 모두 처리하고,resultCode가00이 아니면 예외 처리HolidayChecker를 API 기반으로 변경ConcurrentHashMap에 캐싱 (1년치가 20개 내외라 메모리 부담은 거의 없음)AsyncConfig에 공휴일 조회용holidayExecutor추가 (스레드 2개,MdcTaskDecorator적용)application-local.yml,application-prod.yml에holiday.api-key: ${HOLIDAY_API_KEY:}추가트러블 슈팅 ⚽️
HomeService는 클래스 단위@Transactional(readOnly = true)라서 홈 조회 중 외부 API를 오래 기다리면 그동안 DB 커넥션을 점유합니다. 그래서 자주 조회되는 작년·올해·내년은 서버 시작 시 미리 불러오고 그 외 연도도 최대 2초까지만 기다리도록 했습니다.테스트 결과 📄
./gradlew build성공isHoliday: true스크린샷 📷
리뷰 요구사항 📢
배포 서버 환경변수에
HOLIDAY_API_KEY를 추가했습니다. 노션에도 공유했습니다.캐싱은 서버 메모리 방식이라 서버 재시작 시 다시 불러옵니다.
공휴일 API 호출 관련 시간 설정이 적절한지 의견 부탁드립니다. (설정 이유는 트러블 슈팅 참고 부탁드립니다.)
오늘 탭(
GET /api/v1/home/today)은 응답에 공휴일 여부(isHoliday) 필드가 없어서 공휴일이어도 날짜가 빨간색으로 표시되지 않습니다.공휴일 API(공공데이터포털 천문연구원 특일정보)는 서비스키를 URL 쿼리 파라미터(
ServiceKey)로만 받습니다.공식 문서에도 헤더 인증 방식은 안내되어 있지 않음 그래서 키가 URL에 포함될 수밖에 없어 로그로 키가 노출되지 않도록 아래처럼 처리했습니다.
HOLIDAY_API_KEY)로만 관리하고 서버에서만 호출로그 레벨을 DEBUG로 올리면 JDK
HttpURLConnection이 요청 URL 전체를 찍어서 키가 로그에 남습니다. (로컬에서 확인)최대한 보안 이슈를 신경 써서 처리했지만 놓친 노출 경로가 있는지 한 번 더 확인 부탁드립니다!
📎 참고 자료 (선택)
Summary by CodeRabbit