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 http://jakarta.apache.org/site/getinvolved.html 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 http://issues.apache.org/bugzilla/ (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" attribute. yep > 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() ) { > > log.info("No method with name '" + updateMethodName + "' > found for update"); > } > } 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 ElementRule, > 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 collection > causes the mixed collection elements to not display properly. It seems that > if an ElementRule is > 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 somehow. 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
