fix: 게시글 중복 생성 방지 #649
fix: 게시글 중복 생성 방지 #649
Conversation
- Redis 패키지 및 로직 정리
- 게시글 중복 요청 방지 Redis 로직 추가
JAEHEE25
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough이 변경사항은 Redis 기반의 포스트 캐싱 로직을 중앙화하는 리팩토링입니다. 새로운 PostRedisManager 컴포넌트가 도입되어 포스트 생성 중복 방지, 조회수 캐싱, Redis 키 관리를 담당하게 됩니다.
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/community/post/service/PostCommandService.java (1)
46-68:⚠️ Potential issue | 🟠 Major중복 방지 락이 유효성 검증보다 먼저 설정되어, 검증 실패 시 사용자가 불필요하게 잠길 수 있습니다.
현재 흐름:
- 사용자 조회 ✅
- Redis 락 획득 (Line 52) ← 여기서 락 설정
- 카테고리 유효성 검증 (Line 57)
- 이미지 개수 검증 (Line 58)
- DB 저장 (Line 66)
만약 3번이나 4번에서 예외가 발생하면, 락은 이미 설정된 상태이므로 사용자가 입력을 수정해 재시도하더라도 TTL(5초)이 만료될 때까지
DUPLICATE_POST_CREATE_REQUEST오류를 받게 됩니다.락 획득 시점을 유효성 검증 이후, DB 저장 직전으로 이동시키는 것을 권장합니다.
🔧 락 획득 시점 이동 제안
public PostCreateResponse createPost(long siteUserId, PostCreateRequest postCreateRequest, List<MultipartFile> imageFile) { SiteUser siteUser = siteUserRepository.findById(siteUserId) .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); - // 중복 생성 방지 - if (!postRedisManager.isPostCreationAllowed(siteUserId)) { - throw new CustomException(DUPLICATE_POST_CREATE_REQUEST); - } - // 유효성 검증 validatePostCategory(postCreateRequest.postCategory()); validateFileSize(imageFile); + // 중복 생성 방지 + if (!postRedisManager.isPostCreationAllowed(siteUserId)) { + throw new CustomException(DUPLICATE_POST_CREATE_REQUEST); + } + // 객체 생성 Board board = boardRepository.getByCode(postCreateRequest.boardCode()); Post post = postCreateRequest.toEntity(siteUser, board);
🤖 Fix all issues with AI agents
In
`@src/main/java/com/example/solidconnection/community/post/service/UpdateViewCountService.java`:
- Around line 23-28: Remove the unnecessary DB fetch and add null-safety: in
updateViewCount, drop the unused postRepository.getById(postId) call and instead
call postRedisManager.getAndDeleteViewCount(key) into a Long viewCount variable,
then guard against null (e.g., if viewCount == null || viewCount == 0 return; or
treat null as 0) before calling postRepository.increaseViewCount(postId,
viewCount); ensure you still obtain postId via
postRedisManager.getPostIdFromPostViewCountRedisKey(key).
In `@src/main/java/com/example/solidconnection/redis/RedisService.java`:
- Around line 3-4: Remove the unused static import VALIDATE_VIEW_COUNT_TTL from
RedisService.java; keep the existing static import for VIEW_COUNT_TTL (used as
VIEW_COUNT_TTL.getValue()) and ensure no other references to
VALIDATE_VIEW_COUNT_TTL remain in the class (remove any accidental leftover
usages if present).
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/redis/RedisService.java (1)
40-43:isPresent메서드 네이밍이 실제 동작과 반대입니다.
setIfAbsent는 키가 없을 때 값을 설정하고true를 반환합니다. 즉,isPresent가true를 반환하는 경우는 키가 존재하지 않았던 상황입니다. 메서드 이름이 실제 의미와 반대여서 호출부에서 혼동을 일으킬 수 있습니다.
tryAcquire또는setIfAbsent와 같이 실제 동작을 반영하는 이름으로 변경하면 가독성이 크게 개선됩니다.ttl파라미터를String대신long으로 받으면 호출부마다 파싱할 필요가 없어집니다.♻️ 네이밍 및 파라미터 타입 개선 제안
- public boolean isPresent(String key, String ttl) { - return Boolean.TRUE.equals(redisTemplate.opsForValue() - .setIfAbsent(key, "1", Long.parseLong(ttl), TimeUnit.SECONDS)); + public boolean tryAcquire(String key, long ttlSeconds) { + return Boolean.TRUE.equals(redisTemplate.opsForValue() + .setIfAbsent(key, "1", ttlSeconds, TimeUnit.SECONDS)); }src/test/java/com/example/solidconnection/community/post/service/PostCommandServiceTest.java (1)
265-270: Redis 락 시나리오에 대한 단위 테스트 추가를 추천합니다.현재 테스트 구조를 보면 다음과 같이 정리됩니다.
동시성 테스트의 범위
PostCreateConcurrencyTest는 여러 스레드가 동시에 접근할 때의 동작만 검증하고 있습니다.단위 테스트 레벨의 갭
PostCommandServiceTest의 게시글 생성 테스트들(게시글을_성공적으로_생성한다등)은 정상 경로만 다루고 있으며, Redis 락이 이미 설정된 상태에서DUPLICATE_POST_CREATE_REQUEST예외가 발생하는 시나리오는 없습니다.제안 사항
단위 테스트에서PostRedisManager의isPostCreationAllowed()메서드가false를 반환하는 상황을 직접 시뮬레이션하여,CustomException이 올바르게 던져지는지 검증하는 테스트를 추가하면 좋겠습니다. 이를 통해 비즈니스 로직의 안정성을 더욱 높일 수 있습니다.src/test/java/com/example/solidconnection/concurrency/PostViewCountConcurrencyTest.java (1)
119-146: 기존 코드 관련 참고사항:CountDownLatch사용에 잠재적 문제가 있습니다.이번 PR 변경 범위는 아니지만, 참고로 알려드립니다.
doneSignal이THREAD_NUMS(1000)으로 초기화되었는데, 첫 번째 루프(Line 122-133)에서 1000개 태스크가countDown()을 모두 호출한 뒤, 두 번째 루프(Line 135-146)의countDown()은 이미 0인 latch에 대해 동작하지 않습니다. 따라서 Line 148의await()는 두 번째 배치 완료를 보장하지 않습니다.현재는 Line 155의
Thread.sleep이 충분한 버퍼를 제공하여 동작하지만,CountDownLatch를2 * THREAD_NUMS로 초기화하는 것이 더 정확합니다.src/main/java/com/example/solidconnection/community/post/service/PostRedisManager.java (1)
19-21:getPostIdFromPostViewCountRedisKey: 잘못된 키 입력 시 예외 처리 부재
key.substring(...)후Long.parseLong를 바로 호출하고 있어, prefix가 일치하지 않는 키가 들어오면NumberFormatException이 발생합니다. 현재 호출부(UpdateViewCountScheduler)에서 패턴 매칭된 키만 전달하므로 당장 문제는 없지만, 방어적 코딩 관점에서 참고해 주세요.
| public void updateViewCount(String key) { | ||
| Long postId = redisUtils.getPostIdFromPostViewCountRedisKey(key); | ||
| Long postId = postRedisManager.getPostIdFromPostViewCountRedisKey(key); | ||
| Post post = postRepository.getById(postId); | ||
| postRepository.increaseViewCount(postId, redisService.getAndDelete(key)); | ||
| Long viewCount = postRedisManager.getAndDeleteViewCount(key); | ||
| postRepository.increaseViewCount(postId, viewCount); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the UpdateViewCountService file
find . -name "UpdateViewCountService.java" -type fRepository: solid-connection/solid-connect-server
Length of output: 175
🏁 Script executed:
# If found, read the full file with line numbers
cat -n src/main/java/com/example/solidconnection/community/post/service/UpdateViewCountService.java 2>/dev/null || echo "File not found, searching for it..."Repository: solid-connection/solid-connect-server
Length of output: 1331
🏁 Script executed:
# Search for PostRedisManager to understand getAndDeleteViewCount
rg -n 'class PostRedisManager' -A 50 --type=javaRepository: solid-connection/solid-connect-server
Length of output: 5607
🏁 Script executed:
# Search for getAndDeleteViewCount implementation
rg -n 'getAndDeleteViewCount' --type=java -B 3 -A 10Repository: solid-connection/solid-connect-server
Length of output: 2848
🏁 Script executed:
# Search for increaseViewCount implementation
rg -n 'increaseViewCount' --type=java -B 3 -A 10Repository: solid-connection/solid-connect-server
Length of output: 13808
불필요한 DB 조회 제거 및 null 안전성 처리가 필요합니다.
아래 두 가지 개선 사항을 적용하세요:
-
미사용 엔티티 조회 제거 (라인 25):
Post post = postRepository.getById(postId);로 조회한 엔티티가 이후에 사용되지 않습니다. 불필요한 DB 쿼리이므로 제거하세요. -
Null 안전성 처리 (라인 26–27):
getAndDeleteViewCount(key)는 Redis 키가 만료되거나 존재하지 않을 경우 null을 반환할 수 있습니다. 이 값이 null일 때increaseViewCount()에 전달되면 SQL 쿼리가 예상치 못한 동작을 할 수 있으므로, null 체크를 추가하세요.
수정 제안
`@Transactional`
`@Async`
public void updateViewCount(String key) {
Long postId = postRedisManager.getPostIdFromPostViewCountRedisKey(key);
- Post post = postRepository.getById(postId);
Long viewCount = postRedisManager.getAndDeleteViewCount(key);
- postRepository.increaseViewCount(postId, viewCount);
+ if (viewCount != null && viewCount > 0) {
+ postRepository.increaseViewCount(postId, viewCount);
+ }
}📝 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.
| public void updateViewCount(String key) { | |
| Long postId = redisUtils.getPostIdFromPostViewCountRedisKey(key); | |
| Long postId = postRedisManager.getPostIdFromPostViewCountRedisKey(key); | |
| Post post = postRepository.getById(postId); | |
| postRepository.increaseViewCount(postId, redisService.getAndDelete(key)); | |
| Long viewCount = postRedisManager.getAndDeleteViewCount(key); | |
| postRepository.increaseViewCount(postId, viewCount); | |
| } | |
| public void updateViewCount(String key) { | |
| Long postId = postRedisManager.getPostIdFromPostViewCountRedisKey(key); | |
| Long viewCount = postRedisManager.getAndDeleteViewCount(key); | |
| if (viewCount != null && viewCount > 0) { | |
| postRepository.increaseViewCount(postId, viewCount); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/example/solidconnection/community/post/service/UpdateViewCountService.java`
around lines 23 - 28, Remove the unnecessary DB fetch and add null-safety: in
updateViewCount, drop the unused postRepository.getById(postId) call and instead
call postRedisManager.getAndDeleteViewCount(key) into a Long viewCount variable,
then guard against null (e.g., if viewCount == null || viewCount == 0 return; or
treat null as 0) before calling postRepository.increaseViewCount(postId,
viewCount); ensure you still obtain postId via
postRedisManager.getPostIdFromPostViewCountRedisKey(key).
| import static com.example.solidconnection.redis.RedisConstants.VALIDATE_VIEW_COUNT_TTL; | ||
| import static com.example.solidconnection.redis.RedisConstants.VIEW_COUNT_TTL; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# RedisService.java에서 두 상수의 실제 사용 여부 확인
echo "=== VALIDATE_VIEW_COUNT_TTL 사용 확인 ==="
fd "RedisService.java" --type f -x grep -n "VALIDATE_VIEW_COUNT_TTL" {}
echo -e "\n=== VIEW_COUNT_TTL 사용 확인 ==="
fd "RedisService.java" --type f -x grep -n "VIEW_COUNT_TTL" {}
echo -e "\n=== 전체 import 문 확인 ==="
fd "RedisService.java" --type f -x head -20 {}Repository: solid-connection/solid-connect-server
Length of output: 1325
VALIDATE_VIEW_COUNT_TTL 미사용 import 제거 필요합니다.
확인 결과:
-
VALIDATE_VIEW_COUNT_TTL- 파일 내에서 실제로 사용되지 않으며 import 문에만 존재합니다. 제거해야 합니다. -
VIEW_COUNT_TTL- 29번째 줄에서VIEW_COUNT_TTL.getValue()형태로 활발히 사용 중입니다. 이 import는 유지해야 합니다.
3번째 줄의 VALIDATE_VIEW_COUNT_TTL import를 삭제하시기 바랍니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/example/solidconnection/redis/RedisService.java` around
lines 3 - 4, Remove the unused static import VALIDATE_VIEW_COUNT_TTL from
RedisService.java; keep the existing static import for VIEW_COUNT_TTL (used as
VIEW_COUNT_TTL.getValue()) and ensure no other references to
VALIDATE_VIEW_COUNT_TTL remain in the class (remove any accidental leftover
usages if present).
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
🔍 PR #649 코드 리뷰
보안, 성능, 테스트 3가지 관점에서 리뷰했습니다.
🔴 수정 필요
1. [보안] 락 획득 시점 문제
PostCommandService.createPost()에서 유효성 검증(카테고리, 이미지) 전에 Redis 락을 걸고 있습니다. 검증 실패해도 5초간 재요청이 차단됩니다.
권장: 유효성 검증을 먼저 수행한 후 락을 획득하거나, 검증 실패 시 Redis 키를 삭제하는 rollback 로직 추가.
2. [보안] Redis 장애 시 fallback 부재
Redis 다운 시 RedisConnectionFailureException이 전파되어 게시글 생성 자체가 불가능해집니다. 중복방지는 부가기능인데 핵심기능을 막는 것은 가용성 문제입니다.
권장: try-catch로 감싸서 Redis 장애 시 락 없이 생성 허용 (graceful degradation).
public boolean isPostCreationAllowed(Long siteUserId) {
try {
String key = getPostCreateRedisKey(siteUserId);
return redisService.isPresent(key, VALIDATE_POST_CREATE_TTL.getValue());
} catch (Exception e) {
log.warn("Redis 장애로 중복 생성 방지 비활성화", e);
return true;
}
}3. [성능] RedisService.getAndDelete() NPE 위험
key가 존재하지 않으면 getAndDelete()가 null을 반환하고, Long.valueOf(null) → NumberFormatException 발생. 스케줄러에서 호출되므로 타이밍 이슈로 발생 가능합니다.
권장: null 체크 추가.
public Long getAndDelete(String key) {
String value = redisTemplate.opsForValue().getAndDelete(key);
return value != null ? Long.valueOf(value) : null;
}4. [성능] UpdateViewCountService 불필요한 DB 조회
postRepository.getById(postId) 결과를 사용하지 않습니다. increaseViewCount()는 JPQL UPDATE로 postId만 필요합니다. key 수만큼 불필요한 SELECT가 매 스케줄링마다 발생합니다.
5. [테스트] PostCommandServiceTest에 중복 생성 테스트 없음
핵심 기능인 DUPLICATE_POST_CREATE_REQUEST 예외 발생 단위 테스트가 누락되어 있습니다.
추가 필요한 테스트:
- 동일 사용자 5초 내 중복
createPost()호출 시 예외 발생 - 게시글 생성 실패(유효성 검증 실패) 후 Redis 키 잔존으로 인한 5초 차단 시나리오
🟡 개선 권장
6. [성능] KEYS 명령 사용
RedisUtils.getKeysOrderByExpiration()에서 O(N) 블로킹 명령인 KEYS를 사용합니다. 게시글 수가 많아지면 Redis 전체 성능에 영향을 줄 수 있으므로 SCAN 명령 기반으로 변경을 권장합니다.
7. [테스트] PostViewCountConcurrencyTest CountDownLatch 버그
조회수_조작_문제를_해결한다 테스트에서 CountDownLatch가 첫 번째 루프에서 이미 0이 되어, 두 번째 루프의 완료를 기다리지 않습니다. flaky test가 될 수 있습니다.
8. [보안] isPresent 메서드 네이밍
isPresent는 "이미 존재하는가"를 의미하지만, 실제로는 setIfAbsent(키가 없을 때 true)를 반환합니다. 유지보수 시 혼란을 줄 수 있으므로 네이밍 개선을 권장합니다.
✅ 양호한 부분
- SETNX + TTL 원자성 보장 (단일 Redis 명령)
- Lua 스크립트 기반 조회수 증가 (동시성 안전)
- Redis 키 패턴 보안성 (서버측 인증된 userId 사용)
- 에러 메시지에 내부 구현 미노출
🤖 Generated with Claude Code Agent Teams (보안/성능/테스트 3명 병렬 리뷰)
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class PostRedisManager { |
There was a problem hiding this comment.
흩어진 Post 관련 레디스 메서드 한 곳으로 통합하는 거 좋은 거 같아요 !
3 participants
관련 이슈
작업 내용
1. redis 패키지 분리
2. 게시글 생성 중복 요청 문제 해결
포스트맨으로 iteration 3, delay 0으로 테스트했을 때
특이 사항
리뷰 요구사항 (선택)