레디 방탈출 예약 관리 1 ~ 6 단계 #3

reddevilmidzy · 2024-04-23T09:57:33Z

감사합니당

@controller

* feat: 메인 화면 생성 * chore: devtools 추가 * fix: 메인 화면 조회 오류 수정 * feat: 예약 홈 화면 조회 구현 * test: 예약 홈 화면 조회 테스트 추가 * test: 예약 화면 조회 테스트 생성 * feat: Reservation 객체 생성 * feat: reservation 목록 조회 메서드 생성 * feat: 예약 목록 조회 기능 구현 * fix: html 수정 내역 삭제 * refactor: Reservation 컨트롤러 추가 및 서비스 삭제 * feat: 예약 추가 기능 구현 * feat: 예약 삭제 기능 구현 * refactor: @controller 대신 @RestController로 변경 * refactor: reservationId 변수명 변경 * refactor: 존재하지 않은 예약 삭제 시 404 반환 * test: test 변수 new HashMap에서 Mpt.of로 변경 * test: test 변수 new HashMap에서 Map.of로 변경 * refactor: 예약 저장시 반환타입 ResponseEntity 래핑 제거 * style: 애노테이션과 final 순서 변경 * refactor: 예약 저장 메서드 내 흐름 순서 변경 * refactor: 예약 반환, 응답 dto 레코드이름에서 dto 제거 * refactor: 예약 저장 메서드이름 변경 * test: 존재하지 않는 예약 삭제 테스트 추가 * refactor: AdminControllerTest에 있던 예약 테스트 ReservationControllerTest로 이동 * test: 미션별 단계 테스트 추가 * test: 예약 추가 및 삭제 테스트 분리, 및 RestAssured 대신 assertj 사용 * fix: html의 링크 오류 수정 * fix: 테스트 시 post 변경 * style: 메서드 순서 변경 * test: dto 변환 테스트 추가 * refactor: toEntity 대신 toReservation 으로 메서드명 변경 * fix: AdminControllerTest 테스트 port 설정 * style: 디미터 법칙에 따라 개행 추가 --------- Co-authored-by: juha <[email protected]>

leegwichan

하이 레디! 커찬이야 (글을 편하게 쓰기 위해서 반말로 작성했어)
내가 궁금한 점들을 정리해서 코멘트 달아봤어!

9개만 단 것은, 다른 크루가 리뷰할걸 남겨놓기 위해...
레디 고생많았어! 7~9 단계도 화이팅!

leegwichan · 2024-04-24T00:58:20Z

README.md

+# 방탈출 예약 관리 
+
+## API 명세


👍 👍 👍

leegwichan · 2024-04-24T00:59:12Z

build.gradle

+    implementation 'org.springframework.boot:spring-boot-devtools'
    runtimeOnly 'com.h2database:h2'
    testImplementation 'org.springframework.boot:spring-boot-starter-test'
    testImplementation 'io.rest-assured:rest-assured:5.3.1'
+    testRuntimeOnly 'com.h2database:h2'


각각 라이브러리가 있는 것과 없는 것에는 어떤 차이가 있어?

leegwichan · 2024-04-24T04:37:29Z

src/main/java/roomescape/dto/ReservationResponse.java

+    private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm");
+
+    public static ReservationResponse from(final Reservation reservation) {
+        final String date = reservation.getDate().format(DateTimeFormatter.ISO_DATE);
+        final String time = reservation.getTime().format(TIME_FORMATTER);
+        return new ReservationResponse(reservation.getId(), reservation.getName(), date, time);
+    }


약간 억지(?)일수도 있다고 생각하는데, ISO_DATE 보다는 직접 글자로 명시해주는게 더 직관적일수 있다 생각하는데, 레디는 어떻게 생각해?

Suggested change

private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm");

public static ReservationResponse from(final Reservation reservation) {

final String date = reservation.getDate().format(DateTimeFormatter.ISO_DATE);

final String time = reservation.getTime().format(TIME_FORMATTER);

return new ReservationResponse(reservation.getId(), reservation.getName(), date, time);

}

private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd");

private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm");

public static ReservationResponse from(final Reservation reservation) {

final String date = reservation.getDate().format(DATE_FORMATTER);

final String time = reservation.getTime().format(TIME_FORMATTER);

return new ReservationResponse(reservation.getId(), reservation.getName(), date, time);

}

