-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3 - 4단계 방탈출 예약 대기] 카피(김상혁) 미션 제출합니다. #107
Changes from 24 commits
9d1798e
f5047da
ae81820
f2b0572
a14d4ed
fa12a9b
711f902
f3921df
c34818a
105b664
61eff92
4ffdea9
f537c88
904e2b5
b8efcd6
300be37
b7cf143
aee0764
c68f262
94255cb
8af3ed5
2a92b00
f5234a0
8d5500d
2a06d47
71d23b4
4488375
952c1a6
dd60fc5
8f0f7bd
72340aa
e6922d2
90b837e
e30e447
91161ad
a1df0cf
a439eec
3e8fe2c
aeb44de
26b29a6
e3498d6
0ea3648
fa306e2
f4e466e
4ce74c0
17c2c6a
15083f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package roomescape.controller.api; | ||
|
||
import jakarta.validation.Valid; | ||
import jakarta.validation.constraints.Positive; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.DeleteMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestBody; | ||
import org.springframework.web.bind.annotation.RestController; | ||
import roomescape.auth.AuthenticatedMember; | ||
import roomescape.domain.Member; | ||
import roomescape.domain.Reservation; | ||
import roomescape.domain.ReservationStatus; | ||
import roomescape.service.dto.request.ReservationSaveRequest; | ||
import roomescape.service.dto.response.ReservationResponse; | ||
import roomescape.service.reservation.ReservationCreateService; | ||
import roomescape.service.reservation.ReservationDeleteService; | ||
|
||
import java.net.URI; | ||
|
||
@RestController | ||
public class WaitingApiController { | ||
|
||
private final ReservationCreateService reservationCreateService; | ||
private final ReservationDeleteService reservationDeleteService; | ||
|
||
|
||
public WaitingApiController(ReservationCreateService reservationCreateService, | ||
ReservationDeleteService reservationDeleteService) { | ||
this.reservationCreateService = reservationCreateService; | ||
this.reservationDeleteService = reservationDeleteService; | ||
} | ||
|
||
@PostMapping("/waiting") | ||
public ResponseEntity<ReservationResponse> addWaiting(@RequestBody @Valid ReservationSaveRequest request, | ||
@AuthenticatedMember Member member) { | ||
Reservation newReservation = reservationCreateService.createReservation(request, member, ReservationStatus.WAITING); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. service 를 create, delete, find 로 나누시게 된 이유가 어떤걸까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. service 하나에서 모든 행동을 수행한다면 service가 너무 뚱뚱해지고 유지보수도 어려울 것이라 생각했습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 범용적인 방법은 아니기 때문에 다른 사람들과 함께 작업을 한다면 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일반적으로 사용되는 패턴이 아니기 때문에 팀원들에게 이해와 설명이 필요할 것 같습니다. CRD로 나누지 않고 하나의 클래스로 통일했습니다! |
||
return ResponseEntity.created(URI.create("/waiting/" + newReservation.getId())) | ||
.body(new ReservationResponse(newReservation)); | ||
} | ||
|
||
@DeleteMapping("/waiting/{id}") | ||
public ResponseEntity<Void> deleteWaiting(@PathVariable | ||
@Positive(message = "1 이상의 값만 입력해주세요.") long id) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아무나 id를 알면 지울 수 있을것 같아요🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 예약 대기는 예약자, 관리자만 삭제할 수 있도록 검증 로직을 추가하였습니다! |
||
reservationDeleteService.deleteReservation(id); | ||
return ResponseEntity.noContent().build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,48 @@ | ||||||||
package roomescape.controller.api.admin; | ||||||||
|
||||||||
import jakarta.validation.constraints.Positive; | ||||||||
import org.springframework.http.ResponseEntity; | ||||||||
import org.springframework.web.bind.annotation.DeleteMapping; | ||||||||
import org.springframework.web.bind.annotation.GetMapping; | ||||||||
import org.springframework.web.bind.annotation.PathVariable; | ||||||||
import org.springframework.web.bind.annotation.RestController; | ||||||||
import roomescape.auth.AuthenticatedMember; | ||||||||
import roomescape.domain.Member; | ||||||||
import roomescape.domain.Reservation; | ||||||||
import roomescape.service.dto.response.WaitingResponse; | ||||||||
import roomescape.service.reservation.ReservationDeleteService; | ||||||||
import roomescape.service.reservation.ReservationFindService; | ||||||||
|
||||||||
import java.util.List; | ||||||||
|
||||||||
@RestController | ||||||||
public class AdminWaitingApiController { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기도 AdminReservationApiController로 처리할 수 있지 않을까요? |
||||||||
|
||||||||
private final ReservationFindService reservationFindService; | ||||||||
private final ReservationDeleteService reservationDeleteService; | ||||||||
|
||||||||
public AdminWaitingApiController(ReservationFindService reservationFindService, | ||||||||
ReservationDeleteService reservationDeleteService) { | ||||||||
this.reservationFindService = reservationFindService; | ||||||||
this.reservationDeleteService = reservationDeleteService; | ||||||||
} | ||||||||
|
||||||||
@GetMapping("/admin/waitings") | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 클래스는 watings 인데 다른 클래스는 waiting이네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기존의 뼈대 코드에서 view를 반환하는 컨트롤러의 엔드포인트가 단수여서, api 컨트롤러는 복수로 만들어 놓았습니다. 하지만 view 랜더링하는 컨트롤러의 엔드포인트와 api의 엔드포인트가 동일하게 되어, 둘을 명확히 구분하여 호출하고 중복을 피하기 위해 맨앞에 |
||||||||
public ResponseEntity<List<WaitingResponse>> getWaiting(@AuthenticatedMember Member member) { | ||||||||
List<Reservation> waitings = reservationFindService.findWaitings(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. findByStatus(status) 형태로 |
||||||||
return ResponseEntity.ok( | ||||||||
waitings.stream() | ||||||||
.map(WaitingResponse::new) | ||||||||
.toList() | ||||||||
); | ||||||||
} | ||||||||
|
||||||||
@DeleteMapping("/admin/waitings/{id}") | ||||||||
public ResponseEntity<Void> deleteReservation( | ||||||||
@AuthenticatedMember Member member, | ||||||||
@PathVariable | ||||||||
@Positive(message = "1 이상의 값만 입력해주세요.") long id) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Positive 는 동작안할것 같네요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 클래스 레벨에 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 애플리케이션 플로우에서 검증이 동작할 일이 없을것 같습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 애플리케이션 플로우에서 동작할 일이 없다는 것은 이해를 했습니다! 다만 궁금한 것은 postman을 통해 해당 delete 메서드의 path의 id에 명시적으로 0을 입력한 경우 그리고 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 해봤을때는 500에러가 나는데 "1이상의 값만 입력해주세요" 라는 에러가 뜨는건가요? |
||||||||
reservationDeleteService.deleteReservation(id); | ||||||||
return ResponseEntity.noContent().build(); | ||||||||
} | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package roomescape.domain; | ||
|
||
import jakarta.persistence.EntityListeners; | ||
import jakarta.persistence.MappedSuperclass; | ||
import org.springframework.data.annotation.CreatedDate; | ||
import org.springframework.data.jpa.domain.support.AuditingEntityListener; | ||
|
||
import java.time.LocalDateTime; | ||
|
||
@MappedSuperclass | ||
@EntityListeners(AuditingEntityListener.class) | ||
public abstract class BaseTime { | ||
|
||
@CreatedDate | ||
private LocalDateTime createdAt; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LocalDateTime 으로 시간을 관리하면 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LocalDateTime은 타임존을 반영하지 않기 때문에 특정 지역의 시간대를 나타낼 수 없습니다. 그런데 LocalDateTime이든 ZoneDateTime이든 결국 시간을 관리하기 위해서는 db에 저장을 해야할 것 같습니다. 그래서 현재 생각나는 방식은 db에 UTC를 기준으로 저장하고, 클라이언트의 요청에 저장된 타임존 정보를 db에 저장되어있는 시간에 반영하여 반환해주는 방식을 생각해보았습니다. 실제로는 어떤 방식으로 관리를 하는지 궁금합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instant 타입에 대해서 학습해보면 좋을것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instant 타입은 UTC 기반의 절대 시간을 나타내고, 타임존 정보가 없습니다. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package roomescape.domain; | ||
|
||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
public class BookingStatus { | ||
|
||
private final Map<ReservationTime, Boolean> reservationStatus; | ||
|
||
private BookingStatus(Map<ReservationTime, Boolean> reservationStatus) { | ||
this.reservationStatus = reservationStatus; | ||
} | ||
|
||
public static BookingStatus of(List<ReservationTime> reservedTimes, List<ReservationTime> reservationTimes) { | ||
Map<ReservationTime, Boolean> reservationStatus = new HashMap<>(); | ||
for (ReservationTime reservationTime : reservationTimes) { | ||
reservationStatus.put(reservationTime, isReserved(reservedTimes, reservationTime)); | ||
} | ||
return new BookingStatus(reservationStatus); | ||
} | ||
|
||
private static boolean isReserved(List<ReservationTime> reservedTimes, ReservationTime reservationTime) { | ||
return reservedTimes.stream() | ||
.anyMatch(reservedTime -> reservedTime.equals(reservationTime)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. contains로 충분히 처리 가능하지 않을까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 훨씬 간단하게 표현할 수 있군요! 반영하였습니다! |
||
} | ||
|
||
public Map<ReservationTime, Boolean> getReservationStatus() { | ||
return reservationStatus; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object object) { | ||
if (this == object) { | ||
return true; | ||
} | ||
if (object == null || getClass() != object.getClass()) { | ||
return false; | ||
} | ||
BookingStatus that = (BookingStatus) object; | ||
return Objects.equals(reservationStatus, that.reservationStatus); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(reservationStatus); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||||||||||||||||||
package roomescape.domain; | ||||||||||||||||||||||
|
||||||||||||||||||||||
import jakarta.persistence.Entity; | ||||||||||||||||||||||
import jakarta.persistence.EnumType; | ||||||||||||||||||||||
import jakarta.persistence.Enumerated; | ||||||||||||||||||||||
import jakarta.persistence.FetchType; | ||||||||||||||||||||||
import jakarta.persistence.GeneratedValue; | ||||||||||||||||||||||
import jakarta.persistence.GenerationType; | ||||||||||||||||||||||
|
@@ -11,7 +13,7 @@ | |||||||||||||||||||||
import java.util.Objects; | ||||||||||||||||||||||
|
||||||||||||||||||||||
@Entity | ||||||||||||||||||||||
public class Reservation { | ||||||||||||||||||||||
public class Reservation extends BaseTime { | ||||||||||||||||||||||
|
||||||||||||||||||||||
@Id | ||||||||||||||||||||||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||||||||||||||||||||||
|
@@ -28,23 +30,30 @@ public class Reservation { | |||||||||||||||||||||
@ManyToOne(fetch = FetchType.LAZY) | ||||||||||||||||||||||
private Theme theme; | ||||||||||||||||||||||
|
||||||||||||||||||||||
public Reservation(Long id, Member member, LocalDate date, ReservationTime reservationTime, Theme theme) { | ||||||||||||||||||||||
validate(member, date, reservationTime, theme); | ||||||||||||||||||||||
@Enumerated(EnumType.STRING) | ||||||||||||||||||||||
private ReservationStatus reservationStatus; | ||||||||||||||||||||||
|
||||||||||||||||||||||
public Reservation(Long id, Member member, LocalDate date, ReservationTime reservationTime, | ||||||||||||||||||||||
Theme theme, ReservationStatus reservationStatus) { | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
지금 방법은 어떤 파라미터가 있는지 파악하기 힘든것 같아요ㅠ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분도 반영했습니다! |
||||||||||||||||||||||
validate(member, date, reservationTime, theme, reservationStatus); | ||||||||||||||||||||||
this.id = id; | ||||||||||||||||||||||
this.member = member; | ||||||||||||||||||||||
this.date = date; | ||||||||||||||||||||||
this.reservationTime = reservationTime; | ||||||||||||||||||||||
this.theme = theme; | ||||||||||||||||||||||
this.reservationStatus = reservationStatus; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
public Reservation(Member member, LocalDate date, ReservationTime reservationTime, Theme theme) { | ||||||||||||||||||||||
this(null, member, date, reservationTime, theme); | ||||||||||||||||||||||
public Reservation(Member member, LocalDate date, ReservationTime reservationTime, | ||||||||||||||||||||||
Theme theme, ReservationStatus reservationStatus) { | ||||||||||||||||||||||
this(null, member, date, reservationTime, theme, reservationStatus); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
protected Reservation() { | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
private void validate(Member member, LocalDate date, ReservationTime reservationTime, Theme theme) { | ||||||||||||||||||||||
private void validate(Member member, LocalDate date, ReservationTime reservationTime, | ||||||||||||||||||||||
Theme theme, ReservationStatus reservationStatus) { | ||||||||||||||||||||||
if (member == null) { | ||||||||||||||||||||||
throw new IllegalArgumentException("예약하려는 사용자를 선택해주세요."); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
@@ -57,6 +66,17 @@ private void validate(Member member, LocalDate date, ReservationTime reservation | |||||||||||||||||||||
if (theme == null) { | ||||||||||||||||||||||
throw new IllegalArgumentException("예약하려는 테마를 선택해주세요."); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
if (reservationStatus == null) { | ||||||||||||||||||||||
throw new IllegalArgumentException("예약상태가 지정되지 않았습니다."); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
public boolean isReserved() { | ||||||||||||||||||||||
return reservationStatus.isReserved(); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
public void changeWaitingToReserved() { | ||||||||||||||||||||||
this.reservationStatus = ReservationStatus.RESERVED; | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. state 를 받아서 변경하도록 하는게 좀 더 유연할것 같네요! |
||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
public Long getId() { | ||||||||||||||||||||||
|
@@ -79,6 +99,10 @@ public Theme getTheme() { | |||||||||||||||||||||
return theme; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
public ReservationStatus getReservationStatus() { | ||||||||||||||||||||||
return reservationStatus; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
@Override | ||||||||||||||||||||||
public boolean equals(Object object) { | ||||||||||||||||||||||
if (this == object) { | ||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
한줄에 120자는 넘지 않도록 해주세요!
다른곳들도 챙겨봐주세요 많은 곳들이 120자를 넘는것 같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구현에만 급급해서 가독성 측면에서 많이 부족했던 것 같습니다😅
전체적으로 수정해보았는데 다시 한번 확인해주실 수 있을까요?
파라미터 여러개 존재 하는 경우, 120자가 넘어가는 경우에 줄바꿈을 해주었습니다!