Docs/open api 문서 핫픽스#31
Hidden character warning
Conversation
There was a problem hiding this comment.
Pull request overview
OpenAPI 문서의 API 버전 표기를 v2로 갱신하고, 날짜 도메인에서 offset 문자열을 정규화해 더 다양한 offset 표현을 처리하려는 PR입니다.
Changes:
- OpenAPI 문서 Info 버전을
v1→v2로 변경 ZonedDateAttribute에서 offset 문자열 정규화 로직 추가 및 적용
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/me/gg/pinit/pinittask/infrastructure/config/OpenApiConfig.java | OpenAPI 문서의 API 버전 표기를 v2로 업데이트 |
| src/main/java/me/gg/pinit/pinittask/domain/datetime/ZonedDateAttribute.java | offsetId 정규화 로직을 추가하고 offset 파싱 시 정규화 적용 |
| private String normalizeOffsetId(String raw) { | ||
| String trimmed = Objects.requireNonNull(raw, "offsetId must not be null").trim(); | ||
| if (trimmed.isEmpty()) { | ||
| throw new IllegalArgumentException("offsetId must not be blank"); | ||
| } |
There was a problem hiding this comment.
(1) 문제점: normalizeOffsetId()는 다양한 입력 변형(예: "UTC", "GMT", "+0900" 등)을 허용하도록 복잡한 분기 로직을 추가했지만, 해당 케이스들이 실제로 기대한 대로 동작하는지 검증하는 테스트가 이번 PR에 포함되어 있지 않습니다.
(2) 영향: 운영 환경/DB에 이미 존재하는 값 변형을 제대로 처리하지 못하면 런타임 예외가 발생하거나, 기대와 다른 오프셋으로 해석될 위험이 있습니다.
(3) 수정 제안: ZonedDateAttribute의 toZonedDateTime()/getOffset() 경로로 입력 변형들을 검증하는 단위 테스트를 추가하거나, JPA 로딩 시 offset_id에 "UTC" 같은 값이 들어온 경우에도 정상 동작하는 통합 테스트를 추가해 주세요.
| public ZoneOffset getOffset() { | ||
| Objects.requireNonNull(offsetId, "offsetId must not be null"); | ||
| return ZoneOffset.of(offsetId); | ||
| return ZoneOffset.of(normalizeOffsetId(offsetId)); | ||
| } |
There was a problem hiding this comment.
(1) 문제점: getOffset()은 offsetId를 정규화해서 해석하지만, 필드 자체(offsetId)는 정규화되지 않은 값(예: DB에 "UTC"로 저장된 값)을 그대로 유지합니다. 이 경우 동일한 오프셋을 의미해도 ZonedDateAttribute의 equals/hashCode(문자열 offsetId 기반) 및 JPQL 조건(s.startOfWeek.offsetId = ...)과 의미가 불일치할 수 있습니다.
(2) 영향: 같은 값을 의미하는데도 조회/비교가 실패하거나, 컬렉션 키로 사용할 때 예기치 않은 동작이 발생할 수 있습니다.
(3) 수정 제안: 엔티티 로딩/저장 시점에 offsetId를 정규화해 필드에 반영(@PostLoad/@PrePersist/@PreUpdate 등)하거나, equals/hashCode 및 조회 파라미터 생성 시에도 동일한 정규화 규칙을 적용해 표현을 단일화해 주세요.
| String upper = trimmed.toUpperCase(); | ||
| // Common UTC variants | ||
| if (upper.equals("Z") || upper.equals("UTC") || upper.equals("UT") || upper.equals("GMT") || upper.equals("Z0") || upper.equals("UTC+0") || upper.equals("UTC+00:00") || upper.equals("UTC+0000")) { | ||
| return "Z"; |
There was a problem hiding this comment.
(1) 문제점: UTC 변형 처리(if 조건)가 단일 라인에 많은 OR 조건으로 나열되어 있어 가독성이 낮고, 케이스 추가/수정 시 누락 또는 오타를 만들기 쉽습니다.
(2) 영향: 향후 유지보수 시 버그가 유입되기 쉽고, 리뷰/디버깅 비용이 증가합니다.
(3) 수정 제안: Set.of(...).contains(upper) 형태로 상수 집합을 분리하거나, switch/Map 기반으로 리팩터링해서 규칙을 데이터로 표현해 주세요.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/main/java/me/gg/pinit/pinittask/interfaces/schedule/ScheduleControllerAdvice.java:26
- (1) 문제점: ScheduleControllerAdvice가 기존의 전역 @RestControllerAdvice에서 assignableTypes로 ScheduleControllerV1/V2에만 적용되도록 변경되면서, 여기서 처리하던 MemberNotFoundException 등이 MemberControllerV0/V1/V2 등 다른 컨트롤러에서 더 이상 ErrorResponse로 매핑되지 않습니다.
(2) 영향: 예를 들어 MemberControllerV1/V2의 /zone-offset 호출 시 MemberService.findZoneOffsetOfMember()에서 MemberNotFoundException이 발생하면, 현재는 해당 예외를 처리하는 다른 Advice가 없어 기본 500 응답(또는 기본 에러 바디)로 떨어져 API 응답 포맷/상태코드가 깨질 수 있습니다.
(3) 수정 제안: MemberNotFoundException(및 공통적으로 쓰이는 예외들) 처리를 GlobalControllerAdvice로 옮기거나, assignableTypes에 MemberControllerV0/V1/V2(필요 시 TaskControllerV1/V2 포함)까지 추가하거나, Member 전용 ControllerAdvice를 별도로 두어 예외 매핑 범위를 유지해주세요.
| title = "Pinit Task API", | ||
| version = "v1", | ||
| version = "v2", | ||
| description = "일정 관리와 의존 관계 기능을 제공하는 API", |
There was a problem hiding this comment.
(1) 문제점: PR 설명에 포함된 "offset 문자열 정규화 추가"에 해당하는 변경이 현재 diff(ControllerAdvice 범위 조정, OpenAPI Info.version 변경)에서는 확인되지 않습니다.
(2) 영향: 리뷰어/릴리즈 노트 관점에서 실제 반영된 변경과 PR 설명이 불일치해, 누락된 기능이 배포되었다고 오해하거나 필요한 변경이 빠진 채로 머지될 수 있습니다.
(3) 수정 제안: (a) offset 정규화 변경이 다른 커밋/파일에 있다면 이 PR에 포함되도록 올려주시거나, (b) 이번 PR 범위가 문서 버전 표기 변경이라면 PR 설명 체크리스트/내용에서 해당 항목을 제거 또는 별도 PR로 분리해주세요.
1 participant
변경된 점