Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,16 @@ Optional<LocalDate> findLastConflictingDate(
@Param("breakEnd") LocalTime breakEnd,
@Param("adjustedBreakStart") LocalTime adjustedBreakStart
);

@Query("SELECT DISTINCT bt.storeTable.id FROM BookingTable bt " +
"JOIN bt.booking b " +
"WHERE bt.storeTable.id IN :tableIds " +
"AND (b.bookingDate > :currentDate " +
" OR (b.bookingDate = :currentDate AND b.bookingTime >= :currentTime)) " +
"AND b.status IN ('CONFIRMED', 'PENDING')")
List<Long> findTableIdsWithFutureBookings(
@Param("tableIds") List<Long> tableIds,
@Param("currentDate") LocalDate currentDate,
@Param("currentTime") LocalTime currentTime
);
Comment on lines +106 to +116
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

이전 N+1 문제를 잘 해결한 벌크 쿼리입니다.

쿼리 로직이 정확하고, 기존 existsFutureBookingByTable 단건 호출의 N+1 문제를 해결합니다.

한 가지 고려사항: 반환 타입이 List<Long>이지만, 호출부(TableLayoutCommandServiceImpl)에서는 isEmpty() 여부만 확인합니다. 존재 여부만 필요하다면 SELECT COUNT(...) > 0 또는 JPQL의 EXISTS 서브쿼리를 사용하는 boolean 반환 메서드가 DB 부하를 줄일 수 있습니다.

🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 106 - 116, The current bulk query in
BookingRepository.findTableIdsWithFutureBookings returns List<Long> but callers
(TableLayoutCommandServiceImpl) only check isEmpty(), so replace or add a more
efficient boolean method: implement a JPQL method like
existsFutureBookingByTableIds (or modify findTableIdsWithFutureBookings) that
uses SELECT (COUNT(bt) > 0) or an EXISTS subquery and returns boolean, update
the repository signature accordingly and call that boolean method from
TableLayoutCommandServiceImpl instead of fetching the full List<Long> to reduce
DB load.

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public interface TableLayoutControllerDocs {

- 그리드 크기는 1x1 ~ 10x10 범위 내에서 설정 가능합니다.
- 가게당 활성 배치도는 1개만 존재하며, 새 배치도 생성 시 기존 배치도는 자동으로 비활성화됩니다.
- 기존 배치도에 존재하는 테이블에 예약이 있다면, 배치도 생성 시 LAYOUT400_2 에러메세지를 반환합니다.
- 생성된 배치도는 빈 상태로 생성되며, 이후 테이블 추가 API를 통해 테이블을 배치할 수 있습니다.
"""
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
@Builder
@SQLDelete(sql = "UPDATE table_layout SET is_deleted = true, is_active = false, deleted_at = CURRENT_TIMESTAMP WHERE id = ?")
@SQLRestriction("is_deleted = false")
@Table(name = "table_layout")
@Table(
name = "table_layout",
uniqueConstraints = {@UniqueConstraint(name = "uk_table_layout_store_active", columnNames = {"store_active_key"})}
)
Comment on lines +22 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for migration files and DDL configuration
echo "=== Checking for migration files ==="
find . -type f \( -name "*.sql" -o -name "*.xml" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | grep -iE '(migration|flyway|liquibase|changelog|db)' | head -30

echo ""
echo "=== Checking application configuration files ==="
find . -type f \( -name "application*.properties" -o -name "application*.yml" -o -name "application*.yaml" \) 2>/dev/null | head -10

echo ""
echo "=== Checking for DDL auto settings ==="
find . -type f \( -name "application*.properties" -o -name "application*.yml" -o -name "application*.yaml" \) 2>/dev/null -exec grep -H -i 'ddl-auto\|hibernate.hbm2ddl\|flyway\|liquibase' {} \;

echo ""
echo "=== Checking pom.xml or build.gradle for migration dependencies ==="
fd -e pom.xml -e gradle | head -5 | xargs grep -l -i 'flyway\|liquibase' 2>/dev/null

Repository: Eatsfine/BE

Length of output: 542


🏁 Script executed:

find . -type f -name "TableLayout.java" | head -1 | xargs cat -n

Repository: Eatsfine/BE

Length of output: 2668


DB 레벨에서 활성 배치도 중복 방지 — 접근 방식은 적절하나 DDL 관리 방식 재검토 필요합니다.

GENERATED ALWAYS AS ... STORED 가상 컬럼 + UNIQUE 제약조건을 통해 가게당 활성 배치도가 1개만 존재하도록 강제하는 패턴은 적절합니다. is_active = false일 때 NULL이 되어 UNIQUE 제약조건에 걸리지 않는 점도 정확합니다.

확인/개선 사항:

  1. GENERATED ALWAYS AS ... STOREDMySQL 전용 문법입니다. 현재 프로젝트는 Hibernate ddl-auto: update를 사용하고 있어 스키마 변경이 자동 적용되지만, 프로덕션 환경에서는 Flyway/Liquibase 같은 마이그레이션 도구 도입 또는 수동 마이그레이션 스크립트 작성을 강력히 권장합니다. (auto-DDL은 롤백 안전성이 없음)
  2. CONCAT('store_', store_id) 대신 CAST(store_id AS CHAR)만으로도 동일한 유니크 효과를 얻을 수 있어, 'store_' 접두사는 불필요한 문자열 결합입니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/table_layout/entity/TableLayout.java`
around lines 22 - 25, The `@Table` declaration in TableLayout currently relies on
a MySQL-only GENERATED ALWAYS AS ... STORED virtual column plus the
uk_table_layout_store_active unique constraint to enforce one active layout per
store; update the design to (1) remove the DB-specific virtual column approach
from automatic Hibernate DDL and instead add an explicit migration script
(Flyway/Liquibase) that creates the virtual/generated column and unique
constraint in production, and (2) simplify the generated expression to use
CAST(store_id AS CHAR) rather than CONCAT('store_', store_id) when authoring
that migration; locate the TableLayout entity and the
uk_table_layout_store_active constraint to ensure the migration matches the
entity intent and disable relying on ddl-auto:update for production schema
changes.

public class TableLayout extends BaseEntity {

@Id
Expand All @@ -39,6 +42,16 @@ public class TableLayout extends BaseEntity {
@Column(name = "is_active", nullable = false)
private boolean isActive;

// 가상 컬럼 추가 (JPA에서는 직접 사용하지 않지만 DB 레벨에서 동작)
// is_active = true일 때만 store_id 값을 가지고, false일 때는 NULL
@Column(
name = "store_active_key",
columnDefinition = "VARCHAR(50) GENERATED ALWAYS AS (CASE WHEN is_active = true THEN CONCAT('store_', store_id) ELSE NULL END) STORED",
insertable = false,
updatable = false
)
private String storeActiveKey;

@Column(name = "is_deleted", nullable = false)
@Builder.Default
private boolean isDeleted = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public enum TableLayoutErrorStatus implements BaseErrorCode {
_LAYOUT_NOT_FOUND(HttpStatus.NOT_FOUND, "LAYOUT404", "배치도를 찾을 수 없습니다."),

_LAYOUT_FORBIDDEN(HttpStatus.FORBIDDEN, "LAYOUT403", "해당 가게의 소유자만 접근 가능합니다."),

_CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS(HttpStatus.BAD_REQUEST, "LAYOUT400_2", "미래 예약이 있는 배치도는 삭제할 수 없습니다. 모든 예약이 완료된 후 재생성해주세요.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

에러 메시지가 사용자 동작과 불일치합니다.

사용자는 배치도를 "재생성"하는 동작을 수행하지만, 에러 메시지는 "삭제할 수 없습니다"로 표현되어 있어 혼동을 줄 수 있습니다. 사용자 관점의 동작에 맞게 메시지를 수정하는 것을 권장합니다.

💡 제안
-    _CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS(HttpStatus.BAD_REQUEST, "LAYOUT400_2", "미래 예약이 있는 배치도는 삭제할 수 없습니다. 모든 예약이 완료된 후 재생성해주세요.")
+    _CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS(HttpStatus.BAD_REQUEST, "LAYOUT400_2", "미래 예약이 존재하여 배치도를 재생성할 수 없습니다. 모든 예약이 완료된 후 다시 시도해주세요.")
📝 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
_CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS(HttpStatus.BAD_REQUEST, "LAYOUT400_2", "미래 예약이 있는 배치도는 삭제할 수 없습니다. 모든 예약이 완료된 후 재생성해주세요.")
_CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS(HttpStatus.BAD_REQUEST, "LAYOUT400_2", "미래 예약이 존재하여 배치도를 재생성할 수 없습니다. 모든 예약이 완료된 후 다시 시도해주세요.")
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/table_layout/exception/status/TableLayoutErrorStatus.java`
at line 17, Update the enum constant _CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS
in TableLayoutErrorStatus so the user-facing message matches the user action
(regenerate) instead of saying "cannot delete"; replace the current Korean
message with one that says the layout cannot be regenerated due to future
bookings (e.g., "미래 예약이 있는 배치도는 재생성할 수 없습니다. 모든 예약이 완료된 후 다시 시도해주세요.") so the
error reflects the "재생성" flow.

;

private final HttpStatus httpStatus;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,31 @@
package com.eatsfine.eatsfine.domain.table_layout.service;

import com.eatsfine.eatsfine.domain.booking.repository.BookingRepository;
import com.eatsfine.eatsfine.domain.store.entity.Store;
import com.eatsfine.eatsfine.domain.store.exception.StoreException;
import com.eatsfine.eatsfine.domain.store.repository.StoreRepository;
import com.eatsfine.eatsfine.domain.store.status.StoreErrorStatus;
import com.eatsfine.eatsfine.domain.store.validator.StoreValidator;
import com.eatsfine.eatsfine.domain.storetable.entity.StoreTable;
import com.eatsfine.eatsfine.domain.table_layout.converter.TableLayoutConverter;
import com.eatsfine.eatsfine.domain.table_layout.dto.req.TableLayoutReqDto;
import com.eatsfine.eatsfine.domain.table_layout.dto.res.TableLayoutResDto;
import com.eatsfine.eatsfine.domain.table_layout.entity.TableLayout;
import com.eatsfine.eatsfine.domain.table_layout.exception.TableLayoutException;
import com.eatsfine.eatsfine.domain.table_layout.exception.status.TableLayoutErrorStatus;
import com.eatsfine.eatsfine.domain.table_layout.repository.TableLayoutRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.time.LocalDate;
import java.time.LocalTime;
import java.util.List;
import java.util.Optional;

@Service
@RequiredArgsConstructor
@Transactional
public class TableLayoutCommandServiceImpl implements TableLayoutCommandService {
private final StoreRepository storeRepository;
private final TableLayoutRepository tableLayoutRepository;
private final BookingRepository bookingRepository;
private final StoreValidator storeValidator;

// 테이블 배치도 생성
Expand All @@ -32,7 +38,19 @@ public TableLayoutResDto.LayoutDetailDto createLayout(

Store store = storeValidator.validateStoreOwner(storeId, email);

deactivateExistingLayout(store);
Optional<TableLayout> existingLayout = tableLayoutRepository.findByStoreIdAndIsActiveTrue(storeId);

if (existingLayout.isPresent()) {
// 미래 예약 확인
boolean hasFutureBookings = checkFutureBookingsInLayout(existingLayout.get());

if (hasFutureBookings) {
throw new TableLayoutException(TableLayoutErrorStatus._CANNOT_DELETE_LAYOUT_WITH_FUTURE_BOOKINGS);
}

// 미래 예약이 없으면 배치도 비활성화 후 재생성
tableLayoutRepository.delete(existingLayout.get());
}
Comment on lines +41 to +53
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

delete()save() 실행 시 Hibernate 플러시 순서로 인한 Unique Constraint 위반 가능성이 있습니다.

Hibernate의 기본 플러시 순서는 INSERT → UPDATE → DELETE입니다. @SQLDelete를 사용하면 delete()가 내부적으로 UPDATE로 변환되지만, 엔티티 라이프사이클상 DELETE 단계에서 실행됩니다. 따라서:

  1. 새 배치도 INSERT (is_active=true, store_active_key='store_X') — 먼저 실행
  2. 기존 배치도 @SQLDelete UPDATE (is_active=false → store_active_key=NULL) — 나중 실행

새 배치도가 먼저 INSERT되는 시점에 기존 배치도의 store_active_key가 아직 동일한 값을 가지고 있으므로 uk_table_layout_store_active 유니크 제약조건 위반이 발생할 수 있습니다.

해결 방안: delete() 후 명시적으로 flush()를 호출하거나, delete() 대신 기존 배치도의 isActive를 직접 false로 변경 후 save하는 방식으로 처리하세요.

🐛 제안: flush를 명시적으로 호출
             // 미래 예약이 없으면 배치도 비활성화 후 재생성
             tableLayoutRepository.delete(existingLayout.get());
+            tableLayoutRepository.flush();
         }

As per coding guidelines, "DB 일관성, 롤백 정책이 올바른지 검토".

🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/table_layout/service/TableLayoutCommandServiceImpl.java`
around lines 41 - 53, The delete+save sequence risks a unique constraint
violation due to Hibernate flush ordering; after verifying future bookings with
checkFutureBookingsInLayout and before creating/saving a new TableLayout, either
(A) call tableLayoutRepository.delete(existingLayout.get()) followed immediately
by an explicit flush on the EntityManager/Repository to force the UPDATE
(SQLDelete) to execute before inserting the new row, or (B) avoid delete()
entirely and instead set existingLayout.get().setIsActive(false) (or equivalent
field setter) and save/update that entity so the deactivation UPDATE occurs
deterministically before persisting the new TableLayout; keep the existing
TableLayoutException/TableLayoutErrorStatus logic unchanged.


// 새 배치도 생성
TableLayout newLayout = TableLayout.builder()
Expand All @@ -48,9 +66,20 @@ public TableLayoutResDto.LayoutDetailDto createLayout(
return TableLayoutConverter.toLayoutDetailDto(savedLayout);
}

// 기존 테이블 배치도 비활성화
private void deactivateExistingLayout(Store store) {
tableLayoutRepository.findByStoreIdAndIsActiveTrue(store.getId())
.ifPresent(tableLayoutRepository::delete);
// 미래 예약 확인
private boolean checkFutureBookingsInLayout(TableLayout layout) {
LocalDate currentDate = LocalDate.now();
LocalTime currentTime = LocalTime.now();
Comment on lines +71 to +72
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

LocalDate.now()/LocalTime.now() 사용은 테스트 어려움과 경계값 문제를 유발합니다.

날짜와 시간을 별도로 비교하면 자정 경계에서 경합 조건이 발생할 수 있습니다 (예: now() 호출 사이에 날짜가 변경). LocalDateTime.now()를 한 번 호출하여 사용하거나, Clock을 주입하여 테스트 용이성을 확보하는 것을 권장합니다.

🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/table_layout/service/TableLayoutCommandServiceImpl.java`
around lines 75 - 76, The code calls LocalDate.now() and LocalTime.now()
separately which can cause race conditions at midnight and makes testing hard;
replace both calls by a single LocalDateTime snapshot (e.g. LocalDateTime now =
LocalDateTime.now()) and derive currentDate = now.toLocalDate() and currentTime
= now.toLocalTime(), or better inject a java.time.Clock into the class and use
LocalDateTime.now(clock) (then derive date/time) so you can control time in
tests; update any usages of the variables created from LocalDate/LocalTime
accordingly (look for the LocalDate currentDate and LocalTime currentTime
declarations in TableLayoutCommandServiceImpl).

List<Long> tableIds = layout.getTables().stream()
.map(StoreTable::getId)
.toList();

if (tableIds.isEmpty()) {
return false;
}

List<Long> tableIdsWithFutureBookings = bookingRepository.findTableIdsWithFutureBookings(tableIds, currentDate, currentTime);

return !tableIdsWithFutureBookings.isEmpty();
}
}