FINERACT-2181: Batch API - Fix read-only connection handling

Author: adamsaghy

fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java

@@ -58,6 +58,8 @@
 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 @@
  * 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<BatchFilter> batchFilters;
@@ -94,6 +95,7 @@ public class BatchApiServiceImpl implements BatchApiService {
 
     private final RetryConfigurationAssembler retryConfigurationAssembler;
 
+    private PlatformTransactionManager transactionManager;
     private EntityManager entityManager;
 
     /**
@@ -152,6 +154,9 @@ private List<BatchResponse> callInTransaction(Consumer<TransactionTemplate> tran
             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 @@ private List<BatchResponse> callInTransaction(Consumer<TransactionTemplate> tran
     }
 
     /**
-     * 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 @@ private BatchResponse buildErrorResponse(Long requestId, Integer statusCode, Str
     public void setEntityManager(EntityManager entityManager) {
         this.entityManager = entityManager;
     }
+
+    @Autowired
+    public void setTransactionManager(PlatformTransactionManager transactionManager) {
+        this.transactionManager = transactionManager;
+    }
 }

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.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<BatchRequestPreprocessor> batchPreprocessors = Mockito.spy(List.of());
+    private final ResolutionHelper resolutionHelper = spy(new ResolutionHelper(new FromJsonHelper()));
+    private final List<BatchRequestPreprocessor> 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 @@ void testHandleBatchRequestsWithEnclosingTransactionReadOnly() {
         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<BatchResponse> 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 {}
 
 }