Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.stablecoin.payments.onramp.api;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Positive;

import java.math.BigDecimal;
import java.util.UUID;

public record CollectionRequest(
@NotNull UUID paymentId,
@NotNull UUID correlationId,
@NotNull @Positive BigDecimal amount,
@NotBlank String currency,
@NotBlank String paymentRailType,

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

Enum validation at controller layer throws raw IllegalArgumentException.

paymentRailType and senderAccountType (line 22) are validated as @NotBlank but converted via valueOf() in the controller. Invalid enum values throw IllegalArgumentException, which GlobalExceptionHandler catches and returns 400 with a generic message. Consider using a custom validator or documenting the accepted values in the API contract.

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

In
`@fiat-on-ramp/fiat-on-ramp-api/src/main/java/com/stablecoin/payments/onramp/api/CollectionRequest.java`
at line 15, The DTO currently has paymentRailType and senderAccountType as
`@NotBlank` Strings and the controller converts them via valueOf(), causing raw
IllegalArgumentException on invalid values; change the validation so invalid
enum values produce a clear 400. Either (preferred) change CollectionRequest to
use the actual enum types (e.g., PaymentRailType paymentRailType,
SenderAccountType senderAccountType) so Spring will reject bad values before
reaching the controller, or add a custom constraint annotation (e.g.,
`@ValueOfEnum`) on the String fields and implement a ConstraintValidator that
checks against the enum and returns a descriptive violation message; also remove
or stop relying on direct Enum.valueOf() in the controller to avoid throwing
IllegalArgumentException.

@NotBlank String railCountry,
@NotBlank String railCurrency,
@NotBlank String pspId,
@NotBlank String pspName,
@NotBlank String senderAccountHash,
@NotBlank String senderBankCode,
@NotBlank String senderAccountType,
@NotBlank String senderAccountCountry
) {}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.stablecoin.payments.onramp.client;

import com.stablecoin.payments.onramp.api.CollectionRequest;
import com.stablecoin.payments.onramp.api.CollectionResponse;
import com.stablecoin.payments.onramp.api.RefundRequest;
import com.stablecoin.payments.onramp.api.RefundResponse;
Expand All @@ -15,6 +16,9 @@
@FeignClient(name = "fiat-on-ramp-service", url = "${app.services.fiat-on-ramp.url}")
public interface FiatOnRampClient {

@PostMapping(value = "/v1/collections", consumes = "application/json", produces = "application/json")
CollectionResponse initiateCollection(@RequestBody CollectionRequest request);

@GetMapping(value = "/v1/collections/{collectionId}", produces = "application/json")
CollectionResponse getCollection(@PathVariable("collectionId") UUID collectionId);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package com.stablecoin.payments.onramp.application.controller;

import com.stablecoin.payments.onramp.api.CollectionRequest;
import com.stablecoin.payments.onramp.api.CollectionResponse;
import com.stablecoin.payments.onramp.api.RefundRequest;
import com.stablecoin.payments.onramp.api.RefundResponse;
import com.stablecoin.payments.onramp.domain.model.AccountType;
import com.stablecoin.payments.onramp.domain.model.BankAccount;
import com.stablecoin.payments.onramp.domain.model.CollectionOrder;
import com.stablecoin.payments.onramp.domain.model.Money;
import com.stablecoin.payments.onramp.domain.model.PaymentRail;
import com.stablecoin.payments.onramp.domain.model.PaymentRailType;
import com.stablecoin.payments.onramp.domain.model.PspIdentifier;
import com.stablecoin.payments.onramp.domain.model.Refund;
import com.stablecoin.payments.onramp.domain.service.CollectionCommandHandler;
import com.stablecoin.payments.onramp.domain.service.RefundCommandHandler;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

import java.util.UUID;

/**
* REST controller for collection order lifecycle management.
* <p>
* Thin HTTP handler that delegates all business logic to
* {@link CollectionCommandHandler} and {@link RefundCommandHandler}.
*/
@Slf4j
@RestController
@RequestMapping("/v1/collections")
@RequiredArgsConstructor
public class CollectionController {

private final CollectionCommandHandler collectionCommandHandler;
private final RefundCommandHandler refundCommandHandler;

/**
* Initiates a new collection order.
* <p>
* Idempotent: returns 200 OK with the existing order if the same paymentId
* is submitted again. Returns 201 CREATED on first creation.
*/
@PostMapping
public ResponseEntity<CollectionResponse> initiateCollection(
@Valid @RequestBody CollectionRequest request) {
log.info("POST /v1/collections paymentId={}", request.paymentId());

var amount = new Money(request.amount(), request.currency());
var paymentRail = new PaymentRail(
PaymentRailType.valueOf(request.paymentRailType()),
request.railCountry(),
request.railCurrency());
var psp = new PspIdentifier(request.pspId(), request.pspName());
var senderAccount = new BankAccount(
request.senderAccountHash(),
request.senderBankCode(),
AccountType.valueOf(request.senderAccountType()),
request.senderAccountCountry());

var result = collectionCommandHandler.initiateCollection(
request.paymentId(),
request.correlationId(),
amount,
paymentRail,
psp,
senderAccount);

var response = toCollectionResponse(result.order());
var status = result.created() ? HttpStatus.CREATED : HttpStatus.OK;

return ResponseEntity.status(status).body(response);
}

/**
* Retrieves a collection order by its ID.
*/
@GetMapping("/{collectionId}")
public CollectionResponse getCollection(@PathVariable UUID collectionId) {
log.info("GET /v1/collections/{}", collectionId);
return toCollectionResponse(collectionCommandHandler.getCollection(collectionId));
}

/**
* Retrieves a collection order by its associated payment ID.
*/
@GetMapping
public CollectionResponse getCollectionByPaymentId(@RequestParam UUID paymentId) {
log.info("GET /v1/collections?paymentId={}", paymentId);
return toCollectionResponse(collectionCommandHandler.getCollectionByPaymentId(paymentId));
}

/**
* Initiates a refund for a collected order.
*/
@PostMapping("/{collectionId}/refunds")
public ResponseEntity<RefundResponse> initiateRefund(
@PathVariable UUID collectionId,
@Valid @RequestBody RefundRequest request) {
log.info("POST /v1/collections/{}/refunds", collectionId);

var refundAmount = new Money(request.refundAmount(), request.currency());
var refund = refundCommandHandler.initiateRefund(collectionId, refundAmount, request.reason());

return ResponseEntity.status(HttpStatus.CREATED).body(toRefundResponse(refund));
}

private CollectionResponse toCollectionResponse(CollectionOrder order) {
return new CollectionResponse(
order.collectionId(),
order.paymentId(),
order.status().name(),
order.paymentRail().rail().name(),
order.psp().pspName(),
order.pspReference(),
order.pspSettledAt(),
order.createdAt(),
order.expiresAt());
}
Comment on lines +117 to +128

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 null-safety on optional fields.

pspReference, pspSettledAt, and expiresAt may be null depending on order state. If CollectionResponse requires non-null values for certain fields, consider defensive handling. Current implementation correctly passes nulls for nullable response fields.

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

In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/application/controller/CollectionController.java`
around lines 117 - 128, toCollectionResponse currently forwards possibly-null
values (pspReference, pspSettledAt, expiresAt) from CollectionOrder into the
CollectionResponse; ensure null-safety by either (A) updating the
CollectionResponse constructor/fields to accept nullable values or Optional
types, or (B) defensively mapping nulls before constructing the response (e.g.,
Optional.ofNullable(order.pspReference()),
Optional.ofNullable(order.pspSettledAt()),
Optional.ofNullable(order.expiresAt()) or sensible default values). Locate the
toCollectionResponse method and the CollectionResponse class/constructor to make
the change so the API model no longer assumes non-null inputs and avoids NPEs.


private RefundResponse toRefundResponse(Refund refund) {
return new RefundResponse(
refund.refundId(),
refund.collectionId(),
refund.status().name(),
refund.refundAmount().amount(),
refund.refundAmount().currency(),
refund.initiatedAt(),
refund.completedAt());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package com.stablecoin.payments.onramp.application.controller;

import com.stablecoin.payments.onramp.api.ApiError;
import com.stablecoin.payments.onramp.domain.exception.CollectionOrderNotFoundException;
import com.stablecoin.payments.onramp.domain.exception.RefundAmountExceededException;
import com.stablecoin.payments.onramp.domain.exception.RefundNotAllowedException;
import com.stablecoin.payments.onramp.domain.exception.RefundNotFoundException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.validation.FieldError;
import org.springframework.validation.ObjectError;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;

import java.util.stream.Collectors;

import static org.springframework.http.HttpStatus.BAD_REQUEST;
import static org.springframework.http.HttpStatus.CONFLICT;
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
import static org.springframework.http.HttpStatus.NOT_FOUND;
import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY;

/**
* Global exception handler for the Fiat On-Ramp service.
* <p>
* Maps domain exceptions to appropriate HTTP responses with error codes.
*/
@Slf4j
@RestControllerAdvice
public class GlobalExceptionHandler {

@ResponseStatus(BAD_REQUEST)
@ExceptionHandler(MethodArgumentNotValidException.class)
public ApiError handleValidation(MethodArgumentNotValidException ex) {
var errors = ex.getBindingResult().getFieldErrors().stream()
.collect(Collectors.groupingBy(
FieldError::getField,
Collectors.mapping(ObjectError::getDefaultMessage, Collectors.toList())));
log.info("Validation failed: {}", errors);
return ApiError.withErrors("OR-0001", BAD_REQUEST.getReasonPhrase(),
"Invalid request content", errors);
}

@ResponseStatus(BAD_REQUEST)
@ExceptionHandler(MethodArgumentTypeMismatchException.class)
public ApiError handleTypeMismatch(MethodArgumentTypeMismatchException ex) {
log.info("Type mismatch for parameter '{}': {}", ex.getName(), ex.getMessage());
return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(),
"Invalid value for parameter '" + ex.getName() + "'");
}

@ResponseStatus(NOT_FOUND)
@ExceptionHandler(CollectionOrderNotFoundException.class)
public ApiError handleCollectionNotFound(CollectionOrderNotFoundException ex) {
log.info("Collection order not found: {}", ex.getMessage());
return ApiError.of(CollectionOrderNotFoundException.ERROR_CODE, NOT_FOUND.getReasonPhrase(),
ex.getMessage());
}

@ResponseStatus(NOT_FOUND)
@ExceptionHandler(RefundNotFoundException.class)
public ApiError handleRefundNotFound(RefundNotFoundException ex) {
log.info("Refund not found: {}", ex.getMessage());
return ApiError.of(RefundNotFoundException.ERROR_CODE, NOT_FOUND.getReasonPhrase(),
ex.getMessage());
}

@ResponseStatus(CONFLICT)
@ExceptionHandler(RefundNotAllowedException.class)
public ApiError handleRefundNotAllowed(RefundNotAllowedException ex) {
log.info("Refund not allowed: {}", ex.getMessage());
return ApiError.of(RefundNotAllowedException.ERROR_CODE, CONFLICT.getReasonPhrase(),
ex.getMessage());
}

@ResponseStatus(UNPROCESSABLE_ENTITY)
@ExceptionHandler(RefundAmountExceededException.class)
public ApiError handleRefundAmountExceeded(RefundAmountExceededException ex) {
log.info("Refund amount exceeded: {}", ex.getMessage());
return ApiError.of(RefundAmountExceededException.ERROR_CODE, UNPROCESSABLE_ENTITY.getReasonPhrase(),
ex.getMessage());
}

@ResponseStatus(CONFLICT)
@ExceptionHandler(IllegalStateException.class)
public ApiError handleIllegalState(IllegalStateException ex) {
log.info("Invalid state transition: {}", ex.getMessage());
return ApiError.of("OR-0002", CONFLICT.getReasonPhrase(), ex.getMessage());
}

@ResponseStatus(BAD_REQUEST)
@ExceptionHandler(IllegalArgumentException.class)
public ApiError handleIllegalArgument(IllegalArgumentException ex) {
log.info("Illegal argument: {}", ex.getMessage());
return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(), ex.getMessage());
}
Comment on lines +86 to +98

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

Potential information leakage via generic exception messages.

IllegalStateException and IllegalArgumentException are broad JDK exceptions thrown by frameworks, libraries, and internal code. Exposing ex.getMessage() directly can leak implementation details (class names, internal state, parameter values).

Consider:

  1. Replace with dedicated domain exceptions for controlled messaging.
  2. If catching these is necessary, sanitize or replace messages before returning to client.
🛡️ Safer alternative with generic client messages
 `@ResponseStatus`(CONFLICT)
 `@ExceptionHandler`(IllegalStateException.class)
 public ApiError handleIllegalState(IllegalStateException ex) {
-    log.info("Invalid state transition: {}", ex.getMessage());
-    return ApiError.of("OR-0002", CONFLICT.getReasonPhrase(), ex.getMessage());
+    log.warn("Invalid state transition: {}", ex.getMessage());
+    return ApiError.of("OR-0002", CONFLICT.getReasonPhrase(),
+            "Operation not allowed in current state");
 }

 `@ResponseStatus`(BAD_REQUEST)
 `@ExceptionHandler`(IllegalArgumentException.class)
 public ApiError handleIllegalArgument(IllegalArgumentException ex) {
-    log.info("Illegal argument: {}", ex.getMessage());
-    return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(), ex.getMessage());
+    log.warn("Illegal argument: {}", ex.getMessage());
+    return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(),
+            "Invalid request parameters");
 }

Alternatively, introduce domain-specific exceptions (e.g., InvalidCollectionStateException) with controlled messages and remove these broad catch-all handlers.

📝 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
@ResponseStatus(CONFLICT)
@ExceptionHandler(IllegalStateException.class)
public ApiError handleIllegalState(IllegalStateException ex) {
log.info("Invalid state transition: {}", ex.getMessage());
return ApiError.of("OR-0002", CONFLICT.getReasonPhrase(), ex.getMessage());
}
@ResponseStatus(BAD_REQUEST)
@ExceptionHandler(IllegalArgumentException.class)
public ApiError handleIllegalArgument(IllegalArgumentException ex) {
log.info("Illegal argument: {}", ex.getMessage());
return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(), ex.getMessage());
}
`@ResponseStatus`(CONFLICT)
`@ExceptionHandler`(IllegalStateException.class)
public ApiError handleIllegalState(IllegalStateException ex) {
log.warn("Invalid state transition: {}", ex.getMessage());
return ApiError.of("OR-0002", CONFLICT.getReasonPhrase(),
"Operation not allowed in current state");
}
`@ResponseStatus`(BAD_REQUEST)
`@ExceptionHandler`(IllegalArgumentException.class)
public ApiError handleIllegalArgument(IllegalArgumentException ex) {
log.warn("Illegal argument: {}", ex.getMessage());
return ApiError.of("OR-0001", BAD_REQUEST.getReasonPhrase(),
"Invalid request parameters");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/application/controller/GlobalExceptionHandler.java`
around lines 86 - 98, The handlers in GlobalExceptionHandler (methods
handleIllegalState and handleIllegalArgument) currently return raw
ex.getMessage() which can leak internal details; instead either map these broad
exceptions to domain-specific exceptions (e.g., InvalidCollectionStateException)
and handle those, or sanitize the message before returning to the client by
replacing ex.getMessage() with a generic, non-sensitive message (e.g., "Invalid
request" or "Conflict occurred") when calling ApiError.of, and log the full
exception detail internally (using log.debug/error) for diagnostics; update both
handleIllegalState and handleIllegalArgument to use sanitized messages or new
domain exceptions while keeping ApiError.of usage for structured responses.


@ResponseStatus(INTERNAL_SERVER_ERROR)
@ExceptionHandler(Exception.class)
public ApiError handleUnexpected(Exception ex) {
log.error("Unexpected error: ", ex);
return ApiError.of("OR-9999", INTERNAL_SERVER_ERROR.getReasonPhrase(),
INTERNAL_SERVER_ERROR.getReasonPhrase());
}
}
Loading
Loading