[1 - 2단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #72

helenason · 2024-05-16T07:29:45Z

아서 안녕하세요! 에버입니다.

이번 미션은 JPA에 익숙해지는 과정이었던 것 같아요.
아직 많이 부족하지만 잘 부탁드려요!

스프링으로는 오래 전 CRUD를 구현한 경험이 있습니다.
기초가 부실한 상태라고 생각해요. 참고해주시면 감사하겠습니다!

이번 미션 변경 사항 링크입니다!

감사합니다 😊

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

Hyunta

안녕하세요 에버,
1,2단계 미션 잘 구현해주셨네요!
관련해서 몇가지 커멘트 남겼는데 확인 부탁드립니다.
궁금하시거나 이해가 안가는 부분이 있으면 DM 주시거나 커멘트로 알려주세요.

Hyunta · 2024-05-17T13:16:23Z

src/main/java/roomescape/model/Reservation.java

+    @NotNull
    private LocalDate date;


@Column(nullable=false) 와는 어떤 차이가 있나요?

차이점을 알기 전, 제 코드에 오류가 존재했네요 😢

기존 코드

@NotNull @Embedded private Name name;

@NotBlank private String name;

Embedded 클래스 필드에 대해, @NotNull을 검증하기 위해서는 @Valid 어노테이션을 붙였어야 했는데 그러지 못했습니다. 따라서 null 값이 데이터베이스에 저장됨을 확인하여, @Valid를 붙여주었습니다.

또한, @NotNull 어노테이션을 통해 DDL에 not null 조건을 부여하기 위해서는 두 필드 모두에게 어노테이션을 붙여주어야 했습니다. 따라서 아래와 같이 코드를 변경하였습니다.

변경 코드

@NotNull @Embedded @Valid private Name name;

@NotNull @NotBlank private String name;

두 경우 모두 도메인 자체는 null 값을 가질 수 있으나, 두 경우의 가장 큰 차이점은 검증을 진행하는 시점이라고 생각합니다. @NotNull 은 insert 또는 update 쿼리가 실행되기 전 어플리케이션 단에서 검증을 진행합니다. 반면 @Column(nullable=false)는 쿼리를 실행한 후 데이터베이스가 던지는 에러를 통해 검증됩니다.

후자의 경우 어차피 실패할 불필요한 쿼리를 한 번 더 실행하는 것으로 보입니다. 또한, 전자의 방식을 선택할 경우 not null 조건뿐 아니라 여러 조건 검증으로까지 확장이 가능하므로 현재 저의 방식인 전자의 방식을 유지하는 것이 낫다고 판단하였습니다!

이번에 file change가 너무 많다보니 제가 놓쳤나보네요..ㅠㅠ
그래도 오류가 있던 부분까지 찾아주시고 잘 수정해주셨습니다👍🏻
생각하시는 규격까지 마련해주셔서 이후에 필드값이 추가될 때 편하게 추가할 수 있을 것 같네요ㅎㅎ

에버가 제시해준 근거가 설득력 있다고 생각합니다!

Hyunta · 2024-05-17T13:19:08Z

src/main/java/roomescape/model/Reservation.java

+    @ManyToOne
    private ReservationTime time;


ManyToOne 옵션 중 fetchType을 알아봅시다.

JoinColumn 애노테이션과 현재 어떤 이름으로 컬럼이 주어지는지도 확인해봅시다.

즉시 로딩의 경우, 테이블이 참조하는 모든 데이터를 가져옵니다. 반면 지연 로딩의 경우, 해당 테이블의 데이터만 실제 객체로 조회하며, 연관된 데이터들은 실제 객체를 상속받은 프록시 객체로 가져옵니다. 추후 실제 객체가 필요할 경우 쿼리를 실행하여 실제 객체를 가져오도록 합니다.

위 특징에 따라 즉시 로딩의 경우, 한 번의 쿼리 실행이 불필요한 쿼리를 야기할 수 있습니다. 따라서 필요한 쿼리만 실행시키기 위해 웬만한 경우에서 즉시 로딩보다는 지연 로딩 방식을 선택하는 것이 나아보입니다.

미션 코드를 구현할 때는 깊이 생각하지 않았다가, 미션 제출 후 학습테스트를 더 찬찬히 뜯어 보면서 모든 경우에 지연 로딩을 사용하기로 결론 내렸습니다! 따라서 미션 코드 또한 지연 로딩을 사용하도록 수정하겠습니다.

현재는 자바에서 선언한 필드명인 time을 받아 time_id로 선언되고 있습니다.

@JoinColumn 어노테이션의 name 옵션을 통해 테이블 컬럼의 이름을 명시할 수 있습니다. 필드의 이름과 컬럼의 이름을 달리 하여 테스트 해보니 JPA method에서는 자바에서 선언한 필드의 이름을, sql 쿼리에서는 어노테이션을 통해 명시한 컬럼의 이름을 사용하는 것으로 보입니다!

말해주신 것처럼 fetchType의 경우 되도록이면 Lazy로 설정해둔뒤 필요에 따라서 같이 로딩 할 수 있도록 하는 편이 좋아보입니다.

JoinColumn을 이용하면 JPA에서 자동으로 만들어주는 {fieldName}_id 가 아니라
명시적으로 부여할 수 있어서 저는 작성하는 것을 선호하는 편입니다.

Hyunta · 2024-05-17T13:19:25Z

src/main/java/roomescape/model/Reservation.java

+    private Reservation(long id, LocalDate date, ReservationTime time, Theme theme, Member member) {
        this.id = id;
        this.date = date;
        this.time = time;
        this.theme = theme;
        this.member = member;
    }


생성자에 id를 넣어주지 않아도 됩니다.

비즈니스 로직에서 위 생성자가 필요한 경우는 아직 없지만, 모든 필드를 사용하여 객체를 만드는 생성자를 private으로 선언하고, id가 없는 생성자는 아래에 public으로 열어 private 생성자로 수렴하도록 한 상태입니다. 전체 필드로 객체를 생성하는 생성자는 기본적으로 존재하는 것이 낫다고 판단하여 위와 같이 코드를 구현하였는데, 아서의 생각 또한 궁금합니다!

아하, 본인만의 기준이 있으셨군요 그럼 유지해도 괜찮습니다.
JPA를 사용하는 경우 ID를 주입받아서 사용하는 경우가 거의 없기 때문에 ID를 제외한 필드에 대해서 빌더를 구현해서 사용하는 것을 선호합니다.
그런데 에버의 기준또한 합리적이라고 생각해서 이대로 유지하시는게 좋을 것 같습니다ㅎㅎ

Hyunta · 2024-05-17T13:25:07Z

src/main/java/roomescape/model/Reservation.java

+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private long id;


https://docs.jboss.org/hibernate/orm/5.3/userguide/html_single/Hibernate_User_Guide.html#entity-pojo-identifier

최근 문서에는 직접 명시되어있지 않지만 primitive type long 보다는 wrapper type Long 을 권장하고 있어요.

Hyunta · 2024-05-17T13:25:34Z

src/main/java/roomescape/model/Reservation.java

+    public Reservation() {
    }


기본생성자는 왜 필요한가요?
접근제어자는 어떤 걸 사용할 수 있을까요?

https://www.baeldung.com/jpa-no-argument-constructor-entity-class

엔티티 클래스를 인스턴스화할 때 기본생성자가 요구됩니다.

성능 최적화 또는 지연 로딩의 경우, 엔티티 클래스를 상속받은 프록시 객체를 생성하는데, 해당 클래스를 인스턴스화하는 과정에서 기본 생성자가 요구됩니다.

엔티티와 데이터베이스의 테이블을 서로 매핑하는 과정에서, 엔티티 클래스의 인스턴스를 생성하는데, 기본 생성자가 존재해야만 가능합니다.

접근제어자는 public과 protected를 사용할 수 있습니다.

말씀해주신 것처럼 JPA에서 프록시 객체를 만들기 위해서 기본생성자를 필요로 하고 있습니다.
public으로 제어를 해야할 필요가 없다면 protected로 접근제어자를 지정해줘도 괜찮을 것 같네요.

Hyunta · 2024-05-17T13:28:19Z

src/main/java/roomescape/model/member/Member.java

+    @NotNull
+    @Enumerated(EnumType.STRING)


EnumType.STRING 을 사용해주신 이유가 있을까요?

EnumType을 default인 EnumType.ORDINAL로 설정하는 경우, 역할을 Role 클래스 내 필드의 순서로 저장합니다.

role tinyint not null check (role between 0 and 1),

하지만 이는 역할을 명확하게 표현할 수 있는 방법이 아니라고 생각합니다. 데이터베이스를 확인했을 때, 0과 1이 저장된 형태를 보고 USER와 ADMIN임을 알기 위해서는, 자바 코드를 직접 확인해야하기 때문에요! 뿐만 아니라 자바의 Role 객체에서 각 필드들의 순서 또한 얼마든지 변경될 수 있기에 순서에 의존한 방식은 좋은 방식이 아니라고 판단하였습니다.

따라서 EnumType.STRING 옵션을 통해 조금 더 명확한 의미를 담고자 하였습니다.

role varchar(255) not null check (role in ('USER','ADMIN')),

합리적인 근거라고 생각합니다 👍🏻

src/main/java/roomescape/model/theme/Theme.java

Hyunta · 2024-05-17T13:31:17Z

src/main/java/roomescape/repository/MemberRepository.java

 import java.util.Optional;

 @Repository
-public class MemberRepository {
+public interface MemberRepository extends JpaRepository<Member, Long> {


JpaRepository와 CrudRepository는 무슨 차이가 있나요?

JpaRepository는 CrudRepository를 상속받은 ListCrudRepository를 상속받습니다. 띠라서 JpaRepository가 CrudRepository보다 더 많은 메서드를 제공합니다. 가장 눈에 띄는 차이점으로는, saveAll을 수행할 경우 CrudRepository는 Iterable을 반환하는 반면, JpaRepository는 List를 반환하도록 오버라이딩 했습니다. 따라서 JpaRepository가 CrudRepository보다 더욱 사용하기 편리할 것이라고 생각합니다!

src/main/java/roomescape/repository/MemberRepository.java

Hyunta · 2024-05-17T13:39:29Z

src/test/java/roomescape/service/ReservationServiceTest.java

+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
 class ReservationServiceTest {


해당 클래스에서 테스트하나 깨지는데 확인 바랍니다.

저의 경우에는 테스트가 실패한 적 없는 걸 보아, 아마 @BeforeEach에서 데이터베이스에 시간 데이터를 저장할 때의 시점과, ReservationService 클래스의 validateIsFuture 메서드에서 검증할 때의 시점 사이에 약간의 시간차가 있었던 것 같습니다. 그 짧은 사이에 시간이 흘러간 것 같네요...ㅎ

따라서 데이터베이스에 시간을 저장하는 로직을 saveReservation 메서드와 최대한 가깝게 배치하였습니다. 하지만 이 방법으로는 해당 테스트의 무결함을 완전히 보장할 순 없는 것 같아요. 테스트 코드에서 저장하고자 하는 시간은 데이터베이스에 저장할 때의 시점이고, 검증(비교)하고자 하는 시간은 ReservationService 클래스의 validateIsFuture 메서드 내에서의 시점입니다. 이 두 시간을 일치시키는 방법이 있을까요?

방법이 존재하지 않을 경우에는 가장 앞 단인 DTO 단에서 검증할까 싶었으나, timeId로 받아오는 시간을 알아내기 위해 DTO에서 repository에 접근하는 것은 좋지 않은 설계라고도 생각했습니다.� 이 경우를 어떻게 테스트해야할까요..? 좋은 방법이 있다면 언급 부탁드립니다! 🥹

현재는 3333년으로 수정해주신 것 같은데 이렇게 설정하면 문제가 발생하지 않을 것 같습니다.
년도까지 있는 데이터를 비교할 때는 이렇게 수정해주시는게 제일 빠른 방법이라고 생각합니다.
혹시 제가 깨지는 원인을 잘못 알고 있으면 알려주세요.

helenason · 2024-05-20T15:23:23Z

안녕하세요 아서!

조금 늦게 돌아왔네요 😅

주말동안 휴식이 필요했어서 푹 쉬었어요. 그래도 열심히 코멘트 반영하여 리뷰 요청 드립니다!

이번 리뷰도 잘 부탁드립니다! 궁금한 부분은 댓글로 달아두었어요.

감사합니다.

Hyunta

안녕하세요 에버,
피드백 반영 잘 해주셨네요.
남겨주신 커멘트에 답변 남겨놨습니다.
확인해보시고 궁금하거나 이해가 안가는 부분이 있으면
DM 주시거나 다음 PR 때 커멘트로 알려주세요.

Hyunta · 2024-05-21T11:47:08Z

src/main/java/roomescape/model/member/Member.java

+    @NotNull
+    @Enumerated(EnumType.STRING)


합리적인 근거라고 생각합니다 👍🏻

Hyunta · 2024-05-21T11:49:21Z

src/main/java/roomescape/model/Reservation.java

+    @NotNull
    private LocalDate date;


이번에 file change가 너무 많다보니 제가 놓쳤나보네요..ㅠㅠ
그래도 오류가 있던 부분까지 찾아주시고 잘 수정해주셨습니다👍🏻
생각하시는 규격까지 마련해주셔서 이후에 필드값이 추가될 때 편하게 추가할 수 있을 것 같네요ㅎㅎ

에버가 제시해준 근거가 설득력 있다고 생각합니다!

Hyunta · 2024-05-21T11:53:56Z

src/main/java/roomescape/model/Reservation.java

+    @ManyToOne
    private ReservationTime time;


말해주신 것처럼 fetchType의 경우 되도록이면 Lazy로 설정해둔뒤 필요에 따라서 같이 로딩 할 수 있도록 하는 편이 좋아보입니다.

JoinColumn을 이용하면 JPA에서 자동으로 만들어주는 {fieldName}_id 가 아니라
명시적으로 부여할 수 있어서 저는 작성하는 것을 선호하는 편입니다.

Hyunta · 2024-05-21T11:56:42Z

src/main/java/roomescape/model/Reservation.java

+    private Reservation(long id, LocalDate date, ReservationTime time, Theme theme, Member member) {
        this.id = id;
        this.date = date;
        this.time = time;
        this.theme = theme;
        this.member = member;
    }


아하, 본인만의 기준이 있으셨군요 그럼 유지해도 괜찮습니다.
JPA를 사용하는 경우 ID를 주입받아서 사용하는 경우가 거의 없기 때문에 ID를 제외한 필드에 대해서 빌더를 구현해서 사용하는 것을 선호합니다.
그런데 에버의 기준또한 합리적이라고 생각해서 이대로 유지하시는게 좋을 것 같습니다ㅎㅎ

Hyunta · 2024-05-21T11:57:50Z

src/main/java/roomescape/model/Reservation.java

+    public Reservation() {
    }


말씀해주신 것처럼 JPA에서 프록시 객체를 만들기 위해서 기본생성자를 필요로 하고 있습니다.
public으로 제어를 해야할 필요가 없다면 protected로 접근제어자를 지정해줘도 괜찮을 것 같네요.

Hyunta · 2024-05-21T12:12:45Z

src/test/java/roomescape/service/ReservationServiceTest.java

+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
 class ReservationServiceTest {


현재는 3333년으로 수정해주신 것 같은데 이렇게 설정하면 문제가 발생하지 않을 것 같습니다.
년도까지 있는 데이터를 비교할 때는 이렇게 수정해주시는게 제일 빠른 방법이라고 생각합니다.
혹시 제가 깨지는 원인을 잘못 알고 있으면 알려주세요.

helenason and others added 16 commits May 15, 2024 20:37

migrate previous code

068041b

refactor(all): JPA 전환

3bf6945

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

feat(client): 자신의 예약 조회 페이지

7ec16aa

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

feat(PageController): 자신의 예약 조회 페이지 반환

13e1b20

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

feat(ReservationController): 자신의 예약 조회 API

a2e8fd9

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(Member): 불필요한 생성자 삭제

49398d0

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(MemberRepository): 불필요한 메서드 삭제

0781fe8

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(Member): 리턴 타입 스트링으로 통일

364989e

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(Theme): 불필요한 생성자 삭제

aef828d

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

test(ReservationRepository): 테스트 보완

c1af753

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

test(ReservationRepository): 특정 멤버 예약 조회

4973558

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(ReservationTimeDto): 주생성자 활용

9623778

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

test(ReservationService): 테스트 보완

9638c8d

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

refactor(test): assertAll 활용

dbb8e1e

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

test(ReservationController): 자신의 예약 조회

f275e31

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

docs(README): 기능 구현사항 반영

ce215f9

Co-authored-by: hyxrxn <gpfls981220@gmail.com>

Hyunta requested changes May 17, 2024

View reviewed changes

helenason added 13 commits May 20, 2024 00:37

fix(model): 엔티티의 필드 검증

1b9544c

chore(properties): SQL 쿼리 파라미터 활성화

4c0a8d1

refactor(Reservation): 조인 시 지연 로딩

bf674d4

refactor(Reservation): 필드에 null 들어가지 않도록

8148296

refactor(member): Member 클래스 내부에서 사용되는 클래스 이름 변경

3f96236

refactor(model): PK를 통해 객체의 동등성 비교하도록

6370467

refactor(repository): 불필요한 어노테이션 삭제

ef1866c

test(ReservationService): 현재 시간 저장 로직 위치 이동

c55aab0

style(all): 불필요한 import문 삭제

bd3fbf3

chore(properties): transaction 로그 추가

e4db409

refactor(model): PK 타입 원시 타입에서 참조 타입으로 변경

1a96121

refactor(model): 불필요한 생성자 제거 및 형식 통일

21fa294

fix(ReservationServiceTest): 나노초로 인한 에러 해결

d54dc9f

helenason added 2 commits May 21, 2024 00:19

refactor(Reservation): 불필요한 로직 제거

531d72b

style(ThemeServiceTest): 불필요한 import문 삭제

00758b1

Hyunta approved these changes May 21, 2024

View reviewed changes

Hyunta merged commit b63dc11 into woowacourse:helenason May 21, 2024

helenason mentioned this pull request May 24, 2024

[3 - 4단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1 - 2단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #72

[1 - 2단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #72

helenason commented May 16, 2024 •

edited

Loading

Hyunta left a comment

Hyunta May 17, 2024

helenason May 19, 2024

helenason May 19, 2024 •

edited

Loading

Hyunta May 21, 2024

Hyunta May 17, 2024

helenason May 20, 2024

helenason May 20, 2024

Hyunta May 21, 2024

Hyunta May 17, 2024

helenason May 20, 2024 •

edited

Loading

Hyunta May 21, 2024

Hyunta May 17, 2024

Hyunta May 17, 2024

helenason May 20, 2024

Hyunta May 21, 2024

Hyunta May 17, 2024

helenason May 20, 2024

Hyunta May 21, 2024

Hyunta May 17, 2024

helenason May 20, 2024

Hyunta May 17, 2024

Hyunta May 17, 2024

helenason May 20, 2024 •

edited

Loading

Hyunta May 21, 2024

helenason commented May 20, 2024

Hyunta left a comment

Hyunta May 21, 2024

Hyunta May 21, 2024

Hyunta May 21, 2024

Hyunta May 21, 2024

Hyunta May 21, 2024

Hyunta May 21, 2024

		@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
		class ReservationServiceTest {

[1 - 2단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #72

[1 - 2단계 방탈출 예약 대기] 에버(손채영) 미션 제출합니다. #72

Conversation

helenason commented May 16, 2024 • edited Loading

Hyunta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helenason May 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helenason May 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helenason May 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helenason commented May 20, 2024

Hyunta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helenason commented May 16, 2024 •

edited

Loading

helenason May 19, 2024 •

edited

Loading

helenason May 20, 2024 •

edited

Loading

helenason May 20, 2024 •

edited

Loading