[1 - 2단계 방탈출 예약 대기] 프린(남기범) 미션 제출합니다. #6

GIVEN53 · 2024-05-14T16:04:10Z

안녕하세요 제이미! 프린입니다 😄
2주동안 잘 부탁드려요

이번 미션은 페어의 코드로 진행했고 추가된 커밋입니다

웹 개발 경험

Spring 기반의 프로젝트를 개발하고 ECS 환경에 배포한 경험이 있습니다

초기 데이터 정보

어드민

아이디: admin@gmail.com
패스워드: abc123

유저

아이디: user@gmail.com
패스워드: abc123

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

Co-authored-by: GIVE가N53 <rlqja0523@naver.com>

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

jamie9504

안녕하세요, 프린 😄 리뷰어 제이미입니다.

간결하게 잘 구현해주셨네요 👏
코멘트 조금 남겨두었으니 확인해주세요!

오늘도 좋은 하루 보내세요~!

jamie9504 · 2024-05-16T06:51:10Z

src/main/java/roomescape/controller/web/MemberPageController.java

@@ -16,6 +16,11 @@ public String reservationPage() {
        return "reservation";
    }

+    @GetMapping("/reservation-mine")


Page와 API의 URL 규칙을 다르게 두고 있나요?

Page는 html 파일을 모두 수정해야 해서 그대로 따랐습니다
두 엔드포인트를 맞춰 주는게 나을까요??

맞출 필요는 없지만, 어떠한 기준으로 분기가 된건지 궁금했어요!

코드 수정을 떠나서 프린은 어떠한 기준으로 URL Path를 만드는 것을 선호하나요?

'URL Path가 자원의 위치를 명확하게 나타내는가'에 포커스를 두고 있고 디렉토리 구조와 비슷하게 접근하고 있어요!

전체 예약에서 나의 예약을 조회한다
/reservations 전체 예약에서 /mine 나의 예약을 조회한다
예약은 컬렉션 범주이기 때문에 복수를 사용하고 그 안에서 나의 예약이라는 자원에 접근한다는 의미를 부여하고 싶었습니다

jamie9504 · 2024-05-16T06:53:58Z

src/main/java/roomescape/domain/reservation/ReservationRepository.java

