docs/refactor: resolve all final pass 3 review findings and clean code

Author: tuanhiep

system-design/idempotent-payment-ledger/GATE_CHECKLIST.md

@@ -47,13 +47,13 @@
 
 - [x] Initial failure modes are documented.
 - [x] Ledger imbalance limitation and next slice are documented.
-- [x] Recovery path is implemented for reconciliation.
+- [ ] Recovery path is implemented for reconciliation. (Deferred to transaction reconciliation poller slice)
 - [x] Timeout-after-commit scenario is simulated. (Verified via integration tests in RedisPaymentIntakeIntegrationTest)
 
 ## Security
 
-- [x] Trust boundary for idempotency keys is documented. (Documented in docs/DESIGN_DOC.md)
-- [x] Tenant/auth model is documented. (Documented in docs/DESIGN_DOC.md)
+- [ ] Trust boundary for idempotency keys is documented. (Deferred to authentication & risk-control slice)
+- [ ] Tenant/auth model is documented. (Deferred to authentication & risk-control slice)
 
 ## Engineering Communication
 

system-design/idempotent-payment-ledger/README.md

@@ -29,15 +29,31 @@ The system must make payment intake retry-safe while preserving auditability and
 - Invalid requests must not mutate ledger state.
 - The durable adapter uses PostgreSQL uniqueness and Flyway-managed schema as the production-like correctness boundary.
 
-## Implemented Endpoints
-
-Run:
+## Running the Application
 
+### 1. Start Infrastructure
+Spin up the PostgreSQL and Redis containers:
 ```bash
 docker compose -f system-design/idempotent-payment-ledger/compose.yml up -d
-./mvnw -pl system-design/idempotent-payment-ledger spring-boot:run
 ```
 
+### 2. Run Application
+Choose one of the two active profiles to run the application:
+
+*   **Default Mode (PostgreSQL + In-Memory Cache)**:
+    Uses PostgreSQL for durable ledger entry persistence, but keeps idempotency state in a local JVM-memory store. Ideal for fast local development without external cache dependencies.
+    ```bash
+    ./mvnw -pl system-design/idempotent-payment-ledger spring-boot:run
+    ```
+
+*   **Production-Like Hybrid Mode (PostgreSQL + Redis)**:
+    Uses PostgreSQL for durable ledger entry persistence and Redis for distributed locking (`SETNX`) and idempotency caching. Matches production-scale deployments.
+    ```bash
+    ./mvnw -pl system-design/idempotent-payment-ledger spring-boot:run -Dspring-boot.run.profiles=jpa,redis
+    ```
+
+## Implemented Endpoints
+
 Create a payment:
 
 ```bash
@@ -79,7 +95,7 @@ suite; these tests fail fast rather than falling back to H2.
 
 ## Production Gaps
 
-- The in-memory adapter remains for fast unit-level semantics tests; production-like configuration defaults to Redis for idempotency coordination and JPA/PostgreSQL for ledger durability.
+- The in-memory adapter remains for fast unit-level semantics tests; production-like hybrid configuration uses Redis (jpa,redis profile) for idempotency coordination and JPA/PostgreSQL for ledger durability.
 - No transactional outbox exists yet.
 - No auth or tenant model exist yet.
 - Observability features domain metrics for accepted, replayed, and rejected requests, but does not yet emit structured tracing spans.

system-design/idempotent-payment-ledger/compose.yml

@@ -13,3 +13,14 @@ services:
       interval: 5s
       timeout: 3s
       retries: 20
+
+  redis:
+    image: redis:7-alpine
+    container_name: idempotent-payment-ledger-redis
+    ports:
+      - "6379:6379"
+    healthcheck:
+      test: ["CMD", "redis-cli", "ping"]
+      interval: 5s
+      timeout: 3s
+      retries: 20

system-design/idempotent-payment-ledger/docs/ARCHITECT_NOTES.md

@@ -19,7 +19,7 @@ This document outlines the key architectural decisions, design trade-offs, and p
 ## 2. Concurrency Control & Database-Level Defense-in-Depth
 
 ### 2.1. Uniqueness Guarantee (V3 Migration)
-To guarantee absolute data integrity under concurrent attempts, a database-level unique constraint `uq_payments_key UNIQUE (tenant_id, idempotency_key)` is enforced on the `payments` table. This serves as the ultimate safety net if the distributed cache layer (Redis) fails or experiences key eviction.
+To enforce transaction consistency under concurrent attempts, a database-level unique constraint `uq_payments_key UNIQUE (tenant_id, idempotency_key)` is enforced on the `payments` table. This serves as the safety net if the distributed cache layer (Redis) fails or experiences key eviction.
 
 ### 2.2. Concurrency Race Recovery (Look-and-Replay)
 When concurrent requests with the same idempotency key bypass the cache layer (e.g., during lock thrashing or cache eviction) and execute the write path simultaneously:
@@ -46,4 +46,4 @@ If the database transaction commits successfully but the subsequent cache comple
 Applying the V3 unique constraint to an existing production database with historical transaction data requires pre-flight audits to identify and resolve any duplicate keys. Refer to the [Operations Runbook](OPERATIONS_RUNBOOK.md) for details on pre-migration cleanup queries.
 
 ### 4.2. Load Testing & Capacity Estimation
-While the core concurrency invariants are mathematically verified, future work should include running synthetic load simulations (e.g., 5,000+ concurrent requests) to benchmark Redis lock performance and compute production RAM allocation requirements based on anticipated throughput.
+While the core concurrency invariants are functionally verified under concurrent thread simulations, future work should include running synthetic load simulations (e.g., 5,000+ concurrent requests) to benchmark Redis lock performance and compute production RAM allocation requirements based on anticipated throughput.

system-design/idempotent-payment-ledger/docs/DESIGN_DOC.md

@@ -62,14 +62,14 @@ This module uses a pragmatic ports-and-adapters layout. The payment intake use c
 flowchart TD
     client[Client] --> api[PaymentController]
     api --> service[PaymentIntakeService]
-    service --> idem[IdempotencyStore]
-    idem -->|existing same payload| replay[Replay stored response]
-    idem -->|existing different payload| conflict[409 Conflict]
-    idem -->|new reservation| ledger[LedgerStore]
-    ledger --> infra[InMemoryLedgerStore]
-    infra --> debit[Debit payer account]
-    infra --> credit[Credit merchant account]
-    credit --> complete[Complete idempotency reservation]
+    service --> lock[IdempotencyStore / Redis Lock]
+    lock -->|existing same payload| replay[Replay stored response]
+    lock -->|existing different payload| conflict[409 Conflict]
+    lock -->|new reservation| ledger[LedgerStore]
+    ledger --> jpa[JpaLedgerStore / PostgreSQL]
+    jpa --> debit[Debit payer account]
+    jpa --> credit[Credit merchant account]
+    credit --> complete[Complete idempotency reservation / Redis Unlock]
     complete --> response[Return accepted response]
 ```
 

system-design/idempotent-payment-ledger/docs/OPERATIONS_RUNBOOK.md

@@ -15,24 +15,24 @@ The system runs on a hybrid database architecture:
 
 ### 1. PostgreSQL Database Outage (P0)
 
-*   **Symptoms**: 
+*   **Symptoms**:
     *   API returns HTTP `500 Internal Server Error` with `CannotGetJdbcConnectionException` or `HikariPool-1 - Connection is not available`.
     *   Micrometer metric `payment.intake.requests{status="failed"}` spikes.
 *   **Impact**: Core payment intake is entirely blocked. The application cannot write payments or mutate ledger balances.
 *   **Runbook Actions**:
     1.  Verify DB container/host status: `docker ps` or verify AWS RDS instance status.
     2.  If connection pool exhaustion is suspected, inspect active connections:
         ```sql
-        SELECT pid, age(clock_timestamp(), query_start), usename, state, query 
-        FROM pg_stat_activity 
+        SELECT pid, age(clock_timestamp(), query_start), usename, state, query
+        FROM pg_stat_activity
         WHERE state != 'idle' AND query NOT LIKE '%pg_stat_activity%';
         ```
     3.  Restart database or scale connection pool size if required by adjusting `spring.datasource.hikari.maximum-pool-size` configuration.
     4.  Note: Redis keys marked `PROCESSING` will naturally expire after 120 seconds. Clients attempting retry during DB outage will receive HTTP `425 Too Early` or HTTP `500`, then automatically transition to clean retries once Postgres recovers.
 
 ### 2. Redis Cluster Outage / Cache Eviction (P1)
 
-*   **Symptoms**: 
+*   **Symptoms**:
     *   API returns HTTP `500` or fails to connect to Redis (`RedisConnectionFailureException`).
     *   Prometheus alert triggers for Redis memory exhaustion or service down.
 *   **Impact**: External boundary locking is disabled. Concurrency race protection falls back entirely to PostgreSQL unique constraints.
@@ -71,7 +71,7 @@ If a Spring Boot container crashes abruptly *after* reserving a key in Redis but
 *   **Manual Intervention (Emergency Force-Unlock)**:
     If a P0 merchant transaction is blocked and cannot wait 120 seconds:
     1.  Connect to the Redis instance: `redis-cli`.
-    2.  Locate the stuck key safely using `SCAN` to avoid blocking production: 
+    2.  Locate the stuck key safely using `SCAN` to avoid blocking production:
         `SCAN 0 MATCH idempotency:* COUNT 1000`
     3.  Delete the key: `DEL idempotency:<stuck_key>`.
     4.  Ask the client to retry immediately.
@@ -82,8 +82,8 @@ If a Spring Boot container crashes abruptly *after* reserving a key in Redis but
 *   **Cause**: The client reused an existing key but changed parameters (e.g. payer, amount, currency), which is an API violation.
 *   **Action**: Trace client request parameters in logs. Inspect database payments to see what was originally recorded:
     ```sql
-    SELECT payment_id, payer_account_id, merchant_account_id, amount, currency, status 
-    FROM payments 
+    SELECT payment_id, payer_account_id, merchant_account_id, amount, currency, status
+    FROM payments
     WHERE idempotency_key = 'offending-key';
     ```
     Confirm that the client needs to generate a fresh, unique `Idempotency-Key` for the new transaction.
@@ -110,23 +110,23 @@ HAVING SUM(CASE WHEN entry_type = 'CREDIT' THEN amount ELSE -amount END) != 0.00
 
 Verify that the stored account balance matches the sum of its credit and debit ledger entries, taking into account the initial seeded balances for test accounts (1000.00 for `acct-payer` and `acct-payer-http`).
 ```sql
-SELECT 
+SELECT
     a.account_id,
     a.balance AS current_stored_balance,
     (
-        CASE 
-            WHEN a.account_id IN ('acct-payer', 'acct-payer-http') THEN 1000.0000 
-            ELSE 0.0000 
+        CASE
+            WHEN a.account_id IN ('acct-payer', 'acct-payer-http') THEN 1000.0000
+            ELSE 0.0000
         END
         + COALESCE(SUM(CASE WHEN le.entry_type = 'CREDIT' THEN le.amount ELSE -le.amount END), 0.0000)
     ) AS calculated_ledger_balance
 FROM accounts a
 LEFT JOIN ledger_entries le ON a.account_id = le.account_id
 GROUP BY a.account_id, a.balance
 HAVING a.balance != (
-    CASE 
-        WHEN a.account_id IN ('acct-payer', 'acct-payer-http') THEN 1000.0000 
-        ELSE 0.0000 
+    CASE
+        WHEN a.account_id IN ('acct-payer', 'acct-payer-http') THEN 1000.0000
+        ELSE 0.0000
     END
     + COALESCE(SUM(CASE WHEN le.entry_type = 'CREDIT' THEN le.amount ELSE -le.amount END), 0.0000)
 );
@@ -150,15 +150,17 @@ Before executing the **V3 unique constraint migration** (`V3__add_unique_constra
     GROUP BY tenant_id, idempotency_key
     HAVING COUNT(*) > 1;
     ```
-2.  **Mitigation / Deduplication Action**:
-    If duplicate payments are found, the Flyway migration will fail. You must deduplicate or rename the conflicting keys out-of-band before deploying the V3 application code:
-    ```sql
-    -- Example deduplication script (keeps the oldest payment row per tenant/key)
-    DELETE FROM payments a
-    WHERE a.ctid <> (
-        SELECT min(b.ctid)
-        FROM payments b
-        WHERE a.tenant_id = b.tenant_id 
-          AND a.idempotency_key = b.idempotency_key
-    );
-    ```
+2.  **Mitigation & Incident Response Protocol**:
+    If duplicate combinations are detected, applying the `V3` unique constraint will fail, halting the release pipeline. **DO NOT attempt to delete transaction history directly from the database.** Deleting historical rows will violate audit logs and database foreign key integrity (e.g., from ledger entries).
+
+    Instead, execute the following Incident Response protocol:
+    - **Halt Rollout**: Immediately abort the database migration and notify the on-call and release manager teams.
+    - **Analyze & Locate Winner**: Identify the duplicate entries and determine which database row is the canonical payment (e.g., matching the success status returned to the customer or payment gateway).
+    - **Rekey Conflicting Entries**: For the duplicates that are non-canonical, update the `idempotency_key` column to append a suffix (e.g., `_duplicate_v3_migration_incident_<incident_id>`). This preserves audit logs, honors foreign keys, and satisfies the physical uniqueness constraint:
+      ```sql
+      -- Example suffix rekeying query (safe; preserves historical data for audits)
+      UPDATE payments
+      SET idempotency_key = idempotency_key || '_dup_incident_12345'
+      WHERE payment_id = '<non_canonical_payment_id>';
+      ```
+    - **Audit & Reconcile**: Verify that the associated ledger transactions and entries remain balanced. Execute compensating accounting entries if any discrepancy is found.

system-design/idempotent-payment-ledger/src/main/java/infra/systemdesign/paymentledger/application/PaymentIntakeService.java

@@ -12,7 +12,6 @@
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.time.Clock;
 import java.util.HexFormat;
 
 
@@ -21,17 +20,14 @@ public class PaymentIntakeService {
 
     private final IdempotencyStore idempotencyStore;
     private final LedgerStore ledgerStore;
-    private final Clock clock;
     private final MeterRegistry meterRegistry;
 
     public PaymentIntakeService(
             IdempotencyStore idempotencyStore,
             LedgerStore ledgerStore,
-            Clock clock,
             MeterRegistry meterRegistry) {
         this.idempotencyStore = idempotencyStore;
         this.ledgerStore = ledgerStore;
-        this.clock = clock;
         this.meterRegistry = meterRegistry;
     }
 

system-design/idempotent-payment-ledger/src/main/java/infra/systemdesign/paymentledger/infrastructure/persistence/JpaLedgerStore.java

@@ -178,7 +178,7 @@ public int entryCount() {
     @Override
     @Transactional(propagation = Propagation.REQUIRED, readOnly = true)
     public BigDecimal getAccountBalance(String accountId) {
-        return accountRepository.findReadOnlyByTenantIdAndAccountId(TENANT_ID, accountId)
+        return accountRepository.findAccountByTenantIdAndAccountId(TENANT_ID, accountId)
                 .map(AccountEntity::getBalance)
                 .orElse(BigDecimal.ZERO);
     }

system-design/idempotent-payment-ledger/src/main/java/infra/systemdesign/paymentledger/infrastructure/persistence/repository/AccountJpaRepository.java

@@ -11,5 +11,8 @@ public interface AccountJpaRepository extends JpaRepository<AccountEntity, Strin
     @Lock(LockModeType.PESSIMISTIC_WRITE)
     Optional<AccountEntity> findByTenantIdAndAccountId(String tenantId, String accountId);
 
-    Optional<AccountEntity> findReadOnlyByTenantIdAndAccountId(String tenantId, String accountId);
+    @org.springframework.data.jpa.repository.Query("SELECT a FROM AccountEntity a WHERE a.tenantId = :tenantId AND a.accountId = :accountId")
+    Optional<AccountEntity> findAccountByTenantIdAndAccountId(
+            @org.springframework.data.repository.query.Param("tenantId") String tenantId,
+            @org.springframework.data.repository.query.Param("accountId") String accountId);
 }

system-design/idempotent-payment-ledger/src/main/resources/db/migration/V4__cleanup_test_accounts.sql

@@ -0,0 +1,7 @@
+-- =============================================================================
+-- Idempotent Payment Ledger - PostgreSQL Migration (V4)
+-- Clean up historical test accounts seeded in production migration V2
+-- =============================================================================
+
+DELETE FROM accounts 
+WHERE account_id IN ('acct-payer', 'acct-merchant', 'acct-payer-http', 'acct-merchant-http');