You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "taskain7 (via GitHub)" <gi...@apache.org> on 2023/04/12 04:57:14 UTC

[GitHub] [fineract] taskain7 opened a new pull request, #3112: [FINERACT-1678] Exclude loan IDs from execution context

taskain7 opened a new pull request, #3112:
URL: https://github.com/apache/fineract/pull/3112

   ## Description
   
   Exclude Loan IDs from Spring Batch execution context, send only the min and the max loan IDs to the worker instance


-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163876681


##########
fineract-provider/src/main/java/org/apache/fineract/cob/data/LoanCOBMinANdMaxLoanId.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.cob.data;
+
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+
+@AllArgsConstructor
+@Getter
+@NoArgsConstructor
+@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
+public class LoanCOBMinANdMaxLoanId {

Review Comment:
   fixed



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163877238


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -46,11 +50,15 @@ public class ApplyLoanLockTasklet implements Tasklet {
     private final LoanAccountLockRepository accountLockRepository;
     private final FineractProperties fineractProperties;
     private final JdbcTemplate jdbcTemplate;
+    private final LoanRepository loanRepository;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         ExecutionContext executionContext = contribution.getStepExecution().getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        List<Long> loanIds = new ArrayList<>(
+                loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBMinANdMaxLoanId.getMinLoanId(),

Review Comment:
   I use it with 0 values, and check it in later steps



##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -36,7 +39,9 @@ public LoanItemReader(LoanRepository loanRepository) {
     @SuppressWarnings({ "unchecked" })
     public void beforeStep(@NotNull StepExecution stepExecution) {
         ExecutionContext executionContext = stepExecution.getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        List<Long> loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBMinANdMaxLoanId.getMinLoanId(),

Review Comment:
   I use it with 0 values, and check 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


[GitHub] [fineract] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163809583


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -46,11 +50,15 @@ public class ApplyLoanLockTasklet implements Tasklet {
     private final LoanAccountLockRepository accountLockRepository;
     private final FineractProperties fineractProperties;
     private final JdbcTemplate jdbcTemplate;
+    private final LoanRepository loanRepository;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         ExecutionContext executionContext = contribution.getStepExecution().getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        List<Long> loanIds = new ArrayList<>(
+                loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBMinANdMaxLoanId.getMinLoanId(),

Review Comment:
   NPE if the `loanCOBMinANdMaxLoanId` is null?



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163877891


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);

Review Comment:
   I use it with 0 values, and check it in later steps



-- 
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] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163804543


##########
fineract-provider/src/main/java/org/apache/fineract/cob/data/LoanCOBMinANdMaxLoanId.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.cob.data;
+
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+
+@AllArgsConstructor
+@Getter
+@NoArgsConstructor
+@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
+public class LoanCOBMinANdMaxLoanId {

Review Comment:
   Typo in the class name, it should be `LoanCOBMinAndMaxLoanId`



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163869860


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);
             return RepeatStatus.FINISHED;
         }
-        List<Long> remainingIds = new ArrayList<>(allNonClosedLoanIds);
-
-        List<List<Long>> remainingIdPartitions = Lists.partition(remainingIds, getInClauseParameterSizeLimit());
-        List<LoanAccountLock> loanAccountLocks = new ArrayList<>();
-        remainingIdPartitions.forEach(
-                remainingIdPartition -> loanAccountLocks.addAll(loanAccountLockRepository.findAllByLoanIdIn(remainingIdPartition)));
-

Review Comment:
   native query, that places the locks covers them



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163839383


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,

Review Comment:
   I think responsibility-wise, this shouldn't be here. Why don't we create a separate tasklet before this one?



##########
fineract-provider/src/main/java/org/apache/fineract/cob/data/LoanCOBMinANdMaxLoanId.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.cob.data;
+
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+
+@AllArgsConstructor
+@Getter
+@NoArgsConstructor
+@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
+public class LoanCOBMinANdMaxLoanId {

Review Comment:
   Why don't we call it what it's used for not what it stores?
   For example LoanCOBInput or LoanCOBParameter or smthing like that.



##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -46,11 +50,15 @@ public class ApplyLoanLockTasklet implements Tasklet {
     private final LoanAccountLockRepository accountLockRepository;
     private final FineractProperties fineractProperties;
     private final JdbcTemplate jdbcTemplate;
+    private final LoanRepository loanRepository;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         ExecutionContext executionContext = contribution.getStepExecution().getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);

Review Comment:
   As you can see, the LoanCOB constant you used here is LOAN_IDS, but that's kinda confusing since it's not Loan IDs anymore. That's why I suggested having a class called LoanCOBInput; representing all the input values for the COB makes more sense.



##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -46,11 +50,15 @@ public class ApplyLoanLockTasklet implements Tasklet {
     private final LoanAccountLockRepository accountLockRepository;
     private final FineractProperties fineractProperties;
     private final JdbcTemplate jdbcTemplate;
+    private final LoanRepository loanRepository;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         ExecutionContext executionContext = contribution.getStepExecution().getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        List<Long> loanIds = new ArrayList<>(
+                loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBMinANdMaxLoanId.getMinLoanId(),

Review Comment:
   I think that's reasonable because if the min/max values are not passed down through the context, we're in a huge pile of ... :)
   The only thing we could do is to throw a different exception in case this is null but doesn't add much value imho.



##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);
             return RepeatStatus.FINISHED;
         }
-        List<Long> remainingIds = new ArrayList<>(allNonClosedLoanIds);
-
-        List<List<Long>> remainingIdPartitions = Lists.partition(remainingIds, getInClauseParameterSizeLimit());
-        List<LoanAccountLock> loanAccountLocks = new ArrayList<>();
-        remainingIdPartitions.forEach(
-                remainingIdPartition -> loanAccountLocks.addAll(loanAccountLockRepository.findAllByLoanIdIn(remainingIdPartition)));
-
-        List<Long> alreadySoftLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_COB_PARTITIONING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
-        List<Long> alreadyMarkedForInlineCOBLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_INLINE_COB_PROCESSING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
-        List<Long> alreadyMarkedForChunkProcessingLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_COB_CHUNK_PROCESSING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
 
-        // Remove already hard locked accounts
-        remainingIds.removeAll(alreadyMarkedForChunkProcessingLockedAccounts);
-        remainingIds.removeAll(alreadyMarkedForInlineCOBLockedAccounts);
+        applySoftLock(businessDate);
 
-        List<Long> lockableLoanAccounts = new ArrayList<>(remainingIds);
-        lockableLoanAccounts.removeAll(alreadySoftLockedAccounts);
-
-        applySoftLock(lockableLoanAccounts);
-
-        contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, remainingIds);
+        contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, minAndMaxLoanId);
 
         return RepeatStatus.FINISHED;
     }
 
