From 2f258702f1ebbb8e64ec0fabee4f150394d7fd90 Mon Sep 17 00:00:00 2001 From: "Hiram R. Chirino" Date: Tue, 30 Oct 2012 15:56:31 +0000 Subject: [PATCH] Fixes AMQ-4146: String properties in JMS selector expression should not get auto converted to numbers per spec. If you really want to auto convert string property values to numerics, prefix the selector with 'convert_string_expressions:'. This change makes the last failing Joram test case pass. git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@1403754 13f79535-47bb-0310-9956-ffa450edef68 --- .../src/main/grammar/SelectorParser.jj | 15 +++++++++++++ .../activemq/filter/ComparisonExpression.java | 21 +++++++++++-------- .../transport/stomp/ProtocolConverter.java | 4 +++- .../activemq/joramtests/JoramJmsTest.java | 4 ++-- 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/activemq-core/src/main/grammar/SelectorParser.jj b/activemq-core/src/main/grammar/SelectorParser.jj index 176ec235e6..052d815625 100755 --- a/activemq-core/src/main/grammar/SelectorParser.jj +++ b/activemq-core/src/main/grammar/SelectorParser.jj @@ -67,6 +67,7 @@ import org.apache.activemq.util.LRUCache; public class SelectorParser { private static final Map cache = Collections.synchronizedMap(new LRUCache(100)); + private static final String CONVERT_STRING_EXPRESSIONS_PREFIX = "convert_string_expressions:"; public static BooleanExpression parse(String sql) throws InvalidSelectorException { Object result = cache.get(sql); @@ -75,6 +76,16 @@ public class SelectorParser { } else if (result instanceof BooleanExpression) { return (BooleanExpression) result; } else { + + boolean convertStringExpressions = false; + if( sql.startsWith(CONVERT_STRING_EXPRESSIONS_PREFIX)) { + convertStringExpressions = true; + sql = sql.substring(CONVERT_STRING_EXPRESSIONS_PREFIX.length()); + } + + if( convertStringExpressions ) { + ComparisonExpression.CONVERT_STRING_EXPRESSIONS.set(true); + } try { BooleanExpression e = new SelectorParser(sql).parse(); cache.put(sql, e); @@ -82,6 +93,10 @@ public class SelectorParser { } catch (InvalidSelectorException t) { cache.put(sql, t); throw t; + } finally { + if( convertStringExpressions ) { + ComparisonExpression.CONVERT_STRING_EXPRESSIONS.remove(); + } } } } diff --git a/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java b/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java index 1269fbd883..0f77399c8a 100755 --- a/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java +++ b/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java @@ -30,6 +30,9 @@ import javax.jms.JMSException; */ public abstract class ComparisonExpression extends BinaryExpression implements BooleanExpression { + public static final ThreadLocal CONVERT_STRING_EXPRESSIONS = new ThreadLocal(); + + boolean convertStringExpressions = false; private static final Set REGEXP_CONTROL_CHARS = new HashSet(); /** @@ -38,6 +41,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B */ public ComparisonExpression(Expression left, Expression right) { super(left, right); + convertStringExpressions = CONVERT_STRING_EXPRESSIONS.get()!=null; } public static BooleanExpression createBetween(Expression value, Expression left, Expression right) { @@ -76,7 +80,6 @@ public abstract class ComparisonExpression extends BinaryExpression implements B Pattern likePattern; /** - * @param left */ public LikeExpression(Expression right, String like, int escape) { super(right); @@ -358,7 +361,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B if (lc != rc) { try { if (lc == Boolean.class) { - if (rc == String.class) { + if (convertStringExpressions && rc == String.class) { lv = Boolean.valueOf((String)lv).booleanValue(); } else { return Boolean.FALSE; @@ -374,7 +377,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B lv = new Float(((Number)lv).floatValue()); } else if (rc == Double.class) { lv = new Double(((Number)lv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Byte.valueOf((String)rv); } else { return Boolean.FALSE; @@ -388,7 +391,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B lv = new Float(((Number)lv).floatValue()); } else if (rc == Double.class) { lv = new Double(((Number)lv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Short.valueOf((String)rv); } else { return Boolean.FALSE; @@ -400,7 +403,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B lv = new Float(((Number)lv).floatValue()); } else if (rc == Double.class) { lv = new Double(((Number)lv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Integer.valueOf((String)rv); } else { return Boolean.FALSE; @@ -412,7 +415,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B lv = new Float(((Number)lv).floatValue()); } else if (rc == Double.class) { lv = new Double(((Number)lv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Long.valueOf((String)rv); } else { return Boolean.FALSE; @@ -424,7 +427,7 @@ public abstract class ComparisonExpression extends BinaryExpression implements B rv = new Float(((Number)rv).floatValue()); } else if (rc == Double.class) { lv = new Double(((Number)lv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Float.valueOf((String)rv); } else { return Boolean.FALSE; @@ -436,12 +439,12 @@ public abstract class ComparisonExpression extends BinaryExpression implements B rv = new Double(((Number)rv).doubleValue()); } else if (rc == Float.class) { rv = new Float(((Number)rv).doubleValue()); - } else if (rc == String.class) { + } else if (convertStringExpressions && rc == String.class) { rv = Double.valueOf((String)rv); } else { return Boolean.FALSE; } - } else if (lc == String.class) { + } else if (convertStringExpressions && lc == String.class) { if (rc == Boolean.class) { lv = Boolean.valueOf((String)lv); } else if (rc == Byte.class) { diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java index 629f52e8b6..741ca1c9d1 100644 --- a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java @@ -505,7 +505,9 @@ public class ProtocolConverter { } String selector = headers.remove(Stomp.Headers.Subscribe.SELECTOR); - consumerInfo.setSelector(selector); + if( selector!=null ) { + consumerInfo.setSelector("convert_string_expressions:"+selector); + } IntrospectionSupport.setProperties(consumerInfo, headers, "activemq."); diff --git a/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java b/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java index 08d30f44f2..00c5423a72 100644 --- a/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java +++ b/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java @@ -32,6 +32,7 @@ import org.objectweb.jtests.jms.conform.message.properties.MessagePropertyTest; import org.objectweb.jtests.jms.conform.queue.QueueBrowserTest; import org.objectweb.jtests.jms.conform.queue.TemporaryQueueTest; import org.objectweb.jtests.jms.conform.selector.SelectorSyntaxTest; +import org.objectweb.jtests.jms.conform.selector.SelectorTest; import org.objectweb.jtests.jms.conform.session.QueueSessionTest; import org.objectweb.jtests.jms.conform.session.SessionTest; import org.objectweb.jtests.jms.conform.session.TopicSessionTest; @@ -45,6 +46,7 @@ public class JoramJmsTest extends TestCase { public static Test suite() { TestSuite suite = new TestSuite(); + suite.addTestSuite(SelectorTest.class); suite.addTestSuite(ConnectionTest.class); suite.addTestSuite(TopicConnectionTest.class); suite.addTestSuite(MessageHeaderTest.class); @@ -62,8 +64,6 @@ public class JoramJmsTest extends TestCase { suite.addTestSuite(UnifiedSessionTest.class); suite.addTestSuite(QueueBrowserTest.class); suite.addTestSuite(MessagePropertyTest.class); -// TODO: figure out why the following tests are failing.. -// suite.addTestSuite(SelectorTest.class); return suite; }