Skip to content

[5기 송인재, 김현우] 스프링부트 JPA 위클리 미션 제출 #337

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

Open
wants to merge 27 commits into
base: injae-hyeonwoo
Choose a base branch
from

Conversation

ASak1104
Copy link

@ASak1104 ASak1104 commented Nov 20, 2023

📌 과제 설명

  • 구피의 스프링부트 Part4 강의 실습
  • 이번 코드리뷰가 이루어지는 과정
    • 백둥이끼리 코드리뷰를 진행합니다.
    • 백둥이끼리의 코드리뷰가 마치면, 멘토님에게 2차 리뷰를 요청합니다.

👩‍💻 요구 사항과 구현 내용

미션1 : 2. JPA 소개(단일 엔티티를 이용한 CRUD를 구현)

  • SpringBoot Part4 2. JPA 소개
    • JPA 프로젝트를 세팅해본다.
    • 세팅한 프로젝트를 이용해서 단일 엔티티를 이용한 CRUD를 구현한다.
      • 고객(Customer) 엔티티는 ID(PK), 이름, 성을 가진다.
      • 고객엔티티를 이용한 CRUD를 구현한다.

미션2 : 3. 영속성컨텍스트(customer 엔티티를 이용하여 생명주기 실습)

미션3 : 4-2. 연관관계매핑(order, order_item, item의 연관관계 매핑 실습)

✅ PR 포인트 & 궁금한 점

@ASak1104 ASak1104 changed the title [] [5기 송인재, 김현우] 스프링부트 JPA 위클리 미션 제출 Nov 20, 2023
Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 left a comment

Choose a reason for hiding this comment

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

전체적으로 테스트 코드 부분에서 많이 고민하신 부분이 보여서 정말 많이 배워간 것 같습니다!👍 남긴 코멘트가 거의 개인적인 궁금증으로 채워진 것 같아서 죄송하기도 합니다요... 허허😶‍🌫️

import org.springframework.transaction.annotation.Transactional;

@DataJpaTest
@Transactional(propagation = Propagation.NEVER)
Copy link
Member

Choose a reason for hiding this comment

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

<개인적인 궁금증!>
@transactional 전파 설정을 never로 하면 non-trasaction으로 진행되는 것으로 알고있습니다!
@transactional(propagation = Propagation.NEVER)와 @DataJpaTest과 같이 써준 이유는

  1. @DataJpaTest 사용해보고 싶어서
  2. @DataJpaTest에는 기본 @transactional 설정이 있는데 각 테스트 케이스에 Transaction 사용을 하고 싶지 않아서
    위와 같은 두 이유로 설정한 것인지, 따로 이유가 있어서 그런 것인지 개인적으로 궁금합니당!

Copy link

Choose a reason for hiding this comment

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

현우님과 별개로 제 의견으로는,
말씀하신대로 저희가 테스트 시 행하는 transaction들의 의도와 다른 작동이 날 수도 있으니 Transactional을 적용하지 않고 싶었고 이번 과제에서 DataJpaTest를 사용해야만 하는 아주 큰 이유는 없어서 완벽하게 잘 사용했다고는 못 하겠네요. 말씀대로 DataJpaTest 사용해보고 싶어서도 컸다고 생각합니다.

DataJpaTest를 이번 과제와 게시판 과제를 하면서 개인적으로 많이 찾아봤는데 Jpa를 사용하면 테스트 시 매우 잘 어울리는 어노테이션이라고 생각합니다. Transactional을 테스트마다 걸어줄 수 있는 것을 제외하고도요.

  1. 등록된 빈들을 SpringBootTest처럼 가져올 수 있다.
  2. persistAndFlush(), persistFlushFind()같은 테스트용 메서드를 제공해주는 TestEntityManager를 사용할 수 있다.
  3. DataSource를 별개로 설정할 수 있다.
  4. 알아서 create-drop을 한다.

등등이 있어서 신기했습니다

Copy link
Member

Choose a reason for hiding this comment

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

위에서 말씀해주신 4가지 특징은 저도 새로 알았네요! 앞으로 저도 사용할 때 참고해보겠습니다!👍

Copy link

Choose a reason for hiding this comment

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

아 깜빡했는데, @Transactional이 들어가면 transaction.begin()이 안됩니다!

Copy link
Author

Choose a reason for hiding this comment

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

DataJpaTest 어노테이션을 사용한 이유는 DataSource, JdbcTemplate, EntityManagerFactory 등의 빈을 사용하기 위해서 입니다. 요구사항이 entity가 PersistencyContext 에서 어떻게 관리되는 것인지 확인하는 것이기 때문에 SpringBootTest를 통해 전체 빈을 가져오는 것보다는 DataJpaTest를 통해 JPA 설정과 Bean만 가져오는 방식으로 진행해봤습니다.

