-
Notifications
You must be signed in to change notification settings - Fork 2
[BE] SISC1-89 게임 종료 및 결과 정산 #65
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.
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
The head ref may contain hidden characters: "SISC1-89-BE-\uAC8C\uC784-\uC885\uB8CC-\uBC0F-\uACB0\uACFC-\uC815\uC0B0"
Conversation
WalkthroughBetRound 상태·결산 로직과 UserBet 결과 처리 API가 추가되고, Stock에 settleClosePrice와 symbol 유니크 제약이 추가되었습니다. 저장소에 신규 조회 메서드가 생기고 스케줄러에 정산 작업이 추가되며 ErrorCode와 테스트·서비스 생성자 시그니처가 갱신되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as BettingScheduler
participant Service as BettingService
participant BRRepo as BetRoundRepository
participant StockRepo as StockRepository
participant UBRepo as UserBetRepository
participant PHSvc as PointHistoryService
participant DB as Database
Note right of Scheduler `#ddeeff`: scheduled (Asia/Seoul 22:05)
Scheduler->>Service: settleUserBets()
Service->>BRRepo: findByStatusFalseAndSettleAtIsNullAndLockAtLessThanEqual(now)
BRRepo->>DB: SELECT active rounds
DB-->>BRRepo: List<BetRound>
BRRepo-->>Service: List<BetRound>
loop 각 라운드
Service->>StockRepo: findBySymbol(round.stock.symbol)
StockRepo->>DB: SELECT stock
DB-->>StockRepo: Stock(with settleClosePrice)
StockRepo-->>Service: Optional<Stock>
Service->>Service: round.validate()
Service->>Service: round.settle(finalPrice) -- determineResult -> RISE/FALL
Service->>UBRepo: findAllByRound(round)
UBRepo->>DB: SELECT user_bets
DB-->>UBRepo: List<UserBet]
UBRepo-->>Service: List<UserBet]
loop 각 베팅
alt 승리
Service->>UBRepo: bet.win(reward)
Service->>PHSvc: pointHistoryService.create(...)
else 패배
Service->>UBRepo: bet.lose()
end
end
end
Service->>DB: save(rounds, bets, point histories)
DB-->>Service: OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
Suggested reviewers
시 (코드래빗의 노래)
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (9)
backend/src/main/java/org/sejongisc/backend/betting/entity/Stock.java (1)
34-35: 정산 전 null 허용 의도 명시정산 종가는 초기엔 비어 있을 수 있으므로 null 허용을 명시하면 의도가 더 분명합니다. 기능 영향은 없습니다.
- @Column(precision = 15, scale = 2) + @Column(precision = 15, scale = 2, nullable = true) private BigDecimal settleClosePrice;추가 제안: 종목 식별을 symbol로 일관 사용한다면 유니크 제약을 고려해 주세요.
backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java (1)
66-69: BET_ROUND_CLOSED/BET_ROUND_NOT_CLOSED 메시지 구분 필요검증 결과, 두 에러 코드 모두 동일한 메시지 "베팅 가능 시간이 아닙니다."를 가지고 있습니다. 반대 상황을 나타내므로 메시지를 명확히 구분하는 것이 좋습니다. 현재
BET_ROUND_NOT_CLOSED는 정의만 되어 있고 실제 사용되지 않지만, 향후 유지보수성을 위해 구분되는 메시지를 사용하세요.제안:
- BET_ROUND_CLOSED(HttpStatus.CONFLICT, "베팅 가능 시간이 아닙니다."), + BET_ROUND_CLOSED(HttpStatus.CONFLICT, "이미 마감된 라운드입니다."), ... - BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "베팅 가능 시간이 아닙니다."); + BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "아직 마감되지 않은 라운드입니다.");backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java (2)
66-83: 생성 로직 OK. 테스트 용이성 개선 제안(선택).로직은 문제없습니다. 테스트/일관성 향상을 위해 다음을 고려해 주세요:
- 시간 계산용 Clock 주입(LocalDateTime.now() 대체).
- Random 주입 또는 ThreadLocalRandom 사용(테스트 재현성/성능).
191-194: 보상 계산 TODO 마무리 필요.현재 상수 2 반환은 실제 베팅 금액/배율과 무관합니다. 최소한 stake·배율 기반 산식으로 대체해 주세요(예: 승리 시 stake × multiplier).
원하시면 간단한 산식 구현안을 드리겠습니다.
backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java (1)
41-43: boolean status 대신 Enum(OPEN/CLOSED/SETTLED) 고려.정산 여부(settleAt)와 오픈/클로즈 상태가 섞여 혼동될 수 있습니다. Enum 상태를 도입하면 “정산 완료” 여부를 명시적으로 관리할 수 있어 버그를 줄입니다.
backend/src/test/java/org/sejongisc/backend/betting/service/BettingServiceTest.java (4)
20-20: 명시적 import 사용을 권장합니다.와일드카드 import (
import java.util.*;)는 어떤 클래스가 실제로 사용되는지 불명확하게 만들고, 네이밍 충돌 가능성을 높입니다. 대신 사용하는 클래스들을 명시적으로 import하는 것이 좋습니다.-import java.util.*; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.UUID;
90-90: 더 간결한 AssertJ 메서드를 사용하세요.
isIn()메서드가 더 간결하고 AssertJ 관용구에 부합합니다.- assertThat(List.of(s1, s2)).contains(chosen); + assertThat(chosen).isIn(s1, s2);
311-311: 무료 베팅의 stakePoints 값이 혼란스럽습니다.무료 베팅 요청에서
stakePoints(999)로 설정되어 있지만, 354번 라인에서는 결과가 0이어야 한다고 검증합니다. 서비스가 무료 베팅의 stakePoints를 무시하는 것을 테스트하려는 의도라면, 명확성을 위해 0을 사용하거나 주석으로 의도를 설명하는 것이 좋습니다..free(true) - .stakePoints(999) + .stakePoints(0) // 무료 베팅은 stakePoints가 무시됨 .build();
520-520: TODO 주석을 해결하세요.하드코딩된 보상 계산 로직(2배)에 대한 TODO가 있습니다. 이를 추적하거나 해결해야 합니다.
이 TODO를 추적하기 위한 이슈를 생성하시겠습니까?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java(4 hunks)backend/src/main/java/org/sejongisc/backend/betting/entity/BetStatus.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/entity/Stock.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/repository/StockRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/service/BettingScheduler.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java(5 hunks)backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java(1 hunks)backend/src/test/java/org/sejongisc/backend/betting/service/BettingServiceTest.java(19 hunks)
🔇 Additional comments (7)
backend/src/main/java/org/sejongisc/backend/betting/entity/BetStatus.java (1)
5-6: 상태 전이/정책 명시 확인 요청ACTIVE → CLOSED/DELETED 전이가 도입된 점은 적절합니다. 다만 UserBet의 라운드+유저 UNIQUE 제약과 DELETED의 관계(삭제 후 재베팅 허용 여부)를 명시해 주세요. 재베팅 허용 시 현재 제약으론 삽입이 막힐 수 있습니다.
backend/src/main/java/org/sejongisc/backend/betting/repository/UserBetRepository.java (1)
18-18: 라운드별 조회 메서드 추가 승인 — 현재 상태 필터링은 불필요정산 플로우에 필요한 메서드이며 구현이 정상입니다. 현재 정산 로직(169~187줄)은 라운드의 모든 베팅을 조회한 후 일괄 처리하므로 상태 필터링이 필요하지 않습니다.
BetRound매개변수가 이미 전달되고 루프 내에서 재접근하지 않아 N+1 문제도 없습니다.향후 특정 상태의 베팅만 조회하는 요구사항이 생기면
findAllByRoundAndBetStatus같은 파생 메서드를 추가하고, 대량 조회 시EntityGraph로 최적화하면 됩니다. 현재로서는 불필요합니다.backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java (1)
92-97: 동일가(equal) 처리 정책 확인.finalPrice == previousClosePrice 를 RISE로 분류하고 있습니다. 요구사항에 맞는지 확인해 주세요(동률 시 무승부/환급 등).
backend/src/test/java/org/sejongisc/backend/betting/service/BettingServiceTest.java (4)
56-58: 테스트 구조화가 잘 되어 있습니다.주석을 통한 테스트 그룹화로 가독성이 향상되었습니다.
Also applies to: 100-102, 270-272, 402-404, 475-477
388-400: 적절한 에러 코드 사용입니다.라운드가 종료된 경우
BET_ROUND_CLOSED에러 코드를 사용하는 것이 적절합니다.
528-544: 엣지 케이스 테스트가 잘 작성되었습니다.정산 중 주식을 찾을 수 없는 경우를 적절히 검증하고 있습니다.
104-268: 테스트 리팩토링이 잘 되어 있습니다.
isOpen()기반 상태 관리로의 전환이 테스트에 적절히 반영되었으며, DisplayName도 명확하게 작성되었습니다.
backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/service/BettingScheduler.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
Show resolved
Hide resolved
backend/src/test/java/org/sejongisc/backend/betting/service/BettingServiceTest.java
Outdated
Show resolved
Hide resolved
…T/sisc-web into SISC1-89-BE-게임-종료-및-결과-정산
There was a problem hiding this 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
♻️ Duplicate comments (1)
backend/src/main/java/org/sejongisc/backend/betting/service/BettingScheduler.java (1)
39-47: 금요일 정산 스케줄 중복 실행 여전히 발생금요일 22:05에는
dailySettleScheduler와weeklySettleScheduler가 동일한 cron(0 5 22)으로 연달아 실행되어settleUserBets()가 두 번 호출됩니다. 지난 코멘트에서 지적되었던 부분으로, 타임존은 보완되었지만 중복 트리거 위험은 그대로입니다. 정산 로직이 완전한 멱등성을 보장하지 않으면 이용자 포인트가 이중 반영될 수 있고, 멱등하더라도 동일 작업을 두 번 수행하는 것은 불필요한 부하입니다. 금요일 일정을 분리하거나 요일 범위를 조정해 한 번만 실행되도록 조치해 주세요.- @Scheduled(cron = "0 5 22 * * MON-FRI", zone = "Asia/Seoul") + @Scheduled(cron = "0 5 22 * * MON-THU", zone = "Asia/Seoul") public void dailySettleScheduler() { bettingService.settleUserBets(); } - @Scheduled(cron = "0 5 22 * * FRI", zone = "Asia/Seoul") + @Scheduled(cron = "0 6 22 * * FRI", zone = "Asia/Seoul") public void weeklySettleScheduler() { bettingService.settleUserBets(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java(4 hunks)backend/src/main/java/org/sejongisc/backend/betting/entity/Stock.java(2 hunks)backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/service/BettingScheduler.java(1 hunks)backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java(8 hunks)backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java(1 hunks)backend/src/test/java/org/sejongisc/backend/betting/service/BettingServiceTest.java(19 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/src/main/java/org/sejongisc/backend/betting/repository/BetRoundRepository.java
- backend/src/main/java/org/sejongisc/backend/betting/entity/UserBet.java
- backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java
- backend/src/main/java/org/sejongisc/backend/betting/service/BettingService.java
| public void validate() { | ||
| if (isClosed() || (lockAt != null && LocalDateTime.now().isAfter(lockAt))) { | ||
| throw new CustomException(ErrorCode.BET_ROUND_CLOSED); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lockAt와 동일 시각 베팅 허용되는 경계 오류
LocalDateTime.now().isAfter(lockAt)만 검사하면 현재 시각이 lockAt과 정확히 같을 때는 베팅이 허용됩니다. 잠금 시간을 초 단위로 맞춰 두었다면 동일 시각에도 베팅이 들어올 수 있어 의도와 달리 마감 후 베팅이 수락되는 문제가 발생합니다. isEqual까지 포함하거나 !now.isBefore(lockAt)로 비교하여 lockAt에 도달한 순간부터는 차단되도록 보완해 주세요.
- if (isClosed() || (lockAt != null && LocalDateTime.now().isAfter(lockAt))) {
+ if (isClosed() || (lockAt != null && !LocalDateTime.now().isBefore(lockAt))) {
throw new CustomException(ErrorCode.BET_ROUND_CLOSED);
}🤖 Prompt for AI Agents
In backend/src/main/java/org/sejongisc/backend/betting/entity/BetRound.java
around lines 80 to 84, the current validation uses
LocalDateTime.now().isAfter(lockAt) which still allows bets when now equals
lockAt; change the condition to block betting at lockAt as well by comparing
using isEqual or !now.isBefore(lockAt). Update the if to compute LocalDateTime
now = LocalDateTime.now() and then check if (isClosed() || (lockAt != null &&
!now.isBefore(lockAt))) and throw the same CustomException to ensure bets are
rejected at and after lockAt.
There was a problem hiding this 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/common/exception/ErrorCode.java (1)
69-69: 에러 메시지의 구체성이 감소했습니다.이전 메시지 "이미 마감된 라운드입니다."가 "베팅 가능 시간이 아닙니다."로 변경되면서 에러 상황이 덜 명확해졌습니다. 새 메시지는 라운드가 아직 시작하지 않았는지, 이미 마감되었는지, 또는 다른 이유로 베팅이 불가능한지 구분할 수 없습니다.
디버깅과 사용자 경험 개선을 위해 더 구체적인 메시지를 유지하는 것을 권장합니다. 예: "베팅 라운드가 마감되었습니다." 또는 "베팅 가능한 시간이 아닙니다."처럼 상황에 맞는 구체적인 메시지를 사용하는 것을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java(1 hunks)
🔇 Additional comments (1)
backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java (1)
71-71: LGTM!최소 베팅 포인트 요구사항을 명확하게 전달하는 적절한 에러 메시지입니다.
| BET_ROUND_CLOSED(HttpStatus.CONFLICT, "이미 마감된 라운드입니다."), | ||
| BET_POINT_TOO_LOW(HttpStatus.CONFLICT, "베팅 포인트는 10 이상이어야 합니다."); | ||
| BET_POINT_TOO_LOW(HttpStatus.CONFLICT, "베팅 포인트는 10 이상이어야 합니다."), | ||
| BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "닫히지 않은 배팅입니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
용어 일관성 문제가 있습니다.
에러 코드명은 BET_ROUND_NOT_CLOSED로 "라운드(round)"를 언급하지만, 에러 메시지는 "닫히지 않은 배팅입니다."로 "배팅(bet)"을 언급합니다.
정산 작업은 개별 배팅이 아닌 라운드 단위로 수행되므로, 메시지를 "닫히지 않은 라운드입니다." 또는 "배팅 라운드가 아직 종료되지 않았습니다."로 수정하는 것이 더 명확합니다.
다음 diff를 적용하여 용어를 일관되게 수정하세요:
- BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "닫히지 않은 배팅입니다.");
+ BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "닫히지 않은 라운드입니다.");📝 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.
| BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "닫히지 않은 배팅입니다."); | |
| BET_ROUND_NOT_CLOSED(HttpStatus.CONFLICT, "닫히지 않은 라운드입니다."); |
🤖 Prompt for AI Agents
In backend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.java
around line 72, the enum constant BET_ROUND_NOT_CLOSED uses an inconsistent
message referencing "배팅" while the code name refers to a "라운드"; update the
message to match the enum name and domain language — e.g., change the message to
"닫히지 않은 라운드입니다." (or "배팅 라운드가 아직 종료되지 않았습니다.") so the error text consistently
refers to a round rather than a bet.
Summary by CodeRabbit
새로운 기능
동작 변경
테스트