[Feat] 예약 현황 조회 / 예약 하기 기능 구현 #12
Conversation
- H2, Spring Data Jpa 설정 추가
- 깔끔한 테스트 작성을 위해 @nested 추가 - 실제 데이터 셋팅 로직(given) 추가
[Feat] 예약 현황 조회 기능 구현 및 단위 테스트 작성
- partySize 컬럼 추가 - bookingSlotId에 unique 속성 추가 - 정적 팩터리 메서드 추가
- isBooked 상태 변경 - 예약 상태를 반환
- findFirstByPlaceIdAndDateAndTimeAndIsBookedFalse()
- Step 1. 사용자 검증 - Step 2. 식당 검증 - Step 3. 예약이 가능한지 확인 - Step 4. 예약 생성 - Step 5. 이벤트 발행
| private final UserRepository userRepository; | ||
| private final PlaceRepository placeRepository; |
There was a problem hiding this comment.
🙋🏻♀️ 추후에 User / Place / Booking 모듈이 따로 배포될 정도로 확장이 되는 경우를 생각하면 이렇게 바로 Repository를 의존하면 안될 것 같은데.. 너무 먼 미래를 걱정하는 걸까요? 😂
| @Column(name = "booking_slot_id", nullable = false, unique = true) | ||
| private Long bookingSlotId; |
There was a problem hiding this comment.
🙋🏻♀️ 여러명이 동시에 예약을 하는 경우에 낙관적 락이나 레디스를 통해 해결해야겠다! 라고 생각했었는데, 개발을 하다보니 그냥 bookingSlotId에 unique를 걸어두면, 데이터베이스에 절대 같은 날짜/시간/테이블을 예약할 일이 없더라구요! 실무(?)에서는 어떤 방식으로 동시 예약을 막는지 여쭤봐도 될까요?
There was a problem hiding this comment.
🙋🏻♀️ 엔티티 간에 관계에서 객체를 사용하지 않고, id 값을 가지도록 했는데요! 그 이유는 1️⃣ 객체를 가지고 있으면 엔티티 간에 결합도가 높아져서 추후 모듈을 분리하기 어려울 것 같다고 판단을 하였고, 2️⃣ JPA를 사용하더라도 지연로딩을 걸고, 필요할 때 fetch join을 사용하니, 이렇게 id만 가지도 있더라도 필요할 때 join으로 가져오면 될 것 같다고 판단했습니다!
그런데.. 객체지향적으로 코드를 작성하기 위해 JPA를 사용하는데.. id로 관계를 맺는 것이 과연 맞는 선택인지가 고민이 됩니다.!
| @Column(name = "is_booked") | ||
| private boolean isBooked; |
There was a problem hiding this comment.
🙋🏻♀️ 사실 이 BookingSlot의 id를 가지고 있는 Booking이 있는지 확인하면 예약이 되었는지 알 수 있지만, 이렇게 컬럼을 따로 두면 성능면에서 더 좋을 것 같아서 반정규화를 했습니다! 그런데 처음부터 반정규화를 하는 것보다 데이터 중복을 최소로 하고, 성능 문제가 발생할 때 반정규화를 하는 것이 더 나을까요?
There was a problem hiding this comment.
저는 개인적으로 연관관계를 최소화하는 것이 더 바람직하다고 생각하는 입장이긴합니다. :-) 데이터가 필요할 때, 디비에서 데이터를 가져오는 것이 장기적으로는 더 바람직한 방식이 아닐까 하는 생각이 듭니다.
There was a problem hiding this comment.
확실히 'BookingSlot의 id를 가지고 있는 Booking이 있더라도, 결제 기한이 지났으면 예약할 수 있다.' 와 같이 요구사항이 변경되면.. 변경 포인트가 2개라 더 유지보수 하기 어려울 것 같네요..!🤔
| public BookingStatus book() { | ||
| if (isBooked) { | ||
| throw new IllegalArgumentException("이미 예약된 테이블입니다."); | ||
| } | ||
|
|
||
| isBooked = true; | ||
|
|
||
| return isDepositRequired() ? BookingStatus.PENDING_PAYMENT | ||
| : isConfirmRequired() ? BookingStatus.PENDING_CONFIRM : BookingStatus.CONFIRMED; | ||
| } |
There was a problem hiding this comment.
🙋🏻♀️ 이 메서드가 내부적으로 상태도 바꾸면서, 반환값을 가지는 것이 클라이언트 입장에서 혼란스러울까요..? 메서드가 행위를 하는 것인지, 조회를 하는 것인지 명확하게 분리를 하는 것이 디버깅에도 좋다고 들어서요..!
There was a problem hiding this comment.
말씀하신대로 메소드가 어떤 동작을 할 것인지 명확하게 하는 것이 좋은데, 이런 구조에서는 쉽지 않을 것 같아요. 이 얘기는 저 위에 비지니스 로직을 누가 가질 것인지와 연관지에서 멘토링 시간에 얘기해보면 좋을 것 같습니다.
| @@ -0,0 +1,13 @@ | |||
| package com.nowait.place.domain.model; | |||
|
|
|||
| public enum PlaceType { | |||
There was a problem hiding this comment.
이렇게 하면, PlaceType은 동적으로 추가가 불가능해질 것 같네요. :-)
There was a problem hiding this comment.
엔티티로 만들어서, 관리자가 수정할 수 있도록 하는 것이 좋을까요?
관리자 기능을 만들 생각을 못해서 해당 문제도 생각하지 못했네요😥 백로그에 담아 보겠습니다!
| private final ApplicationEventPublisher eventPublisher; | ||
|
|
||
| @Async | ||
| public void publishBookedEvent(Booking booking, Long placeId) { |
There was a problem hiding this comment.
음 ... 이 코드는 페어가 필요한 것 같은데, 일단 리스너는 아직 구현하지 않으신건가요? 아니면, 제가 놓친 코드가 있을까요? 그리고 그것과 별개로, ApplicationEventPublisher를 써서 하고 싶은 것이 뭐였을지 궁금하네요!!!! 😬
There was a problem hiding this comment.
예약 시나리오에서 예약이 완료되면, 사용자와 가게에 알림을 보내는 로직이 필요해서 넣었습니다! 그런데 아직 알림 기능을 개발하지 못해서 리스너는 구현이 되어 있지 않은 상태입니다😂 미리 코멘트를 남기거나, 필요할 때 작성하는게 더 나았을 것 같네요ㅜㅜ
🔗 관련 이슈
#10 #11
👩🏻💻 구현 내용
Booking,BookingSlot,Place,User엔티티 생성💬 PR 포인트 & 궁금한 점