ACS-3377 Update rule order.

packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/ReorderRules.java

@@ -29,14 +29,18 @@ import static java.util.stream.Collectors.toList;
 
 import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModel;
 import static org.alfresco.utility.report.log.Step.STEP;
+import static org.springframework.http.HttpStatus.FORBIDDEN;
 import static org.springframework.http.HttpStatus.OK;
 
 import java.util.List;
 import java.util.stream.IntStream;
 
+import com.google.common.collect.Lists;
+
 import org.alfresco.rest.RestTest;
 import org.alfresco.rest.model.RestRuleModel;
 import org.alfresco.rest.model.RestRuleSetModel;
+import org.alfresco.utility.constants.UserRole;
 import org.alfresco.utility.model.FolderModel;
 import org.alfresco.utility.model.SiteModel;
 import org.alfresco.utility.model.TestGroup;
@@ -64,10 +68,7 @@ public class ReorderRules extends RestTest
     {
         STEP("Create a folder containing three rules in the existing site");
         FolderModel folder = dataContent.usingUser(user).usingSite(site).createFolder();
-        List<RestRuleModel> rules = IntStream.range(0, 3).mapToObj(index -> {
+        List<RestRuleModel> rules = createRulesInFolder(folder, user);
-            RestRuleModel ruleModel = createRuleModel("ruleName");
-            return restClient.authenticateUser(user).withCoreAPI().usingNode(folder).usingDefaultRuleSet().createSingleRule(ruleModel);
-        }).collect(toList());
 
         STEP("Get the default rule set for the folder including the ordered rule ids");
         RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(folder)
@@ -77,4 +78,127 @@ public class ReorderRules extends RestTest
         restClient.assertStatusCodeIs(OK);
         ruleSet.assertThat().field("ruleIds").is(expectedRuleIds);
     }
