XMLConfiguration addNodes() not behaving correctly

• Key: CONFIGURATION-328
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Oliver Heger
• Reporter: vivek
• Affects Version/s: 1.5
• Fix Version/s: 1.6

Description

Things used to work fine in 1.3, but now when I'm trying 1.5 my JUnit test suite is breaking when adding new properties. Here is what I'm doing,

1) I already have a configuration as following,

<test>
  <property name="isOk">
    <value>true</value>
    <default>false</default>
  </property>
  <property name="intProperty">
    <value>900</value>
    <default>500</default>
  </property>
  <property extra="0" name="stringProperty">
    <default>Bye</default>
  </property>
</test>

2) Now I need to add two new properties under <test>,

<property  name="newFirst">
  <value>first</value>
</property>
<property name="newSecond">
  <value>second</value>
</property>

3) Here is the code I'm using,

a) First create a new HierarchicalConfiguration.Node

private HierarchicalConfiguration.Node createNode(String name, Object value)  {
    HierarchicalConfiguration.Node node = new HierarchicalConfiguration.Node(name);
    node.setValue(value);
    return node;
}

b) Add the node to the list

List<HierarchicalConfiguration.Node> attrNodes = new ArrayList<HierarchicalConfiguration.Node>();
Node attrNode = createNode(attrName, newPropertyName);
attrNode.setAttribute(true);
attrNodes.add(attrNode);

c) Call addNodes on XMLConfiguration,

conf.addNodes(attrPath, attrNodes);

I run this in a look for each new property I need to add. For first property, I get element count for the test.property as 3 (conf.getMaxIndex(key)) - so I insert at test.property(3), which is right, but for the second property I get element count as 5 --> This is wrong, it should have been 4 (because I just added 1).

I check the saved the configuration file after adding those two new properties and it looks wrong too. Here is what I get,

<test>
  <property name="isOk">
    <value>true</value>
    <default>false</default>
  </property>
  <property name="intProperty">
    <value>900</value>
    <default>500</default>
  </property>
  <property extra="0" name="stringProperty">
    <value>Hi</value>
    default>Bye</default>
  </property>
  <property>
    <name>newFirst</name>
  </property>
  <property>
    <value>first</value>
  </property>
  <property>
    <name>newSecond</name>
  </property>
  <property>
    <value>second</value>
  </property>
</test>

The total element count for test.property gives me 7 ==> I was expecting 5.

There seems to have some code change in 1.5 releated to addNodes (for ex., CONFIGURATION-287). I'm not sure if I need to change the way I was calling addNodes before. It works fine with 1.3, but 1.5 completely fails on adding new properties.

I do need 1.5 for other bug fixes ( CONFIGURATION-268 and some other), but I can't use it until the addNodes behave correctly.

Marking this as blocker as I'm blocked because of this issue. If there is a workaround then please let me know.

Comments

[vivek]

Looks like the attribute is getting added as new tag,

<property>
     <name>newFirst</name>
</property>
<property>
     <value>first</value>
</property>

is supposed to be,

<property name="newFirst">
     <value>first</value>
 </property>

In my code,

1) I first add the <property> tag under the <test> element - by calling conf.addNodes
2) Then I add the name attribute to the newly added property - again by calling conf.addNodes
3) Then I add the value tag under property - again by calling conf.addNodes

I do create a new HierarchicalConfiguration.Node every time to add it.

It all used to work fine in 1.3, but it's failing in 1.5.

Note, my reloading strategy is false - not sure if that has anything to do with this.

[vivek]

Emmanuel,

Could you let me know if there is any workaround for this issue or if I can do things differently to make it work? It would be nice if you can give me a patch that I can apply to my local build or let me know what needs to be changed in the XMLConfiguration so I can make my own build.

Thanks,
-vivek

[Oliver Heger]

Vivek, can you please post your complete code for adding the new properties? Based on the fragments you provided I don't really see the parameters (especially the keys) you pass into the addNodes() method.

[vivek]

Let's say we have an existing xml configuration,

<ss>
 <test>
    <property name="testProperty">
         <value>test</value>
    <property>
 </test>
</ss>

Now I want add a new property under existing <test> tag,

<property name="vivekProperty">
   <value>vivek</value>
 </property>

I do following,

1) create a node,

