WICKET-4008 parser differentiates java identifiers from bean properties

Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/697052f7
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/697052f7
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/697052f7

Branch: refs/heads/WICKET-6318-configurable-property-expression-resolver
Commit: 697052f713345484b93f4ef61163587473b7b202
Parents: b0f7da8
Author: Pedro Henrique Oliveira dos Santos <pe...@apache.org>
Authored: Tue Sep 20 22:09:36 2016 -0300
Committer: Pedro Henrique Oliveira dos Santos <pe...@apache.org>
Committed: Tue Sep 20 22:09:36 2016 -0300

----------------------------------------------------------------------
 .../core/util/lang/PropertyExpression.java      | 70 ++++++++++++++---
 .../util/lang/PropertyExpressionParser.java     | 69 ++++++++++------
 .../util/lang/PropertyExpressionParserTest.java | 82 +++++++++++---------
 .../wicket/util/lang/PropertyResolverTest.java  | 10 ++-
 4 files changed, 160 insertions(+), 71 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/697052f7/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 635f95e..dab79ec 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
@@ -18,21 +18,76 @@ package org.apache.wicket.core.util.lang;
 
 public class PropertyExpression
 {
-       Property property;
+       JavaProperty javaProperty;
+       BeanProperty beanProperty;
        CharSequence index;
        PropertyExpression next;
 
-       static class Property
+       static class BeanProperty
+       {
+               CharSequence propertyName;
+               CharSequence index;
+
+               public BeanProperty()
+               {
+               }
+
+               public BeanProperty(String name, String index)
+               {
+                       this.propertyName = name;
+                       this.index = index;
+               }
+
+               @Override
+               public int hashCode()
+               {
+                       final int prime = 31;
+                       int result = 1;
+                       result = prime * result + ((index == null) ? 0 : 
index.hashCode());
+                       result = prime * result + ((propertyName == null) ? 0 : 
propertyName.hashCode());
+                       return result;
+               }
+
+               @Override
+               public boolean equals(Object obj)
+               {
+                       if (this == obj)
+                               return true;
+                       if (obj == null)
+                               return false;
+                       if (getClass() != obj.getClass())
+                               return false;
+                       BeanProperty other = (BeanProperty)obj;
+                       if (index == null)
+                       {
+                               if (other.index != null)
+                                       return false;
+                       }
+                       else if (!index.equals(other.index))
+                               return false;
+                       if (propertyName == null)
+                       {
+                               if (other.propertyName != null)
+                                       return false;
+                       }
+                       else if (!propertyName.equals(other.propertyName))
+                               return false;
+                       return true;
+               }
+
+       }
+
+       static class JavaProperty
        {
                CharSequence javaIdentifier;
                CharSequence index;
                public boolean hasMethodSign;
 
-               public Property()
+               public JavaProperty()
                {
                }
 
-               public Property(String javaIdentifier, String index, boolean 
hasMethodSign)
+               public JavaProperty(String javaIdentifier, String index, 
boolean hasMethodSign)
                {
                        this.javaIdentifier = javaIdentifier;
                        this.index = index;
@@ -59,7 +114,7 @@ public class PropertyExpression
                                return false;
                        if (getClass() != obj.getClass())
                                return false;
-                       Property other = (Property)obj;
+                       JavaProperty other = (JavaProperty)obj;
                        if (hasMethodSign != other.hasMethodSign)
                                return false;
                        if (index == null)
@@ -79,11 +134,6 @@ public class PropertyExpression
                        return true;
                }
 
-               @Override
-               public String toString()
-               {
-                       return javaIdentifier + (index == null ? "" : "[" + 
index + "]");
-               }
        }
 
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/wicket/blob/697052f7/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 3ecb81e..2102456 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
@@ -20,25 +20,25 @@ import static java.lang.Character.isJavaIdentifierPart;
 import static java.lang.Character.isJavaIdentifierStart;
 import static java.lang.String.format;
 
-import org.apache.wicket.core.util.lang.PropertyExpression.Property;
+import org.apache.wicket.core.util.lang.PropertyExpression.BeanProperty;
+import org.apache.wicket.core.util.lang.PropertyExpression.JavaProperty;
 
 /**
  * EBNF like description of the property expression syntax <code>
  * 
- *  java letter                                = "_" | "$" | "A" | "a" | "B" | 
"b" | (...) ;
+ *  java letter                                = "_" | "$" | "A" | "a" | "B" | 
"b" | (...);
  *  java letter or digit       = java letter | "0" | "1" | (...) ;
  *  char                                       = java letter or digit | "." | 
"(" | ")" | "[" | "]" | "!" | "@" | "#" | (...);
- *  index char                         = char - "]"
+ *  index char                         = char - "]";
  *  
- *  java identifier                    = java letter , {java letter or digit}
- *  property name                      = java letter or digit , {java letter 
or digit}
- *  method sign                                = "(" , { " " } , ")"
- *  index                                      = "[" , index char , {index 
char} , "]" ;
+ *  java identifier                    = java letter , {java letter or digit};
+ *  property name                      = java letter or digit , {java letter 
or digit};
+ *  method sign                                = "(" , { " " } , ")";
+ *  index                                      = "[" , index char , { index 
char } , "]";
  *  
- *  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 } ;
+ *  bean property                      = property name, [ index ];
+ *  java property                      = java identifier , [ index | method 
sign ];
+ *  property expression                = [ bean property | java property | 
index ] , { "." , property expression };
  *  
  * </code>
  * 
@@ -99,9 +99,17 @@ public class PropertyExpressionParser
                {
                        expression.index = index();
                }
+               else if (Character.isJavaIdentifierStart(currentToken))
+               {
+                       expression.javaProperty = javaProperty();
+               }
+               else if (Character.isJavaIdentifierPart(currentToken))
+               {
+                       expression.beanProperty = beanProperty();
+               }
                else
                {
-                       expression.property = property();
+                       throw new ParserException("Expecting an expression but 
got: " + currentToken);
                }
                switch (lookaheadToken)
                {
@@ -113,14 +121,27 @@ public class PropertyExpressionParser
                        case END_OF_EXPRESSION :
                                return expression;
                        default :
-                               throw new ParserException(
-                                       "Expecting a new expression but got: '" 
+ lookaheadToken + "'");
+                               throw new ParserException(format(
+                                       "Expecting a new expression but got the 
invalid character '%s' at: '%s<--'",
+                                       lookaheadToken, text.substring(0, 
nextPosition + 1)));
+               }
+       }
+
+       private BeanProperty beanProperty()
+       {
+               BeanProperty property = new BeanProperty();
+               property.propertyName = propertyName();
+               if (lookaheadToken == '[')
+               {
+                       advance();// skips left bracket
+                       property.index = index();
                }
+               return property;
        }
 
-       private Property property()
+       private JavaProperty javaProperty()
        {
-               Property property = new Property();
+               JavaProperty property = new JavaProperty();
                property.javaIdentifier = javaIdentifier();
                switch (lookaheadToken)
                {
@@ -136,13 +157,8 @@ public class PropertyExpressionParser
                return property;
        }
 
-
-       private CharSequence javaIdentifier()
+       private CharSequence propertyName()
        {
-               if (!isJavaIdentifierStart(currentToken))
-               {
-                       throw new ParserException("Expeting a java identifier 
but got a :" + currentToken);
-               }
                int begin = currentPosition;
                while (isJavaIdentifierPart(lookaheadToken))
                {
@@ -151,6 +167,15 @@ public class PropertyExpressionParser
                return text.substring(begin, nextPosition);
        }
 
+       private CharSequence javaIdentifier()
+       {
+               if (!isJavaIdentifierStart(currentToken))
+               {
+                       throw new ParserException("Expeting a java identifier 
but got a :" + currentToken);
+               }
+               return propertyName();
+       }
+
        private CharSequence index()
        {
                advance();// escape bracket

http://git-wip-us.apache.org/repos/asf/wicket/blob/697052f7/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 104a01d..ac5ed26 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
@@ -1,10 +1,11 @@
 package org.apache.wicket.core.util.lang;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
 
-import org.apache.wicket.core.util.lang.PropertyExpression.Property;
-import org.hamcrest.CoreMatchers;
+import org.apache.wicket.core.util.lang.PropertyExpression.BeanProperty;
+import org.apache.wicket.core.util.lang.PropertyExpression.JavaProperty;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -15,57 +16,62 @@ public class PropertyExpressionParserTest
        @Rule
        public ExpectedException expectedException = ExpectedException.none();
        private PropertyExpressionParser parser = new 
PropertyExpressionParser();
-       
+
        @Test
        public void shouldParsePropertyExpressions()
        {
                PropertyExpression expression = parser.parse("person");
-               assertThat(expression.index, CoreMatchers.nullValue());
-               assertThat(expression.property, is(new Property("person", null, 
false)));
-               assertThat(expression.next, CoreMatchers.nullValue());
+               assertThat(expression.index, nullValue());
+               assertThat(expression.beanProperty, nullValue());
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", null, false)));
+               assertThat(expression.next, nullValue());
        }
 
        @Test
        public void shouldParsePropertyExpressionStartingWithDigits()
        {
                PropertyExpression expression = parser.parse("1person");
-               assertThat(expression.index, CoreMatchers.nullValue());
-               assertThat(expression.property, is(new Property("person", null, 
false)));
-               assertThat(expression.next, CoreMatchers.nullValue());
+               assertThat(expression.index, nullValue());
+               assertThat(expression.beanProperty, is(new 
BeanProperty("1person", null)));
+               assertThat(expression.javaProperty, nullValue());
+               assertThat(expression.next, nullValue());
        }
-       
+
        @Test
        public void shouldParseShortPropertyExpressions()
        {
                PropertyExpression expression = parser.parse("a");
-               assertThat(expression.index, CoreMatchers.nullValue());
-               assertThat(expression.property, is(new Property("a", null, 
false)));
-               assertThat(expression.next, CoreMatchers.nullValue());
+               assertThat(expression.index, nullValue());
+               assertThat(expression.beanProperty, nullValue());
+               assertThat(expression.javaProperty, is(new JavaProperty("a", 
null, false)));
+               assertThat(expression.next, nullValue());
        }
 
        @Test
        public void shouldParseIndexedPropertyExpressions()
        {
                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());
+               assertThat(expression.index, nullValue());
+               assertThat(expression.beanProperty, nullValue());
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", "age", false)));
+               assertThat(expression.next, nullValue());
        }
 
        @Test
        public void shouldParseMethodExpressions()
        {
                PropertyExpression expression = parser.parse("person()");
-               assertThat(expression.index, CoreMatchers.nullValue());
-               assertThat(expression.property, is(new Property("person", null, 
true)));
-               assertThat(expression.next, CoreMatchers.nullValue());
+               assertThat(expression.index, nullValue());
+               assertThat(expression.beanProperty, nullValue());
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", null, true)));
+               assertThat(expression.next, nullValue());
        }
 
        @Test
        public void shouldParsePropertyExpressionsWithSpaceInMethod()
        {
                final PropertyExpression parse = parser.parse("person( )");
-               assertThat(parse.property, is(new Property("person", null, 
true)));
+               assertThat(parse.javaProperty, is(new JavaProperty("person", 
null, true)));
        }
 
        @Test
@@ -73,40 +79,40 @@ public class PropertyExpressionParserTest
        {
                PropertyExpression expression = parser.parse("[person#name]");
                assertThat(expression.index, is("person#name"));
-               assertThat(expression.property, CoreMatchers.nullValue());
-               assertThat(expression.next, CoreMatchers.nullValue());
+               assertThat(expression.javaProperty, nullValue());
+               assertThat(expression.next, nullValue());
        }
 
        @Test
        public void shouldParseChainedPropertyExpressions()
        {
                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)));
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", null, false)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("child", null, false)));
        }
 
        @Test
        public void shouldParseShortChainedPropertyExpressions()
        {
                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)));
+               assertThat(expression.javaProperty, is(new JavaProperty("a", 
null, false)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("b", null, false)));
        }
 
        @Test
        public void shouldParseChainedIndexedPropertyExpressions()
        {
                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)));
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", "1", false)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("child", null, false)));
        }
 
        @Test
        public void shouldParseChainedMethodExpressions()
        {
                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)));
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", null, true)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("child", null, false)));
        }
 
        @Test
@@ -114,16 +120,16 @@ public class PropertyExpressionParserTest
        {
                PropertyExpression expression = parser.parse("[person].child");
                assertThat(expression.index, is("person"));
-               assertThat(expression.next.property, is(new Property("child", 
null, false)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("child", null, false)));
        }
 
        @Test
        public void shouldParseDeeperChainedPropertyExpressions()
        {
                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)));
+               assertThat(expression.javaProperty, is(new 
JavaProperty("person", null, false)));
+               assertThat(expression.next.javaProperty, is(new 
JavaProperty("child", null, false)));
+               assertThat(expression.next.next.javaProperty, is(new 
JavaProperty("name", null, false)));
        }
 
 
@@ -139,7 +145,8 @@ public class PropertyExpressionParserTest
        public void shouldFailParsePropertyExpressionsWithSpace()
        {
                expectedException.expect(ParserException.class);
-               expectedException.expectMessage("Expecting a new expression but 
got: ' '");
+               expectedException.expectMessage(
+                       "Expecting a new expression but got the invalid 
character ' ' at: 'per <--'");
                parser.parse("per son");
        }
 
@@ -161,13 +168,12 @@ public class PropertyExpressionParserTest
                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(<--'");
+               expectedException.expectMessage(
+                       "Expecting a new expression but got the invalid 
character '#' at: 'repository.get#<--'");
                parser.parse("repository.get#name()");
        }
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/697052f7/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 14c0a98..5929eef 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
@@ -246,13 +246,21 @@ public class PropertyResolverTest extends WicketTestCase
        }
 
        @Test
-       public void shouldMapKeysWithSpecialCharacters() throws Exception
+       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

Reply via email to