[COZY-203] feat: 친구 상태 조회

[jpark0506]

#️⃣ 요약 설명

친구인지 아닌지, 친구 요청 대기중인지 조회해주는 API입니다.
친구 목록 조회하는 API 경로를 수정했습니다.

📝 작업 내용

이 API는 다음 화면에서 사용하려고 만들었습니다.

간단간단한 로직입니다.

1. 나에게 요청 보낸건지 검사
2. 친구 요청이 있는지 검사하고, 있다면 해당 STATUS 리턴
3. 없다면 STRANGER ENUM 리턴

서비스 코드

    public FriendStatus getFriendStatus(Member member, Long friendId) {

        // 요청하는 사람과 수락하는 사람이 같은지 검사
        if (member.getId().equals(friendId)) {
            throw new GeneralException(ErrorStatus._FRIEND_REQUEST_EQUAL);
        }

        return friendRepository.findBySenderIdAndReceiverIdOrReceiverIdAndSenderId(
                member.getId(), friendId, member.getId(), friendId
            )
            .map(Friend::getStatus)
            .orElse(FriendStatus.STRANGER);
    }

동작 확인

낯선 사람일 때

친구 요청이 대기중일 때

친구 요청을 수락했을 때

💬 리뷰 요구사항(선택)

ENUM에 STRANGER라는 경우의 수를 추가하긴 했는데, DB에는 영향이 없습니다.
적절한 방식인지는 잘 모르겠는데 더 좋은 방법 아시는 분은 공유 부탁드립니다~

[eple0329]

[REVIEW]

안녕하세요. 요청하신 코드 리뷰를 한국어로 작성해 드리겠습니다.

FriendController.java

1. API 문서화 개선

   • @Operation 어노테이션의 description 필드에서 문장 구조와 설명을 개선할 수 있습니다.

   @Operation(
       summary = "[포비] 친구 상태 조회",
       description = "사용자 토큰으로 인증하며, 다음과 같은 친구 상태 문자열을 반환합니다:\n" +
           "- STRANGER: 친구 아님\n" +
           "- WAITING: 친구 요청 대기 중\n" +
           "- ACCEPT: 친구 요청 수락됨\n" +
           "Android에서는 ENUM으로, iOS에서는 타입이나 인터페이스로 처리하시기 바랍니다."
   )

2. URL 경로 일관성

   • /list와 /{memberId} 엔드포인트의 경로가 일관성이 없습니다. 다음과 같이 변경을 고려해보세요:

   @GetMapping("/list")  // 현재
   @GetMapping("")  // 제안: 루트 경로 사용
   
   @GetMapping("/{memberId}")  // 현재
   @GetMapping("/status/{memberId}")  // 제안: 목적을 명확히 하는 경로

3. 변수명 일관성

   • getFriendStatus 메서드에서 @PathVariable Long friendId를 사용하지만, 설명에는 memberId로 되어 있습니다. 일관성을 위해 변수명을 통일하는 것이 좋습니다.

FriendStatus.java

4. 열거형 상수 순서
   • STRANGER를 첫 번째로 배치하는 것이 논리적 순서에 맞습니다.

   public enum FriendStatus {
       STRANGER("STRANGER"), WAITING("WAITING"), ACCEPT("ACCEPT");
       // ...
   }

FriendQueryService.java

5. 예외 처리 개선

   • getFriendStatus 메서드에서 예외를 던질 때, 더 구체적인 예외 타입을 사용하는 것이 좋습니다.

   throw new IllegalArgumentException("친구 요청 대상이 자기 자신입니다.");

6. Optional 사용 개선

   • friendRequest.isPresent()와 get() 대신 map()을 사용하여 코드를 간결하게 만들 수 있습니다.

   return friendRequest
       .map(friend -> friend.getStatus().toString())
       .orElse(FriendStatus.STRANGER.toString());

7. 상수 사용

   • 문자열 "STRANGER"를 여러 번 사용하는 대신, 상수로 정의하여 사용하는 것이 좋습니다.

8. 트랜잭션 관리

   • getFriendStatus 메서드에 @Transactional(readOnly = true) 어노테이션을 추가하여 읽기 전용 트랜잭션으로 최적화할 수 있습니다.

전반적으로 코드는 잘 구조화되어 있으며, 기능적으로 문제가 없어 보입니다. 위의 제안사항들을 고려하여 코드의 가독성과 유지보수성을 더욱 향상시킬 수 있을 것 같습니다.

[eple0329]

[REVIEW]

안녕하세요. 제공해 주신 코드 변경사항에 대한 리뷰를 한국어로 작성해 드리겠습니다.

1. FriendController.java

1.1 친구 목록 API 엔드포인트 변경

@GetMapping("/list")

• 기존의 / 엔드포인트를 /list로 변경한 것은 좋은 개선입니다. API의 목적을 더 명확하게 나타냅니다.