HierarchicalConfiguration.Node node = new HierarchicalConfiguration.Node("property");
  node.setValue(null);

2) Find the path where the "property" tag needs to be added. In my case it's "ss.test(0)"

3) I call addNodes on XMLConfiguration instance,

conf.addNodes(propertyPath, propertyNode);     
    //where propertyPath is ss.test(0) and propertyNode is one created in step 1

4) I check how many property tags are under ss.test(0) - in this case it returns me 2 (1 was just added)

5) I create another node object for the name attribute of property,

HierarchicalConfiguration.Node node = new HierarchicalConfiguration.Node("name");
  node.setValue("vivekProperty);
  node.setAttribute(true);             //setting the node as attribute

6) Find the path where this attribute needs to be added. In this case it's "ss.test(0).property(1)"

7) Add the node,

conf.addNodes(attrPath, attrNode);     
    //where attrPath is ss.test(0).property(1) and attrNode is one created in step 5

8) Create another node for the value tag under property,

HierarchicalConfiguration.Node node = new HierarchicalConfiguration.Node("value");
  node.setValue(""vivek");

9) Now, when I try to find the index of the newly added property ("property name="vivekProperty") I get -1. So instead of "ss.test(0).property(1)", I get "ss.test(0).property(-1)". -1 in my code basically means it couldn't find that. <=== So this is wrong.

This is the code to get the index of the property with a specific attribute name. So I'm trying to find the index of the property with name="vivekProperty",

lookupProp = "ss.test(0).property";
                synchronized(conf){
			elements = conf.configurationsAt(lookupProp);
		}
		
		for (Iterator it = elements.iterator(); it.hasNext();) {
			HierarchicalConfiguration sub = (HierarchicalConfiguration) it
					.next();
			if(sub != null){
				String name = sub.getString("[@" + attrName + "]");
				if(name != null){
					if (name.equals(propName)) {
						isPropFound = true;
						break;
					}
					index = index + 1;
				}	
			}					
		}
		if (!isPropFound) {
			index = -1;
		}

10) Add the node,

conf.addNodes(valuePath, valueNode);     
    //where valuePath is ss.test(0).property(-1) and valueNode is one created in step 8

11) After all this call save on the XMLConfiguration

This in 1.5 is giving me structure like,

<ss>
 <test>
    <property name="testProperty">
         <value>test</value>
    <property>
    <property>
       <name>vivekProperty</name>
    </property>
    <property>
       <value>vivek</value>
    </property>
 </test>
</ss>

I'm not sure if I need to do "addNode" 3 time to add this structure or can it be added in one shot. But, all this used to work fine with 1.3. In our application this is a service so I can call any of the above 3 add nodes individaully in case user may just want to add a new value to an existing property.

So the problem looks like is the code "node.setAttribute(true);" (in Step 5) is not seems to be doing the right thing. Instead of adding a attribute name, it's adding a new element name under property, which doesn't work in my case.

[vivek]

I tried adding attribute as part of the property element itself,

nodeToAdd.addAttribute(new DefaultConfigurationNode(attrName, newPropertyName));

This seems to work fine. So, instead of using "setAttribute(true)" if I just do addAttribute it seems to work fine. May be that's the right way.

I'll leave to you guys to see whether there is any issue in 1.5 or it used to work in 1.3 because of some bug, which has been fixed in 1.5.

For me the workaround is working fine now.

[Oliver Heger]

There was indeed a bug in XMLConfiguration, which prevented attribute nodes from being added using the addNodes() method. A fix was applied. Thank you for spotting this.

Regarding your use case, I think you do not need the addNodes() method, but you can achieve the same effect in an easer way using addProperty(). For instance, in the unit test I wrote to demonstrate this bug I created a structure similar to your test configuration with the following code:

conf.addProperty("testAddNodes.property[@name]", "prop1");
conf.addProperty("testAddNodes.property(0).value", "value1");
conf.addProperty("testAddNodes.property(-1)[@name]", "prop2");
conf.addProperty("testAddNodes.property(1).value", "value2");

This creates two <property> structures. If you look at the last two statements, the first one creates a new property node and the name attribute at the same time. The index -1 means that a new node is to be created. The following statement adds the value child node. Here the index of the last property node must be specified. Hope that helps.