You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Laurie Harper <la...@holoweb.net> on 2005/09/24 06:48:10 UTC

Proposed DynaActionForm enhancement

http://issues.apache.org/bugzilla/show_bug.cgi?id=36794

I just posted the above enhancement request (with a patch to implement 
it, of course ;-). This adds an external dependency on CGLIB; I don't 
know if that will fly, but personally I think the benefits are worth it. 
Read on to see if you agree:

Summary: remove the need to use xxx.map['property'] syntax with 
DynaActionForms

Motivation: apart from it being bothersome to have to use a special 
expression syntax for working with DynaBean based action forms, it also 
makes life rather painful when creating re-usable components that 
interact with form beans. While this isn't a common requirement for 
straight JSPs, users of JSP 2.0 tag files are much more likely to run 
into this. With the special-syntax requirement, tag files (and JSPs) 
must go to additional effort to work with both dyna beans and regular beans.

Result: DynaBean based action forms can be accessed in JSPs and tag 
files exactly the same as regular beans. There is no longer any need to 
use the .map['property'] construct except where the dyna property names 
are grossly non-compliant with JavaBeans naming conventions (for 
example, property names containing spaces, which are currently allowed!).

Here are some examples of things that are equivalent with this patch 
applied:

	${MyForm.map['someProperty']}
	${MyForm.map.someProperty}
	${MyForm['someProperty']}
	${MyForm.someProperty}

	<html:input property="map['someProperty']"/>
	<html:input property="map.someProperty"/>
	<html:input property="someProperty"/>

	${MyForm.map['indexProperty'][0]}
	${MyForm['indexProperty'][0]}
	${MyForm.indexProperty[0]}

	${MyForm.map['mapProperty']['something']}
	${MyForm['mapProperty']['something']}
	${MyForm.mapProperty['something']}

Impact: the patch requires an additional external library (which is 
licensed under the terms of the ASL 2.0) but otherwise there should be 
no significant impact. The existing property access syntax is preserved 
unchanged, so existing JSPs / tag files / code will continue to work 
unchanged. The only possible issue I'm aware of so far would be in the 
case of DynaActionForm sub-classes which already define getter and/or 
setter methods for one or more properties declared in struts-config.xml. 
If this proves to be a problem, it would be easy to make the code detect 
this situation and not attempt to generate those methods dynamically.

Notes: achieving the same thing without modifying Struts Core is 
somewhat non-trivial and cannot be done transparently. Some small code 
changes in Struts Core would allow this functionality to be made 
available as an extension (in contrib or as a seperate project on SF.net 
or something). If adding a dependency on CGLIB is considered too high a 
price to pay, we can discuss the options for making that possible ;-)


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


Re: Proposed DynaActionForm enhancement

Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Thanks for the explanation :)  It does seem like a nice enhancement, I 
hope it will be considered.

Frank

Laurie Harper wrote:
> The patch is actually modifying DynaActionFormClass -- the class that 
> provides the factory method Struts uses to instantiate a DynaActionForm. 
> There's on direct way to tell Struts to use a different implementation 
> of this class. So, to do this as an extension, you have to:
> 
> 1) configure a custom ActionFormConfig in the <form-beans> element, 
> which will be used to construct all form bean configs
> 
> 2) in that class, override getDynaClass to return a custom 
> DynaActionFormClass implementation with this functionality
> 
> 3) sub-class DyanActionForm and all its children, to expose 
> setDynaClass() so it can be called by the custom DynaActionFormClass 
> implementation
> 
> 4) change all form bean definitions that use DyanActionForm or 
> subclasses to reference the copies with exposed setDyanClass()
> 
> You end up with about half a dozen classes, most of them trivial, and 
> have to configure every form bean differently to use the resulting 
> functionality.
> 
> As I say, a few minor tweaks would allow this to be dropped in as an 
> extension so that all existing uses of DynaActionForm and friends would 
> automatically get the added functionality.
> 
> L.
> 
> Frank W. Zammetti wrote:
> 
>> Laurie, I'm curious... looking at the patch it seems like your only
>> modifying DynaActionForm, correct?  If that's true, since you can set the
>> class for an ActionForm in struts-config, what makes it so that you can't
>> just set LauriesDynaActionForm instead of DynaActionForm?  Including 
>> CGLIB
>> in your own app obviously isn't a problem, right?
>>
>> I'm only asking because I think it's an interesting enhancement, but I
>> know the standard answer to things like this has been to make it an 
>> add-on
>> rather than add it to core and I'm trying to understand what makes it
>> non-trivial to go that route?
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
> 
> 
> 
> 
> 

