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 b1d9dd6263..2dcf62071f 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 @@ -393,6 +393,42 @@ public class CreateRulesTests extends RestTest .assertThat().field("isShared").isNull(); } + /** + * Check we get error when attempt to create a rule without any actions. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithoutActionsShouldFail() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + ruleModel.setActions(null); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("A rule must have at least one action"); + } + + /** + * Check we get error when attempt to create a rule with invalid action. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void createRuleWithInvalidActionsShouldFail() + { + final RestRuleModel ruleModel = createRuleModelWithDefaultValues(); + final RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel(); + final String actionDefinitionId = "invalid-definition-value"; + invalidAction.setActionDefinitionId(actionDefinitionId); + invalidAction.setParams(Map.of("dummy-key", "dummy-value"));; + ruleModel.setActions(List.of(invalidAction)); + + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .createSingleRule(ruleModel); + + restClient.assertStatusCodeIs(NOT_FOUND); + restClient.assertLastError().containsSummary(actionDefinitionId); + } + /** * Check we can create a rule with multiple conditions */ 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 35e2cf1812..819dc36a2d 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 @@ -27,6 +27,7 @@ package org.alfresco.rest.rules; import static org.alfresco.rest.rules.RulesTestsUtils.createDefaultActionModel; import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModel; +import static org.alfresco.rest.rules.RulesTestsUtils.createRuleModelWithDefaultValues; import static org.alfresco.utility.constants.UserRole.SiteCollaborator; import static org.alfresco.utility.report.log.Step.STEP; import static org.springframework.http.HttpStatus.BAD_REQUEST; @@ -35,6 +36,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND; import static org.springframework.http.HttpStatus.OK; import java.util.List; +import java.util.Map; import com.google.common.collect.ImmutableMap; @@ -200,6 +202,46 @@ public class UpdateRulesTests extends RestTest updatedRule.assertThat().field("isShared").isNotNull(); } + /** + * Check we get error when attempt to update a rule to one without any actions. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void updateRuleWithoutActionsShouldFail() + { + RestRuleModel rule = createAndSaveRule("Rule name"); + + STEP("Try to update the rule - set no actions."); + rule.setActions(null); + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .include("isShared") + .updateRule(rule.getId(), rule); + + restClient.assertStatusCodeIs(BAD_REQUEST); + restClient.assertLastError().containsSummary("A rule must have at least one action"); + } + + /** + * Check we get error when attempt to update a rule to one with invalid action. + */ + @Test(groups = {TestGroup.REST_API, TestGroup.RULES}) + public void updateRuleWithInvalidActionsShouldFail() + { + RestRuleModel rule = createAndSaveRule("Rule name"); + + STEP("Try to update the rule - set no actions."); + final RestActionBodyExecTemplateModel invalidAction = new RestActionBodyExecTemplateModel(); + final String actionDefinitionId = "invalid-definition-value"; + invalidAction.setActionDefinitionId(actionDefinitionId); + invalidAction.setParams(Map.of("dummy-key", "dummy-value"));; + rule.setActions(List.of(invalidAction)); + restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet() + .include("isShared") + .updateRule(rule.getId(), rule); + + restClient.assertStatusCodeIs(NOT_FOUND); + restClient.assertLastError().containsSummary(actionDefinitionId); + } + /** Check we can use the POST response to create the new rule. */ @Test (groups = { TestGroup.REST_API, TestGroup.RULES, TestGroup.SANITY }) public void updateCopyRuleWithResponseFromPOST() 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 c860ac9556..3bcf0d32ba 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 @@ -48,6 +48,7 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.json.JSONArray; import org.json.JSONException; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; @Experimental public class ActionParameterConverter @@ -67,9 +68,15 @@ public class ActionParameterConverter Map getConvertedParams(Map params, String name) { final Map parameters = new HashMap<>(params.size()); - final ParameterizedItemDefinition definition = actionService.getActionDefinition(name); - if (definition == null) + final ParameterizedItemDefinition definition; + try { + definition = actionService.getActionDefinition(name); + if (definition == null) + { + throw new NotFoundException(NotFoundException.DEFAULT_MESSAGE_ID, new String[]{name}); + } + } catch (NoSuchBeanDefinitionException e) { throw new NotFoundException(NotFoundException.DEFAULT_MESSAGE_ID, new String[]{name}); } 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 2b791981fc..b5cce2a8e8 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 @@ -34,6 +34,7 @@ import org.alfresco.rest.api.Nodes; import org.alfresco.rest.api.Rules; import org.alfresco.rest.api.model.rules.Rule; import org.alfresco.rest.api.model.rules.RuleSet; +import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; import org.alfresco.rest.framework.resource.parameters.ListPage; import org.alfresco.rest.framework.resource.parameters.Paging; @@ -41,6 +42,7 @@ import org.alfresco.service.Experimental; import org.alfresco.service.cmr.action.CompositeAction; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.rule.RuleService; +import org.alfresco.util.collections.CollectionUtils; import org.alfresco.service.namespace.NamespaceService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,6 +51,7 @@ import org.slf4j.LoggerFactory; public class RulesImpl implements Rules { private static final Logger LOGGER = LoggerFactory.getLogger(RulesImpl.class); + private static final String MUST_HAVE_AT_LEAST_ONE_ACTION = "A rule must have at least one action"; private Nodes nodes; private RuleService ruleService; @@ -126,6 +129,10 @@ public class RulesImpl implements Rules private org.alfresco.service.cmr.rule.Rule mapToServiceModelAndValidateActions(Rule rule) { + if (CollectionUtils.isEmpty(rule.getActions())) + { + throw new InvalidArgumentException(MUST_HAVE_AT_LEAST_ONE_ACTION); + } final org.alfresco.service.cmr.rule.Rule serviceModelRule = rule.toServiceModel(nodes, namespaceService); final CompositeAction compositeAction = (CompositeAction) serviceModelRule.getAction(); compositeAction.getActions().forEach(action -> action.setParameterValues( 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 d5b40cbca3..7e5077834c 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 @@ -29,6 +29,7 @@ package org.alfresco.rest.api.impl.rules; import static org.alfresco.service.cmr.repository.StoreRef.STORE_REF_WORKSPACE_SPACESSTORE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; @@ -49,6 +50,7 @@ import org.alfresco.repo.action.executer.RemoveFeaturesActionExecuter; import org.alfresco.repo.action.executer.ScriptActionExecuter; import org.alfresco.repo.action.executer.SetPropertyValueActionExecuter; import org.alfresco.repo.action.executer.SimpleWorkflowActionExecuter; +import org.alfresco.rest.framework.core.exceptions.NotFoundException; import org.alfresco.service.Experimental; import org.alfresco.service.cmr.action.ActionDefinition; import org.alfresco.service.cmr.action.ActionService; @@ -553,6 +555,19 @@ public class ActionParameterConverterTest assertEquals(propType, convertedPropTypeParam); } + @Test + public void testInvalidActionDefinitionConversion() { + final String invalidName = "dummy-definition"; + final Map params = Map.of("dummy-key", "dummy-value"); + given(actionService.getActionDefinition(invalidName)).willReturn(null); + //when-then + assertThrows(NotFoundException.class, () ->objectUnderTest.getConvertedParams(params, invalidName)); + + given(actionService.getActionDefinition(invalidName)).willThrow(NotFoundException.class); + //when-then + assertThrows(NotFoundException.class, () ->objectUnderTest.getConvertedParams(params, invalidName)); + } + @Test public void testQnameServiceModelParamConversion() { given(namespaceService.getPrefixes(any())).willReturn(List.of("cm")); 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 e6797baa3b..c47f0a19d3 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 @@ -49,6 +49,7 @@ import junit.framework.TestCase; import org.alfresco.repo.action.ActionImpl; import org.alfresco.repo.action.CompositeActionImpl; import org.alfresco.rest.api.Nodes; +import org.alfresco.rest.api.model.rules.Action; import org.alfresco.rest.api.model.rules.Rule; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; @@ -103,6 +104,9 @@ public class RulesImplTest extends TestCase private org.alfresco.service.cmr.rule.Rule serviceRuleMock; @Mock private Rule ruleMock; + @Mock + private Action actionMock; + private org.alfresco.service.cmr.rule.Rule ruleModel = createRule(RULE_ID); private CompositeAction compositeAction = new CompositeActionImpl(RULE_NODE_REF, "compositeActionId"); @@ -292,6 +296,7 @@ public class RulesImplTest extends TestCase { List ruleList = List.of(ruleMock); given(ruleMock.toServiceModel(nodesMock, namespaceService)).willReturn(serviceRuleMock); + given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(serviceRuleMock.getAction()).willReturn(compositeAction); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(arg -> arg.getArguments()[1]); given(ruleLoaderMock.loadRule(serviceRuleMock, INCLUDE)).willReturn(ruleMock); @@ -321,6 +326,7 @@ public class RulesImplTest extends TestCase { List ruleList = List.of(ruleMock); given(ruleMock.toServiceModel(nodesMock, namespaceService)).willReturn(serviceRuleMock); + given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(arg -> arg.getArguments()[1]); given(ruleLoaderMock.loadRule(serviceRuleMock, INCLUDE)).willReturn(ruleMock); given(serviceRuleMock.getAction()).willReturn(compositeAction); @@ -363,6 +369,7 @@ public class RulesImplTest extends TestCase List expected = new ArrayList<>(); IntStream.range(0, 3).forEach(i -> { Rule ruleBodyMock = mock(Rule.class); + given(ruleBodyMock.getActions()).willReturn(List.of(actionMock)); ruleBodyList.add(ruleBodyMock); org.alfresco.service.cmr.rule.Rule serviceRuleMockInner = mock(org.alfresco.service.cmr.rule.Rule.class); given(ruleBodyMock.toServiceModel(nodesMock, namespaceService)).willReturn(serviceRuleMockInner); @@ -432,6 +439,27 @@ public class RulesImplTest extends TestCase } } + /** + * Fail on create a rule without any actions. + */ + @Test + public void testCreateRuleWithoutActionsShouldFail() + { + List ruleList = List.of(ruleMock); + given(ruleMock.getActions()).willReturn(null); + + // when + assertThatExceptionOfType(InvalidArgumentException.class) + .isThrownBy(() -> rules.createRules(FOLDER_NODE_REF.getId(), RULE_SET_NODE_REF.getId(), ruleList, INCLUDE)); + + then(nodeValidatorMock).should().validateFolderNode(FOLDER_NODE_ID, true); + then(nodeValidatorMock).should().validateRuleSetNode(RULE_SET_ID, FOLDER_NODE_REF); + then(nodeValidatorMock).shouldHaveNoMoreInteractions(); + then(actionParameterConverterMock).shouldHaveNoInteractions(); + then(actionPermissionValidatorMock).shouldHaveNoInteractions(); + then(ruleServiceMock).shouldHaveNoInteractions(); + } + /** * Check that we can update a rule. */ @@ -439,6 +467,7 @@ public class RulesImplTest extends TestCase public void testUpdateRuleById() { given(ruleMock.toServiceModel(nodesMock, namespaceService)).willReturn(serviceRuleMock); + given(ruleMock.getActions()).willReturn(List.of(actionMock)); given(ruleServiceMock.saveRule(FOLDER_NODE_REF, serviceRuleMock)).willAnswer(a -> a.getArguments()[1]); given(serviceRuleMock.getAction()).willReturn(compositeAction); given(ruleLoaderMock.loadRule(serviceRuleMock, INCLUDE)).willReturn(ruleMock); @@ -520,6 +549,28 @@ public class RulesImplTest extends TestCase } } + /** + * Fail on update a rule without any actions. + */ + @Test + public void testUpdateRuleWithoutActionShouldFail() + { + given(ruleMock.getActions()).willReturn(emptyList()); + + // when + assertThatExceptionOfType(InvalidArgumentException.class) + .isThrownBy(() -> 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).shouldHaveNoInteractions(); + then(actionParameterConverterMock).shouldHaveNoInteractions(); + then(actionPermissionValidatorMock).shouldHaveNoInteractions(); + } + @Test public void testDeleteRuleById() {