ACS-3227 Ensure the rule name is set. (#1240) [tas][ags]

* ACS-3227 Ensure the rule name is set. [tas]

* ACS-3227 Update tests that didn't set rule name.

* Update license year for test file.

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

@@ -115,6 +115,19 @@ public class CreateRulesTests extends RestTest
         restClient.assertLastError().containsSummary("fake-id was not found");
     }
 
+    /** Try to create a rule without a name and check the error. */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void createRuleWithEmptyName()
+    {
+        RestRuleModel ruleModel = new RestRuleModel();
+        ruleModel.setName("");
+
+        restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet().createSingleRule(ruleModel);
+
+        restClient.assertStatusCodeIs(BAD_REQUEST);
+        restClient.assertLastError().containsSummary("Rule name is a mandatory parameter");
+    }
+
     /** Check we can create two rules with the same name. */
     @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
     public void duplicateRuleNameIsAcceptable()
@@ -189,4 +202,23 @@ public class CreateRulesTests extends RestTest
                     .assertThat().field("id").isNotNull()
                     .assertThat().field("name").is(ruleNames.get(i)));
     }
+
+    /** Try to create several rules with an error in one of them. */
+    @Test (groups = { TestGroup.REST_API, TestGroup.RULES })
+    public void createRulesWithOneError()
+    {
+        STEP("Try to create a three rules but the middle one has an error.");
+        RestRuleModel ruleA = new RestRuleModel();
+        ruleA.setName("ruleA");
+        RestRuleModel ruleB = new RestRuleModel();
+        // Don't set a name for Rule B.
+        RestRuleModel ruleC = new RestRuleModel();
+        ruleC.setName("ruleC");
+        List<RestRuleModel> ruleModels = List.of(ruleA, ruleB, ruleC);
+
+        restClient.authenticateUser(user).withCoreAPI().usingNode(ruleFolder).usingDefaultRuleSet().createListOfRules(ruleModels);
+
+        restClient.assertStatusCodeIs(BAD_REQUEST);
+        restClient.assertLastError().containsSummary("Rule name is a mandatory parameter");
+    }
 }

repository/pom.xml

@@ -732,6 +732,11 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.alfresco</groupId>
             <artifactId>alfresco-transform-model</artifactId>

repository/src/main/java/org/alfresco/repo/rule/RuleServiceImpl.java

@@ -783,7 +783,9 @@ public class RuleServiceImpl
             }
 
             // Update the properties of the rule
-            this.nodeService.setProperty(ruleNodeRef, ContentModel.PROP_TITLE, rule.getTitle());
+            String title = rule.getTitle();
+            ParameterCheck.mandatoryString("Rule name", title);
+            this.nodeService.setProperty(ruleNodeRef, ContentModel.PROP_TITLE, title);
             this.nodeService.setProperty(ruleNodeRef, ContentModel.PROP_DESCRIPTION, rule.getDescription());
             this.nodeService.setProperty(ruleNodeRef, RuleModel.PROP_RULE_TYPE, (Serializable)rule.getRuleTypes());
             this.nodeService.setProperty(ruleNodeRef, RuleModel.PROP_APPLY_TO_CHILDREN, rule.isAppliedToChildren());

repository/src/test/java/org/alfresco/AllUnitTestsSuite.java

@@ -73,6 +73,7 @@ import org.junit.runners.Suite;
     org.alfresco.repo.rendition.RenditionNodeManagerTest.class,
     org.alfresco.repo.rendition.RenditionServiceImplTest.class,
     org.alfresco.repo.replication.ReplicationServiceImplTest.class,
+    org.alfresco.repo.rule.RuleServiceImplUnitTest.class,
     org.alfresco.repo.service.StoreRedirectorProxyFactoryTest.class,
     org.alfresco.repo.site.RoleComparatorImplTest.class,
     org.alfresco.repo.tenant.MultiTAdminServiceImplTest.class,

repository/src/test/java/org/alfresco/repo/copy/CopyServiceImplTest.java

@@ -737,6 +737,7 @@ public class CopyServiceImplTest extends TestCase
         // Create a new rule and add it to the source noderef
         Rule rule = new Rule();
         rule.setRuleType(RuleType.INBOUND);
+        rule.setTitle("Rule name");
         
         Map<String, Serializable> props = new HashMap<String, Serializable>(1);
         props.put(AddFeaturesActionExecuter.PARAM_ASPECT_NAME, ContentModel.ASPECT_VERSIONABLE);
@@ -1010,7 +1011,8 @@ public class CopyServiceImplTest extends TestCase
         Map<String, Serializable> params = new HashMap<String, Serializable>(1);
         params.put(MoveActionExecuter.PARAM_DESTINATION_FOLDER, nodeTwo);
         Rule rule = new Rule();
-        rule.setRuleType(RuleType.INBOUND);        
+        rule.setRuleType(RuleType.INBOUND);
+        rule.setTitle("Rule name");
         Action action = actionService.createAction(CopyActionExecuter.NAME, params);
         ActionCondition condition = actionService.createActionCondition(NoConditionEvaluator.NAME);
         action.addActionCondition(condition);

repository/src/test/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java

@@ -99,6 +99,7 @@ import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.GUID;
 import org.junit.experimental.categories.Category;
 import org.springframework.context.ApplicationContext;
 import org.springframework.util.StopWatch;
@@ -194,25 +195,26 @@ public class RuleServiceCoverageTest extends TestCase
                 ContentModel.ASSOC_CHILDREN,
                 ContentModel.TYPE_CONTAINER).getChildRef();      
     }
-	
+
-	private Rule createRule(
+    private Rule createRule(
-			String ruleTypeName, 
+            String ruleTypeName,
-			String actionName, 
+            String actionName,
-			Map<String, Serializable> actionParams, 
+            Map<String, Serializable> actionParams,
-			String conditionName, 
+            String conditionName,
-			Map<String, Serializable> conditionParams)
+            Map<String, Serializable> conditionParams)
-	{
+    {
-		Rule rule = new Rule();
+        Rule rule = new Rule();
-        rule.setRuleType(ruleTypeName);        
+        rule.setTitle(GUID.generate());
-        
+        rule.setRuleType(ruleTypeName);
-        Action action = this.actionService.createAction(actionName, actionParams);        
+
+        Action action = this.actionService.createAction(actionName, actionParams);
         ActionCondition condition = this.actionService.createActionCondition(conditionName, conditionParams);
         action.addActionCondition(condition);
-        rule.setAction(action);  
+        rule.setAction(action);
-        
+
         return rule;
-	}
+    }
-    
+
     /**
      * Create the categories used in the tests
      */

repository/src/test/java/org/alfresco/repo/rule/RuleServiceImplUnitTest.java

@@ -0,0 +1,190 @@
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * 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 <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.repo.rule;
+
+import static org.alfresco.model.ContentModel.ASSOC_CONTAINS;
+import static org.alfresco.repo.rule.RuleModel.ASSOC_ACTION;
+import static org.alfresco.repo.rule.RuleModel.ASSOC_RULE_FOLDER;
+import static org.alfresco.repo.rule.RuleModel.TYPE_RULE;
+import static org.alfresco.service.cmr.security.AccessStatus.ALLOWED;
+import static org.alfresco.service.cmr.security.AccessStatus.DENIED;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.nullable;
+import static org.mockito.BDDMockito.then;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.openMocks;
+
+import java.io.Serializable;
+import java.util.List;
+
+import org.alfresco.repo.action.RuntimeActionService;
+import org.alfresco.repo.cache.SimpleCache;
+import org.alfresco.service.cmr.action.Action;
+import org.alfresco.service.cmr.action.ActionServiceException;
+import org.alfresco.service.cmr.repository.ChildAssociationRef;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.cmr.rule.Rule;
+import org.alfresco.service.cmr.rule.RuleService;
+import org.alfresco.service.cmr.rule.RuleServiceException;
+import org.alfresco.service.cmr.security.PermissionService;
+import org.alfresco.service.namespace.QName;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+
+/** Unit tests for {@link RuleServiceImpl}. */
+public class RuleServiceImplUnitTest
+{
+    private static final NodeRef FOLDER_NODE = new NodeRef("folder://node/");
+    private static final NodeRef RULE_SET_NODE = new NodeRef("rule://set/node");
+    private static final NodeRef RULE_NODE = new NodeRef("rule://node/");
+    private static final NodeRef ACTION_NODE = new NodeRef("action://node/");
+    @InjectMocks
+    private RuleService ruleService = new RuleServiceImpl();
+    @Mock
+    private NodeService nodeService;
+    @Mock
+    private PermissionService permissionService;
+    @Mock
+    private SimpleCache nodeRulesCache;
+    @Mock
+    private NodeService runtimeNodeService;
+    @Mock
+    private RuntimeActionService runtimeActionService;
+    @Mock
+    private Rule mockRule;
+    @Mock
+    private Action mockAction;
+
+    @Before
+    public void setUp()
+    {
+        openMocks(this);
+    }
+
+    @Test
+    public void saveRule()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(ALLOWED);
+        when(nodeService.exists(FOLDER_NODE)).thenReturn(true);
+        ChildAssociationRef ruleSet = mock(ChildAssociationRef.class);
+        when(ruleSet.getChildRef()).thenReturn(RULE_SET_NODE);
+        when(runtimeNodeService.getChildAssocs(FOLDER_NODE, ASSOC_RULE_FOLDER, ASSOC_RULE_FOLDER)).thenReturn(List.of(ruleSet));
+        ChildAssociationRef ruleAssociation = mock(ChildAssociationRef.class);
+        when(ruleAssociation.getChildRef()).thenReturn(RULE_NODE);
+        when(nodeService.createNode(eq(RULE_SET_NODE), eq(ASSOC_CONTAINS), any(QName.class), eq(TYPE_RULE))).thenReturn(ruleAssociation);
+        // Set the rule title and action.
+        when(mockRule.getTitle()).thenReturn("Rule title");
+        when(mockRule.getAction()).thenReturn(mockAction);
+        when(runtimeActionService.createActionNodeRef(mockAction, RULE_NODE, ASSOC_ACTION, ASSOC_ACTION)).thenReturn(ACTION_NODE);
+
+        // Call the method under test.
+        ruleService.saveRule(FOLDER_NODE, mockRule);
+
+        then(nodeService).should(times(2)).hasAspect(FOLDER_NODE, RuleModel.ASPECT_RULES);
+        then(nodeService).should().exists(FOLDER_NODE);
+        then(nodeService).should().addAspect(FOLDER_NODE, RuleModel.ASPECT_RULES, null);
+        then(nodeService).should().createNode(eq(RULE_SET_NODE), eq(ASSOC_CONTAINS), any(QName.class), eq(TYPE_RULE));
+        then(nodeService).should(atLeastOnce()).setProperty(eq(RULE_NODE), any(QName.class), nullable(Serializable.class));
+        then(nodeService).should().getChildAssocs(RULE_NODE, RuleModel.ASSOC_ACTION, RuleModel.ASSOC_ACTION);
+        verifyNoMoreInteractions(nodeService);
+    }
+
+    @Test
+    public void saveRule_missingAction()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(ALLOWED);
+        when(nodeService.exists(FOLDER_NODE)).thenReturn(true);
+        ChildAssociationRef ruleSet = mock(ChildAssociationRef.class);
+        when(ruleSet.getChildRef()).thenReturn(RULE_SET_NODE);
+        when(runtimeNodeService.getChildAssocs(FOLDER_NODE, ASSOC_RULE_FOLDER, ASSOC_RULE_FOLDER)).thenReturn(List.of(ruleSet));
+        ChildAssociationRef ruleAssociation = mock(ChildAssociationRef.class);
+        when(nodeService.createNode(eq(RULE_SET_NODE), eq(ASSOC_CONTAINS), any(QName.class), eq(TYPE_RULE))).thenReturn(ruleAssociation);
+        // Set the title and no action for the rule.
+        when(mockRule.getTitle()).thenReturn("Rule title");
+        when(mockRule.getAction()).thenReturn(null);
+
+        // Call the method under test.
+        assertThatExceptionOfType(RuleServiceException.class).isThrownBy(() -> ruleService.saveRule(FOLDER_NODE, mockRule));
+    }
+
+    @Test
+    public void saveRule_missingTitle()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(ALLOWED);
+        when(nodeService.exists(FOLDER_NODE)).thenReturn(true);
+        ChildAssociationRef ruleSet = mock(ChildAssociationRef.class);
+        when(ruleSet.getChildRef()).thenReturn(RULE_SET_NODE);
+        when(runtimeNodeService.getChildAssocs(FOLDER_NODE, ASSOC_RULE_FOLDER, ASSOC_RULE_FOLDER)).thenReturn(List.of(ruleSet));
+        ChildAssociationRef ruleAssociation = mock(ChildAssociationRef.class);
+        when(nodeService.createNode(eq(RULE_SET_NODE), eq(ASSOC_CONTAINS), any(QName.class), eq(TYPE_RULE))).thenReturn(ruleAssociation);
+        // The rule has an empty title.
+        when(mockRule.getTitle()).thenReturn("");
+
+        assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> ruleService.saveRule(FOLDER_NODE, mockRule));
+    }
+
+    @Test
+    public void saveRule_errorIfFolderHasMultipleRuleSets()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(ALLOWED);
+        when(nodeService.exists(FOLDER_NODE)).thenReturn(true);
+        // Simulate a folder node with several rule sets.
+        ChildAssociationRef childA = mock(ChildAssociationRef.class);
+        ChildAssociationRef childB = mock(ChildAssociationRef.class);
+        when(runtimeNodeService.getChildAssocs(
+                FOLDER_NODE,
+                ASSOC_RULE_FOLDER,
+                ASSOC_RULE_FOLDER)).thenReturn(List.of(childA, childB));
+
+        assertThatExceptionOfType(ActionServiceException.class).isThrownBy(() -> ruleService.saveRule(FOLDER_NODE, mockRule));
+    }
+
+    @Test
+    public void saveRule_nodeDoesNotExist()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(ALLOWED);
+        when(nodeService.exists(FOLDER_NODE)).thenReturn(false);
+
+        assertThatExceptionOfType(RuleServiceException.class).isThrownBy(() -> ruleService.saveRule(FOLDER_NODE, mockRule));
+    }
+
+    @Test
+    public void saveRule_accessDenied()
+    {
+        when(permissionService.hasPermission(FOLDER_NODE, PermissionService.CHANGE_PERMISSIONS)).thenReturn(DENIED);
+
+        assertThatExceptionOfType(RuleServiceException.class).isThrownBy(() -> ruleService.saveRule(FOLDER_NODE, mockRule));
+    }
+}

repository/src/test/java/org/alfresco/repo/thumbnail/ThumbnailServiceImplTest.java

@@ -495,6 +495,7 @@ public class ThumbnailServiceImplTest extends BaseAlfrescoSpringTest
         params.put("aspect-name", ContentModel.ASPECT_GEN_CLASSIFIABLE);
         Rule rule = new Rule();
         rule.setRuleType(RuleType.INBOUND);
+        rule.setTitle("Rule name");
         Action action = this.actionService.createAction(AddFeaturesActionExecuter.NAME, params);
         ActionCondition condition = this.actionService.createActionCondition(NoConditionEvaluator.NAME, null);
         action.addActionCondition(condition);