fix: 탈퇴한 사용자가 물리적 삭제가 되지 않았던 문제를 해결한다 #574
Conversation
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough1. 저장소 메서드 추가: 여러 Repository 인터페이스에 Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (8)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-07-28T18:36:56.462ZApplied to files:
📚 Learning: 2025-07-28T03:14:33.849ZApplied to files:
📚 Learning: 2025-07-29T17:26:08.811ZApplied to files:
🔇 Additional comments (7)
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 |
whqtker
changed the title
whqtker
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/resources/db/migration/V38__add_delete_cascade_option_on_fk.sql (3)
1-89: 마이그레이션 접근법은 명확하지만, 적용 범위와 롤백 전략 확인 필요합니다.전반적으로 마이그레이션 구조는 건전합니다.
site_user테이블 삭제 시 관련 레코드를 연쇄적으로 삭제하는 방식으로 하드 딜리트 문제를 해결하는 것이 PR 목표와 일치합니다. 다음 몇 가지를 확인해 주세요:
외래 키 적용 범위 검증
현재 마이그레이션이site_user를 참조하는 모든 FK를 다루고 있는지 확인이 필요합니다. 마이그레이션 후 누락된 FK가 있으면 일부 연관 레코드가 여전히 삭제되지 않을 수 있습니다.롤백 전략 부재
역방향 마이그레이션(undo 스크립트)이 없으면 문제 발생 시 이전 상태로 복구하기 어렵습니다. Flyway는 일반적으로 단방향이지만, 이 구조적 변경에 대해 롤백 전략이 있는지 확인해 주세요.프로덕션 배포 시 고려 사항
DDL 작업은 영향을 받는 테이블을 잠금합니다. 특히post,comment같은 대형 테이블의 경우 마이그레이션 시간을 확인하고 적절한 점검 시간에 배포하세요.
1-89: 제약 조건 네이밍 컨벤션이 거의 일관성 있으나, 소수 불규칙성 정리 권장합니다.대부분의 새 제약 조건 이름이
fk_[테이블]_[칼럼]패턴을 따르고 있어 좋습니다. 단, 아래 두 가지 예외가 있습니다:
- Line 33-34 (
fk_app_site_user): 테이블 약자와 칼럼명 생략 패턴으로 다른 네이밍과 톤이 다릅니다.- Line 88-89 (
fk_mentor_application_site_user): 다른 제약과 달리_id접미사가 없습니다.장기 유지보수 관점에서 모든 FK 제약을
fk_[테이블]_[칼럼]형식(예:fk_app_site_user_id,fk_mentor_application_site_user_id)으로 정렬하면 일관성이 높아집니다.
1-89: ON DELETE CASCADE 적용 시 의도하지 않은 연쇄 삭제 리스크를 모니터링하세요.이 마이그레이션 후
site_user레코드 삭제 시 위 18개 FK를 통해 관련된 모든 하위 레코드가 자동으로 삭제됩니다. 이는 의도된 동작이지만, 배포 후 몇 가지를 관찰해야 합니다:
- 의도하지 않은 데이터 손실 - 비즈니스 로직상 보존해야 할 데이터가 연쇄 삭제로 인해 손실되지 않는지 확인
- 애플리케이션 코드 정리 - PR 배경에 언급된 대로
UserRemovalScheduler의 연관 레코드 삭제 코드(siteUserRepository.deleteAll전 개별 삭제 로직)가 이제 불필요할 가능성이 있으니 정리 검토 권장- 데이터베이스 트리거/프로시저 - 기존에 수동으로 처리하던 삭제 로직이 있다면 중복 처리로 인한 오류를 피하도록 검토
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/V38__add_delete_cascade_option_on_fk.sql(1 hunks)
🔇 Additional comments (1)
src/main/resources/db/migration/V38__add_delete_cascade_option_on_fk.sql (1)
46-46: 일부 제약 조건에서 이미 같은 이름으로 존재하는 FK를 삭제 후 재생성하고 있습니다.Line 46, 51, 56, 61, 71, 76, 81, 86에서 기존 제약 조건과 동일한 이름으로 FK를 재생성하고 있습니다. 이는 현재 스키마에서 이 제약들이 이미 사람이 정의한 이름으로 존재한다는 의미입니다.
확인 사항:
- 이 제약 조건들이 정말로 같은 이름을 유지하는 것이 의도인지, 아니면 버전 관리 과정에서 일부는 이미 수정된 것인지 검증해 주세요.
- 특히
application테이블(Line 31)의 경우 이미fk_app_site_user이름으로 존재하는 것으로 보아, 이전 마이그레이션에서 수동으로 명명한 제약으로 추정됩니다.Also applies to: 51-51, 56-56, 61-61, 71-71, 76-76, 81-81, 86-86
Hexeong
left a comment
Hexeong
left a comment
There was a problem hiding this comment.
수고하십니다~ FK ID 관련하여 질문 있어서 comment 남겨뒀습니다~
| @@ -0,0 +1,89 @@ | |||
| ALTER TABLE interested_country DROP FOREIGN KEY FK26u5am55jefclcd7r5smk8ai7; | |||
| ALTER TABLE interested_country | |||
There was a problem hiding this comment.
FK ID가 로컬에서와 인스턴스에서가 다를 것 같은데, 하드 코딩된 이유가 따로 있을까요??
There was a problem hiding this comment.
flyway가 stage, prod 환경에서만 동작하고, 확인한 결과 두 환경 모두 FK가 같아서 단순 하드 코딩했습니다
sukangpunch
left a comment
sukangpunch
left a comment
There was a problem hiding this comment.
잘 작성해주신 것 같습니다!!
근데 고민이 하나 있는데, 엔티티 클래스 에서 jpa 연관관계를 다 끊어 놓은 상황에서 db 에서 casecade 구현하는게 괜찮을까? 라는 생각이 들었습니다.
제가 까먹어서 그런데, 연관관계 매핑을 끊었던 이유를 혹시 알고 계신가요? 들었던것 같은데 기억이 안나네요...
|
음 이러면 코드와 디비간의 불일치가 발생할 거 같긴한데 문제는 없어보이네요! |
There was a problem hiding this comment.
Actionable comments posted: 14
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.javasrc/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.javasrc/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.javasrc/main/java/com/example/solidconnection/community/post/repository/PostLikeRepository.javasrc/main/java/com/example/solidconnection/community/post/repository/PostRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentorRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.javasrc/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.javasrc/main/java/com/example/solidconnection/news/repository/NewsRepository.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.javasrc/main/java/com/example/solidconnection/score/repository/GpaScoreRepository.javasrc/main/java/com/example/solidconnection/score/repository/LanguageTestScoreRepository.javasrc/main/java/com/example/solidconnection/siteuser/repository/UserBlockRepository.javasrc/main/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepository.java
🔇 Additional comments (7)
src/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.java (1)
45-46: 메서드 추가 확인 완료
deleteAllBySiteUserId메서드가 올바르게 추가되었습니다. Spring Data JPA의 메서드 네이밍 규칙을 따르고 있어 Comment 엔티티에서 siteUserId 필드를 기준으로 삭제 쿼리가 자동 생성됩니다.src/main/java/com/example/solidconnection/score/repository/LanguageTestScoreRepository.java (1)
17-18: 메서드 추가 확인 완료
deleteAllBySiteUserId메서드가 다른 Repository들과 일관된 패턴으로 추가되었습니다.src/main/java/com/example/solidconnection/report/repository/ReportRepository.java (1)
10-11: 메서드 추가 확인 완료
deleteAllByReporterId메서드가 Report 도메인에 맞게 올바르게 추가되었습니다.src/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.java (1)
12-13: 메서드 추가 확인 완료
deleteAllBySiteUserId메서드가 올바르게 추가되었습니다.src/main/java/com/example/solidconnection/community/post/repository/PostLikeRepository.java (1)
19-20: 메서드 추가 확인 완료
deleteAllBySiteUserId메서드가 올바르게 추가되었습니다.src/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.java (1)
42-43: 메서드 추가 확인 완료
deleteAllByMenteeId메서드가 Mentoring 도메인에 맞게 올바르게 추가되었습니다.src/main/java/com/example/solidconnection/siteuser/repository/UserBlockRepository.java (1)
27-28: 메서드 추가 확인 완료
deleteAllByBlockerIdOrBlockedId메서드가 올바르게 추가되었습니다. 사용자가 차단한 경우와 차단당한 경우 모두를 처리하기 위해 OR 조건을 사용한 것이 적절합니다.
src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java
Outdated
Show resolved
Hide resolved
| @Scheduled(cron = EVERY_MIDNIGHT) | ||
| @Transactional | ||
| public void scheduledUserRemoval() { | ||
| LocalDate cutoffDate = LocalDate.now().minusDays(ACCOUNT_RECOVER_DURATION); | ||
| List<SiteUser> usersToRemove = siteUserRepository.findUsersToBeRemoved(cutoffDate); | ||
| siteUserRepository.deleteAll(usersToRemove); | ||
|
|
||
| usersToRemove.forEach(this::deleteUserAndRelatedData); | ||
| } |
There was a problem hiding this comment.
2. 트랜잭션 경계 문제
@Transactional 애노테이션이 scheduledUserRemoval 메서드에 추가되어 모든 사용자 삭제가 하나의 트랜잭션으로 처리됩니다. 이는 심각한 문제를 초래할 수 있습니다:
- 대량 데이터 처리 시 트랜잭션 타임아웃: 삭제할 사용자가 많을 경우 트랜잭션이 너무 길어져 타임아웃이 발생할 수 있습니다.
- 부분 실패 시 전체 롤백: 100명 중 99번째 사용자 삭제 중 오류가 발생하면 앞의 98명도 모두 롤백됩니다.
- 테이블 락 장기 점유: 긴 트랜잭션으로 인해 다른 작업들이 대기하게 됩니다.
🔎 개별 사용자별 트랜잭션 처리로 개선
@Scheduled(cron = EVERY_MIDNIGHT)
- @Transactional
public void scheduledUserRemoval() {
LocalDate cutoffDate = LocalDate.now().minusDays(ACCOUNT_RECOVER_DURATION);
List<SiteUser> usersToRemove = siteUserRepository.findUsersToBeRemoved(cutoffDate);
usersToRemove.forEach(this::deleteUserAndRelatedData);
}
+ @Transactional
private void deleteUserAndRelatedData(SiteUser user) {
+ try {
long siteUserId = user.getId();
likedNewsRepository.deleteAllBySiteUserId(siteUserId);
// ... 나머지 삭제 로직
siteUserRepository.delete(user);
+ } catch (Exception e) {
+ log.error("Failed to delete user and related data for userId: {}", user.getId(), e);
+ // 개별 사용자 삭제 실패 시에도 다음 사용자 처리 계속
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java
around lines 60 to 67, the @Transactional on scheduledUserRemoval makes the
entire batch run in one transaction; remove the @Transactional from this method,
ensure each deletion runs in its own transaction (e.g., annotate
deleteUserAndRelatedData with @Transactional(propagation =
Propagation.REQUIRES_NEW) or execute each delete via TransactionTemplate), and
wrap each per-user delete call in a try/catch to log failures and continue so a
single failure does not roll back or stop the whole job.
src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java
Show resolved
Hide resolved
| interestedCountryRepository.deleteAllBySiteUserId(siteUserId); | ||
| interestedRegionRepository.deleteAllBySiteUserId(siteUserId); | ||
|
|
||
| s3Service.deleteExProfile(siteUserId); |
There was a problem hiding this comment.
4. S3 삭제 실패 처리
Line 94에서 S3 프로필 삭제 중 오류가 발생하면 현재 트랜잭션이 롤백되어 DB 삭제도 취소됩니다. S3 삭제는 외부 서비스 호출이므로:
- 네트워크 오류 가능성: S3 서비스가 일시적으로 불안정하면 전체 사용자 삭제가 실패합니다.
- 트랜잭션 일관성 문제: S3는 트랜잭션에 참여하지 않으므로, 롤백 시에도 S3에서는 파일이 삭제된 상태로 남을 수 있습니다.
🔎 S3 삭제 오류 격리 처리
+ try {
s3Service.deleteExProfile(siteUserId);
+ } catch (Exception e) {
+ log.warn("Failed to delete S3 profile for userId: {}, continuing with user deletion", siteUserId, e);
+ // S3 삭제 실패해도 사용자 삭제는 진행
+ }
siteUserRepository.delete(user);또는 S3 삭제를 별도의 보상 트랜잭션으로 처리하는 방안을 고려하세요.
📝 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.
| s3Service.deleteExProfile(siteUserId); | |
| try { | |
| s3Service.deleteExProfile(siteUserId); | |
| } catch (Exception e) { | |
| log.warn("Failed to delete S3 profile for userId: {}, continuing with user deletion", siteUserId, e); | |
| // S3 삭제 실패해도 사용자 삭제는 진행 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java
around line 94, calling s3Service.deleteExProfile(siteUserId) directly can cause
DB transaction rollback when S3 fails; change to perform S3 deletion outside the
DB transaction (e.g., register an after-commit action via
TransactionSynchronizationManager or publish a transactional
event/@TransactionalEventListener that runs after commit), wrap the S3 call in
retry logic and exception handling so failures do not throw into the DB
transaction, log detailed errors, and if deletion still fails enqueue a
compensating job or mark the user record with a flag for async cleanup to ensure
idempotent retry and eventual consistency.
src/main/java/com/example/solidconnection/score/repository/GpaScoreRepository.java
Show resolved
Hide resolved
...ain/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepository.java
Show resolved
Hide resolved
whqtker
force-pushed
the
fix/delete-siteuser-correctly
branch
from
d17592f to
07b7851
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
src/main/java/com/example/solidconnection/score/repository/GpaScoreRepository.java (1)
17-17: 구현이 올바릅니다.Derived delete 메서드는 별도 애노테이션 없이 정상 동작합니다. 과거 리뷰 코멘트는 부정확한 정보였습니다.
src/main/java/com/example/solidconnection/news/repository/NewsRepository.java (1)
12-12: 정상적으로 동작합니다.src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java (1)
44-44: 구현이 올바릅니다.src/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.java (1)
46-46: 정상적으로 동작합니다.src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java (3)
60-67: 트랜잭션 경계 설정으로 인한 심각한 위험이 있습니다.
@Transactional애노테이션이scheduledUserRemoval메서드에 적용되어 있어, 모든 사용자 삭제 작업이 하나의 거대한 트랜잭션으로 처리됩니다. 이는 다음과 같은 심각한 문제를 초래할 수 있습니다:
- 대량 데이터 처리 시 트랜잭션 타임아웃: 삭제 대상 사용자가 많을 경우 트랜잭션 지속 시간이 너무 길어져 타임아웃 발생 가능
- 부분 실패 시 전체 롤백: 100명 중 99번째 사용자 삭제 중 오류 발생 시 앞의 98명도 모두 롤백됨
- 테이블 락 장기 점유: 긴 트랜잭션으로 인해 다른 작업들이 블로킹될 수 있음
🔎 개별 사용자별 트랜잭션으로 개선하는 방법
@Scheduled(cron = EVERY_MIDNIGHT) - @Transactional public void scheduledUserRemoval() { LocalDate cutoffDate = LocalDate.now().minusDays(ACCOUNT_RECOVER_DURATION); List<SiteUser> usersToRemove = siteUserRepository.findUsersToBeRemoved(cutoffDate); usersToRemove.forEach(this::deleteUserAndRelatedData); } + @Transactional private void deleteUserAndRelatedData(SiteUser user) { + try { long siteUserId = user.getId(); // ... 기존 삭제 로직 ... siteUserRepository.delete(user); + } catch (Exception e) { + log.error("Failed to delete user and related data for userId: {}", user.getId(), e); + // 개별 사용자 삭제 실패 시에도 다음 사용자 처리 계속 + } }이렇게 변경하면:
- 각 사용자별로 독립적인 트랜잭션 실행
- 한 사용자 삭제 실패가 다른 사용자에게 영향 없음
- 트랜잭션 타임아웃 위험 최소화
94-94: S3 삭제 실패 시 DB 트랜잭션 롤백으로 인한 데이터 불일치 위험이 있습니다.Line 94에서 S3 프로필 삭제를 트랜잭션 내부에서 수행하고 있습니다. 이는 다음과 같은 문제를 야기합니다:
- 네트워크 오류 시 전체 사용자 삭제 실패: S3 서비스가 일시적으로 불안정하면 DB 삭제도 롤백됨
- 데이터 불일치 가능성: S3는 트랜잭션에 참여하지 않으므로, 롤백 시에도 S3에서는 파일이 이미 삭제된 상태로 남을 수 있음
🔎 S3 삭제 오류 격리 처리 방법
방법 1: S3 삭제 실패를 격리하여 DB 삭제는 진행
+ try { s3Service.deleteExProfile(siteUserId); + } catch (Exception e) { + log.warn("Failed to delete S3 profile for userId: {}, continuing with user deletion", siteUserId, e); + // S3 삭제 실패해도 사용자 DB 삭제는 진행 + } siteUserRepository.delete(user);방법 2: S3 삭제를 트랜잭션 커밋 후에 실행 (권장)
- s3Service.deleteExProfile(siteUserId); - siteUserRepository.delete(user); + + // 트랜잭션 커밋 후 S3 삭제 (TransactionSynchronizationManager 활용) + TransactionSynchronizationManager.registerSynchronization( + new TransactionSynchronization() { + @Override + public void afterCommit() { + try { + s3Service.deleteExProfile(siteUserId); + } catch (Exception e) { + log.error("Failed to delete S3 profile after user deletion for userId: {}", siteUserId, e); + // 보상 트랜잭션 큐에 등록하거나 재시도 로직 구현 + } + } + } + );방법 2를 사용하면 DB 삭제가 안전하게 커밋된 후에 S3 삭제를 시도하므로, S3 실패가 DB 트랜잭션에 영향을 주지 않습니다.
69-97: ChatReadStatus 엔티티 삭제가 누락되어 고아 레코드가 남습니다.Line 83에서
chatParticipantRepository.deleteAllBySiteUserId(siteUserId)로 ChatParticipant를 삭제하지만, ChatReadStatus 엔티티는 삭제하지 않고 있습니다.ChatReadStatus는
chatParticipantId를 참조하고 있으며 cascade 설정이 없어 ChatParticipant 삭제 시 자동으로 제거되지 않습니다. 이로 인해 ChatReadStatus 레코드가 고아(orphan) 상태로 데이터베이스에 남게 됩니다.🔎 ChatReadStatus 삭제 추가 방법
Line 83 앞에 ChatReadStatus 삭제 로직을 추가하세요:
+ // ChatParticipant 삭제 전에 먼저 ChatReadStatus를 삭제해야 함 + List<ChatParticipant> participants = chatParticipantRepository.findAllBySiteUserId(siteUserId); + List<Long> participantIds = participants.stream() + .map(ChatParticipant::getId) + .toList(); + if (!participantIds.isEmpty()) { + chatReadStatusRepository.deleteAllByChatParticipantIdIn(participantIds); + } chatParticipantRepository.deleteAllBySiteUserId(siteUserId);또는 ChatReadStatusRepository에
deleteAllByChatParticipantSiteUserId메서드가 있다면:+ chatReadStatusRepository.deleteAllByChatParticipantSiteUserId(siteUserId); chatParticipantRepository.deleteAllBySiteUserId(siteUserId);참고: ChatReadStatusRepository를 의존성으로 주입받아야 합니다.
src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java (1)
31-31: 필수 애노테이션@Modifying이 누락되어 런타임 오류가 발생합니다.Spring Data JPA의 파생 삭제 쿼리 메서드는 반드시
@Modifying애노테이션이 필요합니다. 이 애노테이션 없이 실행하면InvalidDataAccessApiUsageException이 발생합니다.🔎 수정 제안
+ @Modifying void deleteAllBySiteUserId(long siteUserId);파일 상단에 import 추가:
import org.springframework.data.jpa.repository.Modifying;참고: 호출하는 측(UserRemovalScheduler)에서 이미
@Transactional컨텍스트를 제공하고 있으므로, 이 메서드에는@Transactional을 추가하지 않아도 됩니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.javasrc/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.javasrc/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.javasrc/main/java/com/example/solidconnection/community/post/repository/PostLikeRepository.javasrc/main/java/com/example/solidconnection/community/post/repository/PostRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentorRepository.javasrc/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.javasrc/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.javasrc/main/java/com/example/solidconnection/news/repository/NewsRepository.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.javasrc/main/java/com/example/solidconnection/score/repository/GpaScoreRepository.javasrc/main/java/com/example/solidconnection/score/repository/LanguageTestScoreRepository.javasrc/main/java/com/example/solidconnection/siteuser/repository/UserBlockRepository.javasrc/main/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepository.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/example/solidconnection/siteuser/repository/UserBlockRepository.java
- src/main/java/com/example/solidconnection/report/repository/ReportRepository.java
- src/main/java/com/example/solidconnection/community/post/repository/PostLikeRepository.java
- src/main/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepository.java
- src/main/java/com/example/solidconnection/score/repository/LanguageTestScoreRepository.java
🔇 Additional comments (5)
src/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.java (1)
43-43: 메서드 구현이 적절합니다.
deleteAllByMenteeId는 멘티가 탈퇴할 때 해당 멘티와 관련된 멘토링 관계를 정리하는 데 적합합니다. Spring Data JPA가 자동으로 트랜잭션 내에서 처리하므로 정상 동작합니다.src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java (1)
63-63: 구현이 정확합니다.이 파일의 다른 메서드들(
decreaseLikeCount,increaseLikeCount등)은@Query를 사용하므로@Modifying이 필요하지만, derived delete 메서드인deleteAllBySiteUserId는 애노테이션 없이도 정상 작동합니다.src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java (1)
12-12: 애플리케이션 레벨 삭제 방식이 적절합니다.과거 리뷰에서 DB 레벨
ON DELETE CASCADE미적용을 지적했으나, PR 목적에 따르면 의도적으로 애플리케이션 레벨에서 삭제를 처리하기로 결정되었습니다. 이는 다음과 같은 이유로 타당합니다:
- 명시적 제어: 삭제 순서와 로직을 코드에서 명확하게 관리할 수 있습니다.
- 유연성: 향후 삭제 전 로깅, 검증 등 추가 로직 삽입이 용이합니다.
- 추적성: 애플리케이션 로그에서 삭제 과정을 추적할 수 있습니다.
UserRemovalScheduler가 올바른 순서로 연관 레코드를 삭제하는지는 위 LikedNewsRepository 리뷰의 검증 스크립트를 통해 확인해주세요.src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java (1)
13-13: 파생 삭제 메서드는 정상 동작합니다.Spring Data JPA의 파생 삭제 메서드(예:
deleteAllBySiteUserId)는 다음과 같이 작동합니다:
- 자동 트랜잭션 처리:
SimpleJpaRepository구현체가 자동으로 트랜잭션을 관리합니다.- 애노테이션 불필요:
@Modifying과@Transactional은@Query를 사용한 커스텀 메서드에만 필수입니다.현재 구현은 정상적입니다.
UserRemovalScheduler의@Transactional이 모든 삭제 작업을 감싸고 있으며, 삭제 순서도 올바르게 의존성을 처리하고 있습니다.성능 고려사항이 있습니다:
파생 삭제 메서드는 내부적으로 SELECT 후 개별 DELETE를 수행합니다(N+1 패턴). 삭제 대상이 많지 않다면 문제없으나, 대량 삭제 시 최적화를 고려할 수 있습니다.
src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java (1)
3-28: 의존성 추가 및 애플리케이션 레벨 삭제 구조를 확인했습니다.변경 사항:
- 사용자 관련 데이터를 삭제하기 위해 15개 이상의 Repository 의존성 추가
@Transactional애노테이션 추가로 스케줄링 메서드를 트랜잭션 컨텍스트에서 실행애플리케이션 레벨에서 연관 레코드를 명시적으로 삭제하는 방식으로 JPA cascade 제거 이후 발생한 Hard Delete 문제를 해결하는 접근은 타당합니다. 다만, 아래 리뷰 코멘트에서 지적하는 트랜잭션 경계 및 누락된 삭제 로직 문제를 반드시 확인해 주세요.
src/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.java
Show resolved
Hide resolved
whqtker
force-pushed
the
fix/delete-siteuser-correctly
branch
from
07b7851 to
2f5c555
Compare
4 participants
관련 이슈
작업 내용
FK의ON DELETE CASCADE옵션을 사용하여 DB 레벨에서 연관 레코드를 삭제하도록 구현했습니다.FK를 재설정하면서 컨벤션에 맞지 않았던 FK 제약조건 이름도 같이 수정했습니다.
deleteAllByXXX메서드로 스케쥴러에서 사용자 관련 데이터를 삭제하도록 변경했습니다.특이 사항
UserRemovalScheduler에 의하면 탈퇴 후 30일이 지나면 DB에서 Hard Delete가 이루어집니다.이전에 불필요한 연관 관계를 끊고 간접 참조로 리팩터링을 진행한 바 있습니다. 이전에는 JPA의
cascade옵션으로 연관 객체들 또한 잘 삭제되었으나, 연관 관계를 끊은 현재는SiteUser을 삭제하기 전, 연관 레코드를 먼저 삭제해야 합니다.다만,SiteUser와 간접적으로 관계를 맺는 객체가 정말 많기에, 위처럼 직접 연관된 레코드를 삭제하는 경우 코드가 길어지게 됩니다.SiteUser를 참조하는 새로운 엔티티를 생성하는 경우 스케쥴러에서 삭제하는 로직 또한 작성해주어야 하는데, 실수할 여지가 있다고 생각했습니다.리뷰 요구사항 (선택)