[step2] 장바구니 기능 PR 요청#47
Open
jisoo27 wants to merge 9 commits into
Open
Conversation
Gomding
requested changes
May 28, 2023
Comment on lines
+29
to
+35
| # 🚀 2단계 - 장바구니 기능 | ||
|
|
||
| ## 요구사항 | ||
| ### 기능 요구사항 | ||
| > - [x] 사용자 기능 구현 | ||
| > | ||
| > - [x] 사용자 설정 페이지 연동 |
There was a problem hiding this comment.
요구사항 정리 👍
조금 더 나아가자면 아래와 같이 자세하게 적어볼 수 있곘어요 😄
- 사용자 기능 구현
- 사용자 인증 기능을 구현한다.
- 클라이언트 요청값에 사용자 인증 정보는
Authorizationheader 의 값을 사용한다. - 사용자 인증은 이메일과 비밀번호 값이 일치하는지 검사한다.
- 사용자 인증에 실패하면 인증 실패 예외가 발생한다.
- 사용자 인증에 성공하면 해당 path 의 Controller 함수 인자에 사용자 정보를 매핑해준다.
- 클라이언트 요청값에 사용자 인증 정보는
- 사용자 인증 기능을 구현한다.
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class UserArgumentResolver implements HandlerMethodArgumentResolver { |
There was a problem hiding this comment.
HandlerMethodArgumentResolver 사용 👍
HandlerInterceptor 라는 선택지도 있었는데
HandlerMethodArgumentResolver 를 사용하신 이유가 궁금해요! 😄
Comment on lines
+39
to
+44
| private AuthInfo convert(String authorizationHeader) { | ||
| String literal = authorizationHeader.replace("Basic ", ""); | ||
| String decoded = new String(Base64.getDecoder().decode(literal.getBytes())); | ||
| String[] userInfo = decoded.split(":"); | ||
| return new AuthInfo(userInfo[0], userInfo[1]); | ||
| } |
There was a problem hiding this comment.
인증 방식은 추후에 바뀔 수 있는 부분이라고 생각해요 🤔
지금은 Basic Auth 를 사용하고 있지만 추후 Bearer Auth 로 변경될 수도 있다는 생각이 들어요 😄
- 인증 방식에 의한 변환(추출) 부분을 추상화하고
- ArgumentResolver 를 빈으로 만들어주셨으니 DI 를 사용해보면 어떨까요? 😄
|
|
||
| @Configuration | ||
| @RequiredArgsConstructor | ||
| public class WebMvcConfig implements WebMvcConfigurer { |
| } | ||
|
|
||
| @PostMapping | ||
| public ResponseEntity<CartResponse> post(@AuthenticationUser User user, @RequestBody @Valid CartAddRequest cartAddRequest) { |
There was a problem hiding this comment.
User 라는 도메인 객체를 controller 에서 의존하는 것에 대해 어떻게 생각하시나요? 😄
Comment on lines
+13
to
+14
| private User user; | ||
| private Product product; |
There was a problem hiding this comment.
Cart 가 User 와 Product 를 가지도록 설계해주셨군요!! 😄
장바구니 항목이 사용자와 상품에 대해 알아야겠다 라고 결정하신 이유가 궁금해요!
Comment on lines
+21
to
+23
| public boolean checkAuthority(User user) { | ||
| return this.getUser().getId().equals(user.getId()); | ||
| } |
Comment on lines
+39
to
+40
| if (!cart.checkAuthority(user)) { | ||
| throw new JwpCartApplicationException(ErrorCode.UNAUTHORIZED_CART); |
There was a problem hiding this comment.
- boolean을 반환하는 함수에 check 라는 이름을 사용하면 true 가 어떤 값을 의미하는 것인지 한 번 고민하는 과정이 필요해요!
그래서 check 는 보통 유효성 검사를 하는validation함수에 많이 사용한다고 생각해요 🤔
isOwner와 같은 이름은 어떠신가요? 😄cart.isOwner(user) - 논리 부정 연산자(
!) 를 사용하는 경우, 함수 반환값의 결과를 한번 뒤집는 과정을 거쳐야해서 코드의 가독성을 떨어뜨리는 요소라고 생각해요 😄
부정문을 사용한 함수 이름을 사용해보는건 어떨까요?
public isOwner(User user) {
return this.getUser().getId().equals(user.getId());
}
public isNotOwner(User user) {
return !this.isNotOwner(user);
}
if (cart.isNotOwner(user)) {
...
}| } | ||
|
|
||
| @Test | ||
| @DisplayName("장바구니 삭제 시 장바구니를 추가한 유저와 삭제하려는 유저가 다르려면 삭제에 실패한다.") |
There was a problem hiding this comment.
오타발견!! 🦅 🦅
Suggested change
| @DisplayName("장바구니 삭제 시 장바구니를 추가한 유저와 삭제하려는 유저가 다르려면 삭제에 실패한다.") | |
| @DisplayName("장바구니 삭제 시 장바구니를 추가한 유저와 삭제하려는 유저가 다르면 삭제에 실패한다.") |
Comment on lines
+15
to
+17
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT) | ||
| @DisplayName("장바구니 관련 기능 인수테스트") | ||
| class CartAcceptanceTest { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
안녕하세요 리뷰어님!
인증부분이 익숙하지 않아서 구현이 늦어졌네요 🥹
항상 제가 생각하지 못한 부분까지 살펴주셔서 감사합니다 🙇🏻♀️
그럼 이번 리뷰도 잘 부탁드리겠습니다 ~!!