fix(investment): pre-merge review fixes for script terms, write-off limit, amortisation and concurrent posting

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/domain/InvestmentPurchaseRepository.java

@@ -18,6 +18,19 @@
  */
 package org.apache.fineract.portfolio.investment.domain;
 
+import jakarta.persistence.LockModeType;
+import java.util.Optional;
 import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Lock;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
 
-public interface InvestmentPurchaseRepository extends JpaRepository<InvestmentPurchase, Long> {}
+public interface InvestmentPurchaseRepository extends JpaRepository<InvestmentPurchase, Long> {
+
+    /** Locks the deal row so two postings on one deal (double click, two users) run one after the other. */
+    @Lock(value = LockModeType.PESSIMISTIC_WRITE)
+    @Query("select p from InvestmentPurchase p where p.id = :purchaseId")
+    Optional<InvestmentPurchase> findByIdWithLock(@Param("purchaseId") Long purchaseId);
+
+    boolean existsByScriptId(Long scriptId);
+}

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/domain/InvestmentSaleRepository.java

@@ -18,10 +18,23 @@
  */
 package org.apache.fineract.portfolio.investment.domain;
 
+import jakarta.persistence.LockModeType;
 import java.util.List;
+import java.util.Optional;
 import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Lock;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
 
 public interface InvestmentSaleRepository extends JpaRepository<InvestmentSale, Long> {
 
     List<InvestmentSale> findByPurchaseIdOrderByIdAsc(Long purchaseId);
+
+    @Lock(value = LockModeType.PESSIMISTIC_WRITE)
+    @Query("select s from InvestmentSale s where s.id = :saleId")
+    Optional<InvestmentSale> findByIdWithLock(@Param("saleId") Long saleId);
+
+    /** The deal of a sale, without loading either, so the deal can be locked first. */
+    @Query("select s.purchase.id from InvestmentSale s where s.id = :saleId")
+    Optional<Long> findPurchaseIdBySaleId(@Param("saleId") Long saleId);
 }

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/serialization/InvestmentMasterCommandFromApiJsonDeserializer.java

@@ -178,9 +178,14 @@ public final class InvestmentMasterCommandFromApiJsonDeserializer {
                 v.reset().parameter(ISSUE_DATE).value(issueDate).notNull();
             }
         }
