[FEAT] 팔로우 요청/승인 API 구현 #96
Conversation
…com/Hrr-HabbitRoutineResult/hrr-server into feat/87-follow-request-approval-api
📝 WalkthroughWalkthrough팔로우 요청/승인 기능을 도입합니다. Follow 엔티티에 FollowStatus(PENDING/APPROVED)을 추가하고, 비공개 계정은 요청을 PENDING으로 저장합니다. 요청 조회, 승인, 거절 API와 상태 기반 쿼리·DTO·마이그레이션이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as FollowController
participant Service as FollowService
participant Repo as FollowRepository
participant DB as Database
rect rgb(245, 250, 255)
Note over Client,Controller: 팔로우 요청 생성 (공개/비공개 분기)
Client->>Controller: POST /api/.../follow/{followingId}
Controller->>Service: followUser(currentUserId, followingId)
Service->>Repo: findByFollowerIdAndFollowingId(...)
Repo-->>Service: existing? (null or Follow)
alt 신규
Service->>Service: target.isPublic? → status = APPROVED : PENDING
Service->>Repo: save(Follow(status))
Repo->>DB: INSERT follow (status)
DB-->>Repo: saved
Repo-->>Service: Follow(entity)
end
Service-->>Controller: FollowResponseDto(message, id, status)
Controller-->>Client: 200 OK
end
rect rgb(240, 255, 240)
Note over Client,DB: 팔로우 요청 승인
Client->>Controller: POST /me/requests/{requesterId}/approve
Controller->>Service: approveFollowRequest(currentUserId, requesterId)
Service->>Repo: findByFollowerIdAndFollowingIdAndStatus(requesterId, currentUserId, PENDING)
Repo-->>Service: Follow(status=PENDING) / null
alt found
Service->>Service: follow.approve() (status -> APPROVED)
Service->>Repo: save(Follow)
Repo->>DB: UPDATE follow.status=APPROVED
DB-->>Repo: updated
Service-->>Controller: FollowResponseDto(..., APPROVED)
else not found
Service-->>Controller: throw FOLLOW_REQUEST_NOT_FOUND
end
Controller-->>Client: 200 OK / 404
end
rect rgb(255, 245, 250)
Note over Client,DB: 팔로우 요청 거절
Client->>Controller: DELETE /me/requests/{requesterId}/reject
Controller->>Service: rejectFollowRequest(currentUserId, requesterId)
Service->>Repo: findByFollowerIdAndFollowingIdAndStatus(requesterId, currentUserId, PENDING)
Repo-->>Service: Follow(status=PENDING) / null
alt found
Service->>Repo: delete(Follow)
Repo->>DB: DELETE from follow
DB-->>Repo: deleted
Service-->>Controller: FollowResponseDto(..., null)
else not found
Service-->>Controller: throw FOLLOW_REQUEST_NOT_FOUND
end
Controller-->>Client: 200 OK / 404
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
97-100: 동일한 빈 Slice 처리 버그getFollowers() 메서드와 동일한 문제입니다. 빈 Slice를 null로 매핑하지 말고, 위의 수정 사항과 동일하게 처리해주세요.
// 팔로잉이 없으면 빈 Slice 반환 if (followingsSlice.isEmpty()) { - return new SliceResponseDto<>(followingsSlice.map(user -> null)); + return new SliceResponseDto<>(followingsSlice.map(user -> FollowListResponseDto.of(user, false))); }또는 early return을 제거하여 더 간결하게 처리할 수 있습니다.
🧹 Nitpick comments (8)
src/main/resources/db/migration/V2.8__add_follow_status.sql (1)
5-6: 선택적: 중복된 UPDATE 구문Line 3에서 이미
DEFAULT 'APPROVED'를 설정했으므로, 기존 레코드는 자동으로 해당 값을 가지게 됩니다. Line 6의 UPDATE 구문은 기술적으로 불필요합니다.다만, 명시적인 데이터 마이그레이션 의도를 드러내고 특정 DB 버전의 동작 차이를 방지하는 방어적 코드로 볼 수 있어 유지해도 무방합니다.
필요시 다음과 같이 간소화할 수 있습니다:
-- Follow 테이블에 status 컬럼 추가 ALTER TABLE follow ADD COLUMN status VARCHAR(20) NOT NULL DEFAULT 'APPROVED' AFTER following_id; - --- 기존 데이터는 모두 APPROVED로 처리 -UPDATE follow SET status = 'APPROVED';src/main/java/com/hrr/backend/domain/follow/entity/Follow.java (1)
40-43: 선택적: approve() 메서드에 방어 로직 추가현재는 상태 전환을 무조건 수행하지만, 이미 APPROVED 상태인 경우를 체크하면 더 안전합니다. 다만 서비스 레이어에서
ALREADY_APPROVED_FOLLOW에러코드로 검증하고 있다면 현재 구조도 충분합니다.필요시 다음과 같이 방어 로직을 추가할 수 있습니다:
public void approve() { + if (this.status == FollowStatus.APPROVED) { + throw new IllegalStateException("이미 승인된 팔로우입니다."); + } this.status = FollowStatus.APPROVED; }단, 이는 도메인 로직 위치에 대한 선택의 문제로, 서비스 레이어에서 검증하는 현재 방식도 유효합니다.
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (2)
38-39: JPQL에서 enum 문자열 리터럴 사용 주의
'APPROVED'문자열 리터럴을 직접 사용하는 것은 동작하지만, enum 값 변경 시 컴파일 타임에 오류를 감지할 수 없습니다. 파라미터화된 쿼리로 변경하면 타입 안전성을 확보할 수 있습니다.- @Query("SELECT f.follower FROM Follow f WHERE f.following.id = :userId AND f.status = 'APPROVED' ORDER BY f.createdAt DESC") - Slice<User> findFollowersByUserId(@Param("userId") Long userId, Pageable pageable); + @Query("SELECT f.follower FROM Follow f WHERE f.following.id = :userId AND f.status = :status ORDER BY f.createdAt DESC") + Slice<User> findFollowersByUserId(@Param("userId") Long userId, @Param("status") FollowStatus status, Pageable pageable);단, 이 변경 시 호출부에서도
FollowStatus.APPROVED를 명시적으로 전달해야 합니다.
59-69: 메서드명과 파라미터 설계 불일치
findPendingFollowRequests라는 이름은 PENDING 상태만 조회함을 암시하지만,status파라미터를 받아 범용적으로 사용 가능합니다. 일관성을 위해 둘 중 하나를 선택하세요:
- 메서드명 변경:
findFollowRequestsByStatus- 파라미터 제거 후 PENDING 하드코딩
현재 구현이 PENDING 전용이라면:
- @Query("SELECT f FROM Follow f " + - "WHERE f.following.id = :userId " + - "AND f.status = :status " + - "ORDER BY f.createdAt DESC") - List<Follow> findPendingFollowRequests(@Param("userId") Long userId, @Param("status") FollowStatus status); + @Query("SELECT f FROM Follow f " + + "WHERE f.following.id = :userId " + + "AND f.status = 'PENDING' " + + "ORDER BY f.createdAt DESC") + List<Follow> findPendingFollowRequests(@Param("userId") Long userId);src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (1)
189-197: 대용량 팔로우 요청 시 페이지네이션 부재
getPendingFollowRequests는List<Follow>를 반환하며 페이지네이션이 없습니다. 인기 계정의 경우 수천 개의 pending 요청이 있을 수 있어 메모리 및 응답 시간 문제가 발생할 수 있습니다.페이지네이션 적용을 권장합니다:
- public List<FollowRequestDto> getPendingFollowRequests(Long currentUserId) { + public SliceResponseDto<FollowRequestDto> getPendingFollowRequests(Long currentUserId, Pageable pageable) { log.info("받은 팔로우 요청 목록 조회 - userId: {}", currentUserId); - List<Follow> pendingFollows = followRepository.findPendingFollowRequests(currentUserId, FollowStatus.PENDING); + Slice<Follow> pendingFollows = followRepository.findPendingFollowRequests(currentUserId, FollowStatus.PENDING, pageable); - return pendingFollows.stream() + List<FollowRequestDto> content = pendingFollows.stream() .map(FollowRequestDto::from) .collect(Collectors.toList()); + return SliceResponseDto.of(content, pendingFollows.hasNext()); }Repository 메서드도
Slice<Follow>반환으로 변경이 필요합니다.src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (3)
42-43:authentication.getName()파싱 예외 처리 고려
Long.parseLong()은NumberFormatException을 발생시킬 수 있습니다. 인증 설정이 올바르다면 문제없지만, 방어적 코딩 관점에서 공통 유틸리티 메서드로 추출하는 것을 권장합니다.여러 컨트롤러에서 반복되는 패턴이므로, 공통 유틸리티 또는
@AuthenticationPrincipal커스텀 resolver 활용을 고려하세요:// 예: SecurityUtils.getCurrentUserId(Authentication auth) private Long getCurrentUserId(Authentication authentication) { try { return Long.parseLong(authentication.getName()); } catch (NumberFormatException e) { throw new GlobalException(ErrorCode.INVALID_TOKEN); } }
69-71: 페이지 크기(size) 상한 제한 권장
size파라미터에 상한이 없어 악의적인 요청(예:size=10000)으로 서버 부하를 유발할 수 있습니다.최대 크기 제한 추가:
+ private static final int MAX_PAGE_SIZE = 100; + @GetMapping("/me/followings") public ApiResponse<SliceResponseDto<FollowListResponseDto>> getMyFollowings( @RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "20") int size, Authentication authentication ) { Long currentUserId = Long.parseLong(authentication.getName()); + int validatedSize = Math.min(size, MAX_PAGE_SIZE); - Pageable pageable = PageRequest.of(page, size); + Pageable pageable = PageRequest.of(Math.max(0, page), validatedSize);또는
@Max어노테이션으로 Bean Validation을 활용할 수 있습니다.
33-61: 컨트롤러-서비스 간 중복 로깅Controller와 Service 모두 동일한 정보를 로깅하고 있습니다. 한 레이어에서만 로깅하면 로그 가독성이 향상됩니다. 일반적으로 서비스 레이어에서 비즈니스 로직 로깅을, 컨트롤러에서는 요청/응답 로깅만 수행합니다.
Controller의 상세 로깅을 간소화하거나, AOP 기반 로깅으로 통합하는 것을 고려하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/dto/FollowListResponseDto.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/dto/FollowRequestDto.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/dto/FollowResponseDto.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/entity/Follow.java(2 hunks)src/main/java/com/hrr/backend/domain/follow/entity/enums/FollowStatus.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java(1 hunks)src/main/java/com/hrr/backend/domain/follow/service/FollowService.java(1 hunks)src/main/java/com/hrr/backend/global/response/ErrorCode.java(1 hunks)src/main/java/com/hrr/backend/global/response/SuccessCode.java(1 hunks)src/main/resources/db/migration/V2.8__add_follow_status.sql(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (1)
Slf4j(20-198)
🔇 Additional comments (10)
src/main/resources/db/migration/V2.8__add_follow_status.sql (1)
2-3: 컬럼 추가 전략이 안전합니다!기본값을 'APPROVED'로 설정하여 기존 데이터 마이그레이션을 안전하게 처리했습니다. 비공개 계정이 도입되기 전의 팔로우 관계를 모두 승인된 상태로 보는 것이 논리적으로 합당합니다.
src/main/java/com/hrr/backend/domain/follow/entity/enums/FollowStatus.java (1)
8-13: 깔끔한 enum 설계입니다!PENDING/APPROVED 두 가지 상태로 팔로우 요청 플로우를 명확하게 표현했습니다. description 필드는 향후 UI 노출이나 로깅에 유용하게 활용될 수 있습니다.
src/main/java/com/hrr/backend/domain/follow/entity/Follow.java (1)
34-38: EnumType.STRING 사용이 올바릅니다!
EnumType.STRING을 사용하여 DB에서 가독성을 높이고, enum 순서 변경에 안전하게 대응했습니다. 기본값APPROVED도 마이그레이션 전략과 일치합니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
72-79: 팔로우 기능의 에러 케이스를 잘 정의했습니다!자기 자신 팔로우 방지, 중복 팔로우, 권한 검증 등 주요 엣지 케이스를 모두 커버했습니다. HTTP 상태 코드도 적절하게 선택되었습니다:
- 400 (BAD_REQUEST): 잘못된 요청
- 403 (FORBIDDEN): 권한 없음
- 404 (NOT_FOUND): 리소스 없음
- 409 (CONFLICT): 중복 상태
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
38-73: N+1 쿼리 방지 전략이 훌륭합니다!followerIds를 미리 수집한 후 한 번의 쿼리로 팔로우 관계를 조회하는 방식으로 N+1 문제를 효과적으로 해결했습니다. 이는 성능 최적화의 모범 사례입니다.
src/main/java/com/hrr/backend/domain/follow/dto/FollowListResponseDto.java (1)
28-36: 깔끔한 팩토리 메서드 설계입니다!User 엔티티를 DTO로 변환하는 로직이 명확하고,
getUserLevel().name().toLowerCase()로 enum을 소문자 문자열로 변환하는 것도 API 응답 일관성을 위해 좋은 선택입니다. isFollowing 플래그는 서비스에서 계산되어 주입되므로 관심사 분리도 잘 되어 있습니다.src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (2)
29-85: 깔끔한 팔로우 로직 구현! 👍공개/비공개 계정에 따른 상태 분기, 중복 체크, 자기 자신 팔로우 방지 등 요구사항이 잘 반영되어 있습니다.
160-182: 거절 시 APPROVED 상태 팔로우도 삭제 가능 - 의도된 동작인지 확인 필요
rejectFollowRequest는 PENDING 상태 체크 없이 삭제를 수행합니다. 이는 "팔로워 삭제" 기능으로도 사용될 수 있는데, API 이름이 "reject"이므로 PENDING 상태만 처리하는 것이 명확할 수 있습니다.의도된 동작이라면 무시해도 되지만, PENDING 전용이라면:
+ // PENDING 상태 확인 + if (follow.getStatus() != FollowStatus.PENDING) { + log.warn("PENDING 상태가 아닌 팔로우 요청입니다 - followId: {}, status: {}", followId, follow.getStatus()); + throw new GlobalException(ErrorCode.FOLLOW_REQUEST_NOT_FOUND); + } + // 요청 삭제 followRepository.delete(follow);참고: 팔로워 삭제 기능이 별도로 필요하다면
removeFollowerAPI를 분리하는 것을 권장합니다.src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (1)
155-181: 팔로우 요청 승인/거절 API 구조 적절! 👌RESTful 설계 원칙에 따라 approve는 POST, reject는 DELETE로 구현되어 있고, Swagger 문서화도 잘 되어 있습니다.
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
56-57: Empty list handling is already in placeThe calling methods in FollowListService already include defensive checks (
isEmpty()at lines 54 and 98) that return early before callingfindFollowingIdsByFollowerIdAndFollowingIds(). This prevents empty lists from being passed to the method, so the IN clause concern is already addressed.
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java
Show resolved
Hide resolved
| public static FollowRequestDto from(Follow follow) { | ||
| return FollowRequestDto.builder() | ||
| .followId(follow.getId()) | ||
| .requesterId(follow.getFollower().getId()) | ||
| .requesterNickname(follow.getFollower().getNickname()) | ||
| .requesterProfileImage(follow.getFollower().getProfileImage()) | ||
| .requestedAt(follow.getCreatedAt()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for FollowRepository file first
fd -t f "FollowRepository" src/main/java --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 249
🏁 Script executed:
#!/bin/bash
# Find FollowRepository file
find src -name "*FollowRepository*" -type f
# Search for pending/status queries in follow-related repository files
rg -A 10 "findBy.*[Pp]ending|findBy.*[Ss]tatus|@Query" src/main/java/com/hrr/backend/domain/follow --type java
# Search for JOIN FETCH patterns in follow repositories
rg "JOIN FETCH" src/main/java/com/hrr/backend/domain/follow --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 5087
🏁 Script executed:
cat -n src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 3332
findPendingFollowRequests() 쿼리에 JOIN FETCH f.follower 추가 필요
findPendingFollowRequests() 메서드의 쿼리(lines 65-69)에서 follower를 명시적으로 로드하지 않으므로, FollowRequestDto.from()이 follow.getFollower()의 여러 필드에 접근할 때 N+1 쿼리가 발생합니다. 쿼리를 다음과 같이 수정하세요:
@Query("SELECT f FROM Follow f " +
"JOIN FETCH f.follower " +
"WHERE f.following.id = :userId " +
"AND f.status = :status " +
"ORDER BY f.createdAt DESC")
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/follow/dto/FollowRequestDto.java lines
34 to 42, FollowRequestDto.from() accesses multiple follower fields causing N+1
queries because the repository's findPendingFollowRequests() query does not
eagerly load follower; update the repository query (around lines 65–69) to
include "JOIN FETCH f.follower" so the follower is fetched in the same query
(keep the WHERE f.following.id = :userId AND f.status = :status and the ORDER BY
f.createdAt DESC clauses unchanged).
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
52-78: 팔로워 목록 조회 로직은 OK, empty Slice 분기는 더 단순화할 수 있습니다팔로워 조회 흐름(존재 여부 체크 → Slice 조회 → ID 배치 조회 → DTO 매핑)과 N+1 방지 패턴 자체는 잘 잡혀 있습니다. 도메인 서비스에서 ID를 한 번에 모아서
followRepository.findFollowingIdsByFollowerIdAndFollowingIds로 조회하는 구조가 깔끔합니다.다만 Line 56–61의
followersSlice.isEmpty()분기는 논리상 필수는 아닙니다. 아래 ID 수집 +map로직은 빈 Slice도 안전하게 처리하므로, 빈 Slice일 때도 같은 경로를 타게 하면 중복을 줄이고 읽기가 더 쉬워집니다. 쿼리 최적화를 유지하면서도 early-return을 없애려면, ID 리스트 기준으로 한 번 더 분기하는 방식이 유용합니다. 예시는 다음과 같습니다:- // 팔로워가 없으면 빈 Slice 반환 - if (followersSlice.isEmpty()) { - return new SliceResponseDto<>(followersSlice.map(user -> - FollowListResponseDto.of(user, false) - )); - } - - // N+1 방지: 한 번의 쿼리로 현재 사용자가 팔로우 중인 ID 목록 조회 - List<Long> followerIds = followersSlice.getContent().stream() - .map(User::getId) - .toList(); - - Set<Long> followingIds = new HashSet<>( - followRepository.findFollowingIdsByFollowerIdAndFollowingIds(currentUserId, followerIds) - ); + // N+1 방지: 한 번의 쿼리로 현재 사용자가 팔로우 중인 ID 목록 조회 + List<Long> followerIds = followersSlice.getContent().stream() + .map(User::getId) + .toList(); + + Set<Long> followingIds = followerIds.isEmpty() + ? Set.of() + : new HashSet<>(followRepository.findFollowingIdsByFollowerIdAndFollowingIds(currentUserId, followerIds));이렇게 하면 빈 Slice에서도 추가 쿼리 없이 동일한 결과(빈 DTO Slice)를 만들 수 있고, 전체 흐름도 “Slice → IDs → Set → map”으로 일관되게 보입니다. Spring Data JPA의
Slice.map(...)가 빈 Slice에 대해서도 안전하게 동작하는 건 공식 레퍼런스(Slice/Page 섹션)에서도 보장하니, 해당 패턴을 적극 활용하셔도 좋습니다.
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java (1)
102-128: 팔로잉 목록 조회도 패턴 일관성이 좋아서 유지보수에 유리합니다 (공통화 여지)팔로워 조회와 거의 대칭 구조로 팔로잉 목록도 Slice 기반 페이징 + ID 배치 조회 +
FollowListResponseDto.of매핑까지 일관되게 구현된 점이 좋습니다. 호출자가 두 메서드를 읽을 때 흐름이 동일해서, 이후 도메인 정책이 바뀌어도 추론하기 쉽겠습니다.다만 Line 102–127의 로직은
getFollowers와 매우 유사하므로,
“Slice<User>를 받아 현재 사용자의isFollowing플래그를 계산해SliceResponseDto<FollowListResponseDto>로 바꿔주는” private 헬퍼 메서드로 공통화하는 것도 고려해볼 만합니다. 예를 들어:
extractTargetIds(Slice<User> users)resolveFollowingIds(Long currentUserId, List<Long> targetIds)mapToFollowListDtoSlice(Slice<User> users, Set<Long> followingIds)와 같이 역할을 나누거나, 하나의
convertToSliceResponse(Slice<User> slice, Long currentUserId, Function<User, Long> idExtractor)형태로 추상화하면, 추후 FollowStatus 조건이 바뀌거나FollowListResponseDto필드가 늘어났을 때 수정 포인트를 줄일 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java(1 hunks)
src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (1)
114-152: 비공개 계정 접근 제어 - 별도 이슈로 처리 예정 확인이전 리뷰에서 언급된 비공개 계정(
isPublic=false)의 팔로우 목록 접근 제어가 아직 미구현 상태입니다. 작성자께서 별도 이슈로 구현 예정이라고 답변하셨으므로, 해당 이슈가 생성되었는지 확인 부탁드립니다.
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (2)
68-72: 페이지네이션 파라미터에 상한 검증 추가 고려
size파라미터에 상한 제한이 없어, 매우 큰 값이 전달될 경우 메모리 이슈가 발생할 수 있습니다.@Max어노테이션을 활용한 검증을 권장드립니다.+ import jakarta.validation.constraints.Max; + import jakarta.validation.constraints.Min;public ApiResponse<SliceResponseDto<FollowListResponseDto>> getMyFollowings( @Parameter(description = "페이지 번호 (0부터 시작)", example = "0") - @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "0") @Min(0) int page, @Parameter(description = "페이지 크기", example = "20") - @RequestParam(defaultValue = "20") int size, + @RequestParam(defaultValue = "20") @Min(1) @Max(100) int size,동일한 패턴을 다른 페이지네이션 엔드포인트에도 적용하시면 됩니다.
100-110: 대기 중인 팔로우 요청 목록에 페이지네이션 추가 권장현재
getPendingFollowRequests는List를 반환하여 모든 대기 요청을 한 번에 조회합니다. 대기 요청이 많아질 경우 성능 저하가 발생할 수 있으므로, 다른 목록 조회 API와의 일관성 및 확장성을 위해 페이지네이션 적용을 권장드립니다.- @GetMapping("/me/requests") - public ApiResponse<List<FollowRequestDto>> getPendingFollowRequests( - @AuthenticationPrincipal CustomUserDetails customUserDetails + @GetMapping("/me/requests") + public ApiResponse<SliceResponseDto<FollowRequestDto>> getPendingFollowRequests( + @Parameter(description = "페이지 번호 (0부터 시작)", example = "0") + @RequestParam(defaultValue = "0") int page, + @Parameter(description = "페이지 크기", example = "20") + @RequestParam(defaultValue = "20") int size, + @AuthenticationPrincipal CustomUserDetails customUserDetails ) { Long currentUserId = customUserDetails.getUser().getId(); - log.info("팔로우 요청 목록 조회 - currentUserId: {}", currentUserId); + log.info("팔로우 요청 목록 조회 - currentUserId: {}, page: {}, size: {}", currentUserId, page, size); - List<FollowRequestDto> response = followService.getPendingFollowRequests(currentUserId); + Pageable pageable = PageRequest.of(page, size); + SliceResponseDto<FollowRequestDto> response = followService.getPendingFollowRequests(currentUserId, pageable); return ApiResponse.onSuccess(SuccessCode.OK, response); }
FollowService.getPendingFollowRequests메서드도 함께 수정이 필요합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (3)
24-32: 깔끔한 컨트롤러 설정! 👍Lombok과 OpenAPI 어노테이션 사용이 적절하며, 생성자 주입 방식으로 의존성을 관리하고 있습니다.
36-62: RESTful 설계 원칙 준수! 👌팔로우(POST)/언팔로우(DELETE) 엔드포인트가 적절한 HTTP 메서드를 사용하고 있으며, 서비스 레이어에 비즈니스 로직(자기 자신 팔로우 방지, 중복 체크 등)을 위임하고 있습니다.
156-182: 승인/거절 엔드포인트 구현 적절! ✨
POST /approve: 상태 변경에 적합한 메서드DELETE /reject: 요청 삭제(거절)에 적합한 메서드followId를 사용하여 직접 엔티티를 참조하는 방식이 효율적- 권한 검증(요청 수신자 확인)은 서비스 레이어에서 처리
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
117-120: 팔로우 요청용 ErrorCode 추가 방향은 적절합니다
FOLLOW_REQUEST_NOT_FOUND/UNAUTHORIZED_FOLLOW_ACTION둘 다 HTTP status, 코드 prefix, 한글 메시지까지 기존FOLLOW_*패턴과 잘 맞습니다. 🙆♂️ALREADY_APPROVED_FOLLOW추가도 서비스 레벨에서 상태 기반 검증을 명확하게 표현해 주는 좋은 선택입니다.다만, HTTP status 선택에 대해 한 가지 개선 포인트를 제안드립니다.
ALREADY_APPROVED_FOLLOW의 HTTP status 재검토 제안현재는:
ALREADY_APPROVED_FOLLOW(HttpStatus.BAD_REQUEST, "FOLLOW4006", "이미 승인된 팔로우입니다."),으로 되어 있는데, 클라이언트 입장에서 보면 “요청 형식이 잘못됐다(BAD_REQUEST)”라기보다는 리소스의 현재 상태와 충돌(CONFLICT, 409) 에 더 가깝습니다.
이미 승인된 팔로우에 대해 다시 승인 요청을 보내는 상황은 다음 성격을 가집니다.
- 요청 자체는 형식적으로 유효함 (followId 등 정상)
- 하지만 서버에 저장된 리소스의 상태(이미 APPROVED)와 비즈니스 규칙 상 충돌 발생
그래서 다음과 같이 409로 맞추는 것도 고려해볼 수 있습니다:
-ALREADY_APPROVED_FOLLOW(HttpStatus.BAD_REQUEST, "FOLLOW4006", "이미 승인된 팔로우입니다."), +ALREADY_APPROVED_FOLLOW(HttpStatus.CONFLICT, "FOLLOW4092", "이미 승인된 팔로우입니다."),이렇게 하면:
ALREADY_FOLLOWING(이미 팔로우 관계 존재)와 마찬가지로 상태 충돌은 409 계열로 일관되게 가져갈 수 있고- 후속으로 클라이언트에서 멱등 처리/리트라이 로직을 짤 때도 의미 해석이 더 직관적입니다.
물론, 팀 내에서 4xx 세부 코드를 어떻게 쓰기로 했는지 컨벤션이 이미 있다면 그 컨벤션을 우선하면 됩니다. 다만 지금 시점에 follow 쪽 에러 코드가 새로 늘어나는 상황이어서, HTTP status 및 코드 체계를 한 번 정리해 두면 이후 기능 확장 시에도 도움이 될 것 같습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java(3 hunks)src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java(2 hunks)src/main/java/com/hrr/backend/global/response/ErrorCode.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java
- src/main/java/com/hrr/backend/domain/follow/service/FollowListService.java
| log.info("사용자 팔로우 완료 - followerId: {}, followingId: {}, status: {}", | ||
| currentUserId, followedUserId, status); | ||
|
|
||
| return FollowResponseDto.of(message, followedUserId); |
There was a problem hiding this comment.
팔로우 완료/팔로우 요청 이 두 가지 결과에 대해 별도의 메시지로 반환을 하고 있는데 이 응답으로 프론트가 분기 처리를 해야하기 때문에 일관되게 유지할 수 있도록 status를 담아 반환하는 게 좋을 것 같아요!
| * @param followId 팔로우 ID | ||
| * @return FollowResponseDto | ||
| */ | ||
| @Transactional | ||
| public FollowResponseDto approveFollowRequest(Long currentUserId, Long followId) { | ||
| log.info("팔로우 요청 승인 - userId: {}, followId: {}", currentUserId, followId); | ||
|
|
||
| // 팔로우 요청 조회 | ||
| Follow follow = followRepository.findById(followId) | ||
| .orElseThrow(() -> { | ||
| log.warn("팔로우 요청을 찾을 수 없습니다 - followId: {}", followId); | ||
| return new GlobalException(ErrorCode.FOLLOW_REQUEST_NOT_FOUND); | ||
| }); |
There was a problem hiding this comment.
여기서 followId에 대해 정리가 필요할 것 같아요!
지금은 인자로 넘겨받는 걸로 봐서 팔로우되는 사람의 userId인 것 같은데, 그러면 followRepository의 findById는 follow 엔티티의 id를 조회하기 때문에 의도와 다르게 실행될 가능성이 커보입니다.
| * @param followId 팔로우 ID | ||
| * @return FollowResponseDto | ||
| */ | ||
| @Transactional | ||
| public FollowResponseDto rejectFollowRequest(Long currentUserId, Long followId) { | ||
| log.info("팔로우 요청 거절 - userId: {}, followId: {}", currentUserId, followId); | ||
|
|
||
| // 팔로우 요청 조회 | ||
| Follow follow = followRepository.findById(followId) |
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "status", nullable = false) | ||
| @Builder.Default | ||
| private FollowStatus status = FollowStatus.APPROVED; |
There was a problem hiding this comment.
스크립트 파일의 최신 버전이 2.12였나? 그래서 2.13 등으로 변경 부탁드립니다
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/follow/dto/FollowResponseDto.java (1)
20-21: id 필드 의미가 여전히 모호합니다.이전 리뷰에서 지적된 대로 "관련 ID (userId 또는 followId)" 설명이 API 소비자에게 혼란을 줄 수 있습니다. 각 엔드포인트별로 반환하는 ID가 무엇인지 명확히 문서화하거나, 용도별로 DTO를 분리하는 것을 고려해주세요.
🧹 Nitpick comments (4)
src/main/java/com/hrr/backend/domain/user/dto/UserVerificationResponseDto.java (2)
30-58:verifiedAt의 시간대 표현(LocalDateTime +'Z'포맷) 일관성 한번 점검해 주세요지금은
LocalDateTime에@JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'")를 사용하고 있어서, 실제로는 시간대 정보가 없는 로컬 시간인데 응답 문자열은...Z(보통 UTC를 의미)로 내려가게 됩니다.
- 실제로 서버/DB에 UTC로 저장하고 있다면,
OffsetDateTime또는ZonedDateTime으로 타입을 올리고 패턴을...XXX형태로 주는 것도 고려할 수 있습니다.- 반대로 KST 기준 로컬 시간을 표현하려는 의도라면,
'Z'를 제거하거나timezone속성을 명시해 주는 편이 API 소비자 입장에서 덜 헷갈립니다.- 팀에서 정한 “시간/타임존 표준”이 있다면, 이 필드가 그 규칙을 따르고 있는지 한 번만 맞춰봐 주세요.
Jackson
@JsonFormat관련 공식 문서(예:@JsonFormat+LocalDateTime/OffsetDateTime사용법)를 참고하면 팀 컨벤션 정리에도 도움이 됩니다.
60-87: 페이지네이션 DTO를 공용 타입으로 추출하는 것 고려
PaginationDto구조(totalCount,currentPage,totalPages,pageSize)가 매우 범용적인 형태라, 팔로우 목록·챌린지 목록 등 다른 API에서도 동일/유사한 DTO가 생길 가능성이 큽니다.
- 현재는
UserVerificationResponseDto안에 중첩 클래스로 정의되어 있어 이 도메인에만 한정되어 재사용하기가 어렵습니다.- 향후 다른 응답에서도 동일한 형태의 페이지네이션을 쓴다면,
com.hrr.backend.global.dto.PaginationResponseDto같은 공용 DTO로 추출해 두면 중복 정의와 Swagger 스키마 난립을 줄일 수 있습니다.지금 당장 필수는 아니지만, 리스트 API가 늘어날수록 이 refactor의 효과가 커질 것이므로 중·장기적으로 한 번 검토해 보시면 좋겠습니다.
src/main/resources/db/migration/V2.13__add_follow_status.sql (1)
1-2: status 컬럼에 인덱스 추가를 권장합니다.
FollowRepository.findPendingFollowRequests에서status컬럼으로 필터링하는 쿼리가 사용됩니다. 팔로우 데이터가 증가하면 인덱스 없이 성능 저하가 발생할 수 있습니다.-- Follow 테이블에 status 컬럼 추가 ALTER TABLE follow ADD COLUMN status VARCHAR(20) NOT NULL DEFAULT 'APPROVED'; + +-- status 컬럼 인덱스 추가 (팔로우 요청 조회 성능 최적화) +CREATE INDEX idx_follow_status ON follow(status);src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (1)
99-109: 페이지네이션 적용을 권장합니다.다른 목록 조회 API들(
/me/followings,/me/followers)은SliceResponseDto로 페이지네이션을 지원하지만, 이 엔드포인트는List로 전체를 반환합니다. 팔로우 요청이 많아지면 응답 크기와 성능에 문제가 될 수 있습니다.일관성과 확장성을 위해 페이지네이션 적용을 고려해주세요. 참고: Spring Data Slice 문서
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java(3 hunks)src/main/java/com/hrr/backend/domain/follow/dto/FollowResponseDto.java(2 hunks)src/main/java/com/hrr/backend/domain/follow/entity/Follow.java(2 hunks)src/main/java/com/hrr/backend/domain/follow/service/FollowService.java(6 hunks)src/main/java/com/hrr/backend/domain/user/dto/UserVerificationResponseDto.java(1 hunks)src/main/resources/db/migration/V2.13__add_follow_status.sql(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/user/dto/UserVerificationResponseDto.java (1)
src/main/java/com/hrr/backend/domain/follow/dto/FollowResponseDto.java (1)
Getter(10-33)
🔇 Additional comments (5)
src/main/java/com/hrr/backend/domain/user/dto/UserVerificationResponseDto.java (1)
14-28: 컨테이너형 DTO 설계 깔끔합니다 👍
VerificationHistoryDto가verificationList + pagination으로 잘 묶여 있어서, 응답 스키마를 한 곳에서 관리하기 좋습니다. 이런 패턴은 이후 다른 도메인(팔로우 목록, 챌린지 목록 등)에도 재사용하기 좋아 유지보수성이 높습니다.src/main/java/com/hrr/backend/domain/follow/entity/Follow.java (1)
33-43: LGTM! 깔끔한 구현입니다 👍
@Builder.Default로 새 팔로우의 기본값을PENDING으로 설정하고,approve()메서드로 상태 변경을 캡슐화한 점이 좋습니다. DB 마이그레이션에서 기존 레코드는APPROVED로, 엔티티에서 신규 레코드는PENDING으로 설정하는 분리된 설계가 적절합니다.src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java (1)
155-181: 승인/거절 API 구조가 잘 설계되었습니다 ✨
requesterId를 path variable로 받아 현재 사용자와의 관계를 명확히 하고,@AuthenticationPrincipal로 현재 사용자를 안전하게 추출하는 패턴이 적절합니다. 로깅도 충실하게 포함되어 있어 디버깅에 유용할 것입니다.src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (2)
65-84: 공개/비공개 계정 분기 로직이 명확합니다 👏
followedUser.getIsPublic()으로 상태를 결정하고, 응답 메시지와 status를 함께 반환하여 프론트엔드 분기 처리가 용이하도록 구현되었습니다. 이전 리뷰 피드백이 잘 반영되었네요!
124-148: 승인 로직이 정확합니다.
PENDING상태 확인 후follow.approve()호출로 상태를 변경하는 흐름이 명확합니다.@Transactional이 적용되어 있어 dirty checking으로 자동 저장됩니다.
| public List<FollowRequestDto> getPendingFollowRequests(Long currentUserId) { | ||
| log.info("받은 팔로우 요청 목록 조회 - userId: {}", currentUserId); | ||
|
|
||
| List<Follow> pendingFollows = followRepository.findPendingFollowRequests(currentUserId, FollowStatus.PENDING); | ||
|
|
||
| return pendingFollows.stream() | ||
| .map(FollowRequestDto::from) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find FollowRepository and check the findPendingFollowRequests implementation
find . -type f -name "FollowRepository.java" | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 156
🏁 Script executed:
# Find FollowService to understand the full context
find . -type f -name "FollowService.java" | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 150
🏁 Script executed:
# Check for project configuration files
find . -type f \( -name "pom.xml" -o -name "build.gradle" \) | head -10Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 92
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 3332
🏁 Script executed:
cat -n ./build.gradleRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 3057
🏁 Script executed:
# Find FollowRequestDto
find . -type f -name "FollowRequestDto.java" | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 149
🏁 Script executed:
# Find Follow entity
find . -type f -name "Follow.java" | grep -E "domain|entity" | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 142
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/follow/dto/FollowRequestDto.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1734
🏁 Script executed:
cat -n ./src/main/java/com/hrr/backend/domain/follow/entity/Follow.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1541
🏁 Script executed:
# Verify if there are other usages of findPendingFollowRequests in the codebase
rg -n "findPendingFollowRequests" --type javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 463
Fix N+1 query issue and modernize stream collection for Java 17.
The findPendingFollowRequests query does not eagerly load the follower relationship. Since Follow.follower uses FetchType.LAZY and FollowRequestDto::from() accesses follow.getFollower().getId(), getNickname(), and getProfileImage(), this triggers an N+1 query problem. Add JOIN FETCH f.follower to the repository query to load the follower in a single query.
Additionally, with Java 17, replace Collectors.toList() with .toList():
return pendingFollows.stream()
.map(FollowRequestDto::from)
- .collect(Collectors.toList());
+ .toList();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/hrr/backend/domain/follow/service/FollowService.java around
lines 187 to 195, the service maps pending Follow entities to DTOs while the
repository query does not eagerly load the follower, causing an N+1 when
FollowRequestDto::from accesses follower fields; update the repository query
used by findPendingFollowRequests to include JOIN FETCH f.follower so the
follower is loaded in the same query, and in this service method modernize the
stream terminal operation by replacing .collect(Collectors.toList()) with the
Java 17 .toList() call.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
59-69: 받은 요청 조회 쿼리에JOIN FETCH f.follower를 추가해 N+1을 방지하는 것이 좋습니다.
- 현재 쿼리:
@Query("SELECT f FROM Follow f " + "WHERE f.following.id = :userId " + "AND f.status = :status " + "ORDER BY f.createdAt DESC") Slice<Follow> findPendingFollowRequests(...);
FollowService.getPendingFollowRequests에서 이 결과를 받아Slice.map(FollowRequestDto::from)으로 매핑하는데,
FollowRequestDto.from()가follow.getFollower().getId()/getNickname()/getProfileImage()등을 접근하고,
Follow.follower가LAZY라면 팔로우 요청마다 추가 조회가 발생하는 전형적인 N+1 패턴입니다.
(이 이슈는 이전 리뷰 코멘트에서도 이미 지적된 바 있습니다.)개선 예시:
@Query(""" SELECT f FROM Follow f JOIN FETCH f.follower WHERE f.following.id = :userId AND f.status = :status ORDER BY f.createdAt DESC """) Slice<Follow> findPendingFollowRequests(@Param("userId") Long userId, @Param("status") FollowStatus status, Pageable pageable);
- 이렇게 하면 한 번의 쿼리로
Follow+follower를 모두 가져와, 서비스 레이어에서 DTO로 변환할 때 추가 쿼리 없이 처리할 수 있습니다.- 페이지네이션은
Slice기반이라count가 필요 없고,JOIN FETCH도 큰 문제 없이 동작하는 패턴입니다. 다만 데이터량이 매우 커지는 경우에는 인덱스(following_id,status,created_at)도 함께 고려해 보시면 좋겠습니다.src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (1)
185-203: 받은 요청 목록 Javadoc 및 N+1 쿼리 가능성 정리 제안 (이전 리뷰와 동일 이슈).
Javadoc 반환 타입 불일치
- 주석에는
@return List<FollowRequestDto>라고 되어 있지만 실제 시그니처는SliceResponseDto<FollowRequestDto>입니다.- IDE/Javadoc만 보고 사용할 때 혼동을 줄이기 위해, 주석을 실제 반환 타입과 맞춰 수정하는 것이 좋습니다.
예시:
/** * 받은 팔로우 요청 목록 조회 * @param currentUserId 현재 로그인한 사용자 ID * @param pageable 페이징 정보 * @return 받은 팔로우 요청 Slice 응답 DTO */
FollowRequestDto.from매핑 시 N+1 쿼리 가능성 (이전 코멘트와 동일 이슈입니다.)
Slice<Follow> pendingFollowsSlice = followRepository.findPendingFollowRequests(...)후,
pendingFollowsSlice.map(FollowRequestDto::from)으로 DTO를 만들고 있습니다.Follow엔티티의follower가LAZY이고,FollowRequestDto.from()에서follow.getFollower().getId()/getNickname()/getProfileImage()등을 접근한다면,
각 팔로우 요청마다follower를 개별 조회하는 N+1 쿼리가 발생할 수 있습니다.개선 방향:
FollowRepository.findPendingFollowRequests쿼리 쪽에서JOIN FETCH f.follower를 추가해 한 번에 로딩하는 것을 추천드립니다. (아래 리포지토리 코멘트에 예시 포함)- 서비스단 로직은 현재
Slice.map(...)패턴이 깔끔하므로 그대로 두고, 페치 전략만 리포지토리에서 조정하는 편이 좋습니다.
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/follow/service/FollowService.java (2)
62-88: 공개/비공개 분기 & 응답 구조는 요구사항과 잘 맞습니다.
- 공개 계정 →
APPROVED, 비공개 계정 →PENDING분기가 명확하고,FollowResponseDto.of(message, followedUserId, status)로status까지 내려주는 구조라 프론트 분기에도 적합해 보입니다.- 다만 현재는 영어 메시지 문자열에 강하게 의존하고 있어 i18n/문구 변경 시 영향 범위가 커질 수 있습니다. 장기적으로는 메시지는 프론트/문구 리소스로 위임하고, 백엔드는
status+ 코드(enum 등) 위주로 내려주는 쪽을 고려해보셔도 좋겠습니다.
96-119: 언팔로우 시status = null계약을 명시적으로 문서화하는 것을 추천합니다.
- 언팔로우 시 상태에 상관없이(
PENDING/APPROVED) 관계를 삭제하고,FollowResponseDto.of("User unfollowed successfully", unfollowedUserId, null)로status를null로 내려주고 있습니다.- 이는 “현재 팔로우 관계가 없음”을 표현하는 일관된 신호로 유용하지만, 다른 곳에서
status를 enum으로만 가정하면 NPE/분기 누락 위험이 있습니다.제안:
FollowResponseDto의 Javadoc 혹은 클래스-level 주석에 “언팔로우/거절 시status는 null”이라는 계약을 명시해 두면, 이후 사용하는 쪽에서 명시적으로null처리를 하게 되어 안전성이 올라갑니다.src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java (1)
32-57:APPROVED문자열 리터럴 대신 enum 기반 필터로 통일하는 것을 추천합니다.
findFollowersByUserId,findFollowingsByUserId,findFollowingIdsByFollowerIdAndFollowingIds에서 모두
f.status = 'APPROVED'문자열을 직접 사용하고 있습니다.- 이제
FollowStatusenum이 도입되어 있고, 이 파일에서도 이미 임포트하고 있으므로 문자열 리터럴보다는 enum을 사용하는 편이 타입 안정성과 유지보수성 측면에서 더 좋습니다.
- 예를 들어 enum 이름이 리팩터링되거나 매핑 전략이 바뀌면, 문자열 기반 JPQL은 컴파일 타임에 깨지지 않고 런타임에만 터질 수 있습니다.
개선 예시 (둘 중 택1):
- 파라미터 방식
@Query("SELECT f.follower FROM Follow f " + "WHERE f.following.id = :userId " + "AND f.status = :status " + "ORDER BY f.createdAt DESC") Slice<User> findFollowersByUserId(@Param("userId") Long userId, @Param("status") FollowStatus status, Pageable pageable);
- enum 상수 직접 사용
@Query("SELECT f.follower FROM Follow f " + "WHERE f.following.id = :userId " + "AND f.status = com.hrr.backend.domain.follow.entity.enums.FollowStatus.APPROVED " + "ORDER BY f.createdAt DESC")현재
PENDING쿼리(findPendingFollowRequests)는 이미:status파라미터를 사용하고 있으니, 같은 패턴으로 통일하면 읽기도 더 쉬울 것 같습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java(3 hunks)src/main/java/com/hrr/backend/domain/follow/repository/FollowRepository.java(2 hunks)src/main/java/com/hrr/backend/domain/follow/service/FollowService.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/hrr/backend/domain/follow/controller/FollowController.java
| /** | ||
| * 팔로우 요청 승인 | ||
| * @param currentUserId 현재 로그인한 사용자 ID (요청 받은 사람) | ||
| * @param requesterId 팔로우 요청한 사용자 ID | ||
| * @return FollowResponseDto | ||
| */ | ||
| @Transactional | ||
| public FollowResponseDto approveFollowRequest(Long currentUserId, Long requesterId) { | ||
| log.info("팔로우 요청 승인 - currentUserId: {}, requesterId: {}", currentUserId, requesterId); | ||
|
|
||
| // 요청한 사용자 존재 여부 확인 | ||
| userRepository.findById(requesterId) | ||
| .orElseThrow(() -> { | ||
| log.warn("요청한 사용자를 찾을 수 없습니다 - userId: {}", requesterId); | ||
| return new GlobalException(ErrorCode.USER_NOT_FOUND); | ||
| }); | ||
|
|
||
| // PENDING 상태의 팔로우 요청 조회 | ||
| Follow follow = followRepository | ||
| .findByFollowerIdAndFollowingIdAndStatus(requesterId, currentUserId, FollowStatus.PENDING) | ||
| .orElseThrow(() -> { | ||
| log.warn("팔로우 요청을 찾을 수 없습니다 - requesterId: {}, currentUserId: {}", requesterId, currentUserId); | ||
| return new GlobalException(ErrorCode.FOLLOW_REQUEST_NOT_FOUND); | ||
| }); | ||
|
|
||
| // 승인 처리 | ||
| follow.approve(); | ||
| log.info("팔로우 요청 승인 완료 - requesterId: {}, currentUserId: {}", requesterId, currentUserId); | ||
|
|
||
| return FollowResponseDto.of("Follow request approved successfully", requesterId, FollowStatus.APPROVED); | ||
| } | ||
|
|
||
| /** | ||
| * 팔로우 요청 거절/삭제 | ||
| * @param currentUserId 현재 로그인한 사용자 ID (요청 받은 사람) | ||
| * @param requesterId 팔로우 요청한 사용자 ID | ||
| * @return FollowResponseDto | ||
| */ | ||
| @Transactional | ||
| public FollowResponseDto rejectFollowRequest(Long currentUserId, Long requesterId) { | ||
| log.info("팔로우 요청 거절 - currentUserId: {}, requesterId: {}", currentUserId, requesterId); | ||
|
|
||
| // 요청한 사용자 존재 여부 확인 | ||
| userRepository.findById(requesterId) | ||
| .orElseThrow(() -> { | ||
| log.warn("요청한 사용자를 찾을 수 없습니다 - userId: {}", requesterId); | ||
| return new GlobalException(ErrorCode.USER_NOT_FOUND); | ||
| }); | ||
|
|
||
| // PENDING 상태의 팔로우 요청 조회 | ||
| Follow follow = followRepository | ||
| .findByFollowerIdAndFollowingIdAndStatus(requesterId, currentUserId, FollowStatus.PENDING) | ||
| .orElseThrow(() -> { | ||
| log.warn("팔로우 요청을 찾을 수 없습니다 - requesterId: {}, currentUserId: {}", requesterId, currentUserId); | ||
| return new GlobalException(ErrorCode.FOLLOW_REQUEST_NOT_FOUND); | ||
| }); | ||
|
|
||
| // 요청 삭제 | ||
| followRepository.delete(follow); | ||
| log.info("팔로우 요청 거절 완료 - requesterId: {}, currentUserId: {}", requesterId, currentUserId); | ||
|
|
||
| return FollowResponseDto.of("Follow request rejected successfully", requesterId, null); | ||
| } |
There was a problem hiding this comment.
Clarify authorization handling and validation consistency in follow approval/rejection
The code does show an inconsistency worth addressing:
-
Authorization error code usage: The current implementation uses only
FOLLOW_REQUEST_NOT_FOUNDwhen a follow request cannot be found or the user lacks authorization. If the requirement specifies thatUNAUTHORIZED_FOLLOW_ACTIONshould distinguish cases where the request exists but the user lacks permission to approve/reject it, this pattern should be implemented explicitly rather than hidden behind a 404 response. This affects API contract clarity and client-side error handling. Consider clarifying with your team whether:- Unauthorized access should return 403 with
UNAUTHORIZED_FOLLOW_ACTION, or - Security-through-obscurity (404 for both missing and unauthorized) is the intended pattern, in which case document this design choice.
- Unauthorized access should return 403 with
-
Inconsistent user validation: Both
approveFollowRequestandrejectFollowRequestvalidate thatrequesterIdexists but do not validatecurrentUserId. If other methods likefollowUserandunfollowUservalidate both parties' existence, consider adding the same check for consistency:userRepository.findById(currentUserId) .orElseThrow(() -> { log.warn("사용자를 찾을 수 없습니다 - userId: {}", currentUserId); return new GlobalException(ErrorCode.USER_NOT_FOUND); });
This ensures uniform validation across the service.
| import java.time.LocalDateTime; | ||
| import java.util.List; | ||
|
|
||
| public class UserVerificationResponseDto { |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/resources/db/migration/V2.18__add_follow_status.sql (1)
1-2: 마이그레이션 로직은 정확합니다! DEFAULT 'APPROVED' 설정으로 기존 팔로우 데이터가 자동 마이그레이션되는 부분이 잘 처리되었네요.다만 데이터 무결성과 성능을 위해 다음 개선을 고려해주세요:
1. DB 레벨 상태값 검증 (CHECK 제약조건)
현재 상태값 검증이 애플리케이션 레이어에만 의존하고 있습니다. 데이터베이스 차원의 제약을 추가하면 직접 SQL 실행 시에도 잘못된 상태값이 삽입되는 것을 방지할 수 있습니다.
ALTER TABLE follow ADD CONSTRAINT follow_status_check CHECK (status IN ('PENDING', 'APPROVED'));관련 문서: MySQL CHECK Constraint
2. 쿼리 성능 최적화를 위한 인덱싱
status 컬럼은 팔로워/팔로잉 목록 조회(APPROVED), 팔로우 요청 조회(PENDING) 등에서 자주 필터링됩니다. 대규모 데이터셋에서는 다음 인덱스 추가를 권장합니다:
-- 기본 인덱스 CREATE INDEX idx_follow_status ON follow(status); -- 또는 더 최적화된 복합 인덱스 (자주 함께 조회되는 컬럼 포함) CREATE INDEX idx_follow_status_followee ON follow(followee_id, status); CREATE INDEX idx_follow_status_follower ON follow(follower_id, status);관련 문서: MySQL Index Design
There was a problem hiding this comment.
이미 해당 칼럼이 추가되었는데 이 스크립트가 다시 실행될 경우 충돌이 일어날 수 있기 때문에 멱등성을 만족할 수 있도록 변경이 필요합니다
| -- Follow 테이블에 status 컬럼 추가 (없을 경우에만) | ||
| ALTER TABLE follow | ||
| ADD COLUMN IF NOT EXISTS status VARCHAR(20) NOT NULL DEFAULT 'PENDING'; No newline at end of file |
There was a problem hiding this comment.
기존 팔로우 레코드가 PENDING으로 설정되는 치명적 문제
멱등성 수정(IF NOT EXISTS)은 잘 적용되었습니다! 👍 하지만 두 가지 중대한 문제가 있습니다:
-
DEFAULT 값 문제: 현재
DEFAULT 'PENDING'으로 설정되어 있어, 마이그레이션 실행 시 기존의 모든 팔로우 관계가 'PENDING' 상태로 설정됩니다. PR 목표에 따르면 기존 레코드는 'APPROVED'여야 합니다. -
UPDATE 구문 누락: 기존 데이터를 'APPROVED'로 마이그레이션하는 구문이 없습니다.
이 상태로 배포하면 기존 사용자들의 팔로우 관계가 모두 대기 상태가 되어 데이터 무결성이 깨집니다.
🔎 제안하는 수정안
-- Follow 테이블에 status 컬럼 추가 (없을 경우에만)
ALTER TABLE follow
-ADD COLUMN IF NOT EXISTS status VARCHAR(20) NOT NULL DEFAULT 'PENDING';
+ADD COLUMN IF NOT EXISTS status VARCHAR(20) NOT NULL DEFAULT 'APPROVED';
+
+-- 기존 팔로우 레코드를 APPROVED 상태로 마이그레이션
+UPDATE follow
+SET status = 'APPROVED'
+WHERE status = 'APPROVED';참고:
- 새로운 팔로우 요청의 초기 상태는 애플리케이션 레이어(FollowService)에서 계정 공개 여부에 따라 결정되므로, DEFAULT 값은 기존 데이터 보호를 위해 'APPROVED'가 적절합니다.
- UPDATE 구문은 멱등성을 위해 WHERE 조건을 포함했습니다.
💡 선택사항: 타입 안정성 개선
더 나은 타입 안정성을 위해 데이터베이스의 ENUM 타입 사용을 고려해보세요:
-- MySQL의 경우
ALTER TABLE follow
ADD COLUMN IF NOT EXISTS status ENUM('PENDING', 'APPROVED') NOT NULL DEFAULT 'APPROVED';
-- PostgreSQL의 경우 (먼저 타입 생성 필요)
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'follow_status') THEN
CREATE TYPE follow_status AS ENUM ('PENDING', 'APPROVED');
END IF;
END $$;
ALTER TABLE follow
ADD COLUMN IF NOT EXISTS status follow_status NOT NULL DEFAULT 'APPROVED';장점: 잘못된 값 입력 방지, 성능 향상, 명시적 타입 선언
참고: Flyway Best Practices
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V2.18__add_follow_status.sql lines 1-3,
change the migration so existing follow rows are not set to PENDING: set the
column DEFAULT to 'APPROVED' instead of 'PENDING' and add an idempotent UPDATE
statement after adding the column to explicitly set any preexisting rows to
'APPROVED' (use a WHERE that only affects non-APPROVED or NULL statuses so the
migration remains safe to re-run). Optionally consider using a DB enum type for
the status column for stronger typing as suggested.
There was a problem hiding this comment.
아마 IF NOT EXISTS가 안 될 거라 프로시저 기반으로 해주셔야 할 것 같아요. 그리고 기본값 변경 부탁드립니다
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
비공개 계정을 위한 팔로우 요청/승인 시스템 구현
DB 스키마 변경
FollowStatusEnum 추가 (PENDING, APPROVED)Follow엔티티에status필드 추가Repository 수정
status='APPROVED'필터 추가✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.