-
Notifications
You must be signed in to change notification settings - Fork 151
[4기 박상민] Springboot-jpa weekly 미션 3 PR입니다. #334
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
Changes from all commits
a5fa74c
7edb8d9
9249721
24f01c4
6c34145
cadaf52
58b3a66
e098e33
963a060
1e97000
7a7fb07
100445c
236d1ce
c3b0976
a5e4d55
0131918
f9e5cee
beee729
5c9bd47
84fc7b2
33a3c88
da38737
bd8314a
8264c08
6b88225
45c7980
e4853c3
a2660f5
8af6a32
6cc2537
c7d6887
9731909
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
|
|
||
| @Slf4j | ||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { // TODO: 혹시 멘토님들은 여기서 어떤 어떤 에러를 잡으시나요? | ||
| public class GlobalExceptionHandler { | ||
| @ExceptionHandler(EntityNotFoundException.class) | ||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public ErrorResponse handleEntityNotFoundException(EntityNotFoundException e) { | ||
|
|
@@ -23,7 +23,11 @@ public ErrorResponse handleEntityNotFoundException(EntityNotFoundException e) { | |
| @ResponseStatus(HttpStatus.BAD_REQUEST) | ||
| public ErrorResponse handleMethodArgumentNotValidException(MethodArgumentNotValidException e) { | ||
| log.warn(e.getMessage(), e); | ||
| return ErrorResponse.newErrorResponse(e.getMessage()); | ||
| return ErrorResponse.newErrorResponse(e | ||
| .getBindingResult() | ||
| .getFieldErrors() | ||
| .get(0) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 첫 번째 에러만 반환하는 이유가 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 첫 번째 에러에 제가 validation 규칙에서 적용한 message를 담고 있어서 첫번째 에러만 반환했습니다. 뭔가 코드가 찜찜하긴 한데 혹시 다른 방법이 있나요? |
||
| .getDefaultMessage()); | ||
| } | ||
|
|
||
| @ExceptionHandler(HttpMessageNotReadableException.class) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package org.programmers.jpaweeklymission.item.domain; | ||
|
|
||
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.Table; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import org.programmers.jpaweeklymission.global.BaseEntity; | ||
|
|
||
| @Table(name = "items") | ||
| @Getter | ||
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class Item extends BaseEntity { | ||
| @Id | ||
| @Column(name = "id") | ||
| @GeneratedValue(strategy = GenerationType.AUTO) | ||
| private Long id; | ||
|
|
||
| @Column(name = "price") | ||
| private int price; | ||
|
|
||
| @Column(name = "stock_quantity") | ||
| private int stockQuantity; | ||
|
|
||
| @Builder | ||
| public Item(int price, int stockQuantity) { | ||
| this.price = price; | ||
| this.stockQuantity = stockQuantity; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package org.programmers.jpaweeklymission.order.domain; | ||
|
|
||
| import jakarta.persistence.*; | ||
| import jakarta.validation.constraints.NotNull; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import org.programmers.jpaweeklymission.customer.Customer; | ||
| import org.programmers.jpaweeklymission.global.BaseEntity; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Objects; | ||
|
|
||
| @Table(name = "orders") | ||
| @Entity | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class Order extends BaseEntity { | ||
| @Id | ||
| @Column(name = "id") | ||
| @GeneratedValue(strategy = GenerationType.AUTO) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 따로 id 생성 전략을 지정하지 않는 이유가 있나요? |
||
| private Long id; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "status", nullable = false) | ||
| @NotNull | ||
| private OrderStatus status; | ||
|
|
||
| @Lob | ||
| @Column(name = "memo") | ||
| private String memo; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "customer_id", referencedColumnName = "id") | ||
| private Customer customer; | ||
|
|
||
| @OneToMany(mappedBy = "order") | ||
| private List<OrderItem> orderItems; | ||
|
|
||
| @Builder | ||
| public Order(OrderStatus status, String memo) { | ||
| this.status = status; | ||
| this.memo = memo; | ||
| } | ||
|
|
||
| public void setCustomer(Customer customer) { | ||
| if (Objects.nonNull(this.customer)) { | ||
| customer.removeOrder(this); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이게 로직이 맞나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 저부분 여쭤보려고 주석을 달았어야 했는데 깜빡했습니다. 일단 강사님께서 강의중에 저런 로직을 작성하셨습니다. 근데 그 부분에 좀 의문이 들었는데 사실 setCustomer 라는 건 제가 생각하기로는 초기에 주문( 근데 저걸 적으신 강사님이 의도한 건 아래 두 개라고 생각했습니다.
다른 부분 Item, OrderItem 부분에도 nonNull 체크를 전부 적용하셨는데 이게 필요한가요?, 좋은 방법인가요? 멘토님들 의견 궁금합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 로직은 상품이 생성될 때 구매자가 중복 등록되는 것을 방지하는 코드가 맞는 것 같습니다.
아래에서 양방향 매핑이 조금 복잡하다면 단방향 매핑으로 단순하게 시작하는 것도 좋을 것 같네요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 잘 이해한 건지 모르겠는데 "그렇다면 기존 구매자와 주문간 관계를 끊어주어야 하는 것이 아닐까요?" 이 부분을 |
||
| } | ||
| this.customer = customer; | ||
| customer.getOrders().add(this); | ||
| } | ||
|
|
||
| public void addOrderItems(OrderItem orderItem) { | ||
| orderItem.setOrder(this); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 orderItems에는 추가되는 orderItem을 add 해주지 않아도 되나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. setOrder 메서드가 아래처럼 작성 되어 있습니다. this.order = order; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. orderItem이 주도적으로 제어하는 이유가 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 누가 하던지 상관없다고 생각하는데, 키를 갖고 있고 수정 시 쿼리가 나가는 OrderItem 테이블에서 제어해주는게 좋다고 생각했습니다. 또, 양방향 매핑 시에 저런 메서드가 계속적으로 생겨나는데, 키를 갖고 있는 쪽에 저런 메서드를 정의하자라고 기준을 정헀습니다. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package org.programmers.jpaweeklymission.order.domain; | ||
|
|
||
| import jakarta.persistence.*; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import org.programmers.jpaweeklymission.global.BaseEntity; | ||
| import org.programmers.jpaweeklymission.item.domain.Item; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| @Entity | ||
| @Getter | ||
| @Table(name = "order_items") | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class OrderItem extends BaseEntity { | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.AUTO) | ||
| @Column(name = "id") | ||
| private Long id; | ||
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "order_id", referencedColumnName = "id") | ||
| private Order order; | ||
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "item_id", referencedColumnName = "id") | ||
| private Item item; | ||
|
|
||
| public void setOrder(Order order) { | ||
| this.order = order; | ||
| order.getOrderItems().add(this); | ||
| } | ||
|
|
||
| public void setItem(Item item) { | ||
| this.item = item; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package org.programmers.jpaweeklymission.order.domain; | ||
|
|
||
|
|
||
| public enum OrderStatus { | ||
| CREATED, | ||
| SHIPPING, | ||
| DELIVERED, | ||
| CANCELED | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
min 이 지정되어 있는데 not blank도 검증 해주어야 하나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각해보니 " " 같은 부분이 있기 때문에 다시 설정했습니다.