ODE
  1. ODE
  2. ODE-663

DOMUtils.cloneNode results in invalid namespace declaration

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Cannot Reproduce
    • Affects Version/s: 1.3.3, 1.3.4
    • Fix Version/s: 1.4
    • Component/s: BPEL Runtime
    • Labels:
      None

      Description

      The DOMUtils.cloneNode code doesn't handle default namespace declarations correctly when processing an XQuery result from Saxon.

      For example:

      <ns1:parent xmlns:ns1="abc">
      <ns1:child xmlns="def">
      <ns2:nestedChild xmlns:ns2="def"/>
      </ns1:child>
      </ns1:parent>

      results in:

      <ns1:parent xmlns:ns1="abc">
      <ns1:child xmlns:xmlns="def">
      <ns2:nestedChild xmlns:ns2="def"/>
      </ns1:child>
      </ns1:parent>

      Notice that the default namespace has been rewritten as xmlns:xmlns which is invalid.

      Granted the above example is a bit odd, but strange things can happen when passing nodes in and out of XSLT and XQuery with respect to namespace declarations.

      1. ode1.x-663-v1.txt
        8 kB
        David Carver
      2. ode1.x-663-v2.txt
        4 kB
        David Carver
      3. ode1.x-663-v3.txt
        8 kB
        David Carver
      4. ode1.x-663-v4.txt
        4 kB
        David Carver
      5. ode1.x-663-v5.txt
        4 kB
        David Carver

        Activity

        Hide
        David Carver added a comment -

        DOMUtils.cloneNode could probably be simplified to do the following:

        public static Node cloneNode(Document document, Node sourceNode) {
        Node clonedNode = null;

        if (document != null)

        { clonedNode = document.importNode(sourceNode, true); }

        else

        { clonedNode = sourceNode.cloneNode(true); }

        return clonedNode;
        }

        Which does what is need. importNode will clone and bring in the node into a new document, with out hurting the original source document. If there is no document, then just clone it.

        I'll attach a proposed patch.

        Show
        David Carver added a comment - DOMUtils.cloneNode could probably be simplified to do the following: public static Node cloneNode(Document document, Node sourceNode) { Node clonedNode = null; if (document != null) { clonedNode = document.importNode(sourceNode, true); } else { clonedNode = sourceNode.cloneNode(true); } return clonedNode; } Which does what is need. importNode will clone and bring in the node into a new document, with out hurting the original source document. If there is no document, then just clone it. I'll attach a proposed patch.
        Hide
        David Carver added a comment -

        Proposed patch to simplify and correctly handle the issue that is currently failing. Also adds two unit tests to verify that both situations are being handled as expected.

        Show
        David Carver added a comment - Proposed patch to simplify and correctly handle the issue that is currently failing. Also adds two unit tests to verify that both situations are being handled as expected.
        Hide
        Mark Ford added a comment -

        You need a few more unit tests to flesh out some problems with your patch.

        Try the following:

        <ns1:parent xmlns:ns1='abc' xmlns:ns3='nsthree'>
        <ns1:child xmlns='def'>
        <ns2:nestedChild xmlns:ns2='def' encoded='ns3:foo'/>
        </ns1:child>
        </ns1:parent>

        If you clone the document element, everything is fine. However, if you clone ns1:child you'll lose the declaration of ns3. Encoded qnames are missed when using the standard import/clone methods provided by DOM nodes.

        Also, if memory serves there are some cases where the source DOM might be immutable and have the clone/import methods throw exceptions if invoked. I believe the Saxon DOM behaves this way which is what you'd be working with when handling the output of an XQuery.

        Show
        Mark Ford added a comment - You need a few more unit tests to flesh out some problems with your patch. Try the following: <ns1:parent xmlns:ns1='abc' xmlns:ns3='nsthree'> <ns1:child xmlns='def'> <ns2:nestedChild xmlns:ns2='def' encoded='ns3:foo'/> </ns1:child> </ns1:parent> If you clone the document element, everything is fine. However, if you clone ns1:child you'll lose the declaration of ns3. Encoded qnames are missed when using the standard import/clone methods provided by DOM nodes. Also, if memory serves there are some cases where the source DOM might be immutable and have the clone/import methods throw exceptions if invoked. I believe the Saxon DOM behaves this way which is what you'd be working with when handling the output of an XQuery.
        Hide
        David Carver added a comment -

        Yes, the DOM will loose that namespace because at that point in time it doesn't necessarily know that it is in scope for the node as it doesn't look at the parent or ancestors when cloning, and this is done according to spec. There is one potential problem with the example though from a true XML Namespace compliance standpoint. Namespace prefixes aren't significant the namespace it self is significant. Namespaces can be redefined at different levels so a prefix could change.

        <root>
        <ns:child1 xmlns:ns="http://www.example.org/ns2">
        <ns:child2 xmlns:ns="http://www.example.org/ns1"/>
        </ns:child1>
        </root>

        The above is valid XML and valid according to XML Namespaces 1.1 specification. Should the DOMUtils.clone copy in the parent definition, or ignore it? http://www.example.org/ns2 becomes no longer in scope at this point because ns:child2 has the default namespace redefined to http://wwww.example.org/ns1.

        My gut says that if you ns:child2, you should loose the namespace for ns:child1 as it's not in scope. The other thing to consider is the following fragment:

        <root>
        <ns:child1 xmlns:ns="http://www.example.org/ns2">
        <ns:child2 xmlns:ns="http://www.example.org/ns1"/ encoded="ns:child1">
        </ns:child1>
        </root>

        What does child1 refer to, and which namespace is it going to get?

        Just tossing some what ifs out there for discussion.

        Show
        David Carver added a comment - Yes, the DOM will loose that namespace because at that point in time it doesn't necessarily know that it is in scope for the node as it doesn't look at the parent or ancestors when cloning, and this is done according to spec. There is one potential problem with the example though from a true XML Namespace compliance standpoint. Namespace prefixes aren't significant the namespace it self is significant. Namespaces can be redefined at different levels so a prefix could change. <root> <ns:child1 xmlns:ns="http://www.example.org/ns2"> <ns:child2 xmlns:ns="http://www.example.org/ns1"/> </ns:child1> </root> The above is valid XML and valid according to XML Namespaces 1.1 specification. Should the DOMUtils.clone copy in the parent definition, or ignore it? http://www.example.org/ns2 becomes no longer in scope at this point because ns:child2 has the default namespace redefined to http://wwww.example.org/ns1 . My gut says that if you ns:child2, you should loose the namespace for ns:child1 as it's not in scope. The other thing to consider is the following fragment: <root> <ns:child1 xmlns:ns="http://www.example.org/ns2"> <ns:child2 xmlns:ns="http://www.example.org/ns1"/ encoded="ns:child1"> </ns:child1> </root> What does child1 refer to, and which namespace is it going to get? Just tossing some what ifs out there for discussion.
        Hide
        Mark Ford added a comment -

        I'm not suggesting that the clone or import feature in DOM is broken but rather that it isn't well suited to this problem or at least is insufficient on its own to properly clone some nodes. Your point about namespace prefixes changing or being remapped is really only a problem when you are copying between nodes and have embedded QNames. This aspect of XML serialization is particularly ugly and difficult to get right in all cases.

        As for your second example, ns:child1 should expand to http://www.example.org/ns1#child1.

        I think the code needs to traverse the DOM and copy the nodes from the source to a new target node. If it's failing in the above use case (which it was when I wrote the bug) then there's a bug in it somewhere but obviously not too serious or it would have been fixed by now. I also think the code needs to do this traversal to address the immutable DOM problem from Saxon XQuery which I have since confirmed to be true.

        Show
        Mark Ford added a comment - I'm not suggesting that the clone or import feature in DOM is broken but rather that it isn't well suited to this problem or at least is insufficient on its own to properly clone some nodes. Your point about namespace prefixes changing or being remapped is really only a problem when you are copying between nodes and have embedded QNames. This aspect of XML serialization is particularly ugly and difficult to get right in all cases. As for your second example, ns:child1 should expand to http://www.example.org/ns1#child1 . I think the code needs to traverse the DOM and copy the nodes from the source to a new target node. If it's failing in the above use case (which it was when I wrote the bug) then there's a bug in it somewhere but obviously not too serious or it would have been fixed by now. I also think the code needs to do this traversal to address the immutable DOM problem from Saxon XQuery which I have since confirmed to be true.
        Hide
        David Carver added a comment -

        Okay, so if the node that is being cloned has a parent node, then get all the namespaces, then add any missing namespaces to the node being cloned.

        I can get another patch to address this case.

        Show
        David Carver added a comment - Okay, so if the node that is being cloned has a parent node, then get all the namespaces, then add any missing namespaces to the node being cloned. I can get another patch to address this case.
        Hide
        David Carver added a comment -

        this patch fixes the original reported item. Added a couple of unit tests as well to verify it.

        Show
        David Carver added a comment - this patch fixes the original reported item. Added a couple of unit tests as well to verify it.
        Hide
        Rafal Rusin added a comment -

        Thanks David!

        Show
        Rafal Rusin added a comment - Thanks David!
        Hide
        Rafal Rusin added a comment -

        Dave, I reverted this patch for now. Please check out DataHandling20Test, it shows this in target/bpel-test.log:

        05-07@13:29:18 ERROR (MockScheduler.java:172) - Caught an exception during transaction
        java.lang.RuntimeException: Scheduled transaction failed unexpectedly: transaction will not be retried!.
        at org.apache.ode.il.MockScheduler.doExecute(MockScheduler.java:306)
        at org.apache.ode.il.MockScheduler.access$200(MockScheduler.java:47)
        at org.apache.ode.il.MockScheduler$4.call(MockScheduler.java:123)
        at org.apache.ode.il.MockScheduler.execTransaction(MockScheduler.java:168)
        at org.apache.ode.il.MockScheduler.execTransaction(MockScheduler.java:159)
        at org.apache.ode.il.MockScheduler$6.call(MockScheduler.java:190)
        at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:269)
        at java.util.concurrent.FutureTask.run(FutureTask.java:123)
        at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:650)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:675)
        at java.lang.Thread.run(Thread.java:595)
        Caused by: org.apache.ode.bpel.iapi.Scheduler$JobProcessorException: java.lang.RuntimeException: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated
        at org.apache.ode.bpel.engine.BpelEngineImpl.onScheduledJob(BpelEngineImpl.java:471)
        at org.apache.ode.bpel.engine.BpelServerImpl.onScheduledJob(BpelServerImpl.java:450)
        at org.apache.ode.il.MockScheduler.doExecute(MockScheduler.java:304)
        ... 10 more
        Caused by: java.lang.RuntimeException: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated
        at org.apache.ode.jacob.vpu.JacobVPU$JacobThreadImpl.run(JacobVPU.java:464)
        at org.apache.ode.jacob.vpu.JacobVPU.execute(JacobVPU.java:139)
        at org.apache.ode.bpel.engine.BpelRuntimeContextImpl.execute(BpelRuntimeContextImpl.java:960)
        at org.apache.ode.bpel.engine.PartnerLinkMyRoleImpl.invokeNewInstance(PartnerLinkMyRoleImpl.java:208)
        at org.apache.ode.bpel.engine.BpelProcess$1.invoke(BpelProcess.java:283)
        at org.apache.ode.bpel.engine.BpelProcess.invokeProcess(BpelProcess.java:238)
        at org.apache.ode.bpel.engine.BpelProcess.invokeProcess(BpelProcess.java:279)
        at org.apache.ode.bpel.engine.BpelProcess.handleJobDetails(BpelProcess.java:425)
        at org.apache.ode.bpel.engine.BpelEngineImpl.onScheduledJob(BpelEngineImpl.java:455)
        ... 12 more
        Caused by: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated
        at net.sf.saxon.dom.NodeOverNodeInfo.disallowUpdate(NodeOverNodeInfo.java:687)
        at net.sf.saxon.dom.NodeOverNodeInfo.cloneNode(NodeOverNodeInfo.java:395)
        at org.apache.ode.utils.DOMUtils.copyAttributes(DOMUtils.java:1205)
        at org.apache.ode.utils.DOMUtils.cloneNode(DOMUtils.java:1159)
        at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.getResultValue(XQuery10ExpressionRuntime.java:607)
        at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.evaluate(XQuery10ExpressionRuntime.java:432)
        at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.evaluate(XQuery10ExpressionRuntime.java:161)
        at org.apache.ode.bpel.runtime.ExpressionLanguageRuntimeRegistry.evaluate(ExpressionLanguageRuntimeRegistry.java:80)
        at org.apache.ode.bpel.runtime.ASSIGN.evalRValue(ASSIGN.java:224)
        at org.apache.ode.bpel.runtime.ASSIGN.copy(ASSIGN.java:382)
        at org.apache.ode.bpel.runtime.ASSIGN.run(ASSIGN.java:86)
        at sun.reflect.GeneratedMethodAccessor9.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:585)
        at org.apache.ode.jacob.vpu.JacobVPU$JacobThreadImpl.run(JacobVPU.java:451)
        ... 20 more

        Show
        Rafal Rusin added a comment - Dave, I reverted this patch for now. Please check out DataHandling20Test, it shows this in target/bpel-test.log: 05-07@13:29:18 ERROR (MockScheduler.java:172) - Caught an exception during transaction java.lang.RuntimeException: Scheduled transaction failed unexpectedly: transaction will not be retried!. at org.apache.ode.il.MockScheduler.doExecute(MockScheduler.java:306) at org.apache.ode.il.MockScheduler.access$200(MockScheduler.java:47) at org.apache.ode.il.MockScheduler$4.call(MockScheduler.java:123) at org.apache.ode.il.MockScheduler.execTransaction(MockScheduler.java:168) at org.apache.ode.il.MockScheduler.execTransaction(MockScheduler.java:159) at org.apache.ode.il.MockScheduler$6.call(MockScheduler.java:190) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:269) at java.util.concurrent.FutureTask.run(FutureTask.java:123) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:650) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:675) at java.lang.Thread.run(Thread.java:595) Caused by: org.apache.ode.bpel.iapi.Scheduler$JobProcessorException: java.lang.RuntimeException: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated at org.apache.ode.bpel.engine.BpelEngineImpl.onScheduledJob(BpelEngineImpl.java:471) at org.apache.ode.bpel.engine.BpelServerImpl.onScheduledJob(BpelServerImpl.java:450) at org.apache.ode.il.MockScheduler.doExecute(MockScheduler.java:304) ... 10 more Caused by: java.lang.RuntimeException: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated at org.apache.ode.jacob.vpu.JacobVPU$JacobThreadImpl.run(JacobVPU.java:464) at org.apache.ode.jacob.vpu.JacobVPU.execute(JacobVPU.java:139) at org.apache.ode.bpel.engine.BpelRuntimeContextImpl.execute(BpelRuntimeContextImpl.java:960) at org.apache.ode.bpel.engine.PartnerLinkMyRoleImpl.invokeNewInstance(PartnerLinkMyRoleImpl.java:208) at org.apache.ode.bpel.engine.BpelProcess$1.invoke(BpelProcess.java:283) at org.apache.ode.bpel.engine.BpelProcess.invokeProcess(BpelProcess.java:238) at org.apache.ode.bpel.engine.BpelProcess.invokeProcess(BpelProcess.java:279) at org.apache.ode.bpel.engine.BpelProcess.handleJobDetails(BpelProcess.java:425) at org.apache.ode.bpel.engine.BpelEngineImpl.onScheduledJob(BpelEngineImpl.java:455) ... 12 more Caused by: java.lang.UnsupportedOperationException: The Saxon DOM cannot be updated at net.sf.saxon.dom.NodeOverNodeInfo.disallowUpdate(NodeOverNodeInfo.java:687) at net.sf.saxon.dom.NodeOverNodeInfo.cloneNode(NodeOverNodeInfo.java:395) at org.apache.ode.utils.DOMUtils.copyAttributes(DOMUtils.java:1205) at org.apache.ode.utils.DOMUtils.cloneNode(DOMUtils.java:1159) at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.getResultValue(XQuery10ExpressionRuntime.java:607) at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.evaluate(XQuery10ExpressionRuntime.java:432) at org.apache.ode.bpel.elang.xquery10.runtime.XQuery10ExpressionRuntime.evaluate(XQuery10ExpressionRuntime.java:161) at org.apache.ode.bpel.runtime.ExpressionLanguageRuntimeRegistry.evaluate(ExpressionLanguageRuntimeRegistry.java:80) at org.apache.ode.bpel.runtime.ASSIGN.evalRValue(ASSIGN.java:224) at org.apache.ode.bpel.runtime.ASSIGN.copy(ASSIGN.java:382) at org.apache.ode.bpel.runtime.ASSIGN.run(ASSIGN.java:86) at sun.reflect.GeneratedMethodAccessor9.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.ode.jacob.vpu.JacobVPU$JacobThreadImpl.run(JacobVPU.java:451) ... 20 more
        Hide
        Mark Ford added a comment -

        Please see my comments above about the Saxon DOM being immutable. The fix is to traverse the dom and copy its nodes over individually. The previous code was fine but was likely missing an edge case. I recommend adding a unit test for the original bug report, stepping through the original code and fixing the issue.

        Show
        Mark Ford added a comment - Please see my comments above about the Saxon DOM being immutable. The fix is to traverse the dom and copy its nodes over individually. The previous code was fine but was likely missing an edge case. I recommend adding a unit test for the original bug report, stepping through the original code and fixing the issue.
        Hide
        David Carver added a comment -

        I'll right I'll take another look at this and do another approach for testing, setting it up so that SAXON is provding the DOM.

        Show
        David Carver added a comment - I'll right I'll take another look at this and do another approach for testing, setting it up so that SAXON is provding the DOM.
        Hide
        David Carver added a comment -

        Mark the test I added and the XML that is being used is the sample XML you provided in the bug report. I'm adding some additional tests to test with specific parsers, like Saxon's DOM implementation.

        Show
        David Carver added a comment - Mark the test I added and the XML that is being used is the sample XML you provided in the bug report. I'm adding some additional tests to test with specific parsers, like Saxon's DOM implementation.
        Hide
        David Carver added a comment -

        I'm able to replicate the issue with the unit test. Without the current patch all 4 tests fail. With the current patch, the Xerces-J tests pass, the Saxon Tests fail. I'll work on getting all the tests to pass.

        Show
        David Carver added a comment - I'm able to replicate the issue with the unit test. Without the current patch all 4 tests fail. With the current patch, the Xerces-J tests pass, the Saxon Tests fail. I'll work on getting all the tests to pass.
        Hide
        David Carver added a comment -

        There is a problem in general with the exist code and Saxon. From the Saxon website:

        "This DOM interface is read-only, so all attempts to call updating methods throw an appropriate DOM exception....It is not possible to construct the tree using DOM methods such as createElement() and createAttribute()"

        Which means the the Document that is passed into cloneNode(Document, Node) and returned has to be one that can be manipulated.

        So code like the following will not work and will retun an UnsupportedOperationException:

        System.setProperty(DOCUMENT_BUILDER_FACTORY, SAXON_DOM_DOCUMENT_BUILDER_FACTORY);
        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
        factory.setNamespaceAware(true);
        DocumentBuilder saxonBuilder = factory.newDocumentBuilder();
        Document doc = saxonBuilder.parse(new ByteArrayInputStream(testString.getBytes()));

        Node node = doc.getFirstChild();
        Node clonedNode = DOMUtils.cloneNode(doc, node);

        This is due to the use of createElementNS and createAttributeNS methods in the existing code.

        Show
        David Carver added a comment - There is a problem in general with the exist code and Saxon. From the Saxon website: "This DOM interface is read-only, so all attempts to call updating methods throw an appropriate DOM exception....It is not possible to construct the tree using DOM methods such as createElement() and createAttribute()" Which means the the Document that is passed into cloneNode(Document, Node) and returned has to be one that can be manipulated. So code like the following will not work and will retun an UnsupportedOperationException: System.setProperty(DOCUMENT_BUILDER_FACTORY, SAXON_DOM_DOCUMENT_BUILDER_FACTORY); DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setNamespaceAware(true); DocumentBuilder saxonBuilder = factory.newDocumentBuilder(); Document doc = saxonBuilder.parse(new ByteArrayInputStream(testString.getBytes())); Node node = doc.getFirstChild(); Node clonedNode = DOMUtils.cloneNode(doc, node); This is due to the use of createElementNS and createAttributeNS methods in the existing code.
        Hide
        David Carver added a comment -

        Correctly handles both Xerces-J and Saxon DOMs.

        Show
        David Carver added a comment - Correctly handles both Xerces-J and Saxon DOMs.
        Hide
        David Carver added a comment -

        Hmm...looks like DataQuery20 test is still not liking this.
        http://hudson.zones.apache.org/hudson/job/ODE-1.x/132/testReport/junit/org.apache.ode.test/DataHandling20Test/testXQueryExpression/

        I'll have to check and see why locally.

        Show
        David Carver added a comment - Hmm...looks like DataQuery20 test is still not liking this. http://hudson.zones.apache.org/hudson/job/ODE-1.x/132/testReport/junit/org.apache.ode.test/DataHandling20Test/testXQueryExpression/ I'll have to check and see why locally.
        Hide
        David Carver added a comment -

        This removes the use of importNode and corrects a small issue with Attributes in getting the correct namespace or non-namespace depending on the need.

        This also adds a unit test for a Node that is returned by Saxon's XQuery processor which will have no ownerdocument. Due to the lack of an owner document this was causing Xerces-J to throw a Nullpointer exception when using importNode in the original patch.

        Show
        David Carver added a comment - This removes the use of importNode and corrects a small issue with Attributes in getting the correct namespace or non-namespace depending on the need. This also adds a unit test for a Node that is returned by Saxon's XQuery processor which will have no ownerdocument. Due to the lack of an owner document this was causing Xerces-J to throw a Nullpointer exception when using importNode in the original patch.
        Hide
        David Carver added a comment -

        Alright...the NullPointer exceptions that were occurring are gone, but Data20 test is still getting a Divide by zero error somewhere along the way. DOMClone is returning the expected values so if anybody has an idea of where the problem is occuring, I'd appreciate a hint. As far as I can tell the latest cloneNode is working as it should.

        Show
        David Carver added a comment - Alright...the NullPointer exceptions that were occurring are gone, but Data20 test is still getting a Divide by zero error somewhere along the way. DOMClone is returning the expected values so if anybody has an idea of where the problem is occuring, I'd appreciate a hint. As far as I can tell the latest cloneNode is working as it should.
        Hide
        Rafal Rusin added a comment -

        After enabling org.apache.ode.bpel DEBUG logs (in bpel-test/src/test/resources/log4j.properties), you can see this:

        05-13@14:53:34 DEBUG (DebugBpelEventListener.java:50) -
        ActivityExecStartEvent:
        Type = activityLifecycle
        ActivityId = 20
        ActivityName = throw-activity-line-165
        ActivityType = OThrow
        ActivityDeclarationId = 84
        ScopeId = 25
        ScopeDeclarationId = 10
        ParentScopesNames = [__PROCESS_SCOPE:HelloXQueryWorld]
        ScopeName = __PROCESS_SCOPE:HelloXQueryWorld
        ProcessInstanceId = 22
        ProcessId =

        {http://ode/bpel/unit-test}

        HelloXQueryWorld-1
        ProcessName =

        {http://ode/bpel/unit-test}

        HelloXQueryWorld
        Timestamp = Thu May 13 14:53:34 GMT-08:00 2010
        LineNo = 165
        Class = class org.apache.ode.bpel.evt.ActivityExecStartEvent

        It means that in this file src/test/resources/bpel/2.0/TestXQueryExpression/HelloXQueryWorld.bpel following condition was evaluated to true:
        <if>
        <condition>not($tempVar/wsa:EndpointReference/wsa:Metadata/Service/@EndpointName eq "MyEndpoint")</condition>
        <throw faultName="selectionFailure"/>
        </if>

        This looks like somehow cloned tree from tempVar:
        <sref:service-ref xmlns:sref="http://docs.oasis-open.org/wsbpel/2.0/serviceref">
        <wsa:EndpointReference xmlns:pp="http://some-namespace" xmlns:wsa="http://www.w3.org/2005/08/addressing">
        <wsa:Address>http://www.w3.org/2005/08/addressing/anonymous</wsa:Address>
        <wsa:Metadata>
        <Service EndpointName="MyEndpoint">pp:SomeProxy</Service>
        </wsa:Metadata>
        </wsa:EndpointReference>
        </sref:service-ref>

        fails at XPath evaluation (EndpointName attribute is not found).
        You can try to extend unit test case to verify that xpath returns expected value.

        Show
        Rafal Rusin added a comment - After enabling org.apache.ode.bpel DEBUG logs (in bpel-test/src/test/resources/log4j.properties), you can see this: 05-13@14:53:34 DEBUG (DebugBpelEventListener.java:50) - ActivityExecStartEvent: Type = activityLifecycle ActivityId = 20 ActivityName = throw-activity-line-165 ActivityType = OThrow ActivityDeclarationId = 84 ScopeId = 25 ScopeDeclarationId = 10 ParentScopesNames = [__PROCESS_SCOPE:HelloXQueryWorld] ScopeName = __PROCESS_SCOPE:HelloXQueryWorld ProcessInstanceId = 22 ProcessId = {http://ode/bpel/unit-test} HelloXQueryWorld-1 ProcessName = {http://ode/bpel/unit-test} HelloXQueryWorld Timestamp = Thu May 13 14:53:34 GMT-08:00 2010 LineNo = 165 Class = class org.apache.ode.bpel.evt.ActivityExecStartEvent It means that in this file src/test/resources/bpel/2.0/TestXQueryExpression/HelloXQueryWorld.bpel following condition was evaluated to true: <if> <condition>not($tempVar/wsa:EndpointReference/wsa:Metadata/Service/@EndpointName eq "MyEndpoint")</condition> <throw faultName="selectionFailure"/> </if> This looks like somehow cloned tree from tempVar: <sref:service-ref xmlns:sref="http://docs.oasis-open.org/wsbpel/2.0/serviceref"> <wsa:EndpointReference xmlns:pp="http://some-namespace" xmlns:wsa="http://www.w3.org/2005/08/addressing"> <wsa:Address> http://www.w3.org/2005/08/addressing/anonymous </wsa:Address> <wsa:Metadata> <Service EndpointName="MyEndpoint">pp:SomeProxy</Service> </wsa:Metadata> </wsa:EndpointReference> </sref:service-ref> fails at XPath evaluation (EndpointName attribute is not found). You can try to extend unit test case to verify that xpath returns expected value.
        Hide
        David Carver added a comment -

        Will add another test case for this.

        Show
        David Carver added a comment - Will add another test case for this.
        Hide
        David Carver added a comment -

        This is almost back to the original code with the minor change to how attribute nodes are handled.

        Rafal please take a review of this and run locally. Everything seems to work here, but that is what I said about the last couple of patches.

        Show
        David Carver added a comment - This is almost back to the original code with the minor change to how attribute nodes are handled. Rafal please take a review of this and run locally. Everything seems to work here, but that is what I said about the last couple of patches.
        Hide
        Rafal Rusin added a comment -

        This still fails at 165 line.

        ActivityType = OThrow
        ActivityDeclarationId = 84
        ScopeId = 25
        ScopeDeclarationId = 10
        ParentScopesNames = [__PROCESS_SCOPE:HelloXQueryWorld]
        ScopeName = __PROCESS_SCOPE:HelloXQueryWorld
        ProcessInstanceId = 22
        ProcessId =

        {http://ode/bpel/unit-test}

        HelloXQueryWorld-1
        ProcessName =

        {http://ode/bpel/unit-test}

        HelloXQueryWorld
        Timestamp = Thu May 13 15:57:22 GMT-08:00 2010
        LineNo = 165
        Class = class org.apache.ode.bpel.evt.ActivityExecEndEvent

        Show
        Rafal Rusin added a comment - This still fails at 165 line. ActivityType = OThrow ActivityDeclarationId = 84 ScopeId = 25 ScopeDeclarationId = 10 ParentScopesNames = [__PROCESS_SCOPE:HelloXQueryWorld] ScopeName = __PROCESS_SCOPE:HelloXQueryWorld ProcessInstanceId = 22 ProcessId = {http://ode/bpel/unit-test} HelloXQueryWorld-1 ProcessName = {http://ode/bpel/unit-test} HelloXQueryWorld Timestamp = Thu May 13 15:57:22 GMT-08:00 2010 LineNo = 165 Class = class org.apache.ode.bpel.evt.ActivityExecEndEvent
        Hide
        David Carver added a comment -

        Rafal go ahead and revert to revision 942237 and see if it restores the functionality. That is the unchanged cloneNode method. If it still fails with that, then it isn't the changes I've done to the cloneNode, if it is, I'll have another basis point to check and see what is different between the two. It could also be a wrong test that the current implementation is now revealing as well.

        Show
        David Carver added a comment - Rafal go ahead and revert to revision 942237 and see if it restores the functionality. That is the unchanged cloneNode method. If it still fails with that, then it isn't the changes I've done to the cloneNode, if it is, I'll have another basis point to check and see what is different between the two. It could also be a wrong test that the current implementation is now revealing as well.
        Hide
        Rafal Rusin added a comment -

        This is weird.
        Your clone node seems to work well. I extended test case by this:

        { XQDataSource ds = new SaxonXQDataSource(); XQConnection conn = ds.getConnection(); XQExpression exp = conn.createExpression(); exp.bindNode(QName.valueOf("tempVar"), clonedNode, conn.createNodeType()); XQResultSequence rs = exp.executeQuery("declare namespace wsa=\"http://www.w3.org/2005/08/addressing\"; declare variable $tempVar as node() external; not($tempVar/wsa:EndpointReference/wsa:Metadata/Service/@EndpointName eq \"MyEndpoint\")"); rs.next(); XQItem xqitem = rs.getItem(); String result = xqitem.getAtomicValue(); assertEquals("false", result); }

        And it really evaluates properly.
        So the problem may be somewhere else. But I have no clue where.

        Show
        Rafal Rusin added a comment - This is weird. Your clone node seems to work well. I extended test case by this: { XQDataSource ds = new SaxonXQDataSource(); XQConnection conn = ds.getConnection(); XQExpression exp = conn.createExpression(); exp.bindNode(QName.valueOf("tempVar"), clonedNode, conn.createNodeType()); XQResultSequence rs = exp.executeQuery("declare namespace wsa=\"http://www.w3.org/2005/08/addressing\"; declare variable $tempVar as node() external; not($tempVar/wsa:EndpointReference/wsa:Metadata/Service/@EndpointName eq \"MyEndpoint\")"); rs.next(); XQItem xqitem = rs.getItem(); String result = xqitem.getAtomicValue(); assertEquals("false", result); } And it really evaluates properly. So the problem may be somewhere else. But I have no clue where.
        Hide
        David Carver added a comment -

        Okay then so there is something else going on. What other changes have gone as well?

        Show
        David Carver added a comment - Okay then so there is something else going on. What other changes have gone as well?
        Hide
        Rafal Rusin added a comment -

        I don't know actually.
        I think we can replace interface for cloneNode, because in most cases we use it for ElementNode and DocumentNode only (and I think it's enough to have those 2 utils methods). Below are all places, where cloneNode is used:

        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) targetElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) targetElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: final Element clonedElmt = (Element) parentElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: itemValue = DOMUtils.cloneNode(document, ((Document) itemValue).getDocumentElement());
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: itemValue = DOMUtils.cloneNode(document, (Node) itemValue);
        bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: resultValue = DOMUtils.cloneNode(document, (Node) resultValue);
        bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setMessage((Element) message.getMessage().cloneNode(true));
        bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setHeaderPart(partName, (Element) headerParts.get(partName).cloneNode(true));
        bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setHeaderPart(partName, (Element) parts.get(partName).cloneNode(true));

        So we can make less generic interface for cloneNode and we can also implement it using DOMUtils.domToString and stringToDom. This way implementation will be simple too.
        I think it's a good way, because this cloneNode method implementation has so much corner cases that it's not worth digging.
        What do you think?

        Show
        Rafal Rusin added a comment - I don't know actually. I think we can replace interface for cloneNode, because in most cases we use it for ElementNode and DocumentNode only (and I think it's enough to have those 2 utils methods). Below are all places, where cloneNode is used: bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) targetElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) targetElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: final Element clonedElmt = (Element) parentElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xpath20/runtime/JaxpFunctionResolver.java: Element clonedElmt = (Element) parentElmt.cloneNode(true); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: itemValue = DOMUtils.cloneNode(document, ((Document) itemValue).getDocumentElement()); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: itemValue = DOMUtils.cloneNode(document, (Node) itemValue); bpel-runtime/src/main/java/org/apache/ode/bpel/elang/xquery10/runtime/XQuery10ExpressionRuntime.java: resultValue = DOMUtils.cloneNode(document, (Node) resultValue); bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setMessage((Element) message.getMessage().cloneNode(true)); bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setHeaderPart(partName, (Element) headerParts.get(partName).cloneNode(true)); bpel-runtime/src/main/java/org/apache/ode/bpel/engine/MyRoleMessageExchangeImpl.java: clone.setHeaderPart(partName, (Element) parts.get(partName).cloneNode(true)); So we can make less generic interface for cloneNode and we can also implement it using DOMUtils.domToString and stringToDom. This way implementation will be simple too. I think it's a good way, because this cloneNode method implementation has so much corner cases that it's not worth digging. What do you think?
        Hide
        David Carver added a comment -

        As Mark mentioned earlier the cloneNode is an enhanced version of the DOM's version. The main differences are the following:

        1. It doesn't care if there is an owner document or not (SAXON's XQuery doesn't return owner documents for the DOM node it returns. It is read only as well).

        2. It tries to bring in any QName reference to an namespace that may be embedded within the attributes text nodes.

        With out this you loose some of the namespaces that may be needed and referenced from the embedded items.

        The only place in that list where we are using CloneNode is because of the SAXON nodes being returned by XQuery expressions.

        Show
        David Carver added a comment - As Mark mentioned earlier the cloneNode is an enhanced version of the DOM's version. The main differences are the following: 1. It doesn't care if there is an owner document or not (SAXON's XQuery doesn't return owner documents for the DOM node it returns. It is read only as well). 2. It tries to bring in any QName reference to an namespace that may be embedded within the attributes text nodes. With out this you loose some of the namespaces that may be needed and referenced from the embedded items. The only place in that list where we are using CloneNode is because of the SAXON nodes being returned by XQuery expressions.
        Hide
        Hudson added a comment -

        Integrated in ODE-1.x #134 (See http://hudson.zones.apache.org/hudson/job/ODE-1.x/134/)
        ODE-663: Reverted for now

        Show
        Hudson added a comment - Integrated in ODE-1 .x #134 (See http://hudson.zones.apache.org/hudson/job/ODE-1.x/134/ ) ODE-663 : Reverted for now
        Hide
        Hudson added a comment -

        Integrated in ODE-trunk-jdk6 #256 (See http://hudson.zones.apache.org/hudson/job/ODE-trunk-jdk6/256/)
        ODE-663: Reverted for now
        V4 fix for ODE-663. Thanks to David Carver
        fixing ODE-663. Thanks to Dave Carver!

        Show
        Hudson added a comment - Integrated in ODE-trunk-jdk6 #256 (See http://hudson.zones.apache.org/hudson/job/ODE-trunk-jdk6/256/ ) ODE-663 : Reverted for now V4 fix for ODE-663 . Thanks to David Carver fixing ODE-663 . Thanks to Dave Carver!
        Hide
        Rafal Rusin added a comment -

        Mark, could you provide BPEL test case, where it fails?

        I tried to do test case like this:
        <bpws:assign name="assign1">
        <bpws:copy>
        <bpws:from expressionLanguage="urn:oasis:names:tc:wsbpel:2.0:sublang:xquery1.0">
        <![CDATA[
        <hello:HelloResponse xmlns:hello="urn:/HelloWorld2.wsdl">
        <ns1:parent xmlns:ns1="abc">
        <ns1:child xmlns="def">
        <ns2:nestedChild xmlns:ns2="def"/>
        </ns1:child>
        </ns1:parent>
        </hello:HelloResponse>
        ]]>
        </bpws:from>
        <bpws:to><![CDATA[$response.body]]></bpws:to>
        </bpws:copy>
        </bpws:assign>

        But this works properly and returns:
        <?xml version="1.0" encoding="UTF-8"?>
        <HelloResponse xmlns="urn:/HelloWorld2.wsdl"><ns1:parent xmlns:ns1="abc"><ns1:child><ns2:nestedChild xmlns:ns2="def"/></ns1:child></ns1:parent></HelloResponse>

        I put this example here:
        http://github.com/rafalrusin/ode/tree/trunk-ode-663

        You can try it by
        buildr 1.3.5 clean test:XQueryJbiTest
        result is in jbi/target/test/test.log

        Regards

        Show
        Rafal Rusin added a comment - Mark, could you provide BPEL test case, where it fails? I tried to do test case like this: <bpws:assign name="assign1"> <bpws:copy> <bpws:from expressionLanguage="urn:oasis:names:tc:wsbpel:2.0:sublang:xquery1.0"> <![CDATA[ <hello:HelloResponse xmlns:hello="urn:/HelloWorld2.wsdl"> <ns1:parent xmlns:ns1="abc"> <ns1:child xmlns="def"> <ns2:nestedChild xmlns:ns2="def"/> </ns1:child> </ns1:parent> </hello:HelloResponse> ]]> </bpws:from> <bpws:to><![CDATA [$response.body] ]></bpws:to> </bpws:copy> </bpws:assign> But this works properly and returns: <?xml version="1.0" encoding="UTF-8"?> <HelloResponse xmlns="urn:/HelloWorld2.wsdl"><ns1:parent xmlns:ns1="abc"><ns1:child><ns2:nestedChild xmlns:ns2="def"/></ns1:child></ns1:parent></HelloResponse> I put this example here: http://github.com/rafalrusin/ode/tree/trunk-ode-663 You can try it by buildr 1.3.5 clean test:XQueryJbiTest result is in jbi/target/test/test.log Regards
        Hide
        Mark Ford added a comment -

        I don't have a test case handy and I'm not actively using ODE any more. I tested the cloneNode code from the 1.3.x branch and it doesn't currently exhibit this behavior. I couldn't attach the original process or even the original message but I'm sure that it was failing with the xmlns prefix. The sample message I posted above is similar to what I was seeing in the ODE logs for the variable I was returning so my guess is that whatever problems the cloneNode method had have since been worked out. If you can't recreate it with your test then you should close it.

        Show
        Mark Ford added a comment - I don't have a test case handy and I'm not actively using ODE any more. I tested the cloneNode code from the 1.3.x branch and it doesn't currently exhibit this behavior. I couldn't attach the original process or even the original message but I'm sure that it was failing with the xmlns prefix. The sample message I posted above is similar to what I was seeing in the ODE logs for the variable I was returning so my guess is that whatever problems the cloneNode method had have since been worked out. If you can't recreate it with your test then you should close it.
        Hide
        Rafal Rusin added a comment -

        OK, thanks for feedback.

        Show
        Rafal Rusin added a comment - OK, thanks for feedback.

          People

          • Assignee:
            Rafal Rusin
            Reporter:
            Mark Ford
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development