[Feat]#18 트래킹 세션#52
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/main/java/com/semosan/api/domain/tracking/scheduler/TrackingSessionExpiryScheduler.java (1)
23-23: TODO: Configurable expiry threshold and notification policy.The TODO suggests externalizing the 24-hour threshold to a property and considering user notifications on expiration.
Would you like me to draft a configuration class with
@ConfigurationPropertiesfor the expiry duration and outline a notification hook point in the expiration loop?🤖 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/semosan/api/domain/tracking/scheduler/TrackingSessionExpiryScheduler.java` at line 23, Replace the hardcoded 24h expiry with a configurable property: create an `@ConfigurationProperties` class (e.g., ExpiryProperties) exposing a java.time.Duration field (property key like tracking.session.expiry) and register it as a bean; inject ExpiryProperties into TrackingSessionExpiryScheduler and use its Duration instead of the literal 24 hours in the expiration loop (e.g., in the method that checks session age/expireSessions()). Also add a nullable/optional NotificationService hook (interface name NotificationService) and call it when a session is expired (guarded by a new config flag in ExpiryProperties, e.g., notifyOnExpire) so notification behavior can be toggled without changing code.src/main/java/com/semosan/api/domain/tracking/service/TrackingSessionService.java (2)
78-79: TODO: HikingRecord creation integration.The TODO references issue
#20for integrating Redis Stream GPS points and creating HikingRecord/HikingMember entities.Would you like me to open a tracking issue or provide a skeleton implementation outline for the GPS aggregation and record persistence logic?
🤖 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/semosan/api/domain/tracking/service/TrackingSessionService.java` around lines 78 - 79, The TODO in TrackingSessionService needs implementation to aggregate Redis Stream GPS points and create HikingRecord/HikingMember entities when a session ends: add a method (e.g., createHikingRecordFromSession or finalizeSessionAndCreateHikingRecord) inside TrackingSessionService that (1) reads GPS entries from the Redis Stream for the session (use a Redis stream client/consumer to fetch by session key), (2) aggregates and computes statistics (distance, duration, elevation, bounding box, sample count), (3) constructs HikingRecord and associated HikingMember entities and persists them via the existing repositories within a transactional boundary, and (4) mark the tracking session closed and ensure idempotency and error handling (retries/logging) so repeated calls don't create duplicates. Ensure the new method is invoked at the normal-termination path currently closing only state/time so the Redis aggregation + persistence happens as part of session termination.
29-30: ⚡ Quick winExtract duplicated
ACTIVE_STATESconstant to a shared location.
ACTIVE_STATESis defined identically here and inTrackingSessionExpiryScheduler(line 30-31). If the set of active states changes, both locations must be updated, risking inconsistency.Consider defining this constant once in
TrackingSessionStatusenum or a dedicated constants class, then referencing it from both service and scheduler.♻️ Example: Define in enum
In
TrackingSessionStatus:public enum TrackingSessionStatus { IN_PROGRESS, PAUSED, COMPLETED, ABANDONED; public static final Set<TrackingSessionStatus> ACTIVE_STATES = EnumSet.of(IN_PROGRESS, PAUSED); }Then use
TrackingSessionStatus.ACTIVE_STATESin both service and scheduler.🤖 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/semosan/api/domain/tracking/service/TrackingSessionService.java` around lines 29 - 30, Duplicate ACTIVE_STATES should be centralized: remove the EnumSet constant from TrackingSessionService and TrackingSessionExpiryScheduler and define a single public constant on the TrackingSessionStatus enum (e.g., TrackingSessionStatus.ACTIVE_STATES) or in a dedicated constants class, then update both TrackingSessionService and TrackingSessionExpiryScheduler to reference that single symbol (TrackingSessionStatus.ACTIVE_STATES) so the active-state definition lives in one place.src/main/java/com/semosan/api/domain/tracking/repository/TrackingSessionRepository.java (1)
29-37: Add a composite index for scheduler query path.For hourly scans on active states + cutoff, add an index like
(status, started_at)(or(status, updated_at)if you keep current semantics) to avoid table-scan growth.🤖 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/semosan/api/domain/tracking/repository/TrackingSessionRepository.java` around lines 29 - 37, The scheduler query in TrackingSessionRepository (findStaleActiveSessions) will table-scan as the table grows; add a composite DB index on (status, updated_at) or (status, started_at) depending on your semantics to support the WHERE ts.status IN :statuses AND ts.updatedAt < :cutoff path. Update the TrackingSession entity mapping (or a migration) to create the composite index on the status and timestamp column used by this query so the database can use the index for efficient hourly scans.
🤖 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/semosan/api/domain/tracking/controller/docs/TrackingSessionControllerDocs.java`:
- Around line 69-86: Add consistent `@ApiResponses` annotations to the
TrackingSessionControllerDocs interface for the resumeSession, completeSession,
and abandonSession methods: annotate each method (resumeSession,
completeSession, abandonSession) with `@ApiResponses` listing the success response
(200 with TrackingSessionResponse) and the possible error responses 404 (session
not found), 403 (not the owner), and 409 (invalid state transition), matching
the format used by earlier methods in this interface so API docs show these
error codes.
In `@src/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.java`:
- Around line 71-80: The factory method TrackingSession.create currently permits
inconsistent states (isFreeRecording true with non-null course, or
isFreeRecording false with null course); modify TrackingSession.create to
validate that if isFreeRecording is true then course must be null, and if
isFreeRecording is false then course must be non-null, throwing an
IllegalArgumentException (or similar) with a clear message when the contract is
violated before building the TrackingSession; locate and update the create(...)
method and its use of the builder to perform this precondition check so
impossible states are never constructed.
- Around line 33-35: Add JPA optimistic locking to TrackingSession by declaring
a `@Version` field (e.g., private Long version) in the TrackingSession entity
(place it alongside the existing `@Id/`@GeneratedValue field), importing
javax.persistence.Version, and exposing it via the usual accessors if needed;
this enables optimistic locking for concurrent calls to TrackingSession.pause(),
resume(), complete(), and abandon() (BaseEntity only has audit timestamps so it
must be added here).
In
`@src/main/java/com/semosan/api/domain/tracking/repository/TrackingSessionRepository.java`:
- Around line 29-33: Update the JPQL in the `@Query` on TrackingSessionRepository
to use the immutable session creation timestamp instead of the mutable update
timestamp: change the WHERE clause to compare ts.startedAt < :cutoff (keeping
the existing parameters ts.status IN :statuses and :cutoff) so the 24-hour
expiry is based on session start time rather than ts.updatedAt.
---
Nitpick comments:
In
`@src/main/java/com/semosan/api/domain/tracking/repository/TrackingSessionRepository.java`:
- Around line 29-37: The scheduler query in TrackingSessionRepository
(findStaleActiveSessions) will table-scan as the table grows; add a composite DB
index on (status, updated_at) or (status, started_at) depending on your
semantics to support the WHERE ts.status IN :statuses AND ts.updatedAt < :cutoff
path. Update the TrackingSession entity mapping (or a migration) to create the
composite index on the status and timestamp column used by this query so the
database can use the index for efficient hourly scans.
In
`@src/main/java/com/semosan/api/domain/tracking/scheduler/TrackingSessionExpiryScheduler.java`:
- Line 23: Replace the hardcoded 24h expiry with a configurable property: create
an `@ConfigurationProperties` class (e.g., ExpiryProperties) exposing a
java.time.Duration field (property key like tracking.session.expiry) and
register it as a bean; inject ExpiryProperties into
TrackingSessionExpiryScheduler and use its Duration instead of the literal 24
hours in the expiration loop (e.g., in the method that checks session
age/expireSessions()). Also add a nullable/optional NotificationService hook
(interface name NotificationService) and call it when a session is expired
(guarded by a new config flag in ExpiryProperties, e.g., notifyOnExpire) so
notification behavior can be toggled without changing code.
In
`@src/main/java/com/semosan/api/domain/tracking/service/TrackingSessionService.java`:
- Around line 78-79: The TODO in TrackingSessionService needs implementation to
aggregate Redis Stream GPS points and create HikingRecord/HikingMember entities
when a session ends: add a method (e.g., createHikingRecordFromSession or
finalizeSessionAndCreateHikingRecord) inside TrackingSessionService that (1)
reads GPS entries from the Redis Stream for the session (use a Redis stream
client/consumer to fetch by session key), (2) aggregates and computes statistics
(distance, duration, elevation, bounding box, sample count), (3) constructs
HikingRecord and associated HikingMember entities and persists them via the
existing repositories within a transactional boundary, and (4) mark the tracking
session closed and ensure idempotency and error handling (retries/logging) so
repeated calls don't create duplicates. Ensure the new method is invoked at the
normal-termination path currently closing only state/time so the Redis
aggregation + persistence happens as part of session termination.
- Around line 29-30: Duplicate ACTIVE_STATES should be centralized: remove the
EnumSet constant from TrackingSessionService and TrackingSessionExpiryScheduler
and define a single public constant on the TrackingSessionStatus enum (e.g.,
TrackingSessionStatus.ACTIVE_STATES) or in a dedicated constants class, then
update both TrackingSessionService and TrackingSessionExpiryScheduler to
reference that single symbol (TrackingSessionStatus.ACTIVE_STATES) so the
active-state definition lives in one place.
🪄 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: 82e7cca0-9759-4138-836e-7a2d183e6854
📒 Files selected for processing (13)
src/main/java/com/semosan/api/ApiApplication.javasrc/main/java/com/semosan/api/common/status/ErrorStatus.javasrc/main/java/com/semosan/api/common/status/SuccessStatus.javasrc/main/java/com/semosan/api/domain/tracking/controller/TrackingController.javasrc/main/java/com/semosan/api/domain/tracking/controller/TrackingSessionController.javasrc/main/java/com/semosan/api/domain/tracking/controller/docs/TrackingSessionControllerDocs.javasrc/main/java/com/semosan/api/domain/tracking/dto/request/CreateTrackingSessionRequest.javasrc/main/java/com/semosan/api/domain/tracking/dto/response/TrackingSessionResponse.javasrc/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.javasrc/main/java/com/semosan/api/domain/tracking/enums/TrackingSessionStatus.javasrc/main/java/com/semosan/api/domain/tracking/repository/TrackingSessionRepository.javasrc/main/java/com/semosan/api/domain/tracking/scheduler/TrackingSessionExpiryScheduler.javasrc/main/java/com/semosan/api/domain/tracking/service/TrackingSessionService.java
| @Operation(summary = "트래킹 세션 재개", description = "PAUSED 상태에서만 가능. 일시정지 누적 시간이 합산됩니다.") | ||
| ResponseEntity<ApiResponse<TrackingSessionResponse>> resumeSession( | ||
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | ||
| @PathVariable Long sessionId | ||
| ); | ||
|
|
||
| @Operation(summary = "트래킹 세션 정상 종료", | ||
| description = "세션 상태/시각만 마감합니다. 실제 HikingRecord 변환은 #20 에서 구현됩니다.") | ||
| ResponseEntity<ApiResponse<TrackingSessionResponse>> completeSession( | ||
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | ||
| @PathVariable Long sessionId | ||
| ); | ||
|
|
||
| @Operation(summary = "트래킹 세션 포기", description = "기록 없이 세션을 ABANDONED 처리합니다.") | ||
| ResponseEntity<ApiResponse<TrackingSessionResponse>> abandonSession( | ||
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | ||
| @PathVariable Long sessionId | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Complete API documentation with error response codes.
The resumeSession, completeSession, and abandonSession methods are missing @ApiResponses annotations, while earlier methods in the interface consistently document their response codes. These mutation operations can produce error responses (404 for missing session, 403 for ownership violation, 409 for invalid state transitions), and API consumers need this information to implement proper error handling.
📝 Add `@ApiResponses` annotations for consistency
`@Operation`(summary = "트래킹 세션 재개", description = "PAUSED 상태에서만 가능. 일시정지 누적 시간이 합산됩니다.")
+@ApiResponses({
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "재개 성공"),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "409", description = "현재 상태에서 재개 불가",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class)))
+})
ResponseEntity<ApiResponse<TrackingSessionResponse>> resumeSession(
`@Parameter`(hidden = true) `@AuthenticationPrincipal` Long userId,
`@PathVariable` Long sessionId
);
`@Operation`(summary = "트래킹 세션 정상 종료",
description = "세션 상태/시각만 마감합니다. 실제 HikingRecord 변환은 `#20` 에서 구현됩니다.")
+@ApiResponses({
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "종료 성공"),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "409", description = "현재 상태에서 종료 불가",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class)))
+})
ResponseEntity<ApiResponse<TrackingSessionResponse>> completeSession(
`@Parameter`(hidden = true) `@AuthenticationPrincipal` Long userId,
`@PathVariable` Long sessionId
);
`@Operation`(summary = "트래킹 세션 포기", description = "기록 없이 세션을 ABANDONED 처리합니다.")
+@ApiResponses({
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "포기 성공"),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))),
+ `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음",
+ content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class)))
+})
ResponseEntity<ApiResponse<TrackingSessionResponse>> abandonSession(📝 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.
| @Operation(summary = "트래킹 세션 재개", description = "PAUSED 상태에서만 가능. 일시정지 누적 시간이 합산됩니다.") | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> resumeSession( | |
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | |
| @PathVariable Long sessionId | |
| ); | |
| @Operation(summary = "트래킹 세션 정상 종료", | |
| description = "세션 상태/시각만 마감합니다. 실제 HikingRecord 변환은 #20 에서 구현됩니다.") | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> completeSession( | |
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | |
| @PathVariable Long sessionId | |
| ); | |
| @Operation(summary = "트래킹 세션 포기", description = "기록 없이 세션을 ABANDONED 처리합니다.") | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> abandonSession( | |
| @Parameter(hidden = true) @AuthenticationPrincipal Long userId, | |
| @PathVariable Long sessionId | |
| ); | |
| `@Operation`(summary = "트래킹 세션 재개", description = "PAUSED 상태에서만 가능. 일시정지 누적 시간이 합산됩니다.") | |
| `@ApiResponses`({ | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "재개 성공"), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "409", description = "현재 상태에서 재개 불가", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))) | |
| }) | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> resumeSession( | |
| `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long userId, | |
| `@PathVariable` Long sessionId | |
| ); | |
| `@Operation`(summary = "트래킹 세션 정상 종료", | |
| description = "세션 상태/시각만 마감합니다. 실제 HikingRecord 변환은 `#20` 에서 구현됩니다.") | |
| `@ApiResponses`({ | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "종료 성공"), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "409", description = "현재 상태에서 종료 불가", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))) | |
| }) | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> completeSession( | |
| `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long userId, | |
| `@PathVariable` Long sessionId | |
| ); | |
| `@Operation`(summary = "트래킹 세션 포기", description = "기록 없이 세션을 ABANDONED 처리합니다.") | |
| `@ApiResponses`({ | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "200", description = "포기 성공"), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "403", description = "본인 세션 아님", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))), | |
| `@io.swagger.v3.oas.annotations.responses.ApiResponse`(responseCode = "404", description = "세션 없음", | |
| content = `@Content`(schema = `@Schema`(implementation = ApiResponse.class))) | |
| }) | |
| ResponseEntity<ApiResponse<TrackingSessionResponse>> abandonSession( | |
| `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long userId, | |
| `@PathVariable` Long sessionId | |
| ); |
🤖 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/semosan/api/domain/tracking/controller/docs/TrackingSessionControllerDocs.java`
around lines 69 - 86, Add consistent `@ApiResponses` annotations to the
TrackingSessionControllerDocs interface for the resumeSession, completeSession,
and abandonSession methods: annotate each method (resumeSession,
completeSession, abandonSession) with `@ApiResponses` listing the success response
(200 with TrackingSessionResponse) and the possible error responses 404 (session
not found), 403 (not the owner), and 409 (invalid state transition), matching
the format used by earlier methods in this interface so API docs show these
error codes.
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify concurrent mutation entry points and absence/presence of optimistic locking.
rg -n --type=java -C2 '`@Version`|class TrackingSession|void pause\(|void resume\(|void complete\(|void abandon\(' src/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.java
rg -n --type=java -C2 'pause\(|resume\(|complete\(|abandon\(' src/main/java/com/semosan/api/domain/trackingRepository: SEMOSAN/SEMOSAN_BE
Length of output: 10172
🏁 Script executed:
# Find and examine the BaseEntity class
find . -name "BaseEntity.java" -type fRepository: SEMOSAN/SEMOSAN_BE
Length of output: 122
🏁 Script executed:
cat -n ./src/main/java/com/semosan/api/common/base/BaseEntity.javaRepository: SEMOSAN/SEMOSAN_BE
Length of output: 942
Add optimistic locking to prevent concurrent update conflicts.
TrackingSession methods—pause() (line 83), resume() (line 91), complete() (line 103), and abandon() (line 112)—are called from multiple concurrent flows: user-triggered actions via the service layer and the TrackingSessionExpiryScheduler. Without a @Version field, concurrent updates silently overwrite each other (last-write-wins). BaseEntity provides only audit timestamps, which do not provide optimistic locking.
Add a @Version field to enable JPA optimistic locking:
Proposed fix
`@Id`
`@GeneratedValue`(strategy = GenerationType.IDENTITY)
private Long id;
+
+@Version
+@Column(name = "version", nullable = false)
+private Long version;🤖 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/semosan/api/domain/tracking/entity/TrackingSession.java`
around lines 33 - 35, Add JPA optimistic locking to TrackingSession by declaring
a `@Version` field (e.g., private Long version) in the TrackingSession entity
(place it alongside the existing `@Id/`@GeneratedValue field), importing
javax.persistence.Version, and exposing it via the usual accessors if needed;
this enables optimistic locking for concurrent calls to TrackingSession.pause(),
resume(), complete(), and abandon() (BaseEntity only has audit timestamps so it
must be added here).
| public static TrackingSession create(User user, Mountain mountain, Course course, boolean isFreeRecording) { | ||
| return TrackingSession.builder() | ||
| .user(user) | ||
| .mountain(mountain) | ||
| .course(course) | ||
| .isFreeRecording(isFreeRecording) | ||
| .status(TrackingSessionStatus.IN_PROGRESS) | ||
| .startedAt(LocalDateTime.now()) | ||
| .pausedSecondsTotal(0) | ||
| .build(); |
There was a problem hiding this comment.
Enforce isFreeRecording↔course consistency in factory creation.
Line 71 currently allows invalid combinations (isFreeRecording=true with a non-null course, or non-free with null course) despite the entity contract in Line 45. This can persist impossible state.
💡 Proposed fix
public static TrackingSession create(User user, Mountain mountain, Course course, boolean isFreeRecording) {
+ if (isFreeRecording && course != null) {
+ throw new GeneralException(ErrorStatus.TRACKING_SESSION_COURSE_MISMATCH);
+ }
+ if (!isFreeRecording && course == null) {
+ throw new GeneralException(ErrorStatus.TRACKING_SESSION_COURSE_MISMATCH);
+ }
return TrackingSession.builder()
.user(user)
.mountain(mountain)
.course(course)
.isFreeRecording(isFreeRecording)
.status(TrackingSessionStatus.IN_PROGRESS)
.startedAt(LocalDateTime.now())
.pausedSecondsTotal(0)
.build();
}🤖 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/semosan/api/domain/tracking/entity/TrackingSession.java`
around lines 71 - 80, The factory method TrackingSession.create currently
permits inconsistent states (isFreeRecording true with non-null course, or
isFreeRecording false with null course); modify TrackingSession.create to
validate that if isFreeRecording is true then course must be null, and if
isFreeRecording is false then course must be non-null, throwing an
IllegalArgumentException (or similar) with a clear message when the contract is
violated before building the TrackingSession; locate and update the create(...)
method and its use of the builder to perform this precondition check so
impossible states are never constructed.
| @Query(""" | ||
| SELECT ts FROM TrackingSession ts | ||
| WHERE ts.status IN :statuses | ||
| AND ts.updatedAt < :cutoff | ||
| """) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how cutoff is computed/used and whether expiry is currently tied to updatedAt.
rg -n --type=java -C3 'findStaleActiveSessions|cutoff|minusHours|updatedAt|startedAt' src/main/java/com/semosan/api/domain/tracking src/main/java/com/semosan/api/ApiApplication.javaRepository: SEMOSAN/SEMOSAN_BE
Length of output: 6177
🏁 Script executed:
# Find EXPIRY_THRESHOLD definition
rg -n --type=java 'EXPIRY_THRESHOLD' src/main/java/com/semosan/api/domain/trackingRepository: SEMOSAN/SEMOSAN_BE
Length of output: 403
🏁 Script executed:
# Check if pause/resume methods update updatedAt
rg -n --type=java -B2 -A5 'pause\(|resume\(|abandon\(' src/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.javaRepository: SEMOSAN/SEMOSAN_BE
Length of output: 1000
🏁 Script executed:
# Check if there's `@LastModifiedDate` or similar annotation on updatedAt field
rg -n --type=java -B1 -A1 'updatedAt' src/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.javaRepository: SEMOSAN/SEMOSAN_BE
Length of output: 44
🏁 Script executed:
# Get the complete TrackingSession entity to see updatedAt field and its annotations
cat -n src/main/java/com/semosan/api/domain/tracking/entity/TrackingSession.java | head -80Repository: SEMOSAN/SEMOSAN_BE
Length of output: 3326
🏁 Script executed:
# Find BaseEntity to see how updatedAt is managed
find src -name "BaseEntity.java" -type fRepository: SEMOSAN/SEMOSAN_BE
Length of output: 120
🏁 Script executed:
# Once we have the path, read it
cat -n src/main/java/com/semosan/api/common/base/BaseEntity.javaRepository: SEMOSAN/SEMOSAN_BE
Length of output: 942
Replace updatedAt with startedAt to enforce true 24-hour session lifetime.
The query currently uses updatedAt, which resets automatically whenever the session is modified (pause, resume, etc.). This defeats the intent of 24-hour expiry: sessions remain indefinitely alive if periodically updated. Use startedAt instead, which is set once at creation and ensures consistent 24-hour expiry from session initialization regardless of pause/resume activity.
Proposed fix
`@Query`("""
SELECT ts FROM TrackingSession ts
WHERE ts.status IN :statuses
- AND ts.updatedAt < :cutoff
+ AND ts.startedAt < :cutoff
""")
List<TrackingSession> findStaleActiveSessions(
`@Param`("statuses") Collection<TrackingSessionStatus> statuses,
`@Param`("cutoff") LocalDateTime cutoff
);📝 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.
| @Query(""" | |
| SELECT ts FROM TrackingSession ts | |
| WHERE ts.status IN :statuses | |
| AND ts.updatedAt < :cutoff | |
| """) | |
| `@Query`(""" | |
| SELECT ts FROM TrackingSession ts | |
| WHERE ts.status IN :statuses | |
| AND ts.startedAt < :cutoff | |
| """) |
🤖 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/semosan/api/domain/tracking/repository/TrackingSessionRepository.java`
around lines 29 - 33, Update the JPQL in the `@Query` on TrackingSessionRepository
to use the immutable session creation timestamp instead of the mutable update
timestamp: change the WHERE clause to compare ts.startedAt < :cutoff (keeping
the existing parameters ts.status IN :statuses and :cutoff) so the 24-hour
expiry is based on session start time rather than ts.updatedAt.
| @AuthenticationPrincipal Long userId, | ||
| @RequestParam Double lat, | ||
| @RequestParam Double lng | ||
| @RequestParam @Min(-90) @Max(90) Double lat, |
There was a problem hiding this comment.
GeneralExceptionAdvice에는 MethodArgumentNotValidException 처리는 있지만, @RequestParam @Min/@Max에서 발생할 수 있는 ConstraintViolationException 또는HandlerMethodValidationException 처리가 안 되어 있어서 이 경우 Exception 핸들러로 빠져서 잘못된 좌표 요청이 500으로 응답될 수 있을 듯합니당
한 번 체크해봐야할듯!
| * 활성 상태(IN_PROGRESS/PAUSED) 이면서 마지막 업데이트가 cutoff 이전인 세션을 찾는다. | ||
| */ | ||
| @Query(""" | ||
| SELECT ts FROM TrackingSession ts |
There was a problem hiding this comment.
스케줄러는 updatedAt < now - 24h인 활성 세션을 ABANDONED 처리를 하는 것으로 보입니당. 그런데 현재 PR에서 세션의 updatedAt은 세션 생성/일시정지/재개/종료 같은 DB 엔티티 변경 때만 갱신이 되어서 실제 GPS 포인트 수집이 Redis Stream 쪽으로만 이뤄진다면, 사용자가 정상적으로 트래킹 중이어도 tracking_sessions.updatedAt은 갱신되지 않을 가능성이 있을 듯!
| import java.util.EnumSet; | ||
| import java.util.Set; | ||
|
|
||
| @Table(name = "tracking_sessions") |
There was a problem hiding this comment.
이것도 db/migration 패키지 밑에 SQL파일 추가해야 할 듯!
| @Transactional(readOnly = true) | ||
| public class TrackingSessionService { | ||
|
|
||
| private static final Set<TrackingSessionStatus> ACTIVE_STATES = |
There was a problem hiding this comment.
TrackingSessionExpiryScheduler.java:30-31쪽에서도 동일한 상수가 정의된 듯!
| // 자유 기록은 코스 미선택. courseId 가 와도 무시. | ||
| return null; | ||
| } | ||
| if (request.courseId() == null) { |
There was a problem hiding this comment.
이거 null이면 404가 아니라 400이어야 할 듯?
howooyeon
left a comment
There was a problem hiding this comment.
db migration 파일 넣어주세요
나머지는 이상 없습니다 고생 많으셨어요!
[Feat]#45 트래킹 가까운산 API
- @validated + @RequestParam @Min/@max 검증 실패가 500으로 빠지지 않도록 GeneralExceptionAdvice에 ConstraintViolationException 및 HandlerMethodValidationException 핸들러 추가 - TrackingSessionService/Scheduler에 중복 정의된 ACTIVE_STATES 상수를 TrackingSessionStatus enum으로 이동해 단일화 - 자유 기록이 아닌데 courseId가 누락된 경우 응답을 404에서 400(TRACKING_COURSE_ID_REQUIRED)으로 수정
- 컬럼: user/mountain/course FK + is_free_recording + status + 라이프사이클 시각/일시정지 누적 - status check 제약으로 IN_PROGRESS/PAUSED/COMPLETED/ABANDONED 만 허용 - 인덱스 2개: (user_id, status), (status, updated_at) — 활성 세션 조회/24h 만료 스케줄러용
🧾 요약
🔗 이슈
✨ 변경 내용
TrackingSession, 상태 enum)✅ 확인
Summary by CodeRabbit
Release Notes
New Features
Improvements