You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by ja...@apache.org on 2023/02/21 16:43:58 UTC

[daffodil] branch main updated: Fix issue determining next element for chioces

This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git


The following commit(s) were added to refs/heads/main by this push:
     new 8c7ef7680 Fix issue determining next element for chioces
8c7ef7680 is described below

commit 8c7ef76808f5fca6bddaefe77d9689b4a0604a30
Author: Josh Adams <ja...@owlcyberdefense.com>
AuthorDate: Wed Jan 25 08:15:59 2023 -0500

    Fix issue determining next element for chioces
    
    There was an issue with determining the next possible elements for
    choices where a choice branch ends with an optional element.
    
    Essentially what was wrong was when we were determining if a choice
    branch was Closed or Open we would check just the first element of the
    choice branch and if it was Closed we wouldn't check the rest of the
    elements. However, if there are open elements following the initial
    Closed elements of a choice branch, that branch should be considered
    Open.  This was resulting in the following situation for our test
    schema:
    
    <xs:group name="groupOfChoice">
      <xs:sequence>
        <xs:element name="key" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
        <xs:choice dfdl:choiceDispatchKey="{ ./ex:key }">
          <xs:sequence dfdl:choiceBranchKey="a">
            <xs:element name="elt_a" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3" />
          </xs:sequence>
          <xs:sequence dfdl:choiceBranchKey="b">
            <xs:element name="elt_b" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3" />
            <xs:element name="elt_c" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3"
              minOccurs="0" maxOccurs="1" dfdl:occursCountKind="expression" dfdl:occursCount="{ if (fn:exists(../ex:elt_b)) then 1 else 0 }" />
          </xs:sequence>
        </xs:choice>
      </xs:sequence>
    </xs:group>
    
    <xs:element name="dd9">
      <xs:complexType>
        <xs:sequence>
          <xs:element name="prev" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
          <xs:group ref="groupOfChoice" />
          <xs:element name="post" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
        </xs:sequence>
      </xs:complexType>
    </xs:element>
    
    We would succesfully unparse one instance of elt_c and when determining
    if we should continue unparsing elt_c (since it is an array) we would
    end up attempting to determine what the next element (post) would be,
    which can't be found within the context of elt_c. Normally we would
    backtrack up pthe TermRuntimeData stack until we got to groupOfChoice at
    which point we could find 'post'. However we would backtrack to the
    choice at which point the choice would say all my branches are closed,
    therefore something must be found within one of my branches and if it
    can't be found, we would stop backtracking and return an
    UnexpectedElementErrorERD.
    
    The fix was to always consider the following elements regardless if the
    first element is Closed to determine if the overall sequence is Open or
    Closed.
    
    DAFFODIL-2615
---
 .../daffodil/core/runtime1/TermRuntime1Mixin.scala | 52 +++++++++++++--------
 .../infoset/PartialNextElementResolver.scala       | 12 ++++-
 .../daffodil/section15/choice_groups/choice.tdml   | 53 ++++++++++++++++++++++
 .../section15/choice_groups/TestChoice.scala       | 11 ++++-
 4 files changed, 106 insertions(+), 22 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/TermRuntime1Mixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/TermRuntime1Mixin.scala
index c5b3fecf4..924017f53 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/TermRuntime1Mixin.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/TermRuntime1Mixin.scala
@@ -21,6 +21,7 @@ import org.apache.daffodil.core.dsom._
 import org.apache.daffodil.lib.api.WarnID
 import org.apache.daffodil.lib.exceptions.Assert
 import org.apache.daffodil.lib.schema.annotation.props.gen.NilKind
+import org.apache.daffodil.lib.util.Logger
 import org.apache.daffodil.lib.util.Maybe
 import org.apache.daffodil.lib.xml.QNameBase
 import org.apache.daffodil.runtime1.dsom._
@@ -194,6 +195,7 @@ trait TermRuntime1Mixin { self: Term =>
     }
     thisItself
   }
+
   /*
    * Returns a Closed or Open list of posslble elements that could follow this Term, within its
    * lexically enclosing model group.
@@ -229,10 +231,16 @@ trait TermRuntime1Mixin { self: Term =>
       // term itself has not provided a match to the incoming event, and we need to
       // see if things following the term provide a match.
       //
-      case stb: SequenceTermBase =>
+      case stb: SequenceTermBase => {
         followingLexicalSiblingStreamingUnparserElements
-      case _ =>
-        possibleSelfPlusNextLexicalSiblingStreamingUnparserElements
+      }
+      case _: ElementBase | _: ChoiceTermBase => {
+        // If this element is closed, we only want it and not it's siblings
+        if (possibleThisTermNextStreamingUnparserElements.isClosed)
+          possibleThisTermNextStreamingUnparserElements
+        else
+          possibleSelfPlusNextLexicalSiblingStreamingUnparserElements
+      }
     }
     //
     // Check for ambiguity except for namespaces
@@ -332,24 +340,28 @@ trait TermRuntime1Mixin { self: Term =>
           res.getOrElse(Open(Nil))
         }
       }
-      val res = thisItself match {
-        //
-        // check this case separately first to avoid evaluating
-        // followingLexicalSiblingStreamingUnparserElements unless
-        // we have to.
-        //
-        case Closed(pnes1) => thisItself // most common case - a required element.
-        case _ => {
-          val res: PossibleNextElements =
-            (thisItself, followingLexicalSiblingStreamingUnparserElements) match {
-              case (Open(pnes1), Closed(Nil)) =>
-                thisItself // case of Open(...) followed by end of complex element.
-              case (poss1, Closed(pnes2)) => Closed((poss1.pnes ++ pnes2).distinct)
-              case (poss1, poss2) => Open((poss1.pnes ++ poss2.pnes).distinct)
-            }
-          res
+      Logger.log.debug(s"""
+        NextElementResolver -> thisItself: $thisItself\n
+        NextElementResolver -> following: $followingLexicalSiblingStreamingUnparserElements""")
+      val res: PossibleNextElements =
+        (thisItself, followingLexicalSiblingStreamingUnparserElements) match {
+          case (_, Closed(Nil)) => {
+            thisItself // case of Open(...) followed by end of complex element.
+          }
+          case (Closed(pnes1), Open(List())) => {
+            thisItself // case of Closed(...) followed by empty list, which is default for nothing
+          }
+          case (Closed(pnes1), Closed(pnes2)) => {
+            thisItself // When everything is closed, we only want thisItself and not the entire list of Closed possibilities
+          }
+          case (Open(pnes1), Closed(pnes2)) => {
+            // If there are optional elements before a required element, the whole sequence is considered closed
+            Closed((pnes1 ++ pnes2).distinct)
+          }
+          case (poss1, poss2) => {
+            Open((poss1.pnes ++ poss2.pnes).distinct)
+          }
         }
-      }
       res
     }.value
 
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala
index 446fa2983..335822032 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala
@@ -18,6 +18,7 @@
 package org.apache.daffodil.runtime1.infoset
 
 import org.apache.daffodil.lib.exceptions.Assert
+import org.apache.daffodil.lib.util.Logger
 import org.apache.daffodil.lib.util.MStackOf
 import org.apache.daffodil.lib.util.Maybe
 import org.apache.daffodil.lib.util.Maybe._
@@ -45,6 +46,7 @@ trait NextElementResolver { self: InfosetInputter =>
     nameSpace: String,
     hasNamespace: Boolean,
   ): ElementRuntimeData = {
+    Logger.log.debug(s"NextERDResovler -> trdStack: $trdStack")
     val iter = trdStack.iter
     iter.reset()
     var firstOne: Boolean = true
@@ -75,6 +77,10 @@ trait NextElementResolver { self: InfosetInputter =>
         // or it will issue an UnparseError if there are required elements in the
         // set of possible elements.
         //
+        Logger.log.debug(s"""
+          NextERDResovler -> current trd: $trd\n
+          NextERDResovler -> resolver: $resolver\n
+          NextERDResovler -> name: $name\n""")
         maybeERD = resolver.maybeNextElement(name, nameSpace, hasNamespace)
         // The cases where ERD is in a hidden context are addressed where the elements
         // are assigned their value i.e ElementUnparser
@@ -297,6 +303,10 @@ class SeveralPossibilitiesForNextElement(
     namespace: String,
     hasNamespace: Boolean,
   ): Maybe[ElementRuntimeData] = {
+    Logger.log.debug(s"""
+      NextERDResovler -> trd: $trd\n
+      NextERDResovler -> looking for: $local\n
+      NextERDResovler -> nextERDMap: $nextERDMap\n""")
     val optnextERD =
       if (hasNamespace) {
         val sqn = StepQName(
@@ -314,7 +324,7 @@ class SeveralPossibilitiesForNextElement(
           // It was statically determined at compile time that this
           // NextElementResolver does not have any possible NextERDs with the
           // same local name. Because of this, just find the first thing that
-          // matches and return it it is was found.
+          // matches and return it if it was found.
           nextERDMap.find(_._1.local == local).map(_._2)
         } else {
           // It was statically determined at compile time that some nextERDs
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice.tdml
index 90b917f8c..b345a2a99 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice.tdml
@@ -1805,6 +1805,26 @@ it sure is/
       </xs:complexType>
     </xs:element>
 
+    <xs:element name="dd10">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="prev" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
+          <xs:element name="key" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
+          <xs:choice dfdl:choiceDispatchKey="{ ./ex:key }">
+            <xs:sequence dfdl:choiceBranchKey="a">
+              <xs:element name="elt_a" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3" />
+            </xs:sequence>
+            <xs:sequence dfdl:choiceBranchKey="b">
+              <xs:element name="elt_b" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3" />
+              <xs:element name="elt_c" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="3"
+                minOccurs="0" maxOccurs="1" dfdl:occursCountKind="expression" dfdl:occursCount="{ if (fn:exists(../ex:elt_b)) then 1 else 0 }" />
+            </xs:sequence>
+          </xs:choice>
+          <xs:element name="post" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1" />
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
   </tdml:defineSchema>
 
   <tdml:parserTestCase name="direct_dispatch_01" root="dd1"
@@ -2046,4 +2066,37 @@ it sure is/
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="dispatch_group_choice_2" root="dd10"
+    model="direct_dispatch_1" roundTrip="true"
+    description="simple case of direct dispatch - DFDL-15-001R. Uses group containing a choice with direct dispatch">
+    <tdml:document><![CDATA[0bbbbccc1]]></tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset xmlns:ex="http://example.com">
+        <ex:dd10>
+          <ex:prev>0</ex:prev>
+          <ex:key>b</ex:key>
+          <ex:elt_b>bbb</ex:elt_b>
+          <ex:elt_c>ccc</ex:elt_c>
+          <ex:post>1</ex:post>
+        </ex:dd10>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="dispatch_group_choice_3" root="dd10"
+    model="direct_dispatch_1" roundTrip="true"
+    description="simple case of direct dispatch - DFDL-15-001R. Uses group containing a choice with direct dispatch">
+    <tdml:document><![CDATA[0aaaa1]]></tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset xmlns:ex="http://example.com">
+        <ex:dd10>
+          <ex:prev>0</ex:prev>
+          <ex:key>a</ex:key>
+          <ex:elt_a>aaa</ex:elt_a>
+          <ex:post>1</ex:post>
+        </ex:dd10>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice.scala
index 4eead0f6e..1e6a0f02a 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice.scala
@@ -161,7 +161,16 @@ class TestChoice {
   @Test def test_direct_dispatch_17(): Unit = { runnerCH.runOneTest("direct_dispatch_17") }
 
   // DAFFODIL-2562
-  // @Test def test_dispatch_group_choice(): Unit = { runnerCH.runOneTest("dispatch_group_choice") }
+  @Test def test_dispatch_group_choice(): Unit = {
+    runnerCH.runOneTest("dispatch_group_choice")
+  }
+
+  @Test def test_dispatch_group_choice_2(): Unit = {
+    runnerCH.runOneTest("dispatch_group_choice_2")
+  }
+  @Test def test_dispatch_group_choice_3(): Unit = {
+    runnerCH.runOneTest("dispatch_group_choice_3")
+  }
 
   @Test def test_explicit_01(): Unit = { runnerCE.runOneTest("explicit_01") }
   @Test def test_explicit_02(): Unit = { runnerCE.runOneTest("explicit_02") }