From f4979d2c9bc755593e67da703c9a61e27575ae35 Mon Sep 17 00:00:00 2001 From: taskain7 Date: Thu, 29 Sep 2022 13:33:05 +0200 Subject: [PATCH] [FINERACT-1678] business step configuration fix --- .../cob/domain/BatchBusinessStep.java | 3 +- .../domain/BatchBusinessStepRepository.java | 2 + .../BusinessStepConfigUpdateHandler.java | 3 +- .../ConfigJobParameterServiceImpl.java | 39 ++++++++--- .../BusinessStepExceptionMapper.java | 45 ++++++++++++ .../BusinessConfigurationApiTest.java | 70 +++++++++++++++---- .../BusinessStepConfigurationHelper.java | 13 ++++ 7 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/BusinessStepExceptionMapper.java diff --git a/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStep.java b/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStep.java index 43b3511df..82742e182 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStep.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStep.java @@ -30,16 +30,15 @@ import org.apache.fineract.infrastructure.core.domain.AbstractPersistableCustom; @Table(name = "m_batch_business_steps") @NoArgsConstructor @Getter +@Setter public class BatchBusinessStep extends AbstractPersistableCustom { @Column(name = "job_name", nullable = false) private String jobName; - @Setter @Column(name = "step_name", nullable = false) private String stepName; - @Setter @Column(name = "step_order", nullable = false) private Long stepOrder; diff --git a/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStepRepository.java b/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStepRepository.java index d68deb463..014c92162 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStepRepository.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/domain/BatchBusinessStepRepository.java @@ -30,4 +30,6 @@ public interface BatchBusinessStepRepository extends JpaRepository findConfiguredJobNames(); + void deleteAllByJobName(String jobName); + } diff --git a/fineract-provider/src/main/java/org/apache/fineract/cob/service/BusinessStepConfigUpdateHandler.java b/fineract-provider/src/main/java/org/apache/fineract/cob/service/BusinessStepConfigUpdateHandler.java index d113ff26e..c552a3fea 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/cob/service/BusinessStepConfigUpdateHandler.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/service/BusinessStepConfigUpdateHandler.java @@ -21,6 +21,7 @@ package org.apache.fineract.cob.service; import com.google.common.base.Splitter; import java.util.List; import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.exceptions.BusinessStepException; import org.apache.fineract.cob.exceptions.BusinessStepNotBelongsToJobException; import org.apache.fineract.commands.annotation.CommandType; import org.apache.fineract.commands.handler.NewCommandSourceHandler; @@ -38,7 +39,7 @@ public class BusinessStepConfigUpdateHandler implements NewCommandSourceHandler @Override @Transactional - public CommandProcessingResult processCommand(JsonCommand command) throws BusinessStepNotBelongsToJobException { + public CommandProcessingResult processCommand(JsonCommand command) throws BusinessStepNotBelongsToJobException, BusinessStepException { List split = Splitter.on("/").splitToList(command.getUrl()); String jobName = split.get(split.size() - 2); if ("null".equals(jobName)) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/cob/service/ConfigJobParameterServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/cob/service/ConfigJobParameterServiceImpl.java index 71405a472..cc2a0e015 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/cob/service/ConfigJobParameterServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/service/ConfigJobParameterServiceImpl.java @@ -29,22 +29,30 @@ import org.apache.fineract.cob.data.JobBusinessStepConfigData; import org.apache.fineract.cob.data.JobBusinessStepDetail; import org.apache.fineract.cob.domain.BatchBusinessStep; import org.apache.fineract.cob.domain.BatchBusinessStepRepository; +import org.apache.fineract.cob.exceptions.BusinessStepException; import org.apache.fineract.cob.exceptions.BusinessStepNotBelongsToJobException; import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder; +import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.stereotype.Service; @Service @RequiredArgsConstructor -public class ConfigJobParameterServiceImpl implements ConfigJobParameterService { +public class ConfigJobParameterServiceImpl implements ConfigJobParameterService, InitializingBean { private final BatchBusinessStepRepository batchBusinessStepRepository; private final BusinessStepConfigDataParser dataParser; private final BusinessStepCategoryService businessStepCategoryService; private final ApplicationContext applicationContext; private final BusinessStepMapper mapper; + private JobBusinessStepDetail availableBusinessStepsForLoan; + + @Override + public void afterPropertiesSet() throws Exception { + availableBusinessStepsForLoan = getAvailableBusinessStepsByJobName(BusinessStepCategory.LOAN.name()); + } @Override public JobBusinessStepConfigData getBusinessStepConfigByJobName(String jobName) { @@ -59,15 +67,25 @@ public class ConfigJobParameterServiceImpl implements ConfigJobParameterService public CommandProcessingResult updateStepConfigByJobName(JsonCommand command, String jobName) throws BusinessStepNotBelongsToJobException { List businessSteps = dataParser.parseUpdate(command); - List batchBusinessSteps = batchBusinessStepRepository.findAllByJobName(jobName); - businessSteps.forEach(newBusinessStepConfig -> { - BatchBusinessStep batchBusinessStep = batchBusinessSteps.stream() - .filter(oldBusinessStepConfig -> oldBusinessStepConfig.getStepName().equals(newBusinessStepConfig.getStepName())) - .findFirst().orElseThrow(BusinessStepNotBelongsToJobException::new); - batchBusinessStep.setStepName(newBusinessStepConfig.getStepName()); - batchBusinessStep.setStepOrder(newBusinessStepConfig.getOrder()); - batchBusinessStepRepository.save(batchBusinessStep); - }); + if (businessSteps.isEmpty()) { + throw new BusinessStepException("A job needs to have 1 business step at least."); + } + List availableBusinessStepNames = availableBusinessStepsForLoan.getAvailableBusinessSteps().stream() + .map(BusinessStepDetail::getStepName).toList(); + List notValidBusinessStepNames = businessSteps.stream().map(BusinessStep::getStepName) + .filter(businessStepName -> !availableBusinessStepNames.contains(businessStepName)).toList(); + if (notValidBusinessStepNames.isEmpty()) { + batchBusinessStepRepository.deleteAllByJobName(jobName); + businessSteps.forEach(newBusinessStepConfig -> { + BatchBusinessStep batchBusinessStep = new BatchBusinessStep(); + batchBusinessStep.setJobName(jobName); + batchBusinessStep.setStepName(newBusinessStepConfig.getStepName()); + batchBusinessStep.setStepOrder(newBusinessStepConfig.getOrder()); + batchBusinessStepRepository.save(batchBusinessStep); + }); + } else { + throw new BusinessStepException(notValidBusinessStepNames + " Business steps are not configurable for this job."); + } return new CommandProcessingResultBuilder().withCommandId(command.commandId()).build(); } @@ -96,5 +114,4 @@ public class ConfigJobParameterServiceImpl implements ConfigJobParameterService public List getAllConfiguredJobNames() { return batchBusinessStepRepository.findConfiguredJobNames(); } - } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/BusinessStepExceptionMapper.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/BusinessStepExceptionMapper.java new file mode 100644 index 000000000..b489ce63b --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/BusinessStepExceptionMapper.java @@ -0,0 +1,45 @@ +/** + * 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.infrastructure.core.exceptionmapper; + +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; +import lombok.extern.slf4j.Slf4j; +import org.apache.fineract.cob.exceptions.BusinessStepException; +import org.apache.fineract.infrastructure.core.data.ApiParameterError; +import org.springframework.stereotype.Component; + +@Provider +@Component +@Slf4j +public class BusinessStepExceptionMapper implements ExceptionMapper { + + @Override + public Response toResponse(BusinessStepException exception) { + final String globalisationMessageCode = "error.msg.invalid.request.body"; + final String defaultUserMessage = exception.getMessage(); + log.warn("Exception: {}, Message: {}", exception.getClass().getName(), defaultUserMessage); + + final ApiParameterError error = ApiParameterError.parameterError(globalisationMessageCode, defaultUserMessage, "stepName"); + + return Response.status(Response.Status.BAD_REQUEST).entity(error).type(MediaType.APPLICATION_JSON).build(); + } +} diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java index 8f1031396..d6e9ef859 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java @@ -27,10 +27,12 @@ import io.restassured.http.ContentType; import io.restassured.specification.RequestSpecification; import io.restassured.specification.ResponseSpecification; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.fineract.cob.data.BusinessStep; import org.apache.fineract.cob.data.JobBusinessStepConfigData; import org.apache.fineract.cob.data.JobBusinessStepDetail; +import org.apache.fineract.infrastructure.core.data.ApiParameterError; import org.apache.fineract.integrationtests.common.BusinessStepConfigurationHelper; import org.apache.fineract.integrationtests.common.Utils; import org.junit.jupiter.api.Assertions; @@ -44,6 +46,7 @@ public class BusinessConfigurationApiTest { public static final String LOAN_JOB_NAME = "LOAN_CLOSE_OF_BUSINESS"; public static final String LOAN_CATEGORY_NAME = "loan"; public static final String APPLY_CHARGE_TO_OVERDUE_LOANS = "APPLY_CHARGE_TO_OVERDUE_LOANS"; + public static final String NOT_BELONGING_BUSINESS_STEP_NAME = "APPLY_CHARGE_TO_OVERDUE_LOANS_2"; public static final String LOAN_DELINQUENCY_CLASSIFICATION = "LOAN_DELINQUENCY_CLASSIFICATION"; @BeforeEach @@ -82,32 +85,69 @@ public class BusinessConfigurationApiTest { public void shouldUpdateStepOrder() { JobBusinessStepConfigData originalStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); - Long originalOrder = originalStepConfig.getBusinessSteps().stream() // - .filter(businessStep -> APPLY_CHARGE_TO_OVERDUE_LOANS.equals(businessStep.getStepName())) // - .findFirst().get().getOrder(); - List requestBody = getBusinessSteps(originalOrder + 1); + List requestBody = new ArrayList<>(); + requestBody.add(getBusinessSteps(1L, APPLY_CHARGE_TO_OVERDUE_LOANS)); BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); JobBusinessStepConfigData newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); - Long newOrder = newStepConfig.getBusinessSteps().stream() // - .filter(businessStep -> APPLY_CHARGE_TO_OVERDUE_LOANS.equals(businessStep.getStepName())) // - .findFirst().get().getOrder(); - assertEquals(originalOrder + 1, newOrder); + BusinessStep applyChargeStep = newStepConfig.getBusinessSteps().stream() + .filter(businessStep -> APPLY_CHARGE_TO_OVERDUE_LOANS.equals(businessStep.getStepName())).findFirst().get(); + assertEquals(1, newStepConfig.getBusinessSteps().size()); + assertEquals(1L, applyChargeStep.getOrder()); + + requestBody.add(getBusinessSteps(2L, LOAN_DELINQUENCY_CLASSIFICATION)); - List originalBusinessSteps = getBusinessSteps(originalOrder); BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, - BusinessStepConfigurationHelper.toJsonString(originalBusinessSteps)); + BusinessStepConfigurationHelper.toJsonString(requestBody)); + newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); + applyChargeStep = newStepConfig.getBusinessSteps().stream() + .filter(businessStep -> APPLY_CHARGE_TO_OVERDUE_LOANS.equals(businessStep.getStepName())).findFirst().get(); + BusinessStep loanDelinquencyStep = newStepConfig.getBusinessSteps().stream() + .filter(businessStep -> LOAN_DELINQUENCY_CLASSIFICATION.equals(businessStep.getStepName())).findFirst().get(); + assertEquals(2, newStepConfig.getBusinessSteps().size()); + assertEquals(1L, applyChargeStep.getOrder()); + assertEquals(2L, loanDelinquencyStep.getOrder()); + + requestBody.remove(1); + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.toJsonString(requestBody)); + + newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); + applyChargeStep = newStepConfig.getBusinessSteps().stream() + .filter(businessStep -> APPLY_CHARGE_TO_OVERDUE_LOANS.equals(businessStep.getStepName())).findFirst().get(); + assertEquals(1, newStepConfig.getBusinessSteps().size()); + assertEquals(1L, applyChargeStep.getOrder()); + + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.toJsonString(originalStepConfig.getBusinessSteps())); } - private List getBusinessSteps(Long order) { - List businessSteps = new ArrayList<>(); + @Test + public void shouldThrowErrorWhenABusinessStepDoesNotBelongToTheGivenJob() { + ResponseSpecification responseSpecForError = new ResponseSpecBuilder().expectStatusCode(400).build(); + List requestBody = new ArrayList<>(); + requestBody.add(getBusinessSteps(1L, NOT_BELONGING_BUSINESS_STEP_NAME)); + ApiParameterError response = BusinessStepConfigurationHelper.updateBusinessStepOrderWithError(requestSpec, responseSpecForError, + LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); + assertEquals("[APPLY_CHARGE_TO_OVERDUE_LOANS_2] Business steps are not configurable for this job.", response.getDeveloperMessage()); + } + + @Test + public void shouldThrowErrorWhenBusinessStepListIsEmpty() { + ResponseSpecification responseSpecForError = new ResponseSpecBuilder().expectStatusCode(400).build(); + List requestBody = Collections.emptyList(); + ApiParameterError response = BusinessStepConfigurationHelper.updateBusinessStepOrderWithError(requestSpec, responseSpecForError, + LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); + assertEquals("A job needs to have 1 business step at least.", response.getDeveloperMessage()); + } + + private BusinessStep getBusinessSteps(Long order, String stepName) { BusinessStep businessStep = new BusinessStep(); - businessStep.setStepName(APPLY_CHARGE_TO_OVERDUE_LOANS); + businessStep.setStepName(stepName); businessStep.setOrder(order); - businessSteps.add(businessStep); - return businessSteps; + return businessStep; } } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BusinessStepConfigurationHelper.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BusinessStepConfigurationHelper.java index 151f15791..a175e9a8f 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BusinessStepConfigurationHelper.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BusinessStepConfigurationHelper.java @@ -27,6 +27,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.fineract.cob.data.BusinessStep; import org.apache.fineract.cob.data.JobBusinessStepConfigData; import org.apache.fineract.cob.data.JobBusinessStepDetail; +import org.apache.fineract.infrastructure.core.data.ApiParameterError; @Slf4j public final class BusinessStepConfigurationHelper { @@ -47,6 +48,10 @@ public final class BusinessStepConfigurationHelper { return new Gson().fromJson(json, new TypeToken() {}.getType()); } + private static ApiParameterError configuredApiParameterErrorFromJsonString(final String json) { + return new Gson().fromJson(json, new TypeToken() {}.getType()); + } + private static JobBusinessStepDetail availableBusinessStepFromJsonString(final String json) { return new Gson().fromJson(json, new TypeToken() {}.getType()); } @@ -74,6 +79,14 @@ public final class BusinessStepConfigurationHelper { log.info("BusinessStepConfigurationHelper Response: {}", response); } + public static ApiParameterError updateBusinessStepOrderWithError(final RequestSpecification requestSpec, + final ResponseSpecification responseSpec, String jobName, String jsonBodyToSend) { + String response = Utils.performServerPut(requestSpec, responseSpec, + BUSINESS_STEPS_API_URL_START + jobName + BUSINESS_STEPS_API_URL_END, jsonBodyToSend); + log.info("BusinessStepConfigurationHelper Response: {}", response); + return BusinessStepConfigurationHelper.configuredApiParameterErrorFromJsonString(response); + } + private static final class BusinessStepWrapper { private List businessSteps;