[FEAT] 예약 가능한 시간표 도메인 로직 개발 (#259) #260
Conversation
📝 WalkthroughWalkthrough이 PR은 사진작가의 예약 가능성을 관리하는 PhotographerSchedule과 ScheduleOverride 도메인 엔티티를 포함한 완전한 예약 시간표 시스템을 도입합니다. 시간 관리용 값 객체, JPA 커스텀 컨버터, 포괄적인 테스트 커버리지, Spring Boot 애플리케이션 진입점 업데이트를 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PhotographerSchedule as Photographer<br/>Schedule
participant WeekdayRule as Weekday<br/>Schedule Rule
participant ScheduleOverride as Schedule<br/>Override
participant Converter as Converter<br/>(JSONB)
participant DB as Database
rect rgb(200, 220, 240)
Note over Client,DB: 초기화: 사진작가 일정 생성
Client->>PhotographerSchedule: initSchedule(photographerId)
PhotographerSchedule->>PhotographerSchedule: initializeDefaultWeekdayRules()
loop 각 요일 (월-일)
PhotographerSchedule->>WeekdayRule: dayOff(DayOfWeek)
activate WeekdayRule
WeekdayRule->>WeekdayRule: empty AvailableStartTimes
deactivate WeekdayRule
end
PhotographerSchedule-->>Client: PhotographerSchedule(weekdayRules, overrides=[])
end
rect rgb(220, 240, 200)
Note over Client,DB: 근무 규칙 저장
Client->>Converter: convertToDatabaseColumn(weekdayRules)
Converter->>Converter: Map → JSON 직렬화
Converter-->>DB: JSON String (JSONB)
DB->>DB: 저장
end
rect rgb(240, 220, 200)
Note over Client,DB: 스케줄 오버라이드 (휴무일 추가)
Client->>ScheduleOverride: dayOff(photographerId, targetDate)
ScheduleOverride->>ScheduleOverride: validateOverride()
ScheduleOverride-->>Client: ScheduleOverride(targetDate, empty times)
Client->>PhotographerSchedule: overrides.add(override)
PhotographerSchedule->>Converter: convertToDatabaseColumn(override.availableTimes)
Converter-->>DB: JSON String (JSONB)
end
rect rgb(240, 200, 220)
Note over Client,DB: 저장된 일정 조회
Client->>DB: SELECT * FROM photographer_schedule
DB-->>Converter: JSONB String
Converter->>Converter: JSON → Map<DayOfWeek, Rule> 역직렬화
Converter->>WeekdayRule: 각 Rule 복원
Converter-->>PhotographerSchedule: weekdayRules 맵
PhotographerSchedule-->>Client: PhotographerSchedule
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
Test Results (reservation)62 tests 62 ✅ 0s ⏱️ Results for commit 817a5ae. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR implements the domain logic for photographer reservation schedules, enabling photographers to define their availability patterns through weekday rules and date-specific overrides. The implementation follows Domain-Driven Design principles with value objects, entities, and appropriate data converters.
Key Changes:
- Created domain model with
PhotographerScheduleaggregate root managing weekday rules and schedule overrides - Implemented value objects (
AvailableStartTimes,WeekdayScheduleRule) for type-safe time management - Added JPA converters for JSON serialization of complex types to PostgreSQL jsonb columns
- Refactored package structure from
net.catsnap.reservationtonet.catsnap.CatsnapReservation
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
PhotographerSchedule.java |
Aggregate root managing photographer's schedule with weekday rules and overrides |
ScheduleOverride.java |
Entity for date-specific schedule exceptions (custom hours or day off) |
WeekdayScheduleRule.java |
Value object defining repeating schedule rules per day of week |
AvailableStartTimes.java |
Value object wrapping immutable, sorted list of available booking times |
WeekdayRulesConverter.java |
JPA converter for serializing weekday rules map to JSON |
AvailableStartTimesConverter.java |
JPA converter for serializing available times to JSON array |
| Test files | Comprehensive unit tests for domain logic and converters |
build.gradle |
Added Spring Data JPA, PostgreSQL, H2, and Lombok dependencies |
CatsnapReservationApplication.java |
Refactored main application class to new package structure |
application.yml |
Added test configuration for H2 database |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Override | ||
| public String toString() { | ||
| return String.format( | ||
| "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}", |
There was a problem hiding this comment.
The string "AvailabilityOverride" in the toString() method does not match the actual class name "ScheduleOverride". This inconsistency can cause confusion during debugging and logging.
| "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}", | |
| "ScheduleOverride{id=%d, photographerId=%d, targetDate=%s}", |
|
|
||
| @Override | ||
| public String toString() { | ||
| return String.format("ReservationAvailability{id=%d, photographerId=%d}", |
There was a problem hiding this comment.
The string "ReservationAvailability" in the toString() method does not match the actual class name "PhotographerSchedule". This inconsistency can cause confusion during debugging and logging.
| return String.format("ReservationAvailability{id=%d, photographerId=%d}", | |
| return String.format("PhotographerSchedule{id=%d, photographerId=%d}", |
| String result = override.toString(); | ||
|
|
||
| // then | ||
| assertThat(result).contains("AvailabilityOverride"); |
There was a problem hiding this comment.
The test assertion expects the string "AvailabilityOverride" but the actual class name is "ScheduleOverride". This test will pass with the current implementation but validates incorrect behavior. The toString() method should return the correct class name.
| assertThat(result).contains("AvailabilityOverride"); | |
| assertThat(result).contains("ScheduleOverride"); |
| * 예외 규칙들 (Aggregate 내부 관리) | ||
| */ | ||
| @OneToMany( | ||
| mappedBy = "photographerId", |
There was a problem hiding this comment.
The relationship mapping uses 'mappedBy = "photographerId"' but photographerId is not a proper JPA relationship field - it's a primitive Long field in ScheduleOverride. The mappedBy attribute should reference an actual @manytoone or @OnetoOne annotated field that references the PhotographerSchedule entity. This configuration will cause a JPA mapping exception at runtime.
| mappedBy = "photographerId", |
| /** | ||
| * 예약 가능 시작 시간 목록 | ||
| */ | ||
| @Column(columnDefinition = "jsonb") |
There was a problem hiding this comment.
Using 'jsonb' column definition assumes PostgreSQL database, but the configuration may not be portable to other databases. Since this is a converter-based approach, consider using 'TEXT' or 'CLOB' for better database portability, or document the PostgreSQL dependency clearly if it's intentional.
| /** | ||
| * 요일별 기본 규칙 JSON으로 저장: {"MONDAY": {...}, "TUESDAY": {...}} | ||
| */ | ||
| @Column(columnDefinition = "jsonb") |
There was a problem hiding this comment.
Using 'jsonb' column definition assumes PostgreSQL database, but the configuration may not be portable to other databases. Since this is a converter-based approach, consider using 'TEXT' or 'CLOB' for better database portability, or document the PostgreSQL dependency clearly if it's intentional.
| @Column(columnDefinition = "jsonb") | |
| @Column(columnDefinition = "TEXT") |
| @Test | ||
| void toString이_올바르게_동작한다() { | ||
| // given | ||
| Long photographerId = 1L; |
There was a problem hiding this comment.
The variable 'photographerId' is only assigned values of primitive type and is never 'null', but it is declared with the boxed type 'Long'.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
reservation/build.gradlereservation/src/main/java/net/catsnap/CatsnapReservation/CatsnapReservationApplication.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRule.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverter.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.javareservation/src/main/java/net/catsnap/reservation/ReservationApplication.javareservation/src/test/java/net/catsnap/CatsnapReservation/CatsnapReservationApplicationTests.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverrideTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverterTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverterTest.javareservation/src/test/java/net/catsnap/reservation/ReservationApplicationTests.javareservation/src/test/resources/application.yml
💤 Files with no reviewable changes (2)
- reservation/src/main/java/net/catsnap/reservation/ReservationApplication.java
- reservation/src/test/java/net/catsnap/reservation/ReservationApplicationTests.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-13T12:02:01.218Z
Learnt from: redblackblossom
Repo: CUKCatSnap/CUKCATSNAP_Server PR: 218
File: authorization/src/main/java/net/catsnap/shared/infrastructure/web/response/code/CommonResultCode.java:38-41
Timestamp: 2025-12-13T12:02:01.218Z
Learning: In Spring Boot apps, when returning a ResponseEntity with HttpStatus.NO_CONTENT (204), the framework suppresses the response body. Do not add manual body handling for 204; returning a 204 should produce an empty body by default. This applies to any controller methods returning ResponseEntity with 204. If you ever provide a body with 204, rely on framework behavior; consider using ResponseEntity.noContent().build() for clarity.
Applied to files:
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverrideTest.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverterTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/CatsnapReservationApplicationTests.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverter.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.javareservation/src/main/java/net/catsnap/CatsnapReservation/CatsnapReservationApplication.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverterTest.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.javareservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRule.javareservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
🧬 Code graph analysis (12)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverrideTest.java (1)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
reservation/build.gradle (1)
authentication/src/main/java/net/catsnap/CatsnapAuthorization/shared/infrastructure/persistence/JpaConfiguration.java (1)
Configuration(20-24)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.java (1)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverter.java (1)
Converter(18-68)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverterTest.java (2)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.java (1)
DisplayName(13-213)reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.java (1)
DisplayName(14-195)
reservation/src/test/java/net/catsnap/CatsnapReservation/CatsnapReservationApplicationTests.java (1)
authentication/src/test/java/net/catsnap/CatsnapAuthorization/CatsnapAuthorizationApplicationTests.java (2)
SpringBootTest(6-13)Test(9-11)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java (2)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.java (1)
DisplayName(13-213)reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.java (1)
DisplayName(14-195)
reservation/src/main/java/net/catsnap/CatsnapReservation/CatsnapReservationApplication.java (1)
authentication/src/main/java/net/catsnap/CatsnapAuthorization/CatsnapAuthorizationApplication.java (2)
main(9-11)SpringBootApplication(6-13)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverterTest.java (1)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.java (1)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.java (3)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java (1)
Entity(29-110)reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRule.java (1)
Getter(15-109)reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.java (4)
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java (1)
DisplayName(13-61)reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.java (1)
DisplayName(13-213)reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverterTest.java (1)
DisplayName(18-283)reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRule.java (1)
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (1)
AvailableStartTimes(13-119)
🔇 Additional comments (18)
reservation/src/test/resources/application.yml (1)
1-19: 테스트 설정이 적절합니다.H2 인메모리 데이터베이스를 사용한 테스트 환경 설정이 올바르게 구성되어 있습니다.
ddl-auto: create-drop설정으로 테스트 격리가 보장되며, SQL 로깅 활성화로 디버깅이 용이합니다.reservation/src/test/java/net/catsnap/CatsnapReservation/CatsnapReservationApplicationTests.java (1)
6-11: 애플리케이션 컨텍스트 로딩 테스트가 적절합니다.
@SpringBootTest를 사용한 기본 컨텍스트 로딩 테스트가 올바르게 구성되어 있으며,authentication모듈의 테스트 패턴과 일관성이 있습니다. 초기 설정 단계에서 애플리케이션이 정상적으로 부트스트랩되는지 검증하는 용도로 적합합니다.reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java (3)
18-31: 스케줄 초기화 테스트가 잘 작성되었습니다.
PhotographerSchedule.initSchedule()팩토리 메서드를 통한 초기화 로직이 올바르게 검증되고 있습니다. photographerId, weekdayRules, overrides의 초기 상태를 모두 확인하고 있어 포괄적입니다.
33-49: 기본 요일 규칙 초기화 검증이 철저합니다.모든 요일(
DayOfWeek.values())에 대해 반복하면서 각 규칙이 휴무일로 초기화되는지 확인하는 로직이 훌륭합니다. 기본 상태가isWorkingDay() = false이고 예약 가능 시간이 비어있는 것을 명확히 검증하고 있습니다.
51-60: null 검증 테스트가 적절합니다.null
photographerId에 대한 예외 처리가 올바르게 검증되고 있으며, 한글 에러 메시지("작가 ID는 필수입니다.")도 명확합니다. 방어적 프로그래밍 원칙을 잘 따르고 있습니다.reservation/src/main/java/net/catsnap/CatsnapReservation/CatsnapReservationApplication.java (1)
6-11: Spring Boot 애플리케이션 진입점이 올바르게 구성되었습니다.표준적인 Spring Boot 애플리케이션 구조를 따르고 있으며,
authentication모듈의 패턴과 일관성이 있습니다.도메인 엔티티(
PhotographerSchedule,ScheduleOverride)가 모두net.catsnap.CatsnapReservation.schedule.domain패키지 아래에 위치하여@SpringBootApplication어노테이션의 기본 컴포넌트 스캔 범위에 포함됩니다.reservation/build.gradle (1)
46-58: 의존성 추가가 적절합니다.Spring Web, JPA, PostgreSQL, H2, Lombok 의존성이 올바르게 추가되었습니다. 현재 구현에서는 JPA Auditing 기능(
@CreatedDate,@LastModifiedDate등)이 사용되지 않으므로@EnableJpaAuditing설정은 불필요합니다.reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimesTest.java (1)
1-213: LGTM! 포괄적인 테스트 커버리지
AvailableStartTimesVO에 대한 테스트가 잘 구성되어 있습니다:
- 정상 생성, 빈 목록 생성, null 처리
- 자동 정렬 및 중복 제거 검증
contains,first,last메서드 동작 확인- 빈 목록에서의 예외 처리
equals,hashCode,toString검증테스트 구조가 given-when-then 패턴을 일관되게 따르고 있으며, 엣지 케이스를 적절히 다루고 있습니다.
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverrideTest.java (1)
20-45: 테스트 구조가 잘 작성되었습니다.커스텀 시간과 휴무 규칙 생성에 대한 테스트가 명확하고,
AvailableStartTimesVO와의 통합도 올바르게 검증하고 있습니다.reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverterTest.java (1)
1-283: LGTM! 컨버터 테스트가 포괄적으로 작성되었습니다.
WeekdayRulesConverter에 대한 테스트가 잘 구성되어 있습니다:
- DB 컬럼 변환 (Map → JSON): 근무일, 휴무일, null, 빈 맵 처리
- 엔티티 속성 변환 (JSON → Map): 유효한 JSON, null/빈 문자열/공백 처리
- 양방향 변환 일관성 검증
- 모든 요일(7일) 포함 테스트
- 잘못된 JSON 형식 예외 처리
특히 Line 260-282의 모든 요일 규칙 테스트는 실제 사용 시나리오를 잘 반영하고 있습니다.
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverterTest.java (1)
1-159: LGTM! 컨버터 테스트가 잘 작성되었습니다.
AvailableStartTimesConverter에 대한 테스트가 적절히 구성되어 있습니다:
- JSON 직렬화/역직렬화 검증
- null, 빈 문자열, 공백, 빈 배열 처리
- 잘못된 JSON 형식 예외 처리
- 양방향 변환 일관성 검증
WeekdayRulesConverterTest와 일관된 테스트 구조를 유지하고 있어 코드베이스의 일관성이 좋습니다.reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRuleTest.java (1)
1-195: LGTM! 요일별 스케줄 규칙 VO 테스트가 잘 작성되었습니다.
WeekdayScheduleRuleVO에 대한 테스트가 포괄적으로 구성되어 있습니다:
- 근무일 규칙 생성 (AvailableStartTimes, List 두 가지 방식)
- 휴무일 규칙 생성 및 검증
- null 인자 검증
generateAvailableTimes(),isValidStartTime()메서드 동작 확인equals,hashCode,toString검증특히 Line 129-152의
isValidStartTime()테스트는 휴무일과 근무일에 대한 시간 유효성 검증 로직을 잘 다루고 있습니다.reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/WeekdayScheduleRule.java (1)
1-109: 잘 구현된 값 객체입니다.불변성, 팩토리 메서드 패턴, 유효성 검증이 모두 적절히 적용되어 있습니다.
equals/hashCode가 값 객체 시맨틱에 맞게 올바르게 구현되었고,toString도 가독성 있게 작성되었습니다.reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/vo/AvailableStartTimes.java (2)
17-27: 생성자 null 처리가 명확합니다.
of()팩토리에서 null 검증 후 생성자를 호출하고,empty()에서는 null을 전달하여 빈 리스트로 처리하는 패턴이 일관성 있게 적용되어 있습니다.
49-51:contains()메서드의 시간 복잡도 고려.현재
List.contains()는 O(n) 복잡도입니다. 예약 가능 시간 목록이 일반적으로 작으므로 현재 구현으로 충분하지만, 향후 시간 목록이 매우 커질 경우Set기반 조회를 고려할 수 있습니다.reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.java (1)
28-29: 정적ObjectMapper사용이 적절합니다.Jackson의
ObjectMapper는 설정 완료 후 스레드 안전하므로 정적 인스턴스 사용이 올바릅니다.reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java (2)
88-103: 영속화되지 않은 엔티티의equals동작 주의.
id가null인 두 개의 영속화되지 않은(transient) 엔티티가 동일하게 판정됩니다. 이는Set이나Map에서 예기치 않은 동작을 유발할 수 있습니다. 일반적인 JPA 엔티티 패턴이지만, 영속화 전 컬렉션 사용 시 주의가 필요합니다.
84-86:initSchedule팩토리 메서드가 명확합니다.null을 전달하여 기본 휴무일 규칙을 초기화하는 패턴이 깔끔합니다.
| @Column(columnDefinition = "jsonb") | ||
| @Convert(converter = WeekdayRulesConverter.class) | ||
| private Map<DayOfWeek, WeekdayScheduleRule> weekdayRules = new EnumMap<>(DayOfWeek.class); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
필드 초기화와 생성자 할당이 중복됩니다.
Line 46에서 weekdayRules를 new EnumMap<>(DayOfWeek.class)로 초기화하지만, 생성자(Line 68-70)에서 다시 할당합니다. 필드 초기화를 제거하거나 생성자에서 기존 맵을 재사용하는 것이 명확합니다.
🔎 수정 제안
@Column(columnDefinition = "jsonb")
@Convert(converter = WeekdayRulesConverter.class)
- private Map<DayOfWeek, WeekdayScheduleRule> weekdayRules = new EnumMap<>(DayOfWeek.class);
+ private Map<DayOfWeek, WeekdayScheduleRule> weekdayRules;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Column(columnDefinition = "jsonb") | |
| @Convert(converter = WeekdayRulesConverter.class) | |
| private Map<DayOfWeek, WeekdayScheduleRule> weekdayRules = new EnumMap<>(DayOfWeek.class); | |
| @Column(columnDefinition = "jsonb") | |
| @Convert(converter = WeekdayRulesConverter.class) | |
| private Map<DayOfWeek, WeekdayScheduleRule> weekdayRules; |
🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
around lines 44-46, weekdayRules is initialized at the field and then reassigned
in the constructor causing duplicate initialization; either remove the field
initializer (leave the declaration null and assign in the constructor) or change
the constructor to reuse the existing EnumMap (do not reassign weekdayRules;
instead clear() and putAll(...) or populate the map entries into the
already-created EnumMap). Ensure only one place performs allocation so the map
is not redundantly recreated.
| @OneToMany( | ||
| mappedBy = "photographerId", | ||
| cascade = CascadeType.ALL, | ||
| orphanRemoval = true, | ||
| fetch = FetchType.LAZY | ||
| ) | ||
| private List<ScheduleOverride> overrides = new ArrayList<>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ScheduleOverride.java" -type fRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 168
🏁 Script executed:
# 일단 PhotographerSchedule.java 파일의 위치 확인
find . -name "PhotographerSchedule.java" -type fRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 172
🏁 Script executed:
cat -n ./reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.javaRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 3975
@OneToMany(mappedBy = "photographerId") 매핑이 JPA 스펙을 위반합니다.
ScheduleOverride에는 photographerId가 단순 Long 필드일 뿐 @ManyToOne 참조가 없으므로, mappedBy = "photographerId"는 유효하지 않습니다. mappedBy는 자식 엔티티의 관계 필드(예: @ManyToOne PhotographerSchedule schedule)를 참조해야 합니다.
두 가지 방법으로 수정하세요:
ScheduleOverride에@ManyToOne PhotographerSchedule schedule필드를 추가하고mappedBy = "schedule"사용- 양방향 관계가 불필요하면
@JoinColumn(name = "photographer_id")를 사용한 단방향@OneToMany매핑
🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
around lines 51-57, the @OneToMany uses mappedBy = "photographerId" which is
invalid because ScheduleOverride currently stores a plain Long photographerId
rather than a @ManyToOne back-reference; either add a ManyToOne field in
ScheduleOverride (e.g., @ManyToOne PhotographerSchedule schedule) and change
mappedBy to "schedule", or keep a unidirectional mapping by removing mappedBy
and add @JoinColumn(name = "photographer_id") on this @OneToMany; implement one
of these two fixes and update any related getters/setters and constructors
accordingly.
| @Entity | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class ScheduleOverride { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| private Long photographerId; | ||
|
|
||
| private LocalDate targetDate; | ||
|
|
||
| /** | ||
| * 예약 가능 시작 시간 목록 | ||
| */ | ||
| @Column(columnDefinition = "jsonb") | ||
| @Convert(converter = AvailableStartTimesConverter.class) | ||
| private AvailableStartTimes availableTimes; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PhotographerSchedule.java" -type fRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 172
🏁 Script executed:
cat -n ./reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.javaRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 4083
🏁 Script executed:
cat -n ./reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.javaRepository: CUKCatSnap/CUKCATSNAP_Server
Length of output: 3975
JPA 관계 매핑 오류 수정 필요
PhotographerSchedule.java의 @OneToMany(mappedBy = "photographerId")가 ScheduleOverride.java의 photographerId 필드를 참조하고 있으나, 이 필드는 단순 Long 타입입니다. mappedBy는 반드시 @ManyToOne으로 선언된 관계 필드를 참조해야 하므로 현재 설정으로는 JPA 매핑이 정상 작동하지 않습니다.
다음 중 하나를 선택하여 수정하세요:
방안 1: 양방향 관계로 변경
// ScheduleOverride.java
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "photographer_schedule_id")
private PhotographerSchedule photographerSchedule;
// PhotographerSchedule.java
@OneToMany(mappedBy = "photographerSchedule", cascade = CascadeType.ALL, orphanRemoval = true)
private List<ScheduleOverride> overrides = new ArrayList<>();방안 2: 단방향 관계로 유지
// PhotographerSchedule.java
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
@JoinColumn(name = "photographer_id", referencedColumnName = "photographerId")
private List<ScheduleOverride> overrides = new ArrayList<>();🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.java
around lines 22 to 40, the photographerId field is a plain Long but
PhotographerSchedule uses @OneToMany(mappedBy = "photographerId"), which is
invalid because mappedBy must reference an entity relationship field; fix by
converting the association to a proper relation: either (A) make
ScheduleOverride hold a @ManyToOne to PhotographerSchedule (with appropriate
@JoinColumn) and update PhotographerSchedule to use @OneToMany(mappedBy =
"<thatFieldName>", cascade and orphanRemoval as needed), or (B) keep it
unidirectional by removing mappedBy from PhotographerSchedule and instead
annotate its @OneToMany with @JoinColumn(name = "photographer_id",
referencedColumnName = "photographerId") so JPA knows the FK column; choose one
approach and ensure field names, fetch/cascade settings, and column names are
consistent and remove the plain Long photographerId if switching to @ManyToOne.
| private Long photographerId; | ||
|
|
||
| private LocalDate targetDate; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
DB 제약 조건 누락 - @Column(nullable = false) 추가 권장
photographerId와 targetDate는 validation 로직에서 null을 허용하지 않지만, DB 스키마 레벨에서는 제약이 없습니다. 데이터 무결성을 위해 @Column 어노테이션을 추가하는 것이 좋습니다.
🔎 수정 제안
- private Long photographerId;
+ @Column(nullable = false)
+ private Long photographerId;
- private LocalDate targetDate;
+ @Column(nullable = false)
+ private LocalDate targetDate;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Long photographerId; | |
| private LocalDate targetDate; | |
| @Column(nullable = false) | |
| private Long photographerId; | |
| @Column(nullable = false) | |
| private LocalDate targetDate; |
🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.java
around lines 31 to 33, the fields photographerId and targetDate lack DB-level
NOT NULL constraints; add @Column(nullable = false) to both fields, import
javax.persistence.Column if not already present, and ensure any ORM mapping or
migration scripts are updated so the database schema reflects these non-null
constraints.
| @Override | ||
| public String toString() { | ||
| return String.format( | ||
| "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}", | ||
| id, photographerId, targetDate); | ||
| } |
There was a problem hiding this comment.
클래스명과 toString() 불일치
toString() 메서드가 "AvailabilityOverride"를 반환하지만 클래스명은 ScheduleOverride입니다. 일관성을 위해 수정이 필요합니다.
🔎 수정 제안
@Override
public String toString() {
return String.format(
- "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}",
+ "ScheduleOverride{id=%d, photographerId=%d, targetDate=%s}",
id, photographerId, targetDate);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public String toString() { | |
| return String.format( | |
| "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}", | |
| id, photographerId, targetDate); | |
| } | |
| @Override | |
| public String toString() { | |
| return String.format( | |
| "ScheduleOverride{id=%d, photographerId=%d, targetDate=%s}", | |
| id, photographerId, targetDate); | |
| } |
🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverride.java
around lines 108 to 113, the toString() output uses "AvailabilityOverride" while
the class is ScheduleOverride; update the returned string to match the class
name (e.g., "ScheduleOverride{...}") or use a dynamic class name via
getClass().getSimpleName() so the toString() and actual class name remain
consistent.
| private static final ObjectMapper objectMapper = new ObjectMapper() | ||
| .registerModule(new JavaTimeModule()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
JavaTimeModule 등록이 현재 구현에서는 불필요
LocalTime을 직접 toString()/parse()로 변환하고 있어 JavaTimeModule이 실제로 사용되지 않습니다. 현재 코드가 의도된 동작이라면 모듈 등록을 제거하거나, 향후 확장성을 위해 유지한다면 주석으로 명시하는 것이 좋습니다.
🤖 Prompt for AI Agents
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverter.java
around lines 22-23: the ObjectMapper currently registers JavaTimeModule but
LocalTime is being serialized/deserialized manually via toString()/parse(), so
the module is unused; either remove the .registerModule(new JavaTimeModule())
call to avoid dead configuration, or if you want to keep it for future
extensibility, add a clear inline comment explaining that JavaTimeModule is
intentionally registered for potential future date/time types and why LocalTime
is still handled manually.
| List<LocalTime> times = timeStrings.stream() | ||
| .map(LocalTime::parse) | ||
| .toList(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
DateTimeParseException 처리 고려
LocalTime.parse()가 잘못된 형식의 문자열에 대해 DateTimeParseException을 던질 수 있습니다. 현재 JsonProcessingException만 catch하고 있어, 손상된 데이터가 있을 경우 예외가 전파됩니다.
🔎 예외 처리 개선 제안
@Override
public AvailableStartTimes convertToEntityAttribute(String dbData) {
if (dbData == null || dbData.isBlank()) {
return AvailableStartTimes.empty();
}
try {
List<String> timeStrings = objectMapper.readValue(
dbData,
new TypeReference<List<String>>() {
}
);
if (timeStrings.isEmpty()) {
return AvailableStartTimes.empty();
}
List<LocalTime> times = timeStrings.stream()
.map(LocalTime::parse)
.toList();
return AvailableStartTimes.of(times);
- } catch (JsonProcessingException e) {
+ } catch (JsonProcessingException | java.time.format.DateTimeParseException e) {
throw new IllegalArgumentException("JSON을 AvailableStartTimes로 변환 실패", e);
}
}🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/AvailableStartTimesConverter.java
around lines 59 to 61, the code maps timeStrings to LocalTime using
LocalTime.parse but only catches JsonProcessingException elsewhere; add handling
for DateTimeParseException by either filtering/validating parseable strings or
catching DateTimeParseException and rethrowing/wrapping it as a
JsonProcessingException with a clear message (or logging and skipping invalid
entries) so malformed time strings don't propagate unchecked; update the
try/catch to include DateTimeParseException (or validate each string before
parse) and ensure the thrown JsonProcessingException includes which value failed
and why.
| DayOfWeek dayOfWeek = DayOfWeek.valueOf(entry.getKey()); | ||
| Map<String, Object> ruleMap = entry.getValue(); | ||
|
|
||
| boolean isWorkingDay = (Boolean) ruleMap.get("isWorkingDay"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Boolean 캐스팅 시 ClassCastException 가능성.
JSON에서 읽은 값이 예상과 다른 타입일 경우 ClassCastException이 발생할 수 있습니다. 방어적 코딩을 고려해 보세요.
🔎 방어적 캐스팅 제안
- boolean isWorkingDay = (Boolean) ruleMap.get("isWorkingDay");
+ Object isWorkingDayObj = ruleMap.get("isWorkingDay");
+ boolean isWorkingDay = Boolean.TRUE.equals(isWorkingDayObj);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| boolean isWorkingDay = (Boolean) ruleMap.get("isWorkingDay"); | |
| Object isWorkingDayObj = ruleMap.get("isWorkingDay"); | |
| boolean isWorkingDay = Boolean.TRUE.equals(isWorkingDayObj); |
🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.java
around line 83, the direct cast (Boolean) ruleMap.get("isWorkingDay") can throw
ClassCastException for unexpected JSON types; replace it with defensive
extraction: read the raw Object, handle null, then determine boolean by checking
types (Boolean -> cast, Number -> non‑zero -> true, String ->
Boolean.parseBoolean/trim, otherwise default false) and optionally log a warning
when encountering an unexpected type so parsing is robust.
| if (!isWorkingDay) { | ||
| result.put(dayOfWeek, WeekdayScheduleRule.dayOff(dayOfWeek)); | ||
| } else { | ||
| @SuppressWarnings("unchecked") | ||
| List<String> timeStrings = (List<String>) ruleMap.get("availableStartTimes"); | ||
|
|
||
| if (timeStrings != null && !timeStrings.isEmpty()) { | ||
| List<LocalTime> timeList = timeStrings.stream() | ||
| .map(LocalTime::parse) | ||
| .toList(); | ||
|
|
||
| AvailableStartTimes availableStartTimes = AvailableStartTimes.of(timeList); | ||
|
|
||
| result.put(dayOfWeek, WeekdayScheduleRule.workingDay( | ||
| dayOfWeek, | ||
| availableStartTimes | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
근무일인데 시간 목록이 없는 경우 데이터가 누락됩니다.
isWorkingDay가 true이지만 availableStartTimes가 null이거나 비어있는 경우, 해당 요일이 결과 맵에 추가되지 않습니다. 이로 인해 데이터 손실이 발생할 수 있습니다.
🔎 수정 제안
if (!isWorkingDay) {
result.put(dayOfWeek, WeekdayScheduleRule.dayOff(dayOfWeek));
} else {
@SuppressWarnings("unchecked")
List<String> timeStrings = (List<String>) ruleMap.get("availableStartTimes");
if (timeStrings != null && !timeStrings.isEmpty()) {
List<LocalTime> timeList = timeStrings.stream()
.map(LocalTime::parse)
.toList();
AvailableStartTimes availableStartTimes = AvailableStartTimes.of(timeList);
result.put(dayOfWeek, WeekdayScheduleRule.workingDay(
dayOfWeek,
availableStartTimes
));
+ } else {
+ // 근무일로 표시되었지만 시간이 없는 경우 휴무로 처리하거나 예외 발생
+ result.put(dayOfWeek, WeekdayScheduleRule.dayOff(dayOfWeek));
}
}🤖 Prompt for AI Agents
In
reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/infrastructure/converter/WeekdayRulesConverter.java
around lines 85-103, when isWorkingDay is true but availableStartTimes is null
or empty the code currently skips adding the day to the result map; ensure you
always add a workingDay entry: if timeStrings is null or empty create an empty
AvailableStartTimes (e.g. AvailableStartTimes.of(Collections.emptyList()) or
AvailableStartTimes.empty() depending on the API) and put
WeekdayScheduleRule.workingDay(dayOfWeek, thatEmptyAvailableStartTimes) into the
result map so the day is not omitted.
| @Test | ||
| void toString이_올바르게_동작한다() { | ||
| // given | ||
| Long photographerId = 1L; | ||
| LocalDate targetDate = LocalDate.of(2024, 1, 15); | ||
| ScheduleOverride override = ScheduleOverride.dayOff(photographerId, targetDate); | ||
|
|
||
| // when | ||
| String result = override.toString(); | ||
|
|
||
| // then | ||
| assertThat(result).contains("AvailabilityOverride"); | ||
| assertThat(result).contains("photographerId=" + photographerId); | ||
| assertThat(result).contains("targetDate=" + targetDate); | ||
| } |
There was a problem hiding this comment.
클래스명과 toString() 출력 간 불일치 확인 필요
테스트에서 "AvailabilityOverride"를 검증하고 있는데, 클래스명은 ScheduleOverride입니다. ScheduleOverride.java Line 110-112의 toString() 메서드가 "AvailabilityOverride{...}"를 반환하고 있어 일관성이 떨어집니다.
도메인 클래스의 toString() 출력을 클래스명과 일치시키는 것이 좋습니다.
🔎 ScheduleOverride.java 수정 제안
@Override
public String toString() {
return String.format(
- "AvailabilityOverride{id=%d, photographerId=%d, targetDate=%s}",
+ "ScheduleOverride{id=%d, photographerId=%d, targetDate=%s}",
id, photographerId, targetDate);
}테스트도 함께 수정:
-assertThat(result).contains("AvailabilityOverride");
+assertThat(result).contains("ScheduleOverride");Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/ScheduleOverrideTest.java
lines 153-167 and ScheduleOverride.java lines 110-112: the test asserts the
toString contains "AvailabilityOverride" but the domain class is
ScheduleOverride; change the ScheduleOverride.toString() implementation to
return a string that uses the correct class name "ScheduleOverride{...}" (adjust
the prefix only, keep the same field formatting), then run the test to ensure it
now passes; do not change the test assertions.
📌 관련 이슈
✨ PR 세부 내용
작가가 자신의 예약 시간표를 정의하는 도메인을 개발했습니다.
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.