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

[step2 주문 api] 주드 미션 제출합니다. #8

Merged
merged 42 commits into from
Jun 7, 2023

Conversation

kevstevie
Copy link

@kevstevie kevstevie commented May 30, 2023

안녕하세요 소니!
기능 구현 완료하고 미션 제출합니다.
추가 재화 기능으로 포인트를 적용했고 구입금의 일부가 적립금으로 쌓입니다.
포인트는 최소 10원 단위로 사용할 수 있게 설정했습니다.

주문의 경우 클라이언트의 요청과 서버의 db 정보가 다르면 bad request를 띄우는 검증 과정을 거칩니다.
orders 요청의 body(주문 상품 정보) 없이 db의 정보만으로 바로 주문을 실행하는 건 위험의 여지가 클까요?

리뷰 범위 입니다. 여깁니다

리뷰 미리 감사드립니다!🙇‍♂️

Copy link

@sonypark sonypark left a comment

Choose a reason for hiding this comment

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

주드 안녕하세요! 만나서 반가워요. 리뷰어 소니입니다. :)
빠르게 미션 진행해주셨네요 👍
기존과 다르게 프론트 크루와의 협업 미션인만큼 느낀 부분도 많았을 것 같습니다.
궁금한 부분에 대해 코멘트 남겼으니 확인 부탁드립니다🙏

orders 요청의 body(주문 상품 정보) 없이 db의 정보만으로 바로 주문을 실행하는 건 위험의 여지가 클까요?

질문하신 부분이 OrderCreateRequest 내의 cartItemIds로 DB 정보를 조회해서 주문을 하는 부분이 맞을까요? DB에서 데이터 조회후 요청값과 DB에 저장된 값에 대해 유효성 검사를 하는 로직이 있어서 문제는 없을 것 같은데요. 다른 부분을 말씀하신거라면 코멘트 부탁드려요~~ :)

src/main/java/cart/ui/OrderController.java Outdated Show resolved Hide resolved
Comment on lines 62 to 74

