[2단계 - 주문 기능 구현] 글렌(전석진) 미션 제출합니다. #34

seokjin8678 · 2023-06-02T12:48:18Z

안녕하세요 카프카! 이번 장바구니 협업 미션의 리뷰를 받게 된 글렌입니다!
협업 과정에서 API 명세, 추가 기능 협의가 있어 요청이 늦었네요 😭

이번 미션에 중점을 둔 점은 최대한 도메인에서 비즈니스 로직을 가지도록 설계했습니다!
조금 아쉬웠던 점은 기존 뼈대 코드의 설계를 따라가느라, CartItem이라는 클래스를 Cart 클래스를 만들어 관리하도록 했으면 더 도메인에 집중된 코드를 작성할 수 있었을 것 같습니다.
Cart 도메인을 만들어 리팩터링 하려고 해봤지만, 이미 API 명세가 만들어진 상태고, 프론트엔드에서 cartItemId를 의존하고 사용하고 있어서 그대로 놔둘 수 밖에 없었네요. 😂

문서화는 REST Docs를 사용했습니다! 링크

추가 기능은 "포인트"를 구현했습니다!
다만 포인트를 지급할 때, 따로 정책을 구현한게 아닌 상수로 고정을 시켰는데, 첫 리뷰 이후 정책을 추가하여 행사나 어떤 변경으로 코드의 변경 없이 수정할 수 있도록 리팩터링 해보겠습니다!

리뷰 기다리겠습니다!
감사합니다!

2단계 커밋

* test: null 값 수정 * feat: Response 클래스 추가 * feat: CartItemApiController 반환 타입 Response 변경 * feat: ProductApiController 반환 타입 Response 변경

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

- 사용하지 않는 필드 제거 - Transactional 추가 Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

* feat: 예외 클래스 추가 * style: 코드 서식 수정 * fix: 예외 로그 수정 * feat: 주문 검증 로직 추가 - 주문 상품이 없으면 예외 발생 * feat: 주문 조회 시 응답 필드 추가 - 대표 상품 이름, 주문 상품 갯수 * chore: dto 패키지 분리 * feat: Request Validation 추가 * test: Request 검증 테스트 추가

* style: 코드 서식 수정 * refactor: TODO 제거 * test: 상품 생성, 수정 테스트 코드 추가

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

include42 · 2023-06-02T14:25:42Z

안녕하세요 글렌, 만나서 반갑습니다. 저는 리뷰어 카프카입니다. 😄

이제 레벨 2 마지막 미션이네요! 협업이 들어가는 만큼 어려움이 컸을텐데, 잘 진행해주셔서 좋습니다 💯
레벨 2에서는(특히 이번 미션은) 코드 한줄한줄을 꼼꼼히 보기보다는, 보다 큰 그림 아래에서 어떤 방향이 좋을지 논의하는 리뷰가 되기를 바라고 있습니다.
또한 글렌이 협업을 진행하며 리뷰 요청을 주셨으므로, 실제 제가 회사에서 하는 것과 동일한 방식으로 리뷰를 진행하고자 합니다.
다만 리뷰 진행하면서 설명이 부족하거나, 더 궁금한 내용이 생긴다면 편하게 dm 내지 코멘트 부탁드리겠습니다 👍

리뷰는 내일 자정 이전에 가능할 것으로 생각됩니다.
빠르면 내일 정오 이전에도 가능하긴 하나, 다른 리뷰 일정과 개인 일정도 있어 명확치 않아 코멘트 남겼습니다.

글렌의 레벨 2 마무리를 응원합니다!
주말 푹 쉬시고, 즐거운 논의 이어나가기를 바라겠습니다.

include42

안녕하세요 글렌, 리뷰어 카프카입니다.

작성해주신 내용 잘 보았습니다. 문서화 부분도 좋았고, 전반적인 코드의 퀄리티도 좋아서 즐겁게 진행할 수 있었습니다. 💯
세부적인 내용에 대해서는 크게 리뷰할 사항이 보이지 않아 테스트 관련 부분만 수정을 요청드렸습니다.
다만 중간중간 글렌의 학습 현황이나 생각에 대해 질문을 드렸습니다. 이 부분에 코멘트 남겨주시면, 함께 의견을 나누고 더 성장할 수 있지 않을까 기대가 됩니다. 😄

작업하시느라 고생 많으셨습니다! 👍

include42 · 2023-06-03T14:31:18Z

src/docs/asciidoc/index.adoc


