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

[Step1] 엔티티 매핑 #22

Merged
merged 4 commits into from May 28, 2021
Merged

[Step1] 엔티티 매핑 #22

merged 4 commits into from May 28, 2021

Conversation

LenKIM
Copy link

@LenKIM LenKIM commented May 28, 2021

안녕하세요. 리뷰어님,

JPA 1단계 코드 리뷰 요청드립니다 😃

학습테스트를 작성하라는 요구사항이 있는데, 어떤 테스트 코드를 작성해야 하는지 몰라, 간단히 Repository 에 잘 저장되는지 확인하는 테스트 코드만 추가했습니다.

피드백 주시면, 코드 수정하겠습니다!

감사합니다.

@slamdunk7575
Copy link

  • application.properties 관련해서 미션 진행하시면서 참고해보시면 도움이 되실것 같아요.
# SQL 확인
spring.jpa.properties.hibernate.show_sql=true
spring.jpa.properties.hibernate.format_sql=true
logging.level.org.hibernate.type.descriptor.sql=trace

# 외부 h2 DB 연동
spring.datasource.url=jdbc:h2:~/test;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE;MODE=MySQL
spring.datasource.username=sa
spring.h2.console.enabled=true
spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.MySQL57Dialect

@DataJpaTest 는 내장된 임베디드 DB를 사용하는데요.
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) 속성을 주면 메모리 DB가 아닌
실제 DB에 테스트도 가능합니다.

Copy link

@slamdunk7575 slamdunk7575 left a comment

Choose a reason for hiding this comment

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

안녕하세요:) 정규님. JPA 미션 함께하게 된 양경준 입니다.
첫번째 미션 잘 구현해주셨네요 👍
몇가지 코멘트 남겨 드렸는데 다음 미션 진행하시면서 같이 반영해보시면 좋을것 같아요.
궁금한점이나 어려운점 있으면 코멘트 또는 DM 남겨주세요. 함께 고민해보아요 🔥
앞으로 미션도 화이팅 입니다!

Comment on lines +5 to +8
QnA 서비스를 만들기 위해 JPA로 실제 도메인 모델을 어떠헥 구성하고 객체와 테이블을 어떻게 매핑해야 하는지 알아자.

1. 아래 DDL을 보고 엔티티 클래스와 리포지토리 클래스 작성하기
2. `DataJpaTest` 사용하여 학습테스트 하기

Choose a reason for hiding this comment

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

앞으로 미션들에서 자유롭게 작성해보시면 많은 도움이 되실 거에요 👍

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다 : )

Comment on lines +13 to +21
@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
public class BaseEntity {

@Column(nullable = false)
@CreatedDate
private LocalDateTime createAt;
@LastModifiedDate
private LocalDateTime updateAt;

Choose a reason for hiding this comment

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

BaseEntity로 분리 잘해주셨네요 👍

되도록 개행으로 분리하여 가독성을 높여주시면 좋겠어요.
전체적으로 안지켜진 부분이 있는데 적용 부탁드립니다 🙏

Suggested change
@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
public class BaseEntity {
@Column(nullable = false)
@CreatedDate
private LocalDateTime createAt;
@LastModifiedDate
private LocalDateTime updateAt;
@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
public class BaseEntity {
@Column(nullable = false)
@CreatedDate
private LocalDateTime createAt;
@LastModifiedDate
private LocalDateTime updateAt;

Copy link
Author

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 26
private boolean deleted = false;

Choose a reason for hiding this comment

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

boolean type은 default 값이 있어 null이 들어가진 않겠지만
DDL 생성시 not null 조건을 넣어주면 어떨까요? :)

Suggested change
private boolean deleted = false;
@NotNull
@Column(nullable = false)
private boolean deleted = false;

Copy link
Author

Choose a reason for hiding this comment

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

오, 조금더 명시적인 코드를 지향하시는군요 ㅎㅎ 코드 수정하겠습니다.

Comment on lines +24 to 25
@Column(length = 20)
private String userId;

Choose a reason for hiding this comment

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

위에 설정을 하셨지만 아래처럼 unique 설정을 할 수도 있겠네요 :)

Suggested change
@Column(length = 20)
private String userId;
@Column(length = 20, nullable = false, unique = true)
private String userId;

Copy link
Author

Choose a reason for hiding this comment

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

피드백 감사합니다! 코드 수정하겠습니다 👍

Comment on lines +26 to 29
@Column(length = 20)
private String password;
@Column(length = 20)
private String name;

Choose a reason for hiding this comment

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

password, name 에도 not null 조건을 주면 좋겠네요!

Copy link
Author

Choose a reason for hiding this comment

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

네! DDL만 보고 코드를 작성한 것같네요.

() -> assertThat(find).isEqualTo(save)
);
}
}
Copy link

Choose a reason for hiding this comment

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

아래처럼 repository에 있는 메소드들을 테스트 해볼 수 있을 것 같아요 😄
현재 테스트가 조금 부족해 보이는데요. 미션 진행하시면서 다양하게 테스트 해보시면 좋겠어요!

        @Test
	@DisplayName("question id로 삭제되지 않은 answer 리스트를 가지고 올 수 있다.")
	void findByQuestionIdAndDeletedFalse() {
		savedAnswer = answerRepository.save(AnswerTest.A2);

		List<Answer> answersByQuestion = answerRepository.findByQuestionIdAndDeletedFalse(Q1.getId());

		assertAll(
				() -> assertThat(answersByQuestion).hasSize(2),
				() -> assertThat(answersByQuestion).containsExactly(AnswerTest.A1, AnswerTest.A2)
		);
	}

Copy link
Author

Choose a reason for hiding this comment

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

ㅎㅎ 네 그렇게 하겠습니다. 감사합니다.

@slamdunk7575 slamdunk7575 merged commit 4f61848 into next-step:lenkim May 28, 2021
@slamdunk7575
Copy link

GenerationType 관련해서 참고해보시면 좋을 것 같아요.

public enum GenerationType { 
    TABLE, 
    SEQUENCE, 
    IDENTITY, 
    AUTO
}

https://gmlwjd9405.github.io/2019/08/12/primary-key-mapping.html

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

Successfully merging this pull request may close these issues.

None yet

2 participants