-    private void applySoftLock(List<Long> accountsToLock) {
+    private void applySoftLock(LocalDate businessDate) {
         LocalDate cobBusinessDate = ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE);
-        jdbcTemplate.batchUpdate("""
+        jdbcTemplate.update("""

Review Comment:
   Aren't we missing the minLoanId and maxLoanId range filter in this query? In case there's a loan getting created between retrieving the min/max loanIds and the soft locking. The soft locking will take that loan too, but the min/max range will not include it, hence the COB will not pick it up.



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163897558


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/ApplyLoanLockTasklet.java:
##########
@@ -46,11 +50,15 @@ public class ApplyLoanLockTasklet implements Tasklet {
     private final LoanAccountLockRepository accountLockRepository;
     private final FineractProperties fineractProperties;
     private final JdbcTemplate jdbcTemplate;
+    private final LoanRepository loanRepository;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         ExecutionContext executionContext = contribution.getStepExecution().getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);

Review Comment:
   renamed 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


[GitHub] [fineract] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163809195


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);

Review Comment:
   NPE if anyone fetching the `LoanCOBMinANdMaxLoanId` at a later step?



-- 
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] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163817561


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanItemReader.java:
##########
@@ -36,7 +39,9 @@ public LoanItemReader(LoanRepository loanRepository) {
     @SuppressWarnings({ "unchecked" })
     public void beforeStep(@NotNull StepExecution stepExecution) {
         ExecutionContext executionContext = stepExecution.getExecutionContext();
-        List<Long> loanIds = (List<Long>) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        LoanCOBMinANdMaxLoanId loanCOBMinANdMaxLoanId = (LoanCOBMinANdMaxLoanId) executionContext.get(LoanCOBConstant.LOAN_IDS);
+        List<Long> loanIds = loanRepository.findAllNonClosedLoansBehindOrNullByMinAndMaxLoanId(loanCOBMinANdMaxLoanId.getMinLoanId(),

Review Comment:
   NPE if `loanCOBMinANdMaxLoanId` is null?



-- 
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] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163814437


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);
             return RepeatStatus.FINISHED;
         }