1.2 새로운 API 엔드포인트 추가

@GetMapping("/{memberId}")
public ResponseEntity<ApiResponse<FriendStatus>> getFriendStatus(
    @AuthenticationPrincipal MemberDetails memberDetails, @PathVariable Long memberId) {
    // ...
}

• 친구 상태를 조회하는 새로운 API가 추가되었습니다. 이는 유용한 기능입니다.
• 하지만 @SwaggerApiError에 ErrorStatus._FRIEND_REQUEST_EQUAL만 명시되어 있습니다. _MEMBER_NOT_FOUND 등의 다른 가능한 에러도 추가하는 것이 좋겠습니다.

개선 제안:

• API 문서화를 위해 Swagger 어노테이션을 사용하고 있는데, 반환값에 대한 설명을 추가하면 좋겠습니다. 예를 들어:

@ApiResponse(responseCode = "200", description = "친구 상태 조회 성공", 
    content = @Content(schema = @Schema(implementation = FriendStatus.class)))

2. FriendStatus.java

public enum FriendStatus {
    STRANGER("STRANGER"), WAITING("WAITING"), ACCEPT("ACCEPT");
    // ...
}

• STRANGER 상태가 추가된 것은 좋은 개선입니다. 이를 통해 친구 관계가 아직 없는 상태를 명확히 표현할 수 있게 되었습니다.

3. FriendQueryService.java

public FriendStatus getFriendStatus(Member member, Long friendId) {
    // ...
}

• 새로운 메서드가 추가되어 친구 상태를 조회할 수 있게 되었습니다.
• 자기 자신과의 친구 관계를 요청하는 경우에 대한 예외 처리가 잘 되어 있습니다.

개선 제안:

• friendRepository.findBySenderIdAndReceiverIdOrReceiverIdAndSenderId 메서드 이름이 매우 길어 가독성이 떨어집니다. 이를 별도의 메서드로 추출하거나, 쿼리 메서드 이름을 간소화하는 것이 좋겠습니다.

private Optional<Friend> findFriendRelation(Long memberId, Long friendId) {
    return friendRepository.findBySenderIdAndReceiverIdOrReceiverIdAndSenderId(
        memberId, friendId, memberId, friendId);
}

종합 의견:

전반적으로 코드 품질이 양호하며, 새로운 기능 추가가 잘 이루어졌습니다. API 문서화, 예외 처리, 그리고 코드 구조화에 주의를 기울이고 있어 좋습니다. 다만, 일부 메서드 이름과 에러 처리 부분에서 약간의 개선이 가능해 보입니다. 또한, 새로 추가된 기능에 대한 단위 테스트를 작성하는 것도 고려해 보시기 바랍니다.

[eple0329]

[REVIEW]

안녕하세요. 코드 리뷰 내용은 다음과 같습니다:

1. FriendController.java

   • [1] @GetMapping("/list") 메서드에서 @GetMapping("/") 으로 변경하는 것이 더 적절할 것 같습니다. 이는 친구 목록을 조회하는 기본 엔드포인트로 사용하는 것이 더 일반적인 패턴입니다.
   • [2] @GetMapping("/{memberId}") 메서드에서 getFriendStatus 메서드 구현이 잘 되어 있습니다. 특히 요청하는 사람과 수락하는 사람이 같은지 검사하는 부분이 적절합니다.

2. FriendStatus.java

   • [3] FriendStatus 열거형에 STRANGER 상태가 추가되어 친구 관계 상태를 더 잘 표현할 수 있게 되었습니다.

3. FriendQueryService.java

   • [4] getFriendStatus 메서드에서 friendRepository.findBySenderIdAndReceiverIdOrReceiverIdAndSenderId 를 사용하여 친구 관계 상태를 효과적으로 조회하고 있습니다.

전반적으로 친구 관리 기능에 대한 구현이 잘 되어 있습니다. 특히 친구 상태 관리 부분이 잘 설계되어 있습니다. 추가적인 개선 사항은 없어 보이며, 코드 리뷰 시 특별히 지적할 만한 사항은 없습니다.

[veronees]

LGTM~ 프론트를 하나도 몰라서 궁금한게 생겼는데, 프론트에서 기존 해당 화면 조회하는 API에 추가로 친구 상태 조회 API도 요청해서 화면 하나가 나오는건가요?

[genius00hwan]

LGTM

[jpark0506]

LGTM~ 프론트를 하나도 몰라서 궁금한게 생겼는데, 프론트에서 기존 해당 화면 조회하는 API에 추가로 친구 상태 조회 API도 요청해서 화면 하나가 나오는건가요?

맞습니다 사용자 정보 상세 조회랑 친구 상태 조회는 다른 도메인이라 API 요청을 따로 하는 것이 좋다고 생각했습니다~ 그래서 화면 출력할 때 하나의 화면에 API 두개를 사용하도록 설계했어용