[4 - 9단계 방탈출 예약 관리] 로빈(임수빈) 미션 제출합니다. #155

robinjoon · 2024-04-25T04:33:41Z

안녕하세요 제이! 저는 감기 몸살에 걸려서 미션하는데 생각보다 오래 걸렸네요.. ㅠㅠ

이번 미션에서 특이한 점은 API 명세의 응답 상태코드가 일관성 없이 왔다갔다 수정된다는 것이었습니다...
아무리 고민해봐도 무슨 의도가 담긴 것인지 모르겠어서 모두 200 OK 로 응답하도록 했습니다.

미션 중 새로 알게된 내용

SpringBootTest 는 최대한 컨테이너를 재사용하려고 한다. => 설정이 바뀌면 컨테이너를 재사용하지 않는다.
delete 응답에 204 를 쓸 수 있다.
- 하지만, HTTP 상태코드를 엄밀하게 따를 필요도 없다. 각 도메인별 특화된 상황에 꼭 맞는 상태 코드가 없을 가능성이 높기 때문.

빠르게 머지가 된다면 10단계도 해보려고 합니다!

JunHoPark93

안녕하세요 로빈~
미션 구현하느라 고생많으셨습니다.
피드백 몇가지 남겨두었는데 확인해보시고 의견 및 반영 부탁드려요.

JunHoPark93 · 2024-04-25T15:03:03Z

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

    }

    @DeleteMapping("/{reservationId}")
    public void delete(@PathVariable long reservationId) {
-        reservationStorage.delete(reservationId);
+        reservationService.delete(reservationId);


개인선택일 수는 있지만, 개인적으로 짧게쓰는것을 선호해서 path에 id로만써도 충분할거같아요. 참고만 해주세요~

JunHoPark93 · 2024-04-25T15:03:34Z

src/main/java/roomescape/controller/ReservationTimeController.java

+        return reservationTimeService.findAll();
+    }
+
+    @DeleteMapping("/{id}")


요기엔 id로만 써주셨네요!
어느방향이든 일관성을 맞춰주세요

JunHoPark93 · 2024-04-25T15:07:12Z

src/main/java/roomescape/repository/JdbcTemplateReservationRepository.java

