Skip to content
This repository was archived by the owner on Aug 13, 2022. It is now read-only.

Conversation

@ghost
Copy link

@ghost ghost commented Oct 26, 2021

기능을 구현하면서 JPA를 어떻게 적용해야할지 잘 이해가지 않는 부분이 있어 JPA책을 다시 읽어보았습니다.
책을 읽어보고 리팩토링을 해보면 좋겠다고 생각한 지점은 다음과 같습니다. (병합하기전 모두 적용하겠습니다!)

  1. JPA 상속 기능
  • Shop/Business Hour/Shop Delivery에 영속성 전이고아 객체를 사용하여 하나의 Shop 엔티티를 제거하더라도 Business Hour와 Shop Delivery도 함께 제거할 수 있도록 리팩토링 하려고 합니다. (p.307)
  1. 양방향 매핑에서 연관관계 변경시 주의점
  • 2-1. 양방향 매핑시 주인 엔티티가 아닌곳에도 값을 입력하기 (p.190)
  • 2-2. 연관관계 변경 혹은 삭제시 반대변 엔티티와 연관관계 끊기 적용(p.178, p.192)
  • 2-3. 양방향 매핑시 Setter에서 무한루프 확인 및 반대편 엔티티 값 설정 리팩토링 (p.208)
  1. Order 엔티티 Embeded 값 객체 사용시 문제점
  • 3-1. Embeded 타입 사용시 불변객체 만들기 (p.329)
  • 3-2. 메뉴 칼럼 컬렉션(List) 혹은 새로운 테이블 생성해서 사용하기 (p.334)
  1. Order 엔티티 매핑 수정
  • Order <> Shop 매핑을 해야하는데 Merchant와 잘못 매핑 수정하기

@ghost ghost added the feature New feature label Oct 26, 2021
@ghost ghost requested review from f-lab-bright and yusok7 October 26, 2021 07:06
@ghost ghost assigned yusok7 and ghost Oct 26, 2021
Copy link
Author

@ghost ghost left a comment

Choose a reason for hiding this comment

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

JPA 책을 다시 읽고 단순히 매핑만 사용해서 코드를 막 작성했다는 생각이 많이 들었습니다. 또한 코드를 수정하기전에 어떤 과정을 거쳤고 왜 새로운 코드로직을 선택했는지 남기면 좋을것 같아 수정하지 않고 PR을 하였습니다.

수정해야하는 코드는 이번 PR에서 모두 수정하고 병합할 수 있도록 하겠습니다!

Comment on lines 25 to 52
public static FindAllOrderResponse ofList(List<Order> orderList) {
List<MenuListOfResponse> list = new ArrayList<>();
// todo. 반환 가맹점 정보

for (int i = 0 ; i < orderList.size() ; i++){
MenuListOfResponse menuListOfResponse = MenuListOfResponse.builder()
.orderNo(orderList.get(i).getOrderNo())

.merchantNo(orderList.get(i).getMerchantInfo().getId())
.merchantId(orderList.get(i).getMerchantInfo().getUserId())
.merchantName(orderList.get(i).getMerchantInfo().getName())

.orderMenu(orderList.get(i).getOrderMenu())
.orderMenu1(orderList.get(i).getOrderMenu1())
.orderStatus(orderList.get(i).getOrderStatus())
.totalPrice(orderList.get(i).getTotalPrice())
.build();

list.add(menuListOfResponse);
}

return FindAllOrderResponse.builder()
.orderMenuList(list)
.consumerNo(orderList.get(0).getConsumerInfo().getId())
.consumerId(orderList.get(0).getConsumerInfo().getUserId())
.consumerName(orderList.get(0).getConsumerInfo().getName())
.build();
}
Copy link
Author

Choose a reason for hiding this comment

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

API 응답시 Service > Controller로 엔티티 대신 dto를 넘겨주기 위해 사용해 보았습니다. 특히 Order안에는 Consumer 정보가 있는데 꼭 필요한 컬럼값이 아니면 반환하지 않거나 반복하여 출력하지 않게 구현하였습니다.

Comment on lines 34 to 51
public class Order {

@Id
@Column(name = "order_no")
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long orderNo;

// EAGER 오류
// https://www.google.com/search?q=Type+definition+error%3A+%5Bsimple+type%2C+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor%5D%3B+nested+exception+is+com.fasterxml.jackson.databind.exc.InvalidDefinitionException%3A+No+serializer+found+for+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor+and+no+properties+discovered+to+create+BeanSerializer+(to+avoid+exception%2C+disable+SerializationFeature.FAIL_ON_EMPTY_BEANS)+(through+reference+chain%3A+com.flab.foodeats.common.response.ApiResponse%5B%5C%22data%5C%22%5D-%3Ecom.flab.foodeats.api.order.FindAllOrderResponse%5B%5C%22orderList%5C%22%5D-%3Ejava.util.ArrayList%5B0%5D-%3Ecom.flab.foodeats.domain.order.Order%5B%5C%22merchantInfo%5C%22%5D-%3Ecom.flab.foodeats.domain.user.Merchant%24HibernateProxy%24ukqfm0Y8%5B%5C%22hibernateLazyInitializer%5C%22%5D)&oq=Type+definition+error%3A+%5Bsimple+type%2C+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor%5D%3B+nested+exception+is+com.fasterxml.jackson.databind.exc.InvalidDefinitionException%3A+No+serializer+found+for+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor+and+no+properties+discovered+to+create+BeanSerializer+(to+avoid+exception%2C+disable+SerializationFeature.FAIL_ON_EMPTY_BEANS)+(through+reference+chain%3A+com.flab.foodeats.common.response.ApiResponse%5B%5C%22data%5C%22%5D-%3Ecom.flab.foodeats.api.order.FindAllOrderResponse%5B%5C%22orderList%5C%22%5D-%3Ejava.util.ArrayList%5B0%5D-%3Ecom.flab.foodeats.domain.order.Order%5B%5C%22merchantInfo%5C%22%5D-%3Ecom.flab.foodeats.domain.user.Merchant%24HibernateProxy%24ukqfm0Y8%5B%5C%22hibernateLazyInitializer%5C%22%5D)&aqs=chrome..69i57.3604j0j4&sourceid=chrome&ie=UTF-8


@ManyToOne(fetch = FetchType.EAGER)
@JoinColumn(name = "consumer_info")
private Consumer consumerInfo;

@ManyToOne(fetch = FetchType.EAGER)
@JoinColumn(name = "merchant_info")
private Merchant merchantInfo;
Copy link
Author

Choose a reason for hiding this comment

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

Order 엔티티 부분입니다. Merchant 대신 Shop을 매핑했어야 했는데 잘못 매핑하여, PR을 병합하기 전에 수정하여 적용할 수 있도록 하겠습니다.

또한 현재 fetch 전략이 즉시로딩으로 되어있습니다. 지연로딩 적용시 에러가 발생하여 구글링을 통해 즉시로딩을 적용하였는데 왜 즉시로딩을 해야하는지에 대한 이유는 이해하지 못했습니다. 이 부분도 좀 더 찾아보고 코멘트를 남길수 있도록 하겠습니다!

Comment on lines 53 to 75
@Embedded
@AttributeOverride(name = "menuNo", column = @Column(name = "first_munu_no", nullable = false))
@AttributeOverride(name = "menuName", column = @Column(name = "first_munu_name", nullable = false))
@AttributeOverride(name = "menuPrice", column = @Column(name = "first_munu_price", nullable = false))
@AttributeOverride(name = "menuOptionNo1", column = @Column(name = "first_option_no1"))
@AttributeOverride(name = "menuOptionName1", column = @Column(name = "first_option_name1"))
@AttributeOverride(name = "menuOptionPrice1", column = @Column(name = "first_option_price1"))
@AttributeOverride(name = "menuOptionNo2", column = @Column(name = "first_option_no2"))
@AttributeOverride(name = "menuOptionName2", column = @Column(name = "first_option_name2"))
@AttributeOverride(name = "menuOptionPrice2", column = @Column(name = "first_option_price2"))
private OrderMenu orderMenu;

@Embedded
@AttributeOverride(name = "menuNo", column = @Column(name = "second_munu_no"))
@AttributeOverride(name = "menuName", column = @Column(name = "second_munu_name"))
@AttributeOverride(name = "menuPrice", column = @Column(name = "second_munu_price"))
@AttributeOverride(name = "menuOptionNo1", column = @Column(name = "second_option_no1"))
@AttributeOverride(name = "menuOptionName1", column = @Column(name = "second_option_name1"))
@AttributeOverride(name = "menuOptionPrice1", column = @Column(name = "second_option_price1"))
@AttributeOverride(name = "menuOptionNo2", column = @Column(name = "second_option_no2"))
@AttributeOverride(name = "menuOptionName2", column = @Column(name = "second_option_name2"))
@AttributeOverride(name = "menuOptionPrice2", column = @Column(name = "second_option_price2"))
private OrderMenu orderMenu1;
Copy link
Author

Choose a reason for hiding this comment

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

Order 엔티티 안에 주문한 메뉴를 임베디드 타입으로 구현하였습니다. 현재는 2개의 컬럼만 만들었지만 주문시 메뉴의 갯수는 유동적이고 수량에 제한을 둘 수 밖에 없어 임베디드 타입을 구현하고 나서 좋은 방식이라고 생각이 들지는 않았습니다.

임베디드 방식대신 컬렉션타입(List)를 적용해 보거나 테이블을 더 만들어 매핑하는 방법으로 다시 리팩토링 할 수 있도록 하겠습니다.

Comment on lines 80 to 89
@Column(name = "order_status")
@ColumnDefault("true")
private String orderStatus;

// @DynamicInsert vs @PrePersist
// https://dotoridev.tistory.com/6
@PrePersist
public void prePersist() {
orderStatus = orderStatus == null ? "true" : orderStatus;
}
Copy link
Author

Choose a reason for hiding this comment

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

orderStatus에 default 값을 적용하기 위해 prePersist( ) 메소드를 사용하였습니다.
@DynamicInsert 보다는 @PrePersist를 사용하면 코드 로직을 조금더 파악하기 쉬울것 같아 @PrePersist를 사용하였습니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

prePersist에 로직을 넣는게 최선일까요??
이 부분을 Order가 직접 검사할 수는 없을까요??

Comment on lines 37 to 54
public class Order {

@Id
@Column(name = "order_no")
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long orderNo;

// EAGER 오류
// https://www.google.com/search?q=Type+definition+error%3A+%5Bsimple+type%2C+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor%5D%3B+nested+exception+is+com.fasterxml.jackson.databind.exc.InvalidDefinitionException%3A+No+serializer+found+for+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor+and+no+properties+discovered+to+create+BeanSerializer+(to+avoid+exception%2C+disable+SerializationFeature.FAIL_ON_EMPTY_BEANS)+(through+reference+chain%3A+com.flab.foodeats.common.response.ApiResponse%5B%5C%22data%5C%22%5D-%3Ecom.flab.foodeats.api.order.FindAllOrderResponse%5B%5C%22orderList%5C%22%5D-%3Ejava.util.ArrayList%5B0%5D-%3Ecom.flab.foodeats.domain.order.Order%5B%5C%22merchantInfo%5C%22%5D-%3Ecom.flab.foodeats.domain.user.Merchant%24HibernateProxy%24ukqfm0Y8%5B%5C%22hibernateLazyInitializer%5C%22%5D)&oq=Type+definition+error%3A+%5Bsimple+type%2C+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor%5D%3B+nested+exception+is+com.fasterxml.jackson.databind.exc.InvalidDefinitionException%3A+No+serializer+found+for+class+org.hibernate.proxy.pojo.bytebuddy.ByteBuddyInterceptor+and+no+properties+discovered+to+create+BeanSerializer+(to+avoid+exception%2C+disable+SerializationFeature.FAIL_ON_EMPTY_BEANS)+(through+reference+chain%3A+com.flab.foodeats.common.response.ApiResponse%5B%5C%22data%5C%22%5D-%3Ecom.flab.foodeats.api.order.FindAllOrderResponse%5B%5C%22orderList%5C%22%5D-%3Ejava.util.ArrayList%5B0%5D-%3Ecom.flab.foodeats.domain.order.Order%5B%5C%22merchantInfo%5C%22%5D-%3Ecom.flab.foodeats.domain.user.Merchant%24HibernateProxy%24ukqfm0Y8%5B%5C%22hibernateLazyInitializer%5C%22%5D)&aqs=chrome..69i57.3604j0j4&sourceid=chrome&ie=UTF-8


@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "consumer_info")
private Consumer consumerInfo;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "shop_no")
private Shop shopInfo;
Copy link
Author

