diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java index 5406ac5b445..604a07a7a4d 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java @@ -19,6 +19,7 @@ package org.apache.fineract.commands.service; import com.google.gson.JsonElement; +import java.util.List; import java.util.Objects; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -32,6 +33,7 @@ import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.apache.fineract.infrastructure.dataqueries.service.CleanupService; import org.apache.fineract.infrastructure.jobs.service.SchedulerJobRunnerReadService; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; import org.apache.fineract.useradministration.domain.AppUser; @@ -49,6 +51,7 @@ public class PortfolioCommandSourceWritePlatformServiceImpl implements Portfolio private final CommandProcessingService processAndLogCommandService; private final SchedulerJobRunnerReadService schedulerJobRunnerReadService; private final ConfigurationDomainService configurationService; + private final List cleanupServices; @Override public CommandProcessingResult logCommandSource(final CommandWrapper wrapper) { @@ -146,6 +149,11 @@ public Long rejectEntry(final Long makerCheckerId) { final AppUser maker = this.context.authenticatedUser(); commandSourceInput.markAsRejected(maker); this.commandSourceRepository.save(commandSourceInput); + if (cleanupServices != null) { + for (CleanupService cleanupService : cleanupServices) { + cleanupService.cleanup(commandSourceInput); + } + } return makerCheckerId; } } diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/CleanupService.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/CleanupService.java new file mode 100644 index 00000000000..175a3a18b27 --- /dev/null +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/CleanupService.java @@ -0,0 +1,27 @@ +/** + * 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.dataqueries.service; + +import org.apache.fineract.commands.domain.CommandSource; + +public interface CleanupService { + + void cleanup(CommandSource commandSource); + +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableRejectionCleanupService.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableRejectionCleanupService.java new file mode 100644 index 00000000000..c49fc6f2666 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableRejectionCleanupService.java @@ -0,0 +1,55 @@ +/** + * 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.dataqueries.service; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.fineract.commands.domain.CommandSource; +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.stereotype.Component; + +@Component +@RequiredArgsConstructor +@Slf4j +public class DatatableRejectionCleanupService implements CleanupService { + + private final JdbcTemplate jdbcTemplate; + private final DatabaseSpecificSQLGenerator sqlGenerator; + private final FromJsonHelper fromJsonHelper; + + @Override + public void cleanup(CommandSource commandSource) { + + boolean isCreateAction = "CREATE".equals(commandSource.getActionName()); + boolean isDatatableEntity = "DATATABLE".equals(commandSource.getEntityName()); + if (!isCreateAction || !isDatatableEntity) { + return; + } + + final String datatableName = fromJsonHelper.parse(commandSource.getCommandAsJson()).getAsJsonObject().get("datatableName") + .getAsString(); + + final String sql = "DROP TABLE IF EXISTS " + sqlGenerator.escape(datatableName); + log.info("Cleaning up orphaned datatable after rejection: {}", datatableName); + jdbcTemplate.execute(sql); + + } +} diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/MakercheckerTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/MakercheckerTest.java index 91dd1d097e3..a6a92a64158 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/MakercheckerTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/MakercheckerTest.java @@ -36,16 +36,20 @@ import org.apache.fineract.integrationtests.common.AuditHelper; import org.apache.fineract.integrationtests.common.ClientHelper; import org.apache.fineract.integrationtests.common.CommonConstants; +import org.apache.fineract.integrationtests.common.FineractClientHelper; import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper; import org.apache.fineract.integrationtests.common.Utils; import org.apache.fineract.integrationtests.common.commands.MakercheckersHelper; import org.apache.fineract.integrationtests.common.organisation.StaffHelper; import org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper; import org.apache.fineract.integrationtests.common.savings.SavingsProductHelper; +import org.apache.fineract.integrationtests.common.system.DatatableHelper; import org.apache.fineract.integrationtests.useradministration.roles.RolesHelper; import org.apache.fineract.integrationtests.useradministration.users.UserHelper; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @SuppressWarnings({ "unused" }) public class MakercheckerTest { @@ -195,6 +199,78 @@ public void testMakerCheckerOn() { } } + @ParameterizedTest + @ValueSource(strings = { "m_client", "m_group", "m_center", "m_loan", "m_office", "m_savings_account" }) + public void testRejectDatatableCreationCleansUpOrphanedTable(String apptableName) { + + // enable maker-checker globally + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(true)); + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(false)); + + try { + // enable maker-checker for datatable creation + PutPermissionsRequest putPermissionsRequest = new PutPermissionsRequest().putPermissionsItem("CREATE_DATATABLE", true); + rolesHelper.updatePermissions(putPermissionsRequest); + + // create role with permissions for maker and checker + Integer roleId = RolesHelper.createRole(requestSpec, responseSpec); + Map permissionMap = Map.of("CREATE_DATATABLE", true, "CREATE_DATATABLE_CHECKER", true); + RolesHelper.addPermissionsToRole(requestSpec, responseSpec, roleId, permissionMap); + + // create maker user + Integer staffId = StaffHelper.createStaff(this.requestSpec, this.responseSpec); + String maker = Utils.uniqueRandomStringGenerator("user", 8); + Integer makerUserId = (Integer) UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, maker, + "A1b2c3d4e5f$", "resourceId"); + + // create checker user + String checker = Utils.uniqueRandomStringGenerator("user", 8); + UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, checker, "A1b2c3d4e5f$", "resourceId"); + + RequestSpecification makerRequestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build() + .header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey(maker, "A1b2c3d4e5f$")); + + // maker creates datatable with maker-checker enabled, this creates the physical table but queues for + // approval + DatatableHelper makerDatatableHelper = new DatatableHelper(makerRequestSpec, this.responseSpec); + String datatableJson = DatatableHelper.getTestDatatableAsJSON(apptableName, false); + String datatableName = com.google.gson.JsonParser.parseString(datatableJson).getAsJsonObject().get("datatableName") + .getAsString(); + makerDatatableHelper.createDatatable(datatableJson, ""); + + // find the pending command + List> auditDetails = makercheckersHelper + .getMakerCheckerList(Map.of("actionName", "CREATE", "entityName", "DATATABLE", "makerId", makerUserId.toString())); + assertEquals(1, auditDetails.size(), "Error: Expected only one pending CREATE DATATABLE command"); + Long commandId = ((Double) auditDetails.get(0).get("id")).longValue(); + + // checker rejects the command which should drop the orphaned table + MakercheckersHelper.rejectMakerCheckerEntry(FineractClientHelper.createNewFineractClient(checker, "A1b2c3d4e5f$"), commandId); + + // verify the datatable no longer exists by trying to create it again + // verify without maker checker, so transaction rollback in postgres doesn't break the test + putPermissionsRequest = new PutPermissionsRequest().putPermissionsItem("CREATE_DATATABLE", false); + rolesHelper.updatePermissions(putPermissionsRequest); + + DatatableHelper adminDatatableHelper = new DatatableHelper(this.requestSpec, this.responseSpec); + String recreatedName = adminDatatableHelper.createDatatable(datatableJson, "resourceIdentifier"); + assertEquals(datatableName, recreatedName, "Error: Was not able to recreate datatable after rejection cleanup"); + + // cleanup after test + adminDatatableHelper.deleteDatatable(datatableName); + } finally { + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(false)); + globalConfigurationHelper.updateGlobalConfiguration(GlobalConfigurationConstants.ENABLE_SAME_MAKER_CHECKER, + new PutGlobalConfigurationsRequest().enabled(true)); + + PutPermissionsRequest putPermissionsRequest = new PutPermissionsRequest().putPermissionsItem("CREATE_DATATABLE", false); + rolesHelper.updatePermissions(putPermissionsRequest); + } + } + private Integer createSavingsProductDailyPosting() { final String savingsProductJSON = this.savingsProductHelper.withInterestCompoundingPeriodTypeAsDaily() .withInterestPostingPeriodTypeAsDaily().withInterestCalculationPeriodTypeAsDailyBalance().build(); diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/commands/MakercheckersHelper.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/commands/MakercheckersHelper.java index 9e3c740d7b3..22cbaec5af1 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/commands/MakercheckersHelper.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/commands/MakercheckersHelper.java @@ -26,6 +26,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.fineract.client.models.PostMakerCheckersResponse; +import org.apache.fineract.client.util.Calls; +import org.apache.fineract.client.util.FineractClient; import org.apache.fineract.client.util.JSON; import org.apache.fineract.integrationtests.common.Utils; @@ -80,4 +83,8 @@ public void approveMakerCheckerEntry(Long auditId) { String url = MAKERCHECKER_URL + "/" + auditId + "?command=approve&" + Utils.TENANT_IDENTIFIER; return Utils.performServerPost(requestSpec, responseSpec, url, "", ""); } + + public static PostMakerCheckersResponse rejectMakerCheckerEntry(FineractClient client, Long auditId) { + return Calls.ok(client.makerCheckers.approveMakerCheckerEntry(auditId, "reject")); + } }