[feat] : '전체 공개이지만, 등록된 수업이 없는 경우' 예외 처리를 추가한다 #301
Conversation
bbbang105
added
🚀 feat
WalkthroughXML의 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Controller as 컨트롤러
participant Service as FixedScheduleService
participant XML as 원본 XML
Client->>Controller: 에브리타임 시간표 조회 요청
Controller->>Service: 시간표 조회 요청 전달
Service->>XML: XML 컨텐츠 수신/읽기
XML-->>Service: XML 반환
alt XML 존재 및 읽기 성공
Service->>Service: extractStatusFromXml(xml)
alt status == EVERYTIME_PRIVATE_STATUS (-2)
Service-->>Controller: EVERYTIME_TIMETABLE_NOT_PUBLIC 예외 발생
Controller-->>Client: 404 FIXED-002
else status == EVERYTIME_PUBLIC_STATUS (1) && 수업 없음
Service-->>Controller: NOT_FOUND_EVERYTIME_TIMETABLE 예외 발생
Controller-->>Client: 404 FIXED-005
else status == EVERYTIME_PUBLIC_STATUS (1) && 수업 있음
Service-->>Controller: 변환된 시간표 반환
Controller-->>Client: 200 OK
end
else XML 파싱/추출 실패
Service-->>Controller: EVERYTIME_TIMETABLE_PARSE_ERROR 예외 발생
Controller-->>Client: 파싱 오류 응답
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20분
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
bbbang105
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/side/onetime/exception/status/FixedErrorStatus.java(1 hunks)src/main/java/side/onetime/service/FixedScheduleService.java(3 hunks)src/test/java/side/onetime/fixed/FixedControllerTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/side/onetime/service/FixedScheduleService.java (3)
src/main/java/side/onetime/exception/status/ScheduleErrorStatus.java (2)
Override(21-28)Getter(9-39)src/main/java/side/onetime/global/common/status/ErrorStatus.java (1)
Override(36-44)src/main/java/side/onetime/exception/status/EventErrorStatus.java (1)
Override(32-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Push to ECR
🔇 Additional comments (5)
src/test/java/side/onetime/fixed/FixedControllerTest.java (2)
216-222: LGTM! 테스트 케이스가 리팩토링된 에러 처리와 잘 정렬되어 있습니다.DisplayName이 실패 사유를 명확히 표현하고 있으며, 예외 타입이
_EVERYTIME_TIMETABLE_NOT_PUBLIC으로 변경되어 의미가 더 명확해졌습니다.
246-275: LGTM! 빈 시간표 시나리오에 대한 테스트 커버리지가 추가되었습니다.새로운 테스트 케이스가 등록된 수업이 없는 경우를 적절히 검증하고 있으며, 기존 실패 테스트들과 일관된 구조를 유지하고 있습니다.
src/main/java/side/onetime/service/FixedScheduleService.java (2)
9-10: LGTM! 매직 넘버 제거로 코드 가독성이 향상되었습니다.정규식 관련 import와 상태 코드 상수화가 적절하며, 상수명도 의미를 명확히 전달하고 있습니다.
Also applies to: 41-42
155-175: LGTM! status 추출 로직이 적절하게 구현되었습니다.정규식 패턴이 음수를 포함한 숫자 형태의 status 값을 정확히 매칭하며, 파싱 실패 시 적절한 예외 처리가 되어 있습니다.
src/main/java/side/onetime/exception/status/FixedErrorStatus.java (1)
3-3: LGTM! 에러 상태 정의가 명확하게 개선되었습니다.enum 상수명 변경으로 의미가 더 명확해졌고(
_EVERYTIME_TIMETABLE_NOT_PUBLIC), 새로운 상수(_NOT_FOUND_EVERYTIME_TIMETABLE, FIXED-005)가 추가되어 빈 시간표 케이스를 적절히 구분하고 있습니다. import 정리도 잘 되었습니다.Also applies to: 14-14, 17-17
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/side/onetime/service/FixedScheduleService.java (1)
158-178: Jsoup을 사용한 일관된 파싱 방식을 고려해보세요.현재 정규식 기반 접근은 정상적으로 동작하지만, 이 파일의 다른 부분(line 187)에서는 Jsoup을 사용하여 XML을 파싱하고 있습니다. status 속성 추출도 Jsoup으로 통일하면 다음과 같은 이점이 있습니다:
- XML 파싱 방식의 일관성 확보
- 속성값 따옴표 유형(single/double)에 대한 견고성 향상 (현재 패턴은 double quote만 매칭)
- XML 형식 변경에 대한 내구성 향상
Jsoup을 사용한 대체 구현:
private int extractStatusFromXml(String xml) { - // status="숫자" 패턴을 찾음 - Pattern pattern = Pattern.compile("status=\"(-?\\d+)\""); - Matcher matcher = pattern.matcher(xml); - - if (matcher.find()) { - try { - return Integer.parseInt(matcher.group(1)); - } catch (NumberFormatException e) { - // 숫자가 아닌 경우 파싱 에러 처리 - throw new CustomException(FixedErrorStatus._EVERYTIME_TIMETABLE_PARSE_ERROR); - } - } - - // status 속성을 찾지 못한 경우 파싱 에러 처리 - throw new CustomException(FixedErrorStatus._EVERYTIME_TIMETABLE_PARSE_ERROR); + try { + Document doc = Jsoup.parse(xml, "", Parser.xmlParser()); + Element table = doc.selectFirst("table"); + if (table == null || !table.hasAttr("status")) { + throw new CustomException(FixedErrorStatus._EVERYTIME_TIMETABLE_PARSE_ERROR); + } + return Integer.parseInt(table.attr("status")); + } catch (NumberFormatException e) { + throw new CustomException(FixedErrorStatus._EVERYTIME_TIMETABLE_PARSE_ERROR); + } }참고: 현재 정규식 패턴은 작은따옴표(single quote)로 감싼 속성값은 매칭하지 못하지만, Everytime API가 항상 큰따옴표를 사용한다면 문제없습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/side/onetime/service/FixedScheduleService.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to Server
🔇 Additional comments (3)
src/main/java/side/onetime/service/FixedScheduleService.java (3)
9-10: LGTM!정규식 기반 status 추출을 위한 표준 Java import입니다.
41-42: LGTM!매직 넘버를 상수화하여 코드 가독성이 향상되었습니다. 네이밍과 접근 제어자 설정도 적절합니다.
140-152: LGTM!이전 리뷰에서 지적된 예상치 못한 status 값 처리가 올바르게 추가되었습니다. 세 가지 케이스(비공개, 공개이지만 빈 시간표, 파싱 오류)를 명확히 구분하고 있습니다.
| */ | ||
| private int extractStatusFromXml(String xml) { | ||
| // status="숫자" 패턴을 찾음 | ||
| Pattern pattern = Pattern.compile("status=\"(-?\\d+)\""); |
There was a problem hiding this comment.
compile메소드 내부에서 매 호출마다 Pattern 객체가 생성되기 때문에 상수로 빼면 좋겠습니다!
anxi01
force-pushed
the
feature/add-exception
branch
from
9eb52c0 to
280ccaf
Compare
✅ PR 유형
🚀 작업 내용
📝️ 관련 이슈
💬 기타 사항 or 추가 코멘트
1. '전체 공개'가 아닌 경우:
status="-2"2. 전체 공개이지만, 등록된 시간표가 없는 경우:
status="1"Summary by CodeRabbit
버그 수정
신규 에러
테스트
✏️ Tip: You can customize this high-level summary in your review settings.