From 940fd2e17c7e12d05f9157537be1367cf3bb44d7 Mon Sep 17 00:00:00 2001
From: Mike Hodge <hodgem@wustl.edu>
Date: Mon, 11 Jul 2016 12:48:25 -0500
Subject: [PATCH] Minor refactoring of persistent and automation events. Better
 event listener exception handling and reporting. EventClass annotation is now
 an NRG processed annotation. Fix null Python scripting engine issue

---
 build.gradle                                  |  2 +-
 .../nrg/xapi/rest/event/EventHandlerApi.java  | 55 ++++++++++---------
 .../nrg/xnat/configuration/ReactorConfig.java |  6 --
 .../entities/ScriptLaunchRequestEvent.java    |  9 +--
 .../AutomationEventScriptHandler.java         | 16 +++++-
 .../automation/AutomatedScriptRequest.java    |  4 +-
 6 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/build.gradle b/build.gradle
index 8eecde42..0c40993d 100644
--- a/build.gradle
+++ b/build.gradle
@@ -385,7 +385,7 @@ dependencies {
     compile "org.apache.httpcomponents:httpcore-nio:4.4.4"
 
     compile "org.codehaus.groovy:groovy-all:${vGroovy}"
-    compile "org.python:jython:${vJython}"
+    compile "org.python:jython-standalone:${vJython}"
 
     compile "net.sourceforge.saxon:saxon:${vSaxon}"
     compile "xalan:xalan:2.7.2"
diff --git a/src/main/java/org/nrg/xapi/rest/event/EventHandlerApi.java b/src/main/java/org/nrg/xapi/rest/event/EventHandlerApi.java
index e8717e55..166549e9 100644
--- a/src/main/java/org/nrg/xapi/rest/event/EventHandlerApi.java
+++ b/src/main/java/org/nrg/xapi/rest/event/EventHandlerApi.java
@@ -12,8 +12,9 @@ import org.nrg.automation.event.entities.AutomationFilters;
 import org.nrg.automation.services.impl.hibernate.HibernateAutomationEventIdsService;
 import org.nrg.automation.services.impl.hibernate.HibernateAutomationFiltersService;
 import org.nrg.framework.annotations.XapiRestController;
+import org.nrg.framework.event.EventClass;
 import org.nrg.framework.event.Filterable;
-import org.nrg.framework.utilities.Reflection;
+import org.nrg.framework.utilities.BasicXnatResourceLocator;
 import org.nrg.xapi.model.event.EventClassInfo;
 import org.nrg.xdat.XDAT;
 import org.nrg.xdat.om.XnatProjectdata;
@@ -22,11 +23,12 @@ import org.nrg.xdat.security.XDATUser;
 import org.nrg.xdat.security.helpers.Permissions;
 import org.nrg.xdat.security.services.RoleHolder;
 import org.nrg.xft.security.UserI;
-import org.nrg.xnat.event.conf.EventPackages;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.support.PropertiesLoaderUtils;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
@@ -44,6 +46,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 
 /**
  * The Class EventHandlerApi.
@@ -69,13 +72,6 @@ public class EventHandlerApi {
     @Autowired
     private HibernateAutomationFiltersService filtersService;
 
-    /**
-     * The event packages.
-     */
-    @SuppressWarnings("MismatchedQueryAndUpdateOfCollection")
-    @Autowired
-    private EventPackages eventPackages;
-
     /**
      * Inits the this.
      */
@@ -183,7 +179,7 @@ public class EventHandlerApi {
                         Collections.sort(valueList);
                         filterableFields.put(autoFilters.getField(), valueList);
                     } else {
-                        for (String value : autoFilters.getValues()) {
+                        for (final String value : autoFilters.getValues()) {
                             final List<String> values = filterableFields.get(autoFilters.getField());
                             if (!values.contains(value)) {
                                 values.add(value);
@@ -206,23 +202,28 @@ public class EventHandlerApi {
      */
     private List<String> getEventClassList(List<AutomationEventIds> eventIdsList) {
         final List<String> classList = Lists.newArrayList();
-        // ClassList should be pulled from available event classes rather than from events
-        if (eventPackages != null) {
-            for (final String pkg : eventPackages) {
-                try {
-                    for (final Class<?> clazz : Reflection.getClassesForPackage(pkg)) {
-                        if (AutomationEventImplementerI.class.isAssignableFrom(clazz) && !clazz.isInterface() &&
-                            !Modifier.isAbstract(clazz.getModifiers())) {
-                            if (!classList.contains(clazz.getName())) {
-                                classList.add(clazz.getName());
-                            }
-                        }
-                    }
-                } catch (ClassNotFoundException | IOException e) {
-                    // Do nothing.
-                }
-            }
-        }
+        try {
+			for (final Resource resource : BasicXnatResourceLocator.getResources("classpath*:META-INF/xnat/events/*-event.properties")) {
+				final Properties properties = PropertiesLoaderUtils.loadProperties(resource);
+				if (!properties.containsKey(EventClass.EVENT_CLASS)) {
+					continue;
+				}
+				final String clssStr = properties.get(EventClass.EVENT_CLASS).toString();
+				try {
+					final Class<?> clazz = Class.forName(clssStr);
+					if (AutomationEventImplementerI.class.isAssignableFrom(clazz) && !clazz.isInterface() &&
+							!Modifier.isAbstract(clazz.getModifiers())) {
+						if (!classList.contains(clazz.getName())) {
+							classList.add(clazz.getName());
+						}
+					}
+				} catch (ClassNotFoundException cex) {
+					_log.debug("Could not load class for class name (" + clssStr + ")");
+				}
+			}
+		} catch (IOException e) {
+			_log.debug("Could not load event class properties resources (META-INF/xnat/*-event.properties)");
+		}
         // I think for now we'll not pull from the database if we've found event classes.  If the database
         // contains any thing different, it should only be event classes that are no longer available.
         if (classList.size() < 1) {
diff --git a/src/main/java/org/nrg/xnat/configuration/ReactorConfig.java b/src/main/java/org/nrg/xnat/configuration/ReactorConfig.java
index 27138d6c..d831d26c 100755
--- a/src/main/java/org/nrg/xnat/configuration/ReactorConfig.java
+++ b/src/main/java/org/nrg/xnat/configuration/ReactorConfig.java
@@ -26,12 +26,6 @@ public class ReactorConfig {
         return new XftItemEventListener(eventBus);
     }
 
-    @Bean
-    public EventPackages eventPackages() {
-        // NOTE:  These should be treated as parent packages.  All sub-packages should be searched
-        return new EventPackages(new HashSet<>(Arrays.asList(new String[]{"org.nrg.xnat.event", "org.nrg.xft.event", "org.nrg.xdat.event"})));
-    }
-
     /**
      * Env.
      *
diff --git a/src/main/java/org/nrg/xnat/event/entities/ScriptLaunchRequestEvent.java b/src/main/java/org/nrg/xnat/event/entities/ScriptLaunchRequestEvent.java
index ff474487..e5d2d2a0 100644
--- a/src/main/java/org/nrg/xnat/event/entities/ScriptLaunchRequestEvent.java
+++ b/src/main/java/org/nrg/xnat/event/entities/ScriptLaunchRequestEvent.java
@@ -10,15 +10,16 @@ import org.nrg.automation.event.AutomationEventImplementerI;
 import org.nrg.automation.event.entities.AutomationCompletionEvent;
 import org.nrg.automation.event.entities.PersistentEvent;
 import org.nrg.framework.event.EventClass;
-import org.nrg.framework.event.persist.PersistentEventImplementerI;
+
+import com.google.common.collect.Maps;
 
 /**
  * The Class AutomationLaunchRequestEvent.
  */
 @Entity
 @PrimaryKeyJoinColumn(name="ID", referencedColumnName="ID")
-@EventClass(displayName="Script Launch Request Event")
-public class ScriptLaunchRequestEvent extends PersistentEvent implements PersistentEventImplementerI, AutomationEventImplementerI {
+@EventClass(name="ScriptLaunchRequestEvent", description="Script Launch Request Event")
+public class ScriptLaunchRequestEvent extends PersistentEvent implements AutomationEventImplementerI {
 	
 	/** The Constant serialVersionUID. */
 	private static final long serialVersionUID = 7465778737330635218L;
@@ -26,7 +27,7 @@ public class ScriptLaunchRequestEvent extends PersistentEvent implements Persist
 	/** The automation completion event. */
 	private AutomationCompletionEvent automationCompletionEvent;
 	
-	private Map<String,Object> parameterMap;
+	private Map<String,Object> parameterMap = Maps.newHashMap();
 	
 	/**
 	 * Instantiates a new automation launch request event.
diff --git a/src/main/java/org/nrg/xnat/event/listeners/AutomationEventScriptHandler.java b/src/main/java/org/nrg/xnat/event/listeners/AutomationEventScriptHandler.java
index 6f0d32a1..b5e4be61 100644
--- a/src/main/java/org/nrg/xnat/event/listeners/AutomationEventScriptHandler.java
+++ b/src/main/java/org/nrg/xnat/event/listeners/AutomationEventScriptHandler.java
@@ -22,6 +22,7 @@ import org.nrg.framework.constants.Scope;
 import org.nrg.framework.event.Filterable;
 import org.nrg.framework.event.persist.PersistentEventImplementerI;
 import org.nrg.framework.exceptions.NrgServiceException;
+import org.nrg.framework.exceptions.NrgServiceRuntimeException;
 import org.nrg.framework.services.NrgEventService;
 import org.nrg.xdat.XDAT;
 import org.nrg.xdat.security.helpers.Users;
@@ -444,14 +445,23 @@ public class AutomationEventScriptHandler implements Consumer<Event<AutomationEv
             if (PersistentWorkflowUtils.IN_PROGRESS.equals(workflow.getStatus())) {
                 WorkflowUtils.complete(workflow, workflow.buildEvent());
             }
-        } catch (NrgServiceException e) {
-            final String message = String.format("Failed running the script %s by user %s for event %s on data type %s instance %s from project %s",
+        } catch (NrgServiceException | NrgServiceRuntimeException e) {
+            final String message = String.format("Failed running the script %s by user %s for event %s on data type %s instance %s from project %s  (Exception=%s)",
                                                  request.getScriptId(),
                                                  request.getUser().getLogin(),
                                                  request.getEvent(),
                                                  request.getDataType(),
                                                  request.getDataId(),
-                                                 request.getExternalId());
+                                                 request.getExternalId(),
+                                                 e.toString());
+            if (scriptOut==null) {
+            	scriptOut = new ScriptOutput();
+            	scriptOut.setStatus(Status.ERROR);
+            	scriptOut.setOutput(message);
+            }
+            if (PersistentWorkflowUtils.IN_PROGRESS.equals(workflow.getStatus())) {
+                WorkflowUtils.fail(workflow, workflow.buildEvent());
+            }
             AdminUtils.sendAdminEmail("Script execution failure", message);
             logger.error(message, e);
             if (PersistentWorkflowUtils.IN_PROGRESS.equals(workflow.getStatus())) {
diff --git a/src/main/java/org/nrg/xnat/services/messaging/automation/AutomatedScriptRequest.java b/src/main/java/org/nrg/xnat/services/messaging/automation/AutomatedScriptRequest.java
index 1512ed44..c6204ceb 100644
--- a/src/main/java/org/nrg/xnat/services/messaging/automation/AutomatedScriptRequest.java
+++ b/src/main/java/org/nrg/xnat/services/messaging/automation/AutomatedScriptRequest.java
@@ -87,7 +87,9 @@ public class AutomatedScriptRequest implements Serializable {
 	 */
 	public AutomatedScriptRequest(final String srcEventId, final String srcEventClass, final UserI user, final String scriptId, final String event, final String scriptWorkflow, final String dataType, final String dataId, final String externalId, Map<String,Object> argumentMap) {
 		this(srcEventId, srcEventClass, user, scriptId, event, scriptWorkflow, dataType, dataId, externalId);
-		_argumentMap.putAll(argumentMap);
+		if (argumentMap != null) {
+			_argumentMap.putAll(argumentMap);
+		}
 	}	
 
 	/**
-- 
GitLab