public void changeQuantity(int quantity) {
this.quantity = quantity;
@Override
public boolean equals(final Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
final CartItem cartItem = (CartItem) o;
return quantity == cartItem.quantity && checked == cartItem.checked && Objects.equals(id, cartItem.id) && Objects.equals(product, cartItem.product) && Objects.equals(member, cartItem.member);
}

@Override
public int hashCode() {
return Objects.hash(id, quantity, product, member, checked);
}

Choose a reason for hiding this comment

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

  • CartItem에 대한 식별자로 id가 있는데, equals & hashcode 비교에서 id 식별자만으로 비교가 가능하지 않을까요?
  • CartItem에만 equals & hashcode를 재정의하고, 다른 domain 객체에는 재정의하지 않으신 이유가 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

뭔가 로직을 만드는 과정에서 동등성을 비교하기 위해 만들었는데 현재는 사용하지 않고 있군요
소니 말처럼 id만으로 비교하면 충분할 것 같습니다.

Comment on lines +35 to 38
public CartItemResponse findById(Long cartItemId) {
final CartItem cartItem = cartItemDao.findById(cartItemId);
return CartItemResponse.of(cartItem);
}

Choose a reason for hiding this comment

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

조회메서드에서는 Transational에 read-only 옵션을 줄 수 있는데요.
이 옵션의 유무에 따라 어떤 차이가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

db에 읽기전용 트랜잭션이라는 힌트를 준다고 나와있군요!
더 찾아보니 사용하는 db에 따라 조회전용에 대한 최적화나 데이터의 변경 또한 방지를 해준다고 하네요.

Comment on lines 13 to 19

public CartItem(Member member, Product product) {
this.quantity = 1;
this.member = member;
this.product = product;
this.checked = true;
}

Choose a reason for hiding this comment

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

Suggested change
public CartItem(Member member, Product product) {
this.quantity = 1;
this.member = member;
this.product = product;
this.checked = true;
}
public static CartItem of(Member member, Product product) {
return new CartItem(null, 1, product, member, true);
}

정적 팩토리 메서드를 사용하는 방법도 있겠네요.
개인적으로 필드와 1:1 매핑 되는 경우는 생성자로 만들고 그렇지 않은 경우는 정적팩토리 메서드를 사용하는걸 선호합니다 (이건 취향 차이이니 주드의 기준에 맞게 사용하시면 될 것 같아요!)

Comment on lines 45 to 46
CartItem cartItem = cartItemDao.findById(id);
cartItem.checkOwner(member);

Choose a reason for hiding this comment

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

cartItem 가 null 일 경우 NPE가 발생할것 같은데 맞을까요?

public CartItem findById(Long id) {
     ...
    return cartItems.isEmpty() ? null : cartItems.get(0);
}

Copy link
Author

Choose a reason for hiding this comment

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

맞습니다.. 뼈대코드였는데 전부 수정했습니다

jdbcTemplate.update(sql, cartItem.getQuantity(), cartItem.getId());
public void update(CartItem cartItem) {
String sql = "UPDATE cart_item SET quantity = ?, checked = ? WHERE id = ?";
jdbcTemplate.update(sql, cartItem.getQuantity(), cartItem.isChecked(), cartItem.getId());
}

Choose a reason for hiding this comment

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

  • 존재하지 않는 id로 delete, update 를 수행하면 어떻게 되나요?
  • 에러가 발생하지 않는다면, 사용자에게 알리지 않아도 될까요?

Copy link
Author

Choose a reason for hiding this comment

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

response로 404 내리게 수정했습니다!

Comment on lines 66 to 98

private void validateLegalOrder(final List<CartItem> cartItems, final List<CartItemRequest> requests) {
for (final CartItem cartItem : cartItems) {
iterateRequests(requests, cartItem);
}
}

private void iterateRequests(final List<CartItemRequest> requests, final CartItem cartItem) {
for (final CartItemRequest request : requests) {
compareEachCartItemIfIdEquals(cartItem, request);
}
}

private void compareEachCartItemIfIdEquals(final CartItem cartItem, final CartItemRequest request) {
if (cartItem.getId().equals(request.getId())) {
compareEachCartItem(cartItem, request);
}
}

private void compareEachCartItem(final CartItem cartItem, final CartItemRequest request) {
if (isInvalidProduct(cartItem, request) || isInvalidQuantity(cartItem, request) || isNotChecked(cartItem)) {
throw new IllegalArgumentException("주문 정보가 일치하지 않습니다.");
}
}

private boolean isInvalidProduct(final CartItem cartItem, final CartItemRequest request) {
return !cartItem.getProduct().getId().equals(request.getProductId());
}

private boolean isInvalidQuantity(final CartItem cartItem, final CartItemRequest request) {
return cartItem.getQuantity() != request.getQuantity();
}

Choose a reason for hiding this comment

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

다음과 같은 검증 로직은 service가 아닌 cartItem 객체에서 할 수 있지 않을까요?

  • cartItems가 해당 member가 지닌 cartItems인지 검증
  • cartItemIds size와 cartItems size가 일치하는지 검증
  • cartItems 총 금액 계산

이렇게 하면 createOrder() 내 로직도 간결해질 것 같은데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

request와 cartItem의 의존성 때문에 고민했는데 Map에 필요한 정보를 담게하고 cartItem에서 검증하도록 했습니다

Comment on lines +27 to 33
CREATE TABLE IF NOT EXISTS orders
(
id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY,
member_id BIGINT NOT NULL,
used_points INT NOT NULL,
saving_rate INT NOT NULL
);

Choose a reason for hiding this comment

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

주문 테이블에 used_points, saving_rate 을 저장하는 이유가 있을까요?
실제로 서비스에서 사용되는 부분이 있는지 궁금합니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

order 조회시에 당시의 사용포인트와 포인트적립률을 표기하고 있습니다

Comment on lines +195 to +245
}
```

### 장바구니 아이템 삭제

```
DELETE /cart-items/{cartItemId}
Authorization: Basic ${credentials}
```

```
HTTP/1.1 204 No Content
```

## 포인트

* 주문할 때 포인트를 사용한 부분은 제하고 10%를 적립합니다.
* 10원 단위로만 사용할 수 있습니다. (ex. 27원 사용 불가)
* 포인트 정책은 백엔드에서 결정하기 때문에 반드시 포인트 계산 결과를 백엔드에서 가져와서 사용하여야 합니다.

### 현재 장바구니에서 얻을 수 있는 포인트 조회

* `savingRate`는 포인트 적립 비율을 의미합니다. 만약 17000원 주문을 생성한다면, 1700원이 적립됩니다.
* `points`는 현재 장바구니에 담긴 물품을 결제할 시 얻을 수 있는 포인트입니다.

```
GET /cart-points
Authorization: Basic ${credentials}
```

```
HTTP/1.1 200 OK
Content-Type: application/json

{
"savingRate": 10,
"points": 1270
}
```

## 주문

### 주문 조회

```
GET /orders
Authorization: Basic ${credentials}
```

```
HTTP/1.1 200 OK

Choose a reason for hiding this comment

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