Choose a reason for hiding this comment

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

Order 엔티티 매핑 Shop으로 수정했습니다! 또한 기존에 에러로 인해 즉시로딩을 적용했었는데 다시 지연로딩으로 적용하였습니다!

ManyToOne의 기본설정값은 FetchType.EAGER으로 알고 있습니다. 또한 Order에서 Shop정보와 User정보는 Response 값으로 항상 사용이 되므로 즉시로딩을 하는 것이 성능상 좋을수도 있다고 생각합니다. 이부분은 지연로딩을 적용해 놓고 멘토링 시간이후 다시 고민해 볼 수 있도록 하겠습니다

Comment on lines +57 to +58
@OneToMany(mappedBy = "order")
private List<OrderMenu> orderMenuList = new ArrayList<>();
Copy link
Author

Choose a reason for hiding this comment

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

Order 테이블에서 Menu를 어떻게 구성할까 고민이 많았습니다. OrderMenu와 OrderMenuOption 테이블을 구성하여 일대다 양방향 매핑으로 설계하였습니다

Comment on lines +35 to +54
public class OrderMenu {

@Id
@Column(name = "order_menu_no")
@GeneratedValue(strategy = GenerationType.IDENTITY)
private long menuOrderNo;

@Column(name = "menu_no")
private long menuNo;
@Column(name = "menu_name")
private String menuName;
@Column(name = "menu_price")
private int menuPrice;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "order_no")
private Order order;

