[4기 박상민] Springboot-jpa weekly 미션 3 PR입니다. #334

smartandhandsome · 2023-08-05T03:25:24Z

📌 과제 설명

미션2 : 3. 영속성컨텍스트(customer 엔티티를 이용하여 생명주기 실습)
미션3 : 4-2. 연관관계매핑(order, order_item, item의 연관관계 매핑 실습)

강의 중 미션에 대해 매핑만 주어줬는데,
다음 PR 때는 Controller, Service를 구현해 보고 싶습니다.

commit 이 좀 꼬였습니다.. 죄송합니다.. git 좀 더 공부하겠습니다.

✅ 피드백 반영사항

#312 (comment)
에러메세지 출력이 생각한 대로 되지 않아 수정하였습니다.

…ice로 변경

src/main/java/org/programmers/jpaweeklymission/global/exception/GlobalExceptionHandler.java

src/main/java/org/programmers/jpaweeklymission/item/domain/Item.java

dojinyou · 2023-08-05T12:41:50Z

src/main/java/org/programmers/jpaweeklymission/order/domain/Order.java

+
+    public void setCustomer(Customer customer) {
+        if (Objects.nonNull(this.customer)) {
+            customer.removeOrder(this);


이게 로직이 맞나요?

아 저부분 여쭤보려고 주석을 달았어야 했는데 깜빡했습니다.

일단 강사님께서 강의중에 저런 로직을 작성하셨습니다.

근데 그 부분에 좀 의문이 들었는데 사실 setCustomer 라는 건 제가 생각하기로는 초기에 주문(Order)에 고객(Customer)라는 걸 등록하는 거라고 생각했습니다.

근데 저걸 적으신 강사님이 의도한 건 아래 두 개라고 생각했습니다.

초기 주문 작성 시 미리 등록된 Customer가 있는지 한 번 더 체크하고 무시하기 위해서?

주문에 할당된 Customer를 수정하기 위해서?

다른 부분 Item, OrderItem 부분에도 nonNull 체크를 전부 적용하셨는데 이게 필요한가요?, 좋은 방법인가요? 멘토님들 의견 궁금합니다.

해당 로직은 상품이 생성될 때 구매자가 중복 등록되는 것을 방지하는 코드가 맞는 것 같습니다.

if(Objects.nonNull(this.customer))는 현재 주문에 구매자가 등록되어 있는지 확인하는 구문이네요. 그런데 customer.removeOrder(this)는 인자로 넘어온 구매자와 주문간 관계를 끊어주는 것 같습니다. 이 부분이 조금 어색한 것 같아요.

아래에서 this.customer = customer로 기존 구매자를 새롭게 인자로 넘어온 구매자로 대체하는데, 그렇다면 기존 구매자와 주문간 관계를 끊어주어야 하는 것이 아닐까요?

양방향 매핑이 조금 복잡하다면 단방향 매핑으로 단순하게 시작하는 것도 좋을 것 같네요

제가 잘 이해한 건지 모르겠는데 "그렇다면 기존 구매자와 주문간 관계를 끊어주어야 하는 것이 아닐까요?" 이 부분을 customer.removeOrder(this)라는 문장으로 처리합니다.

dojinyou · 2023-08-05T12:43:40Z

src/main/java/org/programmers/jpaweeklymission/order/domain/Order.java

+    }
+
+    public void addOrderItems(OrderItem orderItem) {
+        orderItem.setOrder(this);


현재 orderItems에는 추가되는 orderItem을 add 해주지 않아도 되나요?

setOrder 메서드가 아래처럼 작성 되어 있습니다.

this.order = order;
order.getOrderItems().add(this);

orderItem이 주도적으로 제어하는 이유가 있나요?

누가 하던지 상관없다고 생각하는데, 키를 갖고 있고 수정 시 쿼리가 나가는 OrderItem 테이블에서 제어해주는게 좋다고 생각했습니다.

또, 양방향 매핑 시에 저런 메서드가 계속적으로 생겨나는데, 키를 갖고 있는 쪽에 저런 메서드를 정의하자라고 기준을 정헀습니다.

src/test/java/org/programmers/jpaweeklymission/PersistenceContextTest.java

src/main/java/org/programmers/jpaweeklymission/global/exception/GlobalExceptionHandler.java

seung-hun-h · 2023-08-06T01:44:16Z

src/main/java/org/programmers/jpaweeklymission/order/domain/Order.java

+    @Enumerated(EnumType.STRING)
+    @Column(name = "status", nullable = false)
+    @NotNull
+    OrderStatus status;


접근제어자를 지정하지 않은 이유가 있나요?

깜빡했습니다...

수정했습니다. 9731909

seung-hun-h · 2023-08-07T07:43:34Z

...java/org/programmers/jpaweeklymission/customer/presentation/dto/CustomerCreationRequest.java

 import jakarta.validation.constraints.Size;

 public record CustomerCreationRequest(
+        @NotBlank(message = "First name must not be blank.")


min 이 지정되어 있는데 not blank도 검증 해주어야 하나요?

생각해보니 " " 같은 부분이 있기 때문에 다시 설정했습니다.

seung-hun-h · 2023-08-07T07:44:04Z

src/main/java/org/programmers/jpaweeklymission/global/exception/GlobalExceptionHandler.java

+        return ErrorResponse.newErrorResponse(e
+                .getBindingResult()
+                .getFieldErrors()
+                .get(0)


첫 번째 에러만 반환하는 이유가 있나요?

첫 번째 에러에 제가 validation 규칙에서 적용한 message를 담고 있어서 첫번째 에러만 반환했습니다.

뭔가 코드가 찜찜하긴 한데 혹시 다른 방법이 있나요?

seung-hun-h · 2023-08-07T07:44:29Z

src/main/java/org/programmers/jpaweeklymission/order/domain/Order.java

+public class Order extends BaseEntity {
+    @Id
+    @Column(name = "id")
+    @GeneratedValue(strategy = GenerationType.AUTO)


따로 id 생성 전략을 지정하지 않는 이유가 있나요?

seung-hun-h · 2023-08-07T07:51:43Z

src/main/java/org/programmers/jpaweeklymission/order/domain/Order.java

+
+    public void setCustomer(Customer customer) {
+        if (Objects.nonNull(this.customer)) {
+            customer.removeOrder(this);


해당 로직은 상품이 생성될 때 구매자가 중복 등록되는 것을 방지하는 코드가 맞는 것 같습니다.

if(Objects.nonNull(this.customer))는 현재 주문에 구매자가 등록되어 있는지 확인하는 구문이네요. 그런데 customer.removeOrder(this)는 인자로 넘어온 구매자와 주문간 관계를 끊어주는 것 같습니다. 이 부분이 조금 어색한 것 같아요.

아래에서 this.customer = customer로 기존 구매자를 새롭게 인자로 넘어온 구매자로 대체하는데, 그렇다면 기존 구매자와 주문간 관계를 끊어주어야 하는 것이 아닐까요?

양방향 매핑이 조금 복잡하다면 단방향 매핑으로 단순하게 시작하는 것도 좋을 것 같네요

dojinyou

승훈님 코멘트만 마저 반영해주세요

smartandhandsome added 25 commits July 29, 2023 21:54

2주차 영속성컨텍스트 실습

a5fa74c

feat: orders 추가

7edb8d9

feat: Item 구현

9249721

feat: Order 도메인 구현

24f01c4

feat: merge

6c34145

feat: CustomerUpdateRequest 객체를 Customer로 변환하는 메서드 생성

cadaf52

feat: 새로 만든 changeCustomer 메서드 사용

58b3a66

feat: 400 500 상태코드에 body 추가

e098e33

test: Customer 테스트 작성

963a060

test: Customer Repository 작성

1e97000

feat:

7a7fb07

test: BeforeEach로 데이터 생성 후 1차 캐시 비움

100445c

refactor: import 시 asterisk 제거하고 import 항목 명시

236d1ce

refactor: class와 import 사이 개행 추가

c3b0976

feat: 추상클래스로 변경

a5e4d55

test: 각 field를 반환하여 비교한 것을 usingRecursiveComparison으로 변경

0131918

feat: ErrorResponse 생성 및 GlobalExceptionHandler 적용

f9e5cee

refactor: import 시 asterisk 제거 및 질문 삭제

beee729

feat: ErrorResponse에 Getter 추가, ControllerAdvice -> RestControllerAdv…

5c9bd47

…ice로 변경

feat: message 추가

84fc7b2

refactor: Error Message 변경, ExceptionHandler 분리, 질문 추가

33a3c88

chore

da38737

chore: 질문 삭제

bd8314a

feat: CANCELE 추가

8264c08

test: main 코드 변경에 따른 test 수정

6b88225

smartandhandsome requested a review from seung-hun-h August 5, 2023 03:25

smartandhandsome and others added 2 commits August 5, 2023 12:26

Merge branch 'smart-sangmin' into week3

45c7980

feat: 에러메세지 수정

e4853c3

smartandhandsome requested a review from dojinyou August 5, 2023 03:40

smartandhandsome self-assigned this Aug 5, 2023

smartandhandsome added 🔥 매운맛 Team-Seunghoon labels Aug 5, 2023

dojinyou reviewed Aug 5, 2023

View reviewed changes

seung-hun-h reviewed Aug 6, 2023

View reviewed changes

smartandhandsome added 5 commits August 6, 2023 18:43

feat: import asterisk 제거

a2660f5

chore: conflict 해결

8af6a32

chore: 질문 삭제

6cc2537

feat: NotBlank 추가

c7d6887

feat: private 접근제어자 설정

9731909

seung-hun-h reviewed Aug 7, 2023

View reviewed changes

dojinyou approved these changes Aug 8, 2023

View reviewed changes

smartandhandsome merged commit 874f374 into prgrms-be-devcourse:smart-sangmin Nov 29, 2024

[4기 박상민] Springboot-jpa weekly 미션 3 PR입니다. #334

[4기 박상민] Springboot-jpa weekly 미션 3 PR입니다. #334

Uh oh!

Conversation

smartandhandsome commented Aug 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 과제 설명

✅ 피드백 반영사항

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smartandhandsome Aug 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smartandhandsome Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dojinyou left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

smartandhandsome commented Aug 5, 2023 •

edited

Loading

smartandhandsome Aug 6, 2023 •

edited

Loading

smartandhandsome Aug 8, 2023 •

edited

Loading