README 잘 작성해주셨네요.

  • 이번 미션은 백엔드 크루뿐 아니라 프론트 크루분들과 협업으로 진행했을텐데 API 명세나 구조를 정할때는 어떻게 하셨나요? - API 문서는 README 문서를 보는 것으로 협의하신건가요?
  • API 설계를 클라이언트(웹 프론트엔드, 모바일 안드로이드)와 논의했을 때와 기존에 백엔드 크루와 페어를 했을때 차이점이 있었나요?
  • 서로 의견이 다른 점이 있을때 어떤 방식으로 조율하셨는지도 궁금하네요 😃

Copy link
Author

Choose a reason for hiding this comment

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

  • 다같이 모여서 api 명세를 결정했습니다! 작업을 시작할 땐 readme 문서를 보기로 했고 그 후엔 swagger 배포를 했습니다.

  • 아무래도 서로 다루는 부분이나 관심사가 다르다보니 이해할 시간이 좀 필요했습니다. 백엔드 크루와 페어할 때도 생각이 맞아떨어질 때가 흔치 않은데 더 어려운 부분이 있었던 것 같네요.

  • 서로에게 요구사항을 이야기하면서 이해가.. 될 때까지 대화를 나누었습니다! 최종적으로 사실 의견이 크게 다른 점은 없었던 것 같아요.

seokjin8678 added a commit to seokjin8678/jwp-shopping-order that referenced this pull request Jun 2, 2023
* feat: 예외 클래스 추가

* style: 코드 서식 수정

* fix: 예외 로그 수정

* feat: 주문 검증 로직 추가

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

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

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

* chore: dto 패키지 분리

* feat: Request Validation 추가

* test: Request 검증 테스트 추가
@kevstevie
Copy link
Author

안녕하세요 소니! 리뷰 반영하고 다시 요청드립니다.

orders 요청의 body(주문 상품 정보) 없이 db의 정보만으로 바로 주문을 실행하는 건 위험의 여지가 클까요?

현재 Post /orders에 대한 body로 주문에 대한 상세정보를 받고 이에 대한 서버 db와의 비교를 진행하고 있습니다.
프론트분들과 맨 처음에 요청에 대한 body없이 서버에서 checked가 true인 상품을 모두 주문하는 방식을 이야기했었는데 이러한 방식일 경우 리스크가 발생할 지? 에 대한 질문이었습니다!

추가로 현재 예외 메시지를 그냥 예외 클래스의 이름으로 내려주기로 협의했었는데 ErrorResponse를 통해서 error code와 message를 같이 내려주는 방법에 대해 알게 되었습니다.
이런 error code의 경우 상태코드처럼 정해진 규격이 있나요?

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

@sonypark sonypark left a comment

Choose a reason for hiding this comment

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

주드 안녕하세요!
예외응답클래스를 정의해서 적용보면 좋을 것 같아서 코멘트 남겼으니 확인부탁드릴게요~

DB에 있는 checked 값만 가지고 주문을 할 경우

현재 주문을 할때 장바구니에 담긴 물품을 그대로 주문하는 경우만 있는지 궁금합니다.
주문시 일부 품목을 체크 해제하고 주문을 하면 요청값에 있는 물품이 DB에 저장된 checked 상태와 다른 경우가 생길 수 있지 않나요? 현재 구조상 체크된 물품만 주문가능하도록 보장이 되는 걸까요?

Comment on lines 6 to 18

