Skip to content
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

[4기 윤영운, 김별 ] Springboot-jpa weekly 미션 제출합니다. #324

Open
wants to merge 10 commits into
base: 별,영운-misson
Choose a base branch
from

Conversation

byeolhaha
Copy link
Member

📌 과제 설명

  • 페어프로그래밍

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

  • 세팅한 프로젝트를 이용해서 단일 엔티티를 이용한 CRUD를 구현한다.
  • 고객(Customer) 엔티티는 ID(PK), 이름, 성을 가진다.
  • 고객엔티티를 이용한 CRUD를 구현한다.
  • customer 엔티티를 이용하여 영속성컨텍스트의 엔티티 생명주기를 실습해본다.
  • order, order_item, item 의 연관관계 매핑을 실습해본다.

✅ 피드백 반영사항

  • JPA에서 엔티티 아이디 생성 전략을 AUTO로 하였으나 이는 id 생성을 위해 hibernate_sequence 테이블의 시퀀스 값을 가져와서 사용하여 성능이 좋지 않다고 하셔서 데이터베이스에 id 생성을 위임하는 IDENTITY 전략을 사용하였습니다.
  • JPA만을 테스트하는 경우 @SpringbootTest보다 @DataJpaTest가 빈을 덜 생성하는 방식이라고 하셔서 @DataJpaTest을 적용하였습니다.

id 'io.spring.dependency-management' version '1.1.2'
}

group = 'com.example'
Copy link

Choose a reason for hiding this comment

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

group 이름은
일반적으로 작성하는 회사의 도메인 명을 거꾸로 사용해요
ex) naver.com -> group = com.naver
네이밍 룰은 Package 네이밍 룰을 따르고 하위 값을 추가할 수 있는데, 보통 프로젝트 이름을 쓰곤 합니다.