+                AND (:themeId IS NULL OR r.theme.id = :themeId)
+                AND (:dateFrom IS NULL OR r.date >= :dateFrom)
+                AND (:dateTo IS NULL OR r.date <= :dateTo)
+            """)
    List<Reservation> findAllByConditions(Long memberId, Long themeId, LocalDate dateFrom, LocalDate dateTo);


@Query를 사용한 부분들은 사용하지 않고 구현할 수는 없을까요?

동적쿼리를 위해 사용하게 되었는데 @Query없이 구현한다면 Specification, Criteria, QueryDSL 을 고려해볼 것 같아요
하지만 미션이 이 부분까지 학습하라는 의도로 보여지지 않아서 고민이 돼요
제가 생각하지 못한 다른 방법이 있을까요??

예를 들어 @Query를 사용하지 않는다고 가정했을 때 (번거롭긴 하겠지만) Service에서 각각 파라미터 여부에 따라 Repository의 메서드로 분기를 해준다던가요 😃

이런 저런 방법을 고민해보면 좋을 것 같아서 남겨뒀던 코멘트였습니다!

서비스에서 분기처리하는 방법도 좋은 것 같아요!

하지만 경우의 수를 따졌을 때 (멤버, 테마, 시작날짜, 끝날짜)
4C0 + 4C1 + ... + 4C4 = 1 + 4 + 6 + 4 + 1 = 16

서비스에서 조건 분기 16개, 레포지토리의 메서드 16개가 생길 것 같아요
개수가 적으면 서비스에서 처리해볼 수 있겠으나 현재 상황에서는 지금처럼 진행하는게 나을 듯 합니다!

jamie9504 · 2024-05-16T07:02:18Z

src/test/java/roomescape/controller/api/ReservationControllerTest.java

+    @Test
+    @DisplayName("나의 예약들을 조회한다")
+    void getMyReservations() {
+        addReservation();


여기서 뭔가 생성된 Reservation에 대한 정보를 받아와서 활용하면
해당 메서드만 봤을 때 조금 더 내용이 잘 보일 수도 있을 것 같아요!

반영했습니다!

jamie9504 · 2024-05-16T07:09:14Z

src/main/java/roomescape/domain/member/MemberRepository.java

-    List<Member> findAll();
-
-    default Member getById(Long id) {
+    default Member getByIdentifier(long id) {


Repository에서 find후 orElseThrow를 사용하는 것과 Service에서 하는 것은 어떤 차이가 있을까요?

서비스 계층간 의존을 피하고, 중복 코드를 제거하기 위해 repository에서 default로 구현했습니다
개인적으로 예외를 던지는 것은 비즈니스 로직이라고 생각하지만 페어의 이전 코드와 일관성을 유지하기 위해 따르게 됐어요

현업에서는 어떻게 처리하는지 궁금해요!

그렇다면, 현재 상황에서는 NoSuchElementException를 모두 NOT_FOUND로 핸들링해도 될까요?

현업도 결국 다양한 개발자들이 일하다보니, 논의하거나 팀 컨벤션에 맞춰 가는 부분이라 정답은 없긴 합니다.
그래서 학습할 때, 이런 저런 고민을 해보고 시야를 넓히는 것이 좋다고 생각해요. 😃

GlobalExceptionHandler을 보니까 NoSuchElementException을 모두 404로 처리하고 있었네요!
잘못된 argument일 때 400을 전달할 필요도 있기 때문에 서비스 계층에서 처리하는게 맞는 것 같아요

default 메서드를 제거하고 호출하는 서비스에서 처리하도록 변경했습니다!

Repository는 결국 DB에서 불러와주는 역할을 하다보니, 어떠한 API가 호출되어서 해당 상황에서 없을 때 어떻게 핸들링되는지 통제하기가 어렵죠! 😃

jamie9504 · 2024-05-16T07:10:16Z

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

-    private final String name;
-    private final Role role;
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)


@GeneratedValue의 strategy는 어떠한 종류가 있나요?

AUTO

기본값이고 벤더사에 맞는 적절한 전략을 선택합니다

IDENTITY

MySQL의 auto increment와 같이 자동 증가 값을 사용합니다
insert 후에 id를 알 수 있기 때문에 영속화(persist) 시점에 insert 쿼리가 발생해요
-> 쓰기 지연 불가능

SEQUENCE

@Entity @SequenceGenerator( name = "reservation_sequence", sequenceName = "my_sequence", allocationSize = 30, initialValue = 1 ) public class Reservation { @Id @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "reservation_sequence") private Long id; }

db 시퀀스를 사용하여 키를 생성합니다
영속화될 때 시퀀스 조회를 위한 select 쿼리 발생, 커밋 시점에 insert 쿼리가 발생해요

DB에서는 allocationSize만큼 먼저 값을 증가시켜두고, 트랜잭션이 끝날 때마다 사이즈만큼 값을 추가합니다
JPA에서는 트랜잭션마다 allocationSize만큼 JVM 메모리에 할당 후 사용하기 때문에 다량의 데이터를 저장하는 엔티티일 때 사용해 볼 수 있을 것 같아요

TABLE

@Entity @TableGenerator( name = "reservation_sequence", table = "my_table", pkColumnValue = "seq", allocationSize = 30, initialValue = 1 ) public class Reservation { @Id @GeneratedValue(strategy = GenerationType.TABLE, generator = "reservation_sequence") private Long id; }

키 생성을 위한 db 테이블을 사용합니다

내부동작은 db 시퀀스와 같지만 값 증가를 위한 update 쿼리가 한 번 더 발생해요
MySQL과 같이 시퀀스를 지원하지 않는 벤더사때문에 등장한 것이 아닐까 싶어요

UUID

https://www.baeldung.com/java-hibernate-uuid-primary-key
이번에 새로 알게 되었는데 JPA 3.1버전부터 추가되었더라구요
PK를 UUID 타입으로 설정하고 AUTO 전략을 사용하는 것과 동일합니다

jamie9504 · 2024-05-16T07:10:39Z

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

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    @Column(nullable = false, unique = true)


@Column은 어떠한 애너테이션인가요?

객체 필드를 테이블 컬럼에 매핑할 때 사용하는 어노테이션입니다
기본값으로만 사용하면 생략할 수 있습니다

여기서 nullable false, unique true 등의 설정을 하면 어떻게 사용이 되나요?

컬럼에 대한 제약조건을 추가합니다

nullable

기본값 true

false 설정 시 null을 허용하지 않음

unique

기본값 false

true 설정 시 해당 값은 유일해야 함

현재 email 필드에 @Column이 없을 경우

EMAIL VARCHAR(255)

@Column(nullable = false, unique = true)을 설정할 경우

EMAIL VARCHAR(255) NOT NULL UNIQUE

jamie9504 · 2024-05-16T07:10:45Z

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

+    private String name;
+
+    @Column(nullable = false)
+    @Enumerated(EnumType.STRING)


Enumerated를 String으로 쓰면 어떠한 동작을 하게 되나요?
다른 동작들은 무엇이 있을까요?

STRING은 enum명을 저장합니다 그래서 데이터 크기가 크다는 단점이 있어요
반대로 ORDINAL은 enum의 순서를 저장합니다 이를 인지하지 못하고 enum 순서를 변경하면 에기치 못한 문제가 발생하기 때문에 지양하는 것으로 알고 있어요!

맞아요! 그 외 AttributeConverter를 사용하는 방식도 있습니다 😃

AttributeConverter를 구현해서 db에 저장하는 값 <-> 객체 매핑값을 컨버팅할 수 있군요!

@Column(nullable = false) @Enumerated(EnumType.STRING) @Convert(converter = MyConverter.class) private Role role;

enum 타입을 String으로 저장할 때 데이터 크기 단점을 어느정도 해소할 수 있겠네요 👍🏼

데이터 크기 단점과, 혹시나 Enum 상수명을 바꾸는 경우에도 도움이 될 수 있긴 하죠!
적재적소에 필요한 방식을 사용하는 것이 좋습니다 👍

jamie9504 · 2024-05-16T07:12:46Z

src/test/java/roomescape/controller/api/ReservationControllerTest.java

+        ReservationTimeResponse reservationTimeResponse = reservationResponse.time();
+        ThemeResponse themeResponse = reservationResponse.theme();
+
+        SoftAssertions.assertSoftly(softly -> {


SoftAssertions.assertSoftly를 사용한 이유는 무엇인가요?

assertAll()과 같이 모든 assert를 검증 후 실패한 테스트 코드의 예상값, 실제값을 출력합니다
더불어서 SoftAssertions.assertSoftly는 실패한 코드라인까지 출력하기 때문에 사용하게 되었어요

jamie9504 · 2024-05-16T07:16:17Z

src/test/java/roomescape/repository/ReservationTimeRepositoryTest.java

@@ -101,6 +98,11 @@ INSERT INTO reservation (id, date, member_id, time_id, theme_id)
    @Test
    @DisplayName("예약 시간을 삭제한다.")
    void deleteById() {
+        jdbcTemplate.update("INSERT INTO reservation_time (id, start_at) VALUES (1, '10:00')");


등록은 jdbcTemplate으로 하는 이유가 있나요? 어떠한 이점이 있을까요?

jpa로 변경됐기 때문에 save를 사용해도 무방할 것 같아요
이전 미션의 잔재라서 jpa를 도입한 지금 상황에 이점이 있는진 잘 모르겠어요 🤔

jdbcTemplate을 모두 제거했습니다!

jamie9504 · 2024-05-16T07:18:13Z

src/test/java/roomescape/support/extension/DatabaseCleaner.java

+import org.springframework.transaction.annotation.Transactional;
+
+@Component
+public class DatabaseCleaner {


DatabaseCleaner와 DatabaseClearExtension를 사용하면,
@Transcational을 붙인다던가의 기본 제공 동작을 사용하는 것과 어떠한 차이가 있을까요?

테스트를 격리하기 위한 방법으로 보통 3가지가 많이 거론되는 것 같아요!

테이블 truncate

@DirtyContext

@Transactional

저는 이 중에서 테이블을 truncate하는 방법을 선택했고 재사용과 유지보수를 위해서 extension으로 구현했어요

@DirtyContext는 테스트마다 별도의 컨텍스트를 로드합니다 하지만 컨텍스트를 새로 로드하는 것은 비용이 크고 시간이 오래 걸리기 때문에 꼭 필요한 상황에만 사용하는 것이 좋을 것 같아요

테스트에서 @Transactional 사용은 크루들과도 많이 얘기했던 주제였는데, 저는 @Transactional 사용을 지양하고 있어요
RestAssured를 사용한 인수 테스트는 @SpringBootTest의 WebEnvironment를 DEFINE_PORT 또는 RANDOM_PORT를 사용해서 서버를 띄웁니다 이 때 테스트와 별개의 스레드에서 실행되기 때문에 트랜잭션 롤백이 불가능하게 돼요
https://docs.spring.io/spring-boot/docs/1.5.3.RELEASE/reference/html/boot-features-testing.html#boot-features-testing-spring-boot-applications

영속성 컨텍스트는 트랜잭션과 라이프 사이클을 같이 하고 있어요 (OSIV를 껐을 경우) @SpringBootTest에서 롤백을 위해 @Transactional을 사용하면 의도치 않은 트랜잭션 적용으로 테스트는 통과하고 실제 프로덕션 코드는 정상 동작하지 않은 경우가 많았어요 (LazyInitialization 예외가 발생해야 하는데 테스트에서는 발생하지 않는 등)

제이미는 테스트에서 @Transactional을 사용하는 것에 대해 어떻게 생각하시나요??

현재 Repository Test에서는 @DataJpaTest 애너테이션을 사용하고 있긴 한데요,
해당 애너테이션은 어떠한 동작을 하는 애너테이션일까요?

개인적으론 어떠한 방식을 쓰던 그 방식이 어떻게 동작하고 있는지, 의도에 맞춰서 잘 동작하는지만 알고 있다면 문제가 된다고 생각하지는 않아요!

@DataJpaTest는 내부적으로 @Transactional을 가지고 있기 때문에 테스트 종료후 트랜잭션 롤백합니다

가장 하위 계층이기 때문에 로직과 영속성 컨텍스트가 어느정도 예측이 가능하다고 생각해요
이외의 계층은 여러 트랜잭션이 묶여있고 영속성 컨텍스트를 생각하기가 쉽지 않아서 실수할 여지가 많은 것 같아요
Spring Data JPA는 보통 메서드 하나 당 트랜잭션 하나만 물다보니 @Transactional의 사이드 이펙트가 별로 느껴지지 않았어요
그래서 persistence 계층에서만 사용하고 이외는 사용하지 않는 것이 좋을 것 같습니다

논리적으로 어색한 부분이 있으면 추가 피드백 부탁드립니다!

좋습니다 😃

@Transcational을 사용하는 방식과 사용하지 않는 방식,
DB 자체를 사용하는 통합테스트 방식과 사용하지 않는 단위 테스트 방식 (혹은 Mock 방식)을 모두 익혀두고 염두해둔다면
어떤 상황을 마주치더라도 자연스레 적응하기 좋을 것 같아요 😉

GIVEN53

안녕하세요 제이미 리뷰 감사합니다 😁
리뷰 반영과 함께 코멘트 및 궁금한 점 남겼습니다 확인 부탁드려요!

GIVEN53 · 2024-05-16T08:19:02Z

src/main/java/roomescape/controller/web/MemberPageController.java

@@ -16,6 +16,11 @@ public String reservationPage() {
        return "reservation";
    }

+    @GetMapping("/reservation-mine")


Page는 html 파일을 모두 수정해야 해서 그대로 따랐습니다
두 엔드포인트를 맞춰 주는게 나을까요??

GIVEN53 · 2024-05-17T01:55:23Z

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

-    private final String name;
-    private final Role role;
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)


AUTO

기본값이고 벤더사에 맞는 적절한 전략을 선택합니다

IDENTITY

MySQL의 auto increment와 같이 자동 증가 값을 사용합니다
insert 후에 id를 알 수 있기 때문에 영속화(persist) 시점에 insert 쿼리가 발생해요
-> 쓰기 지연 불가능

SEQUENCE

@Entity @SequenceGenerator( name = "reservation_sequence", sequenceName = "my_sequence", allocationSize = 30, initialValue = 1 ) public class Reservation { @Id @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "reservation_sequence") private Long id; }

db 시퀀스를 사용하여 키를 생성합니다
영속화될 때 시퀀스 조회를 위한 select 쿼리 발생, 커밋 시점에 insert 쿼리가 발생해요

DB에서는 allocationSize만큼 먼저 값을 증가시켜두고, 트랜잭션이 끝날 때마다 사이즈만큼 값을 추가합니다
JPA에서는 트랜잭션마다 allocationSize만큼 JVM 메모리에 할당 후 사용하기 때문에 다량의 데이터를 저장하는 엔티티일 때 사용해 볼 수 있을 것 같아요

TABLE

@Entity @TableGenerator( name = "reservation_sequence", table = "my_table", pkColumnValue = "seq", allocationSize = 30, initialValue = 1 ) public class Reservation { @Id @GeneratedValue(strategy = GenerationType.TABLE, generator = "reservation_sequence") private Long id; }

키 생성을 위한 db 테이블을 사용합니다

내부동작은 db 시퀀스와 같지만 값 증가를 위한 update 쿼리가 한 번 더 발생해요
MySQL과 같이 시퀀스를 지원하지 않는 벤더사때문에 등장한 것이 아닐까 싶어요

UUID

https://www.baeldung.com/java-hibernate-uuid-primary-key
이번에 새로 알게 되었는데 JPA 3.1버전부터 추가되었더라구요
PK를 UUID 타입으로 설정하고 AUTO 전략을 사용하는 것과 동일합니다

GIVEN53 · 2024-05-17T01:55:50Z

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

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    @Column(nullable = false, unique = true)


객체 필드를 테이블 컬럼에 매핑할 때 사용하는 어노테이션입니다
기본값으로만 사용하면 생략할 수 있습니다

GIVEN53 · 2024-05-17T01:57:30Z

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

+    private String name;
+
+    @Column(nullable = false)
+    @Enumerated(EnumType.STRING)


STRING은 enum명을 저장합니다 그래서 데이터 크기가 크다는 단점이 있어요
반대로 ORDINAL은 enum의 순서를 저장합니다 이를 인지하지 못하고 enum 순서를 변경하면 에기치 못한 문제가 발생하기 때문에 지양하는 것으로 알고 있어요!

GIVEN53 · 2024-05-17T01:58:29Z

src/main/java/roomescape/domain/member/MemberRepository.java

-    List<Member> findAll();
-
-    default Member getById(Long id) {
+    default Member getByIdentifier(long id) {


서비스 계층간 의존을 피하고, 중복 코드를 제거하기 위해 repository에서 default로 구현했습니다
개인적으로 예외를 던지는 것은 비즈니스 로직이라고 생각하지만 페어의 이전 코드와 일관성을 유지하기 위해 따르게 됐어요

현업에서는 어떻게 처리하는지 궁금해요!

GIVEN53 · 2024-05-17T02:00:15Z

src/test/java/roomescape/controller/api/ReservationControllerTest.java

+    @Test
+    @DisplayName("나의 예약들을 조회한다")
+    void getMyReservations() {
+        addReservation();


반영했습니다!

GIVEN53 · 2024-05-17T02:05:23Z

src/test/java/roomescape/controller/api/ReservationControllerTest.java

+        ReservationTimeResponse reservationTimeResponse = reservationResponse.time();
+        ThemeResponse themeResponse = reservationResponse.theme();
+
+        SoftAssertions.assertSoftly(softly -> {


assertAll()과 같이 모든 assert를 검증 후 실패한 테스트 코드의 예상값, 실제값을 출력합니다
더불어서 SoftAssertions.assertSoftly는 실패한 코드라인까지 출력하기 때문에 사용하게 되었어요

GIVEN53 · 2024-05-17T02:11:26Z

src/test/java/roomescape/support/extension/DatabaseCleaner.java

+import org.springframework.transaction.annotation.Transactional;
+
+@Component
+public class DatabaseCleaner {


테스트를 격리하기 위한 방법으로 보통 3가지가 많이 거론되는 것 같아요!

테이블 truncate

@DirtyContext

@Transactional

저는 이 중에서 테이블을 truncate하는 방법을 선택했고 재사용과 유지보수를 위해서 extension으로 구현했어요

@DirtyContext는 테스트마다 별도의 컨텍스트를 로드합니다 하지만 컨텍스트를 새로 로드하는 것은 비용이 크고 시간이 오래 걸리기 때문에 꼭 필요한 상황에만 사용하는 것이 좋을 것 같아요

테스트에서 @Transactional 사용은 크루들과도 많이 얘기했던 주제였는데, 저는 @Transactional 사용을 지양하고 있어요
RestAssured를 사용한 인수 테스트는 @SpringBootTest의 WebEnvironment를 DEFINE_PORT 또는 RANDOM_PORT를 사용해서 서버를 띄웁니다 이 때 테스트와 별개의 스레드에서 실행되기 때문에 트랜잭션 롤백이 불가능하게 돼요
https://docs.spring.io/spring-boot/docs/1.5.3.RELEASE/reference/html/boot-features-testing.html#boot-features-testing-spring-boot-applications

영속성 컨텍스트는 트랜잭션과 라이프 사이클을 같이 하고 있어요 (OSIV를 껐을 경우) @SpringBootTest에서 롤백을 위해 @Transactional을 사용하면 의도치 않은 트랜잭션 적용으로 테스트는 통과하고 실제 프로덕션 코드는 정상 동작하지 않은 경우가 많았어요 (LazyInitialization 예외가 발생해야 하는데 테스트에서는 발생하지 않는 등)

제이미는 테스트에서 @Transactional을 사용하는 것에 대해 어떻게 생각하시나요??

GIVEN53 · 2024-05-17T02:13:36Z

src/test/java/roomescape/repository/ReservationTimeRepositoryTest.java

@@ -101,6 +98,11 @@ INSERT INTO reservation (id, date, member_id, time_id, theme_id)
    @Test
    @DisplayName("예약 시간을 삭제한다.")
    void deleteById() {
+        jdbcTemplate.update("INSERT INTO reservation_time (id, start_at) VALUES (1, '10:00')");


jpa로 변경됐기 때문에 save를 사용해도 무방할 것 같아요
이전 미션의 잔재라서 jpa를 도입한 지금 상황에 이점이 있는진 잘 모르겠어요 🤔

jdbcTemplate을 모두 제거했습니다!

GIVEN53 · 2024-05-17T02:16:48Z

src/main/java/roomescape/domain/reservation/ReservationRepository.java

+                AND (:themeId IS NULL OR r.theme.id = :themeId)
+                AND (:dateFrom IS NULL OR r.date >= :dateFrom)
+                AND (:dateTo IS NULL OR r.date <= :dateTo)
+            """)
    List<Reservation> findAllByConditions(Long memberId, Long themeId, LocalDate dateFrom, LocalDate dateTo);


