WICKET-4008 changing PropertyResolver tokenizer to use parsed input
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/c61080e0 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/c61080e0 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/c61080e0 Branch: refs/heads/WICKET-6318-configurable-property-expression-resolver Commit: c61080e0a8687d775cee88dd24fbc1dc157c967c Parents: 697052f Author: Pedro Henrique Oliveira dos Santos <[email protected]> Authored: Fri Sep 23 00:32:08 2016 -0300 Committer: Pedro Henrique Oliveira dos Santos <[email protected]> Committed: Sun Sep 25 02:51:05 2016 -0300 ---------------------------------------------------------------------- .../core/util/lang/PropertyExpression.java | 15 +- .../util/lang/PropertyExpressionParser.java | 25 +++- .../wicket/core/util/lang/PropertyResolver.java | 136 +++++++++++++---- .../util/lang/PropertyExpressionParserTest.java | 4 +- .../wicket/util/lang/PropertyResolverTest.java | 150 +++++++++++-------- 5 files changed, 224 insertions(+), 106 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/c61080e0/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpression.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpression.java b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpression.java index dab79ec..6596651 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpression.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpression.java @@ -16,17 +16,22 @@ */ package org.apache.wicket.core.util.lang; +/** + * Abstract syntax tree of a expression property + * + * @author Pedro Santos + */ public class PropertyExpression { JavaProperty javaProperty; BeanProperty beanProperty; - CharSequence index; + String index; PropertyExpression next; static class BeanProperty { - CharSequence propertyName; - CharSequence index; + String propertyName; + String index; public BeanProperty() { @@ -79,8 +84,8 @@ public class PropertyExpression static class JavaProperty { - CharSequence javaIdentifier; - CharSequence index; + String javaIdentifier; + String index; public boolean hasMethodSign; public JavaProperty() http://git-wip-us.apache.org/repos/asf/wicket/blob/c61080e0/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpressionParser.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpressionParser.java b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpressionParser.java index 2102456..ef05599 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpressionParser.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyExpressionParser.java @@ -31,9 +31,10 @@ import org.apache.wicket.core.util.lang.PropertyExpression.JavaProperty; * char = java letter or digit | "." | "(" | ")" | "[" | "]" | "!" | "@" | "#" | (...); * index char = char - "]"; * + * empty space = { " " }; * java identifier = java letter , {java letter or digit}; * property name = java letter or digit , {java letter or digit}; - * method sign = "(" , { " " } , ")"; + * method sign = "(" , empty space , ")"; * index = "[" , index char , { index char } , "]"; * * bean property = property name, [ index ]; @@ -120,6 +121,9 @@ public class PropertyExpressionParser return expression; case END_OF_EXPRESSION : return expression; + case '(' : + throw new ParserException(format("Expecting a valid method name but got: '%s<--'", + text.substring(0, nextPosition + 1))); default : throw new ParserException(format( "Expecting a new expression but got the invalid character '%s' at: '%s<--'", @@ -157,7 +161,7 @@ public class PropertyExpressionParser return property; } - private CharSequence propertyName() + private String propertyName() { int begin = currentPosition; while (isJavaIdentifierPart(lookaheadToken)) @@ -167,7 +171,7 @@ public class PropertyExpressionParser return text.substring(begin, nextPosition); } - private CharSequence javaIdentifier() + private String javaIdentifier() { if (!isJavaIdentifierStart(currentToken)) { @@ -176,7 +180,7 @@ public class PropertyExpressionParser return propertyName(); } - private CharSequence index() + private String index() { advance();// escape bracket if (currentToken == ']') @@ -196,10 +200,7 @@ public class PropertyExpressionParser private boolean methodSign() { - while (lookaheadToken == ' ') - { - advance();// skips empty spaces - } + emptySpace(); if (lookaheadToken != ')') { throw new ParserException(format("The expression can't have method parameters: '%s<--'", @@ -208,4 +209,12 @@ public class PropertyExpressionParser advance();// skips right bracket return true; } + + private void emptySpace() + { + while (lookaheadToken == ' ') + { + advance();// skips empty spaces + } + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/c61080e0/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyResolver.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyResolver.java b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyResolver.java index ba5c48b..9b1146c 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyResolver.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/lang/PropertyResolver.java @@ -20,6 +20,8 @@ import java.lang.reflect.Array; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -27,7 +29,6 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.wicket.Application; import org.apache.wicket.Session; import org.apache.wicket.WicketRuntimeException; -import org.apache.wicket.core.util.lang.PropertyResolverConverter; import org.apache.wicket.util.convert.ConversionException; import org.apache.wicket.util.lang.Generics; import org.apache.wicket.util.string.Strings; @@ -262,6 +263,84 @@ public final class PropertyResolver } /** + * <b>Temporary</b> tokenizer that can fallback to old tokenization strategy and that should be + * removed in benefit of the abstract syntax tree generated by the property expression parser + * once it is introduced + */ + private static class PropertyResolverTokenizer + { + public static List<String> tokenize(String expression){ + PropertyExpression astNode = new PropertyExpressionParser().parse(expression); + List<String> tokens = new ArrayList<String>(); + do + { + if (astNode.beanProperty != null) + { + tokens.add(astNode.beanProperty.propertyName); + if (astNode.beanProperty.index != null) + { + tokens.add("[" + astNode.beanProperty.index + "]"); + } + } + else if (astNode.javaProperty != null) + { + if (astNode.javaProperty.index != null) + { + tokens.add(astNode.javaProperty.javaIdentifier); + tokens.add("[" + astNode.javaProperty.index + "]"); + } + else if (astNode.javaProperty.hasMethodSign) + { + tokens.add(astNode.javaProperty.javaIdentifier + "()"); + } + else + { + tokens.add(astNode.javaProperty.javaIdentifier); + } + } + else + { + tokens.add("[" + astNode.index + "]"); + } + } + while ((astNode = astNode.next) != null); + return tokens; + } + + public static List<String> legacyTokenize(String expression) + { + List<String> tokens = new ArrayList<String>(); + String expressionBracketsSeperated = Strings.replaceAll(expression, "[", ".[").toString(); + + int index = getNextDotIndex(expressionBracketsSeperated, 0); + while (index == 0 && expressionBracketsSeperated.startsWith(".")) + { + // eat dots at the beginning of the expression since they will confuse + // later steps + expressionBracketsSeperated = expressionBracketsSeperated.substring(1); + index = getNextDotIndex(expressionBracketsSeperated, 0); + } + int lastIndex = 0; + String exp = expressionBracketsSeperated; + while (index != -1) + { + exp = expressionBracketsSeperated.substring(lastIndex, index); + tokens.add(exp); + lastIndex = index + 1; + index = getNextDotIndex(expressionBracketsSeperated, lastIndex); + if (index == -1) + { + exp = expressionBracketsSeperated.substring(lastIndex); + break; + } + } + tokens.add(exp); + return tokens; + + } + } + + /** * Receives the class parameter also, since this method can resolve the type for some * expression, only knowing the target class. * @@ -273,24 +352,30 @@ public final class PropertyResolver */ private static ObjectWithGetAndSet getObjectWithGetAndSet(final String expression, final Object object, final int tryToCreateNull, Class<?> clz) { - String expressionBracketsSeperated = Strings.replaceAll(expression, "[", ".[").toString(); - int index = getNextDotIndex(expressionBracketsSeperated, 0); - while (index == 0 && expressionBracketsSeperated.startsWith(".")) + List<String> tokens = null; + String userOldTokenizer = System.getProperty("wicket.use_old_tokenizer"); + if ("S".equals(userOldTokenizer)) { - // eat dots at the beginning of the expression since they will confuse - // later steps - expressionBracketsSeperated = expressionBracketsSeperated.substring(1); - index = getNextDotIndex(expressionBracketsSeperated, 0); + tokens = PropertyResolverTokenizer.legacyTokenize(expression); } - int lastIndex = 0; + else + { + tokens = PropertyResolverTokenizer.tokenize(expression); + } + Object value = object; - String exp = expressionBracketsSeperated; - while (index != -1) + String exp = null; + Iterator<String> iterator = tokens.iterator(); + while (iterator.hasNext()) { - exp = expressionBracketsSeperated.substring(lastIndex, index); + exp = iterator.next(); + if (!iterator.hasNext()) + { + break; + } + if (exp.length() == 0) { - exp = expressionBracketsSeperated.substring(index + 1); break; } @@ -303,14 +388,17 @@ public final class PropertyResolver { // expression by itself can't be found. try combined with the following // expression (e.g. for a indexed property); - int temp = getNextDotIndex(expressionBracketsSeperated, index + 1); - if (temp == -1) + if (iterator.hasNext()) { - exp = expressionBracketsSeperated.substring(lastIndex); - break; - } else { - index = temp; - continue; + exp = exp + "." + iterator.next(); + if (!iterator.hasNext()) + { + break; + } + else + { + getAndSet = getGetAndSet(exp, clz); + } } } Object nextValue = null; @@ -343,14 +431,6 @@ public final class PropertyResolver // value can be null if we are in the RESOLVE_CLASS clz = value.getClass(); } - - lastIndex = index + 1; - index = getNextDotIndex(expressionBracketsSeperated, lastIndex); - if (index == -1) - { - exp = expressionBracketsSeperated.substring(lastIndex); - break; - } } IGetAndSet getAndSet = getGetAndSet(exp, clz); return new ObjectWithGetAndSet(getAndSet, value); http://git-wip-us.apache.org/repos/asf/wicket/blob/c61080e0/wicket-core/src/test/java/org/apache/wicket/core/util/lang/PropertyExpressionParserTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/core/util/lang/PropertyExpressionParserTest.java b/wicket-core/src/test/java/org/apache/wicket/core/util/lang/PropertyExpressionParserTest.java index ac5ed26..f49d6b5 100644 --- a/wicket-core/src/test/java/org/apache/wicket/core/util/lang/PropertyExpressionParserTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/core/util/lang/PropertyExpressionParserTest.java @@ -181,8 +181,8 @@ public class PropertyExpressionParserTest public void shouldFailParseMethodsStartingWithInvalidCharacter() { expectedException.expect(ParserException.class); - // expectedException.expectMessage( - // "The expression can't have method parameters: 'repository.getPerson(<--'"); + expectedException + .expectMessage("Expecting a valid method name but got: 'repository.0method(<--'"); parser.parse("repository.0method()"); } http://git-wip-us.apache.org/repos/asf/wicket/blob/c61080e0/wicket-core/src/test/java/org/apache/wicket/util/lang/PropertyResolverTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/util/lang/PropertyResolverTest.java b/wicket-core/src/test/java/org/apache/wicket/util/lang/PropertyResolverTest.java index 5929eef..d195a8d 100644 --- a/wicket-core/src/test/java/org/apache/wicket/util/lang/PropertyResolverTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/util/lang/PropertyResolverTest.java @@ -29,9 +29,12 @@ import java.util.Locale; import java.util.Map; import java.util.Vector; +import javax.xml.bind.PropertyException; + import org.apache.wicket.ConverterLocator; import org.apache.wicket.IConverterLocator; import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.core.util.lang.PropertyExpression; import org.apache.wicket.core.util.lang.PropertyResolver; import org.apache.wicket.core.util.lang.PropertyResolver.AbstractGetAndSet; import org.apache.wicket.core.util.lang.PropertyResolver.CachingPropertyLocator; @@ -227,69 +230,6 @@ public class PropertyResolverTest extends WicketTestCase assertEquals(street, "wicket-street"); } - - static class WeirdList extends ArrayList<Integer> - { - private static final long serialVersionUID = 1L; - private Integer integer; - - public void set0(Integer integer) - { - this.integer = integer; - - } - - public Integer get0() - { - return integer; - } - } - - @Test - public void shouldAllowMapKeysWithSpecialCharacters() throws Exception - { - String expression = "[!@#$%^&*()_+-=[{}|]"; - PropertyResolver.setValue(expression, integerMap, AN_INTEGER, CONVERTER); - assertThat(PropertyResolver.getValue(expression, integerMap), is(AN_INTEGER)); - assertThat(integerMap.get(expression), is(AN_INTEGER)); - } - - @Test - public void shouldAllowMapKeysHavingQuotes() throws Exception - { - String expression = "the\"key\""; - PropertyResolver.setValue(expression, integerMap, AN_INTEGER, CONVERTER); - assertThat(PropertyResolver.getValue(expression, integerMap), is(AN_INTEGER)); - assertThat(integerMap.get(expression), is(AN_INTEGER)); - } - - @Test - public void shouldPriorityzeListIndex() throws Exception - { - integerList.set0(AN_INTEGER); - assertThat(PropertyResolver.getValue("integerList.0", this), is(AN_INTEGER)); - } - - @Test - public void shouldPriorityzeMapKeyInSquareBrakets() throws Exception - { - PropertyResolver.setValue("[class]", integerMap, AN_INTEGER, CONVERTER); - assertThat(PropertyResolver.getValue("[class]", integerMap), is(AN_INTEGER)); - } - - @Test - public void shouldPriorityzeMapKeyInSquareBraketsAfterAnExpresison() throws Exception - { - PropertyResolver.setValue("integerMap[class]", this, AN_INTEGER, CONVERTER); - assertThat(PropertyResolver.getValue("integerMap[class]", this), is(AN_INTEGER)); - } - - @Test - public void shouldPriorityzeMethodCallWhenEndedByParentises() throws Exception - { - assertThat(PropertyResolver.getValue("integerMap.getClass()", this), is(HashMap.class)); - } - /** * @throws Exception */ @@ -838,6 +778,72 @@ public class PropertyResolverTest extends WicketTestCase assertEquals("string2", PropertyResolver.getValue("nested.string", document)); } + + // EDGE CASES + @Test + public void shouldAllowEmptySpacesInsideMethodCallBrackets() throws Exception + { + person.setName("bob"); + assertThat("bob", is(PropertyResolver.getValue("person.getName( )", this))); + } + + @Test + public void shouldAllowMapKeysWithSpecialCharactersIncludingOpenSquareBracket() throws Exception + { + String code = "!@#$%^&*()_+-=[{}|"; + String expression = "[" + code + "]"; + PropertyResolver.setValue(expression, integerMap, AN_INTEGER, CONVERTER); + assertThat(PropertyResolver.getValue(expression, integerMap), is(AN_INTEGER)); + assertThat(integerMap.get(code), is(AN_INTEGER)); + } + + @Test + public void shouldAllowMapKeysWithDot() throws Exception + { + String code = "code-1.0"; + String expression = "[" + code + "]"; + PropertyResolver.setValue(expression, integerMap, AN_INTEGER, CONVERTER); + assertThat(PropertyResolver.getValue(expression, integerMap), is(AN_INTEGER)); + assertThat(integerMap.get(code), is(AN_INTEGER)); + } + + @Test + public void shouldAllowMapKeysHavingQuotes() throws Exception + { + String code = "the\"key\""; + String expression = "[" + code + "]"; + PropertyResolver.setValue(expression, integerMap, AN_INTEGER, CONVERTER); + assertThat(PropertyResolver.getValue(expression, integerMap), is(AN_INTEGER)); + assertThat(integerMap.get(code), is(AN_INTEGER)); + } + + @Test + public void shouldPriorityzeListIndex() throws Exception + { + integerList.set0(AN_INTEGER); + assertThat(PropertyResolver.getValue("integerList.0", this), is(AN_INTEGER)); + } + + @Test + public void shouldPriorityzeMapKeyInSquareBrakets() throws Exception + { + PropertyResolver.setValue("[class]", integerMap, AN_INTEGER, CONVERTER); + assertThat(PropertyResolver.getValue("[class]", integerMap), is(AN_INTEGER)); + } + + @Test + public void shouldPriorityzeMapKeyInSquareBraketsAfterAnExpresison() throws Exception + { + PropertyResolver.setValue("integerMap[class]", this, AN_INTEGER, CONVERTER); + assertThat(PropertyResolver.getValue("integerMap[class]", this), is(AN_INTEGER)); + } + + @Test + public void shouldPriorityzeMethodCallWhenEndedByParentises() throws Exception + { + assertThat(PropertyResolver.getValue("integerMap.getClass()", this), is(HashMap.class)); + } + class CustomGetAndSetLocator implements IPropertyLocator { @@ -885,4 +891,22 @@ public class PropertyResolverTest extends WicketTestCase } } } + + + static class WeirdList extends ArrayList<Integer> + { + private static final long serialVersionUID = 1L; + private Integer integer; + + public void set0(Integer integer) + { + this.integer = integer; + + } + + public Integer get0() + { + return integer; + } + } } \ No newline at end of file
