feat: allow guests one-time sleep jetlag trial without login - #18
Merged
Conversation
Closed
📝 WalkthroughWalkthroughThe sleep jetlag API now permits unauthenticated requests. Members retain unlimited persisted calculations, while guests provide ChangesGuest jetlag trial
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SleepJetlagController
participant SleepJetlagService
participant GuestUsageRepository
participant SleepJetlagResultResponse
Client->>SleepJetlagController: POST /api/sleep/jetlag with X-Device-Id
SleepJetlagController->>SleepJetlagService: calculate(memberId, deviceId, request)
SleepJetlagService->>GuestUsageRepository: existsByDeviceId(deviceId)
GuestUsageRepository-->>SleepJetlagService: usage status
SleepJetlagService->>GuestUsageRepository: save GuestUsage
SleepJetlagService->>SleepJetlagResultResponse: guest calculation data
SleepJetlagResultResponse-->>Client: guest response with null resultId
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/cotato/cokerthon/domain/sleep/repository/GuestUsageRepository.java (1)
1-9: 🧹 Nitpick | 🔵 TrivialLGTM!
One longer-term operational note:
guest_usageswill grow unbounded with no retention/cleanup policy (e.g., a scheduled purge of records older than N days), since device IDs are kept indefinitely just to enforce the one-time trial. Not a blocker for this PR.🤖 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/cotato/cokerthon/domain/sleep/repository/GuestUsageRepository.java` around lines 1 - 9, Plan a retention policy for GuestUsage records by adding scheduled cleanup of entries older than a configured number of days, using GuestUsageRepository with an appropriate date-based delete query and a scheduled service method; keep the retention period configurable and document the policy.
🤖 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/cotato/cokerthon/domain/sleep/service/SleepJetlagService.java`:
- Around line 88-90: Validate the client-provided deviceId in SleepJetlagService
before creating or persisting GuestUsage: retain the null/blank check and reject
values exceeding the GuestUsage.deviceId column limit of 100 characters with an
appropriate controlled BusinessException/ErrorCode, preventing database
constraint failures.
- Around line 91-102: Prevent concurrent guest requests from surfacing a
database constraint error: in the service method containing existsByDeviceId and
GuestUsage.create, flush the repository immediately after save so the
unique-constraint violation occurs during request handling, then catch and
translate that DataIntegrityViolationException to
BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED), preserving the intended
response for the losing request.
---
Nitpick comments:
In
`@src/main/java/com/cotato/cokerthon/domain/sleep/repository/GuestUsageRepository.java`:
- Around line 1-9: Plan a retention policy for GuestUsage records by adding
scheduled cleanup of entries older than a configured number of days, using
GuestUsageRepository with an appropriate date-based delete query and a scheduled
service method; keep the retention period configurable and document the policy.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df40b6ee-903f-4085-9536-eb259ff81308
📒 Files selected for processing (8)
src/main/java/com/cotato/cokerthon/domain/sleep/controller/SleepJetlagController.javasrc/main/java/com/cotato/cokerthon/domain/sleep/dto/response/SleepJetlagResultResponse.javasrc/main/java/com/cotato/cokerthon/domain/sleep/entity/GuestUsage.javasrc/main/java/com/cotato/cokerthon/domain/sleep/repository/GuestUsageRepository.javasrc/main/java/com/cotato/cokerthon/domain/sleep/service/SleepJetlagService.javasrc/main/java/com/cotato/cokerthon/global/config/SecurityConfig.javasrc/main/java/com/cotato/cokerthon/global/exception/ErrorCode.javasrc/test/java/com/cotato/cokerthon/domain/sleep/SleepJetlagIntegrationTest.java
Comment on lines
+88
to
+90
| if (deviceId == null || deviceId.isBlank()) { | ||
| throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
No length/format validation on deviceId before it hits the DB column constraint.
deviceId comes straight from a client-controlled header (X-Device-Id) with only a null/blank check. GuestUsage.deviceId is length = 100; an oversized value will fail at insert time with an unhandled exception (500) instead of a controlled validation error.
if (deviceId == null || deviceId.isBlank()) {
throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED);
}
+ if (deviceId.length() > 100) {
+ throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED);
+ }📝 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
| if (deviceId == null || deviceId.isBlank()) { | |
| throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED); | |
| } | |
| if (deviceId == null || deviceId.isBlank()) { | |
| throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED); | |
| } | |
| if (deviceId.length() > 100) { | |
| throw new BusinessException(ErrorCode.GUEST_DEVICE_ID_REQUIRED); | |
| } |
🤖 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/cotato/cokerthon/domain/sleep/service/SleepJetlagService.java`
around lines 88 - 90, Validate the client-provided deviceId in
SleepJetlagService before creating or persisting GuestUsage: retain the
null/blank check and reject values exceeding the GuestUsage.deviceId column
limit of 100 characters with an appropriate controlled
BusinessException/ErrorCode, preventing database constraint failures.
Comment on lines
+91
to
+102
| if (guestUsageRepository.existsByDeviceId(deviceId)) { | ||
| throw new BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED); | ||
| } | ||
|
|
||
| MidSleep currentSleep = midSleepOf(request.currentBedtime(), request.currentWaketime()); | ||
| MidSleep targetSleep = midSleepOf(request.targetBedtime(), request.targetWaketime()); | ||
|
|
||
| Jetlag jetlag = calculateJetlag(currentSleep.midTime(), targetSleep.midTime()); | ||
| City matchedCity = findMatchedCity(jetlag.direction(), jetlag.minutes()); | ||
| City seoul = getSeoul(); | ||
|
|
||
| guestUsageRepository.save(GuestUsage.create(deviceId)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Race condition: concurrent requests with the same deviceId can both pass the exhaustion check.
existsByDeviceId (line 91) and save() (line 102) run in the same transaction, and save() only queues the insert — it doesn't flush/hit the unique constraint until commit. Two concurrent guest calls for the same device can both pass the exists check and both proceed to compute a result; at commit, the second insert throws DataIntegrityViolationException instead of the intended 403 GUEST_TRIAL_EXHAUSTED, surfacing as an unhandled 500.
🔒 Proposed fix: flush immediately and translate the constraint violation
- guestUsageRepository.save(GuestUsage.create(deviceId));
+ try {
+ guestUsageRepository.saveAndFlush(GuestUsage.create(deviceId));
+ } catch (DataIntegrityViolationException e) {
+ throw new BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED);
+ }📝 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
| if (guestUsageRepository.existsByDeviceId(deviceId)) { | |
| throw new BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED); | |
| } | |
| MidSleep currentSleep = midSleepOf(request.currentBedtime(), request.currentWaketime()); | |
| MidSleep targetSleep = midSleepOf(request.targetBedtime(), request.targetWaketime()); | |
| Jetlag jetlag = calculateJetlag(currentSleep.midTime(), targetSleep.midTime()); | |
| City matchedCity = findMatchedCity(jetlag.direction(), jetlag.minutes()); | |
| City seoul = getSeoul(); | |
| guestUsageRepository.save(GuestUsage.create(deviceId)); | |
| if (guestUsageRepository.existsByDeviceId(deviceId)) { | |
| throw new BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED); | |
| } | |
| MidSleep currentSleep = midSleepOf(request.currentBedtime(), request.currentWaketime()); | |
| MidSleep targetSleep = midSleepOf(request.targetBedtime(), request.targetWaketime()); | |
| Jetlag jetlag = calculateJetlag(currentSleep.midTime(), targetSleep.midTime()); | |
| City matchedCity = findMatchedCity(jetlag.direction(), jetlag.minutes()); | |
| City seoul = getSeoul(); | |
| try { | |
| guestUsageRepository.saveAndFlush(GuestUsage.create(deviceId)); | |
| } catch (DataIntegrityViolationException e) { | |
| throw new BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED); | |
| } |
🤖 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/cotato/cokerthon/domain/sleep/service/SleepJetlagService.java`
around lines 91 - 102, Prevent concurrent guest requests from surfacing a
database constraint error: in the service method containing existsByDeviceId and
GuestUsage.create, flush the repository immediately after save so the
unique-constraint violation occurs during request handling, then catch and
translate that DataIntegrityViolationException to
BusinessException(ErrorCode.GUEST_TRIAL_EXHAUSTED), preserving the intended
response for the losing request.
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
1 participant
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.
Summary
X-Device-Id헤더로 수면시차 계산을 1회 체험할 수 있도록POST /api/sleep/jetlag인증을 선택적으로 변경resultId=null), 사용 이력만guest_usages에 기록Test plan
./gradlew build전체 통과Closes #17
Summary by CodeRabbit
New Features
Bug Fixes
Tests