@OneToMany(mappedBy = "orderMenu")
private List<OrderMenuOption> orderMenuOptionList = new ArrayList<>();
Copy link
Author

Choose a reason for hiding this comment

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

OrderMenu 부분입니다. OrderMenuOption과는 일대다 양방향 매핑을 구현하였습니다.

image

Comment on lines +20 to +25
@NotBlank(message = "Input menuNo ")
private long menuNo;
@NotBlank(message = "Input menuName ")
private String menuName;
@NotBlank(message = "Input menuPrice ")
private int menuPrice;
Copy link
Author

Choose a reason for hiding this comment

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

현재 Order엔티티에 메뉴는 Menu엔티티와 매핑되어 있는 것이 아닌, 직접 입력받아 저장할 수 있게 하였습니다.
image
그림처럼 직접 입력하는 것이 아닌 Order <> OrderMenu <> Menu를 매핑하여 테이블을 설계하려고 합니다.

f-lab-bright
f-lab-bright previously approved these changes Nov 2, 2021
Copy link
Collaborator

@f-lab-bright f-lab-bright left a comment

Choose a reason for hiding this comment

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

LGTM


@Id
@Column(name = "order_menu_option_no")
@GeneratedValue(strategy = GenerationType.IDENTITY)
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 전략에 따라서 실제 동작이 어떻게 다른지 한 번 찾아보셔도 좋을 것 같네요 :)

Comment on lines 80 to 89
@Column(name = "order_status")
@ColumnDefault("true")
private String orderStatus;

// @DynamicInsert vs @PrePersist
// https://dotoridev.tistory.com/6
@PrePersist
public void prePersist() {
orderStatus = orderStatus == null ? "true" : orderStatus;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

prePersist에 로직을 넣는게 최선일까요??
이 부분을 Order가 직접 검사할 수는 없을까요??

Comment on lines +28 to +46
public class OrderItem {

@Id
@Column(name = "order_item_no")
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long orderItemNo;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "order_no")
private Order order;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "shop_no")
private Shop shopInfo;


@Column(name = "total_price")
private int totalPrice;

Copy link
Author

Choose a reason for hiding this comment

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

image
image

위 사진을 참고하여 테이블 매핑을 새로 하였습니다. 다대다를 구성하기 보다는 일대다와 다대일로 구성하였습니다. 주문시 라이더 배차역시 위 형식으로 수행할 예정입니다.

Comment on lines 59 to 76
@Column(name = "order_time", nullable = false, updatable = false, insertable = false, columnDefinition = "TIMESTAMP DEFAULT CURRENT_TIMESTAMP")
public Timestamp orderTime;


@Column(name = "total_price")
private int totalPrice;

@Column(name = "order_status",nullable = false, insertable = false, columnDefinition = "boolean default true")
private Boolean orderStatus;



// @DynamicInsert vs @PrePersist
// https://dotoridev.tistory.com/6
// @PrePersist
// public void prePersist() {
// orderStatus = orderStatus == null ? "true" : orderStatus;
// }
Copy link
Author

Choose a reason for hiding this comment

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

PrePersist에 대한 멘토님의 피드백에 따라 다르게 생각해 보았습니다. 제가 default값을 설정하는 부분에 너무 복잡하게 생각했던것 같습니다. @column을 사용하여 PrePersist를 사용하지 않고 Default값을 설정해 보았습니다.

이 부분을 Order가 직접 검사하는 방식은 생성자에서 설정할 수 있다고 생각합니다.
이 부분은 @column을 사용하지 않고 생성자에서 디폴트 값을 설정할 수 있도록 하겠습니다.

Comment on lines 68 to 79
@Builder
public Order(Long orderNo, Consumer consumerInfo, Shop shopInfo,
List<OrderMenu> orderMenuList, Timestamp orderTime, int totalPrice, Boolean orderStatus) {

this.orderNo = orderNo;
this.consumerInfo = consumerInfo;
this.shopInfo = shopInfo;
this.orderMenuList = orderMenuList;
this.orderTime = orderTime;
this.totalPrice = totalPrice;
this.orderStatus = orderStatus == null ? true : orderStatus;
}
Copy link
Author

Choose a reason for hiding this comment

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

멘토님께서 피드백해주신 것을 보고 orderStatus의 디폴트값 설정 방식을 PrePersist나 @column을 사용하지 않고 생성자에 로직을 구현하여 보았습니다.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants