This is an automated email from the ASF dual-hosted git repository.

davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git


The following commit(s) were added to refs/heads/main by this push:
     new 3830307  CAMEL-17073: Fixed simple language caching bug.
3830307 is described below

commit 3830307a412be6bbcc5032b4c03ff3985ace738c
Author: Claus Ibsen <[email protected]>
AuthorDate: Wed Oct 13 18:54:15 2021 +0200

    CAMEL-17073: Fixed simple language caching bug.
---
 .../camel/language/simple/SimpleLanguage.java      | 60 ++++++----------------
 .../camel/language/simple/SimpleTokenizer.java     |  8 +--
 .../language/simple/ast/SimpleFunctionStart.java   |  4 +-
 .../language/simple/SimpleCacheExpressionTest.java | 26 ++++++++++
 4 files changed, 50 insertions(+), 48 deletions(-)

diff --git 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleLanguage.java
 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleLanguage.java
index 6f910d0..ebdd23b 100644
--- 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleLanguage.java
+++ 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleLanguage.java
@@ -45,6 +45,9 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
     // singleton for expressions without a result type
     private static final SimpleLanguage SIMPLE = new SimpleLanguage();
 
+    // a special prefix to avoid cache clash
+    private static final String CACHE_KEY_PREFIX = "@SIMPLE@";
+
     boolean allowEscape = true;
 
     // use caches to avoid re-parsing the same expressions over and over again
@@ -103,7 +106,8 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
     public Predicate createPredicate(String expression) {
         ObjectHelper.notNull(expression, "expression");
 
-        Predicate answer = cachePredicate != null ? 
cachePredicate.get(expression) : null;
+        String key = CACHE_KEY_PREFIX + expression;
+        Predicate answer = cachePredicate != null ? cachePredicate.get(key) : 
null;
         if (answer == null) {
 
             if (isDynamicResource(expression)) {
@@ -129,14 +133,16 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
 
             if (isStaticResource(expression)) {
                 expression = loadResource(expression);
+                key = CACHE_KEY_PREFIX + expression;
             }
 
+            // using the expression cache here with the predicate parser is 
okay
             SimplePredicateParser parser
                     = new SimplePredicateParser(getCamelContext(), expression, 
allowEscape, cacheExpression);
             answer = parser.parsePredicate();
 
             if (cachePredicate != null && answer != null) {
-                cachePredicate.put(expression, answer);
+                cachePredicate.put(key, answer);
             }
         }
 
@@ -166,13 +172,8 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
     public Expression createExpression(String expression) {
         ObjectHelper.notNull(expression, "expression");
 
-        Expression answer = null;
-
-        // only lookup in cache if there are functions or special escape tokens
-        boolean function = hasSimpleFunction(expression) || 
hasEscapeToken(expression);
-        if (function && cacheExpression != null) {
-            answer = cacheExpression.get(expression);
-        }
+        String key = CACHE_KEY_PREFIX + expression;
+        Expression answer = cacheExpression != null ? cacheExpression.get(key) 
: null;
 
         if (answer == null) {
             if (isDynamicResource(expression)) {
@@ -198,24 +199,17 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
             if (isStaticResource(expression)) {
                 // load static resource and re-eval if there are functions
                 expression = loadResource(expression);
-                function = hasSimpleFunction(expression) || 
hasEscapeToken(expression);
+                key = CACHE_KEY_PREFIX + expression;
             }
 
             // only parse if there are simple functions
-            if (function) {
-                SimpleExpressionParser parser
-                        = new SimpleExpressionParser(getCamelContext(), 
expression, allowEscape, cacheExpression);
-                answer = parser.parseExpression();
-
-                if (cacheExpression != null && answer != null) {
-                    cacheExpression.put(expression, answer);
-                }
-            }
-        }
+            SimpleExpressionParser parser
+                    = new SimpleExpressionParser(getCamelContext(), 
expression, allowEscape, cacheExpression);
+            answer = parser.parseExpression();
 
-        if (answer == null) {
-            // it has no functions so its static text
-            answer = ExpressionBuilder.constantExpression(expression);
+            if (cacheExpression != null && answer != null) {
+                cacheExpression.put(key, answer);
+            }
         }
 
         return answer;
@@ -274,24 +268,4 @@ public class SimpleLanguage extends LanguageSupport 
implements StaticService {
         return SIMPLE.createPredicate(predicate);
     }
 
-    /**
-     * Does the expression include a simple function.
-     *
-     * @param  expression the expression
-     * @return            <tt>true</tt> if one or more simple function is 
included in the expression
-     */
-    public static boolean hasSimpleFunction(String expression) {
-        return SimpleTokenizer.hasFunctionStartToken(expression);
-    }
-
-    /**
-     * Does the expression include an escape tokens.
-     *
-     * @param  expression the expression
-     * @return            <tt>true</tt> if one or more escape tokens is 
included in the expression
-     */
-    public static boolean hasEscapeToken(String expression) {
-        return SimpleTokenizer.hasEscapeToken(expression);
-    }
-
 }
diff --git 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleTokenizer.java
 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleTokenizer.java
index 8900de0..0336313 100644
--- 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleTokenizer.java
+++ 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/SimpleTokenizer.java
@@ -33,12 +33,14 @@ public final class SimpleTokenizer {
 
     // optimise to be able to quick check for start functions
     private static final String[] FUNCTION_START = new String[] { "${", 
"$simple{" };
+    // optimise to be able to quick check for end function
+    private static final String FUNCTION_END = "}";
 
     static {
         // add known tokens
-        KNOWN_TOKENS[0] = new SimpleTokenType(TokenType.functionStart, "${");
-        KNOWN_TOKENS[1] = new SimpleTokenType(TokenType.functionStart, 
"$simple{");
-        KNOWN_TOKENS[2] = new SimpleTokenType(TokenType.functionEnd, "}");
+        KNOWN_TOKENS[0] = new SimpleTokenType(TokenType.functionStart, 
FUNCTION_START[0]);
+        KNOWN_TOKENS[1] = new SimpleTokenType(TokenType.functionStart, 
FUNCTION_START[1]);
+        KNOWN_TOKENS[2] = new SimpleTokenType(TokenType.functionEnd, 
FUNCTION_END);
         KNOWN_TOKENS[3] = new SimpleTokenType(TokenType.whiteSpace, " ");
         KNOWN_TOKENS[4] = new SimpleTokenType(TokenType.whiteSpace, "\t");
         KNOWN_TOKENS[5] = new SimpleTokenType(TokenType.whiteSpace, "\n");
diff --git 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/SimpleFunctionStart.java
 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/SimpleFunctionStart.java
index 3e14ac7..ddd8f9e 100644
--- 
a/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/SimpleFunctionStart.java
+++ 
b/core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/SimpleFunctionStart.java
@@ -53,7 +53,7 @@ public class SimpleFunctionStart extends BaseSimpleNode 
implements BlockStart {
 
     @Override
     public String toString() {
-        // output a nice toString so it makes debugging easier as we can see 
the entire block
+        // output a nice toString, so it makes debugging easier, so we can see 
the entire block
         return "${" + block + "}";
     }
 
@@ -171,7 +171,7 @@ public class SimpleFunctionStart extends BaseSimpleNode 
implements BlockStart {
         StringBuilder sb = new StringBuilder();
         boolean quoteEmbeddedFunctions = false;
 
-        // we need to concat the block so we have the expression
+        // we need to concat the block, so we have the expression
         for (SimpleNode child : block.getChildren()) {
             if (child instanceof LiteralNode) {
                 String text = ((LiteralNode) child).getText();
diff --git 
a/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleCacheExpressionTest.java
 
b/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleCacheExpressionTest.java
index 271eeae..faf0fa6 100644
--- 
a/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleCacheExpressionTest.java
+++ 
b/core/camel-core/src/test/java/org/apache/camel/language/simple/SimpleCacheExpressionTest.java
@@ -41,4 +41,30 @@ public class SimpleCacheExpressionTest extends 
LanguageTestSupport {
         assertExpression(exchange, "${header.foo}", 123);
         assertExpression(exchange, "header.foo", "header.foo");
     }
+
+    @Test
+    public void testCachingWithNestedFunction() throws Exception {
+        MyConverter converter = new MyConverter();
+        exchange.getIn().setBody(converter);
+        exchange.getIn().setHeader("input", "foo");
+
+        assertExpression(exchange, "${body.upper(${header.input})}", "FOO");
+        assertExpression(exchange, "body.upper(${header.input})", 
"body.upper(foo)");
+    }
+
+    @Test
+    public void testReversedCachingWithNestedFunction() throws Exception {
+        MyConverter converter = new MyConverter();
+        exchange.getIn().setBody(converter);
+        exchange.getIn().setHeader("input", "foo");
+
+        assertExpression(exchange, "body.upper(${header.input})", 
"body.upper(foo)");
+        assertExpression(exchange, "${body.upper(${header.input})}", "FOO");
+    }
+
+    public static class MyConverter {
+        public String upper(String input) throws Exception {
+            return input.toUpperCase();
+        }
+    }
 }

Reply via email to