leegwichan · 2024-04-24T04:43:39Z

src/test/java/roomescape/repository/ReservationDaoTest.java

+    @DisplayName("db 연결 확인")
+    @Test
+    void checkConnection() {
+        try (final Connection connection = Objects.requireNonNull(jdbcTemplate.getDataSource()).getConnection()) {
+            assertThat(connection).isNotNull();
+            assertThat(connection.getCatalog()).isEqualTo("DATABASE_TEST");
+            assertThat(connection.getMetaData().getTables(null, null, "RESERVATION", null).next()).isTrue();
+        } catch (final SQLException e) {
+            throw new IllegalStateException(e);
+        }
+    }


catch를 하면 예외를 던지기 보다는, 테스트를 실패하게 하는건 어떨까?

아래 메서드를 실행해보면 DB가 정상적으로 연결되는지 확인할 수 있는데, 해당 테스트가 크게 의미 있을까? 메서드들의 정상동작만 테크하면 되지 않을까?

leegwichan · 2024-04-24T04:45:05Z

src/test/java/roomescape/repository/ReservationDaoTest.java

+    @DisplayName("예약 삭제")
+    @Test
+    void removeReservation() {
+        final List<Reservation> beforeSaving = reservationDao.findAll();
+        final Reservation reservation = Reservation.create("레디", "2024-02-03", "15:00");
+        reservationDao.save(reservation);
+        final List<Reservation> afterSaving = reservationDao.findAll();
+        reservationDao.remove(1L);
+        final List<Reservation> afterRemoving = reservationDao.findAll();
+
+        assertAll(
+                () -> assertThat(beforeSaving).isEmpty(),
+                () -> assertThat(afterSaving).hasSize(1),
+                () -> assertThat(afterRemoving).isEmpty()
+        );
+    }


해당 테스트는 save(), remove()가 모두 정상 동작해야만 통과하는데, 이걸 분리해보는 건 어떨까?

leegwichan · 2024-04-24T04:45:43Z

src/test/java/roomescape/repository/ReservationDaoTest.java

+    @DisplayName("단건 조회")
+    @Test
+    void findById() {
+        //given
+        final Reservation reservation = Reservation.create("레디", "2024-02-03", "12:00");
+        final Reservation expected1 = reservation.toReservation(1L);
+
+        //when
+        reservationDao.save(reservation);
+        final Optional<Reservation> findReservation1 = reservationDao.findById(1L);
+
+        //then
+        assertThat(findReservation1).contains(expected1);
+    }


Suggested change

@DisplayName("단건 조회")

@Test

void findById() {

//given

final Reservation reservation = Reservation.create("레디", "2024-02-03", "12:00");

final Reservation expected1 = reservation.toReservation(1L);

//when

reservationDao.save(reservation);

final Optional<Reservation> findReservation1 = reservationDao.findById(1L);

//then

assertThat(findReservation1).contains(expected1);

}

@DisplayName("단건 조회")

@Test

void findById() {

//given

final Reservation reservation = Reservation.create("레디", "2024-02-03", "12:00");

final Reservation expected = reservation.toReservation(1L);

//when

reservationDao.save(reservation);

final Optional<Reservation> findReservation = reservationDao.findById(1L);

//then

assertThat(findReservation).contains(expected);

}

leegwichan · 2024-04-24T04:48:51Z

src/test/java/roomescape/repository/ReservationDaoTest.java

+        final List<Reservation> expected = new ArrayList<>();
+        for (long i = 1; i <= 3; i++) {
+            expected.add(reservations.get((int) (i - 1)).toReservation(i));
+        }


Suggested change

final List<Reservation> expected = new ArrayList<>();

for (long i = 1; i <= 3; i++) {

expected.add(reservations.get((int) (i - 1)).toReservation(i));

}

final List<Reservation> expected = List.of(

reservation1.toReservation(1L), reservation2.toReservation(2L), reservation3.toReservation(3L)

);

leegwichan · 2024-04-24T04:50:41Z

src/main/java/roomescape/controller/ReservationController.java

+        final URI uri = UriComponentsBuilder.fromPath("/reservations/{id}")
+                .buildAndExpand(reservationResponse.id())
+                .toUri();
+        return ResponseEntity.created(uri)


