ConfigurationConverter does not handle CompositeConfiguration correctly

• Key: CONFIGURATION-339
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Oliver Heger
• Reporter: William DeMoss II
• Component/s: Interpolation
• Affects Version/s: 1.5
• Fix Version/s: 1.6

Description

ConfigurationConverter doesn't seem to respect the ordering of a composite configuration. I am having problems describing the behavior, but I thinks the following test case illustrates it best:

@Test
public void showBug() {
PropertiesConfiguration p = new PropertiesConfiguration();
p.addProperty("foo", "initial");
p.addProperty("bar", "${foo}");
p.addProperty("prefix.foo", "override");

CompositeConfiguration cfg = new CompositeConfiguration();
cfg.addConfiguration(p.subset("prefix"));
cfg.addConfiguration(p);

// this assertion passes as expected since the subset
// was added first to the composite configuration
Assert.assertEquals("override", cfg.getString("bar"));

// after converting to properties, this assertion fails and
// reports that the value is 'initial'
Properties properties = ConfigurationConverter.getProperties(cfg);
Assert.assertEquals("override", properties.getProperty("bar"));
}

Comments

[Oliver Heger]

It took a while to figure out what is going on, but I was able to track it down to the getList() implementation of CompositeConfiguration. This method performs two interpolations on the property values it returns:

• One implicit interpolation by calling getList() on the child configuration. This is where the wrong property value comes from.
• One explicit interpolation on the list with property values gathered from the child configurations. Here interpolation across multiple child configurations can be done.

The first of these interpolations is pointless because the second one will produce the desired results.

Because getStringArray() delegates to getList() this method is affected, too. In fact getStringArray() does again an interpolation on the values returned by getList(). Because getList() should already have substituted all variables for which this is possible, this additional interpolation won't add any value.

Fixed in trunk and configuration2 branch.