-== 회원
+include::order-api.adoc[]


개인적으로는 restdocs를 통해 문서화를 수행하고 해당 내용을 협업하는 크루들에게 공유해주신 부분이 인상적이었습니다.
혹시 이번 레벨2 과정에서 해당 내용을 교육을 통해 학습하셨나요? 혹은 개인적으로(팀 단위에서) 학습 후 적용한 것일까요?
구현해주신 퀄리티가 좋아서 코드 수정에 대해 코멘트드릴 부분은 없으나, 제가 현재 교육과정을 잘 몰라서 질문드린 부분입니다.

다른 크루들의 리뷰들을 슬쩍 보니 swagger를 선택한 팀도 있고 restdocs를 선택한 팀도 있는 것으로 생각이 됩니다.
이번 프로젝트에서 restdocs를 선택한 이유와, 양쪽 라이브러리의 장단점에 대해 어느정도 파악하고 계신지 간단히 코멘트 남겨주실 수 있으실까요?

레벨 3 진행 전에 양쪽 라이브러리에 대한 기초 지식과 장단점이 파악이 되어 있으시다면, 큰 도움이 되리라 생각되어서요.
만약 선택한 이유가 명확하지 않다면, 크루들과 이야기 나눠보면서 이 부분에 대해 글렌만의 이유를 정리해 보시면 도움이 되리라 생각됩니다.

이번 미션에서 페어와 restdocs와 swagger 중 어떤 것을 선택할지 고민했는데, restdocs를 사용하기로 했습니다!
restdocs를 선택한 이유는 테스트 코드를 기반으로 API 문서가 생성되기 때문에, 더 신뢰할 수 있는 API 문서를 만들 수 있고, 그에따라 테스트 코드 작성이 필수적이니 코드의 전체적인 완성도를 더 높여준다고 생각했기 때문입니다!
swagger를 사용하면 빠르게 API 문서를 작성할 수 있지만, 프로덕션 코드에 API 문서에 관련된 코드가 들어가게 되고 결과적으로 프로덕션 코드가 swagger에 의존적이게 되니 restdocs를 선택하였습니다!

restdocs는 예전에 사용해 본 적이 있지만, 그저 남이 작성한 코드를 따라서 적용해 보기만 하고, 직접 문서를 찾아서 해보지는 않았습니다. 😂
이전 지하철 노선도 미션에서 리뷰어님이 swagger와 restdocs 중 적용을 고려해보라는 피드백을 받아서, 문서를 보고 이번엔 제대로 적용을 해봤습니다!

좋습니다! 글렌이 선택한 기준이 궁금했는데, 양쪽의 장단점을 바탕으로 잘 판단해 주셨네요 😄
실제로 코드도 잘 작성해 주셨고, 직접 배포 후 공유까지 잘 되도록 해주셨으니 이번에 학습이 많이 되셨으리라 생각이 됩니다. 💯

다만 혹시, 현재 서버에 swagger도 적용해 볼 수 있을까요?
단순하게 적용하는 것은 정말 쉬운만큼, 별도의 어노테이션 없이 의존성만 추가한 뒤 페이지를 확인해보기를 권합니다.
여유가 난다면, 컨트롤러 하나를 정해서 추가로 어노테이션들을 이것저것 달아본 뒤 어떤 결과가 나오는지도 확인해보고요 😄

코드상으로 특별한 부분이 없을테니, 로컬에서만 확인해보고 리뷰하는 코드에 적용하지 않으셔도 괜찮습니다!

레벨3 미션 전에 양쪽 모두를 경험해보시면, 글렌이 레벨 3에서 똑같은 선택의 기로에 섰을 때 더 명확한 자신만의 이유를 가지고 선택할 수 있으리라고 생각되었습니다.
그리고 적어주신 내용 중 프로덕션 코드에 API 문서에 관련된 코드가 들어가게 되고 결과적으로 프로덕션 코드가 swagger에 의존적이게 되는 것에 대해 실제로 경험해보기를 권하고 싶기도 했고요 👍

저는 해당 어노테이션들이 프로덕션 코드를 의존적으로 만든다고 생각하지는 않습니다.

다만 좀...프로덕션 코드를 지저분하게 만드는 건 사실이지요. 다만 이 옵션은 선택 사항이고, 아무 옵션을 설정하지 않아도 기본 생성되는 swagger 퀄리티가 꽤 좋습니다.