+
+    /** Check that a user can view the order of the rules in a rule set if they only have read permission. */
+    @Test
+    public void getRuleSetAndRuleIdsWithReadOnlyPermission()
+    {
+        STEP("Create a site owned by admin and add user as a consumer");
+        SiteModel siteModel = dataSite.usingAdmin().createPrivateRandomSite();
+        dataUser.addUserToSite(user, siteModel, UserRole.SiteConsumer);
+
+        STEP("Use admin to create a folder with a rule set and three rules in it");
+        FolderModel ruleFolder = dataContent.usingAdmin().usingSite(siteModel).createFolder();
+        dataContent.usingAdmin().usingResource(ruleFolder).createFolder();
+        List<RestRuleModel> rules = createRulesInFolder(ruleFolder, dataUser.getAdminUser());
+
+        STEP("Get the rule set with the ordered list of rules");
+        RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder)
+                                             .include("ruleIds").getDefaultRuleSet();
+
+        restClient.assertStatusCodeIs(OK);
+        List<String> ruleIds = rules.stream().map(RestRuleModel::getId).collect(toList());
+        ruleSet.assertThat().field("ruleIds").is(ruleIds);
+    }
+
+    /** Check we can reorder the rules in a rule set. */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void reorderRules()
+    {
+        STEP("Create a folder containing three rules in the existing site");
+        FolderModel folder = dataContent.usingUser(user).usingSite(site).createFolder();
+        List<RestRuleModel> rules = createRulesInFolder(folder, user);
+
+        STEP("Reverse the order of the rules within the rule set");
+        List<String> reversedRuleIds = Lists.reverse(rules.stream().map(RestRuleModel::getId).collect(toList()));
+        RestRuleSetModel ruleSetBody = new RestRuleSetModel();
+        ruleSetBody.setId("-default-");
+        ruleSetBody.setRuleIds(reversedRuleIds);
+        RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(folder)
+                                             .include("ruleIds").updateRuleSet(ruleSetBody);
+
+        restClient.assertStatusCodeIs(OK);
+        ruleSet.assertThat().field("ruleIds").is(reversedRuleIds);
+    }
+
+    /** Check we can reorder the rules in a rule set by editing the response from the GET. */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void reorderRulesUsingResponseFromGET()
+    {
+        STEP("Create a folder containing three rules in the existing site");
+        FolderModel folder = dataContent.usingUser(user).usingSite(site).createFolder();
+        List<RestRuleModel> rules = createRulesInFolder(folder, user);
+
+        STEP("Get the rule set with its id.");
+        RestRuleSetModel ruleSetResponse = restClient.authenticateUser(user).withCoreAPI().usingNode(folder)
+                                                     .include("ruleIds").getDefaultRuleSet();
+
+        STEP("Reverse the order of the rules within the rule set");
+        ruleSetResponse.setRuleIds(Lists.reverse(ruleSetResponse.getRuleIds()));
+        RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(folder)
+                                             .include("ruleIds").updateRuleSet(ruleSetResponse);
+
+        restClient.assertStatusCodeIs(OK);
+        List<String> reversedRuleIds = Lists.reverse(rules.stream().map(RestRuleModel::getId).collect(toList()));
+        ruleSet.assertThat().field("ruleIds").is(reversedRuleIds);
+    }
+
+    /** Check that a user cannot reorder the rules in a rule set if they only have read permission. */
+    @Test
+    public void reorderRulesWithoutPermission()
+    {
+        STEP("Create a site owned by admin and add user as a consumer");
+        SiteModel siteModel = dataSite.usingAdmin().createPrivateRandomSite();
+        dataUser.addUserToSite(user, siteModel, UserRole.SiteContributor);
+
+        STEP("Use admin to create a folder with a rule set and three rules in it");
+        FolderModel ruleFolder = dataContent.usingAdmin().usingSite(siteModel).createFolder();
+        dataContent.usingAdmin().usingResource(ruleFolder).createFolder();
+        List<RestRuleModel> rules = createRulesInFolder(ruleFolder, dataUser.getAdminUser());
+
+        STEP("Try to reorder the rules as the contributor");
+        List<String> reversedRuleIds = Lists.reverse(rules.stream().map(RestRuleModel::getId).collect(toList()));
+        RestRuleSetModel ruleSetBody = new RestRuleSetModel();
+        ruleSetBody.setId("-default-");
+        ruleSetBody.setRuleIds(reversedRuleIds);
+        RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder)
+                                             .include("ruleIds").updateRuleSet(ruleSetBody);
+
+        restClient.assertStatusCodeIs(FORBIDDEN);
+    }
+
+    /** Check that a user cannot reorder the rules in a rule set if they only have read permission. */
+    @Test
+    public void reorderRulesWithPermission()
+    {
+        STEP("Create a site owned by admin and add user as a collaborator");
+        SiteModel siteModel = dataSite.usingAdmin().createPrivateRandomSite();
+        dataUser.addUserToSite(user, siteModel, UserRole.SiteCollaborator);
+
+        STEP("Use admin to create a folder with a rule set and three rules in it");
+        FolderModel ruleFolder = dataContent.usingAdmin().usingSite(siteModel).createFolder();
+        dataContent.usingAdmin().usingResource(ruleFolder).createFolder();
+        List<RestRuleModel> rules = createRulesInFolder(ruleFolder, dataUser.getAdminUser());
+
+        STEP("Try to reorder the rules as the contributor");
+        List<String> reversedRuleIds = Lists.reverse(rules.stream().map(RestRuleModel::getId).collect(toList()));
+        RestRuleSetModel ruleSetBody = new RestRuleSetModel();
+        ruleSetBody.setId("-default-");
+        ruleSetBody.setRuleIds(reversedRuleIds);
+        RestRuleSetModel ruleSet = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder)
+                                             .include("ruleIds").updateRuleSet(ruleSetBody);
+
+        restClient.assertStatusCodeIs(OK);
+        ruleSet.assertThat().field("ruleIds").is(reversedRuleIds);
+    }
+
+    /** Create three rules in the given folder. */
+    private List<RestRuleModel> createRulesInFolder(FolderModel folder, UserModel user)
+    {
+        return IntStream.range(0, 3).mapToObj(index ->
+        {
+            RestRuleModel ruleModel = createRuleModel("ruleName");
+            return restClient.authenticateUser(user).withCoreAPI().usingNode(folder).usingDefaultRuleSet().createSingleRule(ruleModel);
+        }).collect(toList());
+    }
 }

remote-api/src/main/java/org/alfresco/rest/api/RuleSets.java

@@ -59,6 +59,16 @@ public interface RuleSets
      */
     RuleSet getRuleSetById(String folderNodeId, String ruleSetId, List<String> includes);
 
