Repository: wicket
Updated Branches:
  refs/heads/WICKET-4008-property-expression-parser 2f302dbfd -> b0f7da85e


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-4008-property-expression-parser
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);
                        }
                }
        }

Reply via email to