Skip to content
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

[BE] docs: 작성한 리뷰 목록 조회, 회원용 리뷰 그룹 생성, 리뷰 그룹 정보, 리뷰 등록 API 문서를 작성한다. #1017

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

Kimprodp
Copy link
Contributor


🚀 어떤 기능을 구현했나요 ?

다음과 같은 API 문서를 작성했습니다.

  • 작성한 리뷰 목록 조회
  • 회원용 리뷰 그룹 생성
  • 리뷰 그룹 정보
  • 리뷰 등록

🔥 어떻게 해결했나요 ?

  • 리뷰 그룹 정보 api의 경우, 이전에 요구사항은 응답에 그룹의 주인이 회원/비회원인지와 요청에 전달된 회원이 리뷰 그룹의 주인인지를 같이 응답하도록 하는 것으로 합의했습니다.
  • 하지만 리뷰 그룹 정보 응답과 요청으로 전달된 회원의 리뷰 그룹 소유 권한을 확인하는 것은 하나의 기능이 여러 책임을 가지게 된다고 생각했습니다.
  • 따라서 리뷰 그룹 응답에 단순히 리뷰 그룹의 주인 ID(필드명 : revieweeId)를 nullable로 제공하는 것으로 했습니다.
    -> 회원이 생성한 리뷰 그룹인 경우, 회원ID가 함께 제공
    -> 비회원이 생성한 리뷰 그룹인 경우, 회원ID는 null로 제공

클라이언트는 회원 ID가 null인 경우 비회원의 그룹으로 판단할 수 있고, ID가 존재하는 경우 로그인 사용자의 ID와 비교하여 리뷰 그룹의 주인인지를 확인하면 됩니다.

  • 추후 필요한 것 : 프론트가 로그인 사용자의 정보를 따로 저장하지 않는다면, 로그인 세션을 통해 회원 정보(memberId)를 응답하는 API 구현

[백엔드 추가 전달 사항]

  • 기존에 있던 api와 사용 주체(회원/비회원)에 따라서 URI를 통일하여 구분해야 할 것 같아요. 이 부분은 다시 얘기해봅죠.
  • 세션을 어떻게 처리해야할지도 고민이 필요하겠습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

  • 이해 안가면 디코주세요.

Copy link

github-actions bot commented Dec 19, 2024

Test Results

161 tests   158 ✅  4s ⏱️
 58 suites    3 💤
 58 files      0 ❌

Results for commit 20010fc.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

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

따라서 리뷰 그룹 응답에 단순히 리뷰 그룹의 주인 ID(필드명 : revieweeId)를 nullable로 제공하는 것으로 했습니다.
-> 회원이 생성한 리뷰 그룹인 경우, 회원ID가 함께 제공
-> 비회원이 생성한 리뷰 그룹인 경우, 회원ID는 null로 제공

바뀐 방법이 더 좋아보입니다👍


추후 필요한 것 : 프론트가 로그인 사용자의 정보를 따로 저장하지 않는다면, 로그인 세션을 통해 회원 정보(memberId)를 응답하는 API 구현

memberId 를 프로필 사진 / 닉네임을 담는 dto 에 포함하게 할까요?

Comment on lines +1 to +7
==== 비회원이 리뷰 생성

operation::create-review[snippets="curl-request,request-fields,http-response"]
operation::create-review-by-guest[snippets="curl-request,request-fields,http-response"]

==== 회원이 리뷰 생성

operation::create-review-by-member[snippets="curl-request,request-fields,http-response"]
Copy link
Contributor

Choose a reason for hiding this comment

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

하나의 api 를 사용하되, 세션에 따라서 회원과 비회원을 구분한다는 선택지도 있었을 것 같아요.
그것을 선택하지 않고 이렇게 api 를 분리한 이유가 있나요?

Copy link
Contributor

Choose a reason for hiding this comment

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

회원/비회원 리뷰 생성 API를 하나로 했을때를 생각해보았는데요.

    @PostMapping("/v2/reviews")
    public ResponseEntity<Void> createReview(
            @Valid @RequestBody ReviewRegisterRequest request,
            @MemberSession Member member
    ) {
        long savedReviewId = reviewRegisterService.registerReview(request, member);
        return ResponseEntity.created(URI.create("/reviews/" + savedReviewId)).build();
    }

이때, 비회원일 경우 세션이 없지만, resolver에서 예외를 터뜨리거나 하지 않고 컨트롤러까지 다시 member 객체를 null로 반환하는 형식으로 처리를 해줘야합니다. 그리고 서비스에서 이를 확인해서 비회원용 로직으로 처리하게 되겠죠. 이렇게, 하나의 API에서 세션이 null인 것을 예외가 아닌 비회원임으로 인식하고 처리하기 위해서 예외를 터뜨려야하는데 다르게 처리하는 등으로 로직이 복잡해진다고 생각했어요.

Copy link
Contributor

@nayonsoso nayonsoso Dec 20, 2024

Choose a reason for hiding this comment

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

지난 회의에서 이야기 나온 것처럼, 하나의 api를 여러곳에서 쓰고, 권한에 맞게 별도의 로직을 제공하려는 우리의 취지를 생각해봤어요. 그럼 하나의 api에서 처리하는게 맞는 것 같아요.

그리고 저도 확실하지 않아서 지피티 & 클로드에게 물어보니, 회원/비회원 기능을 하나의 api에서 제공하고 내부적으로 분기하는 것은 일반적인 패턴이라고 합니다. 아래의 예시 코드처럼요.

@Service
public class ProductService {
    public ProductResponse getProduct(Long id, User user) {
        Product product = productRepository.findById(id);
        
        ProductResponse response = new ProductResponse(product);
        
        if (user != null) {
            // 회원용 추가 정보 설정
            response.setPersonalizedPrice(calculatePersonalPrice(product, user));
            response.setUserSpecificData(getUserData(product, user));
        } else {
            // 비회원용 기본 정보만 설정
            response.setDefaultPrice(product.getPrice());
        }
        
        return response;
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. 위의 코드로 구현할 수 있지만 앞에서 언급한 문제가 여전히 존재한다고 생각해요.

하나의 API에서 세션이 null인 것을 예외가 아닌 비회원임으로 인식하고 처리하기 위해서, resolver에서 예외를 터뜨려야하는데 다르게 처리하는 등으로 로직이 복잡해진다고 생각했어요.

  1. api 재사용 논의와 관련해서는, 구현하면서 아직 하나의 api를 재사용하고 세션 권한 체계로 구분한다는 것의 상세구현이 그려지지 않았어요. 그래서 일단 세션을 나눠서 적용했을 때 문제가 없게 두가지 api로 나눠 구현했습니다..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

회원/비회원의 동작 차이가 크지 않으면 동일한 비즈니스 로직을 하나로 사용하는 것이 좋다고 생각해요.

  • '리뷰 등록' 이라는 비즈니스 로직은 동일하고
  • 권한에 따라서 추가적인 차이는 리뷰 작성 후 회원의 '내가 작성한 리뷰'에 추가하는 것 뿐
  • 이는 컨트롤러나 컨트롤러와 서비스의 중간 계층에서 충분히 분기 할 수 있고(분기 위치, 이 부분을 정해야 할듯), 서비스는 비즈니스 로직에만 집중해서 권한 체계와 비즈니스 로직이 분리됨

서버는 단순히 로그인 세션이 존재하면 회원이 요청한 것으로 알고 처리, 없으면 비회원이 요청한 것으로 알고 처리. 이렇게 구현하면 리졸버에서 null을 반환하는 것이 자연스러워 보입니다.
오히려 같은 동작인데 api를 분리하면 권한 중심으로 설계된다고 생각해요.

Copy link
Contributor

@nayonsoso nayonsoso Dec 29, 2024

Choose a reason for hiding this comment

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

저도 테드와 같은 생각입니다.
덧붙이자면 우리는 "한가지 api에 대해서 권한에 따라 다르게 기능하기"로 합의했고,
지금은 api 설계 단계이니, 이 목표에 구현을 맞춰야 하지
구현이 되지 않았으므로 설계를 두개로 나눌 필요는 없다 생각해요!

Copy link
Contributor

Choose a reason for hiding this comment

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

중간 계층을 두거나 하는 방식으로 복잡도를 줄이고 '한가지 api에 대해서 권한에 따라 다르게 기능하기'를 최대한 활용해보는 것에 동의합니다! 반영 완료!

backend/src/test/java/reviewme/api/ReviewApiTest.java Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

@skylar1220 의견 남겼는데 확인 부탁합니다.

  • 의견 통일되면 반영하고 검토받고,
  • 갈린다면 디코 스레드에서 투표하는걸로 합시다요

@@ -39,6 +40,13 @@ public ResponseEntity<Void> createReview(@Valid @RequestBody ReviewRegisterReque
return ResponseEntity.created(URI.create("/reviews/" + savedReviewId)).build();
}

@PostMapping("/v2/reviews/member")
public ResponseEntity<Void> createReviewByMember(@Valid @RequestBody ReviewRegisterRequest request) {
// 회원 세션 추후 추가해야 함
Copy link
Contributor Author

Choose a reason for hiding this comment

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

다시 보니, 이 api는 /v2/reviews api 하나로 처리해도 상관없어 보여요.

  • 세션 유무에 따라서 내부 로직은 분기하면 됩니다.
  • 근데 요청 dto가 동일한데, 세션을 받아야 하는 이유가 있었나요? (생각이 안납니다..)

Copy link
Contributor

Choose a reason for hiding this comment

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

근데 요청 dto가 동일한데, 세션을 받아야 하는 이유가 있었나요?

회원이 리뷰를 작성할 땐, '내가 작성한 리뷰'에 추가해줘야 하니깐요!

Copy link
Contributor

Choose a reason for hiding this comment

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

#1017 (comment)
위의 코멘트에 분리하는 것이 낫다고 생각하는 이유를 적었는데 테드의 생각은 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코멘트 남겼어요 확인 부탁합니다~

@@ -75,4 +83,15 @@ public ResponseEntity<ReviewsGatheredBySectionResponse> getReceivedReviewsBySect
reviewGatheredLookupService.getReceivedReviewsBySectionId(reviewGroup, sectionId);
return ResponseEntity.ok(response);
}

@GetMapping("/v2/written")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 api의 url에서 "written"만 있는 것은 어떤 자원을 찾는지 명확하지 않아보여서 변경이 필요해 보이네요.

  1. 기존 보완 -> /reviews/written
  2. 신초의 의견을 반영한 authored 사용 -> /reviews/authored
  3. 특정 회원의 작성된 리뷰 목록 -> /users/{userId}/written-reviews (세션과 id 검증 필요)

Copy link
Contributor

Choose a reason for hiding this comment

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

1번 방법이 좋아보여요. 내가 받은 리뷰가 /v2/reviews인데 이것도 함께 변경하면 좋을 것 같아요.
받은 리뷰는 /v2/reviews/receive , 작성한 리뷰는 /v2/reviews/authored 이런 식으로요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1번 방법이 좋아보여요.
작성한 리뷰는 /v2/reviews/authored 이런 식으로요!

어떤걸 선택한거죠?? 😮😮

Copy link
Contributor

Choose a reason for hiding this comment

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

이 메서드, 내가 작성한 리뷰는 /v2/reviews/authored 로 하고요.
더불어 내가 받은 리뷰는 현재 '/v2/reviews'인데 얘도 함께 'v2/reviews/receive로 바꾸면 어떨까였어요!

왜냐면
내가 받은 리뷰: /v2/reviews
내가 작성한 리뷰: /v2/reviews/authored
이렇게 되면 내가 작성한 리뷰내가 받은 리뷰의 하위 api 처럼 느껴져서요~

}

@PostMapping("/v2/groups/member")
public ResponseEntity<ReviewGroupCreationResponse> createReviewGroup(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

요것도 마찬가지로 groups랑 합칠 수 있을 것 같습니다

  • 세션 유무에 따른 분기처리

Copy link
Contributor

Choose a reason for hiding this comment

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

/v2/groups의 요청 필드에는 groupAccessCode가 필요하고, /v2/groups/member의 요청 필드에는 groupAccessCode가 필요없는데 어떻게 합칠 수 있나요? null을 허용하는 방법일까요? 복잡하지 않은 분기처리 로직이 어떻게 될지 잘 안떠올라서요...

둘을 합칠때의 장점이 뭘까요? 어차피 요청 dto 형태도 다르고, 호출하는 서비스 메서드도 다른데 말이지요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요청은 하나로 합칠 수 있고, 리뷰 그룹이라는 동일한 엔티티를 생성하고 DB에 저장하는 로직은 동일해요.
다만 회원의 경우 리뷰 등록처럼 회원의 리뷰 그룹 목록에 생성된 리뷰 그룹을 추가하는 등의 추가적인 작업만 필요해 보입니다.

리뷰 등록의 코멘트처럼 분리 처리되는 곳의 위치나 방법만 정하면 가능하지 않을까요..?

비회원

  • 요청 : 리뷰이명, 리뷰그룹명, 비밀번호
  • 로직 : 리뷰 그룹 생성
  • 응답 : 생성된 리뷰 그룹 정보

회원

  • 요청 : 리뷰이명, 리뷰그룹명, 로그인 세션
  • 로직 : 리뷰 그룹 생성, 회원의 리뷰 그룹에 생성된 리뷰 그룹 추가
  • 응답 : 생성된 리뷰 그룹 정보

Copy link
Contributor

Choose a reason for hiding this comment

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

(( 종류에 따라 요청 형식이 다른데 / 로직은 동일하다는 점에서
textAnswerRequest 와 checkboxAnswerRequest 가 떠오르는군요... ))

종류에 따라 비지니스 로직이 크게 다르지 않으므로, 하나의 api 를 사용하는 쪽에 한표요!

Copy link
Contributor

Choose a reason for hiding this comment

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

위의 리뷰 작성 api를 통합한 것과 같은 이유로 서비스나 별도 레이어에서 분기해보는 방식에 동의합니다~
따라서 해당 방식으로 반영해보았는데 적용하다보니 request의 groupAccessCode가 nullable이 되는 것이 살짝 걸리네요.
이건 비즈니스 로직에서 검증을 구현해보면서 다시 확인해봐야겠습니다!

Copy link
Contributor Author

@Kimprodp Kimprodp left a comment

Choose a reason for hiding this comment

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

커비 코멘트 답완

Comment on lines +1 to +7
==== 비회원이 리뷰 생성

operation::create-review[snippets="curl-request,request-fields,http-response"]
operation::create-review-by-guest[snippets="curl-request,request-fields,http-response"]

==== 회원이 리뷰 생성

operation::create-review-by-member[snippets="curl-request,request-fields,http-response"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

회원/비회원의 동작 차이가 크지 않으면 동일한 비즈니스 로직을 하나로 사용하는 것이 좋다고 생각해요.

  • '리뷰 등록' 이라는 비즈니스 로직은 동일하고
  • 권한에 따라서 추가적인 차이는 리뷰 작성 후 회원의 '내가 작성한 리뷰'에 추가하는 것 뿐
  • 이는 컨트롤러나 컨트롤러와 서비스의 중간 계층에서 충분히 분기 할 수 있고(분기 위치, 이 부분을 정해야 할듯), 서비스는 비즈니스 로직에만 집중해서 권한 체계와 비즈니스 로직이 분리됨

서버는 단순히 로그인 세션이 존재하면 회원이 요청한 것으로 알고 처리, 없으면 비회원이 요청한 것으로 알고 처리. 이렇게 구현하면 리졸버에서 null을 반환하는 것이 자연스러워 보입니다.
오히려 같은 동작인데 api를 분리하면 권한 중심으로 설계된다고 생각해요.

@@ -39,6 +40,13 @@ public ResponseEntity<Void> createReview(@Valid @RequestBody ReviewRegisterReque
return ResponseEntity.created(URI.create("/reviews/" + savedReviewId)).build();
}

@PostMapping("/v2/reviews/member")
public ResponseEntity<Void> createReviewByMember(@Valid @RequestBody ReviewRegisterRequest request) {
// 회원 세션 추후 추가해야 함
Copy link
Contributor Author

Choose a reason for hiding this comment

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

코멘트 남겼어요 확인 부탁합니다~

@@ -75,4 +83,15 @@ public ResponseEntity<ReviewsGatheredBySectionResponse> getReceivedReviewsBySect
reviewGatheredLookupService.getReceivedReviewsBySectionId(reviewGroup, sectionId);
return ResponseEntity.ok(response);
}

@GetMapping("/v2/written")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

1번 방법이 좋아보여요.
작성한 리뷰는 /v2/reviews/authored 이런 식으로요!

어떤걸 선택한거죠?? 😮😮

}

@PostMapping("/v2/groups/member")
public ResponseEntity<ReviewGroupCreationResponse> createReviewGroup(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

요청은 하나로 합칠 수 있고, 리뷰 그룹이라는 동일한 엔티티를 생성하고 DB에 저장하는 로직은 동일해요.
다만 회원의 경우 리뷰 등록처럼 회원의 리뷰 그룹 목록에 생성된 리뷰 그룹을 추가하는 등의 추가적인 작업만 필요해 보입니다.

리뷰 등록의 코멘트처럼 분리 처리되는 곳의 위치나 방법만 정하면 가능하지 않을까요..?

비회원

  • 요청 : 리뷰이명, 리뷰그룹명, 비밀번호
  • 로직 : 리뷰 그룹 생성
  • 응답 : 생성된 리뷰 그룹 정보

회원

  • 요청 : 리뷰이명, 리뷰그룹명, 로그인 세션
  • 로직 : 리뷰 그룹 생성, 회원의 리뷰 그룹에 생성된 리뷰 그룹 추가
  • 응답 : 생성된 리뷰 그룹 정보

@skylar1220 skylar1220 force-pushed the be/docs/1015-written-review-api-docs branch from 548702b to de17b00 Compare January 5, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
4 participants