Merge pull request #2180 from mwoodiupui/DS-3989

[DS-3989] Always capture curation task output.

dspace-api/src/main/java/org/dspace/ctask/test/WorkflowReportTest.java

@@ -0,0 +1,39 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.ctask.test;
+
+import java.io.IOException;
+
+import org.dspace.content.DSpaceObject;
+import org.dspace.curate.AbstractCurationTask;
+import org.dspace.curate.Curator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Curation task which simply reports its invocation without changing anything.
+ * Meant for testing.
+ *
+ * @author mhwood
+ */
+public class WorkflowReportTest
+        extends AbstractCurationTask {
+    private static final Logger LOG = LoggerFactory.getLogger(WorkflowReportTest.class);
+
+    @Override
+    public int perform(DSpaceObject dso)
+            throws IOException {
+        LOG.info("Class {} as task {} received 'perform' for object {}",
+                WorkflowReportTest.class.getSimpleName(), taskId, dso);
+        curator.report(String.format(
+                "Class %s as task %s received 'perform' for object %s%n",
+                WorkflowReportTest.class.getSimpleName(), taskId, dso));
+        return Curator.CURATE_SUCCESS;
+    }
+}

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

@@ -9,6 +9,10 @@ package org.dspace.curate;
 
 import java.io.BufferedReader;
 import java.io.FileReader;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintStream;
+import java.io.Writer;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -18,6 +22,7 @@ import org.apache.commons.cli.CommandLineParser;
 import org.apache.commons.cli.HelpFormatter;
 import org.apache.commons.cli.Options;
 import org.apache.commons.cli.PosixParser;
+import org.apache.commons.io.output.NullOutputStream;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.core.Context;
 import org.dspace.core.factory.CoreServiceFactory;
@@ -57,7 +62,9 @@ public class CurationCli {
         options.addOption("e", "eperson", true,
                           "email address of curating eperson");
         options.addOption("r", "reporter", true,
-                          "reporter to manage results - use '-' to report to console. If absent, no reporting");
+                "relative or absolute path to the desired report file.  "
+                        + "Use '-' to report to console.  "
+                        + "If absent, no reporting");
         options.addOption("s", "scope", true,
                           "transaction scope to impose: use 'object', 'curation', or 'open'. If absent, 'open' " +
                               "applies");
@@ -165,9 +172,17 @@ public class CurationCli {
         }
 
         Curator curator = new Curator();
