date:20160928

Re: [VOTE] Release Apache Wicket 6.25.0

2016-09-28 Thread Pedro Santos

As we updated our Jetty dependency version from 7.6.13.v20130916 to
7.6.21.v20160908 [1], we unfortunately broke our quickstart's
jetty-maven-plugin since this new version is not released under the old
Jetty host Mortbay [2], as configured in our quickstart pom.xml. Actually I
can't find this plugin version even in the new Jetty host releases [3].

Given that the quickstart is the entry point for new developers in Wicket,
and that I don't know how common is this plugin usage ( I use it to start
wicket-examples ), I'm -0 to release this version.

On another topic, the pom.xml's version in the build/wicket-6.25.0 branch
is 6.26.0-SNAPSHOT. Is it OK? Even the wicket-examples site is showing
me 6.26.0-SNAPSHOT.

Cheers

1 -
https://git1-us-west.apache.org/repos/asf?p=wicket.git;a=blobdiff;f=archetypes/quickstart/src/main/resources/archetype-resources/pom.xml;h=2afc43303ce865479144e2ec35b7efcf41c1450b;hp=07e39fd9955d1d673fbb66f5fa8110fcd4eacb09;hb=52f0b8af;hpb=6ec87eeba13500b89b6c81c757d8a946c68e8222
2 -
https://repo.maven.apache.org/maven2/org/mortbay/jetty/jetty-maven-plugin/
3 -
https://repo.maven.apache.org/maven2/org/eclipse/jetty/jetty-maven-plugin/

Pedro Santos

On Tue, Sep 27, 2016 at 6:43 AM, Andrea Del Bene 
wrote:

> +1 Built and tested random examples
>
>
>
> On 26/09/2016 17:11, Sven Meier wrote:
>
>> +1 release 6.25.0
>>
>> Built, checked md5 and poked some examples.
>>
>> Thanks
>> Sven
>>
>>
>> On 24.09.2016 15:22, Martijn Dashorst wrote:
>>
>>> This is a vote to release Apache Wicket 6.25.0
>>>
>>> Please download the source distributions found in our staging area
>>> linked below.
>>>
>>> I have included the signatures for both the source archives. This vote
>>> lasts for 72 hours minimum.
>>>
>>> [ ] Yes, release Apache Wicket 6.25.0
>>> [ ] No, don't release Apache Wicket 6.25.0, because ...
>>>
>>> Distributions, changelog, keys and signatures can be found at:
>>>
>>>  https://dist.apache.org/repos/dist/dev/wicket/6.25.0
>>>
>>> Staging repository:
>>>
>>> https://repository.apache.org/content/repositories/orgapachewicket-1074/
>>>
>>> The binaries are available in the above link, as are a staging
>>> repository for Maven. Typically the vote is on the source, but should
>>> you find a problem with one of the binaries, please let me know, I can
>>> re-roll them some way or the other.
>>>
>>> Staging git repository data:
>>>
>>>  Repository:  g...@github.com:dashorst/wicket.git
>>>  Branch:  build/wicket-6.25.0
>>>  Release tag: rel/wicket-6.25.0
>>>
>>>
>>> 
>>>
>>>  The signatures for the source release artefacts:
>>>
>>>
>>> Signature for apache-wicket-6.25.0.zip:
>>>
>>>  -BEGIN PGP SIGNATURE-
>>> Comment: GPGTools - https://gpgtools.org
>>>
>>> iEYEABECAAYFAlfmfCEACgkQJBX8W/xy/UXaZACdFBqxuvFcai65YtX+tWD09OEy
>>> AxkAnA9PUgDJQL+0iV23RK6FbLRJCEoZ
>>> =issT
>>> -END PGP SIGNATURE-
>>>
>>> Signature for apache-wicket-6.25.0.tar.gz:
>>>
>>>  -BEGIN PGP SIGNATURE-
>>> Comment: GPGTools - https://gpgtools.org
>>>
>>> iEYEABECAAYFAlfmfCEACgkQJBX8W/xy/UUUgQCfUbwPNEK3DW+EoyJE2ukoCjgF
>>> CPYAnRk58qx7E4s0PlWDEVW2Y+c08LcZ
>>> =kttX
>>> -END PGP SIGNATURE-
>>>
>>> 
>>>
>>>  CHANGELOG for 6.25.0:
>>>
>>> ** Sub-task
>>>
>>>  * [WICKET-6218] - backport fix for WICKET-6172 to Wicket 6.x
>>>
>>> ** Bug
>>>
>>>  * [WICKET-5972] - Datepicker "Close" text overlays 'x' icon.
>>>  * [WICKET-6136] - AutoCompleteTextField issue in Android 5.1.1
>>>  * [WICKET-6209] - requesting focus on disabled field fails with
>>> error in IE8
>>>  * [WICKET-6214] - ModalWindow broken on IE
>>>  * [WICKET-6219] - Fragment fails to report an error in development
>>> mode
>>>  * [WICKET-6225] - Button wrongly sets its model object as 'value'
>>> attribute
>>>  * [WICKET-6227] - CharSequenceResource calculates wrong length
>>> when there are unicode symbols
>>>  * [WICKET-6230] - Infinite redirection when using
>>> UrlPathPageParametersEncoder
>>>  * [WICKET-6232] - When sending binary data from server to client,
>>> wicket-websocket-jquery.js throws error "message.indexOf is not a
>>> function"
>>>  * [WICKET-6235] - TableTree#updateNode() fails if no corresponding
>>> node is visible
>>>  * [WICKET-6236] - Files.remove() causes a 5 seconds delay instead
>>> of 500ms as was intended
>>>  * [WICKET-6237] - PageRequestHandlerTracker doesn't work with
>>> IRequestHandlerDelegate
>>>  * [WICKET-6245] - Open up CsrfPreventionRequestCycleListener for
>>> extension
>>>  * [WICKET-6246] - WebSocket request while Ajax request leads to
>>> error regarding HtmlHeaderCotnainer
>>>
>>> ** Improvement
>>>
>>>  * [WICKET-6206] - Allow to use custom anticache parameter value
>>> for Image component
>>>  * [WICKET-6210] - FileUpload does not

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

