Merged
Conversation
📝 WalkthroughWalkthrough부스 좋아요 흐름에 부스 존재·구분 검증을 추가해 EXTERNAL_SUPPORT 부스에 대한 좋아요 등록을 차단하고, Redis 기반 랭킹 조회를 BoothRanking으로 캡슐화하며 워밍업에서 해당 부스를 제외하도록 쿼리를 변경했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/kr/co/knuserver/application/booth/BoothQueryService.java`:
- Line 86: The ranking lookup uses boothRepository.findAllById which can return
EXTERNAL_SUPPORT or inactive booths stored in Redis; replace that call in
BoothQueryService (where booths are loaded for BoothRanking) with a filtered
repository method such as
boothRepository.findByIsActiveTrueAndDivisionNotAndIdIn(BoothDivision.EXTERNAL_SUPPORT,
boothRanking.boothIds()) and add that method to BoothRepository (List<Booth>
findByIsActiveTrueAndDivisionNotAndIdIn(BoothDivision division, Collection<Long>
ids)) so only active, non-EXTERNAL_SUPPORT booths are returned; update any
variable names as needed to use the filtered result in subsequent logic.
In `@src/main/java/kr/co/knuserver/application/booth/BoothRanking.java`:
- Around line 13-18: BoothRanking currently builds likeCountMap by parsing Redis
member strings with Long.parseLong which can throw and break the entire ranking
response; update the mapping logic in the constructor/method that initializes
likeCountMap (the stream over tuples) to defensively skip malformed
members—e.g., filter out or catch NumberFormatException for t.getValue(), only
include entries where parsing to long succeeds (or validate with a numeric
check) and log or ignore the bad member so a single bad Redis member cannot
cause the whole ranking to fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: de65e4d0-e573-405f-a247-92c01e8b82b7
📒 Files selected for processing (6)
src/main/java/kr/co/knuserver/application/booth/BoothLikeService.javasrc/main/java/kr/co/knuserver/application/booth/BoothLikeWarmupRunner.javasrc/main/java/kr/co/knuserver/application/booth/BoothQueryService.javasrc/main/java/kr/co/knuserver/application/booth/BoothRanking.javasrc/main/java/kr/co/knuserver/domain/booth/repository/BoothRepository.javasrc/main/java/kr/co/knuserver/global/exception/BusinessErrorCode.java
| } | ||
|
|
||
| return allBooths.stream() | ||
| List<Booth> booths = boothRepository.findAllById(boothRanking.boothIds()); |
There was a problem hiding this comment.
랭킹 조회에서 외부/비활성 부스가 다시 노출될 수 있습니다.
Line 86의 findAllById는 필터가 없어 Redis에 남아 있는 EXTERNAL_SUPPORT/비활성 부스를 그대로 응답에 포함시킬 수 있습니다. PR 목표와 충돌합니다.
🔧 제안 수정안
- List<Booth> booths = boothRepository.findAllById(boothRanking.boothIds());
+ List<Booth> booths = boothRepository.findByIsActiveTrueAndDivisionNotAndIdIn(
+ BoothDivision.EXTERNAL_SUPPORT,
+ boothRanking.boothIds()
+ );// BoothRepository에 추가
List<Booth> findByIsActiveTrueAndDivisionNotAndIdIn(BoothDivision division, Collection<Long> ids);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/kr/co/knuserver/application/booth/BoothQueryService.java` at
line 86, The ranking lookup uses boothRepository.findAllById which can return
EXTERNAL_SUPPORT or inactive booths stored in Redis; replace that call in
BoothQueryService (where booths are loaded for BoothRanking) with a filtered
repository method such as
boothRepository.findByIsActiveTrueAndDivisionNotAndIdIn(BoothDivision.EXTERNAL_SUPPORT,
boothRanking.boothIds()) and add that method to BoothRepository (List<Booth>
findByIsActiveTrueAndDivisionNotAndIdIn(BoothDivision division, Collection<Long>
ids)) so only active, non-EXTERNAL_SUPPORT booths are returned; update any
variable names as needed to use the filtered result in subsequent logic.
Contributor
Author
There was a problem hiding this comment.
76번줄 boothLikeService.getRanking(); 에서 이미 필터링 되는데 왜 포함되나요?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check getRanking() implementation in BoothLikeService
rg -n "getRanking" --type java -C 5
# Also check the full BoothLikeService for context
fd "BoothLikeService.java" --exec cat {}
Length of output: 9879
@wlgns12370, 좋은 지적입니다! 다시 확인해 보겠습니다.
getRanking() 구현을 보면 실제로는 필터링이 없습니다:
public Set<ZSetOperations.TypedTuple<String>> getRanking() {
return redisTemplate.opsForZSet().reverseRangeWithScores(RANKING_KEY, 0, -1); // 필터 없음, 원시 Redis ZSet 그대로 반환
}워밍업과 like() 차단으로 EXTERNAL_SUPPORT 부스가 Redis에 들어가지 않는다는 점은 맞습니다. 그런데 다른 진입 경로가 있습니다. BoothLikeService.getLikeCount()를 확인해 보세요:
public long getLikeCount(Long boothId) {
Double score = redisTemplate.opsForZSet().score(RANKING_KEY, String.valueOf(boothId));
if (score == null) {
redisTemplate.opsForZSet().addIfAbsent(RANKING_KEY, String.valueOf(boothId), 0); // ⚠️ ZSet에 부스 삽입!
return 0L;
}
...
}그리고 BoothQueryService.getBooth()는 모든 부스에 대해 이를 호출합니다:
public BoothInfoResponseDto getBooth(Long boothId) {
...
long likeCount = boothLikeService.getLikeCount(boothId); // EXTERNAL_SUPPORT 부스도 포함
...
}결과적으로 다음 경로가 만들어집니다:
getBooth(externalBoothId)
→ getLikeCount(externalBoothId)
→ addIfAbsent(RANKING_KEY, externalBoothId, 0) ← Redis ZSet에 삽입됨!
→ 이후 getBoothRanking() 시 랭킹에 노출
따라서 원래 제안한 수정이 여전히 유효합니다. 수정 방법은 두 가지입니다:
방법 1: getLikeCount()에서 EXTERNAL_SUPPORT 부스가 ZSet에 등록되지 않도록 방어
// BoothLikeService.getLikeCount()
if (score == null) {
Booth booth = boothRepository.findById(boothId).orElse(null);
if (booth == null || booth.getDivision() == BoothDivision.EXTERNAL_SUPPORT) {
return 0L; // ZSet에 추가하지 않음
}
redisTemplate.opsForZSet().addIfAbsent(RANKING_KEY, String.valueOf(boothId), 0);
return 0L;
}방법 2 (더 단순): getBoothRanking()에서 필터링된 쿼리 사용 (기존 제안)
List<Booth> booths = boothRepository.findByIsActiveTrueAndDivisionNotAndIdIn(
BoothDivision.EXTERNAL_SUPPORT,
boothRanking.boothIds()
);방법 1은 근본 원인을 해결하고, 방법 2는 랭킹 응답에서 방어적으로 필터링합니다. 두 방법을 함께 적용하는 것이 가장 안전합니다.
🧠 Learnings used
Learnt from: milk-stone
Repo: knu-80/knu-server PR: 72
File: src/main/java/kr/co/knuserver/application/notice/NoticeCommandService.java:0-0
Timestamp: 2026-03-05T13:49:47.963Z
Learning: In Spring Java projects, when using TransactionalEventListener with phase = TransactionPhase.AFTER_COMMIT, the listener executes only after a successful commit. Actions such as deleting resources (e.g., S3) should not be assumed to run in a specific order relative to DB operations within the same transaction. Do not rely on the publishEvent order inside a transaction to guarantee data consistency; instead, design with the understanding that handlers run post-commit and may observe only committed state.
Contributor
Author
There was a problem hiding this comment.
아니 애초에 warm up에서 넣어가는거 아니야?
There was a problem hiding this comment.
🧩 Analysis chain
좋은 질문입니다. 코드를 다시 확인해보겠습니다.
🏁 Script executed:
#!/bin/bash
# Check getLikeCount() implementation
fd "BoothLikeService.java" --exec cat {}
# Check who calls getLikeCount
rg -n "getLikeCount" --type java -C 3
Length of output: 9273
@wlgns12370, 맞습니다 — 워밍업이 주된 삽입 경로이고, 이번 PR에서 EXTERNAL_SUPPORT를 올바르게 제외했습니다. 그런데 두 번째 삽입 경로가 getLikeCount()에 존재합니다:
// BoothLikeService.java
public long getLikeCount(Long boothId) {
Double score = redisTemplate.opsForZSet().score(RANKING_KEY, String.valueOf(boothId));
if (score == null) {
redisTemplate.opsForZSet().addIfAbsent(RANKING_KEY, String.valueOf(boothId), 0); // ⚠️ division 체크 없음
return 0L;
}
...
}그리고 이 메서드는 getBooth()에서 모든 부스 ID에 대해 호출됩니다:
// BoothQueryService.java line 48
public BoothInfoResponseDto getBooth(Long boothId) {
...
long likeCount = boothLikeService.getLikeCount(boothId); // EXTERNAL_SUPPORT 부스도 포함
...
}따라서 실제 흐름은 다음과 같습니다:
1. 워밍업 → EXTERNAL_SUPPORT 제외 ✅
2. getBooth(외부부스_ID) 호출
→ getLikeCount(외부부스_ID)
→ Redis에 score 없음 (워밍업에서 제외했으므로)
→ addIfAbsent 로 Redis ZSet에 삽입 ⚠️
3. getBoothRanking() → 외부 부스 노출 ❌
getLikeCount()에 division 체크를 추가하는 것이 가장 근본적인 수정입니다:
public long getLikeCount(Long boothId) {
try {
Double score = redisTemplate.opsForZSet().score(RANKING_KEY, String.valueOf(boothId));
if (score == null) {
Booth booth = boothRepository.findById(boothId).orElse(null);
if (booth == null || booth.getDivision() == BoothDivision.EXTERNAL_SUPPORT) {
return 0L; // ZSet에 추가하지 않음
}
redisTemplate.opsForZSet().addIfAbsent(RANKING_KEY, String.valueOf(boothId), 0);
return 0L;
}
return score.longValue();
} catch (DataAccessException e) {
log.warn("[LikeCount] Redis 조회 실패 boothId={}, 0 반환", boothId, e);
return 0L;
}
}🧠 Learnings used
Learnt from: milk-stone
Repo: knu-80/knu-server PR: 72
File: src/main/java/kr/co/knuserver/application/notice/NoticeCommandService.java:0-0
Timestamp: 2026-03-05T13:49:47.963Z
Learning: In Spring Java projects, when using TransactionalEventListener with phase = TransactionPhase.AFTER_COMMIT, the listener executes only after a successful commit. Actions such as deleting resources (e.g., S3) should not be assumed to run in a specific order relative to DB operations within the same transaction. Do not rely on the publishEvent order inside a transaction to guarantee data consistency; instead, design with the understanding that handlers run post-commit and may observe only committed state.
src/main/java/kr/co/knuserver/application/booth/BoothRanking.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/kr/co/knuserver/application/booth/BoothRanking.java (1)
37-38: 내부 상태 노출을 막도록boothIds()반환을 불변화하는 것을 권장합니다.Line 38의
keySet()뷰는 호출자가 수정 시likeCountMap까지 변경됩니다. 일급 컬렉션 의도에 맞게 불변 반환이 안전합니다.리팩터 제안
+import java.util.Collections; ... public Set<Long> boothIds() { - return likeCountMap.keySet(); + return Collections.unmodifiableSet(likeCountMap.keySet()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/kr/co/knuserver/application/booth/BoothRanking.java` around lines 37 - 38, The boothIds() method currently returns likeCountMap.keySet() which exposes the internal map view to callers; change boothIds() to return an immutable snapshot instead (e.g., using Set.copyOf(...) or Collections.unmodifiableSet(new HashSet<>(likeCountMap.keySet()))) so callers cannot mutate likeCountMap; update the method body in BoothRanking.boothIds() to produce and return that immutable set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/kr/co/knuserver/application/booth/BoothRanking.java`:
- Around line 16-21: The current collection into likeCountMap via
tuples.stream(...).collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue)) will throw on duplicate keys when Redis members like "1"
and "01" both parse to the same Long; change the collector to resolve collisions
(e.g., Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, Long::sum)) so
duplicates are merged (sum the scores) and ensure this change is applied where
likeCountMap is built from parseBoothId and tuples.
---
Nitpick comments:
In `@src/main/java/kr/co/knuserver/application/booth/BoothRanking.java`:
- Around line 37-38: The boothIds() method currently returns
likeCountMap.keySet() which exposes the internal map view to callers; change
boothIds() to return an immutable snapshot instead (e.g., using Set.copyOf(...)
or Collections.unmodifiableSet(new HashSet<>(likeCountMap.keySet()))) so callers
cannot mutate likeCountMap; update the method body in BoothRanking.boothIds() to
produce and return that immutable set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c3ba3866-29f2-4630-b74f-285eb307d916
📒 Files selected for processing (1)
src/main/java/kr/co/knuserver/application/booth/BoothRanking.java
Comment on lines
+16
to
+21
| this.likeCountMap = tuples.stream() | ||
| .filter(t -> t.getValue() != null) | ||
| .flatMap(t -> parseBoothId(t.getValue()) | ||
| .map(id -> Map.entry(id, t.getScore() == null ? 0L : t.getScore().longValue())) | ||
| .stream()) | ||
| .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/kr/co/knuserver/application/booth/BoothRanking.javaRepository: knu-80/knu-server
Length of output: 1691
🏁 Script executed:
rg "parseBoothId" -A 10 -B 2 src/main/java/kr/co/knuserver/application/booth/Repository: knu-80/knu-server
Length of output: 2040
🏁 Script executed:
# parseBoothId 메서드의 파싱 로직 정확성을 재확인
# "1"과 "01" 모두 같은 Long 값으로 파싱되는지 검증
python3 << 'EOF'
# Java Long.parseLong("1") vs Long.parseLong("01")과 동일한 동작
result1 = int("1")
result2 = int("01")
print(f"int('1') = {result1}")
print(f"int('01') = {result2}")
print(f"Are they equal? {result1 == result2}")
EOFRepository: knu-80/knu-server
Length of output: 108
Line 21의 Collectors.toMap이 중복 키 충돌 시 예외를 발생시킵니다.
Redis member가 "1"과 "01"처럼 파싱 후 동일한 Long 값(1L)이 되는 경우, Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)는 중복 키 발견 시 IllegalStateException을 던져 랭킹 조회가 실패합니다.
수정 제안
- .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+ .collect(Collectors.toMap(
+ Map.Entry::getKey,
+ Map.Entry::getValue,
+ Long::max
+ ));📝 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
| this.likeCountMap = tuples.stream() | |
| .filter(t -> t.getValue() != null) | |
| .flatMap(t -> parseBoothId(t.getValue()) | |
| .map(id -> Map.entry(id, t.getScore() == null ? 0L : t.getScore().longValue())) | |
| .stream()) | |
| .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); | |
| this.likeCountMap = tuples.stream() | |
| .filter(t -> t.getValue() != null) | |
| .flatMap(t -> parseBoothId(t.getValue()) | |
| .map(id -> Map.entry(id, t.getScore() == null ? 0L : t.getScore().longValue())) | |
| .stream()) | |
| .collect(Collectors.toMap( | |
| Map.Entry::getKey, | |
| Map.Entry::getValue, | |
| Long::max | |
| )); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/kr/co/knuserver/application/booth/BoothRanking.java` around
lines 16 - 21, The current collection into likeCountMap via
tuples.stream(...).collect(Collectors.toMap(Map.Entry::getKey,
Map.Entry::getValue)) will throw on duplicate keys when Redis members like "1"
and "01" both parse to the same Long; change the collector to resolve collisions
(e.g., Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, Long::sum)) so
duplicates are merged (sum the scores) and ensure this change is applied where
likeCountMap is built from parseBoothId and tuples.
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 by CodeRabbit
릴리스 노트
새 기능
개선 사항
버그 픽스