Merged
Conversation
Closed
📝 WalkthroughWalkthrough예약 생성 기능을 추가합니다. 애플리케이션 계층에서 Program·PhotographerSchedule 조회 후 ReservationFactory가 검증(프로그램/스케줄/중복)하여 Reservation을 생성하고 저장합니다. 시간 표현이 LocalDateTime 기반으로 변경되었습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as ReservationController
participant Service as ReservationService
participant ProgramRepo as ProgramRepository
participant ScheduleRepo as PhotographerScheduleRepository
participant ResRepo as ReservationRepository
participant Factory as ReservationFactory
participant Domain as DomainModels
Client->>Controller: POST /reservation (programId, date, time)
Controller->>Service: createReservation(modelId, request)
Service->>ProgramRepo: findById(programId)
ProgramRepo-->>Service: Program
Service->>Domain: program.ensureBookable()
Service->>ScheduleRepo: findByPhotographerId(...)
ScheduleRepo-->>Service: PhotographerSchedule
Service->>ResRepo: findAll(activeReservationsOf)
ResRepo-->>Service: List<Reservation>
Service->>Factory: create(modelId, program, schedule, existing, date, time, holdExpiresAt)
Factory->>Domain: program.ensureBookable()
Factory->>Domain: schedule.ensureAvailable(date, time, today)
Factory->>Domain: timeSlot.overlaps(existing)
Factory->>Domain: Reservation.hold(...)
Factory-->>Service: Reservation
Service->>ResRepo: save(reservation)
Service-->>Controller: ReservationCreateResponse
Controller-->>Client: 201 Created (reservationNumber, amount, holdExpiresAt)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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)377 tests 377 ✅ 9s ⏱️ Results for commit afbabc7. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/dto/request/ReservationCreateRequest.java`:
- Around line 10-19: The request DTO ReservationCreateRequest currently only has
`@NotNull` for programId and reservationDate; add validation to reject negative
program IDs and past dates earlier by annotating the programId field with
`@Positive` and the reservationDate field with `@FutureOrPresent`, and add the
corresponding imports (javax.validation.constraints.Positive and
javax.validation.constraints.FutureOrPresent) so validation triggers in the
presentation layer; keep the existing `@NotNull` annotations and no other
signature changes to the ReservationCreateRequest record.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java`:
- Around line 51-86: The createReservation method has a TOCTOU race between
reservationRepository.findAll(activeReservationsOf(...)) and
reservationRepository.save(...) that can allow double-booking; update this
method to address concurrency by either (a) using a pessimistic lock when
loading the photographer's schedule or reservations (e.g., replace the
findByPhotographerId / findAll calls with repository methods that perform SELECT
... FOR UPDATE on PhotographerSchedule or reservations for that photographer),
or (b) enforce a DB-level unique constraint on photographer+timeslot (and handle
unique-constraint violations by translating to a DomainException and retrying if
desired), or (c) implement optimistic locking with retries; if you cannot
implement a full fix now, add a clear TODO comment in createReservation
referencing reservationRepository.findAll(activeReservationsOf(...)) and
reservationRepository.save(...) that states concurrency handling
(pessimistic/unique-constraint/optimistic) must be implemented before
production.
- Around line 38-49: Replace the direct instantiation of ReservationFactory
inside ReservationService with constructor injection: add a ReservationFactory
parameter to the ReservationService constructor and assign it to
this.reservationFactory (remove new ReservationFactory()), and ensure
ReservationFactory is provided as a Spring bean (e.g., annotate
ReservationFactory or configure it in a `@Configuration`) so tests can mock or
inject a test double for ReservationFactory when constructing ReservationService
in unit tests.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationSpecification.java`:
- Around line 53-64: onDate currently only checks Reservation_.timeSlot ->
ReservationTimeSlot_.startDateTime against the day's range so reservations that
start before midnight and end during the target date are missed; update the
Specification onDate(LocalDate) to detect any time-slot overlap with the day by
checking ReservationTimeSlot_.endDateTime as well (e.g., include predicates for
endDateTime within the day and/or a general overlap check: start <= dayEnd AND
end >= dayStart) and combine these with cb.or/cb.and so all reservations
spanning midnight are returned.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java`:
- Around line 142-146: ensureAvailable currently only checks
getAvailableStartTimesAt(date) and misses rejecting past dates; update
PhotographerSchedule.ensureAvailable(LocalDate date, LocalTime startTime) to
first verify the date is not in the past (same logic used by isAvailableAt) and
throw DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, "해당 시간대는 예약할
수 없습니다.") when date.isBefore(LocalDate.now()) so ensureAvailable and
isAvailableAt behave consistently for past dates before checking startTime in
getAvailableStartTimesAt(date).
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java`:
- Around line 147-155: The test uses a fully-qualified call to
org.assertj.core.api.Assertions.assertThatCode instead of the static import used
for other assertion helpers; add a static import for assertThatCode at the top
of ProgramTest (where other static imports like assertThat/assertThatThrownBy
live) and replace the fully-qualified invocation in the
활성_프로그램은_ensureBookable이_성공한다 test so it calls
assertThatCode(program::ensureBookable) to match the file's import style and
consistency for the ensureBookable assertion.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java`:
- Around line 53-58: The `@AfterEach` cleanup method (cleanup calling
reservationRepository.deleteAll(), programRepository.deleteAll(),
photographerScheduleRepository.deleteAll()) is redundant for tests annotated
with `@Transactional`; either remove the cleanup method and rely on transactional
rollback for those tests, or move `@Transactional` to the class level
(ReservationServiceIntegrationTest) and then delete the `@AfterEach` cleanup to
avoid duplicate teardown for all tests while keeping non-transactional tests
adjusted as needed (e.g., convert them to transactional or add targeted cleanup
only for those specific tests).
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java`:
- Around line 168-194: Add a test that verifies exception-rules override weekday
rules for PhotographerSchedule.ensureAvailable: create a schedule via
PhotographerSchedule.initSchedule(...), set a weekday rule with
AvailableStartTimes containing 10:00 using updateWeekdayRule(...), then set an
exception/day-off rule for the specific targetDate (use the project’s exception
rule API—e.g., updateExceptionRule or equivalent) that either marks the date as
day-off or only exposes 14:00, and finally call ensureAvailable(targetDate,
LocalTime.of(10,0)) asserting that it throws DomainException via
assertThatThrownBy(...). Ensure the new test mirrors the isAvailableAt override
scenario but uses ensureAvailable and references ensureAvailable,
PhotographerSchedule, updateWeekdayRule, and DomainException.
...catsnap/CatsnapReservation/reservation/application/dto/request/ReservationCreateRequest.java
Show resolved
Hide resolved
...src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java
Show resolved
Hide resolved
...src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java
Show resolved
Hide resolved
...tsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationSpecification.java
Show resolved
Hide resolved
...ation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
Outdated
Show resolved
Hide resolved
reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java
Show resolved
Hide resolved
...et/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java
Show resolved
Hide resolved
...n/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
예약 생성 기능을 위해 POST /reservation API를 추가하고, 예약 가능 여부(프로그램/스케줄/기존 예약 겹침) 검증을 도메인 서비스로 분리하여 예약 생성 플로우를 완성하는 PR입니다.
Changes:
POST /reservation예약 생성 API(Controller/Service/DTO) 추가- 예약 시간대 VO를
startDateTime/endDateTime기반으로 리팩터링하고overlaps로 중복 검증 도입 - 예약 조회용
Specification/Repository(JpaSpecificationExecutor)구성 및 관련 테스트 보강
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java | 스케줄 가용성 검증/가용 시간 조회 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/presentation/ReservationControllerTest.java | 예약 생성 API 컨트롤러 요청/권한/검증 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationSpecificationTest.java | 활성 예약 조회 Specification 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/fixture/ReservationFixture.java | 시간대 구조 변경(LocalDateTime 기반)에 맞춰 픽스처 수정 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/domain/vo/ReservationTimeSlotTest.java | timeSlot 구조 변경 및 overlaps 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/domain/ReservationFactoryTest.java | 도메인 팩토리(예약 생성 규칙) 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java | 예약 생성 서비스 통합 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/fixture/ProgramFixture.java | id 주입 가능한 Program fixture 메서드 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/vo/DurationTest.java | Duration.addTo(LocalDateTime) 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java | Program.ensureBookable 테스트 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/shared/domain/error/DomainErrorCode.java | NOT_FOUND/CONFLICT 에러코드 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java | ensureAvailable, getAvailableStartTimesAt 추가 및 isAvailableAt 개선 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/presentation/ReservationController.java | 예약 생성 API 컨트롤러 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationSpecification.java | 활성 예약 조회용 Specification 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationRepository.java | JpaSpecificationExecutor 기반 Repository 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/domain/vo/ReservationTimeSlot.java | 시간대 VO를 LocalDateTime 기반으로 변경 + overlaps 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/domain/ReservationFactory.java | 예약 생성 도메인 서비스(팩토리) 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/domain/Reservation.java | timeSlot 설명 주석 업데이트 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/dto/response/ReservationCreateResponse.java | 예약 생성 응답 DTO 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/dto/request/ReservationCreateRequest.java | 예약 생성 요청 DTO 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java | 예약 생성 애플리케이션 서비스 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Duration.java | Duration.addTo(LocalDateTime) 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/Program.java | Program.ensureBookable 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
Outdated
Show resolved
Hide resolved
...tsnap/CatsnapReservation/reservation/infrastructure/repository/ReservationSpecification.java
Outdated
Show resolved
Hide resolved
...src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java`:
- Around line 142-145: ensureAvailable currently calls LocalDate.now() directly
(same pattern in cleanupPastOverrides), which hinders testability; change
ensureAvailable(LocalDate date, LocalTime startTime) to accept a reference date
(e.g., LocalDate today) or a Clock parameter so the method compares against the
injected date/clock instead of LocalDate.now(), and update callers (and
cleanupPastOverrides) to pass in the current date/clock from the service layer;
ensure you update method signatures and all usages of
PhotographerSchedule.ensureAvailable and
PhotographerSchedule.cleanupPastOverrides to use the new parameter so tests can
supply deterministic dates.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java`:
- Around line 196-211: Test uses a past date so it triggers the "past date"
validation instead of the intended "time slot unavailable" path; change the test
data to a future date (e.g., use LocalDate.now().plusDays(1) or a hardcoded
future year) in the test method 예약_불가능한_시간대이면_예외가_발생한다(), keep saveProgram(),
saveSchedule(date, LocalTime.of(10, 0)) and the
ReservationCreateRequest(program.getId(), date, LocalTime.of(14, 0)) logic the
same, and then assert reservationService.createReservation(MODEL_ID, request)
throws DomainException with message "해당 시간대는 예약할 수 없습니다".
- Around line 213-231: The test 기존_예약과_시간이_겹치면_예외가_발생한다() uses a hard-coded past
date (LocalDate.of(2025, 6, 16)) causing the first
reservationService.createReservation(MODEL_ID, request) to fail with a past-date
validation before the duplicate check; update the test to use a future date
(e.g., LocalDate.now().plusDays(1) or another date guaranteed to be after today)
when calling saveSchedule(...) and constructing ReservationCreateRequest so the
first reservation succeeds and the duplicate-time assertion can run.
- Around line 78-94: The tests in ReservationServiceIntegrationTest use a
hardcoded past date (LocalDate.of(2025,6,16)) which now fails because
PhotographerSchedule.ensureAvailable rejects past dates; replace every
occurrence in this file with a dynamically computed future date (e.g., a
FUTURE_DATE constant or LocalDate.now().plusDays(N) that falls on the same
weekday your saveSchedule expects) and update any assertions that validate
startDateTime/endDateTime (e.g., in 생성된_예약이_DB에_PENDING_상태로_저장된다) to compute
expected datetimes from that same FUTURE_DATE so the schedule setup
(saveSchedule) and ReservationService.createReservation still align. Ensure
references to saveSchedule, ReservationServiceIntegrationTest, and
PhotographerSchedule.ensureAvailable are used to locate and adjust all affected
tests.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java`:
- Around line 196-209: Add a boundary test that verifies today is allowed:
create a new test (e.g., 오늘_날짜는_예약_가능하다) that initializes PhotographerSchedule
via PhotographerSchedule.initSchedule(1L), obtains LocalDate today =
LocalDate.now(), gets DayOfWeek today.getDayOfWeek(), sets an available time
with schedule.updateWeekdayRule(dayOfWeek,
AvailableStartTimes.of(List.of(LocalTime.of(10,0)))), then call
schedule.ensureAvailable(today, LocalTime.of(10,0)) and assert it does not throw
(use assertDoesNotThrow or equivalent) to confirm ensureAvailable accepts today.
...ation/src/main/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerSchedule.java
Outdated
Show resolved
Hide resolved
...et/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java
Show resolved
Hide resolved
...et/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java
Show resolved
Hide resolved
...et/catsnap/CatsnapReservation/reservation/application/ReservationServiceIntegrationTest.java
Show resolved
Hide resolved
...n/src/test/java/net/catsnap/CatsnapReservation/schedule/domain/PhotographerScheduleTest.java
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java`:
- Around line 67-68: The code in ReservationService calls LocalDate.now(clock)
and LocalDateTime.now(clock) separately which can yield inconsistent values
around midnight; instead capture a single LocalDateTime snapshot (e.g., now =
LocalDateTime.now(clock)) and derive both today = now.toLocalDate() and
holdExpiresAt = now.plusMinutes(HOLD_MINUTES) so both are based on the same
instant; update usages that reference today and holdExpiresAt accordingly.
...src/main/java/net/catsnap/CatsnapReservation/reservation/application/ReservationService.java
Show resolved
Hide resolved
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
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.
📌 관련 이슈
✨ PR 세부 내용
주요 변경사항
POST /reservation예약 생성 API 추가ReservationService#createReservation추가ReservationFactory도메인 서비스 추가ReservationSpecification#activeReservationsOf추가 (작가 + 날짜 + 활성 상태)ReservationRepository를JpaSpecificationExecutor기반으로 구성Program#ensureBookable추가PhotographerSchedule#ensureAvailable,getAvailableStartTimesAt추가Duration#addTo(LocalDateTime)추가ReservationTimeSlot구조 변경reservationDate + startTime + endTimestartDateTime + endDateTimeoverlaps기반 중복 검증 로직 추가DomainErrorCode에NOT_FOUND,CONFLICT추가API
POST /reservation@LoginModel(모델 권한 필요)programId(Long)reservationDate(LocalDate)startTime(LocalTime)201 Created)reservationNumber(String)amount(Long)holdExpiresAt(LocalDateTime)도메인 규칙
PENDING,CONFIRMED예약과 시간대가 겹치면 예약 불가holdExpiresAt = now + 15분테스트
ReservationFactory,ReservationTimeSlot,Program,Duration,PhotographerScheduleReservationSpecificationSummary by CodeRabbit
새로운 기능
테스트