Added support for packageItems to the TaskFormProcessor.

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

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

@@ -32,7 +32,6 @@ import org.alfresco.service.cmr.workflow.WorkflowService;
 
 /**
  * @author Nick Smith
- *
  */
 public abstract class AbstractWorkflowFormProcessor<ItemType, PersistType> extends ContentModelFormProcessor<ItemType, PersistType>
 {
@@ -50,7 +49,6 @@ public abstract class AbstractWorkflowFormProcessor<ItemType, PersistType> exten
         addPropertyDataIfRequired(WorkflowModel.PROP_PACKAGE_ITEM_ACTION_GROUP, form, itemData);
     }
 
-
     /* (non-Javadoc)
      * @see org.alfresco.repo.forms.processor.FilteredFormProcessor#internalPersist(java.lang.Object, org.alfresco.repo.forms.FormData)
      */

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

@@ -0,0 +1,263 @@
+/*
+ * Copyright (C) 2005-2010 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * 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/>.
+ */
+
+package org.alfresco.repo.forms.processor.workflow;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.repo.workflow.WorkflowModel;
+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;
+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.apache.commons.collections.CollectionUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+/**
+ * @author Nick Smith
+ */
+public class PackageManager
+{
+    
+    private static final QName PCKG_CONTAINS = WorkflowModel.ASSOC_PACKAGE_CONTAINS;
+    private static final QName PCKG_ASPECT= WorkflowModel.ASPECT_WORKFLOW_PACKAGE;
+    private static final String CM_URL = NamespaceService.CONTENT_MODEL_1_0_URI;
+
+    /** Default Log */
+    private final static Log LOGGER = LogFactory.getLog(PackageManager.class);
+    
+    private final WorkflowService workflowService;
+    private final NodeService nodeService;
+    private final Log logger;
+    
+    private final Set<NodeRef> addItems = new HashSet<NodeRef>();
+    private final Set<NodeRef> removeItems = new HashSet<NodeRef>();
+
+    public PackageManager(WorkflowService workflowService,
+                NodeService nodeService,
+                Log logger)
+    {
+        this.workflowService = workflowService;
+        this.nodeService = nodeService;
+        this.logger = logger ==null ? LOGGER : logger;
+    }
+    
+    public void addItems(List<NodeRef> items)
+    {
+        addItems.addAll(items);
+    }
+
+    /**
+     * Takes a comma-separated list of {@link NodeRef} ids and adds the
+     * specified NodeRefs to the package.
+     * 
+     * @param items
+     */
+    public void addItems(String items)
+    {
+        List<NodeRef> nodes = NodeRef.getNodeRefs(items);
+        addItems(nodes);
+    }
+    
+    public void addItemsAsStrings(List<String> itemStrs)
+    {
+        for (String itemStr : itemStrs)
+        {
+            addItem(itemStr);
+        }
+    }
+
+    public void addItem(NodeRef item)
+    {
+        addItems.add(item);
+    }
+    
+    public void addItem(String itemStr)
+    {
+        addItem(new NodeRef(itemStr));
+    }
+
+    public void removeItems(List<NodeRef> items)
+    {
+        removeItems.addAll(items);
+    }
+
+    /**
+     * Takes a comma-separated list of {@link NodeRef} ids and adds the
+     * specified NodeRefs to the package.
+     * 
+     * @param items
+     */
+    public void removeItems(String items)
+    {
+        List<NodeRef> nodes = NodeRef.getNodeRefs(items);
+        removeItems(nodes);
+    }
+    
+    public void removeItemsAsStrings(List<String> itemStrs)
+    {
+        for (String itemStr : itemStrs)
+        {
+            removeItem(itemStr);
+        }
+    }
+
+    public void removeItem(NodeRef item)
+    {
+        removeItems.add(item);
+    }
+    
+    public void removeItem(String itemStr)
+    {
+        removeItem(new NodeRef(itemStr));
+    }
+
+    /**
+     * Creates a new Workflow package using the specified <code>container</code>.
+     * If the <code>container</code> is null then a new container node is created.
+     * Applies the specified updates to the package after it is created.
+     * @param container
+     * @return the package {@link NodeRef}.
+     * @throws WorkflowException if the specified container is already package.
+     */
+    public NodeRef create(NodeRef container) throws WorkflowException
+    {
+        NodeRef packageRef = workflowService.createPackage(container);
+        update(packageRef);
+        return packageRef;
+    }
+    
+    /**
+     * Applies the specified modifications to the package.
+     * @param packageRef
+     */
+    public void update(final NodeRef packageRef)
+    {
+       AuthenticationUtil.runAs(new RunAsWork<Void>()
+       {
+           public Void doWork() throws Exception
+           {
+               checkPackage(packageRef);
+               checkPackageItems(packageRef);
+               addPackageItems(packageRef);
+               removePackageItems(packageRef);
+               return null;
+           }
+
+       }, AuthenticationUtil.getSystemUserName());
+       addItems.clear();
+       removeItems.clear();
+    }
+
+
+    private void checkPackage(NodeRef packageRef)
+    {
+        boolean isPackage = nodeService.hasAspect(packageRef, PCKG_ASPECT);
+        if(isPackage == false)
+            throw new WorkflowException("The package NodeRef must implement the aspect: " + PCKG_ASPECT);
+    }
+
+    private void removePackageItems(NodeRef packageRef)
+    {
+        for (NodeRef item : removeItems)
+        {
+            nodeService.removeChild(packageRef, item);
+        }
+    }
+
+
+    private void addPackageItems(final NodeRef packageRef)
+    {
+        for (NodeRef item : addItems)
+           {
+               String name = 
+                   (String) nodeService.getProperty(item, ContentModel.PROP_NAME);
+               String localName = QName.createValidLocalName(name);
+               QName qName = QName.createQName(CM_URL, localName);
+               nodeService.addChild(packageRef, item, PCKG_CONTAINS, qName);
+           }
+    }
+
+    private List<NodeRef> getCurrentItems(NodeRef packageRef)
+    {
+        List<ChildAssociationRef> children = nodeService.getChildAssocs(
+                    packageRef, PCKG_CONTAINS, RegexQNamePattern.MATCH_ALL);
+        ArrayList<NodeRef> results = new ArrayList<NodeRef>(children.size());
+        for (ChildAssociationRef child : children)
+        {
+            results.add(child.getChildRef());
+        }
+        return results;
+    }
+    
+    @SuppressWarnings("unchecked")
+    private void checkPackageItems(NodeRef packageRef)
+    {
+        List<NodeRef> currentitems = getCurrentItems(packageRef);
+        Collection<NodeRef> intersection = CollectionUtils.intersection(addItems, removeItems);
+        addItems.removeAll(intersection);
+        removeItems.removeAll(intersection);
+        for (NodeRef node : intersection)
+        {
+            if(logger.isDebugEnabled())
+                logger.debug("Item was added and removed from package! Ignoring item: "+ node);
+        }
+        checkAddedItems(currentitems);
+        checkRemovedItems(currentitems);
+    }
+
+    private void checkRemovedItems(List<NodeRef> currentitems)
+    {
+        for (NodeRef removeItem : removeItems)
+        {
+            if(currentitems.contains(removeItem)==false)
+            {
+                removeItems.remove(removeItem);
+                if (logger.isDebugEnabled())
+                    logger.debug("Ignoring item to remove, item not in package: " + removeItem);
+            }
+        }
+    }
+
+    private void checkAddedItems(List<NodeRef> currentitems)
+    {
+        for (NodeRef addItem : addItems)
+        {
+            if (currentitems.contains(addItem))
+            {
+                addItems.remove(addItem);
+                if (logger.isDebugEnabled())
+                    logger.debug("Ignoring item to add, item already in package: " + addItem);
+            }
+        }
+    }
+
+}

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