+    /**
+     * Update a rule set - for example to reorder the rules within it.
+     *
+     * @param folderNodeId Folder node ID
+     * @param ruleSet The updated rule set.
+     * @param includes List of fields to include in the response.
+     * @return The updated rule set from the server.
+     */
+    RuleSet updateRuleSet(String folderNodeId, RuleSet ruleSet, List<String> includes);
+
     /**
      * Link a rule set to a folder
      */

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

@@ -149,7 +149,7 @@ public class RuleSetLoader
         );
     }
 
-    private List<String> loadRuleIds(NodeRef folderNodeRef)
+    public List<String> loadRuleIds(NodeRef folderNodeRef)
     {
         return ruleService.getRules(folderNodeRef, false).stream()
                           .map(restRuleModelMapper::toRestModel)

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

@@ -28,8 +28,14 @@ package org.alfresco.rest.api.impl.rules;
 
 import static java.util.stream.Collectors.toList;
 
+import static org.alfresco.service.cmr.repository.StoreRef.STORE_REF_WORKSPACE_SPACESSTORE;
+import static org.alfresco.util.collections.CollectionUtils.isEmpty;
+
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
+import java.util.stream.IntStream;
 
 import org.alfresco.repo.rule.RuleModel;
 import org.alfresco.repo.rule.RuntimeRuleService;
@@ -41,7 +47,6 @@ import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo;
 import org.alfresco.rest.framework.resource.parameters.ListPage;
 import org.alfresco.rest.framework.resource.parameters.Paging;
 import org.alfresco.service.Experimental;
-import org.alfresco.service.cmr.repository.AspectMissingException;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.rule.RuleService;
@@ -80,6 +85,43 @@ public class RuleSetsImpl implements RuleSets
         return ruleSetLoader.loadRuleSet(ruleSetNode, folderNode, includes);
     }
 
+    @Override
+    public RuleSet updateRuleSet(String folderNodeId, RuleSet ruleSet, List<String> includes)
+    {
+        // Editing the order of the rules doesn't require permission to edit the rule set itself.
+        NodeRef folderNode = validator.validateFolderNode(folderNodeId, false);
+        NodeRef ruleSetNode = validator.validateRuleSetNode(ruleSet.getId(), folderNode);
+
+        RuleSet returnedRuleSet = ruleSetLoader.loadRuleSet(ruleSetNode, folderNode, includes);
+
+        // Currently the only field that can be updated is ruleIds to reorder the rules.
+        List<String> suppliedRuleIds = ruleSet.getRuleIds();
+        if (!isEmpty(suppliedRuleIds))
+        {
+            // Check there are no duplicate rule ids in the request.
+            Set<String> suppliedRuleIdSet = new HashSet<>(suppliedRuleIds);
+
+            // Check that the set of rule ids hasn't changed.
+            Set<String> existingRuleIds = new HashSet<>(ruleSetLoader.loadRuleIds(folderNode));
+            if (!suppliedRuleIdSet.equals(existingRuleIds))
+            {
+                throw new InvalidArgumentException("Unexpected set of rule ids - received " + suppliedRuleIds + " but expected " + existingRuleIds);
+            }
+
+            IntStream.range(0, suppliedRuleIds.size()).forEach(index ->
+            {
+                NodeRef ruleNode = new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, suppliedRuleIds.get(index));
+                ruleService.setRulePosition(folderNode, ruleNode, index);
+            });
+            if (includes.contains(RuleSetLoader.RULE_IDS))
+            {
+                returnedRuleSet.setRuleIds(suppliedRuleIds);
+            }
+        }
+
+        return returnedRuleSet;
+    }
+
     @Override
     public RuleSetLink linkToRuleSet(String folderNodeId, String linkToNodeId)
     {

remote-api/src/main/java/org/alfresco/rest/api/nodes/NodeRuleSetsRelation.java

@@ -28,8 +28,6 @@ package org.alfresco.rest.api.nodes;
 
 import javax.servlet.http.HttpServletResponse;
 
-import java.util.List;
-
 import org.alfresco.rest.api.RuleSets;
 import org.alfresco.rest.api.model.rules.RuleSet;
 import org.alfresco.rest.framework.WebApiDescription;
@@ -37,7 +35,6 @@ import org.alfresco.rest.framework.core.exceptions.RelationshipResourceNotFoundE
 import org.alfresco.rest.framework.resource.RelationshipResource;
 import org.alfresco.rest.framework.resource.actions.interfaces.RelationshipResourceAction;
 import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo;
-import org.alfresco.rest.framework.resource.parameters.Paging;
 import org.alfresco.rest.framework.resource.parameters.Parameters;
 import org.alfresco.service.Experimental;
 import org.alfresco.util.PropertyCheck;
@@ -50,6 +47,7 @@ import org.springframework.beans.factory.InitializingBean;
 @RelationshipResource(name = "rule-sets", entityResource = NodesEntityResource.class, title = "Folder node rule sets")
 public class NodeRuleSetsRelation implements RelationshipResourceAction.Read<RuleSet>,
                                              RelationshipResourceAction.ReadById<RuleSet>,
+                                             RelationshipResourceAction.Update<RuleSet>,
                                              InitializingBean
 {
     private RuleSets ruleSets;
@@ -106,4 +104,20 @@ public class NodeRuleSetsRelation implements RelationshipResourceAction.Read<Rul
     {
         this.ruleSets = ruleSets;
     }
+
+    /**
+     * Update a rule set, in particular this is useful for reordering rules in a rule set.
+     * <p>
+     * - PUT /nodes/{folderNodeId}/rule-sets/{ruleSetId}
+     *
+     * @param folderNodeId The id for the folder.
+     * @param ruleSet The updated rule set.
+     * @param parameters Contains information about which fields to include in the response.
+     * @return The updated rule set.
+     */
+    @Override
+    public RuleSet update(String folderNodeId, RuleSet ruleSet, Parameters parameters)
+    {
+        return ruleSets.updateRuleSet(folderNodeId, ruleSet, parameters.getInclude());
+    }
 }

remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RuleSetsImplTest.java

@@ -27,6 +27,7 @@ package org.alfresco.rest.api.impl.rules;
 
 import static java.util.Collections.emptyList;
 
+import static org.alfresco.rest.api.impl.rules.RuleSetLoader.RULE_IDS;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -46,7 +47,6 @@ import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
 import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo;
 import org.alfresco.rest.framework.resource.parameters.Paging;
 import org.alfresco.service.Experimental;
-import org.alfresco.service.cmr.repository.AspectMissingException;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
@@ -329,4 +329,102 @@ public class RuleSetsImplTest extends TestCase
         then(ruleServiceMock).shouldHaveNoMoreInteractions();
         then(nodeServiceMock).shouldHaveNoInteractions();
     }
