Merge pull request #13043 from Avishka-Shamendra/apim-governance-core

Fix Governance Bugs and Gateway Visibility Bugs

Author: Avishka-Shamendra

Diff for: components/apimgt/org.wso2.carbon.apimgt.governance.impl/src/main/java/org/wso2/carbon/apimgt/governance/impl/dao/constants/SQLConstants.java

@@ -294,7 +294,7 @@ public class SQLConstants {
 
     public static final String DELETE_REQ_POLICY_MAPPING_FOR_ARTIFACT =
             "DELETE FROM GOV_REQUEST_POLICY " +
-                    "WHERE REQ_ID = (" +
+                    "WHERE REQ_ID IN (" +
                     "    SELECT GR.REQ_ID " +
                     "    FROM GOV_REQUEST GR " +
                     "    JOIN GOV_ARTIFACT GA ON GR.ARTIFACT_KEY = GA.ARTIFACT_KEY " +
@@ -317,8 +317,7 @@ public class SQLConstants {
             "RULESET_ID, RULE_NAME, VIOLATED_PATH, MESSAGE) VALUES (?, ?, ?, ?, ?, ?)";
 
     public static final String DELETE_POLICY_RUN_FOR_ARTIFACT_AND_POLICY = "DELETE FROM GOV_POLICY_RUN " +
-            "WHERE ARTIFACT_KEY IN ( SELECT ARTIFACT_KEY FROM GOV_ARTIFACT WHERE ARTIFACT_REF_ID = ? " +
-            "AND ARTIFACT_TYPE = ? AND ORGANIZATION = ? ) AND POLICY_ID = ?";
+            "WHERE ARTIFACT_KEY = ? AND POLICY_ID = ?";
 
     public static final String DELETE_POLICY_RUN_FOR_POLICY = "DELETE FROM GOV_POLICY_RUN " +
             "WHERE POLICY_ID = ?";
@@ -335,9 +334,11 @@ public class SQLConstants {
             "IN ( SELECT ARTIFACT_KEY FROM GOV_ARTIFACT WHERE ARTIFACT_REF_ID = ? " +
             "AND ARTIFACT_TYPE = ? AND ORGANIZATION = ? )";
 
-    public static final String DELETE_RULESET_RUN_FOR_ARTIFACT_AND_RULESET = "DELETE FROM GOV_RULESET_RUN " +
-            "WHERE ARTIFACT_KEY IN ( SELECT GA.ARTIFACT_KEY FROM GOV_ARTIFACT GA WHERE GA.ARTIFACT_REF_ID = ? " +
-            "AND GA.ARTIFACT_TYPE = ? AND GA.ORGANIZATION = ? ) AND RULESET_ID = ?";
+    public static final String DELETE_RULESET_RUN_FOR_ARTIFACT_AND_POLICY = "DELETE FROM GOV_RULESET_RUN " +
+            "WHERE ARTIFACT_KEY = ? AND RULESET_ID IN ( " +
+            "    SELECT RULESET_ID FROM GOV_POLICY_RULESET " +
+            "    WHERE POLICY_ID = ? " +
+            ")";
 
     public static final String DELETE_RULESET_RUN_FOR_RULESET = "DELETE FROM GOV_RULESET_RUN " +
             "WHERE RULESET_ID = ?";
@@ -363,10 +364,14 @@ public class SQLConstants {
             "WHERE ARTIFACT_KEY IN (SELECT ARTIFACT_KEY FROM GOV_ARTIFACT WHERE ARTIFACT_REF_ID = ? " +
             "AND ARTIFACT_TYPE = ? AND ORGANIZATION = ?)";
 
-    public static final String DELETE_RULE_VIOLATIONS_FOR_ARTIFACT_AND_RULESET = "DELETE FROM GOV_RULE_VIOLATION " +
-            "WHERE RULESET_RUN_ID IN ( SELECT GRR.RULESET_RUN_ID FROM GOV_RULESET_RUN GRR JOIN GOV_ARTIFACT GA " +
-            "ON GRR.ARTIFACT_KEY = GA.ARTIFACT_KEY " +
-            "WHERE GA.ARTIFACT_REF_ID = ? AND GA.ARTIFACT_TYPE = ? AND GA.ORGANIZATION = ? AND GRR.RULESET_ID = ? )";
+    public static final String DELETE_RULE_VIOLATIONS_FOR_ARTIFACT_AND_POLICY = "DELETE FROM GOV_RULE_VIOLATION " +
+            "WHERE RULESET_RUN_ID IN ( " +
+            "    SELECT GRR.RULESET_RUN_ID " +
+            "    FROM GOV_RULESET_RUN GRR " +
+            "    JOIN GOV_POLICY_RULESET GPRS ON GRR.RULESET_ID = GPRS.RULESET_ID " +
+            "    WHERE GRR.ARTIFACT_KEY = ? " +
+            "    AND GPRS.POLICY_ID = ? " +
+            ")";
 
     public static final String DELETE_RULE_VIOLATIONS_FOR_RULESET = "DELETE FROM GOV_RULE_VIOLATION " +
             "WHERE RULESET_ID = ?";

Diff for: components/apimgt/org.wso2.carbon.apimgt.governance.impl/src/main/java/org/wso2/carbon/apimgt/governance/impl/dao/impl/ComplianceMgtDAOImpl.java

@@ -43,13 +43,17 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * This class represents the DAO class related to assessing compliance of APIs
  */
 public class ComplianceMgtDAOImpl implements ComplianceMgtDAO {
 
     private static final Log log = LogFactory.getLog(ComplianceMgtDAOImpl.class);
+    private static final Lock resultWrtiteDelLock = new ReentrantLock();
+
     private ComplianceMgtDAOImpl() {
 
     }
@@ -504,23 +508,25 @@ public void addComplianceEvalResults(String artifactRefId, ArtifactType artifact
                                          Map<String, List<RuleViolation>> rulesetViolationsMap, String organization)
             throws APIMGovernanceException {
 
-        List<String> rulesetIds = new ArrayList<>(rulesetViolationsMap.keySet());
+        resultWrtiteDelLock.lock();
         try (Connection connection = APIMGovernanceDBUtil.getConnection()) {
+            String artifactKey = getArtifactKey(connection, artifactRefId, artifactType, organization);
+            if (artifactKey == null) {
+                throw new APIMGovernanceException(APIMGovExceptionCodes.ARTIFACT_NOT_FOUND, artifactRefId);
+            }
             connection.setAutoCommit(false);
 
             try {
-                clearOldRuleViolations(connection, artifactRefId, artifactType, rulesetIds, organization);
-                clearOldRulesetRuns(connection, artifactRefId, artifactType, rulesetIds, organization);
-                clearOldPolicyRun(connection, artifactRefId, artifactType, policyId, organization);
-                connection.commit();
+                clearOldRuleViolations(connection, artifactKey, policyId);
+                clearOldRulesetRuns(connection, artifactKey, policyId);
+                clearOldPolicyRun(connection, artifactKey, policyId);
+                connection.commit(); // Required to release the locks and avoid deadlocks
 
-                String artifactKey = getArtifactKey(connection, artifactRefId, artifactType, organization);
                 addPolicyRun(connection, artifactKey, policyId);
-
                 for (Map.Entry<String, List<RuleViolation>> entry : rulesetViolationsMap.entrySet()) {
                     String rulesetId = entry.getKey();
                     List<RuleViolation> ruleViolations = entry.getValue();
-                    String rulesetResultId = addRulesetRuns(connection, artifactKey, rulesetId,
+                    String rulesetResultId = addRulesetRun(connection, artifactKey, rulesetId,
                             ruleViolations.isEmpty());
                     addRuleViolations(connection, rulesetResultId, ruleViolations);
                 }
@@ -534,82 +540,64 @@ public void addComplianceEvalResults(String artifactRefId, ArtifactType artifact
         } catch (SQLException e) {
             throw new APIMGovernanceException(APIMGovExceptionCodes.ERROR_WHILE_SAVING_GOVERNANCE_RESULT,
                     e, artifactRefId);
+        } finally {
+            resultWrtiteDelLock.unlock();
         }
     }
 
     /**
      * Clear old policy run results for the artifact
      *
-     * @param connection    Connection
-     * @param artifactRefId Artifact Reference ID (ID of the artifact on APIM side)
-     * @param artifactType  Artifact Type
-     * @param policyId      Policy ID
-     * @param organization  Organization
+     * @param connection  Connection
+     * @param artifactKey Artifact Key
+     * @param policyId    Policy ID
      * @throws SQLException If an error occurs while clearing the old policy result
      */
-    private void clearOldPolicyRun(Connection connection, String artifactRefId, ArtifactType artifactType,
-                                   String policyId, String organization)
+    private void clearOldPolicyRun(Connection connection, String artifactKey, String policyId)
             throws SQLException {
 
         String sqlQuery = SQLConstants.DELETE_POLICY_RUN_FOR_ARTIFACT_AND_POLICY;
         try (PreparedStatement prepStmnt = connection.prepareStatement(sqlQuery)) {
-            prepStmnt.setString(1, artifactRefId);
-            prepStmnt.setString(2, String.valueOf(artifactType));
-            prepStmnt.setString(3, organization);
-            prepStmnt.setString(4, policyId);
+            prepStmnt.setString(1, artifactKey);
+            prepStmnt.setString(2, policyId);
             prepStmnt.executeUpdate();
         }
     }
 
     /**
      * Clear old ruleset runs for the artifact
      *
-     * @param connection    Connection
-     * @param artifactRefId Artifact Reference ID (ID of the artifact on APIM side)
-     * @param artifactType  Artifact Type
-     * @param rulesetIds    List of Ruleset IDs
-     * @param organization  Organization
+     * @param connection  Connection
+     * @param artifactKey Artifact Reference ID (ID of the artifact on APIM side)
+     * @param policyId    Policy ID
      * @throws SQLException If an error occurs while clearing the old ruleset results
      */
-    private void clearOldRulesetRuns(Connection connection, String artifactRefId, ArtifactType artifactType,
-                                     List<String> rulesetIds, String organization) throws SQLException {
+    private void clearOldRulesetRuns(Connection connection, String artifactKey, String policyId) throws SQLException {
 
-        String sqlQuery = SQLConstants.DELETE_RULESET_RUN_FOR_ARTIFACT_AND_RULESET;
+        String sqlQuery = SQLConstants.DELETE_RULESET_RUN_FOR_ARTIFACT_AND_POLICY;
         try (PreparedStatement prepStmnt = connection.prepareStatement(sqlQuery)) {
-            for (String rulesetId : rulesetIds) {
-                prepStmnt.setString(1, artifactRefId);
-                prepStmnt.setString(2, String.valueOf(artifactType));
-                prepStmnt.setString(3, organization);
-                prepStmnt.setString(4, rulesetId);
-                prepStmnt.addBatch();
-            }
-            prepStmnt.executeBatch();
+            prepStmnt.setString(1, artifactKey);
+            prepStmnt.setString(2, policyId);
+            prepStmnt.executeUpdate();
         }
     }
 
     /**
      * Clear rule violations for the artifact and rulesets
      *
-     * @param connection    Connection
-     * @param artifactRefId Artifact Reference ID (ID of the artifact on APIM side)
-     * @param artifactType  Artifact Type
-     * @param rulesetIds    List of Ruleset IDs
-     * @param organization  Organization
+     * @param connection  Connection
+     * @param artifactKey Artifact Key
+     * @param policyId    List of Ruleset IDs
      * @throws SQLException If an error occurs while clearing the rule violations
      */
-    private void clearOldRuleViolations(Connection connection, String artifactRefId, ArtifactType artifactType,
-                                        List<String> rulesetIds, String organization) throws SQLException {
+    private void clearOldRuleViolations(Connection connection, String artifactKey,
+                                        String policyId) throws SQLException {
 
-        String sqlQuery = SQLConstants.DELETE_RULE_VIOLATIONS_FOR_ARTIFACT_AND_RULESET;
+        String sqlQuery = SQLConstants.DELETE_RULE_VIOLATIONS_FOR_ARTIFACT_AND_POLICY;
         try (PreparedStatement prepStmnt = connection.prepareStatement(sqlQuery)) {
-            for (String rulesetId : rulesetIds) {
-                prepStmnt.setString(1, artifactRefId);
-                prepStmnt.setString(2, String.valueOf(artifactType));
-                prepStmnt.setString(3, organization);
-                prepStmnt.setString(4, rulesetId);
-                prepStmnt.addBatch();
-            }
-            prepStmnt.executeBatch();
+            prepStmnt.setString(1, artifactKey);
+            prepStmnt.setString(2, policyId);
+            prepStmnt.execute();
         }
     }
 
@@ -661,7 +649,7 @@ private void addPolicyRun(Connection connection, String artifactKey, String poli
     }
 
     /**
-     * Add a ruleset compliance evaluation result
+     * Add or update a ruleset compliance evaluation result
      *
      * @param connection           Connection
      * @param artifactKey          Artifact Key
@@ -670,8 +658,8 @@ private void addPolicyRun(Connection connection, String artifactKey, String poli
      * @return Ruleset Result ID
      * @throws SQLException If an error occurs while adding the ruleset compliance evaluation result
      */
-    private String addRulesetRuns(Connection connection, String artifactKey, String rulesetId,
-                                  boolean isRulesetEvalSuccess) throws SQLException {
+    private String addRulesetRun(Connection connection, String artifactKey, String rulesetId,
+                                 boolean isRulesetEvalSuccess) throws SQLException {
 
         String sqlQuery = SQLConstants.ADD_RULESET_RUN;
         try (PreparedStatement prepStmnt = connection.prepareStatement(sqlQuery)) {
@@ -1104,6 +1092,8 @@ public List<String> getPendingPoliciesForArtifact(String artifactRefId,
     @Override
     public void deleteArtifact(String artifactRefId, ArtifactType artifactType, String organization)
             throws APIMGovernanceException {
+
+        resultWrtiteDelLock.lock();
         try (Connection connection = APIMGovernanceDBUtil.getConnection()) {
             connection.setAutoCommit(false);
 
@@ -1164,6 +1154,8 @@ public void deleteArtifact(String artifactRefId, ArtifactType artifactType, Stri
         } catch (SQLException e) {
             throw new APIMGovernanceException(APIMGovExceptionCodes
                     .ERROR_WHILE_DELETING_GOVERNANCE_DATA, e, artifactRefId);
+        } finally {
+            resultWrtiteDelLock.unlock();
         }
     }
 }

Diff for: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java

@@ -5847,12 +5847,11 @@ public API getLightweightAPIByUUID(String uuid, String organization) throws APIM
                 checkAccessControlPermission(userNameWithoutChange, api.getAccessControl(),
                         api.getAccessControlRoles());
                 // populate relevant external info environment
-                List<Environment> environments = null;
+                String environmentString = null;
                 if (api.getEnvironments() != null) {
-                    environments = APIUtil.getEnvironmentsOfAPI(api);
+                    environmentString = String.join(",", api.getEnvironments());
                 }
-                api.setEnvironments(APIUtil.extractEnvironmentsForAPI(environments, organization,
-                        userNameWithoutChange));
+                api.setEnvironments(APIUtil.extractEnvironmentsForAPI(environmentString, organization));
                 //CORS . if null is returned, set default config from the configuration
                 if (api.getCorsConfiguration() == null) {
                     api.setCorsConfiguration(APIUtil.getDefaultCorsConfiguration());

Diff for: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/AbstractAPIManager.java

@@ -1211,11 +1211,11 @@ protected void populateAPIInformation(String uuid, String organization, API api)
         Organization org = new Organization(organization);
         api.setOrganization(organization);
         // environment
-        List<Environment> environments = null;
+        String environmentString = null;
         if (api.getEnvironments() != null) {
-            environments = APIUtil.getEnvironmentsOfAPI(api);
+            environmentString = String.join(",", api.getEnvironments());
         }
-        api.setEnvironments(APIUtil.extractEnvironmentsForAPI(environments, organization, username));
+        api.setEnvironments(APIUtil.extractEnvironmentsForAPI(environmentString, organization));
         // workflow status
         APIIdentifier apiId = api.getId();
         WorkflowDTO workflow;