FINERACT-1724 Loan COB Api Filter fix

- [x] Loan COB Api Filter fix for urls
- [x] Unit test

fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java

@@ -18,16 +18,12 @@
  */
 package org.apache.fineract.infrastructure.jobs.filter;
 
-import com.google.common.base.Splitter;
 import java.io.IOException;
 import java.math.BigDecimal;
 import java.util.Collections;
 import java.util.List;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
 import java.util.regex.Pattern;
-import java.util.stream.Stream;
-import java.util.stream.StreamSupport;
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -64,14 +60,11 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte
 
     private static final List<HttpMethod> HTTP_METHODS = List.of(HttpMethod.POST, HttpMethod.PUT, HttpMethod.DELETE);
 
-    private static final Pattern LOAN_PATH_PATTERN = Pattern.compile("/loans/\\d+");
+    public static final Pattern LOAN_PATH_PATTERN = Pattern.compile("\\/?loans\\/(?:external-id\\/)?(\\d+).*");
 
-    private static final Pattern LOAN_GLIMACCOUNT_PATH_PATTERN = Pattern.compile("/loans/glimAccount/\\d+");
+    public static final Pattern LOAN_GLIMACCOUNT_PATH_PATTERN = Pattern.compile("\\/?loans\\/glimAccount\\/(\\d+).*");
     private static final Predicate<String> URL_FUNCTION = s -> LOAN_PATH_PATTERN.matcher(s).find()
             || LOAN_GLIMACCOUNT_PATH_PATTERN.matcher(s).find();
-    private static final Integer LOAN_ID_INDEX_IN_URL = 2;
-    private static final Integer GLIM_ID_INDEX_IN_URL = 3;
-    private static final Integer GLIM_STRING_INDEX_IN_URL = 2;
     private static final String JOB_NAME = "INLINE_LOAN_COB";
 
     @RequiredArgsConstructor
@@ -138,10 +131,9 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte
     }
 
     private List<Long> calculateRelevantLoanIds(String pathInfo) {
-        Iterable<String> split = Splitter.on('/').split(pathInfo);
+
-        Supplier<Stream<String>> streamSupplier = () -> StreamSupport.stream(split.spliterator(), false);
+        boolean isGlim = isGlim(pathInfo);
-        boolean isGlim = isGlim(streamSupplier);
+        Long loanIdFromRequest = getLoanId(isGlim, pathInfo);
-        Long loanIdFromRequest = getLoanId(isGlim, streamSupplier);
         List<Long> loanIds = isGlim ? getGlimChildLoanIds(loanIdFromRequest) : Collections.singletonList(loanIdFromRequest);
         if (isLoanHardLocked(loanIds)) {
             throw new LoanIdsHardLockedException(loanIdFromRequest);
@@ -186,11 +178,11 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte
         filterChain.doFilter(request, response);
     }
 
-    private Long getLoanId(boolean isGlim, Supplier<Stream<String>> streamSupplier) {
+    private Long getLoanId(boolean isGlim, String pathInfo) {
         if (!isGlim) {
-            return streamSupplier.get().skip(LOAN_ID_INDEX_IN_URL).findFirst().map(Long::valueOf).orElse(null);
+            return Long.valueOf(LOAN_PATH_PATTERN.matcher(pathInfo).replaceAll("$1"));
         } else {
-            return streamSupplier.get().skip(GLIM_ID_INDEX_IN_URL).findFirst().map(Long::valueOf).orElse(null);
+            return Long.valueOf(LOAN_GLIMACCOUNT_PATH_PATTERN.matcher(pathInfo).replaceAll("$1"));
         }
     }
 
@@ -201,8 +193,8 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte
         return HTTP_METHODS.contains(HttpMethod.valueOf(method)) && URL_FUNCTION.test(pathInfo);
     }
 
-    private boolean isGlim(Supplier<Stream<String>> streamSupplier) {
+    private boolean isGlim(String pathInfo) {
-        return streamSupplier.get().skip(GLIM_STRING_INDEX_IN_URL).findFirst().map(s -> s.equals("glimAccount")).orElse(false);
+        return LOAN_GLIMACCOUNT_PATH_PATTERN.matcher(pathInfo).matches();
     }
 
     @Override

fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java

@@ -38,6 +38,7 @@ import org.apache.fineract.portfolio.loanaccount.domain.GroupLoanIndividualMonit
 import org.apache.fineract.portfolio.loanaccount.domain.Loan;
 import org.apache.fineract.useradministration.domain.AppUser;
 import org.apache.http.HttpStatus;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
@@ -63,6 +64,29 @@ class LoanCOBApiFilterTest {
     @Mock
     private InlineLoanCOBExecutorServiceImpl inlineLoanCOBExecutorService;
 
+    @Test
+    void shouldLoanAndExternalMatchToo() {
+        Assertions.assertTrue(LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/12").matches());
+        Assertions.assertTrue(LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/12?correct=parameter").matches());
+        Assertions.assertTrue(LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/12").matches());
+        Assertions.assertTrue(LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/12?additional=parameter").matches());
+        Assertions.assertEquals("12", LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/12").replaceAll("$1"));
+        Assertions.assertEquals("12", LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/12?correct=parameter").replaceAll("$1"));
+        Assertions.assertEquals("12", LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/12").replaceAll("$1"));
+        Assertions.assertEquals("12",
+                LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/12?additional=parameter").replaceAll("$1"));
+    }
+
+    @Test
+    void shouldGlimAccountMatch() {
+        Assertions.assertTrue(LoanCOBApiFilter.LOAN_GLIMACCOUNT_PATH_PATTERN.matcher("/loans/glimAccount/12").matches());
+        Assertions
+                .assertTrue(LoanCOBApiFilter.LOAN_GLIMACCOUNT_PATH_PATTERN.matcher("/loans/glimAccount/12?additional=parameter").matches());
+        Assertions.assertEquals("12", LoanCOBApiFilter.LOAN_GLIMACCOUNT_PATH_PATTERN.matcher("/loans/glimAccount/12").replaceAll("$1"));
+        Assertions.assertEquals("12",
+                LoanCOBApiFilter.LOAN_GLIMACCOUNT_PATH_PATTERN.matcher("/loans/glimAccount/12?additional=parameter").replaceAll("$1"));
+    }
+
     @Test
     void shouldProceedWhenUrlDoesNotMatch() throws ServletException, IOException {
         MockHttpServletRequest request = mock(MockHttpServletRequest.class);
@@ -109,6 +133,23 @@ class LoanCOBApiFilterTest {
         verify(filterChain, times(1)).doFilter(request, response);
     }
 
+    @Test
+    void shouldProceedWhenExternalLoanIsNotLocked() throws ServletException, IOException {
+        MockHttpServletRequest request = mock(MockHttpServletRequest.class);
+        MockHttpServletResponse response = mock(MockHttpServletResponse.class);
+        FilterChain filterChain = mock(FilterChain.class);
+        AppUser appUser = mock(AppUser.class);
+
+        given(request.getPathInfo()).willReturn("/loans/external-id/2/charges");
+        given(request.getMethod()).willReturn(HTTPMethods.POST.value());
+        given(loanAccountLockService.isLoanHardLocked(2L)).willReturn(false);
+        given(loanAccountLockService.isLoanSoftLocked(2L)).willReturn(false);
+        given(context.authenticatedUser()).willReturn(appUser);
+
+        testObj.doFilterInternal(request, response, filterChain);
+        verify(filterChain, times(1)).doFilter(request, response);
+    }
+
     @Test
     void shouldRunInlineCOBAndProceedWhenLoanIsSoftLocked() throws ServletException, IOException {
         MockHttpServletRequest request = mock(MockHttpServletRequest.class);