Conversation
(cherry picked from commit 721a695)
(cherry picked from commit 8bb5011)
(cherry picked from commit e7a7c1d)
(cherry picked from commit 564cc52)
(cherry picked from commit 2e49914)
(cherry picked from commit 6a461a9)
(cherry picked from commit 7d89cc9)
(cherry picked from commit ba95d62)
(cherry picked from commit 491bbf0)
(cherry picked from commit ec91ca4)
(cherry picked from commit b90a4f5)
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Redis 인프라 기초 build.gradle |
Redisson Spring Boot Starter 3.52.0 의존성 추가 |
Redis 구성 src/main/java/OneQ/OnSurvey/global/common/config/RedisConfig.java |
RedissonClient 빈 설정으로 단일 Redis 서버 연결 구성 |
Redis 인터페이스 정의 src/main/java/OneQ/OnSurvey/global/infra/redis/RedisLockAction.java, src/main/java/OneQ/OnSurvey/global/infra/redis/RedisCacheAction.java |
분산 잠금 및 캐시 작업을 위한 계약 인터페이스 정의 |
Redis 추상화 구현 src/main/java/OneQ/OnSurvey/global/infra/redis/RedisAgent.java |
Redisson과 Spring Redis를 통합한 통일된 Redis 작업 인터페이스 제공 (잠금, KV, ZSet 작업) |
거래 처리 src/main/java/OneQ/OnSurvey/global/infra/transaction/TransactionHandler.java |
REQUIRES_NEW 전파 수준으로 새 트랜잭션 컨텍스트 내에서 작업 실행 |
답변 처리 메서드 서명 src/main/java/OneQ/OnSurvey/domain/participation/service/answer/AnswerCommand.java, src/main/java/OneQ/OnSurvey/domain/participation/service/answer/AnswerCommandService.java, src/main/java/OneQ/OnSurvey/domain/survey/controller/ParticipationController.java |
upsertAnswers 메서드에 surveyId, userKey, memberId 파라미터 추가 |
Redis 기반 답변 Upsert src/main/java/OneQ/OnSurvey/domain/participation/service/answer/QuestionAnswerCommandService.java |
RedisAgent를 활용한 분산 잠금으로 답변 작업 보호; QuestionRepository 제거; updateResponseAfterQuestionAnswers 서명 변경 |
Redis 기반 응답 생성 src/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.java |
RedisAgent를 통한 트랜잭션 기반 잠금으로 응답 생성 프로세스 리팩토링; StringRedisTemplate 제거 |
설문 상태 및 에러 처리 src/main/java/OneQ/OnSurvey/domain/survey/entity/SurveyInfo.java, src/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.java, src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepository.java, src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.java, src/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommand.java |
updateCompletedCount 메서드 추가; SURVEY_PARTICIPATION_IN_PROCESS 에러 코드 추가; closeDueSurveys 메서드 추가 |
설문 쿼리 서비스 Redis 통합 src/main/java/OneQ/OnSurvey/domain/survey/service/query/SurveyQueryService.java |
StringRedisTemplate을 RedisAgent로 완전히 대체; 분산 잠금 기반 참여 활성화; Redis 예외 처리 강화 |
설문 명령 서비스 Redis 통합 src/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommandService.java |
RedisAgent 기반 heartbeat 및 캐시 적용; 자정 실행 스케줄링으로 기한 경과 설문 자동 종료; TTL 검증 로직 추가 |
설문 통계 서비스 Redis 통합 src/main/java/OneQ/OnSurvey/domain/survey/service/SurveyGlobalStatsService.java |
StringRedisTemplate을 RedisAgent로 대체; 초기화 흐름 변경 (orElse → orElseGet); 일일 사용자 카운트 및 만료 처리 개선 |
인증 및 토큰 Redis 통합 src/main/java/OneQ/OnSurvey/global/auth/application/TossAuthFacade.java, src/main/java/OneQ/OnSurvey/global/auth/token/TokenStore.java, src/main/java/OneQ/OnSurvey/domain/admin/application/AdminFacade.java |
RedisAgent를 통한 일일 사용자 추적 및 토큰 잠금 관리; 공개 유틸리티 메서드 제거; AdminFacade에 @Transactional 추가 |
Sequence Diagram(s)
sequenceDiagram
participant Client as 참여자<br/>(Client)
participant Controller as ParticipationController
participant QAService as QuestionAnswerCommandService
participant RedisAgent as RedisAgent
participant TxHandler as TransactionHandler
participant DB as Database
Client->>Controller: createQuestionAnswer<br/>(surveyId, userKey, memberId, ...)
Controller->>QAService: upsertAnswers<br/>(insertDto, surveyId, userKey, memberId)
QAService->>RedisAgent: executeWithLock<br/>(lockKey, ...)
Note over RedisAgent: 잠금 획득 시도<br/>survey:{surveyId}:user:{userKey}
alt 잠금 획득 성공
RedisAgent->>TxHandler: runInTransaction(...)
TxHandler->>DB: 기존 답변 조회
DB-->>TxHandler: 기존 답변 목록
TxHandler->>DB: 새 답변 생성/업데이트
TxHandler->>DB: 응답 상태 업데이트
DB-->>TxHandler: 성공
TxHandler-->>RedisAgent: 결과
RedisAgent->>RedisAgent: 잠금 해제
RedisAgent-->>QAService: 성공
QAService-->>Controller: true
else 잠금 획득 실패
RedisAgent-->>QAService: RedisException<br/>SURVEY_PARTICIPATION_IN_PROCESS
QAService-->>Controller: CustomException
end
Controller-->>Client: 응답 또는 에러
sequenceDiagram
participant Client as 참여자<br/>(Client)
participant Controller as ParticipationController
participant ResponseService as ResponseCommandService
participant RedisAgent as RedisAgent
participant TxHandler as TransactionHandler
participant DB as Database
participant Redis as Redis ZSet
Client->>Controller: participateSurvey<br/>(surveyId, userKey)
Controller->>ResponseService: createResponse<br/>(surveyId, userKey, memberId)
ResponseService->>RedisAgent: executeWithLock<br/>(lockKey, ...)
Note over RedisAgent: 잠금 획득<br/>survey:{surveyId}:user:{userKey}
alt 잠금 획득 성공
RedisAgent->>TxHandler: runInTransaction(...)
TxHandler->>DB: 응답 존재 여부 조회
alt 응답 미존재
TxHandler->>DB: 응답 생성
TxHandler->>DB: SurveyInfo 완료도 증가
TxHandler->>Redis: 완료 카운트 증가
alt 완료도 == 목표도
TxHandler->>DB: 설문 상태 CLOSED
TxHandler->>Redis: 관련 키 삭제
end
end
DB-->>TxHandler: 성공
TxHandler-->>RedisAgent: 결과
RedisAgent->>RedisAgent: 잠금 해제
RedisAgent-->>ResponseService: 성공
else 잠금 실패 또는 중단
RedisAgent-->>ResponseService: RedisException 또는<br/>InterruptedException
ResponseService-->>Controller: CustomException
end
ResponseService-->>Controller: 응답
Controller-->>Client: 결과
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
- ✨ OMF-32 현재 접속 중인 고객 수 카운트 #87: SurveyGlobalStatsService 및 TossAuthFacade에서 StringRedisTemplate을 RedisAgent로 대체하는 동일한 Redis 통합 작업
- ✨ OMF-132 Redis 분산락 적용 및 사용성 업데이트 #112: build.gradle에 Redisson 추가 및 전반적인 StringRedisTemplate을 중앙화된 Redis 추상화로 교체하는 관련 인프라 변경
- ✨ OMF-60 설문 응답자가 목표인원을 넘지 않도록 카운트 기능 추가 #82: 참여 흐름 리팩토링 및 userKey 파라미터 추가, Redis 기반 참여자 카운팅 도입
Suggested labels
♻️refactor, 🌟feature
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 25.30% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | PR 설명이 템플릿의 필수 섹션(Related Issue, Review Requirements)을 포함하고 있으나, Task Details 섹션이 누락되어 있습니다. | Task Details 섹션을 추가하여 구현된 작업 항목들을 체크리스트로 명시해 주세요. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 제목이 PR의 주요 변경 사항인 Redis 분산락 도입 및 사용성 개선을 명확하게 요약하고 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/OMF-132
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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/OneQ/OnSurvey/domain/participation/service/answer/AnswerCommand.java`:
- Line 7: The public API method AnswerCommand.upsertAnswers(AnswerInsertDto
insertDto, Long surveyId, Long userKey, Long memberId) is unsafe because three
consecutive Long parameters are easy to swap; create a single explicit
command/value object (e.g., AnswerUpsertCommand or AnswerUpsertContext) that
bundles insertDto, surveyId, userKey, and memberId and change the method
signature to accept that object instead of three Longs; update callers to
construct and pass the new command object and adjust any locking/authorization
code that references surveyId/userKey/memberId to read from the command to
restore type safety.
In
`@src/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.java`:
- Around line 47-81: The current transaction lambda in ResponseCommandService
executes markResponded() without verifying the survey is still active; fetch the
Survey (use surveyRepository.getSurveyById(surveyId) or surveyInfoRepository) at
the start of the lambda and validate its status is SurveyStatus.ONGOING and its
deadline is after Instant.now() (or now()); if the checks fail throw an
appropriate CustomException (e.g., a SURVEY_EXPIRED or SURVEY_NOT_ONGOING error
code) before calling response.markResponded(), responseRepository.save(...), or
updateCounter(...), so expired/closed surveys are rejected and not counted.
- Around line 47-48: The current call into executeNewTransactionAfterLock uses a
hardcoded 5s leaseTime which disables Redisson's watchdog and can cause
premature lock release; change the call so that executeNewTransactionAfterLock
(and its usage of RLock.tryLock) either omits the leaseTime to enable the
watchdog or reads leaseTime from configuration (e.g., a property) and passes
that value instead of the literal 5; update the code paths around
surveyLockKeyPrefix + surveyId + ":" + userKey and the
executeNewTransactionAfterLock implementation to accept/configure a variable
leaseTime or the no-lease overload so the lock renewal is handled safely.
In `@src/main/java/OneQ/OnSurvey/domain/survey/entity/SurveyInfo.java`:
- Around line 113-115: updateCompletedCount에서 completedCount를 절대값으로 덮어써서 트랜잭션 간
역전 커밋(race) 가능성이 있으니 수정하세요: SurveyInfo.updateCompletedCount 메서드는 단순 세터로 사용하지 말고
설문 단위 직렬화 또는 DB 원자 업데이트(예: JPA `@Modifying` JPQL로 "update survey_info set
completed_count = completed_count + :delta" 같은 증감 쿼리) 또는 엔티티 레벨 낙관적 락(`@Version`)
중 하나로 바꿔 처리하도록 변경합니다; 호출 위치인 ResponseCommandService의 해당 트랜잭션 경계도 검토해 surveyId 기준
락으로 전환하거나 repository의 증가 전용 메서드(예: incrementCompletedCount(surveyId, delta))를
만들어 사용하도록 리팩토링하세요.
In `@src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepository.java`:
- Line 36: The current void signature of SurveyRepository.closeDueSurveys()
prevents the caller from knowing which surveys were transitioned to CLOSED
(making Redis cleanup impossible); change the API so closeDueSurveys() returns
the collection of closed survey IDs (e.g., List<Long> or Set<Long>) from the
repository implementation (SurveyRepositoryImpl.closeDueSurveys()) and propagate
that return type to callers so the service can iterate and remove per-survey
Redis keys — alternatively, provide a new repository method that performs
lookup+close+redis-cleanup atomically if you prefer to encapsulate cleanup
inside the repository.
In
`@src/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommandService.java`:
- Around line 343-346: The ZSet written via
redisAgent.addToZSet(this.potentialKey + surveyId, ...) is not assigned the same
deadline TTL as the other keys (dueCountKey, completedKey, creatorKey), so stale
entries persist and can make sendSurveyHeartbeat() return true; fix by ensuring
the ZSet key gets the same TTL after adding members (call the Redis TTL helper
e.g. redisAgent.setTTL(this.potentialKey + surveyId, ttl) or an equivalent
expiry method immediately after redisAgent.addToZSet), and/or update the survey
cleanup path to explicitly delete this.potentialKey + surveyId when a survey
expires; reference the symbols potentialKey, addToZSet, setValue, dueCountKey,
completedKey, creatorKey and sendSurveyHeartbeat when locating and applying the
change.
In
`@src/main/java/OneQ/OnSurvey/domain/survey/service/query/SurveyQueryService.java`:
- Around line 416-421: The finally block currently catches Exception as `ignore`
and passes the exception object to log.warn, which causes a stack trace to be
logged; change the log to only log the exception message instead (e.g.,
log.warn("[SURVEY:QUERY] 만료된 참여자 정리 중 오류 발생: {}", ignore.getMessage())) so that
redisAgent.rangeRemoveFromZSet(potentialKey, 0, System.currentTimeMillis() -
potentialDuration.toMillis()) still has its error noted without printing a full
stack trace; update the catch to use the same variable name (`ignore`) and call
getMessage() in the log.warn call.
- Around line 449-460: The setDueCount method can compute a negative Duration
when survey.getDeadline() is in the past; add a defensive check after Duration
duration = Duration.between(LocalDateTime.now(), survey.getDeadline()) to handle
negative or zero durations: if duration.isNegative() || duration.isZero(), do
not call redisAgent.setValue with a negative TTL (either skip the
redisAgent.setValue call and just return surveyInfo.getDueCount(), or call
redisAgent.setValue without an expiry), otherwise call
redisAgent.setValue(this.dueCountKey + surveyId,
String.valueOf(surveyInfo.getDueCount()), duration) as before; update logic in
setDueCount to use survey.getDeadline(), Duration.between(...),
duration.isNegative()/isZero(), redisAgent.setValue, dueCountKey and
surveyInfo.getDueCount references.
In `@src/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.java`:
- Line 20: The error message for the enum constant
SURVEY_PARTICIPATION_IN_PROCESS in SurveyErrorCode is too specific to "제출한 설문
응답" but this code is reused for broader participation processing/lock conflicts;
update the message text for SURVEY_PARTICIPATION_IN_PROCESS to a more general
phrase (e.g., "설문 참여 처리가 진행 중입니다." or "참여 처리가 진행 중입니다.") so it accurately covers
all participation-processing/lock conflict scenarios and keep the error code and
HttpStatus.CONFLICT unchanged.
In `@src/main/java/OneQ/OnSurvey/global/auth/token/TokenStore.java`:
- Around line 16-21: 현재 TokenStore.acquireLock에서
redisAgent.setValueIfAbsent(key, "1", ttl)로 고정 값만 저장하고 releaseLock에서
redisAgent.deleteKeys(List.of(key))로 바로 삭제하기 때문에 이전 소유자가 다른 워커의 락까지 해제할 수 있습니다;
수정 방법: acquireLock에서 고유한 랜덤 토큰(예: UUID)을 값으로 저장하고 반환(혹은 내부 맵에 저장)한 뒤
releaseLock에서 단순 삭제 대신 compare-and-delete(서버 사이드 Lua 스크립트 또는 redisAgent의
conditional delete)를 사용해 키의 값이 동일할 때만 삭제하도록 하거나, 대안으로 TokenStore에서 Redisson의
RLock API(RLock.lock()/RLock.unlock())로 통일하여 락 소유자 검증을 위임하세요; 관련 식별자:
TokenStore.acquireLock, TokenStore.releaseLock, redisAgent.setValueIfAbsent,
redisAgent.deleteKeys, RLock.
In `@src/main/java/OneQ/OnSurvey/global/common/config/RedisConfig.java`:
- Around line 14-24: In RedisConfig.redisson(RedisProperties) the Redisson
Config only uses host/port/password; update this method to fully map
RedisProperties into Redisson configuration: if redisProperties.getSentinel() is
populated use config.useSentinelServers(), if redisProperties.getCluster() is
populated use config.useClusterServers(), otherwise use
config.useSingleServer(); prefer redisProperties.getUrl() when present (parse
scheme/host/port) and honor redisProperties.isSsl() (use rediss/TLS), set
database (setDatabase) and username (setUsername) on the appropriate
ServerConfig (SingleServerConfig/SentinelServersConfig/ClusterServersConfig
where supported), and keep setting password; ensure the final Redisson config
mirrors the same properties StringRedisTemplate/RedisConnectionFactory would use
so both clients connect to the same Redis topology and credentials.
In `@src/main/java/OneQ/OnSurvey/global/infra/redis/RedisAgent.java`:
- Around line 149-151: The setValue method lacks validation for the ttl
parameter and can pass zero/negative Durations to
redisTemplate.opsForValue().set, so add defensive validation inside
RedisAgent.setValue: check that the ttl is non-null and
ttl.compareTo(Duration.ZERO) > 0 and only call
redisTemplate.opsForValue().set(key, value, ttl) when valid; otherwise call the
no-ttl overload (redisTemplate.opsForValue().set(key, value)) or throw/handle
according to project convention. Update or note callers such as
SurveyQueryService.setDueCount (which currently calls setValue without
validation) so they rely on the new defensive behavior rather than assuming
pre-validation done in SurveyCommandService.applySurveyRuntimeCache.
- Line 29: 필드명 camelCase 규칙 위반으로 RedisAgent 클래스의 private final
TransactionHandler transactionhandler를 transactionHandler로 변경하고, 해당 필드를 참조하는 모든
사용처(예: 생성자 파라미터, 멤버 접근, 메서드 내부 참조—특히 RedisAgent 내에서 transactionhandler를 사용하는
부분)도 함께 transactionHandler로 일관되게 수정하세요; 또한 필요한 경우 관련 import나 getter/setter
이름(있다면)도 같은 규칙으로 업데이트해 컴파일 에러가 발생하지 않도록 하십시오.
In `@src/main/java/OneQ/OnSurvey/global/infra/redis/RedisLockAction.java`:
- Around line 8-13: The RedisLockAction interface doesn't capture the real usage
pattern because callers need executeNewTransactionAfterLock from the concrete
RedisAgent; update the abstraction so callers no longer depend on RedisAgent:
either add a method signature like <R> R executeNewTransactionAfterLock(String
lockKey, long waitTime, long leaseTime, Supplier<R> action) throws
InterruptedException, RedisException to RedisLockAction, or extract transaction
responsibility into a new interface (e.g., TransactionalLockAction) and make
RedisAgent implement it; update call sites to depend on the interface
(RedisLockAction or the new TransactionalLockAction) instead of RedisAgent to
restore proper abstraction and testability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f211d2e0-5441-46a3-abd1-13328ff9e4d2
📒 Files selected for processing (22)
build.gradlesrc/main/java/OneQ/OnSurvey/domain/admin/application/AdminFacade.javasrc/main/java/OneQ/OnSurvey/domain/participation/service/answer/AnswerCommand.javasrc/main/java/OneQ/OnSurvey/domain/participation/service/answer/AnswerCommandService.javasrc/main/java/OneQ/OnSurvey/domain/participation/service/answer/QuestionAnswerCommandService.javasrc/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/SurveyErrorCode.javasrc/main/java/OneQ/OnSurvey/domain/survey/controller/ParticipationController.javasrc/main/java/OneQ/OnSurvey/domain/survey/entity/SurveyInfo.javasrc/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepository.javasrc/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/SurveyGlobalStatsService.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommand.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/command/SurveyCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/service/query/SurveyQueryService.javasrc/main/java/OneQ/OnSurvey/global/auth/application/TossAuthFacade.javasrc/main/java/OneQ/OnSurvey/global/auth/token/TokenStore.javasrc/main/java/OneQ/OnSurvey/global/common/config/RedisConfig.javasrc/main/java/OneQ/OnSurvey/global/infra/redis/RedisAgent.javasrc/main/java/OneQ/OnSurvey/global/infra/redis/RedisCacheAction.javasrc/main/java/OneQ/OnSurvey/global/infra/redis/RedisLockAction.javasrc/main/java/OneQ/OnSurvey/global/infra/transaction/TransactionHandler.java
✨ Related Issue
💬 Review Requirements (Optional)
롤백된 커밋 재적용
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항