-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 사용자 차단 구현 및 회원 탈퇴 로직 수정 #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthrough사용자 차단 기능을 새로 추가하고 회원 탈퇴 프로세스를 단계화(즉시 INACTIVE 후 일정 기간 경과 시 완전 탈퇴)하도록 재구성했습니다. User 엔티티 상태 및 표시명 헬퍼가 변경되었고, 차단/언블록/차단목록 API와 관련 저장소·서비스·마이그레이션이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserBlockController
participant UserBlockService
participant UserRepository
participant FollowRepository
participant UserBlockRepository
Client->>UserBlockController: POST /api/v1/blocks/{blockedId}
activate UserBlockController
UserBlockController->>UserBlockService: blockUser(blockerId, blockedId)
activate UserBlockService
UserBlockService->>UserRepository: findById(blockerId)
UserBlockService->>UserRepository: findById(blockedId)
alt valid & not self
UserBlockService->>FollowRepository: deleteByFollowerAndFollowing(blocker, blocked)
UserBlockService->>FollowRepository: deleteByFollowerAndFollowing(blocked, blocker)
Note over UserBlockService: 양방향 팔로우 정리
UserBlockService->>UserBlockRepository: save(new UserBlock(...))
else error
UserBlockService-->>UserBlockController: throw Error
end
deactivate UserBlockService
UserBlockController-->>Client: ApiResponse OK
deactivate UserBlockController
sequenceDiagram
participant Scheduler
participant UserDeleteService
participant AuthService
participant SocialAuthRepository
participant S3Service
participant ChallengeRepository
participant UserRepository
Scheduler->>UserDeleteService: processPermanentWithdrawal(user)
activate UserDeleteService
UserDeleteService->>AuthService: revoke(userId)
AuthService->>SocialAuthRepository: deleteByUser(user)
UserDeleteService->>SocialAuthRepository: deleteByUser(user)
UserDeleteService->>S3Service: deleteFileByKey(profileKey)
UserDeleteService->>ChallengeRepository: findActiveUserChallenges(user)
loop for each challenge
UserDeleteService->>ChallengeRepository: decreaseCurrentParticipantCount(challengeId)
end
UserDeleteService->>UserRepository: save(user.completeWithdrawal())
deactivate UserDeleteService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
261-276:revoke()메서드에@Transactional어노테이션이 누락되었습니다.
revoke()메서드는 DB에서 사용자와 소셜 인증 정보를 조회하므로 트랜잭션 컨텍스트가 필요합니다. 현재 상태에서는LazyInitializationException이 발생할 수 있습니다.🔎 제안된 수정
+ @Transactional(readOnly = true) public void revoke(Long userId) { // 현재 트랜잭션 안에서 유저를 다시 조회 (영속화) User user = userRepository.findById(userId)또는 외부 API 호출 후 DB 상태 변경이 필요하다면
@Transactional만 사용하세요.src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
297-302:getDisplayNickname()사용으로 인한 비교 로직 오류 위험현재 코드는
user.getDisplayNickname()을 통해 닉네임을 비교하고 있습니다. 이 메서드는 비활성 사용자에게"(알 수 없음)"을 반환하므로, 만약 비활성 사용자가 이 메서드에 접근할 수 있다면 다음과 같은 문제가 발생할 수 있습니다:nickname = "test123" (입력) user.getDisplayNickname() = "(알 수 없음)" (반환) → 비교 결과: 다름 → 중복 검사 실행 (의도하지 않음)다행히 현재
updateUserInfoAPI 엔드포인트가UserController에 노출되어 있지 않아 즉시 위험은 없지만, 방어적 코딩 관점에서 라인 297에서user.getDisplayNickname()대신user.getNickname()을 사용하도록 수정하세요. 이는 향후 엔드포인트가 추가되거나 다른 경로에서 호출될 때 발생할 수 있는 잠재적 버그를 방지합니다.제안 수정
// 현재 코드 (라인 297) if (!nickname.equals(user.getDisplayNickname())) { // 수정 제안 if (!nickname.equals(user.getNickname())) {
🧹 Nitpick comments (16)
src/main/java/com/hrr/backend/domain/challenge/repository/ChallengeRepository.java (1)
63-68: clearAutomatically 속성 추가를 권장합니다.같은 파일의 다른
@Modifying쿼리들(24, 28, 50번 라인)과 달리clearAutomatically = true속성이 누락되었습니다. 이 속성이 없으면 영속성 컨텍스트가 자동으로 클리어되지 않아 캐시된 엔티티와 DB 상태가 불일치할 수 있습니다.일관성과 안정성을 위해 다른 메서드와 동일하게 설정하는 것을 권장합니다.
🔎 제안 수정사항
- @Modifying + @Modifying(clearAutomatically = true) @Query("UPDATE Challenge c " + "SET c.currentParticipants = c.currentParticipants - 1 " + "WHERE c.id = :id AND c.currentParticipants > 0") int decreaseCurrentParticipantCount(@Param("id") Long id);src/main/java/com/hrr/backend/domain/user/entity/enums/UserStatus.java (1)
5-7: 주석 개선 확인 완료!새로운 탈퇴 흐름(INACTIVE → DELETED)을 정확히 반영한 주석입니다.
다만, Line 7의 DELETED에만 trailing comma가 있는데, 일관성을 위해 모든 enum 상수에 trailing comma를 추가하거나 모두 제거하는 것을 권장합니다.
🔎 스타일 일관성 개선 제안
public enum UserStatus { - ACTIVE, // 활성 (가입 즉시) + ACTIVE, // 활성 (가입 즉시) INACTIVE, // 비활성화 (탈퇴 요청 직후) - SUSPENDED, // 정지 (관리자에 의해) + SUSPENDED, // 정지 (관리자에 의해) DELETED,// 탈퇴 (탈퇴 후 30일이 지나 완전히 삭제된 상태) }src/main/java/com/hrr/backend/domain/user/repository/UserChallengeRepository.java (1)
69-70: 레포지토리 메서드 추가 확인!Spring Data JPA 네이밍 컨벤션을 따르는 명확한 쿼리 메서드입니다.
다만, 메서드 주석이 "참가 중인 챌린지"라고 하지만 실제로는 모든 상태의 챌린지를 조회할 수 있으므로 주석을 수정하거나, 파라미터명을 간소화하는 것을 고려해보세요.
🔎 가독성 개선 제안
옵션 1: 주석 수정
- // 특정 사용자가 참가 중인 챌린지 정보를 상태에 따라 조회 + // 특정 사용자의 챌린지 정보를 상태별로 조회 List<UserChallenge> findByUserAndStatus(User user, ChallengeJoinStatus challengeJoinStatus);옵션 2: 파라미터명 간소화
- List<UserChallenge> findByUserAndStatus(User user, ChallengeJoinStatus challengeJoinStatus); + List<UserChallenge> findByUserAndStatus(User user, ChallengeJoinStatus status);src/main/java/com/hrr/backend/global/s3/S3Service.java (1)
62-76: 파일 삭제 메서드가 잘 구현되었으나, 예외 처리를 더 구체화할 수 있습니다.현재 구현은 클린업 작업에 적합하게 실패를 로깅하고 조용히 처리합니다. 선택적으로 다음을 고려해보세요:
- 구체적인 예외 처리:
Exception대신S3Exception을 catch하여 S3 관련 오류를 더 명확히 처리할 수 있습니다.- 검증 로직 문서화: Line 63의
key.startsWith("http")검사는 전체 URL 실수로부터 보호하는 좋은 방어 로직입니다. 주석 추가를 권장합니다.🔎 선택적 개선안
+import software.amazon.awssdk.services.s3.model.S3Exception; public void deleteFileByKey(String key) { + // URL이 아닌 S3 key만 허용 (예: uploads/uuid_filename.png) if (key == null || key.isBlank() || key.startsWith("http")) return; try { DeleteObjectRequest deleteObjectRequest = DeleteObjectRequest.builder() .bucket(bucketName) .key(key) .build(); s3Client.deleteObject(deleteObjectRequest); log.info("S3 파일 삭제 완료 (Key: {})", key); - } catch (Exception e) { + } catch (S3Exception e) { log.error("S3 파일 삭제 실패 (Key: {}): {}", key, e.getMessage()); } }src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustom.java (2)
20-21: 메서드 계약을 명확히 문서화하세요."탈퇴한 사용자 제외"라는 주석이 있지만, 어떤 상태들이 필터링되는지 구체적이지 않습니다.
UserStatus.DELETED만 제외되는지,INACTIVE도 포함되는지 명시하세요. 특히 PR 설명에 따르면 탈퇴 로직이 INACTIVE → DELETED로 변경되었으므로, "Active"의 정의가 중요합니다.🔎 문서화 개선 제안
- // 탈퇴한 사용자 제외하고 ID로 유저 조회 + /** + * ID로 활성 상태의 사용자를 조회 + * @param id 조회할 사용자 ID + * @return 활성 상태(ACTIVE)인 사용자, 비활성(INACTIVE) 및 탈퇴(DELETED) 제외 + */ Optional<User> findActiveUserById(Long id);
23-24: 메서드 계약을 명확히 문서화하세요.이 메서드는 상태 필터링과 차단 필터링을 모두 수행하는 것으로 보입니다. 정확한 동작을 Javadoc으로 문서화해야 합니다.
🔎 문서화 개선 제안
- // 탈퇴 & 차단 모두 필터링하고 ID로 유저 조회 + /** + * ID로 활성 상태이면서 차단하지 않은 사용자를 조회 + * @param id 조회할 사용자 ID + * @param me 현재 사용자 (차단 관계 확인에 사용) + * @return 활성 상태이며 me가 차단하지 않은 사용자, 비활성/탈퇴/차단된 경우 empty + */ Optional<User> findActiveUserExcludingBlocks(Long id, User me);src/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.java (2)
17-18:@Modifying어노테이션 추가를 권장합니다.Spring Data JPA에서
deleteBy...메서드는 자동으로 처리되지만, 명시적으로@Modifying어노테이션을 추가하면 의도가 더 명확해지고, 커스텀 delete 쿼리로 변경할 경우에도 안전합니다.🔎 제안된 수정
+ @Modifying // 차단 해제 void deleteByBlockerAndBlocked(User blocker, User blocked);
24-28: JPQL 쿼리가 잘 작성되었습니다.
fetch join으로 N+1 문제를 방지하고,ACTIVE상태 필터링으로 탈퇴한 사용자를 제외한 점이 좋습니다. 다만, 정렬 기준이 없어서 결과 순서가 일관되지 않을 수 있습니다.🔎 정렬 기준 추가 제안
@Query("SELECT b FROM UserBlock b " + "JOIN FETCH b.blocked " + "WHERE b.blocker = :blocker " + - "AND b.blocked.userStatus = com.hrr.backend.domain.user.entity.enums.UserStatus.ACTIVE") + "AND b.blocked.userStatus = com.hrr.backend.domain.user.entity.enums.UserStatus.ACTIVE " + + "ORDER BY b.createdAt DESC") Slice<UserBlock> findByBlocker(@Param("blocker") User blocker, Pageable pageable);
UserBlock엔티티에createdAt필드가 있다면, 최신 차단 순으로 정렬하는 것이 UX에 더 좋습니다.Pageable의Sort를 사용해도 됩니다.src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
250-259:withdraw()메서드 구현이 깔끔합니다.소프트 삭제 로직이 잘 분리되어 있습니다. TODO 주석의 세션/토큰 정리 로직은 후속 작업으로 추적하시는 것을 권장합니다.
TODO 항목에 대한 이슈를 생성해 드릴까요?
src/main/java/com/hrr/backend/domain/user/repository/UserRepository.java (1)
18-20: 쿼리 로직이 올바릅니다!30일 경과 사용자 조회 로직이 정확합니다. 다만, 메서드명이 복수형이면 더 명확합니다.
🔎 네이밍 개선 제안
- List<User> findUserToDelete(LocalDateTime deleteThreshold); + List<User> findUsersToDelete(LocalDateTime deleteThreshold);
List를 반환하므로 복수형findUsersToDelete가 Spring Data 컨벤션에 더 부합합니다.src/main/java/com/hrr/backend/domain/user/controller/UserBlockController.java (1)
89-91: 일관성을 위해userId를 전달하는 것을 고려해보세요.
blockUser와unblock은userId를 전달하지만,getMyBlockList는User객체를 직접 전달합니다.CustomUserDetails에서 가져온User객체가 현재 영속성 컨텍스트에서 분리(detached)되어 있을 수 있어, 예상치 못한 동작이 발생할 수 있습니다.🔎 제안된 수정
Controller:
- return ApiResponse.onSuccess(SuccessCode.OK, userBlockService.getMyBlockList(customUserDetails.getUser(), pageable)); + return ApiResponse.onSuccess(SuccessCode.OK, userBlockService.getMyBlockList(customUserDetails.getUser().getId(), pageable));Service 메서드 시그니처도 변경:
- public SliceResponseDto<UserBlockResponse> getMyBlockList(User blocker, Pageable pageable) { + public SliceResponseDto<UserBlockResponse> getMyBlockList(Long blockerId, Pageable pageable) { + User blocker = userRepository.findById(blockerId) + .orElseThrow(() -> new GlobalException(ErrorCode.USER_NOT_FOUND));이렇게 하면 모든 메서드가 일관된 패턴을 따르고, 영속성 문제를 방지할 수 있습니다.
src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java (1)
40-43: 사소한 일관성 개선 제안
blocker.getId().equals(blockedId)대신 이미 가지고 있는blockerId를 사용하면 불필요한 getter 호출을 줄일 수 있습니다.🔎 제안된 수정
// 셀프 차단 체크 - if (blocker.getId().equals(blockedId)) { + if (blockerId.equals(blockedId)) { throw new GlobalException(ErrorCode.CANNOT_BLOCK_SELF); }src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java (1)
36-61: 트랜잭션 내 외부 서비스 호출 시 일관성 주의
@Transactional내에서authService.revoke()(소셜 API 호출)와s3Service.deleteFileByKey()(S3 삭제)가 호출됩니다. 만약 이후 단계에서 예외가 발생하면:
- DB 변경은 롤백되지만
- 이미 revoke된 소셜 연결이나 삭제된 S3 파일은 복구되지 않습니다
현재 구조에서는 S3 삭제와 revoke를 먼저 수행하고 있어 "삭제 선행" 원칙은 지켜지고 있습니다. 다만, 실패 시나리오에 대한 로깅이나 보상 트랜잭션(compensation)을 고려해볼 수 있습니다.
🔎 개선 제안: 외부 호출을 트랜잭션 외부로 분리
// 1단계: 외부 서비스 호출 (트랜잭션 외부) public void processPermanentWithdrawal(User user) { // 소셜 연결 해제 (실패해도 계속 진행) try { authService.revoke(user.getId()); } catch (Exception e) { log.warn("소셜 연결 해제 실패 (userId: {}): {}", user.getId(), e.getMessage()); } // S3 삭제 (실패해도 계속 진행) s3Service.deleteFileByKey(user.getProfileImage()); // 2단계: DB 작업 (트랜잭션) completeWithdrawalInTransaction(user); } @Transactional protected void completeWithdrawalInTransaction(User user) { socialAuthRepository.deleteByUser(user); user.completeWithdrawal(); // ... 나머지 로직 }src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.java (2)
88-111: 차단 목록 조회 시 N+1은 아니지만 추가 쿼리 2회 발생
notInBlockList에서 내가 차단한 사용자와 나를 차단한 사용자를 각각 조회합니다. 주석에 "서브쿼리보다 성능상 이점"이라고 되어 있는데, 이는 차단 목록이 적을 때 유효합니다.차단 목록이 수천 건 이상이 되면
IN절 크기 제한(대부분 DB에서 1000개 내외)에 걸릴 수 있습니다. 현재 서비스 규모에서는 문제없어 보이지만, 향후 확장 시 고려해주세요.🔎 대안: 단일 UNION 쿼리로 최적화
// 단일 쿼리로 양방향 차단 ID 조회 List<Long> allBlockIds = queryFactory .select(qUserBlock.blocked.id) .from(qUserBlock) .where(qUserBlock.blocker.eq(me)) .union( queryFactory .select(qUserBlock.blocker.id) .from(qUserBlock) .where(qUserBlock.blocked.eq(me)) ) .fetch();또는 차단 목록이 많아질 경우 EXISTS 서브쿼리 사용을 고려해볼 수 있습니다.
114-120:content.remove()호출 시 리스트 타입 주의
queryFactory.fetch()는 수정 가능한ArrayList를 반환하므로 현재는 안전합니다. 다만, 향후 리팩토링 시 불변 리스트가 전달되면UnsupportedOperationException이 발생할 수 있습니다.방어적 코딩을 원하시면 아래처럼 수정할 수 있습니다:
🔎 방어적 코딩 제안
private Slice<User> checkLastPage(Pageable pageable, List<User> content) { boolean hasNext = content.size() > pageable.getPageSize(); + List<User> result = hasNext + ? content.subList(0, pageable.getPageSize()) + : content; - if (hasNext) { - content.remove(pageable.getPageSize()); - } - return new SliceImpl<>(content, pageable, hasNext); + return new SliceImpl<>(new ArrayList<>(result), pageable, hasNext); }src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
213-218: 쿼리에서 이미 자기 자신 제외 - 중복 필터링
findByNicknameContaining쿼리에서 이미qUser.id.ne(me.getId())로 자기 자신을 제외하고 있습니다. Line 217의 추가 필터링은 불필요한 중복입니다.🔎 중복 필터링 제거
// DTO 변환을 위한 필터링된 사용자 리스트 - List<User> targetUsers = usersSlice.getContent().stream() - .filter(target -> !target.getId().equals(currentUserId)) - .toList(); + List<User> targetUsers = usersSlice.getContent();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
src/main/java/com/hrr/backend/domain/auth/entity/SocialAuth.javasrc/main/java/com/hrr/backend/domain/auth/repository/SocialAuthRepository.javasrc/main/java/com/hrr/backend/domain/auth/service/AuthService.javasrc/main/java/com/hrr/backend/domain/auth/service/SocialUserService.javasrc/main/java/com/hrr/backend/domain/challenge/converter/ChallengeConverter.javasrc/main/java/com/hrr/backend/domain/challenge/repository/ChallengeRepository.javasrc/main/java/com/hrr/backend/domain/comment/controller/CommentController.javasrc/main/java/com/hrr/backend/domain/comment/converter/CommentConverter.javasrc/main/java/com/hrr/backend/domain/follow/dto/FollowListResponseDto.javasrc/main/java/com/hrr/backend/domain/follow/dto/FollowRequestDto.javasrc/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javasrc/main/java/com/hrr/backend/domain/user/controller/UserBlockController.javasrc/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.javasrc/main/java/com/hrr/backend/domain/user/dto/UserBlockResponse.javasrc/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.javasrc/main/java/com/hrr/backend/domain/user/entity/User.javasrc/main/java/com/hrr/backend/domain/user/entity/UserBlock.javasrc/main/java/com/hrr/backend/domain/user/entity/UserChallenge.javasrc/main/java/com/hrr/backend/domain/user/entity/enums/UserStatus.javasrc/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.javasrc/main/java/com/hrr/backend/domain/user/repository/UserChallengeRepository.javasrc/main/java/com/hrr/backend/domain/user/repository/UserRepository.javasrc/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustom.javasrc/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.javasrc/main/java/com/hrr/backend/domain/user/service/UserBlockService.javasrc/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javasrc/main/java/com/hrr/backend/domain/user/service/UserService.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.javasrc/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.javasrc/main/java/com/hrr/backend/global/config/S3Config.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/java/com/hrr/backend/global/s3/S3Service.javasrc/main/java/com/hrr/backend/global/scheduler/UserScheduler.javasrc/main/resources/db/migration/V2.20__add_version_to_user.sqlsrc/main/resources/db/migration/V2.21.1__add_unique_constraint_to_user_block.sqlsrc/main/resources/db/migration/V2.21__add_withdrawn_status_to_user.sqlsrc/main/resources/db/migration/V2.22__migrate_user_status_and_add_constraints.sql
💤 Files with no reviewable changes (1)
- src/main/java/com/hrr/backend/domain/auth/entity/SocialAuth.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/auth/service/SocialUserService.javasrc/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javasrc/main/java/com/hrr/backend/domain/user/entity/User.java
🧬 Code graph analysis (6)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
src/main/java/com/hrr/backend/domain/follow/entity/Follow.java (1)
Entity(13-44)
src/main/java/com/hrr/backend/domain/comment/controller/CommentController.java (2)
src/main/java/com/hrr/backend/domain/comment/service/CommentServiceImpl.java (1)
Service(25-219)src/main/java/com/hrr/backend/domain/comment/service/CommentService.java (2)
CommentService(9-22)updateComment(18-18)
src/main/java/com/hrr/backend/domain/user/controller/UserBlockController.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserBlockResponse.java (1)
RequiredArgsConstructor(9-26)
src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserBlockResponse.java (1)
RequiredArgsConstructor(9-26)
src/main/java/com/hrr/backend/domain/follow/dto/FollowListResponseDto.java (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (2)
user(109-110)user(59-60)
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java (2)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
Slf4j(24-278)src/main/java/com/hrr/backend/global/s3/S3Service.java (1)
Service(19-77)
🔇 Additional comments (34)
src/main/java/com/hrr/backend/domain/challenge/converter/ChallengeConverter.java (1)
87-92: 일관된 디스플레이 닉네임 적용, 잘하셨습니다!
getDisplayNickname()사용으로 비활성화/탈퇴한 사용자의 닉네임이 올바르게 마스킹되어 표시됩니다. PR 전체의 리팩토링 방향과 일치하며 구현이 정확합니다.src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (2)
45-55: ProfileDto 변환 로직이 올바릅니다.다른 사용자의 프로필을 조회할 때
getDisplayNickname()을 사용하여 차단/비활성화된 사용자의 닉네임이 적절히 처리됩니다. 디스플레이 닉네임 리팩토링이 일관되게 적용되었습니다.
142-160: No changes needed — code correctly usesgetDisplayNickname()
getDisplayNickname()은 사용자 상태에 따라 닉네임 표시를 다르게 처리하도록 설계된 메서드입니다. 로그인한 사용자는 항상UserStatus.ACTIVE상태이므로, 실제 닉네임을 반환합니다.이 설계는 의도적이며 방어적입니다:
- 비활성화/탈퇴한 사용자의 닉네임이 실수로 노출되는 것을 방지
- 모든 사용자 DTO(
MyInfoDto,ProfileDto,UpdateUserInfoResponseDto등)에서 일관되게 적용현재 구현이 올바르므로 변경할 사항이 없습니다.
src/main/java/com/hrr/backend/domain/follow/dto/FollowRequestDto.java (1)
34-42: 팔로우 요청 DTO 변환이 정확합니다!팔로우 요청자의 닉네임을
getDisplayNickname()으로 표시하여 차단/비활성화된 사용자가 올바르게 처리됩니다. 전체 리팩토링과 일관성 있게 구현되었습니다.src/main/java/com/hrr/backend/domain/user/entity/UserBlock.java (1)
16-18: 중복 차단 방지가 우수하게 구현되어 있습니다! 다만 접근 방식을 이해하세요.유니크 제약 조건이 올바르게 추가되었으며, UserBlockService의
blockUser()메서드에서 이미 중복 차단을 사전에 검증하고 있습니다(46행:existsByBlockerAndBlocked()확인 후ALREADY_BLOCKED예외 발생).현재 구현의 장점:
- 서비스 레이어에서 예외 발생을 미리 방지하므로 DataIntegrityViolationException 처리가 불필요함
ErrorCode.ALREADY_BLOCKED를 통해 사용자에게 명확한 메시지 제공- 데이터베이스 제약조건은 동시성 경합 발생 시 안전장치 역할
이 방식은 예외 처리보다 예방적 검증을 우선하는 패턴이 더 효율적이라는 Spring 권장사항과 일치합니다.
src/main/java/com/hrr/backend/global/config/S3Config.java (1)
25-31: S3Client 빈 구성이 올바릅니다! 👍S3Presigner와 동일한 방식으로 IAM Role 기반 인증과 리전 설정을 사용하여 일관성 있는 구성입니다.
src/main/java/com/hrr/backend/domain/user/dto/UpdateUserInfoResponseDto.java (1)
33-33: 일관된 displayNickname 패턴 적용! 👍다른 DTO들과 동일하게
getDisplayNickname()을 사용하여 사용자 상태에 따른 표시 로직을 일관되게 적용하고 있습니다.src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
71-76: 차단 시 양방향 팔로우 관계 삭제는 이미 올바르게 구현되었습니다.현재 코드(51-52줄)에서는 이미 양방향 팔로우 관계를 모두 삭제하고 있습니다:
deleteByFollowerAndFollowing(blocker, blocked): blocker → blocked 삭제deleteByFollowerAndFollowing(blocked, blocker): blocked → blocker 삭제한 방향만 삭제하는 문제는 발생하지 않습니다.
Likely an incorrect or invalid review comment.
src/main/java/com/hrr/backend/domain/follow/dto/FollowListResponseDto.java (1)
31-31: getDisplayNickname() 구현이 모든 사용자 상태를 올바르게 처리하고 있습니다.검증 결과, User 엔티티의
getDisplayNickname()메서드는 모든 사용자 상태(ACTIVE, INACTIVE, DELETED)를 적절히 처리하고 있습니다:
- ACTIVE 상태: 실제 닉네임 반환
- INACTIVE/DELETED 상태: "(알 수 없음)" 반환
현재 구현은 비활성/탈퇴 사용자의 개인정보 보호를 위한 일관된 마스킹 전략을 올바르게 적용하고 있으므로, 추가 수정이 필요하지 않습니다.
src/main/java/com/hrr/backend/domain/auth/service/SocialUserService.java (1)
187-189: 메서드 주석 개선 확인!INACTIVE 상태 기반의 재로그인 로직을 정확히 설명하는 주석입니다.
src/main/java/com/hrr/backend/domain/verification/converter/VerificationConverter.java (1)
47-47: 닉네임 마스킹이 모든 DTO에 일관되게 적용되었습니다.4개 메서드(
toFeedDto,toResponseDto,toMyProfileDto,toDetailDto)에서 모두getDisplayNickname()을 사용하고 있으며, User 엔티티의 구현(237-240줄)에서 정상 작동을 확인했습니다.userStatus가 ACTIVE가 아니면 "(알 수 없음)"을 반환하므로 탈퇴/비활성 사용자의 닉네임이 적절히 마스킹됩니다. 👍src/main/java/com/hrr/backend/domain/auth/repository/SocialAuthRepository.java (1)
22-25: 벌크 삭제 구현이 트랜잭션 컨텍스트에서 올바르게 처리됨
@Modifying(clearAutomatically = true)와 JPQL DELETE를 통한 효율적인 구현이 돋보입니다. 확인 결과:
- ✓ 트랜잭션 컨텍스트:
UserDeleteService.processPermanentWithdrawal()이@Transactional로 선언되어 있어 쿼리가 올바른 트랜잭션 내에서 실행됩니다.- ✓ 엔티티 생명주기:
SocialAuth에@PreRemove나 다른 라이프사이클 콜백이 없으므로 JPQL DELETE 사용이 안전합니다.추가 학습 자료: Spring Data JPA의
@Modifying쿼리는 일반적으로 대량 삭제에 매우 효율적이지만, 영속성 컨텍스트 상태를 인식하고 있어야 합니다. 필요시 Spring Data JPA 공식 문서의 수정 쿼리 섹션을 참고하세요.src/main/java/com/hrr/backend/global/scheduler/UserScheduler.java (1)
4-4: 필요한 의존성이 적절하게 추가되었습니다.
UserRepository와User엔티티를 통한 개별 사용자 처리 방식으로의 전환이 깔끔하게 구현되었습니다.Also applies to: 9-10, 21-21
src/main/java/com/hrr/backend/domain/comment/converter/CommentConverter.java (2)
76-76: 상태 검증 로직이 캡슐화되어 더 견고해졌습니다.
isNotActive()메서드 사용으로INACTIVE,DELETED,SUSPENDED등 다양한 비활성 상태를 일관되게 처리할 수 있게 되었습니다. PR 목표에 부합하는 좋은 변경입니다.
85-85: 표시용 닉네임 조회 메서드로 일관성이 확보되었습니다.
getDisplayNickname()사용은 PR의 가이드라인을 정확히 따르고 있으며, 차단/비활성/탈퇴 사용자의 닉네임 표시 로직이 중앙화되어 유지보수가 용이합니다. 👍src/main/java/com/hrr/backend/global/s3/S3Service.java (1)
10-12: S3 파일 삭제를 위한 의존성이 적절하게 추가되었습니다.
S3Client와 로깅을 통해 삭제 작업의 관찰성(observability)이 확보되었습니다.Also applies to: 21-21, 24-24
src/main/resources/db/migration/V2.21.1__add_unique_constraint_to_user_block.sql (1)
1-26: 멱등성을 보장하는 안전한 마이그레이션입니다.
user_block테이블에(blocker_id, blocked_id)조합의 유니크 제약조건을 추가하여 중복 차단을 방지합니다. 다음 사항들이 훌륭합니다:
- ✅
information_schema검사로 재실행 시 오류 방지- ✅ 명확한 제약조건 이름 (
uk_blocker_blocked)- ✅ 비즈니스 규칙(한 사용자가 같은 대상을 중복 차단 불가)의 DB 레벨 강제
이는 애플리케이션 레벨 검증과 함께 이중 안전장치를 제공합니다. 🎯
src/main/java/com/hrr/backend/domain/comment/controller/CommentController.java (2)
9-9: 의존성 추가가 적절합니다.
UserBlockService임포트가 새로운 차단 기능을 위해 추가되었습니다.
36-36: 의존성 주입이 올바르게 구현되었습니다.
final필드와@RequiredArgsConstructor를 통한 생성자 주입 패턴이 적절히 사용되었습니다.src/main/java/com/hrr/backend/domain/user/controller/UserController.java (1)
77-80: 구현이 인터페이스 변경과 일치합니다.
getUserProfile호출 시currentUserId대신User객체를 직접 전달하도록 변경되었습니다. 이는 서비스 인터페이스 변경과 일관성 있게 구현되었으며, 불필요한 재조회를 방지합니다.src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
10-13: 시그니처 변경이 모든 계층에서 일관되게 적용되었습니다.인터페이스, 구현체(
UserServiceImpl), 호출부(UserController) 모두에서User currentUser파라미터가 일관되게 사용되고 있습니다. 특히 구현부에서 사용자의 차단 상태를 필터링할 때 (findActiveUserExcludingBlocks(userId, currentUser)) 그리고 팔로우 관계를 확인할 때 (checkIfFollowing(currentUser.getId(), userId)) User 객체를 적절히 활용하고 있으며, 컨트롤러에서도 인증 정보에서 User 객체를 추출하여 올바르게 전달하고 있습니다.ID만 전달하는 방식보다 User 객체를 전달함으로써 서비스 레이어에서 차단 여부, 권한 등 더 풍부한 컨텍스트를 활용할 수 있으며, 코드의 의도도 명확해집니다. 잘 구현되었습니다. ✓
src/main/resources/db/migration/V2.22__migrate_user_status_and_add_constraints.sql (1)
1-36: 마이그레이션 로직이 안전하고 올바르게 구현되었습니다.이 마이그레이션은 V2.21의 실험적 상태값을 정리하고 Java 코드와 DB 스키마를 정렬합니다.
검증 완료:
- ✅ 마이그레이션 순서 의존성: Flyway의 버전 번호 시스템이 자동으로 V2.21 → V2.22 순서 보장 (별도 검증 불필요)
- ✅ 코드베이스 일관성: WITHDRAWN_COMPLETED는 마이그레이션 파일에만 존재하며 Java 코드에서 사용되지 않음 (V2.21이 추가했으나 미사용)
- ✅ 데이터 무결성: 마이그레이션 순서 (DELETED→INACTIVE, WITHDRAWN_COMPLETED→DELETED, NULL→ACTIVE)가 데이터 손실 없이 올바르게 처리됨
- ✅ 스키마 정렬: 최종 ENUM('ACTIVE', 'INACTIVE', 'SUSPENDED', 'DELETED')이 Java UserStatus 정의와 완벽하게 일치
선택적 권장사항:
마이그레이션 실패 시 복구 절차를 문서화하면 운영 안정성을 높일 수 있습니다. 스테이징 환경에서 사전 테스트 시 현재 DB의 status 값 분포를 확인하면(
SELECT status, COUNT(*) FROM user GROUP BY status;) 더욱 안심할 수 있습니다.src/main/java/com/hrr/backend/domain/user/dto/UserBlockResponse.java (1)
11-25: 깔끔한 DTO 구현입니다! 👍
getDisplayNickname()사용이 PR 가이드라인에 잘 맞습니다. 다만,isBlocked필드가 차단 목록 컨텍스트에서 항상true이므로, 이 필드가 API 응답에서 실제로 필요한지 고려해보세요. 현재로서는 정보성 용도로 유용할 수 있습니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
24-26: 에러 코드 추가 잘 하셨습니다!기존 패턴과 일관성을 유지하면서 차단 관련 에러 코드를 추가했네요. HTTP 409 CONFLICT 상태 코드도 적절합니다.
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
65-66:getDisplayName()/getDisplayNickname()사용 변경 확인!PR 가이드라인에 따라 비활성화/탈퇴 사용자의 정보 마스킹을 위해 display 메서드를 사용한 점이 좋습니다.
Also applies to: 122-123, 170-171, 209-210
src/main/java/com/hrr/backend/domain/user/controller/UserBlockController.java (1)
48-51: 차단/해제 엔드포인트가 잘 구현되었습니다!Swagger 문서화와 입력 검증이 잘 되어있습니다.
Also applies to: 64-67
src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java (3)
32-60: 차단 로직이 잘 구현되었습니다!양방향 팔로우 삭제와 기존 차단 확인 등 엣지 케이스를 잘 처리했습니다. 주석에 명시된 대로 탈퇴 여부와 관계없이 차단을 허용하는 것도 합리적인 설계입니다.
78-87:getMyBlockList메서드 확인로직은 정확하지만, 앞서 Controller 리뷰에서 언급한 것처럼
User객체 대신Long blockerId를 받아 서비스 내에서 조회하는 것이 영속성 관리 측면에서 더 안전합니다.
89-99:blockUserByCommentId로 댓글 작성자 차단 기능 좋습니다!기존
blockUser로직을 재사용한 점이 DRY 원칙에 부합합니다. 삭제된 댓글에 대한 처리는commentRepository.findById에서 자연스럽게 처리될 것으로 보입니다.src/main/java/com/hrr/backend/domain/user/entity/User.java (2)
180-190: 재로그인 로직 LGTM!ACTIVE 상태 체크 후 조기 리턴하고, 상태 복원과
deletedAt초기화를 깔끔하게 처리했습니다. 👍
237-250: 표시용 메서드 잘 구현되었습니다!
getDisplayNickname()과getDisplayName()이 일관된 패턴으로 구현되어 있고, PR 목표에 맞게 비활성/탈퇴 사용자의 정보를 마스킹합니다. 다른 파일들(FollowListResponseDto, CommentConverter 등)에서도 적절히 활용되고 있네요.src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.java (1)
28-29: static QUser/QUserBlock 선언 시 주의
static필드로 Querydsl Q-클래스를 선언하는 것은 일반적인 패턴이지만, 멀티스레드 환경에서 안전합니다. Q-클래스는 불변이므로 문제없습니다. 👍src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
59-68: 프로필 조회 시 차단/탈퇴 필터링 적용 LGTM!
findActiveUserExcludingBlocks를 사용하여 비활성 사용자와 차단 관계인 사용자를 한 번에 필터링합니다. PR 목표에 맞게 잘 구현되었습니다.
240-240: 검색 결과에서getDisplayNickname()사용 적절검색 결과 DTO에서
getDisplayNickname()을 사용하는 것은 적절합니다. 쿼리에서 이미 ACTIVE 사용자만 필터링하므로 실제로는 항상 실제 닉네임이 반환됩니다.
src/main/java/com/hrr/backend/domain/auth/service/SocialUserService.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/comment/controller/CommentController.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustom.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java
Show resolved
Hide resolved
| // 상태가 INACTIVE 이고, deletedAt 이 30일 이전인 사용자 조회 | ||
| List<User> usersToClean = userRepository | ||
| .findUserToDelete(threshold); | ||
|
|
||
| for (User user : usersToClean) { | ||
| try { | ||
| userDeleteService.processPermanentWithdrawal(user); | ||
| } catch (Exception e) { | ||
| // 특정 사용자 처리 실패 시 로그를 남기고 다음 사용자 진행 | ||
| log.error("탈퇴 회원 정리 중 오류 발생: {}", user.getId(), e); | ||
| } | ||
| } | ||
| log.info("탈퇴 회원 정리가 완료되었습니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.java" | grep -E "(UserRepository|UserDeleteService)" | head -20Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 393
🏁 Script executed:
rg -n "findUserToDelete" --type java -A 5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 870
🏁 Script executed:
rg -n "processPermanentWithdrawal" --type java -B 3 -A 15Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 3165
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 4156
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 2541
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/user/repository/UserRepository.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 984
🏁 Script executed:
rg -B 5 -A 10 "findUserToDelete" --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 2395
대규모 사용자 정리 시 메모리와 트랜잭션 오버헤드 문제를 반드시 개선하세요.
다음 세 가지 구조적 문제가 확인되었습니다:
-
메모리 오버플로우 위험:
findUserToDelete()는 쿼리에 LIMIT이 없어 조건을 만족하는 모든 사용자를 메모리에 로드합니다. 만약 30일 이상 경과한 탈퇴 사용자가 10,000명이면 한 번에 전부 메모리에 올라옵니다.- 해결책:
Pageable을 매개변수로 추가하여 배치 단위(예: 500명)로 나누어 처리하세요.
- 해결책:
-
트랜잭션 설계 안티패턴:
processPermanentWithdrawal()은@Transactional로 감싸져 있으면서authService.revoke(),s3Service.deleteFileByKey()같은 외부 I/O 작업을 포함합니다. 이는 트랜잭션 락 시간을 불필요하게 증가시키며, 타임아웃이나 연결 풀 고갈 위험을 초래합니다.- 해결책: 외부 I/O 작업을 트랜잭션 외부로 분리하거나, 메서드 단위가 아닌 특정 DB 작업 구간만
@Transactional로 제한하세요.
- 해결책: 외부 I/O 작업을 트랜잭션 외부로 분리하거나, 메서드 단위가 아닌 특정 DB 작업 구간만
-
순차 처리의 성능 병목: 루프 내에서 사용자 1명당 1개의 독립 트랜잭션 + 여러 DB 쿼리 + 외부 서비스 호출이 발생하므로, 사용자 수에 선형으로 시간이 증가합니다.
- 해결책: 배치 처리로 전환하고, 배치 크기를 모니터링하여 새벽 4시 스케줄 내에 완료되도록 최적화하세요.
관련 Spring 공식 문서 참고: Spring Batch (배치 처리), @Transactional의 올바른 사용
…hrr-server into feat/173-block-and-withdraw # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/user/entity/User.java (1)
215-227:socialAuth삭제 전략의 일관성을 확인해주세요
completeWithdrawal()에서는socialAuth를 명시적으로 null로 설정하지 않지만,UserDeleteService.processPermanentWithdrawal()에서는socialAuthRepository.deleteByUser(user)로 명시적 삭제를 수행합니다.현재 접근 방식이 의도적이라면 문제없지만, 이전 리뷰에서 지적된 것처럼 두 가지 삭제 메커니즘(orphanRemoval vs 명시적 삭제)이 혼재되어 있는지 확인이 필요합니다.
#!/bin/bash # socialAuth 관련 삭제 로직 확인 rg -n "socialAuth|deleteByUser" --type java -B 2 -A 2
🧹 Nitpick comments (2)
src/main/resources/db/migration/V2.22__add_withdrawn_status_to_user.sql (1)
1-26: 임시 상태값의 목적 문서화를 권장합니다
WITHDRAWN_COMPLETED가 임시 전환 상태임을 라인 16의 주석에서 언급하고 있지만, 파일 상단에 전체적인 마이그레이션 의도를 설명하는 주석을 추가하면 향후 유지보수에 도움이 됩니다.🔎 제안하는 문서화 예시
+-- ===================================================================== +-- WITHDRAWN_COMPLETED는 V2.21에서 V2.23으로의 마이그레이션을 위한 임시 상태입니다. +-- V2.23 마이그레이션에서 모든 데이터가 DELETED로 변환되고 ENUM에서 제거됩니다. +-- 실제 애플리케이션 코드(UserStatus enum)에서는 이 값을 사용하지 않습니다. +-- ===================================================================== DELIMITER $$src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
249-259: TODO 주석의 세션/토큰 정리 로직 추적이 필요합니다탈퇴 로직이 잘 분리되었습니다. 다만, Line 258의 TODO(
리프레시 토큰 등 세션 정보 삭제)는 보안상 중요한 항목이므로 이슈로 등록하여 추적하는 것을 권장합니다. 탈퇴 후에도 기존 토큰이 유효하면 보안 취약점이 될 수 있습니다.이슈를 생성하여 TODO를 추적할까요?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.javasrc/main/java/com/hrr/backend/domain/comment/controller/CommentController.javasrc/main/java/com/hrr/backend/domain/user/entity/User.javasrc/main/java/com/hrr/backend/domain/user/entity/UserChallenge.javasrc/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustom.javasrc/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.javasrc/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/resources/db/migration/V2.22.1__add_unique_constraint_to_user_block.sqlsrc/main/resources/db/migration/V2.22__add_withdrawn_status_to_user.sqlsrc/main/resources/db/migration/V2.23__migrate_user_status_and_add_constraints.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustom.java
- src/main/java/com/hrr/backend/domain/comment/controller/CommentController.java
- src/main/java/com/hrr/backend/domain/user/entity/UserChallenge.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javasrc/main/java/com/hrr/backend/domain/user/entity/User.javasrc/main/java/com/hrr/backend/domain/auth/service/AuthService.java
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/entity/User.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
Builder(90-161)
🔇 Additional comments (14)
src/main/java/com/hrr/backend/global/response/ErrorCode.java (2)
24-26: 차단 기능 에러 코드 추가, 일관성 유지 👍신고 기능 에러 코드(ALREADY_REPORTED_USER, CANNOT_REPORT_SELF)와 동일한 패턴으로 구현되어 일관성이 우수합니다. HttpStatus.CONFLICT 선택도 상태 충돌을 나타내는 데 적절하며, 코드 번호(USER4097-4099)도 순차적으로 잘 할당되었습니다.
24-26: 제거된 AUTH_WITHDRAWAL_PERIOD_RESTRICTION이 코드베이스 전체에서 완벽하게 정리됨 확인
AUTH_WITHDRAWAL_PERIOD_RESTRICTION에러 코드의 잔여 참조를 검색한 결과, 코드베이스 어디에서도 참조하고 있지 않은 것으로 확인되었습니다. 서비스/컨트롤러 레이어는 물론 전체 코드에서 해당 에러 코드가 제거되어 있으므로, 제거 작업이 깔끔하게 완료된 상태입니다.이는 soft-delete 로직으로의 전환이 일관성 있게 구현되었음을 의미합니다. 30일 재가입 제한에서 재활성화 방식으로 전환된 만큼, 더 이상 필요 없는 에러 코드를 완벽히 정리한 것은 좋은 실천 사례입니다. 🎯
src/main/resources/db/migration/V2.23__migrate_user_status_and_add_constraints.sql (1)
1-36: 마이그레이션 로직이 명확하고 안전하게 구성되어 있습니다 👍저장 프로시저를 활용한 마이그레이션 패턴이 좋습니다. 데이터 이관 순서(DELETED → INACTIVE → WITHDRAWN_COMPLETED → DELETED)가 논리적으로 올바르며,
IF EXISTS체크로 멱등성도 확보했네요.src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (2)
65-66:getDisplayName()/getDisplayNickname()사용이 적절합니다비활성화/탈퇴 사용자의 이름을 마스킹하는 PR 목표에 맞게 수정되었습니다.
261-278: 소셜 연동 해제 로직이 깔끔하게 분리되었습니다
switch문으로 소셜 타입별 처리가 명확합니다.revoke()메서드는UserDeleteService.processPermanentWithdrawal()에서만 호출되고 있는데, 두 서비스가 서로 다른 패키지(com.hrr.backend.domain.auth.service,com.hrr.backend.domain.user.service)에 위치하고 있어 접근 제한자를protected나 package-private으로 변경할 수 없습니다. 도메인 간 서비스 호출에서는public접근 제한자가 적절하므로, 현재 상태를 유지하셔도 문제없습니다.src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (3)
59-68: 차단/탈퇴 사용자 필터링이 잘 적용되었습니다
findActiveUserExcludingBlocks를 활용하여 차단된 사용자와 비활성 사용자를 조회에서 제외하는 로직이 깔끔하게 구현되었습니다.
206-258: 검색 결과에서getDisplayNickname()사용이 적절합니다차단/탈퇴 사용자 마스킹 정책에 맞게
target.getDisplayNickname()을 사용하고 있습니다. N+1 문제 해결을 위한 bulk 조회 패턴도 좋습니다.
294-302: 라인 297에서getNickname()사용이 맞습니다. 재검토 불필요합니다.
getNickname()은 데이터베이스에 실제로 저장된 닉네임값을 비교하기 위해 사용되어야 합니다.getDisplayNickname()은 사용자 상태(userStatus)에 따라 "(알 수 없음)"을 반환할 수 있는 UI 계층 메서드이므로, 데이터 업데이트 로직에서는 부적절합니다. 현재 구현에서:
- 라인 297: 실제 저장된 닉네임과 비교 (정상)
- 라인 298: DB 중복 검사는 저장된 닉네임값 기반 (정상)
UpdateUserInfoResponseDto의 응답값:getDisplayNickname()사용 (정상)코드가 의도된 동작을 올바르게 수행하고 있습니다.
src/main/java/com/hrr/backend/domain/user/repository/UserRepositoryCustomImpl.java (3)
42-57: QueryDSL 기반 커스텀 쿼리가 잘 구현되었습니다차단 관계와 활성 상태 필터링을 분리된 쿼리로 처리하여 서브쿼리 대비 성능상 이점을 확보했습니다. Slice 패턴(+1 조회)도 올바르게 적용되었네요.
82-85: 주석 오타가 수정되었습니다 ✓이전 리뷰에서 지적된 "탈톼" 오타가 "탈퇴"로 수정된 것으로 보입니다.
113-120: [rewritten comment]
[classification tag]src/main/java/com/hrr/backend/domain/user/entity/User.java (3)
88-92: 세미콜론 중복 오타가 수정되었습니다 ✓이전 리뷰에서 지적된
UserStatus.ACTIVE;;이슈가 해결되었습니다.
174-189: 상태 전환 로직이 명확합니다
withdraw()→ INACTIVE,reLogin()→ ACTIVE 상태 전환이 PR 목표(30일 유예 기간 소프트 딜리트)에 맞게 잘 구현되었습니다.reLogin()에서 이미 ACTIVE인 경우 early return 처리도 좋습니다.
234-248: 마스킹 헬퍼 메서드가 깔끔합니다
getDisplayNickname()과getDisplayName()이 상태 기반 마스킹을 캡슐화하여 여러 곳에서 일관되게 사용할 수 있습니다. PR 목표에서 강조한 "닉네임/이름 조회 시 getDisplayNickname(), getDisplayName() 사용" 지침이 잘 반영되었네요.
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java
Show resolved
Hide resolved
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java
Show resolved
Hide resolved
src/main/resources/db/migration/V2.22.1__add_unique_constraint_to_user_block.sql
Show resolved
Hide resolved
There was a problem hiding this 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/resources/db/migration/V2.22.1__add_unique_constraint_to_user_block.sql (1)
7-13: 이전 리뷰 피드백이 잘 반영되었습니다! 👍중복 데이터 제거 로직이 유니크 제약조건 추가 전에 먼저 실행되도록 수정되어, 기존에 중복 데이터가 있더라도 마이그레이션이 안전하게 완료됩니다.
b1.id > b2.id조건으로 가장 오래된 레코드를 유지하는 방식도 적절합니다.다만,
INNER JOIN ... WHERE구문 대신 명시적인ON절을 사용하면 가독성이 더 좋아질 수 있습니다:🔎 가독성 개선 제안 (선택 사항)
DELETE b1 FROM user_block b1 - INNER JOIN user_block b2 - WHERE b1.id > b2.id - AND b1.blocker_id = b2.blocker_id - AND b1.blocked_id = b2.blocked_id; + INNER JOIN user_block b2 + ON b1.blocker_id = b2.blocker_id + AND b1.blocked_id = b2.blocked_id + WHERE b1.id > b2.id;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.javasrc/main/resources/db/migration/V2.22.1__add_unique_constraint_to_user_block.sql
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:06:38.145Z
Learnt from: yerinchun
Repo: Hrr-HabbitRoutineResult/hrr-server PR: 26
File: src/main/java/com/hrr/backend/domain/user/entity/User.java:42-44
Timestamp: 2025-11-06T08:06:38.145Z
Learning: In the hrr-server project User entity (src/main/java/com/hrr/backend/domain/user/entity/User.java), the password field is nullable because the application uses social login (OAuth), and social login users do not have passwords stored in the database.
Applied to files:
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java (2)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
Slf4j(24-280)src/main/java/com/hrr/backend/global/s3/S3Service.java (1)
Service(19-77)
🔇 Additional comments (6)
src/main/resources/db/migration/V2.22.1__add_unique_constraint_to_user_block.sql (2)
15-26: 멱등성(idempotency) 처리가 잘 구현되어 있습니다.
information_schema.STATISTICS를 조회하여 인덱스 존재 여부를 먼저 확인하고, 없는 경우에만 제약조건을 추가하는 방식은 마이그레이션 재실행 시에도 안전합니다. Flyway 마이그레이션 베스트 프랙티스를 잘 따르고 있네요.📚 참고: Flyway 공식 문서 - Idempotent Migrations
31-33: 프로시저 정리 로직이 깔끔합니다.프로시저 실행 후 즉시 삭제하여 데이터베이스에 불필요한 객체가 남지 않도록 처리했습니다. 일회성 마이그레이션 프로시저의 올바른 패턴입니다.
src/main/java/com/hrr/backend/domain/user/service/UserDeleteService.java (4)
36-43: 이전 리뷰 피드백이 잘 반영되었습니다 👍소셜 연동 해제 실패 시에도 내부 데이터 정리를 계속 진행하는 전략이 구현되었네요. 외부 플랫폼이 응답하지 않거나 이미 연결이 끊어진 상태더라도 사용자의 탈퇴 프로세스가 중단되지 않도록 보장합니다.
46-50: 데이터 정리 단계가 안전하게 구현되었습니다
deleteByUser(user): 소셜 인증 정보를 DB에서 제거deleteFileByKey: S3Service 내부에서 null/blank/http 검증 및 예외 처리를 수행하여 안전하게 이미지 삭제각 단계의 실패가 전체 프로세스를 중단시키지 않도록 설계된 점이 우수합니다.
55-66: 챌린지 정리 로직 개선 확인 ✅이전 리뷰에서 요청한 개선사항이 모두 반영되었습니다:
decreaseCurrentParticipantCount의 반환값을 검증하여 업데이트 실패 시 로깅- 로그 메시지가 정확하게
UserChallengeId와uc.getId()값을 일치시킴일부 챌린지 업데이트가 실패하더라도(예: 이미 참가 인원이 0인 경우) 전체 탈퇴 프로세스를 중단하지 않는 설계가 적절합니다.
52-53: PII 마스킹이 제대로 구현되어 있습니다.
completeWithdrawal()메서드를 확인해보니 모든 개인정보가 적절히 처리되고 있습니다. 이름은 "탈퇴한 사용자"로 마스킹되고, 닉네임, 이메일, 전화번호, 비밀번호, 프로필 이미지는 모두 null로 삭제되며, 상태도DELETED로 올바르게 설정됩니다. PR 요구사항을 잘 충족하고 있습니다.
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
기존) 한 번 탈퇴 시 30일 동안 재로그인 및 재가입이 불가하며, 30일 뒤에 모든 데이터 삭제
변경) 탈퇴 시 비활성화 상태로 변경(UserStatus.INACTIVE)되고, 30일 안에 재로그인 시 활성화. 30일 경과 시 탈퇴 상태로 변경(UserStatus.DELETED)되며 데이터는 남아있으나 사용자와의 연결이 끊겨 이후 재가입 시 새로운 계정으로 시작
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
"사용자 차단"과 "회원 비활성화 및 탈퇴"가 구현되었습니다. 지금까지 구현된 API에서 특정 사용자의 데이터를 활용했을 경우 기획 의도에 따라 '내가 차단했는지', '나를 차단했는지', '비활성화 상태인지', '탈퇴 상태인지' 등 차단과 탈퇴에 대한 모든 검증을 거치셔야 합니다. 이는 이후로 구현될 모든 API에도 마찬가지로 적용됩니다.
예를 들어 댓글 같은 경우 부모-자식 간의 관계가 무너지기 때문에 필터링이 아닌 마스킹 방식으로 했고, 차단 목록 조회 같은 경우 비활성화 및 탈퇴 유저만 필터링 하고 나를 차단한 사용자는 필터링 하지 않았습니다.
또한 S3 이미지 삭제 로직을 구현해놨습니다. 활용 방식을 참고하셔서 사진 업로드 관련 기능에서 수정이나 삭제가 필요할 경우 S3에 업로드 되어있는 기존 파일을 먼저 삭제해주셔야 합니다. 수정 또한 삭제 후 새로운 파일을 업로드하는 방식입니다.
📎 참고 자료
Summary by CodeRabbit
새로운 기능
버그 수정 / 개인정보 보호
Chores
✏️ Tip: You can customize this high-level summary in your review settings.