Hi,
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
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
Index:
src/java/org/apache/avalon/framework/configuration/AbstractConfiguration.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon/src/java/org/apache/avalon/framework/configuration/AbstractConfiguration.java,v
retrieving revision 1.10
diff -u -r1.10 AbstractConfiguration.java
---
src/java/org/apache/avalon/framework/configuration/AbstractConfiguration.java
2001/11/19 16:59:15 1.10
+++
src/java/org/apache/avalon/framework/configuration/AbstractConfiguration.java
2001/12/09 12:00:47
@@ -24,8 +24,11 @@
* Returns the prefix of the namespace. This is only used as a
serialization
* hint, therefore is not part of the client API. It should be included in
* all Configuration implementations though.
+ * @return A non-null String (defaults to "")
+ * @throws ConfigurationException if no prefix was defined (prefix is
+ * <code>null</code>.
*/
- protected abstract String getPrefix();
+ protected abstract String getPrefix() throws ConfigurationException;
/**
* Returns the value of the configuration element as an <code>int</code>.
Index: src/java/org/apache/avalon/framework/configuration/Configuration.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon/src/java/org/apache/avalon/framework/configuration/Configuration.java,v
retrieving revision 1.8
diff -u -r1.8 Configuration.java
--- src/java/org/apache/avalon/framework/configuration/Configuration.java
2001/12/09 04:53:33 1.8
+++ src/java/org/apache/avalon/framework/configuration/Configuration.java
2001/12/09 12:00:50
@@ -134,7 +134,7 @@
* @since 4.1
* @return a String identifying the namespace of this Configuration.
*/
- String getNamespace();
+ String getNamespace() throws ConfigurationException;
/**
* Return a new <code>Configuration</code> instance encapsulating the
Index:
src/java/org/apache/avalon/framework/configuration/DefaultConfiguration.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon/src/java/org/apache/avalon/framework/configuration/DefaultConfiguration.java,v
retrieving revision 1.11
diff -u -r1.11 DefaultConfiguration.java
---
src/java/org/apache/avalon/framework/configuration/DefaultConfiguration.java
2001/11/26 09:28:43 1.11
+++
src/java/org/apache/avalon/framework/configuration/DefaultConfiguration.java
2001/12/09 12:00:52
@@ -44,6 +44,13 @@
/**
* Create a new <code>DefaultConfiguration</code> instance.
+ * @param name config node name
+ * @param location Builder-specific locator string
+ * @param ns Namespace string (typically a URI). Should not be null; use ""
+ * if no namespace.
+ * @param prefix A short string prefixed to element names, associating
+ * elements with a longer namespace string. Should not be null; use "" if
no
+ * namespace.
*/
public DefaultConfiguration( final String name,
final String location,
@@ -53,7 +60,7 @@
m_name = name;
m_location = location;
m_namespace = ns;
- m_prefix = prefix; // only used as a serialization hint
+ m_prefix = prefix; // only used as a serialization hint. Cannot be
null
}
/**
@@ -67,17 +74,37 @@
/**
* Returns the namespace of this configuration element
*/
- public String getNamespace()
+ public String getNamespace() throws ConfigurationException
{
- return m_namespace;
- }
+ if( null != m_namespace )
+ {
+ return m_namespace;
+ }
+ else
+ {
+ throw new ConfigurationException( "No namespace (not even default
\"\") is associated with the "+
+ "configuration element \"" +
getName() +
+ "\" at " + getLocation() );
+ }
+ }
/**
* Returns the prefix of the namespace
+ * @throws ConfigurationException if prefix is not present
(<code>null</code>).
*/
- protected String getPrefix()
+ protected String getPrefix() throws ConfigurationException
{
- return m_prefix;
+ if( null != m_prefix )
+ {
+ return m_prefix;
+ }
+ else
+ {
+ throw new ConfigurationException( "No prefix (not even default
\"\") is associated with the "+
+ "configuration element \"" +
getName() +
+ "\" at " + getLocation() );
+ }
+
}
/**
Index:
src/java/org/apache/avalon/framework/configuration/DefaultConfigurationSerializer.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon/src/java/org/apache/avalon/framework/configuration/DefaultConfigurationSerializer.java,v
retrieving revision 1.6
diff -u -r1.6 DefaultConfigurationSerializer.java
---
src/java/org/apache/avalon/framework/configuration/DefaultConfigurationSerializer.java
2001/11/19 16:59:15 1.6
+++
src/java/org/apache/avalon/framework/configuration/DefaultConfigurationSerializer.java
2001/12/09 12:00:56
@@ -83,7 +83,7 @@
* be set before calling this method.
*/
protected void serialize( final Configuration source )
- throws SAXException
+ throws SAXException, ConfigurationException
{
m_namespaceSupport.reset();
m_handler.startDocument();
@@ -95,7 +95,7 @@
* Serialize each Configuration element. This method is called
recursively.
*/
protected void serializeElement( final Configuration element )
- throws SAXException
+ throws SAXException, ConfigurationException
{
m_namespaceSupport.pushContext();
@@ -106,8 +106,12 @@
{
for (int i = 0; i < attrNames.length; i++)
{
- attr.addAttribute("", attrNames[i], attrNames[i], "CDATA",
- element.getAttribute(attrNames[i], ""));
+ attr.addAttribute("", // namespace URI
+ attrNames[i], // local name
+ attrNames[i], // qName
+ "CDATA", // type
+ element.getAttribute(attrNames[i], "") //
value
+ );
}
}
@@ -118,14 +122,31 @@
{
nsPrefix = ((AbstractConfiguration) element).getPrefix();
}
+ // nsPrefix is guaranteed to be non-null at this point.
boolean nsWasDeclared = false;
- // Is this namespace already declared?
final String existingURI = m_namespaceSupport.getURI( nsPrefix );
+
+ // ie, there is no existing URI declared for this prefix or we're
+ // remapping the prefix to a different URI
if ( existingURI == null || !existingURI.equals( nsURI ) )
{
nsWasDeclared = true;
+ if (nsPrefix.equals("") && nsURI.equals(""))
+ {
+ // implicit mapping; don't need to declare
+ }
+ else if (nsPrefix.equals(""))
+ {
+ // (re)declare the default namespace
+ attr.addAttribute("", "xmlns", "xmlns", "CDATA", nsURI);
+ }
+ else
+ {
+ // (re)declare a mapping from nsPrefix to nsURI
+ attr.addAttribute("", "xmlns:"+nsPrefix, "xmlns:"+nsPrefix,
"CDATA", nsURI);
+ }
m_handler.startPrefixMapping( nsPrefix, nsURI );
m_namespaceSupport.declarePrefix( nsPrefix, nsURI );
}
Index:
src/java/org/apache/avalon/framework/configuration/NamespacedSAXConfigurationHandler.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon/src/java/org/apache/avalon/framework/configuration/NamespacedSAXConfigurationHandler.java,v
retrieving revision 1.1
diff -u -r1.1 NamespacedSAXConfigurationHandler.java
---
src/java/org/apache/avalon/framework/configuration/NamespacedSAXConfigurationHandler.java
2001/12/07 13:44:54 1.1
+++
src/java/org/apache/avalon/framework/configuration/NamespacedSAXConfigurationHandler.java
2001/12/09 12:00:56
@@ -122,7 +122,8 @@
final String
namespaceURI,
final String location )
{
- final String prefix = m_namespaceSupport.getPrefix( namespaceURI );
+ String prefix = m_namespaceSupport.getPrefix( namespaceURI );
+ if (prefix == null) prefix = "";
return new DefaultConfiguration( localName, location, namespaceURI,
prefix );
}
Index: tools/ext/avalon-framework.jar
===================================================================
RCS file: /home/cvspublic/jakarta-avalon/tools/ext/avalon-framework.jar,v
retrieving revision 1.9
diff -u -r1.9 avalon-framework.jar
Binary files /tmp/cvsULUegfhsxh and avalon-framework.jar differ
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
