WICKET-4008 edge test cases plus breaking property into different types in the grammar
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/b0f7da85 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/b0f7da85 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/b0f7da85 Branch: refs/heads/WICKET-6318-configurable-property-expression-resolver Commit: b0f7da85e45ea4c798366940ec34cd53528cd549 Parents: 2f302db Author: Pedro Henrique Oliveira dos Santos <pe...@apache.org> Authored: Tue Sep 20 18:51:37 2016 -0300 Committer: Pedro Henrique Oliveira dos Santos <pe...@apache.org> Committed: Tue Sep 20 19:33:01 2016 -0300 ---------------------------------------------------------------------- .../core/util/lang/PropertyExpression.java | 1 + .../util/lang/PropertyExpressionParser.java | 60 ++++---- .../util/lang/PropertyExpressionParserTest.java | 138 ++++++++++++------- .../wicket/util/lang/PropertyResolverTest.java | 113 ++++++++++++--- 4 files changed, 222 insertions(+), 90 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/b0f7da85/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 2021c21..635f95e 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 @@ -19,6 +19,7 @@ package org.apache.wicket.core.util.lang; public class PropertyExpression { Property property; + CharSequence index; PropertyExpression next; static class Property http://git-wip-us.apache.org/repos/asf/wicket/blob/b0f7da85/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 08fd061..3ecb81e 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,11 +31,14 @@ import org.apache.wicket.core.util.lang.PropertyExpression.Property; * index char = char - "]" * * java identifier = java letter , {java letter or digit} - * method sign = "(" , ")" + * property name = java letter or digit , {java letter or digit} + * method sign = "(" , { " " } , ")" * index = "[" , index char , {index char} , "]" ; * - * property = java identifier , [ index | method sign ]; - * property expression = property , { "." , property expression } ; + * bean property = property name, [ index ] + * java property = java identifier , [ index | method sign ] + * map property = index + * property expression = [ bean property | java property | map property ], { "." , property expression } ; * * </code> * @@ -55,12 +58,17 @@ public class PropertyExpressionParser { currentPosition = nextPosition; currentToken = lookaheadToken; - nextPosition += 1; if (nextPosition >= text.length()) + { + lookaheadToken = END_OF_EXPRESSION; + } else + { + lookaheadToken = text.charAt(nextPosition); + } return currentToken; } @@ -71,7 +79,8 @@ public class PropertyExpressionParser { throw new ParserException("No expression was given to be parsed."); } - else if (text.length() == 1) + currentToken = text.charAt(0); + if (text.length() == 1) { lookaheadToken = END_OF_EXPRESSION; } @@ -79,7 +88,6 @@ public class PropertyExpressionParser { lookaheadToken = text.charAt(1); } - currentToken = text.charAt(0); return expression(); } @@ -87,7 +95,14 @@ public class PropertyExpressionParser private PropertyExpression expression() { PropertyExpression expression = new PropertyExpression(); - expression.property = property(); + if (currentToken == '[') + { + expression.index = index(); + } + else + { + expression.property = property(); + } switch (lookaheadToken) { case '.' : @@ -98,33 +113,27 @@ public class PropertyExpressionParser case END_OF_EXPRESSION : return expression; default : - throw new ParserException("expecting a new expression but got: '" + currentToken + "'"); + throw new ParserException( + "Expecting a new expression but got: '" + lookaheadToken + "'"); } } private Property property() { Property property = new Property(); - if (currentToken == '[') - { - property.index = index(); - return property; - } - else + property.javaIdentifier = javaIdentifier(); + switch (lookaheadToken) { - property.javaIdentifier = javaIdentifier(); - if (lookaheadToken == '[') - { + case '[' : advance();// skips left bracket property.index = index(); - } - else if (lookaheadToken == '(') - { + break; + case '(' : advance(); // skips left bracket property.hasMethodSign = methodSign(); - } - return property; + break; } + return property; } @@ -162,9 +171,14 @@ public class PropertyExpressionParser private boolean methodSign() { + while (lookaheadToken == ' ') + { + advance();// skips empty spaces + } if (lookaheadToken != ')') { - throw new ParserException("expecting a method sign but got: '" + currentToken + "'"); + throw new ParserException(format("The expression can't have method parameters: '%s<--'", + text.substring(0, nextPosition))); } advance();// skips right bracket return true; http://git-wip-us.apache.org/repos/asf/wicket/blob/b0f7da85/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 d6c2803..104a01d 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 @@ -5,7 +5,6 @@ import static org.junit.Assert.assertThat; import org.apache.wicket.core.util.lang.PropertyExpression.Property; import org.hamcrest.CoreMatchers; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -16,104 +15,115 @@ public class PropertyExpressionParserTest @Rule public ExpectedException expectedException = ExpectedException.none(); private PropertyExpressionParser parser = new PropertyExpressionParser(); - + @Test public void shouldParsePropertyExpressions() { - PropertyExpression parse = parser.parse("a"); - assertThat(parse.property, is(new Property("a", null, false))); - assertThat(parse.next, CoreMatchers.nullValue()); + PropertyExpression expression = parser.parse("person"); + assertThat(expression.index, CoreMatchers.nullValue()); + assertThat(expression.property, is(new Property("person", null, false))); + assertThat(expression.next, CoreMatchers.nullValue()); } @Test - public void shouldParseShortPropertyExpressions() + public void shouldParsePropertyExpressionStartingWithDigits() { - PropertyExpression parse = parser.parse("person"); - assertThat(parse.property, is(new Property("person", null, false))); - assertThat(parse.next, CoreMatchers.nullValue()); + PropertyExpression expression = parser.parse("1person"); + assertThat(expression.index, CoreMatchers.nullValue()); + assertThat(expression.property, is(new Property("person", null, false))); + assertThat(expression.next, CoreMatchers.nullValue()); } - - // TODO mgrigorov: @pedrosans: I'd expect the error message to complain about the space, not 'r' + @Test - public void shouldFailParsePropertyExpressionsWithSpace() + public void shouldParseShortPropertyExpressions() { - expectedException.expect(ParserException.class); - expectedException - .expectMessage("expecting a new expression but got: ' '"); - parser.parse("per son"); + PropertyExpression expression = parser.parse("a"); + assertThat(expression.index, CoreMatchers.nullValue()); + assertThat(expression.property, is(new Property("a", null, false))); + assertThat(expression.next, CoreMatchers.nullValue()); } - // TODO mgrigorov: @pedrosans: IMO this should pass. Or otherwise should complain about the space, not '(' @Test - public void shouldParsePropertyExpressionsWithSpaceInMethod() + public void shouldParseIndexedPropertyExpressions() { - final PropertyExpression parse = parser.parse("person( )"); - assertThat(parse.property, is(new Property("person", null, true))); + PropertyExpression expression = parser.parse("person[age]"); + assertThat(expression.index, CoreMatchers.nullValue()); + assertThat(expression.property, is(new Property("person", "age", false))); + assertThat(expression.next, CoreMatchers.nullValue()); } @Test - public void shouldParseIndexedPropertyExpressions() + public void shouldParseMethodExpressions() { - PropertyExpression parse = parser.parse("person[age]"); - assertThat(parse.property, is(new Property("person", "age", false))); - assertThat(parse.next, CoreMatchers.nullValue()); + PropertyExpression expression = parser.parse("person()"); + assertThat(expression.index, CoreMatchers.nullValue()); + assertThat(expression.property, is(new Property("person", null, true))); + assertThat(expression.next, CoreMatchers.nullValue()); } @Test - public void shouldParseMethodExpressions() + public void shouldParsePropertyExpressionsWithSpaceInMethod() { - PropertyExpression parse = parser.parse("person()"); + final PropertyExpression parse = parser.parse("person( )"); assertThat(parse.property, is(new Property("person", null, true))); - assertThat(parse.next, CoreMatchers.nullValue()); } @Test public void shouldParseIndexExpressions() { - PropertyExpression parse = parser.parse("[person#name]"); - assertThat(parse.property, is(new Property(null, "person#name", false))); - assertThat(parse.next, CoreMatchers.nullValue()); + PropertyExpression expression = parser.parse("[person#name]"); + assertThat(expression.index, is("person#name")); + assertThat(expression.property, CoreMatchers.nullValue()); + assertThat(expression.next, CoreMatchers.nullValue()); } @Test public void shouldParseChainedPropertyExpressions() { - PropertyExpression parse = parser.parse("person.child"); - assertThat(parse.property, is(new Property("person", null, false))); - assertThat(parse.next.property, is(new Property("child", null, false))); + PropertyExpression expression = parser.parse("person.child"); + assertThat(expression.property, is(new Property("person", null, false))); + assertThat(expression.next.property, is(new Property("child", null, false))); } @Test public void shouldParseShortChainedPropertyExpressions() { - PropertyExpression parse = parser.parse("a.b"); - assertThat(parse.property, is(new Property("a", null, false))); - assertThat(parse.next.property, is(new Property("b", null, false))); + PropertyExpression expression = parser.parse("a.b"); + assertThat(expression.property, is(new Property("a", null, false))); + assertThat(expression.next.property, is(new Property("b", null, false))); } @Test public void shouldParseChainedIndexedPropertyExpressions() { - PropertyExpression parse = parser.parse("person[1].child"); - assertThat(parse.property, is(new Property("person", "1", false))); - assertThat(parse.next.property, is(new Property("child", null, false))); + PropertyExpression expression = parser.parse("person[1].child"); + assertThat(expression.property, is(new Property("person", "1", false))); + assertThat(expression.next.property, is(new Property("child", null, false))); } @Test public void shouldParseChainedMethodExpressions() { - PropertyExpression parse = parser.parse("person().child"); - assertThat(parse.property, is(new Property("person", null, true))); - assertThat(parse.next.property, is(new Property("child", null, false))); + PropertyExpression expression = parser.parse("person().child"); + assertThat(expression.property, is(new Property("person", null, true))); + assertThat(expression.next.property, is(new Property("child", null, false))); + } + + @Test + public void shouldParseChainedIndexExpressions() + { + PropertyExpression expression = parser.parse("[person].child"); + assertThat(expression.index, is("person")); + assertThat(expression.next.property, is(new Property("child", null, false))); } @Test public void shouldParseDeeperChainedPropertyExpressions() { - PropertyExpression parse = parser.parse("person.child.name"); - assertThat(parse.property, is(new Property("person", null, false))); - assertThat(parse.next.property, is(new Property("child", null, false))); - assertThat(parse.next.next.property, is(new Property("name", null, false))); + PropertyExpression expression = parser.parse("person.child.name"); + assertThat(expression.property, is(new Property("person", null, false))); + assertThat(expression.next.property, is(new Property("child", null, false))); + assertThat(expression.next.next.property, is(new Property("name", null, false))); } @@ -126,6 +136,14 @@ public class PropertyExpressionParserTest } @Test + public void shouldFailParsePropertyExpressionsWithSpace() + { + expectedException.expect(ParserException.class); + expectedException.expectMessage("Expecting a new expression but got: ' '"); + parser.parse("per son"); + } + + @Test public void shouldReportEmptyIndexBrackets() { expectedException.expect(ParserException.class); @@ -134,4 +152,32 @@ public class PropertyExpressionParserTest parser.parse("person[]"); } + @Test + public void shouldReportMethodsCantHaveParameters() + { + expectedException.expect(ParserException.class); + expectedException.expectMessage( + "The expression can't have method parameters: 'repository.getPerson(<--'"); + parser.parse("repository.getPerson(filter)"); + } + + //TODO: better exception message + @Test + public void shouldFailParseInvalidMethodName() + { + expectedException.expect(ParserException.class); + // expectedException.expectMessage( + // "The expression can't have method parameters: 'repository.getPerson(<--'"); + parser.parse("repository.get#name()"); + } + + @Test + public void shouldFailParseMethodsStartingWithInvalidCharacter() + { + expectedException.expect(ParserException.class); + // expectedException.expectMessage( + // "The expression can't have method parameters: 'repository.getPerson(<--'"); + parser.parse("repository.0method()"); + } + } http://git-wip-us.apache.org/repos/asf/wicket/blob/b0f7da85/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 1a3278d..14c0a98 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 @@ -16,6 +16,8 @@ */ package org.apache.wicket.util.lang; +import static org.hamcrest.CoreMatchers.is; + import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.ArrayList; @@ -55,7 +57,10 @@ public class PropertyResolverTest extends WicketTestCase private static final PropertyResolverConverter CONVERTER = new PropertyResolverConverter( new ConverterLocator(), Locale.US); + private static final int AN_INTEGER = 10; private Person person; + private Map<String, Integer> integerMap = new HashMap<String, Integer>(); + private WeirdList integerList = new WeirdList(); /** * @throws Exception @@ -81,7 +86,7 @@ public class PropertyResolverTest extends WicketTestCase @Test public void simpleExpression() throws Exception { - String name = (String) PropertyResolver.getValue("name", person); + String name = (String)PropertyResolver.getValue("name", person); assertNull(name); PropertyResolver.setValue("name", person, "wicket", CONVERTER); @@ -217,10 +222,66 @@ public class PropertyResolverTest extends WicketTestCase assertNotNull(hm.get("address.test")); PropertyResolver.setValue("addressMap[address.test].street", person, "wicket-street", CONVERTER); - String street = (String)PropertyResolver.getValue("addressMap[address.test].street", person); + String street = (String)PropertyResolver.getValue("addressMap[address.test].street", + person); 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 shouldMapKeysWithSpecialCharacters() 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 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 */ @@ -746,63 +807,73 @@ public class PropertyResolverTest extends WicketTestCase Object actual = converter.convert(date, Long.class); assertEquals(date.getTime(), actual); } - + /** * WICKET-5623 custom properties */ @Test - public void custom() { + public void custom() + { Document document = new Document(); document.setType("type"); document.setProperty("string", "string"); - + Document nestedCustom = new Document(); nestedCustom.setProperty("string", "string2"); document.setProperty("nested", nestedCustom); - - PropertyResolver.setLocator(tester.getApplication(), new CachingPropertyLocator(new CustomGetAndSetLocator())); - + + PropertyResolver.setLocator(tester.getApplication(), + new CachingPropertyLocator(new CustomGetAndSetLocator())); + assertEquals("type", PropertyResolver.getValue("type", document)); assertEquals("string", PropertyResolver.getValue("string", document)); assertEquals("string2", PropertyResolver.getValue("nested.string", document)); } - - class CustomGetAndSetLocator implements IPropertyLocator { + + class CustomGetAndSetLocator implements IPropertyLocator + { private IPropertyLocator locator = new DefaultPropertyLocator(); - + @Override - public IGetAndSet get(Class<?> clz, String exp) { + public IGetAndSet get(Class<?> clz, String exp) + { // first try default properties IGetAndSet getAndSet = locator.get(clz, exp); - if (getAndSet == null && Document.class.isAssignableFrom(clz)) { + if (getAndSet == null && Document.class.isAssignableFrom(clz)) + { // fall back to document properties getAndSet = new DocumentPropertyGetAndSet(exp); } return getAndSet; } - - public class DocumentPropertyGetAndSet extends AbstractGetAndSet { + + public class DocumentPropertyGetAndSet extends AbstractGetAndSet + { private String name; - public DocumentPropertyGetAndSet(String name) { + public DocumentPropertyGetAndSet(String name) + { this.name = name; } @Override - public Object getValue(Object object) { - return ((Document) object).getProperty(name); + public Object getValue(Object object) + { + return ((Document)object).getProperty(name); } @Override - public Object newValue(Object object) { + public Object newValue(Object object) + { return new Document(); } @Override - public void setValue(Object object, Object value, PropertyResolverConverter converter) { - ((Document) object).setProperty(name, value); + public void setValue(Object object, Object value, PropertyResolverConverter converter) + { + ((Document)object).setProperty(name, value); } } }