2016-09-28 Thread Pedro Santos

Hi,

we can look for a better name, but imo it's correct. Lets say that the user
wrote a very weir list like:

class WeirdList implements List{
  int get0(){ return internal;}
  void set0( p ){ internal = p;}
}

In this case:
 - the expression "weirList.0" has "0" correctly parsed to a bean property
named "0"
 - the expression "weirdList[0]" has the "[0]" correctly parsed to an index

In the case you pointed out, were we have a non weird list:

 - the expression "regularList.0" correctly parses "0" to bean property.
Yes, our resolver will resolve this expression to a list position as if it
was resolving an index expression, but only because it failed to find a
bean property
 - the expression "regularList[0]" correctly parses "[0]" to an index and
will allow the resolver to skip any code trying to resolve "[0]" as a bean
property

Pedro Santos

On Wed, Sep 28, 2016 at 3:58 PM, Sven Meier  wrote:

> Hi,
>
> > the plan is to change PropertyResolver to use a syntax tree insteadof
> the current list of lexemes
>
> good, I missed that.
>
> > return resolveListOrArrayIndex(object, expression.beanProperty);
>
> Well, then expression.beanProperty is wrongly named, isn't it?
>
> Sven
>
>
>
> On 28.09.2016 19:24, Pedro Santos wrote:
>
>> Hi Sven, thx. Sending the response inline.
>>
>> Cheers
>>
>> Pedro Santos
>>
>> On Wed, Sep 28, 2016 at 3:08 AM, Sven Meier  wrote:
>>
>> Hi Pedro,
>>>
>>> very interesting work, thank you very much.
>>>
>>> I have a little nitpick though:
>>>
>>> The expression "addressList.1" is parsed into a PropertyExpression with
>>> JavaProperty-"person" followed by BeanProperty-"1".
>>>
>>> When PropertyResolver converts the parsed expression into {"addressList",
>>> "1"}, the syntactic analysis is lost though.
>>>
>>> Indeed. The plan is to change PropertyResolver to use a syntax tree
>> instead
>> of the current list of lexemes. One of the devs can go ahead and make this
>> change, but I would rather wait to get the team's thoughts on the parser
>> first. As a side note, I moved the current tokenizer code from
>> PropertyResolver to the inner class PropertyResolverTokenizer only to make
>> the current code clearer; this class should be deleted in the future.
>>
>>
>> With the lexical analysis remaining only, the expression results in
>>> invocation of #getAdressList() followed by #get(1) instead,
>>> i.e. "1" is interpreted as a list index.
>>>
>>> It seems our current grammar is more lenient than what your parser is
>>> expecting.
>>>
>>> To be exact, our property resolution logic is lenient, not the current
>> grammar. It's perfectly fine to have a lenient property resolution logic
>> that falls back the property expression "myArray.1" to "myArray[1]". But
>> using a syntax tree, the resolution code would be much clear about it.
>> e.g.
>>
>> PropertyResolver {
>>IGetAndSet resolve(object, expression) {
>> ...
>>  if(expression.beanProperty != null) {
>>try {
>>  return resolveBeanProperty(object, expression.beanProperty);
>>} catch(ParserException e){
>>  if(isANumber(expression.beanProperty))
>>//can't resolve the bean property "1", since it's a number,
>> fallback to test a List/Array index as if the beanProperty were an index
>> expression like "[1]"
>>return resolveListOrArrayIndex(object,
>> expression.beanProperty);
>>  else
>>//fallback to test if the beanProperty can be a map key
>>...
>>}
>>  }
>> ...
>>}
>> }
>>
>>
>> Regards
>>> Sven
>>>
>>>
>>>
>>>
>>> On 25.09.2016 09:40, Pedro Santos wrote:
>>>
>>> Hi devs,

 Currently PropertyResolver is doing all the tokenizer plus property
 resolution logic to resolve property expressions to IGetAndSet
 interfaces.
 To improve its cohesion and to add new messages explaining syntax error
 in
 the property expression entered by users, I propose us to use a parser
 for
 the grammar below.

 I created the branch [1] to show the parser and, if the team agrees with
 the proposal, I will move to the next step that is to change the
 PropertyResolver to use the generated syntax tree object instead of the
 current lexemes. I'm using the ticket WICKET-4008 to track the work.

 Proposed grammar in an EBNF like description:

 java letter = "_" | "$" | "A" | "a" | "B" | "b" | (...);

 java letter or digit = java letter | "0" | "1" | (...) ;
 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 = "(" , empty space , ")";
 index = "[" , index char , { index char } , "]";

 bean property = property name, [ index ];
 java

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

