[🚀 사이클2 - 미션 (예약 변경/취소와 에러 처리)] 보예 미션 제출합니다. by boyekim · Pull Request #445 · woowacourse/spring-roomescape-member

boyekim · 2026-05-14T05:17:45Z

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?

베이스 코드 선택 체크

이전 미션의 내 코드에서 시작
이전 미션의 페어의 코드에서 시작

어떤 부분에 집중하여 리뷰해야 할까요?

1. 예약 생성 정책 검증 추가

지나간 날짜의 예약 생성을 거부하도록 검증을 추가했습니다.
오늘 날짜의 예약인 경우, 현재 시각보다 이전 시간은 예약할 수 없도록 처리했습니다.
같은 날짜 + 시간 + 테마 조합에 이미 예약이 존재하면 중복 예약으로 거부하도록 했습니다.
테스트에서 시간 의존 로직을 안정적으로 검증할 수 있도록 Clock을 주입받는 구조로 변경했습니다.

2. 관리자와 사용자의 예약 삭제 정책 분리

관리자 삭제와 사용자 삭제의 정책이 다르다고 판단하여 메서드를 분리했습니다.

관리자는 지난 예약도 삭제할 수 있습니다.
사용자는 이미 지난 날짜/시간의 예약을 삭제할 수 없습니다.

이를 위해 deleteReservationByAdmin과 deleteUserReservation 흐름을 분리했습니다.
처음에는 같은 삭제 로직을 재사용할 수 있을지 고민했지만, 역할별 정책이 다르기 때문에 명시적으로 분리하는 편이 도메인 규칙을 더 잘 드러낸다고 판단했습니다.

4. 예외 응답 구조 정리

에러 응답은 아래 형태로 주어지는데요, 보통 어떤 식으로 내려주는지 궁금합니다!!

{
  "code": "ERROR_CODE",
  "message": "사용자에게 보여줄 메시지"
}

5. 이전 리뷰 코멘트에 대해

Not found 예외에 대해

#347 (comment)
https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.4 요기를 참고하면 현재 상황에서 NOT_FOUND가 어색한 상황은 아니라고 생각했어요. 그래서 우선 코드를 200으로 수정하지는 않은 상태입니다..!!
다만 말씀해주신 서비스의 관리 측면에서 생각해볼 수 있어서 너무 좋았습니다! 감사합니다 ㅎㅎ 다음에 말씀해주신 상황을 마주하면 이런 해결점을 바로 생각해낼 수 있을 것 같아요 감사합니다~!

로깅에 대해

#347 (comment)
존재하지 않는 자원에 대해 지속해서 요청이 들어올때에는, 캐싱이 잘못되어있는 것일 수도 있구 추적이 필요한 상황이 있을 수도 있다고 생각했는데요!
요 상황보다 말씀해주신 상황 ⬇️

과도한 삭제 요청이라(예를 들면 디도스 공격 처럼 과도한 공격으로 서버 과부하시키는 형태) → rate limiting 도입
잘못된 id를 어딘가에서 받아서 → id 생성/반환 부분 테스트로 검증

이 더 빈번하게 발생하는 것일까요?! 에코의 경험이 궁금합니다

가능 시간 조회 api에 대해

#347 (comment)
남겨주신
"가능한 시간을 여부를 반환하기 때문에 일반적인 조회용으로 사용될 수 있는 times 보다는 의미가 더 내포되어있다고 생각했습니다."
라는 말에 크게 동의합니다~! 엔드포인트 수정해보았습니다.

errors의 패키지에 대해

#347 (comment)
코멘트 반영하여 패키지 분리했습니다!

echo724

안녕하세요 보예!
사이클2도 깔끔하고 가독성 있게 잘 작성해주셨네요! 대부분의 예외/에러 케이스가 커버됐고 메세지와 타입도 적절하게 잘 사용하신 것 같습니다. 이번 변경 요청에서는

