Fixed caching request body

openMF · Sep 17, 2024 · e007171 · e007171
1 parent df1b6b3
commit e007171
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 27 deletions.
diff --git a/fineract-provider/dependencies.gradle b/fineract-provider/dependencies.gradle
@@ -136,7 +136,7 @@ dependencies {
     implementation ('jakarta.xml.bind:jakarta.xml.bind-api') {
         exclude group: 'jakarta.activation'
     }
-    implementation ('org.apache.activemq:activemq-client-jakarta') {
+    implementation ('org.apache.activemq:activemq-client') {
         exclude group: 'org.apache.geronimo.specs'
         exclude group: 'javax.annotation', module: 'javax.annotation-api'
     }

diff --git a/...va/org/apache/fineract/infrastructure/core/http/BodyCachingHttpServletRequestWrapper.java b/...va/org/apache/fineract/infrastructure/core/http/BodyCachingHttpServletRequestWrapper.java
@@ -35,16 +35,18 @@
 public class BodyCachingHttpServletRequestWrapper extends HttpServletRequestWrapper {
 
     private final byte[] cachedBody;
+    private ByteArrayInputStream inputStream;
 
     @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW")
     public BodyCachingHttpServletRequestWrapper(HttpServletRequest request) throws IOException {
         super(request);
         this.cachedBody = IOUtils.toByteArray(request.getInputStream());
+        this.inputStream = new ByteArrayInputStream(cachedBody);
     }
 
     @Override
     public ServletInputStream getInputStream() throws IOException {
-        return new CachedBodyServletInputStream(cachedBody);
+        return new CachedBodyServletInputStream(inputStream);
     }
 
     @Override
@@ -53,12 +55,16 @@ public BufferedReader getReader() throws IOException {
         return new BufferedReader(new InputStreamReader(byteArrayInputStream, UTF_8));
     }
 
+    public void resetStream() {
+        inputStream = new ByteArrayInputStream(cachedBody);
+    }
+
     public static class CachedBodyServletInputStream extends ServletInputStream {
 
         private final InputStream inputStream;
 
-        public CachedBodyServletInputStream(byte[] cachedBody) {
-            this.inputStream = new ByteArrayInputStream(cachedBody);
+        public CachedBodyServletInputStream(ByteArrayInputStream inputStream) {
+            this.inputStream = inputStream;
         }
 
         @Override

diff --git a/...ovider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java b/...ovider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java
@@ -65,7 +65,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
             throws ServletException, IOException {
         request = new BodyCachingHttpServletRequestWrapper(request);
 
-        if (!helper.isOnApiList(request)) {
+        if (!helper.isOnApiList((BodyCachingHttpServletRequestWrapper) request)) {
             proceed(filterChain, request, response);
         } else {
             try {
@@ -74,7 +74,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
                     proceed(filterChain, request, response);
                 } else {
                     try {
-                        List<Long> loanIds = helper.calculateRelevantLoanIds(request);
+                        List<Long> loanIds = helper.calculateRelevantLoanIds((BodyCachingHttpServletRequestWrapper) request);
                         if (!loanIds.isEmpty() && helper.isLoanBehind(loanIds)) {
                             helper.executeInlineCob(loanIds);
                         }

diff --git a/...der/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBFilterHelper.java b/...der/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBFilterHelper.java
@@ -26,7 +26,6 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.Lists;
-import jakarta.servlet.http.HttpServletRequest;
 import java.io.IOException;
 import java.math.BigDecimal;
 import java.util.ArrayList;
@@ -47,6 +46,7 @@
 import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType;
 import org.apache.fineract.infrastructure.core.config.FineractProperties;
 import org.apache.fineract.infrastructure.core.domain.ExternalId;
+import org.apache.fineract.infrastructure.core.http.BodyCachingHttpServletRequestWrapper;
 import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
 import org.apache.fineract.infrastructure.jobs.exception.LoanIdsHardLockedException;
 import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
@@ -113,7 +113,7 @@ private boolean isRescheduleLoans(String pathInfo) {
         return LOAN_PATH_PATTERN.matcher(pathInfo).matches() && pathInfo.contains("/v1/rescheduleloans/");
     }
 
-    public boolean isOnApiList(HttpServletRequest request) throws IOException {
+    public boolean isOnApiList(BodyCachingHttpServletRequestWrapper request) throws IOException {
         String pathInfo = request.getPathInfo();
         String method = request.getMethod();
         if (StringUtils.isBlank(pathInfo)) {
@@ -126,7 +126,7 @@ public boolean isOnApiList(HttpServletRequest request) throws IOException {
         }
     }
 
-    private boolean isBatchApiMatching(HttpServletRequest request) throws IOException {
+    private boolean isBatchApiMatching(BodyCachingHttpServletRequestWrapper request) throws IOException {
         for (BatchRequest batchRequest : getBatchRequests(request)) {
             String method = batchRequest.getMethod();
             String pathInfo = batchRequest.getRelativeUrl();
@@ -137,8 +137,10 @@ private boolean isBatchApiMatching(HttpServletRequest request) throws IOExceptio
         return false;
     }
 
-    private List<BatchRequest> getBatchRequests(HttpServletRequest request) throws IOException {
+    private List<BatchRequest> getBatchRequests(BodyCachingHttpServletRequestWrapper request) throws IOException {
         List<BatchRequest> batchRequests = objectMapper.readValue(request.getInputStream(), new TypeReference<>() {});
+        // since we read body, we have to reset so the upcoming readings are successful
+        request.resetStream();
         for (BatchRequest batchRequest : batchRequests) {
             String pathInfo = "/" + batchRequest.getRelativeUrl();
             if (!isRelativeUrlVersioned(batchRequest.getRelativeUrl())) {
@@ -192,7 +194,7 @@ public boolean isLoanBehind(List<Long> loanIds) {
         return CollectionUtils.isNotEmpty(loanIdAndLastClosedBusinessDates);
     }
 
-    public List<Long> calculateRelevantLoanIds(HttpServletRequest request) throws IOException {
+    public List<Long> calculateRelevantLoanIds(BodyCachingHttpServletRequestWrapper request) throws IOException {
         String pathInfo = request.getPathInfo();
         if (isBatchApi(pathInfo)) {
             return getLoanIdsFromBatchApi(request);
@@ -201,7 +203,7 @@ public List<Long> calculateRelevantLoanIds(HttpServletRequest request) throws IO
         }
     }
 
-    private List<Long> getLoanIdsFromBatchApi(HttpServletRequest request) throws IOException {
+    private List<Long> getLoanIdsFromBatchApi(BodyCachingHttpServletRequestWrapper request) throws IOException {
         List<Long> loanIds = new ArrayList<>();
         for (BatchRequest batchRequest : getBatchRequests(request)) {
             // check the URL for Loan related ID

diff --git a/...er/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java b/...er/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java
@@ -32,6 +32,7 @@
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.math.BigDecimal;
@@ -144,7 +145,9 @@ void shouldProceedWhenUrlDoesNotMatch() throws ServletException, IOException {
 
         given(request.getPathInfo()).willReturn("/v1/jobs/2/inline");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
 
         testObj.doFilterInternal(request, response, filterChain);
         verify(filterChain, times(1)).doFilter(any(HttpServletRequest.class), eq(response));
@@ -165,7 +168,9 @@ void shouldProceedWhenUrlDoesNotMatchWithInvalidLoanId() throws ServletException
 
         given(request.getPathInfo()).willReturn("/v1/loans/invalid2LoanId/charges");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(context.authenticatedUser()).willReturn(appUser);
         given(fineractProperties.getQuery()).willReturn(fineractQueryProperties);
         given(fineractQueryProperties.getInClauseParameterSizeLimit()).willReturn(65000);
@@ -185,7 +190,9 @@ void shouldProceedWhenUserHasBypassPermission() throws ServletException, IOExcep
 
         given(request.getPathInfo()).willReturn("/v1/jobs/2/inline");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(context.authenticatedUser()).willReturn(appUser);
         given(appUser.isBypassUser()).willReturn(true);
 
@@ -208,7 +215,9 @@ void shouldProceedWhenLoanIsNotLockedAndNoLoanIsBehind() throws ServletException
 
         given(request.getPathInfo()).willReturn("/v1/loans/2/charges");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
         given(context.authenticatedUser()).willReturn(appUser);
         given(fineractProperties.getQuery()).willReturn(fineractQueryProperties);
@@ -235,7 +244,9 @@ void shouldProceedWhenExternalLoanIsNotLockedAndNotBehind() throws ServletExcept
         String uuid = UUID.randomUUID().toString();
         given(request.getPathInfo()).willReturn("/v1/loans/external-id/" + uuid + "/charges");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
         given(context.authenticatedUser()).willReturn(appUser);
         given(loanRepository.findIdByExternalId(any())).willReturn(2L);
@@ -263,7 +274,9 @@ void shouldProceedWhenRescheduleLoanIsNotLockedAndNotBehind() throws ServletExce
         Long resourceId = 123L;
         given(request.getPathInfo()).willReturn("/v1/rescheduleloans/" + resourceId + "/charges");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
         given(fineractProperties.getQuery()).willReturn(fineractQueryProperties);
         given(fineractQueryProperties.getInClauseParameterSizeLimit()).willReturn(65000);
@@ -296,7 +309,9 @@ void shouldRunInlineCOBAndProceedWhenLoanIsBehind() throws ServletException, IOE
         given(result.getLastClosedBusinessDate()).willReturn(businessDate.minusDays(2));
         given(request.getPathInfo()).willReturn("/v1/loans/2?command=approve");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
         given(fineractProperties.getQuery()).willReturn(fineractQueryProperties);
         given(fineractQueryProperties.getInClauseParameterSizeLimit()).willReturn(65000);
@@ -327,7 +342,9 @@ void shouldNotRunInlineCOBAndProceedWhenLoanIsNotBehind() throws ServletExceptio
         given(result.getId()).willReturn(2L);
         given(request.getPathInfo()).willReturn("/v1/loans/2?command=approve");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
         given(fineractProperties.getQuery()).willReturn(fineractQueryProperties);
         given(fineractQueryProperties.getInClauseParameterSizeLimit()).willReturn(65000);
@@ -350,7 +367,9 @@ void shouldNotRunInlineCOBAndProceedWhenLoanIsBehindForLoanCreation() throws Ser
 
         given(request.getPathInfo()).willReturn("/v1/loans");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
 
         given(context.authenticatedUser()).willReturn(appUser);
 
@@ -368,7 +387,9 @@ void shouldNotRunInlineCOBForCatchUp() throws ServletException, IOException {
 
         given(request.getPathInfo()).willReturn("/v1/loans/catch-up");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
 
         given(context.authenticatedUser()).willReturn(appUser);
 
@@ -387,7 +408,9 @@ void shouldRejectWhenLoanIsHardLocked() throws ServletException, IOException {
 
         given(request.getPathInfo()).willReturn("/v1/loans/2/charges");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(true);
         given(response.getWriter()).willReturn(writer);
         given(context.authenticatedUser()).willReturn(appUser);
@@ -409,7 +432,9 @@ void shouldRejectWhenGlimLoanIsHardLocked() throws ServletException, IOException
 
         given(request.getPathInfo()).willReturn("/v1/loans/glimAccount/2");
         given(request.getMethod()).willReturn(HTTPMethods.POST.value());
-        given(request.getInputStream()).willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new byte[0]));
+        final byte[] cachedBody = new byte[0];
+        given(request.getInputStream())
+                .willReturn(new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(new ByteArrayInputStream(cachedBody)));
         given(glimAccountInfoRepository.findOneByIsAcceptingChildAndApplicationId(true, BigDecimal.valueOf(2))).willReturn(glimAccount);
         given(glimAccount.getChildLoan()).willReturn(Collections.singleton(loan));
         given(loan.getId()).willReturn(loanId);

diff --git a/...src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBFilterHelperTest.java b/...src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBFilterHelperTest.java
@@ -18,7 +18,7 @@
  */
 package org.apache.fineract.infrastructure.jobs.filter;
 
-import jakarta.servlet.http.HttpServletRequest;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.util.List;
@@ -116,10 +116,10 @@ public void testCOBFilterUnescapedChars() throws IOException {
                 ]
                 """;
 
-        HttpServletRequest httpServletRequest = Mockito.mock(HttpServletRequest.class);
+        BodyCachingHttpServletRequestWrapper httpServletRequest = Mockito.mock(BodyCachingHttpServletRequestWrapper.class);
         Mockito.when(httpServletRequest.getPathInfo()).thenReturn("/v1/batches/endpoint");
         BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream inputStream = new BodyCachingHttpServletRequestWrapper.CachedBodyServletInputStream(
-                json.getBytes(Charset.forName("UTF-8")));
+                new ByteArrayInputStream(json.getBytes(Charset.forName("UTF-8"))));
         Mockito.when(httpServletRequest.getInputStream()).thenReturn(inputStream);
         List<Long> loanIds = helper.calculateRelevantLoanIds(httpServletRequest);
         Assertions.assertEquals(0, loanIds.size());