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 sc...@apache.org on 2007/04/13 22:41:12 UTC

svn commit: r528654 - /webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java

Author: scheu
Date: Fri Apr 13 13:41:11 2007
New Revision: 528654

URL: http://svn.apache.org/viewvc?view=rev&rev=528654
Log:
WSCOMMONS-194
Contributor:Rich Scheuerle
Added more robust and consistent code for SOAPHeader processing.

Modified:
    webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java

Modified: webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java
URL: http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java?view=diff&rev=528654&r1=528653&r2=528654
==============================================================================
--- webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java (original)
+++ webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java Fri Apr 13 13:41:11 2007
@@ -73,23 +73,43 @@
      * @throws OMException
      */
     public SOAPHeader getHeader() throws OMException {
-        // Header must be the first child
-        OMElement header = getFirstElement();
-        if (header == null) {
-            if (builder == null) {
+        SOAPHeader header = null;
+        
+        // We need to be careful when detecting the presence of a header.
+        // The following (old) code expands the tree if the header is 
+        // not present.
+        //SOAPHeader header =
+        //    (SOAPHeader) getFirstChildWithName(
+        //            HEADER_QNAME);
+        
+        
+        // The soap header is the first element in the envelope.
+        OMElement e = getFirstElement();
+        if (e instanceof SOAPHeader) {
+            header = (SOAPHeader) e;
+        } 
+        
+        // The semantics of this method should not depend on 
+        // the state of the builder. The prior code added the header 
+        // if the builder was not present.  This is incorrect.
+        //
+        // Prior Code: funny semantics dependent on presence of builder.
+        // if (builder == null && header == null) {
+        //
+        // CREATE_MISSING_HEADER toggles the semantics
+        
+        boolean CREATE_MISSING_HEADER = false;  // Changing this toggle violates the javadoc
+        if (CREATE_MISSING_HEADER) {
+            if (header == null) {
                 inferFactory();
+                // Creates a SOAPHeader before the SOAPBody
                 header = ((SOAPFactory) factory).createSOAPHeader(this);
-                addChild(header);
-            } else {
-                return null;
             }
-        } else if (!(header instanceof SOAPHeader)) {
-            return null;
         }
-
-        return (SOAPHeader) header;
+        return header;
     }
 
+
     private void inferFactory() {
         if (ns != null) {
             String namespaceURI = ns.getNamespaceURI();
@@ -100,25 +120,49 @@
             }
         }
     }
-
+    
+    /**
+     * Add a SOAPHeader or SOAPBody object
+     */
     public void addChild(OMNode child) {
-        if ((child instanceof OMElement) &&
-                !(child instanceof SOAPHeader || child instanceof SOAPBody)) {
-            throw new SOAPProcessingException(
-                    "SOAP Envelope can not have children other than SOAP Header and Body",
-                    SOAP12Constants.FAULT_CODE_SENDER);
+        if ((child instanceof OMElement) && !(child instanceof SOAPHeader || child instanceof SOAPBody))
+        {
+            throw new SOAPProcessingException("SOAP Envelope can not have children other than SOAP Header and Body", SOAP12Constants.FAULT_CODE_SENDER);
         } else {
-            if (this.done && (child instanceof SOAPHeader)) {
-                SOAPBody body = getBody();
-                if (body != null) {
-                    body.insertSiblingBefore(child);
-                    return;
+            if (child instanceof SOAPHeader) {
+                // The SOAPHeader is added before the SOAPBody
+                // We must be sensitive to the state of the parser.  It is possible that the 
+                // has not been processed yet.
+                if (this.done) {
+                    // Parsing is complete, therefore it is safe to 
+                    // call getBody.
+                    SOAPBody body = getBody();
+                    if (body != null) {
+                        body.insertSiblingBefore(child);
+                        return;
+                    }
+                } else {
+                    // Flow to here indicates that we are still expanding the
+                    // envelope.  The body or body contents may not be 
+                    // parsed yet.  We can't use getBody() yet...it will
+                    // cause a failure.  So instead, carefully find the 
+                    // body and insert the header.  If the body is not found, 
+                    // this indicates that it has not been parsed yet...and
+                    // the code will fall through to the super.addChild.
+                    OMNode node = this.lastChild;
+                    while (node != null) {
+                        if (node instanceof SOAPBody) {
+                            node.insertSiblingBefore(child);
+                            return;
+                        }
+                        node = node.getPreviousOMSibling();
+                    }
                 }
             }
             super.addChild(child);
         }
     }
-
+    
     /**
      * Returns the <CODE>SOAPBody</CODE> object associated with this <CODE>SOAPEnvelope</CODE>
      * object. <P> This SOAPBody will just be a container for all the BodyElements in the



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org


Re: svn commit: r528654 - /webservices/commons/trunk/modules/axiom/modules/axiom-impl/src/main/java/org/apache/axiom/soap/impl/llom/SOAPEnvelopeImpl.java

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Rich!

A couple of brief comments on this commit:

1) We really don't need to leave commented-out code (especially 
commented code that's been gone for a few revisions) in the source.  If 
people want to go back and look at prior versions that's very easy with 
SVN.  Let the SVN logs be the historical record (and the place to 
describe why we removed one bit and replaced it with another), and keep 
the code clean.  So I'd much rather see simply:

     public SOAPHeader getHeader() throws OMException {
         // The soap header is the first element in the envelope.
         OMElement e = getFirstElement();
         if (e instanceof SOAPHeader) {
             return (SOAPHeader)e;
         }

         return null;
     }

I went ahead and committed that change, let me know if you have any issues.

2) +1 for removing the getHeader() side-effect of creating a header, 
I've always thought this was dangerous to rely on myself.

--Glen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org