Allow Commons Configuration to set system properties

• Key: CONFIGURATION-353
• Type: New Feature
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Ralph Goers
• Affects Version/s: 1.6
• Fix Version/s: 1.6

Description

SystemConfiguration allows the system properties to be accessed as a configuration and used in a combined configuration. However, no mechanism is available to set system properties from an external file. Such a facility would alleviate having to place a bunch of -D options on the command line.

Comments

[Ralph Goers]

SystemConfiguration was modified to add a static method to set the system properties from a configuration. DefaultConfigurationBuilder was enhanced to allow the system properties to be set before creating the combined configuration.

[Emmanuel Bourg]

That change is weird. Why does it work only with PropertiesConfiguration? I would have preferred adding a single method taking any configuration, and let the DefaultConfigurationBuilder deal with the source configuration. That looks cleaner to me. Initializing the system properties would look like this:

SystemConfiguration.setSystemProperties(new PropertiesConfiguration("system.properties"));

Alternatively, this could have been solved without modifying SystemConfiguration by using a simple copy:

ConfigurationUtils.copy(new PropertiesConfiguration("system.properties"), new SystemConfiguration());

[Ralph Goers]

That change is weird. Why does it work only with PropertiesConfiguration?

Because a) it is wierd to populate system properties from anything other than properties and b) this is meant to be a simple way of getting system properties set for an application instead of putting lots of -Ds in a startup script. More complicated XML is unnecessary.

SystemConfiguration.setSystemProperties(new PropertiesConfiguration("system.properties"));

DefaultConfigurationBuilder allows the property file name to be variable, which is required for my use case. If you are really inquiring about why not do SystemConfiguration.setSystemProperties(new PropertiesConfiguration(filename)) then I must have missed where that constructor could accept an XML properties file. Also, DefaultConfigurationBuilder's base path needs to be passed in along with the file name.

ConfigurationUtils.copy(new PropertiesConfiguration("system.properties"), new SystemConfiguration());

The intent was not to create a new SystemConfiguration. The intent was to get the system properties set as my first comment indicates.

Here is an example usage. repoURL is defined in app-config.properties along with other system properties used by the application.

<?xml version="1.0" encoding="UTF-8"?>
<!--
Test configuration definition file that demonstrates complex initialization
-->
<configuration systemProperties="app-config.properties">
<header>
<result delimiterParsingDisabled="true" forceReloadCheck="true"
config-class="org.apache.commons.configuration.DynamicCombinedConfiguration" keyPattern="$${mdc:bcId}">
<expressionEngine config-class="org.apache.commons.configuration.tree.xpath.XPathExpressionEngine" />
<nodeCombiner config-class="org.apache.commons.configuration.tree.MergeCombiner" />
</result>
<entity-resolver catalogFiles="${sys:repoURL}/repositoryResolver.xml" debug="true" />
<fileSystem config-class="org.apache.commons.configuration.VFSFileSystem" />
<lookups>
<lookup config-prefix="mdc" config-class="org.slf4j.ext.MDCStrLookup" />
<lookup config-prefix="expr" config-class="org.apache.commons.configuration.interpol.ExprLookup">
<variables>
<variable name="String" value="Class:org.apache.commons.lang.StringUtils" />
</variables>
</lookup>
</lookups>
<providers>
<provider config-tag="multifile"
config-class="org.apache.commons.configuration.DefaultConfigurationBuilder$FileConfigurationProvider"
configurationClass="org.apache.commons.configuration.MultiFileHierarchicalConfiguration" />
</providers>
</header>

<override>
<multifile
filePattern="${sys:repoURL}/fi/$$${mdc:bcIndex}/$$${mdc:canonicalId}/$$${mdc:canonicalId}-env.xml"
delimiterParsingDisabled="true" attributeSplittingDisabled="true"
config-name="fiConfig" schemaValidation="${sys:schemaValidation}">
<reloadingStrategy delay="60000"
config-class="org.apache.commons.configuration.reloading.VFSFileMonitorReloadingStrategy" />
</multifile>
<xml fileName="${sys:repoURL}/defaults/env-defaults.xml" config-name="defaultConfig" optional="true"
delimiterParsingDisabled="true" attributeSplittingDisabled="true"
schemaValidation="${sys:schemaValidation}">
<reloadingStrategy config-class="org.apache.commons.configuration.reloading.VFSFileMonitorReloadingStrategy" />
</xml>
</override>

</configuration>

Commons Configuration is a generic API, not a framework. We cannot make restrictive assumptions on how the API must be used. In particular we cannot judge that system properties are not worth being initialized from something else than a properties file. What if someone wants to use a INI or a plist file?

I understand the intent of the change, but I don't think SystemConfiguration should have been changed. The configuration builder is the only "framework-ish" part of Commons Configuration, this is where such changes can take place. The rest of the API should remain as generic as possible.

[Ralph Goers]

So you are saying you would prefer that the static methods be moved into DefaultConfigurationBuilder? That is easy enough to do. I can certainly try to make it more generic to support more types of configurations, but I don't think putting some restrictions on this is necessarily bad.

[Emmanuel Bourg]

Actually I would remove completely the static methods (or keep a generic setSystemProperties(Configuration) method in SystemConfiguration for the convenience).

In DefaultConfigurationBuilder, instead of declaring a 'systemProperties' attribute on the root configuration element, I would probably implement an "init" attribute on the system element. So initializing the system properties would look like this :

<configuration>
  <system init="system.properties"/>
</configuration>

[Ralph Goers]

I thought of that initially. It doesn't work because it doesn't happen soon enough. For example, notice that the entity resolver references a system property. Putting the attribution on the configuration element insures that system properties get set before anything else happens.

[Emmanuel Bourg]

Ok, that's fine for the configuration descriptor then.