FINERACT-274: Not able to create the same datatable which was rejected by the maker checker before

fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java

@@ -32,6 +32,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.DatatableRejectionCleanupService;
 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 +50,7 @@ public class PortfolioCommandSourceWritePlatformServiceImpl implements Portfolio
     private final CommandProcessingService processAndLogCommandService;
     private final SchedulerJobRunnerReadService schedulerJobRunnerReadService;
     private final ConfigurationDomainService configurationService;
+    private final DatatableRejectionCleanupService ifDatatableRejectionThenCleanup;
 
     @Override
     public CommandProcessingResult logCommandSource(final CommandWrapper wrapper) {
@@ -140,9 +142,11 @@ private void validateIsUpdateAllowed() {
     }
 
     @Override
+    @Transactional
     public Long rejectEntry(final Long makerCheckerId) {
         final CommandSource commandSourceInput = validateMakerCheckerTransaction(makerCheckerId);
         validateIsUpdateAllowed();
+        ifDatatableRejectionThenCleanup.cleanup(commandSourceInput);
         final AppUser maker = this.context.authenticatedUser();
         commandSourceInput.markAsRejected(maker);
         this.commandSourceRepository.save(commandSourceInput);

fineract-core/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableRejectionCleanupService.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 DatatableRejectionCleanupService {
+
+    void cleanup(CommandSource commandSource);
+
+}

fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableRejectionCleanupServiceImpl.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 DatatableRejectionCleanupServiceImpl implements DatatableRejectionCleanupService {
+
+    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);
+
+    }
+}

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<String, Boolean> 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<Map<String, Object>> 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
+            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();

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"));
+    }
 }