@@ -25,6 +25,7 @@ import java.util.List;
 import org.alfresco.repo.forms.processor.node.ItemData;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.cmr.workflow.WorkflowTask;
 import org.alfresco.service.namespace.NamespaceService;
@@ -43,11 +44,12 @@ public class TaskFormPersister extends ContentModelFormPersister<WorkflowTask>
                 NamespaceService namespaceService,
                 DictionaryService dictionaryService,
                 WorkflowService workflowService,
+                NodeService nodeService,
                 Log logger)
     {
         super(itemData, namespaceService, dictionaryService, logger);
         WorkflowTask item = itemData.getItem();
-        this.updater = new TaskUpdater(item.id, workflowService);
+        this.updater = new TaskUpdater(item.id, workflowService, nodeService);
     }
     
     /* (non-Javadoc)
@@ -59,7 +61,6 @@ public class TaskFormPersister extends ContentModelFormPersister<WorkflowTask>
         updater.addAssociation(qName, values);
         return true;
     }
-    
 
     /* (non-Javadoc)
      * @see org.alfresco.repo.forms.processor.workflow.ContentModelFormPersister#removeAssociation(org.alfresco.service.namespace.QName, java.util.List)
@@ -81,6 +82,34 @@ public class TaskFormPersister extends ContentModelFormPersister<WorkflowTask>
         return true;
     }
 
+    /* (non-Javadoc)
+     * @see org.alfresco.repo.forms.processor.workflow.ContentModelFormPersister#addTransientAssociation(java.lang.String, java.util.List)
+     */
+    @Override
+    protected boolean addTransientAssociation(String fieldName, List<NodeRef> values)
+    {
+        if(PackageItemsFieldProcessor.KEY.equals(fieldName))
+        {
+            updater.addPackageItems(values);
+            return true;
+        }
+        return false;
+    }
+    
+    /* (non-Javadoc)
+     * @see org.alfresco.repo.forms.processor.workflow.ContentModelFormPersister#removeTransientAssociation(java.lang.String, java.util.List)
+     */
+    @Override
+    protected boolean removeTransientAssociation(String fieldName, List<NodeRef> values)
+    {
+        if(PackageItemsFieldProcessor.KEY.equals(fieldName))
+        {
+            updater.removePackageItems(values);
+            return true;
+        }
+        return false;
+    }
+    
     /* (non-Javadoc)
      * @see org.alfresco.repo.forms.processor.workflow.ContentModelFormPersister#persist()
      */
@@ -90,4 +119,5 @@ public class TaskFormPersister extends ContentModelFormPersister<WorkflowTask>
         return updater.update();
     }
 
+    
 }

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

@@ -159,6 +159,6 @@ public class TaskFormProcessor extends AbstractWorkflowFormProcessor<WorkflowTas
     protected ContentModelFormPersister<WorkflowTask> makeFormPersister(WorkflowTask item)
     {
         ItemData<WorkflowTask> itemData = makeItemData(item);
-        return new TaskFormPersister(itemData, namespaceService, dictionaryService, workflowService, LOGGER);
+        return new TaskFormPersister(itemData, namespaceService, dictionaryService, workflowService, nodeService, LOGGER);
     }
 }

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

@@ -28,9 +28,7 @@ package org.alfresco.repo.forms.processor.workflow;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyMap;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.Serializable;
@@ -58,8 +56,11 @@ 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.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.workflow.WorkflowException;
+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;
@@ -67,7 +68,6 @@ import org.alfresco.service.cmr.workflow.WorkflowTaskState;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.NamespaceServiceMemoryImpl;
 import org.alfresco.service.namespace.QName;
-import org.mockito.ArgumentCaptor;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -76,40 +76,45 @@ import org.mockito.stubbing.Answer;
  */
 public class TaskFormProcessorTest extends TestCase
 {
-    private static final String  TASK_DEF_NAME = "TaskDef";
+    private static final String TASK_DEF_NAME = "TaskDef";
-    private static final String  TASK_ID       = "Real Id";
+    private static final String TASK_ID = "Real Id";
-    private static final QName   DESC_NAME     = WorkflowModel.PROP_DESCRIPTION;
+    private static final QName DESC_NAME = WorkflowModel.PROP_DESCRIPTION;
-    private static final QName   STATUS_NAME   = WorkflowModel.PROP_STATUS;
+    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 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 ACTORS_NAME = WorkflowModel.ASSOC_POOLED_ACTORS;
-    private static final QName   ASSIGNEE_NAME = WorkflowModel.ASSOC_ASSIGNEE;
+    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 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_NODE = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakeNode");
-
+    private static final NodeRef PCKG_NODE = new NodeRef(NamespaceService.BPM_MODEL_1_0_URI + "/FakePackage");
-    private WorkflowService      workflowService;
-    private TaskFormProcessor    processor;
-    private WorkflowTask         task;
-    private NamespaceService     namespaceService;
-    private WorkflowTask         newTask;
 
+    private WorkflowService workflowService;
+    private NodeService nodeService;
+    private TaskFormProcessor processor;
+    private WorkflowTask task;
+    private NamespaceService namespaceService;
+    private WorkflowTask newTask;
+    private Map<QName, Serializable> actualProperties = null;
+    private Map<QName, List<NodeRef>> actualAdded= null;
+    private Map<QName, List<NodeRef>> actualRemoved= null;
+    
     public void testGetTypedItem() throws Exception
     {
-        try 
+        try
         {
             processor.getTypedItem(null);
             fail("Should have thrown an Exception here!");
         }
-        catch (IllegalArgumentException e) 
+        catch (IllegalArgumentException e)
         {
             // Do nothing!
         }
 
-        try 
+        try
         {
             processor.getTypedItem(new Item("task", "bad id"));
             fail("Should have thrown an Exception here!");
         }
-        catch (WorkflowException e) 
+        catch (WorkflowException e)
         {
             // Do nothing!
         }
@@ -187,7 +192,6 @@ public class TaskFormProcessorTest extends TestCase
         assertTrue(fieldDefs.contains(DESC_NAME.toPrefixString(namespaceService)));
         assertTrue(fieldDefs.contains(STATUS_NAME.toPrefixString(namespaceService)));
 
-        
         Serializable fieldData = (Serializable) Arrays.asList(FAKE_NODE.toString());
         FormData formData = form.getFormData();
         assertEquals(6, formData.getNumberOfFields());
@@ -204,7 +208,6 @@ public class TaskFormProcessorTest extends TestCase
 
         processPersist(dataKey, value);
 
-        Map<QName, Serializable> actualProperties = retrievePropertyies();
         assertEquals(1, actualProperties.size());
         assertEquals(value, actualProperties.get(DESC_NAME));
     }
@@ -217,19 +220,10 @@ public class TaskFormProcessorTest extends TestCase
 
         processPersist(dataKey, value);
 
-        Map<QName, Serializable> actualProperties = retrievePropertyies();
         assertEquals(1, actualProperties.size());
         assertEquals(value, actualProperties.get(PROP_WITH_));
     }
 
-    @SuppressWarnings("unchecked")
-    private Map<QName, Serializable> retrievePropertyies()
-    {
-        ArgumentCaptor<Map> mapArg = ArgumentCaptor.forClass(Map.class);
-        verify(workflowService).updateTask(eq(TASK_ID), mapArg.capture(), anyMap(), anyMap());
-        return mapArg.getValue();
-    }
-
     public void testPersistAssociationAdded() throws Exception
     {
         String fieldName = ACTORS_NAME.toPrefixString(namespaceService);
@@ -240,9 +234,8 @@ public class TaskFormProcessorTest extends TestCase
         String value = nodeRef1 + ", " + nodeRef2;
         processPersist(dataKey, value);
 
-        Map<QName, List<NodeRef>> actualAddedAssocs = retrieveAssociations(true);
+        assertEquals(1, actualAdded.size());
-        assertEquals(1, actualAddedAssocs.size());
+        List<NodeRef> nodeRefs = actualAdded.get(ACTORS_NAME);
-        List<NodeRef> nodeRefs = actualAddedAssocs.get(ACTORS_NAME);
         assertNotNull(nodeRefs);
         assertEquals(2, nodeRefs.size());
         assertTrue(nodeRefs.contains(new NodeRef(nodeRef1)));
@@ -257,14 +250,13 @@ public class TaskFormProcessorTest extends TestCase
         String value = FAKE_NODE.toString();
         processPersist(dataKey, value);
 
-        Map<QName, List<NodeRef>> actualRemovedAssocs = retrieveAssociations(false);
+        assertEquals(1, actualRemoved.size());
-        assertEquals(1, actualRemovedAssocs.size());
+        List<NodeRef> nodeRefs = actualRemoved.get(ASSIGNEE_NAME);
-        List<NodeRef> nodeRefs = actualRemovedAssocs.get(ASSIGNEE_NAME);
         assertNotNull(nodeRefs);
         assertEquals(1, nodeRefs.size());
         assertTrue(nodeRefs.contains(FAKE_NODE));
     }
-    
+
     public void testPersistAssociationAddedWith_() throws Exception
     {
         String fieldName = ASSOC_WITH_.toPrefixString(namespaceService);
@@ -275,30 +267,19 @@ public class TaskFormProcessorTest extends TestCase
         String value = nodeRef1 + ", " + nodeRef2;
         processPersist(dataKey, value);
 
-        Map<QName, List<NodeRef>> actualAddedAssocs = retrieveAssociations(true);
+        assertEquals(1, actualAdded.size());
-        assertEquals(1, actualAddedAssocs.size());
+        List<NodeRef> nodeRefs = actualAdded.get(ASSOC_WITH_);
-        List<NodeRef> nodeRefs = actualAddedAssocs.get(ASSOC_WITH_);
         assertNotNull(nodeRefs);
         assertEquals(2, nodeRefs.size());
         assertTrue(nodeRefs.contains(new NodeRef(nodeRef1)));
         assertTrue(nodeRefs.contains(new NodeRef(nodeRef2)));
     }
 
-    @SuppressWarnings("unchecked")
+    public void testPackageItems() throws Exception
-    private Map<QName, List<NodeRef>> retrieveAssociations(boolean added)
     {
-        ArgumentCaptor<Map> mapArg = ArgumentCaptor.forClass(Map.class);
+        //TODO Implement test.
-        if (added) 
-        {
-            verify(workflowService).updateTask(eq(TASK_ID), anyMap(), mapArg.capture(), anyMap());
-        }
-        else 
-        {
-            verify(workflowService).updateTask(eq(TASK_ID), anyMap(), anyMap(), mapArg.capture());
-        }
-        return mapArg.getValue();
     }
-
+    
     private void processPersist(String dataKey, String value)
     {
         FormData data = new FormData();
@@ -369,19 +350,21 @@ public class TaskFormProcessorTest extends TestCase
         super.setUp();
         task = makeTask();
         workflowService = makeWorkflowService();
+        nodeService = makeNodeService();
         DictionaryService dictionaryService = makeDictionaryService();
         namespaceService = makeNamespaceService();
         MockFieldProcessorRegistry fieldProcessorRegistry = new MockFieldProcessorRegistry(namespaceService,
-                dictionaryService);
+                    dictionaryService);
         DefaultFieldProcessor defaultProcessor = makeDefaultFieldProcessor(dictionaryService);
         processor = makeTaskFormProcessor(dictionaryService, fieldProcessorRegistry, defaultProcessor);
     }
 
     private TaskFormProcessor makeTaskFormProcessor(DictionaryService dictionaryService,
-            MockFieldProcessorRegistry fieldProcessorRegistry, DefaultFieldProcessor defaultProcessor)
+                MockFieldProcessorRegistry fieldProcessorRegistry, DefaultFieldProcessor defaultProcessor)
     {
         TaskFormProcessor processor1 = new TaskFormProcessor();
         processor1.setWorkflowService(workflowService);
+        processor1.setNodeService(nodeService);
         processor1.setNamespaceService(namespaceService);
         processor1.setDictionaryService(dictionaryService);
         processor1.setFieldProcessorRegistry(fieldProcessorRegistry);
@@ -404,6 +387,10 @@ public class TaskFormProcessorTest extends TestCase
         result.state = WorkflowTaskState.IN_PROGRESS;
         result.definition = makeTaskDefinition();
         result.properties = makeTaskProperties();
+
+        result.path = new WorkflowPath();
+        result.path.instance = new WorkflowInstance();
+        result.path.instance.workflowPackage = PCKG_NODE;
         return result;
     }
 
@@ -456,17 +443,17 @@ public class TaskFormProcessorTest extends TestCase
         // 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 = 
+        PropertyDefinition pckgAction = MockClassAttributeDefinition.mockPropertyDefinition(pckgActionGroup, textType,
-            MockClassAttributeDefinition.mockPropertyDefinition(pckgActionGroup, textType, "");
+                    "");
         properties.put(pckgActionGroup, pckgAction);
-        
+
         // Add a Package Action property
         QName pckgItemActionGroup = WorkflowModel.PROP_PACKAGE_ITEM_ACTION_GROUP;
-        PropertyDefinition pckgItemAction = 
+        PropertyDefinition pckgItemAction = MockClassAttributeDefinition.mockPropertyDefinition(pckgItemActionGroup,
-            MockClassAttributeDefinition.mockPropertyDefinition(pckgItemActionGroup, textType, "read_package_item_actions");
+                    textType, "read_package_item_actions");
         properties.put(pckgItemActionGroup, pckgItemAction);
 
         return properties;
@@ -479,17 +466,17 @@ public class TaskFormProcessorTest extends TestCase
 
         // Add Assigneee association
         MockClassAttributeDefinition assigneeDef = MockClassAttributeDefinition.mockAssociationDefinition(
-                ASSIGNEE_NAME, actorName);
+                    ASSIGNEE_NAME, actorName);
         associations.put(ASSIGNEE_NAME, assigneeDef);
 
         // Add Assigneee association
         MockClassAttributeDefinition actorsDef = MockClassAttributeDefinition.mockAssociationDefinition(ACTORS_NAME,
-                actorName);
+                    actorName);
         associations.put(ACTORS_NAME, actorsDef);
 
         // Add association with _
         MockClassAttributeDefinition with_ = MockClassAttributeDefinition.mockAssociationDefinition(ASSOC_WITH_,
-                actorName);
+                    actorName);
         associations.put(ASSOC_WITH_, with_);
 
         return associations;
@@ -516,7 +503,6 @@ public class TaskFormProcessorTest extends TestCase
         WorkflowService service = mock(WorkflowService.class);
         when(service.getTaskById(anyString())).thenAnswer(new Answer<WorkflowTask>()
         {
-
             public WorkflowTask answer(InvocationOnMock invocation) throws Throwable
             {
                 String id = (String) invocation.getArguments()[0];
@@ -526,11 +512,34 @@ public class TaskFormProcessorTest extends TestCase
                     throw new WorkflowException("Task Id not found!");
             }
         });
+        
         this.newTask = new WorkflowTask();
         newTask.id = TASK_ID;
-        when(service.updateTask(anyString(), anyMap(), anyMap(), anyMap())).thenReturn(newTask);
+
+        when(service.updateTask(anyString(), anyMap(), anyMap(), anyMap()))
+        .thenAnswer(new Answer<WorkflowTask>()
+        {
+            public WorkflowTask answer(InvocationOnMock invocation) throws Throwable
+            {
+                Object[] args = invocation.getArguments();
+                Map<QName, Serializable> props = (Map<QName, Serializable>) args[1];
+                actualProperties = new HashMap<QName, Serializable>(props);
+                Map<QName, List<NodeRef>> added = (Map<QName, List<NodeRef>>) args[2];
+                actualAdded = new HashMap<QName, List<NodeRef>>(added);
+                Map<QName, List<NodeRef>> removed = (Map<QName, List<NodeRef>>) args[3];
+                actualRemoved = new HashMap<QName, List<NodeRef>>(removed);
+                return newTask;
+            }
+        });
         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/forms/processor/workflow/TaskUpdater.java

@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Map;
 
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.workflow.WorkflowService;
 import org.alfresco.service.cmr.workflow.WorkflowTask;
 import org.alfresco.service.namespace.QName;
@@ -51,24 +52,21 @@ public class TaskUpdater
 
     private final String taskId;
     private final WorkflowService workflowService;
-    private Map<QName, Serializable> properties = new HashMap<QName, Serializable>();
+    private final PackageManager packageMgr;
-    private Map<QName, List<NodeRef>> add = new HashMap<QName, List<NodeRef>>();
-    private Map<QName, List<NodeRef>> remove = new HashMap<QName, List<NodeRef>>();
 
-    public TaskUpdater(String taskId, WorkflowService workflowService)
+    private final Map<QName, Serializable> properties = new HashMap<QName, Serializable>();
+    private final Map<QName, List<NodeRef>> add = new HashMap<QName, List<NodeRef>>();
+    private final Map<QName, List<NodeRef>> remove = new HashMap<QName, List<NodeRef>>();
+
+    public TaskUpdater(String taskId,
+                WorkflowService workflowService,
+                NodeService nodeService)
     {
         this.taskId = taskId;
         this.workflowService = workflowService;
+        this.packageMgr = new PackageManager(workflowService, nodeService, LOGGER);
     }
 
-    public WorkflowTask update()
-    {
-        WorkflowTask result = workflowService.updateTask(taskId, properties, add, remove);
-        properties = new HashMap<QName, Serializable>();
-        add = new HashMap<QName, List<NodeRef>>();
-        remove = new HashMap<QName, List<NodeRef>>();
-        return result;
-    }
 
     public void addProperty(QName name, Serializable value)
     {
@@ -118,5 +116,28 @@ public class TaskUpdater
         }
         return map;
     }
+    
+    public void addPackageItems(List<NodeRef> items)
+    {
+        packageMgr.addItems(items);
+    }
+    
+    public void removePackageItems(List<NodeRef> items)
+    {
+        packageMgr.removeItems(items);
+    }
+
+    public WorkflowTask update()
+    {
+        WorkflowTask task = workflowService.getTaskById(taskId);
+        NodeRef packageNode = task.path.instance.workflowPackage;
+        packageMgr.update(packageNode);
+        
+        WorkflowTask result = workflowService.updateTask(taskId, properties, add, remove);
+        properties.clear();
+        add.clear();
+        remove.clear();
+        return result;
+    }
 
 }

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

@@ -51,18 +51,16 @@ import org.alfresco.service.namespace.QName;
 public class WorkflowBuilder
 {
     private final WorkflowService workflowService;
+    private final PackageManager packageMgr;
     private final WorkflowDefinition definition;
-    private final NodeService nodeService;
 
     private final Map<QName, Serializable> params = new HashMap<QName, Serializable>();
-    private final Set<NodeRef> packageItems = new HashSet<NodeRef>();
     private NodeRef packageNode = null;
     
-    
     public WorkflowBuilder(WorkflowDefinition definition, WorkflowService workflowService, NodeService nodeService)
     {
         this.workflowService = workflowService;
-        this.nodeService = nodeService;
+        this.packageMgr = new PackageManager(workflowService, nodeService, null);
         this.definition = definition;
     }
     
@@ -79,41 +77,6 @@ public class WorkflowBuilder
         }
     }
     