말씀해주신 것처럼 transaction 어노테이션이 기본으로 설정되어 있기 때문에 인재님께서 설명해주신 "테스트 시 행하는 transaction들의 의도와 다른 작동이 날 수도 있으니 Transactional을 적용하지 않고" 싶어서 추가적으로 설정해봤습니다.

Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 Dec 1, 2023

Choose a reason for hiding this comment

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

아 깜빡했는데, @transactional이 들어가면 transaction.begin()이 안됩니다!

혹시 이 의미가 에러가 난다는 것인가요? 아니면 transaction.begin()이 있지만 있으나 없으나 결과는 동일하다는 뜻인가요!


transaction.commit();

entityManager.close();
Copy link
Member

Choose a reason for hiding this comment

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

제가 과제할 때는 테스트가 끝나면 entityManager.close(); 가 자동으로 될테니(?) 설정을 안해준 것도 있었는데 꼼꼼하게 다들 잘해놓으셨군요! 저도 다음부터는 조심해야겠습니다....

entityManager.close(); 의 기능

  • Transaction 수행 후에는 반드시 EntityManager 를 닫는다.
  • 그래야 내부적으로 DB Connection 을 반환한다.

Copy link
Author

@ASak1104 ASak1104 Nov 30, 2023

Choose a reason for hiding this comment

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

이번 미션에서는 강의에서 다루는 포멧으로 EntityManagerFactory를 사용했기 때문에 해당 cleanup 메서드가 필요했지만 사실 TestEntityManager를 사용하면 DB 초기화와 close를 위한 메서드를 추가로 선언할 필요가 없어서 한 번 사용해 보시는 걸 추천드립니다!

해당 코드를 참고하시면 이해가 편하실 것 같아요!

@@ -0,0 +1,14 @@
spring:
datasource:
driver-class-name: org.h2.Driver
Copy link
Member

Choose a reason for hiding this comment

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

오 저는 저번 과제부터 h2는 안써보고 mysql docker로 돌리고 있는데 혹시 체감되는 차이점이 있을까요...? 없으시다면 넘기셔도 됩니당!!😁

Copy link

Choose a reason for hiding this comment

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

h2의 장점으로는 도커를 안 켜도 됩니다. 기타 번거로운 설정도 필요 없었고요! h2가 별개로 있으시면 강의처럼 web을 켜서 바로 확인도 가능합니다. 그 외의 장점은 잘 모르겠습니다.

Copy link
Author

Choose a reason for hiding this comment

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

아무런 초기 설정 없이 프로젝트를 클론 받자 마자 테스트를 돌릴 수 있다?
저는 이 부분이 참 좋은 것 같아요!


transaction.commit();

ThrowingCallable findCustomer = () -> entityManager.find(Customer.class, customer.getId());
Copy link
Member

Choose a reason for hiding this comment

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

오 신기하네요!! 하나 배워갑니다,,,,!!!👍ThrowingCallable 사용해서 assert 하는 부분이 가독성이 좋은 것 같네요!

Copy link

Choose a reason for hiding this comment

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

저도 이번에 배웠습니다..👍

