refactor: 도움돼요 토글 비관적 락 제거 및 CQRS 구조 개선#169
Conversation
- 비관적 락(PESSIMISTIC_WRITE) 대신 DB 원자적 UPDATE 쿼리로 동시성 처리 - incrementHelpfulCount/decrementHelpfulCount @Modifying 쿼리 추가 - Review 엔티티의 increment/decrementHelpfulCount 메서드 삭제 - toggleHelpful을 ReviewQueryService에서 ReviewCommandService로 이동 - getUserOrThrow를 도움돼요 추가 시에만 호출하도록 수정 - ReviewController에서 reviewCommandService.toggleHelpful 호출로 변경 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Walkthrough리뷰의 "도움됨" 토글 책임을 조회 서비스에서 명령 서비스로 이동했고, 비관적 잠금 제거 후 저장소 수준의 증감(증가/감소) 쿼리와 ReviewHelpful 삭제/생성 흐름으로 동시성 처리를 변경했습니다. Review 엔티티의 도움됨 관련 증감 메서드는 삭제되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ReviewController
participant CmdSvc as ReviewCommandService
participant RHRepo as ReviewHelpfulRepository
participant RRepo as ReviewRepository
participant DB
Client->>Controller: toggleHelpful(reviewId, userId)
Controller->>CmdSvc: toggleHelpful(reviewId, userId)
CmdSvc->>RHRepo: deleteByReviewIdAndUserId(reviewId, userId)
RHRepo->>DB: execute DELETE (returns count)
alt count > 0 (was helpful)
CmdSvc->>RRepo: decrementHelpfulCount(reviewId)
RRepo->>DB: execute UPDATE (helpful_count = max(helpful_count -1,0))
CmdSvc->>RRepo: findHelpfulCountById(reviewId)
else count == 0 (not helpful)
CmdSvc->>RHRepo: save(new ReviewHelpful(userId, reviewId))
RHRepo->>DB: INSERT ReviewHelpful
CmdSvc->>RRepo: incrementHelpfulCount(reviewId)
RRepo->>DB: execute UPDATE (helpful_count +1)
CmdSvc->>RRepo: findHelpfulCountById(reviewId)
end
RRepo->>CmdSvc: helpfulCount
CmdSvc->>Controller: ReviewHelpfulResponse(reviewId, isHelpful, helpfulCount)
Controller->>Client: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/com/ongil/backend/domain/review/repository/ReviewRepository.java`:
- Around line 189-195: Change the update methods to return the number of
affected rows so callers can detect whether the increment/decrement actually
applied: update the repository method signatures incrementHelpfulCount and
decrementHelpfulCount from void to an int (or Integer) return type so Spring
Data JPA will return the update count for those `@Modifying` `@Query` statements,
and then use that int result in the service where you toggle helpful state to
branch on 0 vs 1 (ensure existing transaction/annotation usage remains correct).
In
`@src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java`:
- Around line 203-210: The current logic in ReviewCommandService computes
helpfulCount by adjusting review.getHelpfulCount() in memory which causes race
conditions; instead, after toggling the helpful state (the code that sets or
removes the helpful record), re-query the database for the authoritative count
and use that value when building ReviewHelpfulResponse (do not derive via
in-memory arithmetic). Add/use a repository method such as
findHelpfulCountById(Long reviewId) to fetch the latest helpfulCount, handle the
Optional appropriately (or throw a domain exception), and pass that DB-returned
integer into ReviewHelpfulResponse.helpfulCount(...) while keeping isHelpful set
to the toggled value.
- Around line 188-201: The current toggle in ReviewCommandService uses
existsByReviewIdAndUserId then delete/add which is TOCTOU-prone; change to
row-count-based atomic operations: call
reviewHelpfulRepository.deleteByReviewIdAndUserId(...) that returns the number
of rows deleted and if deletedCount > 0 then call
reviewRepository.decrementHelpfulCount(reviewId), otherwise try to insert a new
ReviewHelpful (build as now) and handle unique-constraint/duplicate-key
exceptions to detect concurrent insert (on success call
reviewRepository.incrementHelpfulCount(reviewId)); ensure the service method is
`@Transactional` and use repository methods that return affected-row counts (or
catch DataIntegrityViolationException) instead of existsByReviewIdAndUserId to
maintain correct helpful count under concurrency.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/ongil/backend/domain/review/controller/ReviewController.javasrc/main/java/com/ongil/backend/domain/review/entity/Review.javasrc/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.javasrc/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.javasrc/main/java/com/ongil/backend/domain/review/service/ReviewQueryService.java
💤 Files with no reviewable changes (2)
- src/main/java/com/ongil/backend/domain/review/service/ReviewQueryService.java
- src/main/java/com/ongil/backend/domain/review/entity/Review.java
src/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
Outdated
Show resolved
Hide resolved
- deleteByReviewIdAndUserId를 @query + @Modifying으로 변경하여 int 반환 - incrementHelpfulCount/decrementHelpfulCount 반환타입 void → int 변경 - exists 체크 제거 후 delete 결과값 기반 분기로 TOCTOU 레이스컨디션 해소 - findHelpfulCountById 추가하여 응답 helpfulCount를 DB에서 직접 조회 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/com/ongil/backend/domain/review/repository/ReviewHelpfulRepository.java`:
- Around line 19-22: When inserting a new ReviewHelpful after calling
deleteByReviewIdAndUserId you must handle race conditions that can cause a
DataIntegrityViolationException due to the uk_review_helpful_review_user unique
constraint; update the code that calls save() (the service method that performs
deleteByReviewIdAndUserId + repository.save(...)) to wrap the save() in a
try/catch for DataIntegrityViolationException and handle it by either
re-querying the existing ReviewHelpful (returning the existing record/flag) or
swallowing/logging and returning a safe failure indicator, and ensure the
operation runs in a transactional context so deleteByReviewIdAndUserId and the
insert behave atomically; reference the repository method
deleteByReviewIdAndUserId and the unique constraint
uk_review_helpful_review_user when applying this change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/ongil/backend/domain/review/repository/ReviewHelpfulRepository.javasrc/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.javasrc/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
| // 리뷰 도움돼요 기록 삭제 (삭제된 row 수 반환) | ||
| @Modifying | ||
| @Query("DELETE FROM ReviewHelpful rh WHERE rh.review.id = :reviewId AND rh.user.id = :userId") | ||
| int deleteByReviewIdAndUserId(@Param("reviewId") Long reviewId, @Param("userId") Long userId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find and inspect ReviewHelpful entity definition
echo "=== Searching for ReviewHelpful entity ==="
fd ReviewHelpful.java src/main/java --exec cat {} \;
echo -e "\n=== Searching for unique constraints/indexes on review_helpful ==="
# Check migration files and DDL
fd -t f "\.(sql|ddl)$" src/main/resources --exec grep -l "review_helpful\|ReviewHelpful" {} \;
fd -t f "\.(sql|ddl)$" src/main/resources --exec cat {} \;
# Check if there are migration files with java/xml
fd -type f -name "*.xml" -o -name "*.java" | grep -i "migration\|flyway\|liquibase" | head -20
echo -e "\n=== Searching for ReviewCommandService or toggle method ==="
fd ReviewCommandService.java src/main/java --exec cat {} \;
echo -e "\n=== Searching for exception handling patterns ==="
rg -i "DataIntegrityViolationException|ConstraintViolationException|DuplicateKeyException" src/main/java -C 3 || trueRepository: IT-Cotato/12th-OnGil-BE
Length of output: 1389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for ReviewCommandService or toggle-related services ==="
fd -type f -name "*CommandService.java" src/main/java | xargs cat 2>/dev/null || true
echo -e "\n=== Searching for toggle or helpful-related business logic ==="
fd -type f -name "*.java" src/main/java | xargs rg -l "toggleHelpful|deleteByReviewIdAndUserId" 2>/dev/null || true
echo -e "\n=== Searching for exception handling of DataIntegrityViolationException ==="
rg "DataIntegrityViolationException|ConstraintViolationException" src/main/java -B 3 -A 5 2>/dev/null || true
echo -e "\n=== Searching for SQL migration/DDL files ==="
find src/main/resources -name "*.sql" -o -name "*.ddl" 2>/dev/null | head -20
# Look for Flyway or Liquibase migration files
find src/main/resources -type d -name db -o -name migration -o -name flyway 2>/dev/null || trueRepository: IT-Cotato/12th-OnGil-BE
Length of output: 6413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full ReviewCommandService content ==="
cat src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
echo -e "\n=== Searching for toggleHelpful or addHelpful method ==="
rg "toggleHelpful|addHelpful|deleteByReviewIdAndUserId" src/main/java/com/ongil/backend/domain/review -B 2 -A 15Repository: IT-Cotato/12th-OnGil-BE
Length of output: 15071
동시성 환경에서 도움돼요 추가 시 미처리 예외 발생 위험
삭제 여부 판단 후 save()로 신규 레코드 추가 시, DataIntegrityViolationException에 대한 예외 처리가 없어 동시 요청 시 미처리 예외가 발생할 수 있습니다. 유니크 제약(uk_review_helpful_review_user)은 엔티티 레벨에 이미 정의되어 있으나, 경합 상황에서 insert 실패 시 예외를 명시적으로 처리하거나 다른 동시성 전략(낙관적 잠금, 멀티스레드 안전 로직 등)이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/ongil/backend/domain/review/repository/ReviewHelpfulRepository.java`
around lines 19 - 22, When inserting a new ReviewHelpful after calling
deleteByReviewIdAndUserId you must handle race conditions that can cause a
DataIntegrityViolationException due to the uk_review_helpful_review_user unique
constraint; update the code that calls save() (the service method that performs
deleteByReviewIdAndUserId + repository.save(...)) to wrap the save() in a
try/catch for DataIntegrityViolationException and handle it by either
re-querying the existing ReviewHelpful (returning the existing record/flag) or
swallowing/logging and returning a safe failure indicator, and ensure the
operation runs in a transactional context so deleteByReviewIdAndUserId and the
insert behave atomically; reference the repository method
deleteByReviewIdAndUserId and the unique constraint
uk_review_helpful_review_user when applying this change.
🔍️ 작업 내용
도움돼요 토글 비관적 락 제거 및 CQRS 구조 개선
✨ 상세 설명
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
릴리스 노트