You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ws.apache.org by ve...@apache.org on 2013/01/13 23:12:30 UTC

svn commit: r1432754 - in /webservices/commons/trunk/modules/axiom/modules: axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/ axiom-testsuite/src/main/java/org/apache/axiom/ts/om/ axiom-testsuite/src/main/java/org/apache/axiom/ts/om/elem...

Author: veithen
Date: Sun Jan 13 22:12:29 2013
New Revision: 1432754

URL: http://svn.apache.org/viewvc?rev=1432754&view=rev
Log:
We can only switch once we get past the last node created by the builder. If we switch when we reach that node, then the information returned may be inaccurate because the node may have been modified.

As a side effect of the fix we no longer need to look ahead one node (at least not in SwitchingWrapper; OMNavigator still fetches the next node). This greatly simplifies the code.

Added:
    webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java   (with props)
Modified:
    webservices/commons/trunk/modules/axiom/modules/axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/SwitchingWrapper.java
    webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/OMTestSuiteBuilder.java

Modified: webservices/commons/trunk/modules/axiom/modules/axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/SwitchingWrapper.java
URL: http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/SwitchingWrapper.java?rev=1432754&r1=1432753&r2=1432754&view=diff
==============================================================================
--- webservices/commons/trunk/modules/axiom/modules/axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/SwitchingWrapper.java (original)
+++ webservices/commons/trunk/modules/axiom/modules/axiom-common-impl/src/main/java/org/apache/axiom/om/impl/common/SwitchingWrapper.java Sun Jan 13 22:12:29 2013
@@ -46,7 +46,6 @@ import org.apache.axiom.om.OMDocType;
 import org.apache.axiom.om.OMDocument;
 import org.apache.axiom.om.OMElement;
 import org.apache.axiom.om.OMEntityReference;
-import org.apache.axiom.om.OMException;
 import org.apache.axiom.om.OMNamespace;
 import org.apache.axiom.om.OMNode;
 import org.apache.axiom.om.OMProcessingInstruction;
@@ -138,14 +137,6 @@ class SwitchingWrapper extends AbstractX
     /** Field elementStack */
     private Stack nodeStack = null;
 
-    // holder for the current node. Needs this to generate events from the
-    // current node
-
-    /** Field currentNode */
-    private OMSerializable currentNode = null;
-
-    // needs this to refer to the last known node
-
     /** Field lastNode */
     private OMSerializable lastNode = null;
 
@@ -179,21 +170,18 @@ class SwitchingWrapper extends AbstractX
         this.cache = cache;
         this.preserveNamespaceContext = preserveNamespaceContext;
 
-        // initiate the next and current nodes
-        // Note - navigator is written in such a way that it first
-        // returns the starting node at the first call to it
-        
-        currentNode = navigator.getNext();
         if (startNode instanceof OMDocument) {
-            currentEvent = -1;
-            try {
-                next();
-            } catch (XMLStreamException ex) {
-                throw new OMException(ex);
-            }
-        } else {
-            currentEvent = START_DOCUMENT;
+            // Bootstrap the navigator: is written in such a way that it first
+            // returns the starting node at the first call to it.
+            // If the start node is an OMElement, then this will occur that the
+            // first call to next().
+            lastNode = navigator.getNext();
+            // Initialize some other state
+            nodeStack = new Stack();
+            nodeStack.push(startNode);
+            isFirst = false;
         }
+        currentEvent = START_DOCUMENT;
     }
 
     /**
@@ -888,19 +876,14 @@ class SwitchingWrapper extends AbstractX
                 currentEvent = END_DOCUMENT;
                 break;
             case NAVIGABLE:
-                // keeps the next event. The parser actually keeps one step ahead to
-                // detect the end of navigation. (at the end of the stream the navigator
-                // returns a null
-                OMSerializable nextNode;
-
                 if (navigator.isNavigable()) {
-                    nextNode = navigator.getNext();
+                    lastNode = navigator.getNext();
                 } else if (navigator.isCompleted()) {
-                    nextNode = null;
+                    lastNode = null;
                 } else if (cache) {
                     builder.next();
                     navigator.step();
-                    nextNode = navigator.getNext();
+                    lastNode = navigator.getNext();
                 } else {
                     // reset caching (the default is ON so it was not needed in the
                     // earlier case!
@@ -915,16 +898,14 @@ class SwitchingWrapper extends AbstractX
                                                      e);
                     }
 
-                    currentEvent = parser.getEventType();
+                    currentEvent = parser.next();
                     updateCompleteStatus();
                     break;
                 }
-                currentEvent = generateEvents(currentNode);
+                currentEvent = generateEvents(lastNode);
                 updateCompleteStatus();
-                lastNode = currentNode;
                 attributeCount = -1;
                 namespaceCount = -1;
-                currentNode = nextNode;
                 break;
             case SWITCHED:
                 currentEvent = parser.next();
@@ -980,7 +961,7 @@ class SwitchingWrapper extends AbstractX
             depth--;
         }
         if (state == NAVIGABLE) {
-            if (rootNode == currentNode) {
+            if (rootNode == lastNode) {
                 if (isFirst) {
                     isFirst = false;
                 } else if (currentEvent == END_DOCUMENT) {

Modified: webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/OMTestSuiteBuilder.java
URL: http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/OMTestSuiteBuilder.java?rev=1432754&r1=1432753&r2=1432754&view=diff
==============================================================================
--- webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/OMTestSuiteBuilder.java (original)
+++ webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/OMTestSuiteBuilder.java Sun Jan 13 22:12:29 2013
@@ -292,6 +292,7 @@ public class OMTestSuiteBuilder extends 
             addTest(new org.apache.axiom.ts.om.element.TestGetXMLStreamReaderWithOMSourcedElementDescendant(metaFactory));
         }
         addTest(new org.apache.axiom.ts.om.element.TestGetXMLStreamReaderWithoutCachingPartiallyBuilt(metaFactory));
+        addTest(new org.apache.axiom.ts.om.element.TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified(metaFactory));
         addTest(new org.apache.axiom.ts.om.element.TestGetXMLStreamReaderWithPreserveNamespaceContext(metaFactory));
         addTest(new org.apache.axiom.ts.om.element.TestIsCompleteAfterAddingIncompleteChild(metaFactory));
         addTest(new org.apache.axiom.ts.om.element.TestMultipleDefaultNS(metaFactory));

Added: webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java
URL: http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java?rev=1432754&view=auto
==============================================================================
--- webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java (added)
+++ webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java Sun Jan 13 22:12:29 2013
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.axiom.ts.om.element;
+
+import java.io.StringReader;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLStreamReader;
+
+import org.apache.axiom.om.OMContainer;
+import org.apache.axiom.om.OMElement;
+import org.apache.axiom.om.OMMetaFactory;
+import org.apache.axiom.om.OMXMLBuilderFactory;
+import org.apache.axiom.ts.AxiomTestCase;
+
+/**
+ * Tests the behavior of {@link OMContainer#getXMLStreamReaderWithoutCaching()} in the specific case
+ * where the element is partially built and the last created node has been modified. In Axiom 1.2.14
+ * the information returned for that node was incorrect because the builder switched too early to
+ * pull through mode.
+ */
+public class TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified extends AxiomTestCase {
+    public TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified(OMMetaFactory metaFactory) {
+        super(metaFactory);
+    }
+
+    protected void runTest() throws Throwable {
+        OMElement root = OMXMLBuilderFactory.createOMBuilder(metaFactory.getOMFactory(),
+                new StringReader("<root><a/><b/><c/></root>")).getDocumentElement();
+        
+        OMElement b = root.getFirstChildWithName(new QName("b"));
+        b.addAttribute("att", "value", null);
+        assertFalse(b.isComplete());
+        
+        XMLStreamReader reader = root.getXMLStreamReaderWithoutCaching();
+        
+        // Skip to the START_ELEMENT event corresponding to b
+        for (int i=0; i<4; i++) {
+            reader.next();
+        }
+        assertEquals(XMLStreamReader.START_ELEMENT, reader.getEventType());
+        assertEquals("b", reader.getLocalName());
+        
+        // The previously added attribute must be visible
+        assertEquals(1, reader.getAttributeCount());
+        assertEquals("att", reader.getAttributeLocalName(0));
+        assertEquals("value", reader.getAttributeValue(0));
+    }
+}

Propchange: webservices/commons/trunk/modules/axiom/modules/axiom-testsuite/src/main/java/org/apache/axiom/ts/om/element/TestGetXMLStreamReaderWithoutCachingPartiallyBuiltModified.java
------------------------------------------------------------------------------
    svn:eol-style = native