Skip to content

feat(s1): application controllers — REST endpoints + CommandHandler (STA-114)#112

Merged
Puneethkumarck merged 3 commits into
mainfrom
feature/STA-114-payment-controllers
Mar 8, 2026
Merged

feat(s1): application controllers — REST endpoints + CommandHandler (STA-114)#112
Puneethkumarck merged 3 commits into
mainfrom
feature/STA-114-payment-controllers

Conversation

@Puneethkumarck

@Puneethkumarck Puneethkumarck commented Mar 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • PaymentController — thin REST handler: POST /v1/payments (201/200 idempotent), GET /v1/payments/{id}, POST /v1/payments/{id}/cancel
  • PaymentCommandHandler — domain service: creates Payment aggregate, saves via repository, starts Temporal workflow, sends cancel signal
  • PaymentResponse DTO with static from(Payment) mapper, InitiatePaymentRequest / CancelPaymentRequest with Jakarta validation
  • PaymentNotFoundException (404, PO-2001), PaymentNotCancellableException (409, PO-2002) + GlobalExceptionHandler updates
  • 22 new tests: 8 CommandHandler, 6 Controller unit, 8 MockMvc integration — 212 total (192 unit + 20 IT)

Test plan

  • POST /v1/payments returns 201 Created on new payment
  • POST /v1/payments returns 200 OK on idempotent replay (same Idempotency-Key)
  • POST /v1/payments returns 400 on invalid request body
  • GET /v1/payments/{id} returns 200 with payment details
  • GET /v1/payments/{id} returns 404 when not found
  • POST /v1/payments/{id}/cancel returns 200 and sends Temporal cancel signal
  • POST /v1/payments/{id}/cancel returns 409 for terminal payments
  • All existing 190 tests still pass

Closes STA-114

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added REST API to initiate, retrieve, and cancel payments with idempotency, proper 201/200 semantics, and richer request/response payloads with validation.
    • Integrated asynchronous workflow start for payment processing.
  • Error Handling

    • Mapped not-found, not-cancellable, and validation errors to clearer HTTP responses.
  • Tests

    • Extensive unit and MVC tests and fixtures covering initiation, idempotent replay, retrieval, cancellation, and error cases.
  • Chores

    • Added webmvc and security test dependencies.

@coderabbitai

coderabbitai Bot commented Mar 8, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Adds payment REST endpoints, DTOs, domain exceptions, a PaymentCommandHandler that manages idempotent initiation and cancellation and starts/signals a Temporal workflow, extends global exception handling, and adds unit/MVC tests plus two test dependencies.

Changes

Cohort / File(s) Summary
API DTOs
payment-orchestrator/.../application/controller/InitiatePaymentRequest.java, .../CancelPaymentRequest.java, .../PaymentResponse.java
New Jakarta-validated request records and a PaymentResponse record with a static mapper from domain Payment.
REST Controller
payment-orchestrator/.../application/controller/PaymentController.java
New Spring REST controller exposing POST /v1/payments (idempotent), GET /v1/payments/{id}, POST /v1/payments/{id}/cancel; delegates to PaymentCommandHandler and returns appropriate status codes.
Domain Service
payment-orchestrator/.../domain/service/PaymentCommandHandler.java
New command handler implementing idempotent initiate (returns InitiateResult with replay flag), getPayment (throws PaymentNotFoundException), cancelPayment (validates state, signals Temporal workflow), and starts Temporal workflows.
Domain Exceptions
payment-orchestrator/.../domain/model/PaymentNotFoundException.java, .../PaymentNotCancellableException.java
New runtime exceptions for missing payments and invalid cancellation attempts with contextual messages.
Global Error Mapping
payment-orchestrator/.../application/controller/GlobalExceptionHandler.java
Added handlers mapping PaymentNotFoundException -> 404 (PAYMENT_NOT_FOUND) and PaymentNotCancellableException -> 409 (PAYMENT_NOT_CANCELLABLE); added HandlerMethodValidationException handling.
Tests (unit & MVC)
payment-orchestrator/.../application/controller/PaymentControllerMvcTest.java, .../PaymentControllerTest.java, .../domain/service/PaymentCommandHandlerTest.java
New MVC and unit tests covering happy/error paths, idempotency replay, Temporal workflow start/signalling, and exception propagation using Mockito and fixtures.
Test Fixtures & Build
payment-orchestrator/.../testFixtures/PaymentFixtures.java, payment-orchestrator/build.gradle.kts
Added test constants and InitiateResult fixtures; added spring-boot-starter-webmvc-test and spring-boot-starter-security-test to testImplementation.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Controller as "PaymentController"
    participant CmdHandler as "PaymentCommandHandler"
    participant Repo as "PaymentRepository"
    participant WorkflowClient as "WorkflowClient"
    participant TemporalWF as "Temporal (PaymentWorkflow)"

    Client->>Controller: POST /v1/payments\n(Idempotency-Key, InitiatePaymentRequest)
    Controller->>CmdHandler: initiatePayment(idempotencyKey, correlationId, ...)
    CmdHandler->>Repo: findByIdempotencyKey(idempotencyKey)
    alt replay found
        Repo-->>CmdHandler: existing Payment
        CmdHandler-->>Controller: InitiateResult(payment, replay=true)
        Controller-->>Client: 200 OK
    else first-time
        Repo-->>CmdHandler: empty
        CmdHandler->>Repo: save(new Payment)
        Repo-->>CmdHandler: saved Payment
        CmdHandler->>WorkflowClient: startWorkflow(payment, ...)
        WorkflowClient->>TemporalWF: start(PaymentRequest)
        TemporalWF-->>WorkflowClient: started
        CmdHandler-->>Controller: InitiateResult(payment, replay=false)
        Controller-->>Client: 201 Created
    end
