-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: 결제 정보를 조회한다 #40
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
base: develop
Are you sure you want to change the base?
Conversation
- PG사 독립적인 PaymentMethod 설계 (추후 Adapter에서 매핑 예정) - 결제 완료 이후 상품 지급 등은 Order 도메인에서 상태를 관리합니다. - payment는 pg사를 이용한 “결제”에 집중하는 도메인입니다.
📋 워크스루결제(Payment) 도메인 모델과 관련 열거형(PaymentMethod, PaymentStatus, PgProvider)을 정의하고, 클린 아키텍처 원칙에 따라 애플리케이션 포트, 어댑터(JPA 저장소, 컨트롤러, DTO), 비즈니스 로직을 구현한 결제 조회 기능의 전체 계층을 추가합니다. 📊 변경사항
🔄 시퀀스 다이어그램sequenceDiagram
actor Client
participant Controller as PaymentController
participant Service as PaymentService
participant Reader as PaymentReaderImpl
participant Repo as PaymentJpaRepository
Client->>Controller: GET /api/payments/{id}
Controller->>Service: getPayment(id)
Service->>Reader: findById(id)
Reader->>Repo: findByIdOrNull(id)
Repo-->>Reader: PaymentJpaEntity?
Reader->>Reader: toDomain()
Reader-->>Service: Payment?
alt Payment exists
Service-->>Controller: Payment
Controller->>Controller: GetPaymentResponse.from(payment)
Controller-->>Client: 200 GetPaymentResponse
else Payment not found
Service-->>Controller: NoSuchElementException
Controller-->>Client: 500 Error
end
⏱️ 예상 코드 리뷰 소요 시간🎯 3 (보통) | ⏱️ ~20분 주의 깊게 검토할 영역:
🔗 관련 PR
👥 검토자 제안
🐰 축하 시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
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: 1
🧹 Nitpick comments (8)
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaRepository.kt (1)
4-4: 사용하지 않는 import를 제거하세요.
org.springframework.data.jpa.repository.Queryimport가 사용되지 않습니다.다음과 같이 수정하세요:
package app.payment.core.adapter.outbound import org.springframework.data.jpa.repository.JpaRepository -import org.springframework.data.jpa.repository.Query interface PaymentJpaRepository : JpaRepository<PaymentJpaEntity, Long> {payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/GetPaymentsResponse.kt (1)
5-17: count 필드의 필요성을 고려하세요.
count필드가payments.size와 동일한 정보를 담고 있어 중복입니다. 클라이언트가 명시적으로 count를 필요로 하는 경우가 아니라면, computed property로 변경하는 것을 고려해보세요.현재 구조가 API 응답 형식의 명시성을 위한 것이라면 그대로 유지해도 좋습니다. 다만, 향후 페이징 기능을 추가할 때는
count가 전체 항목 수를 의미하도록 변경될 수 있으니 네이밍을 고려하세요.payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/Payment.kt (1)
6-20: 빈 클래스 블록(detekt 경고) 제거 제안현재 본문이 비어 있어 detekt EmptyClassBlock 경고가 발생합니다. 본 파일 목적이 순수한 도메인 모델이면 본문 중괄호를 제거해 간단히 해소하는 편이 깔끔합니다. 필요 시 이후 도메인 불변성 검증 로직을 추가하세요.
아래와 같이 수정:
-class Payment( +class Payment( val id: Long, val orderId: String, val userId: Long, val amount: BigDecimal, val method: PaymentMethod, val pgProvider: PgProvider, val status: PaymentStatus, val transactionId: String?, val pgTransactionId: String?, val paidAt: OffsetDateTime?, val createdAt: OffsetDateTime, val updatedAt: OffsetDateTime -) { -} +)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/GetPaymentResponse.kt (1)
9-20: 식별자 노출 범위 재검토(privacy/API 계약)userId, transactionId 노출이 비즈니스/보안 요구사항에 부합하는지 확인 부탁드립니다. 외부 클라이언트 대상이라면 최소화/마스킹 고려가 필요할 수 있습니다.
원하시면 API 계약 정리와 마스킹 전략 초안 드리겠습니다.
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentReaderImpl.kt (1)
8-15: 간결하고 명확합니다Spring Data의 findByIdOrNull + toDomain 사용이 적절합니다. 서비스 계층이 @transactional(readOnly=true)이므로 현재는 충분합니다. 혹시 직접 호출 가능성이 있다면 이 어댑터에도 readOnly 트랜잭션을 추가하는 것을 고려해 주세요.
@Component -class PaymentReaderImpl( +@org.springframework.transaction.annotation.Transactional(readOnly = true) +class PaymentReaderImpl( private val paymentJpaRepository: PaymentJpaRepository ) : PaymentReader {payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaEntity.kt (2)
25-27: 고유 제약 조건 재검토(업무 규칙 확인 필요)
- order_id unique=true: 주문당 결제 1건이라는 강한 제약입니다. 재시도/부분취소/부분결제 시 확장성이 제한됩니다.
- transaction_id/pg_transaction_id: 외부/내부 트랜잭션 식별자라면 고유 제약 및 인덱스가 유용합니다.
업무 규칙에 맞게 다음 중 선택 적용을 검토해 주세요.
- @Column(name = "order_id", nullable = false, unique = true) + @Column(name = "order_id", nullable = false/*, unique = true*/) var orderId: String, @@ - @Column(name = "transaction_id") - var transactionId: String? = null, + @Column(name = "transaction_id", unique = true) + var transactionId: String? = null, @@ - @Column(name = "pg_transaction_id") - var pgTransactionId: String? = null, + @Column(name = "pg_transaction_id", unique = true) + var pgTransactionId: String? = null,필요 시 @table(uniqueConstraints=…)로 복합 고유키도 고려하세요.
Also applies to: 46-50
61-76: toDomain에서 null‑safety 보강id/createdAt/updatedAt를 nullable로 조정하면 매핑 시 확정(non-null) 보장이 필요합니다.
- return Payment( - id = id, + return Payment( + id = requireNotNull(id) { "Persisted PaymentJpaEntity must have id" }, orderId = orderId, userId = userId, amount = amount, method = method, pgProvider = pgProvider, status = status, transactionId = transactionId, pgTransactionId = pgTransactionId, paidAt = paidAt, - createdAt = createdAt, - updatedAt = updatedAt + createdAt = requireNotNull(createdAt), + updatedAt = requireNotNull(updatedAt) )payment/payment-core/application/src/main/kotlin/app/payment/core/application/service/PaymentService.kt (1)
15-18: 도메인 전용 예외 도입으로 명확한 404 처리 권장 (추가 구현 필요)현재
NoSuchElementException대신 도메인 전용 예외PaymentNotFoundException을 사용하면 예외 처리가 명확해집니다. 다만 이 변경을 완료하려면 다음 두 가지를 함께 구현해야 합니다:
- PaymentNotFoundException 클래스 생성 (도메인 예외)
- 전역 예외 처리기(@RestControllerAdvice)에서 PaymentNotFoundException → 404 매핑 (현재 해당 인프라 없음)
PaymentService.kt 변경 예시:
- return paymentReader.findById(paymentId) - ?: throw NoSuchElementException("Payment not found with id: $paymentId") + return paymentReader.findById(paymentId) + ?: throw PaymentNotFoundException(paymentId)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/GetPaymentResponse.kt(1 hunks)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/GetPaymentsResponse.kt(1 hunks)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/PaymentController.kt(1 hunks)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaEntity.kt(1 hunks)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaRepository.kt(1 hunks)payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentReaderImpl.kt(1 hunks)payment/payment-core/application/src/main/kotlin/app/payment/core/application/port/inbound/PaymentUseCase.kt(1 hunks)payment/payment-core/application/src/main/kotlin/app/payment/core/application/port/outbound/PaymentReader.kt(1 hunks)payment/payment-core/application/src/main/kotlin/app/payment/core/application/service/PaymentService.kt(1 hunks)payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/Payment.kt(1 hunks)payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PaymentMethod.kt(1 hunks)payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PaymentStatus.kt(1 hunks)payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PgProvider.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaRepository.kt
[warning] 6-7: The class or object PaymentJpaRepository is empty.
(detekt.empty-blocks.EmptyClassBlock)
payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/Payment.kt
[warning] 19-20: The class or object Payment is empty.
(detekt.empty-blocks.EmptyClassBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-payment
🔇 Additional comments (10)
payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PaymentMethod.kt (1)
3-10: LGTM!결제 수단을 나타내는 enum이 명확하게 정의되어 있습니다. 일반적인 결제 수단들을 모두 포함하고 있어 확장성도 좋습니다.
payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PgProvider.kt (1)
3-6: LGTM!PG 제공자를 나타내는 enum이 명확하게 정의되어 있습니다. 현재는 토스페이먼츠와 카카오페이만 지원하지만, 필요시 쉽게 확장할 수 있는 구조입니다.
payment/payment-core/domain/src/main/kotlin/app/payment/core/domain/PaymentStatus.kt (1)
3-10: LGTM!결제 상태를 나타내는 enum이 명확하게 정의되어 있습니다. 결제 라이프사이클의 모든 주요 상태(준비, 대기, 완료, 부분취소, 취소, 실패)를 포함하고 있어 적절합니다.
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaRepository.kt (1)
6-7: LGTM!Spring Data JPA의 기본 CRUD 기능만 사용하는 경우 빈 인터페이스로 정의하는 것이 적절합니다. detekt의 EmptyClassBlock 경고는 이 경우 false positive입니다.
payment/payment-core/application/src/main/kotlin/app/payment/core/application/port/outbound/PaymentReader.kt (1)
5-7: LGTM!outbound port 인터페이스가 명확하게 정의되어 있습니다. nullable 반환 타입을 사용하여 결제 정보가 없는 경우를 적절히 표현하고 있습니다.
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/PaymentController.kt (1)
8-10: LGTM!생성자 주입을 통한 의존성 관리가 적절하게 구현되어 있습니다.
payment/payment-core/application/src/main/kotlin/app/payment/core/application/port/inbound/PaymentUseCase.kt (1)
5-7: 예외 처리 구현이 올바르게 되어 있습니다.검증 결과,
PaymentService.getPayment메서드는PaymentReader.findById의 nullablePayment?을 Elvis 연산자(?:)를 사용하여 올바르게 처리하고 있습니다. 결제 정보가 없을 때NoSuchElementException을 던지는 방식으로 non-nullablePayment반환 계약을 만족합니다.override fun getPayment(paymentId: Long): Payment { return paymentReader.findById(paymentId) ?: throw NoSuchElementException("Payment not found with id: $paymentId") }이는 Kotlin의 표준 관례이며, 예외 처리 전략이 적절하게 구현되어 있습니다.
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/GetPaymentResponse.kt (1)
21-36: 매핑 로직 LGTM도메인 → 응답 DTO 매핑이 정확합니다. 불필요한 필드(pgProvider, pgTransactionId)를 노출하지 않는 점도 적절합니다.
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/outbound/PaymentJpaEntity.kt (2)
21-24: Kotlin JPA 플러그인이 구성되어 있으므로 핵심 주장(영속화 실패)은 부정확합니다.검증 결과,
payment/payment-core/adapter/build.gradle.kts에kotlin("plugin.jpa")가 구성되어 있습니다. 이 플러그인은 자동으로 no-arg과 all-open 컴파일러 플러그인을 활성화하므로, JPA가val필드에서도 리플렉션을 통해 값을 주입/로딩할 수 있습니다. 따라서 "JPA가 식별자를 주입/로딩하기 어렵다"는 주장은 정확하지 않습니다.다만 진정한 문제점은 다릅니다:
createdAt,updatedAt타임스탬프 버그:OffsetDateTime.now()는 클래스 정의 시점에 평가되며, 각 인스턴스 생성 시점이 아닙니다. 이는val사용과 무관하게 데이터베이스 기본값 또는@PrePersist콜백으로 해결해야 합니다.
status,updatedAt등 변경 가능 필드: 이들은 엔티티 로드 후 업데이트되어야 하므로var로 선언하는 것이 Hibernate 변경 감지와 JPA 모범 사례에 부합합니다.Kotlin JPA 플러그인이 이미 구성되어 있으므로, 지금 즉시 모든 필드를
var로 변경할 필요는 없습니다. 대신 실제 업데이트가 필요한 필드(status,updatedAt등)와 타임스탬프 초기화 방식을 검토하세요.Likely an incorrect or invalid review comment.
52-59: 타임존 설정 확인됨검증 결과, 코드베이스에서 MySQL 타임존이 이미 올바르게 구성되어 있습니다. JDBC URL에
serverTimezone=Asia/Seoul이 설정되어 있으며, 이는 OffsetDateTime과 MySQL 간 직렬화 오차를 방지하는 현대적이고 권장되는 방식입니다. 추가 조치가 필요하지 않습니다.
| @GetMapping("/{id}") | ||
| fun getPayment(@PathVariable id: Long): GetPaymentResponse { | ||
| val payment = paymentUseCase.getPayment(id) | ||
| return GetPaymentResponse.from(payment) | ||
| } |
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.
🧩 Analysis chain
404 에러 처리를 확인하세요.
존재하지 않는 결제 ID에 대한 요청 시 적절한 HTTP 상태 코드(404 Not Found)가 반환되는지 확인이 필요합니다. paymentUseCase.getPayment(id)에서 예외가 발생할 때 이를 처리하는 글로벌 exception handler가 구현되어 있는지 검증하세요.
다음 스크립트로 exception handler 구현을 확인하세요:
🏁 Script executed:
#!/bin/bash
# 글로벌 exception handler 또는 ControllerAdvice 확인
rg -nP --type=kt -C3 '@(RestControllerAdvice|ControllerAdvice|ExceptionHandler)'Length of output: 26
🏁 Script executed:
#!/bin/bash
# 글로벌 exception handler 검색 - 코틀린 파일에서
rg -nP '(RestControllerAdvice|ControllerAdvice|ExceptionHandler)' --type-list | head -20Length of output: 389
🏁 Script executed:
#!/bin/bash
# 코틀린 파일 확장자로 exception handler 검색
rg -n '(RestControllerAdvice|ControllerAdvice|ExceptionHandler)' -g '*.kt'Length of output: 0
🏁 Script executed:
#!/bin/bash
# PaymentUseCase 인터페이스와 구현 검색
rg -n 'fun getPayment' -g '*.kt' -A 5 -B 2Length of output: 2406
🏁 Script executed:
#!/bin/bash
# Exception 클래스 정의 검색 (PaymentNotFound, NotFound 등)
rg -n '(Exception|class.*Exception|EntityNotFoundException)' -g '*.kt' -B 1 | head -50Length of output: 2968
글로벌 exception handler를 구현하여 404 응답을 처리하세요.
검증 결과, 존재하지 않는 결제 ID 요청 시 404 에러가 제대로 처리되지 않고 있습니다.
- 현재
paymentUseCase.getPayment(id)(PaymentService.kt:15-17)는 결제를 찾지 못할 때NoSuchElementException을 발생시킵니다. - 코드베이스에 @RestControllerAdvice, @ControllerAdvice, @ExceptionHandler가 구현되어 있지 않습니다.
- 컨트롤러에서 예외를 처리하지 않으면 500 에러가 반환됩니다.
@RestControllerAdvice를 통해 NoSuchElementException을 HTTP 404 상태 코드로 매핑하는 글로벌 exception handler를 구현하세요.
🤖 Prompt for AI Agents
In
payment/payment-core/adapter/src/main/kotlin/app/payment/core/adapter/inbound/PaymentController.kt
around lines 12-16: requests for non-existent payment currently cause a
NoSuchElementException in the service and result in a 500 because there is no
global exception handler; add a new Kotlin class (e.g.,
GlobalExceptionHandler.kt) in the same inbound package (or a common config
package) annotated with @RestControllerAdvice and implement an
@ExceptionHandler(NoSuchElementException::class) method that returns a
ResponseEntity with HTTP 404 (NOT_FOUND) and a minimal error body or message;
ensure the handler logs the exception and maps NoSuchElementException
consistently to 404 so the controller no longer returns 500 for missing
payments.
| interface PaymentUseCase { | ||
| fun getPayment(paymentId: Long): Payment | ||
| } |
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.
약간 네이밍의 문제같긴 한데, PaymentUseCase/PaymentService는 여기에 관련 기능을 몰아서 넣을지 궁금합니다. 만약 아니라면 Find 등의 행위가 나타나면 좋을것같아요/ 저번 Voucher에 제가 이부분에 대해 언급을 안했나 보네요..
Summary by CodeRabbit
릴리스 노트