71513: -i valid dso or all check + test &

fix so that -t plugin-key instead of entire config &
Added test voor noop curate output

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

@@ -17,13 +17,13 @@ import java.io.OutputStreamWriter;
 import java.io.PrintStream;
 import java.io.Writer;
 import java.sql.SQLException;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
 import org.apache.commons.io.output.NullOutputStream;
 import org.dspace.authorize.AuthorizeException;
+import org.dspace.content.DSpaceObject;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.core.Context;
 import org.dspace.core.factory.CoreServiceFactory;
@@ -31,6 +31,8 @@ import org.dspace.curate.factory.CurateServiceFactory;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
+import org.dspace.handle.factory.HandleServiceFactory;
+import org.dspace.handle.service.HandleService;
 import org.dspace.scripts.DSpaceRunnable;
 import org.dspace.utils.DSpace;
 
@@ -74,13 +76,13 @@ public class CurationCli extends DSpaceRunnable<CurationScriptConfiguration> {
         // process task queue
         if (curationClientOptions == CurationClientOptions.QUEUE) {
             // process the task queue
-            TaskQueue queue = (TaskQueue) CoreServiceFactory.getInstance().getPluginService()
+            TaskQueue taskQueue = (TaskQueue) CoreServiceFactory.getInstance().getPluginService()
                                                             .getSinglePlugin(TaskQueue.class);
-            if (queue == null) {
+            if (taskQueue == null) {
                 super.handler.logError("No implementation configured for queue");
                 throw new UnsupportedOperationException("No queue service available");
             }
-            long timeRun = this.runQueue(queue, curator);
+            long timeRun = this.runQueue(taskQueue, curator);
             this.endScript(timeRun);
         }
     }
@@ -151,8 +153,8 @@ public class CurationCli extends DSpaceRunnable<CurationScriptConfiguration> {
             curator.clear();
             // does entry relate to a DSO or workflow object?
             if (entry.getObjectId().indexOf('/') > 0) {
-                for (String task : entry.getTaskNames()) {
+                for (String taskName : entry.getTaskNames()) {
-                    curator.addTask(task);
+                    curator.addTask(taskName);
                 }
                 curator.curate(context, entry.getObjectId());
             } else {
@@ -306,7 +308,7 @@ public class CurationCli extends DSpaceRunnable<CurationScriptConfiguration> {
 
     /**
      * Fills in required command line options for the task or taskFile option.
-     * Checks if there are is a missing required -i option.
+     * Checks if there are is a missing required -i option and if -i is either 'all' or a valid dso handle.
      * Checks if -t task has a valid task option.
      * Checks if -T taskfile is a valid file.
      */
@@ -314,11 +316,11 @@ public class CurationCli extends DSpaceRunnable<CurationScriptConfiguration> {
         // task or taskFile
         if (this.commandLine.hasOption('t')) {
             this.task = this.commandLine.getOptionValue('t');
-            if (!Arrays.asList(CurationClientOptions.getTaskOptions()).contains(this.task)) {
+            if (!CurationClientOptions.getTaskOptions().contains(this.task)) {
                 super.handler
-                    .logError("-t task must be one of: " + Arrays.toString(CurationClientOptions.getTaskOptions()));
+                    .logError("-t task must be one of: " + CurationClientOptions.getTaskOptions());
                 throw new IllegalArgumentException(
-                    "-t task must be one of: " + Arrays.toString(CurationClientOptions.getTaskOptions()));
+                    "-t task must be one of: " + CurationClientOptions.getTaskOptions());
             }
         } else if (this.commandLine.hasOption('T')) {
             this.taskFile = this.commandLine.getOptionValue('T');
@@ -330,7 +332,25 @@ public class CurationCli extends DSpaceRunnable<CurationScriptConfiguration> {
         }
 
         if (this.commandLine.hasOption('i')) {
-            this.id = this.commandLine.getOptionValue('i');
+            this.id = this.commandLine.getOptionValue('i').toLowerCase();
+            if (!this.id.equalsIgnoreCase("all")) {
+                HandleService handleService = HandleServiceFactory.getInstance().getHandleService();
+                DSpaceObject dso;
+                try {
+                    dso = handleService.resolveToObject(this.context, id);
+                } catch (SQLException e) {
+                    super.handler.logError("SQLException trying to resolve handle " + id + " to a valid dso");
+                    throw new IllegalArgumentException(
+                        "SQLException trying to resolve handle " + id + " to a valid dso");
+                }
+                if (dso == null) {
+                    super.handler.logError("Id must be specified: a valid dso handle or 'all'; " + this.id + " could " +
+                                           "not be resolved to valid dso handle");
+                    throw new IllegalArgumentException(
+                        "Id must be specified: a valid dso handle or 'all'; " + this.id + " could " +
+                        "not be resolved to valid dso handle");
+                }
+            }
         } else {
             super.handler.logError("Id must be specified: a handle, 'all', or no -i and a -q task queue (-h for " +
                                    "help)");

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

@@ -7,10 +7,12 @@
  */
 package org.dspace.curate;
 
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Options;
+import org.apache.commons.lang3.StringUtils;
 import org.dspace.services.ConfigurationService;
 import org.dspace.services.factory.DSpaceServicesFactory;
 
@@ -24,6 +26,8 @@ public enum CurationClientOptions {
     QUEUE,
     HELP;
 
+    private static List<String> taskOptions;
+
     /**
      * This method resolves the CommandLine parameters to figure out which action the curation script should perform
      *
@@ -44,7 +48,7 @@ public enum CurationClientOptions {
     protected static Options constructOptions() {
         Options options = new Options();
 
-        options.addOption("t", "task", true, "curation task name; options: " + Arrays.toString(getTaskOptions()));
+        options.addOption("t", "task", true, "curation task name; options: " + getTaskOptions());
         options.addOption("T", "taskfile", true, "file containing curation task names");
         options.addOption("i", "id", true,
             "Id (handle) of object to perform task on, or 'all' to perform on whole repository");
@@ -62,8 +66,20 @@ public enum CurationClientOptions {
         return options;
     }
 
-    public static String[] getTaskOptions() {
+    /**
-        ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
+     * Creates list of the taskOptions' keys from the configs of plugin.named.org.dspace.curate.CurationTask
-        return configurationService.getArrayProperty("plugin.named.org.dspace.curate.CurationTask");
+     *
+     * @return List of the taskOptions' keys from the configs of plugin.named.org.dspace.curate.CurationTask
+     */
+    public static List<String> getTaskOptions() {
+        if (taskOptions == null) {
+            ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
+            String[] taskConfigs = configurationService.getArrayProperty("plugin.named.org.dspace.curate.CurationTask");
+            taskOptions = new ArrayList<>();
+            for (String taskConfig : taskConfigs) {
+                taskOptions.add(StringUtils.substringAfterLast(taskConfig, "=").trim());
+            }
+        }
+        return taskOptions;
     }
 }

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

@@ -8,23 +8,42 @@
 package org.dspace.curate;
 
 import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import org.dspace.AbstractUnitTest;
 import org.dspace.content.DSpaceObject;
+import org.dspace.content.Item;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.SiteService;
+import org.dspace.ctask.general.NoOpCurationTask;
 import org.dspace.services.ConfigurationService;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
 
 /**
  *
  * @author mhwood
  */
-public class CuratorTest
+@RunWith(MockitoJUnitRunner.class)
-        extends AbstractUnitTest {
+public class CuratorTest extends AbstractUnitTest {
+
+    @InjectMocks
+    private Curator curator;
+
+    @Mock
+    NoOpCurationTask noOpCurationTask;
+
+    @Spy
+    TaskResolver taskResolver;
+
     private static final SiteService SITE_SERVICE = ContentServiceFactory.getInstance().getSiteService();
 
     static final String RUN_PARAMETER_NAME = "runParameter";
@@ -44,8 +63,7 @@ public class CuratorTest
      * @throws java.lang.Exception passed through.
      */
     @Test
-    public void testCurate_DSpaceObject()
+    public void testCurate_DSpaceObject() throws Exception {
-            throws Exception {
         System.out.println("curate");
 
         final String TASK_NAME = "dummyTask";
@@ -80,4 +98,19 @@ public class CuratorTest
         assertEquals("Wrong run parameter", RUN_PARAMETER_VALUE, runParameter);
         assertEquals("Wrong task property", TASK_PROPERTY_VALUE, taskProperty);
     }
+
+    @Test
+    public void testCurate_NoOpTask() throws Exception {
+        StringBuilder reporterOutput = new StringBuilder();
+        curator.setReporter(reporterOutput); // Send any report to our StringBuilder.
+
+        curator.addTask("noop");
+        Item item = mock(Item.class);
+        when(item.getType()).thenReturn(2);
+        when(item.getHandle()).thenReturn("testHandle");
+        curator.curate(context, item);
+
+        assertEquals(Curator.CURATE_SUCCESS, curator.getStatus("noop"));
+        assertEquals(reporterOutput.toString(), "No operation performed on testHandle");
+    }
 }

dspace-server-webapp/src/test/java/org/dspace/curate/CurationScriptIT.java

@@ -1,5 +1,6 @@
 package org.dspace.curate;
 
+import static org.hamcrest.Matchers.containsStringIgnoringCase;
 import static org.hamcrest.Matchers.is;
 import static com.jayway.jsonpath.JsonPath.read;
 import static org.hamcrest.Matchers.containsString;
@@ -8,7 +9,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 import java.io.File;
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
@@ -87,7 +87,7 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
             // Illegal Argument Exception
             .andExpect(status().isBadRequest())
             // Contains the valid options
-            .andExpect(status().reason(containsString(Arrays.toString(CurationClientOptions.getTaskOptions()))));
+            .andExpect(status().reason(containsString(CurationClientOptions.getTaskOptions().toString())));
     }
 
     @Test
@@ -114,7 +114,7 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
         LinkedList<DSpaceCommandLineParameter> parameters = new LinkedList<>();
 
         parameters.add(new DSpaceCommandLineParameter("-i", publicItem1.getHandle()));
-        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions()[0]));
+        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions().get(0)));
 
         List<ParameterValueRest> list = parameters.stream()
                                                   .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter
@@ -159,7 +159,7 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
 
         parameters.add(new DSpaceCommandLineParameter("-e", "nonExistentEmail@test.com"));
         parameters.add(new DSpaceCommandLineParameter("-i", publicItem1.getHandle()));
-        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions()[0]));
+        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions().get(0)));
 
         List<ParameterValueRest> list = parameters.stream()
                                                   .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter
@@ -186,7 +186,7 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
         LinkedList<DSpaceCommandLineParameter> parameters = new LinkedList<>();
 
         parameters.add(new DSpaceCommandLineParameter("-e", admin.getEmail()));
-        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions()[0]));
+        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions().get(0)));
 
         List<ParameterValueRest> list = parameters.stream()
                                                   .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter
@@ -204,6 +204,32 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
             .andExpect(status().reason(containsString("handle")));
     }
 
+    @Test
+    public void curateScript_invalidHandle() throws Exception {
+        String token = getAuthToken(admin.getEmail(), password);
+
+        LinkedList<DSpaceCommandLineParameter> parameters = new LinkedList<>();
+
+        parameters.add(new DSpaceCommandLineParameter("-i", "invalidhandle"));
+        parameters.add(new DSpaceCommandLineParameter("-e", admin.getEmail()));
+        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions().get(0)));
+
+        List<ParameterValueRest> list = parameters.stream()
+                                                  .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter
+                                                      .convert(dSpaceCommandLineParameter, Projection.DEFAULT))
+                                                  .collect(Collectors.toList());
+
+        // Request with missing required -i <handle>
+        getClient(token)
+            .perform(post(CURATE_SCRIPT_ENDPOINT).contentType("multipart/form-data")
+                                                 .param("properties",
+                                                     new Gson().toJson(list)))
+            // Illegal Argument Exception
+            .andExpect(status().isBadRequest())
+            // Contains invalidHandle
+            .andExpect(status().reason(containsStringIgnoringCase("invalidhandle")));
+    }
+
     @Test
     public void curateScript_MissingTaskOrTaskFile() throws Exception {
         context.turnOffAuthorisationSystem();
@@ -324,7 +350,7 @@ public class CurationScriptIT extends AbstractControllerIntegrationTest {
 
         parameters.add(new DSpaceCommandLineParameter("-e", admin.getEmail()));
         parameters.add(new DSpaceCommandLineParameter("-i", publicItem1.getHandle()));
-        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions()[0]));
+        parameters.add(new DSpaceCommandLineParameter("-t", CurationClientOptions.getTaskOptions().get(0)));
 
         List<ParameterValueRest> list = parameters.stream()
                                                   .map(dSpaceCommandLineParameter -> dSpaceRunnableParameterConverter