commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <>
Subject RE: [betwixt] AddDefaultsRule bug in 0.7RC2
Date Wed, 13 Jul 2005 19:43:22 GMT
On Mon, 2005-07-11 at 16:22 -0500, Glenn Goldenberg wrote:
> Hey robert:

hi glenn

> After fixing the "add-adder" bug, I ran into an issue using "updater" in the ElementRule
for a property of type Map 
> and another issue with "add-adder" and mixed Collection property types.

i've committed the fix (together with a test case) to HEAD. (needed some
time to discuss whether the fix belongs in HEAD or in the 0.7 release.)

you help's appreciated. if (in future) you want to help to speed the
process of application, please read and consider creating
some patches (once you get the hang of them, you'll find them easier
than describing things in words). if you find your emails are getting a
little long, then you could open a report in (if you like) and then follow up on
list .  

> The first issue with a Map property is illustrated by this TestBetwixtMapUpdater test
case that follows.  
> The issue is that if "add-adder" is false then the "updater" that is specified for a
Map property is ignored.  
> This issue can't been seen without applying the previous fix for honoring the "add-adder"


> I debugged into the ElementRule class and there was no logic to handle adding a Map "updater".

it's probably on the (very long) to do list...

> I was able to path this with the following changes:
> in XMLIntrospector, line 959, change visibility of setMapAdder() to public:
> 	public void setMapAdder(
> in ElementRule, line 237, update configureProperty() in two places to handle Map adders:
> 	...
> 		if ( updateMethodName.equals( method.getName() ) ) {
>                     // we have a matching name
>                     // updater should have one parameter unless type is Map
>                     int numParams = 1;
>                     if (Map.class.isAssignableFrom(type)) {
>                     	// updater for Map should have two parameters
>                     	numParams = 2;
>                     }
>                     if (methods[i].getParameterTypes().length == numParams) {
>                         // we'll use first match
>                         updateMethod = methods[i];
>                         if ( log.isTraceEnabled() ) {
>                             log.trace("Matched method:" + updateMethod);
>                         } 
>                         // done since we're using the first match
>                         break;
>                     }
>                 }
> 	...
> 		if (updateMethod == null) {
>                 if ( log.isInfoEnabled() ) {
>           "No method with name '" + updateMethodName + "' found for
>                 }
>             } else {
>     			// assign updater to elementDescriptor
> 				if (Map.class.isAssignableFrom(type)) {
> 					Map elementsByPropertyName = new HashMap();
> 					elementsByPropertyName.put(propertyDescriptor.getName(), elementDescriptor);
> 					getXMLIntrospector().setMapAdder(elementsByPropertyName, updateMethod);
> 				} else {
> 	                elementDescriptor.setUpdater( new MethodUpdater( updateMethod ) );
> 	                elementDescriptor.setSingularPropertyType( updateMethod.getParameterTypes()[0]
> 	                if ( log.isTraceEnabled() ) {
> 	                    log.trace( "Set custom updater on " + elementDescriptor);
> 	                }
> 	            }
>             }
> With these changes, the TestBetwixtMapUpdater test passes (along with all other tests).
> It seemed easy to change the visibility of setMapAdder() and reuse that logic in the
> but I don't know if that is the best solution...

reusing the logic is important but so is keeping a compact API. thanks
for pinpointing the issue. i'll probably have a think and maybe post
something on dev... 

> The other issue is that using addDefaults in a class where you want to display a mixed
> causes the mixed collection elements to not display properly.  It seems that if an ElementRule
> specified with no name attribute and a mixed Collection property, than the output XML
only has 
> element tags based on the collection object class type if "add-adders" is false.  It
looks like 
> when adding default Adders, betwixt overwrites the settings for mixed collection properties

sounds about right. one of the basic design issues is that adding
defaults also performs normalization but it's not easy to fix whilst
retaining good backwards compatibility. 

> If you comment out the addChildBean() method, then "add-adders='true'" works, 
> but the beanReader won't be able to convert the xml back into the bean.  
> Here is the result from TestBetwixtMixedCollection which is included below as well:
> with <addDefaults add-properties='true' add-adders='true'/>:
>  <?xml version="1.0"?>
>   <parentBean stuff="stuff" id="1">
>     <childBeans>
>       <childBean/>
>       <childBean/>
>     </childBeans>
>   </parentBean>
> with <addDefaults add-properties='true' add-adders='false'/>:
>  <?xml version="1.0"?>
>   <parentBean id="1">
>     <childBeans>
>       <childBean1/>
>       <childBean2/>
>     </childBeans>
>   </parentBean>
> Please let me know if I'm missing something around these two issues...

i'll need a little time to think on this (and maybe discuss on dev).

BTW are you happy to contribute your test cases (which i've deleted from
the bottom of this mail) to apache? (saves me having to create fresh
ones.) just FYI if you attach the apache software license 2.0 to the top
of any new classes you post then i won't have to ask ;)

- robert

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message