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

[2단계 - 주문 기능 구현] 제이(이재윤) 미션 제출합니다. #9

Merged
merged 135 commits into from Jun 7, 2023

Conversation

sosow0212
Copy link

@sosow0212 sosow0212 commented May 30, 2023

안녕하세요 웨지!

먼저 이번 2단계 미션 진행 기간이랑 예비군 기간이 겹쳐서 상대적으로 느릴 수 있는 점 양해 부탁드리겠습니다!
그래도 최선을 다하겠습니다 :)

이번 단계 변경사항은 다음과 같습니다. 커밋

++ 우테코 EC2는 캠퍼스 안에서만 들어갈 수 있어서 당장 외부 서버에 있는 MySQL로 연동하지는 못했습니다..!


할인 정책

일단, 프론트엔드와 재화 관련해서 기획은 두 가지를 했습니다.
쿠폰, 할인 정책을 도입했고, 쿠폰의 경우 값을 빼주는 쿠폰과, %할인 쿠폰 두 가지를 적용했습니다.

이 과정에서 "쿠폰과 할인 정책을 어떻게 관리해야할까?"라는 고민이 있었습니다.

처음에 ENUM으로 만들어서 관리를 하려고 했으나, ENUM은 정해진 쿠폰만 사용해야한다는 단점이 있었습니다.

쿠폰의 다양성을 위해서 ENUM으로 관리하지 않고, API로 추가 및 삭제를 하고 데이터베이스에서 조회를 하고, 쿠폰의 속성에 따라 Policy인터페이스를 통해서 값을 계산해주는 식으로 진행했습니다.


Entity와 Domain의 불일치

이 고민은 이전 미션을 하면서도 계속 있었지만, 이번 미션에서는 더 심했던 것 같습니다.

아마도 Level1때 처럼 콘솔 프로그램이었다면, Member 도메인 객체에 Cart 를 가지게 했을 것 같습니다. (DB가 없는 콘솔 프로그램에서 Cart 가 어떤 Member의 것인지 식별하기 위해서)

그래서 처음에 설계할 때에도 위와 같이 설계했지만, 생각을 해보니 DB 가 있는데 굳이 Member 밑에 Cart 를 둬야하는지 의문이 들었습니다.

따라서 두 도메인을 독립적으로 분리하고, Service Layer 에서 MemberCart 를 호출한 후 메시지 전달을 통해 각 객체의 역할을 할 수 있도록 변경했습니다.

정리하자면, Level2 에서 DB가 들어오면서 DB에서 연관관계를 정의해줄 수 있기 때문에 객체간 서로의 불필요한 연관관계가 없어도 된다고 생각해서 위와 같은 구조로 바꿨습니다.

하지만, 어떤 부분에서는 Repository 조회를 통해 검증을 하게 되고, 어떤 부분에서는 도메인에서 검증을 하게 돼서 이런 불일치가 있었습니다.

Order라는 도메인을 만들고, 여기서 MemberCart를 관리하고, 도메인 단에서 예외를 처리해줄 수 있지만, 이렇게 됐을 땐 꼭 도메인을 다 만들어줘야해서 불필요한 조회까지 하게 됩니다.

이런 점에서 계속 혼동이 왔고, 정리가 잘 안되는 것 같습니다.


Rest Docs

이번 미션이 협업인만큼, 프론트엔드 팀원에게 최대한 편의를 주려고 노력했습니다.

그래서 문서화를 도입해봤습니다.

예전에 공부했을 땐 Swagger를 썼었지만 이를 쓰기 위해 달아야하는 어노테이션이 너무 많아서 지저분했던 기억이 있었습니다.
그리고 문서가 정확하지 않을 수도 있다는 점도 있었기 때문에 이번에는 Rest Docs를 적용해봤습니다.

테스트를 작성해야 문서로 작성할 수 있다는 점에서 문서에 대한 신뢰도가 높았고, 어노테이션에 종속적이지 않아서 좋았습니다.

