Skip to content
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 - 6단계 - 방탈출 예약 관리] 망쵸(김주환) 미션 제출합니다. #7

Open
wants to merge 30 commits into
base: 3juhwan
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0a286f1
[1 - 3단계 - 방탈출 예약 관리] 망쵸(김주환) 미션 제출합니다. (#32)
3Juhwan Apr 17, 2024
16a398b
fix: API 포맷 변경
3Juhwan Apr 18, 2024
b829c3c
refactor: ContentType 세팅 코드 삭제
3Juhwan Apr 18, 2024
0663d5a
refactor: 예약 요청 DTO 추가
3Juhwan Apr 18, 2024
cfa2452
feat: 상태코드 변경
3Juhwan Apr 18, 2024
c21deef
refactor: admin api의 uri prefix 공통 처리
3Juhwan Apr 18, 2024
948d536
fix: html 네비게이션에 하이퍼링크 수정
3Juhwan Apr 20, 2024
6fd8012
refactor: 테스트 포트 번호 수정, @DirtiesContext 제거
3Juhwan Apr 20, 2024
e941bed
fix: 변경된 상태 코드를 테스트에 반영
3Juhwan Apr 20, 2024
03a8a4a
chore: H2 DB 세팅
3Juhwan Apr 20, 2024
1cec379
refactor: reservations URI를 클래스 단위로 맵핑
3Juhwan Apr 20, 2024
ffbed0e
feat: 예약 repository 인터페이스 추가
3Juhwan Apr 20, 2024
292223e
feat: 예약 controller에 repository 주입
3Juhwan Apr 20, 2024
496ef6b
feat: 예약 repository h2 구현체 추가
3Juhwan Apr 20, 2024
21b5bf4
feat: 예약 pk 생성 책임을 DB에 위임, 저장할 시 엔티티 반환
3Juhwan Apr 20, 2024
5e7ccdf
test: 어드민 예약 페이지 HTTP 요청 테스트 추가
3Juhwan Apr 20, 2024
37aa4e7
refactor: Map에서 SqlParameterSource로 교체
3Juhwan Apr 21, 2024
537a3e3
test: 예약 repository 테스트 추가
3Juhwan Apr 21, 2024
67b42f9
test: 삭제할 예약이 존재하지 않는 경우에 대한 테스트 추가
3Juhwan Apr 21, 2024
f357078
rename: 기존 controller 테스트를 appceptance 테스트로 변경
3Juhwan Apr 21, 2024
fbba62e
feat: 인메모리 repository 추가
3Juhwan Apr 21, 2024
cf6eadd
fix: 애플리케이션 컨테이너 뜰 때마다 table id를 1로 초기화
3Juhwan Apr 21, 2024
78822a9
refactor: 인텔리제이 빌드를 위해 경로 명시
3Juhwan Apr 21, 2024
de4f254
style: 테스트명에 마침표 삭제
3Juhwan Apr 21, 2024
f6d740b
test: 예약 시점을 미래로 수정
3Juhwan Apr 21, 2024
8c2213a
fix: 기존 예약 테스트에 존재했던 DB(auto-increase)에 대한 의존성 제거
3Juhwan Apr 21, 2024
0bfbd46
test: controller 슬라이스 테스트
3Juhwan Apr 21, 2024
d47c5ae
feat: dto에 예약 정책 추가
3Juhwan Apr 21, 2024
248c01a
test: 예약 controller 예외 상황 테스트 추가
3Juhwan Apr 21, 2024
a31c4e0
fix: 성공적인 POST 요청에 대한 HTTP 응답 코드를 200에서 201로 변경
3Juhwan Apr 21, 2024
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ out/

### VS Code ###
.vscode/

### Git Commit Template ###
.gitmessage.txt
11 changes: 11 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# 1단계 기능 요구사항 - 홈 화면

Comment on lines +1 to +2
Copy link
Member

Choose a reason for hiding this comment

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

예약 날짜는 현재보다 이전일 수 없다 등등 여러 도메인 규칙들이 보이는데 이러한 것도 정리되어 있는 것이 좋을 것 같아!

레벨 - 1을 까먹지 말라는 네오의 가르침..!

- [x] `/admin` 으로 `GET` 요청 시 어드민 메인 페이지를 응답한다.

# 2단계 기능 요구사항 - 예약 조회
- [x] `/reservations` 으로 `GET` 요청 시 저장된 예약 정보를 응답한다.
- [x] `/admin/reservation` 으로 `GET` 요청 시 예약 관리 페이지를 응답한다.

# 3단계 기능 요구사항 - 예약 추가/취소
- [x] `/reservations` 으로 `POST` 요청 시 예약을 추가한다.
- [x] `/reservations/{id}` 으로 `DELETE` 요청 시 해당 `id`의 예약을 삭제한다.
Comment on lines +1 to +11
Copy link
Member

Choose a reason for hiding this comment

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

기능 요구사항 👍
API 명세도 있으면 좋을 듯!

19 changes: 19 additions & 0 deletions src/main/java/roomescape/controller/AdminController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package roomescape.controller;

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

@Controller
@RequestMapping("/admin")
public class AdminController {
Comment on lines +7 to +9
Copy link
Member

Choose a reason for hiding this comment

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

어쩌다가 알게된 건데 망쵸의 의견도 궁금하네

@RequestMapping을 통해서 리소스를 계층화 할 수 있잖아
그런데 이런 방식으로 url을 잘라놓으면 전역적으로 찾을 떄 (cmd + shift + f) 찾기 어렵다는 말도 있더라고

/admin/resrervation 을 찾을 수 없음

망쵸는 어떻게 생각!?

Copy link
Author

@3Juhwan 3Juhwan Apr 25, 2024

Choose a reason for hiding this comment

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

나도 그 방식이 좋다고 생각해
다음 미션부터 @RequestMapping 보다 Mapping 애너테이션에 모든 URI를 넣으려고!

@GetMapping
public String home() {
return "admin/index";
}

@GetMapping("/reservation")
public String reservation() {
return "admin/reservation-legacy";
}
}
56 changes: 56 additions & 0 deletions src/main/java/roomescape/controller/ReservationController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package roomescape.controller;

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.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import roomescape.domain.Reservation;
import roomescape.dto.ReservationRequestDto;
import roomescape.dto.ReservationResponseDto;
import roomescape.repository.ReservationRepository;

import java.net.URI;
import java.util.List;

@RestController
@RequestMapping("/reservations")
public class ReservationController {
private final ReservationRepository reservationRepository;

public ReservationController(ReservationRepository reservationRepository) {
this.reservationRepository = reservationRepository;
}

@GetMapping
public ResponseEntity<List<ReservationResponseDto>> findAll() {
List<Reservation> reservations = reservationRepository.findAll();
List<ReservationResponseDto> reservationResponseDtos = reservations.stream()
.map(ReservationResponseDto::from)
.toList();
Comment on lines +30 to +33
Copy link
Member

Choose a reason for hiding this comment

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

매핑하는 과정을 컨트롤러에 두었군
나랑 똑같네 👏

DTO는 뷰라고 생각했고 DTO는 뷰와 도메인을 모두 알고 있는 컨트롤러에서 다루어져야 한다는 개인적인 생각이야


return ResponseEntity.ok(reservationResponseDtos);
}

@PostMapping
public ResponseEntity<ReservationResponseDto> add(@RequestBody ReservationRequestDto reservationRequestDto) {
Reservation savedReservation = reservationRepository.save(new Reservation(
reservationRequestDto.name(),
reservationRequestDto.date(),
reservationRequestDto.time()
Comment on lines +40 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Dto에서 값을 꺼내와서 Reservation 객체를 생성하고 있네 👍
RequestDto에서 toEntity를 구현하는 방법이 있고 이러한 방법이 그렇게 어색하지는 않은가봐

웨지랑 나눈 이야기가 있는데 궁금하면 한번 확인해보시오!
코멘트

));

return ResponseEntity.created(URI.create("/reservations/" + savedReservation.getId()))
.body(ReservationResponseDto.from(savedReservation));
}

@DeleteMapping("/{id}")
public ResponseEntity<Void> delete(@PathVariable(name = "id") long id) {
reservationRepository.deleteById(id);

return ResponseEntity.noContent().build();
}
Comment on lines +50 to +55
Copy link
Member

Choose a reason for hiding this comment

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

@PathVariable의 경로 이름(name = "id")과 변수명이 같을 경우 (long id) 생략 가능하다던데 이는 빌드와 실행 옵션에 따라 달라지는 부분이더라

설정에 따라 동작이 달라질 수 있는 코드는 좋지 않기에 지금처럼 @PathVarable에 파라미터를 함께 전달하는 것이 좋은 것 같아 👍
다만 name은 생략할 수 있을 듯!

}
42 changes: 42 additions & 0 deletions src/main/java/roomescape/domain/Reservation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package roomescape.domain;

import java.time.LocalDate;
import java.time.LocalTime;

public class Reservation {
private final long id;
private final String name;
private final LocalDate date;
private final LocalTime time;

Comment on lines +1 to +11
Copy link
Member

Choose a reason for hiding this comment

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

도메인은 비즈니스 로직을 담은 협력 객체, 엔티티는 DB 테이블과 일대일 매핑되는 영속화 대상 객체

나는 레벨 1 체스 미션에서 위와 가이 학습했었어

다만 망쵸의 현행처럼 도메인과 엔티티를 엄격하게 구분하는 경우는 많지 않다고 하더라!
관련한 코멘트

public Reservation(String name, LocalDate date, LocalTime time) {
this(0L, name, date, time);
}
Comment on lines +12 to +14
Copy link
Member

Choose a reason for hiding this comment

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

0과 Null은 의미가 천차만별이라고 생각해

만약 아이디가 없음을 의도한 것이라면 nullable함을 설정해야된다고 생각함! (Wrapper Type)

Copy link
Member

Choose a reason for hiding this comment

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

추가적으로 ID는 null 표현 때문에 Wrapper 타입이 거의 강제된다고 하더라고 🤔


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

public boolean hasId(long id) {
return this.id == id;
}

public long getId() {
return id;
}

public String getName() {
return name;
}

public LocalDate getDate() {
return date;
}

public LocalTime getTime() {
return time;
}
}
53 changes: 53 additions & 0 deletions src/main/java/roomescape/dto/ReservationRequestDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package roomescape.dto;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.Objects;

public record ReservationRequestDto(
String name,
LocalDate date,
LocalTime time
) {
private static final int NAME_LENGTH_MIN = 2;
private static final int NAME_LENGTH_MAX = 10;
private static final int RESERVATION_TIME_MIN = 9;
private static final int RESERVATION_TIME_MAX = 20;

public ReservationRequestDto {
validateName(name);
validateDate(date);
validateTime(time);
validateDateTime(date, time);
}

public static void validateName(String name) {

Choose a reason for hiding this comment

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

아무래도 가장 고민되는 지점이죠? 우린 레벨1에서 특정 도메인의 값 검증은 해당 도메인 클래스에 위임하는게 좋다고 배웠지만. 막상 스프링 관점에서 보면 DTO에서 검증하는게 크게 나쁘지 않은거 같구요 🤔 (더군다나 스프링의 @Validation을 사용하면 훨씬 편리하고 강력하게 검증을 할 수 있는데 말이죠...ㅎ)

값에 대한 검증 책임은 DTO vs Domain

망쵸의 의견이 궁금해요 ☺️

PS. 무려 7년전에도 같은 주제로 선배 개발자들이 열심히 토론하고있더라구요! 의견 역시 가지각색이고요 👀
https://stackoverflow.com/questions/42280355/spring-rest-api-validation-should-be-in-dto-or-in-entity

Objects.requireNonNull(name);
if (name.length() < NAME_LENGTH_MIN || NAME_LENGTH_MAX < name.length()) {
throw new IllegalArgumentException();
}
}

private void validateDate(LocalDate date) {
Objects.requireNonNull(date);
if (date.isBefore(LocalDate.now())) {
throw new IllegalArgumentException();
}
}

private void validateTime(LocalTime time) {
Objects.requireNonNull(time);
if (time.isBefore(LocalTime.of(RESERVATION_TIME_MIN, 0))
|| time.isAfter(LocalTime.of(RESERVATION_TIME_MAX, 0))) {
throw new IllegalArgumentException();
}
Comment on lines +25 to +44
Copy link
Member

@Libienz Libienz Apr 25, 2024

Choose a reason for hiding this comment

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

어떤 것을 검증하는지 확실한 메서드 명을 주는게 좋을 듯!?

추가적으로 Objects.requireNonNull()은 단순히 NPE 만을 발생시키기에 (물론 발생시키지 않는 것 보다는 괜춘겠지만)
불친절하다는 생각이야 ( 나도 Objects.requireNonNull()만 썻다가 리뷰 받음)

망쵸의 결론도 궁금하군 🤔

}

private void validateDateTime(LocalDate date, LocalTime time) {
LocalDateTime localDateTime = LocalDateTime.of(date, time);
if (localDateTime.isBefore(LocalDateTime.now())) {
throw new IllegalArgumentException();
}
}
}
19 changes: 19 additions & 0 deletions src/main/java/roomescape/dto/ReservationResponseDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package roomescape.dto;

import com.fasterxml.jackson.annotation.JsonFormat;
import roomescape.domain.Reservation;

import java.time.LocalDate;
import java.time.LocalTime;

public record ReservationResponseDto(long id, String name, LocalDate date,
@JsonFormat(pattern = "HH:mm") LocalTime time) {
Comment on lines +9 to +10
Copy link
Member

Choose a reason for hiding this comment

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

@JsonFormat 몰랐는데 알아갑니다 🙇🏻‍♂️

public static ReservationResponseDto from(Reservation reservation) {
return new ReservationResponseDto(
reservation.getId(),
reservation.getName(),
reservation.getDate(),
reservation.getTime()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package roomescape.repository;

import org.springframework.context.annotation.Primary;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.namedparam.BeanPropertySqlParameterSource;
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
import org.springframework.jdbc.core.simple.SimpleJdbcInsert;
import org.springframework.stereotype.Repository;
import roomescape.domain.Reservation;

import javax.sql.DataSource;
import java.util.List;

@Primary

Choose a reason for hiding this comment

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

혹시 당신도.. 대영한카르텔..? 농담입니다ㅎ 우선적으로 주입될 빈을 설정하기 위해서 @Primary 어노테이션을 사용해주셨군요! 굳!

@Repository
public class H2ReservationRepositoryImpl implements ReservationRepository {
Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

...Impl이라는 클래스 명은 인터페이스에 대한 구현체가 단 하나일 때 사용하는 관례적 이름이라고 나는 알고 있어
지금처럼 구현체가 여러개인 경우에는 다른 이름이 좀 더 좋지 않을까 하는 개인적인 생각!

private final JdbcTemplate jdbcTemplate;
private final SimpleJdbcInsert jdbcInsert;

public H2ReservationRepositoryImpl(JdbcTemplate jdbcTemplate, DataSource dataSource) {
this.jdbcTemplate = jdbcTemplate;
this.jdbcInsert = new SimpleJdbcInsert(dataSource)
.withTableName("reservation")
.usingGeneratedKeyColumns("id");
}

@Override
public List<Reservation> findAll() {
String sql = "select * from reservation";

return jdbcTemplate.query(sql, (resultSet, rowNum) -> new Reservation(
resultSet.getLong("id"),
resultSet.getString("name"),
resultSet.getDate("date").toLocalDate(),
resultSet.getTime("time").toLocalTime()
));
}

@Override
public Reservation save(Reservation reservation) {
SqlParameterSource source = new BeanPropertySqlParameterSource(reservation);
long reservationId = jdbcInsert.executeAndReturnKey(source).longValue();

return new Reservation(
reservationId,
reservation.getName(),
reservation.getDate(),
reservation.getTime()
);
}

@Override
public void deleteById(long id) {
String sql = "delete from reservation where id = ?";

jdbcTemplate.update(sql, id);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package roomescape.repository;

import org.springframework.stereotype.Repository;
import roomescape.domain.Reservation;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicLong;

@Repository
public class InMemoryReservationRepositoryImpl implements ReservationRepository {
private final List<Reservation> reservations = Collections.synchronizedList(new ArrayList<>());
private final AtomicLong index = new AtomicLong(0);
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

Collections.synchronizedList(new ArrayList<>());는 처음 보네 멀티 스레드 환경에서 동기화를 보장해주나봐 👀
새로운 것 학습하고 갑니다


@Override
public List<Reservation> findAll() {
return Collections.unmodifiableList(reservations);
}

@Override
public Reservation save(final Reservation reservation) {
Reservation newReservation = new Reservation(index.incrementAndGet(), reservation.getName(),
reservation.getDate(), reservation.getTime());
reservations.add(newReservation);

return newReservation;
}

@Override
public void deleteById(final long id) {
reservations.stream()
.filter(reservation -> reservation.hasId(id))
.findFirst()
.ifPresent(reservations::remove);
}

void deleteAll() {
reservations.clear();
}
}
13 changes: 13 additions & 0 deletions src/main/java/roomescape/repository/ReservationRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package roomescape.repository;

import roomescape.domain.Reservation;

import java.util.List;

public interface ReservationRepository {
List<Reservation> findAll();

Reservation save(Reservation reservation);

void deleteById(long id);
}
3 changes: 3 additions & 0 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
spring.h2.console.enabled=true
spring.h2.console.path=/h2-console
spring.datasource.url=jdbc:h2:mem:database
10 changes: 10 additions & 0 deletions src/main/resources/schema.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
DROP TABLE IF EXISTS reservation;

CREATE TABLE reservation
(
id BIGINT NOT NULL AUTO_INCREMENT,
name VARCHAR(255) NOT NULL,
date VARCHAR(255) NOT NULL,
time VARCHAR(255) NOT NULL,
PRIMARY KEY (id)
);
2 changes: 1 addition & 1 deletion src/main/resources/static/js/reservation-legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function requestCreate(reservation) {

return fetch(RESERVATION_API_ENDPOINT, requestOptions)
.then(response => {
if (response.status === 200) return response.json();
if (response.status === 201) return response.json();
throw new Error('Create failed');
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/templates/admin/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<div class="collapse navbar-collapse" id="navbarSupportedContent">
<ul class="navbar-nav ml-auto">
<li class="nav-item">
<a class="nav-link" href="/reservation">Reservation</a>
<a class="nav-link" href="/admin/reservation">Reservation</a>
</li>
<li class="nav-item">
<a class="nav-link" href="/time">Time</a>
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/templates/admin/reservation-legacy.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<div class="collapse navbar-collapse" id="navbarSupportedContent">
<ul class="navbar-nav ml-auto">
<li class="nav-item">
<a class="nav-link" href="/reservation">Reservation</a>
<a class="nav-link" href="/admin/reservation">Reservation</a>
</li>
<li class="nav-item">
<a class="nav-link" href="/time">Time</a>
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/templates/admin/reservation.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<div class="collapse navbar-collapse" id="navbarSupportedContent">
<ul class="navbar-nav ml-auto">
<li class="nav-item">
<a class="nav-link" href="/reservation">Reservation</a>
<a class="nav-link" href="/admin/reservation">Reservation</a>
</li>
<li class="nav-item">
<a class="nav-link" href="/time">Time</a>
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/templates/admin/time.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<div class="collapse navbar-collapse" id="navbarSupportedContent">
<ul class="navbar-nav ml-auto">
<li class="nav-item">
<a class="nav-link" href="/reservation">Reservation</a>
<a class="nav-link" href="/admin/reservation">Reservation</a>
</li>
<li class="nav-item">
<a class="nav-link" href="/time">Time</a>
Expand Down
Loading