Solution/bhavik patel - #130
Open
Bhavik237 wants to merge 9 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a Spring Boot wallet transfer service backed by PostgreSQL with Flyway migrations, idempotent transfer processing, and concurrency-safe balance updates, plus integration/unit tests and local dev tooling.
Changes:
- Introduces wallet/transfer/ledger/idempotency domain model with JPA repositories and service layer implementing idempotency + deterministic wallet locking.
- Adds Flyway schema + seed migrations and Spring Boot configuration for app and tests (Testcontainers).
- Adds integration/unit/concurrency tests, plus Docker/Maven/Sonar setup for local runs and CI analysis.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/application.properties | Test Spring config enabling Flyway/JPA validation and quieter logs for containers/Hibernate. |
| src/test/java/com/api/wallet/service/TransferServiceTest.java | Unit test for idempotency race-loser replay path using mocks. |
| src/test/java/com/api/wallet/controller/WalletApiIntegrationTest.java | Integration tests for wallet balance + transfer history endpoints with Testcontainers DB. |
| src/test/java/com/api/wallet/controller/TransferApiIntegrationTest.java | Integration tests for transfers: ledger balance, idempotent replay, validation and failure modes. |
| src/test/java/com/api/wallet/concurrency/ConcurrentTransferTest.java | Multi-threaded integration tests asserting correctness under concurrent transfer requests. |
| src/test/java/com/api/wallet/TestcontainersConfiguration.java | Springs Testcontainers @ServiceConnection PostgreSQL container config. |
| src/main/resources/db/migration/V2__seed_wallets.sql | Seeds initial wallets. |
| src/main/resources/db/migration/V1__init_schema.sql | Creates schema for wallets, transfers, ledger entries, and idempotency records. |
| src/main/resources/application.properties | App datasource/Flyway/JPA configuration. |
| src/main/java/com/api/wallet/service/WalletService.java | Read-only wallet balance lookup service. |
| src/main/java/com/api/wallet/service/TransferService.java | Transfer API service with idempotency replay and race handling. |
| src/main/java/com/api/wallet/service/TransferAttemptExecutor.java | Transactional transfer execution with wallet row locks and ledger writes. |
| src/main/java/com/api/wallet/repository/WalletRepository.java | Wallet JPA repository with pessimistic locking query. |
| src/main/java/com/api/wallet/repository/TransferRepository.java | Transfer JPA repository with history query. |
| src/main/java/com/api/wallet/repository/LedgerEntryRepository.java | Ledger entry repository with transfer and wallet queries. |
| src/main/java/com/api/wallet/repository/IdempotencyRecordRepository.java | Idempotency record repository. |
| src/main/java/com/api/wallet/exception/WalletNotFoundException.java | Custom exception for missing wallets. |
| src/main/java/com/api/wallet/exception/GlobalExceptionHandler.java | Maps common exceptions to structured error responses. |
| src/main/java/com/api/wallet/dto/WalletResponse.java | Wallet DTO mapping. |
| src/main/java/com/api/wallet/dto/TransferResponse.java | Transfer response DTO mapping. |
| src/main/java/com/api/wallet/dto/ErrorResponse.java | Standard error response DTO. |
| src/main/java/com/api/wallet/dto/CreateTransferRequest.java | Validated transfer request DTO. |
| src/main/java/com/api/wallet/domain/Wallet.java | Wallet entity with balance mutation helpers. |
| src/main/java/com/api/wallet/domain/TransferStatus.java | Transfer state machine + HTTP mapping. |
| src/main/java/com/api/wallet/domain/Transfer.java | Transfer entity and state transitions. |
| src/main/java/com/api/wallet/domain/LedgerEntry.java | Ledger entry entity and factories. |
| src/main/java/com/api/wallet/domain/IdempotencyRecord.java | Idempotency replay cache entity. |
| src/main/java/com/api/wallet/domain/EntryType.java | Ledger entry type enum. |
| src/main/java/com/api/wallet/controller/WalletController.java | Wallet read endpoints controller. |
| src/main/java/com/api/wallet/controller/TransferController.java | Transfer creation endpoint controller. |
| src/main/java/com/api/wallet/WalletApplication.java | Spring Boot application entry point. |
| sonar-project.properties | SonarQube settings for sources/tests/binaries and coverage paths. |
| pom.xml | Maven build with Spring Boot, JPA, Flyway, Testcontainers, and JaCoCo. |
| mvnw.cmd | Maven wrapper script (Windows). |
| mvnw | Maven wrapper script (Unix). |
| docker-compose.yml | Local Docker Compose for Postgres + the app. |
| doc/design.md | Architecture/design documentation describing schema, flows, and concurrency strategy. |
| Dockerfile | Multi-stage build and runtime image for running the service. |
| .mvn/wrapper/maven-wrapper.properties | Maven wrapper distribution configuration. |
| .gitignore | Ignores build output and IDE metadata. |
| .dockerignore | Excludes build/IDE/git artifacts from Docker build context. |
Suppressed comments (2)
src/test/java/com/api/wallet/concurrency/ConcurrentTransferTest.java:1
ready.await()has no timeout and can hang the test indefinitely if any task fails before counting down. Also,executor.shutdown()withoutawaitTermination(and possiblyshutdownNowon failure) can leave worker threads running when a test fails or times out. Add time-bounded awaits and ensure the executor is terminated reliably to reduce test flakiness and hangs.
src/test/java/com/api/wallet/concurrency/ConcurrentTransferTest.java:1ready.await()has no timeout and can hang the test indefinitely if any task fails before counting down. Also,executor.shutdown()withoutawaitTermination(and possiblyshutdownNowon failure) can leave worker threads running when a test fails or times out. Add time-bounded awaits and ensure the executor is terminated reliably to reduce test flakiness and hangs.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| volumes: | ||
| - wallet-postgres-data:/var/lib/postgresql/data | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "pg_isready -U wallet -d wallet"] |
Comment on lines
+15
to
+16
| public class | ||
| TransferController { |
| - Persist an `idempotency_records` row caching the resulting `TransferResponse` (and HTTP status) for future replays. | ||
| - The transaction commits (or fully rolls back) as a single unit. | ||
| 5. **Concurrent-duplicate race.** If a second request with the same key committed first between step 3's check and step 4's insert, the `saveAndFlush` throws `DataIntegrityViolationException`. `TransferService` catches it, re-reads `idempotency_records` for that key (guaranteed visible now, since Postgres blocks the losing insert until the winning transaction finishes) and replays the winner's response. | ||
| 6. **Response.** The controller returns the transfer's own `status`-derived HTTP code (`201` for `PROCESSED`, `422` for `FAILED`) with the `TransferResponse` body. |
Comment on lines
+8
to
+11
| public boolean canTransit(TransferStatus nextStatus){ | ||
| return switch (this) { | ||
| case PENDING -> nextStatus == PROCESSED || nextStatus== FAILED; | ||
| case FAILED,PROCESSED -> false; |
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
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
Describe your solution briefly.
A Spring Boot service that transfers money between two wallets with four guarantees: idempotent request handling (a retried request never applies twice and always replays the original result), a double-entry ledger (every processed transfer writes exactly one DEBIT and one CREDIT row), correct balance tracking (failed transfers leave both wallets untouched), and safe concurrent execution (pessimistic row locks in a deterministic order, plus a two-layer idempotency guard, prevent overdrafts, lost updates, deadlocks, and double-processing). The schema is four Postgres tables managed by Flyway (wallets, transfers, ledger_entries, idempotency_records); the API is a single POST /transfers write endpoint plus two read endpoints for wallet balance and history.
AI disclosure
Detail how you used AI to help with your submission (including the tools you used, how
you used them and what your prompts were).
Include these points in detail
Schema Design
Describe the tables, constraints, and indexes you introduced.
refer - doc/design.md
Idempotency Strategy
Explain how duplicate requests are handled safely.
Two layers, because an application-level check-then-insert alone has a race window:
Fast path. Before doing anything else, look up idempotency_records by key. If found, deserialize and return the cached response — the transfer is never re-attempted. Cheap, handles the common case (sequential retries).
Race guard. If no cached record exists, the attempt proceeds inside TransferAttemptExecutor.attemptTransfer (@transactional), which creates the Transfer row with status = PENDING and saveAndFlushs it immediately — this is the moment idempotency_key's UNIQUE constraint is actually enforced. If two requests with the same key both pass the fast-path check simultaneously, only one insert can succeed; the other throws DataIntegrityViolationException. The whole attempt transaction for the loser rolls back (no partial wallet mutation persists), and the loser re-reads idempotency_records in a fresh transaction/persistence context (reusing the failed one is unsafe — its state is undefined per the JPA spec after a flush failure) to replay the winner's already-committed response. Postgres blocks the losing insert until the winner's transaction finishes, so that re-read is guaranteed to see the result.
An idempotency_records row is written only after the attempt fully resolves (processed or failed), so it always caches the final outcome, never an intermediate one.
Concurrency Strategy
Explain how you prevent race conditions and double spending.
How to Run
How to Test
./mvnw test runs the full suite:
WalletApiIntegrationTest — wallet read endpoints, including 404s.
TransferApiIntegrationTest — happy path, idempotent replay (identical and differing retry bodies), insufficient funds, invalid-request rejection (no Transfer row created).
TransferServiceTest — unit test simulating a lost idempotency_key insert race via mocked DataIntegrityViolationException.
ConcurrentTransferTest — real Testcontainers Postgres with genuine thread-pool concurrency: 20 simultaneous transfers from one wallet never overdraw or lose an update; 20 simultaneous requests with the same idempotency key produce exactly one Transfer row and one identical response across all callers.
Tradeoffs / Assumptions
Checklist