아직까지는 쓰기 조금 어렵고 시간이 드는 단점은 있었지만, 그 외 단점은 찾지는 못해서 앞으로 문서화가 필요하다면 Rest Docs를 사용할 것 같습니다.


To do

더 완벽하게 만들고 제출하고 싶었지만, 팀원들과 기획이 지속적으로 바뀌어서 다음과 같은 기능 구현은 잠시 멈췄습니다.

  • 예외 응답코드 테스트
  • Member 생성시 자동으로 Member_cart 생성하기
  • Coupon생성 API와 Member에게 지급하는 기능
  • Sale할 상품 선택 및 취소하기

위에 기능들은 일단은 sql 파일에 Mock 데이터로 넣었습니다.
아마 이번주 중으로 확실히 결정될 것 같은데, 그때 추가적으로 만들겠습니다!

감사합니다 :)
부족한 부분은 따끔한 말씀 부탁드리겠습니다!

seokjin8678 added a commit to seokjin8678/jwp-shopping-order that referenced this pull request Jun 2, 2023
* style: 코드 서식 수정

* refactor: TODO 제거

* test: 상품 생성, 수정 테스트 코드 추가
Copy link

@sihyung92 sihyung92 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 76 to 80
private void validateOwner(final Cart cart, final CartItem cartItem) {
if (!cartRepository.hasCartItem(cart, cartItem)) {
throw new MemberNotOwnerException();
}
}

Choose a reason for hiding this comment

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

cartItem이 실존하는지 검사하는데 메서드명과 예외가 잘못되었네요~~ (혹은 레포지토리 코드 호출이 잘못되었거나)

Copy link
Author

Choose a reason for hiding this comment

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

이 부분은 유저가 카트 하나를 가지고,
그 카트에 속한 item id인지 확인해서, 아니라면 다른 유저 상품을 건든다고 알려주는 예외였습니다!

제가 만들다보니 별 생각 없었는데, 의식하면서 보니깐 진짜 이상하네요 ㅎㅎ 감사합니다!

private void validateOwner(final Cart cart, final CartItem cartItem) {
if (!cartRepository.hasCartItem(cart, cartItem)) {
throw new MemberNotOwnerException();
}
}

@Transactional
public void remove(final Member member, final Long cartItemId) {
CartItem cartItem = cartRepository.findCartItemById(cartItemId);

Choose a reason for hiding this comment

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

cartItemId로 Cart를 찾아오는 로직을 만든다면 해당 클래스에서 로직전개가 조금더 깔끔해질거 같아요 ㅎ

Suggested change
CartItem cartItem = cartRepository.findCartItemById(cartItemId);
Cart cart = cartRepository.findCartItemById(cartItemId);
// 멤버 비교 등등

Copy link
Author

Choose a reason for hiding this comment

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

CartItemId가 있는 cartId라서 그렇군요!

레포지토리를 살짝 수정해서 이렇게 한다면 Member와 Cart의 주인이 같은지 확인할 때에도 더욱 명확하게 떨어지겠네요.

@NotNull(message = "정률이라면 %를 입력해주시고, 아니라면 할인할 가격을 입력해주세요.")
private Integer amount;

public CouponCreateRequest() {

Choose a reason for hiding this comment

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

요청객체에 기본 생성자가 열려있더라구요~! jackson 특정 버전 이후 기본생성자를 필요로 하지 않는데, 의도하신건지 궁금합니당

Copy link
Author

Choose a reason for hiding this comment

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

특정 버전부터는 기본 생성자를 필요로 하지 않나요?! 이건 처음 알아서 찾아봐야겠네요 :)

하지만, 이번 미션에서는 기본 생성자를 제거하면 역직렬화가 수행되지 않습니다 ㅠㅠ

혹시 제가 실수한 것이 있을까요..?
말씀해주신 내용과는 다르지만, 일단 기본 생성자의 접근 제어자라도 private로 변경하겠습니다..!

Choose a reason for hiding this comment

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

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.13#no-constructor-deserializer-module

2.13 버전 릴리즈에 포함된 생성자 없이 역직렬화가 가능한 모듈이 빈생성자여도 가능케 해주는데용

아마 테스트 코드에서 빌드도구를 intellij로 하시지 않았나 생각이드네요 ㅎ gradle로 바꾸면 성공할거에요

Copy link
Author

Choose a reason for hiding this comment

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

자료 감사합니다 :)

