-
Notifications
You must be signed in to change notification settings - Fork 1
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: 주문 완료하기 API 구현 #472
feat: 주문 완료하기 API 구현 #472
Conversation
Walkthrough이 변경 사항은 주문 완료 API의 구현을 중심으로 이루어졌습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OnboardingOrderController
participant OrderService
participant OrderRepository
participant PaymentClient
participant OrderValidator
Client->>+OnboardingOrderController: POST /complete
OnboardingOrderController->>+OrderService: completeOrder(request)
OrderService->>+OrderRepository: findByNanoId(request.nanoId)
OrderRepository-->>-OrderService: Order
OrderService->>+OrderValidator: validateCompleteOrder(order, ...)
OrderValidator-->>-OrderService: Validation Result
OrderService->>+PaymentClient: confirmPayment(request.paymentKey)
PaymentClient-->>-OrderService: Payment Confirmation
OrderService->>Order: complete(paymentKey)
OrderService-->>-OnboardingOrderController: Response
OnboardingOrderController-->>-Client: Response
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- src/main/java/com/gdschongik/gdsc/domain/common/vo/Money.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/api/OnboardingOrderController.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/application/OrderService.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/dao/OrderRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/domain/Order.java (2 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/domain/OrderCompletedEvent.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/order/domain/OrderValidator.java (3 hunks)
- src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/infra/feign/payment/client/PaymentClient.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/common/vo/MoneyTest.java (4 hunks)
- src/test/java/com/gdschongik/gdsc/domain/order/application/OrderServiceTest.java (3 hunks)
- src/test/java/com/gdschongik/gdsc/domain/order/domain/MoneyInfoTest.java (3 hunks)
- src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (2 hunks)
- src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (2 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/infra/feign/payment/client/PaymentClient.java
Additional comments not posted (21)
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java (1)
18-18
:FEE
상수 초기화 변경을 승인합니다.
BigDecimal
대신long
값으로 직접 초기화하는 것은 코드를 단순화시키고 효율을 높일 수 있습니다.다른 상수 초기화와 일관성이 있는지 확인하세요.
Verification successful
FEE
상수 초기화 변경을 승인합니다.
BigDecimal
대신long
값으로 직접 초기화하는 것은 코드를 단순화시키고 효율을 높일 수 있습니다. 다른 상수 초기화에서도 일관되게Money.from
메서드가long
값을 사용하고 있습니다.
src/test/java/com/gdschongik/gdsc/global/common/constant/RecruitmentConstant.java
src/test/java/com/gdschongik/gdsc/domain/order/domain/MoneyInfoTest.java
src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java
src/test/java/com/gdschongik/gdsc/domain/order/application/OrderServiceTest.java
src/test/java/com/gdschongik/gdsc/domain/common/vo/MoneyTest.java
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of constant initializations across the codebase. # Test: Search for similar constant initializations. Expect: Consistent usage of Money.from with long values. rg --type java 'Money.from\([0-9]+L\)' src/Length of output: 3659
Script:
#!/bin/bash # Description: Verify if there are any instances of Money.from using types other than long values. # Test: Search for Money.from initializations with BigDecimal values. rg --type java 'Money.from\(new BigDecimal\([0-9]+\)\)' src/Length of output: 62
src/main/java/com/gdschongik/gdsc/domain/order/api/OnboardingOrderController.java (1)
32-34
: 주문 완료 API 변경을 승인합니다.URL 매핑 및 파라미터 변경으로 API가 더 간단해졌습니다.
클라이언트 측 및 애플리케이션의 다른 부분에 미치는 영향을 확인하세요.
src/test/java/com/gdschongik/gdsc/domain/order/domain/MoneyInfoTest.java (3)
15-17
: 금액 타입 변경 확인
Money.from
메서드의 입력 타입이BigDecimal
에서long
으로 변경되었습니다. 테스트 코드에서도 이에 맞추어long
타입을 사용하고 있으며, 이는 적절한 변경으로 보입니다.
30-32
: 예외 처리 테스트 코드 확인금액 계산 결과가 일치하지 않을 때 예외를 발생시키는 로직의 테스트 코드입니다. 입력값으로
long
타입을 사용하여Money.from
메서드의 변경사항을 반영하고 있습니다. 예외 발생 조건과 메시지 검증이 적절히 이루어지고 있습니다.
43-49
: 객체 동등성 검증 테스트
MoneyInfo
객체의 동등성을 검증하는 테스트 코드입니다. 모든 금액이 같을 경우 같은 객체로 인식되어야 합니다. 변경된Money.from
메서드를 사용하여long
타입의 입력값을 제공하고 있으며, 이는 기대한 동작을 정확히 반영하고 있습니다.src/main/java/com/gdschongik/gdsc/domain/common/vo/Money.java (1)
43-47
: 새로운Money.from
메서드 추가 확인
Long
타입의 금액을 입력받아Money
객체를 생성하는 새로운 메서드입니다.BigDecimal.valueOf
를 사용하여Long
을BigDecimal
로 변환하고 있으며, 이는 타입 안전성을 보장합니다. 또한, 입력값의 null 체크를 통해 안정성을 높이고 있습니다. 적절한 구현으로 보입니다.src/test/java/com/gdschongik/gdsc/domain/common/vo/MoneyTest.java (2)
17-18
: 금액 동등성 테스트 코드 변경 확인
Money.from
메서드 호출 시long
타입을 사용하는 변경이 반영되었습니다. 이는 새로운 메서드 구현에 따른 적절한 테스트 코드 수정입니다. 모든 테스트 케이스에서 동일한 금액이면 동일한 객체로 인식되는 것을 확인하는 로직이며, 변경된 메서드를 정확히 테스트하고 있습니다.Also applies to: 27-27
59-60
: 금액 해시코드 테스트 코드 변경 확인
Money.from
메서드의 입력 타입 변경에 따라 해시코드 생성 로직의 테스트 코드도 수정되었습니다.long
타입을 사용하여Money
객체를 생성하고, 해시코드의 일관성을 검증하는 로직이 적절히 구현되어 있습니다. 이는 변경사항을 정확히 반영하고 있는 테스트입니다.Also applies to: 70-70
src/main/java/com/gdschongik/gdsc/domain/order/domain/Order.java (1)
Line range hint
56-112
: 주문 완료 처리 로직 추가 확인새롭게 추가된
paymentKey
필드와complete
메서드를 통해 주문을 완료 처리하는 로직을 구현하였습니다.complete
메서드는 주문 상태를COMPLETED
로 변경하고, 결제 키를 저장한 후, 주문 완료 이벤트를 등록합니다. 이는 비즈니스 요구사항을 정확히 반영하고 있으며, 적절한 예외 처리와 상태 관리가 이루어지고 있습니다. 또한,isCompleted
메서드를 통해 주문의 완료 상태를 확인할 수 있습니다. 이는 주문 관리 로직의 명확성과 유지보수성을 높이는 좋은 구현입니다.src/main/java/com/gdschongik/gdsc/domain/order/domain/OrderValidator.java (3)
14-14
: 새로운 Optional import 추가
java.util.Optional
이 새로 추가되었습니다. 이는validateCompleteOrder
메소드에서 사용됩니다.
42-42
: TODO 주석 추가
validatePendingOrderCreate
메소드에 "주문 완료 검증 로직처럼 Optional로 변경"이라는 TODO 주석이 추가되었습니다. 이는 향후 이 메소드의 인자를 Optional로 변경할 계획임을 나타냅니다.
82-101
: 새로운validateCompleteOrder
메소드 추가주문 완료를 검증하는 새로운 메소드
validateCompleteOrder
가 추가되었습니다. 이 메소드는 주문이 이미 완료되었는지, 쿠폰이 유효한지, 회원이 일치하는지, 최종 결제 금액이 요청된 금액과 일치하는지를 검증합니다. 이 로직은 주문 완료 API의 핵심 부분으로, 예외 처리가 적절하게 구현되어 있습니다.src/main/java/com/gdschongik/gdsc/domain/order/application/OrderService.java (2)
15-21
: 새로운 import 문 추가
OrderCompleteRequest
,PaymentClient
,PaymentConfirmRequest
클래스를 위한 새로운 import 문이 추가되었습니다. 이는 주문 완료 로직에서 사용됩니다.
69-93
: 새로운completeOrder
메소드 추가주문 완료를 처리하는 새로운 메소드
completeOrder
가 추가되었습니다. 이 메소드는 주문 검증, 결제 확인, 주문 상태 업데이트 및 로깅을 포함합니다. 트랜잭셔널 어노테이션이 적절히 사용되었으며, 예외 처리도 잘 구현되어 있습니다.src/test/java/com/gdschongik/gdsc/domain/order/application/OrderServiceTest.java (2)
28-31
: Money 상수 값 변경
Money.from
메소드를 사용하여BigDecimal
에서long
으로 변경된 Money 상수 값들입니다. 이 변경은 코드의 일관성과 효율성을 높이기 위해 수행되었습니다.
75-122
: 새로운 테스트 메소드주문_완료할때
추가주문 완료 기능을 테스트하는 새로운 메소드
주문_완료할때
가 추가되었습니다. 이 테스트는 주문 생성, 완료 및 결제 세부 정보 검증을 포함합니다. 모의 객체와 결제 클라이언트의 상호 작용이 적절히 검증되고 있습니다.src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (1)
66-68
: 새로운PaymentClient
mock bean 추가통합 테스트에
PaymentClient
의 mock bean이 추가되었습니다. 이는 결제 관련 테스트에서 사용될 것입니다. 적절한 위치에 mock bean이 추가되어 있으며, 이는 테스트의 범위를 확장하는 데 도움이 됩니다.src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)
110-120
: 주문 관련 에러 코드 추가 확인새로운 주문 관련 에러 코드들이 추가되었습니다. 각 코드의 상태와 메시지가 적절하게 설정되었는지 확인하시기 바랍니다. 이러한 에러 코드들은 주문 완료 API의 다양한 실패 상황을 잘 나타내고 있습니다.
src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (3)
30-34
: 상수 초기화 값 업데이트 확인
Money
관련 상수들의 초기화 값이 업데이트되었습니다. 이 변경사항은 테스트 코드에서 금액 관련 검증을 위해 사용됩니다. 적절한 값으로 초기화되었는지 확인하시기 바랍니다.
23-24
: 필요한 import 추가 확인
Optional
과Nested
관련 import가 추가되었습니다. 이는 테스트 코드의 구조화를 위해 필요한 변경사항입니다.
70-288
: 테스트 메서드 구조화 및 새로운 테스트 케이스 추가 확인테스트 메서드들이
Nested
클래스로 잘 구조화되었으며, 다양한 주문 검증 시나리오에 대한 새로운 테스트 케이스가 추가되었습니다. 각 테스트 케이스가 명확하게 주문 관련 예외 상황을 검증하고 있는지 확인하시기 바랍니다.
src/main/java/com/gdschongik/gdsc/domain/order/domain/OrderCompletedEvent.java
Show resolved
Hide resolved
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.
확인했습니다
src/main/java/com/gdschongik/gdsc/domain/order/application/OrderService.java
Show resolved
Hide resolved
IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, currentMember); | ||
|
||
// when & then | ||
MoneyInfo moneyInfo = MoneyInfo.of(MONEY_20000_WON, MONEY_5000_WON, MONEY_15000_WON); |
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.
이것도 given 아닌가요?
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.
lgtm
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.
리뷰남겼습니다!
@@ -53,6 +53,8 @@ public class Order extends BaseEntity { | |||
@Embedded | |||
private MoneyInfo moneyInfo; | |||
|
|||
private String paymentKey; |
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.
paymentKey은 결제 후 결과값으로 주는 승인번호(멱등키) 일까요?
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.
아뇨 그냥 결제 자체를 식별하기 위한 키입니다.
토스페이먼츠에서 내려주는 Payment 객체의 경우 클라이언트의 결제 생성 요청 시점부터 존재하며, 서버는 클라이언트가 생성한 결제를 승인하게 될 뿐입니다. paymentKey의 경우 서버 입장에서는 결제를 승인하면서 처음 알게 되므로, 이때 저장해주는 것이라고 이해하시면 됩니다.
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.
아하 이해했습니다! 어쨋든 승인번호는 아니되, 해당 결제를 구분해주는 멱등키라는거군요.
OK입니다
orderValidator.validateCompleteOrder(order, issuedCoupon, currentMember, requestedAmount); | ||
|
||
var paymentRequest = new PaymentConfirmRequest(request.paymentKey(), order.getNanoId(), request.amount()); | ||
paymentClient.confirm(paymentRequest); |
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.
저는 여기서 paymentClient에서 에러가 발생한 경우 해당 order에 대해 에러상태로 변경시키는 로직이 필요할것이라 생각해요!
그부분은 따로 트랜잭션을 분리하든 REQUIRES_NEW 옵션으로 트랜잭션 전파속성 설정해서 에러상황속에서도 저장되게 핸들링해줄 필요가 있을거같습니다!
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.
결제 자체의 상태는 토스페이먼츠에서 관리가 되기 때문에 저희는 주문의 상태에만 신경쓰면 된다고 생각합니다
(PaymentClient에서 승인에 실패한 경우 토스페이먼츠 API 사용하여 Payment 객체를 조회하면 ABORTED라는 상태를 가지게 됩니다)
그리고 주문 상태의 경우 PENDING -> COMPLETED 상태 변경이 중요한 것이지, 말씀하신대로 결제 승인 시도가 실패했을 때 PENDING -> FAIL로 변경하는 것이 그렇게까지 저희 비즈니스에서 필요한 것 같지는 않습니다
즉 중요한 로직은 COMPLETED가 되었을 때 발생하는 것이고, 그 외 PENDING에서 어떤 사유로 COMPLETED 처리에 실패했는지는 중요하지 않으므로 별도 상태로 디비에 저장할 필요가 없다... 라고 봅니다
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.
저는 이부분이 조금 의아한게, 그렇다면 임시주문상태인 PENDING상태에서 결제를 실패하면 여전히 PENDING상태로 남아있고, 해당상태로 쭊 DB에 남아있게 되며, 사용자입장에선 해당 임시주문상태의 주문엔 다시접근할수 없는것 아닌가요??
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.
아 결제와 주문은 1대1 대응이에요
결제가 실패하면 다시 임시 주문 생성해서 요청해야 합니다
주문 테이블이 일종이 결제요청 히스토리처럼 되겠네요
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.
저희가 결제정보 - 주문정보를 따로 둘다 DB 에 저장하는게 아닌
현재 결제정보=주문정보 이렇게 하나의 테이블( ORDER)로 저장하고 잇으면, 해당 결제가 실패했는지 성공했는지 추후 알길이 없습니다. 단지 주문상태가 pending상태로 db에 계속존재하는건 실제로 임시주문생성 후 결제를 진행하지 않은건지, 임시주문생성후 결제시도를 햇을대 실패를 한건지 애매해 지잖아요?
이부분은 반드시 상태값을 구분해줘야할것같습니다.
이미 orderStatus에 canceled라는 상태값도 존재하니간요!
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.
결제가 실패했는지 성공했는지는 토스페이먼츠의 결제 조회 API로 확인 가능합니다!
어드민에서 해당 API 사용할 예정이에요
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.
주문 상태에 FAILED를 추가하게 되면, 오히려 토스페이먼츠의 결제 상태와 일치하지 않는 시점이 발생하게 됩니다
임시 결제 생성 후 10분이 지나면 해당 결제는 자동으로 실패하게 되는데요,
이때 서버 측의 주문 상태와 10분 경과로 인한 실패 건을 동기화할 수 없기 때문에 문제가 발생합니다
따라서 저희 측에서 논리적으로 일치시킬 수 있는 수준에서만 상태를 분리 (요청 상태 / 완료 상태) 함으로써
주문 상태를 최대한 단순화하고 불일치 문제를 해결하려는 것입니다
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.
++ 임시 주문 생성 후 결제를 진행하지 않은 것인지, 결제를 실패했는지는 Order 조회 시점에 몰라도 됩니다
어차피 결제 승인 요청을 해보면 해결될 일이니깐요 (물론 그전에 클라이언트에서 중복 요청을 막겠지만요...)
만약 실패한 건이라면 계속 실패할 것이고, 진행하지 않은 것이라면 시도하고 성공 처리하면 되겠지요
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.
+++ CANCELED는 결제 취소인 경우 (즉 환불 케이스) 이고 결제 실패와는 조금 다릅니다
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.
낮에 given 관련해서 남긴 코멘트 하나만 확인해주세요!
메이저한 부분은 아니니 미리 어푸루브 해두겠습니다~
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java
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.
LGTM
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
Money
객체를Long
값으로 생성할 수 있는 기능이 추가되었습니다.버그 수정
리팩토링
테스트
Money
객체 생성 테스트가BigDecimal
에서long
값으로 변경되었습니다.