commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 30799] - [configuration] Set wrong property when call XMLConfiguration.setProperty
Date Wed, 29 Sep 2004 07:56:01 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=30799>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

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

[configuration] Set wrong property when call XMLConfiguration.setProperty





------- Additional Comments From rwinston@eircom.net  2004-09-29 07:56 -------
The bug seems to be that setXmlProperty() sets the value twice - once in
addXmlProperty(), and *then* in setXmlProperty(). The second time, it just
blindly appends the textual Property value to the parent node, when of course,
an existing text node has already been created in addXmlProperty(), giving
<a>foofoo</a> instead of <a>foo</a>

Perhaps setXmlProperty should clear the property before setting it to its new
value. This however, still means that the attribute gets initialized twice,
which feels like a "code smell", I think you'll agree.

Here is one way to do that , I would urge you to review this and then look for
alternative/more efficient mechanisms if they are available - I wrote this in a
hurry :)

private void setXmlProperty(String name, Object value)
    {
    
...


        if (attName == null)
        {
            // Should remove all existing text nodes before appending a new one
            CharacterData data = document.createTextNode(String.valueOf(value));
            logger.info("Appending " + data.getData() + " to " + element);
            
            Node node = null;
            NodeList children = element.getChildNodes();
            for(int i = 0; i < children.getLength(); ++i) {
            	
            	if((node = (Node)children.item(i)).getNodeType() == Node.TEXT_NODE) {
            		logger.info("Found an existing text node [ "+ node + "] under " +
element + ", removing...");
            		element.removeChild(node);
            	}
            }
            
            element.appendChild(data);  
            logger.info("Child element # of " + element + " is now " +
element.getChildNodes().getLength());
  
        }
...

}


This produces the expected results (at least for my testcase, anyways). I think
a new TestCase should be written to regression-test this bug.

This also brings me to an issue I have with the Configuration codebase, and one
I am frequently guilty of myself - where is the logging?? If there was a
consistent level of logging across the codebase, bugs like this would be *much*
easier to track down.

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


Mime
View raw message