From 307eaff8963861a9be3c92e5b44c3fe32ba697b1 Mon Sep 17 00:00:00 2001 From: Maciej Pichura <41297682+mpichura@users.noreply.github.com> Date: Mon, 10 Oct 2022 17:13:51 +0200 Subject: [PATCH] ACS-3649, ACS-3650 Action validation mechanism + validations against Action Definitions (#1481) * ACS-3572: Action constraints endpoint + logic + unit tests. * ACS-3572: Action constraints endpoint + logic + unit tests. * ACS-3572: Action constraints endpoint fixes. * ACS-3572: Cleanup after removal of GET all Action constraints. * ACS-3572: Fixing formatting in tests. * ACS-3572: Loosening V1 constraints restrictions for aspects, types and properties. * ACS-3649: Action validators - constraints. * ACS-3649: Action validators - constraints. * ACS-3649: Action validators - constraints. * ACS-3572: Adding validation for extra paramater. * ACS-3649: Small refactors and fixes. * ACS-3649: Validations against action definition. * ACS-3649: Adding @Experimental annotation. * ACS-3649: Fixes/refactoring after code review. * ACS-3649: Removing ignored test --- .../alfresco/rest/rules/CreateRulesTests.java | 92 +++++++- .../rest/rules/ExecuteRulesTests.java | 18 +- .../alfresco/rest/rules/RulesTestsUtils.java | 4 +- .../alfresco/rest/rules/UpdateRulesTests.java | 20 +- .../ActionConstraintsEntityResource.java | 59 +++++ .../rest/api/actions/ActionValidator.java | 38 +++ .../alfresco/rest/api/impl/ActionsImpl.java | 2 +- .../rules/RestRuleActionModelMapper.java | 13 +- .../ActionParameterDefinitionValidator.java | 133 +++++++++++ .../alfresco/public-rest-context.xml | 14 +- .../ActionConstraintsEntityResourceTest.java | 66 ++++++ .../rules/RestRuleActionModelMapperTest.java | 12 +- ...ctionParameterDefinitionValidatorTest.java | 217 ++++++++++++++++++ 13 files changed, 654 insertions(+), 34 deletions(-) create mode 100644 remote-api/src/main/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResource.java create mode 100644 remote-api/src/main/java/org/alfresco/rest/api/actions/ActionValidator.java create mode 100644 remote-api/src/main/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidator.java create mode 100644 remote-api/src/test/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResourceTest.java create mode 100644 remote-api/src/test/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidatorTest.java 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 90471102d8..4ce6a37b25 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 @@ -26,7 +26,6 @@ package org.alfresco.rest.rules; import static java.util.stream.Collectors.toList; - import static org.alfresco.rest.actions.access.AccessRestrictionUtil.ERROR_MESSAGE_ACCESS_RESTRICTED; import static org.alfresco.rest.rules.RulesTestsUtils.ID; import static org.alfresco.rest.rules.RulesTestsUtils.INVERTED; @@ -34,6 +33,7 @@ import static org.alfresco.rest.rules.RulesTestsUtils.IS_SHARED; import static org.alfresco.rest.rules.RulesTestsUtils.RULE_NAME_DEFAULT; import static org.alfresco.rest.rules.RulesTestsUtils.createAddAudioAspectAction; import static org.alfresco.rest.rules.RulesTestsUtils.createCompositeCondition; +import static org.alfresco.rest.rules.RulesTestsUtils.createCustomActionModel; import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModel; import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithDefaultValues; import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithModifiedValues; @@ -441,8 +441,94 @@ public class CreateRulesTests extends RestTest restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .createSingleRule(ruleModel); - restClient.assertStatusCodeIs(NOT_FOUND); - restClient.assertLastError().containsSummary(actionDefinitionId); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary(String.format("Invalid action definition requested %s", actionDefinitionId)); + } + + /** + * Check we get error when attempt to create a rule with missing action parameters. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithMissingActionParametersShouldFail() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + final RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel(); + final String actionDefinitionId = "copy"; + invalidAction.setActionDefinitionId(actionDefinitionId); + ruleModel.setActions(List.of(invalidAction)); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary( + String.format("Action parameters should not be null or empty for this action. See Action Definition for action of: %s", + actionDefinitionId)); + } + + /** + * Check we get error when attempt to create a rule with parameter not fulfilling constraint. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithActionParameterNotFulfillingConstraint() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + final String actionDefinitionId = "script"; + final String scriptRef = "script-ref"; + final String scriptNodeId = "dummy-script-node-id"; + final RestActionBodyExecTemplateModel scriptAction = createCustomActionModel(actionDefinitionId, Map.of(scriptRef, scriptNodeId)); + ruleModel.setActions(List.of(scriptAction)); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(BAD_REQUEST); + final String acScriptsConstraint = "ac-scripts"; + restClient.assertLastError().containsSummary( + String.format("Action parameter: %s has invalid value (%s). Look up possible values for constraint name %s", + scriptRef, scriptNodeId, acScriptsConstraint)); + } + + /** + * Check we get error when attempt to create a rule with action parameter that should not be passed. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithoutInvalidActionParameterShouldFail() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + final RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel(); + final String actionDefinitionId = "add-features"; + invalidAction.setActionDefinitionId(actionDefinitionId); + final String invalidParameterKey = "invalidParameterKey"; + invalidAction.setParams(Map.of(invalidParameterKey,"dummyValue")); + ruleModel.setActions(List.of(invalidAction)); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary( + String.format("Action of definition id: %s must not contain parameter of name: %s", actionDefinitionId, invalidParameterKey)); + } + + /** + * Check we get error when attempt to create a rule with missing mandatory action parameter. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithoutMandatoryActionParametersShouldFail() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + final RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel(); + final String actionDefinitionId = "copy"; + invalidAction.setActionDefinitionId(actionDefinitionId); + invalidAction.setParams(Map.of("deep-copy",false)); + ruleModel.setActions(List.of(invalidAction)); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("Missing action's mandatory parameter: destination-folder"); } /** diff --git a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/ExecuteRulesTests.java b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/ExecuteRulesTests.java index f04b400170..19d5468482 100644 --- a/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/ExecuteRulesTests.java +++ b/packaging/tests/tas-restapi/src/test/java/org/alfresco/rest/rules/ExecuteRulesTests.java @@ -54,6 +54,7 @@ import org.alfresco.utility.model.UserModel; import org.springframework.http.HttpStatus; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Ignore; import org.testng.annotations.Test; /** @@ -301,20 +302,5 @@ public class ExecuteRulesTests extends RestTest assertThat(fileNode).containsAspects(AUDIO_ASPECT); } - /** - * Try to execute rule with broken action and receive 500. - */ - @Test(groups = { TestGroup.REST_API, TestGroup.RULES, TestGroup.ACTIONS }) - public void executeRules_brokenActionResultsWith500() - { - STEP("Update folder rule with broken action"); - RestActionBodyExecTemplateModel brokenAction = createCustomActionModel("set-property-value", Map.of("aspect-name", AUDIO_ASPECT)); - childFolderRule.setActions(List.of(brokenAction)); - restClient.authenticateUser(user).withCoreAPI().usingNode(childFolder).usingDefaultRuleSet().updateRule(childFolderRule.getId(), childFolderRule); - restClient.assertStatusCodeIs(HttpStatus.OK); - - STEP("Try to execute rule with broken action"); - restClient.authenticateUser(user).withCoreAPI().usingNode(childFolder).executeRules(createRuleExecutionRequest()); - restClient.assertStatusCodeIs(HttpStatus.INTERNAL_SERVER_ERROR); - } + //TODO: add test(s) that would cover handling executing broken rule and/or broken rule execution (ACS-3699) } 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 9e86d1cfc9..6fae429ace 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 @@ -171,12 +171,10 @@ public class RulesTestsUtils Map.of("destination-folder", "fake-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); // The counter action takes no parameters, so check we can omit the "params" entry. final RestActionBodyExecTemplateModel counterAction = createCustomActionModel("counter", null); final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); - ruleModel.setActions(Arrays.asList(copyAction, checkOutAction, scriptAction, counterAction)); + ruleModel.setActions(Arrays.asList(copyAction, checkOutAction, counterAction)); return ruleModel; } 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 262e072111..94647e82e4 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 @@ -255,8 +255,8 @@ public class UpdateRulesTests extends RestTest .include(IS_SHARED) .updateRule(rule.getId(), rule); - restClient.assertStatusCodeIs(NOT_FOUND); - restClient.assertLastError().containsSummary(actionDefinitionId); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary(String.format("Invalid action definition requested %s", actionDefinitionId)); } /** Check we can use the POST response to create the new rule. */ @@ -472,9 +472,9 @@ public class UpdateRulesTests extends RestTest final Map copyParams = Map.of("destination-folder", "dummy-folder-node", "deep-copy", true); final RestActionBodyExecTemplateModel copyAction = createCustomActionModel("copy", copyParams); - final Map scriptParams = Map.of("script-ref", "dummy-script-node-id"); - final RestActionBodyExecTemplateModel scriptAction = createCustomActionModel("script", scriptParams); - rule.setActions(Arrays.asList(copyAction, scriptAction)); + final Map addAspectParams = Map.of("aspect-name", "cm:taggable"); + final RestActionBodyExecTemplateModel addAspectAction = createCustomActionModel("add-features", addAspectParams); + rule.setActions(Arrays.asList(copyAction, addAspectAction)); final RestRuleModel updatedRule = restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .updateRule(rule.getId(), rule); @@ -519,14 +519,18 @@ public class UpdateRulesTests extends RestTest STEP("Try to update the rule by adding action with invalid parameter (non-existing namespace in value)"); final RestActionBodyExecTemplateModel action = new RestActionBodyExecTemplateModel(); action.setActionDefinitionId("add-features"); - action.setParams(Map.of("aspect-name", "dummy:dummy")); + final String aspectNameParam = "aspect-name"; + final String paramValue = "dummy:dummy"; + action.setParams(Map.of(aspectNameParam, paramValue)); rule.setActions(List.of(action)); restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() .updateRule(rule.getId(), rule); - restClient.assertStatusCodeIs(INTERNAL_SERVER_ERROR); - restClient.assertLastError().containsSummary("Namespace prefix dummy is not mapped to a namespace URI"); + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary( + String.format("Action parameter: %s has invalid value (%s). Look up possible values for constraint name %s", + aspectNameParam, paramValue, "ac-aspects")); } /** Check that a normal user cannot create rules that use private actions. */ diff --git a/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResource.java b/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResource.java new file mode 100644 index 0000000000..fa749d3310 --- /dev/null +++ b/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResource.java @@ -0,0 +1,59 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2017 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.rest.api.actions; + +import javax.servlet.http.HttpServletResponse; + +import org.alfresco.rest.api.Actions; +import org.alfresco.rest.api.model.ActionParameterConstraint; +import org.alfresco.rest.framework.WebApiDescription; +import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; +import org.alfresco.rest.framework.resource.EntityResource; +import org.alfresco.rest.framework.resource.actions.interfaces.EntityResourceAction; +import org.alfresco.rest.framework.resource.parameters.Parameters; +import org.alfresco.service.Experimental; + +@EntityResource(name="action-constraints", title = "Action parameter constraints") +@Experimental +public class ActionConstraintsEntityResource implements EntityResourceAction.ReadById +{ + private final Actions actions; + + public ActionConstraintsEntityResource(Actions actions) + { + this.actions = actions; + } + + @WebApiDescription(title = "Get single action parameters constraint", + description = "Retrieves a single action parameters constraint by constraint name", + successStatus = HttpServletResponse.SC_OK) + @Experimental + @Override + public ActionParameterConstraint readById(String constraintName, Parameters parameters) throws EntityNotFoundException + { + return actions.getActionConstraint(constraintName); + } +} diff --git a/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionValidator.java b/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionValidator.java new file mode 100644 index 0000000000..d88b35552f --- /dev/null +++ b/remote-api/src/main/java/org/alfresco/rest/api/actions/ActionValidator.java @@ -0,0 +1,38 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.actions; + +import org.alfresco.rest.api.model.rules.Action; +import org.alfresco.service.Experimental; + +@Experimental +public interface ActionValidator +{ + void validate(Action action); + + boolean isEnabled(); +} diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java index 02298d6eb3..da958f9a3c 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java @@ -453,7 +453,7 @@ public class ActionsImpl implements Actions map(this::toShortQName). collect(Collectors.toList()); } - + private String toShortQName(QName type) { return type.toPrefixString(prefixResolver); diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapper.java index 9946f80a28..84a070812f 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapper.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapper.java @@ -32,12 +32,14 @@ import static org.alfresco.repo.action.access.ActionAccessRestriction.ACTION_CON import java.io.Serializable; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; import org.alfresco.repo.action.ActionImpl; import org.alfresco.repo.action.CompositeActionImpl; +import org.alfresco.rest.api.actions.ActionValidator; import org.alfresco.rest.api.impl.rules.ActionParameterConverter; import org.alfresco.rest.api.model.mapper.RestModelMapper; import org.alfresco.rest.api.model.rules.Action; @@ -49,10 +51,13 @@ import org.apache.commons.collections.CollectionUtils; public class RestRuleActionModelMapper implements RestModelMapper { private final ActionParameterConverter parameterConverter; + private final List actionValidators; - public RestRuleActionModelMapper(ActionParameterConverter parameterConverter) + public RestRuleActionModelMapper(ActionParameterConverter parameterConverter, + List actionValidators) { this.parameterConverter = parameterConverter; + this.actionValidators = actionValidators; } /** @@ -104,8 +109,14 @@ public class RestRuleActionModelMapper implements RestModelMapper params = Optional.ofNullable(action.getParams()).orElse(emptyMap()); + validateAction(action); final Map convertedParams = parameterConverter.getConvertedParams(params, action.getActionDefinitionId()); return new ActionImpl(null, GUID.generate(), action.getActionDefinitionId(), convertedParams); } + private void validateAction(Action action) { + actionValidators.stream() + .filter(ActionValidator::isEnabled) + .forEach(v -> v.validate(action)); + } } diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidator.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidator.java new file mode 100644 index 0000000000..83778d347d --- /dev/null +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidator.java @@ -0,0 +1,133 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.impl.validator.actions; + +import java.io.Serializable; +import java.util.Map; + +import org.alfresco.rest.api.Actions; +import org.alfresco.rest.api.actions.ActionValidator; +import org.alfresco.rest.api.model.ActionDefinition; +import org.alfresco.rest.api.model.ActionParameterConstraint; +import org.alfresco.rest.api.model.rules.Action; +import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; +import org.alfresco.rest.framework.core.exceptions.NotFoundException; +import org.alfresco.service.Experimental; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; + +/** + * This class will validate all action types against action parameters definitions (mandatory parameters, parameter constraints) + */ +@Experimental +public class ActionParameterDefinitionValidator implements ActionValidator +{ + private static final boolean IS_ENABLED = true; + static final String INVALID_PARAMETER_VALUE = + "Action parameter: %s has invalid value (%s). Look up possible values for constraint name %s"; + static final String MISSING_PARAMETER = "Missing action's mandatory parameter: %s"; + static final String MUST_NOT_CONTAIN_PARAMETER = "Action of definition id: %s must not contain parameter of name: %s"; + static final String PARAMS_SHOULD_NOT_BE_EMPTY = + "Action parameters should not be null or empty for this action. See Action Definition for action of: %s"; + static final String INVALID_ACTION_DEFINITION = "Invalid action definition requested %s"; + + private final Actions actions; + + public ActionParameterDefinitionValidator(Actions actions) + { + this.actions = actions; + } + + /** + * Validates action against its parameters definitions (mandatory parameters, parameter constraints) + * + * @param action Action to be validated + */ + @Override + public void validate(Action action) + { + ActionDefinition actionDefinition; + try + { + actionDefinition = actions.getActionDefinitionById(action.getActionDefinitionId()); + } catch (NotFoundException e) { + throw new InvalidArgumentException(String.format(INVALID_ACTION_DEFINITION, action.getActionDefinitionId())); + } + validateParametersSize(action.getParams(), actionDefinition); + final Map params = action.getParams(); + if (MapUtils.isNotEmpty(params)) + { + params.forEach((key, value) -> checkParameterShouldExist(key, actionDefinition)); + actionDefinition.getParameterDefinitions().forEach(p -> validateParameterDefinitions(p, params)); + } + } + + @Override + public boolean isEnabled() + { + return IS_ENABLED; + } + + private void validateParametersSize(final Map params, final ActionDefinition actionDefinition) + { + if (CollectionUtils.isNotEmpty(actionDefinition.getParameterDefinitions()) && MapUtils.isEmpty(params)) + { + throw new IllegalArgumentException(String.format(PARAMS_SHOULD_NOT_BE_EMPTY, actionDefinition.getName())); + } + } + + private void validateParameterDefinitions(final ActionDefinition.ParameterDefinition parameterDefinition, + final Map params) + { + final Serializable parameterValue = params.get(parameterDefinition.getName()); + if (parameterDefinition.isMandatory() && parameterValue == null) + { + throw new IllegalArgumentException(String.format(MISSING_PARAMETER, parameterDefinition.getName())); + } + if (parameterDefinition.getParameterConstraintName() != null) + { + final ActionParameterConstraint actionConstraint = + actions.getActionConstraint(parameterDefinition.getParameterConstraintName()); + if (parameterValue != null && actionConstraint.getConstraintValues().stream() + .noneMatch(constraintData -> constraintData.getValue().equals(parameterValue.toString()))) + { + throw new IllegalArgumentException(String.format(INVALID_PARAMETER_VALUE, parameterDefinition.getName(), parameterValue, + actionConstraint.getConstraintName())); + } + } + } + + private void checkParameterShouldExist(final String parameterName, final ActionDefinition actionDefinition) + { + if (actionDefinition.getParameterDefinitions().stream().noneMatch(pd -> parameterName.equals(pd.getName()))) + { + throw new IllegalArgumentException(String.format(MUST_NOT_CONTAIN_PARAMETER, actionDefinition.getName(), parameterName)); + } + } + + +} diff --git a/remote-api/src/main/resources/alfresco/public-rest-context.xml b/remote-api/src/main/resources/alfresco/public-rest-context.xml index 500d7f317e..692b13c873 100644 --- a/remote-api/src/main/resources/alfresco/public-rest-context.xml +++ b/remote-api/src/main/resources/alfresco/public-rest-context.xml @@ -588,7 +588,14 @@ - + + + + + + + + org.alfresco.rest.api.Downloads @@ -965,6 +972,11 @@ + + + + + diff --git a/remote-api/src/test/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResourceTest.java b/remote-api/src/test/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResourceTest.java new file mode 100644 index 0000000000..16447288e1 --- /dev/null +++ b/remote-api/src/test/java/org/alfresco/rest/api/actions/ActionConstraintsEntityResourceTest.java @@ -0,0 +1,66 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.actions; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.then; + +import org.alfresco.rest.api.Actions; +import org.alfresco.rest.api.model.ActionParameterConstraint; +import org.alfresco.rest.framework.resource.parameters.Parameters; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ActionConstraintsEntityResourceTest +{ + @Mock + private Actions actionsMock; + @Mock + private Parameters parametersMock; + + @InjectMocks + private ActionConstraintsEntityResource objectUnderTest; + + @Test + public void testReadById() { + final String name = "name"; + final ActionParameterConstraint dummyConstraint = new ActionParameterConstraint(); + given(actionsMock.getActionConstraint(name)).willReturn(dummyConstraint); + + //when + ActionParameterConstraint result = objectUnderTest.readById(name, parametersMock); + + then(actionsMock).should().getActionConstraint(name); + then(actionsMock).shouldHaveNoMoreInteractions(); + assertThat(result).isNotNull().usingRecursiveComparison().isEqualTo(dummyConstraint); + } +} diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapperTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapperTest.java index abf23874a0..b3fd3b1082 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapperTest.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/mapper/rules/RestRuleActionModelMapperTest.java @@ -43,11 +43,14 @@ import java.util.List; import java.util.Map; import org.alfresco.repo.action.ActionImpl; +import org.alfresco.rest.api.actions.ActionValidator; import org.alfresco.rest.api.impl.rules.ActionParameterConverter; import org.alfresco.rest.api.model.rules.Action; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.StoreRef; +import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -65,10 +68,17 @@ public class RestRuleActionModelMapperTest @Mock private ActionParameterConverter parameterConverter; + @Mock + private ActionValidator sampleValidatorMock; - @InjectMocks private RestRuleActionModelMapper objectUnderTest; + @Before + public void setUp() { + objectUnderTest = new RestRuleActionModelMapper(parameterConverter, List.of(sampleValidatorMock)); + given(sampleValidatorMock.isEnabled()).willReturn(true); + } + @Test public void testToRestModel() { diff --git a/remote-api/src/test/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidatorTest.java b/remote-api/src/test/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidatorTest.java new file mode 100644 index 0000000000..bc4c8eb9ae --- /dev/null +++ b/remote-api/src/test/java/org/alfresco/rest/api/impl/validator/actions/ActionParameterDefinitionValidatorTest.java @@ -0,0 +1,217 @@ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2022 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.rest.api.impl.validator.actions; + +import static org.alfresco.rest.api.impl.validator.actions.ActionParameterDefinitionValidator.MISSING_PARAMETER; +import static org.alfresco.rest.api.impl.validator.actions.ActionParameterDefinitionValidator.MUST_NOT_CONTAIN_PARAMETER; +import static org.alfresco.rest.api.impl.validator.actions.ActionParameterDefinitionValidator.PARAMS_SHOULD_NOT_BE_EMPTY; +import static org.alfresco.service.cmr.dictionary.DataTypeDefinition.BOOLEAN; +import static org.alfresco.service.cmr.dictionary.DataTypeDefinition.TEXT; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.BDDMockito.then; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.alfresco.rest.api.Actions; +import org.alfresco.rest.api.model.ActionDefinition; +import org.alfresco.rest.api.model.rules.Action; +import org.alfresco.service.Experimental; +import org.alfresco.service.namespace.QName; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.BDDMockito; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@Experimental +@RunWith(MockitoJUnitRunner.class) +public class ActionParameterDefinitionValidatorTest +{ + private static final String MANDATORY_PARAM_KEY = "paramKey"; + private static final String NON_MANDATORY_PARAM_KEY = "nonMandatoryParamKey"; + + @Mock + private Actions actionsMock; + + @InjectMocks + private ActionParameterDefinitionValidator objectUnderTest; + + @Test + public void testSimpleValidationPasses() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + action.setParams(Map.of(MANDATORY_PARAM_KEY, "paramValue")); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + objectUnderTest.validate(action); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationPassesWhenNoParametersNeeded() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, null); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + objectUnderTest.validate(action); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationPassesWhenNoMandatoryParameters() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + action.setParams(Map.of(MANDATORY_PARAM_KEY, "paramValue")); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null), + createParameterDefinition(NON_MANDATORY_PARAM_KEY, BOOLEAN, false, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + objectUnderTest.validate(action); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationFailsWhenTooManyParameters() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + action.setParams(Map.of(MANDATORY_PARAM_KEY, "paramValue", NON_MANDATORY_PARAM_KEY, false)); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> objectUnderTest.validate(action)) + .withMessageContaining(String.format(MUST_NOT_CONTAIN_PARAMETER, actionDefinitionId, NON_MANDATORY_PARAM_KEY)); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationFailsWhenMissingParameters() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> objectUnderTest.validate(action)) + .withMessageContaining(String.format(PARAMS_SHOULD_NOT_BE_EMPTY, actionDefinitionId)); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationFailsWhenMissingParameterValue() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + final Map params = new HashMap<>(); + params.put(MANDATORY_PARAM_KEY, null); + action.setParams(params); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> objectUnderTest.validate(action)) + .withMessageContaining(String.format(MISSING_PARAMETER, MANDATORY_PARAM_KEY)); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + @Test + public void testValidationFailsWhenMandatoryParameterIsMissing() + { + final Action action = new Action(); + final String actionDefinitionId = "properActionDefinition"; + action.setActionDefinitionId(actionDefinitionId); + action.setParams(Map.of(NON_MANDATORY_PARAM_KEY, true)); + final List parameterDefinitions = + List.of(createParameterDefinition(MANDATORY_PARAM_KEY, TEXT, true, null), + createParameterDefinition(NON_MANDATORY_PARAM_KEY, BOOLEAN, false, null)); + final ActionDefinition actionDefinition = createActionDefinition(actionDefinitionId, parameterDefinitions); + BDDMockito.given(actionsMock.getActionDefinitionById(actionDefinitionId)).willReturn(actionDefinition); + + //when + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> objectUnderTest.validate(action)) + .withMessageContaining(String.format(MISSING_PARAMETER, MANDATORY_PARAM_KEY)); + + then(actionsMock).should().getActionDefinitionById(actionDefinitionId); + then(actionsMock).shouldHaveNoMoreInteractions(); + } + + private ActionDefinition createActionDefinition(final String actionDefinitionId, + List parameterDefinitions) + { + return new ActionDefinition(actionDefinitionId, actionDefinitionId, "title", "description", Collections.emptyList(), false, false, + parameterDefinitions); + } + + private ActionDefinition.ParameterDefinition createParameterDefinition(final String name, final QName qName, final boolean mandatory, + final String constraint) + { + return new ActionDefinition.ParameterDefinition(name, qName.toPrefixString(), false, mandatory, "label", constraint); + } + +}