You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by sl...@apache.org on 2020/01/03 15:37:37 UTC

[incubator-daffodil] branch master updated: Fix unparsing nested arrays when separatorSuppressionPolicy="never"

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

slawrence pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git


The following commit(s) were added to refs/heads/master by this push:
     new d0a25d2  Fix unparsing nested arrays when separatorSuppressionPolicy="never"
d0a25d2 is described below

commit d0a25d2fc43d3b2789d6da14419049be2b06aa56
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Thu Jan 2 14:16:45 2020 -0500

    Fix unparsing nested arrays when separatorSuppressionPolicy="never"
    
    When data has separators and separatorSuppressionPolicy="never", then we
    were not pushing/popping onto the arrayIndexStack during unparsing. This
    meant that if an array with separatorSuppressionPolicy="never" was a
    child of a parent array, then we would incorrectly modify the array
    stack of the parent instead of the childs own array stack. This led to
    incorrect array indices and invariant failures.
    
    To fix this, this copies the array stack logic and invariant checks from
    unparseWithSuppression into the unparseWithNoSuppression.
    
    DAFFODIL-2263
---
 .../unparsers/SeparatedSequenceUnparsers.scala     | 21 ++++++++-
 .../array_optional_elem/ArrayOptionalElem.tdml     | 52 ++++++++++++++++++++++
 .../TestArrayOptionalElem.scala                    |  3 ++
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SeparatedSequenceUnparsers.scala b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SeparatedSequenceUnparsers.scala
index 8132aaf..9bf58ef 100644
--- a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SeparatedSequenceUnparsers.scala
+++ b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SeparatedSequenceUnparsers.scala
@@ -260,7 +260,6 @@ class OrderedSeparatedSequenceUnparser(
           val erd = unparser.erd
           var numOccurrences = 0
           val maxReps = unparser.maxRepeats(state)
-          val isBounded = unparser.isBoundedMax
           //
           // The number of occurrances we unparse is always exactly driven
           // by the number of infoset events for the repeating/optional element.
@@ -467,9 +466,12 @@ class OrderedSeparatedSequenceUnparser(
       //
       childUnparser match {
         case unparser: RepOrderedSeparatedSequenceChildUnparser => {
+          state.arrayIndexStack.push(1L)
           val erd = unparser.erd
           var numOccurrences = 0
           val maxReps = unparser.maxRepeats(state)
+          //val isBounded = unparser.isBoundedMax // not needed for the no-suppression case
+
           //
           // The number of occurrances we unparse is always exactly driven
           // by the number of infoset events for the repeating/optional element.
@@ -493,14 +495,28 @@ class OrderedSeparatedSequenceUnparser(
                   doUnparser = unparser.shouldDoUnparser(unparser, state)
                   doUnparser
                 }) {
+                  //
+                  // These are so we can check invariants on these stacks being
+                  // pushed and popped reliably, and incremented only once
+                  //
+                  val arrayIndexBefore = state.arrayPos
+                  val arrayIndexStackDepthBefore = state.arrayIndexStack.length
+                  val groupIndexBefore = state.groupPos
+                  val groupIndexStackDepthBefore = state.groupIndexStack.length
+
+                  Assert.invariant(erd.isRepresented) // since this is an array, can't have inputValueCalc
+
                   if (isArr) if (state.dataProc.isDefined) state.dataProc.get.beforeRepetition(state, this)
 
                   unparseOne(unparser, erd, state)
                   numOccurrences += 1
-
+                  Assert.invariant(state.arrayIndexStack.length == arrayIndexStackDepthBefore)
                   state.moveOverOneArrayIndexOnly()
+                  Assert.invariant(state.arrayPos == arrayIndexBefore + 1)
 
+                  Assert.invariant(state.groupIndexStack.length == groupIndexStackDepthBefore)
                   state.moveOverOneGroupIndexOnly() // array elements are always represented.
+                  Assert.invariant(state.groupPos == groupIndexBefore + 1)
 
                   if (isArr) if (state.dataProc.isDefined) state.dataProc.get.afterRepetition(state, this)
                 }
@@ -541,6 +557,7 @@ class OrderedSeparatedSequenceUnparser(
             // no event (state.inspect returned false)
             Assert.invariantFailed("No event for unparsing.")
           }
+          state.arrayIndexStack.pop()
         }
         case scalarUnparser => {
           unparseOne(scalarUnparser, trd, state)
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
index 2f5815a..e3c40ff 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
@@ -1070,4 +1070,56 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+ <tdml:defineSchema name="dfdl2263"  elementFormDefault="unqualified">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormatPortable" lengthKind="delimited"/>
+
+    <xs:element name="input">
+      <xs:complexType>
+        <xs:sequence dfdl:separator="%NL;" dfdl:separatorPosition="infix">
+          <xs:element name="header">
+            <xs:complexType>
+              <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix"  dfdl:separatorSuppressionPolicy="anyEmpty">
+                <xs:element name="title" maxOccurs="unbounded" type="xs:string" />
+              </xs:sequence>
+            </xs:complexType>
+          </xs:element>
+          <xs:element name="row" maxOccurs="unbounded">
+            <xs:complexType>
+              <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix" dfdl:separatorSuppressionPolicy="never">
+                <xs:element name="field" maxOccurs="unbounded" type="xs:string"
+                  dfdl:occursCount="{ fn:count(../../header/title) }"
+                  dfdl:occursCountKind="expression" />
+              </xs:sequence>
+            </xs:complexType>
+          </xs:element>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="dfdl2263" root="input" model="dfdl2263">
+
+    <tdml:document>
+      <tdml:documentPart type="text" replaceDFDLEntities="true"><![CDATA[header1,header2%LF;a,]]></tdml:documentPart>
+    </tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <ex:input>
+          <header>
+            <title>header1</title>
+            <title>header2</title>
+          </header>
+          <row>
+            <field>a</field>
+            <field></field>
+          </row>
+        </ex:input>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
index 38e674c..6c6bda3 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
@@ -87,4 +87,7 @@ class TestArrayOptionalElem {
   // DAFFODIL-1886
   @Test def test_manyAdjacentOptionals_01() { runner.runOneTest("manyAdjacentOptionals_01") }
 
+  // DAFFODIL-2263
+  @Test def test_dfdl2263() { runner.runOneTest("dfdl2263") }
+
 }