galovics commented on code in PR #2731:
URL: https://github.com/apache/fineract/pull/2731#discussion_r1015304993
##########
fineract-provider/src/main/java/org/apache/fineract/cob/service/LoanAccountLockServiceImpl.java:
##########
@@ -51,4 +51,11 @@ public boolean isLoanHardLocked(Long loanId) {
||
LockOwner.LOAN_INLINE_COB_PROCESSING.equals(loanAccountLock.getLockOwner())) //
.filter(loanAccountLock ->
StringUtils.isBlank(loanAccountLock.getError())).isPresent();
}
+
+ @Override
+ public boolean isLoanSoftLocked(Long loanId) {
+ Optional<LoanAccountLock> loanAccountLockOptional =
loanAccountLockRepository.findById(loanId);
Review Comment:
I think it's not needed to fetch the actual entity. Let's write a custom
repository method for this.
Something like `existsByIdAndLockOwner`
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/SecurityConfig.java:
##########
@@ -90,7 +89,7 @@ protected void configure(HttpSecurity http) throws Exception {
.addFilterAfter(fineractInstanceModeApiFilter,
SecurityContextPersistenceFilter.class) //
.addFilterAfter(tenantAwareBasicAuthenticationFilter(),
FineractInstanceModeApiFilter.class) //
.addFilterAfter(loanCOBApiFilter,
TenantAwareBasicAuthenticationFilter.class) //
- .addFilterAfter(twoFactorAuthenticationFilter,
BasicAuthenticationFilter.class); //
+ .addFilterAfter(twoFactorAuthenticationFilter,
LoanCOBApiFilter.class); //
Review Comment:
So where do the BasicAuthenticationFilter go?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java:
##########
@@ -67,35 +70,51 @@ protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse
Iterable<String> split =
Splitter.on('/').split(request.getPathInfo());
Supplier<Stream<String>> streamSupplier = () ->
StreamSupport.stream(split.spliterator(), false);
boolean isGlim = isGlim(streamSupplier);
- Long loanId = getLoanId(isGlim, streamSupplier);
- if (isLoanLocked(loanId, isGlim)) {
- reject(loanId, response);
+ Long loanIdFromRequest = getLoanId(isGlim, streamSupplier);
+ List<Long> loanIds = isGlim ?
getGlimChildLoanIds(loanIdFromRequest) :
Collections.singletonList(loanIdFromRequest);
+ if (isLoanHardLocked(loanIds)) {
+ reject(loanIdFromRequest, response);
+ } else if (isLoanSoftLocked(loanIds)) {
+ executeInlineCob(loanIds);
+ proceed(filterChain, request, response);
} else {
proceed(filterChain, request, response);
}
}
}
- private boolean isBypassUser() {
- return context.getAuthenticatedUserIfPresent().isBypassUser();
+ private void executeInlineCob(List<Long> loanIds) {
+ inlineLoanCOBExecutorService.lockLoanAccounts(loanIds);
Review Comment:
Hmm, I'm thinking about the responsibilities. I think it'd be better if the
service itself locks the loan accounts, don't you think?
Like that's a unit in itself. You instruct the service "let's run the inline
COB for these loan accounts" and you should expect that everything is taken
care of.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/service/InlineLoanCOBExecutorServiceImpl.java:
##########
@@ -121,7 +121,7 @@ private List<LoanAccountLock>
getLoanAccountLocks(List<Long> loanIds) {
return loanAccountLocks;
}
- private void execute(List<Long> loanIds, String jobName) {
+ public void execute(List<Long> loanIds, String jobName) {
Review Comment:
Can't we add this method to the interface with a generic T type which refers
to the entity ID type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