@Configuration
public class DataSourceConfig {

Copy link

Choose a reason for hiding this comment

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

SpringBoot AutoConfiguration이 제공해주는 Bean들을 사용하지 않고
직접 등록하신 의도가 궁금합니다

Choose a reason for hiding this comment

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

저도 궁금

Copy link

@young970 young970 Aug 24, 2023

Choose a reason for hiding this comment

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

SpringBoot AutoConfiguration이 제공하는 Bean을 사용해도 되나
transactionManager가 만들어지려면 EntityManagerFactory가 필요하고 Factory가 만들어지려면 DataSource, JpaVendorAdapter가 필요한 이런 개념들을 파악해 두고 싶어
학습에 목적을 두고 빈을 직접 등록했었던 것 같습니다.

말씀하신대로 SpringBoot가 제공하는 Bean을 사용하는 방식으로 바꿔보겠습니다!!

Comment on lines 22 to 25
dataSource.setDriverClassName("org.h2.Driver");
dataSource.setUrl("jdbc:h2:tcp://localhost/~/test");
dataSource.setUsername("sa");
dataSource.setPassword("");
Copy link

Choose a reason for hiding this comment

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

직접 등록하실거라면 리터럴로 관리하는것보다 외부에서 주입하는것이 좋아보입니다

import lombok.NoArgsConstructor;

@NoArgsConstructor
@EqualsAndHashCode
Copy link

Choose a reason for hiding this comment

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

equansAndHashcode 롬복 어노테이션 사용시 주의해야 할점이 몇가지 있어요.

무엇무엇이 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

stackoverflow가 발생할 수 있습니다.
예를 들어 양방향 관계에서 equansAndHashcode를 사용할 경우
서로의 hashCode()를 불러오는 과정이 계속 반복되기 때문입니다.

그래서 이런 문제를 피하기 위해서
@EqualsAndHashCode(exclude = {}) 혹은 '@EqualsAndHashCode.Exclude' 사용힐 수 있다고 합니다.

import lombok.Getter;
import lombok.NoArgsConstructor;

@NoArgsConstructor
Copy link

Choose a reason for hiding this comment

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

아무대서나 기본생성자로 생성할 수 있게 의도하신게 아니라면 범위를 줄여도 될것같습니다

@ActiveProfiles("test")
@DataJpaTest
class PersistenceContextTest {

Copy link

Choose a reason for hiding this comment

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

null과 empty가 저장될일은 없을까요?

Comment on lines 28 to 30
@Autowired
CustomerRepository customerRepository;
Long setUpCustomerId;
Copy link

Choose a reason for hiding this comment

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

못봤네요. 개행하면 가독성이 올라갈것같습니다

}

@Test
@DisplayName("고객 아이디로 삭제되어 저장소 사이즈가 0이 된다.")
Copy link

Choose a reason for hiding this comment

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

존재하지 않아 삭제하지 못한 데이터는 어떻게 될까요?


@Test
@DisplayName("전체 데이터 조회시 setUp()에 넣은 데이터를 조회하여 사이즈가 1이 된다.")
void findAll() {
Copy link

Choose a reason for hiding this comment

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

findAll에 10만건 이상의 데이터가 있을 경우도 고려해보면 좋겠습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

페이지네이션을 적용하였습니다. 🐣

Comment on lines 33 to 40
//when
transaction.begin();

String id = UUID.randomUUID().toString();
Order order = new Order(id, "----", 1000, 1, customer);
Car car = new Car(1000, 1, 500);
OrderItem orderItem = new OrderItem(1000, 1, order, car);
order.addOrderItem(orderItem);
Copy link

Choose a reason for hiding this comment

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

----는 무엇을 의미하나요?

given절에 있ㅇ어도 되는 코드가 when절까지 넘어온것 같습니다

Copy link

@WooSungHwan WooSungHwan left a comment

Choose a reason for hiding this comment

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

영수님이 많이 짚어주셔서 제가 더 짚을부분은 없는 것 같아서 민망하네요 ㅎㅎ 항상 기본은 심플한 코드에서 드러나고 어려운 내용에서는 심플하게 풀어낼 수록 실력이 늘어감을 느끼실거에요. 앞으로 과제에서도 좋은 모습 기대하겠습니다 ㅎㅎ


@Configuration
public class DataSourceConfig {

Choose a reason for hiding this comment

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

저도 궁금

Comment on lines +25 to +27
public Customer(Name name) {
this.name = name;
}

Choose a reason for hiding this comment

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

name 객체가 null인지 체크는 필요할듯 합니다~

Comment on lines 11 to 14
@Getter
@MappedSuperclass
public class BaseEntity {

Choose a reason for hiding this comment

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

보통은 그래서 추상으로 정의하기도 하죠

@byeolhaha byeolhaha requested a review from devYSK August 15, 2023 05:46
@byeolhaha byeolhaha requested a review from WooSungHwan August 23, 2023 04:06
Comment on lines 19 to 30
private static final String DRIVER = "org.h2.Driver";
private static final String DB_URL = "jdbc:h2:tcp://localhost/~/test";
private static final String USER = "sa";
private static final String PASSWORD= "";

@Bean
public DataSource dataSource() {
DriverManagerDataSource dataSource = new DriverManagerDataSource();
dataSource.setDriverClassName("org.h2.Driver");
dataSource.setUrl("jdbc:h2:tcp://localhost/~/test");
dataSource.setUsername("sa");
dataSource.setPassword("");
dataSource.setDriverClassName(DRIVER);
dataSource.setUrl(DB_URL);
dataSource.setUsername(USER);
dataSource.setPassword(PASSWORD);
Copy link

Choose a reason for hiding this comment

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

이렇게 필드로 정보를 관리하고 DataSource를 직접 생성하는 의도가 궁금해요.

설정 정보를 읽어다 Spring AutoConfiguration이 Datasource를 만들어 줍니다.
또한 저렇게 바꾸고 고정시켜놓는다면 데이타소스 변경 등 유연한 대처가 어려울것같은데요?

Comment on lines +24 to +28
@OneToMany(mappedBy = "customer")
private List<Order> orders = new ArrayList<>();

protected Customer() { }

Copy link

Choose a reason for hiding this comment

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

fetch가 default로 LAZY긴 하지만 명시해두는것이 의도가 보여 좋을때도 있어요

Comment on lines 20 to 22
validateName(firstName,lastName );
this.firstName = firstName;
this.lastName = lastName;
Copy link

Choose a reason for hiding this comment

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

코드 정리해서 깔끔하게 유지하도록 노력해봐요. 공백한칸 띄우고 빈칸 지워도 될것같네요

validate 하는 행위랑 초기화 하는 행위랑 문맥이 다르니

한칸 띄워서 가독성을 높여보는것은 어떻게 생각하세요?

Copy link
Member Author

Choose a reason for hiding this comment

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

문맥을 고려해서 개행하도록 하겠습니다. 🐣

Comment on lines +9 to +10
customer.getName().getFirstName(),
customer.getName().getLastName());
Copy link

Choose a reason for hiding this comment

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

가르 get get get get get

객체A 를 조작하는 객체 B 는 객체 A가 어떠한 자료를 갖고 어떤 속사정을 갖고있는지 몰라도 됩니다.

디미터의 법칙에 대해 알아보세요

Copy link
Member Author

Choose a reason for hiding this comment

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

디미터의 법칙은 준수하도록 코드를 수정하도록 하겠습니다. 🐣

Comment on lines +11 to +14
public static CustomerResponses of(Slice<Customer> customers) {
List<CustomerResponse> responses = customers.stream()
.map(CustomerResponse::of)
.collect(Collectors.toList());
Copy link

Choose a reason for hiding this comment

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

좋네요 ㅎㅎ

정적팩토리 메소드에 바로 넣어서 생성 할까? 말까? 도 생각해보면 좋을거같아요


public Price(int cost) {
if (cost < 0) {
throw new IllegalArgumentException("가격 : " + cost);
Copy link

Choose a reason for hiding this comment

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

메시지를 조금 더 구체적이게 넣어주면 더 좋지않을까요

가격이 -1인데 왜 예외가 터진거야? 라고 생각할수도 있을것같네요

Comment on lines +43 to +44
assertThat(findCustomer.getName().getFirstName()).isEqualTo("영운");
assertThat(findCustomer.getName().getLastName()).isEqualTo("윤");
Copy link

Choose a reason for hiding this comment

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

디미터의 법칙

import java.util.List;
import org.springframework.transaction.annotation.Transactional;
import java.util.Random;
Copy link

Choose a reason for hiding this comment

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

java util.random vs SecureRandom에 대해 알아보세요

Comment on lines +102 to +105
Exception exception = catchException(() -> customerService.deleteById(new Random().nextLong()));

//then
assertThat(exception).isInstanceOf(EntityNotFoundException.class);
Copy link

Choose a reason for hiding this comment

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

assertThatExceptionOfType이란게 있어요

Choose a reason for hiding this comment

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

when과 then절을 명확하게 나누고 싶어 위와 같은 방식을 사용했던것 같습니다!!
그냥 assertThatExceptionOfType 메소드를 사용하는 것이 좀 더 올바른 방법일까요??

OrderItem updatedOrderItem = em.find(OrderItem.class, orderItem.getId());
Car item = (Car) updatedOrderItem.getItem();

assertThat(updatedOrder.getOrderItems().get(0).getPrice()).isEqualTo(1000);
assertThat(updatedOrder.getOrderItems().get(0).getPrice().getCost()).isEqualTo(1000);
Copy link

Choose a reason for hiding this comment

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

get get get get

디미터의 법칙

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