그래서 저의 경우 일하는 회사 팀에서 일부 옵션만 적용해 사용하고 있거든요. 글렌에게도 이러한 선택이 가능하려면, 한번 직접 올려보고 옵션들을 적용해보는 시간이 필요하다고 생각해 코멘트 남겼습니다.

고려만 해보고, 적용은 해보지 않았는데 이번 기회에 적용을 해봐야겠네요!

include42 · 2023-06-03T14:37:21Z

src/main/java/cart/domain/Member.java

+    public boolean hasNotEnoughPoint(Point point) {
+        return this.point.getAmount() < point.getAmount();
+    }
+
+    public void increasePoint(Point point) {
+        this.point = this.point.plus(point);
+    }
+
+    public void decreasePoint(Point point) {
+        this.point = this.point.minus(point);
+    }
+
+    public boolean checkPassword(String password) {
+        return this.password.equals(password);
+    }


해당 내용들을 도메인 안에 정리해주신 부분은 매우 좋다고 생각합니다. 비즈니스 요구사항에 관련된 로직을 도메인 객체에 두는 것이 맞겠지요 💯

다만 염려가 되는 부분이, 지금 member 라는 클래스는 로그인 인증을 위해 argumentResolver에서 컨트롤러에 주입해주는 클래스이면서, 동시에 포인트 관련 처리를 담당하는 클래스이기도 해서 두 가지 책임을 진다는 생각이 들었어요.
별도의 Point 서비스와 컨트롤러도 만들어 주셨는데, 해당 부분의 구현도 맴버를 바탕으로 하고 있어서 다른 서비스/컨트롤러들과는 달리 코드 가독성 등에서 아쉬운 면이 있었고요.

제가 생각하기에는 Point 클래스가 memberId를 필드로 가지면, 컨트롤러에서 주입받은 member 정보를 바탕으로 Point 관련 연산을 서비스에서 수행할 수 있을듯 한데요. 지금 member_point 테이블의 구조를 보면 어느정도 그런 방식으로 구현을 고려하신 듯도 하고요. 혹시 최종 구현 과정에서 현재의 형태가 된 히스토리 내지 이유가 있을까요?

다만 이 부분은 제가 코드를 명확히 파악하지 못해 잘못 적은 부분도 있을 수 있습니다. 놓친 부분이 있다면 코멘트 추가로 부탁드립니다. 👍

컨트롤러에서 도메인 객체인 Member를 가져온다는 것이 좋지 않은 부분이라고 생각합니다!
기존 뼈대 코드에서 이렇게 구현이 되어 있어서 리팩터링을 해야했지만, 다른 기능 구현에 비해 우선 순위가 낮아 신경을 쓰지 못했네요 😂
컨트롤러에서 Member 도메인을 받아 오는 것이 아닌, 최소한의 계정 정보를 가진 객체를 가져와, 그것을 서비스에서 사용하는 것이 객체의 책임 분리가 더 명확해질 것 같네요!

저도 말씀해주신 부분에 동의합니다. 그렇게 된다면 member라는 도메인의 의미도 지금과는 달라질 수 있겠군요!
이 부분은 리팩토링 해주시면 확인해 보겠습니다. 혹시 다음 리뷰요청까지 적용이 어렵다면 코멘트 부탁드립니다 👍

include42 · 2023-06-03T14:40:01Z

src/main/java/cart/repository/OrderRepository.java

