Merge pull request #1462 from Alfresco/feature/ACS-3377_UpdateRuleOrder

ACS-3377 Update rule order.
This commit is contained in:
Tom Page
2022-10-04 09:16:30 +01:00
committed by GitHub
6 changed files with 323 additions and 11 deletions

View File

@@ -27,16 +27,20 @@ package org.alfresco.rest.rules;
import static java.util.stream.Collectors.toList;
import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModel;
import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithDefaultValues;
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 -> {
RestRuleModel ruleModel = createRuleModel("ruleName");
return restClient.authenticateUser(user).withCoreAPI().usingNode(folder).usingDefaultRuleSet().createSingleRule(ruleModel);
}).collect(toList());
List<RestRuleModel> rules = createRulesInFolder(folder, user);
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);
restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder)
.include("ruleIds").updateRuleSet(ruleSetBody);
restClient.assertStatusCodeIs(FORBIDDEN);
}
/** Check that a user can reorder the rules in a rule set if they have write 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 = createRuleModelWithDefaultValues();
return restClient.authenticateUser(user).withCoreAPI().usingNode(folder).usingDefaultRuleSet().createSingleRule(ruleModel);
}).collect(toList());
}
}

View File

@@ -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
*/

View File

@@ -148,7 +148,7 @@ public class RuleSetLoader
);
}
private List<String> loadRuleIds(NodeRef folderNodeRef)
public List<String> loadRuleIds(NodeRef folderNodeRef)
{
return ruleService.getRules(folderNodeRef, false).stream()
.map(org.alfresco.service.cmr.rule.Rule::getNodeRef)

View File

@@ -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.size() != suppliedRuleIds.size() || !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)
{

View File

@@ -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());
}
}

View File

@@ -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,126 @@ 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't include a rule twice in a rule set. */
@Test
public void testUpdateRuleSet_DuplicateRuleId()
{
List<String> dbOrder = List.of("RuleA", "RuleB");
List<String> newOrder = List.of("RuleA", "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
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();
}
}