refactor: improve Redis GeoHash code with event-driven pattern#85
Merged
Merged
Conversation
- Add @TransactionalEventListener for transaction consistency - Implement pagination with offset support in geo queries - Add Redis Pipeline for bulk operations (GeoDataInitializer) - Add exception handling with RedisGeoIndexException - Preserve Redis distance order when fetching from MySQL - Add findSearchableMatchIds() for memory efficiency Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthrough이 변경 사항은 매치 지오 인덱싱을 동기식 직접 호출에서 비동기 이벤트 기반 아키텍처로 전환합니다. Redis 지오 인덱스 작업에 대한 대량 연산 지원, 페이지네이션 기능, 그리고 새로운 예외 처리를 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Service as MatchCreator/<br/>MatchReactivator
participant DB as Match Database
participant EventPublisher as ApplicationEventPublisher
participant Listener as MatchGeoIndexEventListener
participant Port as MatchGeoIndexPort
participant Redis as Redis Geo Index
User->>Service: Create/Reactivate Match
Service->>DB: Save Match
DB-->>Service: Match Saved
Service->>EventPublisher: publishEvent(MatchCreatedEvent)
EventPublisher->>Listener: handleMatchCreated() [AFTER_COMMIT]
Listener->>Port: addMatch(matchId, lon, lat)
Port->>Redis: GEOADD + Handle with try-catch
Redis-->>Port: Success/RedisGeoIndexException
Port-->>Listener: Complete
Note over User,Redis: Alternative Flow: Match Cancellation
User->>Service: Cancel/Start Match
Service->>DB: Update Match Status
DB-->>Service: Status Updated
Service->>EventPublisher: publishEvent(MatchRemovedFromGeoIndexEvent)
EventPublisher->>Listener: handleMatchRemovedFromGeoIndex() [AFTER_COMMIT]
Listener->>Port: removeMatch(matchId)
Port->>Redis: ZREM + Handle with try-catch
Redis-->>Port: Success/RedisGeoIndexException
Port-->>Listener: Complete
sequenceDiagram
actor Scheduler
participant Port1 as MatchRepositoryPort
participant Port2 as MatchGeoIndexPort
participant MySQL as MySQL Database
participant Redis as Redis Geo Index
Scheduler->>Port1: findSearchableMatchIds()
MySQL-->>Port1: List of IDs
Port1-->>Scheduler: mysqlIds
Scheduler->>Port2: findAllMatchIds()
Redis-->>Port2: List of IDs
Port2-->>Scheduler: redisIds
Scheduler->>Scheduler: missingIds = mysqlIds - redisIds
Scheduler->>Port1: findMatchesByIds(missingIds)
MySQL-->>Port1: Match Objects
Port1-->>Scheduler: matches
Scheduler->>Scheduler: entries = mapToGeoIndexEntry(matches)
Scheduler->>Port2: addMatchesBulk(entries)
Port2->>Redis: PIPELINE (Multiple GEOADD)
Redis-->>Port2: RedisGeoIndexException on error
Port2-->>Scheduler: Bulk Operation Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/com/hoops/match/adapter/out/redis/exception/RedisGeoIndexException.java`:
- Around line 10-17: The ERROR_CODE constant in RedisGeoIndexException does not
follow the project's error-code pattern; update the constant name and value to a
compliant pattern (e.g., use INVALID_GEO_INDEX or DUPLICATE_GEO_INDEX depending
on the failure semantics) and update the RedisGeoIndexException constructors to
use that new constant (symbol: ERROR_CODE in class RedisGeoIndexException).
Ensure the chosen pattern matches the guide (INVALID_* for validation/format
errors or ALREADY_*/DUPLICATE_* for existence conflicts) and adjust only the
constant value/name so the existing super(...) calls remain correct.
In
`@backend/src/main/java/com/hoops/match/adapter/out/redis/MatchGeoRedisAdapter.java`:
- Around line 80-108: The method findMatchIdsWithinRadius is doing both fetching
and mapping and exceeds preferred length—extract the Redis query and the
result-mapping into two helpers (e.g., a private method
fetchGeoResults(BigDecimal longitude, BigDecimal latitude, double radiusKm, int
fetchCount) that calls redisTemplate.opsForGeo().radius(GEO_KEY, circle, args)
and returns GeoResults<...>, and a private method
mapGeoResultsToIds(GeoResults<RedisGeoCommands.GeoLocation<String>> results, int
offset, int limit) that handles null-check, skipping, limiting and uses
parseMatchId). Replace the inline logic in findMatchIdsWithinRadius to call
these two helpers and keep exception handling as-is.
- Around line 44-65: The addMatchesBulk method is too long and nested; extract
the pipeline logic into a small helper to reduce length and nesting. Create a
private helper (e.g., buildGeoAddPipeline or executeGeoAddPipeline) that accepts
List<GeoIndexEntry> and performs the redisTemplate.executePipelined call using
GEO_KEY, GeoIndexEntry.longitude()/latitude(), and
formatMatchId(entry.matchId()), then call that helper from addMatchesBulk and
preserve the same logging and exception handling (throw RedisGeoIndexException
on failure). Ensure helper is small and focused so addMatchesBulk only validates
entries, delegates to the helper, logs success, and handles exceptions.
In
`@backend/src/main/java/com/hoops/match/application/event/MatchGeoIndexEventListener.java`:
- Around line 21-31: The AFTER_COMMIT event handlers in
MatchGeoIndexEventListener (methods handleMatchCreated and
handleMatchRemovedFromGeoIndex) must not let exceptions propagate to callers;
wrap the calls to matchGeoIndex.addMatch(...) and matchGeoIndex.removeMatch(...)
in try/catch, log the error with context (including event.matchId()), and
instead of throwing re-publish a retry/compensation event or enqueue a job
(e.g., publish a MatchGeoIndexRetryEvent or push to a retry queue) so DB changes
remain committed and geo-index updates can be retried/compensated
asynchronously.
🧹 Nitpick comments (5)
backend/src/main/java/com/hoops/match/application/event/MatchRemovedFromGeoIndexEvent.java (1)
8-10: matchId null 방지 가드 추가를 권장합니다.
Line 8-10: null ID로 이벤트가 발행되면 리스너에서 NPE 위험이 있어 생성 시 방어하는 편이 안전합니다.♻️ 제안 diff
package com.hoops.match.application.event; +import java.util.Objects; + /** * 매치 Geo Index 제거 이벤트 * 트랜잭션 커밋 후 Redis Geo Index에서 매치 제거 * 매치 취소, 시작 등으로 검색 대상에서 제외될 때 발생 */ public record MatchRemovedFromGeoIndexEvent( Long matchId ) implements MatchGeoIndexEvent { + public MatchRemovedFromGeoIndexEvent { + Objects.requireNonNull(matchId, "matchId"); + } }backend/src/main/java/com/hoops/match/application/service/MatchCreator.java (1)
36-74:createMatch메서드 길이가 코딩 가이드라인을 초과합니다.현재 메서드가 약 38줄로, 가이드라인에서 권장하는 20줄 제한을 초과합니다. 객체 생성 로직을 별도의 private 메서드로 분리하는 것을 고려해 주세요.
♻️ 리팩토링 제안
`@Override` public Match createMatch(CreateMatchCommand command) { MatchSchedule schedule = new MatchSchedule( command.matchDate(), command.startTime(), command.endTime() ); policyValidator.validateCreateMatch(schedule, command.maxParticipants()); validateNoOverlappingHosting(command.hostId(), schedule); - HostInfoResult hostInfo = hostInfoPort.getHostInfo(command.hostId()); - LocationInfoResult locationInfo = locationInfoPort.getLocationInfo(command.locationId()); - - MatchHost host = new MatchHost(hostInfo.hostId(), hostInfo.nickname()); - MatchLocation location = new MatchLocation( - locationInfo.latitude(), - locationInfo.longitude(), - locationInfo.address() - ); - - Match match = Match.create( - host, - command.title(), - command.description(), - location, - schedule, - command.maxParticipants() - ); + Match match = buildMatch(command, schedule); Match savedMatch = matchRepository.save(match); eventPublisher.publishEvent(new MatchCreatedEvent( savedMatch.getId(), savedMatch.getLongitude(), savedMatch.getLatitude() )); return savedMatch; } +private Match buildMatch(CreateMatchCommand command, MatchSchedule schedule) { + HostInfoResult hostInfo = hostInfoPort.getHostInfo(command.hostId()); + LocationInfoResult locationInfo = locationInfoPort.getLocationInfo(command.locationId()); + + MatchHost host = new MatchHost(hostInfo.hostId(), hostInfo.nickname()); + MatchLocation location = new MatchLocation( + locationInfo.latitude(), + locationInfo.longitude(), + locationInfo.address() + ); + + return Match.create( + host, + command.title(), + command.description(), + location, + schedule, + command.maxParticipants() + ); +}backend/src/main/java/com/hoops/match/application/service/MatchReactivator.java (1)
30-34: 재활성화 시MatchCreatedEvent사용은 의미적으로 모호할 수 있습니다.기능적으로는 정상 동작하지만, 매치 재활성화에
MatchCreatedEvent를 사용하는 것은 이벤트의 의미를 혼란스럽게 할 수 있습니다.MatchAddedToGeoIndexEvent와 같이 geo-index 추가 의도를 명확히 표현하는 이벤트 이름을 고려해 보세요.backend/src/main/java/com/hoops/match/application/scheduler/GeoDataSyncScheduler.java (2)
30-71:syncGeoData메서드가 길고 여러 책임을 가지고 있습니다.현재 메서드가 약 40줄로 가이드라인(20줄)을 초과하며, "누락된 매치 추가"와 "고아 매치 제거"라는 두 가지 책임을 수행합니다. SRP 원칙에 따라 분리를 권장합니다.
♻️ 리팩토링 제안
public void syncGeoData() { log.info("Starting Geo Index sync check..."); Set<Long> mysqlIds = new HashSet<>(matchRepository.findSearchableMatchIds()); Set<Long> redisIds = new HashSet<>(matchGeoIndex.findAllMatchIds()); - // MySQL에는 있지만 Redis에는 없는 매치 추가 - List<Long> missingIds = mysqlIds.stream() - .filter(id -> !redisIds.contains(id)) - .toList(); - - if (!missingIds.isEmpty()) { - List<Match> missingMatches = matchRepository.findAllByIds(missingIds); - List<GeoIndexEntry> entries = missingMatches.stream() - .map(match -> new GeoIndexEntry( - match.getId(), - match.getLongitude(), - match.getLatitude() - )) - .toList(); - - matchGeoIndex.addMatchesBulk(entries); - - for (Long matchId : missingIds) { - log.warn("Added missing match to Redis: matchId={}", matchId); - } - } - - // Redis에는 있지만 MySQL에는 없는 매치 제거 - int removedCount = 0; - for (Long redisId : redisIds) { - if (!mysqlIds.contains(redisId)) { - matchGeoIndex.removeMatch(redisId); - removedCount++; - log.warn("Removed orphan match from Redis: matchId={}", redisId); - } - } + int addedCount = addMissingMatches(mysqlIds, redisIds); + int removedCount = removeOrphanMatches(mysqlIds, redisIds); log.info("Geo Index sync completed. Added: {}, Removed: {}", addedCount, removedCount); } + +private int addMissingMatches(Set<Long> mysqlIds, Set<Long> redisIds) { + List<Long> missingIds = mysqlIds.stream() + .filter(id -> !redisIds.contains(id)) + .toList(); + + if (missingIds.isEmpty()) { + return 0; + } + + List<Match> missingMatches = matchRepository.findAllByIds(missingIds); + List<GeoIndexEntry> entries = missingMatches.stream() + .map(match -> new GeoIndexEntry( + match.getId(), + match.getLongitude(), + match.getLatitude() + )) + .toList(); + + matchGeoIndex.addMatchesBulk(entries); + missingIds.forEach(id -> log.warn("Added missing match to Redis: matchId={}", id)); + return missingIds.size(); +} + +private int removeOrphanMatches(Set<Long> mysqlIds, Set<Long> redisIds) { + List<Long> orphanIds = redisIds.stream() + .filter(id -> !mysqlIds.contains(id)) + .toList(); + + orphanIds.forEach(id -> { + matchGeoIndex.removeMatch(id); + log.warn("Removed orphan match from Redis: matchId={}", id); + }); + return orphanIds.size(); +}
60-68: 고아 매치 제거에도 bulk 연산 적용을 고려해 주세요.누락된 매치 추가는
addMatchesBulk를 사용하지만, 고아 매치 제거는 개별 호출(removeMatch)을 사용합니다. 일관성과 성능을 위해removeMatchesBulk메서드 추가를 고려해 보세요.
Comment on lines
+10
to
+17
| private static final String ERROR_CODE = "REDIS_GEO_INDEX_ERROR"; | ||
|
|
||
| public RedisGeoIndexException(String message) { | ||
| super(ERROR_CODE, message); | ||
| } | ||
|
|
||
| public RedisGeoIndexException(String message, Throwable cause) { | ||
| super(ERROR_CODE, message, cause); |
There was a problem hiding this comment.
에러 코드 패턴이 가이드라인에 맞지 않습니다.
Line 10-17: ERROR_CODE가 허용 패턴(NOT_FOUND, ALREADY_/DUPLICATE_, NOT_HOST/NOT_PARTICIPANT, INVALID_*)에 맞지 않습니다. 패턴에 맞는 코드로 변경이 필요합니다. 코딩 가이드라인에 따라.
🐛 제안 diff
- private static final String ERROR_CODE = "REDIS_GEO_INDEX_ERROR";
+ private static final String ERROR_CODE = "INVALID_REDIS_GEO_INDEX";🤖 Prompt for AI Agents
In
`@backend/src/main/java/com/hoops/match/adapter/out/redis/exception/RedisGeoIndexException.java`
around lines 10 - 17, The ERROR_CODE constant in RedisGeoIndexException does not
follow the project's error-code pattern; update the constant name and value to a
compliant pattern (e.g., use INVALID_GEO_INDEX or DUPLICATE_GEO_INDEX depending
on the failure semantics) and update the RedisGeoIndexException constructors to
use that new constant (symbol: ERROR_CODE in class RedisGeoIndexException).
Ensure the chosen pattern matches the guide (INVALID_* for validation/format
errors or ALREADY_*/DUPLICATE_* for existence conflicts) and adjust only the
constant value/name so the existing super(...) calls remain correct.
Comment on lines
+44
to
65
| public void addMatchesBulk(List<GeoIndexEntry> entries) { | ||
| if (entries.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| Long removed = redisTemplate.opsForZSet().remove(GEO_KEY, member); | ||
| log.debug("Removed match from geo index: matchId={}, removed={}", matchId, removed); | ||
| try { | ||
| redisTemplate.executePipelined((RedisCallback<Object>) connection -> { | ||
| byte[] keyBytes = GEO_KEY.getBytes(StandardCharsets.UTF_8); | ||
| for (GeoIndexEntry entry : entries) { | ||
| connection.geoCommands().geoAdd( | ||
| keyBytes, | ||
| new Point(entry.longitude().doubleValue(), entry.latitude().doubleValue()), | ||
| formatMatchId(entry.matchId()).getBytes(StandardCharsets.UTF_8) | ||
| ); | ||
| } | ||
| return null; | ||
| }); | ||
| log.info("Bulk added {} matches to geo index", entries.size()); | ||
| } catch (Exception e) { | ||
| throw new RedisGeoIndexException("Failed to bulk add matches to geo index", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
addMatchesBulk 메서드를 분리해 길이/중첩을 줄이세요.
현재 메서드가 20줄을 넘고 중첩 단계가 많습니다. 파이프라인 실행을 헬퍼로 분리하면 규칙을 충족합니다.
♻️ 제안 수정
public void addMatchesBulk(List<GeoIndexEntry> entries) {
if (entries.isEmpty()) {
return;
}
try {
- redisTemplate.executePipelined((RedisCallback<Object>) connection -> {
- byte[] keyBytes = GEO_KEY.getBytes(StandardCharsets.UTF_8);
- for (GeoIndexEntry entry : entries) {
- connection.geoCommands().geoAdd(
- keyBytes,
- new Point(entry.longitude().doubleValue(), entry.latitude().doubleValue()),
- formatMatchId(entry.matchId()).getBytes(StandardCharsets.UTF_8)
- );
- }
- return null;
- });
+ executeBulkGeoAdd(entries);
log.info("Bulk added {} matches to geo index", entries.size());
} catch (Exception e) {
throw new RedisGeoIndexException("Failed to bulk add matches to geo index", e);
}
}
+
+private void executeBulkGeoAdd(List<GeoIndexEntry> entries) {
+ redisTemplate.executePipelined((RedisCallback<Object>) connection -> {
+ byte[] keyBytes = GEO_KEY.getBytes(StandardCharsets.UTF_8);
+ for (GeoIndexEntry entry : entries) {
+ connection.geoCommands().geoAdd(
+ keyBytes,
+ new Point(entry.longitude().doubleValue(), entry.latitude().doubleValue()),
+ formatMatchId(entry.matchId()).getBytes(StandardCharsets.UTF_8)
+ );
+ }
+ return null;
+ });
+}🤖 Prompt for AI Agents
In
`@backend/src/main/java/com/hoops/match/adapter/out/redis/MatchGeoRedisAdapter.java`
around lines 44 - 65, The addMatchesBulk method is too long and nested; extract
the pipeline logic into a small helper to reduce length and nesting. Create a
private helper (e.g., buildGeoAddPipeline or executeGeoAddPipeline) that accepts
List<GeoIndexEntry> and performs the redisTemplate.executePipelined call using
GEO_KEY, GeoIndexEntry.longitude()/latitude(), and
formatMatchId(entry.matchId()), then call that helper from addMatchesBulk and
preserve the same logging and exception handling (throw RedisGeoIndexException
on failure). Ensure helper is small and focused so addMatchesBulk only validates
entries, delegates to the helper, logs success, and handles exceptions.
Comment on lines
+80
to
+108
| public List<Long> findMatchIdsWithinRadius(BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) { | ||
| try { | ||
| Point center = new Point(longitude.doubleValue(), latitude.doubleValue()); | ||
| Distance radius = new Distance(radiusKm, RedisGeoCommands.DistanceUnit.KILOMETERS); | ||
| Circle circle = new Circle(center, radius); | ||
|
|
||
| // Redis GEORADIUS는 offset을 지원하지 않으므로, offset + limit 만큼 조회 후 skip | ||
| int fetchCount = offset + limit; | ||
|
|
||
| RedisGeoCommands.GeoRadiusCommandArgs args = RedisGeoCommands.GeoRadiusCommandArgs | ||
| .newGeoRadiusArgs() | ||
| .sortAscending() | ||
| .limit(fetchCount); | ||
|
|
||
| GeoResults<RedisGeoCommands.GeoLocation<String>> results = | ||
| redisTemplate.opsForGeo().radius(GEO_KEY, circle, args); | ||
|
|
||
| if (results == null) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| return results.getContent().stream() | ||
| .skip(offset) | ||
| .limit(limit) | ||
| .map(result -> parseMatchId(result.getContent().getName())) | ||
| .toList(); | ||
| } catch (Exception e) { | ||
| throw new RedisGeoIndexException("Failed to find matches within radius", e); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
조회 로직을 헬퍼로 분리해 규칙을 맞춰주세요.
현재 메서드가 20줄을 넘고 단계가 많습니다. 조회/매핑 분리로 가독성과 규칙 준수를 확보할 수 있습니다.
♻️ 제안 수정
public List<Long> findMatchIdsWithinRadius(BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) {
try {
- Point center = new Point(longitude.doubleValue(), latitude.doubleValue());
- Distance radius = new Distance(radiusKm, RedisGeoCommands.DistanceUnit.KILOMETERS);
- Circle circle = new Circle(center, radius);
-
- // Redis GEORADIUS는 offset을 지원하지 않으므로, offset + limit 만큼 조회 후 skip
- int fetchCount = offset + limit;
-
- RedisGeoCommands.GeoRadiusCommandArgs args = RedisGeoCommands.GeoRadiusCommandArgs
- .newGeoRadiusArgs()
- .sortAscending()
- .limit(fetchCount);
-
- GeoResults<RedisGeoCommands.GeoLocation<String>> results =
- redisTemplate.opsForGeo().radius(GEO_KEY, circle, args);
-
- if (results == null) {
- return List.of();
- }
-
- return results.getContent().stream()
- .skip(offset)
- .limit(limit)
- .map(result -> parseMatchId(result.getContent().getName()))
- .toList();
+ GeoResults<RedisGeoCommands.GeoLocation<String>> results =
+ queryRadius(longitude, latitude, radiusKm, offset, limit);
+ return toMatchIds(results, offset, limit);
} catch (Exception e) {
throw new RedisGeoIndexException("Failed to find matches within radius", e);
}
}
+
+private GeoResults<RedisGeoCommands.GeoLocation<String>> queryRadius(
+ BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) {
+ Point center = new Point(longitude.doubleValue(), latitude.doubleValue());
+ Distance radius = new Distance(radiusKm, RedisGeoCommands.DistanceUnit.KILOMETERS);
+ Circle circle = new Circle(center, radius);
+ int fetchCount = offset + limit;
+ RedisGeoCommands.GeoRadiusCommandArgs args = RedisGeoCommands.GeoRadiusCommandArgs
+ .newGeoRadiusArgs()
+ .sortAscending()
+ .limit(fetchCount);
+ return redisTemplate.opsForGeo().radius(GEO_KEY, circle, args);
+}
+
+private List<Long> toMatchIds(GeoResults<RedisGeoCommands.GeoLocation<String>> results, int offset, int limit) {
+ if (results == null) {
+ return List.of();
+ }
+ return results.getContent().stream()
+ .skip(offset)
+ .limit(limit)
+ .map(result -> parseMatchId(result.getContent().getName()))
+ .toList();
+}📝 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
| public List<Long> findMatchIdsWithinRadius(BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) { | |
| try { | |
| Point center = new Point(longitude.doubleValue(), latitude.doubleValue()); | |
| Distance radius = new Distance(radiusKm, RedisGeoCommands.DistanceUnit.KILOMETERS); | |
| Circle circle = new Circle(center, radius); | |
| // Redis GEORADIUS는 offset을 지원하지 않으므로, offset + limit 만큼 조회 후 skip | |
| int fetchCount = offset + limit; | |
| RedisGeoCommands.GeoRadiusCommandArgs args = RedisGeoCommands.GeoRadiusCommandArgs | |
| .newGeoRadiusArgs() | |
| .sortAscending() | |
| .limit(fetchCount); | |
| GeoResults<RedisGeoCommands.GeoLocation<String>> results = | |
| redisTemplate.opsForGeo().radius(GEO_KEY, circle, args); | |
| if (results == null) { | |
| return List.of(); | |
| } | |
| return results.getContent().stream() | |
| .skip(offset) | |
| .limit(limit) | |
| .map(result -> parseMatchId(result.getContent().getName())) | |
| .toList(); | |
| } catch (Exception e) { | |
| throw new RedisGeoIndexException("Failed to find matches within radius", e); | |
| } | |
| public List<Long> findMatchIdsWithinRadius(BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) { | |
| try { | |
| GeoResults<RedisGeoCommands.GeoLocation<String>> results = | |
| queryRadius(longitude, latitude, radiusKm, offset, limit); | |
| return toMatchIds(results, offset, limit); | |
| } catch (Exception e) { | |
| throw new RedisGeoIndexException("Failed to find matches within radius", e); | |
| } | |
| } | |
| private GeoResults<RedisGeoCommands.GeoLocation<String>> queryRadius( | |
| BigDecimal longitude, BigDecimal latitude, double radiusKm, int offset, int limit) { | |
| Point center = new Point(longitude.doubleValue(), latitude.doubleValue()); | |
| Distance radius = new Distance(radiusKm, RedisGeoCommands.DistanceUnit.KILOMETERS); | |
| Circle circle = new Circle(center, radius); | |
| int fetchCount = offset + limit; | |
| RedisGeoCommands.GeoRadiusCommandArgs args = RedisGeoCommands.GeoRadiusCommandArgs | |
| .newGeoRadiusArgs() | |
| .sortAscending() | |
| .limit(fetchCount); | |
| return redisTemplate.opsForGeo().radius(GEO_KEY, circle, args); | |
| } | |
| private List<Long> toMatchIds(GeoResults<RedisGeoCommands.GeoLocation<String>> results, int offset, int limit) { | |
| if (results == null) { | |
| return List.of(); | |
| } | |
| return results.getContent().stream() | |
| .skip(offset) | |
| .limit(limit) | |
| .map(result -> parseMatchId(result.getContent().getName())) | |
| .toList(); | |
| } |
🤖 Prompt for AI Agents
In
`@backend/src/main/java/com/hoops/match/adapter/out/redis/MatchGeoRedisAdapter.java`
around lines 80 - 108, The method findMatchIdsWithinRadius is doing both
fetching and mapping and exceeds preferred length—extract the Redis query and
the result-mapping into two helpers (e.g., a private method
fetchGeoResults(BigDecimal longitude, BigDecimal latitude, double radiusKm, int
fetchCount) that calls redisTemplate.opsForGeo().radius(GEO_KEY, circle, args)
and returns GeoResults<...>, and a private method
mapGeoResultsToIds(GeoResults<RedisGeoCommands.GeoLocation<String>> results, int
offset, int limit) that handles null-check, skipping, limiting and uses
parseMatchId). Replace the inline logic in findMatchIdsWithinRadius to call
these two helpers and keep exception handling as-is.
Comment on lines
+21
to
+31
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void handleMatchCreated(MatchCreatedEvent event) { | ||
| log.debug("Handling MatchCreatedEvent: matchId={}", event.matchId()); | ||
| matchGeoIndex.addMatch(event.matchId(), event.longitude(), event.latitude()); | ||
| } | ||
|
|
||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void handleMatchRemovedFromGeoIndex(MatchRemovedFromGeoIndexEvent event) { | ||
| log.debug("Handling MatchRemovedFromGeoIndexEvent: matchId={}", event.matchId()); | ||
| matchGeoIndex.removeMatch(event.matchId()); | ||
| } |
There was a problem hiding this comment.
AFTER_COMMIT 예외 전파로 요청 실패가 발생할 수 있습니다.
커밋 후 예외가 전파되면 호출자는 실패로 인식하지만 DB는 이미 반영됩니다. 예외를 내부에서 처리하고 재시도/보상 경로로 분리하는 방식을 고려하세요.
🐛 제안 수정
`@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)
public void handleMatchCreated(MatchCreatedEvent event) {
- log.debug("Handling MatchCreatedEvent: matchId={}", event.matchId());
- matchGeoIndex.addMatch(event.matchId(), event.longitude(), event.latitude());
+ try {
+ log.debug("Handling MatchCreatedEvent: matchId={}", event.matchId());
+ matchGeoIndex.addMatch(event.matchId(), event.longitude(), event.latitude());
+ } catch (Exception e) {
+ log.error("Failed to update geo index for created match: matchId={}", event.matchId(), e);
+ // TODO: 재시도/보상 경로 연계
+ }
}
`@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)
public void handleMatchRemovedFromGeoIndex(MatchRemovedFromGeoIndexEvent event) {
- log.debug("Handling MatchRemovedFromGeoIndexEvent: matchId={}", event.matchId());
- matchGeoIndex.removeMatch(event.matchId());
+ try {
+ log.debug("Handling MatchRemovedFromGeoIndexEvent: matchId={}", event.matchId());
+ matchGeoIndex.removeMatch(event.matchId());
+ } catch (Exception e) {
+ log.error("Failed to update geo index for removed match: matchId={}", event.matchId(), e);
+ // TODO: 재시도/보상 경로 연계
+ }
}🤖 Prompt for AI Agents
In
`@backend/src/main/java/com/hoops/match/application/event/MatchGeoIndexEventListener.java`
around lines 21 - 31, The AFTER_COMMIT event handlers in
MatchGeoIndexEventListener (methods handleMatchCreated and
handleMatchRemovedFromGeoIndex) must not let exceptions propagate to callers;
wrap the calls to matchGeoIndex.addMatch(...) and matchGeoIndex.removeMatch(...)
in try/catch, log the error with context (including event.matchId()), and
instead of throwing re-publish a retry/compensation event or enqueue a job
(e.g., publish a MatchGeoIndexRetryEvent or push to a retry queue) so DB changes
remain committed and geo-index updates can be retried/compensated
asynchronously.
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
@TransactionalEventListener도입으로 DB 커밋 후 Redis 업데이트RedisGeoIndexException추가로 명확한 에러 핸들링findSearchableMatchIds()메서드 추가로 ID만 조회Changes
New Files (5)
RedisGeoIndexException.java- Redis 예외 클래스MatchGeoIndexEvent.java- sealed interfaceMatchCreatedEvent.java- 매치 생성 이벤트MatchRemovedFromGeoIndexEvent.java- 매치 제거 이벤트MatchGeoIndexEventListener.java- 트랜잭션 이벤트 리스너Modified Files (12)
MatchGeoIndexPort.java- GeoIndexEntry record, addMatchesBulk(), offset 파라미터MatchRepositoryPort.java- findSearchableMatchIds() 추가SpringDataMatchRepository.java- ID 조회 쿼리 추가MatchJpaAdapter.java- findSearchableMatchIds() 구현MatchGeoRedisAdapter.java- 예외 처리, 벌크 연산, offset 지원MatchCreator/Canceller/Reactivator/StatusUpdater.java- 이벤트 발행으로 전환MatchFinder.java- 페이지네이션 + 순서 보장GeoDataInitializer.java- 벌크 연산 적용GeoDataSyncScheduler.java- ID 조회 + 벌크 연산 적용Test plan
./gradlew test모든 테스트 통과 확인🤖 Generated with Claude Code
Summary by CodeRabbit
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.