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단계 방탈출 결제 / 배포] 몰리(김지민) 미션 제출합니다. #46

Merged
merged 26 commits into from
Jun 2, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
37f366d
chore(/src): 기존 코드 마이그레이션
jminkkk May 29, 2024
60459e1
docs(data.sql): 요구사항 분석 작성
jminkkk May 29, 2024
02cfe1d
chore(/resources): 프론트 코드 추가 및 수정
jminkkk May 29, 2024
cf2aaeb
feat(PaymentProperties): 외부 API 결제 키 추가
jminkkk May 29, 2024
f14d05e
feat(ClientConfiguration): 외부 결제 Client 설정 추가
jminkkk May 29, 2024
6526ae9
feat(PaymentClient): 외부 API에 결제 요청을 보내는 PaymentClient 생성
jminkkk May 29, 2024
89e452c
feat(ReservationService, PaymentService): 예약 로직에서 결제 승인하는 기능 추가
jminkkk May 29, 2024
3750a51
feat(PaymentClient): 결제 API 호출 시 예외처리
jminkkk May 29, 2024
40b81f8
refactor(PaymentClient): 결제 관련 외부 Client 객체 추상화
jminkkk May 29, 2024
70cb8a0
refactor(ReservationServiceTest): 예약 시 FakePaymentClient 호출하도록 수정
jminkkk May 29, 2024
4664d31
refactor(ReservationIntegrationTest): 안쓰는 import 제거
jminkkk May 29, 2024
c5f4a34
refactor(GlobalExceptionHandler): 500 대 HttpServerErrorException 에러 처리
jminkkk May 30, 2024
8d4e573
refactor(ClientConfiguration): Authorization 헤더 상수로 변경
jminkkk May 30, 2024
0cb75e4
refactor(ClientConfiguration): 인증 헤더의 시크릿 키에서 상수 추출
jminkkk May 30, 2024
43d3457
refactor(ReservationService): admin, 일반 유저 예약 생성 중복 로직 제거
jminkkk May 30, 2024
cf8f2e9
refactor(RestClientConfiguration): ClientConfiguration를 더 명확하게 이름 변경
jminkkk May 31, 2024
fc576c4
refactor(GlobalExceptionHandler): HttpServerErrorException를 핸들링하는 메서드…
jminkkk May 31, 2024
846f338
refactor(TossClientErrorResponse): Toss에서 제공하는 에러 객체 이름 변경
jminkkk May 31, 2024
f7c1236
refactor(PaymentProperties): 결제 요청 시 필요한 비밀번호 값을 properties로 위치 이동
jminkkk May 31, 2024
e0f50e2
refactor(/payment, /toss): 결제 client 관련 파일들의 위치 수정
jminkkk May 31, 2024
06bbf14
refactor(TossPaymentErrorHandler): Toss 결제 처리 중 발생한 에러를 핸들링하는 객체 구현 및 등록
jminkkk Jun 1, 2024
e8ca752
refactor(TossErrorCodeNotForUser): 유저에게 보여주지 않을 토스 에러 코드 이넘으로 관리
jminkkk Jun 1, 2024
72a0361
test(TossRestClientTest): 토스 결제 승인 실패 시, 토스 에러 코드가 사용자에게 알리지 않을 사유라면 …
jminkkk Jun 1, 2024
09a27a8
test(FakePaymentClient): 결제 승인 요청 시 무조건 예외를 발생하는 keyword 추가 및 결제 오류 시…
jminkkk Jun 1, 2024
a8095d6
test(PaymentRestClientConfiguration): 타임아웃 설정
jminkkk Jun 1, 2024
aab08d3
test(GlobalExceptionHandler): SocketTimeoutException에 대한 핸들링 추가
jminkkk Jun 1, 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
31 changes: 7 additions & 24 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# 요구사항 분석
# 정책

## 예약 대기 정책

Expand All @@ -13,26 +13,9 @@
- 예약 취소 시, 해당 예약에 대해 우선순위가 높은 대기가 자동으로 예약된다.
- 관리자는 예약 대기를 거절할 수 있다.

