avalon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Turner <j...@socialchange.net.au>
Subject Tightening namespace contracts, and bugfixes
Date Sun, 09 Dec 2001 13:09:50 GMT

Since Avalon is in codefreeze, I thought I'd ask first..

I'd like to make explicit two contracts in the Configuration:

 1) getNamespace() will never return null. If no namespace is set,
   getNamespace() will return "".
 2) getPrefix should be exactly the same. Never null, "" as the default.

I think we agreed on 1). 2) was never formally defined, but it makes

By "make explicit", I mean:
 - make getNamespace() and getPrefix() throw ConfigurationExceptions if
   the builder did not set the internal fields. This follows the
   existing behaviour of getValue(), getValueAs*() and
   getAttributeAs*() methods.
 - Document in the javadocs that those methods will never return null.

And now the *real* reason for these changes.. :)

Currently, if you ask DefaultConfigurationSerializer to serialize a
Configuration without namespaces (eg <foo/>), it will throw a NPE. Ie,
it's *very* broken..

I think this bug is a symptom of the loose contract around getPrefix.
Here is the relevant code (~line 121):

	String nsPrefix = "";

    if ( element instanceof AbstractConfiguration )
        nsPrefix = ((AbstractConfiguration) element).getPrefix();
>>> nsPrefix is null here

    boolean nsWasDeclared = false;

    final String existingURI = m_namespaceSupport.getURI( nsPrefix );
>>> getURI(null) will cause a NPE

So tightening the contract so nsPrefix is never null is IMHO a more
correct fix than adding a "if (nsPrefix != null)" check.

The attached patch does the following:
 - tightens the getNamespace() and getPrefix() contracts as proposed
 - Fixes the Serializer NPE bug when parsing non-namespace XML
 - Fixes a Serializer bug where no xmlns declarations are made. Ie,
   <x:foo xmlns:x="http://bar"/> will be serialized into <x:foo/>.

And yes, I know there should be unit tests for all of this.. they're
next on my list.


View raw message