-
Notifications
You must be signed in to change notification settings - Fork 83
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
[1 - 3단계 방탈출 사용자 예약] 페드로(류형욱) 미션 제출합니다. #33
Changes from 35 commits
27cbcb0
47665fb
319e6c8
5f6f8ff
4d62cca
63bec82
1a350aa
bbeb00a
4ccbd4b
01a4652
c677f67
425c94f
4c3b7a1
b19bdd5
fbd7f15
be0322f
a19edca
92bc053
d2af8ac
41f4b4f
4e5a8ab
7034dfc
35294cb
0423fa0
a4ef28d
41587b5
5c7b024
2cff101
2936539
3d4ab2b
7b2c065
2a6e14d
f0fcb30
c90785b
c9923d0
4a8bc9b
6937af0
7a6a868
aade4c3
9f46f9c
ac1adb5
630ff06
2f628e4
07f4c95
7834789
0fc2cdd
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,43 @@ | ||
# 1단계 - 예외 처리와 응답 | ||
## 예약 | ||
- [x] 지나간 날짜와 시간에 대한 예약 생성 시 예외가 발생한다. | ||
- [x] 중복 예약 시 예외가 발생한다. | ||
- [x] 예약 생성 시 예약자명이 비어있으면 예외가 발생한다. | ||
- [x] 날짜가 유효하지 않은 값일 시 예외가 발생한다. | ||
- [x] 등록되지 않은 시간에 대한 예약 생성 시 예외가 발생한다. | ||
- [x] 요청 본문이 JSON 형식이 아닐 경우 예외가 발생한다. | ||
|
||
## 시간 | ||
- [x] 중복된 시간 생성 시 예외가 발생한다. | ||
- [x] 시간 삭제 시 참조된 예약이 있으면 예외가 발생한다. | ||
- [x] 시간이 유효하지 않은 값일 시 예외가 발생한다. | ||
- [x] 요청 본문이 JSON 형식이 아닐 경우 예외가 발생한다. | ||
|
||
# 2단계 - 테마 추가 | ||
## 테마 | ||
- [x] 관리자는 테마를 추가할 수 있다. | ||
- [x] 중복된 테마 이름으로 추가 시 예외가 발생한다. | ||
- [x] 테마 추가 시, 테마 이름, 설명, 썸네일 중 하나라도 비어 있으면 예외가 발생한다. | ||
- [x] 관리자는 테마를 조회할 수 있다. | ||
- [x] 관리자는 테마를 삭제할 수 있다. | ||
- [x] 테마 삭제 시 참조된 예약이 있으면 예외가 발생한다. | ||
|
||
## 예약 | ||
- [x] 방탈출 예약 시 테마 정보를 포함하여 예약을 추가한다. | ||
- [x] 등록되지 않은 테마에 대한 예약 생성 시 예외가 발생한다. | ||
- [x] 동시간대에 이미 예약된 테마를 예약하는 경우 예외가 발생한다. | ||
|
||
## 화면 | ||
- [x] `/admin/theme` 요청 시 `templates/admin/theme.html`가 응답한다. | ||
- [x] `/admin/reservation` 요청 시 `templates/admin/reservation-new.html`가 응답한다. | ||
|
||
# 3단계 - 사용자 기능 | ||
## 예약 | ||
- [x] `/reservation` 요청 시 `templates/reservation.html`가 응답한다. | ||
|
||
## 테마 | ||
- [x] `/` 요청 시 `templates/index.html`가 응답한다. | ||
- [x] 최근 일주일을 기준으로 예약이 많은 상위 10개 테마를 조회할 수 있다. | ||
|
||
## 시간 | ||
- [x] 선택된 테마에 따라 예약 가능 여부가 포함된 시간을 조회한다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
package roomescape.controller; | ||
|
||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.*; | ||
import roomescape.service.dto.theme.ThemeCreateRequest; | ||
import roomescape.service.dto.theme.ThemeResponse; | ||
import roomescape.service.ThemeService; | ||
|
||
import java.net.URI; | ||
import java.util.List; | ||
|
||
@RestController | ||
@RequestMapping("/themes") | ||
public class ThemeController { | ||
private final ThemeService themeService; | ||
|
||
public ThemeController(ThemeService themeService) { | ||
this.themeService = themeService; | ||
} | ||
|
||
@PostMapping | ||
public ResponseEntity<ThemeResponse> createTheme(@RequestBody ThemeCreateRequest themeCreateRequest) { | ||
ThemeResponse theme = themeService.createTheme(themeCreateRequest); | ||
return ResponseEntity.created(URI.create("/themes/" + theme.id())) | ||
.body(theme); | ||
} | ||
|
||
@GetMapping | ||
public ResponseEntity<List<ThemeResponse>> readThemes() { | ||
List<ThemeResponse> themes = themeService.readThemes(); | ||
return ResponseEntity.ok(themes); | ||
Comment on lines
+28
to
+31
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. 개인적인 생각이지만 200을 줄거면 ResponseEntity를 주지 않아도 괜찮다고 생각해요. 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. 좋은 의견 공유 감사합니다👍 |
||
} | ||
|
||
@GetMapping("/{id}") | ||
public ResponseEntity<ThemeResponse> readTheme(@PathVariable Long id) { | ||
ThemeResponse themeResponse = themeService.readTheme(id); | ||
return ResponseEntity.ok(themeResponse); | ||
} | ||
|
||
@GetMapping("/popular") | ||
public ResponseEntity<List<ThemeResponse>> readPopularThemes() { | ||
List<ThemeResponse> themeResponses = themeService.readPopularThemes(); | ||
return ResponseEntity.ok(themeResponses); | ||
} | ||
|
||
@DeleteMapping("/{id}") | ||
public ResponseEntity<Void> deleteTheme(@PathVariable Long id) { | ||
themeService.deleteTheme(id); | ||
return ResponseEntity.noContent().build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package roomescape.controller; | ||
|
||
import org.springframework.stereotype.Controller; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
|
||
|
||
@Controller | ||
@RequestMapping("/reservation") | ||
public class UserReservationController { | ||
@GetMapping | ||
public String readUserReservation() { | ||
return "reservation"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
package roomescape.domain; | ||
|
||
import roomescape.exception.BadRequestException; | ||
|
||
import java.time.LocalDate; | ||
|
||
public class Reservation { | ||
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 boolean isDuplicated(Reservation other) {
return date.equals(other.date)
&& time.getId().equals(other.time.getId())
&& theme.getId().equals(other.theme.getId());
} Reservation에서는 이런 코드를 찾지 못했는데 혹시 어디를 말씀하시는걸까요? id가 부여된 객체의 동등성은 id로 해야겠죠. 코멘트를 남긴 의도는 exist 쿼리 등을 활용해서 존재하면 안 될 객체는 생성하지 않는 게 나을 것 같다는 의도였습니다. 잘 변경해주신 것 같네요. 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.
넵 맞습니다! 처음에는 EXIST 와 같은 쿼리도 '기능'을 DB에 의존적인 쿼리로 해결하는 부분이라고 생각하여 최대한 지양하려고 했는데, 범블비의 코멘트를 보고 생각이 조금 바뀌었습니다. 어차피 도메인의 영속화를 위해 관계형 DB를 사용하기로 결정했다면, DB에서 제공하는 기능들을 잘 활용하는 것도 DB를 DB답게 사용하는 일인 것 같아요. 물론 말씀해 주신 대로 로직과 관련된 부분을 최대한 도메인에 둬야 한다는 것에는 백번 동의합니다👍 |
||
|
@@ -8,20 +10,39 @@ public class Reservation { | |
private final String name; | ||
private final LocalDate date; | ||
private final ReservationTime time; | ||
private final Theme theme; | ||
|
||
public Reservation(Long id, String name, LocalDate date, ReservationTime time) { | ||
public Reservation(Long id, String name, LocalDate date, ReservationTime time, Theme theme) { | ||
validateName(name); | ||
this.id = id; | ||
this.name = name; | ||
this.date = date; | ||
this.time = time; | ||
this.theme = theme; | ||
} | ||
|
||
private void validateName(String name) { | ||
if (name.isBlank()) { | ||
throw new BadRequestException("이름은 공백일 수 없습니다."); | ||
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. 도메인에서 "Request"의 존재를 알아야할까요? 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. 클라이언트에서 웹서버로의 요청 뿐만 아니라, 객체 간의 메시지 전달 역시 확실히 예외가 발생한 이유에 따라 서로 다른 예외를 던지도록 해 두는 것이 유지보수 측면에서 좋을 것 같아요. |
||
} | ||
} | ||
|
||
public Reservation(String name, LocalDate date, ReservationTime reservationTime) { | ||
this(null, name, date, reservationTime); | ||
public Reservation(String name, LocalDate date, ReservationTime reservationTime, Theme theme) { | ||
this(null, name, date, reservationTime, theme); | ||
} | ||
|
||
public Reservation(Long id, Reservation reservation) { | ||
this(id, reservation.getName(), reservation.getDate(), reservation.getTime()); | ||
this(id, reservation.name, reservation.date, reservation.time, reservation.theme); | ||
} | ||
|
||
public boolean isDuplicated(Reservation other) { | ||
return date.equals(other.date) | ||
&& time.getId().equals(other.time.getId()) | ||
&& theme.getId().equals(other.theme.getId()); | ||
} | ||
|
||
public boolean isSameUser(Reservation other) { | ||
return name.equals(other.getName()); | ||
} | ||
|
||
public Long getId() { | ||
|
@@ -39,4 +60,8 @@ public LocalDate getDate() { | |
public ReservationTime getTime() { | ||
return time; | ||
} | ||
|
||
public Theme getTheme() { | ||
return theme; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package roomescape.domain; | ||
|
||
import roomescape.exception.BadRequestException; | ||
|
||
public class Theme { | ||
|
||
private final Long id; | ||
private final String name; | ||
private final String description; | ||
private final String thumbnail; | ||
|
||
public Theme(Long id, String name, String description, String thumbnail) { | ||
validateNotBlank(name, description, thumbnail); | ||
this.id = id; | ||
this.name = name; | ||
this.description = description; | ||
this.thumbnail = thumbnail; | ||
} | ||
|
||
private void validateNotBlank(String name, String description, String thumbnail) { | ||
if (name.isBlank() || description.isBlank() || thumbnail.isBlank()) { | ||
throw new BadRequestException("테마의 정보는 비어있을 수 없습니다."); | ||
} | ||
} | ||
|
||
public Theme(String name, String description, String thumbnail) { | ||
this(null, name, description, thumbnail); | ||
} | ||
|
||
public Theme(Long id, Theme theme) { | ||
this(id, theme.name, theme.description, theme.thumbnail); | ||
} | ||
|
||
public boolean isDuplicated(Theme theme) { | ||
return name.equals(theme.name); | ||
} | ||
|
||
public Long getId() { | ||
return id; | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
||
public String getDescription() { | ||
return description; | ||
} | ||
|
||
public String getThumbnail() { | ||
return thumbnail; | ||
} | ||
} |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package roomescape.exception; | ||
|
||
public class BadRequestException extends RuntimeException { | ||
|
||
public BadRequestException(String message) { | ||
super(message); | ||
} | ||
} |
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.
params는 생략 가능할 것 같아요.
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.
현재
GET /times
에 대한 핸들러가 2개이고, 쿼리 파라미터의 여부에 따라서 다른 핸들러가 호출되도록 해 두었습니다.이 경우에도
params = {}
부분의 생략이 가능한가요?생략할 경우 충돌로 인해 다음과 같은 오류가 발생합니다.
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.
아이고 같은 경로의 API가 존재했군요. 죄송합니다 😅