Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ repositories {

dependencies {
implementation 'org.springframework.boot:spring-boot-starter'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-thymeleaf'

compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'

testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'io.rest-assured:rest-assured:5.3.1'
}
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/roomescape/home/controller/HomeController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package roomescape.home.controller;

import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;

@Controller
public class HomeController {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reservation 패키지와 home 패키지를 나눴군요. 둘은 어떤 차이인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Controller 가 특정 페이지를 반환하는 것 같아서 프론트처럼 페이지처럼 나눴던 것 같습니다.

@GetMapping("/")
public String displayHomePage() {
return "home";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package roomescape.reservation.controller;

import java.net.URI;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
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.ResponseBody;
import roomescape.reservation.dto.ReservationRequest;
import roomescape.reservation.dto.ReservationResponse;
import roomescape.reservation.service.ReservationService;

@Controller
@RequiredArgsConstructor
public class ReservationController {
private final ReservationService reservationService;

@GetMapping("/reservation")
public String displayReservationPage() {
return "reservation";
}

@GetMapping("/reservations")
@ResponseBody
public ResponseEntity<List<ReservationResponse>> getReservations() {
return ResponseEntity.ok(reservationService.getReservations());
}

@PostMapping("/reservations")
@ResponseBody
public ResponseEntity<ReservationResponse> addReservation(@RequestBody ReservationRequest request) {
ReservationResponse response = reservationService.addReservation(request);
URI location = URI.create("/reservations/" + response.id());
return ResponseEntity.created(location).body(response);
}

@DeleteMapping("/reservations/{id}")
public ResponseEntity<Void> cancelReservation(@PathVariable Long id) {
reservationService.deleteReservation(id);
return ResponseEntity.noContent().build();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 적절하게 status code를 써주셨군요!

}

@ExceptionHandler(IllegalArgumentException.class)
Copy link

@hong-sile hong-sile Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExceptionHandler가 IllegalArgumentException을 받도록 하였군요.

별도로 정의한 NotFound가 아닌 Illegal로하신 이유가 있나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금 코드에서 모든 예외가 400으로 처리되는데 NotFound 예외도 같은 400이라 굳이 나눌 필요가 있을까? 라는 생각이 들었습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

알고계시다면 좋습니다. 더 세분화할 필요가 있을땐 따로 더 메서드를 만들고 별도의 Exception class를 넣으면되용

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더불어 ExceptionHandler를 보통 Controller에서 정의하는 것보단 ConrollerAdvice라는 클래스를 정의해서 거기에 작업하는 경우가 많습니다.

이에 대해 한번 학습해보고 적용해보면 좋을 것 같아요

public ResponseEntity<String> handleIllegalArgument(IllegalArgumentException e) {
return ResponseEntity.badRequest().body(e.getMessage());
}
}
12 changes: 12 additions & 0 deletions src/main/java/roomescape/reservation/dto/ReservationRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package roomescape.reservation.dto;

import com.fasterxml.jackson.annotation.JsonFormat;
import java.time.LocalDate;
import java.time.LocalTime;

public record ReservationRequest(
String name,
LocalDate date,
@JsonFormat(pattern = "HH:mm")
LocalTime time) {
}
13 changes: 13 additions & 0 deletions src/main/java/roomescape/reservation/dto/ReservationResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package roomescape.reservation.dto;

import com.fasterxml.jackson.annotation.JsonFormat;
import java.time.LocalDate;
import java.time.LocalTime;

public record ReservationResponse(
Long id,
String name,
LocalDate date,
@JsonFormat(pattern = "HH:mm")
LocalTime time) {
}
36 changes: 36 additions & 0 deletions src/main/java/roomescape/reservation/entity/Reservation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package roomescape.reservation.entity;

import java.time.LocalDate;
import java.time.LocalTime;
import lombok.Data;

@Data

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataAnnotation은 어떤 역할을 해주나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

솔직하게 Getter, Setter, RequiredArgsConstructor 까지만 알고 사용했었는데 @EqualsAndHashCode, @tostring 은 잘 몰랐는데 이부분도 공부해야될 것 같습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아주 좋습니다!! 보고 공부하려는게 아주 좋은것 같아요

(개인적으로) Data 어노테이션 쓰는걸 별로 안좋아하는데요.. Setter가 꼭 필요한 상황이 아니면 안해야 도메인을 잘 지킬 수 있다고 생각해요.

물론 잘 인지하고 쓰면 상관없죠

public class Reservation {
private Long id;
Copy link

@hong-sile hong-sile Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요거는 한번 이야기했어야 했는데 조금 늦었군요.

혹시 자바의 final 키워드에 대해 아시나요? 변수에 대해서 reassign 을 막는 키워드인데, 객체의 불변을 유지하기에 좋습니다.

이 키워드를 이용해서 우리는 immutable하게 값을 사용할 수 있습니다. 최근 개발 패러다임에선 이런 immutable한 값을 사용하는게 선호되고 있는데 왜 그런걸까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변하지 않는다는 것을 가정하에 코드를 보고 유지보수하게되면 코드의 결과나 에러 예측 등이 훨씬 쉬워질 것 같습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다. 사실 가변한 값은 다루기가 굉장히 짜증나요. 디버깅할때 값이 어디서 바뀌었는지 파악하기가 힘들거든요.
그래서 최대한 값을 불변으로 두어서 변경을 하는 범위를 최소화시켜 예측하고 통제할 수 잇는 코드를 만들 수 있습니다.

개인적으론 일단 무조건 final 붙이고 가변한 값이면 final을 떼는 식으로 저는 사용해요.
실제로 최신 프로그래밍 언어는 별다른 키워드를 안붙이면 기본적으로 불변입니다.

private String name;
private LocalDate date;
private LocalTime time;

public Reservation(Long id, String name, LocalDate date, LocalTime time) {
validate(name, date, time);
this.id = id;
this.name = name;
this.date = date;
this.time = time;
}

private void validate(String name, LocalDate date, LocalTime time) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요런것들을 한번 테스트코드로 확인해보면 좋을 거 같아요.
아니면 이러한 테스트는 필요없을까요? MissionStepTest로 원하는 케이스를 모두 확인했으니까요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

같은 날짜, 같은 시각 예약들어오는 부분도 예외 처리를 안해서 그부분까지 관련해서 테스트 코드를 작성했습니다.
이런 도메인의 유효성 관련 코드는 필요하다고 생각합니다.
그런데 api와 관련된 코드들은 주어진 테스트 코드처럼 테스트를 하는건가요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 테스트엔 다양한 형태가 있어요. 저희가 지금까지 배웠던 테스트는 하나의 도메인만 참여해서 흔히 단위 테스트라고 부르고,
여러개의 레이어(스프링 같은 프레임워크)까지 포함되는 테스트를 통합테스트라고 하기도 합니다.
프로그램 전체가 참여해 하나의 시나리오를 이루면 인수테스트라고도 하고요.
요 용어는 테스트 피라미드 라는 키워드를 한번 알아보면 좋을 것 같네요.

api 를 테스트하는 건 저렇게 RestAssured로 테스트를 하기도 하지만, 쓰는 곳은 많이 못봤던 것 같아요.
엄청 세팅하기가 번거롭고 관리하기가 힘들거든요. 그리고 테스트를 아무리 짜도 100% 보장이 되진 않으니까요.

그래서 다르게 테스트를 하기도 합니다. Service Layer 이하를 다 모킹해버려서 APi가 정상적으로 들어오는지만 확인하는 테스트를 짜기도 하고
또는, Spring에서 이렇게 테스트를 짜는게 아니라, 외부에서 주기적으로 API를 call해서 이상을 탐지하는 방식도 있습니다.
요건 BlazeMeter라는 도구가 있습니다.
이전에 있었던 회사에서는 RestAssured를 쓰기보단 이런방식으로 했네요. 아마 당시에 RestAssured랑 스프링에서 테스트 환경을 구축하는 것에 지식이 있는 사람이 있진 않아서 이런식으로 했던 것 같아요.

그리고 RestAssured 큐컴버라고 또 다르게 인수테스트 도구가 있습니다.

암튼 말이 좀 길어졌는데, api와 관련된 코드들은 주어진 테스트 코드처럼 테스트를 하는건가요?
라는 질문엔
네, api 와 관련된 코드 또한 테스트를 합니다. 하지만 이는 무거운 작업이라 회사의 상황에 따라서 어디까지 테스트할지가 갈립니다.

if (name == null || name.trim().isEmpty()) {
throw new IllegalArgumentException("유효한 이름이 아닙니다.");
}
if (date == null || date.isBefore(LocalDate.now())) {
throw new IllegalArgumentException("유효한 날짜 값이 아닙니다.");
}
if (date.equals(LocalDate.now()) && time.isBefore(LocalTime.now())) {
throw new IllegalArgumentException("현재 시간보다 이전 시간으로는 예약할 수 없습니다.");
}
if (time == null) {
throw new IllegalArgumentException("유효한 시간이 아닙니다.");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package roomescape.reservation.exception;

public class NotFoundReservationException extends IllegalArgumentException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

별도로 NotFoundException을 정의해주신 이유는 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자료에 주어지기도 했고, 있어서 나쁠건 없겠다라는 생각으로 만들었던 것 같습니다.
어차피 400번 오류를 띄워야할거라면 커스텀예외를 만들필요가 없었을까요?
만들고나니 클라이언트측에서 예약 삭제에 관한 api 응답으로 400번 오류를 띄워주면 'id가 잘못되었구나'라는 것을 알수도 있을거란 생각이 들었습니다.
만약에 커스텀 예외를 만든다면 id 형태가 잘못된건지, 해당 id와 관련된 예약이 없어서인지 알려주기 위해서일까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커스텀 예외를 만드는 것에서 외부(프론트)에게 전달되는 목적도 있지만 내부적으로 다루기 위함도 있어요.

사실 지금 우리가 NotFound의 경우에는 별도로 예외처리를 안 하고있지만, 이게 추후에 추가될수도 있으니까요.

public NotFoundReservationException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package roomescape.reservation.service;

import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import org.springframework.stereotype.Service;
import roomescape.reservation.dto.ReservationRequest;
import roomescape.reservation.dto.ReservationResponse;
import roomescape.reservation.entity.Reservation;
import roomescape.reservation.exception.NotFoundReservationException;

@Service

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service 클래스를 별도로 작성해주셨군요. Service는 어떤 역할을 해주나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이때까지 공부했던 Service는 비즈니스 로직을 처리한다고 알고 있습니다.
DB와 컨트롤러에서 넘어온 값들을 받아서 무언가를 처리하는 존재로만 알고 있었습니다.
계산하고 확인하고 무언갈 저장, 삭제, 수정 등을 하는 것 같습니다.

public class ReservationService {
private final Map<Long, Reservation> reservations = new ConcurrentHashMap<>();
private final AtomicLong index = new AtomicLong(1);
Copy link

@hong-sile hong-sile Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AtomicLong은 어떤 역할을 하고, 왜 쓰였나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동시에 값을 추가할때 동시성을 보장하고 인조 인덱스처럼 사용하려고 쓰인 것 같습니다.


public List<ReservationResponse> getReservations() {
return reservations.values().stream()
.map(this::toResponseDTO)
.collect(Collectors.toList());
}

public ReservationResponse addReservation(ReservationRequest request) {
if (isDuplicate(request)) {
throw new IllegalArgumentException("이미 해당 날짜와 시간에 예약이 존재합니다.");
}

Long id = index.getAndIncrement();
Reservation newReservation = new Reservation(id, request.name(), request.date(), request.time());
reservations.put(id, newReservation);
return toResponseDTO(newReservation);
}

public void deleteReservation(Long id) {
Reservation target = reservations.remove(id);
if (target == null) {
throw new NotFoundReservationException("해당 ID의 예약이 존재하지 않습니다: ");
}
}

private ReservationResponse toResponseDTO(Reservation reservation) {
return new ReservationResponse(
reservation.getId(),
reservation.getName(),
reservation.getDate(),
reservation.getTime()
);
}

private boolean isDuplicate(ReservationRequest request) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기서 Duplicate 여부를 각각의 Date와 Time을 꺼내서 확인하고 있는데 reservation에서 isSame 같은 메서드를 만들어서 Reservation에게 값이 같은지에 대한 작업을 처리하는 책임을 주면 어떨까요?

return reservations.values().stream().anyMatch(reservation ->
reservation.getDate().equals(request.date()) &&
reservation.getTime().equals(request.time())
);
}
}
114 changes: 114 additions & 0 deletions src/test/java/roomescape/MissionStepTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
package roomescape;

import static org.hamcrest.Matchers.is;

import io.restassured.RestAssured;
import io.restassured.http.ContentType;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.temporal.ChronoUnit;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
Expand All @@ -16,4 +25,109 @@ public class MissionStepTest {
.then().log().all()
.statusCode(200);
}

@Test
void 이단계() {
RestAssured.given().log().all()
.when().get("/reservation")
.then().log().all()
.statusCode(200);

RestAssured.given().log().all()
.when().get("/reservations")
.then().log().all()
.statusCode(200)
.body("size()", is(0));
}

@Test
void 삼단계() {
Map<String, String> params = new HashMap<>();
params.put("name", "브라운");
params.put("date", "2028-08-05");
params.put("time", "15:40");

RestAssured.given().log().all()
.contentType(ContentType.JSON)
.body(params)
.when().post("/reservations")
.then().log().all()
.statusCode(201)
.header("Location", "/reservations/1")
.body("id", is(1));

RestAssured.given().log().all()
.when().get("/reservations")
.then().log().all()
.statusCode(200)
.body("size()", is(1));

RestAssured.given().log().all()
.when().delete("/reservations/1")
.then().log().all()
.statusCode(204);

RestAssured.given().log().all()
.when().get("/reservations")
.then().log().all()
.statusCode(200)
.body("size()", is(0));
}

@Test
void 사단계() {
Map<String, String> params = new HashMap<>();
params.put("name", "브라운");
params.put("date", "");
params.put("time", "");

// 필요한 인자가 없는 경우
RestAssured.given().log().all()
.contentType(ContentType.JSON)
.body(params)
.when().post("/reservations")
.then().log().all()
.statusCode(400);

// 삭제할 예약이 없는 경우
RestAssured.given().log().all()
.when().delete("/reservations/1")
.then().log().all()
.statusCode(400);
}

@Test
@DisplayName("지난 날짜 예외처리")
void invalidDate() {
Map<String, String> params = new HashMap<>();
params.put("name", "브라운");
params.put("date", "2024-01-01");
params.put("time", "15:00");

RestAssured.given().log().all()
.contentType(ContentType.JSON)
.body(params)
.when().post("/reservations")
.then().log().all()
.statusCode(400);
}

@Test
@DisplayName("오늘 현재 시각보다 빠른 시간 예외처리")
void invalidTodayTime() {
LocalDate today = LocalDate.now();
LocalTime pastTime = LocalTime.now().minusMinutes(1);

Map<String, String> params = new HashMap<>();
params.put("name", "브라운");
params.put("date", today.toString());
params.put("time", pastTime.truncatedTo(ChronoUnit.MINUTES).toString());

RestAssured.given().log().all()
.contentType(ContentType.JSON)
.body(params)
.when().post("/reservations")
.then().log().all()
.statusCode(400);
}
}
40 changes: 40 additions & 0 deletions src/test/java/roomescape/ReservationServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package roomescape;

import static org.junit.jupiter.api.Assertions.assertThrows;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import roomescape.reservation.dto.ReservationRequest;
import roomescape.reservation.exception.NotFoundReservationException;

import java.time.LocalDate;
import java.time.LocalTime;
import roomescape.reservation.service.ReservationService;

class ReservationServiceTest {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외케이스에 대한 테스트를 잘 해주셨군요...!

정상 케이스에 대한 테스트를 안하신 이유는 있을까요?


private ReservationService service;

@BeforeEach
void setUp() {
service = new ReservationService();
}

@Test
@DisplayName("동일한 날짜, 시간에 예약하면 예외 발생")
void throwExceptionWhenDuplicateReservation() {
ReservationRequest request = new ReservationRequest("홍길동", LocalDate.now().plusDays(1), LocalTime.of(10, 0));
service.addReservation(request);

ReservationRequest duplicateRequest = new ReservationRequest("김철수", LocalDate.now().plusDays(1), LocalTime.of(10, 0));

assertThrows(IllegalArgumentException.class, () -> service.addReservation(duplicateRequest));
}

@Test
@DisplayName("존재하지 않는 예약 ID로 삭제 시 예외 발생")
void deleteNonExistingReservationThrows() {
assertThrows(NotFoundReservationException.class, () -> service.deleteReservation(999L));
}
}
Loading