-    public void addPackageItems(List<NodeRef> items)
-    {
-        packageItems.addAll(items);
-    }
-
-    /**
-     * Takes a comma-separated list of {@link NodeRef} ids and adds the
-     * specified NodeRefs to the package.
-     * 
-     * @param items
-     */
-    public void addPackageItems(String items)
-    {
-        List<NodeRef> nodes = NodeRef.getNodeRefs(items);
-        addPackageItems(nodes);
-    }
-    
-    public void addPackageItemsAsStrings(List<String> itemStrs)
-    {
-        for (String itemStr : itemStrs)
-        {
-            addPackageItem(itemStr);
-        }
-    }
-
-    public void addPackageItem(NodeRef item)
-    {
-        packageItems.add(item);
-    }
-    
-    public void addPackageItem(String itemStr)
-    {
-        packageItems.add(new NodeRef(itemStr));
-    }
-    
     /**
      * @param packageNode the packageNode to set
      */
@@ -122,9 +85,15 @@ public class WorkflowBuilder
         this.packageNode = packageNode;
     }
     
+    public void addPackageItems(List<NodeRef> items)
+    {
+        packageMgr.addItems(items);
+    }
+    
     public WorkflowInstance build()
     {
-        buildPackage();
+        NodeRef packageRef = packageMgr.create(packageNode);
+        params.put(WorkflowModel.ASSOC_PACKAGE, packageRef);
         WorkflowPath path = workflowService.startWorkflow(definition.id, params);
         signalStartTask(path);
         return path.instance;
@@ -142,27 +111,4 @@ public class WorkflowBuilder
             throw new WorkflowException("Start task not found! Expected 1 task but found: " + tasks.size());
     }
 
-    private void buildPackage()
-    {
-        final NodeRef packageRef = workflowService.createPackage(packageNode);
-        final String url = NamespaceService.CONTENT_MODEL_1_0_URI;
-        final QName packageContains = WorkflowModel.ASSOC_PACKAGE_CONTAINS;
-        AuthenticationUtil.runAs(new RunAsWork<Void>()
-        {
-            public Void doWork() throws Exception
-            {
-                for (NodeRef item : packageItems)
-                {
-                    String name = 
-                        (String) nodeService.getProperty(item, ContentModel.PROP_NAME);
-                    String localName = QName.createValidLocalName(name);
-                    QName qName = QName.createQName(url, localName);
-                    nodeService.addChild(packageRef, item, packageContains, qName);
-                }
-                return null;
-            }
-            
-        }, AuthenticationUtil.getSystemUserName());
-        params.put(WorkflowModel.ASSOC_PACKAGE, packageRef);
-    }
 }

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

@@ -118,8 +118,11 @@ public class WorkflowPackageImpl implements WorkflowPackageComponent
         }
 
         // attach workflow package
-        if (nodeService.hasAspect(container, WorkflowModel.ASPECT_WORKFLOW_PACKAGE)) { throw new WorkflowException(
+        if (nodeService.hasAspect(container, WorkflowModel.ASPECT_WORKFLOW_PACKAGE)) 
-                    "Container '" + container + "' is already a workflow package."); }
+        {
+            String msg = "Container '" + container + "' is already a workflow package.";
+            throw new WorkflowException(msg);
+        }
         nodeService.addAspect(container, WorkflowModel.ASPECT_WORKFLOW_PACKAGE, null);
         nodeService.setProperty(container, WorkflowModel.PROP_IS_SYSTEM_PACKAGE, isSystemPackage);