레디가 내 리뷰에도 남겨줬던것 같은데, 나는 오히려 해당 표현이 불필요하게 길어진다는 느낌을 받는 것 같아.
'+' 기호를 써서 URI를 만드는 것보다 위 표현이 어느 관점에서 더 좋다고 생각해?

leegwichan · 2024-04-24T04:52:00Z

src/main/java/roomescape/RoomescapeApplication.java

 @SpringBootApplication
 public class RoomescapeApplication {
-    public static void main(String[] args) {
+
+    public static void main(final String[] args) {
        SpringApplication.run(RoomescapeApplication.class, args);
    }
-
 }


final 광인의 면모를 다시 느끼고 갑니다.

3Juhwan

레디 미션하느라 고생 많았어~~
코멘트 남겼으니까 확인해줘!
아직 9단계까지 남았겠지? 끝까지 파이팅해 🔥🔥

3Juhwan · 2024-04-25T04:04:34Z

src/main/java/roomescape/repository/ReservationDao.java

+            return Optional.ofNullable(reservation);
+        } catch (final EmptyResultDataAccessException exception) {
+            return Optional.empty();
+        }
+    }


EmptyResultDataAccessException 만 예외 처리한 이유가 무엇인지 궁금해! 그리고 EmptyResultDataAccessException 가 발생할 수 있다는 것을 어떻게 알았는지도 궁금해

3Juhwan · 2024-04-25T04:06:42Z

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

+    public static Reservation create(final String name, final String date, final String time) {
+        return new Reservation(null, name, date, time);
+    }


생성자가 아닌 정적 팩토리 메서드를 만든 이유가 있을까? 그리고 메서드 명을 create로 한 것도 이유가 궁금해!

3Juhwan · 2024-04-25T04:09:58Z

src/test/java/roomescape/MissionStepTest.java

+    void step6() {
+        final Map<String, String> params = Map.of(
+                "name", "브라운",
+                "date", "2023-08-05",
+                "time", "10:00");
+
+        RestAssured.given().log().all()
+                .contentType(ContentType.JSON)
+                .body(params)
+                .when().post("/reservations")
+                .then().log().all()
+                .statusCode(201)
+                .header("Location", "/reservations/1");
+
+        final Integer count = jdbcTemplate.queryForObject("SELECT COUNT(1) FROM reservation", Integer.class);
+        assertThat(count).isEqualTo(1);
+
+        RestAssured.given().log().all()
+                .when().delete("/reservations/1")
+                .then().log().all()
+                .statusCode(204);
+
+        final Integer countAfterDelete = jdbcTemplate.queryForObject("SELECT COUNT(1) FROM reservation", Integer.class);
+        assertThat(countAfterDelete).isZero();
+    }


믿고 보는 레디의 테스트 👍 개인적으로 인사이트를 얻었어 ㅎㅎ 고마워!

3Juhwan · 2024-04-25T04:11:26Z

src/test/java/roomescape/dto/ReservationRequestTest.java

