[Volume-6] PG 결제 연동 및 장애 대응 설계#265
Conversation
결제 게이트웨이 시뮬레이터(pg-simulator) 모듈을 추가한다. - 결제 요청/단건 조회/주문별 조회 API (POST·GET /api/v1/payments) - 100~500ms 지연 및 40% 확률 실패로 PG 불안정성 시뮬레이션 - 이벤트 기반 비동기 처리(PENDING→SUCCESS)와 콜백 전송 - settings.gradle.kts 모듈 등록 및 http 요청 환경/예제 추가 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 Walkthrough변경 목적: 외부 PG 결제를 주문 생성과 분리해 비동기/복구 가능한 흐름으로 전환하고, 실패·타임아웃·콜백 유실을 안전하게 처리하려는 목적입니다. Walkthrough주문 생성은 PENDING 주문만 반환하도록 바뀌고, 결제는 commerce-api의 facade/service와 pg-simulator 모듈로 분리되었다. PG 요청, 콜백 반영, 복구 폴링, 예외 처리, DTO, 설정, 테스트, 문서가 함께 추가되었다. Changes주문-결제 비동기화와 PG 시뮬레이터 추가
Sequence Diagram(s)commerce-api 결제 요청과 콜백sequenceDiagram
participant PaymentV1Controller
participant PaymentFacade
participant PaymentService
participant PgPaymentGatewayAdapter
participant PaymentApi
PaymentV1Controller->>PaymentFacade: pay(loginId, PaymentCommand)
PaymentFacade->>PaymentService: createPending(...)
PaymentFacade->>PgPaymentGatewayAdapter: requestPayment(PgPaymentCommand)
PgPaymentGatewayAdapter->>PaymentApi: POST /api/v1/payments
PaymentV1Controller->>PaymentFacade: reflect(orderId, transactionKey, status, amount, reason)
PaymentFacade->>PaymentService: applyResult(...)
pg-simulator 이벤트 처리sequenceDiagram
participant PaymentApi
participant PaymentApplicationService
participant PaymentCoreEventPublisher
participant PaymentEventListener
participant PaymentCoreRelay
PaymentApi->>PaymentApplicationService: createTransaction(...)
PaymentApplicationService->>PaymentCoreEventPublisher: publish(PaymentCreated)
PaymentCoreEventPublisher->>PaymentEventListener: handle(PaymentCreated)
PaymentEventListener->>PaymentApplicationService: handle(transactionKey)
PaymentApplicationService->>PaymentCoreEventPublisher: publish(PaymentHandled)
PaymentCoreEventPublisher->>PaymentEventListener: handle(PaymentHandled)
PaymentEventListener->>PaymentApplicationService: notifyTransactionResult(transactionKey)
PaymentApplicationService->>PaymentCoreRelay: notify(callbackUrl, transactionInfo)
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (25)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java-33-42 (1)
33-42: 🔒 Security & Privacy | 🟠 Major
apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java:33-42의 콜백 원본 검증을 추가해야 한다
- 현재는
request를 그대로paymentFacade.reflect(...)로 전달하므로, callback URL만 알면 임의 POST가 결제 상태 변경 경로까지 도달할 수 있다.- PG가 합의한 HMAC 서명 헤더, 공유 시크릿, 또는 mTLS/IP allowlist 검증을 엔드포인트 진입부에 넣고, 실패 시
401/403으로 즉시 차단해야 한다.- 기존의 금액 불일치·중복 콜백 테스트 외에
서명 누락,위조 서명,변조된 body가reflect로 전달되지 않는 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java` around lines 33 - 42, The PaymentV1Controller.callback endpoint currently accepts any POST and forwards the body directly to paymentFacade.reflect, so add origin/authenticity validation at the entry point before any reflection happens. Implement the agreed callback verification in callback using the incoming request headers/body (for example HMAC signature, shared secret, or mTLS/IP allowlist) and reject invalid requests with 401/403 before calling paymentFacade.reflect. Also add tests around PaymentV1Controller and PaymentFacade invocation to ensure missing signature, forged signature, and tampered body cases do not reach reflect.apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java-33-37 (1)
33-37: 🩺 Stability & Availability | 🟠 Major
callback의orderId를 문자열로 받지 말고 바인딩 단계에서 400으로 처리해야 한다.
현재는Long.parseLong(request.orderId())가 컨트롤러 내부에서 터져ApiControllerAdvice의 일반 예외 처리로 흘러 500이 된다. PG 콜백은 재시도 성격이 강하므로, 잘못된orderId하나가 불필요한 재전송과 미처리 상태를 만들 수 있다.CallbackRequest.orderId를Long으로 바꾸거나, 최소한 이 파싱 실패를BAD_REQUEST로 변환하라. 숫자가 아닌orderId로 호출했을 때 400이 내려가고paymentFacade.reflect(...)가 호출되지 않는 테스트를 추가하라.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java` around lines 33 - 37, `PaymentV1Controller.callback` is parsing `request.orderId()` inside the controller, which lets invalid values escape as a server error instead of a bad request. Update `PaymentV1Dto.CallbackRequest` to bind `orderId` as a Long at the DTO/request binding level, or convert parsing failures in `callback` into a BAD_REQUEST response, and make sure `paymentFacade.reflect(...)` is only invoked after successful binding. Add a test around `callback` that sends a non-numeric `orderId`, verifies a 400 response, and confirms `paymentFacade.reflect(...)` is not called.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Dto.java-16-20 (1)
16-20: 🔒 Security & Privacy | 🟠 Major카드번호는 record component로 두지 않는 편이 안전하다.
PayRequest,CallbackRequest,PaymentCommand는 기본toString()에 전체 PAN이 포함되므로, 객체가 로그나 예외에 찍히면 운영 로그로 민감정보가 그대로 남는다. 카드번호는 마스킹된 전용 타입으로 분리하거나toString()을 직접 제어해야 한다. 추가로 이들 타입의toString()과 로그 포맷에 전체 카드번호가 포함되지 않는 테스트를 넣어야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Dto.java` around lines 16 - 20, The PayRequest record currently keeps the full card number as a component, so its default toString() can leak PAN into logs and exceptions. Update PayRequest, and the related CallbackRequest and PaymentCommand types, to avoid exposing the raw card number by either moving it to a masked value object or overriding toString() to redact it. Also add tests around these types’ toString() and any logging format to ensure the full card number never appears.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentModel.java-45-46 (1)
45-46: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift카드번호를 원문으로 영속화하면 안 된다.
운영 관점에서
payments테이블, 백업, 장애 분석 덤프만으로 PAN 이 바로 노출되어 사고 반경과 PCI 범위가 크게 커진다.cardNo는 저장하지 않거나 토큰/마스킹 값만 남기고, 정말 보관이 필요하면 별도 키 관리가 있는 필드 암호화로 분리하는 편이 안전하다. 수정 후에는 저장 직후 DB 에 평문 카드번호가 남지 않는지와 조회/로그에 마스킹 값만 노출되는지를 검증하는 테스트를 추가해야 한다.Also applies to: 54-59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentModel.java` around lines 45 - 46, The PaymentModel cardNo mapping is persisting raw PAN data, so update the payment persistence flow to stop storing the original card number directly in the payments entity and database. Adjust the PaymentModel field and the code that writes/reads it so only a token, masked value, or encrypted form is retained, and make sure any logs or queries expose only masked data. Add tests around the payment save path and retrieval path to verify no plaintext card number remains in the DB and that only masked values are observable.apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgPaymentGatewayAdapter.java-65-73 (1)
65-73: 🎯 Functional Correctness | 🟠 Major
ResourceAccessException을 전부TIMEOUT으로 보내지 마라다.
RestClient의ResourceAccessException은 read timeout뿐 아니라 connect timeout, DNS 실패, connection refused 같은 미전송 오류도 포함하므로, 이를PgRequestResult.timeout()으로 묶으면 실제로 PG에 요청이 가지 않은 경우까지 역조회 경로로 흘러가 재시도/거절 판단이 틀어진다다.- read timeout 계열만
TIMEOUT으로 분리하고, 나머지는NOT_ATTEMPTED로 보내며, 로그는t자체를 넘겨 원인 예외와 스택트레이스를 남겨라다.ResourceAccessException의 read timeout과 connect/DNS 실패를 각각 모킹해TIMEOUT/NOT_ATTEMPTED분기가 유지되는 테스트를 추가해라다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgPaymentGatewayAdapter.java` around lines 65 - 73, `PgPaymentGatewayAdapter.requestPaymentFallback` currently treats every `ResourceAccessException` as `PgRequestResult.timeout()`, but only read-timeout cases should map to `TIMEOUT`; adjust the fallback to distinguish read timeout from connect/DNS/refused failures and return `PgRequestResult.notAttempted()` for the non-read-timeout cases. Update the `log.warn` call to pass the throwable itself rather than `t.toString()` so the stack trace is preserved, and keep the generic fallback path for other exceptions as `NOT_ATTEMPTED`. Add tests around `requestPaymentFallback` covering both a read-timeout `ResourceAccessException` and a connect/DNS-style `ResourceAccessException` to verify the `TIMEOUT` vs `NOT_ATTEMPTED` split.apps/commerce-api/src/main/resources/application.yml-32-37 (1)
32-37: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win기본 설정에
localhostPG 엔드포인트를 두면 배포 환경에서 즉시 오동작한다.현재 값이 공통
application.yml에 들어가 있으면 override가 빠진 환경에서commerce-api가 자기 자신이나 로컬 호스트를 향해 결제를 시도하게 되어 장애가 난다. 이 값들은application-local.yml로 내리거나${PG_SIMULATOR_BASE_URL}/${PG_CALLBACK_URL}같은 환경변수 바인딩으로 분리하는 편이 안전하다. 추가로 로컬 프로파일과 비로컬 프로파일에서 각각 올바른 값이 바인딩되는 설정 테스트를 넣어야 한다. As per path instructions, "**/application*.yml: 환경별 분리(프로파일)와 기본값 적절성을 점검하고 ... 운영에 영향을 주는 설정 변경은 근거와 영향 범위를 요구한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/resources/application.yml` around lines 32 - 37, Move the pg-simulator defaults out of the shared application.yml in commerce-api so production does not fall back to localhost; bind base-url and callback-url from environment variables or profile-specific files such as application-local.yml, and ensure the default profile has safe non-local values. Update the configuration around pg-simulator to keep the existing timeout settings, and add configuration tests around the binding so local and non-local profiles resolve the correct URLs through the pg-simulator properties.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentService.java-51-67 (1)
51-67: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win기존
transactionKey와 다른 PG 결과를 거부하지 않아 잘못된 결제 결과가 반영될 수 있다.이미
transactionKey가 저장된 뒤에는 다른 non-null 키가 들어오면 상태 전이를 막아야 한다. 지금처럼 무시한 채SUCCESS/FAILED를 반영하면 잘못된 콜백이나 역조회 응답이 같은 주문의 결제 상태를 덮어써 데이터 정합성이 깨진다.payment.getTransactionKey()가 존재할 때는 입력 키와 일치하는지 먼저 검증하고, 불일치하면 예외로 차단해야 한다. 추가로 "첫 콜백은 key reconcile 허용", "이후 다른 key로 들어온 결과는 거부되고 상태가 유지된다"를 검증하는 테스트가 필요하다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentService.java` around lines 51 - 67, `PaymentService.applyResult` currently accepts any non-null `transactionKey` after one has already been stored, which can let a mismatched PG callback overwrite the payment state. Update the `applyResult` flow so that when `payment.getTransactionKey()` is already present, the incoming `transactionKey` must match it before any status transition; otherwise throw a `CoreException` and stop processing. Keep the existing first-time reconcile behavior in `markRequested`, and add tests around `applyResult` to cover both the initial key assignment and rejection of later results with a different key.apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentRecoveryScheduler.java-42-55 (1)
42-55: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win예외 경로에서 복구 시도 횟수를 올리지 않아 동일 건을 무한 재시도하게 된다.
지금 구현은 PG 조회가 계속 예외를 던지면
recordRecoveryAttempt가 호출되지 않아 해당 결제가 영구히 recoverable 상태로 남는다. 그러면 10초마다 같은 건을 계속 두드리며 로그만 누적되고, PR에서 정의한 "결제당 최대 5회 복구 시도" 보장도 깨진다. 종결 상태를 반영하지 못한 모든 경로에서 시도 횟수가 증가하도록catch또는finally에서 누적하고, 예외 로그는e.toString()만 남기지 말고 stack trace까지 함께 남기는 편이 운영에 유리하다. 추가로 "Gateway 예외가 반복되면 recoveryAttempts가 증가하고 5회 후 조회 대상에서 빠진다"를 검증하는 테스트가 필요하다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentRecoveryScheduler.java` around lines 42 - 55, In PaymentRecoveryScheduler's recovery loop, the RuntimeException path does not increment recovery attempts, so PG failures can leave a payment in recoverable state forever; update the try/catch flow around findByOrderId, reflect, and recordRecoveryAttempt so every non-terminal outcome (including exceptions) still calls the attempt tracker, ideally via finally or by duplicating the increment in the catch path. Also change the warn log to include the exception stack trace instead of only e.toString(), and add a test covering repeated gateway exceptions eventually pushing the payment out of recoverable after the max attempts.apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentService.java-27-33 (1)
27-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win모든 DB 무결성 오류를 중복 결제로 번역하면 장애 원인 분리가 깨진다.
운영에서는
order_id중복 외에도 not-null, FK, 길이 제약 위반이 모두 여기로 들어올 수 있는데, 현재 구현은 전부 409로 바꿔 실제 장애를 잘못 분류하게 만든다.order_id고유 제약 위반만 명시적으로 409로 매핑하고, 나머지DataIntegrityViolationException은 cause를 보존한 채 상위로 전파하거나 별도 5xx로 분리하는 편이 안전하다. 추가로 "중복 orderId는 409", "다른 제약 위반은 원인 예외가 유지된다"를 검증하는 테스트가 필요하다. As per path instructions, "**/*.java: 예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentService.java` around lines 27 - 33, The catch block in PaymentService.createPending currently maps every DataIntegrityViolationException to the same duplicate-payment CoreException, which hides non-orderId integrity failures. Update the handling so only the order_id unique constraint violation becomes the 409 conflict, while other DataIntegrityViolationException cases preserve the original cause and are either rethrown or mapped to a separate 5xx path; use the PaymentService.createPending and paymentRepository.save flow to locate the change. Add tests to verify that duplicate orderId returns 409 and that other constraint violations keep the cause intact.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PaymentJpaRepository.java-14-14 (1)
14-14: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift복구 대상 조회를 배치 단위로 제한해야 한다.
Line 14는 조건만 있고 정렬과 상한이 없어, 적체가 생기면 스케줄러 한 번에 모든
PENDING건을 읽어 DB 스캔과 애플리케이션 메모리 사용량이 함께 커진다. 운영에서는 폴링 주기마다 같은 부하가 반복되어 복구 경로 자체가 병목이 되기 쉽다.Pageable을 받거나findTopNBy...OrderByIdAsc/OrderByCreatedAtAsc형태로 배치 크기와 순서를 고정하는 쪽이 안전하다. 추가 테스트로 recoverable 데이터가 배치 크기를 초과할 때 오래된 건부터 안정적으로 잘리는 Repository 통합 테스트를 넣는 것이 좋다. As per path instructions,**/*Repository*.java: 대량 데이터에서의 병목과 정렬/인덱스 활용 가능성을 점검해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PaymentJpaRepository.java` at line 14, The repository query in PaymentJpaRepository currently fetches all recoverable rows without any ordering or batch cap, which can cause large scans and memory pressure. Update the finder to limit results per poll by either accepting a Pageable or switching to a top-N method, and add a stable sort key such as id or createdAt in the method name/signature. Keep the existing filter conditions in findByStatusAndPgRequestAttemptedTrueAndRecoveryAttemptsLessThanAndDeletedAtIsNull, and add repository integration coverage that verifies excess recoverable rows are returned in a bounded, deterministic order.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgProperties.java-7-13 (1)
7-13: 🩺 Stability & Availability | 🟠 MajorPG 설정값에 바인딩 검증을 추가하라.
PgClientConfig.pgRestClient()와PgPaymentGatewayAdapter가baseUrl,callbackUrl,connectTimeout,readTimeout를 즉시 사용하므로, 누락값이나 빈 문자열, 비정상 duration이 들어가면 문제를 배포 시점이 아니라 결제 흐름 중간에 드러내게 된다.PgProperties에@Validated를 붙이고@NotBlank,@NotNull, 필요하면 duration 하한 제약으로 기동 단계에서 실패시키라.pg-simulator.*에 누락값, 빈 문자열, 0 이하 duration을 넣었을 때 컨텍스트 초기화가 실패하는 테스트도 추가하라.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgProperties.java` around lines 7 - 13, Add startup-time binding validation to PgProperties so pg-simulator settings fail fast instead of surfacing later in PgClientConfig.pgRestClient() or PgPaymentGatewayAdapter. Update the PgProperties record to use validation annotations such as `@Validated`, `@NotBlank` for baseUrl and callbackUrl, `@NotNull` for connectTimeout and readTimeout, and any needed duration minimum constraints. Also add tests that load the application context with missing pg-simulator values, blank strings, and non-positive durations to verify context initialization fails.apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentDto.kt-24-37 (1)
24-37: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
startsWith기반 콜백 URL 검증은 우회 가능하다.
http://localhost:8080.evil.com/...나http://localhost:8080@evil.com/...같은 값도 현재 검증을 통과한다. 이후 콜백 릴레이가 이 값을 그대로 사용하면 시뮬레이터가 공격자 호스트로 outbound 호출을 보내게 된다. 문자열 prefix 비교 대신URI로 파싱해서 scheme/host/port를 정확히 비교하고, 필요하면 허용 path까지 제한해야 한다. 위 우회 예시와 인코딩된 변형 입력을 거부하는 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentDto.kt` around lines 24 - 37, The callback URL validation in PaymentDto.validate is unsafe because PREFIX_CALLBACK_URL prefix matching can be bypassed. Replace the startsWith check with URI parsing and compare the scheme, host, and port explicitly (and restrict the path if required) so values like localhost spoofing variants are rejected. Update the validate method accordingly and add tests around PaymentDto for bypass cases such as embedded userinfo, deceptive subdomains, and encoded variants that should fail validation.apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentApi.kt-30-31 (1)
30-31: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift요청 스레드에서
Thread.sleep으로 지연을 주면 시뮬레이터가 먼저 포화된다.Tomcat 워커를 100~500ms씩 점유하면 동시 요청 시 결제 실패/복구 설계보다 스레드 고갈이 먼저 드러나서 E2E 결과가 왜곡된다. 지연이 필요하다면 비동기 응답(
Callable/DeferredResult)으로 넘기거나, 승인 처리 이벤트 쪽에서 스케줄링으로 지연을 주는 편이 안전하다. 동시 요청 테스트를 추가해서 워커 고갈 없이 응답 지연만 시뮬레이션되는지 확인해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentApi.kt` around lines 30 - 31, The issue is that PaymentApi currently blocks the request thread with Thread.sleep, which can exhaust Tomcat workers under concurrent load. Replace the blocking sleep in PaymentApi with an async approach such as Callable or DeferredResult, or move the delay into the approval-processing/event scheduling path so the request thread is released immediately. Keep the change localized around the payment endpoint handling in PaymentApi, and add a concurrent request test to verify delayed responses do not cause worker starvation.apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentDto.kt-50-69 (1)
50-69: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift거래 상세 응답에 전체 카드번호를 그대로 노출하면 안 된다.
현재
TransactionDetailResponse가 원본cardNo를 그대로 반환하고,Payment엔티티도 전체 카드번호를 보관한다. 로컬 시뮬레이터라도 응답·저장소·로그 경로로 민감정보가 확산되어 유출 범위가 커진다. 저장은 토큰 또는 마스킹/last4 수준으로 축소하고, 응답도****-****-****-1234같은 마스킹 값만 내려주도록 계약을 바꾸는 편이 안전하다. 전체 카드번호가 API 응답과 영속 저장에 남지 않는지, commerce-api 연동 DTO가 변경된 형식과 호환되는지 계약 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentDto.kt` around lines 50 - 69, 거래 상세 응답에서 전체 카드번호를 그대로 노출·저장하는 문제가 있습니다. TransactionDto/TransactionDetailResponse의 from 매핑과 Payment 엔티티 저장 경로에서 cardNo를 원문 대신 마스킹 또는 last4/토큰 형태로 바꾸고, 응답도 동일한 마스킹 계약만 반환하도록 수정하세요. PaymentDto의 TransactionDetailResponse와 관련 저장/조회 로직을 함께 점검해 민감정보가 남지 않게 하며, commerce-api 연동 DTO가 새 형식과 맞는지 확인하는 계약 테스트를 추가하세요.apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentCommand.kt-16-19 (1)
16-19: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win검증 규칙을
PaymentRequest와CreateTransaction에 분산시키지 않는 편이 안전하다.현재
PaymentDto.PaymentRequest.validate()는orderId,cardNo,callbackUrl,amount를 검증하지만, 실제 저장 직전PaymentApplicationService.createTransaction()이 신뢰하는CreateTransaction.validate()는 금액만 막고 있다. 운영에서 HTTP 외 경로가CreateTransaction을 직접 만들면 잘못된 카드번호나 callback URL이 저장되어 비동기 콜백/릴레이 단계에서 늦게 실패할 수 있다.CreateTransaction.validate()로 규칙을 모으고PaymentRequest.validate()는 이를 재사용하도록 정리하는 편이 좋다. 추가 테스트로PaymentApplicationService.createTransaction()에 직접 만든 command가 빈orderId, 잘못된cardNo, 허용되지 않은callbackUrl일 때 모두BAD_REQUEST로 막히는지 확인해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentCommand.kt` around lines 16 - 19, `CreateTransaction.validate()` is missing the full command validation currently duplicated in `PaymentDto.PaymentRequest.validate()`, so move the shared rules there and make `PaymentRequest.validate()` delegate to `CreateTransaction.validate()` rather than maintaining its own checks. Update `PaymentApplicationService.createTransaction()` to rely on the command-level validation, and verify `orderId`, `cardNo`, `callbackUrl`, and `amount` are all rejected with `BAD_REQUEST` when an invalid `CreateTransaction` is passed directly.apps/pg-simulator/src/main/kotlin/com/loopers/PaymentGatewayApplication.kt-15-19 (1)
15-19: 🎯 Functional Correctness | 🟠 Major타임존 설정은
@PostConstruct가 아니라 초기 부팅 단계에서 고정해야 한다.
@PostConstruct시점에는 일부 날짜 처리 빈이 이미 기본 타임존을 읽었을 수 있어, 운영에서 JSON 직렬화 시각과LocalDateTime.now()기준이 엇갈릴 수 있다. 앱 전역 KST가 필요하면main시작 직후에 한 번만 설정하고, Jackson은spring.jackson.time-zone=Asia/Seoul또는Jackson2ObjectMapperBuilderCustomizer로 따로 고정하는 편이 안전하다. 추가로 컨텍스트 기동 직후TimeZone.getDefault()와 날짜 직렬화 결과가 모두 기대값인지 확인하는 부트 테스트를 넣어야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/PaymentGatewayApplication.kt` around lines 15 - 19, The timezone is being set too late in PaymentGatewayApplication.started(), so move the global KST defaulting out of the `@PostConstruct` hook and into the earliest startup path in main before the Spring context is built. Also pin Jackson separately using spring.jackson.time-zone=Asia/Seoul or a Jackson2ObjectMapperBuilderCustomizer, and add a boot test that verifies TimeZone.getDefault() and date serialization both reflect the expected timezone right after startup.apps/pg-simulator/src/main/kotlin/com/loopers/support/error/ErrorType.kt-5-10 (1)
5-10: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
errorCode에 HTTP reason phrase를 쓰지 않는 편이 안전하다.Line 7-10의
code가"Bad Request"같은 상태 문구로 내려가면 클라이언트가 안정적으로 분기하기 어렵고, 문구 변경이 곧 API 계약 변경이 된다.BAD_REQUEST,NOT_FOUND처럼 고정된 애플리케이션 코드로 분리하고, 사용자 노출 문구는message에만 두는 편이 안전하다. 추가로 400/404 응답 직렬화 테스트에서meta.errorCode가 상태 문구가 아니라 고정 코드로 내려가는지 검증해 두는 편이 좋다.수정 예시
enum class ErrorType(val status: HttpStatus, val code: String, val message: String) { /** 범용 에러 */ - INTERNAL_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, HttpStatus.INTERNAL_SERVER_ERROR.reasonPhrase, "일시적인 오류가 발생했습니다."), - BAD_REQUEST(HttpStatus.BAD_REQUEST, HttpStatus.BAD_REQUEST.reasonPhrase, "잘못된 요청입니다."), - NOT_FOUND(HttpStatus.NOT_FOUND, HttpStatus.NOT_FOUND.reasonPhrase, "존재하지 않는 요청입니다."), - CONFLICT(HttpStatus.CONFLICT, HttpStatus.CONFLICT.reasonPhrase, "이미 존재하는 리소스입니다."), + INTERNAL_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "INTERNAL_ERROR", "일시적인 오류가 발생했습니다."), + BAD_REQUEST(HttpStatus.BAD_REQUEST, "BAD_REQUEST", "잘못된 요청입니다."), + NOT_FOUND(HttpStatus.NOT_FOUND, "NOT_FOUND", "존재하지 않는 요청입니다."), + CONFLICT(HttpStatus.CONFLICT, "CONFLICT", "이미 존재하는 리소스입니다."), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/support/error/ErrorType.kt` around lines 5 - 10, `ErrorType`의 `code`가 `HttpStatus.reasonPhrase`를 사용해 상태 문구에 묶여 있으니, `INTERNAL_ERROR`, `BAD_REQUEST`, `NOT_FOUND`, `CONFLICT` 같은 enum 상수의 `code`를 고정된 애플리케이션 코드로 분리하세요. `ErrorType`와 이를 응답으로 직렬화하는 `meta.errorCode` 생성 로직을 함께 수정해 `message`는 사용자 문구만 담당하게 하고, 400/404 응답 테스트에서 `errorCode`가 reason phrase가 아닌 고정 코드로 내려오는지 검증을 추가하세요.apps/pg-simulator/src/main/resources/application.yml-18-19 (1)
18-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win로컬 프로파일을 기본 활성화하지 마십시오.
Line 19의
spring.profiles.active: local은 런타임 오버라이드가 빠진 배포에서 로컬 프로파일과 로컬 DB 설정으로 기동하게 만들어 장애로 이어질 수 있다. 기본 문서에서는 이 값을 제거하고, 실행 환경에서만SPRING_PROFILES_ACTIVE또는 배포 설정으로 주입하는 편이 안전하다. 추가로 프로파일 미지정 상태에서local문서가 자동 활성화되지 않는 스모크 테스트와dev/prd기동 확인 테스트를 넣는 편이 좋다.수정 예시
spring: main: web-application-type: servlet application: name: pg-simulator - profiles: - active: local config: import: - jpa.ymlAs per path instructions,
**/application*.yml: 환경별 분리(프로파일)와 기본값 적절성을 점검한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/resources/application.yml` around lines 18 - 19, Remove the default `spring.profiles.active: local` from the `application.yml` configuration so the app does not boot with local settings when runtime overrides are missing. Keep profile selection externalized via `SPRING_PROFILES_ACTIVE` or deployment config, and ensure the startup/config path in `application.yml` does not implicitly enable `local`. Add or update checks around the app startup behavior to verify that no profile defaults to `local` and that `dev`/`prd` boots work only when explicitly selected.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/argumentresolver/UserInfoArgumentResolver.kt-27-30 (1)
27-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win빈
X-USER-ID도 거부해야 한다.Line 27-30은 헤더가 없는 경우만 막고 공백 문자열은 통과시킨다. 그러면 잘못된 사용자 컨텍스트가 정상 요청처럼 흘러가 조회 격리나 결제 추적에서 운영 장애를 만들 수 있다.
isNullOrBlank()로 검사하고, 통과시키더라도trim()한 값만UserInfo로 생성하는 편이 안전하다. 추가로 missing, empty, whitespace-only 헤더에 대해 모두 400이 내려가는 MVC 테스트를 넣는 편이 좋다.수정 예시
override fun resolveArgument( parameter: MethodParameter, mavContainer: ModelAndViewContainer?, webRequest: NativeWebRequest, binderFactory: WebDataBinderFactory?, ): UserInfo { - val userId = webRequest.getHeader(KEY_USER_ID) - ?: throw CoreException(ErrorType.BAD_REQUEST, "유저 ID 헤더는 필수입니다.") - - return UserInfo(userId) + val userId = webRequest.getHeader(KEY_USER_ID) + if (userId.isNullOrBlank()) { + throw CoreException(ErrorType.BAD_REQUEST, "유저 ID 헤더는 필수입니다.") + } + + return UserInfo(userId.trim()) }As per path instructions,
**/*.kt: null-safety를 최우선으로 점검한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/argumentresolver/UserInfoArgumentResolver.kt` around lines 27 - 30, The UserInfoArgumentResolver currently only rejects a missing X-USER-ID header, so empty or whitespace-only values can still pass through. Update the getHeader(KEY_USER_ID) handling in UserInfoArgumentResolver to validate with isNullOrBlank(), and when accepted, pass only the trimmed value into UserInfo. Also add MVC tests covering missing, empty, and whitespace-only X-USER-ID headers to verify they all return 400.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/TransactionKeyGenerator.kt-12-18 (1)
12-18: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win현재 transactionKey 생성 방식은 충돌 여지가 크다.
날짜 prefix 뒤에 6자리 hex 문자열만 붙이면 하루 기준 키 공간이
16^6으로 제한된다. 동시 요청이나 테스트 반복이 늘면 충돌로 다른 결제건 조회·콜백 반영이 섞이거나 저장 실패가 발생할 수 있어 운영 안정성이 떨어진다. 전체 UUID/ULID를 사용하거나 충분한 길이의 난수와 시각 정보를 포함하고, 가능하면 저장소 unique 제약과 재생성 전략도 함께 두는 편이 안전하다. 추가로 대량 생성 테스트에서 중복이 발생하지 않는지와 포맷이 유지되는지를 검증해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/TransactionKeyGenerator.kt` around lines 12 - 18, 현재 TransactionKeyGenerator.generate()는 날짜 prefix 뒤에 UUID의 6자리만 붙여 충돌 가능성이 큽니다. generate()에서 사용하는 uuid 생성 방식을 전체 UUID/ULID 또는 더 긴 난수+시각 정보 조합으로 바꾸고, KEY_TRANSACTION 포맷은 유지하되 충분히 유일한 키가 되도록 수정하세요. 필요하면 저장소의 unique 제약과 재시도 로직도 함께 고려하고, 대량 생성 시 중복이 없는지와 문자열 포맷이 유지되는지 검증하는 테스트를 추가하세요.apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/TransactionInfo.kt-18-37 (1)
18-37: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win콜백 payload에 전체 카드번호를 싣지 마시기 바란다.
PaymentApplicationService.notifyTransactionResult에서 이 DTO가 그대로 외부 callback payload로 전송된다. 전체 카드번호를 넘기면 downstream 로그·추적 계층까지 민감정보 범위가 넓어져 운영 사고 시 영향이 커진다.cardNo는 제거하거나 마지막 4자리만 남긴 마스킹 필드로 바꾸는 편이 안전하다. 추가로 직렬화 테스트에서 callback payload에 원본 카드번호가 포함되지 않는지 검증해야 한다. As per path instructions, "로깅 시 민감정보 노출 가능성을 점검한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/TransactionInfo.kt` around lines 18 - 37, `TransactionInfo`는 `PaymentApplicationService.notifyTransactionResult`를 통해 외부 callback payload로 그대로 나가므로 전체 카드번호를 담지 않도록 수정해야 합니다. `TransactionInfo.from`과 `TransactionInfo` 필드에서 `cardNo`를 제거하거나 마지막 4자리만 남기는 마스킹 값으로 바꾸고, `notifyTransactionResult`가 사용하는 payload 직렬화 경로도 함께 확인하세요. 또한 callback payload 직렬화 테스트를 추가해 원본 카드번호가 포함되지 않는지 `TransactionInfo` 기준으로 검증하세요.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentRelay.kt-3-6 (1)
3-6: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift도메인 포트가 application DTO를 직접 의존하지 않도록 분리하시기 바란다.
PaymentRelay가TransactionInfo를 받으면서 도메인 계층이application.payment에 역의존하고 있다. 이 구조는 callback payload 형식이 바뀔 때마다 도메인 계약도 함께 흔들려 운영 중 변경 범위를 불필요하게 키운다. 도메인 포트에는 도메인 이벤트나 도메인 전용 결과 모델만 두고, 외부 전송용 DTO 매핑은 application/infrastructure 어댑터에서 수행하는 편이 안전하다. 추가로 패키지 의존 규칙 테스트나 어댑터 단위 테스트로 도메인 패키지가 application 패키지를 참조하지 않는지 검증해야 한다. As per path instructions, "비즈니스 규칙은 도메인에 두고, 인프라 관심사가 섞이면 분리하도록 제안한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentRelay.kt` around lines 3 - 6, PaymentRelay currently depends directly on the application DTO TransactionInfo, so the domain port should be refactored to use a domain-only contract instead. Update the notify signature in PaymentRelay to accept a domain event/result model defined in the domain layer, then move the TransactionInfo-to-domain mapping into the application or infrastructure adapter that calls this port. Also add/adjust a dependency rule or adapter test to ensure the domain package no longer references com.loopers.application.payment.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentApplicationService.kt-32-41 (1)
32-41: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift원본 카드번호를 그대로 저장·전파하지 않는 편이 안전하다.
Line 38에서 원본
cardNo를 그대로 영속화하면 DB 덤프, 장애 대응 로그, 직렬화된 callback payload를 통해 PAN 노출면이 커진다. 제공된TransactionInfo.from(payment)도 이 값을 그대로 실어 보내고, 콜백 소비자apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.java:33-43는orderId,transactionKey,status,amount,reason만 사용하므로 운영상 보관 이점도 거의 없다. 마지막 4자리만 남긴 마스킹 값이나 토큰만 저장하고, callback 전용 DTO에서는 카드 정보를 제외해주기 바란다. 추가 테스트로 저장된 값과 callback JSON 모두에 전체 카드번호가 포함되지 않는지 검증이 필요하다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentApplicationService.kt` around lines 32 - 41, The `PaymentApplicationService` flow is persisting and propagating the full `cardNo`, which should be avoided; update the `save` path so `Payment` does not store the raw PAN, and ensure `TransactionInfo.from(payment)` no longer includes card data in the callback payload. Keep only a masked value or token in the domain model if needed, and verify via tests that both the persisted record and the serialized callback JSON do not contain the full card number.apps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentCoreRelay.kt-11-19 (1)
11-19: 🩺 Stability & Availability | 🟠 Major콜백용
RestTemplate에 connect/read timeout을 명시해야 한다다
RestTemplate()기본 설정은 타임아웃이 없어 느리거나 응답 없는 callback URL이 호출 스레드를 장시간 점유할 수 있다다.RestTemplateBuilder또는 주입된 Bean으로 timeout을 설정하고, 지연 응답과 연결 실패를 구분해 기록하도록 바꾸는 것이 맞다다. 추가로 지연 응답 endpoint에 대해 제한 시간 내 실패하고 다음 콜백이 계속 처리되는 테스트를 넣어야 한다다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentCoreRelay.kt` around lines 11 - 19, The callback RestTemplate in PaymentCoreRelay currently uses default settings with no connect/read timeout, so update the notify flow to use a timeout-configured RestTemplate (preferably via RestTemplateBuilder or an injected bean) instead of the static RestTemplate instance. Keep the change localized to PaymentCoreRelay and its companion object/notify method, and adjust error logging so connection failures and slow response timeouts are distinguishable in the logger.error path.apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/event/payment/PaymentEventListener.kt-16-20 (1)
16-20: 🩺 Stability & Availability | 🟠 Major인터럽트 예외를 처리해 결제 이벤트 유실을 막아야 한다.
apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/event/payment/PaymentEventListener.kt:16-20
Thread.sleep가 인터럽트되면paymentApplicationService.handle(event.transactionKey)까지 도달하지 못해 결제 상태가PENDING에 남을 수 있다.InterruptedException을 잡아Thread.currentThread().interrupt()를 복원하고, 경고 로그와 재처리 가능한 경로를 추가해야 한다. 인터럽트 발생 시 로그가 남고 보상 또는 재시도 흐름으로 이어지는 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/event/payment/PaymentEventListener.kt` around lines 16 - 20, In PaymentEventListener.handle, Thread.sleep can interrupt before paymentApplicationService.handle(event.transactionKey) runs, so add InterruptedException handling around the sleep in the handle method. Catch the interruption, restore the interrupt status with Thread.currentThread().interrupt(), log a warning with enough context, and route the event into a retryable or compensating path so the payment is not left in PENDING. Also add a test covering an interrupted handle path to verify the log/flow still leads to reprocessing or compensation.
🟡 Minor comments (2)
apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/Payment.kt-56-85 (1)
56-85: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win상태 전이 후에도
updatedAt이 갱신되지 않는다.지금 구현이면 승인/실패로 상태가 바뀌어도
updatedAt은 생성 시각에 고정된다. 비동기 결제 흐름에서 마지막 변경 시각이 틀리면 장애 추적과 후속 복구 판단이 왜곡된다. 상태 변경 메서드의 공통 경로에서updatedAt = LocalDateTime.now()를 함께 갱신하거나, 엔티티 라이프사이클 콜백으로 일관되게 처리하는 편이 안전하다. 상태 전이 후updatedAt이 증가하는지 확인하는 단위 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/Payment.kt` around lines 56 - 85, Payment 엔티티의 상태 변경 메서드인 approve, invalidCard, limitExceeded에서 status와 reason만 바꾸고 updatedAt을 갱신하지 않는 문제가 있습니다. 상태 전이 공통 경로에서 updatedAt을 현재 시각으로 함께 업데이트하도록 처리하거나, Payment 엔티티의 라이프사이클 콜백으로 일관되게 반영되게 수정하세요. 또한 상태 변경 후 updatedAt이 실제로 증가하는지 검증하는 테스트를 추가해 주세요.http/pg-simulator/payments.http-6-11 (1)
6-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win예제 요청 본문에서
amount타입을 DTO 계약과 맞추는 편이 낫다.
PaymentDto.PaymentRequest.amount는Long인데 예제는 문자열을 보내고 있다. 지금은 coercion 설정에 따라 우연히 통과할 수 있어도, 설정이 엄격해지면 문서 예제를 그대로 호출한 사용자가 바로 400을 받게 된다. 예제는"5000"대신5000으로 맞추고, 추가 테스트로 문서 스모크 테스트나 E2E 요청 예제가 실제 DTO 타입과 일치하는지 확인하는 편이 좋다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@http/pg-simulator/payments.http` around lines 6 - 11, The example request body uses a string for amount, but PaymentDto.PaymentRequest.amount expects a Long, so update the HTTP example in payments.http to send amount as a numeric value and not a quoted string. Make sure the sample payload matches the DTO contract used by PaymentDto.PaymentRequest, and if there are request examples or smoke/E2E checks tied to this payload, align them with the same amount type.
🧹 Nitpick comments (6)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Dto.java (1)
16-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win요청 검증을 DTO 내부 수동 예외 대신 Bean Validation으로 올려야 한다.
현재
toCommand()가 검증과 매핑을 동시에 수행해서 바인딩 오류, 필드 누락, 형식 오류의 400 계약이 컨트롤러별로 흩어진다. 운영에서는 같은 잘못된 요청도 예외 종류에 따라 응답 포맷과 로깅 지점이 달라져 장애 분석이 어려워진다.PayRequest컴포넌트에@NotNull,@Pattern을 선언하고 컨트롤러에@Valid를 붙여toCommand()는 순수 매핑만 담당하게 바꾸는 편이 안전하다. 추가로 주문 ID 누락, 카드 종류 누락, 카드번호 형식 오류가 모두 동일한 400 응답 스키마로 내려가는 API 테스트를 보강해야 한다. As per path instructions,**/*Controller*.java: Controller는 요청 검증(Bean Validation)과 응답 조립에 집중하고 상태 코드와 에러 응답 포맷이 일관되는지 점검한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Dto.java` around lines 16 - 32, `PayRequest` in `PaymentV1Dto` is doing both validation and mapping inside `toCommand()`, so move the request checks to Bean Validation annotations on the record fields and keep `toCommand()` as pure conversion only. Add `@Valid` to the payment controller request binding so `orderId`, `cardType`, and `cardNo` failures are handled consistently before mapping, and remove the manual `CoreException` throws from `toCommand()`. Update the controller/tests to verify missing order ID, missing card type, and invalid card number all return the same 400 response shape.Source: Path instructions
apps/commerce-api/src/test/java/com/loopers/interfaces/api/PaymentV1ApiE2ETest.java (1)
157-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
NOT_ATTEMPTED경로 테스트도 고정해야 한다.운영 관점에서
TIMEOUT과NOT_ATTEMPTED는 재시도 정책이 다르므로 한쪽만 검증하면 리팩터링 시 안전 재시도 규칙이 쉽게 무너진다.PgRequestResult.notAttempted()시나리오를 추가해transactionKey == null,pgRequestAttempted == false, 그리고 같은 주문에 대한 후속 요청이 현재 계약대로 동작하는지까지 검증하는 편이 좋다. 수정 후에는 두 번째 결제 요청의 상태 코드와 저장된 결제 상태를 함께 확인하는 테스트를 추가해야 한다. As per path instructions, "**/*Test*.java: Java 테스트 리뷰 기준이다. 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/PaymentV1ApiE2ETest.java` around lines 157 - 173, The payment E2E test currently covers only the TIMEOUT path, but the NOT_ATTEMPTED contract also needs to be locked down. Extend the `PaymentV1ApiE2ETest.timeout_keepsPendingWithoutKey` coverage by adding a `PgRequestResult.notAttempted()` scenario that asserts `PaymentModel.getTransactionKey()` stays null, `PaymentModel.isPgRequestAttempted()` remains false, and the follow-up request for the same order behaves as expected. Use the existing `requestPayment`, `paymentRepository.findByOrderId`, and `paymentGateway.requestPayment` flow to verify both the response status and persisted payment state.Source: Path instructions
apps/commerce-api/src/test/java/com/loopers/domain/payment/PaymentServiceUnitTest.java (1)
50-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win실패 콜백 반영 경로도 테스트해야 한다.
현재
applyResult테스트는 SUCCESS 중심이고, 이번 PR의 핵심인 실패 결과 반영(FAILED+reason) 경로는 비어 있다. 운영에서는 이 경로가 깨져도 결제가 계속PENDING으로 남아 복구 스케줄러가 불필요하게 재시도할 수 있다.PaymentStatus.FAILED가 들어왔을 때 상태와 사유가 정확히 저장되는 케이스를 추가하는 편이 안전하다. 추가 테스트로 실패 반영 후 뒤늦은 SUCCESS가 와도 상태가 뒤집히지 않는 시나리오까지 함께 검증하는 것이 좋다. As per path instructions,**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/test/java/com/loopers/domain/payment/PaymentServiceUnitTest.java` around lines 50 - 100, `PaymentServiceUnitTest.ApplyResult`에서 실패 콜백 반영 경로가 빠져 있으므로, `paymentService.applyResult`에 `PaymentStatus.FAILED`와 `reason`이 들어왔을 때 `PaymentModel`의 상태와 사유가 정확히 저장되는 테스트를 추가하세요. `rejectsAmountMismatch`, `appliesSuccess`, `reconcilesMissingTransactionKey`, `ignoresWhenAlreadyFinal`와 같은 기존 패턴을 따라 `FAILED` 처리 후 `getStatus()`와 실패 사유가 기대값인지 검증하고, 이어서 뒤늦은 `SUCCESS`가 와도 상태가 `FAILED`에서 뒤집히지 않는 멱등 시나리오도 함께 추가하세요.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/support/error/CoreException.kt (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
CoreException가 원인 예외와 내부 문맥을 보존하지 못한다.현재 구조에서는
customMessage가 곧RuntimeException.message가 되고,ApiControllerAdvice가 그 값을 응답과 로그에 함께 사용한다. 운영 중 인프라 예외를 감싸는 순간 근본 원인과 내부 진단 문맥이 사라져 장애 분석이 어려워진다.userMessage,logMessage,cause를 분리한 생성자로 바꾸고, 응답에는 사용자 메시지만, 로그에는 내부 메시지와cause를 사용하도록 분리하는 편이 안전하다.CoreException래핑 시cause가 유지되는지와, 핸들러가 사용자 메시지만 응답에 노출하는지 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/support/error/CoreException.kt` around lines 3 - 6, CoreException currently collapses user-facing text, internal diagnostics, and the original cause into a single RuntimeException message, so update the CoreException constructor to accept separate userMessage, logMessage, and cause values and preserve the cause when wrapping errors. Then adjust ApiControllerAdvice to return only the userMessage in the API response while logging the logMessage plus the underlying cause for diagnostics, using the CoreException and ApiControllerAdvice symbols to locate the changes. Add tests to verify wrapped exceptions keep their cause and that the handler exposes only the user-facing message externally.apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/ApiControllerAdvice.kt (1)
26-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
CoreException경로에서는 사용자 메시지와 운영 로그를 분리하는 편이 낫다.지금처럼 4xx성
CoreException마다 사용자 응답 문구와 스택트레이스를 함께 WARN으로 남기면 운영 로그가 과도하게 쌓이고, 요청값이 포함된 메시지가 있으면 그대로 로그에 노출될 수 있다. 예상 가능한 비즈니스 예외는errorType.code와 식별자 정도만 구조화해서 남기고 스택트레이스는 제외하며, 원인 예외가 있는 5xx만 cause를 보존해 별도로 로깅하는 방식으로 분리하는 편이 안전하다. 추가 테스트로CoreException발생 시 응답 메시지는 유지되되, 로그 appender 테스트에서 4xx 경로에 스택트레이스가 찍히지 않는지 확인하는 편이 좋다. As per path instructions, "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다. 로깅 시 민감정보 노출 가능성을 점검한다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/ApiControllerAdvice.kt` around lines 26 - 29, The CoreException handling in ApiControllerAdvice.handle should separate user-facing failure text from operational logging: keep failureResponse using e.customMessage, but change the WARN log to record only a structured business identifier such as e.errorType.code and a minimal identifier context, without the full exception or stacktrace. Reserve cause-preserving logging for unexpected 5xx paths in the same advice class, and add/adjust tests so CoreException still returns the same response while the 4xx log path does not emit a stacktrace or sensitive request text.Source: Path instructions
apps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentJpaRepository.kt (1)
8-8: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win주문별 정렬을 조회 계약에 포함시키는 편이 안전하다.
현재 시그니처는 정렬 없는 리스트를 반환하게 해서 호출부가 전건 로드 후 메모리에서
updatedAt기준 정렬을 하게 만든다. 복구 조회가 반복되는 경로에서는 거래 수가 늘수록 불필요한 I/O와 힙 사용이 커진다.findByUserIdAndOrderIdOrderByUpdatedAtDesc(...)처럼 DB 정렬을 계약으로 올리고 호출부 정렬은 제거해주기 바란다. 추가 테스트로 동일orderId에 여러 거래가 있을 때 최신updatedAt순으로 반환되는 저장소 테스트가 필요하다. As per path instructions,**/*Repository*.kt:정렬/인덱스 활용 가능성, 대량 데이터에서의 성능 병목을 점검한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentJpaRepository.kt` at line 8, The repository method on PaymentJpaRepository currently exposes an unsorted query contract, which pushes ordering work into callers. Update findByUserIdAndOrderId to a derived query that includes DB-side ordering by updatedAt descending, and remove any in-memory sort at the call site so the latest payment is returned first by default. Add or update a repository test around PaymentJpaRepository to verify that multiple Payment records for the same userId and orderId come back in updatedAt-desc order.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b1be570-1501-457a-8407-123ea5e929e3
📒 Files selected for processing (66)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentCommand.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentInfo.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentRecoveryScheduler.javaapps/commerce-api/src/main/java/com/loopers/domain/order/PaymentCommand.javaapps/commerce-api/src/main/java/com/loopers/domain/order/PaymentGateway.javaapps/commerce-api/src/main/java/com/loopers/domain/order/PaymentResult.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/CardType.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentGateway.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentModel.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentService.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PaymentStatus.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PgPaymentCommand.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PgRequestResult.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/PgTransaction.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/FakePaymentGatewayAdapter.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PaymentJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PaymentRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgPaymentGatewayAdapter.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/payment/PgProperties.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/payment/PaymentV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/config/PgClientConfig.javaapps/commerce-api/src/main/java/com/loopers/support/config/SchedulingConfig.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/domain/payment/PaymentModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/payment/PaymentServiceUnitTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/OrderV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/PaymentV1ApiE2ETest.javaapps/pg-simulator/README.mdapps/pg-simulator/build.gradle.ktsapps/pg-simulator/src/main/kotlin/com/loopers/PaymentGatewayApplication.ktapps/pg-simulator/src/main/kotlin/com/loopers/application/payment/OrderInfo.ktapps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentApplicationService.ktapps/pg-simulator/src/main/kotlin/com/loopers/application/payment/PaymentCommand.ktapps/pg-simulator/src/main/kotlin/com/loopers/application/payment/TransactionInfo.ktapps/pg-simulator/src/main/kotlin/com/loopers/config/web/WebMvcConfig.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/CardType.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/Payment.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentEvent.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentEventPublisher.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentRelay.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/PaymentRepository.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/TransactionKeyGenerator.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/payment/TransactionStatus.ktapps/pg-simulator/src/main/kotlin/com/loopers/domain/user/UserInfo.ktapps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentCoreEventPublisher.ktapps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentCoreRelay.ktapps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentCoreRepository.ktapps/pg-simulator/src/main/kotlin/com/loopers/infrastructure/payment/PaymentJpaRepository.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/ApiControllerAdvice.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/ApiResponse.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/argumentresolver/UserInfoArgumentResolver.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentApi.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/payment/PaymentDto.ktapps/pg-simulator/src/main/kotlin/com/loopers/interfaces/event/payment/PaymentEventListener.ktapps/pg-simulator/src/main/kotlin/com/loopers/support/error/CoreException.ktapps/pg-simulator/src/main/kotlin/com/loopers/support/error/ErrorType.ktapps/pg-simulator/src/main/resources/application.ymlhttp/http-client.env.jsonhttp/pg-simulator/payments.httpmodules/redis/src/main/resources/redis.ymlsettings.gradle.kts
💤 Files with no reviewable changes (4)
- apps/commerce-api/src/main/java/com/loopers/domain/order/PaymentCommand.java
- apps/commerce-api/src/main/java/com/loopers/infrastructure/order/FakePaymentGatewayAdapter.java
- apps/commerce-api/src/main/java/com/loopers/domain/order/PaymentResult.java
- apps/commerce-api/src/main/java/com/loopers/domain/order/PaymentGateway.java
2 participants
🧭 Context & Decision
문제 정의
선택지와 결정
[결정 1] 트랜잭션 구조 — 단일 트랜잭션 vs 트랜잭션 경계 분리
고려한 대안:
최종 결정: B
트레이드오프: A는 동시 요청이 둘 다 PG를 호출한 뒤 INSERT에서 하나만 실패해 이미 PG엔 이중 거래가 생긴다. 또 외부 호출이 트랜잭션 안에 있어 응답 대기 동안 커넥션을 점유한다. B는 PENDING이 먼저 커밋돼야 UNIQUE가 락 역할을 해 동시 요청 중 하나만 통과(중복 결제 차단)하고, 외부 호출을 트랜잭션 밖에 둬 풀 고갈을 막는다.
[결정 2] Retry 대상 — 모든 일시적 실패 vs 거래 미생성이 보장되는 실패만
[결정 3] 복구 경로 — 콜백 단독 vs 콜백 + 스케줄러 폴링(+ 시도 상한횟수 설정)
🤔 고민한 점 / 막혔던 부분