You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ch...@apache.org on 2012/10/30 16:56:31 UTC

svn commit: r1403754 - in /activemq/trunk/activemq-core/src: main/grammar/ main/java/org/apache/activemq/filter/ main/java/org/apache/activemq/transport/stomp/ test/java/org/apache/activemq/joramtests/

Author: chirino
Date: Tue Oct 30 15:56:31 2012
New Revision: 1403754

URL: http://svn.apache.org/viewvc?rev=1403754&view=rev
Log:
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.

Modified:
    activemq/trunk/activemq-core/src/main/grammar/SelectorParser.jj
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java
    activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
    activemq/trunk/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java

Modified: activemq/trunk/activemq-core/src/main/grammar/SelectorParser.jj
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/grammar/SelectorParser.jj?rev=1403754&r1=1403753&r2=1403754&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/grammar/SelectorParser.jj (original)
+++ activemq/trunk/activemq-core/src/main/grammar/SelectorParser.jj Tue Oct 30 15:56:31 2012
@@ -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();
+                }
             }
         }
     }

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java?rev=1403754&r1=1403753&r2=1403754&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/filter/ComparisonExpression.java Tue Oct 30 15:56:31 2012
@@ -30,6 +30,9 @@ import javax.jms.JMSException;
  */
 public abstract class ComparisonExpression extends BinaryExpression implements BooleanExpression {
 
+    public static final ThreadLocal<Boolean> CONVERT_STRING_EXPRESSIONS = new ThreadLocal<Boolean>();
+
+    boolean convertStringExpressions = false;
     private static final Set<Character> REGEXP_CONTROL_CHARS = new HashSet<Character>();
 
     /**
@@ -38,6 +41,7 @@ public abstract class ComparisonExpressi
      */
     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 ComparisonExpressi
         Pattern likePattern;
 
         /**
-         * @param left
          */
         public LikeExpression(Expression right, String like, int escape) {
             super(right);
@@ -358,7 +361,7 @@ public abstract class ComparisonExpressi
         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 ComparisonExpressi
                         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 ComparisonExpressi
                         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 ComparisonExpressi
                         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 ComparisonExpressi
                         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 ComparisonExpressi
                         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 ComparisonExpressi
                         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) {

Modified: activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java?rev=1403754&r1=1403753&r2=1403754&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java (original)
+++ activemq/trunk/activemq-core/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java Tue Oct 30 15:56:31 2012
@@ -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.");
 

Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java?rev=1403754&r1=1403753&r2=1403754&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/joramtests/JoramJmsTest.java Tue Oct 30 15:56:31 2012
@@ -32,6 +32,7 @@ import org.objectweb.jtests.jms.conform.
 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 TestCa
 
     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 TestCa
         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;
     }