feat(s4): infrastructure persistence — JPA entities, repos, adapters (STA-134)#124
Merged
Puneethkumarck merged 2 commits intoMar 8, 2026
Merged
Conversation
Puneethkumarck
added
enhancement
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (24)
WalkthroughAdds a complete JPA-based persistence layer for custody domain types: entities, Spring Data repositories, MapStruct mappers/updaters, persistence adapters implementing repository ports, and a DB migration making chain_id nullable. Changes
Sequence Diagram(s)mermaid 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: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/ChainTransferPersistenceAdapter.java`:
- Around line 27-35: The save method in ChainTransferPersistenceAdapter should
be made transactional to prevent TOCTOU races: annotate the save(ChainTransfer
transfer) method with Spring's `@Transactional` so the sequence of
jpa.findById(...), updater.updateEntity(...), and jpa.save(...) executes in one
transaction; ensure the annotation is applied to the
ChainTransferPersistenceAdapter.save method (or class) and import
org.springframework.transaction.annotation.Transactional so
updater.updateEntity(existing.get(), transfer) and mapper/jpa save calls run
atomically.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferEntity.java`:
- Around line 21-22: Add a composite DB index for queries filtering by paymentId
and transferType: update the ChainTransferEntity class annotation (the `@Table` on
class ChainTransferEntity) to include an `@Index` for the column pair (payment_id,
transfer_type) or alternatively add a DB migration that creates the composite
index; ensure the index name is unique and matches the entity columns used by
the findByPaymentIdAndTransferType query so the JPA repository uses the
composite index instead of scanning/filtering in memory.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferJpaRepository.java`:
- Line 15: The current repository method findByStatus in
ChainTransferJpaRepository returns an unbounded List which can OOM for
high-volume statuses; update the repository API to support pagination by
changing the signature to return Page<ChainTransferEntity> and accept a Pageable
(e.g., Page<ChainTransferEntity> findByStatus(TransferStatus status, Pageable
pageable)), or alternatively add a `@Query` with a LIMIT parameter for bounded
fetches; ensure callers that perform batch processing are updated to pass a
Pageable (or page/size) and handle Page metadata (totalPages/hasNext)
accordingly.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventEntity.java`:
- Around line 32-36: In TransferLifecycleEventEntity replace the String fields
state and participantType with the appropriate enum types (e.g., TransferState
and ParticipantType), annotate both with `@Enumerated`(EnumType.STRING) and keep
the existing `@Column`(...) annotations; update the field declarations, imports,
and any getters/setters or usages in methods/constructors to use the enum types
so mapping aligns with TransferParticipantEntity and preserves compile-time
safety.
- Around line 29-30: Add a DB index on transfer_id to avoid full-table scans for
the repository method (e.g., findByTransferId). Update your migration scripts to
create an index like idx_transfer_lifecycle_events_transfer_id on the
transfer_lifecycle_events(transfer_id); then run/apply the migration and ensure
the JPA entity TransferLifecycleEventEntity (field transferId) remains
unchanged; also verify any existing repository methods referencing transferId
still work after migration.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventJpaRepository.java`:
- Line 10: The repository method findByTransferId(UUID transferId) returns
events in arbitrary order; update it to return events ordered by the event
timestamp (occurredAt) so lifecycle reconstruction is deterministic. Modify the
TransferLifecycleEventJpaRepository method (e.g., change findByTransferId to
findByTransferIdOrderByOccurredAtAsc or accept a Sort parameter) or add a
JPQL/SQL query with ORDER BY e.occurredAt ASC so TransferLifecycleEventEntity
results are returned chronologically. Ensure the ordering key matches the entity
property name occurredAt.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferParticipantEntity.java`:
- Around line 28-49: Update TransferParticipantEntity to enforce non-null schema
constraints on required fields by adding nullable = false to the JPA column
mappings for transferId, participantType (add nullable = false on the `@Column`
paired with `@Enumerated`), address, amount, and assetCode (and optionally the
primary key participantId). Modify the annotations on the fields transferId,
participantType, address, amount, assetCode (and participantId if desired) so
their `@Column` declarations include nullable = false to ensure the database
schema rejects missing values for these mandatory fields.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletBalanceEntity.java`:
- Around line 49-50: The field lastIndexedBlock in WalletBalanceEntity is a
primitive long that can't represent "not yet indexed"; change its type to the
boxed Long in the WalletBalanceEntity class (field lastIndexedBlock) and update
any associated getter/setter and usages to accept/handle null; also ensure the
JPA mapping (`@Column`(name = "last_indexed_block")) allows nulls (or set a
default if you intend 0 as sentinel) and adjust any code paths that assume a
non-null primitive to null-safe checks.
- Around line 31-38: Add a compound index on (wallet_id, stablecoin) to support
the findByWalletIdAndStablecoin query: update the WalletBalanceEntity mapping or
a DB migration to create an index on wallet_id and stablecoin (either add
`@Table`(indexes = `@Index`(name = "idx_walletid_stablecoin", columnList =
"wallet_id,stablecoin")) to the WalletBalanceEntity class or add a new V2
migration SQL that executes CREATE INDEX idx_walletid_stablecoin ON
wallet_balance (wallet_id, stablecoin)); ensure the index name is unique and the
migration is applied alongside existing unique constraint logic.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletEntity.java`:
- Around line 20-27: Add equals() and hashCode() implementations to WalletEntity
that base equality on the business identifier walletId to ensure consistent
behavior in Sets/Map keys across JPA states; implement equals(Object) to return
true when same instance or when classes match and both walletId are non-null and
equal, and implement hashCode() in a stable way (e.g., using class hash or
walletId when non-null) so transient entities do not break collections. Update
WalletEntity by adding these methods (equals and hashCode) and avoid relying on
Lombok-generated identity defaults.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletJpaRepository.java`:
- Line 1: Move the WalletJpaRepository interface out of the entity package into
a new dedicated repository sub-package to separate concerns: create a new
package (e.g.,
com.stablecoin.payments.custody.infrastructure.persistence.repository), update
the package declaration in the WalletJpaRepository source, move the file there,
and adjust any imports/usages (e.g., places referencing WalletJpaRepository,
component scanning or Spring annotations) so they point to the new package;
leave WalletEntity in the entity package and ensure build and tests still
compile.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/TransferLifecycleEventPersistenceMapper.java`:
- Around line 10-37: The current default implementations in
TransferLifecycleEventPersistenceMapper (methods toEntity and toDomain) bypass
MapStruct code generation; remove these default method bodies and replace them
with abstract mapping method signatures so MapStruct can generate the mappings
between TransferLifecycleEventEntity and TransferLifecycleEvent (and vice
versa). Keep the method names toEntity and toDomain and reference the types
TransferLifecycleEventEntity and TransferLifecycleEvent; if you need custom
handling for builder/constructor mapping, add appropriate MapStruct annotations
(`@Mapper`, `@Mapping`, `@BeanMapping` or `@Builder`) on the interface or methods so
MapStruct knows how to construct TransferLifecycleEventEntity (builder) and
TransferLifecycleEvent (constructor) instead of using manual default
implementations.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/TransferParticipantPersistenceMapper.java`:
- Around line 7-38: The interface TransferParticipantPersistenceMapper currently
contains default implementations for toEntity and toDomain which bypass
MapStruct generation; either (A) convert those methods to abstract (remove the
default implementations and the "default" keyword) so MapStruct will generate
the mapper implementation for TransferParticipantPersistenceMapper mapping
between TransferParticipant and TransferParticipantEntity (optionally add
componentModel if you need Spring integration), or (B) if you intend to keep the
hand-written mapping, remove the `@Mapper` annotation and turn the interface into
a concrete class (e.g., annotate with `@Component`) keeping the existing toEntity
and toDomain implementations; pick one approach and apply it consistently.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletEntityUpdater.java`:
- Around line 11-17: The updateEntity method in WalletEntityUpdater can NPE if
the mapping target entity is null; add a defensive null-check at the start of
WalletEntityUpdater.updateEntity (check the 'entity' parameter) and return early
if entity is null (similar to the existing wallet null-check), so subsequent
calls to entity.setActive and entity.setDeactivatedAt are safe.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/TransferParticipantPersistenceAdapter.java`:
- Line 14: The `@Slf4j` annotation on TransferParticipantPersistenceAdapter is
unused; either remove the annotation or add relevant log statements for
persistence operations (e.g., in methods save, findById, delete or constructor)
so the generated 'log' field is actually used; update
TransferParticipantPersistenceAdapter to remove `@Slf4j` if you don't want
logging, or insert debug/info/error logs in key methods of that class (use
method names like save, findById, delete or the class constructor) to record
operation start, success, and failure.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletPersistenceAdapter.java`:
- Line 18: The `@Slf4j` annotation on the WalletPersistenceAdapter class is
unused; either remove it or add logging statements for persistence operations.
To fix, open the WalletPersistenceAdapter class and either delete the `@Slf4j`
import/annotation or add meaningful log calls (e.g., log.debug/info/error)
inside methods like saveWallet, findWalletById, and deleteWallet to record
entry/exit, key parameters, and error conditions; keep logs concise and
sensitive-data-free and ensure imports are adjusted to match the change.
- Around line 27-35: The save() method in WalletPersistenceAdapter does a
read-then-write (jpa.findById(...) then jpa.save(...)) which can race; annotate
the WalletPersistenceAdapter.save(Wallet) method with `@Transactional` so the
find-and-update path (including updater.updateEntity(existing.get(), wallet) and
subsequent jpa.save(existing.get())) executes in a single transaction, ensuring
atomicity and preventing lost updates or constraint violations; import and use
the platform-appropriate transactional annotation (e.g.,
org.springframework.transaction.annotation.Transactional) on the save method or
the class as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1070b018-9c01-44b7-a1f9-d22314ce3bb4
📒 Files selected for processing (24)
blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/ChainTransferPersistenceAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/TransferLifecycleEventPersistenceAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/TransferParticipantPersistenceAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletBalancePersistenceAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletPersistenceAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferEntity.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferJpaRepository.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventEntity.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventJpaRepository.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferParticipantEntity.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferParticipantJpaRepository.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletBalanceEntity.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletBalanceJpaRepository.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletEntity.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/WalletJpaRepository.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/ChainTransferEntityUpdater.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/ChainTransferPersistenceMapper.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/TransferLifecycleEventPersistenceMapper.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/TransferParticipantPersistenceMapper.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletBalanceEntityUpdater.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletBalancePersistenceMapper.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletEntityUpdater.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletPersistenceMapper.javablockchain-custody/blockchain-custody/src/main/resources/db/migration/V3__fix_chain_id_nullable.sql
Comment on lines
+27
to
+35
| @Override | ||
| public ChainTransfer save(ChainTransfer transfer) { | ||
| var existing = jpa.findById(transfer.transferId()); | ||
| if (existing.isPresent()) { | ||
| updater.updateEntity(existing.get(), transfer); | ||
| return mapper.toDomain(jpa.save(existing.get())); | ||
| } | ||
| return mapper.toDomain(jpa.save(mapper.toEntity(transfer))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if callers of save() are wrapped in `@Transactional`
rg -n 'ChainTransferRepository' --type java -A 3 | head -60Repository: Puneethkumarck/stablebridge-platform
Length of output: 2467
🏁 Script executed:
#!/bin/bash
# Read full ChainTransferPersistenceAdapter to see all annotations
cat -n blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/ChainTransferPersistenceAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2523
🏁 Script executed:
#!/bin/bash
# Find all usages of ChainTransferRepository.save() and callers
rg -n '\.save\(' blockchain-custody/blockchain-custody/src/main/java --type java -B 2 -A 2 | grep -E '(save|Transactional|interface|class)' | head -80Repository: Puneethkumarck/stablebridge-platform
Length of output: 2275
🏁 Script executed:
#!/bin/bash
# Check use case files that might call ChainTransferRepository
fd -e 'java' -path '*use*case*' blockchain-custody/blockchain-custody/src/main/java | head -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Search for files that import or use ChainTransferRepository
rg -l 'ChainTransferRepository' blockchain-custody/blockchain-custody/src/main/java --type java | grep -v PersistenceAdapterRepository: Puneethkumarck/stablebridge-platform
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Search for use case or service files that might use ChainTransferRepository
find blockchain-custody/blockchain-custody/src/main/java -name '*UseCase.java' -o -name '*Service.java' | head -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Look for `@Transactional` annotations on methods that use repositories
rg -n '@Transactional' blockchain-custody/blockchain-custody/src/main/java --type java | head -30Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Broader search for `@Transactional` across all modules
find blockchain-custody -name '*.java' -exec grep -l '@Transactional' {} \; | head -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Search for methods calling repository.save()
rg -n 'repository\.save\(' blockchain-custody --type javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Look for any Spring service or component classes
find blockchain-custody -name '*.java' -exec grep -l '@Service\|@Component' {} \; | head -15Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Check for application.yml/properties to understand Spring config
find blockchain-custody -name 'application*.properties' -o -name 'application*.yml'Repository: Puneethkumarck/stablebridge-platform
Length of output: 254
🏁 Script executed:
#!/bin/bash
# Check application.yml for transaction settings
cat blockchain-custody/blockchain-custody/src/main/resources/application.ymlRepository: Puneethkumarck/stablebridge-platform
Length of output: 3143
🏁 Script executed:
#!/bin/bash
# Look for Spring Data JPA repository base class or implicit `@Transactional`
rg -n 'extends JpaRepository\|extends CrudRepository' blockchain-custody --type javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Check if there are integration tests that show how repositories are used
find blockchain-custody -path '*test*' -name '*.java' | head -10Repository: Puneethkumarck/stablebridge-platform
Length of output: 1289
🏁 Script executed:
#!/bin/bash
# Search more broadly for any method that injects ChainTransferRepository
rg -n 'ChainTransferRepository' blockchain-custody --type java -A 5Repository: Puneethkumarck/stablebridge-platform
Length of output: 3511
🏁 Script executed:
#!/bin/bash
# Check ChainTransferJpaRepository to understand if it has any transaction handling
find blockchain-custody -name 'ChainTransferJpaRepository.java'Repository: Puneethkumarck/stablebridge-platform
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Look for any Kafka event handler or listener that might call the repository
rg -n '@KafkaListener\|@EventListener' blockchain-custody --type java -A 5Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Check if there are any outbox or event publishing patterns that would use the repository
rg -n 'Outbox\|Event' blockchain-custody/blockchain-custody/src/main/java --type java -l | head -15Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Read ChainTransferJpaRepository to see if it extends Spring Data interfaces
cat blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferJpaRepository.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 693
🏁 Script executed:
#!/bin/bash
# Search for all files that mention 'save' and 'ChainTransfer' to find callers
rg -n 'ChainTransfer' blockchain-custody/blockchain-custody/src/main/java --type java | grep -i 'save\|create\|update' | head -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 1833
🏁 Script executed:
#!/bin/bash
# Check if there are domain use cases that would use the repository
find blockchain-custody -type f -name '*.java' -path '*/domain/*' | xargs grep -l 'ChainTransferRepository' 2>/dev/nullRepository: Puneethkumarck/stablebridge-platform
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Broader search for any file containing 'save' method calls on repositories
rg -n '\.save\(' blockchain-custody/blockchain-custody/src --type java -B 3 | grep -E '(class|interface|public|private|save\()' | head -40Repository: Puneethkumarck/stablebridge-platform
Length of output: 2275
🏁 Script executed:
#!/bin/bash
# Final check: look at WalletPersistenceAdapter to see if it has `@Transactional` since it uses same pattern
cat blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletPersistenceAdapter.java | head -50Repository: Puneethkumarck/stablebridge-platform
Length of output: 1971
Add @Transactional to save() for atomicity.
The find-then-update-then-save pattern (lines 29–34) is not atomic without an explicit transaction boundary. Each operation runs independently, creating a TOCTOU window for concurrent saves. Spring Data's JpaRepository.save() inherits the caller's transactional context—it does not provide its own.
While no callers currently exist, this pattern appears across adapters (e.g., WalletPersistenceAdapter), pushing transaction responsibility to clients. Simpler to make it safe at the adapter layer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/ChainTransferPersistenceAdapter.java`
around lines 27 - 35, The save method in ChainTransferPersistenceAdapter should
be made transactional to prevent TOCTOU races: annotate the save(ChainTransfer
transfer) method with Spring's `@Transactional` so the sequence of
jpa.findById(...), updater.updateEntity(...), and jpa.save(...) executes in one
transaction; ensure the annotation is applied to the
ChainTransferPersistenceAdapter.save method (or class) and import
org.springframework.transaction.annotation.Transactional so
updater.updateEntity(existing.get(), transfer) and mapper/jpa save calls run
atomically.
Comment on lines
+21
to
+22
| @Entity | ||
| @Table(name = "chain_transfers") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing indexes on chain_transfers table in migration files
rg -n 'CREATE.*INDEX.*chain_transfers' --type sqlRepository: Puneethkumarck/stablebridge-platform
Length of output: 494
🏁 Script executed:
# Check the exact definitions of the indexes in the migration file
rg -A 2 'CREATE.*INDEX.*chain_transfers' blockchain-custody/blockchain-custody/src/main/resources/db/migration/V1__initial_schema.sqlRepository: Puneethkumarck/stablebridge-platform
Length of output: 426
🏁 Script executed:
# Find and examine ChainTransferJpaRepository to see actual query methods
fd -n 'ChainTransferJpaRepository' blockchain-custody/blockchain-custody/srcRepository: Puneethkumarck/stablebridge-platform
Length of output: 310
🏁 Script executed:
# Find ChainTransferJpaRepository with correct syntax
fd 'ChainTransferJpaRepository' blockchain-custody/blockchain-custody/srcRepository: Puneethkumarck/stablebridge-platform
Length of output: 229
🏁 Script executed:
# Also search for any query methods that use transfer_type and payment_id together
rg -A 5 'findByPaymentId' blockchain-custody/blockchain-custody/src --type javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2867
🏁 Script executed:
# Verify transfer_type field exists in ChainTransferEntity
rg -A 2 'transfer_type|transferType' blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferEntity.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 209
Add composite index on (payment_id, transfer_type) for query optimization.
The findByPaymentIdAndTransferType query will table-scan or use index on payment_id alone, then filter by transfer_type in-memory. Add composite index (payment_id, transfer_type) in the migration or via @Index on @Table.
Note: status index already exists in migration, so no action needed there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferEntity.java`
around lines 21 - 22, Add a composite DB index for queries filtering by
paymentId and transferType: update the ChainTransferEntity class annotation (the
`@Table` on class ChainTransferEntity) to include an `@Index` for the column pair
(payment_id, transfer_type) or alternatively add a DB migration that creates the
composite index; ensure the index name is unique and matches the entity columns
used by the findByPaymentIdAndTransferType query so the JPA repository uses the
composite index instead of scanning/filtering in memory.
|
|
||
| Optional<ChainTransferEntity> findByPaymentIdAndTransferType(UUID paymentId, TransferType type); | ||
|
|
||
| List<ChainTransferEntity> findByStatus(TransferStatus status); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pagination for findByStatus.
This returns an unbounded List which could be problematic if many transfers accumulate in a given status (e.g., PENDING or FAILED during outages). If this is used for batch processing, consider Page<ChainTransferEntity> findByStatus(TransferStatus status, Pageable pageable) or at minimum add LIMIT via @Query.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/ChainTransferJpaRepository.java`
at line 15, The current repository method findByStatus in
ChainTransferJpaRepository returns an unbounded List which can OOM for
high-volume statuses; update the repository API to support pagination by
changing the signature to return Page<ChainTransferEntity> and accept a Pageable
(e.g., Page<ChainTransferEntity> findByStatus(TransferStatus status, Pageable
pageable)), or alternatively add a `@Query` with a LIMIT parameter for bounded
fetches; ensure callers that perform batch processing are updated to pass a
Pageable (or page/size) and handle Page metadata (totalPages/hasNext)
accordingly.
Comment on lines
+29
to
+30
| @Column(name = "transfer_id") | ||
| private UUID transferId; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding an index on transfer_id.
The findByTransferId query in the repository will scan without an index. For an append-only event log, this column will be queried frequently and the table will grow unbounded.
Add a table index in your migration script:
CREATE INDEX idx_transfer_lifecycle_events_transfer_id
ON transfer_lifecycle_events(transfer_id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventEntity.java`
around lines 29 - 30, Add a DB index on transfer_id to avoid full-table scans
for the repository method (e.g., findByTransferId). Update your migration
scripts to create an index like idx_transfer_lifecycle_events_transfer_id on the
transfer_lifecycle_events(transfer_id); then run/apply the migration and ensure
the JPA entity TransferLifecycleEventEntity (field transferId) remains
unchanged; also verify any existing repository methods referencing transferId
still work after migration.
Comment on lines
+32
to
+36
| @Column(name = "state", length = 50) | ||
| private String state; | ||
|
|
||
| @Column(name = "participant_type", length = 10) | ||
| private String participantType; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using enums for state and participantType.
TransferParticipantEntity uses @Enumerated(EnumType.STRING) for participantType. Using String here creates inconsistency and loses compile-time type safety.
If the domain types are enums, prefer:
`@Enumerated`(EnumType.STRING)
`@Column`(name = "state", length = 50)
private TransferState state;
`@Enumerated`(EnumType.STRING)
`@Column`(name = "participant_type", length = 10)
private ParticipantType participantType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/entity/TransferLifecycleEventEntity.java`
around lines 32 - 36, In TransferLifecycleEventEntity replace the String fields
state and participantType with the appropriate enum types (e.g., TransferState
and ParticipantType), annotate both with `@Enumerated`(EnumType.STRING) and keep
the existing `@Column`(...) annotations; update the field declarations, imports,
and any getters/setters or usages in methods/constructors to use the enum types
so mapping aligns with TransferParticipantEntity and preserves compile-time
safety.
Comment on lines
+7
to
+38
| @Mapper | ||
| public interface TransferParticipantPersistenceMapper { | ||
|
|
||
| default TransferParticipantEntity toEntity(TransferParticipant participant) { | ||
| if (participant == null) { | ||
| return null; | ||
| } | ||
| return TransferParticipantEntity.builder() | ||
| .participantId(participant.participantId()) | ||
| .transferId(participant.transferId()) | ||
| .participantType(participant.participantType()) | ||
| .address(participant.address()) | ||
| .walletId(participant.walletId()) | ||
| .amount(participant.amount()) | ||
| .assetCode(participant.assetCode()) | ||
| .build(); | ||
| } | ||
|
|
||
| default TransferParticipant toDomain(TransferParticipantEntity entity) { | ||
| if (entity == null) { | ||
| return null; | ||
| } | ||
| return new TransferParticipant( | ||
| entity.getParticipantId(), | ||
| entity.getTransferId(), | ||
| entity.getParticipantType(), | ||
| entity.getAddress(), | ||
| entity.getWalletId(), | ||
| entity.getAmount(), | ||
| entity.getAssetCode() | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Default methods bypass MapStruct code generation.
The @Mapper annotation is present but both methods are default implementations. This means MapStruct generates an empty implementation class while you maintain the mapping logic manually. Either leverage MapStruct's generation (abstract methods) or remove the annotation and use a plain utility class.
♻️ Option 1: Leverage MapStruct generation
`@Mapper`
public interface TransferParticipantPersistenceMapper {
- default TransferParticipantEntity toEntity(TransferParticipant participant) {
- if (participant == null) {
- return null;
- }
- return TransferParticipantEntity.builder()
- .participantId(participant.participantId())
- .transferId(participant.transferId())
- .participantType(participant.participantType())
- .address(participant.address())
- .walletId(participant.walletId())
- .amount(participant.amount())
- .assetCode(participant.assetCode())
- .build();
- }
+ TransferParticipantEntity toEntity(TransferParticipant participant);
- default TransferParticipant toDomain(TransferParticipantEntity entity) {
- if (entity == null) {
- return null;
- }
- return new TransferParticipant(
- entity.getParticipantId(),
- entity.getTransferId(),
- entity.getParticipantType(),
- entity.getAddress(),
- entity.getWalletId(),
- entity.getAmount(),
- entity.getAssetCode()
- );
- }
+ TransferParticipant toDomain(TransferParticipantEntity entity);
}♻️ Option 2: Remove MapStruct, use plain class
`@Component`
public class TransferParticipantPersistenceMapper {
// Keep current implementations
}📝 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 TransferParticipantPersistenceMapper { | |
| default TransferParticipantEntity toEntity(TransferParticipant participant) { | |
| if (participant == null) { | |
| return null; | |
| } | |
| return TransferParticipantEntity.builder() | |
| .participantId(participant.participantId()) | |
| .transferId(participant.transferId()) | |
| .participantType(participant.participantType()) | |
| .address(participant.address()) | |
| .walletId(participant.walletId()) | |
| .amount(participant.amount()) | |
| .assetCode(participant.assetCode()) | |
| .build(); | |
| } | |
| default TransferParticipant toDomain(TransferParticipantEntity entity) { | |
| if (entity == null) { | |
| return null; | |
| } | |
| return new TransferParticipant( | |
| entity.getParticipantId(), | |
| entity.getTransferId(), | |
| entity.getParticipantType(), | |
| entity.getAddress(), | |
| entity.getWalletId(), | |
| entity.getAmount(), | |
| entity.getAssetCode() | |
| ); | |
| } | |
| `@Mapper` | |
| public interface TransferParticipantPersistenceMapper { | |
| TransferParticipantEntity toEntity(TransferParticipant participant); | |
| TransferParticipant toDomain(TransferParticipantEntity entity); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/TransferParticipantPersistenceMapper.java`
around lines 7 - 38, The interface TransferParticipantPersistenceMapper
currently contains default implementations for toEntity and toDomain which
bypass MapStruct generation; either (A) convert those methods to abstract
(remove the default implementations and the "default" keyword) so MapStruct will
generate the mapper implementation for TransferParticipantPersistenceMapper
mapping between TransferParticipant and TransferParticipantEntity (optionally
add componentModel if you need Spring integration), or (B) if you intend to keep
the hand-written mapping, remove the `@Mapper` annotation and turn the interface
into a concrete class (e.g., annotate with `@Component`) keeping the existing
toEntity and toDomain implementations; pick one approach and apply it
consistently.
Comment on lines
+11
to
+17
| default void updateEntity(@MappingTarget WalletEntity entity, Wallet wallet) { | ||
| if (wallet == null) { | ||
| return; | ||
| } | ||
| entity.setActive(wallet.active()); | ||
| entity.setDeactivatedAt(wallet.deactivatedAt()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add null check for entity parameter to prevent NPE.
While callers should ensure a non-null entity, defensive programming in mappers avoids runtime surprises if wiring changes.
🛡️ Proposed fix
default void updateEntity(`@MappingTarget` WalletEntity entity, Wallet wallet) {
+ if (entity == null || wallet == null) {
+ return;
+ }
- if (wallet == null) {
- return;
- }
entity.setActive(wallet.active());
entity.setDeactivatedAt(wallet.deactivatedAt());
}📝 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
| default void updateEntity(@MappingTarget WalletEntity entity, Wallet wallet) { | |
| if (wallet == null) { | |
| return; | |
| } | |
| entity.setActive(wallet.active()); | |
| entity.setDeactivatedAt(wallet.deactivatedAt()); | |
| } | |
| default void updateEntity(`@MappingTarget` WalletEntity entity, Wallet wallet) { | |
| if (entity == null || wallet == null) { | |
| return; | |
| } | |
| entity.setActive(wallet.active()); | |
| entity.setDeactivatedAt(wallet.deactivatedAt()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/mapper/WalletEntityUpdater.java`
around lines 11 - 17, The updateEntity method in WalletEntityUpdater can NPE if
the mapping target entity is null; add a defensive null-check at the start of
WalletEntityUpdater.updateEntity (check the 'entity' parameter) and return early
if entity is null (similar to the existing wallet null-check), so subsequent
calls to entity.setActive and entity.setDeactivatedAt are safe.
| import java.util.List; | ||
| import java.util.UUID; | ||
|
|
||
| @Slf4j |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused @Slf4j annotation.
Logger is declared but never used. Either add logging for persistence operations or remove the annotation.
♻️ Option A: Remove unused annotation
-@Slf4j
`@Repository`
`@RequiredArgsConstructor`♻️ Option B: Add debug logging
`@Override`
public TransferParticipant save(TransferParticipant participant) {
- return mapper.toDomain(jpa.save(mapper.toEntity(participant)));
+ log.debug("Saving transfer participant: {}", participant.participantId());
+ var saved = mapper.toDomain(jpa.save(mapper.toEntity(participant)));
+ log.debug("Saved transfer participant: {}", saved.participantId());
+ return saved;
}📝 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
| @Slf4j | |
| `@Repository` | |
| `@RequiredArgsConstructor` |
Suggested change
| @Slf4j | |
| `@Override` | |
| public TransferParticipant save(TransferParticipant participant) { | |
| log.debug("Saving transfer participant: {}", participant.participantId()); | |
| var saved = mapper.toDomain(jpa.save(mapper.toEntity(participant))); | |
| log.debug("Saved transfer participant: {}", saved.participantId()); | |
| return saved; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/TransferParticipantPersistenceAdapter.java`
at line 14, The `@Slf4j` annotation on TransferParticipantPersistenceAdapter is
unused; either remove the annotation or add relevant log statements for
persistence operations (e.g., in methods save, findById, delete or constructor)
so the generated 'log' field is actually used; update
TransferParticipantPersistenceAdapter to remove `@Slf4j` if you don't want
logging, or insert debug/info/error logs in key methods of that class (use
method names like save, findById, delete or the class constructor) to record
operation start, success, and failure.
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| @Slf4j |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused @Slf4j or add logging.
The annotation is present but no log.* calls exist. Either add debug/trace logging for persistence operations or remove the annotation to reduce noise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletPersistenceAdapter.java`
at line 18, The `@Slf4j` annotation on the WalletPersistenceAdapter class is
unused; either remove it or add logging statements for persistence operations.
To fix, open the WalletPersistenceAdapter class and either delete the `@Slf4j`
import/annotation or add meaningful log calls (e.g., log.debug/info/error)
inside methods like saveWallet, findWalletById, and deleteWallet to record
entry/exit, key parameters, and error conditions; keep logs concise and
sensitive-data-free and ensure imports are adjusted to match the change.
Comment on lines
+27
to
+35
| @Override | ||
| public Wallet save(Wallet wallet) { | ||
| var existing = jpa.findById(wallet.walletId()); | ||
| if (existing.isPresent()) { | ||
| updater.updateEntity(existing.get(), wallet); | ||
| return mapper.toDomain(jpa.save(existing.get())); | ||
| } | ||
| return mapper.toDomain(jpa.save(mapper.toEntity(wallet))); | ||
| } |
There was a problem hiding this comment.
Add @Transactional to ensure atomic find-then-update semantics.
The save() method performs a read followed by a write. Without an explicit transaction boundary, concurrent calls for the same walletId may race, leading to constraint violations or lost updates. Wrapping in @Transactional ensures the entire operation is atomic.
🔒 Proposed fix
+import org.springframework.transaction.annotation.Transactional;
...
+ `@Transactional`
`@Override`
public Wallet save(Wallet wallet) {
var existing = jpa.findById(wallet.walletId());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/persistence/WalletPersistenceAdapter.java`
around lines 27 - 35, The save() method in WalletPersistenceAdapter does a
read-then-write (jpa.findById(...) then jpa.save(...)) which can race; annotate
the WalletPersistenceAdapter.save(Wallet) method with `@Transactional` so the
find-and-update path (including updater.updateEntity(existing.get(), wallet) and
subsequent jpa.save(existing.get())) executes in a single transaction, ensuring
atomicity and preventing lost updates or constraint violations; import and use
the platform-appropriate transactional annotation (e.g.,
org.springframework.transaction.annotation.Transactional) on the save method or
the class as appropriate.
…(STA-134) Add complete persistence layer for S4 Blockchain & Custody service: - 5 JPA entities: ChainTransferEntity, WalletEntity, WalletBalanceEntity, TransferParticipantEntity, TransferLifecycleEventEntity - 5 JPA repositories with custom query methods (including pessimistic locking for WalletBalance) - 8 MapStruct mappers: persistence mappers for domain<->entity conversion with ChainId/StablecoinTicker VO decomposition, and entity updaters for in-place JPA managed entity updates - 5 persistence adapters implementing domain port interfaces with create-or-update semantics for aggregates and append-only for participants/lifecycle events Closes STA-134 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ChainTransfer starts in PENDING state with chain_id = null; it is set during the selectChain() transition to CHAIN_SELECTED. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Puneethkumarck
force-pushed
the
feature/STA-134-s4-infrastructure-persistence
branch
from
92374a3 to
e7baefb
Compare
This was referenced Mar 8, 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
ChainTransferEntity,WalletEntity,WalletBalanceEntity,TransferParticipantEntity,TransferLifecycleEventEntitywith@Versionon WalletBalance,@Lock(PESSIMISTIC_WRITE)for ForUpdatechain_transfers.chain_idto nullable — PENDING transfers have null chainId untilselectChain()transitionCloses STA-134
Test plan
./gradlew :blockchain-custody:blockchain-custody:compileJava— compilation passes./gradlew :blockchain-custody:blockchain-custody:test— all 249 existing tests pass (241 unit + 8 ArchUnit)./gradlew :blockchain-custody:blockchain-custody:spotlessCheck— formatting clean🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Chores