feat(s5): Circle USDC redemption adapter + 7 WireMock tests (STA-151)#155
Merged
Puneethkumarck merged 3 commits intoMar 10, 2026
Merged
Conversation
… 24 ITs (STA-149, STA-150) 3 JPA entities (PayoutOrder with @Version optimistic locking, StablecoinRedemption, OffRampTransaction with @JdbcTypeCode(SqlTypes.JSON) for JSONB), 3 Spring Data JPA repositories, 4 MapStruct mappers (including PayoutOrderEntityUpdater for in-place Hibernate update), 3 persistence adapters implementing domain ports, V2 migration adding BankAccount (4 cols) and MobileMoneyAccount (3 cols) to payout_orders. 24 integration tests: 13 PayoutOrder (CRUD, state machine happy/failure paths, unique constraints, findBy queries), 5 StablecoinRedemption (save, findById, findByPayoutId, ticker round-trip), 6 OffRampTransaction (append-only, JSONB round-trip, multiple per payout). 181 total tests (149 unit + 8 ArchUnit + 24 IT). Closes STA-149 Closes STA-150 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… ordered queries, new ITs (STA-150) - Fail-fast on partial bank/mobile columns in PayoutOrderPersistenceMapper - Scope sp_readonly GRANT to service-specific tables in V2 migration - Add deterministic ordering to OffRampTransactionJpaRepository (receivedAt ASC) - Add optimistic locking IT for PayoutOrderEntity @Version - Add FK constraint IT for StablecoinRedemption Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CircleRedemptionAdapter implements RedemptionGateway port via Circle Business Account Payouts API (POST /v1/businessAccount/payouts). Bearer API key auth, @CIRCUITBREAKER, HTTP/1.1. Package-private ACL DTOs. FallbackAdaptersConfig provides dev RedemptionGateway. 188 total tests (149 unit + 8 ArchUnit + 7 WireMock + 24 IT). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Puneethkumarck
added
phase-3
WalkthroughThis PR implements the persistence layer for off-ramp transactions, payout orders, and stablecoin redemptions using Spring Data JPA and MapStruct. It adds a Circle-based redemption provider with circuit breaker fallback, database schema modifications to flatten value objects, and comprehensive integration tests validating end-to-end persistence flows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/config/FallbackAdaptersConfig.java`:
- Around line 25-38: The fallbackRedemptionGateway() bean uses Instant.now()
directly which breaks test determinism; change the bean to accept the existing
Clock bean (add Clock clock parameter to fallbackRedemptionGateway(Clock clock))
and replace Instant.now() with clock.instant() inside the RedemptionGateway
lambda so the timestamp is driven by the injected Clock; keep the rest of the
RedemptionResult construction and logging intact.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/PayoutOrderEntity.java`:
- Around line 90-91: The failureReason field in PayoutOrderEntity is currently
mapped without a length and may be truncated; update the mapping for the field
"failureReason" in class PayoutOrderEntity to allow larger text — either add a
length attribute (e.g., length=1000) on `@Column`, or annotate with `@Lob` or use
columnDefinition="TEXT" to store arbitrary-length messages; pick one approach,
apply it to the failureReason field, and keep the column name "failure_reason"
unchanged.
- Around line 119-127: Add JPA lifecycle callbacks to PayoutOrderEntity to
auto-populate createdAt and updatedAt: implement a `@PrePersist` method that sets
createdAt and updatedAt to Instant.now() when saving a new entity, and a
`@PreUpdate` method that updates updatedAt to Instant.now() on updates; import
jakarta.persistence.PrePersist and jakarta.persistence.PreUpdate and ensure the
methods reference the existing fields createdAt and updatedAt so timestamps are
consistently populated regardless of how the entity is persisted.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/OffRampTransactionPersistenceMapper.java`:
- Around line 7-8: The `@Mapper` on OffRampTransactionPersistenceMapper must be
configured to generate a Spring bean; update the `@Mapper` annotation on the
OffRampTransactionPersistenceMapper interface to include componentModel =
"spring" so MapStruct produces a Spring-managed implementation that can be
injected into OffRampTransactionPersistenceAdapter (which uses constructor
injection via `@RequiredArgsConstructor`). Ensure the mapper interface keeps its
methods unchanged and rebuild so MapStruct generates the Spring bean.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/PayoutOrderPersistenceMapper.java`:
- Around line 62-65: The mapping for PartnerIdentifier in
PayoutOrderPersistenceMapper currently returns null when only one of
entity.getOffRampPartnerId() or entity.getOffRampPartnerName() is present, which
is inconsistent with mapBankAccount and mapMobileMoneyAccount; change the logic
to validate partial data and throw an exception (or IllegalStateException) when
exactly one of offRampPartnerId/offRampPartnerName is non-null, and only
construct new PartnerIdentifier(entity.getOffRampPartnerId(),
entity.getOffRampPartnerName()) when both are non-null, mirroring the
partial-data checks used in mapBankAccount/mapMobileMoneyAccount.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/StablecoinRedemptionPersistenceMapper.java`:
- Around line 33-35: StablecoinTicker.of(entity.getStablecoin()) can throw
IllegalArgumentException for unsupported tickers; update
StablecoinRedemptionPersistenceMapper to guard that call (wrap the
StablecoinTicker.of(...) invocation in a try-catch catching
IllegalArgumentException), on exception log a warning including the raw
entity.getStablecoin() value and set stablecoin to null (or an Optional.empty
equivalent), and add a TODO to enforce a DB constraint or upstream validation to
prevent unsupported values; reference the StablecoinRedemptionPersistenceMapper
class and the StablecoinTicker.of(...) call when making the change.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/PayoutOrderPersistenceAdapter.java`:
- Around line 26-34: The save method in PayoutOrderPersistenceAdapter does a
read-modify-write via jpa.findById(...) then jpa.save(...) without a
transactional boundary which can cause lost updates; annotate the
save(PayoutOrder order) method with `@Transactional` and add the import
org.springframework.transaction.annotation.Transactional so the find-update-save
sequence in updater.updateEntity(...) and jpa.save(...) executes atomically
within a transaction.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/StablecoinRedemptionPersistenceAdapter.java`:
- Around line 22-25: Replace the direct jpa.save call in
StablecoinRedemptionPersistenceAdapter.save with an explicit upsert: use the
repository (jpa) to findById(redemption.getRedemptionId()), if present update
the existing entity with fields from mapper.toEntity(redemption) (preserve the
existing entity instance and apply changes), then save the updated entity; if
not present, convert the domain to a new entity via mapper.toEntity(redemption)
and save it; finally return mapper.toDomain(savedEntity). Adjust mapper/use of
toEntity/toDomain and repository calls accordingly in the save method.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleProperties.java`:
- Around line 8-21: The CircleProperties constructor currently silently replaces
missing values for baseUrl, apiKey, destinationId, and timeoutSeconds; update it
to (1) emit a clear warning via the logger whenever you apply a default for
baseUrl, destinationId or apiKey (e.g., "Using default baseUrl: ...", "Using
default destinationId: ...") referencing the CircleProperties class and those
field names, and (2) enforce stricter validation for required fields by throwing
a configuration exception (or at least failing fast) when destinationId or
baseUrl are absent in non-development environments; also keep the timeoutSeconds
defaulting but log that default as well so configuration issues are visible.
- Around line 12-14: The CircleProperties class currently silently replaces a
blank apiKey with "SAND_API_KEY_DEFAULT", which masks misconfiguration; update
CircleProperties to stop using a hardcoded fallback: validate apiKey on
construction/initialization (e.g., via `@Validated` + `@NotBlank` on the apiKey
field or explicitly in the CircleProperties constructor/getter) and throw an
IllegalArgumentException when apiKey is null/blank in non-development profiles
(allowing an explicit dev-only default only behind a profile check), ensuring
the apiKey field is required in production instead of being silently
substituted.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`:
- Around line 83-88: The redeemFallback method currently logs and re-throws an
IllegalStateException which defeats graceful degradation; update
redeemFallback(RedemptionRequest request, Exception ex) to instead return a
degraded RedemptionResult (e.g., status PENDING or QUEUED) and include
identifying info from request (payoutId, any request id) so callers can track
it, or if you truly want fail-fast remove the Hystrix/CircuitBreaker fallback
usage entirely; locate the redeemFallback method and either (A) replace the
throw new IllegalStateException(...) with a constructed RedemptionResult
representing a pending/queued state (populate payoutId, status, and an
informative message) or (B) remove the fallback annotation so the exception
propagates.
- Around line 63-73: CircleRedemptionAdapter currently assumes
restClient.post().retrieve().body(...) returns non-null; add null-safety around
the returned variable response and response.data() (the local variables response
and payoutData) before accessing payoutData.id(), payoutData.status(), or
payoutData.amount(); if response or payoutData is null, log a clear error via
log.error including context (request.payoutId() and any response metadata) and
handle the failure path (throw an appropriate exception or return a failed
result) instead of dereferencing a null payoutData. Ensure you reference the
variables response and payoutData and perform the checks immediately after
deserialization in the method that contains this snippet in
CircleRedemptionAdapter.
- Around line 75-80: The RedemptionResult uses Instant.now() which hinders
testability; modify CircleRedemptionAdapter to accept a Clock (inject via
constructor and store as a field), replace Instant.now() with clock.instant()
when building redeemedAt in the method that returns the RedemptionResult, and
update any callers/bean configuration to provide the existing
FallbackAdaptersConfig Clock bean so tests can supply a fixed Clock.
In `@fiat-off-ramp/fiat-off-ramp/src/main/resources/application.yml`:
- Around line 120-125: Current YAML provides a default API key placeholder for
redemption.circle.api-key which can mask misconfiguration; either remove the
fallback by changing the property to use ${CIRCLE_API_KEY} with no default, or
add startup validation in your configuration binding: annotate the
CircleProperties class (the `@ConfigurationProperties` bean that maps
redemption.circle.*) with `@Validated` and mark the apiKey field with `@NotBlank` so
the application fails fast on missing secrets (ensure the bean is registered as
a configuration properties bean so validation runs).
In
`@fiat-off-ramp/fiat-off-ramp/src/main/resources/db/migration/V2__add_bank_and_mobile_money_columns.sql`:
- Around line 4-17: The migration adds payout_orders columns for BankAccount and
MobileMoneyAccount but does not enforce the stated invariant "at least one of
bank or mobile money must be set"; add a table-level CHECK constraint (e.g.,
chk_payout_account_required) on payout_orders after the column additions that
requires either both bank_account_number and bank_code to be non-NULL or both
mobile_money_provider and mobile_money_phone to be non-NULL so the database
prevents records missing both account types; include the constraint name
chk_payout_account_required and reference the columns bank_account_number,
bank_code, mobile_money_provider, and mobile_money_phone when adding it.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterTest.java`:
- Around line 108-110: The tests in CircleRedemptionAdapterTest currently assert
any Exception from adapter.redeem(aRedemptionRequest()), which is too broad;
update each test to assert the specific exception type thrown by Spring's
RestClient for that scenario (e.g., replace isInstanceOf(Exception.class) in the
redeem test with HttpClientErrorException.BadRequest, in redeem_unauthorized use
HttpClientErrorException.Unauthorized, in redeem_timeout assert
ResourceAccessException (or the specific timeout exception your RestTemplate
throws), and in redeem_serverError assert
HttpServerErrorException.InternalServerError) so each test asserts the precise
exception type from adapter.redeem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d663d16f-f83c-4ab8-bb7a-cf443a92eccf
📒 Files selected for processing (24)
fiat-off-ramp/fiat-off-ramp/src/integration-test/java/com/stablecoin/payments/offramp/infrastructure/persistence/OffRampTransactionPersistenceAdapterIT.javafiat-off-ramp/fiat-off-ramp/src/integration-test/java/com/stablecoin/payments/offramp/infrastructure/persistence/PayoutOrderPersistenceAdapterIT.javafiat-off-ramp/fiat-off-ramp/src/integration-test/java/com/stablecoin/payments/offramp/infrastructure/persistence/StablecoinRedemptionPersistenceAdapterIT.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/config/FallbackAdaptersConfig.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/OffRampTransactionPersistenceAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/PayoutOrderPersistenceAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/StablecoinRedemptionPersistenceAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/OffRampTransactionEntity.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/OffRampTransactionJpaRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/PayoutOrderEntity.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/PayoutOrderJpaRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/StablecoinRedemptionEntity.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/StablecoinRedemptionJpaRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/OffRampTransactionPersistenceMapper.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/PayoutOrderEntityUpdater.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/PayoutOrderPersistenceMapper.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/StablecoinRedemptionPersistenceMapper.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CirclePayoutRequest.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CirclePayoutResponse.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleProperties.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/resources/application.ymlfiat-off-ramp/fiat-off-ramp/src/main/resources/db/migration/V2__add_bank_and_mobile_money_columns.sqlfiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterTest.java
Comment on lines
+25
to
+38
| @Bean | ||
| @ConditionalOnMissingBean | ||
| public RedemptionGateway fallbackRedemptionGateway() { | ||
| return request -> { | ||
| log.warn("[FALLBACK-REDEMPTION] Using dev redemption gateway payoutId={} amount={}", | ||
| request.payoutId(), request.amount()); | ||
| return new RedemptionResult( | ||
| "dev-redeem-" + UUID.randomUUID(), | ||
| request.amount().multiply(new BigDecimal("0.92")), | ||
| "EUR", | ||
| Instant.now() | ||
| ); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use injected Clock instead of Instant.now() for testability.
The same config defines a Clock bean, but the fallback gateway uses Instant.now() directly. For consistency and test determinism:
Proposed fix
`@Bean`
`@ConditionalOnMissingBean`
-public RedemptionGateway fallbackRedemptionGateway() {
- return request -> {
+public RedemptionGateway fallbackRedemptionGateway(Clock clock) {
+ return request -> {
log.warn("[FALLBACK-REDEMPTION] Using dev redemption gateway payoutId={} amount={}",
request.payoutId(), request.amount());
return new RedemptionResult(
"dev-redeem-" + UUID.randomUUID(),
request.amount().multiply(new BigDecimal("0.92")),
"EUR",
- Instant.now()
+ clock.instant()
);
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/config/FallbackAdaptersConfig.java`
around lines 25 - 38, The fallbackRedemptionGateway() bean uses Instant.now()
directly which breaks test determinism; change the bean to accept the existing
Clock bean (add Clock clock parameter to fallbackRedemptionGateway(Clock clock))
and replace Instant.now() with clock.instant() inside the RedemptionGateway
lambda so the timestamp is driven by the injected Clock; keep the rest of the
RedemptionResult construction and logging intact.
Comment on lines
+90
to
+91
| @Column(name = "failure_reason") | ||
| private String failureReason; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding length constraint to failureReason.
Without explicit length, Hibernate defaults to 255 characters. Long failure messages (stack traces, API error details) may be silently truncated. Either increase the limit or use @Lob/columnDefinition = "TEXT".
♻️ Proposed change
- `@Column`(name = "failure_reason")
+ `@Column`(name = "failure_reason", columnDefinition = "TEXT")
private String failureReason;📝 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
| @Column(name = "failure_reason") | |
| private String failureReason; | |
| `@Column`(name = "failure_reason", columnDefinition = "TEXT") | |
| private String failureReason; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/PayoutOrderEntity.java`
around lines 90 - 91, The failureReason field in PayoutOrderEntity is currently
mapped without a length and may be truncated; update the mapping for the field
"failureReason" in class PayoutOrderEntity to allow larger text — either add a
length attribute (e.g., length=1000) on `@Column`, or annotate with `@Lob` or use
columnDefinition="TEXT" to store arbitrary-length messages; pick one approach,
apply it to the failureReason field, and keep the column name "failure_reason"
unchanged.
Comment on lines
+119
to
+127
| @Version | ||
| @Column(name = "version") | ||
| private Long version; | ||
|
|
||
| @Column(name = "created_at", updatable = false, nullable = false) | ||
| private Instant createdAt; | ||
|
|
||
| @Column(name = "updated_at", nullable = false) | ||
| private Instant updatedAt; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider JPA lifecycle callbacks for audit timestamps.
createdAt and updatedAt require manual population. Using @PrePersist and @PreUpdate ensures consistency regardless of how the entity is saved.
♻️ Proposed lifecycle callbacks
+ `@PrePersist`
+ protected void onCreate() {
+ createdAt = Instant.now();
+ updatedAt = Instant.now();
+ }
+
+ `@PreUpdate`
+ protected void onUpdate() {
+ updatedAt = Instant.now();
+ }
+
`@Version`
`@Column`(name = "version")
private Long version;Add import: import jakarta.persistence.PrePersist; and import jakarta.persistence.PreUpdate;
📝 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
| @Version | |
| @Column(name = "version") | |
| private Long version; | |
| @Column(name = "created_at", updatable = false, nullable = false) | |
| private Instant createdAt; | |
| @Column(name = "updated_at", nullable = false) | |
| private Instant updatedAt; | |
| `@PrePersist` | |
| protected void onCreate() { | |
| createdAt = Instant.now(); | |
| updatedAt = Instant.now(); | |
| } | |
| `@PreUpdate` | |
| protected void onUpdate() { | |
| updatedAt = Instant.now(); | |
| } | |
| `@Version` | |
| `@Column`(name = "version") | |
| private Long version; | |
| `@Column`(name = "created_at", updatable = false, nullable = false) | |
| private Instant createdAt; | |
| `@Column`(name = "updated_at", nullable = false) | |
| private Instant updatedAt; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/entity/PayoutOrderEntity.java`
around lines 119 - 127, Add JPA lifecycle callbacks to PayoutOrderEntity to
auto-populate createdAt and updatedAt: implement a `@PrePersist` method that sets
createdAt and updatedAt to Instant.now() when saving a new entity, and a
`@PreUpdate` method that updates updatedAt to Instant.now() on updates; import
jakarta.persistence.PrePersist and jakarta.persistence.PreUpdate and ensure the
methods reference the existing fields createdAt and updatedAt so timestamps are
consistently populated regardless of how the entity is persisted.
Comment on lines
+7
to
+8
| @Mapper | ||
| public interface OffRampTransactionPersistenceMapper { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how OffRampTransactionPersistenceMapper is instantiated in the adapter
rg -n "OffRampTransactionPersistenceMapper" --type java -C 3Repository: Puneethkumarck/stablebridge-platform
Length of output: 4176
🏁 Script executed:
cat -n fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/OffRampTransactionPersistenceMapper.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 1970
Add componentModel = "spring" to register the mapper as a Spring bean.
Without it, MapStruct generates a plain implementation class that Spring cannot inject. Since OffRampTransactionPersistenceAdapter uses constructor injection via @RequiredArgsConstructor, this will fail at runtime with a NoSuchBeanDefinitionException.
-@Mapper
+@Mapper(componentModel = "spring")
public interface OffRampTransactionPersistenceMapper {📝 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
| @Mapper | |
| public interface OffRampTransactionPersistenceMapper { | |
| `@Mapper`(componentModel = "spring") | |
| public interface OffRampTransactionPersistenceMapper { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/OffRampTransactionPersistenceMapper.java`
around lines 7 - 8, The `@Mapper` on OffRampTransactionPersistenceMapper must be
configured to generate a Spring bean; update the `@Mapper` annotation on the
OffRampTransactionPersistenceMapper interface to include componentModel =
"spring" so MapStruct produces a Spring-managed implementation that can be
injected into OffRampTransactionPersistenceAdapter (which uses constructor
injection via `@RequiredArgsConstructor`). Ensure the mapper interface keeps its
methods unchanged and rebuild so MapStruct generates the Spring bean.
Comment on lines
+62
to
+65
| PartnerIdentifier offRampPartner = null; | ||
| if (entity.getOffRampPartnerId() != null && entity.getOffRampPartnerName() != null) { | ||
| offRampPartner = new PartnerIdentifier(entity.getOffRampPartnerId(), entity.getOffRampPartnerName()); | ||
| } |
There was a problem hiding this comment.
Inconsistent partial-data validation for PartnerIdentifier.
mapBankAccount and mapMobileMoneyAccount throw on partial columns, but PartnerIdentifier silently returns null if only one of partnerId or partnerName is set. This could mask data corruption.
Consistent validation
PartnerIdentifier offRampPartner = null;
- if (entity.getOffRampPartnerId() != null && entity.getOffRampPartnerName() != null) {
+ boolean hasPartnerId = entity.getOffRampPartnerId() != null;
+ boolean hasPartnerName = entity.getOffRampPartnerName() != null;
+ if (hasPartnerId && hasPartnerName) {
offRampPartner = new PartnerIdentifier(entity.getOffRampPartnerId(), entity.getOffRampPartnerName());
+ } else if (hasPartnerId || hasPartnerName) {
+ throw new IllegalStateException(
+ "Corrupted payout_orders row: partial partner columns — partnerId=%s, partnerName=%s"
+ .formatted(hasPartnerId, hasPartnerName));
}📝 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
| PartnerIdentifier offRampPartner = null; | |
| if (entity.getOffRampPartnerId() != null && entity.getOffRampPartnerName() != null) { | |
| offRampPartner = new PartnerIdentifier(entity.getOffRampPartnerId(), entity.getOffRampPartnerName()); | |
| } | |
| PartnerIdentifier offRampPartner = null; | |
| boolean hasPartnerId = entity.getOffRampPartnerId() != null; | |
| boolean hasPartnerName = entity.getOffRampPartnerName() != null; | |
| if (hasPartnerId && hasPartnerName) { | |
| offRampPartner = new PartnerIdentifier(entity.getOffRampPartnerId(), entity.getOffRampPartnerName()); | |
| } else if (hasPartnerId || hasPartnerName) { | |
| throw new IllegalStateException( | |
| "Corrupted payout_orders row: partial partner columns — partnerId=%s, partnerName=%s" | |
| .formatted(hasPartnerId, hasPartnerName)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/persistence/mapper/PayoutOrderPersistenceMapper.java`
around lines 62 - 65, The mapping for PartnerIdentifier in
PayoutOrderPersistenceMapper currently returns null when only one of
entity.getOffRampPartnerId() or entity.getOffRampPartnerName() is present, which
is inconsistent with mapBankAccount and mapMobileMoneyAccount; change the logic
to validate partial data and throw an exception (or IllegalStateException) when
exactly one of offRampPartnerId/offRampPartnerName is non-null, and only
construct new PartnerIdentifier(entity.getOffRampPartnerId(),
entity.getOffRampPartnerName()) when both are non-null, mirroring the
partial-data checks used in mapBankAccount/mapMobileMoneyAccount.
Comment on lines
+75
to
+80
| return new RedemptionResult( | ||
| payoutData.id(), | ||
| new BigDecimal(payoutData.amount().amount()), | ||
| payoutData.amount().currency(), | ||
| Instant.now() | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider injecting Clock for testability.
Using Instant.now() directly couples to system time. FallbackAdaptersConfig already provides a Clock bean. Injecting it here would allow deterministic testing of redeemedAt.
♻️ Proposed Clock injection
public class CircleRedemptionAdapter implements RedemptionGateway {
private final RestClient restClient;
private final CircleProperties properties;
+ private final Clock clock;
- public CircleRedemptionAdapter(CircleProperties properties) {
+ public CircleRedemptionAdapter(CircleProperties properties, Clock clock) {
this.properties = properties;
+ this.clock = clock;
// ... rest of constructor
}
// In redeem method:
return new RedemptionResult(
payoutData.id(),
new BigDecimal(payoutData.amount().amount()),
payoutData.amount().currency(),
- Instant.now()
+ clock.instant()
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`
around lines 75 - 80, The RedemptionResult uses Instant.now() which hinders
testability; modify CircleRedemptionAdapter to accept a Clock (inject via
constructor and store as a field), replace Instant.now() with clock.instant()
when building redeemedAt in the method that returns the RedemptionResult, and
update any callers/bean configuration to provide the existing
FallbackAdaptersConfig Clock bean so tests can supply a fixed Clock.
Comment on lines
+83
to
+88
| @SuppressWarnings("unused") | ||
| private RedemptionResult redeemFallback(RedemptionRequest request, Exception ex) { | ||
| log.error("[CIRCLE] Circuit breaker open — redemption failed payoutId={}", | ||
| request.payoutId(), ex); | ||
| throw new IllegalStateException("Circle redemption unavailable", ex); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fallback re-throws, defeating graceful degradation.
The circuit breaker fallback logs and throws IllegalStateException. This propagates the failure rather than providing a degraded response. If the intent is fail-fast, consider removing the fallback annotation altogether. If graceful degradation is desired, return a pending/queued result instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`
around lines 83 - 88, The redeemFallback method currently logs and re-throws an
IllegalStateException which defeats graceful degradation; update
redeemFallback(RedemptionRequest request, Exception ex) to instead return a
degraded RedemptionResult (e.g., status PENDING or QUEUED) and include
identifying info from request (payoutId, any request id) so callers can track
it, or if you truly want fail-fast remove the Hystrix/CircuitBreaker fallback
usage entirely; locate the redeemFallback method and either (A) replace the
throw new IllegalStateException(...) with a constructed RedemptionResult
representing a pending/queued state (populate payoutId, status, and an
informative message) or (B) remove the fallback annotation so the exception
propagates.
Comment on lines
+120
to
+125
| redemption: | ||
| circle: | ||
| base-url: ${CIRCLE_BASE_URL:https://api-sandbox.circle.com} | ||
| api-key: ${CIRCLE_API_KEY:SAND_API_KEY_DEFAULT} | ||
| destination-id: ${CIRCLE_DESTINATION_ID:default-wire-id} | ||
| timeout-seconds: 10 |
There was a problem hiding this comment.
Consider removing the default API key placeholder.
The SAND_API_KEY_DEFAULT fallback could mask misconfiguration in non-dev environments. Prefer failing fast when CIRCLE_API_KEY is unset:
api-key: ${CIRCLE_API_KEY}Alternatively, add startup validation in CircleProperties using @Validated with @NotBlank to ensure required secrets are present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fiat-off-ramp/fiat-off-ramp/src/main/resources/application.yml` around lines
120 - 125, Current YAML provides a default API key placeholder for
redemption.circle.api-key which can mask misconfiguration; either remove the
fallback by changing the property to use ${CIRCLE_API_KEY} with no default, or
add startup validation in your configuration binding: annotate the
CircleProperties class (the `@ConfigurationProperties` bean that maps
redemption.circle.*) with `@Validated` and mark the apiKey field with `@NotBlank` so
the application fails fast on missing secrets (ensure the bean is registered as
a configuration properties bean so validation runs).
Comment on lines
+4
to
+17
| -- PayoutOrder domain model has BankAccount and MobileMoneyAccount | ||
| -- value objects that need to be flattened into the payout_orders table. | ||
| -- At least one of bank or mobile money must be set (application-level invariant). | ||
|
|
||
| -- BankAccount VO columns | ||
| ALTER TABLE payout_orders ADD COLUMN bank_account_number VARCHAR(50); | ||
| ALTER TABLE payout_orders ADD COLUMN bank_code VARCHAR(50); | ||
| ALTER TABLE payout_orders ADD COLUMN bank_account_type VARCHAR(20); | ||
| ALTER TABLE payout_orders ADD COLUMN bank_country VARCHAR(2); | ||
|
|
||
| -- MobileMoneyAccount VO columns | ||
| ALTER TABLE payout_orders ADD COLUMN mobile_money_provider VARCHAR(30); | ||
| ALTER TABLE payout_orders ADD COLUMN mobile_money_phone VARCHAR(30); | ||
| ALTER TABLE payout_orders ADD COLUMN mobile_money_country VARCHAR(2); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Application-level invariant not enforced at database level.
The comment states "at least one of bank or mobile money must be set" but no CHECK constraint enforces this. Consider adding a database constraint to prevent invalid data if application bugs bypass validation.
Optional CHECK constraint
-- Add after the column definitions
ALTER TABLE payout_orders ADD CONSTRAINT chk_payout_account_required
CHECK (
(bank_account_number IS NOT NULL AND bank_code IS NOT NULL)
OR (mobile_money_provider IS NOT NULL AND mobile_money_phone IS NOT NULL)
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/resources/db/migration/V2__add_bank_and_mobile_money_columns.sql`
around lines 4 - 17, The migration adds payout_orders columns for BankAccount
and MobileMoneyAccount but does not enforce the stated invariant "at least one
of bank or mobile money must be set"; add a table-level CHECK constraint (e.g.,
chk_payout_account_required) on payout_orders after the column additions that
requires either both bank_account_number and bank_code to be non-NULL or both
mobile_money_provider and mobile_money_phone to be non-NULL so the database
prevents records missing both account types; include the constraint name
chk_payout_account_required and reference the columns bank_account_number,
bank_code, mobile_money_provider, and mobile_money_phone when adding it.
Comment on lines
+108
to
+110
| assertThatThrownBy(() -> adapter.redeem(aRedemptionRequest())) | ||
| .isInstanceOf(Exception.class); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Assert specific exception type for more meaningful test coverage.
Asserting .isInstanceOf(Exception.class) catches any exception. Spring's RestClient throws specific exceptions (HttpClientErrorException.BadRequest) that would make the test more precise.
As per coding guidelines: "Assert on meaningful values — avoid assertTrue(result != null)"
♻️ Proposed specific assertion
assertThatThrownBy(() -> adapter.redeem(aRedemptionRequest()))
- .isInstanceOf(Exception.class);
+ .isInstanceOf(org.springframework.web.client.HttpClientErrorException.BadRequest.class);Same applies to redeem_unauthorized (line 127), redeem_timeout (line 156), and redeem_serverError (line 215).
📝 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
| assertThatThrownBy(() -> adapter.redeem(aRedemptionRequest())) | |
| .isInstanceOf(Exception.class); | |
| } | |
| assertThatThrownBy(() -> adapter.redeem(aRedemptionRequest())) | |
| .isInstanceOf(org.springframework.web.client.HttpClientErrorException.BadRequest.class); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterTest.java`
around lines 108 - 110, The tests in CircleRedemptionAdapterTest currently
assert any Exception from adapter.redeem(aRedemptionRequest()), which is too
broad; update each test to assert the specific exception type thrown by Spring's
RestClient for that scenario (e.g., replace isInstanceOf(Exception.class) in the
redeem test with HttpClientErrorException.BadRequest, in redeem_unauthorized use
HttpClientErrorException.Unauthorized, in redeem_timeout assert
ResourceAccessException (or the specific timeout exception your RestTemplate
throws), and in redeem_serverError assert
HttpServerErrorException.InternalServerError) so each test asserts the precise
exception type from adapter.redeem.
This was referenced Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CircleRedemptionAdapterimplementsRedemptionGatewayport — calls Circle Business Account Payouts API (POST /v1/businessAccount/payouts) with Bearer API key auth,@CircuitBreaker, HTTP/1.1CircleProperties(@ConfigurationProperties), package-private ACL DTOs (CirclePayoutRequest/CirclePayoutResponse)FallbackAdaptersConfigupdated with@ConditionalOnMissingBeandevRedemptionGateway(simulates 0.92 FX rate)Test plan
spotlessCheckcleanCloses STA-151
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores