From b940b5fdffeb9c0f4e9e190eecd772279dc2c105 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 4 Jun 2026 04:48:16 +0000 Subject: [PATCH] Fix RFQ execution-correctness bugs in RFQExecutionSaga - Guard against zero/negative/null notional and price (validation was never enforced; negative amounts previously inflated inventory and credit) - Normalise monetary amounts to 2dp with explicit BigDecimal HALF_UP rounding - Harden Bond/Counterparty deduct/add to reject non-positive amounts so balances can never go negative (defends PATCH endpoints too) - Add InvalidAmountException and clearer ResourceNotFoundException messages - Add RFQExecutionSagaTest covering happy path, insufficient credit/notional, rollback-on-failure, rounding, zero/negative qty, unknown IDs Co-Authored-By: Achal Channarasappa --- .../splitthemonolith/entity/Bond.java | 9 + .../splitthemonolith/entity/Counterparty.java | 9 + .../exception/InvalidAmountException.java | 12 ++ .../exception/ResourceNotFoundException.java | 7 + .../saga/RFQExecutionSaga.java | 44 ++++- .../RFQExecutionSagaTest.java | 179 ++++++++++++++++++ 6 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 monolith/src/main/java/com/javieraviles/splitthemonolith/exception/InvalidAmountException.java create mode 100644 monolith/src/test/java/com/javieraviles/splitthemonolith/RFQExecutionSagaTest.java diff --git a/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Bond.java b/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Bond.java index 9504e16..6e76868 100644 --- a/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Bond.java +++ b/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Bond.java @@ -11,6 +11,7 @@ import javax.validation.constraints.PositiveOrZero; import com.javieraviles.splitthemonolith.exception.InsufficientNotionalException; +import com.javieraviles.splitthemonolith.exception.InvalidAmountException; @Entity(name = "bonds") public class Bond { @@ -46,16 +47,24 @@ public Bond(final String isin, final String issuer, final BigDecimal couponRate, } public void addNotional(final BigDecimal amount) { + requirePositive(amount); this.availableNotional = this.availableNotional.add(amount); } public void deductNotional(final BigDecimal amount) { + requirePositive(amount); if (amount.compareTo(this.availableNotional) > 0) { throw new InsufficientNotionalException(); } this.availableNotional = this.availableNotional.subtract(amount); } + private static void requirePositive(final BigDecimal amount) { + if (amount == null || amount.signum() <= 0) { + throw new InvalidAmountException("Notional amount must be positive"); + } + } + public long getId() { return id; } diff --git a/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Counterparty.java b/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Counterparty.java index de7aedc..e832cf2 100644 --- a/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Counterparty.java +++ b/monolith/src/main/java/com/javieraviles/splitthemonolith/entity/Counterparty.java @@ -12,6 +12,7 @@ import javax.validation.constraints.Size; import com.javieraviles.splitthemonolith.exception.InsufficientCreditException; +import com.javieraviles.splitthemonolith.exception.InvalidAmountException; @Entity(name = "counterparties") public class Counterparty { @@ -52,16 +53,24 @@ public Counterparty(final String name, final String lei, final BigDecimal credit } public void addCredit(final BigDecimal amount) { + requirePositive(amount); this.availableCredit = this.availableCredit.add(amount); } public void deductCredit(final BigDecimal amount) { + requirePositive(amount); if (amount.compareTo(this.availableCredit) > 0) { throw new InsufficientCreditException(); } this.availableCredit = this.availableCredit.subtract(amount); } + private static void requirePositive(final BigDecimal amount) { + if (amount == null || amount.signum() <= 0) { + throw new InvalidAmountException("Credit amount must be positive"); + } + } + public long getId() { return id; } diff --git a/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/InvalidAmountException.java b/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/InvalidAmountException.java new file mode 100644 index 0000000..e3ea050 --- /dev/null +++ b/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/InvalidAmountException.java @@ -0,0 +1,12 @@ +package com.javieraviles.splitthemonolith.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(code = HttpStatus.BAD_REQUEST, reason = "Invalid amount") +public class InvalidAmountException extends RuntimeException { + + public InvalidAmountException(final String message) { + super(message); + } +} diff --git a/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/ResourceNotFoundException.java b/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/ResourceNotFoundException.java index 2ae7ac5..701e058 100644 --- a/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/ResourceNotFoundException.java +++ b/monolith/src/main/java/com/javieraviles/splitthemonolith/exception/ResourceNotFoundException.java @@ -6,4 +6,11 @@ @ResponseStatus(code = HttpStatus.NOT_FOUND) public class ResourceNotFoundException extends RuntimeException { + public ResourceNotFoundException() { + super(); + } + + public ResourceNotFoundException(final String message) { + super(message); + } } \ No newline at end of file diff --git a/monolith/src/main/java/com/javieraviles/splitthemonolith/saga/RFQExecutionSaga.java b/monolith/src/main/java/com/javieraviles/splitthemonolith/saga/RFQExecutionSaga.java index 79def7d..ca1f77a 100644 --- a/monolith/src/main/java/com/javieraviles/splitthemonolith/saga/RFQExecutionSaga.java +++ b/monolith/src/main/java/com/javieraviles/splitthemonolith/saga/RFQExecutionSaga.java @@ -1,5 +1,8 @@ package com.javieraviles.splitthemonolith.saga; +import java.math.BigDecimal; +import java.math.RoundingMode; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -9,6 +12,7 @@ import com.javieraviles.splitthemonolith.entity.Counterparty; import com.javieraviles.splitthemonolith.entity.Rfq; import com.javieraviles.splitthemonolith.entity.RfqStatus; +import com.javieraviles.splitthemonolith.exception.InvalidAmountException; import com.javieraviles.splitthemonolith.exception.ResourceNotFoundException; import com.javieraviles.splitthemonolith.repository.BondRepository; import com.javieraviles.splitthemonolith.repository.CounterpartyRepository; @@ -17,6 +21,10 @@ @Component public class RFQExecutionSaga { + /** Monetary values settle to whole cents using a deterministic rounding mode. */ + private static final int MONEY_SCALE = 2; + private static final RoundingMode MONEY_ROUNDING = RoundingMode.HALF_UP; + @Autowired private RfqRepository rfqRepository; @@ -29,20 +37,38 @@ public class RFQExecutionSaga { @Transactional public Rfq executeRfq(final RfqDto rfqDto) { + // Validate and normalise monetary inputs up front so we fail fast with a + // clear domain exception before mutating any inventory or credit. + final BigDecimal notionalAmount = normaliseMoney(rfqDto.getNotionalAmount(), "notionalAmount"); + final BigDecimal executionPrice = normaliseMoney(rfqDto.getExecutionPrice(), "executionPrice"); + final Bond bond = bondRepository.findById(rfqDto.getBondId()) - .orElseThrow(() -> new ResourceNotFoundException()); + .orElseThrow(() -> new ResourceNotFoundException("Bond not found: " + rfqDto.getBondId())); final Counterparty counterparty = counterpartyRepository.findById(rfqDto.getCounterpartyId()) - .orElseThrow(() -> new ResourceNotFoundException()); + .orElseThrow(() -> new ResourceNotFoundException( + "Counterparty not found: " + rfqDto.getCounterpartyId())); - bond.deductNotional(rfqDto.getNotionalAmount()); + bond.deductNotional(notionalAmount); /* - * This is all part of one transaction due to @Transactional annotation. - * No need for saga compensation as credit will only be deducted if the - * bond had sufficient available notional. + * Both deductions run inside the single @Transactional boundary. If the + * credit deduction throws (insufficient credit), the bond notional + * deduction above is rolled back too, so the two updates always commit + * or roll back together. */ - counterparty.deductCredit(rfqDto.getExecutionPrice()); + counterparty.deductCredit(executionPrice); + + return rfqRepository.save(new Rfq(counterparty, bond, notionalAmount, + rfqDto.getSide(), RfqStatus.EXECUTED, executionPrice)); + } - return rfqRepository.save(new Rfq(counterparty, bond, rfqDto.getNotionalAmount(), - rfqDto.getSide(), RfqStatus.EXECUTED, rfqDto.getExecutionPrice())); + private static BigDecimal normaliseMoney(final BigDecimal amount, final String field) { + if (amount == null) { + throw new InvalidAmountException(field + " must be provided"); + } + final BigDecimal normalised = amount.setScale(MONEY_SCALE, MONEY_ROUNDING); + if (normalised.signum() <= 0) { + throw new InvalidAmountException(field + " must be positive"); + } + return normalised; } } diff --git a/monolith/src/test/java/com/javieraviles/splitthemonolith/RFQExecutionSagaTest.java b/monolith/src/test/java/com/javieraviles/splitthemonolith/RFQExecutionSagaTest.java new file mode 100644 index 0000000..f51ce43 --- /dev/null +++ b/monolith/src/test/java/com/javieraviles/splitthemonolith/RFQExecutionSagaTest.java @@ -0,0 +1,179 @@ +package com.javieraviles.splitthemonolith; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.math.BigDecimal; +import java.time.LocalDate; + +import com.javieraviles.splitthemonolith.dto.RfqDto; +import com.javieraviles.splitthemonolith.entity.Bond; +import com.javieraviles.splitthemonolith.entity.Counterparty; +import com.javieraviles.splitthemonolith.entity.Rfq; +import com.javieraviles.splitthemonolith.entity.RfqStatus; +import com.javieraviles.splitthemonolith.entity.Side; +import com.javieraviles.splitthemonolith.exception.InsufficientCreditException; +import com.javieraviles.splitthemonolith.exception.InsufficientNotionalException; +import com.javieraviles.splitthemonolith.exception.InvalidAmountException; +import com.javieraviles.splitthemonolith.exception.ResourceNotFoundException; +import com.javieraviles.splitthemonolith.repository.BondRepository; +import com.javieraviles.splitthemonolith.repository.CounterpartyRepository; +import com.javieraviles.splitthemonolith.saga.RFQExecutionSaga; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; + +/** + * Edge-case coverage for {@link RFQExecutionSaga}. Test methods are intentionally + * non-transactional so the saga runs in its own transaction; re-fetching entities + * afterwards reflects committed (or rolled-back) state. + * + *

Shares the same context configuration as {@code IntegrationTest} so Spring + * reuses a single cached application context and the seed {@code CommandLineRunner} + * runs only once against the shared in-memory database. + */ +@SpringBootTest +@AutoConfigureMockMvc +public class RFQExecutionSagaTest { + + @Autowired + private RFQExecutionSaga saga; + + @Autowired + private BondRepository bondRepository; + + @Autowired + private CounterpartyRepository counterpartyRepository; + + private Bond newBond(final String isin, final BigDecimal availableNotional) { + return bondRepository.save(new Bond(isin, "US Treasury", new BigDecimal("2.5000"), + LocalDate.of(2032, 5, 15), availableNotional)); + } + + private Counterparty newCounterparty(final String lei, final BigDecimal creditLimit) { + return counterpartyRepository.save(new Counterparty("Test Fund", lei, creditLimit)); + } + + private RfqDto rfqDto(final long counterpartyId, final long bondId, + final BigDecimal notional, final BigDecimal price) { + final RfqDto dto = new RfqDto(); + dto.setCounterpartyId(counterpartyId); + dto.setBondId(bondId); + dto.setNotionalAmount(notional); + dto.setExecutionPrice(price); + dto.setSide(Side.BUY); + return dto; + } + + @Test + public void happyPath_deductsNotionalAndCreditAndPersistsRfq() { + final Bond bond = newBond("US0000000H01", new BigDecimal("10000000.00")); + final Counterparty cp = newCounterparty("549300HAPPYPATH0001", new BigDecimal("10000000.00")); + + final Rfq rfq = saga.executeRfq(rfqDto(cp.getId(), bond.getId(), + new BigDecimal("1000000.00"), new BigDecimal("998750.00"))); + + assertEquals(RfqStatus.EXECUTED, rfq.getStatus()); + assertEquals(0, bondRepository.findById(bond.getId()).get().getAvailableNotional() + .compareTo(new BigDecimal("9000000.00"))); + assertEquals(0, counterpartyRepository.findById(cp.getId()).get().getAvailableCredit() + .compareTo(new BigDecimal("9001250.00"))); + } + + @Test + public void insufficientNotional_throwsAndLeavesStateUnchanged() { + final Bond bond = newBond("US0000000N01", new BigDecimal("1000000.00")); + final Counterparty cp = newCounterparty("549300NOTIONAL00001", new BigDecimal("50000000.00")); + + assertThrows(InsufficientNotionalException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), bond.getId(), new BigDecimal("2000000.00"), new BigDecimal("1999000.00")))); + + assertEquals(0, bondRepository.findById(bond.getId()).get().getAvailableNotional() + .compareTo(new BigDecimal("1000000.00"))); + assertEquals(0, counterpartyRepository.findById(cp.getId()).get().getAvailableCredit() + .compareTo(new BigDecimal("50000000.00"))); + } + + @Test + public void insufficientCredit_throws() { + final Bond bond = newBond("US0000000C01", new BigDecimal("50000000.00")); + final Counterparty cp = newCounterparty("549300CREDIT0000001", new BigDecimal("500000.00")); + + assertThrows(InsufficientCreditException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), bond.getId(), new BigDecimal("1000000.00"), new BigDecimal("600000.00")))); + } + + @Test + public void rollbackOnFailure_notionalDeductionRolledBackWhenCreditFails() { + // Bond has enough notional, but the counterparty cannot cover the price. + final Bond bond = newBond("US0000000R01", new BigDecimal("5000000.00")); + final Counterparty cp = newCounterparty("549300ROLLBACK00001", new BigDecimal("100000.00")); + + assertThrows(InsufficientCreditException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), bond.getId(), new BigDecimal("1000000.00"), new BigDecimal("200000.00")))); + + // The notional deduction that ran before the credit failure must be rolled back. + assertEquals(0, bondRepository.findById(bond.getId()).get().getAvailableNotional() + .compareTo(new BigDecimal("5000000.00"))); + assertEquals(0, counterpartyRepository.findById(cp.getId()).get().getAvailableCredit() + .compareTo(new BigDecimal("100000.00"))); + } + + @Test + public void monetaryMath_usesBigDecimalWithExplicitRounding() { + final Bond bond = newBond("US0000000D01", new BigDecimal("1000000.00")); + final Counterparty cp = newCounterparty("549300ROUNDING00001", new BigDecimal("1000000.00")); + + // Sub-cent inputs must be rounded HALF_UP to whole cents before settling. + final Rfq rfq = saga.executeRfq(rfqDto(cp.getId(), bond.getId(), + new BigDecimal("200.005"), new BigDecimal("100.125"))); + + assertEquals(2, rfq.getExecutionPrice().scale()); + assertEquals(0, rfq.getExecutionPrice().compareTo(new BigDecimal("100.13"))); + assertEquals(0, rfq.getNotionalAmount().compareTo(new BigDecimal("200.01"))); + assertEquals(0, counterpartyRepository.findById(cp.getId()).get().getAvailableCredit() + .compareTo(new BigDecimal("999899.87"))); + assertEquals(0, bondRepository.findById(bond.getId()).get().getAvailableNotional() + .compareTo(new BigDecimal("999799.99"))); + } + + @Test + public void zeroQuantity_throwsInvalidAmount() { + final Bond bond = newBond("US0000000Z01", new BigDecimal("1000000.00")); + final Counterparty cp = newCounterparty("549300ZEROQTY000001", new BigDecimal("1000000.00")); + + assertThrows(InvalidAmountException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), bond.getId(), BigDecimal.ZERO, new BigDecimal("100.00")))); + } + + @Test + public void negativeQuantity_throwsInvalidAmountAndDoesNotIncreaseInventory() { + final Bond bond = newBond("US0000000Z02", new BigDecimal("1000000.00")); + final Counterparty cp = newCounterparty("549300NEGQTY0000001", new BigDecimal("1000000.00")); + + assertThrows(InvalidAmountException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), bond.getId(), new BigDecimal("-1000.00"), new BigDecimal("100.00")))); + + // A negative quantity must never inflate available notional. + assertEquals(0, bondRepository.findById(bond.getId()).get().getAvailableNotional() + .compareTo(new BigDecimal("1000000.00"))); + } + + @Test + public void unknownBond_throwsResourceNotFound() { + final Counterparty cp = newCounterparty("549300UNKNOWNBOND01", new BigDecimal("1000000.00")); + + assertThrows(ResourceNotFoundException.class, () -> saga.executeRfq(rfqDto( + cp.getId(), 999999L, new BigDecimal("1000.00"), new BigDecimal("100.00")))); + } + + @Test + public void unknownCounterparty_throwsResourceNotFound() { + final Bond bond = newBond("US0000000U01", new BigDecimal("1000000.00")); + + assertThrows(ResourceNotFoundException.class, () -> saga.executeRfq(rfqDto( + 999999L, bond.getId(), new BigDecimal("1000.00"), new BigDecimal("100.00")))); + } +}