-
Notifications
You must be signed in to change notification settings - Fork 151
[5기 이세희] Springboot-jpa weekly 미션1, 2, 3 제출합니다. #338
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
base: SeheeLee01
Are you sure you want to change the base?
[5기 이세희] Springboot-jpa weekly 미션1, 2, 3 제출합니다. #338
Conversation
testCompileOnly 'org.projectlombok:lombok' | ||
testAnnotationProcessor 'org.projectlombok:lombok' |
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.
compileOnly와 별개로 testCompileOnly를 또 넣어주시는 이유가 궁금합니다!! compileOnly만으로는 테스트 시 문제가 생기나요?
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.
|
||
@Bean | ||
public JpaVendorAdapter jpaVendorAdapter(JpaProperties jpaProperties) { | ||
AbstractJpaVendorAdapter adapter = new HibernateJpaVendorAdapter(); |
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.
HibernateJpaVendorAdapter
나 JpaVendorAdapter
가 아닌 AbstractJpaVendorAdapter
를 사용하신 이유가 있으신지 궁금합니다!
@Column(name = "created_by") | ||
private String createdBy; | ||
|
||
@Column(name = "created_at", columnDefinition = "TIMESTAMP") |
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.
개인적으로 궁금한건데.. columnDefinition="TIMESTAMP"
가 없다면 이상이 생길까요??
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.
제가 너무 늦게 답변을 드려서 죄송합니다! ㅠㅠㅠ
저번에 의진님 코드리뷰 통해서 의진님이 알려주신 것 같아서 이 부분은 넘어가겠습니다!
저도 놓친 부분이었는데 물어봐주셔서 감사합니다!!!
|
||
@Test | ||
@DisplayName("고객을 저장하고 조회할 수 있다.") | ||
void insertTest() { |
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.
저도 처음에 이런 식으로 했다가 고민이 됐던 부분이 있고 쓸데없는 고민이다 생각하실 수도 있지만 개인적으로 의견이 궁금합니다.
테스트 대상의 데이터를 테스트 클래스 멤버 변수로 선언하고 setUp에서 공통적으로 생성하셨습니다. customer의 생성이 중복 코드이니 setUp에서 미리 생성을 해주시고 given이 이미 생성이 돼있으니 생성 성공 테스트에서 given을 스킵하셨는데요,
어떻게 보면 개별적인 테스트들이 필요한 given 데이터를 전처리 과정에서 공통적으로 생성하고 테스트 내에선 명시를 넘어가는 것이 바람직할까 하는 생각도 듭니다.
이런 경우 개발자 편의와 생산성을 위해 감안하는 것 vs 그래도 개별적인 테스트 내에서 명시를 해주는 것 중 무엇이 괜찮다고 생각하시나요?
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.
저는 작성 당시에는 테스트의 양도 적고, 해당 테스트 메서드(insertTest) 뿐만아니라 다른 테스트 메서드에도 해당 코드가 필요하기 때문에 한 번에 묶어서 처리를 해주었습니다!
생각해보니 given을 안 써주면 나중에 다른 개발자가 이 insertTest를 보았을 때 given으로 주어진 상황이 없겠다라고 착각을 할 수 있는 여지가 있다고 생각해요! (물론 위로 스크롤을 해서 확인한다면 다행이겠지만요!)
- 테스트 코드가 길다면 중복이 약간은 생기더라도 명시해준다.
- 짧다면 주석을 남기거나 위의 setUp 코드에 //given 정보가 있다는 것을 명시하는 방법을 찾는다.
저는 이 기준으로 생각해볼 것 같습니다! 한 번 더 생각하게 해주셔서 감사합니다! 👍
transaction.begin(); | ||
persistNewOrderWithoutMemberAndOrderItem(); | ||
persistNewMemberWithoutOrder(); | ||
order.setMember(member); // 주문 -> 회원만 보기 가능한 상태. 직접 만든 setMember가 아니라 기본 setter로 해야합니당. |
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.
소스 코드에 변경이 필요 없이 setMember가 두 개가 있고 그 중 하나를 선택할 수 있는건가요??
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.
학습용으로 진행하다보니 소스 코드에 변경을 통해서 테스트를 진행했던 것 같아요! 이 부분은 다른 메서드로 할 수 있도록 구현해놓겠습니다!
log.info("{} and {}", order.getMember().getOrders().size(), entityManager.contains(order)); // order -> 준영속 상태 | ||
// assertThat(member.getOrders().size()).isEqualTo(0); // member에는 등록되지 않았다. | ||
log.info("{}", retrievedOrder.getMember().getOrders().size()); // 그래프 탐색 + commit 된 후 불러온 것에는 자동으로 되어있다. | ||
log.info("{}", retrievedMember.getOrders().size()); // 그래프 탐색 + commit 된 후 불러온 것에는 자동으로 되어있다. |
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.
무슨 말인가 했더니 order를 생성할 때 member의 orders라는 리스트는 건든 것이 없으니 변경된 것이 없지만, 엔티티 매니저를 통해 동등한 order와 member를 쿼리하면 해당 member의 orders에 추가된 것을 의도하신 것이었군요.
열심히 공부하신 것 같습니다!
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.
아니 제가 강의보면서 실습하면서 남겨놓다가 log로 거의 해놓았네요... 된다면 빠른 시일 내에 assertJ를 이용할 수 있도록 바꿔보겠습니다ㅠㅠㅠㅠ
보시기 어려웠을텐데 꼼곰하게 리뷰해주셔서 감사합니다!
transaction.commit(); | ||
log.info("COMMIT & FLUSH!"); | ||
|
||
// 사실상 "커밋 전"에 영속 상태이면 트랜잭션 안이 아니라도 커밋된다. |
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.
트랜잭션이 열린 것이 없는데 커밋되고 쿼리가 날아가는 건가요??!? 신기하네요
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.
앗 정확히 말하자면 [한 트랜잭션이 시작되고 끝]이라는 과정 전에 영속 상태일 때를 이야기 한 것입니다!! 주석을 좀 헷갈리게 달았던 것 같아요!!
쿼리는 transaction.commit(); 때 날아갑니다!!!!
log.info("[retrievedCustomer2] {} {}", retrievedCustomer2.getFirstName(), retrievedCustomer2.getLastName()); // kang sehee | ||
|
||
entityManager.detach(customer); // 1차 캐시에서 조회 못하도록 준영속 상태 | ||
log.info("[Does customer managed at persist context?] {}", entityManager.contains(customer)); |
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.
assertThat으로 정답을 알려주시면 어떨까요? 저도 궁금합니다..
log.info("[retrivedCustomer] {} {}", retrievedCustomer.getFirstName(), retrievedCustomer.getLastName()); // Lee sehee | ||
|
||
entityManager.flush(); // TransactionRequiredException: 트랜잭션이 시작 안되면 발생 | ||
log.info("FLUSH!"); // 쿼리 출력(저장은 안되고 DB 동기화가 일어나는 것 같다. DB에 안보임.) |
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.
정말 flush만 하면 엔티티 매니저가 받아올 수 있고 hibernate도 insert statement를 출력하지만 DB에 실제로 반영되어 있지는 않네요!!
그럼 flush는 변경 사항에 대한 prepared statement를 작성하는 작업일 수도 있겠네요?!? 덕분에 알아갑니다!!
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.
맞습니다! 저도 테스트 하면서 너무 이상해서 찾아보니 뭔가 DB에 완전히 데이터가 반영되기 전에 DB단에서 동기화 작업(변경사항 저장 등)을 해주는 것 같더라구요! 정확한 정보는 나와있지가 않아서 DB 구조를 좀 살펴보던가 해야될 것 같아요!
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.
열심히 공부하셨네요! 세희님이 공부하신 내용을 엿볼 수 있었던 것 같고 생각 못 해봤던 점도 있어 재미있었습니다!
위클리 미션이 학습 목표의 실습인 느낌이 강해서인지 주석이 많고 테스트에서 assert 대신 주석으로 쓰신 경우가 많았는데, assert로 결과를 보여주시면 리뷰가 더 수월할 것 같습니다!
고생 많으셨습니다!!
|
||
@Configuration | ||
@EnableJpaRepositories("com.sehee.weeklyjpa.domain") | ||
public class DataSourceConfig { |
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.
yml로 설정하지 않은 이유가 있나요?
private String lastName; | ||
private String firstName; |
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.
Name을 VO로 만들어 보면 어떨까요?
|
||
@Entity | ||
@Table(name = "customers") | ||
public class Customer extends BaseEntity { |
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.
(질문) 여긴 lombook을 안쓴 다른 이유가 있나요?
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
@Slf4j | ||
@SpringBootTest |
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.
@DataJpaTest를 안쓰는 이유가 있나요?
컨텍스트를 작게 유지하면 좋을거 같아요.
Order retrievedOrder = entityManager.find(Order.class, order.getUuid()); | ||
assertThat(retrievedOrder.getUuid()).isEqualTo(order.getUuid()); | ||
|
||
log.info("{} and {}", order.getMember().getOrders().size(), entityManager.contains(order)); // order -> 준영속 상태 |
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.
로그를 이렇게 다 찍어주신 이유가 무엇인가요?
📌 과제 설명
SpringBoot JPA 1, 2, 3 주차 미션 구현내용입니다!
👩💻 요구 사항과 구현 내용
src/test/java/com/sehee/weeklyjpa/CustomerRepositoryTest.java
src/test/java/com/sehee/weeklyjpa/PersistenceContextTest.java
,src/test/java/com/sehee/weeklyjpa/OrderPersistenceTest.java
src/main/java/com/sehee/weeklyjpa/domain/order
✅ PR 포인트 & 궁금한 점