-        List<Long> remainingIds = new ArrayList<>(allNonClosedLoanIds);
-
-        List<List<Long>> remainingIdPartitions = Lists.partition(remainingIds, getInClauseParameterSizeLimit());
-        List<LoanAccountLock> loanAccountLocks = new ArrayList<>();
-        remainingIdPartitions.forEach(
-                remainingIdPartition -> loanAccountLocks.addAll(loanAccountLockRepository.findAllByLoanIdIn(remainingIdPartition)));
-

Review Comment:
   why remove all other locking?



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163877559


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepository.java:
##########
@@ -74,7 +75,8 @@ public interface LoanRepository extends JpaRepository<Loan, Long>, JpaSpecificat
 
     String FIND_ALL_NON_CLOSED = "select loan.id from Loan loan where loan.loanStatus in (100,200,300,303,304)";
 
-    String FIND_ALL_NON_CLOSED_LOANS_BY_LAST_CLOSED_BUSINESS_DATE = "select loan.id from Loan loan where loan.loanStatus in (100,200,300,303,304) and (:businessDate = loan.lastClosedBusinessDate or loan.lastClosedBusinessDate is NULL)";
+    String FIND_MIN_AND_MAX_NON_CLOSED_LOAN_IDS_BY_LAST_CLOSED_BUSINESS_DATE = "select new org.apache.fineract.cob.data.LoanCOBMinANdMaxLoanId(min(loan.id), max(loan.id)) from Loan loan where loan.loanStatus in (100,200,300,303,304) "
+            + "and (:businessDate = loan" + ".lastClosedBusinessDate or loan.lastClosedBusinessDate is" + " NULL)";

Review Comment:
   fixed



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163939829


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,
+                businessDate);
+        if (Objects.isNull(minAndMaxLoanId)
+                || (Objects.isNull(minAndMaxLoanId.getMinLoanId()) && Objects.isNull(minAndMaxLoanId.getMaxLoanId()))) {
+            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, null);
             return RepeatStatus.FINISHED;
         }
-        List<Long> remainingIds = new ArrayList<>(allNonClosedLoanIds);
-
-        List<List<Long>> remainingIdPartitions = Lists.partition(remainingIds, getInClauseParameterSizeLimit());
-        List<LoanAccountLock> loanAccountLocks = new ArrayList<>();
-        remainingIdPartitions.forEach(
-                remainingIdPartition -> loanAccountLocks.addAll(loanAccountLockRepository.findAllByLoanIdIn(remainingIdPartition)));
-
-        List<Long> alreadySoftLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_COB_PARTITIONING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
-        List<Long> alreadyMarkedForInlineCOBLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_INLINE_COB_PROCESSING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
-        List<Long> alreadyMarkedForChunkProcessingLockedAccounts = loanAccountLocks.stream()
-                .filter(e -> LockOwner.LOAN_COB_CHUNK_PROCESSING.equals(e.getLockOwner())).map(LoanAccountLock::getLoanId).toList();
 
-        // Remove already hard locked accounts
-        remainingIds.removeAll(alreadyMarkedForChunkProcessingLockedAccounts);
-        remainingIds.removeAll(alreadyMarkedForInlineCOBLockedAccounts);
+        applySoftLock(businessDate);
 
