From bc4b3555f6b0ca6a13dd016fdee5620dfdd82a10 Mon Sep 17 00:00:00 2001 From: Adam Saghy Date: Fri, 11 Jul 2025 19:12:56 +0200 Subject: [PATCH] FINERACT-2181: Batch API - Fix read-only connection handling --- .../batch/service/BatchApiServiceImpl.java | 22 +++++--- .../service/BatchApiServiceImplTest.java | 51 +++++++++++++++++-- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java b/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java index fbd9b7925..fc41c0f60 100644 --- a/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java +++ b/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java @@ -58,6 +58,8 @@ import org.apache.fineract.infrastructure.core.exception.ErrorHandler; import org.apache.fineract.infrastructure.core.filters.BatchCallHandler; import org.apache.fineract.infrastructure.core.filters.BatchFilter; import org.apache.fineract.infrastructure.core.filters.BatchRequestPreprocessor; +import org.apache.fineract.infrastructure.core.persistence.ExtendedJpaTransactionManager; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.dao.NonTransientDataAccessException; import org.springframework.lang.NonNull; @@ -74,9 +76,9 @@ import org.springframework.transaction.support.TransactionTemplate; * CommandStrategy from CommandStrategyProvider. * * @author Rishabh Shukla - * @see org.apache.fineract.batch.domain.BatchRequest - * @see org.apache.fineract.batch.domain.BatchResponse - * @see org.apache.fineract.batch.command.CommandStrategyProvider + * @see BatchRequest + * @see BatchResponse + * @see CommandStrategyProvider */ @Service @RequiredArgsConstructor @@ -85,7 +87,6 @@ public class BatchApiServiceImpl implements BatchApiService { private final CommandStrategyProvider strategyProvider; private final ResolutionHelper resolutionHelper; - private final PlatformTransactionManager transactionManager; private final ErrorHandler errorHandler; private final List batchFilters; @@ -94,6 +95,7 @@ public class BatchApiServiceImpl implements BatchApiService { private final RetryConfigurationAssembler retryConfigurationAssembler; + private PlatformTransactionManager transactionManager; private EntityManager entityManager; /** @@ -152,6 +154,9 @@ public class BatchApiServiceImpl implements BatchApiService { try { TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); transactionTemplate.setIsolationLevel(TransactionDefinition.ISOLATION_REPEATABLE_READ); + if (transactionManager instanceof ExtendedJpaTransactionManager extendedJpaTransactionManager) { + transactionTemplate.setReadOnly(extendedJpaTransactionManager.isReadOnlyConnection()); + } transactionConfigurator.accept(transactionTemplate); return transactionTemplate.execute(status -> { BatchRequestContextHolder.setEnclosingTransaction(status); @@ -175,8 +180,8 @@ public class BatchApiServiceImpl implements BatchApiService { } /** - * Returns the response list by getting a proper {@link org.apache.fineract.batch.command.CommandStrategy}. - * execute() method of acquired commandStrategy is then provided with the separate Request. + * Returns the response list by getting a proper {@link CommandStrategy}. execute() method of acquired + * commandStrategy is then provided with the separate Request. * * @param requestList * @param uriInfo @@ -395,4 +400,9 @@ public class BatchApiServiceImpl implements BatchApiService { public void setEntityManager(EntityManager entityManager) { this.entityManager = entityManager; } + + @Autowired + public void setTransactionManager(PlatformTransactionManager transactionManager) { + this.transactionManager = transactionManager; + } } diff --git a/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java b/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java index 573aed1a9..ccdcb6628 100644 --- a/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java +++ b/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java @@ -19,11 +19,14 @@ package org.apache.fineract.batch.service; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,6 +54,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -86,8 +91,8 @@ class BatchApiServiceImplTest { @InjectMocks private RetryConfigurationAssembler retryConfigurationAssembler; - private final ResolutionHelper resolutionHelper = Mockito.spy(new ResolutionHelper(new FromJsonHelper())); - private final List batchPreprocessors = Mockito.spy(List.of()); + private final ResolutionHelper resolutionHelper = spy(new ResolutionHelper(new FromJsonHelper())); + private final List batchPreprocessors = spy(List.of()); @InjectMocks private BatchApiServiceImpl batchApiService; @@ -96,8 +101,9 @@ class BatchApiServiceImplTest { @BeforeEach void setUp() { - batchApiService = new BatchApiServiceImpl(strategyProvider, resolutionHelper, transactionManager, errorHandler, List.of(), - batchPreprocessors, retryConfigurationAssembler); + batchApiService = new BatchApiServiceImpl(strategyProvider, resolutionHelper, errorHandler, List.of(), batchPreprocessors, + retryConfigurationAssembler); + batchApiService.setTransactionManager(transactionManager); batchApiService.setEntityManager(entityManager); request = new BatchRequest(); request.setRequestId(1L); @@ -227,6 +233,43 @@ class BatchApiServiceImplTest { Mockito.verifyNoInteractions(entityManager); } + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void testCallInTransactionReadOnlyFlag(boolean isReadOnly) { + // Given + ExtendedJpaTransactionManager extendedJpaTransactionManager = mock(ExtendedJpaTransactionManager.class); + + // Create a transaction status with the correct read-only flag + DefaultTransactionStatus transactionStatus = new DefaultTransactionStatus("txn_name", null, true, true, false, isReadOnly, false, + null); + + // Mock getTransaction to return our status when the read-only flag matches + when(extendedJpaTransactionManager.isReadOnlyConnection()).thenReturn(isReadOnly); + when(extendedJpaTransactionManager + .getTransaction(argThat(definition -> definition != null && definition.isReadOnly() == isReadOnly))) + .thenReturn(transactionStatus); + + // Mock other required dependencies + when(strategyProvider.getCommandStrategy(any())).thenReturn(commandStrategy); + when(commandStrategy.execute(any(), any())).thenReturn(response); + + batchApiService.setTransactionManager(extendedJpaTransactionManager); + + // Set up a request that will trigger the read-only behavior we want to test + BatchRequest testRequest = new BatchRequest(); + testRequest.setRequestId(1L); + testRequest.setMethod(isReadOnly ? "GET" : "POST"); // Use GET for read-only, POST for read-write + testRequest.setRelativeUrl("/test/endpoint"); + + // When + List responses = batchApiService.handleBatchRequestsWithEnclosingTransaction(List.of(testRequest), uriInfo); + + // Then + assertFalse(responses.isEmpty()); + verify(extendedJpaTransactionManager) + .getTransaction(argThat(definition -> definition != null && definition.isReadOnly() == isReadOnly)); + } + private static final class RetryException extends RuntimeException {} }