Author: jleroux
Date: Thu May 24 20:42:19 2018
New Revision: 1832199
URL: http://svn.apache.org/viewvc?rev=1832199&view=rev
Log:
Improved: Refactor `JavaEventHandler` class
(OFBIZ-10410)
Instead of relying on manual synchronisation, use a concurrent map for caching
loaded classes.
Inline private `invoke` delegate which intent was fuzzy.
Spread arguments when calling `Method::invoke` and `Class::getMethod`.
jleroux: interesting, it's the 1st use in OFBiz of the "::" syntax for calling
methods
(cf.
https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.13-500)
I note that the string "::" is used in many places as a separator, notably
in caches, as cache key separator but this should not be confusing.
Thanks: Mathieu Lirzin
Modified:
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/JavaEventHandler.java
Modified:
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/JavaEventHandler.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/JavaEventHandler.java?rev=1832199&r1=1832198&r2=1832199&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/JavaEventHandler.java
(original)
+++
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/JavaEventHandler.java
Thu May 24 20:42:19 2018
@@ -19,8 +19,7 @@
package org.apache.ofbiz.webapp.event;
import java.lang.reflect.Method;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
@@ -40,7 +39,20 @@ public class JavaEventHandler implements
public static final String module = JavaEventHandler.class.getName();
- private Map<String, Class<?>> eventClassMap = new HashMap<String,
Class<?>>();
+ /* Cache for event handler classes. */
+ private ConcurrentHashMap<String, Class<?>> classes = new
ConcurrentHashMap<>();
+
+ /* Return class corresponding to path or null. */
+ private static Class<?> loadClass(String path) {
+ try {
+ ClassLoader l = Thread.currentThread().getContextClassLoader();
+ return l.loadClass(path);
+ } catch (ClassNotFoundException e) {
+ Debug.logError(e, "Error loading class with name: "+ path
+ + ", will not be able to run event...", module);
+ return null;
+ }
+ }
/**
* @see
org.apache.ofbiz.webapp.event.EventHandler#init(javax.servlet.ServletContext)
@@ -51,56 +63,29 @@ public class JavaEventHandler implements
/**
* @see
org.apache.ofbiz.webapp.event.EventHandler#invoke(ConfigXMLReader.Event,
ConfigXMLReader.RequestMap, javax.servlet.http.HttpServletRequest,
javax.servlet.http.HttpServletResponse)
*/
- public String invoke(Event event, RequestMap requestMap,
HttpServletRequest request, HttpServletResponse response) throws
EventHandlerException {
- Class<?> eventClass = this.eventClassMap.get(event.path);
-
- if (eventClass == null) {
- synchronized (this) {
- eventClass = this.eventClassMap.get(event.path);
- if (eventClass == null) {
- try {
- ClassLoader loader =
Thread.currentThread().getContextClassLoader();
- eventClass = loader.loadClass(event.path);
- } catch (ClassNotFoundException e) {
- Debug.logError(e, "Error loading class with name: " + event.path
+ ", will not be able to run event...", module);
- }
- if (eventClass != null) {
- eventClassMap.put(event.path, eventClass);
- }
- }
- }
- }
- if (Debug.verboseOn()) Debug.logVerbose("[Set path/method]: " + event.path +
" / " + event.invoke, module);
-
- Class<?>[] paramTypes = new Class<?>[] {HttpServletRequest.class,
HttpServletResponse.class};
-
+ public String invoke(Event event, RequestMap requestMap,
+ HttpServletRequest request, HttpServletResponse response)
+ throws EventHandlerException {
+ Class<?> k = classes.computeIfAbsent(event.path,
JavaEventHandler::loadClass);
if (Debug.verboseOn()) Debug.logVerbose("*[[Event invocation]]*",
module);
- Object[] params = new Object[] {request, response};
-
- return invoke(event.path, event.invoke, eventClass, paramTypes,
params, event.transactionTimeout);
- }
-
- private String invoke(String eventPath, String eventMethod, Class<?> eventClass,
Class<?>[] paramTypes, Object[] params, int transactionTimeout) throws
EventHandlerException {
- boolean beganTransaction = false;
- if (eventClass == null) {
- throw new EventHandlerException("Error invoking event, the class " +
eventPath + " was not found");
+ if (k == null) {
+ throw new EventHandlerException("Error invoking event, the class "
+ + event.path + " was not found");
}
- if (eventPath == null || eventMethod == null) {
+ if (event.path == null || event.invoke == null) {
throw new EventHandlerException("Invalid event method or path; call
initialize()");
}
- if (Debug.verboseOn()) Debug.logVerbose("[Processing]: JAVA Event",
module);
+ if (Debug.verboseOn()) Debug.logVerbose("[Processing]: Java Event",
module);
+ boolean began = false;
try {
- if (transactionTimeout > 0) {
- beganTransaction = TransactionUtil.begin(transactionTimeout);
- } else {
- beganTransaction = TransactionUtil.begin();
- }
- Method m = eventClass.getMethod(eventMethod, paramTypes);
- String eventReturn = (String) m.invoke(null, params);
-
- if (Debug.verboseOn()) Debug.logVerbose("[Event Return]: " +
eventReturn, module);
- return eventReturn;
+ int timeout = Integer.max(event.transactionTimeout, 0);
+ began = TransactionUtil.begin(timeout);
+ Method m = k.getMethod(event.invoke, HttpServletRequest.class,
+ HttpServletResponse.class);
+ String ret = (String) m.invoke(null, request, response);
+ if (Debug.verboseOn()) Debug.logVerbose("[Event Return]: " + ret,
module);
+ return ret;
} catch (java.lang.reflect.InvocationTargetException e) {
Throwable t = e.getTargetException();
@@ -116,7 +101,7 @@ public class JavaEventHandler implements
throw new EventHandlerException("Problems processing event: " +
e.toString(), e);
} finally {
try {
- TransactionUtil.commit(beganTransaction);
+ TransactionUtil.commit(began);
} catch (GenericTransactionException e) {
Debug.logError(e, module);
}