-        if (this.fromApiJsonHelper.parameterExists(MATURITY_DATE, element)) {
+        if (this.fromApiJsonHelper.parameterExists(MATURITY_DATE, element) || (create && debt)) {
             final LocalDate maturityDate = this.fromApiJsonHelper.extractLocalDateNamed(MATURITY_DATE, element);
-            v.reset().parameter(MATURITY_DATE).value(maturityDate).ignoreIfNull().validateDateAfter(issueDate);
+            final DataValidatorBuilder maturity = v.reset().parameter(MATURITY_DATE).value(maturityDate);
+            if (debt) {
+                // Interest, sale, redemption and amortisation all run to the maturity date.
+                maturity.notNull();
+            }
+            maturity.ignoreIfNull().validateDateAfter(issueDate);
         }
 
         validateOptionalString(element, v, COMPANY_NAME, 150);

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentAmortisationService.java

@@ -152,7 +152,7 @@ public class InvestmentAmortisationService {
         final Long purchaseId = command.longValueOfParameterNamed(PURCHASE_ID);
         final List<InvestmentPurchase> targets = new ArrayList<>();
         if (purchaseId != null) {
-            final InvestmentPurchase p = purchase(purchaseId);
+            final InvestmentPurchase p = lockedPurchase(purchaseId);
             if (!eligible(p)) {
                 throw InvestmentSaleQuoter.rule("amortise.not.eligible",
                         "Deal " + p.getDealNo() + " has no premium left to amortise (only active HTM holdings bought above face value)");
@@ -164,9 +164,13 @@ public class InvestmentAmortisationService {
             }
             targets.add(p);
         } else {
-            for (final InvestmentPurchase p : this.purchaseRepository.findAll(Sort.by(Sort.Direction.ASC, "id"))) {
-                if (eligible(p) && fyEnd.equals(nextYearEnd(p))) {
-                    targets.add(p);
+            for (final InvestmentPurchase found : this.purchaseRepository.findAll(Sort.by(Sort.Direction.ASC, "id"))) {
+                if (eligible(found) && fyEnd.equals(nextYearEnd(found))) {
+                    // Lock, then check again: another run may have posted this year meanwhile.
+                    final InvestmentPurchase p = lockedPurchase(found.getId());
+                    if (eligible(p) && fyEnd.equals(nextYearEnd(p))) {
+                        targets.add(p);
+                    }
                 }
             }
             if (targets.isEmpty()) {
@@ -177,7 +181,9 @@ public class InvestmentAmortisationService {
         Long lastId = null;
         for (final InvestmentPurchase p : targets) {
             final InvestmentSale s = post(p, fyEnd);
-            vouchers.add(s.getVoucherNo());
+            if (s.getVoucherNo() != null) {
+                vouchers.add(s.getVoucherNo());
+            }
             lastId = s.getId();
         }
         return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(lastId)
@@ -188,11 +194,13 @@ public class InvestmentAmortisationService {
         final LocalDate start = periodStart(p);
         final BigDecimal amount = share(this.quoter.unamortisedPremium(p), start, fyEnd, p.getMaturityDate());
         final String fy = (fyEnd.getYear() - 1) + "-" + String.format("%02d", fyEnd.getYear() % 100);
-        final CommandProcessingResult posted = this.journalPoster.post(fyEnd, p.getDealNo(),
-                "Premium amortisation FY " + fy + ": deal " + p.getDealNo() + ", " + p.getScript().getScriptCode() + " "
-                        + p.getScript().getScriptDesc() + " (" + ChronoUnit.DAYS.between(start, fyEnd) + " days)",
-                new InvestmentJournalPoster.Lines().debit(this.journalPoster.glId(PREMIUM_AMORTISATION_GL_CODE), amount)
-                        .credit(InvestmentSaleQuoter.premiumGlId(p), amount));
+        // A share that rounds to nothing is still recorded, so the year is done and the next one can follow; it needs no JV.
+        final CommandProcessingResult posted = amount.signum() == 0 ? null
+                : this.journalPoster.post(fyEnd, p.getDealNo(),
+                        "Premium amortisation FY " + fy + ": deal " + p.getDealNo() + ", " + p.getScript().getScriptCode() + " "
+                                + p.getScript().getScriptDesc() + " (" + ChronoUnit.DAYS.between(start, fyEnd) + " days)",
+                        new InvestmentJournalPoster.Lines().debit(this.journalPoster.glId(PREMIUM_AMORTISATION_GL_CODE), amount)
+                                .credit(InvestmentSaleQuoter.premiumGlId(p), amount));
         final InvestmentSale s = new InvestmentSale();
         s.setPurchase(p);
         s.setSaleType(TYPE_AMORTISE);
@@ -212,8 +220,8 @@ public class InvestmentAmortisationService {
         s.setBankGlAccount(p.getCrGlAccount());
         s.setRemarks("Premium amortisation FY " + fy);
         s.setStatus(STATUS_SETTLED);
-        s.setGlTransactionId(posted.getTransactionId());
-        final Object voucherNo = posted.getChanges() == null ? null : posted.getChanges().get("voucherNo");
+        s.setGlTransactionId(posted == null ? null : posted.getTransactionId());
+        final Object voucherNo = posted == null || posted.getChanges() == null ? null : posted.getChanges().get("voucherNo");
         s.setVoucherNo(voucherNo == null ? null : voucherNo.toString());
         s.setSettledOnDate(DateUtils.getBusinessLocalDate());
         return this.saleRepository.saveAndFlush(s);
@@ -231,7 +239,7 @@ public class InvestmentAmortisationService {
 
     private boolean eligible(final InvestmentPurchase p) {
         return STATUS_ACTIVE.equals(p.getStatus()) && p.getCategory() != null && p.getCategory().startsWith(HTM_PREFIX)
-                && this.quoter.unamortisedPremium(p).signum() > 0;
+                && p.getMaturityDate() != null && this.quoter.unamortisedPremium(p).signum() > 0;
     }
 
     /** The first 31 March after the last amortisation (or the settlement) that is still before maturity. */
@@ -269,6 +277,10 @@ public class InvestmentAmortisationService {
                 .postable(postable).build();
     }
 
+    private InvestmentPurchase lockedPurchase(final Long purchaseId) {
+        return this.purchaseRepository.findByIdWithLock(purchaseId).orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
+    }
+
     private InvestmentPurchase purchase(final Long purchaseId) {
         return this.purchaseRepository.findById(purchaseId).orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
     }

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentInterestPostingWritePlatformServiceImpl.java

@@ -72,7 +72,7 @@ public class InvestmentInterestPostingWritePlatformServiceImpl implements Invest
     @Transactional
     public CommandProcessingResult postInterest(final Long purchaseId, final JsonCommand command) {
         this.validator.validateForPost(command.json());
-        final InvestmentPurchase p = this.purchaseRepository.findById(purchaseId)
+        final InvestmentPurchase p = this.purchaseRepository.findByIdWithLock(purchaseId)
                 .orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
         if (!STATUS_ACTIVE.equals(p.getStatus())) {
             throw rule("not.active", "Deal " + p.getDealNo() + " is not an active (settled) holding");

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentJournalPoster.java

@@ -66,6 +66,13 @@ public class InvestmentJournalPoster {
         }
 
         private static void add(final JsonArray lines, final Long glAccountId, final BigDecimal amount) {
+            if (amount != null && amount.signum() < 0) {
+                // A negative line means the deal's figures are inconsistent; dropping it would post an unbalanced entry.
+                throw new org.apache.fineract.infrastructure.core.exception.GeneralPlatformDomainRuleException(
+                        "error.msg.investment.journal.negative.line", "A journal line of " + amount.toPlainString()
+                                + " came out negative, so the entry was not posted; check the deal's book value",
+                        glAccountId);
+            }
             if (amount != null && amount.signum() > 0) {
                 final JsonObject line = new JsonObject();
                 line.addProperty("glAccountId", glAccountId);

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentMasterWritePlatformServiceImpl.java

@@ -50,6 +50,9 @@ import static org.apache.fineract.portfolio.investment.api.InvestmentApiConstant
 import static org.apache.fineract.portfolio.investment.api.InvestmentApiConstants.SLR_TYPE;
 
 import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
 import java.util.function.Consumer;
 import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang3.StringUtils;
@@ -63,6 +66,7 @@ import org.apache.fineract.infrastructure.core.exception.GeneralPlatformDomainRu
 import org.apache.fineract.infrastructure.core.exception.PlatformDataIntegrityException;
 import org.apache.fineract.portfolio.investment.domain.InvestmentParty;
 import org.apache.fineract.portfolio.investment.domain.InvestmentPartyRepository;
+import org.apache.fineract.portfolio.investment.domain.InvestmentPurchaseRepository;
 import org.apache.fineract.portfolio.investment.domain.InvestmentScript;
 import org.apache.fineract.portfolio.investment.domain.InvestmentScriptRepository;
 import org.apache.fineract.portfolio.investment.exception.InvestmentPartyNotFoundException;
@@ -80,6 +84,7 @@ public class InvestmentMasterWritePlatformServiceImpl implements InvestmentMaste
     private final InvestmentMasterCommandFromApiJsonDeserializer validator;
     private final InvestmentPartyRepository partyRepository;
     private final InvestmentScriptRepository scriptRepository;
+    private final InvestmentPurchaseRepository purchaseRepository;
     private final GLAccountRepositoryWrapper glAccountRepository;
     private final JdbcTemplate jdbcTemplate;
 
@@ -199,6 +204,7 @@ public class InvestmentMasterWritePlatformServiceImpl implements InvestmentMaste
             throw new GeneralPlatformDomainRuleException("error.msg.investmentscript.bondtype.immutable",
                     "Bond Type of an existing script cannot be changed", script.getScriptCode());
         }
+        requireTermsUnchangedOnceUsed(script, command);
         applyScript(script, command);
         if (command.parameterExists(ACTIVE)) {
             script.setActive(command.booleanPrimitiveValueOfParameterNamed(ACTIVE));
@@ -208,6 +214,33 @@ public class InvestmentMasterWritePlatformServiceImpl implements InvestmentMaste
                 .withResourceIdAsString(script.getScriptCode()).build();
     }
 
+    /**
+     * Unit value, coupon rate and maturity drive the interest, sale, redemption and amortisation of every deal on the
+     * script, so they are fixed once a deal uses it; a different security is a new script (or a GOI category transfer).
+     */
+    private void requireTermsUnchangedOnceUsed(final InvestmentScript s, final JsonCommand command) {
+        final List<String> changed = new ArrayList<>();
+        if (command.parameterExists(FACE_VALUE_PER_UNIT) && command.bigDecimalValueOfParameterNamed(FACE_VALUE_PER_UNIT) != null
+                && !sameAmount(command.bigDecimalValueOfParameterNamed(FACE_VALUE_PER_UNIT), s.getFaceValuePerUnit())) {
+            changed.add("face value per unit");
+        }
+        if (command.parameterExists(INT_RATE) && !sameAmount(command.bigDecimalValueOfParameterNamed(INT_RATE), s.getIntRate())) {
+            changed.add("interest rate");
+        }
+        if (command.parameterExists(MATURITY_DATE)
+                && !Objects.equals(command.localDateValueOfParameterNamed(MATURITY_DATE), s.getMaturityDate())) {
+            changed.add("maturity date");
+        }
+        if (!changed.isEmpty() && this.purchaseRepository.existsByScriptId(s.getId())) {
+            throw new GeneralPlatformDomainRuleException("error.msg.investmentscript.terms.in.use", "Script " + s.getScriptCode()
+                    + " is already used by deals, so its " + String.join(", ", changed) + " cannot be changed", s.getScriptCode());
+        }
+    }
+
+    private static boolean sameAmount(final BigDecimal a, final BigDecimal b) {
+        return a == null ? b == null : b != null && a.compareTo(b) == 0;
+    }
+
     private void applyScript(final InvestmentScript s, final JsonCommand command) {
         setIfPresent(command, SLR_TYPE, s::setSlrType);
         setIfPresent(command, SCRIPT_DESC, s::setScriptDesc);

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentPurchaseWritePlatformServiceImpl.java

@@ -110,6 +110,11 @@ public class InvestmentPurchaseWritePlatformServiceImpl implements InvestmentPur
             throw rule("script.bondtype.mismatch",
                     "Script " + script.getScriptCode() + " is a " + script.getBondType() + ", not " + bondType);
         }
+        if (org.apache.fineract.portfolio.investment.api.InvestmentApiConstants.DEBT_BOND_TYPES.contains(bondType)
+                && script.getMaturityDate() == null) {
+            throw rule("script.no.maturity",
+                    "Script " + script.getScriptCode() + " has no maturity date; enter it in Script Master before buying");
+        }
         if (!party.isActive()) {
             throw rule("party.inactive", "Party " + party.getPartyCode() + " is inactive");
         }
@@ -160,7 +165,7 @@ public class InvestmentPurchaseWritePlatformServiceImpl implements InvestmentPur
     @Override
     @Transactional
     public CommandProcessingResult deletePurchase(final Long purchaseId) {
-        final InvestmentPurchase p = this.purchaseRepository.findById(purchaseId)
+        final InvestmentPurchase p = this.purchaseRepository.findByIdWithLock(purchaseId)
                 .orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
         if (!STATUS_PENDING_SETTLEMENT.equals(p.getStatus())) {
             throw rule("purchase.settled", "Deal " + p.getDealNo() + " is already settled and posted to GL, so it cannot be dropped");
@@ -182,7 +187,7 @@ public class InvestmentPurchaseWritePlatformServiceImpl implements InvestmentPur
     @Override
     @Transactional
     public CommandProcessingResult settlePurchase(final Long purchaseId, final JsonCommand command) {
-        final InvestmentPurchase p = this.purchaseRepository.findById(purchaseId)
+        final InvestmentPurchase p = this.purchaseRepository.findByIdWithLock(purchaseId)
                 .orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
         if (!STATUS_PENDING_SETTLEMENT.equals(p.getStatus())) {
             throw rule("settle.not.pending", "Deal " + p.getDealNo() + " is not pending settlement");

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentSaleWritePlatformServiceImpl.java

@@ -98,11 +98,13 @@ public class InvestmentSaleWritePlatformServiceImpl implements InvestmentSaleWri
     @Override
     @Transactional
     public CommandProcessingResult settleSale(final Long saleId, final JsonCommand command) {
+        // Lock the deal before the sale, as every other posting on the deal does, so two settlements cannot interleave.
+        final InvestmentPurchase p = purchase(this.saleRepository.findPurchaseIdBySaleId(saleId)
+                .orElseThrow(() -> InvestmentSaleQuoter.rule("sale.not.found", "Investment sale " + saleId + " does not exist")));
         final InvestmentSale pending = sale(saleId);
         if (!STATUS_PENDING_SETTLEMENT.equals(pending.getStatus())) {
             throw InvestmentSaleQuoter.rule("settle.not.pending", "This sale is not pending settlement");
         }
-        final InvestmentPurchase p = pending.getPurchase();
         if (pending.getSettlementDate().isAfter(DateUtils.getBusinessLocalDate())) {
             throw InvestmentSaleQuoter.rule("settle.before.date",
                     "The sale can be settled on or after its settlement date " + pending.getSettlementDate().format(DATE));
@@ -194,13 +196,17 @@ public class InvestmentSaleWritePlatformServiceImpl implements InvestmentSaleWri
         this.validator.validateForWriteOff(command.json());
         final InvestmentPurchase p = purchase(purchaseId);
         final BigDecimal amount = command.bigDecimalValueOfParameterNamed(AMOUNT).setScale(2, RoundingMode.HALF_UP);
-        final BigDecimal book = this.quoter.remainingBook(p);
         if (!STATUS_ACTIVE.equals(p.getStatus())) {
             throw InvestmentSaleQuoter.rule("not.active", "Deal " + p.getDealNo() + " is not an active (settled) holding");
         }
-        if (amount.compareTo(book) >= 0) {
-            throw InvestmentSaleQuoter.rule("writeoff.exceeds", "The write-off must be less than the book value of "
-                    + book.stripTrailingZeros().toPlainString() + "; sell or redeem the security to close it");
+        // The loss comes off the investment GL; premium still in its own GL (100707) is not there to write off, and taking
+        // more would leave the investment GL short when the deal is later sold or redeemed.
+        final BigDecimal limit = p.getPremiumGlAccount() != null ? this.quoter.remainingBook(p).subtract(this.quoter.unamortisedPremium(p))
+                : this.quoter.remainingBook(p);
+        if (amount.compareTo(limit) >= 0) {
+            throw InvestmentSaleQuoter.rule("writeoff.exceeds", "The write-off must be less than "
+                    + limit.stripTrailingZeros().toPlainString() + " (book value less premium still to be amortised); "
+                    + "sell or redeem the security to close it");
         }
         final LocalDate today = DateUtils.getBusinessLocalDate();
         final CommandProcessingResult posted = this.journalPoster.post(today, p.getDealNo(),
@@ -272,11 +278,11 @@ public class InvestmentSaleWritePlatformServiceImpl implements InvestmentSaleWri
     }
 
     private InvestmentPurchase purchase(final Long purchaseId) {
-        return this.purchaseRepository.findById(purchaseId).orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
+        return this.purchaseRepository.findByIdWithLock(purchaseId).orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
     }
 
     private InvestmentSale sale(final Long saleId) {
-        return this.saleRepository.findById(saleId)
+        return this.saleRepository.findByIdWithLock(saleId)
                 .orElseThrow(() -> InvestmentSaleQuoter.rule("sale.not.found", "Investment sale " + saleId + " does not exist"));
     }
 }

fineract-provider/src/main/java/org/apache/fineract/portfolio/investment/service/InvestmentSecurityTransferService.java

@@ -85,7 +85,7 @@ public class InvestmentSecurityTransferService {
     @Transactional
     public CommandProcessingResult transfer(final Long purchaseId, final JsonCommand command) {
         validate(command.json());
-        final InvestmentPurchase p = this.purchaseRepository.findById(purchaseId)
+        final InvestmentPurchase p = this.purchaseRepository.findByIdWithLock(purchaseId)
                 .orElseThrow(() -> new InvestmentPurchaseNotFoundException(purchaseId));
         final Long scriptId = command.longValueOfParameterNamed(SCRIPT_ID);
         final InvestmentScript to = this.scriptRepository.findById(scriptId).orElseThrow(() -> new InvestmentScriptNotFoundException(scriptId));