+import org.springframework.stereotype.Repository;
+
+@Repository
+public class OrderRepository {


OrderRepository는 주문 관련 연관관계 처리를 위해 만들어 준 컴포넌트일까요?
해당 부분 구현도 좋고, 어떤 아이디어로 만들게 된 것인지도 바로 파악할 수 있었습니다.
다만 글렌이 해당 클래스를 만들면서 진행한 고민 내지 이유를 적어주시면, 이에 대해 더 논의를 해볼 수도 있을듯하여 코멘트를 남깁니다.

Order 도메인에 비즈니스 로직을 구현하려면 OrderItem을 가진, 완성된 상태의 Order 도메인이 데이터베이스에서 불러와져야 합니다!
Order는List<OrderItem>을 필드로 가지는데, 이때 OrderDao에서 Order 객체를 가져오려면 우선 빈 상태의 List<OrderItem>을 설정하거나, OrderDao가 OrderItem 테이블을 다뤄야 합니다.
빈 상태의 List<OrderItem>을 가져오면, 이후에 setter로 주입시켜줘야 하므로 좋지 않은 방법이라 판단했고, OrderDao가 OrderItem 테이블을 다루는 것은 OrderDao가 하나 이상의 책임을 가지는 것이라 판단했습니다.
따라서 Order의 데이터베이스 테이블을 그대로 바라보는 OrderEntity를 만들고, Order가 setter 없이 완성된 상태로 불러와질 수 있게 했습니다!

적용해주신 이유가 분명하고, 책임을 적절하게 위임하는 방향을 선택해 주셨군요! 💯 💯
저 역시 같은 상황이라면 이렇게 구현했을 것이라고 코드를 보며 생각했어요.
다만 다른 선택지(단순히 하나의 DAO로 해결)를 선택하는 크루들이 많았기 때문에,
글렌이 어떠한 이유로 이런 선택을 하였는지 그 고민의 과정을 질문드리고 싶었습니다.

이러한 좋은 코드를 향해 고민하는 과정은 성장에 큰 도움이 된다고 생각합니다.
깔끔하게 잘 정리해 전달해주셔서 감사합니다. 👍

include42 · 2023-06-03T14:57:06Z

src/main/resources/application.yml

@@ -0,0 +1,3 @@
+spring:
+  profiles:
+    active: local


yml 파일을 이용하도록 프로퍼티 파일을 개선해주신 부분 아주 좋습니다.
다만 여기서 local profile을 적용하는 의미가 정확히 어떻게 될까요?
위의 application-local 파일의 작명 이유가 궁금합니다.

프로덕션 환경에서 실행하는 것이 아닌, 개인 PC에서 테스트를 하려고 local 프로파일을 만들어 기본적으로 실행될 때 local 환경으로 실행되도록 application.yml을 설정해두었습니다!
application.yml에 설정을 해줄 수 있지만, 명시적으로 local 이라는 환경을 지정하여 프로퍼티 파일을 분리하였습니다!

프로퍼티 파일을 분리해주신 부분이 좋다고 생각합니다 👍

다만 궁금한 점이, 그렇다면 현재 EC2에 올라가있는 서비스에는 local 프로파일이 적용되어 있을까요?
저의 경우 local, alpha/beta, real 등 다양한 프로파일을 나눠서 설정을 적용하는 편이라서요,
local 프로파일이 운영용 프로파일로도 적용이 되는 것인지, 혼동이 되어서 질문드린 점도 있었습니다.

아니요! EC2에 올라가있는 서비스에선 따로 prod 프로파일을 만들어 다른 프로파일로 작동하도록 설정하였습니다!

include42 · 2023-06-03T14:58:38Z

src/test/java/cart/application/OrderServiceTest.java

+@DisplayNameGeneration(ReplaceUnderscores.class)
+@SuppressWarnings("NonAsciiCharacters")
+@Transactional
+class OrderServiceTest {


현재 controller 테스트는 webmvctest + mockito를 사용하도록 잘 구현이 되어 있더군요.
다만 그에 비해 서비스 테스트는 @SpringBootTest 기반으로 구현이 된 것으로 보입니다.
혹시 여기에 mockito를 적용하지 않은 이유가 있을까요?

관련해서 이전에 리뷰나 교육 등에서 배우지 않았다면, 관련된 내용을 검색해본 뒤 적용해보는 것도 좋은 기회가 될 것이라 생각됩니다.
다만 mock을 통한 테스트에 호불호가 있기도 해서, 관련된 의견을 편하게 코멘트로 적어주시면 참고해서 진행 방향을 잡아볼 수 있겠네요 😄

도메인 객체에 비즈니스 로직을 최대한 구현하도록 설계하다 보니, 서비스 계층에서 하는 일이 비교적 단순해졌습니다!
서비스 계층은 단순히 리파지터리 계층에서 도메인 객체를 불러온 뒤, 도메인 객체에 메시지를 호출하는 역할밖에 하지 않는다 생각합니다!
다만 그렇다해서 Service 계층을 테스트할 필요가 없다고 생각하지는 않습니다!

저는 Mock을 사용해서 테스트 하는 것을 선호하는 편입니다!
하지만 미션의 마감기한과 우선순위가 높은 다른 요구사항들 때문에 빠르게 테스트 코드를 작성할 필요가 있다고 판단하여, Mock을 사용하지 않았습니다. 😂
Service 테스트를 Mock을 사용하도록 리팩터링 해보겠습니다!

적어주신 내용에 공감하지만, 그럼에도 mocking 후 verify를 꼼꼼하게 해준다면 해당 서비스 계층에서 어떤 클래스들을 의존하고 호출하는지 명확히 확인할 수 있다고 생각합니다.

만약 TDD로 컨트롤러->서비스->레포지토리+도메인 코드를 작성하게 되었다면 mock을 통해 서비스 테스트를 작성했을 수도 있겠네요. 다만 빠르게 작업해야 하는 상황에서 이미 프로덕션 코드가 존재하니, 급하게 테스트만 추가한 상황이 충분히 이해가 갑니다.

이 부분은 리팩토링된 내용을 참조해보고 더 보강할 부분이 있다면 다시 코멘트 드리겠습니다 👍

include42 · 2023-06-03T15:05:03Z

src/test/java/cart/ui/MemberControllerTest.java

+    MemberDao memberDao;
+
+    @Test
+    @DisplayName("회원의 포인트를 조회한다.")


다른 테스트에서는 @DisplayName 어노테이션을 제거한 듯 한데, 여기에는 아직 남아있네요 😄
이번에 테스트 이름 부분을 일괄적으로 수정한 이유가 있을까요? (수정 요청이나 지적이 아닌, 순수한 궁금증으로 남긴 질문입니다 👍 )

이것도 뼈대 코드에 이미 구현된 테스트 코드라 다른 기능 구현에 우선 순위가 밀려 리팩터링하지 못했네요 😂
피드백을 제출하면서 일관성 있도록 리팩터링 해보겠습니다!

include42

안녕하세요 글렌, 리뷰어 카프카입니다.

작성해주신 내용 잘 보았습니다. 문서화 부분도 좋았고, 전반적인 코드의 퀄리티도 좋아서 즐겁게 진행할 수 있었습니다. 💯
세부적인 내용에 대해서는 크게 리뷰할 사항이 보이지 않아 테스트 관련 부분만 수정을 요청드렸습니다.
다만 중간중간 글렌의 학습 현황이나 생각에 대해 질문을 드렸습니다. 이 부분에 코멘트 남겨주시면, 함께 의견을 나누고 더 성장할 수 있지 않을까 기대가 됩니다. 😄

작업하시느라 고생 많으셨습니다! 👍

seokjin8678 · 2023-06-04T16:40:27Z

안녕하세요 카프카! 피드백 반영하고 다시 리뷰 요청 합니다!

이번 리뷰 요청에서 반영한 점은 다음과 같습니다!

OrderService를 Mock을 사용하도록 테스트를 작성하면서 기존에 있던 PointService를 삭제했습니다!
OrderService로 주문을 생성하는데, 주문을 생성하면 사용자가 포인트를 사용할 수 있는지 조회하고, 포인트를 차감한 뒤, 다시 증가시켜야 합니다.
하지만 OrderService에서 포인트에 대한 작업들을 PointSerivce에 위치시키니, 테스트 코드를 작성하기가 난감해지는 상황이 발생하더군요 😂
PointService를 Mock으로 만들지 않으니, OrderService에 대한 단위 테스트가 아니게 되고
PointService를 Mock으로 만드니, 주문을 할 때 회원의 포인트가 변경되는 중요한 비즈니스 로직을 테스트 할 수 없었습니다.
따라서 OrderService에서 회원의 포인트를 직접 조작할 수 밖에 없었네요.
인증에 대한 로직을 리팩터링 하였습니다!
MemberArgumentResolver가 MemberDao를 의존하고 도메인 객체인 Member를 반환하는 것을 Attribute에서 User라는 인증 정보를 가진 객체를 반환하게 하였습니다.
인증에 대한 책임은 Interceptor가 AuthProvider를 사용하도록 하여, 보다 책임의 분리를 지키도록 하였습니다!
AuthProvider는 인터페이스로 만들어, JWT로 인증 방식이 바뀌더라도 변경을 최소화 하도록 했습니다!
포인트 정책을 만들어 새로운 정책이 생기더라도, 변경에 쉽도록 하였습니다!
단순히 금액의 일정 비율로 포인트를 지급하는 FixedPercentPointPolicy, 금액에 상관 없이 고정된 포인트를 지급하는 FixedAmountPointPolicy, 여러 포인트 정책을 조합할 수 있는 AdditionalPointPolicy를 구현하였습니다!
포인트 정책을 사용하는 것은 Order 도메인 객체가 파라미터로 넘어온 PointPolicy를 사용하도록 했습니다!

그리고 간단하게 Swagger를 적용해봤습니다!
RestDocs로 문서화를 하느라 고생했는데, Swagger를 적용하니 10분도 안되서 문서화를 작성할 수 있는 것을 보고 현타가 오네요 😂

리뷰 기다리겠습니다!
감사합니다!

… 받도록 변경

include42 · 2023-06-05T01:27:49Z

안녕하세요 글렌, 리뷰어 카프카입니다.

이번에 코드를 잘 작성해 주시고 레벨 2동안 학습한 내용을 잘 반영해 주셔서 크게 수정할 부분이 없었네요! 💯
그럼에도 고민이 되던 부분들을 적절히 리팩토링 해주시고, Swagger 적용까지 잘 수행해 주셔서 기쁜 마음입니다.
변경된 부분에 대해서도 함께 논의해보고, 보다 깊게 고민할 부분이 있다면 함께 고민하며 성장하기를 바라겠습니다.

리뷰는 금일 저녁 10시 ~ 12시 사이에 진행 예정입니다.
레벨 2 마지막까지 응원하겠습니다! 미션 진행하시느라 고생 많으셨습니다 👍

include42

안녕하세요 글렌, 리뷰어 카프카입니다.

이번 미션 진행하시느라 고생 많으셨습니다! 리뷰 진행하면서 요구드린 사항들이 많았는데, 고민한 내용을 공유해주시면서 적절히 리팩토링해주셔서 크게 걸리는 부분 없이 리뷰를 진행할 수 있었습니다 💯
이제 레벨 2의 마지막 미션도 마무리가 되겠네요! 그래도 해당 과정에서 소화해야 할 내용들을 충실히 챙겨가시는 것이 보여 기분좋게 마무리를 할 수 있었습니다.
혹시나 궁금하신 사항이 있다면 코멘트 남겨주시거나 편하게 DM 부탁드립니다 👍

글렌의 남은 우테코 과정을 진심으로 응원합니다!
미션 approve 하겠습니다 😄

include42 · 2023-06-05T12:58:50Z

src/main/java/cart/application/OrderService.java

+    private void decreaseMemberPoint(Member member, Point spendPoint) {
+        validatePoint(member, spendPoint);
+        member.decreasePoint(spendPoint);
+    }
+


이번 리뷰 반영 사항 중 OrderService를 Mock을 사용하도록 테스트를 작성하면서 기존에 있던 PointService를 삭제 부분 잘 확인하였습니다! 그래도 큰 코드 변화 없이 잘 수행해 주셨네요! 💯

다만 이 리팩토링의 방향에 대해서는 저와 글렌의 생각이 다른 것으로 보입니다.
만약 저라면 기존의 PointService를 유지하는 방향도 고려했을 듯 해서요.

물론 OrderService와 PointService의 의존성이 강하기 때문에 테스트에서 mocking할 경우 테스트가 의미가 있나? 라고 생각하실 수 있습니다. 이 생각에는 공감하고 있습니다.

다만 제가 생각했을 때에는, 아래와 같이 테스트가 수행되면 문제가 없을 것으로 생각되었어요.

OrderServiceTest: 주문 관련 로직이 정상 동작하는지 확인, 이 과정에서 mock으로 처리된 PointService의 메소드들이 적절히 호출되는지 verify를 함께 수행해 줌

PointServiceTest: 포인트 관련 로직이 정상 동작하는지 확인

위처럼 테스트가 작성되었다면, 각각의 서비스의 책임은 온전히 검증되었다고 생각합니다.

물론 실제로 운영될 때 예기치 못한(검증하지 못한) 문제가 생길 수 있습니다. 다만 이런 부분들에 대해서는 시나리오를 작성해서 별도의 통합 테스트를 추가하면 해결되지 않을까라는 생각이 들었습니다.

다만 글렌이 적어주신 것처럼 PointService와 OrderService가 나뉠 때 중요한 비즈니스 로직의 책임이 분산된다고 느낄 여지도 분명히 존재하긴 하니까요,
현재처럼 포인트 관련 로직이 많지 않은 상황에서는 함께 수행하도록 코드를 짜고, 이후에 포인트 도메인이 두터워질때 책임을 분리하는 것도 가능하겠다는 생각이 들었습니다.

그러한 이유로 이 부분에 대해서 수정 요청을 드리지 않지만, 글렌도 제 의견을 바탕으로 고민해 볼 수 있다면 좋겠습니다 😄

include42 · 2023-06-05T13:01:31Z

build.gradle

@@ -29,6 +29,9 @@ dependencies {
    runtimeOnly 'mysql:mysql-connector-java:8.0.28'

    asciidoctorExt 'org.springframework.restdocs:spring-restdocs-asciidoctor'
+
+    implementation 'io.springfox:springfox-boot-starter:3.0.0'
+    implementation 'io.springfox:springfox-swagger-ui:3.0.0'


swagger 3을 잘 적용해 주셨네요! 아직 swagger 관련 자료가 2 버전 기반의 레거시가 많아서, 이부분을 코멘트해야 할지 고민했었습니다. (저도 팀에서 2버전을 쓰다가 3버전으로 넘어갔는데, 개선이 많이 되었더군요 😄 )

그리고 좋았던 부분이, 다양한 어노테이션을 잘 활용해서 문서를 만들어주신게 눈에 띄더라고요 💯
이번에 학습한 내용을 잘 정리해두시고, 이후 미션에서 swagger를 활용하게 된다면 어느 어노테이션을 적용할지 글렌만의 룰을 만들어보는 것을 권합니다.

물론 모든 어노테이션을 다 적용해서 꼼꼼한 문서를 만들 수도 있지만, Controller 내지 DTO 코드의 가독성도 고려하면, 적절한 타협선을 찾아야 한다는 생각이 들었어요 👍

그리고 현타가 오셨다는 말에 아주 공감갔습니다 😂
RestDocs와 비교했을때 강력한 편의성이 Swagger의 무기라는 생각이 들어요.

include42 · 2023-06-05T13:05:21Z

src/main/java/cart/config/point/PointPolicyConfig.java

+@Configuration
+public class PointPolicyConfig {
+    private static final double PERCENT = 10;
+
+    @Bean
+    public PointPolicy pointPolicy() {
+        return new FixedPercentPointPolicy(PERCENT);
+    }
+}


이 부분을 Configuration으로 구현해 주신 부분 매우 좋습니다 💯
레벨 2에서 학습이 필요한 스프링 개념들을 글렌이 잘 소화하고 있어서 즐겁게 리뷰 진행할 수 있었네요 👍

include42 · 2023-06-05T13:08:24Z

src/main/java/cart/dto/User.java

+public class User {
+    @ApiModelProperty(hidden = true)
+    private final Long memberId;
+    @ApiModelProperty(hidden = true)
+    private final String email;
+
+    public User(Long memberId, String email) {
+        this.memberId = memberId;
+        this.email = email;
+    }
+
+    public Long getMemberId() {
+        return memberId;
+    }
+
+    public String getEmail() {
+        return email;
+    }
+}


이전 리뷰에서 Member 클래스가 가지는 문제에 대해 길게 논의를 했었는데, 적절한 방법으로 잘 풀어내 주셨습니다 👍
저도 이전에 우테코 레벨3 프로젝트를 하면서 동일한 방법으로 해당 문제를 풀었던 기억이 나네요 😄

Kim0914 and others added 30 commits May 24, 2023 15:27

feat: cors 설정

8b6fe24

build: 상품 더미 데이터 추가

0fa3f44

build: utf 8 설정

6059060

build: Spring REST Docs 모듈 추가 (woowacourse#1)

b06893a

feat: 관리자용 장바구니 관리 페이지 추가 (woowacourse#2)

555046c

test: REST Docs Helper 클래스 추가

d4d490f

build: asciidoctor 누락 설정 추가

1d10c19

docs: 장바구니 담기 API docs 작성

3845457

Merge remote-tracking branch 'origin/step1' into step1

33077b1

docs: doctype 추가

c54aaf2

feat: 상품 API 문서 추가 (woowacourse#3)

08a46ff

chore: 벨리곰 이미지 URL 수정 (woowacourse#5)

f618dd8

응답에 message 추가, 결과 값 result로 포장 (woowacourse#6)

612b718

* test: null 값 수정 * feat: Response 클래스 추가 * feat: CartItemApiController 반환 타입 Response 변경 * feat: ProductApiController 반환 타입 Response 변경

fix: Response 응답 추가에 따른 테스트 코드 수정

9e4fc27

refactor: 불필요한 생성자 제거 및 빌더 패턴 적용

e7db00f

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

docs: 기능 목록 작성

54e524a

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 주문과 관련된 기본 도메인 추가

6bebeef

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 금액 검증 기능 추가

3cb8a47

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 포인트 객체 생성

89e8385

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 수량 검증 기능 추가

ce03354

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

refactor: 주문 상품 가격 필드 제거

5d59d8f

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

test: 주문 상품 기능 테스트 추가

eeadb50

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

refactor: 가격 필드 타입 변경

5db3c03

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

test: 상품 생성 테스트 추가

5a98a70

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 회원 도메인에 포인트 필드 추가

d4bab88

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: 주문 도메인 비즈니스 로직 추가

2321c8f

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

refactor: 주문 상품 도메인에 가격 필드 추가

f723a1f

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

refactor: 생성자 파라미터 원시값 받도록 변경

1201a9f

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: OrderDao 추가

d350559

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

feat: OrderItemDao 추가

081309d

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

Kim0914 and others added 10 commits May 31, 2023 21:05

test: OrderService 테스트 추가

759ab16

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

test: 테스트 코드 수정

b8d278c

- 사용하지 않는 필드 제거 - Transactional 추가 Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

build: MySQL 의존성 추가

e29810a

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

build: local, production 환경 분리

a07c7c2

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

fix: RowMapper, SQL문 수정 (woowacourse#7)

bcdef1a

코드 서식 수정, 상품 생성, 수정 테스트 코드 추가 (woowacourse#9)

a6c4b0f

* style: 코드 서식 수정 * refactor: TODO 제거 * test: 상품 생성, 수정 테스트 코드 추가

docs: 기능 목록 업데이트

7b5b001

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>

refactor: 사용하지 않는 클래스 제거

bbbb60f

refactor: 포인트 배율 상수로 추출

84c7ca0

include42 requested changes Jun 3, 2023

View reviewed changes

seokjin8678 added 11 commits June 4, 2023 17:40

refactor: PointService 제거, OrderService로 책임 분리

1dd027b

test: 테스트 코드 DisplayName 수정

fe5befc

refactor: 인증 처리 로직 리팩터링

3b177b1

refactor: Dao 메소드 명 수정

ced299c

refactor: 패키지 구조 세분화

e7eee44

feat: PointPolicy 추가

2f39599

refactor: 구현체 클래스 infrastructure 패키지로 이동

567e614

refactor: 테스트용 AdminController 삭제

a45543e

build: Swagger 의존성 추가

8708b6d

feat: Swagger 적용

007f71e

refactor: PathVariable 명확하게 변경

4e8dcfe

seokjin8678 added 2 commits June 5, 2023 01:50

test: FixedAmountPointPolicy 테스트 코드 추가

e78f4f0

refactor: AuthInterceptor 스프링 빈 등록 제외, WebMvcConfig에서 AuthProvider 주입…

655a5b2

… 받도록 변경

include42 approved these changes Jun 5, 2023

View reviewed changes

include42 merged commit 4260cde into woowacourse:seokjin8678 Jun 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 주문 기능 구현] 글렌(전석진) 미션 제출합니다. #34

[2단계 - 주문 기능 구현] 글렌(전석진) 미션 제출합니다. #34

seokjin8678 commented Jun 2, 2023 •

edited

include42 commented Jun 2, 2023

include42 left a comment

include42 Jun 3, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023

seokjin8678 Jun 4, 2023

include42 Jun 4, 2023

include42 Jun 4, 2023 •

edited

seokjin8678 Jun 4, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023

include42 Jun 4, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023 •

edited

include42 Jun 4, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023

include42 Jun 4, 2023

seokjin8678 Jun 4, 2023

include42 Jun 3, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023

include42 Jun 4, 2023

include42 Jun 3, 2023

seokjin8678 Jun 4, 2023

include42 left a comment

seokjin8678 commented Jun 4, 2023 •

edited

include42 commented Jun 5, 2023

include42 left a comment

include42 Jun 5, 2023

include42 Jun 5, 2023

include42 Jun 5, 2023

include42 Jun 5, 2023

include42 Jun 5, 2023


		== 회원
		include::order-api.adoc[]

[2단계 - 주문 기능 구현] 글렌(전석진) 미션 제출합니다. #34

[2단계 - 주문 기능 구현] 글렌(전석진) 미션 제출합니다. #34

Conversation

seokjin8678 commented Jun 2, 2023 • edited

include42 commented Jun 2, 2023

include42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

include42 Jun 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokjin8678 Jun 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

include42 left a comment

Choose a reason for hiding this comment

seokjin8678 commented Jun 4, 2023 • edited

include42 commented Jun 5, 2023

include42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokjin8678 commented Jun 2, 2023 •

edited

include42 Jun 4, 2023 •

edited

seokjin8678 Jun 4, 2023 •

edited

seokjin8678 commented Jun 4, 2023 •

edited