Skip to content

Sisc1 be 베팅게임 수정#137

Merged
Kosw6 merged 11 commits intomainfrom
SISC1-BE-베팅게임-수정
Nov 24, 2025

Hidden character warning

The head ref may contain hidden characters: "SISC1-BE-\ubca0\ud305\uac8c\uc784-\uc218\uc815"
Merged

Sisc1 be 베팅게임 수정#137
Kosw6 merged 11 commits intomainfrom
SISC1-BE-베팅게임-수정

Conversation

@cksdid202
Copy link
Contributor

@cksdid202 cksdid202 commented Nov 24, 2025

Summary by CodeRabbit

  • 버그 수정

    • 베팅 취소 시 동시성(낙관적 락) 처리로 경쟁 조건 방지 및 안정성 향상
    • 중복/다중 취소 요청에 대한 검증 및 적절한 오류 매핑 적용
    • 취소 시 포인트 환불과 통계 차감이 트랜잭션 내에서 일관되게 적용되도록 수정
    • 취소 관련 사용자 안내 메시지 문구 개선
  • 신규 기능

    • 삭제 대신 상태 변경으로 취소 이력 보존하여 기록 유지

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

배팅 취소 흐름을 JPQL 원자 업데이트에서 엔티티 기반 낙관적 잠금으로 전환: 저장소의 JPQL 업데이트 메서드 제거, UserBet에 @Version과 cancel() 추가, BettingService 취소 로직을 버전 충돌 처리(예외 캐치) 방식으로 재구성 및 메시지 문구 소폭 수정.

Changes

코호트 / 파일(s) 변경 요약
저장소 메서드 제거
backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java
updateStatusToCanceled(UUID id, UUID userId, BetStatus oldStatus, BetStatus newStatus) 메서드 삭제 — JPQL 기반 원자적 상태 업데이트 API 제거
엔티티 변경 (낙관적 잠금)
backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java
@Version 필드(private Long version) 추가로 낙관적 잠금 도입. public void cancel() 메서드 추가하여 내부에서 betStatusDELETED로 변경
서비스 로직 리팩토링
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
취소 흐름을 엔티티 조회·검증 기반으로 재구성. saveAndFlush로 버전 충돌을 감지하고 ObjectOptimisticLockingFailureException을 캐치하여 BET_ALREADY_PROCESSED로 매핑. 환불 및 통계 차감 시점 조정
에러 메시지 수정
backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java
BET_ALREADY_PROCESSED 메시지 문법 수정: "이미 취소 되거나 처리된 베팅입니다." → "이미 취소되었거나 처리된 베팅입니다."

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant BettingService
    participant UserBetRepo as UserBet Repository
    participant UserBetEntity as UserBet (Entity)
    participant BetRoundRepo as BetRound Repository
    participant Database
    participant ExceptionHandler

    User->>BettingService: cancelUserBet(userId, userBetId)
    
    BettingService->>UserBetRepo: findById(userBetId)
    UserBetRepo-->>BettingService: UserBet 엔티티
    
    BettingService->>UserBetEntity: 상태 검증 (ACTIVE 여부)
    BettingService->>BetRoundRepo: findById(roundId)
    BetRoundRepo-->>BettingService: BetRound 데이터
    
    rect rgb(220,235,255)
        note right of BettingService: 낙관적 잠금 적용 경로
        BettingService->>UserBetEntity: cancel() 호출 (betStatus -> DELETED)
        BettingService->>Database: saveAndFlush(UserBet) (version 검증)
    end

    alt 성공
        Database-->>BettingService: 저장 완료
        BettingService->>Database: 환불 처리, 통계 차감
        BettingService-->>User: 취소 완료 응답
    else 동시성 충돌
        Database->>ExceptionHandler: ObjectOptimisticLockingFailureException
        ExceptionHandler-->>BettingService: BET_ALREADY_PROCESSED 매핑
        BettingService-->>User: 이미 처리됨 오류 응답
    end
Loading

Estimated code review effort

🎯 3 (중간) | ⏱️ ~25분

  • 추가 주의점:
    • 저장소 메서드 제거로 인한 다른 호출 지점(컴파일 오류) 확인
    • 낙관적 잠금 예외 처리( ObjectOptimisticLockingFailureException ) 매핑 및 테스트 커버리지
    • 환불·통계 차감 순서와 트랜잭션 경계 검증
    • 성능 영향(일괄 JPQL → 개별 엔티티 로드/저장) 검토

Possibly related PRs

