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 2023/04/17 15:47:38 UTC

[daffodil] branch main updated: Fix min/maxOccurs validation for optional elements

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

slawrence 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 c8e7059ed Fix min/maxOccurs validation for optional elements
c8e7059ed is described below

commit c8e7059ed70ba4e5f97c610b946dc9d33b4a2027
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Mon Apr 17 09:56:11 2023 -0400

    Fix min/maxOccurs validation for optional elements
    
    Commit dd60b6067d made changes to validation so that it took into
    account absent elements that incremented the array iteration count but
    did not actually change the array size. It did this by just counting the
    actual elements in the infoset. Unfortunately, that change was very
    subtly broken for optional elements since it assumed maybeLastChild was
    always a DIArray, but it could actually be a DISimple/DIComplex for
    optional elements since they use very similar array logic.
    
    We could modify this logic to take DISimple/DIComplex nodes into
    account, but later commit 16c738c38f added new state to differentiate
    between the occurs index and the iteration index. So this just modifies
    the array validation logic so it is very similar to the old logic, but
    uses the new state that accounts for absent elements.
    
    DAFFODIL-2808
---
 .../runtime1/SequenceChildUnparsers.scala          |  3 +-
 .../processors/parsers/SequenceChildBases.scala    | 30 ++++----------
 .../section02/validation_errors/Validation.tdml    | 47 +++++++++++++++++++++-
 .../validation_errors/TestValidationErr.scala      |  2 +
 4 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SequenceChildUnparsers.scala b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SequenceChildUnparsers.scala
index c6cb56798..2263cf89b 100644
--- a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SequenceChildUnparsers.scala
+++ b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SequenceChildUnparsers.scala
@@ -112,8 +112,7 @@ abstract class RepeatingChildUnparser(
     }
 
     // State could be Success or Failure here.
-
-    endArray(state)
+    endArray(state, state.occursPos - 1)
   }
 
   /**
diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SequenceChildBases.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SequenceChildBases.scala
index 85323885c..0fb555991 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SequenceChildBases.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SequenceChildBases.scala
@@ -346,8 +346,8 @@ abstract class RepeatingChildParser(
    */
   def endArray(state: PState): Unit = {
     state.mpstate.arrayIterationIndexStack.pop()
-    state.mpstate.occursIndexStack.pop()
-    super.endArray(state)
+    val occurrences = state.mpstate.occursIndexStack.pop() - 1
+    super.endArray(state, occurrences)
   }
 
 }
@@ -517,7 +517,7 @@ trait EndArrayChecksMixin {
 
   def erd: ElementRuntimeData
 
-  def endArray(state: ParseOrUnparseState): Unit = {
+  def endArray(state: ParseOrUnparseState, occurrences: Long): Unit = {
     if (state.processorStatus eq Success) {
 
       val shouldValidate =
@@ -528,38 +528,24 @@ trait EndArrayChecksMixin {
         val maxO = erd.maxOccurs
         val isUnbounded = maxO == -1
 
-        // At this point, state.infoset is the parent node of the DIArray that
-        // we are trying to validate. If any elements were added for this
-        // array, then the DIArray will be the last child of that parent node
-        // AND it will have the same erd as this parser. If that's not the
-        // case, and the parent has no children or it does but the last child
-        // has a different erd than what we expect, then it must mean nothing
-        // was added for this array and the occurrences are zero.
-        val occurrence = {
-          val maybeLastChild = state.infoset.maybeLastChild
-          if (maybeLastChild.isEmpty || maybeLastChild.get.erd != erd)
-            0
-          else
-            maybeLastChild.get.numChildren
-        }
-        if (isUnbounded && occurrence < minO)
+        if (isUnbounded && occurrences < minO)
           state.validationError(
             "%s occurred '%s' times when it was expected to be a " +
               "minimum of '%s' and a maximum of 'UNBOUNDED' times.",
             erd.diagnosticDebugName,
-            occurrence,
+            occurrences,
             minO,
           )
-        else if (!isUnbounded && (occurrence < minO || occurrence > maxO))
+        else if (!isUnbounded && (occurrences < minO || occurrences > maxO)) {
           state.validationError(
             "%s occurred '%s' times when it was expected to be a " +
               "minimum of '%s' and a maximum of '%s' times.",
             erd.diagnosticDebugName,
-            occurrence,
+            occurrences,
             minO,
             maxO,
           )
-        else {
+        } else {
           // ok
         }
       }
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml
index b37b6cb37..4656a0ba4 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section02/validation_errors/Validation.tdml
@@ -2157,5 +2157,50 @@
       <tdml:error>5.0</tdml:error>
     </tdml:validationErrors>
   </tdml:parserTestCase>
-  
+
+  <tdml:defineSchema name="validationOptional">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd" />
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited" />
+
+    <xs:element name="a">
+      <xs:complexType>
+        <xs:sequence dfdl:separator="/">
+          <xs:element name="b" type="xs:string" />
+          <xs:element name="num" type="xs:int" minOccurs="0" />
+          <xs:element name="c" minOccurs="0">
+            <xs:complexType>
+              <xs:sequence dfdl:separator="/">
+                <xs:element name="str" type="xs:string" />
+                <xs:element name="float" type="xs:float" dfdl:textNumberPattern="0.0" />
+                <xs:element name="int" type="xs:int" />
+              </xs:sequence>
+            </xs:complexType>
+          </xs:element>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="optional_element_1" root="a"
+    model="validationOptional"
+    roundTrip="onePass"
+    validation="on">
+    <tdml:document>B/2/A/2.0/4</tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <a>
+          <b>B</b>
+          <num>2</num>
+          <c>
+            <str>A</str>
+            <float>2.0</float>
+            <int>4</int>
+          </c>
+        </a>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+    <tdml:validationErrors />
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala
index 2506fec69..d1fef3f67 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section02/validation_errors/TestValidationErr.scala
@@ -186,4 +186,6 @@ class TestValidationErr {
   @Test def test_doubleInclusiveInf(): Unit = { runner.runOneTest("doubleInclusiveInf") }
   @Test def test_doubleInclusiveNegInf(): Unit = { runner.runOneTest("doubleInclusiveNegInf") }
   @Test def test_doubleInclusiveNaN(): Unit = { runner.runOneTest("doubleInclusiveNaN") }
+
+  @Test def test_optional_element_1(): Unit = { runner.runOneTest("optional_element_1") }
 }