From a568c87571d7ec39e7ef33f57ebb24f05f28ba70 Mon Sep 17 00:00:00 2001 From: Maciej Pichura <41297682+mpichura@users.noreply.github.com> Date: Thu, 25 Aug 2022 12:46:42 +0200 Subject: [PATCH] ACS-3429, ACS-3336: Rule action mappings (create), validations (update) - part 2 (#1332) * ACS-3429, ACS-3336: Adding initial TAS tests for create rule with multiple actions, adding action parameters mappings and rule/action validations for rule update. * ACS-3429: Removing duplicated assertion. * ACS-3429: Changing NodeRef conversions to use only node id. --- .../alfresco/rest/rules/CreateRulesTests.java | 49 +++++++++++-- .../alfresco/rest/rules/RulesTestsUtils.java | 20 +++++ .../alfresco/rest/rules/UpdateRulesTests.java | 3 +- .../impl/rules/ActionParameterConverter.java | 30 +++++++- .../rest/api/impl/rules/RulesImpl.java | 23 +++++- .../rules/ActionParameterConverterTest.java | 73 +++++++++++++------ .../rules/ActionPermissionValidatorTest.java | 45 ++++++++++-- .../rest/api/impl/rules/RulesImplTest.java | 22 ++++-- 8 files changed, 214 insertions(+), 51 deletions(-) diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java index ccb3e2cdf1..4add0e321a 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/CreateRulesTests.java @@ -28,6 +28,8 @@ package org.alfresco.rest.rules; import static java.util.stream.Collectors.toList; import static org.alfresco.rest.rules.RulesTestsUtils.RULE_NAME_DEFAULT; +import static org.alfresco.rest.rules.RulesTestsUtils.addActionContextParams; +import static org.alfresco.rest.rules.RulesTestsUtils.createCustomActionModel; import static org.alfresco.rest.rules.RulesTestsUtils.createDefaultActionModel; import static org.alfresco.rest.rules.RulesTestsUtils.createEmptyConditionModel; import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModel; @@ -46,10 +48,14 @@ import static org.springframework.http.HttpStatus.CREATED; import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.NOT_FOUND; +import java.io.Serializable; +import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.stream.IntStream; import org.alfresco.rest.RestTest; +import org.alfresco.rest.model.RestActionBodyExecTemplateModel; import org.alfresco.rest.model.RestRuleModel; import org.alfresco.rest.model.RestRuleModelsCollection; import org.alfresco.utility.constants.UserRole; @@ -95,14 +101,12 @@ public class CreateRulesTests extends RestTest .createSingleRule(ruleModel); RestRuleModel expectedRuleModel = createRuleModelWithDefaultValues(); + expectedRuleModel.setActions(addActionContextParams(expectedRuleModel.getActions())); expectedRuleModel.setConditions(createEmptyConditionModel()); restClient.assertStatusCodeIs(CREATED); - // TODO fix actions mapping and remove it from ignored fields, actual issue - difference: - // actual: actions=[RestActionBodyExecTemplateModel{actionDefinitionId='add-features', params={actionContext=rule, aspect-name={http://www.alfresco.org/model/audio/1.0}audio}}] - // expected: actions=[RestActionBodyExecTemplateModel{actionDefinitionId='set-property-value', params={aspect-name={http://www.alfresco.org/model/audio/1.0}audio, actionContext=rule}}] - rule.assertThat().isEqualTo(expectedRuleModel, IGNORE_ID, IGNORE_IS_SHARED, "actions") - .assertThat().field("id").isNotNull() - .assertThat().field("isShared").isNull(); + rule.assertThat().isEqualTo(expectedRuleModel, IGNORE_ID, IGNORE_IS_SHARED) + .assertThat().field("id").isNotNull() + .assertThat().field("isShared").isNull(); } /** Check creating a rule in a non-existent folder returns an error. */ @@ -362,4 +366,37 @@ public class CreateRulesTests extends RestTest return restClient.authenticateUser(userWithRole).withCoreAPI().usingNode(privateFolder).usingDefaultRuleSet().createSingleRule(ruleModel); } + + /** + * Check we can create a rule with several actions. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithActions() + { + final Map copyParams = + Map.of("destination-folder", "dummy-folder-node", "deep-copy", true); + final RestActionBodyExecTemplateModel copyAction = createCustomActionModel("copy", copyParams); + final Map checkOutParams = + Map.of("destination-folder", "dummy-folder-node", "assoc-name", "cm:checkout", "assoc-type", + "cm:contains"); + final RestActionBodyExecTemplateModel checkOutAction = createCustomActionModel("check-out", checkOutParams); + final Map scriptParams = Map.of("script-ref", "dummy-script-node-id"); + final RestActionBodyExecTemplateModel scriptAction = createCustomActionModel("script", scriptParams); + final RestRuleModel ruleModel = createRuleModelWithDefaultName(); + ruleModel.setActions(Arrays.asList(copyAction, checkOutAction, scriptAction)); + + final UserModel admin = dataUser.getAdminUser(); + + final RestRuleModel rule = restClient.authenticateUser(admin).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + final RestRuleModel expectedRuleModel = createRuleModelWithDefaultName(); + expectedRuleModel.setActions(addActionContextParams(Arrays.asList(copyAction, checkOutAction, scriptAction))); + expectedRuleModel.setConditions(createEmptyConditionModel()); + expectedRuleModel.setTriggers(List.of("inbound")); + + restClient.assertStatusCodeIs(CREATED); + rule.assertThat().isEqualTo(expectedRuleModel, IGNORE_ID, IGNORE_IS_SHARED) + .assertThat().field("isShared").isNull(); + } } diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java index aa2d8f723f..9178099302 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/RulesTestsUtils.java @@ -25,6 +25,8 @@ */ package org.alfresco.rest.rules; +import java.io.Serializable; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -100,6 +102,24 @@ public class RulesTestsUtils return restActionModel; } + public static List addActionContextParams(List inputActions) + { + inputActions.forEach(inputAction -> { + final Map params = new HashMap<>((Map) inputAction.getParams()); + params.put("actionContext", "rule"); + inputAction.setParams(params); + }); + return inputActions; + } + + public static RestActionBodyExecTemplateModel createCustomActionModel(String actionDefinitionId, Map params) + { + RestActionBodyExecTemplateModel restActionModel = new RestActionBodyExecTemplateModel(); + restActionModel.setActionDefinitionId(actionDefinitionId); + restActionModel.setParams(params); + return restActionModel; + } + public static RestCompositeConditionDefinitionModel createEmptyConditionModel() { RestCompositeConditionDefinitionModel conditions = new RestCompositeConditionDefinitionModel(); diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java index de4629a138..88c6cbb4ec 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/UpdateRulesTests.java @@ -161,8 +161,7 @@ public class UpdateRulesTests extends RestTest RestRuleModel rule = createAndSaveRule("Rule name"); STEP("Try to update the rule to have no name."); - RestRuleModel updatedRuleModel = new RestRuleModel(); - updatedRuleModel.setName(""); + RestRuleModel updatedRuleModel = createRuleModel(""); restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet().updateRule(rule.getId(), updatedRuleModel); restClient.assertLastError().statusCodeIs(BAD_REQUEST) diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java index b138b0301d..c860ac9556 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/ActionParameterConverter.java @@ -41,6 +41,8 @@ import org.alfresco.service.cmr.action.ParameterizedItemDefinition; import org.alfresco.service.cmr.dictionary.DataTypeDefinition; import org.alfresco.service.cmr.dictionary.DictionaryException; import org.alfresco.service.cmr.dictionary.DictionaryService; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; @@ -62,7 +64,8 @@ public class ActionParameterConverter this.namespaceService = namespaceService; } - Map getConvertedParams(Map params, String name) { + Map getConvertedParams(Map params, String name) + { final Map parameters = new HashMap<>(params.size()); final ParameterizedItemDefinition definition = actionService.getActionDefinition(name); if (definition == null) @@ -89,6 +92,21 @@ public class ActionParameterConverter return parameters; } + public Serializable convertParamFromServiceModel(Serializable param) + { + if (param instanceof QName) + { + return ((QName) param).toPrefixString(namespaceService); + } + else if (param instanceof NodeRef) { + return ((NodeRef) param).getId(); + } + else + { + return param; + } + } + private Serializable convertValue(QName typeQName, Object propertyValue) throws JSONException { Serializable value; @@ -117,12 +135,18 @@ public class ActionParameterConverter list.add(convertValue(typeQName, ((JSONArray) propertyValue).get(i))); } value = (Serializable) list; - } else + } + else { if (typeQName.equals(DataTypeDefinition.QNAME) && typeQName.toString().contains(":")) { value = QName.createQName(propertyValue.toString(), namespaceService); - } else + } + else if (typeQName.isMatch(DataTypeDefinition.NODE_REF)) + { + value = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, propertyValue.toString()); + } + else { value = (Serializable) DefaultTypeConverter.INSTANCE.convert(dictionaryService.getDataType(typeQName), propertyValue); } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java index 81a3c141ee..5f63d37cd7 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/rules/RulesImpl.java @@ -27,6 +27,7 @@ package org.alfresco.rest.api.impl.rules; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import org.alfresco.rest.api.Nodes; @@ -65,7 +66,7 @@ public class RulesImpl implements Rules validator.validateRuleSetNode(ruleSetId, folderNodeRef); final List rules = ruleService.getRules(folderNodeRef).stream() - .map(ruleModel -> ruleLoader.loadRule(ruleModel, includes)) + .map(ruleModel -> loadRuleAndConvertActionParams(ruleModel, includes)) .collect(Collectors.toList()); return ListPage.of(rules, paging); @@ -78,7 +79,7 @@ public class RulesImpl implements Rules final NodeRef ruleSetNodeRef = validator.validateRuleSetNode(ruleSetId, folderNodeRef); final NodeRef ruleNodeRef = validator.validateRuleNode(ruleId, ruleSetNodeRef); - return ruleLoader.loadRule(ruleService.getRule(ruleNodeRef), includes); + return loadRuleAndConvertActionParams(ruleService.getRule(ruleNodeRef), includes); } @Override @@ -94,7 +95,7 @@ public class RulesImpl implements Rules return rules.stream() .map(this::mapToServiceModelAndValidateActions) .map(rule -> ruleService.saveRule(folderNodeRef, rule)) - .map(rule -> ruleLoader.loadRule(rule, includes)) + .map(rule -> loadRuleAndConvertActionParams(rule, includes)) .collect(Collectors.toList()); } @@ -107,7 +108,7 @@ public class RulesImpl implements Rules NodeRef ruleSetNodeRef = validator.validateRuleSetNode(ruleSetId, folderNodeRef); validator.validateRuleNode(ruleId, ruleSetNodeRef); - return ruleLoader.loadRule(ruleService.saveRule(folderNodeRef, rule.toServiceModel(nodes)), includes); + return ruleLoader.loadRule(ruleService.saveRule(folderNodeRef, mapToServiceModelAndValidateActions(rule)), includes); } @Override @@ -130,6 +131,20 @@ public class RulesImpl implements Rules return actionPermissionValidator.validateRulePermissions(serviceModelRule); } + private Rule loadRuleAndConvertActionParams(org.alfresco.service.cmr.rule.Rule ruleModel, List includes) + { + + final Rule rule = ruleLoader.loadRule(ruleModel, includes); + rule.getActions() + .forEach(a -> a.setParams(a.getParams().entrySet() + .stream() + .collect(Collectors + .toMap(Map.Entry::getKey, e -> actionParameterConverter.convertParamFromServiceModel(e.getValue()))) + ) + ); + return rule; + } + public void setNodes(Nodes nodes) { this.nodes = nodes; diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java index 0b7be657d8..d5b40cbca3 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionParameterConverterTest.java @@ -36,6 +36,7 @@ import static org.mockito.BDDMockito.then; import static org.mockito.Mockito.times; import java.io.Serializable; +import java.util.List; import java.util.Map; import org.alfresco.repo.action.executer.AddFeaturesActionExecuter; @@ -79,10 +80,7 @@ public class ActionParameterConverterTest private static final String IDENTIFIER_ASPECT = NamespaceService.CONTENT_MODEL_PREFIX + QName.NAMESPACE_PREFIX + IDENTIFIER; private static final String DUMMY_FOLDER_NODE_ID = "dummy-folder-node"; - private static final String DUMMY_FOLDER_NODE_REF = STORE_REF_WORKSPACE_SPACESSTORE + "/" + DUMMY_FOLDER_NODE_ID; private static final String DUMMY_SCRIPT_NODE_ID = "dummy-script-ref"; - private static final String DUMMY_SCRIPT_NODE_REF = STORE_REF_WORKSPACE_SPACESSTORE + "/" + DUMMY_SCRIPT_NODE_ID; - @Mock private DictionaryService dictionaryService; @@ -148,7 +146,7 @@ public class ActionParameterConverterTest final String name = CopyActionExecuter.NAME; final String destinationFolderKey = CopyActionExecuter.PARAM_DESTINATION_FOLDER; final String deepCopyKey = CopyActionExecuter.PARAM_DEEP_COPY; - final Map params = Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_REF, deepCopyKey, true); + final Map params = Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_ID, deepCopyKey, true); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(destinationFolderKey)).willReturn(actionDefinitionParam1); @@ -159,7 +157,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam2.getType()).willReturn(bool); given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); given(dictionaryService.getDataType(bool)).willReturn(dataTypeDefinition2); given(dataTypeDefinition2.getJavaClassName()).willReturn(Boolean.class.getName()); @@ -172,7 +169,7 @@ public class ActionParameterConverterTest then(actionDefinition).should().getParameterDefintion(deepCopyKey); then(actionDefinition).shouldHaveNoMoreInteractions(); then(dictionaryService).should(times(2)).getDataType(bool); - then(dictionaryService).should(times(2)).getDataType(nodeRef); + then(dictionaryService).should().getDataType(nodeRef); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).shouldHaveNoInteractions(); @@ -190,7 +187,7 @@ public class ActionParameterConverterTest { final String name = ScriptActionExecuter.NAME; final String executeScriptKey = ScriptActionExecuter.PARAM_SCRIPTREF; - final Map params = Map.of(executeScriptKey, DUMMY_SCRIPT_NODE_REF); + final Map params = Map.of(executeScriptKey, DUMMY_SCRIPT_NODE_ID); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(executeScriptKey)).willReturn(actionDefinitionParam1); @@ -198,7 +195,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam1.getType()).willReturn(scriptNodeRef); given(dictionaryService.getDataType(scriptNodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); //when final Map convertedParams = objectUnderTest.getConvertedParams(params, name); @@ -207,7 +203,7 @@ public class ActionParameterConverterTest then(actionService).shouldHaveNoMoreInteractions(); then(actionDefinition).should().getParameterDefintion(executeScriptKey); then(actionDefinition).shouldHaveNoMoreInteractions(); - then(dictionaryService).should(times(2)).getDataType(scriptNodeRef); + then(dictionaryService).should().getDataType(scriptNodeRef); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).shouldHaveNoInteractions(); @@ -222,7 +218,7 @@ public class ActionParameterConverterTest { final String name = MoveActionExecuter.NAME; final String destinationFolderKey = MoveActionExecuter.PARAM_DESTINATION_FOLDER; - final Map params = Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_REF); + final Map params = Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_ID); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(destinationFolderKey)).willReturn(actionDefinitionParam1); @@ -230,7 +226,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam1.getType()).willReturn(nodeRef); given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); //when final Map convertedParams = objectUnderTest.getConvertedParams(params, name); @@ -239,7 +234,7 @@ public class ActionParameterConverterTest then(actionService).shouldHaveNoMoreInteractions(); then(actionDefinition).should().getParameterDefintion(destinationFolderKey); then(actionDefinition).shouldHaveNoMoreInteractions(); - then(dictionaryService).should(times(2)).getDataType(nodeRef); + then(dictionaryService).should().getDataType(nodeRef); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).shouldHaveNoInteractions(); @@ -300,7 +295,7 @@ public class ActionParameterConverterTest final String assocNameKey = CheckOutActionExecuter.PARAM_ASSOC_QNAME; final String assocTypeKey = CheckOutActionExecuter.PARAM_ASSOC_TYPE_QNAME; final Map params = - Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_REF, assocNameKey, CHECKOUT_ASPECT, assocTypeKey, CONTAINS_ASPECT); + Map.of(destinationFolderKey, DUMMY_FOLDER_NODE_ID, assocNameKey, CHECKOUT_ASPECT, assocTypeKey, CONTAINS_ASPECT); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(destinationFolderKey)).willReturn(actionDefinitionParam1); @@ -313,7 +308,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam3.getType()).willReturn(qname); given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); given(dictionaryService.getDataType(qname)).willReturn(dataTypeDefinition2); given(namespaceService.getNamespaceURI(any())).willReturn(NamespaceService.DICTIONARY_MODEL_1_0_URI); @@ -327,7 +321,7 @@ public class ActionParameterConverterTest then(actionDefinition).should().getParameterDefintion(assocTypeKey); then(actionDefinition).shouldHaveNoMoreInteractions(); then(dictionaryService).should(times(2)).getDataType(qname); - then(dictionaryService).should(times(2)).getDataType(nodeRef); + then(dictionaryService).should().getDataType(nodeRef); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).should(times(2)).getNamespaceURI(any()); then(namespaceService).shouldHaveNoMoreInteractions(); @@ -354,7 +348,7 @@ public class ActionParameterConverterTest final String name = LinkCategoryActionExecuter.NAME; final String categoryAspectKey = LinkCategoryActionExecuter.PARAM_CATEGORY_ASPECT; final String categoryValueKey = LinkCategoryActionExecuter.PARAM_CATEGORY_VALUE; - final Map params = Map.of(categoryAspectKey, CLASSIFIABLE_ASPECT, categoryValueKey, DUMMY_FOLDER_NODE_REF); + final Map params = Map.of(categoryAspectKey, CLASSIFIABLE_ASPECT, categoryValueKey, DUMMY_FOLDER_NODE_ID); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(categoryAspectKey)).willReturn(actionDefinitionParam1); @@ -365,7 +359,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam2.getType()).willReturn(nodeRef); given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); given(dictionaryService.getDataType(qname)).willReturn(dataTypeDefinition2); given(namespaceService.getNamespaceURI(any())).willReturn(NamespaceService.DICTIONARY_MODEL_1_0_URI); @@ -378,7 +371,7 @@ public class ActionParameterConverterTest then(actionDefinition).should().getParameterDefintion(categoryValueKey); then(actionDefinition).shouldHaveNoMoreInteractions(); then(dictionaryService).should().getDataType(qname); - then(dictionaryService).should(times(2)).getDataType(nodeRef); + then(dictionaryService).should().getDataType(nodeRef); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).should().getNamespaceURI(any()); then(namespaceService).shouldHaveNoMoreInteractions(); @@ -440,8 +433,8 @@ public class ActionParameterConverterTest final String approve = "Approve"; final String reject = "Reject"; final Map params = - Map.of(approveStepKey, approve, approveFolderKey, DUMMY_FOLDER_NODE_REF, approveMoveKey, true, - rejectStepKey, reject, rejectFolderKey, DUMMY_FOLDER_NODE_REF, rejectMoveKey, true); + Map.of(approveStepKey, approve, approveFolderKey, DUMMY_FOLDER_NODE_ID, approveMoveKey, true, + rejectStepKey, reject, rejectFolderKey, DUMMY_FOLDER_NODE_ID, rejectMoveKey, true); given(actionService.getActionDefinition(name)).willReturn(actionDefinition); given(actionDefinition.getParameterDefintion(rejectStepKey)).willReturn(actionDefinitionParam1); @@ -458,7 +451,6 @@ public class ActionParameterConverterTest given(actionDefinitionParam3.getType()).willReturn(bool, bool); given(dictionaryService.getDataType(nodeRef)).willReturn(dataTypeDefinition1); - given(dataTypeDefinition1.getJavaClassName()).willReturn(NodeRef.class.getName()); given(dictionaryService.getDataType(text)).willReturn(dataTypeDefinition2); given(dataTypeDefinition2.getJavaClassName()).willReturn(String.class.getName()); given(dictionaryService.getDataType(bool)).willReturn(dataTypeDefinition3); @@ -477,7 +469,7 @@ public class ActionParameterConverterTest then(actionDefinition).should().getParameterDefintion(rejectMoveKey); then(actionDefinition).shouldHaveNoMoreInteractions(); then(dictionaryService).should(times(4)).getDataType(text); - then(dictionaryService).should(times(4)).getDataType(nodeRef); + then(dictionaryService).should(times(2)).getDataType(nodeRef); then(dictionaryService).should(times(4)).getDataType(bool); then(dictionaryService).shouldHaveNoMoreInteractions(); then(namespaceService).shouldHaveNoInteractions(); @@ -560,4 +552,41 @@ public class ActionParameterConverterTest assertTrue(convertedPropTypeParam instanceof String); assertEquals(propType, convertedPropTypeParam); } + + @Test + public void testQnameServiceModelParamConversion() { + given(namespaceService.getPrefixes(any())).willReturn(List.of("cm")); + final String qname = "{cm-dummy-prefix}audio"; + final Serializable qnameParam = QName.createQName(qname); + + //when + final Serializable convertedParam = objectUnderTest.convertParamFromServiceModel(qnameParam); + + then(namespaceService).should().getPrefixes(any()); + then(namespaceService).shouldHaveNoMoreInteractions(); + assertEquals("cm:audio", convertedParam); + } + + @Test + public void testNodeRefServiceModelParamConversion() { + //given + final Serializable nodeRefParam = new NodeRef(STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_FOLDER_NODE_ID); + + //when + final Serializable convertedParam = objectUnderTest.convertParamFromServiceModel(nodeRefParam); + + then(namespaceService).shouldHaveNoInteractions(); + assertEquals(DUMMY_FOLDER_NODE_ID, convertedParam); + } + + @Test + public void testOtherServiceModelParamConversion() { + //given + final Serializable dummyStringParam = "dummy-param"; + //when + final Serializable convertedParam = objectUnderTest.convertParamFromServiceModel(dummyStringParam); + + then(namespaceService).shouldHaveNoInteractions(); + assertEquals(dummyStringParam, convertedParam); + } } diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionPermissionValidatorTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionPermissionValidatorTest.java index 12be2eba6d..2d3792c0d0 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionPermissionValidatorTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/ActionPermissionValidatorTest.java @@ -26,15 +26,20 @@ package org.alfresco.rest.api.impl.rules; +import static org.alfresco.service.cmr.rule.RuleType.INBOUND; +import static org.alfresco.service.cmr.rule.RuleType.OUTBOUND; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.BDDMockito.then; -import java.util.ArrayList; import java.util.List; import junit.framework.TestCase; import org.alfresco.repo.action.ActionImpl; import org.alfresco.repo.action.CompositeActionImpl; import org.alfresco.repo.action.RuntimeActionService; +import org.alfresco.repo.action.executer.CheckOutActionExecuter; +import org.alfresco.repo.action.executer.CopyActionExecuter; +import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.repository.NodeRef; @@ -59,15 +64,20 @@ public class ActionPermissionValidatorTest extends TestCase private ActionPermissionValidator objectUnderTest; @Test - public void testPositiveRulePermissionValidation() { + public void testPositiveRulePermissionValidation() + { //given - final CompositeActionImpl compositeAction = new CompositeActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "composite-id"); - final Action action1 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-1", "actionDef-1"); + final CompositeActionImpl compositeAction = + new CompositeActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "composite-id"); + final Action action1 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-1", + CopyActionExecuter.NAME); compositeAction.addAction(action1); - final Action action2 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-2", "actionDef-2"); + final Action action2 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-2", + CheckOutActionExecuter.NAME); compositeAction.addAction(action2); final Rule inputRule = new Rule(); inputRule.setAction(compositeAction); + inputRule.setRuleTypes(List.of(INBOUND)); //when final Rule validatedRule = objectUnderTest.validateRulePermissions(inputRule); @@ -80,5 +90,28 @@ public class ActionPermissionValidatorTest extends TestCase .forEach(action -> Assertions.assertThat(action.getParameterValue("actionContext")).isEqualTo("rule")); } - //TODO: when Rule mappings are done - we need to add negative test(s) here + @Test + public void testNegativeRulePermissionValidation() + { + //given + final CompositeActionImpl compositeAction = + new CompositeActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "composite-id"); + final Action action1 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-1", + CopyActionExecuter.NAME); + compositeAction.addAction(action1); + final Action action2 = new ActionImpl(new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, DUMMY_NODE_ID), "id-2", + CheckOutActionExecuter.NAME); + compositeAction.addAction(action2); + final Rule inputRule = new Rule(); + inputRule.setAction(compositeAction); + inputRule.setRuleTypes(List.of(OUTBOUND)); + + //when + assertThatExceptionOfType(InvalidArgumentException.class).isThrownBy(() -> objectUnderTest.validateRulePermissions(inputRule)); + + then(runtimeActionService).should().verifyActionAccessRestrictions(action1); + then(runtimeActionService).should().verifyActionAccessRestrictions(action2); + then(runtimeActionService).shouldHaveNoMoreInteractions(); + } + } diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java index 4300376fb4..f7c4aad7d5 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/rules/RulesImplTest.java @@ -125,6 +125,7 @@ public class RulesImplTest extends TestCase @Test public void testGetRules() { + given(ruleLoaderMock.loadRule(ruleModel, emptyList())).willReturn(ruleMock); // when final CollectionWithPagingInfo rulesPage = rules.getRules(FOLDER_NODE_ID, RULE_SET_ID, INCLUDE, PAGING); @@ -133,6 +134,8 @@ public class RulesImplTest extends TestCase then(nodeValidatorMock).shouldHaveNoMoreInteractions(); then(ruleServiceMock).should().getRules(FOLDER_NODE_REF); then(ruleServiceMock).shouldHaveNoMoreInteractions(); + then(ruleLoaderMock).should().loadRule(ruleModel, emptyList()); + then(ruleLoaderMock).shouldHaveNoMoreInteractions(); assertThat(rulesPage) .isNotNull() .extracting(CollectionWithPagingInfo::getCollection) @@ -428,22 +431,25 @@ public class RulesImplTest extends TestCase @Test public void testUpdateRuleById() { - Rule ruleBody = mock(Rule.class); - org.alfresco.service.cmr.rule.Rule serviceRuleBody = mock(org.alfresco.service.cmr.rule.Rule.class); - given(ruleBody.toServiceModel(nodesMock)).willReturn(serviceRuleBody); - org.alfresco.service.cmr.rule.Rule serviceRule = mock(org.alfresco.service.cmr.rule.Rule.class); - given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleBody)).willReturn(serviceRule); - given(ruleLoaderMock.loadRule(serviceRule, INCLUDE)).willReturn(ruleMock); + given(ruleMock.toServiceModel(nodesMock)).willReturn(serviceRuleMock); + given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(a -> a.getArguments()[1]); + given(serviceRuleMock.getAction()).willReturn(compositeAction); + given(ruleLoaderMock.loadRule(serviceRuleMock, INCLUDE)).willReturn(ruleMock); + given(actionPermissionValidatorMock.validateRulePermissions(any())).willAnswer(arg -> arg.getArguments()[0]); // when - Rule updatedRule = rules.updateRuleById(FOLDER_NODE_REF.getId(), RULE_SET_NODE_REF.getId(), RULE_ID, ruleBody, INCLUDE); + Rule updatedRule = rules.updateRuleById(FOLDER_NODE_REF.getId(), RULE_SET_NODE_REF.getId(), RULE_ID, ruleMock, INCLUDE); then(nodeValidatorMock).should().validateFolderNode(FOLDER_NODE_ID, true); then(nodeValidatorMock).should().validateRuleSetNode(RULE_SET_ID, FOLDER_NODE_REF); then(nodeValidatorMock).should().validateRuleNode(RULE_ID, RULE_SET_NODE_REF); then(nodeValidatorMock).shouldHaveNoMoreInteractions(); - then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, serviceRuleBody); + then(ruleServiceMock).should().saveRule(FOLDER_NODE_REF, serviceRuleMock); then(ruleServiceMock).shouldHaveNoMoreInteractions(); + then(actionParameterConverterMock).should().getConvertedParams(DUMMY_PARAMS, ACTION_DEFINITION_NAME); + then(actionParameterConverterMock).shouldHaveNoMoreInteractions(); + then(actionPermissionValidatorMock).should().validateRulePermissions(serviceRuleMock); + then(actionPermissionValidatorMock).shouldHaveNoMoreInteractions(); assertThat(updatedRule).isEqualTo(ruleMock); }