Niall Pemberton wrote:
Craig, thanks, I appreciate your feedback.
Its not an issue I have a strong opinion on - all I'm interested in is helping resolve the outstanding beanutils bugs to move it nearer a release. The question then is whats the best course of action in relation to these bugs?
The principle of "if the object you pass implements Map, it is *always* treated as a Map" seems sound to me and I'm happy to change the fixes I've worked on to do that. The issues I see are:
A) setNestedProperty already contains the behaviour you are against (getNestedProperty was inconsistent and doesn't) - PropertyUtils was modified back in Nov 2002 (version 1.34) under Bug 14440 so that if the bean was a Map it first checked for a regular property before setting it in the Map. So the current Struts 1.1 behaviour is based on that. I guess there are three options:
1) Change getNestedProperty to implement the same behaviour - what I was proposing, but you are against 2) Remove this behaviour from setNestedProperty - this fits in with the principle you stated, but since Struts 1.1 is operating with this version seems to go against what you were saying about being backward compatible. 3) Do nothing, leaving the inconsistency - this is the one option I'm dead against
B) The other part of the change was to the get/setSimpleProperty - they currently only update "regular" bean properties and don't handle Maps. Bug 26904 asked that they did. Now I'm not sure whether you are against this part of the change or not and I guess we have three options here as well:
1) Do nothing - don't handle Maps (but this breaks the principle of if its a Map then ignore regular properties) 2) Change get/setSimpleProperty as I was proposing (and what the bug asked for) which is if there isn't a simple property and its a Map, then set the Map's value. 3) Change get/setSimpleProperty so that it first checks if the bean is a Map before looking for a regular property - that fits your principle but breaks backward compatibility.
The more I write this email the more I like the principle, because its straight forward to explain/understand. However we would have to alter setNestedProperty to not set regular bean properties and change get/setSimpleProperty to check if its a Map first before setting the regaular property. I like this idea but the big issue would be backwards compatibility.
I gues the pragmatic approach is just to alter setNestedProperty to not set regular bean properties - makes it consistent with getNestedProperty and fits the principle - but leaves get/setSimpleProperty inconsistent with get/setNestedProperty.
I guess I'm for the pargmatic option - because I'm just think of the flood of emails/bugs with doing the whole thing, what do you think?
That's probably the best bet for now :-(.
Maybe, as we go towards beanification, we can establish standard instances that operate in the sorts of ways that people might want for particular kinds of backwards compatibility. But we should really start thinking of a 2.0 sort of jump, and establish a new default set of behaviors that are internally consistent.
Craig
Niall
----- Original Message ----- From: "Craig McClanahan" <[EMAIL PROTECTED]>
To: "Jakarta Commons Developers List" <[EMAIL PROTECTED]>
Sent: Monday, July 12, 2004 4:37 AM
Subject: Re: [beanutils] Summary of propsed/committed changes for Bugs
concernsNiall Pemberton wrote:
4) Bug 23815, 26904 and 29571
Bug 23815 PropertyUtils.getNestedProperty() doesn't allow getXxxx on Map-Instances any longer Bug 26904 (g|s)etSimpleProperty fails when passing a Map Bug 29571 Inconsistent behaviour of BeanUtils.setProperty() and BeanUtils.getProperty() methods http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23815 http://nagoya.apache.org/bugzilla/show_bug.cgi?id=26904 http://nagoya.apache.org/bugzilla/show_bug.cgi?id=29571
These bugs all related to the same issue. Craig McClanahan posted
in one of the bugs that what was being requested would break backward compatibility. I believe that the change in behaviour I'm proposing satisfies both camps and resolves the inconsistencies between some of the methods. I've documented on Bug 23815 the changes in behaviour to PropertyUtilsBean I'm proposing.
Niall,
Thanks for taking the time to propose a solution to this problem, as well as all the work you've done on the other BeanUtils issues. As I just posted in the comments on bug 23815, I'm deeply concerned that changing this behavior will break Struts 1.1 apps (yes, I know that people only now converting from Struts 1.0 would find it easier; but it's the Struts 1.0 behavior that was badly broken). What's worse is the fact that this isn't the only case where a map is treated specially -- exactly the same thing happens with the standard implementations of dynabeans (including DynaActionForm in Struts 1.1).
The standard implementations of expression evaluation (JSTL 1.0/1.1, JSP 2.0, JSF 1.0/1.1) all conform to the way that BeanUtils currently works -- if the object you pass implements Map, it is *always* treated as a Map. This is further evidence that the right path for default behavior lies in maintaining what we currently have, not going back to what was broken.
For making the Struts 1.0 transition easier, I'd be +1 on setting up a BeanUtilsFactory that allows you to set the characteristics of the BeanUtils instance to be created (somewhat like what JAXP does to set up a parser instance that is either validating or non-validating, for example). But I'm -1 on changing the default behavior of these methods until we do the beanification step.
Craig
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
