feat: 훈련 시작/종료 API 구현 - #40 - #41
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough훈련 세션의 시작·정상 종료·강제 종료 API를 추가했습니다. 세션 상태 이벤트를 발행하고, 10분 초과 세션을 스케줄러로 실패 처리합니다. 낙관적 잠금 오류와 관련 API 코드 및 테스트를 추가했습니다. Changes훈련 세션 생명주기
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/test/java/com/saferoute/domain/training/controller/TrainingSessionControllerTest.java (1)
69-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win실패 응답 계약을 검증하십시오.
현재 실패 테스트는 HTTP 상태만 검사합니다. 실패 플래그 또는 API 오류 코드가 잘못되어도 테스트가 통과합니다. 각 실패 경로에서 전역 예외 응답의 실패 플래그와
TrainingErrorCode에 대응하는 오류 코드를 검증하십시오.end와force-end의TRAINING_SESSION_NOT_FOUND경로도 추가하십시오.🤖 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/test/java/com/saferoute/domain/training/controller/TrainingSessionControllerTest.java` around lines 69 - 131, Update the failure tests around startTrainingSession_invalidTransition_returnsConflict, startTrainingSession_notFound_returns404, endTrainingSession_invalidTransition_returnsConflict, and forceEndTrainingSession_invalidTransition_returnsConflict to assert the global error response’s failure flag and the error code corresponding to each TrainingErrorCode, not only the HTTP status. Add TRAINING_SESSION_NOT_FOUND failure tests for both endTrainingSession and forceEndTrainingSession with matching status and error-body assertions.
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@src/main/java/com/saferoute/domain/training/service/TrainingSessionService.java`:
- Around line 85-125: Make the status validation and update atomic across start,
end, forceEnd, and timeout handling by applying locking to the TrainingSession
entity or its lookup method, using either `@Version` with conflict handling or a
pessimistic row lock. Ensure concurrent transitions cannot both succeed or
overwrite the final state, and apply the same protection to timeout-related
session reads.
- Around line 43-45: Update create in TrainingSessionService to validate that
the user loaded from request.getAdminId() has UserRole.MANAGER before passing it
to TrainingSession.create. Throw the appropriate existing API exception with its
error code when the role is not MANAGER, while preserving the current
ADMIN_NOT_FOUND behavior for missing users.
In `@src/main/resources/static/test.html`:
- Around line 39-40: Update the WebSocket URL default in the test page’s wsUrl
input to use the secure wss:// scheme instead of ws://, while retaining any
non-secure ws:// usage only through explicit approval or separate management.
In
`@src/test/java/com/saferoute/domain/training/service/TrainingSessionServiceTest.java`:
- Around line 116-127: Add missing-session tests for both
TrainingSessionService.end and forceEnd by stubbing findById(sessionId) with
Optional.empty(). Assert each call throws ApiException with
TRAINING_SESSION_NOT_FOUND and verify
trainingEventPublisher.publishTrainingStatusUpdatedAfterCommit is never invoked.
---
Nitpick comments:
In
`@src/test/java/com/saferoute/domain/training/controller/TrainingSessionControllerTest.java`:
- Around line 69-131: Update the failure tests around
startTrainingSession_invalidTransition_returnsConflict,
startTrainingSession_notFound_returns404,
endTrainingSession_invalidTransition_returnsConflict, and
forceEndTrainingSession_invalidTransition_returnsConflict to assert the global
error response’s failure flag and the error code corresponding to each
TrainingErrorCode, not only the HTTP status. Add TRAINING_SESSION_NOT_FOUND
failure tests for both endTrainingSession and forceEndTrainingSession with
matching status and error-body assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ef655d4-e9a1-44ea-8334-a0cc4b54b1ed
📒 Files selected for processing (13)
src/main/java/com/saferoute/SafeRouteApplication.javasrc/main/java/com/saferoute/domain/training/controller/TrainingSessionController.javasrc/main/java/com/saferoute/domain/training/entity/TrainingSession.javasrc/main/java/com/saferoute/domain/training/repository/TrainingSessionRepository.javasrc/main/java/com/saferoute/domain/training/scheduler/TrainingTimeoutScheduler.javasrc/main/java/com/saferoute/domain/training/service/TrainingSessionService.javasrc/main/java/com/saferoute/global/api/error/TrainingErrorCode.javasrc/main/java/com/saferoute/global/api/response/TrainingSuccessCode.javasrc/main/resources/static/test.htmlsrc/test/java/com/saferoute/domain/training/controller/TrainingSessionControllerTest.javasrc/test/java/com/saferoute/domain/training/scheduler/TrainingTimeoutSchedulerTest.javasrc/test/java/com/saferoute/domain/training/service/TrainingSessionServiceTest.javasrc/test/java/com/saferoute/infrastructure/websocket/service/WebSocketIntegrationTest.java
Comment on lines
+85
to
+125
| @Transactional | ||
| public TrainingSessionResponse start(UUID sessionId) { | ||
| TrainingSession session = findSession(sessionId); | ||
|
|
||
| if (session.getStatus() != TrainingStatus.SCHEDULED) { | ||
| throw new ApiException(TrainingErrorCode.INVALID_STATUS_TRANSITION); | ||
| } | ||
|
|
||
| session.start(Instant.now()); | ||
| trainingEventPublisher.publishTrainingStatusUpdatedAfterCommit(session); | ||
|
|
||
| return TrainingSessionResponse.from(session); | ||
| } | ||
|
|
||
| @Transactional | ||
| public TrainingSessionResponse end(UUID sessionId) { | ||
| TrainingSession session = findSession(sessionId); | ||
|
|
||
| if (session.getStatus() != TrainingStatus.RUNNING) { | ||
| throw new ApiException(TrainingErrorCode.INVALID_STATUS_TRANSITION); | ||
| } | ||
|
|
||
| session.complete(Instant.now()); | ||
| trainingEventPublisher.publishTrainingStatusUpdatedAfterCommit(session); | ||
|
|
||
| return TrainingSessionResponse.from(session); | ||
| } | ||
|
|
||
| @Transactional | ||
| public TrainingSessionResponse forceEnd(UUID sessionId) { | ||
| TrainingSession session = findSession(sessionId); | ||
|
|
||
| if (session.getStatus() != TrainingStatus.RUNNING) { | ||
| throw new ApiException(TrainingErrorCode.INVALID_STATUS_TRANSITION); | ||
| } | ||
|
|
||
| session.stop(Instant.now()); | ||
| trainingEventPublisher.publishTrainingStatusUpdatedAfterCommit(session); | ||
|
|
||
| return TrainingSessionResponse.from(session); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
상태 확인과 상태 갱신을 원자적으로 처리하십시오.
start, end, forceEnd, 타임아웃 처리는 상태를 읽고 검증한 뒤 갱신합니다. 행 잠금과 @Version이 없으므로 두 트랜잭션이 같은 RUNNING 상태를 동시에 읽을 수 있습니다. 예를 들어 정상 종료와 강제 종료가 모두 성공한 뒤 마지막 커밋이 최종 상태를 덮어쓸 수 있습니다. 모든 상태 전이와 타임아웃 조회에 낙관적 잠금과 충돌 처리 또는 비관적 행 잠금을 적용하십시오.
Also applies to: 127-138
🤖 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/saferoute/domain/training/service/TrainingSessionService.java`
around lines 85 - 125, Make the status validation and update atomic across
start, end, forceEnd, and timeout handling by applying locking to the
TrainingSession entity or its lookup method, using either `@Version` with conflict
handling or a pessimistic row lock. Ensure concurrent transitions cannot both
succeed or overwrite the final state, and apply the same protection to
timeout-related session reads.
Comment on lines
+39
to
+40
| <label>WebSocket URL</label> | ||
| <input id="wsUrl" value="ws://localhost:8080/ws" /> |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate test.html =="
fd -a 'test\.html$' . || true
echo "== file outline/size =="
if [ -f src/main/resources/static/test.html ]; then
wc -l src/main/resources/static/test.html
ast-grep outline src/main/resources/static/test.html || true
echo "== relevant slices =="
sed -n '1,120p' src/main/resources/static/test.html | cat -n
fi
echo "== search wsUrl/STOMP/websocket usages in repository =="
rg -n "wsUrl|STOMP|new SockJS|addEventListener\\(|ws://|wss://" . --glob '!build/**' --glob '!target/**' --glob '!node_modules/**' || true
echo "== inspect java websocket/stomp related files =="
fd -i 'websocket|stomp|config|security' . || trueRepository: DS-SafeRoute/SafeRoute-BE
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== static path/config/security files =="
fd -i 'SecurityConfig|WebSecurity|Security|WebSocketConfig|WebSocketMessage|WebSocket|test.html|test.*html' src/main src/test | sort
echo "== SecurityConfig =="
fd -i 'SecurityConfig' src/main/java | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
done
echo "== Static resource handling =="
rg -n "addResourceHandlers|WebMvcConfigurer|ResourceHandlerRegistry|static|classPathResource|spring.web.resources|AuthorizationRequestMapper|authorizeHttpRequests|HttpSecurity" src/main/java --glob '*.java' || true
echo "== WebSocket authorization/acceptance =="
sed -n '1,180p' src/main/java/com/saferoute/infrastructure/websocket/config/WebSocketConfig.java | cat -n
sed -n '1,120p' src/main/java/com/saferoute/infrastructure/websocket/security/StompAuthChannelInterceptor.java | cat -n
sed -n '1,340p' src/test/java/com/saferoute/infrastructure/websocket/service/WebSocketIntegrationTest.java | cat -n
echo "== test.html static route access hints =="
rg -n "test.html|static/test|/test|/session/\\{/webjars/\" " src/main/resources src/main/java --glob '*.java' --glob '*.yml' --glob '*.yaml' --glob '*.properties' || true
echo "== small AST parse of connect function and brokerURL handling =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/main/resources/static/test.html')
s = p.read_text(errors='ignore')
start = s.index('function connect()')
end = s.index('function disconnect()')
print(s[start:end])
print("has ws prefix validation:", 'wsUrl.startsWith("wss://")' in s or "wsUrl.startsWith('wss://')" in s)
print("has ws prefix validation regex:", 'wsUrl.match' in s)
PYRepository: DS-SafeRoute/SafeRoute-BE
Length of output: 41511
Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External
이 테스트 페이지의 ws:// 연결을 운영 환경에 내놓지 마세요.
ws://에서는 Authorization: Bearer ... 헤더가 평신호 전송 구간으로 내려갑니다. 테스트용 페이지라 해도 wss://를 기본으로 설정하고, ws://는 명시적으로 승인하는 등 별도 관리가 필요합니다.
🤖 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/resources/static/test.html` around lines 39 - 40, Update the
WebSocket URL default in the test page’s wsUrl input to use the secure wss://
scheme instead of ws://, while retaining any non-secure ws:// usage only through
explicit approval or separate management.
TrainingSessionResponse에 세션 UUID(id)가 빠져 있어, 세션 생성 후 start/end/force-end 호출에 필요한 sessionId를 응답에서 알 수 없던 문제 수정. 웹소켓 수동 테스트용 test.html은 더 이상 필요 없어 삭제.
- create()에서 관리자 역할이 MANAGER가 아니면 FORBIDDEN 예외를 던지도록 검증 추가 - TrainingSession에 @Version을 추가해 start/end/force-end/timeout 간 동시 상태 전이 시 하나만 성공하고 나머지는 낙관적 락 충돌로 실패하도록 변경 - ObjectOptimisticLockingFailureException을 409 CONFLICT로 매핑하는 전역 예외 핸들러 추가 - end/forceEnd의 세션 없음 케이스 테스트 추가 - WebSocketIntegrationTest에서 서비스가 갱신한 stale 엔티티를 delete()하며 발생하던 낙관적 락 충돌을 deleteById()로 수정
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/saferoute/domain/training/service/TrainingSessionService.java (1)
132-144: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift타임아웃 처리를 제한된 배치로 실행하십시오.
Line 136-137은 모든 타임아웃 세션을
List로 읽습니다. Line 139-143은 모든 세션을 하나의 트랜잭션에서 갱신하고 세션마다 커밋 후 이벤트를 등록합니다.스케줄러 중단 후 backlog가 증가하면 메모리 사용량, flush 시간, 잠금 유지 시간, 이벤트 콜백 수가 함께 증가할 수 있습니다. repository 조회에
Pageable또는 limit을 적용하십시오. 제한된 크기의 chunk를 별도 트랜잭션으로 처리하십시오.As per path instructions, 트랜잭션 경계와 데이터 접근 부하를 중점적으로 확인해야 합니다.
🤖 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/saferoute/domain/training/service/TrainingSessionService.java` around lines 132 - 144, failTimedOutSessions가 모든 타임아웃 세션을 한 번에 조회·처리하지 않도록 trainingSessionRepository 조회에 Pageable 또는 limit을 적용하십시오. 제한된 chunk 단위로 별도 트랜잭션에서 세션을 갱신하고, 각 세션의 상태 변경·publishTrainingStatusUpdatedAfterCommit·로그 처리는 유지하십시오. 스케줄러가 계속 backlog를 처리할 수 있도록 chunk별 트랜잭션 경계가 실제로 분리되게 구성하십시오.Source: Path instructions
🤖 Prompt for all review comments with 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.
Inline comments:
In
`@src/main/java/com/saferoute/domain/training/service/TrainingSessionService.java`:
- Around line 45-52: Ensure the RUNNING status always has a non-null startedAt
when creating a session. Add validation in TrainingSessionService.create for
CreateSessionRequest, or enforce the invariant in TrainingSession.create by
rejecting RUNNING with a null startedAt, while preserving start()’s existing
transition behavior.
---
Nitpick comments:
In
`@src/main/java/com/saferoute/domain/training/service/TrainingSessionService.java`:
- Around line 132-144: failTimedOutSessions가 모든 타임아웃 세션을 한 번에 조회·처리하지 않도록
trainingSessionRepository 조회에 Pageable 또는 limit을 적용하십시오. 제한된 chunk 단위로 별도 트랜잭션에서
세션을 갱신하고, 각 세션의 상태 변경·publishTrainingStatusUpdatedAfterCommit·로그 처리는 유지하십시오.
스케줄러가 계속 backlog를 처리할 수 있도록 chunk별 트랜잭션 경계가 실제로 분리되게 구성하십시오.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ca529d67-85df-4a58-8e5e-02a93fac4ed9
📒 Files selected for processing (6)
src/main/java/com/saferoute/domain/training/dto/TrainingSessionResponse.javasrc/main/java/com/saferoute/domain/training/entity/TrainingSession.javasrc/main/java/com/saferoute/domain/training/service/TrainingSessionService.javasrc/main/java/com/saferoute/global/api/exception/GlobalExceptionHandler.javasrc/test/java/com/saferoute/domain/training/service/TrainingSessionServiceTest.javasrc/test/java/com/saferoute/infrastructure/websocket/service/WebSocketIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/saferoute/domain/training/entity/TrainingSession.java
- src/test/java/com/saferoute/infrastructure/websocket/service/WebSocketIntegrationTest.java
- src/test/java/com/saferoute/domain/training/service/TrainingSessionServiceTest.java
hakSick
approved these changes
Aug 5, 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
2 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.
##📌 관련 이슈
Closes #40
Summary
POST /sessions/{sessionId}/start), 정상 종료(POST /sessions/{sessionId}/end), 강제 종료(POST /sessions/{sessionId}/force-end) API 구현Test plan
TrainingSessionServiceTest— 상태별 전이 성공/실패, 세션 없음, 권한 검증, RUNNING 생성 시 startedAt 검증TrainingSessionControllerTest— start/end/force-end MockMvc 테스트TrainingTimeoutSchedulerTest— 스케줄러가 서비스에 위임하는지 확인WebSocketIntegrationTest— 시작/종료/강제종료 각각 TRAINING_STATUS_UPDATED 이벤트 수신 확인./gradlew test전체 통과closes #40
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
테스트