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

haeseonlee · 2026-05-13T11:20:12Z

안녕하세요. 에코!

사이클2도 잘 부탁드립니다. 😊

체크 리스트

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

베이스 코드 선택 체크

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

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

사용자 본인 예약 변경 및 취소 API

메서드	URL	설명	성공 응답
DELETE	/reservations/{id}	예약 취소	204
PATCH	/reservations/{id}	예약 날짜 및 시간 변경	200

에러 응답 설계

에러 케이스

상황	상태코드
중복 예약	409 Conflict
지나간 날짜 예약	400 Bad Request
존재하지 않는 리소스	404 Not Found
예약이 존재하는 시간/테마 삭제	400 Bad Request
유효하지 않은 입력값	400 Bad Request

에러 응답 설계

GlobalExceptionHandler 클래스 생성
기존에는 예외 클래스에 @ResponseStatus를 붙이는 방식으로 에러 응답을 처리했습니다. 이 방식은 상태코드는 지정할 수 있지만, 응답 본문을 제어할 수 없어 Spring 기본 에러 페이지(HTML)가 그대로 노출되는 문제가 있었습니다. 특히
IllegalArgumentException처럼 @ResponseStatus가 없는 예외는 500으로 응답되어 사용자에게 서버 내부 오류가 그대로 노출됐습니다.

이를 해결하기 위해 @RestControllerAdvice를 사용한 GlobalExceptionHandler를 도입하여 모든 에러 응답을 아래 형식의 JSON으로 통일했습니다.

{
  “status”: INVALID_DATE,
  “message”: “지나간 날짜에는 예약할 수 없습니다.”
}

echo724 · 2026-05-13T17:36:46Z

안녕하세요 주니! 이번 PR 코드 변경사항에 없는 피드백 여기에다가 적어두도록 하겠습니다!

ReservationTimeRequest
1. 다른 Request/Response 클래스들도 마찬가지인데, 도메인 디렉토리에 있네요..? 서비스 계층에서 사용되고 따로 비지니스 로직은 없는 것으로보이는데 서비스에 더 가까운 클래스들 아닐까요?
2. 생성자에서 @JsonFormat 어노테이션이 들어가있네요? JSON Format은 API 계층과 관련된 것 같은데 다른 곳에 검증 포인트가 있어야하지 않을까요?
3. 만약 빈 바디로 /admin/times로 요청할 경우, 따로 검증이 없어서 NullPointException이 뜨고 500에러가 뜨네요!

echo724

반갑습니다 주니!
사이클 2 빠르게 구현해주셨네요! 고생 많았습니다~
대부분의 요구사항과 예외 상황 테스트 잘 작성해주셨네요! 적절하게 커스텀 예외와 메세지로 에러 상황을 유저에게 잘 전달했다고 생각합니다.
일단 남긴 커멘트들 중에서 좀 더 확인해주셨으면 하는 부분은:

대부분의 경우에는 커스텀 예외와 핸들러로 처리해주셨는데 500 에러가 발생하는 부분이 있습니다! 커멘트 부분 말고도 혹시 놓친 부분이 있을 수 있을지 고민해보시면 좋을 것 같아요. 프로덕션 환경에서 500 에러만큼 무서운게 없습니다 ㅠ
예약시 날짜에 대한 검증은 잘 해주셨는데 지나간 시간에 대한 검증은 없는 것 같습니다! 해당 케이스 검증과 테스트 추가해주세요~
예약 취소 상황에 대한 검증이 부족합니다! 확인해주시면 좋을 것 같습니다~
예외 상황에 대한 controller/e2e 테스트가 부족하다고 생각합니다.
마지막으로 404 HTTP status code에 대한 의견 남겨주시면 좋을 것 같습니다!

확인해보시고 혹시 이해한되시거나 설명이 더 필요한 부분 있으시면 커멘트나 메세지 남겨주세요~ 화이팅입니다!

