You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@struts.apache.org by Chris Pratt <th...@gmail.com> on 2008/03/06 06:29:58 UTC

More Struts Insanity

I am seeing some very unexpected behavior using OGNL in the Struts
tags.  I have two objects that contain equivalent Name objects and
when I run the following code:

<!-- Original: "<s:property value='%{originalMember.name}'/>"
(<s:property value='%{originalMember.name.class.name}'/>) -->
<!-- Current:  "<s:property value='%{currentMember.name}'/>"
(<s:property value='%{currentMember.name.class.name}'/>) -->
<!-- Equals(): <s:property
value='%{originalMember.name.equals(currentMember.name)}'/> -->
<!-- Equals:   <s:property value='%{originalMember.name ==
currentMember.name}'/> -->

it outputs the following:

<!-- Original: "CHRIS PRATT" (com.domain.model.Name) -->
<!-- Current:  "CHRIS PRATT" (com.domain.model.Name) -->
<!-- Equals(): true -->
<!-- Equals:    -->

Notice that the last statement (using the OGNL == operator) doesn't
return anything at all, which makes it useless in a <s:if> test.
According to the OGNL documentation, this should work:

"Equality is tested for as follows. If either value is null, they are
equal if and only if both are null. If they are the same object or the
equals() method says they are equal, they are equal. If they are both
Numbers, they are equal if their values as double-precision floating
point numbers are equal. Otherwise, they are not equal. These rules
make numbers compare equal more readily than they would normally, if
just using the equals method."

Does anyone know what's wrong?
  (*Chris*)

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Chris Pratt <th...@gmail.com>.
On Wed, Mar 5, 2008 at 10:53 PM, Dale Newfield <Da...@newfield.org> wrote:
> Chris Pratt wrote:
>  > <!-- Equals(): <s:property
>  > value='%{originalMember.name.equals(currentMember.name)}'/> -->
>
> > <!-- Equals(): <s:property
>  > value='%{CurrentMember.name.equals(originalMember.name)}'/> -->
>
>  Was that case typo (CurrentMember instead of currentMember) introduced
>  in transcription?  I would assume it would not have produced "true" if
>  it were in your code...
>

Yup, it was a typo in the test source, but surprisingly OGNL accepted
it and appeared to do the right thing.  I corrected it and ran the
test again with the same results.
  (*Chris*)

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dale Newfield <Da...@Newfield.org>.
Chris Pratt wrote:
> <!-- Equals(): <s:property
> value='%{originalMember.name.equals(currentMember.name)}'/> -->
> <!-- Equals(): <s:property
> value='%{CurrentMember.name.equals(originalMember.name)}'/> -->

Was that case typo (CurrentMember instead of currentMember) introduced 
in transcription?  I would assume it would not have produced "true" if 
it were in your code...

-Dale

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dave Newton <ne...@yahoo.com>.
--- Dale Newfield <Da...@Newfield.org> wrote:
> There's a chance those bugs were introduced between 2.6.11 and 2.7, but 
> I'm betting not.

Nope; it's in 2.6.9 at least. The behavior happens because of the
isAssignableFrom sequence I mentioned in my previous post.

Dave


---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dale Newfield <Da...@Newfield.org>.
Chris Pratt wrote:
> I added some instrumentation and it appears that OGNL is not calling
> the equals method when I use the == operator, only when I explicitly
> call .equals().

Look!  A clue!

> I believe the jar file is ognl-2.6.11.jar

Yep:

http://jira.opensymphony.com/browse/OGNL-96
http://jira.opensymphony.com/browse/OGNL-104

There's a chance those bugs were introduced between 2.6.11 and 2.7, but 
I'm betting not.

http://blog.opencomponentry.com/2008/02/01/ognl-272-released/

If you replace the .jar (you might also have/want to add javassist: 
http://www.csg.is.titech.ac.jp/~chiba/javassist ), does the problem go away?

-Dale

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Chris Pratt <th...@gmail.com>.
On Wed, Mar 5, 2008 at 9:56 PM, Dale Newfield <Da...@newfield.org> wrote:
> Chris Pratt wrote:
>  > <!-- Equals(): <s:property
>  > value='%{originalMember.name.equals(currentMember.name)}'/> -->
>
>  Just to be pedantic, can you also verify that:
>
>  <!-- Equals(): <s:property
>  value='%{currentMember.name.equals(originalMember.name)}'/> -->
>
>  yields:
>
>  <!-- Equals(): true -->
>
>  ?
>  All correctly implemented equals methods are symmetric, so in the event
>  OGNL decides that neither of these are null and that it should call the
>  equals() method, it could theoretically do so in either order.  If the
>  reversed order (above) throws an exception then that would explain the
>  observed output.

Yup, running:

<!-- Original: "<s:property value='%{originalMember.name}'/>" -->
<!-- Current:  "<s:property value='%{currentMember.name}'/>" -->
<!-- Equals(): <s:property
value='%{originalMember.name.equals(currentMember.name)}'/> -->
<!-- Equals(): <s:property
value='%{CurrentMember.name.equals(originalMember.name)}'/> -->
<!-- Equals:   <s:property value='%{originalMember.name ==
currentMember.name}'/> -->