서버 배포시에는 빌드를 gradle로 해서 괜찮겠지만,
intelliJ 빌드로 한다면 기본 생성자가 있어야할 것 같습니다.

아마도 협업을 할 땐 로컬 환경에서 IntelliJ로 빌드를 하시는 분들도 있을 것 같은데, 이런 부분에서 문제는 없을까요?
보수적으로 기본 생성자를 넣는 건 어떻게 생각하시나요?

Choose a reason for hiding this comment

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

기본적으로 서버 내에서 구동 환경이 gradle 이므로 gradle 기준의 테스트가 성공해야 한다고 생각하긴 하지만, 이 케이스는 사소한 편이라 인텔리제이 빌드가 생산성을 올려준다고 생각한다면 팀 내 컨벤션으로 기본 생성자를 넣는것도 괜찮다고 생각합니다 ㅎ

Copy link
Author

Choose a reason for hiding this comment

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

웨지 덕분에 하나 더 알고가서 기분 좋습니다 :)
앞으로 참고 하겠습니다!


@GetMapping("/coupons")
public ResponseEntity<PaymentUsingCouponsResponse> applyCoupon(@OrderAuth final MemberCoupons memberCoupons,
@RequestParam("couponsId") @Valid final List<Long> ids) {

Choose a reason for hiding this comment

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

multi 파라미터라서 couponsIds 여도 괜찮겠어용

public class OrderHistory {

private final Long id;
private final String orderTime;

Choose a reason for hiding this comment

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

시간단위를 문자열로 다루는 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

DB에서 데이터를 가져올 때 String으로 받으면, 혹시 프론트에서 원하는 형태로 데이터를 만들어달라고 할 때 유연하게 만들 수 있기 때문에 String으로 받았습니다!

Choose a reason for hiding this comment

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

@jsonformat을 학습해 보시고 적용해보셔요 ㅎㅎ 말씀하신 프론트엔드 요구사항을 위한 직렬화 도구는 대부분 마련되어 있는 편입니다

Copy link
Author

Choose a reason for hiding this comment

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

이런 것도 있었군요..!

개발자가 직접 안해줘도 돼서 유용하네요.
꿀팁 감사합니다 ㅎㅎ 바로 적용해봐야겠습니다.

@@ -0,0 +1,26 @@
package cart.entity.product;

public class ProductSaleEntity {

Choose a reason for hiding this comment

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

ProductEntity에 salePrice가 생겼으니 이제 policy와의 연관관계가 필요없을 거 같아요 ㅎㅎ
아예 끊어주고 product의 salePrice만 이용하도록 리팩토링 해보는건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

앗 이부분은 일부러 냅뒀습니다 !

저번에 Product와 Policy의 연관관계를 끊을 때,
테이블에서 Product에 policy_id가 아닌 salePrice 필드를 두었습니다.

salePrice는 null 값이 될 수도 있지만, 값이 들어올 수도 있습니다.

저는 세일을 할 때, policy에 상품에 대한 세일 정률 혹은 정액에 대한 값이 들어오게 만들었습니다. (혹시나 같은 정책이 있다면, 중복되는 정책을 새로 만들지는 않습니다.)

그리고 나서 이 policy를 이용해서 product_sale 테이블에 따로 할인하는 여부를 주었습니다.

이렇게 준 이유는 쇼핑몰의 기능들을 생각했을 때, 세일하는 상품들만 보는 기능이 추가될 수도 있다고 생각했기 때문입니다. :)

Choose a reason for hiding this comment

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

아하 그렇군요~~ 일종의 세일 정보에 대한 테이블로 보면 되겠군요 ㅎ

저는 policy 테이블을 상품과 쿠폰이 모두 쓰는 것에 대해서 불편함을 느끼고 있습니다. (그래서 안 쓰는 방향으로 리뷰를 드리게 되네요) 저는 상품의 세일정책과 쿠폰의 세일정책은 별도 도메인이라고 생각하는데요, 별도의 개념을 하나의 테이블에 담아두면 향후 DDL을 수정할 때 항상 상품과 쿠폰 양쪽 도메인을 생각해야 된다고 봐서요 ㅎㅎ 그 부분이 해소되면 괜찮을거 같네요

다만 세일 확장성 고려없이 말씀해주신 요구사항은 salePrice가 있냐 없냐를 두고 판단할 수 있지 않나? 생각이 드네요
( select * from product where salePrice is not null )
product.isOnSale 칼럼도 굳이 필드로 있을 필요가 없고 salePrice가 null인지 아닌지 보면 되는거 같구요

Copy link
Author

Choose a reason for hiding this comment

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

그러네요! null 값이 아닌 것들로만 조회해도 세일 상품만을 볼 순 있겠네요.

말씀해주신 것처럼 Policy를 쿠폰과 상품 모두에게 적용한다면 문제가 될 수 있을 것 같습니다.
만약 서비스가 고도화 될 때, 정책 또한 복잡해진다면 언젠간 분리할 일이 생길 수 있다고 생각합니다.

쿠폰의 할인 정책과 상품의 할인 정책은 엄연히 다르니, Policy 또한 고도화 된다면 필드가 추가되면서 분리하는 순간이 오겠죠..
저는 할인에 대해서는 같은 도메인이라고 생각했는데, 웨지의 리뷰를 보니 맞는 말이라고 고개만 끄덕였습니다 ㅎㅎㅋㅋ

말씀해주신 방법대로 리팩토링 진행했습니다. 좋은 지적 감사합니다.

Choose a reason for hiding this comment

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

물론 잘 알고 계시겠지만.. 제 말은 정답이 아니고 제 개발 경험과 습관에서 비롯된 코멘트임을 참고해주셔요 ㅎ


public static void validateCoupon(final boolean isPercentage, final int amount) {
if (isPercentage && (amount > MAXIMUM_AMOUNT || amount < MINIMUM_AMOUNT)) {
throw new CouponCreateBadRequestException();

Choose a reason for hiding this comment

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

예외 정보를 좀 더 풍부화 해볼까요? (몇 % 의 요청이 들어와서 실패했는지)
작업은 작은데 디버깅이나 장애 상황에서 생산성을 매우 끌어올려주는 좋은 작업입니다. 꼭 응답 메시지에 포함할 필요는 없지만, 로깅을 위해서 추가하면 매우 좋습니다 (개인적으론 필수라고 생각해요)

코드 전체적으로 예외 시 상황만 알려주고 구체적인 케이스 정보는 안 넘겨주는 것 같아요. (ex -> "멤버가 없다" 는 예외지만 몇번 ID로 요청이 들어왔는지는 남지 않음)
예외가 어떤 상황에서 발생했는지 전체적으로 수정해볼까요?

Copy link
Author

Choose a reason for hiding this comment

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

이런 꿀팁 너무 감사합니다 ㅎㅎ

앞으로 예외 작성할 때 웨지가 말씀해주신 내용이 항상 떠오를 것 같네요!
구체적인 상황을 명시해주지 않으면 디버깅으로 찾을 수도 있겠네요.
즉 상황 명시해주는 시간이 조금 들지만, 이걸로 추후에 오래 걸릴만한 작업의 시간도 줄여주는거군요!

감사합니다 :)

@@ -0,0 +1,121 @@
package cart.controller.cart;

Choose a reason for hiding this comment

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

테스트가 전체적으로 과한 감이 있다는 생각이 들었습니다.
통합테스트 1개와 각 레이어별 슬라이스 테스트 정도가 적당하다고 생각하는 편인데, 모든 하위 레이어를 포함하는 통합테스트가 각 레이어별로 있으니 "유지보수 시에 감당이 될까?" 라는 생각이 들더라구요 ㅎ

당연히 꼼꼼한 테스트는 좋지만 실제로 현업에서 코드 치다가 테스트 전체 주석 몇번 해보면... 테스트의 유지보수성에도 고찰을 하게 되는 거 같습니다 (갑자기 모르는 영역에서 테스트 깨지면 CI 뚫어야 하니까 아예 테스트를 지워버리는 경우도 자주 생깁니다, 시간 많고 여유있으면 해당 영역 공부해서 고칠텐데 최소한 저는 시간이 많아 본 적이 한 번도 없습니다 -.-ㅋ)

Copy link
Author

Choose a reason for hiding this comment

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

흠 그런 일이 일어나군요 ㅎㅎ

확실히 공부를 하는 지금은 시간이 많아서 테스트를 많이 작성하는게 좋다고 생각했지만, 실제 현업에서는 시간이 많이 없어서 이런 부분에서 타협을 하겠네요.

사실 아직도 저만의 테스트 기준이 명확하게 잡히지 않은 것 같습니다.
그리서 그냥 지금은 레이어의 통합 테스트와 단위 테스트를 수행하고 E2E 테스트까지 만들어서 너무 많은 테스트가 있는 것 같습니다.

지금은 그래도 배우는 입장이라서 우테코 기간동안은 웬만하면 이렇게 진행하겠지만, 어느정도 이 테스트들의 순위를 스스로 정해보는 연습도 해야할 것 같습니다. (그래야 바쁠 때 중요한 테스트 순서로 할 것 같아요 ㅎㅎ)

테스트는 오래걸려도 많으면 무조건 좋다고 생각했는데, 이런 부분에서 다시 한 번 생각하고 갑니다!
테스트도 어떻게 보면 비용의 문제이고 유지보수의 대상이 되니깐, 꼭 모든 테스트를 다 해야하는 것은 아니겠네요!

웨지의 좋은 경험이 담긴 피드백 너무 감사합니다🙇🏻‍♂️

@@ -0,0 +1,320 @@
:toc: left

Choose a reason for hiding this comment

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

문서 초 깔끔 이쁘네요 ㅎ

include42 pushed a commit that referenced this pull request Jun 5, 2023
* feat: cors 설정

* build: 상품 더미 데이터 추가

* build: utf 8 설정

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

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

* test: REST Docs Helper 클래스 추가

* build: asciidoctor 누락 설정 추가

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

* docs: doctype 추가

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

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

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

* test: null 값 수정

* feat: Response 클래스 추가

* feat: CartItemApiController 반환 타입 Response 변경

* feat: ProductApiController 반환 타입 Response 변경

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

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

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

* docs: 기능 목록 작성

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

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

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

* feat: 금액 검증 기능 추가

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

* feat: 포인트 객체 생성

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

* feat: 수량 검증 기능 추가

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

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

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

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

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

* refactor: 가격 필드 타입 변경

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

* test: 상품 생성 테스트 추가

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

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

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

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

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

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

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

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

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

* feat: OrderDao 추가

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

* feat: OrderItemDao 추가

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

* refactor: 포인트 파라미터 값 타입으로 변경

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

* feat: OrderRepository 추가

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

* refactor: MemberDao 메서드 명 변경

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

* build: orders, order_item 테이블 추가

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

* refactor: DAO 메소드 명 수정

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

* refactor: DAO 조회 반환 타입 Optional로 변경, 코드 리팩터링

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

* build: Spring Validation 의존성 추가

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

* feat: PointService 추가

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

* feat: 회원의 포인트를 조회하는 기능 추가

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

* feat: 주문 기능 추가

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

* feat: OrderController 추가

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

* chore: 코드 서식 정리

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

* feat: 주문 생성 검증 추가

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

* fix: 회원 업데이트 시 포인트도 업데이트 되도록 수정

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

* test: OrderService 테스트 추가

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

* test: 테스트 코드 수정

- 사용하지 않는 필드 제거
- Transactional 추가

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

* build: MySQL 의존성 추가

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

* build: local, production 환경 분리

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

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

* 예외 및 검증 기능 추가 (#8)

* feat: 예외 클래스 추가

* style: 코드 서식 수정

* fix: 예외 로그 수정

* feat: 주문 검증 로직 추가

- 주문 상품이 없으면 예외 발생

* feat: 주문 조회 시 응답 필드 추가

- 대표 상품 이름, 주문 상품 갯수

* chore: dto 패키지 분리

* feat: Request Validation 추가

* test: Request 검증 테스트 추가

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

* style: 코드 서식 수정

* refactor: TODO 제거

* test: 상품 생성, 수정 테스트 코드 추가

* docs: 기능 목록 업데이트

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

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

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

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

* test: 테스트 코드 DisplayName 수정

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

* refactor: Dao 메소드 명 수정

* refactor: 패키지 구조 세분화

* feat: PointPolicy 추가

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

* refactor: 테스트용 AdminController 삭제

* build: Swagger 의존성 추가

* feat: Swagger 적용

* refactor: PathVariable 명확하게 변경

* test: FixedAmountPointPolicy 테스트 코드 추가

* refactor: AuthInterceptor 스프링 빈 등록 제외, WebMvcConfig에서 AuthProvider 주입 받도록 변경

---------

Co-authored-by: DongUk <68818952+Kim0914@users.noreply.github.com>
Copy link

@sihyung92 sihyung92 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제이~~
금방 리뷰 요청을 주셨는데 좀 늦어져서 죄송합니다 🙏
간단한 리뷰 몇 개 남겼으니 확인해보셔요~!

@NotNull(message = "정률이라면 %를 입력해주시고, 아니라면 할인할 가격을 입력해주세요.")
private Integer amount;

public CouponCreateRequest() {

Choose a reason for hiding this comment

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

기본적으로 서버 내에서 구동 환경이 gradle 이므로 gradle 기준의 테스트가 성공해야 한다고 생각하긴 하지만, 이 케이스는 사소한 편이라 인텔리제이 빌드가 생산성을 올려준다고 생각한다면 팀 내 컨벤션으로 기본 생성자를 넣는것도 괜찮다고 생각합니다 ㅎ

public class OrderHistory {

private final Long id;
private final String orderTime;

Choose a reason for hiding this comment

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

@jsonformat을 학습해 보시고 적용해보셔요 ㅎㅎ 말씀하신 프론트엔드 요구사항을 위한 직렬화 도구는 대부분 마련되어 있는 편입니다

@@ -0,0 +1,26 @@
package cart.entity.product;

public class ProductSaleEntity {

Choose a reason for hiding this comment

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

아하 그렇군요~~ 일종의 세일 정보에 대한 테이블로 보면 되겠군요 ㅎ

저는 policy 테이블을 상품과 쿠폰이 모두 쓰는 것에 대해서 불편함을 느끼고 있습니다. (그래서 안 쓰는 방향으로 리뷰를 드리게 되네요) 저는 상품의 세일정책과 쿠폰의 세일정책은 별도 도메인이라고 생각하는데요, 별도의 개념을 하나의 테이블에 담아두면 향후 DDL을 수정할 때 항상 상품과 쿠폰 양쪽 도메인을 생각해야 된다고 봐서요 ㅎㅎ 그 부분이 해소되면 괜찮을거 같네요

다만 세일 확장성 고려없이 말씀해주신 요구사항은 salePrice가 있냐 없냐를 두고 판단할 수 있지 않나? 생각이 드네요
( select * from product where salePrice is not null )
product.isOnSale 칼럼도 굳이 필드로 있을 필요가 없고 salePrice가 null인지 아닌지 보면 되는거 같구요

Copy link

@sihyung92 sihyung92 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제이~~
리뷰반영 순식간에 해주시는데 제가 자꾸 늦네요 ㅠ
리뷰 반영 해주신 부분에서 한번만 더 가겠습니당~~


public Product(final String name, final int price, final String imageUrl) {
this.name = name;
this.price = price;
this.imageUrl = imageUrl;
this.isOnSale = false;
this.policy = new PolicyPercentage(0);
salePrice = 0;

Choose a reason for hiding this comment

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

생성시 초기화를 0으로 해서 처리를 하기 전에는 계속 0일 것 같은데요, Integer로 다룬 이유가 있을까요? (Null 인 상황이 나올지)
또 무료 상품인지 세일 안하는 상품인지 오인할 수 있을 것 같아요. 저라면 null을 다루기 싫으니 salePrice를 VO로 만들어 null 케이스를 표현해 줄거 같습니다.

class SalePrice {
  // DB상 null이거나 0이거나 하면 아래 객체로 표현
  public static SalePrice NOT_SALE = new SalePrice(0);
  
  private int value;
  
  new SalePrice(int price){
    value = price;
  }
  ...
  
 public int calc(int originPrice) {
    return price - salePrice;
 }
}

Copy link
Author

Choose a reason for hiding this comment

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

헛 그렇네요 ㅠㅠ

이번 미션하면서 도메인 엔터티 개념이 너무 충돌되고 그래서 원시값 포장할 생각도 못했습니다.

생각해보면 SalePrice 객체를 만들어서 역할을 위임할 수 있는데 말이죠.. ㅎㅎ

레벨 1때 배운 것들을 최대한 살리려고 미션을 할 때 의식하면서 했는데 이번 미션에서는 그러질 못했네요!
반성하고 갑니다 ㅠ.ㅠ

말씀하신 것처럼 DB에서는 값이 NULL로 올 수도 있고, 값으로 올 수도 있기 때문에 Product 생성자 파라미터에는 Integer로 받고, 나머지는 SalePrice 객체 내부적으로 처리해주었습니다!

import java.util.List;
import java.util.stream.Collectors;

public class OrderResponse {

private Long orderId;
private final String orderedTime;
private final Date orderedTime;

Choose a reason for hiding this comment

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

Date는 "구려서"(말 그대로) 잘 안 쓰는 편입니다 ㅎ 대신 LocalDateTime을 써보시면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

이부분을 잘 몰라서 찾아 봤는데, Java8부터 등장했네요!
기존의 Date, Calendar가 문제점이 많았다고 하네요 ㅎㅎ

감사합니다~

Choose a reason for hiding this comment

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

자바의 장점이자 단점이라고 생각하는데요 ㅎㅎ

  • 장점 : 하위 호환성을 매우 잘 지켜준다 (LocalDateTime이 나왔다고 Date를 없애지 않는다 -> Date를 활용하고 있는 수많은 어플리케이션이 깨지니까)
  • 단점 : 그래서 쓰는 사람 헷갈린다

웹개발자 입장에선 API 수정 배포시 하위호환을 고려해야할 일이 많아 "하위호환" 키워드에 대해 생각해보게 만드는 지점인거 같습니다

Copy link

@sihyung92 sihyung92 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제이~~ 웨지입니다!
리뷰 반영 잘 해주셨네요 ㅎㅎ 이만 머지하겠습니다 🙏
방학 잘 보내시고 남은 미션도 화이팅입니다! 🚀

@@ -118,6 +118,6 @@ void un_apply_sale_on_product() {
productService.unapplySale(id);

// then
assertThat(product.getSalePrice()).isEqualTo(null);
assertThat(product.getSalePrice()).isEqualTo(0);

Choose a reason for hiding this comment

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

ㅋㅋ 저도 상품 도메인을 다루는데 똑같은 필드명이 있어서 요 문장이 참 헷갈리네요
저는 salePrice를 "실제 판매가"로 써서요 (세일 금액이 아니라)
리뷰는 아니고 그냥 그렇습니다

import java.util.List;
import java.util.stream.Collectors;

public class OrderResponse {

private Long orderId;
private final String orderedTime;
private final Date orderedTime;

Choose a reason for hiding this comment

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

자바의 장점이자 단점이라고 생각하는데요 ㅎㅎ

  • 장점 : 하위 호환성을 매우 잘 지켜준다 (LocalDateTime이 나왔다고 Date를 없애지 않는다 -> Date를 활용하고 있는 수많은 어플리케이션이 깨지니까)
  • 단점 : 그래서 쓰는 사람 헷갈린다

웹개발자 입장에선 API 수정 배포시 하위호환을 고려해야할 일이 많아 "하위호환" 키워드에 대해 생각해보게 만드는 지점인거 같습니다

@sihyung92 sihyung92 merged commit 698d357 into woowacourse:sosow0212 Jun 7, 2023
Laterality pushed a commit that referenced this pull request Jun 8, 2023
* feat: cors 설정

* build: 상품 더미 데이터 추가

* build: utf 8 설정

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

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

* test: REST Docs Helper 클래스 추가

* build: asciidoctor 누락 설정 추가

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

* docs: doctype 추가

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

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

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

* test: null 값 수정

* feat: Response 클래스 추가

* feat: CartItemApiController 반환 타입 Response 변경

* feat: ProductApiController 반환 타입 Response 변경

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

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

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

* docs: 기능 목록 작성

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

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

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

* feat: 금액 검증 기능 추가

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

* feat: 포인트 객체 생성

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

* feat: 수량 검증 기능 추가

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

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

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

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

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

* refactor: 가격 필드 타입 변경

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

* test: 상품 생성 테스트 추가

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

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

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

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

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

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

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

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

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

* feat: OrderDao 추가

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

* feat: OrderItemDao 추가

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

* refactor: 포인트 파라미터 값 타입으로 변경

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

* feat: OrderRepository 추가

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

* refactor: MemberDao 메서드 명 변경

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

* build: orders, order_item 테이블 추가

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

* refactor: DAO 메소드 명 수정

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

* refactor: DAO 조회 반환 타입 Optional로 변경, 코드 리팩터링

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

* build: Spring Validation 의존성 추가

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

* feat: PointService 추가

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

* feat: 회원의 포인트를 조회하는 기능 추가

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

* feat: 주문 기능 추가

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

* feat: OrderController 추가

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

* chore: 코드 서식 정리

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

* feat: 주문 생성 검증 추가

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

* fix: 회원 업데이트 시 포인트도 업데이트 되도록 수정

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

* test: OrderService 테스트 추가

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

* test: 테스트 코드 수정

- 사용하지 않는 필드 제거
- Transactional 추가

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

* build: MySQL 의존성 추가

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

* build: local, production 환경 분리

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

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

* 예외 및 검증 기능 추가 (#8)

* feat: 예외 클래스 추가

* style: 코드 서식 수정

* fix: 예외 로그 수정

* feat: 주문 검증 로직 추가

- 주문 상품이 없으면 예외 발생

* feat: 주문 조회 시 응답 필드 추가

- 대표 상품 이름, 주문 상품 갯수

* chore: dto 패키지 분리

* feat: Request Validation 추가

* test: Request 검증 테스트 추가

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

* style: 코드 서식 수정

* refactor: TODO 제거

* test: 상품 생성, 수정 테스트 코드 추가

* docs: 기능 목록 업데이트

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

* refactor: OrderItem builder 패턴 적용

* refactor: Order builder 패턴 적용

* refactor: Member builder 패턴 적용

* refactor: Member builder 패턴 적용

* fix: LocalDateTime 테스트 확인 메서드 변경

* refactor: Request 클래스 명 변경

* refactor: 사용하지 않는 어노테이션 삭제, 클래스 명 변경

* feat: 장바구니를 찾을 수 없는 경우 예외 상황 핸들링 추가

* refactor: API 응답 클래스 명 변경

* refactor: 포인트 사용 시 잔여 포인트 확인 메서드 회원 클래스로 이동

* refactor: Member, CartItem 클래스 빌더 제거

* refactor: Order, OrderItem, Product 클래스 빌더 제거

* refactor: 컨트롤러 반환 타입 구체적으로 명시

* refactor: 테스트 메서드 명 및 Location 검증 추가

* refactor: DB에서 ID 가져올 시 Long 타입으로 가져오도록 변경

---------

Co-authored-by: seokjin8678 <seokjin8678@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants