refactored EL resolution for conditional handlers

src/main/java/net/engio/mbassy/dispatch/FilteredMessageDispatcher.java

@@ -1,7 +1,6 @@
 package net.engio.mbassy.dispatch;
 
 import net.engio.mbassy.bus.MessagePublication;
-import net.engio.mbassy.dispatch.el.ElFilter;
 import net.engio.mbassy.listener.IMessageFilter;
 
 /**
@@ -12,7 +11,7 @@ import net.engio.mbassy.listener.IMessageFilter;
  * @author bennidi
  *         Date: 11/23/12
  */
-public class FilteredMessageDispatcher extends DelegatingMessageDispatcher {
+public final class FilteredMessageDispatcher extends DelegatingMessageDispatcher {
 
     private final IMessageFilter[] filter;
 
@@ -38,20 +37,9 @@ public class FilteredMessageDispatcher extends DelegatingMessageDispatcher {
 
     @Override
     public void dispatch(MessagePublication publication, Object message, Iterable listeners){
-        if (passesFilter(message) && passesELFilter(message)) {
+        if (passesFilter(message)) {
             getDelegate().dispatch(publication, message, listeners);
         }
     }
 
-	/*************************************************************************
-	 * This will test the EL expression defined on the Handler annotation.
-	 * This is like a "parameterizable" filter.
-	 * @param me the message object to filter with the EL expression if there is one.
-	 * @return true if the event is allowed, false if it is rejected.
-	 ************************************************************************/
-        
-	private boolean passesELFilter(Object message) {
-		ElFilter filter = ElFilter.getInstance();
-		return filter != null && filter.accepts(message, getContext().getHandlerMetadata());
-	}
 }

src/main/java/net/engio/mbassy/dispatch/el/ElFilter.java

@@ -1,11 +1,11 @@
 package net.engio.mbassy.dispatch.el;
 
-import javax.el.ExpressionFactory;
-import javax.el.ValueExpression;
-
 import net.engio.mbassy.listener.IMessageFilter;
 import net.engio.mbassy.listener.MessageHandler;
 
+import javax.el.ExpressionFactory;
+import javax.el.ValueExpression;
+
 /*****************************************************************************
  * A filter that will use a expression from the handler annotation and 
  * parse it as EL.
@@ -13,101 +13,60 @@ import net.engio.mbassy.listener.MessageHandler;
 
 public class ElFilter implements IMessageFilter {
 
-	private static ElFilter instance;
-	
-	static {
-		try {
-			instance = new ElFilter();
-		} catch (Exception e) {
-			// Most likely the javax.el package is not available.
-			instance = null;
-		}
-	}
-	
-	private ExpressionFactory elFactory;
-	
-	/*************************************************************************
-	 * Constructor
-	 ************************************************************************/
-	
-	private ElFilter() {
-		super();
-		initELFactory();
-	}
-	
-	/*************************************************************************
-	 * Get an implementation of the ExpressionFactory. This uses the 
-	 * Java service lookup mechanism to find a proper implementation.
-	 * If none if available we do not support EL filters.
-	 ************************************************************************/
+    // thread-safe initialization of EL factory singleton
+    public static final class ExpressionFactoryHolder{
 
-	private void initELFactory() {
-		try {
-			this.elFactory = ExpressionFactory.newInstance();
-		} catch (RuntimeException e) {
-			// No EL implementation on the class path.
-			elFactory = null;
-		}
-	}
-	
-	/*************************************************************************
-	 * accepts
-	 * @see net.engio.mbassy.listener.IMessageFilter#accepts(java.lang.Object, net.engio.mbassy.listener.MessageHandler)
-	 ************************************************************************/
+        // if runtime exception is thrown, this will
+        public static final ExpressionFactory ELFactory = getELFactory();
+
+        /*************************************************************************
+         * Get an implementation of the ExpressionFactory. This uses the
+         * Java service lookup mechanism to find a proper implementation.
+         * If none if available we do not support EL filters.
+         ************************************************************************/
+        private static final ExpressionFactory getELFactory(){
+            try {
+                return ExpressionFactory.newInstance();
+            } catch (RuntimeException e) {
+                return null;
+            }
+        }
+    }
+
+    public static final boolean isELAvailable(){
+        return ExpressionFactoryHolder.ELFactory != null;
+    }
+
+    public static final ExpressionFactory ELFactory(){
+        return ExpressionFactoryHolder.ELFactory;
+    }
+
+    /**
+     * Accepts a message if the associated EL expression of the message handler resolves to 'true'
+     *
+     * @param message the message to be handled by the handler
+     * @param  metadata the metadata object which describes the message handler
+     * @return
+     */
 	@Override
 	public boolean accepts(Object message, MessageHandler metadata) {
 		String expression = metadata.getCondition();
-		if (expression == null || expression.trim().length() == 0) {
-			return true;
-		}
-		if (elFactory == null) {
-			// TODO should we test this some where earlier? Perhaps in MessageHandler.validate()  ?
-			throw new IllegalStateException("A handler uses an EL filter but no EL implementation is available.");
-		}
-		
-		expression = cleanupExpression(expression);
-		
-		EventContext context = new EventContext();
-		context.bindToEvent(message);
-		
+		StandardELResolutionContext context = new StandardELResolutionContext(message);
 		return evalExpression(expression, context);
 	}
 
-	/*************************************************************************
-	 * @param expression
-	 * @param context
-	 * @return
-	 ************************************************************************/
-	
-	private boolean evalExpression(String expression, EventContext context) {
-		ValueExpression ve = elFactory.createValueExpression(context, expression, Boolean.class);
-		Object result = ve.getValue(context);
-		if (!(result instanceof Boolean)) {
-			throw new IllegalStateException("A handler uses an EL filter but the output is not \"true\" or \"false\".");
-		}
-		return (Boolean)result;
-	}
-
-	/*************************************************************************
-	 * Make it a valid expression because the parser expects it like this.
-	 * @param expression
-	 * @return
-	 ************************************************************************/
-	
-	private String cleanupExpression(String expression) {
-		 
-		if (!expression.trim().startsWith("${") && !expression.trim().startsWith("#{")) {
-			expression = "${"+expression+"}";
-		}
-		return expression;
-	}
-
-	/*************************************************************************
-	 * @return the one and only
-	 ************************************************************************/
-	
-	public static synchronized ElFilter getInstance() {
-		return instance;
+	private boolean evalExpression(String expression, StandardELResolutionContext context) {
+		ValueExpression ve = ELFactory().createValueExpression(context, expression, Boolean.class);
+		try{
+            Object result = ve.getValue(context);
+            return (Boolean)result;
+             }
+        catch(Throwable exception){
+            // TODO: BusRuntime should be available in this filter to propagate resolution errors
+            // -> this is generally a good feature for filters
+            return false;
+            //throw new IllegalStateException("A handler uses an EL filter but the output is not \"true\" or \"false\".");
+        }
 	}
 
 }

src/main/java/net/engio/mbassy/dispatch/el/EventContext.java

@@ -1,102 +0,0 @@
-package net.engio.mbassy.dispatch.el;
-
-import java.lang.reflect.Method;
-
-import javax.el.BeanELResolver;
-import javax.el.CompositeELResolver;
-import javax.el.ELContext;
-import javax.el.ELResolver;
-import javax.el.FunctionMapper;
-import javax.el.ValueExpression;
-import javax.el.VariableMapper;
-
-/*****************************************************************************
- * An EL context that knows how to resolve everything from a 
- * given message but event.
- ****************************************************************************/
-
-public class EventContext extends ELContext {
-
-	private final CompositeELResolver resolver;
-	private final FunctionMapper functionMapper;
-	private final VariableMapper variableMapper;
-	private RootResolver rootResolver;
-
-	/*************************************************************************
-	 * Constructor
-	 * 
-	 * @param me
-	 ************************************************************************/
-
-	public EventContext() {
-		super();
-		this.functionMapper = new NoopFunctionMapper();
-		this.variableMapper = new NoopMapperImpl();
-		
-		this.resolver = new CompositeELResolver();
-		this.rootResolver = new RootResolver();
-		this.resolver.add(rootResolver);
-		this.resolver.add(new BeanELResolver(true));
-	}
-	
-	 /*************************************************************************
-	 * Binds an event object with the EL expression. This will allow access
-	 * to all properties of a given event.
-	 * @param event to bind.
-	 ************************************************************************/
-	
-	public void bindToEvent(Object event) {
-		 this.rootResolver.setRoot(event);
-	 }
-
-	/*************************************************************************
-	 * The resolver for the event object.
-	 * @see javax.el.ELContext#getELResolver()
-	 ************************************************************************/
-	@Override
-	public ELResolver getELResolver() {
-		return this.resolver;
-	}
-
-	/*************************************************************************
-	 * @see javax.el.ELContext#getFunctionMapper()
-	 ************************************************************************/
-	@Override
-	public FunctionMapper getFunctionMapper() {
-		return this.functionMapper;
-	}
-
-	/*************************************************************************
-	 * @see javax.el.ELContext#getVariableMapper()
-	 ************************************************************************/
-	@Override
-	public VariableMapper getVariableMapper() {
-		return this.variableMapper;
-	}
-
-	/*****************************************************************************
-	 *  Dummy implementation.
-	 ****************************************************************************/
-	
-	private class NoopMapperImpl extends VariableMapper {
-		public ValueExpression resolveVariable(String s) {
-			return null;
-		}
-
-		public ValueExpression setVariable(String s,
-				ValueExpression valueExpression) {
-			return null;
-		}
-	}
-
-	/*****************************************************************************
-	 *  Dummy implementation.
-	 ****************************************************************************/
-	
-	private class NoopFunctionMapper extends FunctionMapper {
-		public Method resolveFunction(String s, String s1) {
-			return null;
-		}
-	}
-
-}

src/main/java/net/engio/mbassy/dispatch/el/RootResolver.java

@@ -1,89 +0,0 @@
-package net.engio.mbassy.dispatch.el;
-
-import java.beans.FeatureDescriptor;
-import java.util.Iterator;
-
-import javax.el.ELContext;
-import javax.el.ELResolver;
-
-/*****************************************************************************
- * A resolver that will resolve the "msg" variable to the event object that 
- * is posted.
- ****************************************************************************/
-
-public class RootResolver extends ELResolver {
-
-	private static final String ROOT_VAR_NAME = "msg";
-	public Object rootObject; 
-	
-	/*************************************************************************
-	 * @param rootObject
-	 ************************************************************************/
-	
-	public void setRoot(Object rootObject) {
-		this.rootObject = rootObject;
-	}
-	
-	/*************************************************************************
-	 * getValue
-	 * @see javax.el.ELResolver#getValue(javax.el.ELContext, java.lang.Object, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public Object getValue(ELContext context, Object base, Object property) {
-		if (context == null) {
-			throw new NullPointerException();
-		}
-		if (base == null && ROOT_VAR_NAME.equals(property)) {
-			context.setPropertyResolved(true);
-			return this.rootObject;
-		}
-		return null;
-	}
-
-	
-	/*************************************************************************
-	 * getCommonPropertyType
-	 * @see javax.el.ELResolver#getCommonPropertyType(javax.el.ELContext, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public Class<?> getCommonPropertyType(ELContext context, Object base) {
-		return String.class;
-	}
-
-	/*************************************************************************
-	 * getFeatureDescriptors
-	 * @see javax.el.ELResolver#getFeatureDescriptors(javax.el.ELContext, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public Iterator<FeatureDescriptor> getFeatureDescriptors(ELContext context, Object base) {
-		return null;
-	}
-
-	/*************************************************************************
-	 * getType
-	 * @see javax.el.ELResolver#getType(javax.el.ELContext, java.lang.Object, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public Class<?> getType(ELContext context, Object base, Object property) {
-		return null;
-	}
-
-	/*************************************************************************
-	 * isReadOnly
-	 * @see javax.el.ELResolver#isReadOnly(javax.el.ELContext, java.lang.Object, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public boolean isReadOnly(ELContext context, Object base, Object property) {
-		return true;
-	}
-
-	/*************************************************************************
-	 * setValue
-	 * @see javax.el.ELResolver#setValue(javax.el.ELContext, java.lang.Object, java.lang.Object, java.lang.Object)
-	 ************************************************************************/
-	@Override
-	public void setValue(ELContext context, Object base, Object property, Object value) {
-		// Do nothing
-	}
-
-}

src/main/java/net/engio/mbassy/dispatch/el/StandardELResolutionContext.java

@@ -0,0 +1,92 @@
+package net.engio.mbassy.dispatch.el;
+
+import javax.el.*;
+import java.lang.reflect.Method;
+
+/**
+ *  This ELContext implementation provides support for standard BeanEL resolution in conditional message handlers.
+ *  The message parameter of the message handlers is bound to 'msg' such that it can be referenced int the EL expressions.
+ *
+ *  Example:
+ *  @Handler(condition = "msg.type == 'onClick'")
+ *  public void handle(ButtonEvent event)
+ *
+ */
+public class StandardELResolutionContext extends ELContext {
+
+	private final ELResolver resolver;
+	private final FunctionMapper functionMapper;
+	private final VariableMapper variableMapper;
+    private final Object message;
+
+
+	public StandardELResolutionContext(Object message) {
+		super();
+        this.message = message;
+		this.functionMapper = new NoopFunctionMapper();
+		this.variableMapper = new MsgMapper();
+        // Composite resolver not necessary as the only resolution type currently supported is standard BeanEL
+		//this.resolver = new CompositeELResolver();
+        this.resolver = new BeanELResolver(true);
+	}
+	
+
+
+	/*************************************************************************
+	 * The resolver for the event object.
+	 * @see javax.el.ELContext#getELResolver()
+	 ************************************************************************/
+	@Override
+	public ELResolver getELResolver() {
+		return this.resolver;
+	}
+
+	/*************************************************************************
+	 * @see javax.el.ELContext#getFunctionMapper()
+	 ************************************************************************/
+	@Override
+	public FunctionMapper getFunctionMapper() {
+		return this.functionMapper;
+	}
+
+	/*************************************************************************
+	 * @see javax.el.ELContext#getVariableMapper()
+	 ************************************************************************/
+	@Override
+	public VariableMapper getVariableMapper() {
+		return this.variableMapper;
+	}
+
+    /**
+     * This mapper resolves the variable identifies "msg" to the message
+     * object of the current handler invocation
+     */
+	private class MsgMapper extends VariableMapper {
+        private static final String msg = "msg";
+        // reuse the same expression as it always resolves to the same object
+        private final ValueExpression msgExpression = ElFilter.ELFactory().createValueExpression(message, message.getClass());
+
+		public ValueExpression resolveVariable(final String s) {
+            // resolve 'msg' to the message object of the handler invocation
+            return !s.equals(msg) ? null : msgExpression;
+		}
+
+		public ValueExpression setVariable(String s,
+				ValueExpression valueExpression) {
+            // not necessary - the mapper resolves only "msg" and nothing else
+			return null;
+		}
+	}
+
+    /**
+     * This function mapper does nothing, i.e. custom EL functions are not
+     * supported by default. It may be supported in the future to pass in
+     * custom function mappers at bus instanciation time.
+     */
+	private class NoopFunctionMapper extends FunctionMapper {
+		public Method resolveFunction(String s, String s1) {
+			return null;
+		}
+	}
+
+}

src/main/java/net/engio/mbassy/listener/MessageHandler.java

@@ -1,6 +1,7 @@
 package net.engio.mbassy.listener;
 
 import net.engio.mbassy.dispatch.HandlerInvocation;
+import net.engio.mbassy.dispatch.el.ElFilter;
 
 import java.lang.reflect.Method;
 import java.util.HashMap;
@@ -44,6 +45,9 @@ public class MessageHandler {
             if(handler == null){
                 throw new IllegalArgumentException("The message handler configuration may not be null");
             }
+            if(filter == null){
+                filter = new IMessageFilter[]{};
+            }
             net.engio.mbassy.listener.Enveloped enveloped = handler.getAnnotation(Enveloped.class);
             Class[] handledMessages = enveloped != null
                     ? enveloped.messages()
@@ -51,8 +55,21 @@ public class MessageHandler {
             handler.setAccessible(true);
             Map<String, Object> properties = new HashMap<String, Object>();
             properties.put(HandlerMethod, handler);
-            properties.put(Filter, filter != null ? filter : new IMessageFilter[]{});
-            properties.put(Condition, handlerConfig.condition());
+            // add EL filter if a condition is present
+            if(handlerConfig.condition() != null){
+                if (!ElFilter.isELAvailable()) {
+                    throw new IllegalStateException("A handler uses an EL filter but no EL implementation is available.");
+                }
+
+                IMessageFilter[] expandedFilter = new IMessageFilter[filter.length + 1];
+                for(int i = 0; i < filter.length ; i++){
+                   expandedFilter[i] = filter[i];
+                }
+                expandedFilter[filter.length] = new ElFilter();
+                filter = expandedFilter;
+            }
+            properties.put(Filter, filter);
+            properties.put(Condition, cleanEL(handlerConfig.condition()));
             properties.put(Priority, handlerConfig.priority());
             properties.put(Invocation, handlerConfig.invocation());
             properties.put(InvocationMode, handlerConfig.delivery());
@@ -63,6 +80,14 @@ public class MessageHandler {
             properties.put(HandledMessages, handledMessages);
             return properties;
         }
+
+        private static String cleanEL(String expression) {
+
+            if (!expression.trim().startsWith("${") && !expression.trim().startsWith("#{")) {
+                expression = "${"+expression+"}";
+            }
+            return expression;
+        }
     }
 
 

src/test/java/net/engio/mbassy/ConditionTest.java

@@ -3,10 +3,16 @@ package net.engio.mbassy;
 import net.engio.mbassy.bus.MBassador;
 import net.engio.mbassy.bus.config.BusConfiguration;
 import net.engio.mbassy.common.MessageBusTest;
+import net.engio.mbassy.listener.Enveloped;
 import net.engio.mbassy.listener.Handler;
-
+import net.engio.mbassy.listener.Listener;
+import net.engio.mbassy.listener.References;
+import net.engio.mbassy.subscription.MessageEnvelope;
 import org.junit.Test;
 
+import java.util.HashSet;
+import java.util.Set;
+
 /*****************************************************************************
  * Some unit tests for the "condition" filter.
  ****************************************************************************/
@@ -15,7 +21,7 @@ public class ConditionTest extends MessageBusTest {
 
 	public static class TestEvent {
 
-		public Object result;
+		private Set<String> handledBy = new HashSet<String>();
 		private String type;
 		private int size;
 
@@ -32,34 +38,57 @@ public class ConditionTest extends MessageBusTest {
 		public int getSize() {
 			return size;
 		}
+
+        public boolean wasHandledBy(String ...handlers){
+            for(String handler : handlers){
+                if (!handledBy.contains(handler)) return false;
+            }
+            return true;
+        }
+
+        public void handledBy(String handler){
+            handledBy.add(handler);
+        }
 		
 	}
 
+    @Listener(references = References.Strong)
 	public static class ConditionalMessageListener {
 
 		@Handler(condition = "msg.type == 'TEST'")
 		public void handleTypeMessage(TestEvent message) {
-			message.result = "handleTypeMessage";
+			message.handledBy("handleTypeMessage");
 		}
 
 		@Handler(condition = "msg.size > 4")
 		public void handleSizeMessage(TestEvent message) {
-			message.result = "handleSizeMessage";
+			message.handledBy("handleSizeMessage");
 		}
+
+        @Handler(condition = "msg.foo > 4")
+        public void handleInvalidEL(TestEvent message) {
+            message.handledBy("handleInvalidEL");
+        }
 		
 		@Handler(condition = "msg.size > 2 && msg.size < 4")
 		public void handleCombinedEL(TestEvent message) {
-			message.result = "handleCombinedEL";
+			message.handledBy( "handleCombinedEL");
 		}
 		
 		@Handler(condition = "msg.getType().equals('XYZ') && msg.getSize() == 1")
 		public void handleMethodAccessEL(TestEvent message) {
-			message.result = "handleMethodAccessEL";
+			message.handledBy("handleMethodAccessEL");
 		}
 
+        @Handler(condition = "msg.type == 'TEST'")
+        @Enveloped(messages = {TestEvent.class, Object.class})
+        public void handleEnvelopedMessage(MessageEnvelope envelope) {
+            envelope.<TestEvent>getMessage().handledBy("handleEnvelopedMessage");
+        }
 		
 	}
 
+
 	/*************************************************************************
 	 * @throws Exception
 	 ************************************************************************/
@@ -71,9 +100,11 @@ public class ConditionTest extends MessageBusTest {
 		TestEvent message = new TestEvent("TEST", 0);
 		bus.publish(message);
 
-		assertEquals("handleTypeMessage", message.result);
+		assertTrue(message.wasHandledBy("handleTypeMessage", "handleEnvelopedMessage"));
+        assertFalse(message.wasHandledBy("handleInvalidEL"));
 	}
-	
+
+
 	/*************************************************************************
 	 * @throws Exception
 	 ************************************************************************/
@@ -85,7 +116,8 @@ public class ConditionTest extends MessageBusTest {
 		TestEvent message = new TestEvent("", 5);
 		bus.publish(message);
 
-		assertEquals("handleSizeMessage", message.result);
+		assertTrue(message.wasHandledBy("handleSizeMessage"));
+        assertFalse(message.wasHandledBy("handleInvalidEL"));
 	}
 	
 	/*************************************************************************
@@ -99,7 +131,8 @@ public class ConditionTest extends MessageBusTest {
 		TestEvent message = new TestEvent("", 3);
 		bus.publish(message);
 
-		assertEquals("handleCombinedEL", message.result);
+        assertTrue(message.wasHandledBy("handleCombinedEL"));
+        assertFalse(message.wasHandledBy("handleInvalidEL"));
 	}
 	
 	/*************************************************************************
@@ -113,7 +146,7 @@ public class ConditionTest extends MessageBusTest {
 		TestEvent message = new TestEvent("", 0);
 		bus.publish(message);
 
-		assertTrue(message.result == null);
+		assertTrue(message.handledBy.isEmpty());
 	}
 	
 	/*************************************************************************
@@ -127,7 +160,9 @@ public class ConditionTest extends MessageBusTest {
 		TestEvent message = new TestEvent("XYZ", 1);
 		bus.publish(message);
 
-		assertEquals("handleMethodAccessEL", message.result);
-	}
+        assertTrue(message.wasHandledBy("handleMethodAccessEL"));
+        assertFalse(message.wasHandledBy("handleInvalidEL"));
+
+    }
 
 }

src/test/java/net/engio/mbassy/common/MessageBusTest.java

@@ -29,6 +29,7 @@ public abstract class MessageBusTest extends AssertSupport {
     protected static final IPublicationErrorHandler TestFailingHandler = new IPublicationErrorHandler() {
         @Override
         public void handleError(PublicationError error) {
+            error.getCause().printStackTrace();
             Assert.fail();
         }
     };