Feat/시간 정보를 utc 저장으로 변경#33
Merged
GoGradually merged 42 commits intomasterfrom Feb 2, 2026
Hidden character warning
The head ref may contain hidden characters: "feat/\uc2dc\uac04-\uc815\ubcf4\ub97c-UTC-\uc800\uc7a5\uc73c\ub85c-\ubcc0\uacbd"
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
이 PR은 시간대 정보를 UTC offset에서 IANA timezone ID로 변경하여 DST(Daylight Saving Time) 및 시간대 변경을 올바르게 처리할 수 있도록 합니다. UTC로 시간을 저장하고 사용자의 IANA zoneId를 이용해 로컬 시간으로 변환하는 방식으로 개선되었습니다.
Changes:
ZoneOffset기반 시간대 처리를 IANAZoneId기반으로 전환- 도메인 모델에
zoneId필드 추가 (deadline_zone_id,start_of_week_zone_id컬럼) - V1/V0 컨트롤러 제거 (deprecated)
- API 문서 및 DTO 업데이트 (
DateWithOffset,DateTimeWithZone) - 시간대별 조회를 위한 새로운 통합 테스트 추가
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/me/gg/pinit/pinittask/domain/datetime/ZonedDateAttribute.java | IANA zoneId 필드 추가 및 동적 offset 계산 로직 구현 |
| src/main/java/me/gg/pinit/pinittask/domain/task/vo/TemporalConstraint.java | zoneId 기반 마감일 저장으로 변경 |
| src/main/java/me/gg/pinit/pinittask/domain/task/repository/TaskRepository.java | zoneId 기반 쿼리 추가 및 날짜 범위 조회 메서드 추가 |
| src/main/java/me/gg/pinit/pinittask/application/task/service/TaskService.java | DST 대응 마감일 필터링 로직 구현 (UTC 구간 기반) |
| src/main/java/me/gg/pinit/pinittask/application/task/service/TaskArchiveService.java | ZoneId 우선, ZoneOffset 레거시 지원 |
| src/main/java/me/gg/pinit/pinittask/application/member/service/MemberService.java | ZoneId 조회 메서드 추가, ZoneOffset은 동적 계산 |
| src/main/java/me/gg/pinit/pinittask/application/schedule/service/ScheduleService.java | MemberService 의존성 제거, 요청의 zoneId 사용 |
| src/main/java/me/gg/pinit/pinittask/interfaces/task/TaskControllerV2.java | ZoneId 파라미터 추가, MemberService에서 사용자 zoneId 조회 |
| src/main/java/me/gg/pinit/pinittask/interfaces/dto/DateWithOffset.java | zoneId 필수 필드로 변경, offset은 선택적 |
| src/main/java/me/gg/pinit/pinittask/infrastructure/events/auth/MemberCreatedEventListener.java | 신규 회원에 zoneId 할당 (현재 하드코딩) |
| src/test/java/me/gg/pinit/pinittask/interfaces/web/ScheduleControllerV2TimeZoneIntegrationTest.java | 시간대별 조회 동작 검증 통합 테스트 추가 |
| src/test/java/me/gg/pinit/pinittask/domain/datetime/ZonedDateAttributeTest.java | DST offset 재계산 검증 단위 테스트 |
| TaskControllerV1.java, ScheduleControllerV1.java, ScheduleControllerV0.java, StatisticsControllerV1.java, StatisticsControllerV0.java | Deprecated 컨트롤러 제거 |
Comments suppressed due to low confidence (1)
src/test/java/me/gg/pinit/pinittask/domain/task/repository/TaskRepositoryTest.java:199
- 문제점: 이 테스트는 더 이상 사용되지 않는
findAllByOwnerIdAndDeadlineDate메서드를 테스트하고 있습니다. 실제 프로덕션 코드는findAllByOwnerIdAndDeadlineDateBetween과 인메모리 필터링을 사용합니다 (TaskService.java:149-166).
영향: 테스트가 실제 사용되는 코드 경로를 검증하지 못하고 있습니다.
수정 제안:
- 이 테스트를 삭제하거나
TaskService.getTasksByDeadline메서드를 직접 테스트하는 통합 테스트로 변경하세요.
현재 TaskServiceTest에 DST를 고려한 테스트가 추가되었으므로 (line 286-307), 이 저수준 repository 테스트는 제거하는 것이 좋습니다.
Comment on lines
+285
to
307
| @Test | ||
| void getTasksByDeadline_filtersByUtcRangeWithDst() { | ||
| Long ownerId = 21L; | ||
| ZoneId requestZone = ZoneId.of("Asia/Tokyo"); // UTC+9 | ||
| LocalDate requestDate = LocalDate.of(2025, 3, 9); // DST start day in NY | ||
|
|
||
| Task matching = buildTask(ownerId); | ||
| ZonedDateTime nyStart = requestDate.atStartOfDay(ZoneId.of("America/New_York")); | ||
| ReflectionTestUtils.setField(matching, "temporalConstraint", | ||
| new TemporalConstraint(nyStart, Duration.ZERO)); | ||
|
|
||
| Task nonMatching = buildTask(ownerId); | ||
| ZonedDateTime nextDayNy = requestDate.plusDays(1).atStartOfDay(ZoneId.of("America/New_York")); | ||
| ReflectionTestUtils.setField(nonMatching, "temporalConstraint", | ||
| new TemporalConstraint(nextDayNy, Duration.ZERO)); | ||
|
|
||
| when(taskRepository.findAllByOwnerIdAndDeadlineDateBetween(ownerId, requestDate.minusDays(1), requestDate.plusDays(1))) | ||
| .thenReturn(List.of(matching, nonMatching)); | ||
|
|
||
| List<Task> result = taskService.getTasksByDeadline(ownerId, requestDate, requestZone); | ||
|
|
||
| Assertions.assertThat(result).containsExactly(matching); | ||
| } |
There was a problem hiding this comment.
문제점: 이 테스트는 DST 동작을 실제로 검증하지 않습니다.
requestDate = 2025-03-09는 뉴욕에서 DST 전환일이지만, 테스트는 이 전환이 올바르게 처리되는지 확인하지 않습니다.matching태스크의 마감일은 실제로 2025-03-09가 아니라 다른 로컬 날짜일 수 있습니다 (requestDate.atStartOfDay(ZoneId.of("America/New_York"))는 뉴욕 시간 기준이므로).- 테스트는 단순히 다른 날짜의 태스크가 필터링되는지만 확인합니다.
영향: DST 관련 버그가 발견되지 않을 수 있습니다.
수정 제안: DST 경계 케이스를 실제로 테스트하세요:
- 한 작업의 로컬 날짜가 DST 전환으로 인해 UTC 날짜와 다른 경우를 테스트
- 예: 뉴욕 2025-03-09 00:00 (UTC-05:00)는 UTC 2025-03-09 05:00이지만, 뉴욕 2025-11-02 00:00 (DST 종료 후, UTC-05:00)와 2025-11-02 01:00 (DST 전, UTC-04:00)는 DST 전환 시각 전후입니다.
- Tokyo 시간대(UTC+9)에서 2025-03-09를 조회할 때, 그 날의 모든 작업이 포함되는지 확인하세요.
Comment on lines
36
to
44
| public ZoneId resolveZoneId(ZoneId fallbackZoneId) { | ||
| Objects.requireNonNull(fallbackZoneId, "fallbackZoneId must not be null"); | ||
| return zoneId == null ? fallbackZoneId : zoneId; | ||
| } | ||
|
|
||
| public ZoneOffset resolveOffset(ZoneId fallbackZoneId) { | ||
| ZoneId effectiveZone = resolveZoneId(fallbackZoneId); | ||
| return date.atStartOfDay(effectiveZone).getOffset(); | ||
| } |
There was a problem hiding this comment.
문제점: resolveZoneId 메서드가 zoneId가 null일 때 fallbackZoneId를 반환하지만, compact constructor에서 이미 zoneId를 @NotNull로 요구하고 있습니다. 즉, zoneId는 절대 null일 수 없으므로 이 메서드의 null 체크는 불필요합니다.
영향: 불필요한 방어 코드가 혼란을 야기할 수 있습니다.
수정 제안: resolveZoneId 메서드를 제거하거나, @NotNull 어노테이션을 제거하고 compact constructor의 null 체크도 제거하여 실제로 nullable하게 만드세요. 현재 API 문서에서는 zoneId를 필수로 명시하고 있으므로, 메서드를 제거하는 것이 더 일관성 있습니다.
Suggested change
| public ZoneId resolveZoneId(ZoneId fallbackZoneId) { | |
| Objects.requireNonNull(fallbackZoneId, "fallbackZoneId must not be null"); | |
| return zoneId == null ? fallbackZoneId : zoneId; | |
| } | |
| public ZoneOffset resolveOffset(ZoneId fallbackZoneId) { | |
| ZoneId effectiveZone = resolveZoneId(fallbackZoneId); | |
| return date.atStartOfDay(effectiveZone).getOffset(); | |
| } | |
| public ZoneOffset resolveOffset() { | |
| return date.atStartOfDay(zoneId).getOffset(); | |
| } |
| return new ZonedDateAttribute(date, offset.getId()); | ||
| ZoneOffset expected = date.atStartOfDay(zoneId).getOffset(); | ||
| if (!expected.equals(offset)) { | ||
| throw new IllegalArgumentException("제공된 offset이 zoneId의 규칙과 일치하지 않습니다."); |
There was a problem hiding this comment.
문제점: of(LocalDate, ZoneId, ZoneOffset) 메서드에서 offset 검증 시, 제공된 offset이 일치하지 않으면 예외를 던집니다. 그러나 에러 메시지가 구체적이지 않아 사용자가 어떤 값이 예상되고 어떤 값이 제공되었는지 알 수 없습니다.
영향: 디버깅이 어려워집니다.
수정 제안: 예외 메시지에 예상값과 실제값을 포함하세요:
throw new IllegalArgumentException(
String.format("제공된 offset(%s)이 zoneId(%s)의 규칙과 일치하지 않습니다. 예상: %s",
offset, zoneId, expected)
);
Suggested change
| throw new IllegalArgumentException("제공된 offset이 zoneId의 규칙과 일치하지 않습니다."); | |
| throw new IllegalArgumentException( | |
| String.format( | |
| "제공된 offset(%s)이 zoneId(%s)의 규칙과 일치하지 않습니다. date=%s, 예상 offset=%s", | |
| offset, zoneId, date, expected | |
| ) | |
| ); |
Comment on lines
+21
to
31
| @Column(name = "zone_id") | ||
| private String zoneId; | ||
|
|
||
| protected ZonedDateAttribute() { | ||
| } | ||
|
|
||
| private ZonedDateAttribute(LocalDate date, String offsetId) { | ||
| this.date = date; | ||
| this.offsetId = offsetId; | ||
| private ZonedDateAttribute(LocalDate date, String offsetId, String zoneId) { | ||
| this.date = Objects.requireNonNull(date, "date must not be null"); | ||
| this.offsetId = Objects.requireNonNull(offsetId, "offsetId must not be null"); | ||
| this.zoneId = Objects.requireNonNull(zoneId, "zoneId must not be null"); | ||
| } |
There was a problem hiding this comment.
문제점:
- 프로덕션 설정에서
ddl-auto: validate를 사용하고 있습니다. 이는 새로운 컬럼(deadline_zone_id,start_of_week_zone_id)이 데이터베이스 스키마에 존재하지 않으면 애플리케이션 시작이 실패한다는 것을 의미합니다. - 명시적인 데이터베이스 마이그레이션 스크립트(Flyway/Liquibase)가 없어 기존 데이터에 대한 처리 전략이 불분명합니다.
- 기존 레코드의
deadline_zone_id는 null이거나 기본값이 필요하지만, 코드는 이를@NotNull로 요구합니다.
영향:
- 프로덕션 배포 시 애플리케이션 시작 실패 또는 데이터 무결성 문제가 발생할 수 있습니다.
- 기존 데이터가 손상될 수 있습니다.
수정 제안:
- Flyway 또는 Liquibase 마이그레이션 스크립트를 추가하여:
- 새 컬럼을
nullable=true로 추가 - 기존 데이터의
deadline_zone_id를deadline_offset_id에서 유추하거나 기본값(예: "UTC")으로 채우기 - 이후 컬럼을
NOT NULL로 변경
- 새 컬럼을
- 또는 개발/프로덕션 모두에서 Hibernate의 자동 DDL 업데이트에 의존하지 말고 명시적 마이그레이션 전략을 수립하세요.
| @ApiResponse(responseCode = "400", description = "날짜 형식이 올바르지 않습니다.", content = @Content(schema = @Schema(implementation = ErrorResponse.class))) | ||
| }) | ||
| public List<ScheduleSimpleResponse> getSchedules(@Parameter(hidden = true) @MemberId Long memberId, | ||
| @Parameter(description = "사용자 로컬 시각. 반드시 TZ 없이 직렬화한 LocalDateTime 사용 금지 — 아래 zoneId와 함께 보낸다.", example = "2026-02-01T09:00:00") |
There was a problem hiding this comment.
문제점: 사용자 로컬 시각에 대한 설명에서 "반드시 TZ 없이 직렬화한 LocalDateTime 사용 금지"라는 표현이 이중 부정으로 인해 혼란스럽습니다.
영향: API 사용자가 요구사항을 잘못 이해할 수 있습니다.
수정 제안: 긍정적이고 명확한 표현으로 변경하세요:
"사용자 로컬 시각 (LocalDateTime). 시간대 정보 없이 직렬화된 값을 사용하세요."
또는
"사용자 로컬 시각. ISO 8601 형식의 LocalDateTime을 전송하세요 (예: 2026-02-01T09:00:00). UTC 오프셋을 포함하지 마세요."
Suggested change
| @Parameter(description = "사용자 로컬 시각. 반드시 TZ 없이 직렬화한 LocalDateTime 사용 금지 — 아래 zoneId와 함께 보낸다.", example = "2026-02-01T09:00:00") | |
| @Parameter(description = "사용자 로컬 시각. ISO 8601 형식의 LocalDateTime을 전송하세요 (예: 2026-02-01T09:00:00). UTC 오프셋을 포함하지 마세요.", example = "2026-02-01T09:00:00") |
| @ApiResponse(responseCode = "400", description = "날짜 형식이 올바르지 않습니다.", content = @Content(schema = @Schema(implementation = ErrorResponse.class))) | ||
| }) | ||
| public List<ScheduleSimpleResponse> getWeeklySchedules(@Parameter(hidden = true) @MemberId Long memberId, | ||
| @Parameter(description = "사용자 로컬 시각. 주차 계산의 기준 anchor.", example = "2026-02-03T10:00:00") |
There was a problem hiding this comment.
문제점: 사용자 로컬 시각에 대한 설명에서 "반드시 TZ 없이 직렬화한 LocalDateTime 사용 금지"라는 표현이 이중 부정으로 인해 혼란스럽습니다.
영향: API 사용자가 요구사항을 잘못 이해할 수 있습니다.
수정 제안: 긍정적이고 명확한 표현으로 변경하세요:
"사용자 로컬 시각 (LocalDateTime). 시간대 정보 없이 직렬화된 값을 사용하세요."
또는
"사용자 로컬 시각. ISO 8601 형식의 LocalDateTime을 전송하세요 (예: 2026-02-01T09:00:00). UTC 오프셋을 포함하지 마세요."
Suggested change
| @Parameter(description = "사용자 로컬 시각. 주차 계산의 기준 anchor.", example = "2026-02-03T10:00:00") | |
| @Parameter(description = "사용자 로컬 시각. ISO 8601 형식의 LocalDateTime을 전송하세요 (예: 2026-02-03T10:00:00). UTC 오프셋을 포함하지 마세요.", example = "2026-02-03T10:00:00") |
| private ZoneId resolveZoneId() { | ||
| // TODO 이벤트 페이로드에 timezone 정보를 추가해야 하나? 아니면 gRPC로 MemberService에 조회를 하나? | ||
| // 어쩌면 이벤트가 아닌 프론트에서 직접 등록 API를 호출하게 하는 것이 맞을수도 (일단 킵 | ||
| return ZoneId.of("Asia/Seoul"); |
There was a problem hiding this comment.
문제점: 모든 새 회원에 대해 하드코딩된 "Asia/Seoul" 시간대가 할당됩니다. TODO 주석은 이것이 임시 해결책임을 나타냅니다.
영향:
- 서울 시간대가 아닌 지역의 사용자에게 잘못된 시간대가 설정됩니다.
- 프로덕션 환경에서 데이터 정확성 문제가 발생할 수 있습니다.
수정 제안:
- 이벤트 페이로드에 timezone 정보를 추가하거나
- 회원 서비스에서 gRPC로 timezone을 조회하거나
- 프론트엔드에서 사용자의 timezone을 포함하여 직접 등록 API를 호출하도록 변경하세요.
이 임시 해결책이 프로덕션에 배포되기 전에 해결해야 합니다.
Suggested change
| return ZoneId.of("Asia/Seoul"); | |
| return ZoneId.systemDefault(); |
Comment on lines
+64
to
73
| private ZoneId resolveZoneId(Long ownerId, ZoneId requestZoneId, ZoneOffset requestOffset) { | ||
| if (requestZoneId != null) { | ||
| return requestZoneId; | ||
| } | ||
| if (requestOffset != null) { | ||
| return requestOffset; | ||
| // 과거 호환용: 오프셋만 받은 경우에는 Offset 시간대를 생성해 사용한다. | ||
| return ZoneId.ofOffset("UTC", requestOffset); | ||
| } | ||
| return memberService.findZoneOffsetOfMember(ownerId); | ||
| return memberService.findZoneIdOfMember(ownerId); | ||
| } |
There was a problem hiding this comment.
문제점: ZoneId.ofOffset("UTC", requestOffset)를 사용하는 하위 호환성 로직이 있지만, 이는 실제 IANA 시간대가 아닌 고정 오프셋 시간대를 생성합니다.
영향:
- 고정 오프셋 시간대는 DST(일광 절약 시간) 규칙을 적용하지 않습니다
- 이전 API를 사용하는 클라이언트가 예상하지 못한 동작을 경험할 수 있습니다
- 데이터 일관성 문제가 발생할 수 있습니다 (예: 같은 날짜에 대해 다른 시간대로 저장될 수 있음)
수정 제안:
- 레거시
offset파라미터 지원을 명시적으로 deprecated로 표시하고 경고 로그를 추가하세요 - 가능하면 오프셋을 일반적인 IANA 시간대로 매핑하는 로직을 추가하세요 (예: +09:00 → Asia/Seoul)
- API 문서에 이 제한사항을 명확히 설명하세요
Comment on lines
17
to
19
| @RestControllerAdvice(assignableTypes = { | ||
| TaskControllerV2.class, | ||
| TaskControllerV1.class | ||
| TaskControllerV2.class | ||
| }) |
There was a problem hiding this comment.
문제점: V0, V1 API 버전이 완전히 삭제되었지만 (TaskControllerV0/V1, ScheduleControllerV0/V1 등), TaskControllerAdvice는 여전히 이전에 TaskControllerV1도 대상으로 했던 것으로 보입니다.
영향:
- 기존 클라이언트가 V0/V1 API를 사용 중이라면 서비스가 중단됩니다
- 이는 중요한 API 하위 호환성 깨짐(breaking change)입니다
- PR 설명에 이러한 API 제거에 대한 명시적 언급이 없습니다
수정 제안:
- V0, V1 API를 당장 삭제하지 말고
@Deprecated어노테이션을 추가하고 적절한 마이그레이션 기간을 제공하세요 - 또는 PR 설명에 "V0, V1 API 완전 제거 - 클라이언트 팀 조율 완료"를 명시하세요
- API 버전 폐기(deprecation) 정책을 문서화하세요
Comment on lines
+27
to
+28
| @Mock | ||
| Clock clock; |
There was a problem hiding this comment.
문제점: Clock이 주입되어 사용되고 있지만, 일부 테스트 파일에서는 @Mock Clock clock;이 선언되었음에도 실제로 사용되지 않습니다.
영향: 테스트 코드에 불필요한 모의 객체가 존재하여 혼란을 야기할 수 있습니다.
수정 제안: 사용하지 않는 @Mock Clock clock; 선언을 제거하세요.
Comment on lines
+12
to
+38
| class ZonedDateAttributeTest { | ||
|
|
||
| @Test | ||
| void preservesZoneIdAndComputesOffsetFromRules() { | ||
| ZoneId zoneId = ZoneId.of("Asia/Seoul"); | ||
| LocalDate date = LocalDate.of(2026, 2, 1); | ||
|
|
||
| ZonedDateAttribute attribute = ZonedDateAttribute.of(date, zoneId); | ||
|
|
||
| ZonedDateTime zoned = attribute.toZonedDateTime(); | ||
| assertThat(zoned.getZone()).isEqualTo(zoneId); | ||
| assertThat(attribute.getOffset()).isEqualTo(ZoneOffset.of("+09:00")); | ||
| } | ||
|
|
||
| @Test | ||
| void recalculatesOffsetForDstDates() { | ||
| ZoneId zoneId = ZoneId.of("America/New_York"); | ||
|
|
||
| ZonedDateAttribute winter = ZonedDateAttribute.of(LocalDate.of(2026, 3, 8), zoneId); // DST switch day | ||
| ZonedDateAttribute summer = ZonedDateAttribute.of(LocalDate.of(2026, 7, 1), zoneId); | ||
|
|
||
| assertThat(winter.getOffset()).isEqualTo(ZoneOffset.of("-05:00")); | ||
| assertThat(summer.getOffset()).isEqualTo(ZoneOffset.of("-04:00")); | ||
| assertThat(winter.getZoneId()).isEqualTo(zoneId); | ||
| assertThat(summer.getZoneId()).isEqualTo(zoneId); | ||
| } | ||
| } |
There was a problem hiding this comment.
문제점: ZonedDateAttribute.of(LocalDate, ZoneId, ZoneOffset) 메서드의 검증 로직에 대한 테스트가 없습니다. 특히 offset이 zoneId와 일치하지 않을 때 예외가 발생하는지 테스트하는 코드가 누락되었습니다.
영향: 핵심 검증 로직이 테스트되지 않아 회귀 버그가 발생할 수 있습니다.
수정 제안: 다음과 같은 테스트를 추가하세요:
- offset과 zoneId가 일치하는 경우의 정상 케이스
- offset과 zoneId가 일치하지 않을 때
IllegalArgumentException이 발생하는지 확인하는 테스트
Comment on lines
53
to
60
| @CreationTimestamp | ||
| @Column(updatable = false, columnDefinition = "DATETIME(6)") | ||
| @Convert(converter = InstantToDatetime6UtcConverter.class) | ||
| private Instant createdAt; | ||
| @UpdateTimestamp | ||
| @Column(columnDefinition = "DATETIME(6)") | ||
| @Convert(converter = InstantToDatetime6UtcConverter.class) | ||
| private Instant updatedAt; |
There was a problem hiding this comment.
문제점: InstantToDatetime6UtcConverter가 createdAt과 updatedAt 필드에 추가되었습니다. 그러나 이 변환기는 autoApply = false로 설정되어 있어 명시적으로 지정해야 합니다.
영향:
- 기존 데이터가 다른 형식으로 저장되어 있을 수 있습니다 (예: 로컬 시간대)
- 이 변경으로 인해 기존 데이터와의 호환성 문제가 발생할 수 있습니다
- Hibernate의 기본 동작과 다른 방식으로 timestamp를 처리합니다
수정 제안:
- 데이터 마이그레이션 계획을 확인하세요 (특히
createdAt,updatedAt값이 UTC로 변환되어야 하는 경우) - 이 변경사항이 기존 데이터에 미치는 영향을 문서화하세요
- 또는 새로운 컬럼을 추가하고 기존 컬럼은 유지하는 방안을 고려하세요
| ZoneOffset offset | ||
| @Schema(description = "UTC 기준 오프셋(+HH:mm). IANA `zoneId`로 계산된 값을 명시하고 싶을 때 선택적으로 포함", example = "+09:00") | ||
| ZoneOffset offset, | ||
| @Schema(description = "IANA 시간대 ID (필수)", example = "Asia/Seoul") |
There was a problem hiding this comment.
문제점: @NotNull 애노테이션이 zoneId 필드에 없지만, compact constructor에서 null 체크를 하고 있습니다.
영향: API 검증 계층과 도메인 로직 간에 불일치가 있어 혼란을 야기할 수 있습니다. Schema에는 "필수"라고 표시되어 있지만 Bean Validation이 누락되었습니다.
수정 제안: zoneId 필드에 @NotNull 애노테이션을 추가하여 일관성을 유지하세요:
@NotNull
@Schema(description = "IANA 시간대 ID (필수)", example = "Asia/Seoul")
ZoneId zoneId
Suggested change
| @Schema(description = "IANA 시간대 ID (필수)", example = "Asia/Seoul") | |
| @Schema(description = "IANA 시간대 ID (필수)", example = "Asia/Seoul") | |
| @NotNull |
| public record DateTimeWithZone( | ||
| @NotNull | ||
| @Schema(description = "지역 시각", example = "2024-03-01T18:00:00") | ||
| @Schema(description = "사용자 로컬 시각(오프셋 없이). `toISOString()`이 아닌 로컬 기준 문자열을 사용", example = "2026-02-01T10:00:00") |
There was a problem hiding this comment.
문제점: Schema 설명에 영문 전문 용어 toISOString()이 포함되어 있지만, 이것이 무엇을 의미하는지 명확하지 않습니다.
영향: API 문서를 읽는 개발자가 혼란스러울 수 있습니다. 특히 한국어 설명에 영어 메서드 이름이 섞여 있어 일관성이 떨어집니다.
수정 제안: 설명을 더 명확하게 수정하세요:
@Schema(description = "사용자 로컬 시각(오프셋이나 시간대 정보 없이). JavaScript의 toISOString()처럼 UTC로 변환하지 말고, 사용자가 보는 그대로의 로컬 시간을 전달", example = "2026-02-01T10:00:00")또는 더 간단하게:
@Schema(description = "사용자 로컬 시각(시간대 정보 제외). zoneId와 함께 해석됨", example = "2026-02-01T10:00:00")
Suggested change
| @Schema(description = "사용자 로컬 시각(오프셋 없이). `toISOString()`이 아닌 로컬 기준 문자열을 사용", example = "2026-02-01T10:00:00") | |
| @Schema(description = "사용자 로컬 시각(오프셋이나 시간대 정보 없이). JavaScript의 toISOString()처럼 UTC로 변환하지 말고, 사용자가 보는 그대로의 로컬 시간을 전달", example = "2026-02-01T10:00:00") |
Comment on lines
+65
to
+66
| ZoneId zone = getZoneId(); | ||
| return date.atStartOfDay(zone).getOffset(); |
There was a problem hiding this comment.
문제점: getOffset() 메서드가 매번 date.atStartOfDay(zone).getOffset()을 계산합니다. 그러나 offsetId가 이미 저장되어 있고, 생성 시점에 검증되었습니다.
영향:
- 불필요한 계산 오버헤드가 발생합니다
- 기존에 저장된
offsetId필드의 값과 동적으로 계산된 값이 다를 수 있습니다 (예: DST 규칙 변경 후) - 데이터 일관성 문제가 발생할 수 있습니다
수정 제안: getOffset() 메서드를 다음과 같이 수정하세요:
public ZoneOffset getOffset() {
Objects.requireNonNull(offsetId, "offsetId must not be null");
return ZoneOffset.of(offsetId);
}또는 저장된 offsetId를 신뢰할 수 없다면, 주석으로 이유를 설명하세요.
Suggested change
| ZoneId zone = getZoneId(); | |
| return date.atStartOfDay(zone).getOffset(); | |
| // 생성 시점에 검증·저장된 offsetId를 신뢰하여, 매번 zone 규칙에 따라 재계산하지 않는다. | |
| Objects.requireNonNull(offsetId, "offsetId must not be null"); | |
| return ZoneOffset.of(offsetId); |
| taskRepository.saveAll(List.of(target1, target2, otherDate, otherOwner)); | ||
|
|
||
| List<Task> result = taskRepository.findAllByOwnerIdAndDeadlineDate(1L, LocalDate.of(2025, 2, 1)); | ||
| List<Task> result = taskRepository.findAllByOwnerIdAndDeadlineDate(1L, LocalDate.of(2025, 2, 1), "Z"); |
There was a problem hiding this comment.
문제점: 테스트에서 findAllByOwnerIdAndDeadlineDate 메서드를 호출할 때 3번째 파라미터로 "Z"를 전달하고 있습니다. 이는 UTC를 의미하는 것으로 보이지만, 실제 IANA 시간대 ID는 "UTC"이어야 합니다.
영향:
"Z"는 유효한 IANA 시간대 ID가 아니므로ZoneId.of("Z")는 예외를 발생시킬 수 있습니다 (실제로는ZoneId.of("Z")가 UTC로 파싱되긴 하지만, 일관성을 위해"UTC"를 사용하는 것이 더 명확합니다)- 테스트 데이터와 실제 사용 패턴 간에 불일치가 있을 수 있습니다
수정 제안: "Z" 대신 "UTC" 또는 실제 사용될 시간대 ID를 사용하세요:
List<Task> result = taskRepository.findAllByOwnerIdAndDeadlineDate(1L, LocalDate.of(2025, 2, 1), "UTC");
Comment on lines
148
to
167
| @Transactional(readOnly = true) | ||
| public List<Task> getTasksByDeadline(Long ownerId, LocalDate deadlineDate) { | ||
| return taskRepository.findAllByOwnerIdAndDeadlineDate(ownerId, deadlineDate); | ||
| public List<Task> getTasksByDeadline(Long ownerId, LocalDate deadlineDate, ZoneId zoneId) { | ||
| ZonedDateTime dayStart = deadlineDate.atStartOfDay(zoneId); | ||
| Instant startUtc = dayStart.toInstant(); | ||
| Instant endUtc = dayStart.plusDays(1).toInstant(); | ||
|
|
||
| // 넉넉한 날짜 범위로 조회 후, 실제 UTC 구간으로 필터 (DST 대응) | ||
| List<Task> candidates = taskRepository.findAllByOwnerIdAndDeadlineDateBetween( | ||
| ownerId, | ||
| deadlineDate.minusDays(1), | ||
| deadlineDate.plusDays(1) | ||
| ); | ||
|
|
||
| return candidates.stream() | ||
| .filter(task -> { | ||
| Instant deadlineUtc = task.getTemporalConstraint().getDeadline().toInstant(); | ||
| return !deadlineUtc.isBefore(startUtc) && deadlineUtc.isBefore(endUtc); | ||
| }) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
문제점: getTasksByDeadline 메서드에서 ±1일 범위로 조회한 후 메모리에서 필터링하고 있습니다. 이는 DST를 고려한 설계이지만, 데이터가 많을 경우 성능 문제가 발생할 수 있습니다.
영향:
- 3일치 데이터를 모두 메모리에 로드한 후 필터링하므로 메모리 사용량이 증가합니다
- 사용자가 많은 작업을 가지고 있을 경우 성능 저하가 발생할 수 있습니다
- 데이터베이스 인덱스를 효율적으로 활용하지 못합니다
수정 제안:
- DST 경계가 발생하는 특수한 경우에만 ±1일 범위를 사용하도록 최적화하세요
- 페이지네이션을 고려하세요
- 또는 주석을 추가하여 이 설계 결정의 이유와 예상되는 데이터 규모를 명시하세요
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.
변경된 점