Fix NamespaceDifferencesOnly Warning when namespaces are same

- currently the code doesn't check if the namespaces are the same and reports that namespaces are different as long as the local names are duplicated. This is incorrect and we add a fix that only generates the error if the namespaces are actually different - fix incorrect de-duplication of Sibs where we were overwriting the initial duplicates with the later duplicates. Now we prioritize the first "duplicates" over any secondary ones DAFFODIL-2938
apache · Oct 11, 2024 · c67a0fe · c67a0fe
1 parent a1f7f96
commit c67a0fe
Show file tree

Hide file tree

Showing 4 changed files with 142 additions and 18 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
@@ -255,15 +255,18 @@ trait TermRuntime1Mixin { self: Term =>
       val groupedByName = possibles.pnes.groupBy(_.e.namedQName.local)
       groupedByName.foreach { case (_, sameNamesEB) =>
         if (sameNamesEB.length > 1) {
-          SDW(
-            WarnID.NamespaceDifferencesOnly,
-            "Neighboring QNames differ only by namespaces. " +
-              "Infoset representations that do not support namespaces " +
-              "cannot differentiate between these elements and " +
-              "may fail to unparse. QNames are: %s",
-            sameNamesEB.map(_.e.namedQName.toExtendedSyntax).mkString(", ")
-          )
-          hasNamesDifferingOnlyByNS = true
+          val groupedByNamespace = sameNamesEB.groupBy(_.e.namedQName.namespace)
+          if (groupedByNamespace.size > 1) {
+            SDW(
+              WarnID.NamespaceDifferencesOnly,
+              "Neighboring QNames differ only by namespaces. " +
+                "Infoset representations that do not support namespaces " +
+                "cannot differentiate between these elements and " +
+                "may fail to unparse. QNames are: %s",
+              sameNamesEB.map(_.e.namedQName.toExtendedSyntax).mkString(", ")
+            )
+            hasNamesDifferingOnlyByNS = true
+          }
         }
       }
     }
@@ -458,12 +461,15 @@ trait TermRuntime1Mixin { self: Term =>
         // the way one would normally expect a collection to be.
         // So Map[NamedQName, ElementRuntimeData] is not a subtype of Map[QNameBase, ElementRuntimeData]
         // So we need a cast upward to Map[QNameBase,ElementRuntimeData]
-        //
+        // We need the fold because .toMAp overwrites earlier element with duplicate later elements,
+        // but we don't want that, so instead we deduplicate here with earlier elements taking precedence
         val eltMap = sibs
           .map { sib =>
             (sib.e.namedQName, sib.e.erd)
           }
-          .toMap
+          .foldLeft(Map.empty[QNameBase, ElementRuntimeData]) { case (map, (key, value)) =>
+            if (map.contains(key)) map else map + (key -> value)
+          }
           .asInstanceOf[Map[QNameBase, ElementRuntimeData]]
         val resolver = eltMap.size match {
           case 0 => new NoNextElement(trd, isRequiredStreamingUnparserEvent)

diff --git a/...ime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala b/...ime1/src/main/scala/org/apache/daffodil/runtime1/infoset/PartialNextElementResolver.scala
@@ -304,9 +304,9 @@ class SeveralPossibilitiesForNextElement(
     hasNamespace: Boolean
   ): Maybe[ElementRuntimeData] = {
     Logger.log.debug(s"""
-      NextERDResovler -> trd: $trd\n
-      NextERDResovler -> looking for: $local\n
-      NextERDResovler -> nextERDMap: $nextERDMap\n""")
+      NextERDResolver -> trd: $trd\n
+      NextERDResolver -> looking for: $local\n
+      NextERDResolver -> nextERDMap: $nextERDMap\n""")
     val optnextERD =
       if (hasNamespace) {
         val sqn = StepQName(

diff --git a/...-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml b/...-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml
@@ -60,7 +60,7 @@
        Schema: emptySDEschema
       Purpose: This test demonstrates that DFDL allows complexTypes with empty content (not to be confused with NO content)
   -->
-  
+
   <tdml:parserTestCase name="complexEmptyContent" root="complexEmptyContent"
     model="emptySDEschema" description="Section 14 - DFDL-14-007R.">
     <tdml:document>
@@ -1206,12 +1206,58 @@
           dfdl:lengthKind="delimited" />
       </xs:choice>
     </xs:group>
+    <xs:group name="hg_sep">
+      <xs:choice>
+        <xs:element name="inty" type="xs:int" dfdl:initiator="+"
+          dfdl:lengthKind="delimited" dfdl:outputValueCalc="{ 1 }" />
+        <xs:element name="stringy" type="xs:string"
+          dfdl:lengthKind="delimited" />
+      </xs:choice>
+    </xs:group>
+    <xs:group name="vg_sep">
+      <xs:choice>
+        <xs:element name="inty" type="xs:int"  dfdl:initiator="-"
+          dfdl:lengthKind="delimited" />
+        <xs:element name="stringy" type="xs:string"
+          dfdl:lengthKind="delimited" />
+      </xs:choice>
+    </xs:group>
+
+    <xs:element name="root2">
+      <xs:complexType>
+        <xs:group ref="ex:g3" dfdl:separator=","/>
+      </xs:complexType>
+    </xs:element>
+    <xs:group name="g3">
+      <xs:sequence dfdl:separatorPosition="infix">
+        <xs:element name="g3c0" type="xs:string" />
+        <xs:element name="g3c1" type="xs:string" />
+        <xs:element name="g3c2" type="xs:string" />
+        <xs:group ref="ex:hg" />
+        <xs:group ref="ex:vg" />
+      </xs:sequence>
+    </xs:group>
+    <xs:element name="root3">
+      <xs:complexType>
+        <xs:group ref="ex:g4" dfdl:separator=","/>
+      </xs:complexType>
+    </xs:element>
+    <xs:group name="g4">
+      <xs:sequence dfdl:separatorPosition="infix">
+        <xs:element name="c0" type="xs:string" />
+        <xs:element name="c1" type="xs:string" />
+        <xs:element name="c2" type="xs:string" />
+        <xs:group ref="ex:hg_sep" />
+        <xs:group ref="ex:vg_sep" />
+      </xs:sequence>
+    </xs:group>
 
   </tdml:defineSchema>
 
 	<tdml:parserTestCase name="nestedGroupRefs2" root="root"
 		model="nestedGroupRefs2" description="Verify that nested hidden groups and choices work correctly - DFDL-14-039R"
-		roundTrip="twoPass">
+		roundTrip="twoPass"
+    ignoreUnexpectedWarnings="false">
 
 		<tdml:document>
 		  <tdml:documentPart type="text" replaceDFDLEntities="true"><![CDATA[A,B,C,1,x|D,E,F,2,y|G,H,I,3,z%LF;A,B,C,x,1|D,E,F,y,2|G,H,I,z,3%LF;A,B,C,4,u|D,E,F,5,v|G,H,I,6,w]]></tdml:documentPart>
@@ -1286,12 +1332,81 @@
 
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="nestedGroupRefs3" root="root2"
+                       model="nestedGroupRefs2" description="Verify that nested groups and choices work correctly - DFDL-14-039R"
+                       roundTrip="onePass"
+                       ignoreUnexpectedWarnings="false">
+
+    <tdml:document>
+      <tdml:documentPart type="text" replaceDFDLEntities="true"><![CDATA[A,B,C,1,x]]></tdml:documentPart>
+    </tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root2>
+          <g3c0>A</g3c0>
+          <g3c1>B</g3c1>
+          <g3c2>C</g3c2>
+          <inty>1</inty>
+          <stringy>x</stringy>
+        </root2>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="nestedGroupRefs4" root="root2"
+                       model="nestedGroupRefs2" description="Verify that nested groups and choices work correctly - DFDL-14-039R"
+                       roundTrip="onePass"
+                       ignoreUnexpectedWarnings="false">
+
+    <tdml:document>
+      <tdml:documentPart type="text" replaceDFDLEntities="true"><![CDATA[A,B,C,x,1]]></tdml:documentPart>
+    </tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root2>
+          <g3c0>A</g3c0>
+          <g3c1>B</g3c1>
+          <g3c2>C</g3c2>
+          <stringy>x</stringy>
+          <inty>1</inty>
+        </root2>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="nestedGroupRefs5" root="root3"
+                       model="nestedGroupRefs2" description="Verify that nested groups and choices work correctly - DFDL-14-039R"
+                       roundTrip="onePass"
+                       ignoreUnexpectedWarnings="false">
+
+    <tdml:document>
+      <tdml:documentPart type="text" replaceDFDLEntities="true"><![CDATA[A,B,C,x,1]]></tdml:documentPart>
+    </tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root3>
+          <c0>A</c0>
+          <c1>B</c1>
+          <c2>C</c2>
+          <stringy>x</stringy>
+          <stringy>1</stringy>
+        </root3>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+  </tdml:parserTestCase>
+
   <tdml:defineSchema name="noDefault">
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited" />
 
     <xs:element name="emptySeq" type="ex:complexEmptySeq" dfdl:lengthKind="implicit"/>
-    
+
     <xs:complexType name="complexEmptySeq">
       <xs:sequence/>
     </xs:complexType>
@@ -1302,7 +1417,7 @@
        Schema: emptySDEschema
       Purpose: This test demonstrates that DFDL allows complexTypes with empty content (not to be confused with NO content)
   -->
-  
+
   <tdml:parserTestCase name="noDefaultSeqKind" root="emptySeq"
     model="noDefault" description="Section 14 - DFDL-14-007R.">
     <tdml:document>

diff --git a/...est/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala b/...est/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala
@@ -106,6 +106,9 @@ class TestSequenceGroups {
   @Test def test_hiddenGroupNested2(): Unit = { runner_02.runOneTest("hiddenGroupNested2") }
   @Test def test_nestedGroupRefs(): Unit = { runner_02.runOneTest("nestedGroupRefs") }
   @Test def test_nestedGroupRefs2(): Unit = { runner_02.runOneTest("nestedGroupRefs2") }
+  @Test def test_nestedGroupRefs3(): Unit = { runner_02.runOneTest("nestedGroupRefs3") }
+  @Test def test_nestedGroupRefs4(): Unit = { runner_02.runOneTest("nestedGroupRefs4") }
+  @Test def test_nestedGroupRefs5(): Unit = { runner_02.runOneTest("nestedGroupRefs5") }
   @Test def test_hiddenGroupChoice(): Unit = { runner_02.runOneTest("hiddenGroupChoice") }
   @Test def test_hiddenGroupChoice2(): Unit = { runner_02.runOneTest("hiddenGroupChoice2") }
   @Test def test_hiddenGroupIgnoredProps(): Unit = {