private int getCustomerCount() {
transaction.begin();

int count = entityManager.createNativeQuery(CUSTOMER_COUNT)
Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 Nov 23, 2023

Choose a reason for hiding this comment

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

여기 쿼리(CUSTOMER_COUNT)에서는 조회만 하고 있는데 혹시 위,아래 트랜잭션 설정(begin, commit)을 안 넣어도 괜찮을까요? 저도 잘 몰라서 여쭤봅니당!🥲

Copy link

Choose a reason for hiding this comment

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

엔티티 매니저가 열려있는 Session이 없다고 예외를 반환하지 않을까 싶네요!

Copy link
Author

Choose a reason for hiding this comment

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

해당 코드를 작성할 때 인재님 말씀처럼 entityManager가 없을 수도 있다고 생각해서 방어적인 역할로서 작성했습니다!
사실 세희님 말씀처럼 해당 트랜잭션 코드가 없어도 잘 작동하기 때문에 수정하는 것이 더 좋을 것 같네요!

Customer actualCustomer = optionalCustomer.get();

assertThat(actualCustomer).hasNoNullFieldsOrPropertiesExcept("orders");
assertThat(actualCustomer).usingRecursiveComparison()
Copy link
Member

Choose a reason for hiding this comment

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

오오,,, 굉장히 assertj를 잘 활용하시는 것 같습니다!!👍

import org.hibernate.annotations.UpdateTimestamp;

@MappedSuperclass
public class BaseEntity {

Choose a reason for hiding this comment

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

@EntityListeners(AuditingEntityListener.class)

이게 없어도 될까요?

Copy link

Choose a reason for hiding this comment

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

@CreatedDate와 @LastModifiedDate는 Spring Data JPA에서 제공해주는 어노테이션입니다. 말씀대로 둘을 사용할 때는 엔티티 리스너를 추가하고 @EnableJpaAuditing을 추가해주어야 정상적으로 작동합니다!

@CreationTimestamp와 @UpdateTimestamp는 hibernate에서 제공해주는 어노테이션입니다. hibernate에서 제공해주기 때문에 jpa에서 제공하는 위의 두 어노테이션과는 달리 추가가 필요 없습니다.

저희가 hibernate에서 제공하는 @CreationTimestamp와 @UpdateTimestamp는 사용한 이유는 jpa에서 제공하는 @CreatedDate와 @LastModifiedDate를 사용할 때 윈도우의 LocalDateTime precision이 DB보다 더 구체적이었기 때문입니다.

예를 들어, DB에서의 타입이 datetime(6)로 설정되어 있으면 flush 하기 전 LocalDateTime은 HH:MM:SS.000000000가 나오지만 DB에서 가져올 때에는 HH:MM:SS.000000가 나옵니다. DB에 저장되면서 뒤의 세자리는 반올림 되어 들어갑니다. 작은 차이지만 테스트를 통과하지 못하고 반올림 되면서 날짜가 하루 늘어날 가능성도 있다고 생각을 했습니다.

열심히 구글링을 해보니 hibernate에 종속적인 어노테이션들보다 jpa에서 제공하는 어노테이션을 많이 사용한다기에 처음엔 코드를 이리 저리 추가해보며 이를 해결하려고 했습니다. 고민 끝에 코드를 추가하는 것보다 @CreationTimestamp와 @UpdateTimestamp를 사용했을 때 문제가 없으니 이대로 사용하기로 결정했습니다! jpa의 주요 구현체가 hibernate가 아닐 것이라 상상이 잘 안되고, 저희가 hibernate의 어노테이션을 쓰며 아직 불편을 느끼지 못해 사용하는 점도 있는 것 같습니다.

저희는 미처 생각을 못 했지만 다른 팀을 보니 @jsonformat으로 해결한 팀도 있었던 것 같습니다! 저희도 추후에 @CreatedDate와 @LastModifiedAt을 사용한다면 @jsonformat을 사용하는 방법이 좋을 것 같다고 생각합니다!!

@CreationTimestamp
private LocalDateTime createdAt;
@Column(name = "updated_at", nullable = false)
@UpdateTimestamp

Choose a reason for hiding this comment

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

@LastModifiedDate
이걸 안쓴 이유가 있나요?

Comment on lines +26 to +28
@Id
@GeneratedValue(strategy = GenerationType.UUID)
private UUID id;

Choose a reason for hiding this comment

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

(질문) AutoIncrement를 사용하지 않고 UUID를 사용한 이유가 있나요?

Copy link

Choose a reason for hiding this comment

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

큰 이유는 없습니다. UUID를 사용하면 자동으로 잘 생성되어 저장될까 궁금하기도 했고, MySQL에 UUID를 사용할 때 binary로 타입을 지정하는 경우도 있고 문자열로 지정하는 경우도 있는데 UUID를 생성 전략으로 하면 어떻게 저장될까도 궁금했습니다.

결론적으로, 강의에서 auto increment를 다뤘으니 UUID를 사용해보고 싶어서 사용했던 것 같습니다!

Comment on lines +29 to +32
@Column(name = "first_name", nullable = false, length = 15)
private String firstName;
@Column(name = "last_name", nullable = false, length = 15)
private String lastName;

Choose a reason for hiding this comment

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

Name이라는것을 VO로 한번 묶어보면 어떨까요?

@Transactional(propagation = Propagation.NOT_SUPPORTED)
class CustomerTest {

static final String DELETE_FROM_CUSTOMER = "DELETE FROM customer";

Choose a reason for hiding this comment

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

@transactional의 롤백을 안하는 이유가 있나요?

Copy link

Choose a reason for hiding this comment

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

@DataJpaTest를 사용해보고 싶어 사용했습니다. 그리고 트랜잭션을 직접 제어해서 영속성 컨텍스트의 여러 케이스를 테스트 해보고 싶었는데, @DataJpaTest에 붙어있는 @transaction 때문에 transaction.begin()시 예외가 raise되어서 NOT_SUPPORTED로 바꿨습니다.

테스트마다 개별적으로 여러 케이스의 트랜잭션을 테스트 해보며 커밋 할 필요가 있어서 @AfterEach로 DB의 데이터들을 지우는 방향으로 선택했습니다!

Copy link
Author

@ASak1104 ASak1104 Nov 30, 2023

Choose a reason for hiding this comment

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

제가 조금 첨언을 하자면 위클리 미션의 주제가 컨텍스트를 통한 엔티티의 생명주기를 실습하는 것이기 때문에 JpaRepository를 사용하면 단순한 CRUD 확인 밖에 진행할 수 없다고 생각해서 실제 트렌젝션을 열고 닫으면서 entity의 state에 따른 차이를 확인하고자 transactional 롤백을 진행하지 않았습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants