This is an automated email from the ASF dual-hosted git repository.
taskain pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new 8090c614e FINERACT-1724 Loan COB Api Filter external id fix - [x] UUID
az external id to test non-numeric id - [x] Unit test
8090c614e is described below
commit 8090c614ed2e5b599528ef1461e2a744676adf88
Author: Janos Haber <[email protected]>
AuthorDate: Wed Mar 1 12:08:56 2023 +0100
FINERACT-1724 Loan COB Api Filter external id fix
- [x] UUID az external id to test non-numeric id
- [x] Unit test
---
.../jobs/filter/LoanCOBApiFilter.java | 38 +++++++++++++++----
.../jobs/filter/LoanCOBApiFilterTest.java | 44 ++++++++++++++++++----
2 files changed, 68 insertions(+), 14 deletions(-)
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<HttpMethod> 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<String> URL_FUNCTION = s ->
LOAN_PATH_PATTERN.matcher(s).find()
@@ -142,16 +143,27 @@ public class LoanCOBApiFilter extends
OncePerRequestFilter implements BatchFilte
private List<Long> calculateRelevantLoanIds(String pathInfo) {
- boolean isGlim = isGlim(pathInfo);
- Long loanIdFromRequest = getLoanId(isGlim, pathInfo);
- List<Long> loanIds = isGlim ? getGlimChildLoanIds(loanIdFromRequest) :
Collections.singletonList(loanIdFromRequest);
+ List<Long> loanIds = getLoanIdList(pathInfo);
if (isLoanHardLocked(loanIds)) {
- throw new LoanIdsHardLockedException(loanIdFromRequest);
+ throw new LoanIdsHardLockedException(loanIds.get(0));
} else {
return loanIds;
}
}
+ private List<Long> 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<Long> 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<Long> 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<BusinessDateType, LocalDate> 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());
