-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 사용자, 팔로우 관련 API에 차단/탈퇴 로직 추가 #213
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팔로우·프로필·인증 API와 서비스에 현재 사용자 컨텍스트와 차단/비활성 검사 로직을 도입했습니다. FollowRepository의 팔로워/팔로잉 쿼리에 Changes
Sequence DiagramsequenceDiagram
participant Client as 클라이언트
participant Ctrl as UserController
participant FList as FollowListService
participant URepo as UserRepository
participant BlockRepo as UserBlockRepository
participant FRepo as FollowRepository
participant DB as Database
Client->>Ctrl: GET /follow/{userId}/followers?page=0
Ctrl->>Ctrl: 인증 사용자 추출
Ctrl->>FList: getFollowers(userId, currentUser, pageable)
FList->>URepo: findById(userId)
URepo-->>FList: targetUser
FList->>URepo: findById(currentUser.id)
URepo-->>FList: currentUser
alt 타겟이 요청자 차단
FList->>BlockRepo: existsByBlockerAndBlocked(targetUser, currentUser)
BlockRepo-->>FList: true
FList-->>Ctrl: USER_NOT_FOUND
else 요청자가 타겟 차단
FList->>BlockRepo: existsByBlockerAndBlocked(currentUser, targetUser)
BlockRepo-->>FList: true/false
alt 차단함
FList-->>Ctrl: 빈 Slice 반환
else
FList->>FRepo: findFollowersByUserId(userId, currentUserId, pageable)
FRepo->>DB: JPQL (ACTIVE + UserBlock 서브쿼리 제외)
DB-->>FRepo: 결과
FRepo-->>FList: 필터된 목록
FList-->>Ctrl: 응답 반환
end
end
Ctrl-->>Client: JSON 응답
sequenceDiagram
participant Client as 클라이언트
participant Ctrl as UserController
participant FService as FollowService
participant URepo as UserRepository
participant BlockRepo as UserBlockRepository
participant FRepo as FollowRepository
Client->>Ctrl: POST /follow/{followedUserId}
Ctrl->>Ctrl: 인증 사용자 추출
Ctrl->>FService: followUser(followedUserId, currentUser)
FService->>URepo: findById(followedUserId)
URepo-->>FService: followedUser
alt 대상 없음 또는 비활성/타겟이 요청자 차단
FService-->>Ctrl: USER_NOT_FOUND
else 요청자가 대상 차단
FService->>BlockRepo: existsByBlockerAndBlocked(currentUser, followedUser)
BlockRepo-->>FService: true
FService-->>Ctrl: CANNOT_FOLLOW_BLOCKED_USER
else 정상
FService->>FRepo: createFollow(currentUser, followedUser)
FRepo-->>FService: follow 생성
FService-->>Ctrl: 성공 응답
end
Ctrl-->>Client: 결과 반환
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🤖 Fix all issues with AI agents
In @src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java:
- Line 130: In the getFollowings logic of FollowListService you’re calling the
wrong repository method: replace the call to
followRepository.findFollowersByUserId(userId, currentUserId, pageable) with
followRepository.findFollowingsByUserId(userId, currentUserId, pageable) so the
service returns the user’s followings instead of followers; locate the call
inside the getFollowings method of FollowListService and update the repository
method reference accordingly.
In @src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java:
- Around line 50-65: The current unblock logic unconditionally decrements
follower/following counts when deleting follow records, which can mis-count if
the follow was in PENDING state; change the two blocks that call
followRepository.findByFollowerIdAndFollowingId(...) to check the follow's
status and only call blocker.decrementFollowingCount(),
blocked.decrementFollowerCount(), blocked.decrementFollowingCount(), or
blocker.decrementFollowerCount() when follow.getStatus() ==
FollowStatus.APPROVED; also add the import for
com.hrr.backend.domain.follow.entity.enums.FollowStatus and ensure you only
delete the follow after evaluating status (or delete regardless but
conditionally adjust counts).
🧹 Nitpick comments (7)
src/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.java (1)
29-37: LGTM! 상호 차단 관계 확인을 위한 유틸리티 메서드 👍null 체크와 양방향 차단 확인 로직이 깔끔하게 구현되었습니다.
다만,
FollowService와VerificationServiceImpl에서는 이 메서드 대신existsByBlockerAndBlocked를 직접 두 번 호출하고 있습니다. 일관성을 위해 해당 서비스들에서도 이isBlockedRelation메서드를 사용하는 것을 고려해 보세요.src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (1)
66-74: 차단 관계 검증 로직 잘 구현되었습니다 👍
- 상대방이 나를 차단한 경우
USER_NOT_FOUND를 반환하여 정보 노출을 방지합니다.- 내가 상대방을 차단한 경우 명확한 에러 메시지를 제공합니다.
한 가지 사소한 점: Line 73의
// 해당 에러코드 추가 필요주석은 이미ErrorCode.java에 추가되었으므로 제거해 주세요.🔎 주석 제거
// 내가 차단한 유저인 경우 체크 -> 팔로우 불가 if (userBlockRepository.existsByBlockerAndBlocked(currentUser, followedUser)) { - throw new GlobalException(ErrorCode.CANNOT_FOLLOW_BLOCKED_USER); // 해당 에러코드 추가 필요 + throw new GlobalException(ErrorCode.CANNOT_FOLLOW_BLOCKED_USER); }src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
20-20: Wildcard import 대신 명시적 import 권장
org.springframework.data.domain.*대신 필요한 클래스만 명시적으로 import하면 코드 가독성과 IDE 지원이 향상됩니다.import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.domain.SliceImpl;src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
90-117: 차단/비활성 사용자 접근 제어 로직 LGTM!타인 프로필 조회 시 탈퇴/비활성 체크, 상대방의 차단 여부 체크, 내가 상대를 차단한 경우 빈 리스트 반환 로직이 PR 목표에 맞게 잘 구현되었습니다. 👍
단, Line 115에서
java.util.Collections.emptyList()를 사용하고 있는데, Line 5에서 이미Collections가 import되어 있으므로 간결하게Collections.emptyList()로 변경할 수 있습니다.🔎 제안된 수정
- if (userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser)) { - return new SliceResponseDto<>(new SliceImpl<>(java.util.Collections.emptyList(), PageRequest.of(page, size), false)); - } + if (userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser)) { + return new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false)); + }
150-154: userChallengeId 조회 시 userId 사용 확인 필요Line 151-154에서
userChallengeRepository.findByUserIdAndChallengeId(userId, challengeId)를 호출하고 있습니다. 이 메서드가 내 챌린지 조회(/me/challenge/ongoing)와 타인 챌린지 조회(/{userId}/challenge/ongoing) 양쪽에서 공용으로 사용되므로, 타인 조회 시에는targetUser.getId()를 사용하는 것이 명확합니다.현재 로직상
userId가 타인 조회 시 타인의 ID이므로 동작은 올바르지만, 가독성을 위해targetUser.getId()로 통일하는 것을 권장합니다.🔎 제안된 수정
Long userChallengeId = userChallengeRepository - .findByUserIdAndChallengeId(userId, challengeId) + .findByUserIdAndChallengeId(targetUser.getId(), challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) .getId();src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
107-123: 중복 코드 패턴: 차단/탈퇴 검증 로직 추출 고려
getFollowers(Lines 46-62)와getFollowings(Lines 107-123)에서 동일한 차단/탈퇴 검증 로직이 반복됩니다. 향후 유지보수를 위해validateTargetUserAccess(User targetUser, User me, Pageable pageable)같은 private 헬퍼 메서드로 추출하는 것을 권장합니다.src/main/java/com/hrr/backend/domain/user/controller/UserController.java (1)
119-137: 타인 챌린지 조회 API - currentUser 컨텍스트 전달 LGTM!차단 상태 검증을 위해
CustomUserDetails파라미터가 추가되고 서비스에 올바르게 전달됩니다.API 문서(Swagger)에 차단된 사용자 조회 시 404 또는 빈 리스트가 반환된다는 설명을
description에 추가하면 API 사용자에게 도움이 될 것입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javasrc/main/java/com/hrr/backend/domain/follow/service/FollowListService.javasrc/main/java/com/hrr/backend/domain/follow/service/FollowService.javasrc/main/java/com/hrr/backend/domain/user/controller/UserController.javasrc/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.javasrc/main/java/com/hrr/backend/domain/user/service/UserBlockService.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/service/VerificationService.javasrc/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java (1)
Service(22-113)
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.java (1)
VerificationResponseDto(17-197)
🔇 Additional comments (12)
src/main/java/com/hrr/backend/domain/user/service/UserService.java (1)
16-21: LGTM! 차단 상태 검증을 위한 인터페이스 변경 ✓
currentUser파라미터 추가로 다른 사용자의 진행 중 챌린지 조회 시 차단 관계 확인이 가능해졌습니다.src/main/java/com/hrr/backend/domain/verification/service/VerificationService.java (1)
80-85: LGTM! 명확한 파라미터 네이밍과 컨텍스트 추가 👌
userId→targetUserId로의 변경과User me파라미터 추가로 메서드의 의도가 더 명확해졌습니다. 차단 관계 기반 접근 제어를 위한 좋은 설계입니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
158-158: LGTM! 차단한 유저 팔로우 방지를 위한 에러 코드 추가 ✓기존 FOLLOW 에러 코드 컨벤션과 일관성을 유지하며 적절하게 추가되었습니다.
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (3)
601-608: 차단한 유저 조회 시isPublic=true반환이 적절한지 검토 필요현재 내가 상대방을 차단한 경우
isPublic=true와 빈 목록을 반환합니다. 이는 클라이언트 입장에서 "공개 계정인데 인증 기록이 없다"로 해석될 수 있습니다.의도한 동작이라면 괜찮지만, 차단 관계임을 명시하고 싶다면 별도의 플래그나
isPublic=null등을 고려해 볼 수 있습니다. PR 목적에 따르면 "records of users I have blocked return an empty list"이므로 현재 구현이 요구사항과 일치합니다.
605-605:getDisplayNickname()vsgetNickname()사용의 일관성 확인Line 605에서는
getDisplayNickname()을, Line 615와 634에서는getNickname()을 사용합니다.PR 목적에 "차단/탈퇴 시 프로필 마스킹"이 언급되어 있어 의도된 동작으로 보이지만,
getDisplayNickname()이 어떤 값을 반환하는지(예: "(알 수 없음)") 확인하고 주석의 설명과 일치하는지 검토해 주세요.
597-599: 보안을 고려한 좋은 에러 처리 패턴 👍비활성 사용자와 나를 차단한 사용자를 동일한
USER_NOT_FOUND로 처리하여 차단 여부를 추론할 수 없게 했습니다. 이는 사용자 프라이버시를 보호하는 좋은 설계입니다.src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (3)
38-50: 팔로워 목록 쿼리 - 차단 필터링 로직 LGTM!
currentUserId를 추가하여 내가 차단한 사용자를 서브쿼리로 제외하는 로직이 잘 구현되었습니다.userStatus = 'ACTIVE'필터와 함께 탈퇴/비활성 사용자도 제외됩니다.
58-70: 팔로잉 목록 쿼리도 일관성 있게 잘 구현되었습니다.
90-98: 팔로우 요청 목록에서 나를 차단한 사용자 제외 로직 LGTM!나를 차단한 사용자(
ub.blocker)의 팔로우 요청을 제외하는 서브쿼리가 올바르게 구현되었습니다. 탈퇴 사용자(ACTIVE필터)도 함께 제외됩니다.src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
46-62: getFollowers 메서드의 차단/탈퇴 검증 로직 잘 구현되었습니다!타인 조회 시 탈퇴 유저 및 나를 차단한 유저에 대한 404 처리, 내가 차단한 유저 조회 시 빈 리스트 반환 로직이 PR 요구사항에 맞게 구현되었습니다.
src/main/java/com/hrr/backend/domain/user/controller/UserController.java (2)
104-106: 내 진행 중인 챌린지 조회 - currentUser 전달 LGTM!서비스 레이어의 시그니처 변경에 맞춰
currentUser를 올바르게 전달하고 있습니다.
214-233: 타인 인증 기록 조회 API - 접근 제어 일관성 확인됨
getOtherUserVerificationHistory도CustomUserDetails를 받아 차단/탈퇴 검증에 사용합니다. 다른 타인 조회 API들과 일관된 패턴입니다. 👍
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java
Outdated
Show resolved
Hide resolved
…hrr-server into feat/195-add-block-status-to-user-api
yc3697
left a 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.
- 타인 팔로우/팔로워 목록 조회 시 내가 차단한 유저는 제외되어야 합니다.
- 코드래빗 리뷰 두 개 다 수정해주셔야 할 것 같아요.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
12-12: 중복 import 제거 필요
UserBlockRepository가 line 12와 line 25에서 두 번 import되어 있습니다. 하나를 제거해주세요.🔧 수정 제안
import com.hrr.backend.domain.user.entity.enums.UserStatus; -import com.hrr.backend.domain.user.repository.UserBlockRepository; import com.hrr.backend.global.common.enums.ChallengeDays;Also applies to: 25-25
🤖 Fix all issues with AI agents
In
@src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java:
- Around line 625-642: The blocked-user response currently uses
targetUser.getDisplayNickname() while non-blocked branches use
targetUser.getNickname(), causing inconsistency (see
userBlockRepository.existsByBlockerAndBlocked and
VerificationResponseDto.OtherUserHistoryResponse). Either replace the
getDisplayNickname() call with targetUser.getNickname() to match the
private/public branches and remove the misleading comment, or if you
intentionally want a masked/alternate display for blocked users, document that
reason with a clear comment and implement a dedicated masking method (e.g.,
User.getMaskedNickname()) so the intent is explicit and consistent across
branches.
- Around line 617-623: Controller may receive a null customUserDetails because
@AuthenticationPrincipal allows null; add required=true on the
@AuthenticationPrincipal parameter or add an explicit null check before calling
customUserDetails.getUser() to avoid NPE. In the service, replace direct
repository null-unsafe call to
userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser) with the
null-safe helper userBlockRepository.isBlockedRelation(targetUser, currentUser)
and keep the existing targetUser.isNotActive() check; this ensures safe null
handling for blocker/blocked checks.
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.java (1)
32-39: 간결하고 명확한 상호 차단 체크 구현입니다! 👍로직은 정확하지만, 두 번의 DB 조회(
existsByBlockerAndBlocked)가 발생합니다. 현재 사용 빈도가 높지 않다면 문제없지만, 호출이 빈번해질 경우 단일 쿼리로 최적화를 고려해보세요.♻️ 단일 쿼리 최적화 제안
Repository에 다음 쿼리 메서드를 추가하면 DB 호출을 1회로 줄일 수 있습니다:
@Query("SELECT COUNT(b) > 0 FROM UserBlock b " + "WHERE (b.blocker = :userA AND b.blocked = :userB) " + "OR (b.blocker = :userB AND b.blocked = :userA)") boolean existsBlockRelation(@Param("userA") User userA, @Param("userB") User userB);src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
21-21: 와일드카드 import 사용
org.springframework.data.domain.*와일드카드 import가 사용되었습니다. 명시적 import가 코드 가독성에 더 좋지만, 팀 컨벤션에 따라 결정하세요. 큰 이슈는 아닙니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.javasrc/main/java/com/hrr/backend/domain/verification/service/VerificationService.javasrc/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (3)
src/main/java/com/hrr/backend/domain/comment/service/CommentServiceImpl.java (1)
Service(26-250)src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
Service(45-422)src/main/java/com/hrr/backend/domain/verification/dto/VerificationResponseDto.java (1)
VerificationResponseDto(17-197)
🔇 Additional comments (2)
src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java (1)
610-661: 차단/비활성 로직이 PR 요구사항에 맞게 구현되었습니다.
- 대상 사용자가 비활성이거나 나를 차단한 경우 → 404 반환
- 내가 대상을 차단한 경우 → 빈 리스트 반환
- 비공개 계정 → 빈 리스트 + isPublic=false
- 공개 계정 → 정상 데이터 반환
로직 흐름이 명확합니다. 위에서 언급한 null 체크와 닉네임 메서드 일관성만 확인해주세요.
src/main/java/com/hrr/backend/domain/verification/service/VerificationService.java (1)
81-86: API 시그니처 변경이 잘 적용되었습니다.
User me파라미터 추가를 통해 차단 관계 확인이 가능해진 점이 좋습니다. 검증 결과, UserController의 호출 사이트에서 새 시그니처(targetUserId, me, page, size)에 맞게 모두 업데이트되었으며, VerificationServiceImpl의 구현체도 일관되게 변경되어 있습니다.인터페이스에서는
me, 구현체에서는currentUser로 파라미터명이 다르지만 이는 자바 관례상 문제없으며, 컨트롤러에서 현재 사용자 정보가 제대로 전달되고 있어 접근 제어 로직이 의도대로 작동할 것으로 보입니다.
| // 사용자 조회 | ||
| User targetUser = userRepository.findById(targetUserId) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.USER_NOT_FOUND)); | ||
|
|
||
| // 2. 비공개 계정 체크 | ||
| if (!user.getIsPublic()) { | ||
| log.info("User {} is private. Returning empty list.", userId); | ||
| if (targetUser.isNotActive() || userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser)) { | ||
| throw new GlobalException(ErrorCode.USER_NOT_FOUND); | ||
| } |
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 call sites of getOtherUserVerificationHistory
rg -n "getOtherUserVerificationHistory" --type java -B 3 -A 3Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 212
🏁 Script executed:
# Find the method signature of getOtherUserVerificationHistory
rg -n "def|public.*getOtherUserVerificationHistory" --type java -A 2Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 16062
🏁 Script executed:
# Find existsByBlockerAndBlocked method implementation
rg -n "existsByBlockerAndBlocked" --type java -B 2 -A 5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 12969
🏁 Script executed:
# Check if currentUser parameter has null safety annotations
rg -n "getOtherUserVerificationHistory" --type java -B 10 | head -40Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 4510
🏁 Script executed:
# Get the full controller method implementation
cat -n src/main/java/com/hrr/backend/domain/user/controller/UserController.java | sed -n '214,240p'Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1432
🏁 Script executed:
# Check if there's any null handling in the controller before calling service
rg -n "getOtherUserVerificationHistory" -A 20 src/main/java/com/hrr/backend/domain/user/controller/UserController.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1836
🏁 Script executed:
# Check the full method signature and implementation in the service interface
cat -n src/main/java/com/hrr/backend/domain/verification/service/VerificationService.java | sed -n '78,90p'Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 383
🏁 Script executed:
# Check if endpoint has security annotations
rg -n "@PreAuthorize|@Secured" src/main/java/com/hrr/backend/domain/user/controller/UserController.java | grep -A 5 -B 5 "getOtherUserVerificationHistory"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
🏁 Script executed:
# Check if @AuthenticationPrincipal has a required attribute
rg -n "@AuthenticationPrincipal" src/main/java/com/hrr/backend/domain/user/controller/UserController.java | head -20Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 907
🏁 Script executed:
# Check how existsByBlockerAndBlocked is implemented (if it's a derived query or custom)
cat -n src/main/java/com/hrr/backend/domain/user/repository/UserBlockRepository.java | head -50Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 2386
🏁 Script executed:
# Check if there's global security config that forces authentication
fd "SecurityConfig\|WebSecurityConfig" --type f -x cat -n {}Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
컨트롤러에서 customUserDetails null 체크 누락, 서비스에서 안전한 메서드 미사용
Line 230의 customUserDetails.getUser()는 인증되지 않은 사용자가 접근할 경우 즉시 NPE를 발생시킵니다. @AuthenticationPrincipal은 기본적으로 null을 허용하므로 required=true 속성을 추가하거나 controller에서 null 체크를 추가하세요.
또한 service 레벨에서는 이미 UserBlockRepository에 null-safe한 isBlockedRelation() 메서드(lines 35-39에서 null 체크 포함)가 존재하는데, line 621과 626에서는 null 처리 없이 existsByBlockerAndBlocked()를 직접 호출하고 있습니다. 이를 isBlockedRelation()으로 변경하면 더 안전합니다.
// 현재 코드 (line 621)
if (targetUser.isNotActive() || userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser)) {
// 개선안
if (targetUser.isNotActive() || userBlockRepository.isBlockedRelation(targetUser, currentUser)) {🤖 Prompt for AI Agents
In
@src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java
around lines 617 - 623, Controller may receive a null customUserDetails because
@AuthenticationPrincipal allows null; add required=true on the
@AuthenticationPrincipal parameter or add an explicit null check before calling
customUserDetails.getUser() to avoid NPE. In the service, replace direct
repository null-unsafe call to
userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser) with the
null-safe helper userBlockRepository.isBlockedRelation(targetUser, currentUser)
and keep the existing targetUser.isNotActive() check; this ensures safe null
handling for blocker/blocked checks.
| // 차단 관계 시 빈 리스트 반환 | ||
| if (userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser)) { | ||
| return VerificationResponseDto.OtherUserHistoryResponse.builder() | ||
| .isPublic(true) | ||
| .nickname(targetUser.getDisplayNickname()) // (알 수 없음) 표시 | ||
| .verifications(new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false))) | ||
| .build(); | ||
| } | ||
|
|
||
| // 비공개 계정 체크 | ||
| if (!targetUser.getIsPublic()) { | ||
| log.info("User {} is private. Returning empty list.", targetUserId); | ||
| return VerificationResponseDto.OtherUserHistoryResponse.builder() | ||
| .isPublic(false) | ||
| .nickname(user.getNickname()) | ||
| .nickname(targetUser.getNickname()) | ||
| .verifications(new SliceResponseDto<>(Page.empty())) | ||
| .build(); | ||
| } |
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 User entity class and examine both nickname methods
find . -name "*.java" -type f | xargs grep -l "getDisplayNickname\|getNickname" | head -20Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1155
🏁 Script executed:
# Search for getDisplayNickname() implementation
ast-grep --pattern 'getDisplayNickname() {
$$$
}'Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
🏁 Script executed:
# Search for getNickname() implementation
ast-grep --pattern 'getNickname() {
$$$
}'Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/user/entity/User.java | head -200Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 6748
🏁 Script executed:
# Search for both methods in User class
rg -A 8 "getDisplayNickname|getNickname" ./src/main/java/com/hrr/backend/domain/user/entity/User.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 275
🏁 Script executed:
# Check if getNickname is a simple @Getter field
rg "getNickname" ./src/main/java/com/hrr/backend/domain/user/entity/User.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
🏁 Script executed:
# Look at the VerificationServiceImpl context around lines 625-660
sed -n '620,665p' ./src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1893
차단된 사용자의 닉네임 처리 방식 재검토 필요
Line 629에서 getDisplayNickname()을 사용하고 있으나, 이 시점에서 targetUser는 이미 Line 620의 isNotActive() 체크를 통과했으므로 반드시 ACTIVE 상태입니다. 따라서 getDisplayNickname()은 실제 닉네임을 반환하며, 주석의 의도인 "(알 수 없음)" 마스킹은 발생하지 않습니다.
Line 639(비공개 계정)와 Line 658(공개 계정)에서는 getNickname()을 사용하므로, 의도와 실제 동작을 맞추기 위해:
- Line 629를
targetUser.getNickname()으로 변경하고 불필요한 주석 제거 - 또는 차단된 사용자의 닉네임을 의도적으로 다르게 처리해야 한다면, 그 이유를 명확한 주석으로 설명
일관성 있는 코드 유지보수를 위해 위 중 하나로 정리해주세요.
🤖 Prompt for AI Agents
In
@src/main/java/com/hrr/backend/domain/verification/service/VerificationServiceImpl.java
around lines 625 - 642, The blocked-user response currently uses
targetUser.getDisplayNickname() while non-blocked branches use
targetUser.getNickname(), causing inconsistency (see
userBlockRepository.existsByBlockerAndBlocked and
VerificationResponseDto.OtherUserHistoryResponse). Either replace the
getDisplayNickname() call with targetUser.getNickname() to match the
private/public branches and remove the misleading comment, or if you
intentionally want a masked/alternate display for blocked users, document that
reason with a clear comment and implement a dedicated masking method (e.g.,
User.getMaskedNickname()) so the intent is explicit and consistent across
branches.
…b.com/Hrr-HabbitRoutineResult/hrr-server into feat/195-add-block-status-to-user-api
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: 2
🤖 Fix all issues with AI agents
In
@src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java:
- Around line 57-68: The followings query in findFollowingsByUserId incorrectly
applies the block filters to f.follower.id instead of the followed user; update
the two subqueries that currently use f.follower.id to target f.following.id
(i.e., change the NOT IN checks to compare f.following.id with
UserBlock.ub.blocked.id and ub.blocker.id respectively) so the method
findFollowingsByUserId and its @Query exclude blocked followees the same way
findFollowersByUserId does.
In @src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java:
- Around line 85-93: The ProfileDto always sets isFollowing to false; change it
to use the existing following check: when isBlockedByMe is true keep isFollowing
false, otherwise call the existing checkIfFollowing(...) and set isFollowing to
its result. Update the UserServiceImpl code around
UserResponseDto.ProfileDto.builder() to pass the conditional value (use
isBlockedByMe ? false : checkIfFollowing(requestingUserId, targetUser.getId()))
so non-blocked users get the real follow status.
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
158-161:userId대신targetUser.getId()사용 필요Line 159에서
userChallengeRepository.findByUserIdAndChallengeId(userId, challengeId)를 호출하고 있습니다. 그러나getOngoingChallenges메서드는targetUser의 챌린지를 조회하는 것이므로, 일관성을 위해targetUser.getId()를 사용하는 것이 더 명확합니다.현재는
userId와targetUser.getId()가 동일하지만, 코드 가독성과 유지보수 측면에서 명시적으로targetUser.getId()를 사용하는 것을 권장합니다.♻️ 리팩토링 제안
Long userChallengeId = userChallengeRepository - .findByUserIdAndChallengeId(userId, challengeId) + .findByUserIdAndChallengeId(targetUser.getId(), challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) .getId();
122-122: 정적 import 사용 권장Line 5에서
java.util.Collections를 import했지만, Line 122에서는java.util.Collections.emptyList()로 fully qualified name을 사용하고 있습니다. 일관성을 위해 import된Collections를 사용하세요.- return new SliceResponseDto<>(new SliceImpl<>(java.util.Collections.emptyList(), PageRequest.of(page, size), false)); + return new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false));src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
46-50: 중복 코드 추출 고려
getFollowers와getFollowings에서 사용자 조회 및 차단 검증 로직이 거의 동일합니다. 향후 유지보수성을 위해 공통 메서드로 추출하는 것을 고려해 보세요.private void validateAccessPermission(User targetUser, User currentUser, Long userId, Long currentUserId) { if (!userId.equals(currentUserId)) { if (targetUser.isNotActive() || userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser)) { throw new GlobalException(ErrorCode.USER_NOT_FOUND); } } } private boolean shouldReturnEmptyList(User me, User targetUser, Long userId, Long currentUserId) { return !userId.equals(currentUserId) && userBlockRepository.existsByBlockerAndBlocked(me, targetUser); }Also applies to: 107-111
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javasrc/main/java/com/hrr/backend/domain/follow/service/FollowListService.javasrc/main/java/com/hrr/backend/domain/user/service/UserBlockService.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
UserResponseDto(14-210)
🔇 Additional comments (6)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
107-124: 타인 프로필 조회 시 차단/비활성 체크 로직 잘 구현됨블록 관계와 비활성 사용자 체크를 명확하게 분리하여 처리한 점이 좋습니다:
- 비활성 사용자 → 404
- 상대방이 나를 차단 → 404
- 내가 상대방을 차단 → 빈 리스트 반환
PR 목표에 부합하는 구현입니다. 👍
src/main/java/com/hrr/backend/domain/user/service/UserBlockService.java (1)
51-71: 이전 리뷰 피드백 반영 완료 - PENDING 상태 처리 수정됨
FollowStatus.APPROVED상태일 때만 카운트를 감소시키도록 수정되었습니다. PENDING 상태의 팔로우 요청이 삭제될 때 카운트가 잘못 감소하는 문제가 해결되었습니다.삭제는 상태와 무관하게 수행하고, 카운트 감소는 APPROVED 상태에서만 수행하는 로직이 정확합니다. 👏
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (2)
38-49:findFollowersByUserId쿼리 - 차단 로직 정확함팔로워 목록 조회 시:
f.follower.id NOT IN (내가 차단한 사용자)- 내가 차단한 사용자 제외f.following.id NOT IN (나를 차단한 사용자)- 나를 차단한 사용자의 팔로워 목록 접근 차단쿼리 레벨에서 차단 관계를 필터링하여 N+1 문제 없이 효율적으로 처리됩니다.
88-96: PENDING 팔로우 요청에서 나를 차단한 유저 제외 - 적절한 구현나를 차단한 사용자의 팔로우 요청을 목록에서 제외하는 로직이 잘 구현되었습니다. 탈퇴 유저 필터(
ACTIVE상태)와 함께 적용되어 일관된 사용자 경험을 제공합니다.src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (2)
130-130: 이전 리뷰 피드백 반영 완료 - 올바른 Repository 메서드 호출
getFollowings메서드에서 이제findFollowingsByUserId를 올바르게 호출하고 있습니다. 이전에 지적된findFollowersByUserId호출 버그가 수정되었습니다. 👍
46-62: 팔로워 목록 조회 시 차단/비활성 검증 로직 잘 구현됨사용자 존재 확인 후 차단 관계를 체계적으로 검증합니다:
- 타인 조회 시 비활성 또는 나를 차단한 유저 → 404
- 내가 차단한 유저 → 빈 리스트 반환
getFollowings에서도 동일한 패턴이 적용되어 일관성이 유지됩니다.
| @Query("SELECT f.following FROM Follow f " + | ||
| "WHERE f.follower.id = :userId " + | ||
| "AND f.status = 'APPROVED' " + | ||
| "AND f.following.userStatus = 'ACTIVE' " + | ||
| "AND f.follower.id NOT IN (SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId) " + | ||
| "AND f.follower.id NOT IN (SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId) " + | ||
| "ORDER BY f.createdAt DESC") | ||
| Slice<User> findFollowingsByUserId( | ||
| @Param("userId") Long userId, | ||
| @Param("currentUserId") Long currentUserId, | ||
| Pageable pageable | ||
| ); |
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.
findFollowingsByUserId 쿼리에서 차단 필터 대상이 잘못됨
팔로잉 목록 조회 시 차단 필터가 f.follower.id를 대상으로 하고 있습니다. 그러나 팔로잉 목록에서 제외해야 할 대상은 f.following (팔로잉하는 사용자)입니다.
현재 쿼리는 팔로워(조회 대상)가 차단 관계에 있는지를 확인하지만, 실제로는 팔로잉 대상이 차단 관계에 있는지를 확인해야 합니다.
🐛 수정 제안
@Query("SELECT f.following FROM Follow f " +
"WHERE f.follower.id = :userId " +
"AND f.status = 'APPROVED' " +
"AND f.following.userStatus = 'ACTIVE' " +
- "AND f.follower.id NOT IN (SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId) " +
- "AND f.follower.id NOT IN (SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId) " +
+ "AND f.following.id NOT IN (SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId) " +
+ "AND f.following.id NOT IN (SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId) " +
"ORDER BY f.createdAt DESC")참고: findFollowersByUserId의 차단 필터 (lines 42-43)와 비교하면 패턴이 다름을 확인할 수 있습니다.
🤖 Prompt for AI Agents
In @src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java
around lines 57 - 68, The followings query in findFollowingsByUserId incorrectly
applies the block filters to f.follower.id instead of the followed user; update
the two subqueries that currently use f.follower.id to target f.following.id
(i.e., change the NOT IN checks to compare f.following.id with
UserBlock.ub.blocked.id and ub.blocker.id respectively) so the method
findFollowingsByUserId and its @Query exclude blocked followees the same way
findFollowersByUserId does.
yc3697
left a 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.
리뷰 남긴 것만 수정해주세요
| "AND f.status = 'APPROVED' " + | ||
| "AND f.follower.userStatus = 'ACTIVE' " + | ||
| "AND f.follower.id NOT IN ( SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId )" + | ||
| "AND f.following.id NOT IN ( SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId )" + |
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.
여기는 팔로워 조회니까 조건절에서 f.follower가 맞는 것 같아요
| "AND f.follower.id NOT IN (SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId) " + | ||
| "AND f.follower.id NOT IN (SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId) " + |
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.
여기도 코드래빗 리뷰처럼 following이 맞는 것 같습니다
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: 2
🤖 Fix all issues with AI agents
In @src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java:
- Around line 87-95: Replace the manual builder call with the static factory
that also sets the blocked flag: call
UserResponseDto.ProfileDto.from(targetUser, isFollowing, isBlockedByMe) instead
of using the builder; if ProfileDto.from does not support your masked
profileImage workflow, keep the builder but add .isBlocked(isBlockedByMe) to the
builder chain so the isBlocked field is populated.
🧹 Nitpick comments (5)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (2)
57-68: 동일한 성능 최적화 제안이 적용됩니다.이 메서드도
findFollowersByUserId와 동일한NOT IN서브쿼리 패턴을 사용하고 있습니다. 위에서 제안한NOT EXISTS최적화를 동일하게 적용할 수 있습니다.⚡ NOT EXISTS를 사용한 성능 개선 제안
- @Query("SELECT f.following FROM Follow f " + - "WHERE f.follower.id = :userId " + - "AND f.status = 'APPROVED' " + - "AND f.following.userStatus = 'ACTIVE' " + - "AND f.following.id NOT IN (SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId) " + - "AND f.following.id NOT IN (SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId) " + - "ORDER BY f.createdAt DESC") + @Query("SELECT f.following FROM Follow f " + + "WHERE f.follower.id = :userId " + + "AND f.status = 'APPROVED' " + + "AND f.following.userStatus = 'ACTIVE' " + + "AND NOT EXISTS (SELECT 1 FROM UserBlock ub WHERE ub.blocker.id = :currentUserId AND ub.blocked.id = f.following.id) " + + "AND NOT EXISTS (SELECT 1 FROM UserBlock ub WHERE ub.blocked.id = :currentUserId AND ub.blocker.id = f.following.id) " + + "ORDER BY f.createdAt DESC")
88-96: 로직은 정확하나, 성능 최적화를 적용하세요.팔로우 요청 목록에서는 "나를 차단한 유저"만 제외하는 단방향 체크가 정확합니다. 하지만 여기서도
NOT IN대신NOT EXISTS를 사용하면 성능이 개선됩니다.⚡ NOT EXISTS를 사용한 성능 개선 제안
@Query("SELECT f FROM Follow f " + "WHERE f.following.id = :userId " + "AND f.status = :status " + "AND f.follower.userStatus = 'ACTIVE' " + - "AND f.follower.id NOT IN ( " + - " SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :userId" + - ") " + + "AND NOT EXISTS ( " + + " SELECT 1 FROM UserBlock ub WHERE ub.blocked.id = :userId AND ub.blocker.id = f.follower.id" + + ") " + "ORDER BY f.createdAt DESC")src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (3)
76-77: 변수명 개선 제안
isBlockedByOther변수명이 다소 모호합니다. "다른 사용자에게 차단되었는지"보다는 "조회 대상 사용자가 나를 차단했는지"를 명확히 표현하는 것이 좋습니다.♻️ 더 명확한 변수명 제안
- boolean isBlockedByMe = userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser); - boolean isBlockedByOther = userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser); + boolean isBlockedByMe = userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser); + boolean hasTargetBlockedMe = userBlockRepository.existsByBlockerAndBlocked(targetUser, currentUser); String profileImage = isBlockedByMe ? null : targetUser.getProfileImage(); - if (isBlockedByOther) { + if (hasTargetBlockedMe) { throw new GlobalException(ErrorCode.USER_NOT_FOUND); }
124-124:Collections.emptyList()사용 시 불필요한 정규화Line 5에서
Collections를 이미 import했으므로,java.util.Collections.emptyList()대신Collections.emptyList()를 사용하는 것이 일관성 있습니다.♻️ 간결한 표현 제안
- return new SliceResponseDto<>(new SliceImpl<>(java.util.Collections.emptyList(), PageRequest.of(page, size), false)); + return new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false));
161-161: 일관성을 위한targetUser.getId()사용 제안현재
userId파라미터를 직접 사용하고 있는데, 이는 기능적으로 올바르지만 Line 106-133에서targetUser엔티티를 도입한 패턴과 약간의 불일치가 있습니다.
targetUser.getId()를 사용하면 "누구의 챌린지를 조회하는지"가 더 명확해집니다.♻️ 일관성 개선 제안
Long userChallengeId = userChallengeRepository - .findByUserIdAndChallengeId(userId, challengeId) + .findByUserIdAndChallengeId(targetUser.getId(), challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) .getId();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
UserResponseDto(14-210)
🔇 Additional comments (3)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
100-100: 명확한 주석 개선, 좋습니다! 👍파라미터의 역할을 명확하게 설명해서 코드 가독성이 향상되었습니다.
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
110-126: 차단/비활성 사용자 처리 로직 확인 완료타인 프로필 조회 시 다음 시나리오를 정확하게 처리합니다:
- 탈퇴/비활성 사용자: 404 반환 ✅
- 상대방이 나를 차단: 404 반환 ✅
- 내가 상대방을 차단: 빈 리스트 반환 ✅
로직이 PR 목표(#195)와 일치하며 구현이 명확합니다.
99-104: 메서드 시그니처 개선 및 타 사용자 챌린지 조회 지원 확인
getOngoingChallenges메서드에currentUser파라미터를 추가하고, Line 133에서targetUser로 챌린지를 조회하도록 변경한 것은 타인의 진행 중인 챌린지 조회 기능을 올바르게 구현한 것입니다.이전에는 요청자 본인의 챌린지만 조회 가능했지만, 이제
userId로 지정된 대상 사용자의 챌린지를 조회할 수 있습니다. 🎯Also applies to: 133-133
| @Query("SELECT f.follower FROM Follow f " + | ||
| "WHERE f.following.id = :userId " + | ||
| "AND f.status = 'APPROVED' " + | ||
| "AND f.follower.userStatus = 'ACTIVE' " + | ||
| "AND f.follower.id NOT IN ( SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId )" + | ||
| "AND f.follower.id NOT IN ( SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId )" + | ||
| "ORDER BY f.createdAt DESC") | ||
| Slice<User> findFollowersByUserId( | ||
| @Param("userId") Long userId, | ||
| @Param("currentUserId") Long currentUserId, | ||
| Pageable pageable | ||
| ); |
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:
#!/bin/bash
# Get repository structure overview
git ls-files | head -20
echo "---"
# Find Java files related to Follow and UserBlock
fd -e java | grep -E "(Follow|UserBlock)" | head -20Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Find the exact location of FollowRepository
fd -name "FollowRepository.java"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 308
🏁 Script executed:
#!/bin/bash
# Find UserBlock related files
fd -name "*UserBlock*"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 308
🏁 Script executed:
#!/bin/bash
# Read FollowRepository.java
cat "src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 4227
🏁 Script executed:
#!/bin/bash
# Find and read UserBlock entity
fd -type f -name "*.java" -path "*userblock*" -o -path "*block*" | grep -i userblock | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Search for UserBlock class definition
rg "class UserBlock" -t javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 504
🏁 Script executed:
#!/bin/bash
# Read UserBlock entity file
cat "src/main/java/com/hrr/backend/domain/user/entity/UserBlock.java"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 941
🏁 Script executed:
#!/bin/bash
# Check for other entities related to Follow and User for full context
cat "src/main/java/com/hrr/backend/domain/follow/entity/Follow.java"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1233
🏁 Script executed:
#!/bin/bash
# Check User entity for userStatus field
fd -name "User.java" | grep entityRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 308
🏁 Script executed:
#!/bin/bash
# Find User entity - search for it properly
find . -path "*/user/entity/User.java" -type fRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Read User entity
cat "src/main/java/com/hrr/backend/domain/user/entity/User.java"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 6326
🏁 Script executed:
#!/bin/bash
# Verify if there are any @Index annotations in UserBlock or related migration files
rg "@Index" src/main/java/com/hrr/backend/domain/user/entity/ || echo "No @Index found in user entity folder"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 392
🏁 Script executed:
#!/bin/bash
# Check if there are migration/schema files that define indexes
find . -name "*.sql" -o -name "*migration*" -o -name "schema*" | head -10Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 706
🏁 Script executed:
#!/bin/bash
# Check the migration files for user_block table structure
cat "src/main/resources/db/migration/V1.0__init.sql" | grep -A 10 "user_block"Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 60
🏁 Script executed:
#!/bin/bash
# Check all migration files for user_block references
rg "user_block|UserBlock" "src/main/resources/db/migration/" -A 3Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 6037
차단 로직의 정확성은 검증되었으나, 쿼리 성능 최적화가 필요합니다.
NOT IN 서브쿼리는 데이터 증가에 따라 성능 저하를 초래합니다. 특히 두 개의 NOT IN 서브쿼리가 연쇄적으로 실행되면서 전체 쿼�리 계획이 복잡해집니다.
현재 상황:
UserBlock테이블에는(blocker_id, blocked_id)복합 고유 제약만 존재- 서브쿼리는 단일 컬럼으로만 조회 (
WHERE ub.blocker.id = :currentUserId또는WHERE ub.blocked.id = :currentUserId) - 복합 인덱스로는 단일 컬럼 조회 성능을 보장하지 못함
필수 개선 사항:
NOT EXISTS로 변경하면 대부분의 경우 더 효율적입니다UserBlock테이블에blocker_id와blocked_id각각에 대한 개별 인덱스를 추가하세요
⚡ NOT EXISTS를 사용한 성능 개선 예시
- @Query("SELECT f.follower FROM Follow f " +
- "WHERE f.following.id = :userId " +
- "AND f.status = 'APPROVED' " +
- "AND f.follower.userStatus = 'ACTIVE' " +
- "AND f.follower.id NOT IN ( SELECT ub.blocked.id FROM UserBlock ub WHERE ub.blocker.id = :currentUserId )" +
- "AND f.follower.id NOT IN ( SELECT ub.blocker.id FROM UserBlock ub WHERE ub.blocked.id = :currentUserId )" +
- "ORDER BY f.createdAt DESC")
+ @Query("SELECT f.follower FROM Follow f " +
+ "WHERE f.following.id = :userId " +
+ "AND f.status = 'APPROVED' " +
+ "AND f.follower.userStatus = 'ACTIVE' " +
+ "AND NOT EXISTS (SELECT 1 FROM UserBlock ub WHERE ub.blocker.id = :currentUserId AND ub.blocked.id = f.follower.id) " +
+ "AND NOT EXISTS (SELECT 1 FROM UserBlock ub WHERE ub.blocked.id = :currentUserId AND ub.blocker.id = f.follower.id) " +
+ "ORDER BY f.createdAt DESC")마이그레이션 파일에서 다음과 같이 인덱스를 추가하세요:
CREATE INDEX idx_user_block_blocker_id ON user_block(blocker_id);
CREATE INDEX idx_user_block_blocked_id ON user_block(blocked_id);참고 자료:
- Hibernate 쿼리 성능 - NOT IN vs NOT EXISTS
- JPA 쿼리 성능 최적화 가이드
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
Outdated
Show resolved
Hide resolved
yc3697
left a 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.
ProfileDto 생성 시의 isBlocked 누락 관련된 코드래빗 리뷰만 수정하면 될 것 같은데 from 메소드 말고 빌더로 바꾼 이유가 있을까요?
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
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/user/service/UserServiceImpl.java (1)
151-156: 치명적인 버그: 타인 프로필 조회 시 잘못된 사용자의 챌린지를 조회합니다.Line 153에서
userId를 사용하여 userChallenge를 조회하고 있는데, 이는 타인의 프로필을 조회할 때 문제가 됩니다.targetUser.getId()를 사용해야 합니다.현재 문제점:
- 사용자 A가 사용자 B의 프로필을 조회할 때
- Line 125에서는 올바르게
targetUser의 챌린지를 조회하지만- Line 153에서는
userId(즉, 사용자 B의 ID)로 userChallenge를 찾으려고 시도- 하지만 인증 정보를 확인하려면 실제로 챌린지를 진행 중인 사용자(targetUser)의 userChallengeId가 필요
🐛 수정 제안
// 유저-챌린지 매핑 (userChallengeId 필요) Long userChallengeId = userChallengeRepository - .findByUserIdAndChallengeId(userId, challengeId) + .findByUserIdAndChallengeId(targetUser.getId(), challengeId) .orElseThrow(() -> new GlobalException(ErrorCode.CHALLENGE_NOT_FOUND)) .getId();src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
48-60: DTO 변환 로직에서isBlocked필드가 누락되었습니다.
ProfileDto에는isBlocked필드(Line 45)가 선언되어 있지만,from메서드의 builder에서.isBlocked(isBlocked)를 설정하지 않고 있습니다.영향 분석:
- 현재 구현: API 응답에서
isBlocked필드가 항상null로 반환됩니다.- 프로필 이미지는 마스킹되지만, 클라이언트는 차단 여부를 명시적으로 알 수 없습니다.
의도 확인이 필요합니다:
- 차단 여부를 클라이언트에 노출하지 않으려는 의도라면 → 필드 자체를 제거하는 것이 명확합니다.
- 차단 여부를 노출해야 한다면 → builder에
.isBlocked(isBlocked)추가가 필요합니다.🔧 옵션 1: 차단 여부를 노출하는 경우
.followerCount(user.isNotActive() ? 0L : user.getFollowerCount()) .followingCount(user.isNotActive() ? 0L : user.getFollowingCount()) .isFollowing(isFollowing) + .isBlocked(isBlocked) .build();🔧 옵션 2: 차단 여부를 숨기는 경우 (추천)
@Schema(description = "팔로잉 여부", example = "false") private Boolean isFollowing; - @Schema(description = "차단 여부", example = "false") - private Boolean isBlocked; - // Entity -> DTO 변환 public static ProfileDto from(User user, Boolean isFollowing, Boolean isBlocked) {그리고
from메서드 시그니처도 단순화:- public static ProfileDto from(User user, Boolean isFollowing, Boolean isBlocked) { + public static ProfileDto from(User user, Boolean isFollowing, boolean isBlockedByMe) { return ProfileDto.builder() .userId(user.getId()) .nickname(user.getDisplayNickname()) // 내가 차단한 유저라면 사진을 null로 마스킹 - .profileImage(isBlocked ? null : user.getProfileImage()) + .profileImage(isBlockedByMe ? null : user.getProfileImage())
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
79-80: 불필요한 변수 할당을 제거하세요.
profileImage변수를 선언하고 있지만 실제로 사용되지 않습니다. Line 87에서ProfileDto.from()이 내부적으로 마스킹 로직을 처리하므로 이 코드는 중복입니다.♻️ 제안하는 리팩토링
- String profileImage = isBlockedByMe ? null : targetUser.getProfileImage(); - if (isBlockedByOther) {
116-116: import 사용을 일관되게 유지하세요.Line 5에서
Collections를 import했지만 여기서는java.util.Collections.emptyList()로 fully qualified name을 사용하고 있습니다.♻️ 일관성을 위한 수정
- return new SliceResponseDto<>(new SliceImpl<>(java.util.Collections.emptyList(), PageRequest.of(page, size), false)); + return new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false));
📜 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/dto/UserResponseDto.javasrc/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
UserResponseDto(14-211)
🔇 Additional comments (3)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
85-85: 팔로잉 상태 계산 로직이 올바릅니다.차단한 사용자에 대해 팔로잉 상태를
false로 처리하는 로직이 적절합니다. 사용자 경험 측면에서 합리적인 판단입니다.
91-118: 타인 프로필 조회 시 차단 검증 로직이 잘 구현되었습니다.본인 프로필 조회와 타인 프로필 조회를 구분하여 적절한 검증을 수행하고 있습니다:
- 비활성/탈퇴 사용자 체크 (404 반환)
- 상대방이 나를 차단한 경우 (404 반환)
- 내가 상대방을 차단한 경우 (빈 리스트 반환)
각 케이스별 응답 전략이 명확하고 일관적입니다.
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
52-57: 마스킹 및 카운트 조정 로직이 적절합니다.차단된 사용자의 프로필 이미지를
null로 마스킹하고, 비활성 사용자의 팔로워/팔로잉 수를 0으로 설정하는 로직이 명확하게 구현되어 있습니다. 주석도 의도를 잘 설명하고 있습니다.
yc3697
left a 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.
찐막
| if (isRegistrationDay) { | ||
| // 유저-챌린지 매핑 (userChallengeId 필요) | ||
| Long userChallengeId = userChallengeRepository | ||
| .findByUserIdAndChallengeId(userId, challengeId) |
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.
여기에서 userId보다 targetUser.getId()가 더 좋을 것 같아요! 큰 차이는 없는데 가독성 측면에서..ㅎㅎ
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 (2)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (2)
79-80: 사용되지 않는 변수profileImage가 있습니다.
profileImage변수가 선언되었지만 이후 코드에서 사용되지 않습니다.ProfileDto.from()메서드 내부에서 이미isBlocked파라미터를 기반으로 프로필 이미지 마스킹 처리를 하고 있습니다 (.profileImage(isBlocked ? null : user.getProfileImage())).중복된 로직이므로 제거하는 것이 좋습니다.
♻️ 수정 제안
- String profileImage = isBlockedByMe ? null : targetUser.getProfileImage(); - if (isBlockedByOther) {
101-118: 차단/비활성 사용자 접근 제어 로직이 잘 구현되었습니다.타인 프로필 조회 시 다음 순서로 체크하는 로직이 PR 요구사항에 부합합니다:
- 탈퇴/비활성 상태 → 404
- 상대방이 나를 차단 → 404
- 내가 상대방 차단 → 빈 목록 반환
단, Line 116에서
java.util.Collections.emptyList()대신 이미 임포트된Collections.emptyList()를 사용하면 일관성이 높아집니다.♻️ 수정 제안
// 내가 상대방을 차단했는지 체크 if (userBlockRepository.existsByBlockerAndBlocked(currentUser, targetUser)) { - return new SliceResponseDto<>(new SliceImpl<>(java.util.Collections.emptyList(), PageRequest.of(page, size), false)); + return new SliceResponseDto<>(new SliceImpl<>(Collections.emptyList(), PageRequest.of(page, size), false)); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (1)
src/main/java/com/hrr/backend/domain/user/dto/UserResponseDto.java (1)
UserResponseDto(14-211)
🔇 Additional comments (4)
src/main/java/com/hrr/backend/domain/user/service/UserServiceImpl.java (4)
85-87: 차단 상태 반영 로직이 적절합니다.내가 차단한 사용자에 대해 팔로잉 여부를
false로 처리하는 로직이 PR 목표에 맞게 잘 구현되었습니다. 👍
124-125: 대상 사용자 기준으로 챌린지 조회가 올바르게 변경되었습니다.
targetUser를 기준으로 진행 중인 챌린지를 조회하도록 수정되어, 타인 프로필 조회 시 해당 사용자의 챌린지 목록이 정확히 반환됩니다.
152-155:targetUser.getId()사용이 올바릅니다.인증 완료 여부 확인을 위해 대상 사용자의
userChallengeId를 조회하도록 변경된 부분이 전체 로직과 일관성 있게 구현되었습니다. 잘했어요! 🎯
91-96: 인터페이스 및 호출부 일관성 확인 완료 ✓
getOngoingChallenges메서드의User currentUser파라미터 추가가 깔끔하게 반영되었습니다.UserService인터페이스와UserServiceImpl구현 클래스의 시그니처가 완벽하게 일치하며,UserController의 두 호출부 모두 새로운 파라미터를 올바르게 전달하고 있습니다. 인터페이스 계약과 구현부의 동기화가 잘 유지되어 있네요.
yc3697
left a 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.
수고많으셨습니다 ㅎㅎ
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
1. 팔로우 관련 API (Follow API)
2. 차단 관련 API (User Block API)
3. 다른 사용자가 참가중인 챌린지 / 인증 기록 조회 API
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
사용자 API ( 다른사용자 정보, 참가중인 챌린지, 인증기록 조회 API)
1. 처음에 ID 3번 조회 시
2. ID 3번을 차단함
나를 차단한 사용자 조회 시
팔로우 관련 API
1. 다른 사용자 팔로워/팔로잉 목록 조회 API
2. 팔로우 API
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
새로운 기능
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.