You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by "Mark Hansen (JIRA)" <ax...@ws.apache.org> on 2004/12/10 16:50:42 UTC

[jira] Created: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

SAAJ 1.2 (DOM) node trees are invalid - missing parents
-------------------------------------------------------

         Key: AXIS-1713
         URL: http://nagoya.apache.org/jira/browse/AXIS-1713
     Project: Axis
        Type: Bug
  Components: SAAJ  
    Versions: current (nightly), 1.2RC2    
 Environment: Linux (Debian), JDK 1.4.2
    Reporter: Mark Hansen
    Priority: Critical


The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.

The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.

A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?

Here is the diff for NodeImpl.java (vs Axis-1_2RC2):

194,196d193
<       if ( textRep == null ) {
< 	return null;
<       } else {
199d195
<     }
305,309c301,302
<                 Document doc = node.getOwnerDocument();
<                 if (doc == null) {
<                     node = node.getParent();
<                 } else {
<                     return doc;
---
>                 if(node.getOwnerDocument() != null) {
>                     return node.getOwnerDocument();
310a304
>                 node = node.getParent();
382c376
<         return (Node) getParent();
---
>         return parent;
391,407c385
< 	if ( parent == null ) {
< 	  return null;
< 	}
< // This while loop doesn't ever set previousSibling.
< // Further, getChildElements() may have side effects.  
< // ... safer to use DOM getChildNodes()
< 
< //         Iterator iter = parent.getChildElements();
< //         Node previousSibling = null;
< //         while (iter.hasNext()) {
< //             if (iter.next() == this) {
< //                 return previousSibling;
< //             }
< //         }
< 	NodeList nl = parent.getChildNodes();
< 	int len = nl.getLength();
< 	int i = 0;
---
>         Iterator iter = parent.getChildElements();
409,410c387,388
< 	while( i < len ) {
< 	  if ( nl.item(i) == this ) {
---
>         while (iter.hasNext()) {
>             if (iter.next() == this) {
413,414d390
< 	  previousSibling = nl.item(i);
< 	  i++;
497d472
<      *
500,503d474
<       if ( newChild == null ) {
< 	throw new DOMException
< 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
<       }
505d475
<       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
507d476
<       ((NodeImpl)newChild).parent = this;
522,531d490
<       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
< 	setDirty(true);
< 	return oldChild;
<       }
<       throw new DOMException(DOMException.NOT_FOUND_ERR,
< 			     "NodeImpl Not found");
<     }
< 
<   private boolean removeNodeFromChildList(NodeImpl n) {
< 
536,537c495,496
<       final NodeImpl node = (NodeImpl) itr.next();
<       if (node == n) {
---
>             final Node node = (Node) itr.next();
>             if (node == oldChild) {
542,543c501,507
<     return removed;
< 
---
>         if (!removed) {
>             throw new DOMException(DOMException.NOT_FOUND_ERR,
>                     "NodeImpl Not found");
>         } else {
>             setDirty(removed);
>         }
>         return oldChild;
545,546d508
< 
< 
791,792c753
<      * Set the parent node and invoke appendChild(this) to 
<      * add this node to the parent's list of children.
---
>      * set the parent node
837,838c798
<     public boolean isDirty() {
<  return _isDirty; }
---
>     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Updated: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Davanum Srinivas (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=history ]

Davanum Srinivas updated AXIS-1713:
-----------------------------------

    Attachment: diff.txt

"diff -u"

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Jongjin Choi (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56622 ]
     
Jongjin Choi commented on AXIS-1713:
------------------------------------

I did not track AXIS2 thread for a long time but, I think that AXIS2's linked-list based OM is related to this. Can it or it's idea be applied to Axis 1.X?


> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, NodeImpl.java, TestDOM.java, diff.txt, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Updated: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Mark Hansen (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=history ]

Mark Hansen updated AXIS-1713:
------------------------------

    Attachment: NodeImpl.java
                diff.txt

Revised NodeImpl.java to address Jarek's comments.  The appendChild(...) method now only scans the list of children if the newChild node being appended already has a parent - indicating that it is already in the tree.  So, serializers appending newly created nodes (without parents) shouldn't take the performance hit of scanning the list of children.

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, NodeImpl.java, TestDOM.java, diff.txt, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Mark Hansen (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56623 ]
     
Mark Hansen commented on AXIS-1713:
-----------------------------------

Jongjin,

Thanks for the pointer to Axis 2.0.  I'll check it out.

For now, I've attached a simple fix that should address the serializer performance issue raised by Jarek.

-- Mark

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, NodeImpl.java, TestDOM.java, diff.txt, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Mark Hansen (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56601 ]
     
Mark Hansen commented on AXIS-1713:
-----------------------------------

Jarek,

Good point.  In order to have both DOM/SAAJ compliance (the Javadoc is based on the DOM specification) and good performance, we would need to change the data structure of NodeImpl.children from ArrayList to something that allows removal of a single element from the list without traversing the entire list.  Node that in the current implementation the NodeImpl.detachNode() and NodeImpl.removeChild() have the same problem.

What do you think?  Should I go ahead and "enhance" the ArrayList implementation of NodeImpl.children?

Thanks for your help,

Mark

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Jarek Gawor (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56592 ]
     
Jarek Gawor commented on AXIS-1713:
-----------------------------------

Changing the appendNode() method to call removeChildFromNodeList() absolutely kills the performance of deerialization because on each append the entire list of children needs to be traversed. I know this is how the function must work from the JavaDocs of org.w3c.dom.Node.appendNode()  but I think we need more optimized way of adding the children during deserialization (because at that point we know for sure that a node is not anyones child)

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Updated: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Mark Hansen (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=history ]

Mark Hansen updated AXIS-1713:
------------------------------

    Attachment: NodeImpl.java
                TestDOM.java

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Assigned: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Jarek Gawor (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=history ]

Jarek Gawor reassigned AXIS-1713:
---------------------------------

    Assign To: Jarek Gawor

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Resolved: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Davanum Srinivas (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=history ]
     
Davanum Srinivas resolved AXIS-1713:
------------------------------------

    Resolution: Fixed

Fixed. Thanks.

-- dims

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, NodeImpl.java, TestDOM.java, diff.txt, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Davanum Srinivas (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56594 ]
     
Davanum Srinivas commented on AXIS-1713:
----------------------------------------

Mark,

Can you please rework the patch as per Jarek's comment and submit a patch against latest CVS?

thanks,
dims

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Assignee: Jarek Gawor
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java, diff.txt
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (AXIS-1713) SAAJ 1.2 (DOM) node trees are invalid - missing parents

Posted by "Davanum Srinivas (JIRA)" <ax...@ws.apache.org>.
     [ http://nagoya.apache.org/jira/browse/AXIS-1713?page=comments#action_56516 ]
     
Davanum Srinivas commented on AXIS-1713:
----------------------------------------

Mark,

Can you please do a "diff -u" and upload the file as attachment?

thanks,
dims

> SAAJ 1.2 (DOM) node trees are invalid - missing parents
> -------------------------------------------------------
>
>          Key: AXIS-1713
>          URL: http://nagoya.apache.org/jira/browse/AXIS-1713
>      Project: Axis
>         Type: Bug
>   Components: SAAJ
>     Versions: current (nightly), 1.2RC2
>  Environment: Linux (Debian), JDK 1.4.2
>     Reporter: Mark Hansen
>     Priority: Critical
>  Attachments: NodeImpl.java, TestDOM.java
>
> The SAAJ 1.2 Node implemenation (org.apache.axis.message.NodeImpl) is still buggy.  I believe that MessageElement is also buggy.
> The specific bug addressed here concerns NodeImpl.appendChild(Node newChild).  A bug in this method causes the org.apache.axis.Message(...) constructors to create invalid DOM trees - some nodes lack parents.  I traced this back to a problem with the implementation of appendChild(...) not setting the parent when adding a child.
> A fixed version of NodeImpl.java is attached.  I've also attached a test (included the method testForParent() in test.saaj.TestDOM.  Can the person commiting this change to NodeImpl also commit the change to test.saaj.TestDOM so that Axis has a check for this "bad DOM tree" condition?
> Here is the diff for NodeImpl.java (vs Axis-1_2RC2):
> 194,196d193
> <       if ( textRep == null ) {
> < 	return null;
> <       } else {
> 199d195
> <     }
> 305,309c301,302
> <                 Document doc = node.getOwnerDocument();
> <                 if (doc == null) {
> <                     node = node.getParent();
> <                 } else {
> <                     return doc;
> ---
> >                 if(node.getOwnerDocument() != null) {
> >                     return node.getOwnerDocument();
> 310a304
> >                 node = node.getParent();
> 382c376
> <         return (Node) getParent();
> ---
> >         return parent;
> 391,407c385
> < 	if ( parent == null ) {
> < 	  return null;
> < 	}
> < // This while loop doesn't ever set previousSibling.
> < // Further, getChildElements() may have side effects.  
> < // ... safer to use DOM getChildNodes()
> < 
> < //         Iterator iter = parent.getChildElements();
> < //         Node previousSibling = null;
> < //         while (iter.hasNext()) {
> < //             if (iter.next() == this) {
> < //                 return previousSibling;
> < //             }
> < //         }
> < 	NodeList nl = parent.getChildNodes();
> < 	int len = nl.getLength();
> < 	int i = 0;
> ---
> >         Iterator iter = parent.getChildElements();
> 409,410c387,388
> < 	while( i < len ) {
> < 	  if ( nl.item(i) == this ) {
> ---
> >         while (iter.hasNext()) {
> >             if (iter.next() == this) {
> 413,414d390
> < 	  previousSibling = nl.item(i);
> < 	  i++;
> 497d472
> <      *
> 500,503d474
> <       if ( newChild == null ) {
> < 	throw new DOMException
> < 	  (DOMException.HIERARCHY_REQUEST_ERR, "Can't append a null node.");
> <       }
> 505d475
> <       removeNodeFromChildList((NodeImpl) newChild); // per DOM spec
> 507d476
> <       ((NodeImpl)newChild).parent = this;
> 522,531d490
> <       if ( removeNodeFromChildList((NodeImpl) oldChild) ) {
> < 	setDirty(true);
> < 	return oldChild;
> <       }
> <       throw new DOMException(DOMException.NOT_FOUND_ERR,
> < 			     "NodeImpl Not found");
> <     }
> < 
> <   private boolean removeNodeFromChildList(NodeImpl n) {
> < 
> 536,537c495,496
> <       final NodeImpl node = (NodeImpl) itr.next();
> <       if (node == n) {
> ---
> >             final Node node = (Node) itr.next();
> >             if (node == oldChild) {
> 542,543c501,507
> <     return removed;
> < 
> ---
> >         if (!removed) {
> >             throw new DOMException(DOMException.NOT_FOUND_ERR,
> >                     "NodeImpl Not found");
> >         } else {
> >             setDirty(removed);
> >         }
> >         return oldChild;
> 545,546d508
> < 
> < 
> 791,792c753
> <      * Set the parent node and invoke appendChild(this) to 
> <      * add this node to the parent's list of children.
> ---
> >      * set the parent node
> 837,838c798
> <     public boolean isDirty() {
> <  return _isDirty; }
> ---
> >     public boolean isDirty() { return _isDirty; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira