ALF-3976 Added more unit tests for TaskFormProcessor and WorkflowFormProcessor.

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@21469 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/forms/processor/node/AssociationFieldProcessor.java

@@ -72,7 +72,7 @@ public class AssociationFieldProcessor extends QNameFieldProcessor<AssociationDe
         Serializable values = data.getAssociationValue(name);
         if (values == null)
         {
-            return Collections.EMPTY_LIST;
+            return Collections.emptyList();
         }
         if (values instanceof Collection<?>)
         {

source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessor.java

@@ -26,6 +26,7 @@
 package org.alfresco.repo.forms.processor.workflow;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -44,7 +45,6 @@ import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.util.StringUtils;
 
 /**
  * 
@@ -107,7 +107,7 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas
     protected String getItemURI(WorkflowTask item)
     {
         // TODO Check this URL is OK.
-        return "/api/task-instances/" + item.id;
+        return "api/task-instances/" + item.id;
     }
 
     /*
@@ -152,10 +152,15 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas
      * @param item
      * @return
      */
-    private Object getPackageItemValues(WorkflowTask item)
+    private Object getPackageItemValues(WorkflowTask task)
     {
-        List<NodeRef> items = workflowService.getPackageContents(item.getId());
+        List<NodeRef> items = workflowService.getPackageContents(task.getId());
-        return StringUtils.collectionToCommaDelimitedString(items);
+        ArrayList<String> results = new ArrayList<String>(items.size());
+        for (NodeRef item : items)
+        {
+            results.add(item.toString());
+        }
+        return results;
     }
 
     private String getTransitionValues(WorkflowTask item)

source/java/org/alfresco/repo/forms/processor/workflow/TaskFormProcessorTest.java

@@ -34,8 +34,10 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.alfresco.repo.forms.processor.node.FormFieldConstants.*;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -61,6 +63,7 @@ import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.cmr.dictionary.TypeDefinition;
+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.workflow.WorkflowException;
@@ -75,6 +78,8 @@ import org.alfresco.service.cmr.workflow.WorkflowTransition;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.NamespaceServiceMemoryImpl;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.QNamePattern;
+import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -152,7 +157,7 @@ public class TaskFormProcessorTest extends TestCase
         assertEquals(item.getKind(), formItem.getKind());
         String expType = NamespaceService.BPM_MODEL_PREFIX + ":" + TASK_DEF_NAME;
         assertEquals(expType, formItem.getType());
-        assertEquals("/api/task-instances/" + TASK_ID, formItem.getUrl());
+        assertEquals("api/task-instances/" + TASK_ID, formItem.getUrl());
     }
 
     public void testGenerateSingleProperty()
@@ -249,7 +254,7 @@ public class TaskFormProcessorTest extends TestCase
         // Check empty package
         String fieldName = PackageItemsFieldProcessor.KEY;
         Form form = processForm(fieldName);
-        String packageItems = "";
+        Serializable packageItems = (Serializable) Collections.emptyList();
         checkSingleAssociation(form, fieldName, packageItems);
         
         // Effectively add 3 items to package.
@@ -258,7 +263,9 @@ public class TaskFormProcessorTest extends TestCase
             .thenReturn(value);
         
         form = processForm(fieldName);
-        packageItems = FAKE_NODE+","+FAKE_NODE2+","+FAKE_NODE3;
+        packageItems = (Serializable) Arrays.asList(FAKE_NODE.toString(),
+                    FAKE_NODE2.toString(),
+                    FAKE_NODE3.toString());
         checkSingleAssociation(form, fieldName, packageItems);
     }
 
@@ -273,7 +280,7 @@ public class TaskFormProcessorTest extends TestCase
     public void testPersistPropertyChanged() throws Exception
     {
         String fieldName = DESC_NAME.toPrefixString(namespaceService);
-        String dataKey = makeDataKeyName(fieldName, FormFieldConstants.PROP_DATA_PREFIX);
+        String dataKey = makeDataKeyName(fieldName);
         String value = "New Description";
 
         processPersist(dataKey, value);
@@ -285,7 +292,7 @@ public class TaskFormProcessorTest extends TestCase
     public void testPersistPropertyWith_() throws Exception
     {
         String fieldName = PROP_WITH_.toPrefixString(namespaceService);
-        String dataKey = makeDataKeyName(fieldName, FormFieldConstants.PROP_DATA_PREFIX);
+        String dataKey = makeDataKeyName(fieldName);
         String value = "New _ Value";
 
         processPersist(dataKey, value);
@@ -297,8 +304,7 @@ public class TaskFormProcessorTest extends TestCase
     public void testPersistAssociationAdded() throws Exception
     {
         String fieldName = ACTORS_NAME.toPrefixString(namespaceService);
-        String dataKey = makeDataKeyName(fieldName, FormFieldConstants.ASSOC_DATA_PREFIX);
+        String dataKey = makeDataKeyName(fieldName, true);
-        dataKey = dataKey + FormFieldConstants.ASSOC_DATA_ADDED_SUFFIX;
         String nodeRef1 = FAKE_NODE.toString() + "1";
         String nodeRef2 = FAKE_NODE.toString() + "2";
         String value = nodeRef1 + ", " + nodeRef2;
@@ -315,8 +321,7 @@ public class TaskFormProcessorTest extends TestCase
     public void testPersistAssociationsRemoved() throws Exception
     {
         String fieldName = ASSIGNEE_NAME.toPrefixString(namespaceService);
-        String dataKey = makeDataKeyName(fieldName, FormFieldConstants.ASSOC_DATA_PREFIX);
+        String dataKey = makeDataKeyName(fieldName, false);
-        dataKey = dataKey + FormFieldConstants.ASSOC_DATA_REMOVED_SUFFIX;
         String value = FAKE_NODE.toString();
         processPersist(dataKey, value);
 
@@ -330,19 +335,15 @@ public class TaskFormProcessorTest extends TestCase
     public void testPersistAssociationAddedWith_() throws Exception
     {
         String fieldName = ASSOC_WITH_.toPrefixString(namespaceService);
-        String dataKey = makeDataKeyName(fieldName, FormFieldConstants.ASSOC_DATA_PREFIX);
+        String dataKey = makeDataKeyName(fieldName, true);
-        dataKey = dataKey + FormFieldConstants.ASSOC_DATA_ADDED_SUFFIX;
+        String value = FAKE_NODE + ", " + FAKE_NODE2;
-        String nodeRef1 = FAKE_NODE.toString() + "1";
-        String nodeRef2 = FAKE_NODE.toString() + "2";
-        String value = nodeRef1 + ", " + nodeRef2;
         processPersist(dataKey, value);
-
         assertEquals(1, actualAdded.size());
         List<NodeRef> nodeRefs = actualAdded.get(ASSOC_WITH_);
         assertNotNull(nodeRefs);
         assertEquals(2, nodeRefs.size());
-        assertTrue(nodeRefs.contains(new NodeRef(nodeRef1)));
+        assertTrue(nodeRefs.contains(FAKE_NODE));
-        assertTrue(nodeRefs.contains(new NodeRef(nodeRef2)));
+        assertTrue(nodeRefs.contains(FAKE_NODE2));
     }
     
     @SuppressWarnings("unchecked")
@@ -355,7 +356,7 @@ public class TaskFormProcessorTest extends TestCase
         verify(workflowService, never()).endTask(eq(TASK_ID), anyString());
         
         // Check default transition.
-        String dataKey =FormFieldConstants.PROP_DATA_PREFIX+TransitionFieldProcessor.KEY;
+        String dataKey =makeDataKeyName(TransitionFieldProcessor.KEY);
         processPersist(dataKey, null);
         verify(workflowService, times(1)).endTask(TASK_ID, null);
         
@@ -364,6 +365,62 @@ public class TaskFormProcessorTest extends TestCase
         verify(workflowService, times(1)).endTask(TASK_ID, "foo");
     }
     
+    public void testPersistPackageItemsAdded() throws Exception
+    {
+        mockPackageItems(FAKE_NODE3);
+        String dataKey = makeDataKeyName(PackageItemsFieldProcessor.KEY, true); 
+        String value = FAKE_NODE + ", " + FAKE_NODE2;
+        processPersist(dataKey, value);
+        checkAddPackageItem(FAKE_NODE, true);
+        checkAddPackageItem(FAKE_NODE2, true);
+        checkAddPackageItem(FAKE_NODE3, false);
+    }
+    
+    public void testPersistPackageItemsRemoved() throws Exception
+    {
+        mockPackageItems(FAKE_NODE, FAKE_NODE2);
+        String dataKey = makeDataKeyName(PackageItemsFieldProcessor.KEY, false); 
+        String value = FAKE_NODE + ", " + FAKE_NODE2+ "," + FAKE_NODE3;
+        processPersist(dataKey, value);
+        
+        // Check nodes 1 and 2 removed correctly.
+        checkRemovedPackageItem(FAKE_NODE, true);
+        checkRemovedPackageItem(FAKE_NODE2, true);
+        
+        // Check node 3 is not removed as it was not in the package to start with.
+        checkRemovedPackageItem(FAKE_NODE3, false);
+    }
+
+    private void mockPackageItems(NodeRef... children)
+    {
+        ArrayList<ChildAssociationRef> results = new ArrayList<ChildAssociationRef>(children.length);
+        for (NodeRef nodeRef : children)
+        {
+            ChildAssociationRef child = new ChildAssociationRef(WorkflowModel.ASSOC_PACKAGE_CONTAINS, PCKG_NODE, null, nodeRef);
+            results.add(child);
+        }
+        when(nodeService.getChildAssocs(eq(PCKG_NODE), (QNamePattern)any(), (QNamePattern)any()))
+        .thenReturn(results);
+        
+    }
+
+    private void checkRemovedPackageItem(NodeRef child, boolean wasCalled)
+    {
+        int times = wasCalled ? 1 : 0;
+        verify(nodeService, times(times))
+            .removeChild(PCKG_NODE, child);
+    }
+
+    private void checkAddPackageItem(NodeRef child, boolean wasCalled)
+    {
+        int times = wasCalled ? 1 : 0;
+        verify(nodeService, times(times))
+            .addChild(eq(PCKG_NODE),
+                        eq(child),
+                        eq(WorkflowModel.ASSOC_PACKAGE_CONTAINS),
+                        (QName)any());
+    }
+    
     private void processPersist(String dataKey, String value)
     {
         FormData data = new FormData();
@@ -397,37 +454,45 @@ public class TaskFormProcessorTest extends TestCase
 
     private void checkSingleProperty(Form form, String fieldName, Serializable fieldData)
     {
-        checkSingleField(form, fieldName, fieldData, "prop_");
+        String expDataKey = makeDataKeyName(fieldName);
+        checkSingleField(form, fieldName, fieldData, expDataKey);
 
     }
 
     private void checkSingleAssociation(Form form, String fieldName, Serializable fieldData)
     {
-        checkSingleField(form, fieldName, fieldData, "assoc_");
+        String expDataKey = makeAssociationDataKey(fieldName);
+        checkSingleField(form, fieldName, fieldData, expDataKey);
 
     }
 
-    private void checkSingleField(Form form, String fieldName, Serializable fieldData, String prefix)
+    private void checkSingleField(Form form, String fieldName, Serializable fieldData, String expDataKey)
     {
         List<FieldDefinition> fieldDefs = form.getFieldDefinitions();
         assertEquals(1, fieldDefs.size());
         FieldDefinition fieldDef = fieldDefs.get(0);
         assertEquals(fieldName, fieldDef.getName());
         String dataKey = fieldDef.getDataKeyName();
-        String expDataKey = makeDataKeyName(fieldName, prefix);
         assertEquals(expDataKey, dataKey);
         FieldData data = form.getFormData().getFieldData(dataKey);
         assertEquals(fieldData, data.getValue());
     }
 
-    /**
+    private String makeDataKeyName(String fieldName)
-     * @param fieldName
-     * @param prefix
-     * @return
-     */
-    private String makeDataKeyName(String fieldName, String prefix)
     {
-        return prefix + fieldName.replace(":", "_");
+        return PROP_DATA_PREFIX + fieldName.replace(":", "_");
+    }
+
+    private String makeDataKeyName(String fieldName, boolean added)
+    {
+        String assocDataKey = makeAssociationDataKey(fieldName);
+        String suffix = added ? ASSOC_DATA_ADDED_SUFFIX : ASSOC_DATA_REMOVED_SUFFIX;
+        return assocDataKey + suffix;
+    }
+    
+    private String makeAssociationDataKey(String fieldName)
+    {
+        return ASSOC_DATA_PREFIX + fieldName.replace(":", "_");
     }
 
     /*

source/java/org/alfresco/repo/forms/processor/workflow/WorkflowFormProcessor.java

@@ -79,7 +79,7 @@ public class WorkflowFormProcessor extends AbstractWorkflowFormProcessor<Workflo
     protected Map<String, Object> getTransientValues(WorkflowDefinition item)
     {
         return Collections.<String, Object>singletonMap(
-                    PackageItemsFieldProcessor.KEY, Collections.EMPTY_LIST);
+                    PackageItemsFieldProcessor.KEY, Collections.emptyList());
     }
 
     /* (non-Javadoc)

source/java/org/alfresco/repo/forms/processor/workflow/WorkflowFormProcessorTest.java

@@ -19,8 +19,61 @@
 
 package org.alfresco.repo.forms.processor.workflow;
 
+import static org.alfresco.repo.forms.processor.node.FormFieldConstants.ASSOC_DATA_ADDED_SUFFIX;
+import static org.alfresco.repo.forms.processor.node.FormFieldConstants.ASSOC_DATA_PREFIX;
+import static org.alfresco.repo.forms.processor.node.FormFieldConstants.ASSOC_DATA_REMOVED_SUFFIX;
+import static org.alfresco.repo.forms.processor.node.FormFieldConstants.PROP_DATA_PREFIX;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyMap;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import junit.framework.TestCase;
 
+import org.alfresco.repo.forms.FieldDefinition;
+import org.alfresco.repo.forms.Form;
+import org.alfresco.repo.forms.FormData;
+import org.alfresco.repo.forms.FormNotFoundException;
+import org.alfresco.repo.forms.Item;
+import org.alfresco.repo.forms.FormData.FieldData;
+import org.alfresco.repo.forms.processor.node.DefaultFieldProcessor;
+import org.alfresco.repo.forms.processor.node.MockClassAttributeDefinition;
+import org.alfresco.repo.forms.processor.node.MockFieldProcessorRegistry;
+import org.alfresco.repo.workflow.WorkflowModel;
+import org.alfresco.service.cmr.dictionary.AssociationDefinition;
+import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
+import org.alfresco.service.cmr.dictionary.DictionaryService;
+import org.alfresco.service.cmr.dictionary.PropertyDefinition;
+import org.alfresco.service.cmr.dictionary.TypeDefinition;
+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.workflow.WorkflowDefinition;
+import org.alfresco.service.cmr.workflow.WorkflowInstance;
+import org.alfresco.service.cmr.workflow.WorkflowNode;
+import org.alfresco.service.cmr.workflow.WorkflowPath;
+import org.alfresco.service.cmr.workflow.WorkflowService;
+import org.alfresco.service.cmr.workflow.WorkflowTask;
+import org.alfresco.service.cmr.workflow.WorkflowTaskDefinition;
+import org.alfresco.service.namespace.NamespaceService;
+import org.alfresco.service.namespace.NamespaceServiceMemoryImpl;
+import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.QNamePattern;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
 /**
  * 
  * @since 3.4
@@ -29,8 +82,524 @@ import junit.framework.TestCase;
  */
 public class WorkflowFormProcessorTest extends TestCase
 {
-    public void testNothing()
+    private static final String TASK_DEF_NAME = "TaskDef";
+    private static final String WF_DEF_NAME = "foo$wf:bar";
+    private static final QName PRIORITY_NAME = WorkflowModel.PROP_PRIORITY;
+    private static final QName DESC_NAME = WorkflowModel.PROP_DESCRIPTION;
+    private static final QName STATUS_NAME = WorkflowModel.PROP_STATUS;
+    private static final QName PROP_WITH_ = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "some_prop");
+    private static final QName ACTORS_NAME = WorkflowModel.ASSOC_POOLED_ACTORS;
+    private static final QName ASSIGNEE_NAME = WorkflowModel.ASSOC_ASSIGNEE;
+    private static final QName ASSOC_WITH_ = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "some_assoc");
+    private static final NodeRef FAKE_NODE = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakeNode");
+    private static final NodeRef FAKE_NODE2 = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakeNode2");
+    private static final NodeRef FAKE_NODE3 = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakeNode3");
+    private static final NodeRef PCKG_NODE = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakePackage");
+    private static final Item item = new Item("workflow", WF_DEF_NAME);
+
+    private NamespaceService namespaceService;
+    private NodeService nodeService;
+    private WorkflowService workflowService;
+    private WorkflowFormProcessor processor;
+    private WorkflowInstance newInstance;
+    private WorkflowDefinition definition;
+    private Map<QName, Serializable> actualProperties = null;
+    
+    public void testGetTypedItem() throws Exception
     {
-        //Do nothings.
+        try
+        {
+            processor.getTypedItem(null);
+            fail("Should have thrown an Exception here!");
         }
+        catch (FormNotFoundException e)
+        {
+            // Do nothing!
+        }
+
+        try
+        {
+            processor.getTypedItem(new Item("task", "bad id"));
+            fail("Should have thrown an Exception here!");
+        }
+        catch (FormNotFoundException e)
+        {
+            // Do nothing!
+        }
+
+        WorkflowDefinition result = processor.getTypedItem(item);
+        assertNotNull(result);
+        assertEquals(WF_DEF_NAME, result.getName());
+        
+        // Check URI-encoded id.
+        Item itemWith_ = new Item("workflow", WF_DEF_NAME.replace('$', '_'));
+        result = processor.getTypedItem(itemWith_);
+        assertNotNull(result);
+        assertEquals(WF_DEF_NAME, result.getName());
+    }
+
+    public void testGenerateSetsItemAndUrl() throws Exception
+    {
+        Form form = processor.generate(item, null, null, null);
+        Item formItem = form.getItem();
+        assertEquals(item.getId(), formItem.getId());
+        assertEquals(item.getKind(), formItem.getKind());
+        assertEquals(WF_DEF_NAME, formItem.getType());
+        assertEquals("api/workflow-definitions/" + definition.getId(), formItem.getUrl());
+    }
+
+    public void testGenerateSingleProperty()
+    {
+        // Check Status field is added to Form.
+        String fieldName = PRIORITY_NAME.toPrefixString(namespaceService);
+        List<String> fields = Arrays.asList(fieldName);
+        Form form = processForm(fields);
+        checkSingleProperty(form, fieldName, "2");
+
+        // Check Status field is added to Form, when explicitly typed as a
+        // property.
+        String fullPropertyName = "prop:" + fieldName;
+        fields = Arrays.asList(fullPropertyName);
+        form = processForm(fields);
+        checkSingleProperty(form, fieldName, "2");
+        checkPackageActionGroups(form.getFormData());
+    }
+
+    public void testGenerateSingleAssociation()
+    {
+        Serializable values = (Serializable) Collections.emptyList();
+        // Check Assignee field is added to Form.
+        String fieldName = ASSIGNEE_NAME.toPrefixString(namespaceService);
+        List<String> fields = Arrays.asList(fieldName);
+        Form form = processForm(fields);
+        checkSingleAssociation(form, fieldName, values);
+
+        // Check Assignee field is added to Form, when explicitly typed as an
+        // association.
+        String fullAssociationName = "assoc:" + fieldName;
+        fields = Arrays.asList(fullAssociationName);
+        form = processForm(fields);
+        checkSingleAssociation(form, fieldName, values);
+        checkPackageActionGroups(form.getFormData());
+    }
+
+    public void testIgnoresUnknownFields() throws Exception
+    {
+        String fakeFieldName = NamespaceService.BPM_MODEL_PREFIX + ":" + "Fake Field";
+        String priorityField = PRIORITY_NAME.toPrefixString(namespaceService);
+        List<String> fields = Arrays.asList(fakeFieldName, priorityField);
+        Form form = processForm(fields);
+        checkSingleProperty(form, priorityField, "2");
+        checkPackageActionGroups(form.getFormData());
+    }
+
+    public void testGenerateDefaultForm() throws Exception
+    {
+        Form form = processForm();
+        List<String> fieldDefs = form.getFieldDefinitionNames();
+        assertTrue(fieldDefs.contains(ASSIGNEE_NAME.toPrefixString(namespaceService)));
+        assertTrue(fieldDefs.contains(ACTORS_NAME.toPrefixString(namespaceService)));
+        assertTrue(fieldDefs.contains(DESC_NAME.toPrefixString(namespaceService)));
+        assertTrue(fieldDefs.contains(PRIORITY_NAME.toPrefixString(namespaceService)));
+        assertTrue(fieldDefs.contains(PackageItemsFieldProcessor.KEY));
+
+        Serializable fieldData = (Serializable) Collections.emptyList();
+        FormData formData = form.getFormData();
+        assertEquals(fieldData, formData.getFieldData("assoc_bpm_assignee").getValue());
+        checkPackageActionGroups(formData);
+        assertEquals("2", formData.getFieldData("prop_bpm_priority").getValue());
+    }
+
+    public void testGeneratePackageItems() throws Exception
+    {
+        // Check empty package
+        String fieldName = PackageItemsFieldProcessor.KEY;
+        Form form = processForm(fieldName);
+        Serializable packageItems = (Serializable) Collections.emptyList();
+        checkSingleAssociation(form, fieldName, packageItems);
+    }
+
+    public void testPersistPropertyChanged() throws Exception
+    {
+        String fieldName = DESC_NAME.toPrefixString(namespaceService);
+        String dataKey = makeDataKeyName(fieldName);
+        String value = "New Description";
+
+        processPersist(dataKey, value);
+
+        // Check adds description property and Package.
+        assertEquals(2, actualProperties.size());
+        assertEquals(value, actualProperties.get(DESC_NAME));
+        assertEquals(PCKG_NODE, actualProperties.get(WorkflowModel.ASSOC_PACKAGE));
+    }
+
+    public void testPersistPropertyWith_() throws Exception
+    {
+        String fieldName = PROP_WITH_.toPrefixString(namespaceService);
+        String dataKey = makeDataKeyName(fieldName);
+        String value = "New _ Value";
+
+        processPersist(dataKey, value);
+
+        assertEquals(2, actualProperties.size());
+        assertEquals(value, actualProperties.get(PROP_WITH_));
+    }
+
+    public void testPersistAssociationAdded() throws Exception
+    {
+        String fieldName = ACTORS_NAME.toPrefixString(namespaceService);
+        String dataKey = makeDataKeyName(fieldName, true);
+        String value = FAKE_NODE + ", " + FAKE_NODE2;
+        processPersist(dataKey, value);
+
+        assertEquals(2, actualProperties.size());
+        List<?> nodeRefs = (List<?>) actualProperties.get(ACTORS_NAME);
+        assertNotNull(nodeRefs);
+        assertEquals(2, nodeRefs.size());
+        assertTrue(nodeRefs.contains(FAKE_NODE));
+        assertTrue(nodeRefs.contains(FAKE_NODE2));
+    }
+
+    public void testIgnoreAssociationsRemoved() throws Exception
+    {
+        String fieldName = ASSIGNEE_NAME.toPrefixString(namespaceService);
+        String dataKey = makeDataKeyName(fieldName, false);
+        String value = FAKE_NODE.toString();
+        processPersist(dataKey, value);
+
+        assertEquals(1, actualProperties.size());
+        Serializable nodeRefs = actualProperties.get(ASSIGNEE_NAME);
+        assertNull(nodeRefs);
+    }
+
+    public void testPersistAssociationAddedWith_() throws Exception
+    {
+        String fieldName = ASSOC_WITH_.toPrefixString(namespaceService);
+        String dataKey = makeDataKeyName(fieldName, true);
+        String value = FAKE_NODE+ ", " + FAKE_NODE2;
+        processPersist(dataKey, value);
+
+        assertEquals(2, actualProperties.size());
+        List<?> nodeRefs = (List<?>) actualProperties.get(ASSOC_WITH_);
+        assertNotNull(nodeRefs);
+        assertEquals(2, nodeRefs.size());
+        assertTrue(nodeRefs.contains(FAKE_NODE));
+        assertTrue(nodeRefs.contains(FAKE_NODE2));
+    }
+    
+
+    public void testPersistPackageItemsAdded() throws Exception
+    {
+        mockPackageItems(FAKE_NODE3);
+        String dataKey = makeDataKeyName(PackageItemsFieldProcessor.KEY, true); 
+        String value = FAKE_NODE + ", " + FAKE_NODE2;
+        processPersist(dataKey, value);
+        checkAddPackageItem(FAKE_NODE, true);
+        checkAddPackageItem(FAKE_NODE2, true);
+        checkAddPackageItem(FAKE_NODE3, false);
+    }
+    
+    public void testPersistPackageItemsRemovedIgnored() throws Exception
+    {
+        mockPackageItems(FAKE_NODE, FAKE_NODE2);
+        String dataKey = makeDataKeyName(PackageItemsFieldProcessor.KEY, false); 
+        String value = FAKE_NODE + ", " + FAKE_NODE2+ "," + FAKE_NODE3;
+        processPersist(dataKey, value);
+        
+        // Check nodes 1 and 2 removed correctly.
+        checkRemovedPackageItem(FAKE_NODE, false);
+        checkRemovedPackageItem(FAKE_NODE2, false);
+        checkRemovedPackageItem(FAKE_NODE3, false);
+    }
+
+    private void mockPackageItems(NodeRef... children)
+    {
+        ArrayList<ChildAssociationRef> results = new ArrayList<ChildAssociationRef>(children.length);
+        for (NodeRef nodeRef : children)
+        {
+            ChildAssociationRef child = new ChildAssociationRef(WorkflowModel.ASSOC_PACKAGE_CONTAINS, PCKG_NODE, null, nodeRef);
+            results.add(child);
+        }
+        when(nodeService.getChildAssocs(eq(PCKG_NODE), (QNamePattern)any(), (QNamePattern)any()))
+        .thenReturn(results);
+        
+    }
+
+    private void checkRemovedPackageItem(NodeRef child, boolean wasCalled)
+    {
+        int times = wasCalled ? 1 : 0;
+        verify(nodeService, times(times))
+            .removeChild(PCKG_NODE, child);
+    }
+
+    private void checkAddPackageItem(NodeRef child, boolean wasCalled)
+    {
+        int times = wasCalled ? 1 : 0;
+        verify(nodeService, times(times))
+            .addChild(eq(PCKG_NODE),
+                        eq(child),
+                        eq(WorkflowModel.ASSOC_PACKAGE_CONTAINS),
+                        (QName)any());
+    }
+
+    private void processPersist(String dataKey, String value)
+    {
+        FormData data = new FormData();
+        data.addFieldData(dataKey, value);
+        WorkflowInstance persistedItem = (WorkflowInstance) processor.persist(item, data);
+        assertEquals(newInstance, persistedItem);
+    }
+
+    private Form processForm(String... fields)
+    {
+        return processForm(Arrays.asList(fields));
+    }
+    
+    private Form processForm(List<String> fields)
+    {
+        return processor.generate(item, fields, null, null);
+    }
+
+    private void checkPackageActionGroups(FormData formData)
+    {
+        FieldData pckgActionData = formData.getFieldData("prop_bpm_packageActionGroup");
+        assertNotNull(pckgActionData);
+        assertEquals("add_package_item_actions", pckgActionData.getValue());
+        FieldData pckgItemActionData = formData.getFieldData("prop_bpm_packageItemActionGroup");
+        assertNotNull(pckgItemActionData);
+        assertEquals("start_package_item_actions", pckgItemActionData.getValue());
+    }
+
+    private void checkSingleProperty(Form form, String fieldName, Serializable fieldData)
+    {
+        String expDataKey = makeDataKeyName(fieldName);
+        checkSingleField(form, fieldName, fieldData, expDataKey);
+
+    }
+
+    private void checkSingleAssociation(Form form, String fieldName, Serializable fieldData)
+    {
+        String expDataKey = makeAssociationDataKey(fieldName);
+        checkSingleField(form, fieldName, fieldData, expDataKey);
+    }
+
+    private void checkSingleField(Form form, String fieldName, Serializable fieldData, String expDataKey)
+    {
+        List<FieldDefinition> fieldDefs = form.getFieldDefinitions();
+        assertEquals(1, fieldDefs.size());
+        FieldDefinition fieldDef = fieldDefs.get(0);
+        assertEquals(fieldName, fieldDef.getName());
+        String dataKey = fieldDef.getDataKeyName();
+        assertEquals(expDataKey, dataKey);
+        FieldData data = form.getFormData().getFieldData(dataKey);
+        assertEquals(fieldData, data.getValue());
+    }
+
+
+    private String makeDataKeyName(String fieldName)
+    {
+        return PROP_DATA_PREFIX + fieldName.replace(":", "_");
+    }
+
+    private String makeDataKeyName(String fieldName, boolean added)
+    {
+        String assocDataKey = makeAssociationDataKey(fieldName);
+        String suffix = added ? ASSOC_DATA_ADDED_SUFFIX : ASSOC_DATA_REMOVED_SUFFIX;
+        return assocDataKey + suffix;
+    }
+    
+    private String makeAssociationDataKey(String fieldName)
+    {
+        return ASSOC_DATA_PREFIX + fieldName.replace(":", "_");
+    }
+
+    /*
+     * @see junit.framework.TestCase#setUp()
+     */
+    @Override
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        definition = makeWorkflowDefinition();
+        workflowService = makeWorkflowService();
+        nodeService = makeNodeService();
+        DictionaryService dictionaryService = makeDictionaryService();
+        namespaceService = makeNamespaceService();
+        MockFieldProcessorRegistry fieldProcessorRegistry = new MockFieldProcessorRegistry(namespaceService,
+                    dictionaryService);
+        DefaultFieldProcessor defaultProcessor = makeDefaultFieldProcessor(dictionaryService);
+        processor = makeTaskFormProcessor(dictionaryService, fieldProcessorRegistry, defaultProcessor);
+    }
+
+    private WorkflowFormProcessor makeTaskFormProcessor(DictionaryService dictionaryService,
+                MockFieldProcessorRegistry fieldProcessorRegistry, DefaultFieldProcessor defaultProcessor)
+    {
+        WorkflowFormProcessor processor1 = new WorkflowFormProcessor();
+        processor1.setWorkflowService(workflowService);
+        processor1.setNodeService(nodeService);
+        processor1.setNamespaceService(namespaceService);
+        processor1.setDictionaryService(dictionaryService);
+        processor1.setFieldProcessorRegistry(fieldProcessorRegistry);
+        return processor1;
+    }
+
+    private DefaultFieldProcessor makeDefaultFieldProcessor(DictionaryService dictionaryService) throws Exception
+    {
+        DefaultFieldProcessor defaultProcessor = new DefaultFieldProcessor();
+        defaultProcessor.setDictionaryService(dictionaryService);
+        defaultProcessor.setNamespaceService(namespaceService);
+        defaultProcessor.afterPropertiesSet();
+        return defaultProcessor;
+    }
+
+    private WorkflowDefinition makeWorkflowDefinition()
+    {
+        String id = "foo$workflowDefId";
+        String name = WF_DEF_NAME;
+        String version = "1.0";
+        String title = "Foo Bar Title";
+        String description = "Foo Bar Description";
+        WorkflowTaskDefinition startTaskDefinition = makeTaskDefinition();
+        return new WorkflowDefinition(id, name, version, title, description, startTaskDefinition);
+    }
+
+    private WorkflowTaskDefinition makeTaskDefinition()
+    {
+        WorkflowTaskDefinition taskDef = new WorkflowTaskDefinition();
+        taskDef.id = "foo$startTaskDefId";
+        taskDef.metadata = makeTypeDef();
+        taskDef.node = new WorkflowNode();
+        return taskDef;
+    }
+
+    private TypeDefinition makeTypeDef()
+    {
+        TypeDefinition typeDef = mock(TypeDefinition.class);
+        QName name = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, TASK_DEF_NAME);
+        when(typeDef.getName()).thenReturn(name);
+
+        // Set up task property definitions
+        Map<QName, PropertyDefinition> propertyDefs = makeTaskPropertyDefs();
+        when(typeDef.getProperties()).thenReturn(propertyDefs);
+
+        // Set up task association definitions.
+        Map<QName, AssociationDefinition> associationDefs = makeTaskAssociationDefs();
+        when(typeDef.getAssociations()).thenReturn(associationDefs);
+        return typeDef;
+    }
+
+    private Map<QName, PropertyDefinition> makeTaskPropertyDefs()
+    {
+        Map<QName, PropertyDefinition> properties = new HashMap<QName, PropertyDefinition>();
+        QName intType = DataTypeDefinition.INT;
+        MockClassAttributeDefinition priorityDef = MockClassAttributeDefinition.mockPropertyDefinition(PRIORITY_NAME, intType, "2");
+        properties.put(PRIORITY_NAME, priorityDef);
+        
+        QName textType = DataTypeDefinition.TEXT;
+
+        // Add a Description property
+        PropertyDefinition descValue = MockClassAttributeDefinition.mockPropertyDefinition(DESC_NAME, textType);
+        properties.put(DESC_NAME, descValue);
+
+        // Add a Status property
+        PropertyDefinition titleValue = MockClassAttributeDefinition.mockPropertyDefinition(STATUS_NAME, textType);
+        properties.put(STATUS_NAME, titleValue);
+
+        // Add a Status property
+        PropertyDefinition with_ = MockClassAttributeDefinition.mockPropertyDefinition(PROP_WITH_, textType);
+        properties.put(PROP_WITH_, with_);
+
+        // Add a Package Action property
+        QName pckgActionGroup = WorkflowModel.PROP_PACKAGE_ACTION_GROUP;
+        PropertyDefinition pckgAction = MockClassAttributeDefinition.mockPropertyDefinition(pckgActionGroup, textType,
+                    "add_package_item_actions");
+        properties.put(pckgActionGroup, pckgAction);
+
+        // Add a Package Action property
+        QName pckgItemActionGroup = WorkflowModel.PROP_PACKAGE_ITEM_ACTION_GROUP;
+        PropertyDefinition pckgItemAction = MockClassAttributeDefinition.mockPropertyDefinition(pckgItemActionGroup,
+                    textType, "start_package_item_actions");
+        properties.put(pckgItemActionGroup, pckgItemAction);
+
+        return properties;
+    }
+
+    private Map<QName, AssociationDefinition> makeTaskAssociationDefs()
+    {
+        Map<QName, AssociationDefinition> associations = new HashMap<QName, AssociationDefinition>();
+        QName actorName = QName.createQName(NamespaceService.BPM_MODEL_1_0_URI, "Actor");
+
+        // Add Assigneee association
+        MockClassAttributeDefinition assigneeDef = MockClassAttributeDefinition.mockAssociationDefinition(
+                    ASSIGNEE_NAME, actorName);
+        associations.put(ASSIGNEE_NAME, assigneeDef);
+
+        // Add Assigneee association
+        MockClassAttributeDefinition actorsDef = MockClassAttributeDefinition.mockAssociationDefinition(ACTORS_NAME,
+                    actorName);
+        associations.put(ACTORS_NAME, actorsDef);
+
+        // Add association with _
+        MockClassAttributeDefinition with_ = MockClassAttributeDefinition.mockAssociationDefinition(ASSOC_WITH_,
+                    actorName);
+        associations.put(ASSOC_WITH_, with_);
+
+        return associations;
+    }
+    
+    private NamespaceService makeNamespaceService()
+    {
+        NamespaceServiceMemoryImpl nsService = new NamespaceServiceMemoryImpl();
+        nsService.registerNamespace(NamespaceService.BPM_MODEL_PREFIX, NamespaceService.BPM_MODEL_1_0_URI);
+        nsService.registerNamespace(NamespaceService.WORKFLOW_MODEL_PREFIX, NamespaceService.WORKFLOW_MODEL_1_0_URI);
+        return nsService;
+    }
+
+    @SuppressWarnings("unchecked")
+    private DictionaryService makeDictionaryService()
+    {
+        DictionaryService mock = mock(DictionaryService.class);
+        TypeDefinition taskTypeDef = definition.getStartTaskDefinition().getMetadata();
+        when(mock.getAnonymousType((QName) any(), (Collection<QName>) any())).thenReturn(taskTypeDef);
+        return mock;
+    }
+
+    @SuppressWarnings("unchecked")
+    private WorkflowService makeWorkflowService()
+    {
+        WorkflowService service = mock(WorkflowService.class);
+        when(service.getDefinitionByName(WF_DEF_NAME)).thenReturn(definition);
+        
+        newInstance = new WorkflowInstance();
+        newInstance.id = "foo$instanceId";
+        WorkflowTask startTask = new WorkflowTask();
+        startTask.id = "foo$taskId";
+        final WorkflowPath path = new WorkflowPath();
+        path.id = "foo$pathId";
+        path.instance = newInstance;
+        
+        when(service.startWorkflow(eq(definition.getId()), anyMap()))
+            .thenAnswer(new Answer<WorkflowPath>()
+            {
+                public WorkflowPath answer(InvocationOnMock invocation) throws Throwable
+                {
+                    Object[] arguments = invocation.getArguments();
+                    actualProperties = (Map<QName, Serializable>) arguments[1];
+                    return path;
+                }
+            });
+        when(service.getTasksForWorkflowPath(path.getId()))
+            .thenReturn(Collections.singletonList(startTask));
+        when(service.createPackage(null)).thenReturn(PCKG_NODE);
+        return service;
+    }
+    
+    private NodeService makeNodeService()
+    {
+        NodeService service = mock(NodeService.class);
+        when(service.hasAspect(PCKG_NODE, WorkflowModel.ASPECT_WORKFLOW_PACKAGE))
+            .thenReturn(true);
+        return service;
+    }
+
 }