Suggested reviewers

  • discipline24

Poem

낙관의 귀를 세우고,
버전 페이지를 토닥이며,
삭제 대신 기록을 남긴다.
배치의 물결 대신 작은 발자국으로,
햇살 속에서 취소는 조용히 풀리네 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive 제목이 변경 사항의 주요 내용을 명확하게 설명하지 못하고 있습니다. '베팅게임 수정'은 매우 일반적이고 모호한 표현으로, 구체적인 변경 내용(낙관적 잠금 추가, 베팅 취소 로직 개선 등)을 전달하지 못합니다. 제목을 더 구체적으로 수정하세요. 예: '베팅 취소 시 낙관적 잠금(Optimistic Locking) 추가' 또는 '베팅 취소 기능 개선 및 동시성 제어 추가' 같이 주요 변경 사항을 명확히 반영하는 제목을 사용하기를 권장합니다.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch SISC1-BE-베팅게임-수정

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java (1)

34-42: 통계 감소 쿼리에서 음수 방어 로직을 추가하는 것을 고려해 주세요

decrementUpStats / decrementDownStats 가 증가 로직과 잘 대응되어 있고, 원자적 업데이트 관점에서는 괜찮습니다.
다만 현재 쿼리는 단순 - 1, - :points 이라, 통계가 어긋난 상태(예: 과거 버그, 수동 수정 등)에서 호출되면 upBetCount / downBetCount*TotalPoints 가 음수가 될 수 있습니다.

정상 흐름만 놓고 보면 문제는 없겠지만, 방어적으로는 다음과 같은 형태를 고려해볼 만 합니다.

  • CASE WHEN 을 사용해 최소 0 을 보장하는 형태
  • 혹은 WHERE 절에 b.upBetCount > 0 / b.downBetCount > 0b.*TotalPoints >= :points 와 같은 조건 추가

비즈니스에서 통계 음수가 절대 허용되지 않는다면, 위와 같이 방어 로직을 추가하는 것이 장기적으로 안정성에 도움이 될 것 같습니다.

backend/src/main/java/org/sejongisc/backend/betting/dto/UserBetResponse.java (1)

27-33: BetRound 지역 변수를 builder 전체에서 재사용하면 가독성이 좋아집니다

현재 from 내부에서

BetRound round = bet.getRound();

를 선언해 두셨는데, 아래 builder 부분에서는 여전히 bet.getRound() 를 반복 호출하고 있습니다. 기능상 문제는 없지만, 다음처럼 round 를 일관되게 쓰면 코드가 더 읽기 쉽고, 나중에 LAZY 로딩/N+1 이슈를 추적할 때도 명확해집니다.

-        return UserBetResponse.builder()
-                .userBetId(bet.getUserBetId())
-                .betRoundId(bet.getRound().getBetRoundID())
-                .roundTitle(bet.getRound().getTitle())
-                .symbol(bet.getRound().getSymbol())
+        return UserBetResponse.builder()
+                .userBetId(bet.getUserBetId())
+                .betRoundId(round.getBetRoundID())
+                .roundTitle(round.getTitle())
+                .symbol(round.getSymbol())
                 ...
-                .previousClosePrice(round.getPreviousClosePrice())
-                .settleClosePrice(round.getSettleClosePrice())
-                .upBetCount(round.getUpBetCount())
-                .downBetCount(round.getDownBetCount())
+                .previousClosePrice(round.getPreviousClosePrice())
+                .settleClosePrice(round.getSettleClosePrice())
+                .upBetCount(round.getUpBetCount())
+                .downBetCount(round.getDownBetCount())

통계/가격 필드 매핑 자체는 일관적이고 DTO 확장 방향도 좋아 보입니다.

Also applies to: 36-37, 50-53

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba009c4 and fa4e95d.

📒 Files selected for processing (5)
  • backend/src/main/java/org/sejongisc/backend/betting/dto/UserBetResponse.java (3 hunks)
  • backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java (1 hunks)
  • backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java (2 hunks)
  • backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1 hunks)
  • backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java (1 hunks)
🔇 Additional comments (1)
backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java (1)

4-10: JPQL 기반 상태 전환 메서드 설계가 명확하고 취소 idempotency 확보에 적합해 보입니다

updateStatusToCanceled 를 JPQL UPDATE 로 구현하고, userBetId + userId + oldStatus 를 모두 WHERE 절에 건 구조가 명확해서 동시 취소 시 하나의 요청만 성공하도록 잘 설계된 것 같습니다.
int 반환값으로 호출 측에서 updatedCount == 0 인 경우를 구분하는 패턴도 자연스럽습니다.

