Skip to content

[fix] 예약/결제 동시성 취약점 수정 (레이스 컨디션 및 오버부킹) #158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zerochani wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

[fix] 예약/결제 동시성 취약점 수정 (레이스 컨디션 및 오버부킹) #158

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3ced665
fix: 예약/결제 동시성 취약점 수정 (레이스 컨디션 및 오버부킹) #157
zerochani May 7, 2026
6e3c14a
fix: 예약 취소 시 BookingTable 삭제로 유니크 제약 해제 #157
zerochani May 7, 2026
d89fd22
fix: 결제 확정 시 Booking 상태 전이 조건을 PENDING으로 한정 #157
zerochani May 7, 2026
bab83c3
fix: 스케줄러 배치 취소를 예약별 독립 트랜잭션으로 분리 #157
zerochani May 7, 2026
8ab24a9
fix: 예약 취소 시 BookingTable 삭제 대신 is_active=null 소프트 딜리트로 변경 #157
zerochani May 7, 2026
9117e44
fix: DB 유니크 제약 위반을 도메인 예외로 변환 #157
zerochani May 7, 2026
ebaed6b
chore: Flyway 도입 및 booking_table 슬롯 제약 마이그레이션 스크립트 추가 #157
zerochani May 7, 2026
8a28e52
fix: DataIntegrityViolationException을 제약 이름 기반으로 선별 변환 #157
zerochani May 7, 2026
3442430
fix: GeneralException cause 생성자 누락 수정 (CI 빌드 오류 해결) #157
zerochani May 7, 2026
aa1cbed
fix: confirmPayment에서 CANCELED 예약에 대한 보상 환불 및 상태 머신 위반 수정 #157
zerochani May 7, 2026
d01060b
fix: 스케줄러 지표 정확도 개선 — cancelIfPending boolean 반환 #157
zerochani May 7, 2026
568d29e
perf: 스케줄러 만료 예약 조회 시 ID만 직접 조회하도록 개선 #157
zerochani May 8, 2026
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
11 changes: 9 additions & 2 deletions src/main/java/com/eatsfine/domain/booking/entity/Booking.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public class Booking extends BaseEntity {
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Version
private Long version;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "user_id", nullable = false)
private User user;
Expand Down Expand Up @@ -87,6 +90,8 @@ public void addBookingTable(StoreTable storeTable) {
BookingTable bookingTable = BookingTable.builder()
.booking(this)
.storeTable(storeTable)
.bookingDate(this.bookingDate)
.bookingTime(this.bookingTime)
.build();
this.bookingTables.add(bookingTable);
}
Expand All @@ -99,10 +104,12 @@ public void confirm() {
this.status = BookingStatus.CONFIRMED;
}

public void cancel(String cancelReason)
{
public void cancel(String cancelReason) {
this.status = BookingStatus.CANCELED;
this.cancelReason = cancelReason;
// 유니크 제약 해제: 취소된 예약의 테이블 점유를 풀어 동일 시간대 재예약 허용
// orphanRemoval = true이므로 리스트 비우면 DB에서 자동 삭제됨
this.bookingTables.clear();
}

//예약과 관련된 결제 중 결제 완료된 결제키 조회
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/eatsfine/domain/booking/entity/mapping/BookingTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,21 @@
import jakarta.persistence.*;
import lombok.*;

import java.time.LocalDate;
import java.time.LocalTime;

@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
@Builder
@Getter
@Table(
name = "booking_table",
uniqueConstraints = @UniqueConstraint(
name = "uq_booking_table_slot",
columnNames = {"store_table_id", "booking_date", "booking_time"}
)
Comment on lines +18 to +21

@gemini-code-assist gemini-code-assist Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

uq_booking_table_slot 유니크 제약 조건은 오버부킹을 방지하는 강력한 수단이지만, 현재 구현상 심각한 부작용이 있습니다. 예약이 취소(CANCELED)되어도 BookingTable 레코드가 삭제되지 않고 유지되기 때문에, 취소된 예약이 점유했던 동일한 시간대에 다른 사용자가 예약을 시도하면 DB 제약 조건 위반으로 인해 재예약이 불가능해집니다. 이를 해결하려면 예약 취소 시 연관된 BookingTable 데이터를 삭제(또는 Soft Delete 처리 후 유니크 인덱스에서 제외)하는 로직이 반드시 추가되어야 합니다.

)
Comment on lines +16 to +22

@coderabbitai coderabbitai Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

기존 테이블에 NOT NULL 컬럼 추가 시 데이터 백필 마이그레이션 필요

booking_date/booking_time 컬럼은 nullable = false로 추가되지만, 기존 booking_table 행에는 해당 값이 없습니다. 마이그레이션 스크립트 없이 적용하면 NOT NULL 제약 위반으로 마이그레이션이 실패합니다.

권장 마이그레이션 순서:

  1. 컬럼을 NULLABLE로 추가
  2. 기존 행에 연관 booking의 booking_date/booking_time으로 백필
  3. 취소된 예약의 BookingTable 행을 삭제(유니크 제약 충돌 방지, Booking.cancel() 수정과 함께)
  4. NOT NULL 제약 추가
  5. 유니크 제약 추가

또한 BookingCommandServiceImpl.createBooking의 bookingRepository.flush()(Line 130)에서 유니크 제약 위반 시 DataIntegrityViolationException이 발생하므로, 글로벌 예외 핸들러에서 이를 명시적으로 "이미 예약된 슬롯" 오류로 처리해야 합니다.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/eatsfine/domain/booking/entity/mapping/BookingTable.java`
around lines 16 - 22, 테이블 매이그레이션과 예외처리를 분리해 해결하세요: BookingTable 엔티티의
booking_date/booking_time을 즉시 NOT NULL로 변경하지 말고 마이그레이션에서(1) 컬럼을 NULL 허용으로
추가하고(2) 기존 Booking 레코드의 값으로 backfill(참조: Booking → booking_date/booking_time
매핑)하며(3) 취소된 예약의 잔여 BookingTable 행은 삭제하도록 Booking.cancel() 로직을 보강해 유니크 충돌을 방지한
뒤(4) 컬럼을 NOT NULL로 변경하고(5) 유니크 제약(uq_booking_table_slot) 추가하세요; 또한
BookingCommandServiceImpl.createBooking 내 bookingRepository.flush() 호출로 발생하는
DataIntegrityViolationException을 글로벌 예외 핸들러에서 잡아 “이미 예약된 슬롯” 오류로 변환 처리하도록 예외 매핑을
추가하세요.

public class BookingTable {

@Id
Expand All @@ -23,4 +33,11 @@ public class BookingTable {
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "booking_id")
private Booking booking;

// 유니크 제약 적용을 위해 Booking의 날짜/시간을 비정규화하여 저장
@Column(name = "booking_date", nullable = false)
private LocalDate bookingDate;

@Column(name = "booking_time", nullable = false)
Comment on lines +38 to +41
private LocalTime bookingTime;
}
7 changes: 7 additions & 0 deletions src/main/java/com/eatsfine/domain/booking/repository/BookingRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
import org.springframework.data.domain.Pageable;
import org.springframework.data.repository.query.Param;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;

import jakarta.persistence.LockModeType;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
Expand Down Expand Up @@ -70,6 +73,10 @@ List<Booking> findActiveBookingsByTableAndDate(
@Param("tableId") Long tableId,
@Param("date") LocalDate date);

@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("SELECT b FROM Booking b WHERE b.id = :id")
Optional<Booking> findByIdWithLock(@Param("id") Long id);
Comment on lines +76 to +78

@coderabbitai coderabbitai Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

findByIdWithLock에 락 타임아웃이 설정되지 않아 무한 블로킹 위험

@Lock(PESSIMISTIC_WRITE)는 SELECT ... FOR UPDATE를 발행하지만, 타임아웃 힌트가 없으면 락을 보유한 트랜잭션이 느릴 때(예: cancelBooking/cancelBookingByOwner에서 Toss 외부 API 호출 중) 대기 트랜잭션이 무한정 블로킹되어 DB 커넥션 풀이 고갈될 수 있습니다.

🛡️ 제안 수정 
+import org.springframework.data.jpa.repository.QueryHints;
+import jakarta.persistence.QueryHint;
...
 `@Lock`(LockModeType.PESSIMISTIC_WRITE)
+@QueryHints(`@QueryHint`(name = "jakarta.persistence.lock.timeout", value = "3000"))
 `@Query`("SELECT b FROM Booking b WHERE b.id = :id")
 Optional<Booking> findByIdWithLock(`@Param`("id") Long id);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/eatsfine/domain/booking/repository/BookingRepository.java`
around lines 76 - 78, findByIdWithLock currently uses `@Lock`(PESSIMISTIC_WRITE)
without a lock timeout, which can cause indefinite blocking and exhaustion of DB
connections when callers like cancelBooking or cancelBookingByOwner perform long
external calls; add a pessimistic lock timeout hint to the repository method
(e.g., via `@QueryHints/`@QueryHint with "javax.persistence.lock.timeout" set to a
sensible millisecond value such as 5000) on the findByIdWithLock method so the
DB will fail the wait after the configured timeout instead of blocking forever.


Optional<Booking> findByIdAndStatus(Long bookingId, BookingStatus status);

/**
Expand Down
39 changes: 39 additions & 0 deletions src/main/java/com/eatsfine/domain/booking/service/BookingCancelExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.eatsfine.domain.booking.service;

import com.eatsfine.domain.booking.enums.BookingStatus;
import com.eatsfine.domain.booking.repository.BookingRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;

import java.time.LocalDateTime;
import java.util.List;

@Component
@RequiredArgsConstructor
@Slf4j
public class BookingCancelExecutor {

private final BookingRepository bookingRepository;

@Transactional(readOnly = true)
public List<Long> findExpiredPendingIds(LocalDateTime threshold) {
return bookingRepository.findAllByStatusAndCreatedAtBefore(BookingStatus.PENDING, threshold)
.stream()
.map(booking -> booking.getId())
.toList();
}

// REQUIRES_NEW: 호출마다 독립 트랜잭션 — 하나 실패해도 다른 예약에 영향 없음
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void cancelIfPending(Long bookingId) {
bookingRepository.findByIdWithLock(bookingId).ifPresent(booking -> {
if (booking.getStatus() == BookingStatus.PENDING) {
booking.cancel("결제 시간 초과로 인한 자동 취소");
log.info("예약 ID {} 자동 취소 완료", bookingId);
}
});
}
Comment on lines +22 to +40

@coderabbitai coderabbitai Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

락/REQUIRES_NEW 적용은 잘 되어 있으며, 매직 문자열 상수화 및 ID 전용 프로젝션 쿼리 권장

전체 구조(락 재조회 + 독립 트랜잭션 + PENDING 재검증)는 적절합니다. 두 가지 사소한 개선 의견입니다.

  1. "결제 시간 초과로 인한 자동 취소"는 매직 문자열입니다. 향후 다른 자동 취소 사유가 추가되거나 다국어화 시 분기점이 되도록 상수로 추출하는 편이 안전합니다. (코드 품질 가이드: 매직 넘버/하드코딩 값 점검)
  2. findExpiredPendingIds가 만료 후보 Booking 엔티티 전체를 로드한 뒤 ID만 추출합니다. 만료 건수가 많아질 경우 메모리/연관 lazy 프록시 비용이 누적되므로, 리포지토리에 @Query("select b.id from Booking b where b.status = ?1 and b.createdAt < ?2") 형태의 ID 전용 쿼리를 추가하는 편이 효율적입니다. (성능 가이드: 불필요한 DB 쿼리/Stream 효율성)
♻️ 제안 수정 — 상수 추출 및 메소드 레퍼런스 
+    private static final String CANCEL_REASON_PAYMENT_TIMEOUT = "결제 시간 초과로 인한 자동 취소";
+
     `@Transactional`(readOnly = true)
     public List<Long> findExpiredPendingIds(LocalDateTime threshold) {
         return bookingRepository.findAllByStatusAndCreatedAtBefore(BookingStatus.PENDING, threshold)
                 .stream()
-                .map(booking -> booking.getId())
+                .map(Booking::getId)
                 .toList();
     }

     // REQUIRES_NEW: 호출마다 독립 트랜잭션 — 하나 실패해도 다른 예약에 영향 없음
     `@Transactional`(propagation = Propagation.REQUIRES_NEW)
     public void cancelIfPending(Long bookingId) {
         bookingRepository.findByIdWithLock(bookingId).ifPresent(booking -> {
             if (booking.getStatus() == BookingStatus.PENDING) {
-                booking.cancel("결제 시간 초과로 인한 자동 취소");
+                booking.cancel(CANCEL_REASON_PAYMENT_TIMEOUT);
                 log.info("예약 ID {} 자동 취소 완료", bookingId);
             }
         });
     }

위 코딩 가이드라인의 “3. 성능 및 효율성: 불필요한 DB 쿼리 호출…”과 “2. 코드 품질 & 가독성: 매직 넘버, 하드코딩된 값이 존재하는지 점검”에 따른 의견입니다.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/eatsfine/domain/booking/service/BookingCancelExecutor.java`
around lines 22 - 38, Extract the hard-coded cancel reason in cancelIfPending
("결제 시간 초과로 인한 자동 취소") into a constant (e.g., AUTO_CANCEL_REASON or
Booking.CANCEL_REASON_AUTO_TIMEOUT) and use that constant in Booking.cancel call
and any related logs; also change findExpiredPendingIds to avoid loading full
Booking entities by adding an ID-only repository query (e.g., add a method like
findIdsByStatusAndCreatedAtBefore in BookingRepository implemented with
`@Query`("select b.id from Booking b where b.status = ?1 and b.createdAt < ?2"))
and update findExpiredPendingIds to call that new repository method instead of
findAllByStatusAndCreatedAtBefore to reduce memory/ORM overhead.

}
6 changes: 3 additions & 3 deletions src/main/java/com/eatsfine/domain/booking/service/BookingCommandServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public BookingResponseDTO.CreateBookingResultDTO createBooking(Long userId, Long
@Transactional
public BookingResponseDTO.ConfirmPaymentResultDTO confirmPayment(Long bookingId, BookingRequestDTO.PaymentConfirmDTO dto) {

Booking booking = bookingRepository.findById(bookingId)
Booking booking = bookingRepository.findByIdWithLock(bookingId)
.orElseThrow(() -> new BookingException(BookingErrorStatus._BOOKING_NOT_FOUND));

//이미 예약이 확정됐는지 최종 확인
Expand Down Expand Up @@ -182,7 +182,7 @@ public BookingResponseDTO.ConfirmPaymentResultDTO confirmPayment(Long bookingId,
public BookingResponseDTO.CancelBookingResultDTO cancelBooking(Long userId, Long bookingId, BookingRequestDTO.CancelBookingDTO dto) {


Booking booking = bookingRepository.findById(bookingId)
Booking booking = bookingRepository.findByIdWithLock(bookingId)
.orElseThrow(() -> new BookingException(BookingErrorStatus._BOOKING_NOT_FOUND));


Expand Down Expand Up @@ -220,7 +220,7 @@ public BookingResponseDTO.OwnerCancelBookingResultDTO cancelBookingByOwner(Long
storeValidator.validateStoreOwner(storeId, email);

// 1. 예약 존재 확인
Booking booking = bookingRepository.findById(bookingId)
Booking booking = bookingRepository.findByIdWithLock(bookingId)
.orElseThrow(() -> new BookingException(BookingErrorStatus._BOOKING_NOT_FOUND));

// 2. 데이터 무결성 검증
Expand Down
34 changes: 17 additions & 17 deletions src/main/java/com/eatsfine/domain/booking/service/BookingScheduler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package com.eatsfine.domain.booking.service;

import com.eatsfine.domain.booking.entity.Booking;
import com.eatsfine.domain.booking.enums.BookingStatus;
import com.eatsfine.domain.booking.repository.BookingRepository;
import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Scheduled;
Expand All @@ -17,33 +13,37 @@
@Slf4j
public class BookingScheduler {

private final BookingRepository bookingRepository;
private final BookingCancelExecutor bookingCancelExecutor;

/**
* 결제 미완료(PENDING) 상태로 10분이 경과한 예약을 주기적으로 취소 처리
* cron: 0분부터 10분 단위로 실행 (0, 10, 20, 30, 40, 50분)
*
* 각 예약을 독립 트랜잭션(REQUIRES_NEW)으로 처리하여
* 낙관적 락 충돌 등 일부 실패가 전체 배치에 영향을 주지 않음
*/
@Scheduled(cron = "0 0/10 * * * *")
@Transactional
public void cleanupExpiredPendingBookings() {
LocalDateTime threshold = LocalDateTime.now().minusMinutes(10);

// 1. 10분 전보다 이전에 생성되었고, 여전히 PENDING인 예약 조회
List<Booking> expiredBookings = bookingRepository.findAllByStatusAndCreatedAtBefore(
BookingStatus.PENDING,
threshold
);
List<Long> expiredIds = bookingCancelExecutor.findExpiredPendingIds(threshold);

if (expiredBookings.isEmpty()) {
if (expiredIds.isEmpty()) {
return;
}

log.info("스케줄러 실행: 만료된 PENDING 예약 {}건을 취소 처리합니다.", expiredBookings.size());
log.info("스케줄러 실행: 만료된 PENDING 예약 {}건 처리 시작", expiredIds.size());

// 2. 상태 변경 및 로그 기록
expiredBookings.forEach(booking -> {
booking.cancel("결제 시간 초과로 인한 자동 취소");
});
int successCount = 0;
for (Long id : expiredIds) {
try {
bookingCancelExecutor.cancelIfPending(id);
successCount++;
} catch (Exception e) {
log.warn("예약 ID {} 자동 취소 실패 — 다음 실행에서 재시도: {}", id, e.getMessage());
}
}

log.info("스케줄러 완료: {}건 성공 / {}건 시도", successCount, expiredIds.size());
}
}
13 changes: 9 additions & 4 deletions src/main/java/com/eatsfine/domain/payment/service/PaymentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.eatsfine.domain.payment.service;

import com.eatsfine.domain.booking.entity.Booking;
import com.eatsfine.domain.booking.enums.BookingStatus;
import com.eatsfine.domain.booking.repository.BookingRepository;
import com.eatsfine.domain.payment.dto.request.PaymentWebhookDTO;
import com.eatsfine.domain.payment.dto.request.PaymentConfirmDTO;
Expand Down Expand Up @@ -115,11 +116,15 @@ public PaymentResponseDTO.PaymentSuccessResultDTO confirmPayment(PaymentConfirmD
provider,
response.receipt() != null ? response.receipt().url() : null);

Booking booking = payment.getBooking(); // 결제 엔티티에 매핑된 예약 객체 가져오기
Booking booking = payment.getBooking();
if (booking != null) {
// 예약 상태를 CONFIRMED로 변경
booking.confirm();
log.info("Booking confirmed for OrderID: {}", dto.orderId());
// 비관적 락으로 재조회하여 스케줄러 / 다른 스레드와의 동시 수정 방지
Booking lockedBooking = bookingRepository.findByIdWithLock(booking.getId())
.orElse(null);
if (lockedBooking != null && lockedBooking.getStatus() == BookingStatus.PENDING) {
lockedBooking.confirm();
Comment on lines +123 to +146
log.info("Booking confirmed for OrderID: {}", dto.orderId());
}
}


Expand Down
Loading