echo724 · 2026-05-13T17:37:58Z

        if (reservationReq.date().isBefore(LocalDate.now())) {
-            throw new IllegalArgumentException("현재보다 이전의 날짜는 예약할 수 없습니다.");
+            throw new InvalidReservationException();


지나간 날짜·시간에 대한 예약 생성은 불가능하다.

요구사항대로면 시간도 검증을 해야할 것 같아요!

지나간 시간에 대해서도 검증 로직을 추가하였습니다!

ReservationService에서 현재 날짜이면서 현재 시간보다 이전인 날짜를 선택할 경우 InvalidReservationException이 발생하도록 리팩터링 후 테스트 코드도 작성하였습니다.

echo724 · 2026-05-13T17:45:06Z

+
    public void delete(Long id) {
        reservationUpdatingDao.delete(id);
    }


변경·취소 시 발생하는 에러 케이스(이미 지난 예약을 취소, 변경하려는 시간이 이미 차 있음 등)도 2단계의 규칙에 맞춰 처리한다.

이번 사이클 요구사항에서 취소 케이스여도 검증을 해줘야할 것 같은데 어떤 검증이 있을 수 있을까요?

또한 예약 취소 = 삭제인지도 고민해보시면 좋을 것 같아요! 답이 있는건 아니지만 비지니스적으로 취소할때 예약을 삭제하는 경우 어떤 영향이 있을 수 있을까요?

echo724 · 2026-05-13T17:46:24Z

+        if (reservationReq.date().isBefore(LocalDate.now())) {
+            throw new InvalidReservationException();
+        }


여기도 마찬가지로 시간이 지났는지에 대한 검증이 필요할 것 같습니다!

echo724 · 2026-05-13T17:49:29Z


+    public ReservationResponse update(Long id, ReservationRequest reservationReq) {
+        Reservation existedReservation = reservationQueryingDao.findReservationById(id)
+                .orElseThrow(() -> new ReservationNotFoundException(id));


HTTP에서 404가 NotFound의 의미이긴 하지만, 상황에 따라 두 가지 다른 상황이 있을 수 있습니다. 혹시 어떤 상황들이 있는지 아실까요? 힌트로 브라우저에서 존재하지 않았는 URL을 입력했을때와 위의 코드와의 차이는 무엇일까요?

echo724 · 2026-05-13T18:09:41Z

    }
+
+    @Test
+    void 예약_날짜_시간_변경() {


예약시 예약 예외 상황들 테스트가 부족한 것 같습니다!

Time과 Theme 관련 예외 상황들도 테스트가 많이 부족한 것 같네요 ㅠ

서비스 계층에서 커버가 됐다 하더라도 예외 상황에서 의도한 메세지와 예외코드가 반환됐는지도 확인해야하지 않을까요?

echo724 · 2026-05-13T18:19:06Z

+
+public class ReferencedDataException extends RuntimeException {
+    public ReferencedDataException() {
+        super("참조 중인 데이터가 있어 삭제할 수 없습니다.");


유저 혹은 클라이언트 입장에서 DB 테이블 구조를 모르는 상황에서 참조 중인 데이터라는게 어떤 데이터를 의미하는지 모를 수 있을 것 같아요.

echo724 · 2026-05-13T18:22:33Z

+            throw new InvalidInputException("이름은 필수입니다.");
        }
    }

    private static void validateDate(LocalDate date) {
        if (date == null) {
-            throw new IllegalArgumentException("날짜는 필수입니다.");
+            throw new InvalidInputException("날짜는 필수입니다.");
        }
    }

    private static void validateTimeId(Long timeId) {
        if (timeId == null) {
-            throw new IllegalArgumentException("시간은 필수입니다.");
+            throw new InvalidInputException("시간은 필수입니다.");
        }
    }

    private static void validateThemeId(Long themeId) {
        if (themeId == null) {
-            throw new IllegalArgumentException("테마는 필수입니다.");
+            throw new InvalidInputException("테마는 필수입니다.");
        }


아무래도 Request Body의 필드 값들에 대한 검증이므로 필드 이름을 그대로 넣어주는게 나을 것 같아요. 예를 들어 시간이라고만 하면 어떤 시간 값을 안넣은건지, 아니면 timeId를 넣지 않은건지 헷갈릴 수 있을 것 같습니다. 요청에 timeId 필드가 비어있습니다 이런식으로요.

추가적으로, 만약 여러 필드 값이 없는 경우, 여러 번의 요청을 통해서 알 수 있을텐데 모두 모아서 한 번에 알려주는 방식은 어떨까요? 예를 들어, "[timeId, themeId]가 비어있습니다."이런식으로요. 그러면 유저가 여러 번의 요청 없이 한 번에 알 수 있겠죠? 어떻게 생각하시나요?

echo724 · 2026-05-13T18:27:18Z

+    public Reservation withUpdatedDateAndTime(LocalDate date, ReservationTime time) {
+        return new Reservation(id, this.name, date, time, this.theme, this.createdAt);
+    }


가독성있고 명시적인 메서드 잘 작성해주셨네요! 다만, 위의 메서드 이름은 reservationWithId인데, 이 둘의 메서드 이름이 일관성이 깨진 것 같습니다. 비슷한 행위를 하는 것 같은데 with**으로 통일하는 것은 어떨까요?

haeseonlee added 13 commits May 12, 2026 16:15

docs: README.md 작성

045b3d1

feat: @RestControllerAdvice로 전역 에외 처리 추가

8b5f78a

feat: 500 에러가 사용자에게 노출되지 않도록 처리 및 애러 응답 JSON 형식으로 통일

1f28ca5

refactor: data.sql 예약 날짜 수정

7f60e6e

refactor: 오늘 날짜에서 이미 지난 시간 예약할 수 없도록 변경

c5f06d5

feat: 브라우저에서 에러 발생 시 사용자에게 메세지 표시

5dab8e0

docs: 예약 API 명세서에 예약 날짜 및 시간 변경 API 추가

2e29a42

feat: 본인의 예약 날짜·시간 변경 API 추가

8f37ae8

feat: 예약 변경 화면 구현 및 에러 케이스 처리

ec79b75

test: ReservationService 예약 변경 관련 테스트 추가

7d96a91

test: UserReservationTest 예약 날짜 및 시간 변경 테스트 추가

99ec61c

refactor: 에러 응답 코드를 도메인 의미 기반으로 변경

65f36bb

refactor: 예약 페이지 아이콘 정렬 수정

703d55a

echo724 self-requested a review May 13, 2026 17:10

echo724 assigned echo724 and unassigned echo724 May 13, 2026

echo724 requested changes May 13, 2026

View reviewed changes

Conversation

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

Uh oh!

체크 리스트

베이스 코드 선택 체크

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

사용자 본인 예약 변경 및 취소 API

에러 응답 설계

에러 케이스

에러 응답 설계

Uh oh!

echo724 commented May 13, 2026

Uh oh!

echo724 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haeseonlee May 14, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

haeseonlee commented May 13, 2026 •

edited

Loading

haeseonlee May 14, 2026 •

edited

Loading