-- 
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com


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


Re: Proposed DynaActionForm enhancement

Posted by Laurie Harper <la...@holoweb.net>.
The patch is actually modifying DynaActionFormClass -- the class that 
provides the factory method Struts uses to instantiate a DynaActionForm. 
There's on direct way to tell Struts to use a different implementation 
of this class. So, to do this as an extension, you have to:

1) configure a custom ActionFormConfig in the <form-beans> element, 
which will be used to construct all form bean configs

2) in that class, override getDynaClass to return a custom 
DynaActionFormClass implementation with this functionality

3) sub-class DyanActionForm and all its children, to expose 
setDynaClass() so it can be called by the custom DynaActionFormClass 
implementation

4) change all form bean definitions that use DyanActionForm or 
subclasses to reference the copies with exposed setDyanClass()

You end up with about half a dozen classes, most of them trivial, and 
have to configure every form bean differently to use the resulting 
functionality.

As I say, a few minor tweaks would allow this to be dropped in as an 
extension so that all existing uses of DynaActionForm and friends would 
automatically get the added functionality.

L.

Frank W. Zammetti wrote:
> Laurie, I'm curious... looking at the patch it seems like your only
> modifying DynaActionForm, correct?  If that's true, since you can set the
> class for an ActionForm in struts-config, what makes it so that you can't
> just set LauriesDynaActionForm instead of DynaActionForm?  Including CGLIB
> in your own app obviously isn't a problem, right?
> 
> I'm only asking because I think it's an interesting enhancement, but I
> know the standard answer to things like this has been to make it an add-on
> rather than add it to core and I'm trying to understand what makes it
> non-trivial to go that route?
> 


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


Re: Proposed DynaActionForm enhancement

Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Laurie, I'm curious... looking at the patch it seems like your only
modifying DynaActionForm, correct?  If that's true, since you can set the
class for an ActionForm in struts-config, what makes it so that you can't
just set LauriesDynaActionForm instead of DynaActionForm?  Including CGLIB
in your own app obviously isn't a problem, right?

I'm only asking because I think it's an interesting enhancement, but I
know the standard answer to things like this has been to make it an add-on
rather than add it to core and I'm trying to understand what makes it
non-trivial to go that route?

-- 
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com

On Thu, September 29, 2005 3:31 pm, Laurie Harper said:
> Anyone get a chance to look at this? I plan to use this functionality
> one way or another, and right now I either have to deploy a locally
> modified Struts build or use some pretty ugly hacks to bolt it onto an
> unmodified Struts.
>
> If getting this into Struts Core isn't going to be an option, I'd like
> to propose some alternative (fairly minor) changes that would allow this
> to be an easy-to-deploy extension.
>
> L.
>
> Laurie Harper wrote:
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=36794
>>
>> I just posted the above enhancement request (with a patch to implement
>> it, of course ;-). This adds an external dependency on CGLIB; I don't
>> know if that will fly, but personally I think the benefits are worth it.
>> Read on to see if you agree:
>>
>> Summary: remove the need to use xxx.map['property'] syntax with
>> DynaActionForms
>>
>> Motivation: apart from it being bothersome to have to use a special
>> expression syntax for working with DynaBean based action forms, it also
>> makes life rather painful when creating re-usable components that
>> interact with form beans. While this isn't a common requirement for
>> straight JSPs, users of JSP 2.0 tag files are much more likely to run
>> into this. With the special-syntax requirement, tag files (and JSPs)
>> must go to additional effort to work with both dyna beans and regular
>> beans.
>>
>> Result: DynaBean based action forms can be accessed in JSPs and tag
>> files exactly the same as regular beans. There is no longer any need to
>> use the .map['property'] construct except where the dyna property names
>> are grossly non-compliant with JavaBeans naming conventions (for
>> example, property names containing spaces, which are currently
>> allowed!).
>>
>> Here are some examples of things that are equivalent with this patch
>> applied:
>>
>>     ${MyForm.map['someProperty']}
>>     ${MyForm.map.someProperty}
>>     ${MyForm['someProperty']}
>>     ${MyForm.someProperty}
>>
>>     <html:input property="map['someProperty']"/>
>>     <html:input property="map.someProperty"/>
>>     <html:input property="someProperty"/>
>>
>>     ${MyForm.map['indexProperty'][0]}
>>     ${MyForm['indexProperty'][0]}
>>     ${MyForm.indexProperty[0]}
>>
>>     ${MyForm.map['mapProperty']['something']}
>>     ${MyForm['mapProperty']['something']}
>>     ${MyForm.mapProperty['something']}
>>
>> Impact: the patch requires an additional external library (which is
>> licensed under the terms of the ASL 2.0) but otherwise there should be
>> no significant impact. The existing property access syntax is preserved
>> unchanged, so existing JSPs / tag files / code will continue to work
>> unchanged. The only possible issue I'm aware of so far would be in the
>> case of DynaActionForm sub-classes which already define getter and/or
>> setter methods for one or more properties declared in struts-config.xml.
>> If this proves to be a problem, it would be easy to make the code detect
>> this situation and not attempt to generate those methods dynamically.
>>
>> Notes: achieving the same thing without modifying Struts Core is
>> somewhat non-trivial and cannot be done transparently. Some small code
>> changes in Struts Core would allow this functionality to be made
>> available as an extension (in contrib or as a seperate project on SF.net
>> or something). If adding a dependency on CGLIB is considered too high a
>> price to pay, we can discuss the options for making that possible ;-)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>


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


Re: Proposed DynaActionForm enhancement

Posted by Laurie Harper <la...@holoweb.net>.
Anyone get a chance to look at this? I plan to use this functionality 
one way or another, and right now I either have to deploy a locally 
modified Struts build or use some pretty ugly hacks to bolt it onto an 
unmodified Struts.

If getting this into Struts Core isn't going to be an option, I'd like 
to propose some alternative (fairly minor) changes that would allow this 
to be an easy-to-deploy extension.

L.

Laurie Harper wrote:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=36794
> 
> I just posted the above enhancement request (with a patch to implement 
> it, of course ;-). This adds an external dependency on CGLIB; I don't 
> know if that will fly, but personally I think the benefits are worth it. 
> Read on to see if you agree:
> 
> Summary: remove the need to use xxx.map['property'] syntax with 
> DynaActionForms
> 
> Motivation: apart from it being bothersome to have to use a special 
> expression syntax for working with DynaBean based action forms, it also 
> makes life rather painful when creating re-usable components that 
> interact with form beans. While this isn't a common requirement for 
> straight JSPs, users of JSP 2.0 tag files are much more likely to run 
> into this. With the special-syntax requirement, tag files (and JSPs) 
> must go to additional effort to work with both dyna beans and regular 
> beans.
> 
> Result: DynaBean based action forms can be accessed in JSPs and tag 
> files exactly the same as regular beans. There is no longer any need to 
> use the .map['property'] construct except where the dyna property names 
> are grossly non-compliant with JavaBeans naming conventions (for 
> example, property names containing spaces, which are currently allowed!).
> 
> Here are some examples of things that are equivalent with this patch 
> applied:
> 
>     ${MyForm.map['someProperty']}
>     ${MyForm.map.someProperty}
>     ${MyForm['someProperty']}
>     ${MyForm.someProperty}
> 
>     <html:input property="map['someProperty']"/>
>     <html:input property="map.someProperty"/>
>     <html:input property="someProperty"/>
> 
>     ${MyForm.map['indexProperty'][0]}
>     ${MyForm['indexProperty'][0]}
>     ${MyForm.indexProperty[0]}
> 
>     ${MyForm.map['mapProperty']['something']}
>     ${MyForm['mapProperty']['something']}
>     ${MyForm.mapProperty['something']}
> 
> Impact: the patch requires an additional external library (which is 
> licensed under the terms of the ASL 2.0) but otherwise there should be 
> no significant impact. The existing property access syntax is preserved 
> unchanged, so existing JSPs / tag files / code will continue to work 
> unchanged. The only possible issue I'm aware of so far would be in the 
> case of DynaActionForm sub-classes which already define getter and/or 
> setter methods for one or more properties declared in struts-config.xml. 
> If this proves to be a problem, it would be easy to make the code detect 
> this situation and not attempt to generate those methods dynamically.
> 
> Notes: achieving the same thing without modifying Struts Core is 
> somewhat non-trivial and cannot be done transparently. Some small code 
> changes in Struts Core would allow this functionality to be made 
> available as an extension (in contrib or as a seperate project on SF.net 
> or something). If adding a dependency on CGLIB is considered too high a 
> price to pay, we can discuss the options for making that possible ;-)


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