-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Feat] 예약 관련 컨트롤러 작성 및 통합 테스트 작성 #6
Conversation
- 관련 의존성끼리 모이도록 정리
- 소셜 로그인 페이지 조회 - 소셜 로그인 요청 - 토큰 재발급
- 가게의 예약 현황 조회 - 테이블 예약 - 예악금 정보 조회 - 예약 정보 상세 조회 - 예약 확정 - 예약 취소
- 빈자리 알림 신청
- com/nowait/nowait → com/nowait
- 테스트가 통과하도록 프로덕션 코드 변경 (Red-Green)
- 테스트가 통과하도록 프로덕션 코드 변경(Red-Green)
- 테스트가 통과하도록 프로덕션 코드 변경(Red-Green)
- placeName: Long → String - placeDescription: Long → String - paymentAmount: String → Integer
- paidAt, bookedAt 추가
- 테스트가 통과하도록 프로덕션 코드 변경(Red-Green)
- 테스트가 통과하도록 프로덕션 코드 변경(Red-Green)
- 테스트가 통과하도록 프로덕션 코드 변경(Red-Green)
nowait-api/build.gradle
Outdated
implementation 'org.springframework.boot:spring-boot-starter-web' | ||
implementation 'org.springframework.boot:spring-boot-starter-validation' | ||
|
||
// Lombok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lombok gradle plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d1f3854 반영했습니다:)
* @return 소셜 로그인 페이지 응답 | ||
*/ | ||
@GetMapping("/oauth/login/{socialType}") | ||
public ResponseEntity<ApiResult<GetLoginPageRes>> getLoginPage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseEntity는 왜 붙인 것인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseEntity
를 사용하면 응답 상태나 http 헤더 부분을 조정할 수 있어서 사용했습니다!
그런데 기본적으로 응답이 200으로 내려가니, 다른 응답 상태가 필요하거나 헤더를 조정할 일이 있을 때만 사용하는 것이 더 깔끔할 것 같네요!
e841cc4 반영했습니다
* @param time 예약 시간 | ||
* @param partySize 예약 인원 (default: 1) | ||
*/ | ||
@Builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
record 타입의 의도와 builder를 썼을 때, 사람들이 어떻게 받아들일 지에 대해서
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
record
는 불변 데이터 객체를 표현하는 자바의 키워드이고, 일반적으로 "해당 클래스가 단순히 데이터를 담는 컨테이너 역할을 함"을 의미합니다!
저는 단순히 저의 입장에서 조금 더 가독성있고, 편하게 BookingReq
를 생성하려고 @Builder
를 썼습니다! 그런데 제 3자의 입장에서 @Builder
를 보게 된다면 _'해당 record를 생성할 때, 필드를 선택적으로 설정할 수 있다'_라는 의미로 받아들여 질 수 있다는 것을 생각하지 못했네요😂
75439b7 반영했습니다!
class BookingApiIntegrationTest { | ||
|
||
@Autowired | ||
private MockMvc mockMvc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private MockMvc mockMvc; | |
private TestRestTemplate testRestTemplate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e5d9859 반영했습니다:)
그런데 TestRestTemplate
으로 변경하니.. given, when 부분에서 조금 가독성이 떨어지네요..ㅜㅜ 아무래도 응답이 ApiResult<T>로 내려가서 타입 추론을 위한 설정이 필요해진 것도 한몫한 것 같습니다😢 ApiResult를 그냥 없앨까 싶기도 하네요.. 사실 프론트엔드가 없는 플젝이라 장점이 크게 두드러지지 않는 것 같아서요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
의도하지 않은 필드가 들어갔는지 아닌지는 어떻게 테스트할 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json-schema-validator
와 같은 JSON 검증 라이브러리를 사용하면, API 응답이 사전에 정의된 JSON 스키마를 따르는지 검증할 수 있습니다! 혹시 이 방법을 의도하신 것이 맞을까요?
import lombok.Builder; | ||
|
||
@Builder | ||
public record GetBookingInfoRes( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각각의 필드들을 다 풀어서 쓴 거 좋네요. 중첩된 구조가 더 좋다고들 생각하는데, 사실 완전히 겹치는 내용이 있는게 아니라면, 중첩 구조보다는 풀어서 써주는게 client입장에서 더 도움이 되는 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실 여쭤보려고 했던 부분입니다:)
이전에 프로젝트를 진행할 때, 프론트엔드에서는 depth가 깊어지지 않는 것이 더 좋다고 하셨는데 사실 인가 보군요!
@RequestBody @Valid BookingReq request | ||
) { | ||
// TODO: 테이블 예약 비즈니스 로직 호출 | ||
ApiResult<BookingRes> result = ApiResult.of( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이정도는 전혀 문제 없어보이긴합니다만, 뭐랄까 ... ApiResult가 편한 것 같으면서도 약간 코드양이 늘어나는 부분이 있는 것 같긴하네요. ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞습니다 허허 ApiResult
를 도입한게 client 편의성을 위한 것이기도 하고, 개인적으로 응답 포맷이 맞춰져 있는게 더 기분이 좋아서였는데요😂 사실 생산성이 저하되면 안 쓰고 싶기도 해서.. 고민이 되긴하네요😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 고생하셨어요. 추후 변경사항은 다른 PR로 작업하시면 좋을 것 같습니다!!!
- 기존 의존성은 제거
- MockMvc는 실제 Http 요청을 보내지 않음으로 더 신뢰할 수 있는 통합 테스트를 위해 변경
🔗 관련 이슈
Resolves #5
👩🏻💻 구현 내용
XXXApi.class
) 작성💬 코멘트
💬 궁금한 점
API가 크게 고객을 위한 기능과 가게 매니저를 위한 기능(ex. 예약 확정)이 나눠져 있는데, 애초에 모듈이나 API를 구분하는 것이 나을까요? 예약 현황 조회 기능 같은 경우에도.. 요청한 사람이 고객인지 가게 매니저인지에 따라 다른 응답을 기대할 것 같아서요!
현재는 예약 요청 후 예약금 결제가 필요하더라도, 일단 예약 상태가 '결제 대기 중'인 예약 리소스가 생성되고, 예약금 조회나 결제 요청을 다시 하도록 API가 분리되어 있는데요! 사실 일반적인 어플에서는 결제가 완료되야 정상적으로 예약이 생성되는 것처럼 보이는데.. 혹시 이 방식에 대해서 의견을 들을 수 있을까요?
결제는 카카오 페이나 네이버 페이를 통해 간편 결제로 구현을 고려 중인데.. 제가 결제 시스템을 개발해 본 적이 없어서 지금 작성한 결제 API가 맞는지 의문이 드네요.. 결제 같은 경우에는 좀 더 공부를 하고 API를 작성하는 것이 나을까요..?
현재 통합 테스트에는 해피 케이스 밖에 없는데요! 예외 케이스도 추가하는게 좋을 것 같은데, 비즈니스적인 예외말고도, 단순히 입력 형식이 올바르지 않은 모든 경우를 다 만드는 것이 좋을까요?