public class CorsTest extends IntegrationTest {

@Test
void CORS_기능을_검증한다() {
RestAssured.given().log().all()
.header("Origin", "https://solo5star.github.io/react-shopping-cart-prod")
.header("Access-Control-Request-Method", "GET")
.when()
.get("/admin")
.then().log().all()
.statusCode(HttpStatus.OK.value());

Copy link

Choose a reason for hiding this comment

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

혹시 이 테스트 통과하시나요?

Comment on lines 6 to 9

public class CorsTest extends IntegrationTest {

@Test
Copy link

Choose a reason for hiding this comment

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

Junit5 의 테스트 클래스,메서드에는 public을 붙이지 않아도 됩니다. (다른 테스트 클래스는 public을 붙이지 않았는데 몇군데만 붙인게 보여서 코멘트 남깁니다 :) )

Comment on lines +8 to +11

@Sql(value = "/truncate.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
@Sql({"/schema.sql", "/data.sql"})
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
Copy link

Choose a reason for hiding this comment

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

sql 파일 import 👍

Comment on lines 9 to 39
@RestControllerAdvice
public class GlobalExceptionHandler {

@ExceptionHandler(AuthenticationException.class)
public ResponseEntity<Void> handlerAuthenticationException(AuthenticationException e) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
}

@ExceptionHandler(CartItemException.IllegalMember.class)
public ResponseEntity<Void> handleException(CartItemException.IllegalMember e) {
return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}

@ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<String> handleIllegalArgumentException(IllegalArgumentException exception) {
return ResponseEntity.badRequest().body(exception.getMessage());
}

@ExceptionHandler(NotFoundException.class)
public ResponseEntity<String> handleProductNotFoundException(NotFoundException exception) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(exception.getClass().getSimpleName());
}

@ExceptionHandler({
InvalidOrderCheckedException.class,
InvalidOrderQuantityException.class,
InvalidOrderProductException.class,
IllegalPointUsageException.class
})
public ResponseEntity<String> handleCustomApiException(RuntimeException exception) {
return ResponseEntity.badRequest().body(exception.getClass().getSimpleName());
Copy link

Choose a reason for hiding this comment

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

말씀하신것처럼 현재 예외가 발생했을때 클래스 정보만 응답으로 내려주고 있는데요.
클래스 정보만으로 어떤 예외가 발생했는지 판단할 수 있을까요? (예외가 늘어날때마다 클래스도 계속 생성될 것 같네요. 이러면 오히려 더 관리가 어려워질 수 있지 않을까요?)
ErrorResponse도 기본적으로 정해진 양식이 있습니다. (디테일한 부분은 팀마다 다를 수 있지만 기본적인 구조는 같습니다.)

Exception 관련해서는 아래 글에 자세히 나와있으니 참고해서 수정해볼까요?
https://cheese10yun.github.io/spring-guide-exception/

Comment on lines 67 to 78
@Override
public boolean equals(final Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
final CartItem cartItem = (CartItem) o;
return id.equals(cartItem.id);
}

@Override
public int hashCode() {
return Objects.hash(id);
}
Copy link

Choose a reason for hiding this comment

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

오버라이딩한 equals & hashcode 메서드는 최하단에 위치시키는게 일반적입니다 :)

@kevstevie
Copy link
Author

안녕하세요 소니 리뷰 반영하고 질문드립니다.
다음 협업땐 예외처리와 코드에 대해서 확실하게 협의하고 가야겠군요!

현재 주문을 할때 장바구니에 담긴 물품을 그대로 주문하는 경우만 있는지 궁금합니다.
주문시 일부 품목을 체크 해제하고 주문을 하면 요청값에 있는 물품이 DB에 저장된 checked 상태와 다른 경우가 생길 수 있지 않나요? 현재 구조상 체크된 물품만 주문가능하도록 보장이 되는 걸까요?

이 부분도 비즈니스로직을 통해 검증하고 있지만 사실 체크 상태 변경과 주문 요청이 클라이언트에서 동시에 일어날 경우 어떤 결과가 나올진 잘 모르겠습니다..

Copy link

@sonypark sonypark 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 12 to 34

@ExceptionHandler(AuthenticationException.class)
public ResponseEntity<Void> handlerAuthenticationException(AuthenticationException e) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
public ResponseEntity<ErrorResponse> handlerAuthenticationException(AuthenticationException e) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
.body(new ErrorResponse(e.getMessage(), e.getClass().getSimpleName()));
}

@ExceptionHandler(CartItemException.IllegalMember.class)
public ResponseEntity<Void> handleException(CartItemException.IllegalMember e) {
return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
public ResponseEntity<ErrorResponse> handleException(CartItemException.IllegalMember e) {
return ResponseEntity.status(HttpStatus.FORBIDDEN)
.body(new ErrorResponse(e.getMessage(), e.getClass().getSimpleName()));
}

@ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<String> handleIllegalArgumentException(IllegalArgumentException exception) {
return ResponseEntity.badRequest().body(exception.getMessage());
public ResponseEntity<ErrorResponse> handleIllegalArgumentException(IllegalArgumentException exception) {
return ResponseEntity.badRequest()
.body(new ErrorResponse(exception.getMessage(), exception.getClass().getSimpleName()));
}

@ExceptionHandler(NotFoundException.class)
public ResponseEntity<String> handleProductNotFoundException(NotFoundException exception) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(exception.getClass().getSimpleName());
public ResponseEntity<ErrorResponse> handleProductNotFoundException(NotFoundException exception) {
return ResponseEntity.status(HttpStatus.NOT_FOUND)
.body(new ErrorResponse(exception.getMessage(), exception.getClass().getSimpleName()));
Copy link

Choose a reason for hiding this comment

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

네 좋습니다 👍
예외 메시지를 추가한 만큼 테스트코드에서도 예외메시지까지 검증해주는걸 추가했으면 더 좋았을것 같네요😃

@sonypark sonypark merged commit b4f61e0 into woowacourse:kevstevie 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