[FEATURE]: 가게 심야 영업 허용 및 예약-브레이크타임 충돌 방어 로직 구현#128
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough브레이크타임 예약 충돌 검사로 인한 지연(effectiveDate) 적용 흐름과 예정 브레이크타임 보관/적용을 위한 엔티티·레포·스케줄러가 추가되었고, 심야·24시간 케이스를 반영한 검증·조회 로직 및 DTO/컨트롤러/상태 메시지·로깅이 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as "Client"
participant Controller as "BusinessHoursController"
participant Service as "BusinessHoursCommandServiceImpl"
participant BookingRepo as "BookingRepository"
participant BHRepo as "BusinessHoursRepository"
participant Scheduler as "BusinessHoursScheduler"
participant DB as "Database"
Client->>Controller: updateBreakTime(storeId, dto)
Controller->>Service: updateBreakTime(storeId, dto)
Service->>BookingRepo: findLastConflictingDate(storeId, breakStart, breakEnd, adjustedStart)
BookingRepo-->>Service: Optional<lastConflictingDate>
alt conflict exists
Service->>Service: effectiveDate = lastConflictingDate.plusDays(1)
Service->>BHRepo: save pending newBreakStart/newBreakEnd + effectiveDate
BHRepo->>DB: persist pending fields
Service-->>Controller: UpdateBreakTimeDto(effectiveDate=future)
Controller-->>Client: 200 OK (DELAYED)
else no conflict
Service->>Service: effectiveDate = today
Service->>BHRepo: apply immediate breakStart/breakEnd
BHRepo->>DB: persist immediate changes
Service-->>Controller: UpdateBreakTimeDto(effectiveDate=today)
Controller-->>Client: 200 OK (SUCCESS)
end
Note over Scheduler,BHRepo: 매일 00:00 실행
Scheduler->>BHRepo: findAllByEffectiveDateLessThanEqualAndEffectiveDateIsNotNull(today)
BHRepo-->>Scheduler: pending list
loop 각 pending 항목
Scheduler->>Scheduler: transactionTemplate.execute -> processEachPendingTime(id)
Scheduler->>DB: load BusinessHours, applyPendingBreakTime 또는 clearPendingBreakTime
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BusinessHoursValidator.java (1)
36-46: 🛠️ Refactor suggestion | 🟠 Major
validateOpenCloseTime이 사실상 no-op 메서드가 되었습니다.24시간 영업(open == close)인 경우
continue하고, 그 외 모든 경우에는 아무런 검증 없이 통과합니다. 심야 영업 허용을 위해 기존open < close검증을 제거한 의도는 이해하지만, 현재 이 메서드는 실질적으로 아무것도 검증하지 않습니다.
- 메서드가 불필요하다면 호출부(
validateForCreate,validateForUpdate)에서 제거하세요.- 혹은 24시간 영업 허용을 별도 로직으로 분리하고, 메서드명/주석(Line 36:
// open < close 검증)도 현재 동작에 맞게 수정하세요.♻️ 메서드 제거 또는 정리 제안
옵션 1: 메서드 및 호출부 제거
- // open < close 검증 - private static void validateOpenCloseTime(List<BusinessHoursReqDto.Summary> dto) { - for(BusinessHoursReqDto.Summary s: dto) { - if(!s.isClosed()){ - // 24시간 영업 허용 - if(s.openTime().equals(s.closeTime())) { - continue; - } - } - } - }옵션 2: 의미 있는 검증 유지 (예: 24시간 영업이 아닌데 open == close인 케이스 방지 등)
- // open < close 검증 + // 영업시간 유효성 검증 (24시간 영업 및 심야 영업 허용) private static void validateOpenCloseTime(List<BusinessHoursReqDto.Summary> dto) { for(BusinessHoursReqDto.Summary s: dto) { if(!s.isClosed()){ // 24시간 영업 허용 if(s.openTime().equals(s.closeTime())) { continue; } + // 심야 영업 허용 (open > close)이므로 추가 시간 검증 불필요 } } }
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/dto/BusinessHoursReqDto.java`:
- Around line 48-51: The `@Schema` description on the breakEndTime field in
BusinessHoursReqDto is incorrect; update the annotation on the field named
breakEndTime (in class BusinessHoursReqDto) to use the correct description "브레이크
종료 시간" instead of "브레이크 시작 시간" so the OpenAPI docs reflect the field's purpose
accurately.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java`:
- Around line 19-20: The enum constant _BREAK_TIME_OUT_OF_BUSINESS_HOURS in
BusinessHoursErrorStatus is unused and should be removed; delete the
_BREAK_TIME_OUT_OF_BUSINESS_HOURS entry from the BusinessHoursErrorStatus enum,
ensure the enum punctuation (commas/semicolon) remains valid after removal, and
verify BreakTimeValidator references use _INVALID_BREAK_TIME (no other code
references _BREAK_TIME_OUT_OF_BUSINESS_HOURS).
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BreakTimeValidator.java`:
- Around line 17-18: The comment and behavior in BreakTimeValidator are
inconsistent: the comment says "no break times for 24-hour operation" but the
code just returns (allowing breaks to be saved), which causes 24-hour stores to
retain breaks; update BreakTimeValidator to enforce the intended rule by
throwing a ValidationException (or the project's equivalent) when
openTime.equals(closeTime) to prevent break times from being saved for 24-hour
operation (refer to BreakTimeValidator and ensure
BusinessHoursCommandServiceImpl will surface the exception), or if the intended
behavior is to allow breaks for 24-hour stores then change the comment text to
reflect that decision; modify only BreakTimeValidator (and use the same
exception type used elsewhere in the service) and ensure tests/business logic
that reference StoreQueryServiceImpl remain consistent.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`:
- Around line 117-122: Move the break-time check so it runs after the
business-hours-range check inside StoreQueryServiceImpl (the logic using
variables like bh, time and isToday); currently the break-time block (checking
bh.getBreakStartTime()/getBreakEndTime() and comparing time) executes before
verifying the overall open-hours range—reorder the code so you first validate
whether the requested time falls within the store's business hours range and
only if it does perform the break-time comparison, keeping the same null-checks
on bh.getBreakStartTime()/getBreakEndTime() and the same time comparisons.
src/main/java/com/eatsfine/eatsfine/domain/businesshours/dto/BusinessHoursReqDto.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BreakTimeValidator.java
Outdated
Show resolved
Hide resolved
Comment on lines
+31
to
+38
| } else { | ||
| // 심야 영업: [close ~ open] 사이(영업 안 하는 시간)에 브레이크 타임이 있으면 에러 | ||
| if (!breakStartTime.isBefore(closeTime) && breakStartTime.isBefore(openTime)) { | ||
| throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME); | ||
| } | ||
| if (!breakEndTime.isBefore(closeTime) && breakEndTime.isBefore(openTime)) { | ||
| throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME); | ||
| } |
There was a problem hiding this comment.
심야 영업 브레이크 타임 검증에서 비영업 구간을 걸치는 경우가 감지되지 않습니다.
현재 로직은 breakStartTime과 breakEndTime을 각각 독립적으로 비영업 구간([close, open))에 포함되는지 검증합니다. 그러나 두 끝점이 모두 영업 구간에 있으면서 브레이크가 비영업 구간을 관통하는 경우(예: open=22:00, close=04:00에서 break=03:00~23:00)는 감지하지 못합니다.
Line 21의 breakEnd > breakStart 제약으로 인해 자정을 넘는 브레이크는 불가하므로 실질적으로 발생 가능성은 낮지만, 브레이크 시간이 실제 영업 시간보다 긴 경우를 추가로 검증하면 더 안전합니다.
src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java
Outdated
Show resolved
Hide resolved
twodo0
force-pushed
the
refactor/night-open
branch
from
c59ff83 to
60f43a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursSuccessStatus.java (1)
23-40:⚠️ Potential issue | 🔴 Critical
isSuccess(false)— Success 상태인데isSuccess가false로 설정되어 있습니다.
BusinessHoursSuccessStatus는 성공 응답을 나타내는 enum인데,getReason()(Line 26)과getReasonHttpStatus()(Line 36) 모두isSuccess(false)로 빌드하고 있습니다. API 응답에서 성공임에도isSuccess: false가 반환되어 클라이언트 측에서 실패로 오인할 수 있습니다.🐛 수정 제안
`@Override` public ReasonDto getReason() { return ReasonDto.builder() - .isSuccess(false) + .isSuccess(true) .code(code) .message(message) .build(); } `@Override` public ReasonDto getReasonHttpStatus() { return ReasonDto.builder() .httpStatus(httpStatus) - .isSuccess(false) + .isSuccess(true) .code(code) .message(message) .build(); }src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BusinessHoursValidator.java (1)
37-46:⚠️ Potential issue | 🟠 Major
validateOpenCloseTime메서드는 실질적으로 검증을 수행하지 않는 no-op 메서드입니다.메서드는 현재 24시간 영업(
openTime.equals(closeTime)) 경우에만continue를 실행하고, 그 외 모든 경우에는 아무 검증도 수행하지 않습니다. 특히:
- 심야 영업 지원을 위해 기존 검증이 제거되었으나, 대체 로직이 추가되지 않아 유효하지 않은 시간 조합(예: 영업시작 10:00, 종료 08:00)도 무조건 통과합니다.
- 미사용 에러 상태:
_INVALID_BUSINESS_TIME(BusinessHoursErrorStatusLine 15)은 정의되어 있지만 코드 어디에서도 사용되지 않습니다.- 메서드 이름과 실제 동작의 불일치: 36번 라인 주석은 "open < close 검증"이라 명시하지만, 실제로는 검증이 없습니다.
- 활성 호출:
validateForCreate(19번 라인),validateForUpdate(26번 라인)에서 호출되고 있어 미사용 코드가 아닙니다.해결 방안:
- 메서드를 완전히 제거하거나 (의도한 검증 없음)
- 명확한 검증 로직 추가 (예: 심야 영업 허용 시
openTime < closeTime || (openTime > closeTime && 심야 영업 플래그)형태)src/main/java/com/eatsfine/eatsfine/domain/businesshours/controller/BusinessHoursController.java (2)
55-66:⚠️ Potential issue | 🔴 Critical
updateBreakTime이 두 번 호출되며,effectiveDateNPE 위험이 있습니다.두 가지 심각한 문제가 있습니다:
서비스 이중 호출: 즉시 반영(non-delayed) 경로에서 Line 55와 Line 65에서
updateBreakTime이 두 번 호출됩니다. 첫 번째 호출에서 이미 데이터가 변경되므로, 불필요한 중복 쓰기가 발생합니다.NPE 위험: Line 58에서
result.effectiveDate()가null일 수 있습니다 (즉시 반영 시effectiveDate가 null로 설정될 가능성).null.isAfter()는NullPointerException을 발생시킵니다.🐛 서비스 단일 호출 및 null-safe 처리 제안
){ - BusinessHoursResDto.UpdateBreakTimeDto result = businessHoursCommandService.updateBreakTime(storeId, dto, user.getUsername()); - - // 1. 날짜가 미래라면 DELAYED(지연) 코드 반환 - if (result.effectiveDate().isAfter(LocalDate.now())) { - return ApiResponse.of( - BusinessHoursSuccessStatus._UPDATE_BREAKTIME_DELAYED, - result); - } - return ApiResponse.of( - BusinessHoursSuccessStatus._UPDATE_BREAKTIME_SUCCESS, - businessHoursCommandService.updateBreakTime(storeId, dto, user.getUsername()) - ); + BusinessHoursResDto.UpdateBreakTimeDto result = businessHoursCommandService.updateBreakTime(storeId, dto, user.getUsername()); + + if (result.effectiveDate() != null && result.effectiveDate().isAfter(LocalDate.now())) { + return ApiResponse.of( + BusinessHoursSuccessStatus._UPDATE_BREAKTIME_DELAYED, + result); + } + return ApiResponse.of( + BusinessHoursSuccessStatus._UPDATE_BREAKTIME_SUCCESS, + result + ); }
50-53:⚠️ Potential issue | 🟡 Minor
@RequestBody파라미터에@Valid어노테이션을 추가하세요.
UpdateBreakTimeDto는@NotNull검증 어노테이션을 가지고 있지만, 컨트롤러 메서드에@Valid가 없어 Spring이 Bean Validation을 수행하지 않습니다. 잘못된 입력값이 서비스 레이어로 전파될 수 있습니다.♻️ `@Valid` 추가 제안
public ApiResponse<BusinessHoursResDto.UpdateBreakTimeDto> updateBreakTime( `@PathVariable` Long storeId, - `@RequestBody` BusinessHoursReqDto.UpdateBreakTimeDto dto, + `@RequestBody` `@Valid` BusinessHoursReqDto.UpdateBreakTimeDto dto, `@CurrentUser` User user
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 84-89: In BookingRepository update the JPQL in the annotated query
that currently uses fully-qualified enum constants to match the style used
elsewhere: replace the FQCN enum references in the query on Booking (the `@Query`
selecting max(b.bookingDate) with conditions on b.status) with string literal
status values (e.g. 'CONFIRMED', 'PENDING') so the status filter syntax is
consistent with the other repository queries; keep the rest of the predicate
(storeId, bookingDate >= CURRENT_DATE, bookingTime range) unchanged.
- Around line 84-89: The JPQL in BookingRepository that currently checks
"b.bookingTime >= :breakStart and b.bookingTime < :breakEnd" misses reservations
that start before a break but extend into it; update the query in
BookingRepository so the overlap check uses booking duration: replace that
condition with an overlap expression such as "b.bookingTime < :breakEnd AND
(b.bookingTime + b.store.bookingIntervalMinutes) > :breakStart" (or the
equivalent JPQL/DB function to add minutes to b.bookingTime using
store.bookingIntervalMinutes) so the query accounts for
store.bookingIntervalMinutes when determining conflicts with :breakStart and
:breakEnd.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/dto/BusinessHoursResDto.java`:
- Around line 45-46: BusinessHoursResDto contains an internal-only field
effectiveDate annotated with `@JsonIgnore` which mixes internal logic into a
response DTO; remove effectiveDate from BusinessHoursResDto and move it into a
separate internal result/model (e.g., BusinessHoursInternalResult or include it
in the service return type) produced by the service method that currently
returns BusinessHoursResDto, then map/transform that internal result to
BusinessHoursResDto in the controller (or provide a toDto mapper) so the DTO
only contains fields meant for JSON responses.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/entity/BusinessHours.java`:
- Around line 88-98: applyPendingBreakTime currently silently ignores cases
where only one of newBreakStartTime or newBreakEndTime is set; add a defensive
validation in the caller (the scheduler that invokes
BusinessHours.applyPendingBreakTime) to detect the inconsistent state
(newBreakStartTime != null XOR newBreakEndTime != null) and handle it
explicitly: log a warning with the business id/context and the values of
newBreakStartTime/newBreakEndTime, and then either clear both pending fields
(newBreakStartTime/newBreakEndTime and effectiveDate) or surface an error so the
inconsistency can be corrected; reference the
BusinessHours.applyPendingBreakTime method and the fields newBreakStartTime,
newBreakEndTime, and effectiveDate when implementing the check in the scheduler.
- Around line 69-85: The method updateBreakTime currently calls LocalDate.now()
directly which hinders testing; change updateBreakTime(LocalTime startTime,
LocalTime endTime, LocalDate effectiveDate) to accept a reference date (e.g.,
add a parameter LocalDate referenceDate) and replace LocalDate.now() with that
referenceDate in the comparison, updating the immediate vs. scheduled branches
that set breakStartTime, breakEndTime, newBreakStartTime, newBreakEndTime, and
effectiveDate accordingly; update all callers/tests to pass a controllable
reference date (or add an overloaded convenience method that delegates to the
new signature for backward compatibility).
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/repository/BusinessHoursRepository.java`:
- Line 14: The repository method findAllByEffectiveDate(LocalDate date) in
BusinessHoursRepository only matches exact dates and causes past effective_date
records to be skipped if the scheduler missed a day; update the query to
findAllByEffectiveDateLessThanEqual(LocalDate date) (or add an equivalent
method) so callers (e.g., any service using findAllByEffectiveDate) will
retrieve all records with effective_date <= given date, and update those call
sites to use the new/modified method to ensure missed past records are
processed.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java`:
- Around line 70-77: The loop that clears break time in
BusinessHoursCommandServiceImpl currently calls bh.updateBreakTime(null, null,
LocalDate.now()) for every BusinessHours from store.getBusinessHours() even on
closed days; change it to mirror the set-logic by skipping closed days: only
call updateBreakTime(null, null, ...) when the BusinessHours instance is not
closed (i.e., add the if (!bh.isClosed()) guard used elsewhere), then return the
same BusinessHoursConverter.toUpdateBreakTimeDto(storeId, dto, null) as before.
- Around line 79-88: In BusinessHoursCommandServiceImpl, move the break-time
validation to run before calling bookingRepository.findLastConflictingDate to
avoid unnecessary DB queries; specifically, validate the DTO's
breakStartTime()/breakEndTime() (the existing break time validation block
currently at lines ~90-99) first and only if it passes call
bookingRepository.findLastConflictingDate(storeId, dto.breakStartTime(),
dto.breakEndTime()) to compute effectiveDate; adjust the code so
findLastConflictingDate and lastConflictDate mapping occur after the validation
succeeds.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java`:
- Around line 21-35: The current applyPendingBreakTimes method runs inside a
single `@Transactional` and uses
targets.forEach(BusinessHours::applyPendingBreakTime), so one entity exception
rolls back the whole batch and the final log uses targets.size() which may not
equal applied count; change the method to process each BusinessHours
individually: remove or avoid the single long transaction, iterate over
businessHoursRepository.findAllByEffectiveDate(LocalDate.now()) with a try-catch
per entity around BusinessHours.applyPendingBreakTime, persist each successful
change (e.g., call businessHoursRepository.save or ensure a short transaction
per entity), increment a success counter only when applyPendingBreakTime
actually performed an update (not just skipped on null), and log the success
counter instead of targets.size(); keep failure logging for the caught
exceptions.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursSuccessStatus.java`:
- Around line 13-15: The success codes in BusinessHoursSuccessStatus are
inconsistent; update the code strings on the enum entries
_UPDATE_BUSINESS_HOURS_SUCCESS, _UPDATE_BREAKTIME_SUCCESS, and
_UPDATE_BREAKTIME_DELAYED so they follow a consistent pattern (either change
"BUSINESS_HOURS200" to "BUSINESS_HOURS2000" or rename the others to
"BUSINESS_HOURS201" and "BUSINESS_HOURS202"); modify the string literals in
those enum constants to the chosen consistent sequence and ensure any consumers
referencing those exact codes are updated accordingly.
Comment on lines
+45
to
+46
| @JsonIgnore | ||
| LocalDate effectiveDate |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
응답 DTO에 @JsonIgnore 내부 전용 필드 혼재.
effectiveDate는 클라이언트에 노출되지 않는 내부 판단용 필드입니다. 응답 DTO에 @JsonIgnore 필드를 추가하는 것은 DTO의 역할(직렬화 대상 데이터 전달)과 맞지 않습니다. 서비스 계층에서 별도의 내부 결과 객체를 반환하고 컨트롤러에서 응답 DTO로 변환하는 구조가 더 명확합니다.
현재 규모에서는 실용적인 접근이지만, 향후 필드가 늘어나면 분리를 고려해 주세요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/dto/BusinessHoursResDto.java`
around lines 45 - 46, BusinessHoursResDto contains an internal-only field
effectiveDate annotated with `@JsonIgnore` which mixes internal logic into a
response DTO; remove effectiveDate from BusinessHoursResDto and move it into a
separate internal result/model (e.g., BusinessHoursInternalResult or include it
in the service return type) produced by the service method that currently
returns BusinessHoursResDto, then map/transform that internal result to
BusinessHoursResDto in the controller (or provide a toDto mapper) so the DTO
only contains fields meant for JSON responses.
Comment on lines
+69
to
+85
| public void updateBreakTime(LocalTime startTime, LocalTime endTime, LocalDate effectiveDate) { | ||
| // 오늘(혹은 과거) 날짜라면 -> 즉시 반영 | ||
| if (effectiveDate == null || !effectiveDate.isAfter(LocalDate.now())) { | ||
| this.breakStartTime = startTime; | ||
| this.breakEndTime = endTime; | ||
| // 대기 중인 데이터 초기화 | ||
| this.newBreakStartTime = null; | ||
| this.newBreakEndTime = null; | ||
| this.effectiveDate = null; | ||
| } | ||
| // 미래 날짜라면 -> 대기열에 저장 | ||
| else { | ||
| this.newBreakStartTime = startTime; | ||
| this.newBreakEndTime = endTime; | ||
| this.effectiveDate = effectiveDate; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LocalDate.now() 직접 호출은 테스트 용이성을 저해합니다.
엔티티 내부에서 LocalDate.now()를 직접 호출하면 단위 테스트 시 현재 날짜를 제어할 수 없습니다. effectiveDate와 비교할 기준 날짜를 파라미터로 받거나, Clock을 주입하는 방식을 고려해 주세요.
♻️ 파라미터로 기준 날짜를 전달하는 방식 제안
- public void updateBreakTime(LocalTime startTime, LocalTime endTime, LocalDate effectiveDate) {
- // 오늘(혹은 과거) 날짜라면 -> 즉시 반영
- if (effectiveDate == null || !effectiveDate.isAfter(LocalDate.now())) {
+ public void updateBreakTime(LocalTime startTime, LocalTime endTime, LocalDate effectiveDate, LocalDate today) {
+ // 오늘(혹은 과거) 날짜라면 -> 즉시 반영
+ if (effectiveDate == null || !effectiveDate.isAfter(today)) {🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/entity/BusinessHours.java`
around lines 69 - 85, The method updateBreakTime currently calls LocalDate.now()
directly which hinders testing; change updateBreakTime(LocalTime startTime,
LocalTime endTime, LocalDate effectiveDate) to accept a reference date (e.g.,
add a parameter LocalDate referenceDate) and replace LocalDate.now() with that
referenceDate in the comparison, updating the immediate vs. scheduled branches
that set breakStartTime, breakEndTime, newBreakStartTime, newBreakEndTime, and
effectiveDate accordingly; update all callers/tests to pass a controllable
reference date (or add an overloaded convenience method that delegates to the
new signature for backward compatibility).
Comment on lines
88
to
98
| public void applyPendingBreakTime() { | ||
| if (this.newBreakStartTime != null && this.newBreakEndTime != null) { | ||
| this.breakStartTime = newBreakStartTime; | ||
| this.breakEndTime = newBreakEndTime; | ||
|
|
||
| // 반영 후 대기열 비우기 | ||
| this.newBreakStartTime = null; | ||
| this.newBreakEndTime = null; | ||
| this.effectiveDate = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
applyPendingBreakTime() — 한쪽만 null인 비정상 상태에 대한 로깅 부재.
newBreakStartTime과 newBreakEndTime 중 하나만 null인 경우 조건문을 통과하지 못해 아무 동작 없이 무시됩니다. 데이터 정합성 문제가 발생해도 인지할 수 없으므로, else 분기에서 경고 로그를 남기거나 방어 코드를 추가하는 것을 권장합니다. 다만 엔티티에서 로깅은 적절하지 않으므로, 이 검증을 호출하는 스케줄러 쪽에서 처리하는 것이 더 적합합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/entity/BusinessHours.java`
around lines 88 - 98, applyPendingBreakTime currently silently ignores cases
where only one of newBreakStartTime or newBreakEndTime is set; add a defensive
validation in the caller (the scheduler that invokes
BusinessHours.applyPendingBreakTime) to detect the inconsistent state
(newBreakStartTime != null XOR newBreakEndTime != null) and handle it
explicitly: log a warning with the business id/context and the values of
newBreakStartTime/newBreakEndTime, and then either clear both pending fields
(newBreakStartTime/newBreakEndTime and effectiveDate) or surface an error so the
inconsistency can be corrected; reference the
BusinessHours.applyPendingBreakTime method and the fields newBreakStartTime,
newBreakEndTime, and effectiveDate when implementing the check in the scheduler.
|
|
||
| public interface BusinessHoursRepository extends JpaRepository<BusinessHours, Long> { | ||
| Optional<BusinessHours> findByStoreAndDayOfWeek(Store store, DayOfWeek dayOfWeek); | ||
| List<BusinessHours> findAllByEffectiveDate(LocalDate date); |
There was a problem hiding this comment.
스케줄러 미실행 시 과거 effective_date 레코드가 영구 누락됩니다.
findAllByEffectiveDate는 정확히 오늘 날짜만 매칭하므로, 서버 다운타임이나 스케줄러 미실행 시 해당 날짜의 레코드가 적용되지 않고 영구히 남습니다. findAllByEffectiveDateLessThanEqual(LocalDate date) 또는 그에 상응하는 쿼리를 사용하면 누락된 과거 날짜 레코드도 함께 처리할 수 있습니다.
♻️ 누락 방지를 위한 쿼리 변경 제안
- List<BusinessHours> findAllByEffectiveDate(LocalDate date);
+ List<BusinessHours> findAllByEffectiveDateLessThanEqual(LocalDate date);📝 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.
Suggested change
| List<BusinessHours> findAllByEffectiveDate(LocalDate date); | |
| List<BusinessHours> findAllByEffectiveDateLessThanEqual(LocalDate date); |
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/repository/BusinessHoursRepository.java`
at line 14, The repository method findAllByEffectiveDate(LocalDate date) in
BusinessHoursRepository only matches exact dates and causes past effective_date
records to be skipped if the scheduler missed a day; update the query to
findAllByEffectiveDateLessThanEqual(LocalDate date) (or add an equivalent
method) so callers (e.g., any service using findAllByEffectiveDate) will
retrieve all records with effective_date <= given date, and update those call
sites to use the new/modified method to ensure missed past records are
processed.
...java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java
Outdated
Show resolved
Hide resolved
Comment on lines
13
to
+15
| _UPDATE_BUSINESS_HOURS_SUCCESS(HttpStatus.OK, "BUSINESS_HOURS200", "영업시간이 성공적으로 수정되었습니다."), | ||
| _UPDATE_BREAKTIME_SUCCESS(HttpStatus.OK, "BUSINESS_HOURS2001", "브레이크타임이 성공적으로 설정되었습니다.") | ||
| _UPDATE_BREAKTIME_SUCCESS(HttpStatus.OK, "BUSINESS_HOURS2001", "브레이크타임이 성공적으로 설정되었습니다."), | ||
| _UPDATE_BREAKTIME_DELAYED(HttpStatus.OK, "BUSINESS_HOURS2002", "기존 예약이 존재하여, 마지막 예약 종료 후 다음 날부터 반영됩니다.") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
성공 코드 네이밍이 일관적이지 않습니다.
"BUSINESS_HOURS200" → "BUSINESS_HOURS2001" → "BUSINESS_HOURS2002" 패턴이 불규칙합니다. 첫 번째 코드를 "BUSINESS_HOURS2000"으로 변경하거나, 나머지를 "BUSINESS_HOURS201", "BUSINESS_HOURS202"로 통일하는 것이 좋습니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursSuccessStatus.java`
around lines 13 - 15, The success codes in BusinessHoursSuccessStatus are
inconsistent; update the code strings on the enum entries
_UPDATE_BUSINESS_HOURS_SUCCESS, _UPDATE_BREAKTIME_SUCCESS, and
_UPDATE_BREAKTIME_DELAYED so they follow a consistent pattern (either change
"BUSINESS_HOURS200" to "BUSINESS_HOURS2000" or rename the others to
"BUSINESS_HOURS201" and "BUSINESS_HOURS202"); modify the string literals in
those enum constants to the chosen consistent sequence and ensure any consumers
referencing those exact codes are updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 84-99: The method findLastConflictingDate has an unclear parameter
adjustedBreakStart (used with breakStart/breakEnd) — document its meaning or
make it explicit: add Javadoc to the repository method (findLastConflictingDate)
explaining that adjustedBreakStart = breakStart minus the booking interval
(bookingIntervalMinutes) and show the unit and how callers should compute it, or
rename the parameter to something explicit (e.g.,
breakStartMinusBookingInterval) and update all call sites (e.g., the caller in
BusinessHoursCommandServiceImpl) to pass the computed value with matching name
so future callers won't pass the wrong value.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java`:
- Around line 84-91: The code only handles both breakStartTime and breakEndTime
being null or both non-null, so when exactly one is null it will cause a
NullPointerException during adjustedBreakStart calculation; add an explicit
guard in BusinessHoursCommandServiceImpl (the update break-time flow that uses
dto.breakStartTime() and dto.breakEndTime()) to detect the “one-null” case and
fail fast: validate that either both are null or both are non-null, and if not
throw a clear IllegalArgumentException (or domain BadRequest) with a message
like "breakStartTime and breakEndTime must be both set or both null"; ensure
this check occurs before using BreakTimeValidator and before computing
adjustedBreakStart and calling BusinessHours.updateBreakTime so NPE is avoided.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java`:
- Around line 30-41: The loop increments successCount unconditionally after
calling processEachPendingTime(BusinessHours), which causes runs that invoke
clearPendingBreakTime() and then return (the XOR path) to be counted as
successes; change the flow so successCount is only incremented for real
successful applies: have processEachPendingTime(BusinessHours) return a clear
status (e.g., enum or boolean) indicating whether the record was applied vs.
only cleared, or throw a distinct exception for the clear-only path, then in the
pendingList loop increment successCount only when the returned status equals
APPLIED; additionally add a warnCount (or log and do not increment successCount)
for the clearPendingBreakTime() case so it’s not treated as a success.
- Around line 37-39: In the catch block inside BusinessHoursScheduler where you
currently log via log.error("[Scheduler Exception] 반영 실패 - BH ID: {}, Error:
{}", bh.getId(), e.getMessage()), change the logging to include the full
throwable so the stack trace is recorded (e.g., pass the Exception `e` as the
last argument or use a logging overload that accepts a Throwable). Update the
log call in the catch (Exception e) handler that references bh.getId() so it
logs both the BH ID and the exception object (not just e.getMessage()) to
capture stack trace and aid debugging.
- Around line 45-53: processEachPendingTime currently mutates a potentially
detached BusinessHours (bh) passed from applyPendingBreakTimes; re-query the
entity inside the `@Transactional` method so changes happen on a managed instance:
call businessHoursRepository.findById(bh.getId()) (or orElseThrow) at start of
processEachPendingTime, operate on that returned managed entity
(applyPendingBreakTime / clearPendingBreakTime on the managed instance) and
persist/flush as needed to ensure updates are applied within the transaction.
- Around line 32-53: processEachPendingTime currently annotated with
`@Transactional` is invoked via self-call from the same class so Spring AOP won’t
start a transaction; refactor by either extracting processEachPendingTime into a
separate `@Component/bean` and inject that bean into BusinessHoursScheduler (move
the `@Transactional` there), or keep it in this class but inject a
TransactionTemplate and replace the direct call with
transactionTemplate.execute(status -> { processEachPendingTime(bh); return null;
}) so applyPendingBreakTime() / clearPendingBreakTime() run inside a
transaction; ensure you update successCount only after the transactional
execution completes without exception and keep references to
processEachPendingTime, applyPendingBreakTime, clearPendingBreakTime,
TransactionTemplate, and BusinessHoursScheduler to locate the changes.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java`:
- Around line 17-20: The enum BusinessHoursErrorStatus has a duplicated error
code: both _INVALID_CLOSED_DAY and _BREAK_TIME_NOT_ALLOWED_FOR_24H use
"BUSINESS_HOURS4005"; update the entry for _BREAK_TIME_NOT_ALLOWED_FOR_24H to a
unique code (e.g., "BUSINESS_HOURS4007") so each enum constant has a distinct
error code, ensuring changes are applied in the BusinessHoursErrorStatus enum
where those constants are defined.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BreakTimeValidator.java`:
- Around line 17-20: In BreakTimeValidator remove the stray semicolon after the
closing brace of the 24-hour check; specifically, in the block that tests
openTime.equals(closeTime) and throws new
BusinessHoursException(BusinessHoursErrorStatus._BREAK_TIME_NOT_ALLOWED_FOR_24H)
delete the unnecessary `;` following `}` so the method/class punctuation is
clean.
- Around line 13-15: The current BreakTimeValidator returns early when any of
openTime, closeTime, breakStartTime, or breakEndTime is null, allowing cases
where only one of breakStartTime/breakEndTime is null to pass; update
BreakTimeValidator so that it still returns early when openTime or closeTime are
null, but specifically detect the asymmetric case (breakStartTime == null XOR
breakEndTime == null) and throw an IllegalArgumentException (or domain-specific
validation exception) with a clear message; reference BreakTimeValidator and its
validation method and ensure callers such as BusinessHoursCommandServiceImpl can
rely on this defensive check.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`:
- Around line 121-123: The bug is that when open.equals(close) (24-hour
operation) the code unconditionally sets isWithinBusinessHours = true; update
the logic in StoreQueryServiceImpl so that the 24-hour branch only sets
isWithinBusinessHours = true when the entry applies to the current day (check
isToday) instead of always; locate the block that checks open.equals(close) and
change it to require isToday (e.g., open.equals(close) && isToday) before
assigning true to isWithinBusinessHours so 24-hour hours do not bleed into the
next day's holiday.
src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java
Show resolved
Hide resolved
...java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java
Outdated
Show resolved
Hide resolved
Comment on lines
+13
to
15
| if (openTime == null || closeTime == null || breakStartTime == null || breakEndTime == null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
breakStartTime과 breakEndTime 중 하나만 null인 경우 검증이 무시됩니다.
네 파라미터 중 하나라도 null이면 즉시 return하므로, breakStartTime만 null이고 breakEndTime은 존재하는 비정상 상태가 검증 없이 통과합니다. 호출부(BusinessHoursCommandServiceImpl)에서 null 쌍을 먼저 처리하더라도, 방어적으로 한쪽만 null인 경우에는 예외를 던지는 것이 안전합니다.
🛡️ 방어 코드 추가 제안
// 휴무일은 검증 대상이 아님
- if (openTime == null || closeTime == null || breakStartTime == null || breakEndTime == null) {
+ if (openTime == null || closeTime == null) {
+ return;
+ }
+ if (breakStartTime == null && breakEndTime == null) {
return;
}
+ if (breakStartTime == null || breakEndTime == null) {
+ throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME);
+ }🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BreakTimeValidator.java`
around lines 13 - 15, The current BreakTimeValidator returns early when any of
openTime, closeTime, breakStartTime, or breakEndTime is null, allowing cases
where only one of breakStartTime/breakEndTime is null to pass; update
BreakTimeValidator so that it still returns early when openTime or closeTime are
null, but specifically detect the asymmetric case (breakStartTime == null XOR
breakEndTime == null) and throw an IllegalArgumentException (or domain-specific
validation exception) with a clear message; reference BreakTimeValidator and its
validation method and ensure callers such as BusinessHoursCommandServiceImpl can
rely on this defensive check.
src/main/java/com/eatsfine/eatsfine/domain/businesshours/validator/BreakTimeValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 87-96: The JPQL time-range condition using adjustedBreakStart
fails when adjustedBreakStart wraps to the previous day (adjustedBreakStart >
breakStart), so update the BookingRepository query logic: detect when
adjustedBreakStart > breakStart and in that case either omit the Case B clause
or replace the single wrapped range with two explicit ranges that cross midnight
(e.g., bookingTime >= adjustedBreakStart OR bookingTime < breakStart) to
correctly match crossings; implement this by modifying the repository method
that contains the shown `@Query` (and/or split into two `@Query` branches) and keep
the status filter consistent by using the same string literal form ('CONFIRMED',
'PENDING') as other queries.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java`:
- Around line 73-97: Move the null-check for
dto.breakStartTime()/dto.breakEndTime() to before the validation loop in
BusinessHoursCommandServiceImpl to avoid pointless iteration over
store.getBusinessHours() when the request is to clear break times or is invalid;
specifically, first handle the "both null" case by updating each BusinessHours
via bh.updateBreakTime(null, null, LocalDate.now()) and return the DTO, then
guard the "one null" case by throwing
BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME); only after
these checks call BreakTimeValidator.validateBreakTime(...) for each non-closed
BusinessHours and catch/ log BusinessHoursException as currently done.
- Around line 99-104: The computed adjustedBreakStart can wrap past midnight
(adjustedBreakStart.isAfter(dto.breakStartTime())), so detect that case and
handle it by splitting the conflict check into two ranges instead of a single
JPQL range: when wrapping, call bookingRepository for the late-night range
(adjustedBreakStart .. 23:59:59) and for the early-morning range (00:00 ..
dto.breakStartTime()) and combine their results (e.g., take the latest
Optional<LocalDate> from both); otherwise keep the existing single call to
bookingRepository.findLastConflictingDate(storeId, dto.breakStartTime(),
dto.breakEndTime(), adjustedBreakStart). Ensure you reference
adjustedBreakStart, dto.breakStartTime(), store.getBookingIntervalMinutes(), and
bookingRepository.findLastConflictingDate when implementing the fix.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java`:
- Around line 31-52: The final log in BusinessHoursScheduler currently only
prints successCount; update the end-of-loop log (after the for over pendingList
and transactionTemplate.execute calls to processEachPendingTime) to include
failCount and warnCount as well (e.g., "[Scheduler] 반영 작업 완료. (성공: {}/{} 건, 실패:
{} 건, 불일치: {} 건)") so the aggregated failCount and warnCount variables are
surfaced for operations monitoring; keep the existing successCount increments
and exception handling (catch block logging the exception for BH id) intact.
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java`:
- Line 19: The enum entry _INVALID_BREAK_TIME in BusinessHoursErrorStatus is
misleading for BreakTimeValidator because that validator uses it for three
distinct failure reasons; update the code by either (A) adding separate error
codes/messages for each specific failure case referenced in BreakTimeValidator
(e.g., "BREAK_START_AFTER_END", "BREAK_OUTSIDE_BUSINESS_HOURS",
"BREAK_OVERLAPS_NIGHT_OFF_PERIOD") and throw the corresponding enum from
BreakTimeValidator, or (B) change the _INVALID_BREAK_TIME message to a clear,
non-sensitive, encompassing message like "브레이크 타임 조건을 만족하지 않습니다(시작/종료 시간, 영업시간
범위 및 심야 비영업 구간 확인 필요)" and ensure BreakTimeValidator continues to use that
single enum; locate and update the enum constant _INVALID_BREAK_TIME in
BusinessHoursErrorStatus and the throw sites in BreakTimeValidator to match the
new codes/messages.
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`:
- Around line 112-144: Add unit tests for the isEffectiveOpen method in
StoreQueryServiceImpl covering boundary and edge cases: test normal business
hours (inside/outside), night/overnight hours with isToday=true and
isToday=false, 24-hour open (open.equals(close)) with both isToday values, break
time inclusion/exclusion (when bh.getBreakStartTime()/getBreakEndTime() are set)
including boundary moments equal to break start and end, closed day
(bh.isClosed() == true), and exact boundary times equal to open and close;
construct BusinessHours instances with appropriate open/close/break times and
assert boolean results for each LocalTime/isToday combination. Ensure tests
reference StoreQueryServiceImpl.isEffectiveOpen and BusinessHours getters to
validate behavior across all listed scenarios.
Comment on lines
87
to
96
| @Query("select max(b.bookingDate) from Booking b " + | ||
| "where b.store.id = :storeId " + | ||
| "and b.status IN (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING) " + | ||
| "and b.bookingDate >= CURRENT_DATE " + | ||
| "and (" + | ||
| " (b.bookingTime >= :breakStart and b.bookingTime < :breakEnd) " + // 케이스 A: 브레이크 중에 예약 시작 | ||
| " OR " + | ||
| " (b.bookingTime >= :adjustedBreakStart and b.bookingTime < :breakStart)" + // 케이스 B: 브레이크 전에 시작해서 걸침 | ||
| ")" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find BusinessHoursCommandServiceImpl and check adjustedBreakStart calculation
fd "BusinessHoursCommandServiceImpl" --type=f -x cat -n {}Repository: Eatsfine/BE
Length of output: 5658
🏁 Script executed:
#!/bin/bash
# Find Store entity and check bookingIntervalMinutes
fd "Store\.java" --type=f -x cat -n {}Repository: Eatsfine/BE
Length of output: 7611
🏁 Script executed:
#!/bin/bash
# Check BreakTimeValidator constraints
fd "BreakTimeValidator" --type=f -x cat -n {}Repository: Eatsfine/BE
Length of output: 2224
🏁 Script executed:
#!/bin/bash
# Get full BookingRepository.java to see query context and other queries
fd "BookingRepository\.java" --type=f -x cat -n {}Repository: Eatsfine/BE
Length of output: 5457
심야 영업 시 adjustedBreakStart 래핑으로 Case B 조건이 작동 불가 상태가 됩니다.
breakStartTime이 자정 직후(예: 01:00)이고 bookingIntervalMinutes가 충분히 크면(예: 90분), adjustedBreakStart는 전날로 래핑됩니다(23:30). 이 경우 Case B 조건인 b.bookingTime >= 23:30 AND b.bookingTime < 01:00은 LocalTime 비교에서 23:30 > 01:00이므로 빈 범위가 되어, 자정을 기준으로 교차하는 예약 충돌을 감지하지 못합니다.
권장 해결책: adjustedBreakStart > breakStart인 경우를 감지하여 Case B를 건너뛰거나, 두 개의 분리된 시간 범위로 쿼리를 나누세요. 또는 서비스 레이어에서 사전 검증으로 이러한 시나리오를 방어하세요.
추가로, JPQL 상태 필터에서 일관성을 유지하세요: 다른 쿼리들은 문자열 리터럴('CONFIRMED', 'PENDING')을 사용하지만, 이 쿼리는 FQCN 열거형을 사용합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 87 - 96, The JPQL time-range condition using adjustedBreakStart
fails when adjustedBreakStart wraps to the previous day (adjustedBreakStart >
breakStart), so update the BookingRepository query logic: detect when
adjustedBreakStart > breakStart and in that case either omit the Case B clause
or replace the single wrapped range with two explicit ranges that cross midnight
(e.g., bookingTime >= adjustedBreakStart OR bookingTime < breakStart) to
correctly match crossings; implement this by modifying the repository method
that contains the shown `@Query` (and/or split into two `@Query` branches) and keep
the status filter consistent by using the same string literal form ('CONFIRMED',
'PENDING') as other queries.
Comment on lines
73
to
+97
| for(BusinessHours bh : store.getBusinessHours()) { | ||
| if(bh.isClosed()) continue; | ||
| BreakTimeValidator.validateBreakTime(bh.getOpenTime(), bh.getCloseTime(), dto.breakStartTime(), dto.breakEndTime()); | ||
| try { | ||
| BreakTimeValidator.validateBreakTime(bh.getOpenTime(), bh.getCloseTime(), dto.breakStartTime(), dto.breakEndTime()); | ||
| } catch (BusinessHoursException e) { | ||
| log.error("브레이크 타임 검증 실패 - 요일: {}, 영업시간: {}~{}, 브레이크: {}~{}", | ||
| bh.getDayOfWeek(), bh.getOpenTime(), bh.getCloseTime(), | ||
| dto.breakStartTime(), dto.breakEndTime()); | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| // 브레이크 타임 해제 요청인 경우 (두 시간 모두 null) | ||
| if (dto.breakStartTime() == null && dto.breakEndTime() == null) { | ||
| for(BusinessHours bh : store.getBusinessHours()) { | ||
| bh.updateBreakTime(null, null, LocalDate.now()); | ||
| } | ||
| return BusinessHoursConverter.toUpdateBreakTimeDto(storeId, dto, null); | ||
|
|
||
| } | ||
|
|
||
| // 한쪽만 null인 비정상 요청 방어 | ||
| if (dto.breakStartTime() == null || dto.breakEndTime() == null) { | ||
| throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
null 체크가 검증 루프 이후에 위치하여 불필요한 반복이 발생합니다.
breakStartTime/breakEndTime이 null인 경우, BreakTimeValidator는 조기 반환하므로 Lines 73-83의 루프가 모든 요일에 대해 무의미하게 실행됩니다. null 체크(Lines 85-97)를 검증 루프 앞으로 이동하면 가독성과 효율성이 모두 향상됩니다.
♻️ 순서 변경 제안
Store store = storeValidator.validateStoreOwner(storeId, email);
+ // 브레이크 타임 해제 요청인 경우 (두 시간 모두 null)
+ if (dto.breakStartTime() == null && dto.breakEndTime() == null) {
+ for(BusinessHours bh : store.getBusinessHours()) {
+ bh.updateBreakTime(null, null, LocalDate.now());
+ }
+ return BusinessHoursConverter.toUpdateBreakTimeDto(storeId, dto, null);
+ }
+
+ // 한쪽만 null인 비정상 요청 방어
+ if (dto.breakStartTime() == null || dto.breakEndTime() == null) {
+ throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME);
+ }
+
for(BusinessHours bh : store.getBusinessHours()) {
if(bh.isClosed()) continue;
try {
BreakTimeValidator.validateBreakTime(bh.getOpenTime(), bh.getCloseTime(), dto.breakStartTime(), dto.breakEndTime());
} catch (BusinessHoursException e) {
log.error("브레이크 타임 검증 실패 - 요일: {}, 영업시간: {}~{}, 브레이크: {}~{}",
bh.getDayOfWeek(), bh.getOpenTime(), bh.getCloseTime(),
dto.breakStartTime(), dto.breakEndTime());
throw e;
}
}
- // 브레이크 타임 해제 요청인 경우 (두 시간 모두 null)
- if (dto.breakStartTime() == null && dto.breakEndTime() == null) {
- for(BusinessHours bh : store.getBusinessHours()) {
- bh.updateBreakTime(null, null, LocalDate.now());
- }
- return BusinessHoursConverter.toUpdateBreakTimeDto(storeId, dto, null);
- }
-
- // 한쪽만 null인 비정상 요청 방어
- if (dto.breakStartTime() == null || dto.breakEndTime() == null) {
- throw new BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME);
- }
-
LocalTime adjustedBreakStart = dto.breakStartTime().minusMinutes(store.getBookingIntervalMinutes());As per coding guidelines, "불필요한 DB 쿼리 호출, N+1 문제 가능성이 있는지 확인."
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java`
around lines 73 - 97, Move the null-check for
dto.breakStartTime()/dto.breakEndTime() to before the validation loop in
BusinessHoursCommandServiceImpl to avoid pointless iteration over
store.getBusinessHours() when the request is to clear break times or is invalid;
specifically, first handle the "both null" case by updating each BusinessHours
via bh.updateBreakTime(null, null, LocalDate.now()) and return the DTO, then
guard the "one null" case by throwing
BusinessHoursException(BusinessHoursErrorStatus._INVALID_BREAK_TIME); only after
these checks call BreakTimeValidator.validateBreakTime(...) for each non-closed
BusinessHours and catch/ log BusinessHoursException as currently done.
Comment on lines
+99
to
+104
| LocalTime adjustedBreakStart = dto.breakStartTime().minusMinutes(store.getBookingIntervalMinutes()); | ||
|
|
||
| // 1. 예약 충돌 확인 | ||
| Optional<LocalDate> lastConflictDate = bookingRepository.findLastConflictingDate( | ||
| storeId, dto.breakStartTime(), dto.breakEndTime(), adjustedBreakStart | ||
| ); |
There was a problem hiding this comment.
adjustedBreakStart가 자정을 넘어 래핑되는 경우에 대한 방어가 없습니다.
BookingRepository.findLastConflictingDate 리뷰에서 언급한 것과 동일한 이슈입니다. breakStartTime이 자정 직후이고 bookingIntervalMinutes가 해당 시간보다 크면, LocalTime.minusMinutes()가 전날 23:xx로 래핑됩니다. 이 경우 JPQL의 Case B 범위 조건(>= adjustedBreakStart AND < breakStart)이 빈 범위가 되어 충돌을 감지하지 못합니다.
래핑 발생 여부를 감지하는 방어 코드를 추가하세요:
🛡️ 방어 코드 예시
LocalTime adjustedBreakStart = dto.breakStartTime().minusMinutes(store.getBookingIntervalMinutes());
+ // adjustedBreakStart가 breakStart보다 크면 자정 래핑 발생 → Case B 무효화
+ if (adjustedBreakStart.isAfter(dto.breakStartTime())) {
+ adjustedBreakStart = LocalTime.MIDNIGHT; // 또는 Case B를 건너뛰도록 breakStart와 동일하게 설정
+ }🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursCommandServiceImpl.java`
around lines 99 - 104, The computed adjustedBreakStart can wrap past midnight
(adjustedBreakStart.isAfter(dto.breakStartTime())), so detect that case and
handle it by splitting the conflict check into two ranges instead of a single
JPQL range: when wrapping, call bookingRepository for the late-night range
(adjustedBreakStart .. 23:59:59) and for the early-morning range (00:00 ..
dto.breakStartTime()) and combine their results (e.g., take the latest
Optional<LocalDate> from both); otherwise keep the existing single call to
bookingRepository.findLastConflictingDate(storeId, dto.breakStartTime(),
dto.breakEndTime(), adjustedBreakStart). Ensure you reference
adjustedBreakStart, dto.breakStartTime(), store.getBookingIntervalMinutes(), and
bookingRepository.findLastConflictingDate when implementing the fix.
Comment on lines
+31
to
+52
| int successCount = 0; | ||
| int failCount = 0; | ||
| int warnCount = 0; // 데이터 불일치(XOR) 건수 | ||
|
|
||
| // 전체 대상 건수 로그 | ||
| log.info("[Scheduler] 처리 대상 건수: {}건", pendingList.size()); | ||
|
|
||
| for (BusinessHours bh : pendingList) { | ||
| try { | ||
| Boolean isApplied = transactionTemplate.execute(status -> processEachPendingTime(bh.getId())); | ||
| if(Boolean.TRUE.equals(isApplied)) { | ||
| successCount++; | ||
| } else { | ||
| warnCount++; // XOR 등으로 인해 초기화만 된 경우 | ||
| } | ||
| } catch (Exception e) { | ||
| failCount++; | ||
| // 개별 건 처리 중 에러 발생 시 로그 남기고 다음 건 진행 | ||
| log.error("[Scheduler Exception] 반영 실패 - BH ID: {}", bh.getId(), e); | ||
| } | ||
| } | ||
| log.info("[Scheduler] 반영 작업 완료. (성공: {}/{} 건)", successCount, pendingList.size()); |
There was a problem hiding this comment.
failCount와 warnCount가 선언/집계되지만 최종 로그에 포함되지 않습니다.
Line 52의 최종 로그에서 successCount만 출력합니다. 운영 모니터링을 위해 실패 건수와 데이터 불일치 건수도 함께 기록해야 합니다.
🔧 로그 개선 제안
- log.info("[Scheduler] 반영 작업 완료. (성공: {}/{} 건)", successCount, pendingList.size());
+ log.info("[Scheduler] 반영 작업 완료. (성공: {}건, 실패: {}건, 데이터 불일치: {}건 / 총 {}건)", successCount, failCount, warnCount, pendingList.size());As per coding guidelines, "예외가 적절히 처리되었는지 확인해줘. (try-catch, throws, ExceptionAdvice)"
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/service/BusinessHoursScheduler.java`
around lines 31 - 52, The final log in BusinessHoursScheduler currently only
prints successCount; update the end-of-loop log (after the for over pendingList
and transactionTemplate.execute calls to processEachPendingTime) to include
failCount and warnCount as well (e.g., "[Scheduler] 반영 작업 완료. (성공: {}/{} 건, 실패:
{} 건, 불일치: {} 건)") so the aggregated failCount and warnCount variables are
surfaced for operations monitoring; keep the existing successCount increments
and exception handling (catch block logging the exception for BH id) intact.
| _BUSINESS_HOURS_DAY_NOT_FOUND(HttpStatus.NOT_FOUND, "BUSINESS_HOURS404", "해당 요일에 대한 영업시간 정보가 존재하지 않습니다."), | ||
| _INVALID_BREAK_TIME(HttpStatus.BAD_REQUEST, "BUSINESS_HOURS4006", "브레이크타임 시작 시간은 종료 시간보다 빨라야 합니다."), | ||
| _BREAK_TIME_OUT_OF_BUSINESS_HOURS(HttpStatus.BAD_REQUEST, "BUSINESS_HOURS4007", "브레이크타임은 영업시간 내에만 설정할 수 있습니다."), | ||
| _INVALID_BREAK_TIME(HttpStatus.BAD_REQUEST, "BUSINESS_HOURS4006", "브레이크타임 시작 시간은 종료 시간보다 빨라야 합니다.(심야 영업 시간 고려 필수)"), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
_INVALID_BREAK_TIME 메시지가 실제 사용 범위와 일치하지 않습니다.
이 에러 코드는 BreakTimeValidator에서 세 가지 경우에 사용됩니다:
- 브레이크 시작 >= 종료 (Line 23-24)
- 일반 영업 시간 밖 (Line 30-31)
- 심야 영업 비영업 구간 포함 (Line 35-39)
하지만 메시지는 "시작 시간은 종료 시간보다 빨라야 합니다"로, 첫 번째 경우만 설명합니다. 클라이언트가 2번/3번 사유로 실패 시 혼란을 겪을 수 있습니다. 각 사유에 맞는 별도 에러 코드를 부여하거나, 메시지를 포괄적으로 수정하는 것을 권장합니다.
As per coding guidelines, "예외 메시지에 민감 정보(DB 정보, 사용자 정보 등)가 노출되지 않게 했는지 점검."과 "의미 있는 변수명과 메소드명을 사용했는지" 관점에서, 에러 메시지도 명확한 의미 전달이 필요합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/businesshours/status/BusinessHoursErrorStatus.java`
at line 19, The enum entry _INVALID_BREAK_TIME in BusinessHoursErrorStatus is
misleading for BreakTimeValidator because that validator uses it for three
distinct failure reasons; update the code by either (A) adding separate error
codes/messages for each specific failure case referenced in BreakTimeValidator
(e.g., "BREAK_START_AFTER_END", "BREAK_OUTSIDE_BUSINESS_HOURS",
"BREAK_OVERLAPS_NIGHT_OFF_PERIOD") and throw the corresponding enum from
BreakTimeValidator, or (B) change the _INVALID_BREAK_TIME message to a clear,
non-sensitive, encompassing message like "브레이크 타임 조건을 만족하지 않습니다(시작/종료 시간, 영업시간
범위 및 심야 비영업 구간 확인 필요)" and ensure BreakTimeValidator continues to use that
single enum; locate and update the enum constant _INVALID_BREAK_TIME in
BusinessHoursErrorStatus and the throw sites in BreakTimeValidator to match the
new codes/messages.
Comment on lines
+112
to
144
| private boolean isEffectiveOpen(BusinessHours bh, LocalTime time, boolean isToday) { | ||
| if (bh.isClosed()) return false; | ||
|
|
||
| LocalTime open = bh.getOpenTime(); | ||
| LocalTime close = bh.getCloseTime(); | ||
|
|
||
| boolean isWithinBusinessHours; | ||
|
|
||
| // 1. 영업 시간 범위 먼저 체크 | ||
| if (open.equals(close)) { | ||
| isWithinBusinessHours = isToday; | ||
| // 24시간 영업 | ||
| } else if (open.isBefore(close)) { | ||
| // 일반 영업 (예: 09:00 ~ 18:00) | ||
| isWithinBusinessHours = isToday && (!time.isBefore(open) && time.isBefore(close)); | ||
| } else { | ||
| // 심야 영업 (예: 22:00 ~ 03:00) | ||
| isWithinBusinessHours = isToday ? !time.isBefore(open) : time.isBefore(close); | ||
| } | ||
|
|
||
| // 2. 영업 시간일 경우에만 브레이크 타임 검사 | ||
| if (isWithinBusinessHours) { | ||
| if (bh.getBreakStartTime() != null && bh.getBreakEndTime() != null) { | ||
| // 브레이크 타임 안에 있으면 false (영업 아님) 반환 | ||
| if (!time.isBefore(bh.getBreakStartTime()) && time.isBefore(bh.getBreakEndTime())) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; // 영업 시간이고 브레이크 타임도 아님 | ||
| } | ||
|
|
||
| return false; // 영업 시간 자체가 아님 | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
isEffectiveOpen 헬퍼에 대한 단위 테스트가 필요합니다.
심야 영업, 24시간 영업, 브레이크타임 등 다양한 경계값 케이스가 존재하는 핵심 로직입니다. 코딩 가이드라인에 따라 다음 케이스들에 대한 테스트를 권장합니다:
- 일반 영업시간 내/외 시간
- 심야 영업: 자정 전(isToday=true), 자정 후(isToday=false)
- 24시간 영업: isToday=true/false
- 브레이크타임 내/외 시간
- 휴무일(
isClosed=true) - 영업 시작/종료 경계값(정확히 open, close 시간)
As per coding guidelines, "단위 테스트가 충분히 작성되었는지, 핵심 로직의 검증이 누락되지 않았는지" 및 "경계값 테스트, 예외 케이스 테스트가 포함되어 있는지".
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/store/service/StoreQueryServiceImpl.java`
around lines 112 - 144, Add unit tests for the isEffectiveOpen method in
StoreQueryServiceImpl covering boundary and edge cases: test normal business
hours (inside/outside), night/overnight hours with isToday=true and
isToday=false, 24-hour open (open.equals(close)) with both isToday values, break
time inclusion/exclusion (when bh.getBreakStartTime()/getBreakEndTime() are set)
including boundary moments equal to break start and end, closed day
(bh.isClosed() == true), and exact boundary times equal to open and close;
construct BusinessHours instances with appropriate open/close/break times and
assert boolean results for each LocalTime/isToday combination. Ensure tests
reference StoreQueryServiceImpl.isEffectiveOpen and BusinessHours getters to
validate behavior across all listed scenarios.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`:
- Around line 83-86: Update the Javadoc for the method in BookingRepository
(referencing parameters adjustedBreakStart, breakStart, breakEnd, storeId) to
include missing `@param` entries: document breakStart and breakEnd (including
whether they may cross midnight and how that is handled), describe storeId as
the target restaurant identifier, and keep the existing adjustedBreakStart
description (already explaining it's breakStart minus bookingIntervalMinutes);
ensure the `@param` wording clearly states units (e.g., LocalTime or minutes) and
any assumptions used by the method.
- Line 89: This query in BookingRepository uses the FQCN
com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED/PENDING while
other queries in the same file use string literals ('CONFIRMED','PENDING'); make
them consistent by replacing the FQCN usage in this query with the same string
literals ('CONFIRMED','PENDING') or alternatively migrate the other queries to
use the BookingStatus FQCN consistently—update the condition in the repository
query accordingly and ensure references to BookingStatus (enum) are uniform
across BookingRepository.
Comment on lines
+83
to
+86
| /** | ||
| * 특정 식당의 브레이크 타임과 겹치는 가장 늦은 예약 날짜를 조회합니다. | ||
| * @param adjustedBreakStart 브레이크 시작 시간에서 식당의 예약 간격(bookingIntervalMinutes)을 뺀 시간 | ||
| */ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Javadoc에 adjustedBreakStart 설명이 추가되었으나, 다른 파라미터 문서가 누락되어 있습니다.
adjustedBreakStart에 대한 설명이 추가된 것은 좋습니다. 다만 breakStart, breakEnd, storeId 파라미터에 대한 @param 문서도 함께 기술하면 메서드 시그니처의 전체적인 이해가 쉬워집니다. 특히 breakStart와 breakEnd가 자정 교차 가능 여부에 대한 제약 조건도 명시하면 좋겠습니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 83 - 86, Update the Javadoc for the method in BookingRepository
(referencing parameters adjustedBreakStart, breakStart, breakEnd, storeId) to
include missing `@param` entries: document breakStart and breakEnd (including
whether they may cross midnight and how that is handled), describe storeId as
the target restaurant identifier, and keep the existing adjustedBreakStart
description (already explaining it's breakStart minus bookingIntervalMinutes);
ensure the `@param` wording clearly states units (e.g., LocalTime or minutes) and
any assumptions used by the method.
| */ | ||
| @Query("select max(b.bookingDate) from Booking b " + | ||
| "where b.store.id = :storeId " + | ||
| "and b.status IN (com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED, com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.PENDING) " + |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
상태 필터에서 FQCN 열거형 사용이 같은 파일 내 다른 쿼리들과 일관성이 없습니다.
같은 파일 내의 다른 쿼리들(Line 28, 35, 43, 58, 67)은 모두 'CONFIRMED', 'PENDING' 문자열 리터럴을 사용하고 있지만, 이 쿼리만 com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED FQCN을 사용합니다.
FQCN 방식이 타입 안전성 면에서 더 나은 접근이지만, 파일 내 일관성을 위해 한쪽으로 통일하는 것을 권장합니다. 만약 FQCN 방식을 채택한다면 다른 쿼리들도 함께 마이그레이션하는 것이 좋겠습니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
at line 89, This query in BookingRepository uses the FQCN
com.eatsfine.eatsfine.domain.booking.enums.BookingStatus.CONFIRMED/PENDING while
other queries in the same file use string literals ('CONFIRMED','PENDING'); make
them consistent by replacing the FQCN usage in this query with the same string
literals ('CONFIRMED','PENDING') or alternatively migrate the other queries to
use the BookingStatus FQCN consistently—update the condition in the repository
query accordingly and ensure references to BookingStatus (enum) are uniform
across BookingRepository.
zerochani
approved these changes
Feb 12, 2026
sonjunkyu
approved these changes
Feb 12, 2026
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
3 participants
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.
💡 작업 개요
✅ 작업 내용
🧪 테스트 내용
📝 기타 참고 사항
Summary by CodeRabbit
Summary by CodeRabbit
주요 변경사항
New Features
Bug Fixes
Documentation