+    }
+
+    @Override
+    public Reservation save(ReservationRequest reservationRequest) {


controller에서 받아오는 request가 repository까지 쓰이고있는데 의도하신 구조가 맞을까요?
정답은 없지만 로빈이 생각하시는 계층형구조는 무엇일까요?

controller에서 받아오는 request가 repository까지 쓰이고있는데 의도하신 구조가 맞을까요? 정답은 없지만 로빈이 생각하시는 계층형구조는 무엇일까요?

유사한 역할을 가진 클래스들을 하나의 계층으로 묶어 추상화하고, 각 계층 사이에 메세지를 전달하는 것을 계층형 구조라고 생각합니다. 또한 각 계층은 이웃한 계층하고만 통신을 해야 합니다. 이 말은, 멀리 떨어진 계층일수록 서로 맡은 역할이 더 멀리 떨어져있다는 의미가 됩니다. 따라서 사용자로부터 도착한 어떤 요청이 repository까지 전달되는 과정에서 repository의 역할이 아닌 부분들은 희석되어 사라진 채 메세지가 전달되는 것이 좋다고 생각합니다.

따라서 처음 구조를 설계할 때는 service 레이어에서 엔티티를 생성해서 repository에 전달하는 구조를 생각했습니다. 그런데 이 방식을 실제 구현하는 과정에서 몇가지 불편한 점을 느껴 지금의 코드를 작성하게 되었습니다. 아래는 이 때 느낀 불편한 점과 지금의 구조로 도달한 과정을 정리한 것입니다.

불완전한 상태의 ReservationTime 생성 문제

엔티티 객체의 id 가 null인 경우 데이터베이스에 저장되지 않은 엔티티를 의미하고, id가 null이 아닌 경우 데이터베이스에 저장되어있는 데이터를 온전히 가지고 있으면 편할 것 같다고 생각했습니다. 이는 과거에 따로 공부할 때 JPA 를 사용하면서 느낀 장점이었습니다.

그런데, request를 repository로 전달하지 않으려면 아래 JSON을 역직렬화 한 Reservation 이 Repository로 넘어가야합니다.

{ "id": null, "name": "name_b8d1b86b5dc1", "date": "2024-04-28", "time": { "id": 31, "startAt": null } }

이 두 원칙을 모두 지킬 수 있는 방법으로 Repository의 매개변수를 늘리거나, 온전한 Reservation 객체를 Service 계층에서 생성하거나, 아예 DAO와 Repository 를 또 다시 계층을 분리하는 방법이 있다고 생각했습니다.

지금의 요구사항을 구현하는데 이런 것들을 고민할 필요가 있을까?
여러 방법 중 하나를 선택하기 위해 고민하던 중, 문뜩 이 고민 자체의 필요성에 대해 의문이 들었습니다.

계층형 구조 뿐 아니라 어떤 구조를 도입하더라도, 그 구조로 인한 장점을 얻을 수 있고 그것이 단점보다 커야 도입이 타당합니다. 그런데, 지금 상황에서 앞선 고민을 해결해서 얻는 장점이 이를 해결하기 위해 추가된 코드의 복잡도보다 작을것이라 생각했습니다. 즉, 애초에 controller - service - repository 의 3단계 계층형 구조를 도입할 단계가 아니라는 생각이 들었습니다.

그럼에도 요구사항에

라고 명확히 쓰여있기 때문에 계층형 구조를 적용하되 dto의 범위를 제한하지 않고 request를 repository로 전달하는 구조를 채택했습니다. (아무래도 이런 고민을 해보라는 코치님들의 함정이 아니었나 싶습니다..)

잘 고민해주시고 정리도 잘해보셨네요!
습관적으로 작성하게 되는 코드들에 대한 고민을 한번쯤 해보는 기회가 되신거같아서 좋은거같습니다.

JunHoPark93 · 2024-04-25T15:10:10Z

src/main/java/roomescape/service/ReservationService.java

+        Reservation saved = reservationRepository.save(reservationRequest);
+        return toResponse(saved);


이 구조는 좋은거같네요 👍🏼

JunHoPark93 · 2024-04-25T15:10:58Z

src/main/resources/schema.sql

+    id   BIGINT       NOT NULL AUTO_INCREMENT,
+    name VARCHAR(255) NOT NULL,
+    date VARCHAR(255) NOT NULL,
+    time_id BIGINT,                           -- 컬럼 수정


요구사항상 time id는 항상 존재해야하지 않을까요?

JunHoPark93 · 2024-04-25T15:15:27Z

src/test/java/roomescape/integration/AdminIntegrationTest.java

+        jdbcTemplate.update("delete from reservation");
+        jdbcTemplate.update("ALTER TABLE reservation alter column id restart with 1");


이 부분은 테스트 실행 속도때문에 만들어주신거죠?

이 부분은 테스트 실행 속도때문에 만들어주신거죠?

각 테스트 메서드에서 데이터베이스가 격리되지 않기 때문에 추가한 코드입니다. 실행 속도때문이라고 추측하신 이유가 무엇인가요?? 왜 그런 말씀을 하신건지 전혀 감이 안오네요. ㅠㅠ

아 그렇군요~ 다른크루분들이 하셨던 고민들과 비슷해보여서 말씀드려보았어요. (ex #105 (comment))
방법의 차이니 그냥 넘어가셔도될거같습니다!

JunHoPark93 · 2024-04-25T15:15:57Z

src/test/java/roomescape/repository/JdbcTemplateReservationTimeRepositoryTest.java

+    }
+
+    @Test
+    @DisplayName("ReservationTime 을 잘 지우하는지 확인한다.")


테스트들도 꼼꼼하게 잘 작성해주셨네요!

테스트들도 꼼꼼하게 잘 작성해주셨네요!

체스 미션때 테스트에 대해 말씀해주셔서 방학 내내 테스트 관련 책만 봤습니다 ㅎㅎ

(방학때는 쉬셔야..🥲)

JunHoPark93 · 2024-04-25T15:18:38Z

src/main/java/roomescape/controller/ReservationTimeController.java

+    }
+
+    @DeleteMapping("/{id}")
+    public void delete(@PathVariable long id) {


이번 미션에서 특이한 점은 API 명세의 응답 상태코드가 일관성 없이 왔다갔다 수정된다는 것이었습니다...
아무리 고민해봐도 무슨 의도가 담긴 것인지 모르겠어서 모두 200 OK 로 응답하도록 했습니다.

상태코드에 대해서 참고자료를 보셨었나요?
구체적으로 어떤 부분이 이해가 안된지 말씀주시면 같이 이야기해봐도 좋을거같아요.

이번 미션에서 특이한 점은 API 명세의 응답 상태코드가 일관성 없이 왔다갔다 수정된다는 것이었습니다...
아무리 고민해봐도 무슨 의도가 담긴 것인지 모르겠어서 모두 200 OK 로 응답하도록 했습니다.

상태코드에 대해서 참고자료를 보셨었나요? 구체적으로 어떤 부분이 이해가 안된지 말씀주시면 같이 이야기해봐도 좋을거같아요.

결론 : LMS 내의 오타(?)가 굴린 스노우볼이었다.

LMS 에서 미션 설명 해주는 페이지에서

이런식으로 테스트 코드를 제공해 줬는데요, 지금은 LMS가 수정되었지만 PR 보내는 시점에만 해도 테스트 코드에서 상태 코드를 검증하는 부분이 API 명세와는 다르게 200이 아닌 특화된 상태코드였습니다. 이마저도 다음 단계 설명에서 제공해주는 테스트코드는 또 200으로 검증하기도 했습니다... 이 부분을 코치님들한테 여쭤보았을 때 모호하게 대답해주셔서 그저 200 으로 지정했습니다.

robinjoon · 2024-04-27T19:46:32Z

안녕하세요 제이! 미션에 집중해야하는데, 이사를 해야 해서 집 치우느라 이제야 다시 리뷰 요청 드립니다! 짚어주신 id 관련 통일성 부분과 데이터베이스 스키마 부분 반영했고, 질문주신 코멘트에 답변 달았습니다!

JunHoPark93

안녕하세요 로빈~
리뷰반영하느라 고생많으셨습니다.
계층형구조에 대해서 고민도 많이 해주셨네요!
다음 미션도 화이팅입니다~

JunHoPark93 · 2024-04-28T11:58:06Z

src/test/java/roomescape/repository/CollectionReservationTimeRepository.java

@@ -37,9 +37,9 @@ public List<ReservationTime> findAll() {
    }

    @Override
-    public void delete(long timeId) {
+    public void delete(long id) {


다른부분들도 잘 찾아서 반영해주셨네요 👍🏼

robinjoon added 16 commits April 23, 2024 10:42

feat: 데이터베이스 연동을 위한 설정 및 테스트 코드 추가

ffe11dc

fix: 데이터베이스 연동 테스트 코드에서 필요 없는 설정 삭제

4e47c32

feat: 데이터베이스 연동 구현

1234c01

fix: 테스트에서 데이터베이스를 사용하지 않는 오류 수정

3f11120

docs: step 7 - 9 요구 사항 문서 정리

71ad3fc

feat: 7단계 요구사항 구현

ac73aea

feat: 8단계 요구사항 구현

5997b65

fix: Reservation 주 생성자 하나로 수정

ed2abfe

docs: 8단계에서 변경된 API 명세 반영

00eb67f

docs: 9단계 요구사항 추가

91824dd

feat: 9단계 요구사항 구현

3cc4e18

refactor: 테스트에서만 사용하는 코드 제거

96df623

test: Repository Test 추가

0a25fd6

refactor: 메서드 분리

749d788

style: 필요없는 세미콜론 삭제

7418291

style: DisplayName 추가

6a7413d

JunHoPark93 requested changes Apr 25, 2024

View reviewed changes

robinjoon added 2 commits April 27, 2024 13:52

refactor: 식별자를 의미하는 변수명 id로 통일

fe28aa4

fix: reservation 테이블의 time_id 컬럼 NOT NULL 속성 부여

5d6b6ad

JunHoPark93 approved these changes Apr 28, 2024

View reviewed changes

JunHoPark93 merged commit bf936e6 into woowacourse:robinjoon Apr 28, 2024

robinjoon mentioned this pull request May 4, 2024

로빈 1단계 제출 woowacourse-6th-code-review-study/spring-roomescape-member#8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4 - 9단계 방탈출 예약 관리] 로빈(임수빈) 미션 제출합니다. #155

[4 - 9단계 방탈출 예약 관리] 로빈(임수빈) 미션 제출합니다. #155

robinjoon commented Apr 25, 2024

JunHoPark93 left a comment

JunHoPark93 Apr 25, 2024

JunHoPark93 Apr 25, 2024

JunHoPark93 Apr 25, 2024

robinjoon Apr 27, 2024

JunHoPark93 Apr 28, 2024

JunHoPark93 Apr 25, 2024

JunHoPark93 Apr 25, 2024

JunHoPark93 Apr 25, 2024

robinjoon Apr 27, 2024

JunHoPark93 Apr 28, 2024

JunHoPark93 Apr 25, 2024

robinjoon Apr 27, 2024

JunHoPark93 Apr 28, 2024

JunHoPark93 Apr 25, 2024

robinjoon Apr 26, 2024

robinjoon commented Apr 27, 2024

JunHoPark93 left a comment

JunHoPark93 Apr 28, 2024

		Reservation saved = reservationRepository.save(reservationRequest);
		return toResponse(saved);

		jdbcTemplate.update("delete from reservation");
		jdbcTemplate.update("ALTER TABLE reservation alter column id restart with 1");

[4 - 9단계 방탈출 예약 관리] 로빈(임수빈) 미션 제출합니다. #155

[4 - 9단계 방탈출 예약 관리] 로빈(임수빈) 미션 제출합니다. #155

Conversation

robinjoon commented Apr 25, 2024

미션 중 새로 알게된 내용

JunHoPark93 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

robinjoon commented Apr 27, 2024

JunHoPark93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment