Merge pull request #2596 from atmire/DS-4390_scripts-bug-scriptlauncher

[DS-4390] rewrote ScriptLauncher to include scripts from other config…

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

@@ -13,6 +13,12 @@ import java.lang.reflect.Method;
 import java.util.List;
 import java.util.TreeMap;
 
+import org.apache.commons.cli.ParseException;
+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;
@@ -27,6 +33,9 @@ import org.jdom.input.SAXBuilder;
  * @author Mark Diggory
  */
 public class ScriptLauncher {
+
+    private static final Logger log = Logger.getLogger(ScriptLauncher.class);
+
     /**
      * The service manager kernel
      */
@@ -76,8 +85,9 @@ public class ScriptLauncher {
         }
 
         // Look up command in the configuration, and execute.
-        int status;
+
-        status = runOneCommand(commandConfigs, args);
+        CommandLineDSpaceRunnableHandler commandLineDSpaceRunnableHandler = new CommandLineDSpaceRunnableHandler();
+        int status = handleScript(args, commandConfigs, commandLineDSpaceRunnableHandler, kernelImpl);
 
         // Destroy the service kernel if it is still alive
         if (kernelImpl != null) {
@@ -86,6 +96,50 @@ public class ScriptLauncher {
         }
 
         System.exit(status);
+
+    }
+
+    /**
+     * 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) {
+        int status;
+        DSpaceRunnable script = ScriptServiceFactory.getInstance().getScriptService().getScriptForName(args[0]);
+        if (script != null) {
+            status = executeScript(args, dSpaceRunnableHandler, script);
+        } else {
+            status = runOneCommand(commandConfigs, args, kernelImpl);
+        }
+        return status;
+    }
+
+    /**
+     * This method will simply execute the script
+     * @param args                  The arguments of the script with the script name as first place in the array
+     * @param dSpaceRunnableHandler The relevant DSpaceRunnableHandler
+     * @param script                The script to be executed
+     * @return                      A 1 or 0 depending on whether the script failed or passed respectively
+     */
+    private static int executeScript(String[] args, DSpaceRunnableHandler dSpaceRunnableHandler,
+                                     DSpaceRunnable script) {
+        try {
+            script.initialize(args, dSpaceRunnableHandler);
+            script.run();
+            return 0;
+        } catch (ParseException e) {
+            script.printHelp();
+            e.printStackTrace();
+            return 1;
+        }
     }
 
     protected static int runOneCommand(Document commandConfigs, String[] args) {
@@ -98,7 +152,7 @@ public class ScriptLauncher {
      * @param commandConfigs  Document
      * @param args the command line arguments given
      */
-    public static int runOneCommand(Document commandConfigs, String[] args, DSpaceKernelImpl kernelImpl) {
+    protected static int runOneCommand(Document commandConfigs, String[] args, DSpaceKernelImpl kernelImpl) {
         String request = args[0];
         Element root = commandConfigs.getRootElement();
         List<Element> commands = root.getChildren("command");

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,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.scripts;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.commons.lang3.StringUtils;
+import org.dspace.core.Context;
+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);
+    }
+
+    @Override
+    public List<DSpaceRunnable> getDSpaceRunnables(Context context) {
+        return dSpaceRunnables.stream().filter(
+            dSpaceRunnable -> dSpaceRunnable.isAllowedToExecute(context)).collect(Collectors.toList());
+    }
+}

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,33 @@
+/**
+ * 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 java.util.List;
+
+import org.dspace.core.Context;
+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);
+
+    /**
+     * This method will return a list of DSpaceRunnable objects for which the given Context is authorized to use them
+     * @param context   The relevant DSpace context
+     * @return          The list of accessible DSpaceRunnable scripts for this context
+     */
+    List<DSpaceRunnable> getDSpaceRunnables(Context context);
+}

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;
@@ -49,9 +50,6 @@ public class ScriptRestRepository extends DSpaceRestRepository<ScriptRest, Strin
 
     private static final Logger log = LogManager.getLogger();
 
-    @Autowired
-    private List<DSpaceRunnable> dspaceRunnables;
-
     @Autowired
     private ScriptConverter scriptConverter;
 
@@ -61,18 +59,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 (dSpaceRunnable != 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");
@@ -80,9 +82,8 @@ public class ScriptRestRepository extends DSpaceRestRepository<ScriptRest, Strin
 
     @Override
     public Page<ScriptRest> findAll(Context context, Pageable pageable) {
-        return utils.getPage(dspaceRunnables.stream().filter(
+        List<DSpaceRunnable> dSpaceRunnables = scriptService.getDSpaceRunnables(context);
-            dSpaceRunnable -> dSpaceRunnable.isAllowedToExecute(context)).collect(Collectors.toList()), pageable)
+        return utils.getPage(dSpaceRunnables, pageable).map(scriptConverter);
-                    .map(scriptConverter);
     }
 
     @Override
@@ -108,7 +109,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 +146,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-server-webapp/src/test/java/org/dspace/app/rest/test/AbstractIntegrationTestWithDatabase.java

@@ -15,6 +15,7 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.launcher.ScriptLauncher;
 import org.dspace.app.rest.builder.AbstractBuilder;
+import org.dspace.app.scripts.handler.impl.TestDSpaceRunnableHandler;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Community;
 import org.dspace.core.Context;
@@ -41,7 +42,7 @@ public class AbstractIntegrationTestWithDatabase extends AbstractDSpaceIntegrati
      * log4j category
      */
     private static final Logger log = LogManager
-            .getLogger(AbstractIntegrationTestWithDatabase.class);
+        .getLogger(AbstractIntegrationTestWithDatabase.class);
 
     /**
      * Context mock object to use in the tests.
@@ -179,8 +180,9 @@ public class AbstractIntegrationTestWithDatabase extends AbstractDSpaceIntegrati
 
             // Clear the search core.
             MockSolrServiceImpl searchService = DSpaceServicesFactory.getInstance()
-                    .getServiceManager()
+                                                                     .getServiceManager()
-                    .getServiceByName(SearchService.class.getName(), MockSolrServiceImpl.class);
+                                                                     .getServiceByName(SearchService.class.getName(),
+                                                                                       MockSolrServiceImpl.class);
             searchService.reset();
 
             // Reload our ConfigurationService (to reset configs to defaults again)
@@ -227,7 +229,13 @@ public class AbstractIntegrationTestWithDatabase extends AbstractDSpaceIntegrati
             }
 
             // Look up command in the configuration, and execute.
-            return ScriptLauncher.runOneCommand(commandConfigs, args, kernelImpl);
+            TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
+            int status =  ScriptLauncher.handleScript(args, commandConfigs, testDSpaceRunnableHandler, kernelImpl);
+            if (testDSpaceRunnableHandler.getException() != null) {
+                throw testDSpaceRunnableHandler.getException();
+            } else {
+                return status;
+            }
         } finally {
             if (!context.isValid()) {
                 setUp();

dspace-server-webapp/src/test/java/org/dspace/app/scripts/handler/impl/TestDSpaceRunnableHandler.java

@@ -0,0 +1,36 @@
+/**
+ * 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.app.scripts.handler.impl;
+
+import org.dspace.scripts.handler.impl.CommandLineDSpaceRunnableHandler;
+
+/**
+ * This class will be used as a DSpaceRunnableHandler for the Tests so that we can stop the handler
+ * from calling System.exit() when a script would throw an exception
+ */
+public class TestDSpaceRunnableHandler extends CommandLineDSpaceRunnableHandler {
+
+    private Exception exception = null;
+
+    /**
+     * We're overriding this method so that we can stop the script from doing the System.exit() if
+     * an exception within the script is thrown
+     */
+    @Override
+    public void handleException(String message, Exception e) {
+        exception = e;
+    }
+
+    /**
+     * Generic getter for the exception
+     * @return the exception value of this TestDSpaceRunnableHandler
+     */
+    public Exception getException() {
+        return exception;
+    }
+}

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"/>