From 7c0c0b6f825d98793525595a7be5ed43f6b99e43 Mon Sep 17 00:00:00 2001 From: Benjamin Diedrichsen Date: Thu, 22 May 2014 12:52:28 +0200 Subject: [PATCH] refactored EL resolution for conditional handlers --- .../dispatch/FilteredMessageDispatcher.java | 16 +- .../engio/mbassy/dispatch/el/ElFilter.java | 141 +++++++----------- .../mbassy/dispatch/el/EventContext.java | 102 ------------- .../mbassy/dispatch/el/RootResolver.java | 89 ----------- .../el/StandardELResolutionContext.java | 92 ++++++++++++ .../engio/mbassy/listener/MessageHandler.java | 29 +++- .../java/net/engio/mbassy/ConditionTest.java | 61 ++++++-- .../engio/mbassy/common/MessageBusTest.java | 1 + 8 files changed, 220 insertions(+), 311 deletions(-) delete mode 100644 src/main/java/net/engio/mbassy/dispatch/el/EventContext.java delete mode 100644 src/main/java/net/engio/mbassy/dispatch/el/RootResolver.java create mode 100644 src/main/java/net/engio/mbassy/dispatch/el/StandardELResolutionContext.java diff --git a/src/main/java/net/engio/mbassy/dispatch/FilteredMessageDispatcher.java b/src/main/java/net/engio/mbassy/dispatch/FilteredMessageDispatcher.java index 89efa9c..2b3b446 100644 --- a/src/main/java/net/engio/mbassy/dispatch/FilteredMessageDispatcher.java +++ b/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()); - } } diff --git a/src/main/java/net/engio/mbassy/dispatch/el/ElFilter.java b/src/main/java/net/engio/mbassy/dispatch/el/ElFilter.java index 14f9163..d15b013 100644 --- a/src/main/java/net/engio/mbassy/dispatch/el/ElFilter.java +++ b/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\"."); + } } } diff --git a/src/main/java/net/engio/mbassy/dispatch/el/EventContext.java b/src/main/java/net/engio/mbassy/dispatch/el/EventContext.java deleted file mode 100644 index 41adb2c..0000000 --- a/src/main/java/net/engio/mbassy/dispatch/el/EventContext.java +++ /dev/null @@ -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; - } - } - -} diff --git a/src/main/java/net/engio/mbassy/dispatch/el/RootResolver.java b/src/main/java/net/engio/mbassy/dispatch/el/RootResolver.java deleted file mode 100644 index c9d6fdb..0000000 --- a/src/main/java/net/engio/mbassy/dispatch/el/RootResolver.java +++ /dev/null @@ -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 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 - } - -} diff --git a/src/main/java/net/engio/mbassy/dispatch/el/StandardELResolutionContext.java b/src/main/java/net/engio/mbassy/dispatch/el/StandardELResolutionContext.java new file mode 100644 index 0000000..603e89c --- /dev/null +++ b/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; + } + } + +} diff --git a/src/main/java/net/engio/mbassy/listener/MessageHandler.java b/src/main/java/net/engio/mbassy/listener/MessageHandler.java index 4008273..2549e31 100644 --- a/src/main/java/net/engio/mbassy/listener/MessageHandler.java +++ b/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 properties = new HashMap(); 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; + } } diff --git a/src/test/java/net/engio/mbassy/ConditionTest.java b/src/test/java/net/engio/mbassy/ConditionTest.java index 333642b..07993c8 100644 --- a/src/test/java/net/engio/mbassy/ConditionTest.java +++ b/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 handledBy = new HashSet(); 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.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")); + + } } diff --git a/src/test/java/net/engio/mbassy/common/MessageBusTest.java b/src/test/java/net/engio/mbassy/common/MessageBusTest.java index 518e8db..e42ce90 100644 --- a/src/test/java/net/engio/mbassy/common/MessageBusTest.java +++ b/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(); } };