diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java index 571fb3a25..9812f6f55 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java @@ -39,6 +39,7 @@ import org.apache.fineract.cob.service.InlineLoanCOBExecutorServiceImpl; import org.apache.fineract.cob.service.LoanAccountLockService; import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType; import org.apache.fineract.infrastructure.core.data.ApiGlobalErrorResponse; +import org.apache.fineract.infrastructure.core.domain.ExternalId; import org.apache.fineract.infrastructure.core.filters.BatchFilter; import org.apache.fineract.infrastructure.core.filters.BatchFilterChain; import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil; @@ -65,7 +66,7 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte private static final List HTTP_METHODS = List.of(HttpMethod.POST, HttpMethod.PUT, HttpMethod.DELETE); - public static final Pattern LOAN_PATH_PATTERN = Pattern.compile("\\/?loans\\/(?:external-id\\/)?(\\d+).*"); + public static final Pattern LOAN_PATH_PATTERN = Pattern.compile("\\/?loans\\/(?:external-id\\/)?([^\\/\\?]+).*"); public static final Pattern LOAN_GLIMACCOUNT_PATH_PATTERN = Pattern.compile("\\/?loans\\/glimAccount\\/(\\d+).*"); private static final Predicate URL_FUNCTION = s -> LOAN_PATH_PATTERN.matcher(s).find() @@ -142,16 +143,27 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte private List calculateRelevantLoanIds(String pathInfo) { - boolean isGlim = isGlim(pathInfo); - Long loanIdFromRequest = getLoanId(isGlim, pathInfo); - List loanIds = isGlim ? getGlimChildLoanIds(loanIdFromRequest) : Collections.singletonList(loanIdFromRequest); + List loanIds = getLoanIdList(pathInfo); if (isLoanHardLocked(loanIds)) { - throw new LoanIdsHardLockedException(loanIdFromRequest); + throw new LoanIdsHardLockedException(loanIds.get(0)); } else { return loanIds; } } + private List getLoanIdList(String pathInfo) { + boolean isGlim = isGlim(pathInfo); + Long loanIdFromRequest = getLoanId(isGlim, pathInfo); + if (loanIdFromRequest == null) { + return Collections.emptyList(); + } + if (isGlim) { + return getGlimChildLoanIds(loanIdFromRequest); + } else { + return Collections.singletonList(loanIdFromRequest); + } + } + private void executeInlineCob(List loanIds) { inlineLoanCOBExecutorService.execute(loanIds, JOB_NAME); } @@ -190,12 +202,24 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte private Long getLoanId(boolean isGlim, String pathInfo) { if (!isGlim) { - return Long.valueOf(LOAN_PATH_PATTERN.matcher(pathInfo).replaceAll("$1")); + String id = LOAN_PATH_PATTERN.matcher(pathInfo).replaceAll("$1"); + if (isExternal(pathInfo)) { + String externalId = id; + return loanRepository.findIdByExternalId(new ExternalId(externalId)); + } else if (StringUtils.isNumeric(id)) { + return Long.valueOf(id); + } else { + return null; + } } else { return Long.valueOf(LOAN_GLIMACCOUNT_PATH_PATTERN.matcher(pathInfo).replaceAll("$1")); } } + private boolean isExternal(String pathInfo) { + return LOAN_PATH_PATTERN.matcher(pathInfo).matches() && pathInfo.contains("external-id"); + } + private boolean isOnApiList(String pathInfo, String method) { if (StringUtils.isBlank(pathInfo)) { return false; @@ -219,7 +243,7 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte } else { try { List result = calculateRelevantLoanIds("/" + batchRequest.getRelativeUrl()); - if (isLoanSoftLocked(result)) { + if (isLoanSoftLocked(result) || isLoanBehind(result)) { executeInlineCob(result); } return chain.serviceCall(batchRequest, uriInfo); diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java index 0ccf932f3..e31e89647 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilterTest.java @@ -18,6 +18,7 @@ */ package org.apache.fineract.infrastructure.jobs.filter; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; @@ -33,6 +34,7 @@ import java.time.LocalDate; import java.time.ZoneId; import java.util.Collections; import java.util.HashMap; +import java.util.UUID; import javax.servlet.FilterChain; import javax.servlet.ServletException; import org.apache.fineract.cob.service.InlineLoanCOBExecutorServiceImpl; @@ -77,15 +79,18 @@ class LoanCOBApiFilterTest { @Test void shouldLoanAndExternalMatchToo() { + String externalId = UUID.randomUUID().toString(); 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.assertTrue(LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/" + externalId).matches()); + Assertions.assertTrue( + LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/" + externalId + "?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")); + Assertions.assertEquals(externalId, + LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/" + externalId).replaceAll("$1")); + Assertions.assertEquals(externalId, + LoanCOBApiFilter.LOAN_PATH_PATTERN.matcher("/loans/external-id/" + externalId + "?additional=parameter").replaceAll("$1")); } @Test @@ -111,6 +116,30 @@ class LoanCOBApiFilterTest { verify(filterChain, times(1)).doFilter(request, response); } + @Test + void shouldProceedWhenUrlDoesNotMatchWithInvalidLoanId() throws ServletException, IOException { + MockHttpServletRequest request = mock(MockHttpServletRequest.class); + MockHttpServletResponse response = mock(MockHttpServletResponse.class); + FilterChain filterChain = mock(FilterChain.class); + AppUser appUser = mock(AppUser.class); + ThreadLocalContextUtil.setTenant(new FineractPlatformTenant(1L, "default", "Default", "Asia/Kolkata", null)); + HashMap businessDates = new HashMap<>(); + LocalDate businessDate = LocalDate.now(ZoneId.systemDefault()); + businessDates.put(BusinessDateType.BUSINESS_DATE, businessDate); + businessDates.put(BusinessDateType.COB_DATE, businessDate.minusDays(1)); + ThreadLocalContextUtil.setBusinessDates(businessDates); + + given(request.getPathInfo()).willReturn("/loans/invalid2LoanId/charges"); + given(request.getMethod()).willReturn(HTTPMethods.POST.value()); + given(context.authenticatedUser()).willReturn(appUser); + given(loanRepository.findAllNonClosedLoansBehindByLoanIds( + eq(ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE)), anyList())) + .willReturn(Collections.emptyList()); + + testObj.doFilterInternal(request, response, filterChain); + verify(filterChain, times(1)).doFilter(request, response); + } + @Test void shouldProceedWhenUserHasBypassPermission() throws ServletException, IOException { MockHttpServletRequest request = mock(MockHttpServletRequest.class); @@ -165,12 +194,13 @@ class LoanCOBApiFilterTest { businessDates.put(BusinessDateType.BUSINESS_DATE, businessDate); businessDates.put(BusinessDateType.COB_DATE, businessDate.minusDays(1)); ThreadLocalContextUtil.setBusinessDates(businessDates); - - given(request.getPathInfo()).willReturn("/loans/external-id/2/charges"); + String uuid = UUID.randomUUID().toString(); + given(request.getPathInfo()).willReturn("/loans/external-id/" + uuid + "/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); + given(loanRepository.findIdByExternalId(any())).willReturn(2L); given(loanRepository.findAllNonClosedLoansBehindByLoanIds( eq(ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE)), anyList())) .willReturn(Collections.emptyList());