You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/11/07 10:19:48 UTC
[GitHub] [fineract] taskain7 opened a new pull request, #2731: [FINERACT-1678] Introducing synchronous COB execution for loan account changing APIs
taskain7 opened a new pull request, #2731:
URL: https://github.com/apache/fineract/pull/2731
## Description
When a loan is soft locked by COB partitioning during write operation, then the Inline COB runs on the loan instead of regular COB and after that the write operation goes through.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2731: [FINERACT-1678] Introducing synchronous COB execution for loan account changing APIs
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2731: [FINERACT-1678] Introducing synchronous COB execution for loan account changing APIs
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2731:
URL: https://github.com/apache/fineract/pull/2731#discussion_r1015663181
##########
fineract-provider/src/main/java/org/apache/fineract/cob/service/InlineLoanCOBExecutorServiceImpl.java:
##########
@@ -66,7 +66,7 @@
@Service
@Slf4j
@RequiredArgsConstructor
-public class InlineLoanCOBExecutorServiceImpl implements InlineExecutorService, InitializingBean {
+public class InlineLoanCOBExecutorServiceImpl implements InlineExecutorService<List<Long>>, InitializingBean {
Review Comment:
What if we simply don't specify the List part of the generic parameter? Just the Long. That way all the interface implementors will have the same List input type for the method.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/InlineExecutorService.java:
##########
@@ -21,7 +21,9 @@
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
-public interface InlineExecutorService {
+public interface InlineExecutorService<T> {
CommandProcessingResult executeInlineJob(JsonCommand command, String jobName);
+
+ void execute(T id, String jobName);
Review Comment:
So rather this should look like:
```
void execute(List<T> elements, String jobName);
```
Plus, it'd be good to provide a way to do the same thing for a single item, so like we could add this:
```
default void execute(T element, String jobName) {
execute(singletonList(element), jobName)
}
```
Thoughts?
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2731: [FINERACT-1678] Introducing synchronous COB execution for loan account changing APIs
Posted by GitBox <gi...@apache.org>.
galovics merged PR #2731:
URL: https://github.com/apache/fineract/pull/2731
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] taskain7 commented on a diff in pull request #2731: [FINERACT-1678] Introducing synchronous COB execution for loan account changing APIs
Posted by GitBox <gi...@apache.org>.
taskain7 commented on code in PR #2731:
URL: https://github.com/apache/fineract/pull/2731#discussion_r1016374509
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/InlineExecutorService.java:
##########
@@ -21,7 +21,9 @@
import org.apache.fineract.infrastructure.core.api.JsonCommand;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
-public interface InlineExecutorService {
+public interface InlineExecutorService<T> {
CommandProcessingResult executeInlineJob(JsonCommand command, String jobName);
+
+ void execute(T id, String jobName);
Review Comment:
makes sense, I've chenged it.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org