[MNT-23816] Prevent rules aspect removal when there are existing rules (#2114) (#2139)

* [MNT-23816] Prevent rules aspect removal when there are existing rules

(cherry picked from commit 1d56eb1dd1)

remote-api/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java

@@ -68,6 +68,7 @@ import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.rendition2.RenditionDefinition2;
 import org.alfresco.repo.rendition2.RenditionDefinitionRegistry2;
 import org.alfresco.repo.rendition2.RenditionService2;
+import org.alfresco.repo.rule.RuleModel;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.repo.security.permissions.AccessDeniedException;
@@ -148,6 +149,7 @@ import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.Path;
 import org.alfresco.service.cmr.repository.Path.Element;
 import org.alfresco.service.cmr.repository.StoreRef;
+import org.alfresco.service.cmr.rule.RuleService;
 import org.alfresco.service.cmr.security.AccessPermission;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.cmr.security.AuthorityService;
@@ -216,6 +218,7 @@ public class NodesImpl implements Nodes
     private LockService lockService;
     private VirtualStore smartStore; // note: remove as part of REPO-1173
     private ClassDefinitionMapper classDefinitionMapper;
+    private RuleService ruleService;
 
     private enum Activity_Type
     {
@@ -338,6 +341,11 @@ public class NodesImpl implements Nodes
         this.classDefinitionMapper = classDefinitionMapper;
     }
 
+    public void setRuleService(RuleService ruleService)
+    {
+        this.ruleService = ruleService;
+    }
+
     // excluded namespaces (aspects, properties, assoc types)
     private static final List<String> EXCLUDED_NS = Arrays.asList(NamespaceService.SYSTEM_MODEL_1_0_URI);
 
@@ -2543,6 +2551,8 @@ public class NodesImpl implements Nodes
             Set<QName> aspectsToAdd = new HashSet<>(3);
             Set<QName> aspectsToRemove = new HashSet<>(3);
 
+            boolean hasRules = ruleService.hasRules(nodeRef);
+
             for (QName aspectQName : aspectQNames)
             {
                 if (EXCLUDED_NS.contains(aspectQName.getNamespaceURI()) || excludedAspects.contains(aspectQName) || aspectQName.equals(ContentModel.ASPECT_AUDITABLE))
@@ -2558,7 +2568,7 @@ public class NodesImpl implements Nodes
 
             for (QName existingAspect : existingAspects)
             {
-                if (EXCLUDED_NS.contains(existingAspect.getNamespaceURI()) || excludedAspects.contains(existingAspect) || existingAspect.equals(ContentModel.ASPECT_AUDITABLE))
+                if (EXCLUDED_NS.contains(existingAspect.getNamespaceURI()) || excludedAspects.contains(existingAspect) || existingAspect.equals(ContentModel.ASPECT_AUDITABLE) || existingAspect.equals(RuleModel.ASPECT_RULES) && hasRules)
                 {
                     continue; // ignore
                 }

remote-api/src/main/resources/alfresco/public-rest-context.xml

@@ -537,6 +537,7 @@
         <property name="poster" ref="activitiesPoster" />
         <property name="smartStore" ref="smartStore"/>
         <property name="classDefinitionMapper" ref="classDefinitionMapper" />
+        <property name="ruleService" ref="RuleService" />
     </bean>
 
     <bean id="Nodes" class="org.springframework.aop.framework.ProxyFactoryBean">

remote-api/src/test/java/org/alfresco/rest/api/tests/NodeApiTest.java

@@ -49,8 +49,10 @@ import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
 import org.alfresco.model.ContentModel;
+import org.alfresco.repo.action.executer.AddFeaturesActionExecuter;
 import org.alfresco.repo.content.ContentLimitProvider.SimpleFixedLimitProvider;
 import org.alfresco.repo.content.MimetypeMap;
+import org.alfresco.repo.rule.RuleModel;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.tenant.TenantService;
 import org.alfresco.repo.tenant.TenantUtil;
@@ -83,11 +85,16 @@ import org.alfresco.rest.api.tests.util.MultiPartBuilder;
 import org.alfresco.rest.api.tests.util.MultiPartBuilder.FileData;
 import org.alfresco.rest.api.tests.util.MultiPartBuilder.MultiPartRequest;
 import org.alfresco.rest.api.tests.util.RestApiUtil;
+import org.alfresco.service.cmr.action.Action;
+import org.alfresco.service.cmr.action.ActionService;
 import org.alfresco.service.cmr.lock.LockType;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.Path;
 import org.alfresco.service.cmr.repository.StoreRef;
+import org.alfresco.service.cmr.rule.Rule;
+import org.alfresco.service.cmr.rule.RuleService;
+import org.alfresco.service.cmr.rule.RuleType;
 import org.alfresco.service.cmr.security.AccessPermission;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.cmr.security.AuthorityService;
@@ -130,7 +137,8 @@ public class NodeApiTest extends AbstractSingleNetworkSiteTest
     protected AuthorityService authorityService;
     private NodeService nodeService;
     private NamespaceService namespaceService;
-
+    private RuleService ruleService;
+    private ActionService actionService;
 
     private String rootGroupName = null;
     private String groupA = null;
@@ -145,6 +153,8 @@ public class NodeApiTest extends AbstractSingleNetworkSiteTest
         authorityService = (AuthorityService) applicationContext.getBean("AuthorityService");
         nodeService = applicationContext.getBean("NodeService", NodeService.class);
         namespaceService= (NamespaceService) applicationContext.getBean("NamespaceService");
+        ruleService = (RuleService) applicationContext.getBean("RuleService", RuleService.class);
+        actionService = (ActionService) applicationContext.getBean("ActionService", ActionService.class);
     }
     
     @After
@@ -6398,5 +6408,42 @@ public class NodeApiTest extends AbstractSingleNetworkSiteTest
         setRequestContext(user1);
         deleteSite(site1Id, true, 204);
     }
+
+    @Test
+    public void testRuleAspectAfterUpdate() throws Exception
+    {
+        // Change to User1 context
+        setRequestContext(networkOne.getId(), user1, null);
+        AuthenticationUtil.setFullyAuthenticatedUser(user1);
+
+        // Create folder
+        String folderId = createUniqueFolder(getMyNodeId());
+        NodeRef folderNodeRef = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, folderId);
+
+        assertFalse("Folder shouldn't have the rule aspect", nodeService.hasAspect(folderNodeRef, RuleModel.ASPECT_RULES));
+
+        // Create Rule
+        Rule rule = new Rule();
+        rule.setTitle("My Rule");
+        rule.setRuleType(RuleType.INBOUND);
+        Action action = this.actionService.createAction(AddFeaturesActionExecuter.NAME);
+        action.setParameterValue(AddFeaturesActionExecuter.PARAM_ASPECT_NAME, ContentModel.ASPECT_CLASSIFIABLE);
+        rule.setAction(action);
+        this.ruleService.saveRule(folderNodeRef, rule);
+
+        assertTrue("Folder should have the rule aspect", nodeService.hasAspect(folderNodeRef, RuleModel.ASPECT_RULES));
+
+        // Update folder with empty aspectNames
+        Folder folderUpdate = new Folder();
+        folderUpdate.setAspectNames(Arrays.asList());
+
+        put(URL_NODES, folderId, toJsonAsStringNonNull(folderUpdate), null, 200);
+
+        assertTrue("Folder should have the rule aspect", nodeService.hasAspect(folderNodeRef, RuleModel.ASPECT_RULES));
+        assertEquals("Folder should have 1 rule.", 1, ruleService.countRules(folderNodeRef));
+
+        // Cleanup
+        delete(URL_NODES, folderId, 204);
+    }
 }