You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commons-dev@ws.apache.org by ve...@apache.org on 2009/02/26 00:47:47 UTC

svn commit: r747970 - in /webservices/commons/trunk/modules/axiom/modules: axiom-api/src/main/java/org/apache/axiom/om/ axiom-api/src/test/java/org/apache/axiom/om/ axiom-dom/src/main/java/org/apache/axiom/om/impl/dom/ axiom-impl/src/main/java/org/apac...

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;
     }