Yields:

<!-- Original: "CHRIS PRATT" -->
<!-- Current:  "CHRIS PRATT" -->
<!-- Equals(): true -->
<!-- Equals(): true -->
<!-- Equals:    -->


>
>  Assuming that's not it, I'd suggest instrumenting your model to log when
>  the equals and getName methods are called so you can verify it's making
>  the calls you expect.

I added some instrumentation and it appears that OGNL is not calling
the equals method when I use the == operator, only when I explicitly
call .equals().

>
>  I agree it does seem strange.
>  Also:  Which version of ognl are you using?
>

I'm using the one that comes with 2.0.11.1 (and I've seen this on
2.0.11 also), I believe the jar file is ognl-2.6.11.jar
  (*Chris*)

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dale Newfield <Da...@Newfield.org>.
Chris Pratt wrote:
> <!-- Equals(): <s:property
> value='%{originalMember.name.equals(currentMember.name)}'/> -->

Just to be pedantic, can you also verify that:

<!-- Equals(): <s:property 
value='%{currentMember.name.equals(originalMember.name)}'/> -->

yields:

<!-- Equals(): true -->

?
All correctly implemented equals methods are symmetric, so in the event 
OGNL decides that neither of these are null and that it should call the 
equals() method, it could theoretically do so in either order.  If the 
reversed order (above) throws an exception then that would explain the 
observed output.

Assuming that's not it, I'd suggest instrumenting your model to log when 
the equals and getName methods are called so you can verify it's making 
the calls you expect.

I agree it does seem strange.
Also:  Which version of ognl are you using?

-Dale

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: More Struts Insanity

Posted by Dave Newton <ne...@yahoo.com>.
--- Chris Pratt <th...@gmail.com> wrote:
> Since they're both the same Class (though not the same instance) they
> definitely should be assignable, but Name implements Cloneable and
> Serializable, not Comparable so it shouldn't be calling compareTo.

You know, I'm looking at the OGNL source again and I'm not sure why I said
that stuff before. Probably because I've been up for 20 hours. I think I read
the code totally wrong.

That said, implementing Comparable fixed it for me with 2.6.11; I have the
source for 2.6.9 and it looks like it should work as it's written, but can't
get my Eclipse to cooperate with me at the moment and am unable to test under
2.6.9.

Dave



---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: More Struts Insanity

Posted by Chris Pratt <th...@gmail.com>.
On Wed, Mar 5, 2008 at 10:20 PM, Dave Newton <ne...@yahoo.com> wrote:
> --- Chris Pratt <th...@gmail.com> wrote:
>  > Notice that the last statement (using the OGNL == operator) doesn't
>  > return anything at all, which makes it useless in a <s:if> test.
>
>  OGNL actually checks for isAssignableFrom; if either class isAssignableFrom
>  the other it checks for Comparable and calls compareTo, which is most likely
>  what's happening in your case. Only if neither class isAssignableFrom will it
>  call the instance's equals method.
>

Since they're both the same Class (though not the same instance) they
definitely should be assignable, but Name implements Cloneable and
Serializable, not Comparable so it shouldn't be calling compareTo.
  (*Chris*)

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dave Newton <ne...@yahoo.com>.
--- Dale Newfield <Da...@Newfield.org> wrote:
> ...some of my models implement Comparable, but not nearly all.  Not sure 
> how that slipped through...

Probably most of the time people aren't comparing entire objects, but simple
properties like Strings or numbery-things, so it hasn't come up until now. I
know I never have, anyway.

> ...are there interfaces that extend Comparable, or any standard base 
> classes that implement it that we should be aware of and know to 
> override compareTo?

Probably not; I think this would have come up already (he said, waving his
hands in the appropriate magical gesture). I can't think of too many
use-cases where someone would want to compare framework objects directly via
OGNL, though. But I'm sleepy.

Dave



---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: [struts] More Struts Insanity

Posted by Dale Newfield <Da...@Newfield.org>.
Dave Newton wrote:
> --- Chris Pratt <th...@gmail.com> wrote:
>> Notice that the last statement (using the OGNL == operator) doesn't
>> return anything at all, which makes it useless in a <s:if> test.
> 
> OGNL actually checks for isAssignableFrom; if either class isAssignableFrom
> the other it checks for Comparable and calls compareTo, which is most likely
> what's happening in your case. Only if neither class isAssignableFrom will it
> call the instance's equals method.

Hrm...
...some of my models implement Comparable, but not nearly all.  Not sure 
how that slipped through...

...are there interfaces that extend Comparable, or any standard base 
classes that implement it that we should be aware of and know to 
override compareTo?

-Dale

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: More Struts Insanity

Posted by Dave Newton <ne...@yahoo.com>.
--- Chris Pratt <th...@gmail.com> wrote:
> Notice that the last statement (using the OGNL == operator) doesn't
> return anything at all, which makes it useless in a <s:if> test.

OGNL actually checks for isAssignableFrom; if either class isAssignableFrom
the other it checks for Comparable and calls compareTo, which is most likely
what's happening in your case. Only if neither class isAssignableFrom will it
call the instance's equals method.

Dave



---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org