Fix updating action adding updating or removing rule.

wso2 · Jan 8, 2025 · fa24ffe · fa24ffe
1 parent f8f90f2
commit fa24ffe
Show file tree

Hide file tree

Showing 5 changed files with 183 additions and 115 deletions.
diff --git a/...n/java/org/wso2/carbon/identity/action/management/dao/impl/ActionManagementDAOFacade.java b/...n/java/org/wso2/carbon/identity/action/management/dao/impl/ActionManagementDAOFacade.java
@@ -309,7 +309,7 @@ private void addEncryptedAuthSecretsToBuilder(ActionDTOBuilder actionDTOBuilder,
 
     private void addActionRule(ActionDTOBuilder actionDTOBuilder, String tenantDomain) throws ActionMgtException {
 
-        if (actionDTOBuilder.getActionRule() == null) {
+        if (actionDTOBuilder.getActionRule() == null || actionDTOBuilder.getActionRule().getRule() == null) {
             return;
         }
 
@@ -341,25 +341,56 @@ private void loadActionRule(ActionDTOBuilder actionDTOBuilder, String tenantDoma
     }
 
     private void updateActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO,
-                                  String tenantDomain)
-            throws ActionMgtException {
-
-        if (existingActionDTO.getActionRule() == null && updatingActionDTOBuilder.getActionRule() != null) {
-            // This means a new action rule is added when updating the action.
+                                  String tenantDomain) throws ActionMgtException {
+
+         /*
+             When updating an action, the action rule can be added, removed or updated.
+             When action rule is added, the Rule object is added to the ActionRule of the ActionDTO.
+             When action rule is removed, the Rule object is set as null in the ActionRule of the ActionDTO.
+             This happens as the API accepts the removal of the rule in an action update via an empty rule JSON object.
+             e.g., rule: {}. If rule is not present in the payload that means rule is not updated.
+             When action rule is updated, the Rule object is updated in the ActionRule of the ActionDTO.
+        */
+        if (isAddingNewActionRule(updatingActionDTOBuilder, existingActionDTO)) {
             addActionRule(updatingActionDTOBuilder, tenantDomain);
-        } else if (existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() == null) {
-            // This means the existing action rule is removed when updating the action.
+        } else if (isRemovingExistingActionRule(updatingActionDTOBuilder, existingActionDTO)) {
             deleteActionRule(existingActionDTO, tenantDomain);
-        } else if (existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() !=
-                null) {  // This means the existing action rule is updated when updating the action.
-            try {
-                updatingActionDTOBuilder.getActionRule().getRule().setId(existingActionDTO.getActionRule().getId());
-                ActionMgtServiceComponentHolder.getInstance()
-                        .getRuleManagementService()
-                        .updateRule(updatingActionDTOBuilder.getActionRule().getRule(), tenantDomain);
-            } catch (RuleManagementException e) {
-                throw new ActionMgtServerException("Error while updating the Rule associated with the Action.", e);
-            }
+        } else if (isUpdatingExistingActionRule(updatingActionDTOBuilder, existingActionDTO)) {
+            updateExistingActionRule(updatingActionDTOBuilder, existingActionDTO, tenantDomain);
+        }
+    }
+
+    private boolean isAddingNewActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO)
+            throws ActionMgtException {
+
+        return existingActionDTO.getActionRule() == null && updatingActionDTOBuilder.getActionRule() != null &&
+                updatingActionDTOBuilder.getActionRule().getRule() != null;
+    }
+
+    private boolean isRemovingExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder,
+                                                 ActionDTO existingActionDTO) throws ActionMgtException {
+
+        return existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() != null &&
+                updatingActionDTOBuilder.getActionRule().getRule() == null;
+    }
+
+    private boolean isUpdatingExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder,
+                                                 ActionDTO existingActionDTO) throws ActionMgtException {
+
+        return existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() != null &&
+                updatingActionDTOBuilder.getActionRule().getRule() != null;
+    }
+
+    private void updateExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO,
+                                          String tenantDomain) throws ActionMgtException {
+
+        try {
+            updatingActionDTOBuilder.getActionRule().getRule().setId(existingActionDTO.getActionRule().getId());
+            ActionMgtServiceComponentHolder.getInstance()
+                    .getRuleManagementService()
+                    .updateRule(updatingActionDTOBuilder.getActionRule().getRule(), tenantDomain);
+        } catch (RuleManagementException e) {
+            throw new ActionMgtServerException("Error while updating the Rule associated with the Action.", e);
         }
     }