Loading
sequenceDiagram
    actor Client
    participant Controller as "PaymentController"
    participant CmdHandler as "PaymentCommandHandler"
    participant Repo as "PaymentRepository"
    participant WorkflowClient as "WorkflowClient"
    participant TemporalWF as "Temporal (PaymentWorkflow)"

    Client->>Controller: POST /v1/payments/{paymentId}/cancel\n(CancelPaymentRequest)
    Controller->>CmdHandler: cancelPayment(paymentId, reason)
    CmdHandler->>Repo: findById(paymentId)
    alt not found
        Repo-->>CmdHandler: empty
        CmdHandler-->>Controller: throws PaymentNotFoundException
        Controller-->>Client: 404 Not Found
    else found
        Repo-->>CmdHandler: Payment
        alt terminal state
            CmdHandler-->>Controller: throws PaymentNotCancellableException
            Controller-->>Client: 409 Conflict
        else cancellable
            CmdHandler->>WorkflowClient: getWorkflowStub(paymentId)
            WorkflowClient->>TemporalWF: signal(cancelPayment, CancelRequest)
            TemporalWF-->>WorkflowClient: acknowledged
            CmdHandler-->>Controller: Payment
            Controller-->>Client: 200 OK
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Suggested labels

phase-2, service-s1, feature, test

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main changes: REST endpoints and CommandHandler for payment operations (STA-114).
Description check ✅ Passed Description covers Summary, Related Issue, Type of Change, What Changed, How Was It Tested, and Test Plan with comprehensive coverage of new endpoints and tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/STA-114-payment-controllers

Comment @coderabbitai help to get the list of available commands and usage tips.

…(STA-114)

Add PaymentController (POST/GET/cancel), PaymentCommandHandler with
idempotent replay via Idempotency-Key, Temporal workflow start/cancel
signalling, and 14 new tests (192 total unit, 20 IT).