컨트롤러, e2e 테스트 케이스가 아예 없는데 실제로 요청 바디가 비거나 잘못된 값의 요청이 정상 처리로 되는 경우들이 테스트 케이스로 검증이 됐을 것 같아요. 한 번 문제 상황 커멘트로 남겼으니 확인해보시고 해당 케이스들 검증할 수 있는 테스트들 작성해보시면 좋을 것 같습니다!
위에서 언급했던 것처럼 몇몇 예외 상황 커버가 안된 것 같습니다. 확인해보시고 적절한 예외처리해주시면 좋을 것 같습니다!

아래는 질문에 대한 답입니다! (몇몇은 코드 커멘트로 남겼습니다)

Not Found: 저도 이 부분 크루들 공통적으로 피드백이 드렸던 부분이라 저도 좀 더 설명을 드리기 위해 알아봤습니다. 일단 404의 경우 두 가지 의미가 있을 수 있는데요, 1. 실제 url이 존재하지 않는 경우 (엔드포인트 맵핑/라우터/컨트롤러 등이 없는 상황, 브라우저에서 흔히 보는 404) 2. 비지니스적으로 유저가 요청한 데이터가 없는 경우가 있을 것입니다. 전자의 경우 예를 들어, 새로운 엔드포인트를 기준으로 새로운 기능이 만들었을때, 프론트엔드나 앱은 배포 했는데 서버가 배포가 되지 않는 상황에서 요청 응답으로 404 코드가 갑자기 증가할 수 있죠? 이런 경우는 장애 상황이라 별도의 조치가 필요해집니다. 근데 비지니스 케이스의 경우에도 404를 반환하면 장애 상황이 아닌데 장애 상황으로 인식될 수 있겠죠? 이런 상황 때문에 특히 현업에서는 비지니스 케이스에 대해 404를 반환하지 않는 것 같습니다.

커멘트 확인해보시고 이해 안되시거나 더 궁금하신 부분 있으면 메세지 보내주세요. 화이팅입니다~

echo724 · 2026-05-14T17:26:39Z