+
+    @Test
+    public void testUpdateRuleSet()
+    {
+        given(ruleSetMock.getId()).willReturn(RULE_SET_ID);
+        given(nodeValidatorMock.validateFolderNode(FOLDER_ID, false)).willReturn(FOLDER_NODE);
+        given(nodeValidatorMock.validateRuleSetNode(RULE_SET_ID, FOLDER_NODE)).willReturn(RULE_SET_NODE);
+        RuleSet ruleSetResponse = mock(RuleSet.class);
+        given(ruleSetLoaderMock.loadRuleSet(RULE_SET_NODE, FOLDER_NODE, emptyList())).willReturn(ruleSetResponse);
+
+        //when
+        RuleSet ruleSet = ruleSets.updateRuleSet(FOLDER_ID, ruleSetMock, emptyList());
+
+        assertEquals("Unexpected rule set returned.", ruleSetResponse, ruleSet);
+
+        then(nodeValidatorMock).should().validateFolderNode(FOLDER_ID, false);
+        then(nodeValidatorMock).should().validateRuleSetNode(RULE_SET_ID, FOLDER_NODE);
+        then(ruleSetLoaderMock).should().loadRuleSet(RULE_SET_NODE, FOLDER_NODE, emptyList());
+    }
+
+    /** Simulate rules being reordered from [RuleA, RuleB] to [RuleB, RuleA]. */
+    @Test
+    public void testUpdateRuleSet_reorderRules()
+    {
+        List<String> dbOrder = List.of("RuleA", "RuleB");
+        List<String> newOrder = List.of("RuleB", "RuleA");
+        List<String> includes = List.of(RULE_IDS);
+
+        RuleSet dbRuleSet = mock(RuleSet.class);
+        RuleSet requestRuleSet = mock(RuleSet.class);
+        given(requestRuleSet.getId()).willReturn(RULE_SET_ID);
+        given(requestRuleSet.getRuleIds()).willReturn(newOrder);
+
+        given(ruleSetLoaderMock.loadRuleSet(RULE_SET_NODE, FOLDER_NODE, includes)).willReturn(dbRuleSet);
+        given(ruleSetLoaderMock.loadRuleIds(FOLDER_NODE)).willReturn(dbOrder);
+        given(nodeValidatorMock.validateFolderNode(FOLDER_ID, false)).willReturn(FOLDER_NODE);
+        given(nodeValidatorMock.validateRuleSetNode(RULE_SET_ID, FOLDER_NODE)).willReturn(RULE_SET_NODE);
+
+        //when
+        RuleSet ruleSet = ruleSets.updateRuleSet(FOLDER_ID, requestRuleSet, includes);
+
+        assertEquals("Unexpected rule set returned.", dbRuleSet, ruleSet);
+
+        then(nodeValidatorMock).should().validateFolderNode(FOLDER_ID, false);
+        then(nodeValidatorMock).should().validateRuleSetNode(RULE_SET_ID, FOLDER_NODE);
+        then(ruleSetLoaderMock).should().loadRuleSet(RULE_SET_NODE, FOLDER_NODE, includes);
+        then(dbRuleSet).should().setRuleIds(newOrder);
+    }
+
+    /** Check that we can't remove a rule by updating the rule set. */
+    @Test
+    public void testUpdateRuleSet_tryToChangeSetOfRuleIds()
+    {
+        List<String> dbOrder = List.of("RuleA", "RuleB");
+        List<String> newOrder = List.of("RuleA");
+        List<String> includes = List.of(RULE_IDS);
+
+        RuleSet dbRuleSet = mock(RuleSet.class);
+        RuleSet requestRuleSet = mock(RuleSet.class);
+        given(requestRuleSet.getId()).willReturn(RULE_SET_ID);
+        given(requestRuleSet.getRuleIds()).willReturn(newOrder);
+
+        given(ruleSetLoaderMock.loadRuleSet(RULE_SET_NODE, FOLDER_NODE, includes)).willReturn(dbRuleSet);
+        given(ruleSetLoaderMock.loadRuleIds(FOLDER_NODE)).willReturn(dbOrder);
+        given(nodeValidatorMock.validateFolderNode(FOLDER_ID, false)).willReturn(FOLDER_NODE);
+        given(nodeValidatorMock.validateRuleSetNode(RULE_SET_ID, FOLDER_NODE)).willReturn(RULE_SET_NODE);
+
+        //when
+        assertThatExceptionOfType(InvalidArgumentException.class).isThrownBy(
+                () -> ruleSets.updateRuleSet(FOLDER_ID, requestRuleSet, includes)
+        );
+    }
+
+    /** Check that we can update the rule ids without returning them. */
+    @Test
+    public void testUpdateRuleSet_dontIncludeRuleIds()
+    {
+        List<String> dbOrder = List.of("RuleA", "RuleB");
+        List<String> newOrder = List.of("RuleB", "RuleA");
+        List<String> includes = emptyList();
+
+        RuleSet dbRuleSet = mock(RuleSet.class);
+        RuleSet requestRuleSet = mock(RuleSet.class);
+        given(requestRuleSet.getId()).willReturn(RULE_SET_ID);
+        given(requestRuleSet.getRuleIds()).willReturn(newOrder);
+
+        given(ruleSetLoaderMock.loadRuleSet(RULE_SET_NODE, FOLDER_NODE, includes)).willReturn(dbRuleSet);
+        given(ruleSetLoaderMock.loadRuleIds(FOLDER_NODE)).willReturn(dbOrder);
+        given(nodeValidatorMock.validateFolderNode(FOLDER_ID, false)).willReturn(FOLDER_NODE);
+        given(nodeValidatorMock.validateRuleSetNode(RULE_SET_ID, FOLDER_NODE)).willReturn(RULE_SET_NODE);
+
+        //when
+        RuleSet ruleSet = ruleSets.updateRuleSet(FOLDER_ID, requestRuleSet, includes);
+
+        // Expect the DB rule set to be returned, but no extra fields to be populated.
+        assertEquals("Unexpected rule set returned.", dbRuleSet, ruleSet);
+        then(dbRuleSet).shouldHaveNoInteractions();
+    }
 }