-        List<Long> lockableLoanAccounts = new ArrayList<>(remainingIds);
-        lockableLoanAccounts.removeAll(alreadySoftLockedAccounts);
-
-        applySoftLock(lockableLoanAccounts);
-
-        contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, remainingIds);
+        contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, minAndMaxLoanId);
 
         return RepeatStatus.FINISHED;
     }
 
-    private void applySoftLock(List<Long> accountsToLock) {
+    private void applySoftLock(LocalDate businessDate) {
         LocalDate cobBusinessDate = ThreadLocalContextUtil.getBusinessDateByType(BusinessDateType.COB_DATE);
-        jdbcTemplate.batchUpdate("""
+        jdbcTemplate.update("""

Review Comment:
   I added this filter



-- 
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] adamsaghy merged pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3112:
URL: https://github.com/apache/fineract/pull/3112


-- 
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] adamsaghy commented on a diff in pull request #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163827500


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepository.java:
##########
@@ -74,7 +75,8 @@ public interface LoanRepository extends JpaRepository<Loan, Long>, JpaSpecificat
 
     String FIND_ALL_NON_CLOSED = "select loan.id from Loan loan where loan.loanStatus in (100,200,300,303,304)";
 
-    String FIND_ALL_NON_CLOSED_LOANS_BY_LAST_CLOSED_BUSINESS_DATE = "select loan.id from Loan loan where loan.loanStatus in (100,200,300,303,304) and (:businessDate = loan.lastClosedBusinessDate or loan.lastClosedBusinessDate is NULL)";
+    String FIND_MIN_AND_MAX_NON_CLOSED_LOAN_IDS_BY_LAST_CLOSED_BUSINESS_DATE = "select new org.apache.fineract.cob.data.LoanCOBMinANdMaxLoanId(min(loan.id), max(loan.id)) from Loan loan where loan.loanStatus in (100,200,300,303,304) "
+            + "and (:businessDate = loan" + ".lastClosedBusinessDate or loan.lastClosedBusinessDate is" + " NULL)";

Review Comment:
        + "and (:businessDate = loan" + ".lastClosedBusinessDate or loan.lastClosedBusinessDate is" + " NULL)"
        would you reformat this?



-- 
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 #3112: [FINERACT-1678] Exclude loan IDs from execution context

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3112:
URL: https://github.com/apache/fineract/pull/3112#discussion_r1163939589


##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/FetchAndLockLoanTasklet.java:
##########
@@ -46,67 +40,44 @@ public class FetchAndLockLoanTasklet implements Tasklet {
 
     private static final Long NUMBER_OF_DAYS_BEHIND = 1L;
 
-    private final LoanAccountLockRepository loanAccountLockRepository;
-
     private final RetrieveLoanIdService retrieveLoanIdService;
 
-    private final FineractProperties fineractProperties;
-
     private final JdbcTemplate jdbcTemplate;
 
     @Override
     public RepeatStatus execute(@NotNull StepContribution contribution, @NotNull ChunkContext chunkContext) throws Exception {
         String businessDateParameter = (String) contribution.getStepExecution().getJobExecution().getExecutionContext()
                 .get(LoanCOBConstant.BUSINESS_DATE_PARAMETER_NAME);
         LocalDate businessDate = LocalDate.parse(Objects.requireNonNull(businessDateParameter));
-        List<Long> allNonClosedLoanIds = retrieveLoanIdService.retrieveLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND, businessDate);
-        if (allNonClosedLoanIds.isEmpty()) {
-            contribution.getStepExecution().getJobExecution().getExecutionContext().put(LoanCOBConstant.LOAN_IDS, Collections.emptyList());
+        LoanCOBMinANdMaxLoanId minAndMaxLoanId = retrieveLoanIdService.retrieveMinAndMaxLoanIdsNDaysBehind(NUMBER_OF_DAYS_BEHIND,

Review Comment:
   I split 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