+        if (request.startWhen() != null) {
+            reservationDate = reservationDateRepository.findByDate(request.startWhen())
+                .orElseThrow(() -> new NotFoundException(ReservationDateErrors.RESERVATION_DATE_NOT_EXIST));
+        }
+        return reservationDate;
+    }
+
+    private ReservationTime getReservationTime(UpdateReservationRequest request, ReservationTime reservationTime) {
+        if (request.startAt() != null) {


현재 구조에서는 request.startAt과 request.startWhen 둘 다 null이 되는 경우가 없습니다! 실제로 빈 바디로 리퀘스트로 보내면 204가 뜨는데 이유가 뭘까요?

둘 다 null일 경우 처리해주는 로직이 없다는 의미인가요?!
빈 바디로 보내면 기존 예약의 시간과 날짜를 그대로 사용하여 로직을 실행하기 때문에 204라고 생각합니다!
수정할 것이 없으면 기존 데이터를 그대로 사용하는게 자연스럽다고 생각했는데 잘못된 생각일까요??

네네 제가 다시 봐도 문장을 너무 헷갈리게 작성했네요;; 죄송합니다.
네네 둘 다 null일 경우를 처리해주는 로직이 없다는 의미였습니다! 잘 이해하고 답 남겨주셨네요 ㅎㅎ

네네 말씀하신대로, 빈 바디면 기존 데이터를 그대로 수정할 것이 없다는 의미에서 204를 반환하는 것도 의미상으로 괜찮다고 생각합니다. 이 영역은 API 사용하는 당사자들끼리 합의하면 좋은 문제인 것 같아요.
제가 생각했던 내용은, 아무래도 업데이트 요청에 빈 바디로 보내는 행위 자체가 유저가 잘못 요청한 상황이다라고 판단했던 건데 이 부분은 해석하기에 따라 다를것 같네요! 설명 적어주셔서 감사합니다~

echo724 · 2026-05-14T19:56:12Z

    }
+
+    @Transactional
+    public void deleteUserReservation(Long id) {


서비스 메서드이니 db operation 이름보단 비지니스 케이스인 cancel를 이름에 담는 것은 어떨까요? 나중에 내부적으로 cancel을 처리하는 로직이 바뀌더라도 (is_cancelled를 도입한다거나 softdelete를 한다거나) 메서드 이름이 변경될 일이 없을 것 같습니다!

오 너무 좋은 것 같아요! 반영했습니다!! 어드민용 서비스 메서드도 cancel을 쓰도록 변경했어요. 추가로 Controller에서도 메서드 네이밍을 cancel로 변경해주었습니다!

echo724 · 2026-05-14T20:01:03Z

+    public boolean existsReservation(Long timeId, Long dateId, Long themeId) {
+        Integer count = jdbcTemplate.queryForObject(
+            COUNT_RESERVATION_BY_TIME_AND_DATE_AND_THEME_SQL,
+            Integer.class,
+            timeId,
+            dateId,
+            themeId
+        );
+        return count != null && count > 0;
+    }
+
+    @Override
+    public boolean existsOtherReservation(Long id, Long timeId, Long dateId, Long themeId) {
+        Integer count = jdbcTemplate.queryForObject(
+            COUNT_OTHER_RESERVATION_BY_TIME_AND_DATE_AND_THEME_SQL,
+            Integer.class,
+            id,
+            timeId,
+            dateId,
+            themeId
+        );
+        return count != null && count > 0;
+    }


두 메서드 모두 존재 여부만 판단하는데 쿼리는 count를 쓰네요? EXISTS 쿼리가 의도에 더 맞지 않을까요?

그러네요..!! exist로 변경하고, 쿼리문 네이밍도 의도를 더 잘 나타내도록 EXIST로 변경해줬습니다.

echo724 · 2026-05-14T20:02:24Z


 @RestController
 @RequiredArgsConstructor
 public class ReservationController {


서비스 테스트의 경우에는 잘 작성해주셨는데 Controller 혹은 e2e 테스트는 따로 안보이네요? 비지니스 로직이 어차피 서비스에서 테스트가 됐다고 생각하실 수 도 있는데, 각각의 관심사와 대상이 조금은 다르다고 생각합니다. 어떤 테스트 케이스들이 있을 수 있을까요?

boyekim

안녕하세요 에코! 꼼꼼하게 리뷰해주셔서 감사합니다.

컨트롤러 테스트와 e2e 테스트를 제가 잘 작성하는 편이 아니다보니 학습을 하며 진행해보았습니다. 리뷰 요청이 늦어져서 죄송해요.

이번에 리뷰 반영을 진행하면서 컨트롤러 테스트, e2e 테스트를 처음으로 좀 학습해본 것 같아요.
학습하며 궁금한 점이 생겼는데요, 다음과 같습니다!

컨트롤러 테스트와 e2e 테스트로 어느범위까지 검증해야할까요?
- 컨트롤러 테스트와 e2e 테스트 전부 필수 요청 필드의 누락, 파라미터 누락, 헤더 토큰 누락과 같이 요청값이 잘못된 경우를 테스트했습니다. 다만 타입 검증 등 도메인 정책이나 비즈니스 로직에 관련된 것은 서비스 테스트로 검증이 된다고 생각해서 추가하지 않았는데,,, 이 경계를 정하는 것이 좀 애매하다고 느껴졌습니다. 그리고 컨트롤러 테스트와 e2e 테스트가 검증하는 방법은 서로 다르지만, 검증하는 범위가 다소 겹친다고 생각이 들어서, 둘의 테스트 목적을 어떻게 정의해야하는지에 대해 에코의 의견이 궁금합니다.

boyekim · 2026-05-16T12:20:13Z

    }
+
+    @Transactional
+    public void deleteUserReservation(Long id) {


오 너무 좋은 것 같아요! 반영했습니다!! 어드민용 서비스 메서드도 cancel을 쓰도록 변경했어요. 추가로 Controller에서도 메서드 네이밍을 cancel로 변경해주었습니다!

boyekim · 2026-05-16T13:23:39Z

+        if (request.startWhen() != null) {
+            reservationDate = reservationDateRepository.findByDate(request.startWhen())
+                .orElseThrow(() -> new NotFoundException(ReservationDateErrors.RESERVATION_DATE_NOT_EXIST));
+        }
+        return reservationDate;
+    }
+
+    private ReservationTime getReservationTime(UpdateReservationRequest request, ReservationTime reservationTime) {
+        if (request.startAt() != null) {


둘 다 null일 경우 처리해주는 로직이 없다는 의미인가요?!
빈 바디로 보내면 기존 예약의 시간과 날짜를 그대로 사용하여 로직을 실행하기 때문에 204라고 생각합니다!
수정할 것이 없으면 기존 데이터를 그대로 사용하는게 자연스럽다고 생각했는데 잘못된 생각일까요??

boyekim · 2026-05-16T14:12:01Z

+    public boolean existsReservation(Long timeId, Long dateId, Long themeId) {
+        Integer count = jdbcTemplate.queryForObject(
+            COUNT_RESERVATION_BY_TIME_AND_DATE_AND_THEME_SQL,
+            Integer.class,
+            timeId,
+            dateId,
+            themeId
+        );
+        return count != null && count > 0;
+    }
+
+    @Override
+    public boolean existsOtherReservation(Long id, Long timeId, Long dateId, Long themeId) {
+        Integer count = jdbcTemplate.queryForObject(
+            COUNT_OTHER_RESERVATION_BY_TIME_AND_DATE_AND_THEME_SQL,
+            Integer.class,
+            id,
+            timeId,
+            dateId,
+            themeId
+        );
+        return count != null && count > 0;
+    }


그러네요..!! exist로 변경하고, 쿼리문 네이밍도 의도를 더 잘 나타내도록 EXIST로 변경해줬습니다.

echo724

안녕하세요 보예!
e2e와 controller 테스트들 다 작성해주셨네요! 아무래도 학습까지 같이 하면서 작성하는게 쉽지 않았을텐데, 고생 많으셨습니다! 대부분 커멘트도 잘 반영해주시고 특별히 요청 사항이 없어서 이번에 머지하도록 하겠습니다! 이전 리뷰 몇몇 커멘트에 답 남겼습니다. 확인 같이 해보시고 다음 선택 미션과 남은 레벨도 화이팅입니다~!

echo724 · 2026-05-19T05:44:57Z

+import roomescape.support.exception.errors.ReservationErrors;
+
+@WebMvcTest(ReservationController.class)
+class ReservationControllerTest {


컨트롤러 테스트도 제대로 잘 작성하셨네요! 시간상 e2e 테스트 controller테스트 둘 다 작성하는게 오래 걸렸을텐데 학습도 하시고 하신거면 꽤나 시간을 쓰셨을 것 같습니다!

echo724 · 2026-05-19T05:48:11Z

+
+    @Test
+    @DisplayName("예약자 이름 조회 시 이름이 누락되면 예외가 발생한다.")
+    void getUserReservationsWithoutName() throws Exception {


controller vs e2e 텟스트에 대한 질문을 주셨는데, 해당 경우가 아무래도 controller 테스트에 들어가면 좋고 e2e에 들어가기에는 안좋은 케이스 일 것 같습니다. 물론 e2e에서 예외 상황과 엣지 케이스를 커버해도 되긴 하지만, 아무래도 dependency setup부터 관련된 모든 의존 객체를 띄우기 때문에 비용이 비싸고 따라서 테스트 실행 시간이 오래 걸립니다.

이런 요청값이나 파라미터 누락의 경우, 컨트롤러의 책임이기도 하고 e2e에 비해 비용이 싸기 때문에 모든 예외와 엣지케이스를 처리해도 무리가 없겠죠?

boyekim added 17 commits May 12, 2026 15:11

test: Fake 객체 리팩토링

7dd3764

refactor: 지난 날짜 및 시간 예약 불가 예외 처리

c884f2d

refactor: 중복 예약 예외 처리 추가

ccdd2de

refactor: AdminReservationController 분리

96c1806

feat: 사용자 이름으로 예약 조회 기능 추가

853acd8

feat: 사용자 이름으로 예약 삭제 기능 추가

a3a8fac

refactor: 사용자 예약 조회 api url 수정

75541fa

refactor: 에러 포맷팅 누락 해결

55b2aa8

feat: 예약 수정 기능 구현

5a9b56e

feat: 추가된 기능 클라이언트 코드 추가

a1bc6ec

refactor: 더미데이터 수정

58a4e85

docs: 기능 명세 업데이트

ea28c0e

refactor: @Valid로 입력값 검증

f7f3535

refactor: 불필요한 개행 삭제

30bbe9c

refactor: ErrorCode 네이밍 변경

ca09e57

refactor: 가능 시간 조회 api url 수정

af36d71

refactor: errors 패키지 추가

248db1c

echo724 self-requested a review May 14, 2026 16:28

echo724 requested changes May 14, 2026

View reviewed changes

echo724 mentioned this pull request May 15, 2026

[🚀 사이클2 - 미션 (예약 변경/취소와 에러 처리)] 루드비코 미션 제출합니다. #482

Merged

5 tasks

boyekim added 10 commits May 16, 2026 20:07

refactor: 엣지케이스 예외 추가

22eb9e0

refactor: 불필요한 @Transactional 어노테이션 삭제

33bfaaa

refactor: 불필요한 로깅 삭제

07f4ed3

refactor: 메서드 네이밍 변경

fc2b64d

refactor: 날짜와 시간 추출 로직 수정

b199646

refactor: 시간 조회 시 theme id및 date id 검증 로직 추가

62c27a9

refactor: 동일 자료형 검증 형태 통일

e84bd8a

test: 테마 테스트 추가

54adb7b

refactor: 쿼리문 수정

991c108

test: ReservationControllerTest 추가

71f01e8

boyekim added 14 commits May 17, 2026 16:12

test: AdminReservationControllerTest 추가

88d4f62

refactor: ReservationDate 어드민 컨트롤러 분리

3cb6be4

test: ReservationDate 컨트롤러 테스트 추가

d823f2d

refactor: ReservationTime 어드민 기능 분리

0bf7829

test: ReservationTime 컨트롤러 테스트 추가

45a9695

refactor: Theme 컨트롤러 관리자 기능 분리

4fe8125

refactor: ReservationTime 패키지 수정

c574b71

test: Theme 컨트롤러 테스트 추가

82f6ff4

refactor: yml 필요한 필드 추가

c799ec4

test: Theme e2e 테스트 추가

de703ac

test: ReservationTime e2e 테스트 추가

3b1c672

test: ReservationDate e2e 테스트 추가

5d2ff24

test: Reservation e2e 테스트 추가

f0af145

refactor: 테스트 네이밍 컨벤션 통일

877355c

boyekim commented May 18, 2026

View reviewed changes

boyekim added 2 commits May 18, 2026 14:42

refactor: 메서드 네이밍 수정

c0697ab

feat: 테스트마다 고유한 db 이름 사용하도록 yml 추가

4f236dd

echo724 approved these changes May 19, 2026

View reviewed changes

echo724 merged commit 3755ac2 into woowacourse:boyekim May 19, 2026

Conversation

boyekim commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

체크 리스트

베이스 코드 선택 체크

어떤 부분에 집중하여 리뷰해야 할까요?

1. 예약 생성 정책 검증 추가

2. 관리자와 사용자의 예약 삭제 정책 분리

4. 예외 응답 구조 정리

5. 이전 리뷰 코멘트에 대해

Uh oh!

echo724 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

boyekim May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

echo724 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

boyekim commented May 14, 2026 •

edited

Loading

boyekim May 16, 2026 •

edited

Loading

boyekim left a comment •

edited

Loading

boyekim May 16, 2026 •

edited

Loading