Skip to content

Conversation

@Shinjongyun
Copy link
Contributor

@Shinjongyun Shinjongyun commented Oct 13, 2025

Related issue 🛠

  • closed #이슈넘버

Work Description 📝

  • 기존에 있는 회원 탈퇴 기능 오류를 수정하였습니다!

Screenshot 📸

Uncompleted Tasks 😅

  • Task1

To Reviewers 📢

Summary by CodeRabbit

  • 버그 수정
    • 계정 삭제 시 연관된 멤버 데이터도 함께 정리되어 삭제 실패 가능성을 줄였습니다.
    • 삭제 트랜잭션 커밋 이후에 액세스 토큰을 무효화하도록 조정해 로그아웃/재인증 동작의 일관성과 안정성을 개선했습니다.
    • 계정 삭제 절차의 트랜잭션 처리를 강화해 간헐적으로 발생하던 오류를 예방했습니다.

@coderabbitai
Copy link

coderabbitai bot commented Oct 13, 2025

Walkthrough

MemberRepository에 userId 기반 삭제 및 존재 확인 메서드가 추가되었다. UserService의 deleteAccount는 트랜잭션으로 감싸지고, 사용자 삭제 전 관련 Member 레코드를 삭제하며, 액세스 토큰 무효화는 커밋 이후 콜백으로 지연 실행되도록 변경되었다.

Changes

Cohort / File(s) Summary of Changes
MemberRepository 업데이트
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java
deleteByUserId(Long userId) 추가(@Modifying, @query). existsByUserId(Long userId) 추가. 기존 메서드 변경 없음.
사용자 삭제 플로우 조정
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java
deleteAccount@Transactional 적용. 사용자 삭제 전 회원(Member) 연관 삭제 로직 추가. 토큰 무효화를 트랜잭션 커밋 후 TransactionSynchronization으로 지연 처리. 관련 import 추가.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant US as UserService
  participant MR as MemberRepository
  participant UR as UserRepository
  participant TS as Tx Manager

  rect rgba(200,230,255,0.3)
  note over US: deleteAccount(userId) 시작 (@Transactional)
  U->>US: 요청: 계정 삭제
  US->>MR: existsByUserId(userId)?
  alt Member 존재
    US->>MR: deleteByUserId(userId)
  end
  US->>UR: deleteById(userId)
  US->>TS: afterCommit 콜백 등록(토큰 무효화)
  note over US: 트랜잭션 커밋
  end

  rect rgba(220,255,220,0.3)
  note over TS: afterCommit
  TS-->>US: 콜백 호출
  US->>US: 액세스 토큰 무효화
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • yskim6772

Poem

계정은 고요히 문을 닫고,
멤버 흔적 먼저 지워 간 뒤,
커밋 지나 속삭이듯 토큰 안녕—
트랜잭션 숨 고르고 일이 끝난다.
방 정돈 끝! 다음 손님을 맞이해요. 🧹✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 이 PR의 제목은 회원탈퇴 로직 오류 수정을 주된 변경사항으로 명확하게 요약하고 불필요한 세부사항 없이 간결하게 작성되어 있어 주요 변경사항을 잘 반영합니다.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#16-delete-account

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@yskim6772 yskim6772 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿 !!!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (1)

188-190: 선택적 최적화: existsByUserId 체크를 생략할 수 있습니다.

deleteByUserId는 레코드가 없어도 안전하게 실행되므로, existsByUserId 체크 없이 바로 호출해도 됩니다. 다만 가독성이나 명시적인 의도 표현을 위해 현재처럼 유지하는 것도 괜찮습니다.

원한다면 다음과 같이 간소화할 수 있습니다:

-        if (memberRepository.existsByUserId(userId)) {
-            memberRepository.deleteByUserId(userId);
-        }
+        memberRepository.deleteByUserId(userId);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2bbc59 and 2f9ac42.

📒 Files selected for processing (2)
  • src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java (1 hunks)
  • src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (2 hunks)
🔇 Additional comments (6)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java (2)

29-31: LGTM! 사용자 ID 기반 Member 삭제 메서드가 올바르게 구현되었습니다.

@Modifying 어노테이션과 JPQL 쿼리가 적절하게 사용되었고, clearAutomatically와 flushAutomatically 플래그도 제대로 설정되어 있네요. UserService의 회원 탈퇴 로직에서 외래 키 제약 조건 문제를 해결하는 데 필요한 메서드입니다.


33-33: LGTM! 존재 여부 확인 메서드가 올바르게 선언되었습니다.

Spring Data JPA의 네이밍 컨벤션을 따르고 있어서 구현체가 자동으로 생성됩니다. UserService에서 불필요한 삭제 쿼리 실행을 방지하는 데 활용되고 있습니다.

src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/service/UserService.java (4)

28-29: LGTM! 트랜잭션 동기화에 필요한 임포트가 추가되었습니다.

afterCommit 콜백 구현을 위해 필요한 클래스들이 올바르게 임포트되었습니다.


183-183: 굿! @transactional 어노테이션이 추가되어 트랜잭션 무결성이 보장됩니다.

회원 탈퇴 로직이 이제 트랜잭션으로 관리되어, Member 삭제와 User 삭제가 원자적으로 처리됩니다. 만약 중간에 오류가 발생하면 전체가 롤백되므로 데이터 일관성이 유지됩니다.


196-201: 좋은 개선입니다! 토큰 무효화를 트랜잭션 커밋 이후로 지연시켰습니다.

TransactionSynchronizationafterCommit 콜백을 사용하여, 데이터베이스 작업이 성공적으로 커밋된 후에만 액세스 토큰이 무효화되도록 보장합니다. 이렇게 하면 트랜잭션이 롤백될 경우 토큰이 무효화되지 않아 일관성이 유지됩니다.


203-203: SecurityContextHolder 클리어 타이밍 확인이 필요합니다.

Line 203의 SecurityContextHolder.clearContext()가 트랜잭션 커밋 전에 즉시 실행됩니다. 만약 이후에 트랜잭션이 롤백되면 사용자는 여전히 존재하지만 보안 컨텍스트는 이미 클리어된 상태가 됩니다.

토큰 무효화처럼 afterCommit 콜백 안에서 실행하는 것이 더 안전할 수 있습니다. 현재 동작이 의도된 것인지 확인해 주세요.

필요하다면 다음과 같이 수정할 수 있습니다:

         TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
             @Override
             public void afterCommit() {
                 jwtService.invalidAccessToken(accessToken);
+                SecurityContextHolder.clearContext();
             }
         });
-
-        SecurityContextHolder.clearContext();

@Shinjongyun Shinjongyun merged commit b670a6a into develop Oct 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants