Author: veithen
Date: Wed Feb 25 23:47:46 2009
New Revision: 747970
URL: http://svn.apache.org/viewvc?rev=747970&view=rev
Log:
1. Consolidated the specification (Javadoc) of
OMElement#addAttribute(OMAttribute) by defining a consistent set of
requirements covering all possible cases and by accurately describing the side
effect of addAttribute on namespace declarations. The specification is
consistent with the current LLOM behavior, with two exceptions:
* If addAttribute replaces an existing attribute, it should obviously set the
owner of the replaced attribute to null.
* If the attribute passed as argument is already owned by the element,
addAttribute should be a no-op.
2. Added a set of test cases checking compliance with these requirements.
3. Fixed the addAttribute methods in OMElement (LLOM) and ElementImpl (DOOM) to
conform to the requirements.
4. Fixed some other issues in DOOM that were preventing addAttribute from
working properly.
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/om/OMElement.java
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/test/java/org/apache/axiom/om/OMElementTestBase.java
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttrImpl.java
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttributeMap.java
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ElementImpl.java
webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMElementImpl.java
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/om/OMElement.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/om/OMElement.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/om/OMElement.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/om/OMElement.java
Wed Feb 25 23:47:46 2009
@@ -156,11 +156,32 @@
/**
* Adds an attribute to this element.
- * <p/>
- * <p>There is no order implied by added attributes.</p>
+ * <p>
+ * If the attribute already has an owner, the attribute is cloned (i.e.
its name, value and
+ * namespace are copied to a new attribute) and the new attribute is added
to the element.
+ * Otherwise the existing instance specified by the <code>attr</code>
parameter is added to
+ * the element. In both cases the owner of the added attribute is set to
be the particular
+ * <code>OMElement</code>.
+ * <p>
+ * If there is already an attribute with the same name and namespace URI,
it will be replaced
+ * and its owner set to <code>null</code>.
+ * <p>
+ * In the particular case where the attribute specified by
<code>attr</code> is already owned
+ * by the element, calling this method has no effect.
+ * <p>
+ * Attributes are not stored in any particular order. In particular, there
is no guarantee
+ * that the added attribute will be returned as the last item by the
iterator produced by
+ * a subsequent call to {...@link #getAllAttributes()}.
+ * <p>
+ * If the attribute being added has a namespace, but no corresponding
namespace declaration is
+ * in scope for the element (i.e. declared on the element or one of its
ancestors), a new
+ * namespace declaration is added to the element. Note that both the
namespace prefix and URI
+ * are taken into account when looking for an existing namespace
declaration.
*
* @param attr The attribute to add.
- * @return Returns the passed in attribute.
+ * @return The attribute that was added to the element. As described above
this may or may
+ * not be the same as <code>attr</code>, depending on whether the
attribute specified
+ * by this parameter already has an owner or not.
*/
OMAttribute addAttribute(OMAttribute attr);
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/test/java/org/apache/axiom/om/OMElementTestBase.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-api/src/test/java/org/apache/axiom/om/OMElementTestBase.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/test/java/org/apache/axiom/om/OMElementTestBase.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-api/src/test/java/org/apache/axiom/om/OMElementTestBase.java
Wed Feb 25 23:47:46 2009
@@ -20,6 +20,8 @@
package org.apache.axiom.om;
import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
import javax.xml.namespace.QName;
import javax.xml.stream.XMLStreamConstants;
@@ -138,4 +140,98 @@
assertNotNull(ns);
assertEquals("urn:a", ns.getNamespaceURI());
}
+
+ /**
+ * Test that calling {...@link OMElement#addAttribute(OMAttribute)} with
an attribute that is
+ * already owned by another element will clone the attribute.
+ */
+ public void testAddAttributeAlreadyOwnedByOtherElement() {
+ OMFactory factory = getOMFactory();
+ OMElement element1 = factory.createOMElement(new QName("test"));
+ OMElement element2 = factory.createOMElement(new QName("test"));
+ OMAttribute att1 = element1.addAttribute("test", "test", null);
+ OMAttribute att2 = element2.addAttribute(att1);
+ assertSame(element1, att1.getOwner());
+ assertNotSame(att1, att2);
+ assertSame(element2, att2.getOwner());
+ }
+
+ /**
+ * Test that calling {...@link OMElement#addAttribute(OMAttribute)} with
an attribute that is
+ * already owned by the element is a no-op.
+ */
+ public void testAddAttributeAlreadyOwnedByElement() {
+ OMFactory factory = getOMFactory();
+ OMElement element = factory.createOMElement(new QName("test"));
+ OMAttribute att = element.addAttribute("test", "test", null);
+ OMAttribute result = element.addAttribute(att);
+ assertSame(result, att);
+ assertSame(element, att.getOwner());
+ Iterator it = element.getAllAttributes();
+ assertTrue(it.hasNext());
+ assertSame(att, it.next());
+ assertFalse(it.hasNext());
+ }
+
+ /**
+ * Test that {...@link OMElement#addAttribute(OMAttribute)} behaves
correctly when an attribute
+ * with the same name and namespace URI already exists.
+ */
+ public void testAddAttributeReplace() {
+ OMFactory factory = getOMFactory();
+ // Use same namespace URI but different prefixes
+ OMNamespace ns1 = factory.createOMNamespace("urn:ns", "p1");
+ OMNamespace ns2 = factory.createOMNamespace("urn:ns", "p2");
+ OMElement element = factory.createOMElement(new QName("test"));
+ OMAttribute att1 = factory.createOMAttribute("test", ns1, "test");
+ OMAttribute att2 = factory.createOMAttribute("test", ns2, "test");
+ element.addAttribute(att1);
+ element.addAttribute(att2);
+ Iterator it = element.getAllAttributes();
+ assertTrue(it.hasNext());
+ assertSame(att2, it.next());
+ assertFalse(it.hasNext());
+ assertNull(att1.getOwner());
+ assertSame(element, att2.getOwner());
+ }
+
+ // This methods filters out the ("", "") namespace declaration (empty
namespace
+ // to default). This declaration is present on OMElements produced by DOOM.
+ // TODO: check if this is not a bug in DOOM
+ private Iterator getRealAllDeclaredNamespaces(OMElement element) {
+ List namespaces = new LinkedList();
+ for (Iterator it = element.getAllDeclaredNamespaces(); it.hasNext(); )
{
+ OMNamespace ns = (OMNamespace)it.next();
+ if (!("".equals(ns.getPrefix()) &&
"".equals(ns.getNamespaceURI()))) {
+ namespaces.add(ns);
+ }
+ }
+ return namespaces.iterator();
+ }
+
+ public void testAddAttributeWithoutExistingNamespaceDeclaration() {
+ OMFactory factory = getOMFactory();
+ OMElement element = factory.createOMElement(new QName("test"));
+ OMNamespace ns = factory.createOMNamespace("urn:ns", "p");
+ OMAttribute att = factory.createOMAttribute("test", ns, "test");
+ element.addAttribute(att);
+ assertEquals(ns, element.findNamespace(ns.getNamespaceURI(),
ns.getPrefix()));
+ Iterator it = getRealAllDeclaredNamespaces(element);
+ assertTrue(it.hasNext());
+ assertEquals(ns, it.next());
+ assertFalse(it.hasNext());
+ }
+
+ public void testAddAttributeWithExistingNamespaceDeclaration() {
+ OMFactory factory = getOMFactory();
+ OMElement element = factory.createOMElement(new QName("test"));
+ OMNamespace ns = factory.createOMNamespace("urn:ns", "p");
+ element.declareNamespace(ns);
+ OMAttribute att = factory.createOMAttribute("test", ns, "test");
+ element.addAttribute(att);
+ Iterator it = getRealAllDeclaredNamespaces(element);
+ assertTrue(it.hasNext());
+ assertEquals(ns, it.next());
+ assertFalse(it.hasNext());
+ }
}
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttrImpl.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttrImpl.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttrImpl.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttrImpl.java
Wed Feb 25 23:47:46 2009
@@ -400,6 +400,8 @@
}
}
clone.isSpecified(true);
+ clone.setParent(null);
+ clone.setUsed(false);
return clone;
}
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttributeMap.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttributeMap.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttributeMap.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/AttributeMap.java
Wed Feb 25 23:47:46 2009
@@ -149,6 +149,7 @@
}
//Set the owner node
attr.ownerNode = (DocumentImpl) this.ownerNode.getOwnerDocument();
+ attr.parent = this.ownerNode;
attr.isOwned(true); // To indicate that this attr belong to an element
int i = findNamePoint(attr.getNamespaceURI(), attr.getLocalName());
@@ -159,6 +160,7 @@
nodes.setElementAt(attr, i);
previous.ownerNode = (DocumentImpl) this.ownerNode
.getOwnerDocument();
+ previous.parent = null;
previous.isOwned(false);
// make sure it won't be mistaken with defaults in case it's reused
previous.isSpecified(true);
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ElementImpl.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ElementImpl.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ElementImpl.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ElementImpl.java
Wed Feb 25 23:47:46 2009
@@ -675,6 +675,16 @@
/** @see org.apache.axiom.om.OMElement#addAttribute
(org.apache.axiom.om.OMAttribute) */
public OMAttribute addAttribute(OMAttribute attr) {
+ // If the attribute already has an owner element then clone the
attribute (except if it is owned
+ // by the this element)
+ OMElement owner = attr.getOwner();
+ if (owner != null) {
+ if (owner == this) {
+ return attr;
+ }
+ attr = (OMAttribute)((AttrImpl)attr).cloneNode(false);
+ }
+
OMNamespace namespace = attr.getNamespace();
if (namespace != null && namespace.getNamespaceURI() != null
&& !"".equals(namespace.getNamespaceURI())
@@ -684,10 +694,11 @@
}
if (attr.getNamespace() != null) { // If the attr has a namespace
- return (AttrImpl) this.setAttributeNode((Attr) attr);
+ this.setAttributeNodeNS((Attr) attr);
} else {
- return (AttrImpl) this.setAttributeNodeNS((Attr) attr);
+ this.setAttributeNode((Attr) attr);
}
+ return attr;
}
/**
Modified:
webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMElementImpl.java
URL:
http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMElementImpl.java?rev=747970&r1=747969&r2=747970&view=diff
==============================================================================
---
webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMElementImpl.java
(original)
+++
webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/om/impl/llom/OMElementImpl.java
Wed Feb 25 23:47:46 2009
@@ -574,8 +574,13 @@
* @see OMAttributeImpl#equals(Object)
*/
public OMAttribute addAttribute(OMAttribute attr){
- // If the attribute already has an owner element then clone the
attribute
- if (attr.getOwner() !=null){
+ // If the attribute already has an owner element then clone the
attribute (except if it is owned
+ // by the this element)
+ OMElement owner = attr.getOwner();
+ if (owner != null) {
+ if (owner == this) {
+ return attr;
+ }
attr = new OMAttributeImpl(
attr.getLocalName(), attr.getNamespace(),
attr.getAttributeValue(), attr.getOMFactory());
}
@@ -594,7 +599,11 @@
// Set the owner element of the attribute
((OMAttributeImpl)attr).owner = this;
- attributes.put(attr.getQName(), attr);
+ OMAttributeImpl oldAttr =
(OMAttributeImpl)attributes.put(attr.getQName(), attr);
+ // Did we replace an existing attribute?
+ if (oldAttr != null) {
+ oldAttr.owner = null;
+ }
return attr;
}
