galovics commented on code in PR #3154:
URL: https://github.com/apache/fineract/pull/3154#discussion_r1187412071
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanCatchUpSupport.java:
##########
@@ -18,16 +18,15 @@
*/
package org.apache.fineract.cob.loan;
-import org.springframework.batch.core.StepContribution;
+import org.apache.fineract.cob.common.CustomJobParameterResolver;
+import org.apache.fineract.cob.exceptions.CustomJobParameterNotFoundException;
import org.springframework.batch.core.StepExecution;
public interface LoanCatchUpSupport {
- default boolean isCatchUp(StepContribution contribution) {
- return isCatchUp(contribution.getStepExecution());
- }
-
- default boolean isCatchUp(StepExecution execution) {
- return
"true".equalsIgnoreCase(execution.getExecutionContext().getString(LoanCOBConstant.IS_CATCH_UP_PARAMETER_NAME,
"false"));
+ default boolean isCatchUp(CustomJobParameterResolver resolver,
StepExecution execution) {
+ return resolver.getCustomJobParameterSet(execution,
CustomJobParameterNotFoundException::new).stream()
+ .filter(parameter ->
LoanCOBConstant.IS_CATCH_UP_PARAMETER_NAME.equals(parameter.getParameterName())).findFirst()
+ .map(jobParameter ->
"true".equalsIgnoreCase(jobParameter.getParameterValue())).orElse(false);
Review Comment:
I think checking for "true" as a string is not necessarily the best idea.
why don't we convert the value to a boolean and then check against the bool
true value?
##########
fineract-provider/src/main/java/org/apache/fineract/cob/common/CustomJobParameterResolver.java:
##########
@@ -50,20 +56,45 @@ public void afterPropertiesSet() throws Exception {
public void resolve(StepContribution contribution, ChunkContext
chunkContext, String customJobParameterKey,
String parameterNameInExecutionContext) {
- Set<JobParameterDTO> jobParameterDTOList =
getCustomJobParameterSet(chunkContext);
+ Set<JobParameterDTO> jobParameterDTOList =
getCustomJobParameterSet(chunkContext.getStepContext().getStepExecution(),
+ CustomJobParameterNotFoundException::new);
JobParameterDTO businessDateParameter = jobParameterDTOList.stream()
.filter(jobParameterDTO ->
customJobParameterKey.equals(jobParameterDTO.getParameterName())) //
.findFirst().orElseThrow(() -> new
CustomJobParameterNotFoundException(customJobParameterKey));
contribution.getStepExecution().getExecutionContext().put(parameterNameInExecutionContext,
businessDateParameter.getParameterValue());
}
- private Set<JobParameterDTO> getCustomJobParameterSet(ChunkContext
chunkContext) {
- Long customJobParameterId = (Long)
chunkContext.getStepContext().getJobParameters()
- .get(SpringBatchJobConstants.CUSTOM_JOB_PARAMETER_ID_KEY);
+ /**
+ * Get parameter set from custom job parameter table
+ *
+ * @param stepExecution
+ * @param keyNotFoundExceptionResolver
+ * @return
+ */
+ public Set<JobParameterDTO> getCustomJobParameterSet(StepExecution
stepExecution,
+ LongFunction<RuntimeException> keyNotFoundExceptionResolver) {
Review Comment:
I think with passing the exception function, you're actually hiding a very
important line between layers.
If a custom job parameter is not found, the appropriate exception shall be
thrown. In other layers where this resolver is used, you could of course catch
this CustomJobParameterNotFound exception and transform it to the layer's
appropriate one (for example I saw a LoanNotFoundException somewhere).
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanCatchUpSupport.java:
##########
@@ -18,16 +18,15 @@
*/
package org.apache.fineract.cob.loan;
-import org.springframework.batch.core.StepContribution;
+import org.apache.fineract.cob.common.CustomJobParameterResolver;
+import org.apache.fineract.cob.exceptions.CustomJobParameterNotFoundException;
import org.springframework.batch.core.StepExecution;
public interface LoanCatchUpSupport {
Review Comment:
Btw why interface and why don't we use a simple bean instead of this? That
way we don't even need to pass the resolver as a param.
--
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]
