Merge pull request DSpace#9001 from mwoodiupui/workflow-curate

Make workflow curation tasks actually work.

dspace-api/src/main/java/org/dspace/curate/XmlWorkflowCuratorServiceImpl.java

@@ -13,6 +13,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
@@ -30,6 +31,7 @@
 import org.dspace.workflow.FlowStep;
 import org.dspace.workflow.Task;
 import org.dspace.workflow.TaskSet;
+import org.dspace.xmlworkflow.Role;
 import org.dspace.xmlworkflow.RoleMembers;
 import org.dspace.xmlworkflow.WorkflowConfigurationException;
 import org.dspace.xmlworkflow.factory.XmlWorkflowFactory;
@@ -47,14 +49,17 @@
  * Manage interactions between curation and workflow.  A curation task can be
  * attached to a workflow step, to be executed during the step.
  *
+ * <p>
+ * <strong>NOTE:</strong> when run in workflow, curation tasks <em>run with
+ * authorization disabled</em>.
+ *
  * @see CurationTaskConfig
  * @author mwood
  */
 @Service
 public class XmlWorkflowCuratorServiceImpl
         implements XmlWorkflowCuratorService {
-    private static final Logger LOG
-            = org.apache.logging.log4j.LogManager.getLogger();
+    private static final Logger LOG = LogManager.getLogger();
 
     @Autowired(required = true)
     protected XmlWorkflowFactory workflowFactory;
@@ -97,7 +102,18 @@ public boolean doCuration(Context c, XmlWorkflowItem wfi)
             throws AuthorizeException, IOException, SQLException {
         Curator curator = new Curator();
         curator.setReporter(reporter);
-        return curate(curator, c, wfi);
+        c.turnOffAuthorisationSystem();
+        boolean wasAnonymous = false;
+        if (null == c.getCurrentUser()) { // We need someone to email
+            wasAnonymous = true;
+            c.setCurrentUser(ePersonService.getSystemEPerson(c));
+        }
+        boolean failedP = curate(curator, c, wfi);
+        if (wasAnonymous) {
+            c.setCurrentUser(null);
+        }
+        c.restoreAuthSystemState();
+        return failedP;
     }
 
     @Override
@@ -123,7 +139,7 @@ public boolean curate(Curator curator, Context c, XmlWorkflowItem wfi)
             item.setOwningCollection(wfi.getCollection());
             for (Task task : step.tasks) {
                 curator.addTask(task.name);
-                curator.curate(item);
+                curator.curate(c, item);
                 int status = curator.getStatus(task.name);
                 String result = curator.getResult(task.name);
                 String action = "none";
@@ -223,8 +239,12 @@ protected void notifyContacts(Context c, XmlWorkflowItem wfi,
             String status, String action, String message)
             throws AuthorizeException, IOException, SQLException {
         List<EPerson> epa = resolveContacts(c, task.getContacts(status), wfi);
-        if (epa.size() > 0) {
+        if (!epa.isEmpty()) {
             workflowService.notifyOfCuration(c, wfi, epa, task.name, action, message);
+        } else {
+            LOG.warn("No contacts were found for workflow item {}:  "
+                    + "task {} returned action {} with message {}",
+                    wfi.getID(), task.name, action, message);
         }
     }
 
@@ -247,8 +267,7 @@ protected List<EPerson> resolveContacts(Context c, List<String> contacts,
             // decode contacts
             if ("$flowgroup".equals(contact)) {
                 // special literal for current flowgoup
-                ClaimedTask claimedTask = claimedTaskService.findByWorkflowIdAndEPerson(c, wfi, c.getCurrentUser());
-                String stepID = claimedTask.getStepID();
+                String stepID = getFlowStep(c, wfi).step;
                 Step step;
                 try {
                     Workflow workflow = workflowFactory.getWorkflow(wfi.getCollection());
@@ -258,19 +277,26 @@ protected List<EPerson> resolveContacts(Context c, List<String> contacts,
                             String.valueOf(wfi.getID()), e);
                     return epList;
                 }
-                RoleMembers roleMembers = step.getRole().getMembers(c, wfi);
-                for (EPerson ep : roleMembers.getEPersons()) {
-                    epList.add(ep);
-                }
-                for (Group group : roleMembers.getGroups()) {
-                    epList.addAll(group.getMembers());
+                Role role = step.getRole();
+                if (null != role) {
+                    RoleMembers roleMembers = role.getMembers(c, wfi);
+                    for (EPerson ep : roleMembers.getEPersons()) {
+                        epList.add(ep);
+                    }
+                    for (Group group : roleMembers.getGroups()) {
+                        epList.addAll(group.getMembers());
+                    }
+                } else {
+                    epList.add(ePersonService.getSystemEPerson(c));
                 }
             } else if ("$colladmin".equals(contact)) {
+                // special literal for collection administrators
                 Group adGroup = wfi.getCollection().getAdministrators();
                 if (adGroup != null) {
                     epList.addAll(groupService.allMembers(c, adGroup));
                 }
             } else if ("$siteadmin".equals(contact)) {
+                // special literal for site administrator
                 EPerson siteEp = ePersonService.findByEmail(c,
                         configurationService.getProperty("mail.admin"));
                 if (siteEp != null) {

dspace-api/src/main/java/org/dspace/curate/service/XmlWorkflowCuratorService.java

@@ -42,9 +42,9 @@ public boolean needsCuration(Context c, XmlWorkflowItem wfi)
      *
      * @param c the context
      * @param wfi the workflow item
-     * @return true if curation was completed or not required,
+     * @return true if curation was completed or not required;
      *         false if tasks were queued for later completion,
-     *         or item was rejected
+     *         or item was rejected.
      * @throws AuthorizeException if authorization error
      * @throws IOException if IO error
      * @throws SQLException if database error
@@ -58,7 +58,9 @@ public boolean doCuration(Context c, XmlWorkflowItem wfi)
      * @param curator the curation context
      * @param c the user context
      * @param wfId the workflow item's ID
-     * @return true if curation failed.
+     * @return true if curation curation was completed or not required;
+     *         false if tasks were queued for later completion,
+     *         or item was rejected.
      * @throws AuthorizeException if authorization error
      * @throws IOException if IO error
      * @throws SQLException if database error
@@ -72,7 +74,9 @@ public boolean curate(Curator curator, Context c, String wfId)
      * @param curator the curation context
      * @param c the user context
      * @param wfi the workflow item
-     * @return true if curation failed.
+     * @return true if workflow curation was completed or not required;
+     *         false if tasks were queued for later completion,
+     *         or item was rejected.
      * @throws AuthorizeException if authorization error
      * @throws IOException if IO error
      * @throws SQLException if database error

dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java

@@ -47,6 +47,7 @@
 import org.dspace.eperson.service.SubscribeService;
 import org.dspace.event.Event;
 import org.dspace.orcid.service.OrcidTokenService;
+import org.dspace.services.ConfigurationService;
 import org.dspace.util.UUIDUtils;
 import org.dspace.versioning.Version;
 import org.dspace.versioning.VersionHistory;
@@ -101,6 +102,8 @@ public class EPersonServiceImpl extends DSpaceObjectServiceImpl<EPerson> impleme
     protected VersionDAO versionDAO;
     @Autowired(required = true)
     protected ClaimedTaskService claimedTaskService;
+    @Autowired(required = true)
+    protected ConfigurationService configurationService;
     @Autowired
     protected OrcidTokenService orcidTokenService;
 
@@ -113,6 +116,30 @@ public EPerson find(Context context, UUID id) throws SQLException {
         return ePersonDAO.findByID(context, EPerson.class, id);
     }
 
+    /**
+     * Create a fake EPerson which can receive email.  Its address will be the
+     * value of "mail.admin", or "postmaster" if all else fails.
+     * @param c
+     * @return
+     * @throws SQLException
+     */
+    @Override
+    public EPerson getSystemEPerson(Context c)
+            throws SQLException {
+        String adminEmail = configurationService.getProperty("mail.admin");
+        if (null == adminEmail) {
+            adminEmail = "postmaster"; // Last-ditch attempt to send *somewhere*
+        }
+        EPerson systemEPerson = findByEmail(c, adminEmail);
+
+        if (null == systemEPerson) {
+            systemEPerson = new EPerson();
+            systemEPerson.setEmail(adminEmail);
+        }
+
+        return systemEPerson;
+    }
+
     @Override
     public EPerson findByIdOrLegacyId(Context context, String id) throws SQLException {
         if (StringUtils.isNumeric(id)) {

dspace-api/src/main/java/org/dspace/eperson/service/EPersonService.java

@@ -13,6 +13,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Set;
+import javax.validation.constraints.NotNull;
 
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Item;
@@ -157,6 +158,19 @@ public List<EPerson> findAll(Context context, int sortField)
     public List<EPerson> findAll(Context context, int sortField, int pageSize, int offset)
         throws SQLException;
 
+    /**
+     * The "System EPerson" is a fake account that exists only to receive email.
+     * It has an email address that should be presumed usable.  It does not
+     * exist in the database and is not complete.
+     *
+     * @param context current DSpace session.
+     * @return an EPerson that can presumably receive email.
+     * @throws SQLException
+     */
+    @NotNull
+    public EPerson getSystemEPerson(Context context)
+            throws SQLException;
+
     /**
      * Create a new eperson
      *