동적쿼리를 위해 사용하게 되었는데 @Query없이 구현한다면 Specification, Criteria, QueryDSL 을 고려해볼 것 같아요
하지만 미션이 이 부분까지 학습하라는 의도로 보여지지 않아서 고민이 돼요
제가 생각하지 못한 다른 방법이 있을까요??

jamie9504

안녕하세요, 프린 😄 리뷰어 제이미입니다.

코멘트 조금 더 남겨두었어요!
다음 리뷰 요청시에는 Approve & Merge 예정입니다.

오늘도 좋은 하루 보내세요!

jamie9504 · 2024-05-19T10:42:41Z

src/main/java/roomescape/controller/web/MemberPageController.java

@@ -16,6 +16,11 @@ public String reservationPage() {
        return "reservation";
    }

+    @GetMapping("/reservation-mine")


맞출 필요는 없지만, 어떠한 기준으로 분기가 된건지 궁금했어요!

코드 수정을 떠나서 프린은 어떠한 기준으로 URL Path를 만드는 것을 선호하나요?

jamie9504 · 2024-05-19T10:46:46Z

src/main/java/roomescape/domain/reservation/ReservationRepository.java

+                AND (:themeId IS NULL OR r.theme.id = :themeId)
+                AND (:dateFrom IS NULL OR r.date >= :dateFrom)
+                AND (:dateTo IS NULL OR r.date <= :dateTo)
+            """)
    List<Reservation> findAllByConditions(Long memberId, Long themeId, LocalDate dateFrom, LocalDate dateTo);


예를 들어 @Query를 사용하지 않는다고 가정했을 때 (번거롭긴 하겠지만) Service에서 각각 파라미터 여부에 따라 Repository의 메서드로 분기를 해준다던가요 😃

이런 저런 방법을 고민해보면 좋을 것 같아서 남겨뒀던 코멘트였습니다!

jamie9504 · 2024-05-19T10:47:08Z

src/main/java/roomescape/domain/member/MemberRepository.java

-    List<Member> findAll();
-
-    default Member getById(Long id) {
+    default Member getByIdentifier(long id) {


그렇다면, 현재 상황에서는 NoSuchElementException를 모두 NOT_FOUND로 핸들링해도 될까요?

현업도 결국 다양한 개발자들이 일하다보니, 논의하거나 팀 컨벤션에 맞춰 가는 부분이라 정답은 없긴 합니다.
그래서 학습할 때, 이런 저런 고민을 해보고 시야를 넓히는 것이 좋다고 생각해요. 😃

jamie9504 · 2024-05-19T10:48:54Z

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

+    private String name;
+
+    @Column(nullable = false)
+    @Enumerated(EnumType.STRING)


맞아요! 그 외 AttributeConverter를 사용하는 방식도 있습니다 😃

jamie9504 · 2024-05-19T10:49:24Z

src/test/java/roomescape/support/extension/DatabaseCleaner.java

+import org.springframework.transaction.annotation.Transactional;
+
+@Component
+public class DatabaseCleaner {


현재 Repository Test에서는 @DataJpaTest 애너테이션을 사용하고 있긴 한데요,
해당 애너테이션은 어떠한 동작을 하는 애너테이션일까요?

개인적으론 어떠한 방식을 쓰던 그 방식이 어떻게 동작하고 있는지, 의도에 맞춰서 잘 동작하는지만 알고 있다면 문제가 된다고 생각하지는 않아요!

jamie9504 · 2024-05-19T11:04:21Z

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

+    @Enumerated(EnumType.STRING)
+    private Role role;
+
+    protected Member() {


생성자에 Validate가 있는데, 필드를 포장해서 역할을 위임시켜보는 건 어떨까요?

반영했습니다!

jamie9504 · 2024-05-19T11:08:36Z

src/test/java/roomescape/repository/MemberRepositoryTest.java

    @Test
    @DisplayName("이메일로 회원을 조회한다.")
    void findByEmail() {
-        jdbcTemplate.update("INSERT INTO member (email, password, name, role) "
-                + "VALUES ('example@gmail.com', 'password', '구름', 'USER')");
+        Member member = MemberFixture.email("example@gmail.com");


Repository는 어떤 테스트를 하는 것이 좋다고 생각하시나요?
프린의 기준이 궁금하네요!

SimpleJpaRepository에 구현되어 있는 메서드는 테스트하지 않는 편입니다

@query를 사용했거나 query creation은 테스트가 필요하다고 생각해요 (제공되는 기능이 아닌 개발자가 직접 생성했기 때문에)
보통 예상하는 결괏값과 같은지 확인하고 있어요
현재 테스트의 경우 "이메일로 회원을 조회한다"이므로 input email과 output email이 같은지 확인합니다

jamie9504 · 2024-05-19T11:10:24Z

src/test/java/roomescape/repository/MemberRepositoryTest.java

 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
-import org.springframework.jdbc.core.JdbcTemplate;
 import roomescape.domain.member.Member;
 import roomescape.domain.member.MemberRepository;


product와 패키지 기준이 다른 것은 의도한 부분인가요?

아니요 전혀 의도하지 않았어요,,, 😅
기존에 해당 패키지에 JdbcXXXRepositoryTest가 있었고, 이 클래스를 jpa 테스트로 리팩토링해서 이런 패키지 구조가 된 것 같아요

product의 패키지 구조와 같도록 변경했습니다

jamie9504 · 2024-05-19T11:14:42Z

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

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    @Column(nullable = false, unique = true)


여기서 nullable false, unique true 등의 설정을 하면 어떻게 사용이 되나요?

jamie9504 · 2024-05-19T11:17:09Z

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

+    @Test
+    @DisplayName("나의 예약들을 조회한다.")
+    void getReservationsByMemberId() {
+        Member member = memberRepository.save(new Member("new@gmail.com", "password", "nickname", Role.USER));


문자열은 위와 아래가 동일한데, 별도의 상수/변수로 추출하지 않고 하드코딩한 이유가 있나요?

놓쳤던 부분이네요 🙃 fixture로 추출했습니다

GIVEN53

안녕하세요 제이미! 즐거운 월요일이네요 ㅎㅎㅎ
이번주도 화이팅! 🫡

GIVEN53 · 2024-05-19T11:51:23Z

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

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    @Column(nullable = false, unique = true)


컬럼에 대한 제약조건을 추가합니다

nullable

기본값 true

false 설정 시 null을 허용하지 않음

unique

기본값 false

true 설정 시 해당 값은 유일해야 함

현재 email 필드에 @Column이 없을 경우

EMAIL VARCHAR(255)

@Column(nullable = false, unique = true)을 설정할 경우

EMAIL VARCHAR(255) NOT NULL UNIQUE

GIVEN53 · 2024-05-19T12:02:34Z

src/main/java/roomescape/controller/web/MemberPageController.java

@@ -16,6 +16,11 @@ public String reservationPage() {
        return "reservation";
    }

+    @GetMapping("/reservation-mine")


'URL Path가 자원의 위치를 명확하게 나타내는가'에 포커스를 두고 있고 디렉토리 구조와 비슷하게 접근하고 있어요!

전체 예약에서 나의 예약을 조회한다
/reservations 전체 예약에서 /mine 나의 예약을 조회한다
예약은 컬렉션 범주이기 때문에 복수를 사용하고 그 안에서 나의 예약이라는 자원에 접근한다는 의미를 부여하고 싶었습니다

GIVEN53 · 2024-05-19T12:08:17Z

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

+    private String name;
+
+    @Column(nullable = false)
+    @Enumerated(EnumType.STRING)


AttributeConverter를 구현해서 db에 저장하는 값 <-> 객체 매핑값을 컨버팅할 수 있군요!

@Column(nullable = false) @Enumerated(EnumType.STRING) @Convert(converter = MyConverter.class) private Role role;

enum 타입을 String으로 저장할 때 데이터 크기 단점을 어느정도 해소할 수 있겠네요 👍🏼

GIVEN53 · 2024-05-19T12:25:01Z

src/main/java/roomescape/domain/member/MemberRepository.java

-    List<Member> findAll();
-
-    default Member getById(Long id) {
+    default Member getByIdentifier(long id) {


GlobalExceptionHandler을 보니까 NoSuchElementException을 모두 404로 처리하고 있었네요!
잘못된 argument일 때 400을 전달할 필요도 있기 때문에 서비스 계층에서 처리하는게 맞는 것 같아요

default 메서드를 제거하고 호출하는 서비스에서 처리하도록 변경했습니다!

GIVEN53 · 2024-05-19T12:39:54Z

src/main/java/roomescape/domain/reservation/ReservationRepository.java

+                AND (:themeId IS NULL OR r.theme.id = :themeId)
+                AND (:dateFrom IS NULL OR r.date >= :dateFrom)
+                AND (:dateTo IS NULL OR r.date <= :dateTo)
+            """)
    List<Reservation> findAllByConditions(Long memberId, Long themeId, LocalDate dateFrom, LocalDate dateTo);


