[Feat]#19 트래킹 GPS#53
Merged
Merged
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 | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 8
🤖 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/common/config/SecurityConfig.java`:
- Around line 91-99: The WebSocket CORS config is too permissive: update the
CorsConfiguration instance named wsConfig to use an environment-driven explicit
allowlist instead of setAllowedOriginPatterns(List.of("*")), keep
setAllowCredentials(true) only if the allowlist is not wildcard, and ensure
setAllowedOriginPatterns (or setAllowedOrigins) is populated from a configurable
property (e.g., a comma-separated env/property) so production can restrict to
mobile app origins; also mirror the same change in WebSocketConfig (Line 29) so
both places read the same allowlist property.
In
`@src/main/java/com/semosan/api/domain/tracking/config/TrackingStreamListenerConfig.java`:
- Around line 48-52: The code currently uses container.receiveAutoAck(...) which
acknowledges entries immediately; change to container.receive(...)
(non-auto-ack) using the same
Consumer.from(trackingProperties.getConsumerGroup(), buildConsumerName()),
StreamOffset.create(trackingProperties.getStreamKey(),
ReadOffset.lastConsumed()), and trackingStreamConsumer so records are delivered
but not auto-acked. Then update your trackingStreamConsumer processing logic to
call the Redis stream acknowledge API (e.g., StreamOperations.acknowledge(...)
or the container/RedisTemplate opsForStream().acknowledge) after successful
processing using trackingProperties.getConsumerGroup() and the record id; do not
acknowledge on failure so entries remain in the Pending Entries List for
retries.
In
`@src/main/java/com/semosan/api/domain/tracking/controller/TrackingGpsWebSocketController.java`:
- Line 41: Replace the generic IllegalStateException thrown in
TrackingGpsWebSocketController (the "throw new
IllegalStateException(\"Unauthenticated WebSocket message\");" statement) with
your domain authentication/business exception type so STOMP error semantics are
preserved; locate the throw in TrackingGpsWebSocketController and swap it to the
standard auth exception used across the project (e.g., AuthException,
UnauthenticatedException, or your project's DomainAuthException) and include the
same message when constructing that exception.
In
`@src/main/java/com/semosan/api/domain/tracking/dto/message/GpsPointMessage.java`:
- Around line 12-16: Add range validation to the GpsPointMessage DTO: keep the
existing `@NotNull` on the lat and lng fields and add `@DecimalMin` and `@DecimalMax`
constraints (or `@Min/`@Max if using integral types) to enforce lat between -90.0
and 90.0 and lng between -180.0 and 180.0; update the constraint messages to
reflect the range (e.g., "위도(lat)는 -90에서 90 사이여야 합니다.") and import the
corresponding javax.validation.constraints annotations so validation fails at
the DTO boundary for invalid coordinates.
In
`@src/main/java/com/semosan/api/domain/tracking/service/TrackingGpsPublisher.java`:
- Around line 42-45: The publish method currently does a per-message DB read via
trackingSessionRepository.findById in TrackingGpsPublisher.publish which causes
DB pressure; change the flow so publish no longer queries the DB on each
GpsPointMessage by using a cached session/ownership lookup (e.g., a Redis-backed
cache or in-memory cache refreshed at session start) and only fall back to the
DB asynchronously or on cache-miss/stale events; specifically, replace the
direct call to trackingSessionRepository.findById and the immediate
TrackingSession.isOwnedBy check with a fast cache lookup (keyed by sessionId)
that stores ownership/state and add a cache-refresh/validation path elsewhere
(session start/stop handlers or a background refresher) so publish stays
lightweight and only touches Redis/in-memory cache.
In
`@src/main/java/com/semosan/api/domain/tracking/service/TrackingSessionStatsService.java`:
- Around line 48-85: The current read-modify-write in
TrackingSessionStatsService (calls to hash.entries(key), computing
distance/ascent/descent/pointCount, then hash.putAll(key) and
redisTemplate.expire(key, TTL)) is not atomic and can lose concurrent updates;
replace it with an atomic Redis Lua script (invoked via RedisTemplate.execute or
RedisScript) that: reads F_LAST_LAT/F_LAST_LNG/F_LAST_ALTITUDE, computes the
haversine delta and altitude delta inside the script (or pass precomputed deltas
from Java and only read last coords inside the script), applies
HINCRBYFLOAT/HINCRBY for F_DISTANCE_TOTAL, F_ASCENT_TOTAL, F_DESCENT_TOTAL and
F_POINT_COUNT, and HSETs the new F_LAST_* and F_LAST_RECORDED_AT and sets the
TTL; ensure the code removes the separate hash.entries/putAll/expire calls and
instead calls the single Redis execute with the script using the same key and
field constants (F_LAST_LAT, F_LAST_LNG, F_LAST_ALTITUDE, F_DISTANCE_TOTAL,
F_ASCENT_TOTAL, F_DESCENT_TOTAL, F_POINT_COUNT, F_LAST_RECORDED_AT).
In
`@src/main/java/com/semosan/api/domain/tracking/service/TrackingStreamConsumer.java`:
- Around line 52-53: The buffers map in TrackingStreamConsumer holds Queues of
PendingPoint per session but never removes entries when a session's queue is
drained, leading to unbounded growth; update the code paths that consume/drain a
session queue (the logic interacting with buffers and Queue<PendingPoint>) to
remove the map entry once the queue becomes empty — e.g., after
polling/processing items, check if queue.isEmpty() and call
buffers.remove(sessionId, queue) (or use buffers.computeIfPresent to atomically
drain and remove) so stale session keys are cleaned up; ensure the removal uses
the queue instance or atomic map operations to avoid races.
- Around line 76-78: In TrackingStreamConsumer, update the catch block that
currently logs the full GPS payload (message.getId(), body) so it no longer
writes raw location data to logs; replace the body argument with either a
sanitized summary (e.g., masked/omitted coordinates, payload length, or a fixed
token) or remove it entirely and only log non-sensitive identifiers like
message.getId() and the exception; change the log.warn call in the
catch(RuntimeException e) handler accordingly and ensure any helper used for
sanitization does not reconstruct raw lat/lng/altitude values.
🪄 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: 409094a3-9e87-4cca-bdbe-cb017e9e3e60
📒 Files selected for processing (13)
build.gradlesrc/main/java/com/semosan/api/common/config/SecurityConfig.javasrc/main/java/com/semosan/api/domain/tracking/config/TrackingStreamListenerConfig.javasrc/main/java/com/semosan/api/domain/tracking/controller/TrackingGpsWebSocketController.javasrc/main/java/com/semosan/api/domain/tracking/dto/message/GpsPointMessage.javasrc/main/java/com/semosan/api/domain/tracking/entity/TrackingPoint.javasrc/main/java/com/semosan/api/domain/tracking/repository/TrackingPointRepository.javasrc/main/java/com/semosan/api/domain/tracking/service/TrackingGpsPublisher.javasrc/main/java/com/semosan/api/domain/tracking/service/TrackingSessionStatsService.javasrc/main/java/com/semosan/api/domain/tracking/service/TrackingStreamConsumer.javasrc/main/java/com/semosan/api/domain/tracking/websocket/StompAuthChannelInterceptor.javasrc/main/java/com/semosan/api/domain/tracking/websocket/UserIdPrincipal.javasrc/main/java/com/semosan/api/domain/tracking/websocket/WebSocketConfig.java
Comment on lines
+91
to
+99
| // WebSocket(STOMP) 핸드셰이크는 다양한 origin(모바일/로컬 테스트 페이지 등)에서 들어옴. | ||
| // /ws/** 만 별도 정책으로 풀어준다. | ||
| // TODO: production 에서는 모바일 앱 origin 만 명시적으로 허용하도록 좁힐 것. | ||
| CorsConfiguration wsConfig = new CorsConfiguration(); | ||
| wsConfig.setAllowedOriginPatterns(List.of("*")); | ||
| wsConfig.setAllowedMethods(List.of("GET")); | ||
| wsConfig.setAllowedHeaders(List.of("*")); | ||
| wsConfig.setAllowCredentials(true); | ||
|
|
There was a problem hiding this comment.
Restrict WebSocket origins now (don’t ship wildcard + credentials).
Line 95 currently allows any origin for /ws/** while Line 98 enables credentials. This leaves the handshake policy overly permissive for a privileged channel. Replace * with an explicit allowlist and keep it environment-driven. Also align WebSocketConfig Line 29 to the same allowlist.
🔒 Suggested change
- wsConfig.setAllowedOriginPatterns(List.of("*"));
+ wsConfig.setAllowedOrigins(List.of(
+ "http://localhost:3000",
+ "https://lgenius.site"
+ ));Also applies to: 101-101
🤖 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/common/config/SecurityConfig.java` around lines
91 - 99, The WebSocket CORS config is too permissive: update the
CorsConfiguration instance named wsConfig to use an environment-driven explicit
allowlist instead of setAllowedOriginPatterns(List.of("*")), keep
setAllowCredentials(true) only if the allowlist is not wildcard, and ensure
setAllowedOriginPatterns (or setAllowedOrigins) is populated from a configurable
property (e.g., a comma-separated env/property) so production can restrict to
mobile app origins; also mirror the same change in WebSocketConfig (Line 29) so
both places read the same allowlist property.
Comment on lines
+48
to
+52
| container.receiveAutoAck( | ||
| Consumer.from(trackingProperties.getConsumerGroup(), buildConsumerName()), | ||
| StreamOffset.create(trackingProperties.getStreamKey(), ReadOffset.lastConsumed()), | ||
| trackingStreamConsumer | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring Data Redis StreamMessageListenerContainer, does receiveAutoAck acknowledge entries before listener processing succeeds, and what pattern is recommended for at-least-once processing with consumer groups?
💡 Result:
Yes. In StreamMessageListenerContainer, using receiveAutoAck (auto-acknowledge) acknowledges messages when they are received, not after your listener code completes successfully. This is explicitly stated as “Every message is acknowledged when received” for receiveAutoAck, whereas the non-auto-ack mode requires acknowledging via StreamOperations.acknowledge after processing completes successfully [1][2][3]. Recommended pattern for at-least-once with consumer groups: 1) Use external acknowledgement: call receive(...) (autoAck=false) and only acknowledge with StreamOperations.acknowledge after your listener processing succeeds [1][4][3]. This keeps failed/unacknowledged deliveries in the consumer group’s Pending Entries List (PEL) [3][5]. 2) Handle PENDING recovery explicitly: start (or periodically run) a “drain PEL” phase for your consumer group (e.g., read pending/previously delivered entries), and only when the PEL is empty switch to reading new messages (ID “>”). Redis documents that re-delivered pending entries remain pending until explicitly acknowledged via XACK, and the typical recovery approach is to first process the pending backlog (using an initial ID such as 0), then proceed to “>” for new messages [6]. In code terms, the container doesn’t provide a built-in “always re-process PENDING until it succeeds before polling for new” switch; you implement the two-phase (PEL-drain then new) flow by choosing the appropriate StreamRead offsets/requests (PEL history first, then “>”), and by acknowledging only after successful processing [1][3][6].
Citations:
- 1: https://docs.spring.vmware.com/spring-data-redis/docs/3.0.13/api/org/springframework/data/redis/stream/StreamMessageListenerContainer.html
- 2: https://docs.enterprise.spring.io/spring-data-redis/docs/3.0.13/api/org/springframework/data/redis/stream/StreamMessageListenerContainer.html
- 3: https://docs.enterprise.spring.io/spring-data-redis/docs/3.0.14/reference/html/
- 4: https://github.com/spring-projects/spring-data-redis/blob/master/src/main/java/org/springframework/data/redis/stream/StreamMessageListenerContainer.java
- 5: https://docs.spring.io/spring-data/redis/reference/redis/redis-streams.html
- 6: https://redis.io/docs/latest/commands/XREADGROUP/
Use manual acknowledgement instead of auto-ack to ensure at-least-once delivery for GPS points.
receiveAutoAck() acknowledges entries when received, before your listener processes them. If downstream processing fails, acknowledged entries are permanently lost and cannot be retried from the consumer group's pending backlog.
Switch to receive() (non-auto-ack) and call StreamOperations.acknowledge() only after processing succeeds. This keeps failed entries in the Pending Entries List for recovery and retries.
🤖 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/config/TrackingStreamListenerConfig.java`
around lines 48 - 52, The code currently uses container.receiveAutoAck(...)
which acknowledges entries immediately; change to container.receive(...)
(non-auto-ack) using the same
Consumer.from(trackingProperties.getConsumerGroup(), buildConsumerName()),
StreamOffset.create(trackingProperties.getStreamKey(),
ReadOffset.lastConsumed()), and trackingStreamConsumer so records are delivered
but not auto-acked. Then update your trackingStreamConsumer processing logic to
call the Redis stream acknowledge API (e.g., StreamOperations.acknowledge(...)
or the container/RedisTemplate opsForStream().acknowledge) after successful
processing using trackingProperties.getConsumerGroup() and the record id; do not
acknowledge on failure so entries remain in the Pending Entries List for
retries.
| if (principal instanceof UserIdPrincipal userPrincipal) { | ||
| return userPrincipal.getUserId(); | ||
| } | ||
| throw new IllegalStateException("Unauthenticated WebSocket message"); |
There was a problem hiding this comment.
Use domain auth exception instead of IllegalStateException.
Line 41 currently throws a generic runtime exception, which can surface as a 500-style internal failure path. Please throw your standard auth/business exception type so STOMP error semantics stay consistent.
🤖 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/TrackingGpsWebSocketController.java`
at line 41, Replace the generic IllegalStateException thrown in
TrackingGpsWebSocketController (the "throw new
IllegalStateException(\"Unauthenticated WebSocket message\");" statement) with
your domain authentication/business exception type so STOMP error semantics are
preserved; locate the throw in TrackingGpsWebSocketController and swap it to the
standard auth exception used across the project (e.g., AuthException,
UnauthenticatedException, or your project's DomainAuthException) and include the
same message when constructing that exception.
Comment on lines
+12
to
+16
| @NotNull(message = "위도(lat)는 필수입니다.") | ||
| Double lat, | ||
|
|
||
| @NotNull(message = "경도(lng)는 필수입니다.") | ||
| Double lng, |
There was a problem hiding this comment.
Add latitude/longitude range validation.
@NotNull alone is not enough here. Invalid values can enter the stream and skew session stats. Please enforce lat [-90, 90] and lng [-180, 180] at DTO boundary.
✅ Suggested validation annotations
import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.DecimalMax;
+import jakarta.validation.constraints.DecimalMin;
@@
- `@NotNull`(message = "위도(lat)는 필수입니다.")
+ `@NotNull`(message = "위도(lat)는 필수입니다.")
+ `@DecimalMin`(value = "-90.0", message = "위도(lat)는 -90 이상이어야 합니다.")
+ `@DecimalMax`(value = "90.0", message = "위도(lat)는 90 이하여야 합니다.")
Double lat,
@@
- `@NotNull`(message = "경도(lng)는 필수입니다.")
+ `@NotNull`(message = "경도(lng)는 필수입니다.")
+ `@DecimalMin`(value = "-180.0", message = "경도(lng)는 -180 이상이어야 합니다.")
+ `@DecimalMax`(value = "180.0", message = "경도(lng)는 180 이하여야 합니다.")
Double lng,🤖 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/dto/message/GpsPointMessage.java`
around lines 12 - 16, Add range validation to the GpsPointMessage DTO: keep the
existing `@NotNull` on the lat and lng fields and add `@DecimalMin` and `@DecimalMax`
constraints (or `@Min/`@Max if using integral types) to enforce lat between -90.0
and 90.0 and lng between -180.0 and 180.0; update the constraint messages to
reflect the range (e.g., "위도(lat)는 -90에서 90 사이여야 합니다.") and import the
corresponding javax.validation.constraints annotations so validation fails at
the DTO boundary for invalid coordinates.
Comment on lines
+42
to
+45
| public void publish(Long userId, Long sessionId, GpsPointMessage message) { | ||
| TrackingSession session = trackingSessionRepository.findById(sessionId) | ||
| .orElseThrow(() -> new GeneralException(ErrorStatus.TRACKING_SESSION_NOT_FOUND)); | ||
| if (!session.isOwnedBy(userId)) { |
There was a problem hiding this comment.
Remove per-point DB lookup from ingress hot path.
Line 43 does a DB read for every GPS message before publishing to Redis. At scale, this undermines the PR’s low-latency ingestion goal and creates avoidable DB pressure. Consider caching session ownership/state in Redis (or validating once at session start and refreshing asynchronously) so publish remains lightweight.
🤖 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/TrackingGpsPublisher.java`
around lines 42 - 45, The publish method currently does a per-message DB read
via trackingSessionRepository.findById in TrackingGpsPublisher.publish which
causes DB pressure; change the flow so publish no longer queries the DB on each
GpsPointMessage by using a cached session/ownership lookup (e.g., a Redis-backed
cache or in-memory cache refreshed at session start) and only fall back to the
DB asynchronously or on cache-miss/stale events; specifically, replace the
direct call to trackingSessionRepository.findById and the immediate
TrackingSession.isOwnedBy check with a fast cache lookup (keyed by sessionId)
that stores ownership/state and add a cache-refresh/validation path elsewhere
(session start/stop handlers or a background refresher) so publish stays
lightweight and only touches Redis/in-memory cache.
Comment on lines
+48
to
+85
| Map<String, String> prev = hash.entries(key); | ||
|
|
||
| double distanceTotal = parseDouble(prev.get(F_DISTANCE_TOTAL)); | ||
| double ascentTotal = parseDouble(prev.get(F_ASCENT_TOTAL)); | ||
| double descentTotal = parseDouble(prev.get(F_DESCENT_TOTAL)); | ||
| long pointCount = parseLong(prev.get(F_POINT_COUNT)); | ||
|
|
||
| Double prevLat = parseNullableDouble(prev.get(F_LAST_LAT)); | ||
| Double prevLng = parseNullableDouble(prev.get(F_LAST_LNG)); | ||
| if (prevLat != null && prevLng != null) { | ||
| distanceTotal += haversineMeters(prevLat, prevLng, lat, lng); | ||
| } | ||
| Double prevAltitude = parseNullableDouble(prev.get(F_LAST_ALTITUDE)); | ||
| if (altitude != null && prevAltitude != null) { | ||
| double delta = altitude - prevAltitude; | ||
| if (delta > 0) { | ||
| ascentTotal += delta; | ||
| } else if (delta < 0) { | ||
| descentTotal += -delta; | ||
| } | ||
| } | ||
| pointCount += 1; | ||
|
|
||
| Map<String, String> next = new HashMap<>(); | ||
| next.put(F_LAST_LAT, String.valueOf(lat)); | ||
| next.put(F_LAST_LNG, String.valueOf(lng)); | ||
| if (altitude != null) { | ||
| next.put(F_LAST_ALTITUDE, String.valueOf(altitude)); | ||
| } | ||
| next.put(F_LAST_RECORDED_AT, recordedAt.toString()); | ||
| next.put(F_DISTANCE_TOTAL, String.valueOf(distanceTotal)); | ||
| next.put(F_ASCENT_TOTAL, String.valueOf(ascentTotal)); | ||
| next.put(F_DESCENT_TOTAL, String.valueOf(descentTotal)); | ||
| next.put(F_POINT_COUNT, String.valueOf(pointCount)); | ||
|
|
||
| hash.putAll(key, next); | ||
| redisTemplate.expire(key, TTL); | ||
| } |
There was a problem hiding this comment.
Make stats updates atomic to prevent lost increments.
Lines 48–85 do non-atomic read-modify-write on a shared Redis hash; concurrent consumers can race and overwrite totals/point counts.
Possible direction
- Map<String, String> prev = hash.entries(key);
- // compute...
- hash.putAll(key, next);
+ // Move this update into a Lua script (EVAL) that:
+ // 1) reads current fields
+ // 2) computes deltas
+ // 3) writes updated fields
+ // 4) refreshes TTL
+ // in one atomic Redis operation🤖 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/TrackingSessionStatsService.java`
around lines 48 - 85, The current read-modify-write in
TrackingSessionStatsService (calls to hash.entries(key), computing
distance/ascent/descent/pointCount, then hash.putAll(key) and
redisTemplate.expire(key, TTL)) is not atomic and can lose concurrent updates;
replace it with an atomic Redis Lua script (invoked via RedisTemplate.execute or
RedisScript) that: reads F_LAST_LAT/F_LAST_LNG/F_LAST_ALTITUDE, computes the
haversine delta and altitude delta inside the script (or pass precomputed deltas
from Java and only read last coords inside the script), applies
HINCRBYFLOAT/HINCRBY for F_DISTANCE_TOTAL, F_ASCENT_TOTAL, F_DESCENT_TOTAL and
F_POINT_COUNT, and HSETs the new F_LAST_* and F_LAST_RECORDED_AT and sets the
TTL; ensure the code removes the separate hash.entries/putAll/expire calls and
instead calls the single Redis execute with the script using the same key and
field constants (F_LAST_LAT, F_LAST_LNG, F_LAST_ALTITUDE, F_DISTANCE_TOTAL,
F_ASCENT_TOTAL, F_DESCENT_TOTAL, F_POINT_COUNT, F_LAST_RECORDED_AT).
Comment on lines
+52
to
+53
| private final Map<Long, Queue<PendingPoint>> buffers = new ConcurrentHashMap<>(); | ||
|
|
There was a problem hiding this comment.
Remove drained session buffers to avoid unbounded map growth.
buffers entries are never removed after queues become empty, so long-running workloads can accumulate stale session keys.
Cleanup diff
protected void flushSession(Long sessionId) {
Queue<PendingPoint> queue = buffers.get(sessionId);
if (queue == null || queue.isEmpty()) {
+ buffers.remove(sessionId, queue);
return;
}
@@
if (!batch.isEmpty()) {
trackingPointRepository.saveAll(batch);
log.debug("Flushed {} GPS points for session {}", batch.size(), sessionId);
}
+ if (queue.isEmpty()) {
+ buffers.remove(sessionId, queue);
+ }
}Also applies to: 90-117
🤖 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/TrackingStreamConsumer.java`
around lines 52 - 53, The buffers map in TrackingStreamConsumer holds Queues of
PendingPoint per session but never removes entries when a session's queue is
drained, leading to unbounded growth; update the code paths that consume/drain a
session queue (the logic interacting with buffers and Queue<PendingPoint>) to
remove the map entry once the queue becomes empty — e.g., after
polling/processing items, check if queue.isEmpty() and call
buffers.remove(sessionId, queue) (or use buffers.computeIfPresent to atomically
drain and remove) so stale session keys are cleaned up; ensure the removal uses
the queue instance or atomic map operations to avoid races.
Comment on lines
+76
to
+78
| } catch (RuntimeException e) { | ||
| log.warn("Failed to process GPS stream message: id={} body={}", message.getId(), body, e); | ||
| } |
There was a problem hiding this comment.
Do not log raw GPS payloads on failures.
Line 77 logs full message body (lat/lng/altitude), which can leak sensitive location data in logs.
Safer logging diff
- log.warn("Failed to process GPS stream message: id={} body={}", message.getId(), body, e);
+ log.warn("Failed to process GPS stream message: id={} keys={}", message.getId(), body.keySet(), e);🤖 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/TrackingStreamConsumer.java`
around lines 76 - 78, In TrackingStreamConsumer, update the catch block that
currently logs the full GPS payload (message.getId(), body) so it no longer
writes raw location data to logs; replace the body argument with either a
sanitized summary (e.g., masked/omitted coordinates, payload length, or a fixed
token) or remove it entirely and only log non-sensitive identifiers like
message.getId() and the exception; change the log.warn call in the
catch(RuntimeException e) handler accordingly and ensure any helper used for
sanitization does not reconstruct raw lat/lng/altitude values.
pooreumjung
reviewed
May 20, 2026
|
|
||
| Queue<TrackingPointFlushService.PendingPoint> queue = | ||
| buffers.computeIfAbsent(sessionId, k -> new ConcurrentLinkedQueue<>()); | ||
| queue.offer(new TrackingPointFlushService.PendingPoint(lat, lng, altitude, recordedAt)); |
Member
There was a problem hiding this comment.
buffers에서 세션 키가 제거되지 않아서 메모리 누수 발생 가능성이 있을 듯!
[Refactor] 유저 회원 탈퇴 로직 수정
fix: 관악산 중복 row 정리
PostGIS extension 미설치로 인한 Flyway V5 마이그레이션 실패 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
[Feat]#18 트래킹 세션 머지
카카오 로그인 액세스 토큰 방식 적용
[Feat] 랜덤 닉네임 생성 기능 추가
# Conflicts: # build.gradle
- TrackingStreamConsumer.flushSession: DB flush 예외 발생 시 batch 를 큐로 되돌려 다음 주기에 재시도 (이전에는 poll 로 큐에서 꺼낸 뒤 실패하면 유실) - TrackingPointFlushService.flush: recordedAt 검증 — null/미래(+5분 초과)/ 과거(-24시간 초과) 인 점은 저장 전에 폐기, 정상 점만 saveAll
- TrackingSessionTerminatedEvent 추가 (sessionId) - 세션 종료(complete/abandon) 및 24h 자동 만료 시 이벤트 발행 - TrackingStreamConsumer 가 @TransactionalEventListener(AFTER_COMMIT) 로 받아서 잔여 점 final flush 후 buffers 에서 키 제거 (chunk 실패는 폐기, 재시도 X)
GPS 수집이 Redis Stream → tracking_points 경로라 tracking_sessions.updated_at 이
갱신되지 않아 정상 트래킹 중인 세션도 24h 후 잘못 abandon 될 수 있었음.
- TrackingSessionActivityService 추가: Redis 키 tracking:session:{id}:lastActive
에 마지막 GPS 수신 시각 기록 (TTL 25h)
- TrackingStreamConsumer.onMessage: 점 수신마다 markActive 호출
- TrackingSessionExpiryScheduler: DB updatedAt 으로 1차 후보 추린 뒤 Redis 의
lastActive 가 cutoff 이내면 skip
- 컬럼: tracking_session FK + location(PostGIS Point) + altitude + recorded_at - 인덱스: (tracking_session_id, recorded_at) — 세션별 시간순 점 조회용
This was referenced May 22, 2026
Merged
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
3 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.
🧾 요약
🔗 이슈
✨ 변경 내용
TrackingPoint도메인 모델 및 세션 통계(거리/페이스 등) 계산 로직 추가✅ 확인
Summary by CodeRabbit
New Features
Security