Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough사용자 저장소에 JPQL 기반 ID 삭제 메서드 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Service as UserService
participant Auth as AppleAuthClient
participant Repo as UserRepository
participant DB as Database
participant Events as EventPublisher
Client->>Service: deleteUser(userId)
Service->>Auth: revokeTokenIfPresent(user)
Auth-->>Service: tokenRevoked
Service->>Repo: deleteByUserId(userId)
Repo->>DB: execute JPQL DELETE (id = userId)
DB-->>Repo: deleteResult
Repo-->>Service: deletionAcknowledged
Service->>Events: publish(UserWithdrawnEvent)
Events-->>Service: published
Service-->>Client: deletionComplete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 시
🚥 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)
Comment |
There was a problem hiding this comment.
Pull request overview
이 PR은 사용자 삭제 로직을 변경하여 JPA의 엔티티 기반 삭제 방식에서 JPQL DELETE 쿼리를 사용한 직접 삭제 방식으로 전환합니다. 그러나 PR 설명에 변경 이유와 해결하려는 문제가 명시되어 있지 않아 변경의 목적을 파악하기 어렵습니다.
Changes:
- UserRepository에 JPQL DELETE 쿼리를 사용하는 deleteByUserId 메서드 추가
- UserService의 deleteUser 메서드에서 delete(user)를 deleteByUserId(userId)로 변경
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java | JPQL DELETE 쿼리를 사용하는 새로운 deleteByUserId 메서드 추가 |
| src/main/java/gg/agit/konect/domain/user/service/UserService.java | 기존 delete(user) 호출을 deleteByUserId(userId)로 변경 |
Comments suppressed due to low confidence (1)
src/main/java/gg/agit/konect/domain/user/service/UserService.java:227
- JPQL DELETE 쿼리를 사용하면 JPA의 엔티티 라이프사이클을 우회하게 되어 문제가 발생합니다:
-
User 엔티티를 조회한 후(line 215) JPQL DELETE로 삭제하면, 이미 조회된 user 객체는 여전히 영속성 컨텍스트에 남아있지만 데이터베이스에서는 삭제된 상태가 됩니다.
-
UserWithdrawnEvent를 발행할 때(line 225-227) 이미 삭제된 user 객체를 참조하게 되어 데이터 불일치가 발생할 수 있습니다.
-
데이터베이스의 ON DELETE CASCADE 설정으로 관련 테이블의 데이터는 자동 삭제되지만, JPA 1차 캐시에는 삭제되지 않은 채로 남아있어 트랜잭션 내에서 예상치 못한 동작이 발생할 수 있습니다.
기존의 userRepository.delete(user)를 사용하면 JPA가 적절히 엔티티 상태를 관리하고, ON DELETE CASCADE로 관련 데이터도 자동으로 삭제되므로 원래 방식을 유지하는 것이 안전합니다.
User user = userRepository.getById(userId);
validateNotClubPresident(userId);
if (user.getProvider() == Provider.APPLE) {
appleTokenRevocationService.revoke(user.getAppleRefreshToken());
}
userRepository.deleteByUserId(userId);
applicationEventPublisher.publishEvent(
UserWithdrawnEvent.from(user.getEmail(), user.getProvider().name())
);
| @Modifying | ||
| @Query("DELETE FROM User u WHERE u.id = :userId") | ||
| void deleteByUserId(@Param("userId") Integer userId); |
There was a problem hiding this comment.
@Modifying 어노테이션은 있지만 clearAutomatically 속성이 설정되지 않았습니다. JPQL을 사용한 벌크 연산(INSERT, UPDATE, DELETE)은 영속성 컨텍스트를 우회하므로, @Modifying(clearAutomatically = true)를 추가하여 쿼리 실행 후 영속성 컨텍스트를 자동으로 클리어해야 합니다.
다만, 앞서 지적한 것처럼 이 접근 방식 자체가 문제가 있으므로 기존 delete(user) 방식으로 되돌리는 것을 권장합니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/gg/agit/konect/domain/user/repository/UserRepository.java`:
- Around line 62-64: Update the `@Modifying` annotation on the deleteByUserId
method in UserRepository to clear and flush the persistence context after
executing the JPQL delete; change the annotation to
`@Modifying`(clearAutomatically = true, flushAutomatically = true) for the method
deleteByUserId(Integer userId) so the persistence context is flushed and cleared
after the delete.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/domain/user/service/UserService.java
🔇 Additional comments (1)
src/main/java/gg/agit/konect/domain/user/service/UserService.java (1)
223-223: 현재 코드는 안전합니다. 데이터베이스 스키마에 모든user_id외래 키에 대해ON DELETE CASCADE가 설정되어 있으므로, 사용자 삭제 시 관련된 모든 레코드(club_member, notification_device_token, chat_room_member 등)가 자동으로 삭제됩니다. JPQL bulk delete는 JPA 콜백을 실행하지 않지만, User 엔티티에는 현재@PreRemove콜백이 없으므로 이는 영향을 주지 않습니다. 기존 제안의 핵심 우려사항(고아 데이터, FK 위반)은 존재하지 않습니다.
| @Modifying | ||
| @Query("DELETE FROM User u WHERE u.id = :userId") | ||
| void deleteByUserId(@Param("userId") Integer userId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate and examine the UserRepository file
find . -type f -name "UserRepository.java" | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 140
🏁 Script executed:
# Check the structure and find Spring Data JPA configuration
git ls-files | grep -E "(pom\.xml|build\.gradle|UserRepository)" | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 235
🏁 Script executed:
# Search for the actual file
fd UserRepository.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 222
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2862
🏁 Script executed:
cat build.gradle | grep -A 50 "dependencies"Repository: BCSDLab/KONECT_BACK_END
Length of output: 1928
🏁 Script executed:
# Search for usage of deleteByUserId method
rg "deleteByUserId" --type java -B 3 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 2704
🏁 Script executed:
# Search for event listeners or transaction boundaries that might be affected
rg -E "(ApplicationEvent|EventListener|@Transactional|ApplicationEventPublisher)" src/main/java/gg/agit/konect/domain/user --type java | head -30Repository: BCSDLab/KONECT_BACK_END
Length of output: 177
🏁 Script executed:
# Search for UserService and the context around deleteByUserId
rg "deleteByUserId\|applicationEventPublisher" src/main/java/gg/agit/konect/domain/user/service/UserService.java -B 10 -A 10Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
# Search for UserWithdrawnEvent and its listeners
rg "UserWithdrawnEvent" --type java -B 2 -A 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 3406
🏁 Script executed:
# Check if UserService method has `@Transactional`
rg -B 5 "deleteByUserId" src/main/java/gg/agit/konect/domain/user/service/UserService.java | head -40Repository: BCSDLab/KONECT_BACK_END
Length of output: 254
🏁 Script executed:
# Get the full method that calls deleteByUserId
rg -B 20 "userRepository.deleteByUserId" src/main/java/gg/agit/konect/domain/user/service/UserService.java | head -50Repository: BCSDLab/KONECT_BACK_END
Length of output: 769
🏁 Script executed:
# Check for `@Transactional` annotation on the method
rg "@Transactional" src/main/java/gg/agit/konect/domain/user/service/UserService.java -B 2 -A 10Repository: BCSDLab/KONECT_BACK_END
Length of output: 1635
🏁 Script executed:
# Check what happens after deleteByUserId in the deleteUser method
rg -A 5 "userRepository.deleteByUserId" src/main/java/gg/agit/konect/domain/user/service/UserService.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 259
🏁 Script executed:
# Check if there are any other `@Modifying` methods with similar pattern
rg "@Modifying" src/main/java --type java -A 2 | grep -E "(`@Modifying`|@Query|clearAutomatically)"Repository: BCSDLab/KONECT_BACK_END
Length of output: 667
[LEVEL: medium] JPQL 삭제 후 영속성 컨텍스트가 정리되지 않습니다.
현재 @Modifying 쿼리는 1차 캐시를 자동으로 비우지 않아 deleteUser 직전 getById()로 로드된 User 엔티티가 그대로 남습니다. 이후 코드에서 같은 트랜잭션 내에 해당 User를 다시 조회하면 삭제된 엔티티가 반환되어 오동작할 수 있습니다.
@Modifying(clearAutomatically = true, flushAutomatically = true)로 변경하여 삭제 후 명시적으로 영속성 컨텍스트를 정리하세요.
최소 수정 예시
- `@Modifying`
+ `@Modifying`(clearAutomatically = true, flushAutomatically = true)
`@Query`("DELETE FROM User u WHERE u.id = :userId")
void deleteByUserId(`@Param`("userId") Integer userId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java`
around lines 62 - 64, Update the `@Modifying` annotation on the deleteByUserId
method in UserRepository to clear and flush the persistence context after
executing the JPQL delete; change the annotation to
`@Modifying`(clearAutomatically = true, flushAutomatically = true) for the method
deleteByUserId(Integer userId) so the persistence context is flushed and cleared
after the delete.
1 participant
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)