2016-09-28 Thread Sven Meier

Hi,

> the plan is to change PropertyResolver to use a syntax tree insteadof 
the current list of lexemes

good, I missed that.

> return resolveListOrArrayIndex(object, expression.beanProperty);

Well, then expression.beanProperty is wrongly named, isn't it?

Sven

On 28.09.2016 19:24, Pedro Santos wrote:

Hi Sven, thx. Sending the response inline.

Cheers

Pedro Santos

On Wed, Sep 28, 2016 at 3:08 AM, Sven Meier  wrote:

Hi Pedro,

very interesting work, thank you very much.

I have a little nitpick though:

The expression "addressList.1" is parsed into a PropertyExpression with
JavaProperty-"person" followed by BeanProperty-"1".

When PropertyResolver converts the parsed expression into {"addressList",
"1"}, the syntactic analysis is lost though.

Indeed. The plan is to change PropertyResolver to use a syntax tree instead
of the current list of lexemes. One of the devs can go ahead and make this
change, but I would rather wait to get the team's thoughts on the parser
first. As a side note, I moved the current tokenizer code from
PropertyResolver to the inner class PropertyResolverTokenizer only to make
the current code clearer; this class should be deleted in the future.

With the lexical analysis remaining only, the expression results in
invocation of #getAdressList() followed by #get(1) instead,
i.e. "1" is interpreted as a list index.

It seems our current grammar is more lenient than what your parser is
expecting.

To be exact, our property resolution logic is lenient, not the current
grammar. It's perfectly fine to have a lenient property resolution logic
that falls back the property expression "myArray.1" to "myArray[1]". But
using a syntax tree, the resolution code would be much clear about it. e.g.

PropertyResolver {
   IGetAndSet resolve(object, expression) {
...
 if(expression.beanProperty != null) {
   try {
 return resolveBeanProperty(object, expression.beanProperty);
   } catch(ParserException e){
 if(isANumber(expression.beanProperty))
   //can't resolve the bean property "1", since it's a number,
fallback to test a List/Array index as if the beanProperty were an index
expression like "[1]"
   return resolveListOrArrayIndex(object, expression.beanProperty);
 else
   //fallback to test if the beanProperty can be a map key
   ...
   }
 }
...
   }
}

Regards
Sven

On 25.09.2016 09:40, Pedro Santos wrote:

Hi devs,

Currently PropertyResolver is doing all the tokenizer plus property
resolution logic to resolve property expressions to IGetAndSet interfaces.
To improve its cohesion and to add new messages explaining syntax error in
the property expression entered by users, I propose us to use a parser for
the grammar below.

I created the branch [1] to show the parser and, if the team agrees with
the proposal, I will move to the next step that is to change the
PropertyResolver to use the generated syntax tree object instead of the
current lexemes. I'm using the ticket WICKET-4008 to track the work.

Proposed grammar in an EBNF like description:

java letter = "_" | "$" | "A" | "a" | "B" | "b" | (...);

java letter or digit = java letter | "0" | "1" | (...) ;
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 = "(" , empty space , ")";
index = "[" , index char , { index char } , "]";

bean property = property name, [ index ];
java property = java identifier , [ index | method sign ];
property expression = [ bean property | java property | index ] , {
"." ,
property expression };

Main goals:

- To remove from PropertyResolver its current tokenizer code
- To validate property expressions syntax

To exemplify the new messages explaining syntax errors:

- Given the expression: "bean.method(parameter)", we can explain that no
parameter is allowed inside the brackets
- Given the expression "array[]", we can explain that the empty brackets
aren't allowed
- Given the expression "bean.prop#name", we can explain that
"bean.prop#name" isn't a valid bean property name
- Given the expression "bean.0method()", we can explain that "0method"
isn't a valid java identifier, thus the method call is invalid

Collateral benefits:

- We will be able to map keys in map expressions using the '[' character
like "may[key[weird]"
   Obs. 1: even having the open square bracket '[', the parser is able
to
understand the key because of grammar's production rule: index char = char
- "]";
   Obs. 2: a better solution was proposed in the thread
http://wicket-dev.markmail.org/thread/unwdqpxulw7tcd5l
- Allows empty spaces inside method call brackets ( )
  Obs.: because of grammar's production rule: method sign = "(" , empty
space, ")";
- Improves

Re: [1/2] wicket git commit: Detaches LDM after exception during load()

2016-09-28 Thread Martin Grigorov

This fix looks useful for 6.x as well.
Any particular reason it is not there ?

On Sep 27, 2016 4:43 PM,  wrote:
>
> Repository: wicket
> Updated Branches:
>   refs/heads/wicket-7.x bdcf92890 -> 6c6f08a30
>
>
> Detaches LDM after exception during load()
>
> When an Exception occurs during the `load()` phase of a
> LoadableDetachableModel, the state of the LDM is "ATTACHING", but it
> can't ever recover from it through detaching. Detaching should always
> happen when the model doesn't have a state (state == null) and the
> model hasn't been detached prior during the request. (the inverse
> happens for attaching).
>
> Fixes WICKET-6249
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6045149e
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6045149e
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6045149e
>
> Branch: refs/heads/wicket-7.x
> Commit: 6045149e004efd6073e660473b69f8e0f82cae7a
> Parents: 796255d
> Author: Martijn Dashorst 
> Authored: Tue Sep 27 15:41:29 2016 +0200
> Committer: Martijn Dashorst 
> Committed: Tue Sep 27 15:42:55 2016 +0200
>
> --
>  .../wicket/model/LoadableDetachableModel.java   |  3 +-
>  .../model/LoadableDetachableModelTest.java  | 45 ++--
>  2 files changed, 44 insertions(+), 4 deletions(-)
> --
>
>
>
http://git-wip-us.apache.org/repos/asf/wicket/blob/6045149e/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
> --
> diff --git
a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
> index 75e91bd..a012245 100644
> ---
a/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
> +++
b/wicket-core/src/main/java/org/apache/wicket/model/LoadableDetachableModel.java
> @@ -103,7 +103,8 @@ public abstract class LoadableDetachableModel
implements IModel
> @Override
> public void detach()
> {
> -   if (state == InternalState.ATTACHED)
> +   // even if LDM is in partial attached state (ATTACHING)
it should be detached
> +   if (state != null && state != InternalState.DETACHED)
> {
> try
> {
>
>
http://git-wip-us.apache.org/repos/asf/wicket/blob/6045149e/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
> --
> diff --git
a/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
b/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
> index 0e96509..fe599fe 100644
> ---
a/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
> +++
b/wicket-core/src/test/java/org/apache/wicket/model/LoadableDetachableModelTest.java
> @@ -62,6 +62,35 @@ public class LoadableDetachableModelTest extends
WicketTestCase
> assertThat(ldm.isAttached(), is(true));
> }
>
> +   @Test
> +   public void onAttachCalled()
> +   {
> +   class AttachingLoadableModel extends
LoadableDetachableModel
> +   {
> +   private static final long serialVersionUID = 1L;
> +
> +   private boolean attachCalled = false;
> +
> +   @Override
> +   protected Integer load()
> +   {
> +   return null;
> +   }
> +
> +   @Override
> +   protected void onAttach()
> +   {
> +   attachCalled = true;
> +   }
> +   }
> +
> +   AttachingLoadableModel m = new AttachingLoadableModel();
> +   m.getObject();
> +
> +   assertThat(m.isAttached(), is(true));
> +   assertThat(m.attachCalled, is(true));
> +   }
> +
> /**
>  * Checks whether the LDM can escape recursive calls.
>  */
> @@ -72,11 +101,19 @@ public class LoadableDetachableModelTest extends
WicketTestCase
> {
> private static final long serialVersionUID = 1L;
>
> +   private boolean detachCalled = false;
> +
> @Override
> protected Integer load()
> {
> throw new RuntimeException();
> }
> +
> +   @Override
> +   protected void

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

2016-09-28 Thread Pedro Santos

Hi Sven, thx. Sending the response inline.

Cheers

Pedro Santos

On Wed, Sep 28, 2016 at 3:08 AM, Sven Meier  wrote:

> Hi Pedro,
>
> very interesting work, thank you very much.
>
> I have a little nitpick though:
>
> The expression "addressList.1" is parsed into a PropertyExpression with
> JavaProperty-"person" followed by BeanProperty-"1".
>
> When PropertyResolver converts the parsed expression into {"addressList",
> "1"}, the syntactic analysis is lost though.
>

Indeed. The plan is to change PropertyResolver to use a syntax tree instead
of the current list of lexemes. One of the devs can go ahead and make this
change, but I would rather wait to get the team's thoughts on the parser
first. As a side note, I moved the current tokenizer code from
PropertyResolver to the inner class PropertyResolverTokenizer only to make
the current code clearer; this class should be deleted in the future.


> With the lexical analysis remaining only, the expression results in
> invocation of #getAdressList() followed by #get(1) instead,
> i.e. "1" is interpreted as a list index.
>
> It seems our current grammar is more lenient than what your parser is
> expecting.
>

To be exact, our property resolution logic is lenient, not the current
grammar. It's perfectly fine to have a lenient property resolution logic
that falls back the property expression "myArray.1" to "myArray[1]". But
using a syntax tree, the resolution code would be much clear about it. e.g.

PropertyResolver {
  IGetAndSet resolve(object, expression) {
...
if(expression.beanProperty != null) {
  try {
return resolveBeanProperty(object, expression.beanProperty);
  } catch(ParserException e){
if(isANumber(expression.beanProperty))
  //can't resolve the bean property "1", since it's a number,
fallback to test a List/Array index as if the beanProperty were an index
expression like "[1]"
  return resolveListOrArrayIndex(object, expression.beanProperty);
else
  //fallback to test if the beanProperty can be a map key
  ...
  }
}
...
  }
}


>
> Regards
> Sven
>
>
>
>
> On 25.09.2016 09:40, Pedro Santos wrote:
>
>> Hi devs,
>>
>> Currently PropertyResolver is doing all the tokenizer plus property
>> resolution logic to resolve property expressions to IGetAndSet interfaces.
>> To improve its cohesion and to add new messages explaining syntax error in
>> the property expression entered by users, I propose us to use a parser for
>> the grammar below.
>>
>> I created the branch [1] to show the parser and, if the team agrees with
>> the proposal, I will move to the next step that is to change the
>> PropertyResolver to use the generated syntax tree object instead of the
>> current lexemes. I'm using the ticket WICKET-4008 to track the work.
>>
>> Proposed grammar in an EBNF like description:
>>
>>java letter = "_" | "$" | "A" | "a" | "B" | "b" | (...);
>>
>>java letter or digit = java letter | "0" | "1" | (...) ;
>>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 = "(" , empty space , ")";
>>index = "[" , index char , { index char } , "]";
>>
>>bean property = property name, [ index ];
>>java property = java identifier , [ index | method sign ];
>>property expression = [ bean property | java property | index ] , {
>> "." ,
>> property expression };
>>
>> Main goals:
>>
>> - To remove from PropertyResolver its current tokenizer code
>> - To validate property expressions syntax
>>
>> To exemplify the new messages explaining syntax errors:
>>
>> - Given the expression: "bean.method(parameter)", we can explain that no
>> parameter is allowed inside the brackets
>> - Given the expression "array[]", we can explain that the empty brackets
>> aren't allowed
>> - Given the expression "bean.prop#name", we can explain that
>> "bean.prop#name" isn't a valid bean property name
>> - Given the expression "bean.0method()", we can explain that "0method"
>> isn't a valid java identifier, thus the method call is invalid
>>
>> Collateral benefits:
>>
>> - We will be able to map keys in map expressions using the '[' character
>> like "may[key[weird]"
>>   Obs. 1: even having the open square bracket '[', the parser is able
>> to
>> understand the key because of grammar's production rule: index char = char
>> - "]";
>>   Obs. 2: a better solution was proposed in the thread
>> http://wicket-dev.markmail.org/thread/unwdqpxulw7tcd5l
>> - Allows empty spaces inside method call brackets ( )
>>  Obs.: because of grammar's production rule: method sign = "(" , empty
>> space, ")";
>> - Improves Wicket's performance since every detected syntax error prevents
>> PropertyResolver from to test bean's classes via reflection,

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

2016-09-28 Thread Sven Meier


Hi Pedro,

very interesting work, thank you very much.

I have a little nitpick though:

The expression "addressList.1" is parsed into a PropertyExpression with 
JavaProperty-"person" followed by BeanProperty-"1".


When PropertyResolver converts the parsed expression into 
{"addressList", "1"}, the syntactic analysis is lost though.


With the lexical analysis remaining only, the expression results in 
invocation of #getAdressList() followed by #get(1) instead,

i.e. "1" is interpreted as a list index.

It seems our current grammar is more lenient than what your parser is 
expecting.


Regards
Sven



On 25.09.2016 09:40, Pedro Santos wrote:

Hi devs,

Currently PropertyResolver is doing all the tokenizer plus property
resolution logic to resolve property expressions to IGetAndSet interfaces.
To improve its cohesion and to add new messages explaining syntax error in
the property expression entered by users, I propose us to use a parser for
the grammar below.

I created the branch [1] to show the parser and, if the team agrees with
the proposal, I will move to the next step that is to change the
PropertyResolver to use the generated syntax tree object instead of the
current lexemes. I'm using the ticket WICKET-4008 to track the work.

Proposed grammar in an EBNF like description:

   java letter = "_" | "$" | "A" | "a" | "B" | "b" | (...);

   java letter or digit = java letter | "0" | "1" | (...) ;
   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 = "(" , empty space , ")";
   index = "[" , index char , { index char } , "]";

   bean property = property name, [ index ];
   java property = java identifier , [ index | method sign ];
   property expression = [ bean property | java property | index ] , { "." ,
property expression };

Main goals:

- To remove from PropertyResolver its current tokenizer code
- To validate property expressions syntax

To exemplify the new messages explaining syntax errors:

- Given the expression: "bean.method(parameter)", we can explain that no
parameter is allowed inside the brackets
- Given the expression "array[]", we can explain that the empty brackets
aren't allowed
- Given the expression "bean.prop#name", we can explain that
"bean.prop#name" isn't a valid bean property name
- Given the expression "bean.0method()", we can explain that "0method"
isn't a valid java identifier, thus the method call is invalid

Collateral benefits:

- We will be able to map keys in map expressions using the '[' character
like "may[key[weird]"
  Obs. 1: even having the open square bracket '[', the parser is able to
understand the key because of grammar's production rule: index char = char
- "]";
  Obs. 2: a better solution was proposed in the thread
http://wicket-dev.markmail.org/thread/unwdqpxulw7tcd5l
- Allows empty spaces inside method call brackets ( )
 Obs.: because of grammar's production rule: method sign = "(" , empty
space, ")";
- Improves Wicket's performance since every detected syntax error prevents
PropertyResolver from to test bean's classes via reflection, know to add
performance overhead [2].

Future benefits:

- The parser enables us to easily enforce that expressions like
"bean.map[key]" will have the key tested only against a map, and won't be
resolved as to object's get/set methods.
- PropertyResolver resolves expressions like "bean.attributeAt.1" to the
methods: bean.getAttributeAt(1) and bean.setAttributeAt(1, value). This
resolution logic can be improved to only test for this kind of method
signatures if the analyzed input allows such usage (currently the resolver
always test for such signatures).

1 -
https://github.com/apache/wicket/commits/WICKET-4008-property-expression-parser
2 - http://docs.oracle.com/javase/tutorial/reflect/index.html

Cheers

Pedro Santos

Re: [VOTE] Release Apache Wicket 6.25.0

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

Re: [1/2] wicket git commit: Detaches LDM after exception during load()

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

Re: Proposal to improve property expression resolution for Wicket 8 by using a parser

6 matches

Site Navigation

Mail list logo

Footer information