NewInstance

Today I faced a configuration bug in my web application, that can be made easier to detect if xwork does some more checks loading its action mapping.

I have my xwork.xml as following:

<?xml version="1.0" encoding="ISO-8859-1" ?>
<!DOCTYPE xwork PUBLIC "-//OpenSymphony Group//XWork 1.0//EN" "http://www.opensymphony.com/xwork/xwork-1.0.dtd">

<xwork>
   <include file="first.xml" />
   <include file="second.xml" />

   ...
   <include file="last.xml" />
</xwork>

In two different included files I mapped actions in packages with the same name ie (first.xml):

<?xml version="1.0" encoding="ISO-8859-1" ?>

<!DOCTYPE xwork PUBLIC "-//OpenSymphony Group//XWork 1.0//EN" "http://www.opensymphony.com/xwork/xwork-1.0.dtd">

<xwork>
   <package name="fist-package-name" namespace="/first">

   ...
   </package>
</xwork>

and second.xml

<?xml version="1.0" encoding="ISO-8859-1" ?>
<!DOCTYPE xwork PUBLIC "-//OpenSymphony Group//XWork 1.0//EN" "http://www.opensymphony.com/xwork/xwork-1.0.dtd">

<xwork>
   <package name="fist-package-name" namespace="/second">
   ...
   </package>

</xwork>

Notice that in second.xml I have misspelled "first-package-name" instead of using "second-package-name".
The result of this mistake is that xwork does not find actions from one of the two packages, as the two packages have the same name. You may think that having the same name the second package loaded from xwork configurator has overwritten the first one, as it actually does.

It's not really a fault of xwork, but, I believe that xWork can make simpler to detect programming errors like this, for example giving an error or throwing an exception while loading the configuration file: in this case one package is hiding (we would better say: overwriting) another one. I am thinking if, in some cases, this hiding behaviour can be useful programmatically (ie using jar components that replace a package), but I think that at least a warning should be given. Uhmm... no, I think there's no advantage of having an overwriting feature like this, so I think it would be better to throw an exception and stop xwork from loading.

I checked the code that loads the Configuration and I found the piece responsible of this behaviour.
When com.opensymphony.xwork.config.Configuration.addPackageConfig(String name, PackageConfig packageConfig) is called its implementation (com.opensymphony.xwork.config.impl.DefaultConfiguration) does the following:

    public void addPackageConfig(String name, PackageConfig packageContext) {
        packageContexts.put(name, packageContext);
    }

I think that something like this could be done:

    public void addPackageConfig(String name, PackageConfig packageContext) {
        PackageConfig check = packageContexts.get(name);
        if (check != null) {
             LOG.error("name " + name + " is already been used by another package: " + check);
            // would be better to throw ConfigurationException("name already used");

        }
        packageContexts.put(name, packageContext);
    }

Have you ever faced this problem? I posted this in WebWork Support Forums.