source/java/org/alfresco/repo/workflow/PackageManager.java

@@ -22,6 +22,7 @@ package org.alfresco.repo.workflow;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
@@ -36,6 +37,7 @@ import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
+import org.alfresco.util.GUID;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -212,6 +214,8 @@ public class PackageManager
            {
                String name = 
                    (String) nodeService.getProperty(item, ContentModel.PROP_NAME);
+               if(name == null)
+                   name = GUID.generate();
                String localName = QName.createValidLocalName(name);
                QName qName = QName.createQName(CM_URL, localName);
                nodeService.addChild(packageRef, item, PCKG_CONTAINS, qName);
@@ -248,11 +252,12 @@ public class PackageManager
 
     private void checkRemovedItems(List<NodeRef> currentitems)
     {
-        for (NodeRef removeItem : removeItems)
+        for (Iterator<NodeRef> iter = removeItems.iterator(); iter.hasNext();)
         {
+            NodeRef removeItem= iter.next();
             if(currentitems.contains(removeItem)==false)
             {
-                removeItems.remove(removeItem);
+                iter.remove();
                 if (logger.isDebugEnabled())
                     logger.debug("Ignoring item to remove, item not in package: " + removeItem);
             }
@@ -261,11 +266,12 @@ public class PackageManager
 
     private void checkAddedItems(List<NodeRef> currentitems)
     {
-        for (NodeRef addItem : addItems)
+        for (Iterator<NodeRef> iter = addItems.iterator(); iter.hasNext();)
         {
-            if (currentitems.contains(addItem))
+            NodeRef addItem= iter.next();
+            if(currentitems.contains(addItem))
             {
-                addItems.remove(addItem);
+                iter.remove();
                 if (logger.isDebugEnabled())
                     logger.debug("Ignoring item to add, item already in package: " + addItem);
             }

source/java/org/alfresco/repo/workflow/WorkflowBuilder.java

@@ -95,7 +95,7 @@ public class WorkflowBuilder
 
     private void signalStartTask(WorkflowPath path)
     {
-        List<WorkflowTask> tasks = workflowService.getTasksForWorkflowPath(path.id);
+        List<WorkflowTask> tasks = workflowService.getTasksForWorkflowPath(path.getId());
         if(tasks.size() == 1)
         {
             WorkflowTask startTask = tasks.get(0);