Also applies to: 24-27

Comment on lines 206 to 258
/**
* 사용자 베팅 취소
* 사용자 베팅 취소 (수정됨)
*/
@Transactional
public void cancelUserBet(UUID userId, UUID userBetId) {
// 1. 먼저 베팅 정보를 조회 (검증용)
UserBet userBet = userBetRepository.findByUserBetIdAndUserId(userBetId, userId)
.orElseThrow(() -> new CustomException(ErrorCode.BET_NOT_FOUND));

BetRound betRound = userBet.getRound();
// 2. [핵심] 상태를 ACTIVE -> DELETED로 변경 시도
// 이 쿼리는 동시에 여러 요청이 와도 단 하나만 1을 반환합니다. (나머지는 0)
int updatedCount = userBetRepository.updateStatusToCanceled(
userBetId,
userId,
BetStatus.ACTIVE,
BetStatus.DELETED //
);

if (updatedCount == 0) {
// 이미 취소되었거나 처리된 베팅임 -> 중복 처리 방지
throw new CustomException(ErrorCode.BET_ALREADY_PROCESSED);
}

// 3. 상태 변경에 성공한 딱 1개의 요청만 아래 환불/통계 로직 수행
// [수정] ID를 이용해 BetRound를 직접 다시 조회 (확실한 세션 보장)
UUID roundId = userBet.getRound().getBetRoundID(); // ID만 꺼내는 건 에러 안 남
BetRound betRound = betRoundRepository.findById(roundId)
.orElseThrow(() -> new CustomException(ErrorCode.BET_ROUND_NOT_FOUND));

// 이제 안전하게 검증 가능
betRound.validate();

// 포인트 환불
if (!userBet.isFree() && userBet.getStakePoints() > 0) {
pointHistoryService.createPointHistory(
userId,
userBet.getStakePoints(),
PointReason.BETTING,
PointOrigin.BETTING,
userBet.getUserBetId()
betRound.getBetRoundID() // 밑에서 설명할 targetId 이슈 확인 필요
);
}

userBetRepository.delete(userBet);
// 통계 차감
int stake = userBet.getStakePoints();
if (userBet.getOption() == BetOption.RISE) {
betRoundRepository.decrementUpStats(betRound.getBetRoundID(), stake);
} else {
betRoundRepository.decrementDownStats(betRound.getBetRoundID(), stake);
}

// 삭제(delete)는 하지 않음 (이력 관리를 위해)
}
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

🧩 Analysis chain

취소 JPQL bulk update 와 정산(saveAll) 간 동시성 충돌 가능성이 있습니다

현재 취소 흐름 자체(원자적 상태 변경 + 통계 감소 + 포인트 환불)는 잘 정리되어 있습니다만, UserBet 행에 대해 취소 트랜잭션과 정산 트랜잭션이 동시에 접근하는 경우 다음과 같은 경쟁 상태가 생길 수 있습니다.

  1. 정산(settleUserBets) 트랜잭션이 findAllByRound(round)UserBet 엔티티들을 ACTIVE 상태로 미리 로딩합니다.
  2. 그 사이 다른 요청에서 cancelUserBet 가 실행되어, updateStatusToCanceled(...) JPQL bulk update 로 해당 bet 의 betStatus 를 DB 상에서 ACTIVE -> DELETED 로 변경합니다.
  3. 정산 쪽은 여전히 1차 캐시에 남아 있는 bet 엔티티를 보고 bet.getBetStatus() == BetStatus.ACTIVE 로 판단하여 bet.win(...) 또는 bet.lose() 를 호출합니다.
  4. 마지막에 userBetRepository.saveAll(userBets) 가 flush 되면서, 정산 트랜잭션이 DELETED 로 바뀌어 있던 DB row 를 WIN/LOSE 등으로 다시 덮어써 버릴 수 있습니다 (UserBet 에 @Version 이 없다면).

이렇게 되면,

  • 이미 취소된 베팅이 다시 정산되고,
  • 취소 환불과 정산 보상/손실이 동시에 적용되어 포인트 이력이 꼬일 수 있습니다.