+class ReservationRequestTest {
+
+    @DisplayName("dto 변환")
+    @Test
+    void toReservation() {
+        final long id = 1L;
+        final String name = "레디";
+        final String date = "2024-04-18";
+        final String time = "13:00";
+        final ReservationRequest reservationRequest = new ReservationRequest(name, date, time);
+
+        final Reservation reservation = new Reservation(id, name, date, time);
+
+        assertThat(reservationRequest.toReservation(id)).isEqualTo(reservation);
+    }


dto 테스트를 작성하게 된 스토리가 궁금한 걸? 나는 테스트하지 않아도 괜찮다고 생각해서 하지 않았거든!

3Juhwan · 2024-04-25T04:18:18Z

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

+    public Reservation toReservation(final long id) {
+        return new Reservation(id, name, date, time);
+    }


개인적으로 이 메서드명도 의미가 명확하지 않다고 느껴지는 것 같아!
예를 들어, reservation.toReservation(id) 이런 식으로 코드를 작성할 것 같은데, 이 문장 자체는 이미 존재하는 reservation을 toReservation한다? 예약을 예약으로 변경하는 것으로 느껴져서 어색하다고 생각해

3Juhwan · 2024-04-25T04:20:17Z

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

+    @Override
+    public boolean equals(final Object target) {
+        if (this == target) {
+            return true;
+        }
+        if (target == null || getClass() != target.getClass()) {
+            return false;
+        }
+        final Reservation reservation = (Reservation) target;
+        return Objects.equals(getId(), reservation.getId()) && Objects.equals(getName(), reservation.getName())
+                && Objects.equals(getDate(), reservation.getDate()) && Objects.equals(getTime(),
+                reservation.getTime());
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(getId(), getName(), getDate(), getTime());
+    }


적절한 동등성 부여인지 고민이 필요할 것 같아! id만 같으면 동등한 객체로 보아도 괜찮지 않을까?

reddevilmidzy and others added 30 commits April 18, 2024 17:35

chore: h2 디비 경로 추가

91636e7

feat: 예약 테이블 스키마 생성

fe6c2b4

test: 데이터베이스 적용 테스트 추가

366736c

feat: 예약 저장 및 조회 기능 구현

a2c158f

feat: 예약 저장 기능을 컨트롤러와 연결

401ed5e

feat: 예약 삭제 기능 구현

9bceeea

style: sql 예약어 소문자에서 대문자로 변경

8b4a779

test: 테스트 코드 리턴 타입 변경 및 상태 코드 변경

3e15676

refactor: 예약 저장 및 삭제시 응답 상태코드 변경

bae93d1

style: 와일드카드 import 제거

296bcfc

refactor: 예약 저장시 location 헤더 추가 로직 상수 사용 및 정적 팩토리 메서드 사용

bdb14c6

fix: 예약 저장 및 삭제시 반영되지 않던 뷰 수정

684ba2f

test: 테스트 명 변경 및 displayName 추가

4296b21

docs: api 명세 생성

7eb4d31

refactor: 예약 저장 시 ResponseEntity 제공 메서드 활용 및 Uri 변환 로직 변경

8fbaf55

test: 예약 조회 테스트 추가

335c725

chore: test용 데이터베이스 분리

f9ca722

fix: 테스트용 테이블 명 수정

cde4dbd

refactor: 예약 삭제 시 controller와 service의 책임 분리

1b67fb1

refactor: dao의 save 파라미터를 도메인(reservation)으로 변경

087d49e

refactor: controller에서 도메인 제거

27818d7

test: 불필요한 @DirtiesContext 제거

3e1dc45

chore: 테스트시 콘솔 확인하지 못하도록 변경 및 콘솔 주소 디폴트로 설정

ec84b87

style: 괄호 위치 컨벤션에 맞도록 변경

1e55564

refactor: 단건 조회 테스트 단순화

771aa3d

test: 예약 조회 테스트 추가

521c7ee

test: db 연결 확인 테스트 추가

00aa82a

refactor: id를 주고 새로 Reservation 생성하는 메서드 활용

066bfe1

refactor: 메서드 명에서 entity 제거

e15440a

reddevilmidzy added 6 commits April 23, 2024 13:51

refactor: 파라미터에 있는 Long을 long으로 변경

d1cae20

refactor: 테스트 용 메서드를 제외한 기능은 하드 코딩

19a70a8

refactor: 테스트 용 메서드를 제외한 기능은 하드 코딩했던 것 롤백

aa5e221

refactor: 테스트 용 db 분리

92a74e6

fix: 테스트 용 application 이름 수정

7ed40ca

style: sql 쿼리 예약어 대문자로 변경

b69aaab

leegwichan reviewed Apr 24, 2024

View reviewed changes

3Juhwan reviewed Apr 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

레디 방탈출 예약 관리 1 ~ 6 단계 #3

레디 방탈출 예약 관리 1 ~ 6 단계 #3

reddevilmidzy commented Apr 23, 2024

leegwichan left a comment

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

leegwichan Apr 24, 2024

3Juhwan left a comment

3Juhwan Apr 25, 2024

3Juhwan Apr 25, 2024

3Juhwan Apr 25, 2024

3Juhwan Apr 25, 2024

3Juhwan Apr 25, 2024

3Juhwan Apr 25, 2024

레디 방탈출 예약 관리 1 ~ 6 단계 #3

Are you sure you want to change the base?

레디 방탈출 예약 관리 1 ~ 6 단계 #3

Conversation

reddevilmidzy commented Apr 23, 2024

leegwichan 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

3Juhwan 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