You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Niall Pemberton <ni...@gmail.com> on 2005/11/30 10:33:00 UTC
CGLib DynaActioForm problem (was [VOTE] Confirm the Struts Action Library 1.3.0 release plan)
On 11/30/05, Laurie Harper <la...@holoweb.net> wrote:
> Niall Pemberton wrote:
> > On 11/29/05, Niall Pemberton <ni...@gmail.com> wrote:
> >> On 11/29/05, Laurie Harper <la...@holoweb.net> wrote:
> >>> Niall Pemberton wrote:
> >>>> Also looked like the property types were the wrong way round for
> >>>> indexed methods, so I also switched them.
> >>> Hmm, with that change, accessing a List property is broken. Without the
> >>> change, all my tests are working. You can deploy the exercises app with
> >>> the addition I just committed and see for yourself:
> >>> http://localhost:8080/struts-examples/exercise/enhanced-dynabean.do
> >>>
> >>> I've rolled back your change and everything works fine again, including
> >>> indexed property access for arrays and lists. The unit test you added
> >>> passes either way.
> >> I just refreshed and the JUnit test I wrote is now failing for me.
> >
> > Its failing in gump as well
> > http://vmgump.apache.org/gump/public/struts/struts-action/gump_work/build_struts_struts-action.html
> >
> > I've had problems with the maven build - when I've used build-all if a
> > sub-project test fails maven just carries on ignoring it and it
> > appears you get a successful build. Now I do the sub-projects
> > separately.
> >
> > I'm wondering if thats what you used?
>
> I was using 'maven test' without problem. It seems my checkout was
> warped, though, because even though svn was telling me it was
> up-to-date, after an 'svn update' I'm now seeing the failure.
This doesn't make any sense to me, unless you commit first and test after?
> I'll look at it tomorrow and figure out what's wrong.
OK heres my take on what the issue is. The java bean specification
defines indexed properties in relation to arrays, so if you have
methods like...
public String[] getFoo() // simple read
public void setFoo(String[] foo) // simple write
public String getFoo(int idx) // indexed read
public void setFoo(int idx, String foo) // indexed write
...it will create an IndexedPropertyDescriptor instead of a
PropertyDescriptor for the Foo property. In previous JDK versions
(i.e. pre JDK 1.4 I believe) Sun was lenient about the simple
properties - it didn't care about the parameter/return type, so if you
defined your simple read/write methods with a java.util.List, it still
worked. However in JDK 1.4 they changed things, so that if the simple
read/write didn't return an array then it would still create an
IndexedPropertyDescriptor, but the getReadMethod/getWriteMethod return
null, effectively masking the simple read/write methods.
In DynaBeanInterceptor the indexed read/write methods it was creating
were incorrectly defined and so it wasn't recognizing those as indexed
properties (and your List worked). I believe my change fixed this
issue, and effectively masked the simple read/write methods for the
indexed property defined with a List, although I don't understand why
your test page didn't work because of this, since JSTL should have
been able to use the indexed read/write methods.
It would be interesting to see what happens with your test page with a
"regular" incorrectly defined indexed property, i.e.
public List getFoo()
public void setFoo(List foo)
public String getFoo(int idx)
public void setFoo(int idx, String foo)
OK I guess the question that stands out is, if I'm right on this how
come your test page worked with incorrectly defined indexed
properties. My guess is that JSTL is smart enough to handle simple
properties for arrays and Lists, but if thats true, the fact that your
page didn't work after my change means that its also too dumb to use
the indexed read/write methods.
Course I could have this wrong as this is all theory and I haven't put
any effort into actually proving it.
Niall
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: CGLib DynaActioForm problem (was [VOTE] Confirm the Struts Action
Library 1.3.0 release plan)
Posted by Laurie Harper <la...@holoweb.net>.
Niall Pemberton wrote:
> On 11/30/05, Laurie Harper <la...@holoweb.net> wrote:
>> Niall Pemberton wrote:
>>> On 11/29/05, Niall Pemberton <ni...@gmail.com> wrote:
>>>> On 11/29/05, Laurie Harper <la...@holoweb.net> wrote:
>>>>> Niall Pemberton wrote:
>>>>>> Also looked like the property types were the wrong way round for
>>>>>> indexed methods, so I also switched them.
>>>>> Hmm, with that change, accessing a List property is broken. Without the
>>>>> change, all my tests are working. You can deploy the exercises app with
>>>>> the addition I just committed and see for yourself:
>>>>> http://localhost:8080/struts-examples/exercise/enhanced-dynabean.do
>>>>>
>>>>> I've rolled back your change and everything works fine again, including
>>>>> indexed property access for arrays and lists. The unit test you added
>>>>> passes either way.
>>>> I just refreshed and the JUnit test I wrote is now failing for me.
>>> Its failing in gump as well
>>> http://vmgump.apache.org/gump/public/struts/struts-action/gump_work/build_struts_struts-action.html
>>>
>>> I've had problems with the maven build - when I've used build-all if a
>>> sub-project test fails maven just carries on ignoring it and it
>>> appears you get a successful build. Now I do the sub-projects
>>> separately.
>>>
>>> I'm wondering if thats what you used?
>> I was using 'maven test' without problem. It seems my checkout was
>> warped, though, because even though svn was telling me it was
>> up-to-date, after an 'svn update' I'm now seeing the failure.
>
> This doesn't make any sense to me, unless you commit first and test after?
Nope, I just somehow had a working copy that *looked* up to date but
wasn't... Doesn't make any sense to me either, but I checked out a clean
copy to make sure things were consistent so I should be fine now.
>
>> I'll look at it tomorrow and figure out what's wrong.
>
> OK heres my take on what the issue is. The java bean specification
> defines indexed properties in relation to arrays, so if you have
> methods like...
>
> public String[] getFoo() // simple read
> public void setFoo(String[] foo) // simple write
> public String getFoo(int idx) // indexed read
> public void setFoo(int idx, String foo) // indexed write
>
> ...it will create an IndexedPropertyDescriptor instead of a
> PropertyDescriptor for the Foo property. In previous JDK versions
> (i.e. pre JDK 1.4 I believe) Sun was lenient about the simple
> properties - it didn't care about the parameter/return type, so if you
> defined your simple read/write methods with a java.util.List, it still
> worked. However in JDK 1.4 they changed things, so that if the simple
> read/write didn't return an array then it would still create an
> IndexedPropertyDescriptor, but the getReadMethod/getWriteMethod return
> null, effectively masking the simple read/write methods.
>
> In DynaBeanInterceptor the indexed read/write methods it was creating
> were incorrectly defined and so it wasn't recognizing those as indexed
> properties (and your List worked). I believe my change fixed this
> issue, and effectively masked the simple read/write methods for the
> indexed property defined with a List, although I don't understand why
> your test page didn't work because of this, since JSTL should have
> been able to use the indexed read/write methods.
>
> It would be interesting to see what happens with your test page with a
> "regular" incorrectly defined indexed property, i.e.
>
> public List getFoo()
> public void setFoo(List foo)
> public String getFoo(int idx)
> public void setFoo(int idx, String foo)
>
> OK I guess the question that stands out is, if I'm right on this how
> come your test page worked with incorrectly defined indexed
> properties. My guess is that JSTL is smart enough to handle simple
> properties for arrays and Lists, but if thats true, the fact that your
> page didn't work after my change means that its also too dumb to use
> the indexed read/write methods.
>
> Course I could have this wrong as this is all theory and I haven't put
> any effort into actually proving it.
>
> Niall
Right, that all makes sense. I'll add some tracing to see what JSTL is
actually trying to invoke in each case, and do some testing w/ a
non-dynamic form bean for comparison.
L.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: CGLib DynaActioForm problem (was [VOTE] Confirm the Struts Action Library 1.3.0 release plan)
Posted by Martin Cooper <ma...@apache.org>.
On 11/30/05, Laurie Harper <la...@holoweb.net> wrote:
>
> Niall Pemberton wrote:
> > On 11/30/05, Laurie Harper <la...@holoweb.net> wrote:
> >> Niall Pemberton wrote:
> >>> On 11/29/05, Niall Pemberton <ni...@gmail.com> wrote:
> >>>> On 11/29/05, Laurie Harper <la...@holoweb.net> wrote:
> >>>>> Niall Pemberton wrote:
> >>>>>> Also looked like the property types were the wrong way round for
> >>>>>> indexed methods, so I also switched them.
> >>>>> Hmm, with that change, accessing a List property is broken. Without
> the
> >>>>> change, all my tests are working. You can deploy the exercises app
> with
> >>>>> the addition I just committed and see for yourself:
> >>>>>
> http://localhost:8080/struts-examples/exercise/enhanced-dynabean.do
> >>>>>
> >>>>> I've rolled back your change and everything works fine again,
> including
> >>>>> indexed property access for arrays and lists. The unit test you
> added
> >>>>> passes either way.
> >>>> I just refreshed and the JUnit test I wrote is now failing for me.
> >>> Its failing in gump as well
> >>>
> http://vmgump.apache.org/gump/public/struts/struts-action/gump_work/build_struts_struts-action.html
> >>>
> >>> I've had problems with the maven build - when I've used build-all if a
> >>> sub-project test fails maven just carries on ignoring it and it
> >>> appears you get a successful build. Now I do the sub-projects
> >>> separately.
> >>>
> >>> I'm wondering if thats what you used?
> >> I was using 'maven test' without problem. It seems my checkout was
> >> warped, though, because even though svn was telling me it was
> >> up-to-date, after an 'svn update' I'm now seeing the failure.
> >
> > This doesn't make any sense to me, unless you commit first and test
> after?
> >
> >> I'll look at it tomorrow and figure out what's wrong.
> >
> > OK heres my take on what the issue is. The java bean specification
> > defines indexed properties in relation to arrays, so if you have
> > methods like...
> >
> > public String[] getFoo() // simple read
> > public void setFoo(String[] foo) // simple write
> > public String getFoo(int idx) // indexed read
> > public void setFoo(int idx, String foo) // indexed write
> >
> > ...it will create an IndexedPropertyDescriptor instead of a
> > PropertyDescriptor for the Foo property. In previous JDK versions
> > (i.e. pre JDK 1.4 I believe) Sun was lenient about the simple
> > properties - it didn't care about the parameter/return type, so if you
> > defined your simple read/write methods with a java.util.List, it still
> > worked. However in JDK 1.4 they changed things, so that if the simple
> > read/write didn't return an array then it would still create an
> > IndexedPropertyDescriptor, but the getReadMethod/getWriteMethod return
> > null, effectively masking the simple read/write methods.
> >
> > In DynaBeanInterceptor the indexed read/write methods it was creating
> > were incorrectly defined and so it wasn't recognizing those as indexed
> > properties (and your List worked). I believe my change fixed this
> > issue, and effectively masked the simple read/write methods for the
> > indexed property defined with a List, although I don't understand why
> > your test page didn't work because of this, since JSTL should have
> > been able to use the indexed read/write methods.
> >
> > It would be interesting to see what happens with your test page with a
> > "regular" incorrectly defined indexed property, i.e.
> >
> > public List getFoo()
> > public void setFoo(List foo)
> > public String getFoo(int idx)
> > public void setFoo(int idx, String foo)
> >
> > OK I guess the question that stands out is, if I'm right on this how
> > come your test page worked with incorrectly defined indexed
> > properties. My guess is that JSTL is smart enough to handle simple
> > properties for arrays and Lists, but if thats true, the fact that your
> > page didn't work after my change means that its also too dumb to use
> > the indexed read/write methods.
> >
> > Course I could have this wrong as this is all theory and I haven't put
> > any effort into actually proving it.
> >
> > Niall
>
> OK, so here's what's going on: JSTL always uses the simple read/write
> methods, never the indexed methods. As you surmise, in this case
> getReadMethod / getWriteMethod return null. Since JSTL (or at least the
> Jakarta Taglibs impl ;-) depends on the simple getter, this causes
> things to break.
>
> So it looks like the solution is to either (a) not generate the indexed
> getter/setter at all, as was the case with the original code; or (b)
> only do so for array-typed properties; or (c) dynamically convert
> List-typed properties into array properties.
>
> (c) isn't attractive; if I define a bean property of type List, I expect
> to be able to access it as such. It does seem worthwhile providing the
> indexed getter/setter for array-typed properties, so I think (b) would
> be the way to go.
+1 for (b).
--
Martin Cooper
I'll give it a try now and see it that does the trick.
>
> L.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
Re: CGLib DynaActioForm problem (was [VOTE] Confirm the Struts Action
Library 1.3.0 release plan)
Posted by Laurie Harper <la...@holoweb.net>.
Niall Pemberton wrote:
> On 11/30/05, Laurie Harper <la...@holoweb.net> wrote:
>> Niall Pemberton wrote:
>>> On 11/29/05, Niall Pemberton <ni...@gmail.com> wrote:
>>>> On 11/29/05, Laurie Harper <la...@holoweb.net> wrote:
>>>>> Niall Pemberton wrote:
>>>>>> Also looked like the property types were the wrong way round for
>>>>>> indexed methods, so I also switched them.
>>>>> Hmm, with that change, accessing a List property is broken. Without the
>>>>> change, all my tests are working. You can deploy the exercises app with
>>>>> the addition I just committed and see for yourself:
>>>>> http://localhost:8080/struts-examples/exercise/enhanced-dynabean.do
>>>>>
>>>>> I've rolled back your change and everything works fine again, including
>>>>> indexed property access for arrays and lists. The unit test you added
>>>>> passes either way.
>>>> I just refreshed and the JUnit test I wrote is now failing for me.
>>> Its failing in gump as well
>>> http://vmgump.apache.org/gump/public/struts/struts-action/gump_work/build_struts_struts-action.html
>>>
>>> I've had problems with the maven build - when I've used build-all if a
>>> sub-project test fails maven just carries on ignoring it and it
>>> appears you get a successful build. Now I do the sub-projects
>>> separately.
>>>
>>> I'm wondering if thats what you used?
>> I was using 'maven test' without problem. It seems my checkout was
>> warped, though, because even though svn was telling me it was
>> up-to-date, after an 'svn update' I'm now seeing the failure.
>
> This doesn't make any sense to me, unless you commit first and test after?
>
>> I'll look at it tomorrow and figure out what's wrong.
>
> OK heres my take on what the issue is. The java bean specification
> defines indexed properties in relation to arrays, so if you have
> methods like...
>
> public String[] getFoo() // simple read
> public void setFoo(String[] foo) // simple write
> public String getFoo(int idx) // indexed read
> public void setFoo(int idx, String foo) // indexed write
>
> ...it will create an IndexedPropertyDescriptor instead of a
> PropertyDescriptor for the Foo property. In previous JDK versions
> (i.e. pre JDK 1.4 I believe) Sun was lenient about the simple
> properties - it didn't care about the parameter/return type, so if you
> defined your simple read/write methods with a java.util.List, it still
> worked. However in JDK 1.4 they changed things, so that if the simple
> read/write didn't return an array then it would still create an
> IndexedPropertyDescriptor, but the getReadMethod/getWriteMethod return
> null, effectively masking the simple read/write methods.
>
> In DynaBeanInterceptor the indexed read/write methods it was creating
> were incorrectly defined and so it wasn't recognizing those as indexed
> properties (and your List worked). I believe my change fixed this
> issue, and effectively masked the simple read/write methods for the
> indexed property defined with a List, although I don't understand why
> your test page didn't work because of this, since JSTL should have
> been able to use the indexed read/write methods.
>
> It would be interesting to see what happens with your test page with a
> "regular" incorrectly defined indexed property, i.e.
>
> public List getFoo()
> public void setFoo(List foo)
> public String getFoo(int idx)
> public void setFoo(int idx, String foo)
>
> OK I guess the question that stands out is, if I'm right on this how
> come your test page worked with incorrectly defined indexed
> properties. My guess is that JSTL is smart enough to handle simple
> properties for arrays and Lists, but if thats true, the fact that your
> page didn't work after my change means that its also too dumb to use
> the indexed read/write methods.
>
> Course I could have this wrong as this is all theory and I haven't put
> any effort into actually proving it.
>
> Niall
OK, so here's what's going on: JSTL always uses the simple read/write
methods, never the indexed methods. As you surmise, in this case
getReadMethod / getWriteMethod return null. Since JSTL (or at least the
Jakarta Taglibs impl ;-) depends on the simple getter, this causes
things to break.
So it looks like the solution is to either (a) not generate the indexed
getter/setter at all, as was the case with the original code; or (b)
only do so for array-typed properties; or (c) dynamically convert
List-typed properties into array properties.
(c) isn't attractive; if I define a bean property of type List, I expect
to be able to access it as such. It does seem worthwhile providing the
indexed getter/setter for array-typed properties, so I think (b) would
be the way to go.
I'll give it a try now and see it that does the trick.
L.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org