가능한 대응 방향 예시입니다.

  • UserBet 엔티티에 @Version 컬럼을 추가해 낙관적 락을 적용하고, 정산/취소 중 한쪽이 먼저 커밋되면 다른 쪽은 OptimisticLockException 으로 실패하도록 설계
    → 실패 시 재시도 또는 “이미 처리된 베팅입니다”류 에러로 매핑
  • 혹은 정산 로직도 취소와 마찬가지로 조건부 JPQL UPDATE 를 사용해, WHERE betStatus = ACTIVE 인 행만 상태/포인트를 갱신하고 영향받은 row 수로 성공/실패를 판단
  • 동시 실행 타이밍/스케줄 상 “정산 시점에는 더 이상 취소가 절대 안 들어온다”는 강한 운영 가정을 두고 있다면, 코드나 주석으로 그 전제를 명시해 두는 것도 좋겠습니다.

또 한 가지, @Modifying(clearAutomatically = true) 때문에 updateStatusToCanceled 이후의 userBet 는 detach 상태입니다. 현재는 읽기만 하므로 문제는 없지만, 이 메서드에서 나중에 userBet 를 수정하려는 변경이 들어올 경우 반영이 안 되는 함정이 있을 수 있어, “이 아래에서는 userBet 는 읽기 전용으로만 사용한다” 정도의 주석을 남겨두면 유지보수에 도움이 될 것 같습니다.

마지막으로, 환불 시 targetIdbetRound.getBetRoundID() 로 통일하신 것은 postUserBet / settleUserBets 와 일관성이 있어 보입니다. 기존에 userBetId 를 쓰던 코드/리포트가 있었다면, 이 변경으로 영향 받는 곳이 없는지만 한 번 더 확인해 보시면 좋겠습니다.



---

🏁 Script executed:

```shell
#!/bin/bash
# 1. UserBet 엔티티 파일 찾기 및 @Version 필드 확인
echo "=== Searching for UserBet entity ==="
fd -e java "UserBet" --type f | grep -i entity

echo -e "\n=== Checking UserBet for @Version and @OptimisticLock ==="
fd -e java "UserBet" --type f | xargs grep -l "class UserBet" | head -1 | xargs cat

Length of output: 1588


🏁 Script executed:

#!/bin/bash
# 2. settleUserBets 메서드 조회
echo "=== Searching for settleUserBets method ==="
rg -n "settleUserBets" --type java -A 50

Length of output: 7247


🏁 Script executed:

#!/bin/bash
# 3. updateStatusToCanceled 메서드 확인
echo "=== Searching for updateStatusToCanceled method ==="
rg -n "updateStatusToCanceled" --type java -B 3 -A 10

Length of output: 2519


🏁 Script executed:

#!/bin/bash
# 4. pointHistoryService 호출시 targetId/userBetId 사용 패턴
echo "=== Searching for pointHistoryService.createPointHistory calls ==="
rg -n "createPointHistory" --type java -B 5 -A 5

Length of output: 16257


🏁 Script executed:

#!/bin/bash
# 5. BetStatus 필드와 bet.win/lose 메서드 확인
echo "=== Checking UserBet win/lose methods ==="
rg -n "\.win\(|\.lose\(" --type java -B 2 -A 2 | head -40

Length of output: 1259



---

🏁 Script executed:

```shell
#!/bin/bash
# UserBet 엔티티 파일 직접 조회
echo "=== Checking UserBet entity file ==="
cat backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java

Length of output: 1763


🏁 Script executed:

#!/bin/bash
# win/lose 메서드 구현 확인
echo "=== Searching for bet.win and bet.lose implementations ==="
rg -n "public void win|public void lose" --type java -A 10

Length of output: 1324


🏁 Script executed:

#!/bin/bash
# 낙관적 락 관련 어노테이션 확인 (전체 betting 모듈)
echo "=== Searching for @Version in betting module ==="
rg -n "@Version" backend/src/main/java/org/sejongisc/backend/betting/

Length of output: 48


🏁 Script executed:

#!/bin/bash
# settleUserBets와 cancelUserBet의 트랜잭션 격리 수준 확인
echo "=== Checking Transactional isolation levels in BettingService ==="
rg -n "@Transactional" backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java -B 2 -A 20 | head -80

Length of output: 3159


취소(JPQL bulk update)와 정산(엔티티 기반 업데이트) 간 동시성 충돌 위험이 확인되었습니다.

검증 결과, 리뷰 코멘트의 우려가 정확합니다:

  1. @Version 필드가 없어서 낙관적 락이 적용되지 않음: UserBet 엔티티에 버전 관리 필드가 없습니다.

  2. 동시성 경쟁 상태 재현:

    • settleUserBetsfindAllByRound(round)UserBet들을 ACTIVE 상태로 1차 캐시에 로드
    • 이 사이 cancelUserBetupdateStatusToCanceled JPQL로 DB 행을 ACTIVE → DELETED로 변경
    • settleUserBetsbet.win(reward) / bet.lose() 호출 시 betStatus가 CLOSED로 설정되고 (UserBet.java:60, 66 참조)
    • userBetRepository.saveAll(userBets) flush 시 1차 캐시의 엔티티가 DB의 DELETED 행을 CLOSED로 덮어씀
    • 결과: 취소된 베팅이 다시 정산되고 포인트 이력 중복 기록
  3. targetId 변경은 타당함: 이전 테스트 코드(line 429)에서 userBetId 사용을 시도했으나, 현재는 모든 호출에서 roundId(betRound.getBetRoundID())로 통일되어 일관성이 있습니다.

해결 방안:

  • UserBet 엔티티에 @Version 컬럼을 추가하고 두 메서드에서 모두 엔티티 수정/저장 방식으로 통일
  • 또는 settleUserBets에서도 조건부 JPQL UPDATE로 변경 (WHERE betStatus = ACTIVE 포함)
  • 또는 격리 수준을 SERIALIZABLE로 상향하고 정산 시 비관적 락 추가
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
around lines 206 to 258, there's a race between JPQL bulk UPDATE in
cancelUserBet and entity-based updates in settleUserBets causing canceled bets
to be re-settled; fix by introducing optimistic locking and/or aligning update
strategies: add a @Version field to the UserBet entity (and database
column/migration) so JPQL bulk updates will conflict with stale entity saves,
then update cancelUserBet and settleUserBets to perform entity-based state
changes (load entity, set status, save) or change settleUserBets to perform
conditional JPQL UPDATEs that include WHERE betStatus = :ACTIVE; alternatively,
if opting for pessimistic control, escalate transaction isolation and apply
explicit pessimistic locks when loading UserBet/BetRound to prevent concurrent
overwrite.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java (1)

57-60: UserBet에 @Version + cancel 도입 방향 좋습니다

엔티티에 @Version 필드와 cancel() 도메인 메서드를 추가해서 상태 변경을 엔티티 내부에 모으고, 정산/취소 간 동시성도 낙관적 락으로 막을 수 있게 된 점이 좋습니다. 운영 측면에서는 해당 version 컬럼에 대한 DB 마이그레이션이 실제로 반영되었는지만 한 번만 확인해 두시면 될 것 같습니다.

한 가지 사소한 부분으로, 서비스 코드 주석에서는 “ACTIVE → CANCELED”라고 표현하면서 실제 상태값은 BetStatus.DELETED를 쓰고 있어서, 향후 유지보수 시 혼동을 막기 위해 용어(취소 vs 삭제)만 맞춰 두면 더 깔끔할 것 같습니다.

Also applies to: 73-76

backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

265-305: 정산(settleUserBets)와 취소 간 동시성은 해결되었지만, 한 건 충돌 시 전체 정산 롤백 가능성은 인지 필요

UserBet@Version을 추가하고 취소도 엔티티 기반으로 변경했기 때문에, 이제 정산 중인 베팅을 동시에 취소하는 경우 후순위 트랜잭션이 saveAll(userBets) 단계에서 낙관적 락 예외로 실패하게 되어, 이전 리뷰에서 지적된 “취소된 베팅이 다시 정산되는” 문제는 해소된 것으로 보입니다.

다만 이 구조에서는 단일 베팅의 충돌로 해당 라운드 전체 settleUserBets 트랜잭션이 롤백될 수 있으므로, 스케줄러/호출 측에서 ObjectOptimisticLockingFailureException 혹은 그에 매핑된 도메인 예외를 잡아 로그를 남기고 재시도/스킵 전략을 어떻게 가져갈지 한 번만 정리해 두시면 좋겠습니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa4e95d and 7112170.

📒 Files selected for processing (4)
  • backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java (2 hunks)
  • backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java (1 hunks)
  • backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (2 hunks)
  • backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java
🔇 Additional comments (2)
backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java (1)

15-24: 취소용 JPQL bulk update 제거 결정 적절합니다

updateStatusToCanceled 같은 상태 변경용 JPQL bulk update를 제거하고, 이제는 UserBet 엔티티 + @Version 기반으로 취소를 처리하는 방향이므로, 리포지토리를 조회용 메서드 위주로 정리한 것은 동시성·도메인 일관성 측면에서 좋은 정리 같습니다.

backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

92-100: getAllMyBets에 read‑only 트랜잭션 추가 적절합니다

조회 전용 메서드에 @Transactional(readOnly = true)를 붙여서 UserBetUserBetResponse 변환 시 Lazy 로딩/프록시 문제 없이 안전하게 동작하도록 한 부분 좋습니다.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

21-21: 임시용 주석(// import 확인)은 제거하는 편이 더 깔끔합니다

ObjectOptimisticLockingFailureException import 자체는 적절하지만, “import 확인” 주석은 일회성 확인 용도로 보입니다. 유지보수 시 혼란을 줄이기 위해 해당 주석은 제거하는 것을 권장드립니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7112170 and 45bd32b.

📒 Files selected for processing (1)
  • backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (2 hunks)
🔇 Additional comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

92-92: getAllMyBets@Transactional(readOnly = true) 적용은 적절합니다

조회 전용 메서드에 read‑only 트랜잭션을 지정해 세션을 유지하면서 DTO 변환을 수행하는 접근이 합리적입니다. Lazy 로딩/DTO 매핑 의도와도 잘 맞고, 쓰기 방지 측면에서도 도움이 됩니다.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

266-307: settleUserBets 메서드에 OptimisticLockException 처리 추가 필수

검증 결과: 메서드 내 예외 처리가 완전히 부재하며, 예외가 발생하면 거래 전체가 롤백됩니다.

문제점:

  • Line 305 userBetRepository.saveAll(userBets)에서 OptimisticLockException 발생 시 catch되지 않음
  • BettingScheduler와 BettingAdminController 모두 예외 처리 없음
  • 스케줄러 컨텍스트에서는 Spring이 로깅하지만 재시도하지 않음
  • 동시 취소 시 일부 베팅이 정산되지 않고 다음 스케줄까지 대기

해결 방안:

  1. settleUserBets@Retryable 추가 (PointHistoryService처럼)
  2. 또는 개별 베팅마다 try-catch로 부분 실패 허용
  3. 또는 saveAll 전에 재조회하여 현재 버전 확인
🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

212-260: 낙관적 잠금 구현이 올바르게 적용되었습니다

이전 리뷰에서 지적된 문제들이 모두 해결되었습니다:

  • userBet.cancel() 호출로 상태 변경이 올바르게 수행됩니다 (Line 232)
  • saveAndFlush()로 버전 충돌을 즉시 감지합니다 (Line 233)
  • 포인트 환불과 통계 차감이 상태 변경 성공 이후에만 실행됩니다
  • 동시성 충돌 시 BET_ALREADY_PROCESSED로 적절히 매핑됩니다 (Line 258)

다만 두 가지 사소한 개선 사항이 있습니다:

  1. Line 231 주석 정확도: "커밋 시점에 버전 충돌 여부를 체크함"이라고 되어 있으나, saveAndFlush() 사용으로 실제로는 flush 시점(즉시)에 체크가 발생합니다.

  2. Line 254 주석 제거 필요: saveAndFlush()가 이미 Line 233에서 실행되었으므로 이 주석은 더 이상 의미가 없습니다.

-            // 여기서 @Version 필드 덕분에 커밋 시점에 버전 충돌 여부를 체크함
+            // 여기서 @Version 필드 덕분에 flush 시점에 버전 충돌 여부를 체크함
             userBet.cancel(); 
             userBetRepository.saveAndFlush(userBet); // 명시적 flush로 버전 충돌 즉시 감지
 
@@ -251,8 +251,6 @@
             } else {
                 betRoundRepository.decrementDownStats(betRound.getBetRoundID(), stake);
             }
-
-            // userBetRepository.save(userBet); // Transactional이라 자동 저장되지만 명시해도 됨
 
         } catch (ObjectOptimisticLockingFailureException e) {
             // 동시에 취소 요청이 들어온 경우 하나만 성공하고 나머지는 여기서 걸러짐
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45bd32b and d760fce.

📒 Files selected for processing (1)
  • backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (2 hunks)
🔇 Additional comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (1)

21-21: import 추가가 적절합니다

낙관적 잠금 예외 처리를 위한 ObjectOptimisticLockingFailureException import가 올바르게 추가되었습니다.

@Kosw6 Kosw6 merged commit 68ac69c into main Nov 24, 2025
1 check passed
@Kosw6 Kosw6 deleted the SISC1-BE-베팅게임-수정 branch November 24, 2025 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants