avalon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Donald <pe...@apache.org>
Subject Re: Tightening namespace contracts, and bugfixes
Date Sun, 09 Dec 2001 23:31:02 GMT
On Mon, 10 Dec 2001 00:09, Jeff Turner wrote:
> Since Avalon is in codefreeze, I thought I'd ask first..

I would consider these show stopper bugs that need to be fixed before we 
release. So just go ahead and fix them .. unless someone else has objections?

Anyways they sound good to me but I don't actually use that bit of code much 
so .. ;)

>
> 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
> sense.
>
> 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
>    above
>  - 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.
>
>
> --Jeff

-- 
Cheers,

Pete

--------------------------------------------------------------
"Science is like sex: sometimes something useful comes out, 
but that is not the reason we are doing it" -- Richard Feynman
--------------------------------------------------------------

--
To unsubscribe, e-mail:   <mailto:avalon-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:avalon-dev-help@jakarta.apache.org>


Mime
View raw message