-        if (reporterName != null) {
-            curator.setReporter(reporterName);
+        OutputStream reporter;
+        if (null == reporterName) {
+            reporter = new NullOutputStream();
+        } else if ("-".equals(reporterName)) {
+            reporter = System.out;
+        } else {
+            reporter = new PrintStream(reporterName);
         }
+        Writer reportWriter = new OutputStreamWriter(reporter);
+        curator.setReporter(reportWriter);
+
         if (scope != null) {
             Curator.TxScope txScope = Curator.TxScope.valueOf(scope.toUpperCase());
             curator.setTransactionScope(txScope);

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

@@ -15,6 +15,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
@@ -69,16 +70,12 @@ public class Curator {
         INTERACTIVE, BATCH, ANY
     }
 
-    ;
-
     // transaction scopes
     public static enum TxScope {
         OBJECT, CURATION, OPEN
     }
 
-    ;
-
-    private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(Curator.class);
+    private static final Logger log = LogManager.getLogger();
 
     protected static final ThreadLocal<Context> curationCtx = new ThreadLocal<>();
 
@@ -86,7 +83,7 @@ public class Curator {
     protected Map<String, TaskRunner> trMap = new HashMap<>();
     protected List<String> perfList = new ArrayList<>();
     protected TaskQueue taskQ = null;
-    protected String reporter = null;
+    protected Appendable reporter = null;
     protected Invoked iMode = null;
     protected TaskResolver resolver = new TaskResolver();
     protected TxScope txScope = TxScope.OPEN;
@@ -193,7 +190,7 @@ public class Curator {
      *                 causes reporting to standard out.
      * @return return self (Curator instance) with reporter set
      */
-    public Curator setReporter(String reporter) {
+    public Curator setReporter(Appendable reporter) {
         this.reporter = reporter;
         return this;
     }
@@ -346,9 +343,10 @@ public class Curator {
      * @param message the message to output to the reporting stream.
      */
     public void report(String message) {
-        // Stub for now
-        if ("-".equals(reporter)) {
-            System.out.println(message);
+        try {
+            reporter.append(message);
+        } catch (IOException ex) {
+            log.error("Task reporting failure", ex);
         }
     }
 

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

@@ -0,0 +1,88 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.curate;
+
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.Writer;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.GregorianCalendar;
+
+import org.dspace.services.ConfigurationService;
+import org.dspace.utils.DSpace;
+
+/**
+ * Save a curation report to a unique file in the reports directory.
+ * Reports are named by the date and time of day, for example:
+ * "curation-20180916T113903045.report".
+ *
+ * @author mhwood
+ */
+public class FileReporter
+        implements Reporter {
+    private final Writer writer;
+
+    /**
+     * Open a writer to a file in a directory named by the configuration
+     * property {@code report.dir}, or in {@code [DSpace]/reports} if not
+     * configured.
+     *
+     * @throws IOException if there is a problem with the file path.
+     */
+    public FileReporter()
+            throws IOException {
+        // Calculate a unique(?) file name.
+        Date now = GregorianCalendar.getInstance().getTime();
+        SimpleDateFormat sdf = new SimpleDateFormat("yyyyMMdd'T'hhmmssSSS");
+        String filename = String.format("curation-%s.report", sdf.format(now));
+
+        // Build a path to the directory which is to receive the file.
+        ConfigurationService cfg = new DSpace().getConfigurationService();
+        String reportDir = cfg.getProperty("report.dir");
+        Path reportPath;
+        if (null == reportDir) {
+            reportPath = Paths.get(cfg.getProperty("dspace.dir"),
+                    "reports",
+                    filename);
+        } else {
+            reportPath = Paths.get(reportDir, filename);
+        }
+
+        // Open the file.
+        writer = new FileWriter(reportPath.toFile());
+    }
+
+    @Override
+    public Appendable append(CharSequence cs)
+            throws IOException {
+        writer.append(cs);
+        return this;
+    }
+
+    @Override
+    public Appendable append(CharSequence cs, int i, int i1)
+            throws IOException {
+        writer.append(cs, i, i1);
+        return this;
+    }
+
+    @Override
+    public Appendable append(char c) throws IOException {
+        writer.append(c);
+        return this;
+    }
+
+    @Override
+    public void close() throws Exception {
+        writer.close();
+    }
+}

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

@@ -0,0 +1,62 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.curate;
+
+import java.io.IOException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Write curation report records through the logging framework.
+ * Whole lines (strings ending in '\n') are written to the log category "curation".
+ * Any partial line is flushed when the reporter is {@code close()}d.
+ *
+ * @author mhwood
+ */
+public class LogReporter
+        implements Reporter {
+    private static final Logger LOG = LoggerFactory.getLogger("curation");
+    private final StringBuilder buffer = new StringBuilder();
+
+    @Override
+    public Appendable append(CharSequence cs)
+            throws IOException {
+        for (int pos = 0; pos < cs.length(); pos++) {
+            char c = cs.charAt(pos);
+            if (c == '\n') {
+                LOG.info(buffer.toString());
+                buffer.delete(0, buffer.length()); // Clear the buffer
+            } else {
+                buffer.append(c);
+            }
+        }
+        return this;
+    }
+
+    @Override
+    public Appendable append(CharSequence cs, int i, int i1)
+            throws IOException {
+        return append(cs.subSequence(i, i1));
+    }
+
+    @Override
+    public Appendable append(char c)
+            throws IOException {
+        return append(String.valueOf(c));
+    }
+
+    @Override
+    public void close()
+            throws Exception {
+        if (buffer.length() > 0) {
+            LOG.info(buffer.toString());
+        }
+    }
+}

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

@@ -0,0 +1,18 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.curate;
+
+/**
+ * A marker interface needed to make curation reporter classes into plugins.
+ *
+ * @author mhwood
+ */
+public interface Reporter
+        extends Appendable, AutoCloseable {
+}

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

@@ -30,6 +30,8 @@ import org.dspace.content.Item;
 import org.dspace.content.service.CollectionService;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
+import org.dspace.core.factory.CoreServiceFactory;
+import org.dspace.core.service.PluginService;
 import org.dspace.curate.service.WorkflowCuratorService;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
@@ -56,9 +58,10 @@ import org.springframework.beans.factory.annotation.Autowired;
 public class WorkflowCuratorServiceImpl implements WorkflowCuratorService {
 
     /**
-     * log4j logger
+     * Logging category
      */
-    private Logger log = org.apache.logging.log4j.LogManager.getLogger(WorkflowCuratorServiceImpl.class);
+    private static final Logger log
+        = org.apache.logging.log4j.LogManager.getLogger();
 
     protected Map<String, TaskSet> tsMap = new HashMap<String, TaskSet>();
 
@@ -118,6 +121,7 @@ public class WorkflowCuratorServiceImpl implements WorkflowCuratorService {
             Curator curator = new Curator();
             // are we going to perform, or just put on queue?
             if (step.queue != null) {
+                // The queue runner will call setReporter
                 for (Task task : step.tasks) {
                     curator.addTask(task.name);
                 }
@@ -125,7 +129,18 @@ public class WorkflowCuratorServiceImpl implements WorkflowCuratorService {
                 basicWorkflowItemService.update(c, wfi);
                 return false;
             } else {
-                return curate(curator, c, wfi);
+                PluginService plugins = CoreServiceFactory.getInstance()
+                        .getPluginService();
+                try (Reporter reporter
+                        = (Reporter) plugins
+                        .getSinglePlugin(Reporter.class);) {
+                    curator.setReporter(reporter);
+                    boolean status = curate(curator, c, wfi);
+                    reporter.close();
+                    return status;
+                } catch (Exception e) {
+                    log.error("Failed to close report", e);
+                }
             }
         }
         return true;

dspace-api/src/test/java/org/dspace/curate/CuratorTest.java

@@ -58,7 +58,7 @@ public class CuratorTest
 
         // Get and configure a Curator.
         Curator instance = new Curator();
-        instance.setReporter("-"); // Send any report to standard out. FIXME when DS-3989 is merged
+        instance.setReporter(System.out); // Send any report to standard out.
         instance.addTask(TASK_NAME);
 
         // Configure the run.

dspace-api/src/test/java/org/dspace/curate/ITCurator.java

@@ -0,0 +1,202 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+
+package org.dspace.curate;
+
+import java.io.IOException;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Pattern;
+
+import org.dspace.AbstractUnitTest;
+import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.Community;
+import org.dspace.content.DSpaceObject;
+import org.dspace.content.Site;
+import org.dspace.content.factory.ContentServiceFactory;
+import org.dspace.services.ConfigurationService;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Drive the Curator and check results.
+ *
+ * @author mhwood
+ */
+public class ITCurator
+        extends AbstractUnitTest {
+    Logger LOG = LoggerFactory.getLogger(ITCurator.class);
+
+    public ITCurator() {
+    }
+
+    @BeforeClass
+    public static void setUpClass() {
+    }
+
+    @AfterClass
+    public static void tearDownClass() {
+    }
+
+    @Before
+    public void setUp() {
+    }
+
+    @After
+    public void tearDown() {
+    }
+
+    /**
+     * The report should contain contributions from all tasks and all curated objects.
+     *
+     * @throws SQLException passed through.
+     * @throws IOException passed through.
+     * @throws AuthorizeException passed through.
+     */
+    @Test
+    public void testCurate_Reporting()
+            throws SQLException, IOException, AuthorizeException {
+        // Configure for testing.
+        ConfigurationService cfg = kernelImpl.getConfigurationService();
+        cfg.setProperty("plugin.named.org.dspace.curate.CurationTask",
+                Task1.class.getName() + " = task1");
+        cfg.addPropertyValue("plugin.named.org.dspace.curate.CurationTask",
+                Task2.class.getName() + " = task2");
+
+        // Create some structure.
+        context.turnOffAuthorisationSystem();
+        Site site = ContentServiceFactory.getInstance()
+                .getSiteService()
+                .findSite(context);
+        Community community = ContentServiceFactory.getInstance()
+                .getCommunityService()
+                .create(null, context);
+
+        // Run some tasks.
+        ListReporter reporter = new ListReporter();
+        Curator curator = new Curator();
+        curator.setReporter(reporter);
+        curator.addTask("task1");
+        curator.addTask("task2");
+        curator.curate(context, site);
+
+        // Validate the results.
+        List<String> report = reporter.getReport();
+        for (String aReport : report) {
+            LOG.info("Report:  {}", aReport);
+        }
+        Pattern pattern;
+        pattern = Pattern.compile(String.format("task1.*%s", site.getHandle()));
+        Assert.assertTrue("A report should mention 'task1' and site's handle",
+                reportMatcher(report, pattern));
+        pattern = Pattern.compile(String.format("task1.*%s", community.getHandle()));
+        Assert.assertTrue("A report should mention 'task1' and the community's handle",
+                reportMatcher(report, pattern));
+        pattern = Pattern.compile(String.format("task2.*%s", site.getHandle()));
+        Assert.assertTrue("A report should mention 'task2' and the Site's handle",
+                reportMatcher(report, pattern));
+        pattern = Pattern.compile(String.format("task2.*%s", community.getHandle()));
+        Assert.assertTrue("A report should mention 'task2' and the community's handle",
+                reportMatcher(report, pattern));
+    }
+
+    /**
+     * Match a collection of strings against a regular expression.\
+     *
+     * @param reports strings to be searched.
+     * @param pattern expression to be matched.
+     * @return true if at least one string matches the expression.
+     */
+    private boolean reportMatcher(List<String> reports, Pattern pattern) {
+        for (String aReport : reports) {
+            if (pattern.matcher(aReport).find()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Dummy curation task for testing.  Reports how it was invoked.
+     *
+     * @author mhwood
+     */
+    public static class Task1 extends AbstractCurationTask {
+        public Task1() {
+        }
+
+        @Override
+        public int perform(DSpaceObject dso)
+                throws IOException {
+            curator.report(String.format(
+                    "Task1 received 'perform' on taskId '%s' for object '%s'%n",
+                    taskId, dso.getHandle()));
+            return Curator.CURATE_SUCCESS;
+        }
+    }
+
+    /**
+     * Dummy curation task for testing.  Reports how it was invoked.
+     *
+     * @author mhwood
+     */
+    public static class Task2 extends AbstractCurationTask {
+        public Task2() {
+        }
+
+        @Override
+        public int perform(DSpaceObject dso) throws IOException {
+            curator.report(String.format(
+                    "Task2 received 'perform' on taskId '%s' for object '%s'%n",
+                    taskId, dso.getHandle()));
+            return Curator.CURATE_SUCCESS;
+        }
+    }
+
+    /**
+     * Absorb report strings into a sequential collection.
+     */
+    class ListReporter
+            implements Appendable {
+        private final List<String> report = new ArrayList<>();
+
+        /**
+         * Get the content of the report accumulator.
+         * @return accumulated reports.
+         */
+        List<String> getReport() {
+            return report;
+        }
+
+        @Override
+        public Appendable append(CharSequence cs)
+                throws IOException {
+            report.add(cs.toString());
+            return this;
+        }
+
+        @Override
+        public Appendable append(CharSequence cs, int i, int i1)
+                throws IOException {
+            throw new UnsupportedOperationException("Not supported yet.");
+        }
+
+        @Override
+        public Appendable append(char c)
+                throws IOException {
+            throw new UnsupportedOperationException("Not supported yet.");
+        }
+    }
+}

dspace-services/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java

@@ -346,6 +346,26 @@ public final class DSpaceConfigurationService implements ConfigurationService {
         }
     }
 
+    @Override
+    public synchronized boolean addPropertyValue(String name, Object value) {
+        if (name == null) {
+            throw new IllegalArgumentException("name cannot be null for setting configuration");
+        }
+        if (value == null) {
+            throw new IllegalArgumentException("configuration value may not be null");
+        }
+
+        // If the value is a type of String, trim any leading/trailing spaces before saving it.
+        if (String.class.isInstance(value)) {
+            value = ((String) value).trim();
+        }
+
+        Configuration configuration = getConfiguration();
+        boolean isNew = !configuration.containsKey(name);
+        configuration.addProperty(name, value);
+        return isNew;
+    }
+
     /* (non-Javadoc)
      * @see org.dspace.services.ConfigurationService#setProperty(java.lang.String, java.lang.Object)
      */

dspace-services/src/main/java/org/dspace/services/ConfigurationService.java

@@ -237,6 +237,16 @@ public interface ConfigurationService {
      */
     public boolean hasProperty(String name);
 
+    /**
+     * Add a value to a configuration property.
+     *
+     * @param name the property name.  May not be null.
+     * @param value the property value.  May not be null.
+     * @return true if a new property was created.
+     * @throws IllegalArgumentException if the name or value is null.
+     */
+    public boolean addPropertyValue(String name, Object value);
+
     /**
      * Set a configuration property (setting) in the system.
      * Type is not important here since conversion happens automatically

dspace/config/modules/submission-curation.cfg

@@ -2,7 +2,15 @@
 #------------SUBMISSION CURATION CONFIGURATIONS-----------------#
 #---------------------------------------------------------------#
 # This file contains configuration properties solely relating   #
-# to the scheduling of curation tasks during submission.        #
+# to the scheduling of curation tasks during submission -- that #
+# is:  when tasks are attached to a workflow.                   #
 #---------------------------------------------------------------#
 # Scan for viruses
-submission-curation.virus-scan = false
+submission-curation.virus-scan = false
+
+# Report serializer plugin, to capture submission task reports.
+# Uncomment exactly one, or configure your own.
+# FileReporter writes reports to ${report.dir}/curation-yyyyMMddThhmmssSSS.report
+plugin.single.org.dspace.curate.Reporter = org.dspace.curate.FileReporter
+# LogReporter writes report lines to the DSpace log.
+#plugin.single.org.dspace.curate.Reporter = org.dspace.curate.LogReporter