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단계 - 방탈출 예약 관리] 리비(이근희) 미션 제출합니다. #6

Open
wants to merge 21 commits into
base: libienz
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
48c754f
[1 - 3단계 - 방탈출 예약 관리] 리비(이근희) 미션 제출합니다. (#21)
Libienz Apr 20, 2024
4a8f81e
refactor: 시간 충돌 확인 메서드 가독성 개선 작업
Libienz Apr 21, 2024
e899ccb
refactor: NullPointerException 예외 발생 구체화 개선
Libienz Apr 21, 2024
67fda49
feat: Reservation 테이블 스키마 정의
Libienz Apr 21, 2024
6bfe713
chore: DB 설정 작성
Libienz Apr 21, 2024
0c6e7b2
refactor: 레포지토리 테스트 설정된 빈으로 수행하도록 수정
Libienz Apr 22, 2024
5e199c7
test: 데이터 베이스 커넥션 및 테이블 정보 테스트 추가
Libienz Apr 22, 2024
6b8086a
feat: ReservationTime 시작 시간과 끝나는 시간 반환 기능 추가
Libienz Apr 22, 2024
451c499
feat: Reservation 예약 시작시간과 끝나는 시간 반환 기능 추가
Libienz Apr 22, 2024
b4bfc78
test: 스프링 부트 테스트 컨텍스트를 재사용하도록 설정 통일
Libienz Apr 22, 2024
bbef49f
feat: H2ReservationRepository 작성 및 빈 등록
Libienz Apr 22, 2024
38d89f8
feat: ReservationResponse Json 직렬화 설정
Libienz Apr 22, 2024
e87fbbd
chore: 복수 테스트 컨텍스트의 로드 시점 DDL이 충돌하지 않도록 SQL 수정
Libienz Apr 22, 2024
2f9a19c
fix: 통합 테스트와 레포지토리 테스트 같은 DB 컨텍스트에서 발생하는 문제 수정
Libienz Apr 23, 2024
cfdf9ed
refactor: 역직렬화 필요없는 ResponseDto에서 JsonCreator 및 JsonProperty 애노테이션 제거 개선
Libienz Apr 23, 2024
62c4a3e
refactor: 뷰 경로에서 불필요한 확장자 제거 개선
Libienz Apr 23, 2024
b69655c
test: 예약 지속 시간 테스트 추가
Libienz Apr 23, 2024
addeb22
refactor: import를 통한 가독성 개선
Libienz Apr 23, 2024
25f0775
refactor: 쿼리를 통해 예약 충돌을 해결하게 변경됨 - 불필요한 로직 제거 개선
Libienz Apr 23, 2024
545ef8c
refactor: 불필요한 확장성 제거 개선 및 구버전 레포지토리 삭제
Libienz Apr 23, 2024
01299a5
test: Service 계층 슬라이스 테스트 인메모리 레포지토리 주입 테스트로 변경
Libienz Apr 23, 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
21 changes: 21 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 방탈출 API

| Method | Endpoint | Description |
|--------|----------------------|--------------|
| GET | `/admin` | 어드민 페이지 요청 |
| GET | `/admin/reservation` | 예약 관리 페이지 요청 |
| GET | `/reservations` | 예약 정보 |
| POST | `/reservations` | 예약 추가 |
| DELETE | `/reservations/{id}` | 예약 취소 |

# 방탈출 예약 도메인 명세

## Name

- [x] 예약자 이름은 null이거나 빈 문자열일 수 없다.
- [x] 예약자 이름은 1~5사이의 길이를 가져야 한다.

## ReservationTime

- [x] 예약하려는 시간에 다른 예약이 존재한다면 예약이 불가능하다.
- [x] 예약 시간은 예약 시작 시간으로부터 한 시간의 길이를 가진다.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ dependencies {
runtimeOnly 'com.h2database:h2'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'io.rest-assured:rest-assured:5.3.1'
testImplementation 'org.mockito:mockito-core:3.6.28'
}

test {
Expand Down
1 change: 0 additions & 1 deletion src/main/java/roomescape/RoomescapeApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ public class RoomescapeApplication {
public static void main(String[] args) {
SpringApplication.run(RoomescapeApplication.class, args);
}

}
19 changes: 19 additions & 0 deletions src/main/java/roomescape/controller/AdminViewController.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;

@RequestMapping("/admin")
@Controller
public class AdminViewController {
@GetMapping()
public String showAdminMainPage() {
Comment on lines +10 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.

뒤에 괄호 없애는게 조금 더 깔끔할 거 같은 느낌!
@GetMapping() -> @GetMapping

Copy link
Member Author

Choose a reason for hiding this comment

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

몰랐던 부분 👍 레디 감사!

return "admin/index";
}

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

import java.util.List;
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.controller.dto.ReservationRequest;
import roomescape.controller.dto.ReservationResponse;
import roomescape.entity.Reservation;
import roomescape.service.ReservationService;

@RequestMapping("/reservations")
@RestController
Comment on lines +17 to +18
Copy link
Member

Choose a reason for hiding this comment

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

리비만의 애노테이션 순서 컨벤션이 있는지 궁금해

Copy link
Member Author

Choose a reason for hiding this comment

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

없움 🫠
다른 거 신경쓰다가 놓친 부분이네
관련해서 아는 게 아무것도 없다 보니까 크루들과 토의 해보아야겠다는 생각이 드네 ㅎㅎ

짚어줘서 땡큐! 안 짚어줬으면 그냥 넘어갔을 듯

public class ReservationController {
private final ReservationService reservationService;

public ReservationController(ReservationService reservationService) {
this.reservationService = reservationService;
}

@GetMapping()
Copy link
Member

Choose a reason for hiding this comment

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

확실히 ()는 생략해도 좋을듯!
그리고 @RequestMapping으로 코드 중복을 줄이는 것도 좋은데, 메서드마다 URI 정보를 모두 포함해 주는 것은 어때? URI로 검색했을 때 어떤 메서드에서 처리하는 요청인지 찾을 수 있게!

public ResponseEntity<List<ReservationResponse>> readAllReservations() {
List<ReservationResponse> reservations = reservationService.readAll()
.stream()
.map(ReservationResponse::from)
.toList();
return ResponseEntity.ok().body(reservations);
}

@PostMapping()
public ResponseEntity<ReservationResponse> createReservation(@RequestBody ReservationRequest reservationRequest) {
Reservation savedReservation = reservationService.saveReservation(reservationRequest.toEntity());
return ResponseEntity.ok().body(ReservationResponse.from(savedReservation));
}

@DeleteMapping("/{id}")
public ResponseEntity<Void> deleteReservationById(@PathVariable("id") long id) {
reservationService.deleteReservation(id);
return ResponseEntity.ok().build();
}
}
36 changes: 36 additions & 0 deletions src/main/java/roomescape/controller/dto/ReservationRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package roomescape.controller.dto;

import java.time.LocalDate;
import java.time.LocalTime;
import roomescape.entity.Reservation;

public class ReservationRequest {
Copy link
Member

Choose a reason for hiding this comment

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

dto를 record가 아닌 class로 설정한 이유가 궁금해

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 별차이 없다고 생각해
데이터 객체를 만드는 귀찮음을 감수하느냐 그렇지 않느냐가 현재에서 record를 고민하는 맥락이라고 생각함!
그런데 나는 눈에 보이는 쪽이 괜찮다는 생각이 들었네 🤔

record를 잘모르는 사람으로써 record 키워드를 보면 어떤 생성자가 있고 getter가 있는지 setter가 있는지 잘 기억이 나지 않아서리..

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

public ReservationRequest() {
}

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

public Reservation toEntity() {
return new Reservation(name, date, time);
}

public String getName() {
return name;
}

public LocalDate getDate() {
return date;
}

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

import java.time.LocalDate;
import java.time.LocalTime;
import roomescape.entity.Reservation;

public class ReservationResponse {
private final Long id;
private final String name;
private final LocalDate date;
private final LocalTime time;

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

public static ReservationResponse from(Reservation reservation) {
return new ReservationResponse(
reservation.getId(),
reservation.getName(),
reservation.getStartDate(),
reservation.getStartTime());
}

public long getId() {
return id;
}

public String getName() {
return name;
}

public LocalDate getDate() {
return date;
}

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

public class Name {
private static final int MIN_LENGTH = 1;
private static final int MAX_LENGTH = 5;

private final String name;

public Name(String name) {
validate(name);
this.name = name;
}

Comment on lines +7 to +13
Copy link
Member

Choose a reason for hiding this comment

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

이름 VO 만든거 최고...👍

private void validate(String name) {
validateNonNull(name);
validateLength(name);
}

private void validateNonNull(String name) {
if (name == null) {
throw new NullPointerException("예약자 이름은 Null이 될 수 없습니다");
}
}
Comment on lines +19 to +23
Copy link
Member

Choose a reason for hiding this comment

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

굳이 이 메서드가 없어도 validateLength 에서 name.length() 메서드를 호출할 때 NPE가 발생할 거 같은데 만약 name이 null이라면 그대로 NPE를 발생시키는게 아니라 IllegalArgumentException으로 처리하는건 어때?

Copy link
Member Author

Choose a reason for hiding this comment

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

NPE에 메시지를 담는다는 맥락에서 단순 NPE발생보다는 효율적이라는 개인적인 생각!

다만 레디말처럼 다른 예외로 변환할 수도 있겠네
고민해보아야겠으!


private void validateLength(String name) {
if (MAX_LENGTH < name.length() || name.length() < MIN_LENGTH) {
throw new IllegalArgumentException(
"예약자 이름은 " + MIN_LENGTH + "자 이상, " + MAX_LENGTH + "자 미만이어야 합니다: " + name);
}
}

public String getName() {
return name;
}
}
45 changes: 45 additions & 0 deletions src/main/java/roomescape/entity/Reservation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package roomescape.entity;

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

public class Reservation {
private final Long id;
private final Name name;
private final ReservationTime time;

public Reservation(Long id, String name, LocalDate reservationDate, LocalTime reservationStartTime) {
this.id = id;
this.name = new Name(name);
this.time = new ReservationTime(LocalDateTime.of(reservationDate, reservationStartTime));
}

public Reservation(String name, LocalDate reservationDate, LocalTime reservationStartTime) {
this(null, name, reservationDate, reservationStartTime);
}

public long getId() {
return id;
}
Comment on lines +22 to +24
Copy link
Member

Choose a reason for hiding this comment

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

[질문]
id의 필드는 Long (래퍼클래스)인데 id의 getter는 long(원시타입)인 이유가 궁금해

Copy link
Member Author

Choose a reason for hiding this comment

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

약간의 의도를 담았는데 그 의도가 희석되었을 수도 있겠네

Reservation의 Id는 Null이 될 수도 있고 값을 담고 있을 수도 있어.
그런데 getId를 하는 시점에서는 id가 널이면 안된다는 의도를 담고 싶었고 결과적으로 위와 같은 메서드 형태가 등장하게 되었네 🤔


public String getName() {
return name.getName();
}

public LocalDateTime getStartDateTime() {
return time.getStartDateTime();
}

public LocalDateTime getEndDateTime() {
return time.getEndDateTime();
}

public LocalDate getStartDate() {
return time.getStartDate();
}

public LocalTime getStartTime() {
return time.getStart();
}
}
38 changes: 38 additions & 0 deletions src/main/java/roomescape/entity/ReservationTime.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package roomescape.entity;

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

public class ReservationTime {
public static final int RESERVATION_DURATION_HOUR = 1;

private final LocalDateTime start;

public ReservationTime(LocalDateTime start) {
validateNonNull(start);
this.start = start;
}

private void validateNonNull(LocalDateTime startTime) {
if (startTime == null) {
throw new NullPointerException("예약 시간은 Null이 될 수 없습니다");
}
}
Comment on lines +17 to +21
Copy link
Member

Choose a reason for hiding this comment

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

name의 validateNonNull이랑 동일한 이야기!


public LocalDateTime getStartDateTime() {
return start;
}

public LocalDateTime getEndDateTime() {
return start.plusHours(RESERVATION_DURATION_HOUR);
}

public LocalDate getStartDate() {
return start.toLocalDate();
}

public LocalTime getStart() {
return start.toLocalTime();
}
}
90 changes: 90 additions & 0 deletions src/main/java/roomescape/repository/ReservationRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package roomescape.repository;

import java.sql.Date;
import java.sql.PreparedStatement;
import java.sql.Statement;
import java.sql.Time;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.format.DateTimeFormatter;
import java.util.List;
import java.util.Optional;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.support.GeneratedKeyHolder;
import org.springframework.jdbc.support.KeyHolder;
import org.springframework.stereotype.Repository;
import roomescape.entity.Reservation;
import roomescape.entity.ReservationTime;

@Repository
public class ReservationRepository {
private final JdbcTemplate jdbcTemplate;

public ReservationRepository(JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public List<Reservation> readAll() {
String sql = "select * from reservation";
return jdbcTemplate.query(sql, reservationRowMapper());
}

public Reservation save(Reservation reservation) {
String sql = "insert into reservation (name, date, time) values(?, ?, ?)";
KeyHolder keyHolder = new GeneratedKeyHolder();

jdbcTemplate.update(connection -> {
PreparedStatement ps = connection.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS);
ps.setString(1, reservation.getName());
ps.setDate(2, Date.valueOf(reservation.getStartDate()));
ps.setTime(3, Time.valueOf(reservation.getStartTime()));
return ps;
}, keyHolder);
long savedId = keyHolder.getKey().longValue();
return new Reservation(savedId, reservation.getName(), reservation.getStartDate(), reservation.getStartTime());
}

public Optional<Reservation> findById(long id) {
String sql = "select * from reservation where id=?";
try {
return Optional.of(jdbcTemplate.queryForObject(sql, new Object[]{id}, reservationRowMapper()));
} catch (EmptyResultDataAccessException ex) {
return Optional.empty();
}
}

public void deleteById(long id) {
String sql = "delete from reservation where id=?";
jdbcTemplate.update(sql, id);
}

public boolean isAnyReservationConflictWith(Reservation reservation) {
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");
String startDateTime = reservation.getStartDateTime().format(formatter);
String endDateTime = reservation.getEndDateTime().format(formatter);

String sql = "select exists (" +
" select 1 " +
" from reservation " +
" where ? between (date || ' ' || time) and dateadd('HOUR', ?, (date || ' ' || time)) " +
" or ? between (date || ' ' || time) and dateadd('HOUR', ?, (date || ' ' || time)) " +
") as exists_overlap;";

boolean conflict = jdbcTemplate.queryForObject(sql, Boolean.class, endDateTime,
ReservationTime.RESERVATION_DURATION_HOUR, startDateTime, ReservationTime.RESERVATION_DURATION_HOUR);
return conflict;
}
Comment on lines +63 to +78
Copy link
Member

Choose a reason for hiding this comment

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

예외 처리를 열심히 했네 👍

다만, 이 메서드가 너무 구체적이라는 생각이 드네! 재사용성을 생각해서 repository의 메서드는 좀 범용적인 것이 좋다고 생각해. 예를 들어 특정 범위에 있는 예약을 조회하는 방법이 있을 것 같아. 추가로 해당 sql문의 성능적인 측면도 궁금하고! 효율적인 sql문일까?

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 쿼리에 대한 평가가 절실했는데 망쵸가 달아주었군 😎
확실히 레포지토리레이어가 알기에 지나치게 비즈니스로직 종속적인 것 같아 좋은 피드백 땡큐!

sql문의 성능적인 측면은 어떨까.. 🤔
내가 sql 쌩초보이기도 해서 잘 모르겠긴 해.
다만 내가 아는 지식내에서 판단을 해볼게

우선 나는 sql의 성능은 Join의 횟수에서 많이 달라진다고 생각해
해당 sql문의 where절이 같은 연산을 여러번 하는 등 비효율적인 것 같지만 이는 큰 영향은 아니라고 생각함!


private RowMapper<Reservation> reservationRowMapper() {
return (rs, rowNum) -> {
long id = rs.getLong("id");
String name = rs.getString("name");
LocalDate date = rs.getDate("date").toLocalDate();
LocalTime time = rs.getTime("time").toLocalTime();

return new Reservation(id, name, date, time);
};
}
}
Loading