diff --git a/fineract-investor/src/main/java/org/apache/fineract/investor/domain/ExternalAssetOwnerRepository.java b/fineract-investor/src/main/java/org/apache/fineract/investor/domain/ExternalAssetOwnerRepository.java index 90e554247..dacf5455f 100644 --- a/fineract-investor/src/main/java/org/apache/fineract/investor/domain/ExternalAssetOwnerRepository.java +++ b/fineract-investor/src/main/java/org/apache/fineract/investor/domain/ExternalAssetOwnerRepository.java @@ -22,10 +22,14 @@ import java.util.Optional; import org.apache.fineract.infrastructure.core.domain.ExternalId; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; +import org.springframework.data.jpa.repository.Query; public interface ExternalAssetOwnerRepository extends JpaRepository, JpaSpecificationExecutor { Optional findByExternalId(ExternalId externalId); + @Query("SELECT e.id FROM ExternalAssetOwner e WHERE e.externalId = :externalId") + Optional findIdByExternalId(ExternalId externalId); + } diff --git a/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnerHelper.java b/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnerHelper.java new file mode 100644 index 000000000..7558c67f1 --- /dev/null +++ b/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnerHelper.java @@ -0,0 +1,46 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.investor.service; + +import lombok.RequiredArgsConstructor; +import org.apache.fineract.infrastructure.core.domain.ExternalId; +import org.apache.fineract.investor.domain.ExternalAssetOwner; +import org.apache.fineract.investor.domain.ExternalAssetOwnerRepository; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; + +@Service +@RequiredArgsConstructor +public class ExternalAssetOwnerHelper { + + private final ExternalAssetOwnerRepository repository; + + // REQUIRES_NEW isolates the INSERT into a separate transaction and persistence context, + // so a constraint violation does not corrupt the caller's session or mark the + // outer transaction as rollback-only, allowing a safe retry. + @Transactional(propagation = Propagation.REQUIRES_NEW) + public Long findOrCreateId(final ExternalId externalId) { + return repository.findIdByExternalId(externalId).orElseGet(() -> { + final ExternalAssetOwner owner = new ExternalAssetOwner(); + owner.setExternalId(externalId); + return repository.saveAndFlush(owner).getId(); + }); + } +} diff --git a/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceImpl.java b/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceImpl.java index 8fd3873bd..13ee21e47 100644 --- a/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceImpl.java +++ b/fineract-investor/src/main/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceImpl.java @@ -25,6 +25,7 @@ import static org.apache.fineract.investor.data.ExternalTransferStatus.PENDING_I import com.google.gson.JsonElement; import com.google.gson.reflect.TypeToken; import java.lang.reflect.Type; +import java.sql.SQLException; import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; @@ -65,6 +66,9 @@ import org.apache.fineract.investor.serialization.ExternalAssetOwnerValidator; import org.apache.fineract.portfolio.loanaccount.domain.LoanRepository; import org.apache.fineract.portfolio.loanaccount.domain.LoanStatus; import org.apache.fineract.portfolio.loanaccount.exception.LoanNotFoundException; +import org.springframework.dao.DataAccessException; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.orm.jpa.JpaSystemException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -77,6 +81,8 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW ExternalTransferStatus.ACTIVE); private static final List BUYBACK_READY_STATUSES_FOR_DELAY_SETTLEMENT = List .of(ExternalTransferStatus.ACTIVE_INTERMEDIATE, ExternalTransferStatus.ACTIVE); + private static final String SQL_STATE_INTEGRITY_CONSTRAINT_VIOLATION = "23"; + private final ExternalAssetOwnerTransferRepository externalAssetOwnerTransferRepository; private final ExternalAssetOwnerRepository externalAssetOwnerRepository; private final FromJsonHelper fromApiJsonHelper; @@ -85,6 +91,7 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW private final ConfigurationDomainService configurationDomainService; private final ExternalAssetOwnersReadService externalAssetOwnersReadService; private final ExternalAssetOwnerValidator externalAssetOwnerValidator; + private final ExternalAssetOwnerHelper externalAssetOwnerHelper; @Override @Transactional @@ -171,15 +178,16 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW if (effectiveTransfers.size() == 2) { throw new ExternalAssetOwnerInitiateTransferException("This loan cannot be sold, there is already an in progress transfer"); } else if (effectiveTransfers.size() == 1) { - if (PENDING.equals(effectiveTransfers.get(0).getStatus())) { + if (PENDING.equals(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( "External asset owner transfer is already in PENDING state for this loan"); - } else if (ExternalTransferStatus.ACTIVE.equals(effectiveTransfers.get(0).getStatus())) { + } else if (ExternalTransferStatus.ACTIVE.equals(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be sold, because it is owned by an external asset owner"); } else { - throw new ExternalAssetOwnerInitiateTransferException(String.format( - "This loan cannot be sold, because it is incorrect state! (transferId = %s)", effectiveTransfers.get(0).getId())); + throw new ExternalAssetOwnerInitiateTransferException( + String.format("This loan cannot be sold, because it is incorrect state! (transferId = %s)", + effectiveTransfers.getFirst().getId())); } } } @@ -188,7 +196,7 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW if (effectiveTransfers.size() > 1) { throw new ExternalAssetOwnerInitiateTransferException("This loan cannot be sold, there is already an in progress transfer"); } else if (effectiveTransfers.size() == 1) { - if (!ACTIVE_INTERMEDIATE.equals(effectiveTransfers.get(0).getStatus())) { + if (!ACTIVE_INTERMEDIATE.equals(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be sold, because it is not in ACTIVE-INTERMEDIATE state."); } @@ -204,15 +212,16 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW if (effectiveTransfers.size() > 1) { throw new ExternalAssetOwnerInitiateTransferException("This loan cannot be sold, there is already an in progress transfer"); } else if (effectiveTransfers.size() == 1) { - if (PENDING_INTERMEDIATE.equals(effectiveTransfers.get(0).getStatus())) { + if (PENDING_INTERMEDIATE.equals(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( "External asset owner transfer is already in PENDING_INTERMEDIATE state for this loan"); - } else if (ExternalTransferStatus.ACTIVE.equals(effectiveTransfers.get(0).getStatus())) { + } else if (ExternalTransferStatus.ACTIVE.equals(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be sold, because it is owned by an external asset owner"); } else { - throw new ExternalAssetOwnerInitiateTransferException(String.format( - "This loan cannot be sold, because it is incorrect state! (transferId = %s)", effectiveTransfers.get(0).getId())); + throw new ExternalAssetOwnerInitiateTransferException( + String.format("This loan cannot be sold, because it is incorrect state! (transferId = %s)", + effectiveTransfers.getFirst().getId())); } } } @@ -232,17 +241,17 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW } else if (effectiveTransfers.size() == 2) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be bought back, external asset owner buyback transfer is already in progress"); - } else if (!BUYBACK_READY_STATUSES.contains(effectiveTransfers.get(0).getStatus())) { + } else if (!BUYBACK_READY_STATUSES.contains(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( String.format("This loan cannot be bought back, effective transfer is not in right state: %s", - effectiveTransfers.get(0).getStatus())); - } else if (DateUtils.isBefore(settlementDate, effectiveTransfers.get(0).getSettlementDate())) { + effectiveTransfers.getFirst().getStatus())); + } else if (DateUtils.isBefore(settlementDate, effectiveTransfers.getFirst().getSettlementDate())) { throw new ExternalAssetOwnerInitiateTransferException( String.format("This loan cannot be bought back, settlement date is earlier than effective transfer settlement date: %s", - effectiveTransfers.get(0).getSettlementDate())); + effectiveTransfers.getFirst().getSettlementDate())); } - return effectiveTransfers.get(0); + return effectiveTransfers.getFirst(); } private ExternalAssetOwnerTransfer fetchAndValidateEffectiveTransferForBuybackWithDelayedSettlement( @@ -265,17 +274,17 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW || Set.of(ExternalTransferStatus.ACTIVE, ExternalTransferStatus.BUYBACK).equals(effectiveTransferStatuses)) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be bought back, external asset owner buyback transfer is already in progress"); - } else if (!BUYBACK_READY_STATUSES_FOR_DELAY_SETTLEMENT.contains(effectiveTransfers.get(0).getStatus())) { + } else if (!BUYBACK_READY_STATUSES_FOR_DELAY_SETTLEMENT.contains(effectiveTransfers.getFirst().getStatus())) { throw new ExternalAssetOwnerInitiateTransferException( String.format("This loan cannot be bought back, effective transfer is not in right state: %s", - effectiveTransfers.get(0).getStatus())); - } else if (DateUtils.isBefore(settlementDate, effectiveTransfers.get(0).getSettlementDate())) { + effectiveTransfers.getFirst().getStatus())); + } else if (DateUtils.isBefore(settlementDate, effectiveTransfers.getFirst().getSettlementDate())) { throw new ExternalAssetOwnerInitiateTransferException( String.format("This loan cannot be bought back, settlement date is earlier than effective transfer settlement date: %s", - effectiveTransfers.get(0).getSettlementDate())); + effectiveTransfers.getFirst().getSettlementDate())); } - return effectiveTransfers.get(0); + return effectiveTransfers.getFirst(); } private ExternalAssetOwnerTransfer fetchAndValidateEffectiveTransferForCancel(final Long transferId) { @@ -287,10 +296,9 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW .findEffectiveTransfersOrderByIdDesc(selectedTransfer.getLoanId(), DateUtils.getBusinessLocalDate()); if (effective.isEmpty()) { throw new ExternalAssetOwnerInitiateTransferException( - String.format("This loan cannot be cancelled, there is no effective transfer for this loan")); - } else if (!Objects.equals(effective.get(0).getId(), selectedTransfer.getId())) { - throw new ExternalAssetOwnerInitiateTransferException( - String.format("This loan cannot be cancelled, selected transfer is not the latest")); + "This loan cannot be cancelled, there is no effective transfer for this loan"); + } else if (!Objects.equals(effective.getFirst().getId(), selectedTransfer.getId())) { + throw new ExternalAssetOwnerInitiateTransferException("This loan cannot be cancelled, selected transfer is not the latest"); } else if (selectedTransfer.getStatus() != PENDING && selectedTransfer.getStatus() != ExternalTransferStatus.BUYBACK) { throw new ExternalAssetOwnerInitiateTransferException( "This loan cannot be cancelled, the selected transfer status is not pending or buyback"); @@ -318,8 +326,7 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW private ExternalTransferStatus determineStatusAfterBuyback(ExternalAssetOwnerTransfer effectiveTransfer) { return switch (effectiveTransfer.getStatus()) { - case PENDING -> ExternalTransferStatus.BUYBACK; - case ACTIVE -> ExternalTransferStatus.BUYBACK; + case PENDING, ACTIVE -> ExternalTransferStatus.BUYBACK; case ACTIVE_INTERMEDIATE -> ExternalTransferStatus.BUYBACK_INTERMEDIATE; default -> throw new ExternalAssetOwnerInitiateTransferException(String.format( "This loan cannot be bought back, effective transfer is not in right state: %s", effectiveTransfer.getStatus())); @@ -582,11 +589,33 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW return fromApiJsonHelper.extractStringNamed(ExternalTransferRequestParameters.PURCHASE_PRICE_RATIO, json); } - private ExternalAssetOwner getOwner(JsonElement json) { - String ownerExternalId = fromApiJsonHelper.extractStringNamed(ExternalTransferRequestParameters.OWNER_EXTERNAL_ID, json); - Optional byExternalId = externalAssetOwnerRepository - .findByExternalId(ExternalIdFactory.produce(ownerExternalId)); - return byExternalId.orElseGet(() -> createAndGetAssetOwner(ownerExternalId)); + private ExternalAssetOwner getOwner(final JsonElement json) { + final String ownerExternalId = fromApiJsonHelper.extractStringNamed(ExternalTransferRequestParameters.OWNER_EXTERNAL_ID, json); + final ExternalId externalId = ExternalIdFactory.produce(ownerExternalId); + return externalAssetOwnerRepository.findByExternalId(externalId).orElseGet(() -> { + final Long ownerId = findOrCreateOwnerId(externalId); + // getReferenceById returns a lazy proxy without hitting the DB. findById would fail + // here because the outer transaction's persistence context does not contain the entity + // committed by the inner REQUIRES_NEW transaction. + return externalAssetOwnerRepository.getReferenceById(ownerId); + }); + } + + private Long findOrCreateOwnerId(final ExternalId externalId) { + try { + return externalAssetOwnerHelper.findOrCreateId(externalId); + } catch (JpaSystemException | DataIntegrityViolationException e) { + if (!isConstraintViolation(e)) { + throw e; + } + // Another thread created the owner concurrently - retry + return externalAssetOwnerHelper.findOrCreateId(externalId); + } + } + + private boolean isConstraintViolation(final DataAccessException e) { + return e.getMostSpecificCause() instanceof SQLException sqlEx && sqlEx.getSQLState() != null + && sqlEx.getSQLState().startsWith(SQL_STATE_INTEGRITY_CONSTRAINT_VIOLATION); } private ExternalAssetOwner createAndGetAssetOwner(String externalId) { @@ -611,7 +640,7 @@ public class ExternalAssetOwnersWriteServiceImpl implements ExternalAssetOwnersW String ownerExternalId = command.stringValueOfParameterNamed(ExternalTransferRequestParameters.OWNER_EXTERNAL_ID); Optional optExternalId = externalAssetOwnerRepository .findByExternalId(ExternalIdFactory.produce(ownerExternalId)); - if (!optExternalId.isEmpty()) { + if (optExternalId.isPresent()) { throw new ExternalAssetOwnerDuplicateException(ownerExternalId); } diff --git a/fineract-investor/src/test/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceTest.java b/fineract-investor/src/test/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceTest.java index 1276875d5..7ab96df1c 100644 --- a/fineract-investor/src/test/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceTest.java +++ b/fineract-investor/src/test/java/org/apache/fineract/investor/service/ExternalAssetOwnersWriteServiceTest.java @@ -38,6 +38,7 @@ import com.google.gson.JsonElement; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.math.BigDecimal; import java.math.RoundingMode; +import java.sql.SQLException; import java.time.LocalDate; import java.time.ZoneId; import java.util.ArrayList; @@ -79,7 +80,9 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.jpa.domain.Specification; +import org.springframework.orm.jpa.JpaSystemException; @ExtendWith(MockitoExtension.class) public class ExternalAssetOwnersWriteServiceTest { @@ -346,7 +349,7 @@ public class ExternalAssetOwnersWriteServiceTest { testContext.fromApiJsonHelper.extractLocalDateNamed(ExternalTransferRequestParameters.SETTLEMENT_DATE, jsonCommandElement)) .thenReturn(LocalDate.EPOCH); lenient().when(testContext.fromApiJsonHelper.extractLocalDateNamed(ExternalTransferRequestParameters.SETTLEMENT_DATE, - jsonCommandElement, testContext.DATE_FORMAT, Locale.GERMANY)).thenReturn(LocalDate.EPOCH); + jsonCommandElement, TestContext.DATE_FORMAT, Locale.GERMANY)).thenReturn(LocalDate.EPOCH); // given final JsonCommand command = createJsonCommand(testContext.jsonCommand, testContext.loanId); @@ -364,7 +367,7 @@ public class ExternalAssetOwnersWriteServiceTest { verify(testContext.externalAssetOwnerRepository, times(0)).saveAndFlush(any(ExternalAssetOwner.class)); verify(testContext.loanRepository).findLoanDataForExternalTransferByLoanId(testContext.loanId); verify(testContext.delayedSettlementAttributeService).isEnabled(testContext.loanProductId); - Assertions.assertEquals(thrownException.getMessage(), "Settlement date cannot be in the past"); + Assertions.assertEquals("Settlement date cannot be in the past", thrownException.getMessage()); } private static Stream effectiveTransferDataProvider() { @@ -440,14 +443,17 @@ public class ExternalAssetOwnersWriteServiceTest { when(testContext.loanRepository.findLoanDataForExternalTransferByLoanId(testContext.loanId)) .thenReturn(Optional.of(testContext.loanDataForExternalTransfer)); when(testContext.externalAssetOwnerRepository.findByExternalId(any(ExternalId.class))).thenReturn(Optional.empty()); + when(testContext.externalAssetOwnerHelper.findOrCreateId(any(ExternalId.class))).thenReturn(42L); + when(testContext.externalAssetOwnerRepository.getReferenceById(42L)).thenReturn(testContext.externalAssetOwner); // when CommandProcessingResult result = testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command); // then verify(testContext.externalAssetOwnerRepository).findByExternalId(any(ExternalId.class)); + verify(testContext.externalAssetOwnerHelper).findOrCreateId(any(ExternalId.class)); + verify(testContext.externalAssetOwnerRepository).getReferenceById(42L); verify(testContext.externalAssetOwnerTransferRepository).saveAndFlush(externalAssetOwnerTransferArgumentCaptor.capture()); - verify(testContext.externalAssetOwnerRepository).saveAndFlush(any(ExternalAssetOwner.class)); verify(testContext.externalAssetOwnerTransferRepository).findEffectiveTransfersOrderByIdDesc(eq(testContext.loanId), any(LocalDate.class)); verify(testContext.loanRepository).findLoanDataForExternalTransferByLoanId(testContext.loanId); @@ -462,6 +468,89 @@ public class ExternalAssetOwnersWriteServiceTest { assertEquals(savedTransfer.getExternalLoanId(), result.getSubResourceExternalId()); } + @Test + public void verifyWhenOwnerCreationHitsConstraintViolationWithJpaSystemExceptionThenRetrySucceeds() { + final TestContext testContext = new TestContext(); + final ArgumentCaptor transferCaptor = ArgumentCaptor.forClass(ExternalAssetOwnerTransfer.class); + + // given + final JsonCommand command = createJsonCommand(testContext.jsonCommand, testContext.loanId); + + final SQLException sqlException = new SQLException("Duplicate entry", "23505"); + final JpaSystemException jpaException = new JpaSystemException(new RuntimeException(sqlException)); + + when(testContext.delayedSettlementAttributeService.isEnabled(testContext.loanProductId)).thenReturn(false); + when(testContext.loanRepository.findLoanDataForExternalTransferByLoanId(testContext.loanId)) + .thenReturn(Optional.of(testContext.loanDataForExternalTransfer)); + when(testContext.externalAssetOwnerRepository.findByExternalId(any(ExternalId.class))).thenReturn(Optional.empty()); + when(testContext.externalAssetOwnerHelper.findOrCreateId(any(ExternalId.class))).thenThrow(jpaException).thenReturn(99L); + when(testContext.externalAssetOwnerRepository.getReferenceById(99L)).thenReturn(testContext.externalAssetOwner); + + // when + CommandProcessingResult result = testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command); + + // then + verify(testContext.externalAssetOwnerHelper, times(2)).findOrCreateId(any(ExternalId.class)); + verify(testContext.externalAssetOwnerRepository).getReferenceById(99L); + verify(testContext.externalAssetOwnerTransferRepository).saveAndFlush(transferCaptor.capture()); + + ExternalAssetOwnerTransfer savedTransfer = transferCaptor.getValue(); + assertAssertOwnerTransferValues(testContext, savedTransfer, ExternalTransferStatus.PENDING); + assertEquals(savedTransfer.getId(), result.getResourceId()); + } + + @Test + public void verifyWhenOwnerCreationHitsConstraintViolationWithDataIntegrityViolationExceptionThenRetrySucceeds() { + final TestContext testContext = new TestContext(); + final ArgumentCaptor transferCaptor = ArgumentCaptor.forClass(ExternalAssetOwnerTransfer.class); + + // given + final JsonCommand command = createJsonCommand(testContext.jsonCommand, testContext.loanId); + + final SQLException sqlException = new SQLException("Duplicate entry", "23000"); + final DataIntegrityViolationException diveException = new DataIntegrityViolationException("Duplicate", sqlException); + + when(testContext.delayedSettlementAttributeService.isEnabled(testContext.loanProductId)).thenReturn(false); + when(testContext.loanRepository.findLoanDataForExternalTransferByLoanId(testContext.loanId)) + .thenReturn(Optional.of(testContext.loanDataForExternalTransfer)); + when(testContext.externalAssetOwnerRepository.findByExternalId(any(ExternalId.class))).thenReturn(Optional.empty()); + when(testContext.externalAssetOwnerHelper.findOrCreateId(any(ExternalId.class))).thenThrow(diveException).thenReturn(99L); + when(testContext.externalAssetOwnerRepository.getReferenceById(99L)).thenReturn(testContext.externalAssetOwner); + + // when + CommandProcessingResult result = testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command); + + // then + verify(testContext.externalAssetOwnerHelper, times(2)).findOrCreateId(any(ExternalId.class)); + verify(testContext.externalAssetOwnerRepository).getReferenceById(99L); + verify(testContext.externalAssetOwnerTransferRepository).saveAndFlush(transferCaptor.capture()); + + ExternalAssetOwnerTransfer savedTransfer = transferCaptor.getValue(); + assertAssertOwnerTransferValues(testContext, savedTransfer, ExternalTransferStatus.PENDING); + assertEquals(savedTransfer.getId(), result.getResourceId()); + } + + @Test + public void verifyWhenOwnerCreationThrowsNonConstraintJpaSystemExceptionThenExceptionPropagates() { + final TestContext testContext = new TestContext(); + + // given + final JsonCommand command = createJsonCommand(testContext.jsonCommand, testContext.loanId); + + final JpaSystemException jpaException = new JpaSystemException(new RuntimeException("Connection lost")); + + when(testContext.delayedSettlementAttributeService.isEnabled(testContext.loanProductId)).thenReturn(false); + when(testContext.loanRepository.findLoanDataForExternalTransferByLoanId(testContext.loanId)) + .thenReturn(Optional.of(testContext.loanDataForExternalTransfer)); + when(testContext.externalAssetOwnerRepository.findByExternalId(any(ExternalId.class))).thenReturn(Optional.empty()); + when(testContext.externalAssetOwnerHelper.findOrCreateId(any(ExternalId.class))).thenThrow(jpaException); + + // when & then + assertThrows(JpaSystemException.class, () -> testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command)); + + verify(testContext.externalAssetOwnerHelper, times(1)).findOrCreateId(any(ExternalId.class)); + } + @Test public void verifyWhenLoanIsNotFoundThenExceptionIsThrown() { final TestContext testContext = new TestContext(); @@ -574,7 +663,7 @@ public class ExternalAssetOwnersWriteServiceTest { ExternalAssetOwnerInitiateTransferException exception = assertThrows(ExternalAssetOwnerInitiateTransferException.class, () -> testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command)); - assertEquals(exception.getMessage(), "This loan cannot be sold, there is already an in progress transfer"); + assertEquals("This loan cannot be sold, there is already an in progress transfer", exception.getMessage()); // then verify(testContext.fromApiJsonHelper, times(2)).parse(command.json()); @@ -607,7 +696,7 @@ public class ExternalAssetOwnersWriteServiceTest { ExternalAssetOwnerInitiateTransferException exception = assertThrows(ExternalAssetOwnerInitiateTransferException.class, () -> testContext.externalAssetOwnersWriteServiceImpl.saleLoanByLoanId(command)); - assertEquals(exception.getMessage(), "This loan cannot be sold, no effective transfer found."); + assertEquals("This loan cannot be sold, no effective transfer found.", exception.getMessage()); // then verify(testContext.fromApiJsonHelper, times(2)).parse(command.json()); verify(testContext.loanRepository).findLoanDataForExternalTransferByLoanId(testContext.loanId); @@ -699,7 +788,7 @@ public class ExternalAssetOwnersWriteServiceTest { ExternalAssetOwnerTransfer savedTransfer = savedTransferCaptor.getValue(); assertNotNull(savedTransfer); assertFalse(transfers.isEmpty()); - ExternalAssetOwnerTransfer expectedEffectiveTransfer = transfers.get(0); + ExternalAssetOwnerTransfer expectedEffectiveTransfer = transfers.getFirst(); assertEquals(testContext.transferExternalId, savedTransfer.getExternalId().getValue()); assertEquals(expectedEffectiveTransfer.getOwner(), savedTransfer.getOwner()); assertEquals(expectedStatus, savedTransfer.getStatus()); @@ -867,6 +956,9 @@ public class ExternalAssetOwnersWriteServiceTest { @Mock private ExternalAssetOwnersReadService externalAssetOwnersReadService; + @Mock + private ExternalAssetOwnerHelper externalAssetOwnerHelper; + @InjectMocks private ExternalAssetOwnersWriteServiceImpl externalAssetOwnersWriteServiceImpl; @@ -938,7 +1030,6 @@ public class ExternalAssetOwnersWriteServiceTest { lenient().when(configurationDomainService.getAllowedLoanStatusesOfDelayedSettlementForExternalAssetTransfer()) .thenReturn(List.of("ACTIVE", "TRANSFER_IN_PROGRESS", "TRANSFER_ON_HOLD", "OVERPAID", "CLOSED_OBLIGATIONS_MET")); lenient().when(externalAssetOwnersReadService.retrieveActiveTransferData(any(Long.class), any(), any())).thenReturn(null); - } } } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/investor/externalassetowner/InitiateExternalAssetOwnerTransferTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/investor/externalassetowner/InitiateExternalAssetOwnerTransferTest.java index 7262613a9..b6bf1b4dc 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/investor/externalassetowner/InitiateExternalAssetOwnerTransferTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/investor/externalassetowner/InitiateExternalAssetOwnerTransferTest.java @@ -46,12 +46,17 @@ import java.time.LocalDate; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import lombok.RequiredArgsConstructor; import okhttp3.ResponseBody; import org.apache.fineract.accounting.common.AccountingConstants; @@ -1361,6 +1366,78 @@ public class InitiateExternalAssetOwnerTransferTest extends BaseLoanIntegrationT }); } + @Test + public void saleTransferWithSameOwnerExternalIdInParallelShouldNotFail() { + try { + globalConfigurationHelper.manageConfigurations(GlobalConfigurationConstants.ENABLE_AUTO_GENERATED_EXTERNAL_ID, true); + setInitialBusinessDate("2020-03-02"); + + final int threadCount = 10; + final String sharedOwnerExternalId = UUID.randomUUID().toString(); + + final List loanIDs = new ArrayList<>(); + for (int i = 0; i < threadCount; i++) { + Integer clientID = createClient(); + Integer loanID = createLoanForClient(clientID); + loanIDs.add(loanID); + } + + final ExecutorService executorService = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + final List results = Collections.synchronizedList(new ArrayList<>()); + final List exceptions = Collections.synchronizedList(new ArrayList<>()); + + for (int i = 0; i < threadCount; i++) { + final Integer loanID = loanIDs.get(i); + executorService.execute(() -> { + try { + startLatch.await(); + PostInitiateTransferResponse response = EXTERNAL_ASSET_OWNER_HELPER.initiateTransferByLoanId(loanID.longValue(), + "sale", + new ExternalAssetOwnerRequest().settlementDate("2020-03-02").dateFormat("yyyy-MM-dd").locale("en") + .transferExternalId(UUID.randomUUID().toString()) + .transferExternalGroupId(UUID.randomUUID().toString()).ownerExternalId(sharedOwnerExternalId) + .purchasePriceRatio("1.0")); + results.add(response); + } catch (Exception e) { + exceptions.add(e); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + assertTrue(doneLatch.await(30, TimeUnit.SECONDS), "All threads should complete within timeout"); + executorService.shutdown(); + assertTrue(executorService.awaitTermination(10, TimeUnit.SECONDS), "ExecutorService should terminate"); + + assertTrue(exceptions.isEmpty(), + "Expected no exceptions but got " + exceptions.size() + ": " + exceptions.stream().map(Throwable::getMessage).toList()); + assertEquals(threadCount, results.size(), "All transfers should succeed"); + results.forEach(response -> { + assertNotNull(response.getResourceId()); + assertNotNull(response.getResourceExternalId()); + }); + + // Verify all transfers reference the same owner + for (Integer loanID : loanIDs) { + PageExternalTransferData transfers = EXTERNAL_ASSET_OWNER_HELPER.retrieveTransfersByLoanId(loanID.longValue()); + assertEquals(1, transfers.getTotalElements()); + assertNotNull(transfers.getContent()); + assertNotNull(transfers.getContent().getFirst().getOwner()); + assertEquals(sharedOwnerExternalId, transfers.getContent().getFirst().getOwner().getExternalId(), + "All transfers should reference the same owner"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } finally { + cleanUpAndRestoreBusinessDate(); + } + } + private void updateBusinessDateAndExecuteCOBJob(String date) { BusinessDateHelper.updateBusinessDate(REQUEST_SPEC, RESPONSE_SPEC, BUSINESS_DATE, LocalDate.parse(date)); SCHEDULER_JOB_HELPER.executeAndAwaitJob("Loan COB");