Closes LIN-114

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Puneethkumarck Puneethkumarck force-pushed the feature/STA-114-payment-controllers branch from 56c8981 to 8b7b204 Compare March 8, 2026 09:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/CancelPaymentRequest.java`:
- Around line 8-11: Add a maximum length constraint to the CancelPaymentRequest
record's reason field to prevent arbitrarily large inputs: update the reason
declaration in CancelPaymentRequest by adding a `@Size`(max =
<appropriate_length>) annotation alongside `@NotBlank` (choose a sensible max,
e.g. 255 or what your DB/logging supports) so validation will reject overly long
cancellation reasons before they reach persistence or logs.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/InitiatePaymentRequest.java`:
- Around line 20-22: The sourceAmount field in InitiatePaymentRequest lacks an
upper bound; add a `@DecimalMax` annotation to enforce a sensible ceiling (or
reference a business-configured constant) and update the validation message
accordingly. Locate the sourceAmount declaration in the InitiatePaymentRequest
record/class and add a `@DecimalMax`(value = "<ceiling>", message = "sourceAmount
must be <= <ceiling>") (or wire the ceiling from configuration/business rules)
so extremely large values are rejected by validation.
- Around line 24-34: Update the InitiatePaymentRequest DTO to validate ISO
formats: add `@Pattern` annotations to sourceCurrency and targetCurrency enforcing
ISO 4217 (e.g., regex "^[A-Z]{3}$" or case-insensitive variant) and to
sourceCountry and targetCountry enforcing ISO 3166-1 alpha-2 (e.g.,
"^[A-Z]{2}$"); include clear validation messages for each field and ensure the
annotations are imported/used on the existing fields (sourceCurrency,
targetCurrency, sourceCountry, targetCountry) in the InitiatePaymentRequest
class, or implement an equivalent custom validator if you need more complex
logic (e.g., checking against a known list of codes).

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java`:
- Line 42: The current log statement in PaymentController (log.info("POST
/v1/payments idempotencyKey={}, senderId={}", idempotencyKey,
request.senderId())) and the similar one that logs request.reason() must not
emit raw, user-controlled idempotency keys or free-text reasons; update these
log calls to either (a) log stable internal identifiers (e.g., paymentId or a
correlationId) instead of idempotencyKey, or (b) redact or hash the
idempotencyKey/reason before logging (e.g., replace with a fixed token like
"<redacted>" or a one-way hash), and keep senderId or other non-sensitive
metadata as needed; locate the log usage in PaymentController and replace the
offending parameters accordingly (avoid logging request.reason() raw and do not
log idempotencyKey directly).
- Around line 39-41: The Idempotency-Key header parameter in
PaymentController.initiatePayment currently allows blank/whitespace values which
bypass idempotency checks; add a javax/jakarta validation constraint by
annotating the idempotencyKey parameter with `@NotBlank` so Spring method-level
validation rejects empty/whitespace headers, e.g., update the parameter
declaration of PaymentController.initiatePayment to include `@NotBlank` on the
idempotencyKey parameter and ensure the controller can use the framework's
built-in method validation.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentResponse.java`:
- Around line 34-53: PaymentResponse.from currently dereferences
payment.corridor() which can be null when Payment is built via the Lombok
builder; update the mapper to null-guard both corridor accesses (the expressions
that call payment.corridor().sourceCountry() and
payment.corridor().targetCountry()) and return null for those fields when
corridor() is null. Locate the PaymentResponse.from(...) method and replace the
direct corridor().sourceCountry()/targetCountry() calls with ternary-style null
checks (or use Optional.ofNullable(payment.corridor()).map(...).orElse(null)) to
avoid NPEs; alternatively, enforce corridor non-null at the Payment record level
via a compact constructor in Payment if you prefer validation at construction
time.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`:
- Around line 60-65: The service currently logs raw user-controlled values
(idempotencyKey and cancel reasons) in PaymentCommandHandler (see the logging
around paymentRepository.findByIdempotencyKey and the later cancel branch), so
change those log statements to avoid printing raw keys/reasons: replace direct
idempotencyKey/cancel reason with either the internal paymentId/correlationId or
a non-reversible fingerprint (e.g., SHA-256 hex of the idempotencyKey) and
redact/omit cancel reason text; keep existing log contexts like paymentId and
correlationId intact and ensure any new fingerprinting uses a stable helper so
logs remain traceable without exposing raw values.
- Around line 62-85: The idempotency race occurs because
paymentRepository.findByIdempotencyKey() and paymentRepository.save(payment)
allow two concurrent requests to both persist the same idempotencyKey; add a
database-level unique constraint on the idempotencyKey column and modify the
Initiate flow to catch duplicate-key errors from paymentRepository.save(payment)
(or the underlying persistence exception), on duplicate-key reload the existing
Payment via paymentRepository.findByIdempotencyKey(idempotencyKey) and return
InitiateResult(existing, true) without calling startWorkflow; ensure only the
thread that successfully inserted the row calls startWorkflow(saved, ...).
Reference: paymentRepository.findByIdempotencyKey, Payment.initiate,
paymentRepository.save, startWorkflow, InitiateResult.
- Around line 80-82: The code currently calls startWorkflow(saved,
sourceCountry, targetCountry) inside initiatePayment() immediately after
paymentRepository.save(payment), which may start a Temporal workflow before the
surrounding `@Transactional` method commits and risk orphaned workflows on
rollback; refactor so initiatePayment() only saves the entity and publishes a
domain event (e.g., PaymentInitiatedEvent) within the transaction, then move the
WorkflowClient.start() call out of startWorkflow() into a separate event
listener annotated with `@TransactionalEventListener`(phase = AFTER_COMMIT) that
receives the PaymentInitiatedEvent and invokes startWorkflow(savedId,
sourceCountry, targetCountry), or alternatively implement a transactional outbox
that the workflow-starter reads after commit. Ensure
PaymentCommandHandler.save/paymentRepository.save remains inside the transaction
and the actual workflow start happens only from the AFTER_COMMIT listener/outbox
processor.

In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java`:
- Around line 52-139: Add a new test method in the InitiatePayment nested class
(e.g., shouldReturn400ForMissingIdempotencyKey) that performs a POST to
"/v1/payments" with a valid JSON body but omits the "Idempotency-Key" header and
asserts status().isBadRequest() (and optionally the error JSON code if your
error handler produces one). This targets the controller handler that declares
`@RequestHeader`("Idempotency-Key") so Spring will reject requests missing that
header; keep the test alongside shouldReturn201/shouldReturn200 to document the
required header contract.

In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java`:
- Around line 65-126: The test currently only asserts persistence; update the
"should create payment, save, and start Temporal workflow" case to verify the
Temporal side effect by asserting that
workflowClient.newWorkflowStub(PaymentWorkflow.class, ...) is invoked and that
the returned workflow stub's start method (e.g., startWorkflow or the actual
starter method on PaymentWorkflow) is called with expected arguments;
conversely, in "should return existing on replay" assert that
workflowClient.newWorkflowStub and any calls on the PaymentWorkflow stub are
never invoked. Locate these checks around handler.initiatePayment in
PaymentCommandHandlerTest and add Mockito verifications
(verify/then(...).should()/should(never())) against workflowClient and the
mocked PaymentWorkflow to enforce the workflow start behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 66c722d5-43f7-485b-a200-7d678bd1ccfd

📥 Commits

Reviewing files that changed from the base of the PR and between d1b15a1 and 56c8981.

📒 Files selected for processing (13)
  • payment-orchestrator/payment-orchestrator/build.gradle.kts
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/CancelPaymentRequest.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/GlobalExceptionHandler.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/InitiatePaymentRequest.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentResponse.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/model/PaymentNotCancellableException.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/model/PaymentNotFoundException.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerTest.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java
  • payment-orchestrator/payment-orchestrator/src/testFixtures/java/com/stablecoin/payments/orchestrator/fixtures/PaymentFixtures.java

Comment on lines +8 to +11
public record CancelPaymentRequest(
@NotBlank(message = "reason is required")
String reason
) {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider adding a length constraint on reason.

Without a @Size(max = ...) limit, clients could submit arbitrarily large cancellation reasons, potentially causing DB storage issues or log bloat downstream.

🛡️ Optional: Add max length constraint
 public record CancelPaymentRequest(
-        `@NotBlank`(message = "reason is required")
+        `@NotBlank`(message = "reason is required")
+        `@Size`(max = 500, message = "reason must not exceed 500 characters")
         String reason
 ) {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/CancelPaymentRequest.java`
around lines 8 - 11, Add a maximum length constraint to the CancelPaymentRequest
record's reason field to prevent arbitrarily large inputs: update the reason
declaration in CancelPaymentRequest by adding a `@Size`(max =
<appropriate_length>) annotation alongside `@NotBlank` (choose a sensible max,
e.g. 255 or what your DB/logging supports) so validation will reject overly long
cancellation reasons before they reach persistence or logs.

Comment on lines +20 to +22
@NotNull(message = "sourceAmount is required")
@DecimalMin(value = "0.01", message = "sourceAmount must be positive")
BigDecimal sourceAmount,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider adding an upper bound on sourceAmount.

Without @DecimalMax, clients could submit astronomically large values. Business rules should define a sensible ceiling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/InitiatePaymentRequest.java`
around lines 20 - 22, The sourceAmount field in InitiatePaymentRequest lacks an
upper bound; add a `@DecimalMax` annotation to enforce a sensible ceiling (or
reference a business-configured constant) and update the validation message
accordingly. Locate the sourceAmount declaration in the InitiatePaymentRequest
record/class and add a `@DecimalMax`(value = "<ceiling>", message = "sourceAmount
must be <= <ceiling>") (or wire the ceiling from configuration/business rules)
so extremely large values are rejected by validation.

Comment on lines +24 to +34
@NotBlank(message = "sourceCurrency is required")
String sourceCurrency,

@NotBlank(message = "targetCurrency is required")
String targetCurrency,

@NotBlank(message = "sourceCountry is required")
String sourceCountry,

@NotBlank(message = "targetCountry is required")
String targetCountry

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider ISO validation for currency and country codes.

sourceCurrency/targetCurrency should ideally conform to ISO 4217 (3-letter codes), and country codes to ISO 3166-1 alpha-2. A @Pattern annotation or custom validator could enforce this at the edge.

💡 Example: Pattern validation for currency codes
-        `@NotBlank`(message = "sourceCurrency is required")
+        `@NotBlank`(message = "sourceCurrency is required")
+        `@Pattern`(regexp = "^[A-Z]{3}$", message = "sourceCurrency must be a valid 3-letter currency code")
         String sourceCurrency,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@NotBlank(message = "sourceCurrency is required")
String sourceCurrency,
@NotBlank(message = "targetCurrency is required")
String targetCurrency,
@NotBlank(message = "sourceCountry is required")
String sourceCountry,
@NotBlank(message = "targetCountry is required")
String targetCountry
`@NotBlank`(message = "sourceCurrency is required")
`@Pattern`(regexp = "^[A-Z]{3}$", message = "sourceCurrency must be a valid 3-letter currency code")
String sourceCurrency,
`@NotBlank`(message = "targetCurrency is required")
String targetCurrency,
`@NotBlank`(message = "sourceCountry is required")
String sourceCountry,
`@NotBlank`(message = "targetCountry is required")
String targetCountry
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/InitiatePaymentRequest.java`
around lines 24 - 34, Update the InitiatePaymentRequest DTO to validate ISO
formats: add `@Pattern` annotations to sourceCurrency and targetCurrency enforcing
ISO 4217 (e.g., regex "^[A-Z]{3}$" or case-insensitive variant) and to
sourceCountry and targetCountry enforcing ISO 3166-1 alpha-2 (e.g.,
"^[A-Z]{2}$"); include clear validation messages for each field and ensure the
annotations are imported/used on the existing fields (sourceCurrency,
targetCurrency, sourceCountry, targetCountry) in the InitiatePaymentRequest
class, or implement an equivalent custom validator if you need more complex
logic (e.g., checking against a known list of codes).

public ResponseEntity<PaymentResponse> initiatePayment(
@RequestHeader("Idempotency-Key") String idempotencyKey,
@Valid @RequestBody InitiatePaymentRequest request) {
log.info("POST /v1/payments idempotencyKey={}, senderId={}", idempotencyKey, request.senderId());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Stop logging raw idempotency keys and cancel reasons.

Both values are user-controlled. Idempotency-Key is a replay token, and reason is free text that can carry PII or secrets. Log stable identifiers like paymentId/correlation metadata instead, or redact/hash before emission.

Also applies to: 79-79

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java`
at line 42, The current log statement in PaymentController (log.info("POST
/v1/payments idempotencyKey={}, senderId={}", idempotencyKey,
request.senderId())) and the similar one that logs request.reason() must not
emit raw, user-controlled idempotency keys or free-text reasons; update these
log calls to either (a) log stable internal identifiers (e.g., paymentId or a
correlationId) instead of idempotencyKey, or (b) redact or hash the
idempotencyKey/reason before logging (e.g., replace with a fixed token like
"<redacted>" or a one-way hash), and keep senderId or other non-sensitive
metadata as needed; locate the log usage in PaymentController and replace the
offending parameters accordingly (avoid logging request.reason() raw and do not
log idempotencyKey directly).

Comment on lines +60 to +65
log.info("Initiating payment idempotencyKey={}, correlationId={}", idempotencyKey, correlationId);

Optional<Payment> existing = paymentRepository.findByIdempotencyKey(idempotencyKey);
if (existing.isPresent()) {
log.info("Idempotent replay for idempotencyKey={}, paymentId={}",
idempotencyKey, existing.get().paymentId());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove raw idempotency keys and cancel reasons from service logs.

These are user-controlled values and should not be written verbatim at INFO level. Keep paymentId/correlation identifiers, or emit a redacted/hash form if the key must be traceable.

Also applies to: 106-106

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`
around lines 60 - 65, The service currently logs raw user-controlled values
(idempotencyKey and cancel reasons) in PaymentCommandHandler (see the logging
around paymentRepository.findByIdempotencyKey and the later cancel branch), so
change those log statements to avoid printing raw keys/reasons: replace direct
idempotencyKey/cancel reason with either the internal paymentId/correlationId or
a non-reversible fingerprint (e.g., SHA-256 hex of the idempotencyKey) and
redact/omit cancel reason text; keep existing log contexts like paymentId and
correlationId intact and ensure any new fingerprinting uses a stable helper so
logs remain traceable without exposing raw values.

Comment on lines +80 to +82
var saved = paymentRepository.save(payment);

startWorkflow(saved, sourceCountry, targetCountry);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

In Spring Framework, when does a @transactional method commit relative to method return, and what patterns are recommended to avoid calling external systems before the transaction successfully commits?

💡 Result:

When does a @Transactional method commit (relative to return)?

In Spring’s imperative (thread-bound) transaction model, the transaction is committed after the transactional method’s invocation completes successfully—i.e., the interceptor runs its “commit after returning” step after your method body finishes, and before control is finally returned to the caller. If commit fails, the exception is raised instead of a “successful” return. [1]

How to avoid calling external systems before the transaction is safely committed

Recommended patterns are all about deferring side effects until “after commit”:

  1. Register an after-commit callback (low-level, explicit)

    • Use Spring transaction synchronizations and perform the external call in an afterCommit() callback (only runs if commit actually happens). [2]
  2. Use @TransactionalEventListener(phase = AFTER_COMMIT) (high-level, idiomatic Spring)

    • Publish a domain/application event inside the transaction, and handle it with @TransactionalEventListener in the AFTER_COMMIT phase (the default phase is AFTER_COMMIT). This ensures the handler runs only after a successful commit. [3]
    • Commonly combined with @Async so the post-commit work doesn’t block the request thread.
  3. Transactional Outbox (most robust for reliable external publication)

    • Persist an “outbox” record (what you intend to send) in the same DB transaction, then have a separate process/thread publish it to the external system and mark it sent. This avoids the “DB committed but external call didn’t happen” / “external call happened but DB rolled back” mismatch. [4]

Sources: TransactionAspectSupport Javadoc (commit-after-returning) [1], TransactionSynchronization Javadoc (afterCommit) [2], Spring Framework Data Access reference (@TransactionalEventListener phases / default) [3], Spring blog on outbox strategies [4].


🏁 Script executed:

find . -name "PaymentCommandHandler.java" -type f

Repository: Puneethkumarck/stablebridge-platform

Length of output: 216


🏁 Script executed:

cat -n ./payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java | head -160

Repository: Puneethkumarck/stablebridge-platform

Length of output: 7518


Defer Temporal workflow initiation until after database transaction commits.

WorkflowClient.start() executes before the @Transactional method commits. If the transaction rolls back post-workflow-start, you create an orphan workflow for a payment that never persisted. Use Spring's @TransactionalEventListener(phase = AFTER_COMMIT) to publish a domain event inside the transaction, then start the workflow in the event handler—or adopt a transactional outbox pattern for reliable async handoff.

This applies to both initiatePayment() (lines 80–82) and startWorkflow() (lines 125–148).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`
around lines 80 - 82, The code currently calls startWorkflow(saved,
sourceCountry, targetCountry) inside initiatePayment() immediately after
paymentRepository.save(payment), which may start a Temporal workflow before the
surrounding `@Transactional` method commits and risk orphaned workflows on
rollback; refactor so initiatePayment() only saves the entity and publishes a
domain event (e.g., PaymentInitiatedEvent) within the transaction, then move the
WorkflowClient.start() call out of startWorkflow() into a separate event
listener annotated with `@TransactionalEventListener`(phase = AFTER_COMMIT) that
receives the PaymentInitiatedEvent and invokes startWorkflow(savedId,
sourceCountry, targetCountry), or alternatively implement a transactional outbox
that the workflow-starter reads after commit. Ensure
PaymentCommandHandler.save/paymentRepository.save remains inside the transaction
and the actual workflow start happens only from the AFTER_COMMIT listener/outbox
processor.

Comment on lines +52 to +139
@Nested
@DisplayName("POST /v1/payments")
class InitiatePayment {

@Test
@DisplayName("should return 201 Created for new payment")
void shouldReturn201() throws Exception {
// given
var result = anInitiateResult();
given(commandHandler.initiatePayment(
eq(IDEMPOTENCY_KEY), any(UUID.class),
eq(SENDER_ID), eq(RECIPIENT_ID), eq(SOURCE_AMOUNT_VALUE),
eq(SOURCE_CURRENCY), eq(TARGET_CURRENCY),
eq(SOURCE_COUNTRY), eq(TARGET_COUNTRY)))
.willReturn(result);

// when/then
mockMvc.perform(post("/v1/payments")
.contentType(MediaType.APPLICATION_JSON)
.header("Idempotency-Key", IDEMPOTENCY_KEY)
.content("""
{
"senderId": "%s",
"recipientId": "%s",
"sourceAmount": %s,
"sourceCurrency": "%s",
"targetCurrency": "%s",
"sourceCountry": "%s",
"targetCountry": "%s"
}
""".formatted(SENDER_ID, RECIPIENT_ID, SOURCE_AMOUNT_VALUE,
SOURCE_CURRENCY, TARGET_CURRENCY,
SOURCE_COUNTRY, TARGET_COUNTRY)))
.andExpect(status().isCreated())
.andExpect(jsonPath("$.paymentId").exists())
.andExpect(jsonPath("$.state").value("INITIATED"));
}

@Test
@DisplayName("should return 200 OK for idempotent replay")
void shouldReturn200ForReplay() throws Exception {
// given
var result = anIdempotentReplayResult();
given(commandHandler.initiatePayment(
eq(IDEMPOTENCY_KEY), any(UUID.class),
eq(SENDER_ID), eq(RECIPIENT_ID), eq(SOURCE_AMOUNT_VALUE),
eq(SOURCE_CURRENCY), eq(TARGET_CURRENCY),
eq(SOURCE_COUNTRY), eq(TARGET_COUNTRY)))
.willReturn(result);

// when/then
mockMvc.perform(post("/v1/payments")
.contentType(MediaType.APPLICATION_JSON)
.header("Idempotency-Key", IDEMPOTENCY_KEY)
.content("""
{
"senderId": "%s",
"recipientId": "%s",
"sourceAmount": %s,
"sourceCurrency": "%s",
"targetCurrency": "%s",
"sourceCountry": "%s",
"targetCountry": "%s"
}
""".formatted(SENDER_ID, RECIPIENT_ID, SOURCE_AMOUNT_VALUE,
SOURCE_CURRENCY, TARGET_CURRENCY,
SOURCE_COUNTRY, TARGET_COUNTRY)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.paymentId").exists())
.andExpect(jsonPath("$.state").value("INITIATED"));
}

@Test
@DisplayName("should return 400 Bad Request for invalid request body")
void shouldReturn400ForInvalidBody() throws Exception {
// when/then
mockMvc.perform(post("/v1/payments")
.contentType(MediaType.APPLICATION_JSON)
.header("Idempotency-Key", IDEMPOTENCY_KEY)
.content("""
{
"sourceAmount": -1
}
"""))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("PO-0001"));
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if Idempotency-Key is required in PaymentController
ast-grep --pattern $'@RequestHeader($$$"Idempotency-Key"$$$) String $$$'

Repository: Puneethkumarck/stablebridge-platform

Length of output: 62


🏁 Script executed:

# Find PaymentController
fd -t f "PaymentController.java" -x cat {}

Repository: Puneethkumarck/stablebridge-platform

Length of output: 3180


🏁 Script executed:

# Search for Idempotency-Key handling across the codebase
rg -i "idempotency" -A 3 -B 3

Repository: Puneethkumarck/stablebridge-platform

Length of output: 50393


🏁 Script executed:

# Look at the test file to understand its imports and setup
cat payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java | head -60

Repository: Puneethkumarck/stablebridge-platform

Length of output: 3238


🏁 Script executed:

# Check total lines in test file and search for additional test methods
wc -l payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java

Repository: Puneethkumarck/stablebridge-platform

Length of output: 229


🏁 Script executed:

# Search for test methods related to missing header
rg -i "missing.*header|header.*missing|idempotency" payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java

Repository: Puneethkumarck/stablebridge-platform

Length of output: 717


🏁 Script executed:

# Check if there are any tests in other nested classes or if more tests follow line 139
tail -50 payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java

Repository: Puneethkumarck/stablebridge-platform

Length of output: 2335


Add test for missing Idempotency-Key header on POST /v1/payments.

The controller enforces @RequestHeader("Idempotency-Key") at the method parameter level, which Spring will reject with 400 BAD_REQUEST if absent. No test currently documents this behavior. While validation errors (invalid body) are covered, the required header enforcement should have an explicit test to clarify the contract.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java`
around lines 52 - 139, Add a new test method in the InitiatePayment nested class
(e.g., shouldReturn400ForMissingIdempotencyKey) that performs a POST to
"/v1/payments" with a valid JSON body but omits the "Idempotency-Key" header and
asserts status().isBadRequest() (and optionally the error JSON code if your
error handler produces one). This targets the controller handler that declares
`@RequestHeader`("Idempotency-Key") so Spring will reject requests missing that
header; keep the test alongside shouldReturn201/shouldReturn200 to document the
required header contract.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`:
- Around line 105-123: The cancelPayment method in PaymentCommandHandler only
reads the Payment via paymentRepository.findById and signals the Temporal
workflow (cancelPayment on PaymentWorkflow) without persisting changes; mark
this method as read-only by annotating cancelPayment with
`@Transactional`(readOnly = true) to clarify intent and allow read-replica
routing, or if you expect future local state updates leave it unchanged and add
a TODO comment; ensure PaymentNotFoundException and
PaymentNotCancellableException behavior remains unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fad9bb15-7c5f-4244-8995-23c10f157cc2

📥 Commits

Reviewing files that changed from the base of the PR and between 56c8981 and 8b7b204.

📒 Files selected for processing (13)
  • payment-orchestrator/payment-orchestrator/build.gradle.kts
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/CancelPaymentRequest.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/GlobalExceptionHandler.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/InitiatePaymentRequest.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentResponse.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/model/PaymentNotCancellableException.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/model/PaymentNotFoundException.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerMvcTest.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/application/controller/PaymentControllerTest.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java
  • payment-orchestrator/payment-orchestrator/src/testFixtures/java/com/stablecoin/payments/orchestrator/fixtures/PaymentFixtures.java

Comment on lines +105 to +123
public Payment cancelPayment(UUID paymentId, String reason) {
log.info("Cancelling payment paymentId={}, reason={}", paymentId, reason);

var payment = paymentRepository.findById(paymentId)
.orElseThrow(() -> new PaymentNotFoundException(paymentId));

if (payment.isTerminal()) {
throw new PaymentNotCancellableException(paymentId, payment.state());
}

var workflowStub = workflowClient.newWorkflowStub(
PaymentWorkflow.class,
"payment-" + paymentId
);
workflowStub.cancelPayment(new CancelRequest(paymentId, reason, "API"));

log.info("Cancel signal sent for payment paymentId={}", paymentId);
return payment;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider marking cancelPayment as read-only or restructuring.

cancelPayment retrieves a payment but does not persist any state change—the actual state transition happens within the Temporal workflow. Declaring @Transactional(readOnly = true) clarifies intent and enables DB read-replica routing if applicable. Alternatively, if future iterations update state here, disregard.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`
around lines 105 - 123, The cancelPayment method in PaymentCommandHandler only
reads the Payment via paymentRepository.findById and signals the Temporal
workflow (cancelPayment on PaymentWorkflow) without persisting changes; mark
this method as read-only by annotating cancelPayment with
`@Transactional`(readOnly = true) to clarify intent and allow read-replica
routing, or if you expect future local state updates leave it unchanged and add
a TODO comment; ensure PaymentNotFoundException and
PaymentNotCancellableException behavior remains unchanged.

Puneethkumarck and others added 2 commits March 8, 2026 11:09
…ge (STA-114)

- Handle concurrent duplicate idempotencyKey via DataIntegrityViolationException
  catch on save — reload existing payment without starting workflow
- Add @notblank validation on Idempotency-Key header for proper 400 responses
- Add HandlerMethodValidationException handler for Spring MVC 7 method-level
  parameter validation
- Add Temporal workflow start/no-start verification in CommandHandler tests
- Add concurrent duplicate race condition test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (4)
payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java (2)

81-91: ⚠️ Potential issue | 🔴 Critical

Do not start Temporal from inside this transaction.

Line 83 is not a durable idempotency barrier, but Line 156 performs the external side effect immediately. If this repository is JPA-backed, save(...) commonly defers the insert/unique-constraint check until flush or commit, so two concurrent requests can still both reach WorkflowClient.start(...), and any later rollback leaves an orphan workflow. Persist/flush the row first, then hand off workflow start in AFTER_COMMIT or via an outbox.

In Spring Data JPA, does Repository.save(...) execute the SQL insert and surface unique-constraint violations before transaction commit, or is saveAndFlush()/EntityManager.flush() required? Also, what happens if a DataIntegrityViolationException is caught inside a `@Transactional` method after the transaction has been marked rollback-only?

Also applies to: 134-157

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`
around lines 81 - 91, The code starts a Temporal workflow (startWorkflow) while
still inside the transaction that calls paymentRepository.save, risking orphan
workflows because save may not flush DB constraints; change to persist-and-flush
the Payment (use paymentRepository.save + paymentRepository.flush or
EntityManager.flush) to ensure the unique constraint is enforced, and move
workflow startup out of the transactional commit by invoking startWorkflow only
in an AFTER_COMMIT hook
(TransactionSynchronizationManager/TransactionSynchronization or an
`@TransactionalEventListener`) or by publishing an outbox/event that a separate
process consumes; keep the existing DataIntegrityViolationException handling
around save/flush and still return InitiateResult(concurrent, true) on
duplicate.

61-66: ⚠️ Potential issue | 🟠 Major

Redact idempotency keys and cancel reasons from INFO logs.

These are user-controlled values and should not be written verbatim at INFO level. Keep paymentId/correlationId, or log a stable one-way fingerprint for the idempotency key if replay tracing is required.

Also applies to: 85-85, 115-115

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`
around lines 61 - 66, The INFO logs in PaymentCommandHandler leak
user-controlled idempotency keys and cancel reasons; change the logging at the
calls around paymentRepository.findByIdempotencyKey and other log.info usages in
this class to avoid printing raw idempotencyKey or cancelReason — either omit
them and retain paymentId/correlationId, or compute and log a stable one-way
fingerprint (e.g., SHA-256 hex) of the idempotencyKey instead; ensure the
idempotencyKey variable is never directly interpolated in log.info calls and
apply the same redaction/fingerprinting pattern to the other occurrences
referenced in this class (e.g., the idempotency replay log and cancel-reason log
sites).
payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java (1)

44-45: ⚠️ Potential issue | 🟠 Major

Stop logging raw replay tokens and free-text cancel reasons.

Idempotency-Key is a user-controlled replay token, and reason is arbitrary text that can carry PII or secrets. Log stable internal identifiers instead, or redact/hash these values before emission.

Suggested change
-        log.info("POST /v1/payments idempotencyKey={}, senderId={}", idempotencyKey, request.senderId());
+        log.info("POST /v1/payments senderId={}", request.senderId());
...
-        log.info("POST /v1/payments/{}/cancel reason={}", paymentId, request.reason());
+        log.info("POST /v1/payments/{}/cancel", paymentId);

Also applies to: 81-82

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java`
around lines 44 - 45, The logs currently emit user-controlled sensitive strings
(idempotencyKey and free-text cancel reasons); update PaymentController to stop
logging raw values by hashing or redacting them before emission (e.g., add a
private helper like hashToken(String) or use a shared util to compute a
SHA-256/HMAC and log a truncated hex fingerprint) and replace direct uses of
idempotencyKey and the cancel reason in the log.info calls with the
hashed/truncated fingerprint or with stable internal identifiers (e.g.,
paymentId) so both the POST log (log.info("POST /v1/payments...")) and the
cancel log (the calls around request.cancelReason()) only emit non-sensitive
fingerprints or internal IDs. Ensure the helper is deterministic, fast, and
documented in code comments.
payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java (1)

83-104: ⚠️ Potential issue | 🟡 Minor

Assert the workflow start, not just stub creation.

This still passes if PaymentCommandHandler.initiatePayment(...) stops invoking the starter method on workflowStub after newWorkflowStub(...). Add a verification on the actual workflow method call so the create-path test covers the Temporal side effect end-to-end.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java`
around lines 83 - 104, The test only verifies newWorkflowStub was created but
not that the workflow was actually started; update the test for
PaymentCommandHandler.initiatePayment to also verify the workflow starter method
on the mocked workflowStub was invoked (e.g., verify that
workflowStub.startPayment(...) or whatever the PaymentWorkflow starter method is
called) with the expected arguments (idempotency/correlation/payment details),
using then(workflowStub).should() (or equivalent) so the Temporal side-effect is
asserted end-to-end in addition to newWorkflowStub invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java`:
- Line 12: Remove the unused import of PaymentRequest from the
PaymentCommandHandlerTest file: delete the line importing
com.stablecoin.payments.orchestrator.domain.workflow.dto.PaymentRequest so that
the class PaymentCommandHandlerTest no longer contains an unused import and
Spotless/CI will pass; ensure no other references to PaymentRequest remain in
PaymentCommandHandlerTest before committing.

---

Duplicate comments:
In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java`:
- Around line 44-45: The logs currently emit user-controlled sensitive strings
(idempotencyKey and free-text cancel reasons); update PaymentController to stop
logging raw values by hashing or redacting them before emission (e.g., add a
private helper like hashToken(String) or use a shared util to compute a
SHA-256/HMAC and log a truncated hex fingerprint) and replace direct uses of
idempotencyKey and the cancel reason in the log.info calls with the
hashed/truncated fingerprint or with stable internal identifiers (e.g.,
paymentId) so both the POST log (log.info("POST /v1/payments...")) and the
cancel log (the calls around request.cancelReason()) only emit non-sensitive
fingerprints or internal IDs. Ensure the helper is deterministic, fast, and
documented in code comments.

In
`@payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java`:
- Around line 81-91: The code starts a Temporal workflow (startWorkflow) while
still inside the transaction that calls paymentRepository.save, risking orphan
workflows because save may not flush DB constraints; change to persist-and-flush
the Payment (use paymentRepository.save + paymentRepository.flush or
EntityManager.flush) to ensure the unique constraint is enforced, and move
workflow startup out of the transactional commit by invoking startWorkflow only
in an AFTER_COMMIT hook
(TransactionSynchronizationManager/TransactionSynchronization or an
`@TransactionalEventListener`) or by publishing an outbox/event that a separate
process consumes; keep the existing DataIntegrityViolationException handling
around save/flush and still return InitiateResult(concurrent, true) on
duplicate.
- Around line 61-66: The INFO logs in PaymentCommandHandler leak user-controlled
idempotency keys and cancel reasons; change the logging at the calls around
paymentRepository.findByIdempotencyKey and other log.info usages in this class
to avoid printing raw idempotencyKey or cancelReason — either omit them and
retain paymentId/correlationId, or compute and log a stable one-way fingerprint
(e.g., SHA-256 hex) of the idempotencyKey instead; ensure the idempotencyKey
variable is never directly interpolated in log.info calls and apply the same
redaction/fingerprinting pattern to the other occurrences referenced in this
class (e.g., the idempotency replay log and cancel-reason log sites).

In
`@payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java`:
- Around line 83-104: The test only verifies newWorkflowStub was created but not
that the workflow was actually started; update the test for
PaymentCommandHandler.initiatePayment to also verify the workflow starter method
on the mocked workflowStub was invoked (e.g., verify that
workflowStub.startPayment(...) or whatever the PaymentWorkflow starter method is
called) with the expected arguments (idempotency/correlation/payment details),
using then(workflowStub).should() (or equivalent) so the Temporal side-effect is
asserted end-to-end in addition to newWorkflowStub invocation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f0e80fc-3817-42cb-8d63-e405037a903c

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7b204 and 52ba109.

📒 Files selected for processing (4)
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/GlobalExceptionHandler.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/application/controller/PaymentController.java
  • payment-orchestrator/payment-orchestrator/src/main/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandler.java
  • payment-orchestrator/payment-orchestrator/src/test/java/com/stablecoin/payments/orchestrator/domain/service/PaymentCommandHandlerTest.java

@Puneethkumarck Puneethkumarck merged commit f035336 into main Mar 8, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant