[CONFIGURATION-444] SubsetConfiguration does not properly handle interpolation when used on a HierarchicalConfiguration

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Not A Problem
• Affects Version/s: 1.6
• Component/s: Interpolation

Environment

all

• Assignee: Unassigned
• Reporter: Fabien Nisol
• Created: 31/Mar/11 17:41
• Updated: 08/Apr/11 16:58
• Resolved: 08/Apr/11 16:58

Description

Imaging next text files:

<properties>
	<var>a_value</var>
	<prop2>
		<prop
			attr1="${var}"
			attr2="attr2"
		/>
	</prop2>
</properties>

...
  XMLConfiguration config2 = new XMLConfiguration("test/test2.xml");
  Configuration subset2 = config2.subset("prop2");
  Configuration subset3 = new SubsetConfiguration(subset2,"prop");
  System.err.println(subset3.getString("[@attr1]"))
...

the result is wrong:

${var}

it should be:

a_value

I think the problem is related to the interpolate() method in SubsetConfiguration(), which seems to involve some kind of recursive trick that seem overcomplicated (and inefficient, since it's creating unnecessary SubsetConfiguration)..
But maybe I'm missing something.

    protected Object interpolate(Object base)
    {
        if (delimiter == null && "".equals(prefix))
        {
            return super.interpolate(base);
        }
        else
        {
            SubsetConfiguration config = new SubsetConfiguration(parent, "");
            return config.interpolate(base);
        }
    }

I think the code below would be more appropriate:

    protected Object interpolate(Object base)
    {
        if(parent instanceof AbstractConfiguration)
        {
            return ((AbstractConfiguration)parent).interpolate(base);
        } else {
            return base;
        }
    }

There's no reason to try interpolation if the parent is not implementing interpolation.

This brings other questions about the whole interpolation thing. I really wonder why the Configuration interface does not define that method, leaving or the the option to implementation to actually implement it. But that is another debate.