## 3단계
- 예약 대기 요청 API 구현
- [x] 예약 대기 스키마 설계
- [x] 예약 대기 엔티티 구현
- [x] 예약 대기 요청 로직 구현
- [x] 예약 대기 취소 API 구현
- [x] 예약 조회 API 변경
- 예약 조회 시, 대기 목록을 포함한다.

## 4단계
- [x] 에약 대기 승인 기능 구현
- 예약 취소 API에서 대기 번호 1번인 대기가 예약이 되도록 변경
- [x] 예약 대기 거절 API 구현
- [x] 어드민 예약 대기 목록 조회 API 구현

---
## 1단계
- [x] gradle 의존성 추가
- [x] 엔티티 매핑
- [x] 연관관계 매핑

## 2단계
- [x] 내 예약 조회 API 구현
# 요구사항 분석

Choose a reason for hiding this comment

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

👍

- 예약 API 변경
- [x] 예약 요청 시 결제 기능 추가
- [x] 결제 실패 시 예외 처리
- [x] 사용자에게 결재 실패 사유 전달
- [x] 외부 API 연동
31 changes: 31 additions & 0 deletions src/main/java/roomescape/common/GlobalExceptionHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonMappingException.Reference;
import io.jsonwebtoken.JwtException;
import java.net.SocketTimeoutException;
import java.util.NoSuchElementException;
import java.util.logging.Logger;
import java.util.stream.Collectors;
Expand All @@ -15,8 +16,11 @@
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.HttpServerErrorException;
import roomescape.common.exception.ForbiddenException;
import roomescape.common.exception.UnAuthorizationException;
import roomescape.payment.client.toss.TossClientErrorResponse;

@ControllerAdvice
public class GlobalExceptionHandler {
Expand Down Expand Up @@ -49,6 +53,24 @@ public ResponseEntity<ProblemDetail> catchHttpMessageNotReadableException(HttpMe
.body(ProblemDetail.forStatusAndDetail(HttpStatus.BAD_REQUEST, exceptionMessage));
}

@ExceptionHandler
public ResponseEntity<ProblemDetail> catchHttpClientErrorException(HttpClientErrorException ex) {
logger.warning(EXCEPTION_PREFIX + ex.getMessage());

TossClientErrorResponse response = ex.getResponseBodyAs(TossClientErrorResponse.class);
return ResponseEntity.status(ex.getStatusCode())
.body(ProblemDetail.forStatusAndDetail(ex.getStatusCode(), response.message()));
}

@ExceptionHandler
public ResponseEntity<ProblemDetail> catchHttpServerErrorException(HttpServerErrorException ex) {
logger.warning(EXCEPTION_PREFIX + ex.getMessage());

TossClientErrorResponse response = ex.getResponseBodyAs(TossClientErrorResponse.class);
return ResponseEntity.status(ex.getStatusCode())
.body(ProblemDetail.forStatusAndDetail(ex.getStatusCode(), response.message()));
}

@ExceptionHandler
public ResponseEntity<ProblemDetail> catchBadRequestException(IllegalArgumentException ex) {
logger.warning(EXCEPTION_PREFIX + ex.getMessage());
Expand Down Expand Up @@ -90,6 +112,15 @@ public ResponseEntity<ProblemDetail> catchConflictException(IllegalStateExceptio
.body(ProblemDetail.forStatusAndDetail(HttpStatus.CONFLICT, ex.getMessage()));
}

@ExceptionHandler
public ResponseEntity<ProblemDetail> catchSocketTimeoutException(SocketTimeoutException ex) {
logger.warning(EXCEPTION_PREFIX + ex.getClass() + " " + ex.getMessage());
String exceptionMessage = "요청 처리 중 처리 중단(Timeout)이 발생했습니다. 잠시 후 다시 시도해 주세요.";

return ResponseEntity.status(500)
.body(ProblemDetail.forStatusAndDetail(HttpStatus.INTERNAL_SERVER_ERROR, exceptionMessage));
}

@ExceptionHandler
public ResponseEntity<ProblemDetail> catchInternalServerException(Exception ex) {
logger.warning(EXCEPTION_PREFIX + ex.getClass() + " " + ex.getMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package roomescape.common.exception;

public class ClientException extends RuntimeException {
public ClientException(final String message) {
super(message);
}
}
9 changes: 9 additions & 0 deletions src/main/java/roomescape/payment/client/PaymentClient.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package roomescape.payment.client;

import roomescape.payment.dto.request.ConfirmPaymentRequest;
import roomescape.payment.model.Payment;

public interface PaymentClient {

Choose a reason for hiding this comment

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

👍 인터페이스화 한거 너무 좋네요!

Copy link
Author

Choose a reason for hiding this comment

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

꼭 toss 말고 다른 결제 client를 사용하게 될 수도 있어서 추상화가 필요한 부분이라고 생각했어요 ㅎㅎ


Payment confirm(ConfirmPaymentRequest confirmPaymentRequest);
}
23 changes: 23 additions & 0 deletions src/main/java/roomescape/payment/client/PaymentProperties.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package roomescape.payment.client;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(value = "payment")

Choose a reason for hiding this comment

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

👍

해당 어노테이션은 어떤 역할을 해줘요?

Copy link
Author

Choose a reason for hiding this comment

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

Spring boot가 자동으로 외부 설정 파일(properties 또는 yml)의 속성값을 인식하여 클래스의 필드값으로 바인딩해주는 어노테이션입니다 !

기존에 사용했던 @value보다 쉽게 값들을 주입받을 수 있고 클래스로 더 편하게 관리할 수 있다고 생각하여 사용했습니다 😁

public class PaymentProperties {

private final String secretKey;
private final String password;

public PaymentProperties(final String secretKey, final String password) {
this.secretKey = secretKey;
this.password = password;
}

public String getSecretKey() {
return secretKey;
}

public String getPassword() {
return password;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package roomescape.payment.client;

import java.nio.charset.StandardCharsets;
import java.util.Base64;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpHeaders;
import org.springframework.http.client.ClientHttpRequestFactory;
import org.springframework.http.client.SimpleClientHttpRequestFactory;
import org.springframework.web.client.RestClient;
import roomescape.payment.client.toss.TossPaymentErrorHandler;

@Configuration
public class PaymentRestClientConfiguration {

private static final String BASIC_PREFIX = "Basic ";

private final PaymentProperties paymentProperties;

public PaymentRestClientConfiguration(final PaymentProperties paymentProperties) {
this.paymentProperties = paymentProperties;
}

@Bean
public RestClient tossRestClient() {
byte[] encodedBytes = Base64.getEncoder()
.encode((paymentProperties.getSecretKey() + paymentProperties.getPassword())
.getBytes(StandardCharsets.UTF_8));

String authorizations = BASIC_PREFIX + new String(encodedBytes);

return RestClient.builder()
.baseUrl("https://api.tosspayments.com/v1/payments")
.defaultHeader(HttpHeaders.AUTHORIZATION, authorizations)
.defaultStatusHandler(new TossPaymentErrorHandler())
.requestFactory(getClientHttpRequestFactory())
.build();
}

private ClientHttpRequestFactory getClientHttpRequestFactory() {
SimpleClientHttpRequestFactory simpleClientHttpRequestFactory = new SimpleClientHttpRequestFactory();
simpleClientHttpRequestFactory.setConnectTimeout(3);
simpleClientHttpRequestFactory.setReadTimeout(30);
Comment on lines +42 to +43
Copy link
Author

Choose a reason for hiding this comment

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

타임 아웃 설정 시간 기준

토스페이먼츠 API의 Read Timeout은 30초에서 60초로 설정하면 돼요. 타임아웃(Timeout) | 토스페이먼츠 개발자센터

  • ReadTimeout
    • 해당 타임타웃은 토스 문서에 제공된 권장 시간인 30 ~ 60초 중 가장 빠른 시간으로 설정했습니다 !
  • ConnectTimeout
    • 해당 타임아웃은 사실 어느 정도가 적절한 수준인지 감이 안왔었는데요. 수업 시간 내용을 토대로 3초로 설정했습니다 😅
    • 연결에서 3초 이상이 걸린다는 것은 비정상적인 상황이라고 하더라구요 ! 때문에 차라리 빠른 예외를 터뜨려서 사용자가 재시도 등을 시도할 수 있게 하는 것이 적절하다고 생각하였습니다.

Copy link

Choose a reason for hiding this comment

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

굿굿! 좋네요 💯

return simpleClientHttpRequestFactory;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package roomescape.payment.client.toss;

public record TossClientErrorResponse(String code,
String message) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package roomescape.payment.client.toss;


import java.util.Arrays;

public enum TossErrorCodeNotForUser {
INVALID_API_KEY,
NOT_FOUND_TERMINAL_ID,
INVALID_AUTHORIZE_AUTH,
INVALID_UNREGISTERED_SUBMALL,
NOT_REGISTERED_BUSINESS,
UNAPPROVED_ORDER_ID,
UNAUTHORIZED_KEY,
FORBIDDEN_REQUEST,
INCORRECT_BASIC_AUTH_FORMAT,
;
Comment on lines +7 to +16
Copy link
Author

Choose a reason for hiding this comment

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

조앤 이 부분도 수업 시간을 통해 다시 생각해보게 되면서 추가한 부분인데요 !

1차 리뷰 요청 당시에는 토스 측의 에러 메시지를 전부 그대로 매핑하여 에러를 터뜨렸었어요.

"토스 측의 모든 에러 메시지가 사용자 또는 클라이언트가 알게 할 필요가 있을까?"

그런데 위 질문을 통해 고민해본 결과 토스 client 요청 중 발생할 수 있는 에러 중에서 사용자에게 알릴 코드와 아닌 코드를 분리해서 처리하는 것이 적절하다고 생각했어요.
따라서 토스 문서 (에러 코드 - 결제 승인)에서, 제가 판단하기에 클라이언트나 사용자 측에서 조치를 할 수 없는 에러인 경우라면, 해당 메시지를 전달하는 것이 의미가 없다고 생각하여 다른 메시지로 바꾸어 전달하고자 했어요.
그러기 위해서 변경이 필요한 토스 에러 code를 enum으로 관리하도록 했는데요.


  1. enum으로 변경해야 할 외부의 에러 코드를 관리하는 것에 대해 조앤은 어떻게 생각하시는지 궁금합니다 🤔 (뭔가 설명할 수 없는 찝찝함이 있어요 😭)
  2. 또, 실무에서는 보통 이런 에러 메시지를 변경할 것인가에 대한 문제 또한 기획자와 상의하나요? 아니면 개발자의 역량일까요?
  3. 개발자의 역량이라면, 앞서 제가 말했듯 "제가 생각하기에 클라이언트나 사용자 측에서 조치를 할 수 없는 에러라면, 해당 메시지를 전달하는 것이 의미가 없다"라는 변경 기준이 적절한 기준이 맞을까요?

Copy link

Choose a reason for hiding this comment

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

예를 들어 토스쪽에서 해당 에러코드를 breaking change 내는 경우, 사용하는 쪽에서 1:1로 매핑하고 있다면 일일이 다 바꿔줘야하는 불편함이 있을 것 같아요. 또한 그걸 모르고 계속 유지보수한다면 예상치 못한 상황이 발생할 수도 있고요ㅎㅎ
근데 지금은 결제 요구사항이기도 하고 디펜던시가 굉장히 큰 상황에서 에러 코드를 직접 관리해도 괜찮을 것 같다는 생각이 들어요. 대신 메시지는 커스텀해서 내릴 수 있을 것 같아요.

다 다를 것 같긴 한데 제 경험으로는 요구사항을 바탕으로 발생가능한 예외 상황에 대해 다같이 논의해요! 저는 좀 스타트업쪽이라 그런데 큰회사는 요구사항이 찍혀내려오는 경우도 있다고 들었어요.

저도 좀 비슷하게 생각하긴 해요. 에러라는게 사실 피드백인데, 다음에 어떤 행위를 해야 개선할 수 있는지 가이드가 되지 않는다면 의미 없다고 생각하긴 해요. 하지만 사용성의 측면에서 유저에게 지금이 어떤 상황인지 정도를 알리는 용도로서 적절한 메시지로 변환해서 내릴 순 있을 것 같아요.


public static boolean hasContains(final String message) {
return Arrays.stream(TossErrorCodeNotForUser.values())
.anyMatch(tossErrorCodeNotForUser -> tossErrorCodeNotForUser.name().equals(message));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package roomescape.payment.client.toss;

import org.springframework.stereotype.Component;
import org.springframework.web.client.RestClient;
import roomescape.payment.client.PaymentClient;
import roomescape.payment.dto.request.ConfirmPaymentRequest;
import roomescape.payment.model.Payment;

@Component
public class TossPaymentClient implements PaymentClient {

private final RestClient restClient;

public TossPaymentClient(final RestClient restClient) {
this.restClient = restClient;
}

@Override
public Payment confirm(ConfirmPaymentRequest confirmPaymentRequest) {
return restClient.post()
.uri("/confirm")
.body(confirmPaymentRequest)
.retrieve()
.toEntity(Payment.class)
.getBody();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package roomescape.payment.client.toss;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.web.client.ResponseErrorHandler;
import roomescape.common.exception.ClientException;

public class TossPaymentErrorHandler implements ResponseErrorHandler {
private static final ObjectMapper objectMapper = new ObjectMapper();

@Override
public boolean hasError(final ClientHttpResponse response) throws IOException {
return response.getStatusCode().is4xxClientError() || response.getStatusCode().is5xxServerError();
}

@Override
public void handleError(final ClientHttpResponse response) throws IOException {
TossClientErrorResponse tossClientErrorResponse = objectMapper.readValue(response.getBody(),
TossClientErrorResponse.class);

if (TossErrorCodeNotForUser.hasContains(tossClientErrorResponse.code())) {
throw new ClientException("결제 오류입니다. 같은 문제가 반복된다면 문의해주세요.");
}

throw new ClientException(tossClientErrorResponse.message());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package roomescape.payment.dto.request;

import roomescape.reservation.dto.request.CreateMyReservationRequest;

public record ConfirmPaymentRequest(String paymentKey,
String orderId,
Long amount) {
public static ConfirmPaymentRequest from(final CreateMyReservationRequest createMyReservationRequest) {
return new ConfirmPaymentRequest(
createMyReservationRequest.paymentKey(),
createMyReservationRequest.orderId(),
createMyReservationRequest.amount());
}
}
25 changes: 25 additions & 0 deletions src/main/java/roomescape/payment/model/Payment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package roomescape.payment.model;

public class Payment {
private String paymentKey;
private String orderId;
private Long amount;

public Payment(final String paymentKey, final String orderId, final Long amount) {
this.paymentKey = paymentKey;
this.orderId = orderId;
this.amount = amount;
}

public String getPaymentKey() {
return paymentKey;
}

public String getOrderId() {
return orderId;
}

public Long getAmount() {
return amount;
}
}
20 changes: 20 additions & 0 deletions src/main/java/roomescape/payment/service/PaymentService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package roomescape.payment.service;

import org.springframework.stereotype.Service;
import roomescape.payment.client.PaymentClient;
import roomescape.payment.dto.request.ConfirmPaymentRequest;
import roomescape.payment.model.Payment;

@Service
public class PaymentService {

private final PaymentClient paymentClient;

Choose a reason for hiding this comment

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

다른 서비스에서 Client를 직접 호출하는 게 아니라 Service로 한번 더 감싸주셨군요?

Copy link
Author

Choose a reason for hiding this comment

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

현재는 "결제 승인"이라는 기능에 대해 단순 외부 client 연동만 하면 된다는 미션 요구사항이 제공되었지만,
외부 client에 요청을 한다는 것 또한 결제라는 기능을 위한 비즈니스 요구사항이라고 생각하여 Service 계층을 만들어 해당 역할을 하는 객체를 호출했습니다 !
또 결제에 대한 추가적인 요구사항이 변경될 경우에도 해당 계층 덕분에 더 유연할 것이라고도 생각했습니다 😁


public PaymentService(PaymentClient paymentClient) {
this.paymentClient = paymentClient;
}

public Payment createPayment(ConfirmPaymentRequest paymentRequest) {
return paymentClient.confirm(paymentRequest);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,14 @@ public record CreateMyReservationRequest(

@Positive(message = "예약 테마 식별자는 양수만 가능합니다.")
@NotNull(message = "예약 등록 시 테마는 필수입니다.")
Long themeId) {
Long themeId,

@NotNull(message = "결제 키를 입력해주세요.")
String paymentKey,

@NotNull(message = "주문을 입력해주세요.")
String orderId,

@NotNull(message = "결제 금액를 입력해주세요.")
Long amount) {
}
Loading