[DS-4390] cleanup retrieval of DSpaceRunnable by name and cleanup in ScriptLauncher.java

dspace-api/src/main/java/org/dspace/app/launcher/ScriptLauncher.java

@@ -14,15 +14,14 @@ import java.util.List;
 import java.util.TreeMap;
 
 import org.apache.commons.cli.ParseException;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.dspace.scripts.DSpaceRunnable;
+import org.dspace.scripts.factory.ScriptServiceFactory;
 import org.dspace.scripts.handler.DSpaceRunnableHandler;
 import org.dspace.scripts.handler.impl.CommandLineDSpaceRunnableHandler;
 import org.dspace.servicemanager.DSpaceKernelImpl;
 import org.dspace.servicemanager.DSpaceKernelInit;
 import org.dspace.services.RequestService;
-import org.dspace.utils.DSpace;
 import org.jdom.Document;
 import org.jdom.Element;
 import org.jdom.input.SAXBuilder;
@@ -101,36 +100,47 @@ public class ScriptLauncher {
 
     }
 
-    public static Integer handleScript(String[] args, Document commandConfigs,
+    /**
+     * This method will take the arguments from a commandline input and it'll find the script that the first argument
+     * refers to and it'll execute this script.
+     * It can return a 1 or a 0 depending on whether the script failed or passed respectively
+     * @param args                  The arguments for the script and the script as first one in the array
+     * @param commandConfigs        The Document
+     * @param dSpaceRunnableHandler The DSpaceRunnableHandler for this execution
+     * @param kernelImpl            The relevant DSpaceKernelImpl
+     * @return                      A 1 or 0 depending on whether the script failed or passed respectively
+     */
+    public static int handleScript(String[] args, Document commandConfigs,
                                        DSpaceRunnableHandler dSpaceRunnableHandler,
                                        DSpaceKernelImpl kernelImpl) {
-        Integer status;
+        int status;
-        status = runDSpaceScriptWithArgs(args, dSpaceRunnableHandler);
+        DSpaceRunnable script = ScriptServiceFactory.getInstance().getScriptService().getScriptForName(args[0]);
-        if (status == null) {
+        if (script != null) {
+            status = executeScript(args, dSpaceRunnableHandler, script);
+        } else {
             status = runOneCommand(commandConfigs, args, kernelImpl);
         }
         return status;
     }
 
-    private static Integer runDSpaceScriptWithArgs(String[] args, DSpaceRunnableHandler dSpaceRunnableHandler) {
+    /**
-        List<DSpaceRunnable> scripts = new DSpace().getServiceManager().getServicesByType(DSpaceRunnable.class);
+     * This method will simply execute the script
-        String command = args[0];
+     * @param args                  The arguments of the script with the script name as first place in the array
-        for (DSpaceRunnable script : scripts) {
+     * @param dSpaceRunnableHandler The relevant DSpaceRunnableHandler
-            if (StringUtils.equalsIgnoreCase(script.getName(), command)) {
+     * @param script                The script to be executed
-                try {
+     * @return                      A 1 or 0 depending on whether the script failed or passed respectively
-                    script.initialize(args, dSpaceRunnableHandler);
+     */
-                    script.run();
+    private static int executeScript(String[] args, DSpaceRunnableHandler dSpaceRunnableHandler,
-                    return 0;
+                                     DSpaceRunnable script) {
-                } catch (ParseException e) {
+        try {
-                    script.printHelp();
+            script.initialize(args, dSpaceRunnableHandler);
-                    log.error(e.getMessage(), e);
+            script.run();
-                    System.out.println(e.getMessage());
+            return 0;
-                    e.printStackTrace();
+        } catch (ParseException e) {
-                    return 1;
+            script.printHelp();
-                }
+            e.printStackTrace();
-            }
+            return 1;
         }
-        return null;
     }
 
     protected static int runOneCommand(Document commandConfigs, String[] args) {

dspace-api/src/main/java/org/dspace/content/factory/ProcessServiceFactory.java

@@ -1,34 +0,0 @@
-/**
- * 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.content.factory;
-
-import org.dspace.scripts.service.ProcessService;
-import org.dspace.services.factory.DSpaceServicesFactory;
-
-/**
- * Abstract factory to get services for the Process workload, use ProcessServiceFactory.getInstance() to retrieve an
- * implementation
- *
- */
-public abstract class ProcessServiceFactory {
-
-    /**
-     * This method will return an instance of the ProcessService
-     * @return  An instance of the ProcessService
-     */
-    public abstract ProcessService getProcessService();
-
-    /**
-     * Use this method to retrieve an implementation of the ProcessServiceFactory to use to retrieve the different beans
-     * @return  An implementation of the ProcessServiceFactory
-     */
-    public static ProcessServiceFactory getInstance() {
-        return DSpaceServicesFactory.getInstance().getServiceManager()
-                                    .getServiceByName("processServiceFactory", ProcessServiceFactory.class);
-    }
-}

dspace-api/src/main/java/org/dspace/scripts/ScriptServiceImpl.java

@@ -0,0 +1,31 @@
+/**
+ * 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.scripts;
+
+import java.util.List;
+
+import org.apache.commons.lang3.StringUtils;
+import org.dspace.scripts.service.ScriptService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+/**
+ * The implementation for the {@link ScriptService}
+ */
+public class ScriptServiceImpl implements ScriptService {
+
+    @Autowired
+    private List<DSpaceRunnable> dSpaceRunnables;
+
+    @Override
+    public DSpaceRunnable getScriptForName(String name) {
+        return dSpaceRunnables.stream()
+                              .filter(dSpaceRunnable -> StringUtils.equalsIgnoreCase(dSpaceRunnable.getName(), name))
+                              .findFirst()
+                              .orElse(null);
+    }
+}

dspace-api/src/main/java/org/dspace/scripts/factory/ScriptServiceFactory.java

@@ -0,0 +1,41 @@
+/**
+ * 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.scripts.factory;
+
+import org.dspace.scripts.service.ProcessService;
+import org.dspace.scripts.service.ScriptService;
+import org.dspace.services.factory.DSpaceServicesFactory;
+
+/**
+ * Abstract factory to get services for the Script workload, use ScriptServiceFactory.getInstance() to retrieve an
+ * implementation
+ *
+ */
+public abstract class ScriptServiceFactory {
+
+    /**
+     * This method will return an instance of the ScriptService
+     * @return An instance of the ScriptService
+     */
+    public abstract ScriptService getScriptService();
+
+    /**
+     * This method will return an instance of the ProcessService
+     * @return  An instance of the ProcessService
+     */
+    public abstract ProcessService getProcessService();
+
+    /**
+     * Use this method to retrieve an implementation of the ScriptServiceFactory to use to retrieve the different beans
+     * @return An implementation of the ScriptServiceFactory
+     */
+    public static ScriptServiceFactory getInstance() {
+        return DSpaceServicesFactory.getInstance().getServiceManager()
+                                    .getServiceByName("scriptServiceFactory", ScriptServiceFactory.class);
+    }
+}

dspace-api/src/main/java/org/dspace/scripts/factory/impl/ScriptServiceFactoryImpl.java

@@ -5,20 +5,29 @@
  *
  * http://www.dspace.org/license/
  */
-package org.dspace.content.factory.impl;
+package org.dspace.scripts.factory.impl;
 
-import org.dspace.content.factory.ProcessServiceFactory;
+import org.dspace.scripts.factory.ScriptServiceFactory;
 import org.dspace.scripts.service.ProcessService;
+import org.dspace.scripts.service.ScriptService;
 import org.springframework.beans.factory.annotation.Autowired;
 
 /**
- * The implementation for the {@link ProcessServiceFactory}
+ * The implementation for the {@link ScriptServiceFactory}
  */
-public class ProcessServiceFactoryImpl extends ProcessServiceFactory {
+public class ScriptServiceFactoryImpl extends ScriptServiceFactory {
+
+    @Autowired(required = true)
+    private ScriptService scriptService;
 
     @Autowired(required = true)
     private ProcessService processService;
 
+    @Override
+    public ScriptService getScriptService() {
+        return scriptService;
+    }
+
     @Override
     public ProcessService getProcessService() {
         return processService;

dspace-api/src/main/java/org/dspace/scripts/service/ScriptService.java

@@ -0,0 +1,23 @@
+/**
+ * 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.scripts.service;
+
+import org.dspace.scripts.DSpaceRunnable;
+
+/**
+ * This service will deal with logic to handle DSpaceRunnable objects
+ */
+public interface ScriptService {
+
+    /**
+     * This method will return the DSpaceRunnable that has the name that's equal to the name given in the parameters
+     * @param name  The name that the script has to match
+     * @return      The matching DSpaceRunnable script
+     */
+    DSpaceRunnable getScriptForName(String name);
+}

dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java

@@ -29,6 +29,7 @@ import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.context.request.WebRequest;
+import org.springframework.web.multipart.MultipartException;
 import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
 
 /**
@@ -54,8 +55,8 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH
         }
     }
 
-    @ExceptionHandler(IllegalArgumentException.class)
+    @ExceptionHandler({IllegalArgumentException.class, MultipartException.class})
-    protected void handleIllegalArgumentException(HttpServletRequest request, HttpServletResponse response,
+    protected void handleWrongRequestException(HttpServletRequest request, HttpServletResponse response,
                                                   Exception ex) throws IOException {
         sendErrorResponse(request, response, ex, ex.getMessage(), HttpServletResponse.SC_BAD_REQUEST);
     }

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ScriptRestRepository.java

@@ -35,6 +35,7 @@ import org.dspace.core.Context;
 import org.dspace.scripts.DSpaceCommandLineParameter;
 import org.dspace.scripts.DSpaceRunnable;
 import org.dspace.scripts.service.ProcessService;
+import org.dspace.scripts.service.ScriptService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
@@ -61,18 +62,22 @@ public class ScriptRestRepository extends DSpaceRestRepository<ScriptRest, Strin
     @Autowired
     private ProcessConverter processConverter;
 
+    @Autowired
+    private ScriptService scriptService;
+
     @Autowired
     private DSpaceRunnableParameterConverter dSpaceRunnableParameterConverter;
 
+
     @Override
     public ScriptRest findOne(Context context, String name) {
-        for (DSpaceRunnable dSpaceRunnable : dspaceRunnables) {
+
-            if (StringUtils.equalsIgnoreCase(dSpaceRunnable.getName(), name)) {
+        DSpaceRunnable dSpaceRunnable = scriptService.getScriptForName(name);
-                if (dSpaceRunnable.isAllowedToExecute(context)) {
+        if (dspaceRunnables != null) {
-                    return scriptConverter.fromModel(dSpaceRunnable);
+            if (dSpaceRunnable.isAllowedToExecute(context)) {
-                } else {
+                return scriptConverter.fromModel(dSpaceRunnable);
-                    throw new AccessDeniedException("The current user was not authorized to access this script");
+            } else {
-                }
+                throw new AccessDeniedException("The current user was not authorized to access this script");
             }
         }
         throw new DSpaceBadRequestException("The script with name: " + name + " could not be found");
@@ -108,7 +113,7 @@ public class ScriptRestRepository extends DSpaceRestRepository<ScriptRest, Strin
         String properties = requestService.getCurrentRequest().getServletRequest().getParameter("properties");
         List<DSpaceCommandLineParameter> dSpaceCommandLineParameters =
             processPropertiesToDSpaceCommandLineParameters(properties);
-        DSpaceRunnable scriptToExecute = getdSpaceRunnableForName(scriptName);
+        DSpaceRunnable scriptToExecute = scriptService.getScriptForName(scriptName);
         if (scriptToExecute == null) {
             throw new DSpaceBadRequestException("The script for name: " + scriptName + " wasn't found");
         }
@@ -145,17 +150,6 @@ public class ScriptRestRepository extends DSpaceRestRepository<ScriptRest, Strin
         return dSpaceCommandLineParameters;
     }
 
-    private DSpaceRunnable getdSpaceRunnableForName(String scriptName) {
-        DSpaceRunnable scriptToExecute = null;
-        for (DSpaceRunnable script : dspaceRunnables) {
-            if (StringUtils.equalsIgnoreCase(script.getName(), scriptName)) {
-                scriptToExecute = script;
-                break;
-            }
-        }
-        return scriptToExecute;
-    }
-
     private List<String> constructArgs(List<DSpaceCommandLineParameter> dSpaceCommandLineParameters) {
         List<String> args = new ArrayList<>();
         for (DSpaceCommandLineParameter parameter : dSpaceCommandLineParameters) {

dspace-server-webapp/src/main/java/org/dspace/app/rest/scripts/handler/impl/RestDSpaceRunnableHandler.java

@@ -17,12 +17,12 @@ import org.apache.commons.cli.Options;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.logging.log4j.Logger;
 import org.dspace.content.ProcessStatus;
-import org.dspace.content.factory.ProcessServiceFactory;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.scripts.DSpaceCommandLineParameter;
 import org.dspace.scripts.DSpaceRunnable;
 import org.dspace.scripts.Process;
+import org.dspace.scripts.factory.ScriptServiceFactory;
 import org.dspace.scripts.handler.DSpaceRunnableHandler;
 import org.dspace.scripts.service.ProcessService;
 
@@ -33,7 +33,7 @@ public class RestDSpaceRunnableHandler implements DSpaceRunnableHandler {
     private static final Logger log = org.apache.logging.log4j.LogManager
         .getLogger(RestDSpaceRunnableHandler.class);
 
-    private ProcessService processService = ProcessServiceFactory.getInstance().getProcessService();
+    private ProcessService processService = ScriptServiceFactory.getInstance().getProcessService();
 
     private Integer processId;
     private String scriptName;

dspace-server-webapp/src/test/java/org/dspace/app/rest/builder/AbstractBuilder.java

@@ -19,7 +19,6 @@ import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.authorize.service.ResourcePolicyService;
 import org.dspace.content.Bitstream;
 import org.dspace.content.factory.ContentServiceFactory;
-import org.dspace.content.factory.ProcessServiceFactory;
 import org.dspace.content.service.BitstreamFormatService;
 import org.dspace.content.service.BitstreamService;
 import org.dspace.content.service.BundleService;
@@ -40,6 +39,7 @@ import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.eperson.service.GroupService;
 import org.dspace.eperson.service.RegistrationDataService;
+import org.dspace.scripts.factory.ScriptServiceFactory;
 import org.dspace.scripts.service.ProcessService;
 import org.dspace.services.factory.DSpaceServicesFactory;
 import org.dspace.versioning.factory.VersionServiceFactory;
@@ -134,7 +134,7 @@ public abstract class AbstractBuilder<T, S> {
         relationshipService = ContentServiceFactory.getInstance().getRelationshipService();
         relationshipTypeService = ContentServiceFactory.getInstance().getRelationshipTypeService();
         entityTypeService = ContentServiceFactory.getInstance().getEntityTypeService();
-        processService = ProcessServiceFactory.getInstance().getProcessService();
+        processService = ScriptServiceFactory.getInstance().getProcessService();
 
         // Temporarily disabled
         claimedTaskService = XmlWorkflowServiceFactory.getInstance().getClaimedTaskService();

dspace/config/spring/api/core-factory-services.xml

@@ -22,7 +22,7 @@
     <bean id="authorizeServiceFactory" class="org.dspace.authorize.factory.AuthorizeServiceFactoryImpl"/>
     <bean id="checkerServiceFactory" class="org.dspace.checker.factory.CheckerServiceFactoryImpl"/>
     <bean id="contentServiceFactory" class="org.dspace.content.factory.ContentServiceFactoryImpl"/>
-    <bean id="processServiceFactory" class="org.dspace.content.factory.impl.ProcessServiceFactoryImpl"/>
+    <bean id="scriptServiceFactory" class="org.dspace.scripts.factory.impl.ScriptServiceFactoryImpl"/>
     <bean id="coreServiceFactory" class="org.dspace.core.factory.CoreServiceFactoryImpl"/>
     <bean id="curateServiceFactory" class="org.dspace.curate.factory.CurateServiceFactoryImpl"/>
 

dspace/config/spring/api/core-services.xml

@@ -57,6 +57,7 @@
     <bean class="org.dspace.content.RelationshipMetadataServiceImpl"/>
 
     <bean class="org.dspace.scripts.ProcessServiceImpl"/>
+    <bean class="org.dspace.scripts.ScriptServiceImpl"/>
 
     <bean class="org.dspace.content.authority.ChoiceAuthorityServiceImpl"/>
     <bean class="org.dspace.content.authority.MetadataAuthorityServiceImpl" lazy-init="true"/>