서비스에서 분기처리하는 방법도 좋은 것 같아요!

하지만 경우의 수를 따졌을 때 (멤버, 테마, 시작날짜, 끝날짜)
4C0 + 4C1 + ... + 4C4 = 1 + 4 + 6 + 4 + 1 = 16

서비스에서 조건 분기 16개, 레포지토리의 메서드 16개가 생길 것 같아요
개수가 적으면 서비스에서 처리해볼 수 있겠으나 현재 상황에서는 지금처럼 진행하는게 나을 듯 합니다!

GIVEN53 · 2024-05-19T12:43:13Z

src/test/java/roomescape/repository/MemberRepositoryTest.java

 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
-import org.springframework.jdbc.core.JdbcTemplate;
 import roomescape.domain.member.Member;
 import roomescape.domain.member.MemberRepository;


아니요 전혀 의도하지 않았어요,,, 😅
기존에 해당 패키지에 JdbcXXXRepositoryTest가 있었고, 이 클래스를 jpa 테스트로 리팩토링해서 이런 패키지 구조가 된 것 같아요

product의 패키지 구조와 같도록 변경했습니다

GIVEN53 · 2024-05-19T12:45:05Z

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

+    @Test
+    @DisplayName("나의 예약들을 조회한다.")
+    void getReservationsByMemberId() {
+        Member member = memberRepository.save(new Member("new@gmail.com", "password", "nickname", Role.USER));


놓쳤던 부분이네요 🙃 fixture로 추출했습니다

GIVEN53 · 2024-05-19T12:48:13Z

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

+    @Enumerated(EnumType.STRING)
+    private Role role;
+
+    protected Member() {


반영했습니다!

GIVEN53 · 2024-05-19T13:00:30Z

src/test/java/roomescape/support/extension/DatabaseCleaner.java

+import org.springframework.transaction.annotation.Transactional;
+
+@Component
+public class DatabaseCleaner {


@DataJpaTest는 내부적으로 @Transactional을 가지고 있기 때문에 테스트 종료후 트랜잭션 롤백합니다

가장 하위 계층이기 때문에 로직과 영속성 컨텍스트가 어느정도 예측이 가능하다고 생각해요
이외의 계층은 여러 트랜잭션이 묶여있고 영속성 컨텍스트를 생각하기가 쉽지 않아서 실수할 여지가 많은 것 같아요
Spring Data JPA는 보통 메서드 하나 당 트랜잭션 하나만 물다보니 @Transactional의 사이드 이펙트가 별로 느껴지지 않았어요
그래서 persistence 계층에서만 사용하고 이외는 사용하지 않는 것이 좋을 것 같습니다

논리적으로 어색한 부분이 있으면 추가 피드백 부탁드립니다!

GIVEN53 · 2024-05-19T13:23:46Z

src/test/java/roomescape/repository/MemberRepositoryTest.java

    @Test
    @DisplayName("이메일로 회원을 조회한다.")
    void findByEmail() {
-        jdbcTemplate.update("INSERT INTO member (email, password, name, role) "
-                + "VALUES ('example@gmail.com', 'password', '구름', 'USER')");
+        Member member = MemberFixture.email("example@gmail.com");


SimpleJpaRepository에 구현되어 있는 메서드는 테스트하지 않는 편입니다

@query를 사용했거나 query creation은 테스트가 필요하다고 생각해요 (제공되는 기능이 아닌 개발자가 직접 생성했기 때문에)
보통 예상하는 결괏값과 같은지 확인하고 있어요
현재 테스트의 경우 "이메일로 회원을 조회한다"이므로 input email과 output email이 같은지 확인합니다

jamie9504

안녕하세요, 프린 😄 리뷰어 제이미입니다.

어제 코멘트를 보냈다고 생각했는데, 최종적으로 보내지 못해서 조금 늦어졌네요 😢

간단한 코멘트만 남겨두었고, 이만 Approve & Merge 합니다.
다음 단계도 화이팅입니다 💪

jamie9504 · 2024-05-20T09:00:24Z

src/main/java/roomescape/domain/member/MemberRepository.java

-    List<Member> findAll();
-
-    default Member getById(Long id) {
+    default Member getByIdentifier(long id) {


Repository는 결국 DB에서 불러와주는 역할을 하다보니, 어떠한 API가 호출되어서 해당 상황에서 없을 때 어떻게 핸들링되는지 통제하기가 어렵죠! 😃

jamie9504 · 2024-05-20T09:02:42Z

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

+    private String name;
+
+    @Column(nullable = false)
+    @Enumerated(EnumType.STRING)


데이터 크기 단점과, 혹시나 Enum 상수명을 바꾸는 경우에도 도움이 될 수 있긴 하죠!
적재적소에 필요한 방식을 사용하는 것이 좋습니다 👍

jamie9504 · 2024-05-20T09:03:02Z

src/test/java/roomescape/support/extension/DatabaseCleaner.java

+import org.springframework.transaction.annotation.Transactional;
+
+@Component
+public class DatabaseCleaner {


좋습니다 😃

@Transcational을 사용하는 방식과 사용하지 않는 방식,
DB 자체를 사용하는 통합테스트 방식과 사용하지 않는 단위 테스트 방식 (혹은 Mock 방식)을 모두 익혀두고 염두해둔다면
어떤 상황을 마주치더라도 자연스레 적응하기 좋을 것 같아요 😉

alstn113 and others added 21 commits May 14, 2024 14:31

migrate second mission

58791d4

chore: co-authored 추가

f129591

feat: jpa option 추가

285f391

feat: 엔티티 매핑 추가

dbbe52d

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: jpa로 전환

8fd5ae0

feat: 테스트 jpa config 설정

e6a39cb

test: DataJpaTest로 전환

2136972

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: 미션 3 web 추가

1ec71e5

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: 내 예약 페이지 작성

310ec19

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: response 명세에 맞춰 값 설정

ad585b3

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: status enum 추가

a5f947b

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

feat: 자신의 예약을 조회하는 기능 추가

50b973e

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

chore: 예약 상태 컬럼 추가

88e14bf

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

test: DatabaseCleaner extension 추

0521921

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

test: 나의 예약 조회 테스

8eae4b7

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

test: 코드 정

5d2bde6

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

refactor: status column not null 추가

561d023

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

docs: 나의 예약 조회 명세서 추가

1394c53

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

test: status 반환 테스트

eaba78d

Co-authored-by: GIVE가N53 <rlqja0523@naver.com>

refactor: DatabaseCleaner 리팩토링

62f3cf4

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

chore: 사용하지 않는 필드 제거

7fa714b

Co-authored-by: GIVEN53 <rlqja0523@naver.com>

GIVEN53 force-pushed the step1 branch from 2fbffdb to 7fa714b Compare May 14, 2024 16:23

GIVEN53 added 5 commits May 15, 2024 14:03

refactor: join column 추가

2bafd5f

refactor: unique 제약 조건 추가

1a25087

refactor: 테스트 클래스명 변경

6baa266

style: import 정리

c854e5b

chore: 불필요한 파일 삭제

919e32e

jamie9504 requested changes May 16, 2024

View reviewed changes

jamie9504 assigned jamie9504 and GIVEN53 May 16, 2024

GIVEN53 added 6 commits May 18, 2024 02:09

chore: sql 파일명 변경

95dd882

chore: 0 추가

a570f9c

test: fixture 추가

b7b9d7f

refactor: jdbc template 제거

40b06c7

refactor: 저장 후 리턴 값 사용

bbe9d83

refactor: nativeQuery 제거

0a39db0

GIVEN53 commented May 17, 2024

View reviewed changes

jamie9504 requested changes May 19, 2024

View reviewed changes

GIVEN53 added 8 commits May 19, 2024 21:51

refactor: product와 같은 패키지 구조로 변경

ca5840b

refactor: fixture 추출

3e86268

refactor: 불필요한 필드 매핑 제거

02417f5

refactor: email 분리

2904ac3

refactor: password 분리

b94a3ad

refactor: memberName 분리

7aee888

refactor: interface default 제거

184cda7

refactor: 테마명 분리

9b8d7ba

GIVEN53 commented May 20, 2024

View reviewed changes

jamie9504 approved these changes May 21, 2024

View reviewed changes

jamie9504 merged commit b4066cd into woowacourse:given53 May 21, 2024

[1 - 2단계 방탈출 예약 대기] 프린(남기범) 미션 제출합니다. #6

[1 - 2단계 방탈출 예약 대기] 프린(남기범) 미션 제출합니다. #6

Conversation

GIVEN53 commented May 14, 2024 • edited Loading

웹 개발 경험

초기 데이터 정보

jamie9504 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

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

AUTO

IDENTITY

SEQUENCE

TABLE

UUID

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

Choose a reason for hiding this comment

GIVEN53 May 17, 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

GIVEN53 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AUTO

IDENTITY

SEQUENCE

TABLE

UUID

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

GIVEN53 May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie9504 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

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

GIVEN53 commented May 14, 2024 •

edited

Loading

GIVEN53 May 17, 2024 •

edited

Loading

GIVEN53 May 17, 2024 •

edited

Loading