Skip to content

Change TDMLRunner to use XMLTextInfosetInputter/Outputter as default #1650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
olabusayoT wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Change TDMLRunner to use XMLTextInfosetInputter/Outputter as default #1650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e630050
Change TDMLRunner to use XMLTextInfosetInputter/Outputter as default
olabusayoT Apr 2, 2026
0ac13c9
fixup!
olabusayoT Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions daffodil-core/src/main/resources/org/apache/daffodil/xsd/tdml.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,21 @@
</simpleType>

<simpleType name="validationType">
<restriction base="xs:token">
<enumeration value="on"/>
<enumeration value="limited"/>
<enumeration value="off"/>
</restriction>
<union>
<simpleType>
<restriction base="xs:token">
<enumeration value="on"/>
<enumeration value="limited"/>
<enumeration value="off"/>
</restriction>
</simpleType>

<simpleType>
<restriction base="xs:token">
<pattern value="[A-Za-z0-9_]+"/>
</restriction>
</simpleType>
</union>
</simpleType>

<element name="document" type="tns:documentType"/>
Expand Down
39 changes: 30 additions & 9 deletions daffodil-core/src/main/scala/org/apache/daffodil/lib/xml/DaffodilConstructingLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,16 @@ object Position {
* behavior of normalizing CRLF to LF, and solitary CR to LF.
* Defaults to true. Should only be changed in special circumstances
* as not normalizing CRLFs is non-standard for XML.
*
* @param removeComments True to remove comments. This is used to keep the XML as close to the original as possible
* @param removeProcInstr True to remove processing instructions. This is used to keep the XML as close to the original as possible
*/
class DaffodilConstructingLoader private[xml] (
uri: URI,
errorHandler: org.xml.sax.ErrorHandler,
addPositionAttributes: Boolean,
normalizeCRLFtoLF: Boolean
normalizeCRLFtoLF: Boolean,
removeComments: Boolean,
removeProcInstr: Boolean
) extends ConstructingParser(
{
// Note: we must open the XML carefully since it might be in some non
Expand All @@ -122,7 +125,14 @@ class DaffodilConstructingLoader private[xml] (
errorHandler: org.xml.sax.ErrorHandler,
addPositionAttributes: Boolean = false
) =
this(uri, errorHandler, addPositionAttributes, normalizeCRLFtoLF = true)
this(
uri,
errorHandler,
addPositionAttributes,
normalizeCRLFtoLF = true,
removeComments = true,
removeProcInstr = true
)

/**
* Ensures that DOCTYPES aka DTDs, if encountered, are rejected.
Expand Down Expand Up @@ -316,19 +326,30 @@ class DaffodilConstructingLoader private[xml] (
}

/**
* Drops comments
* Drops comments if removeComments is true
*
* This is optional controlled by a constructor parameter.
*/
override def comment(pos: Int, s: String): Comment = {
// returning null drops comments
null
if (removeComments) {
// returning null drops comments
null
} else {
super.comment(pos, s)
}
}

/**
* Drops processing instructions
* Drops processing instructions if removeProcInstr is false
*
* This is optional controlled by a constructor parameter.
*/
override def procInstr(pos: Int, target: String, txt: String) = {
// returning null drops processing instructions
null
if (removeProcInstr) { // returning null drops processing instructions
null
} else {
super.procInstr(pos, target, txt)
}
}

private def parseXMLPrologAttributes(
Expand Down
31 changes: 11 additions & 20 deletions daffodil-core/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,31 +702,20 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
* @param optSchemaURI Optional URI for XML schema for the XML source document.
* @param addPositionAttributes True to add dafint:file dafint:line attributes to all elements.
* Defaults to false.
* @return an scala.xml.Node (Element actually) which is the document element of the source.
*/
def load(
source: DaffodilSchemaSource,
optSchemaURI: Option[URI],
addPositionAttributes: Boolean = false
): scala.xml.Node =
load(source, optSchemaURI, addPositionAttributes, normalizeCRLFtoLF = true)

/**
* package private constructor gives access to normalizeCRLFtoLF feature.
*
* @param source The URI for the XML document which may be a XML or DFDL schema, or just XML data.
* @param optSchemaURI Optional URI for XML schema for the XML source document.
* @param addPositionAttributes True to add dafint:file dafint:line attributes to all elements.
* Defaults to false.
* @param normalizeCRLFtoLF True to normalize CRLF and isolated CR to LF. This should usually be true,
* but some special case situations may require preservation of CRLF/CR.
* @param removeComments True to remove comments. This is used to keep the XML as close to the original as possible
* @param removeProcInstr True to remove processing instructions. This is used to keep the XML as close to the original as possible
*
* @return an scala.xml.Node (Element actually) which is the document element of the source.
*/
private[xml] def load(
def load(
source: DaffodilSchemaSource,
optSchemaURI: Option[URI],
addPositionAttributes: Boolean,
normalizeCRLFtoLF: Boolean
addPositionAttributes: Boolean = false,
normalizeCRLFtoLF: Boolean = true,
removeComments: Boolean = true,
removeProcInstr: Boolean = true
): scala.xml.Node = {
//
// First we invoke the validator to explicitly validate the XML against
Expand Down Expand Up @@ -819,7 +808,9 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
source.uriForLoading,
errorHandler,
addPositionAttributes,
normalizeCRLFtoLF
normalizeCRLFtoLF,
removeComments,
removeProcInstr
)
val res =
try {
Expand Down
131 changes: 106 additions & 25 deletions daffodil-core/src/main/scala/org/apache/daffodil/lib/xml/XMLUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import org.apache.daffodil.lib.iapi.URISchemaSource
import org.apache.daffodil.lib.schema.annotation.props.LookupLocation
import org.apache.daffodil.lib.util.Maybe
import org.apache.daffodil.lib.util.Misc
import org.apache.daffodil.runtime1.infoset.XMLTextInfoset

import org.apache.commons.io.IOUtils
import org.xml.sax.XMLReader
Expand Down Expand Up @@ -599,6 +600,14 @@ object XMLUtils {

def removeComments(e: Node): Node = {
e match {
case x @ Elem(
null,
XMLTextInfoset.stringAsXml,
Null,
NamespaceBinding(null, null | "", _),
_*
) =>
x
case Elem(prefix, label, attribs, scope, child*) => {
val newChildren = child.filterNot { _.isInstanceOf[Comment] }.map { removeComments(_) }
Elem(prefix, label, attribs, scope, true, newChildren*)
Expand Down Expand Up @@ -644,34 +653,66 @@ object XMLUtils {

private def removeMixedWhitespace(ns: Node): Node = {
if (!ns.isInstanceOf[Elem]) return ns
val e = ns.asInstanceOf[Elem]
val children = e.child
val noMixedChildren =
if (children.exists(_.isInstanceOf[Elem])) {
children
.filter {
case Text(data) if data.matches("""\s*""") => false
case Text(data) =>
throw new Exception("Element %s contains mixed data: %s".format(e.label, data))
case _ => true
}
.map(removeMixedWhitespace)
} else {
children.filter {
//
// So this is a bit strange, but we're dropping nodes that are Empty String.
//
// In XML we cannot tell <foo></foo> where there is a Text("") child, from <foo></foo> with Nil children
//
case Text("") => false // drop empty strings
case _ => true

ns match {
// NOTE: this is specifically for the stringAsXml feature as we avoid
// making changes to any of its children except removing any surrounding
// whitespace, requiring that stringAsXml in the infoset match results exactly.
case e @ Elem(
null,
XMLTextInfoset.stringAsXml,
Null,
NamespaceBinding(null, null | "", _),
_*
) => {
val (elemChildren, nonElemChildren) = e.child.partition {
_.isInstanceOf[Elem]
}
if (elemChildren.length != 1)
throw new Exception("stringAsXml must contain a single child element.")
nonElemChildren.foreach {
case Text(data) if data.matches("""\s*""") => // no-op, empty text siblings are fine
case x =>
throw new Exception(
"%s is some kind of mixed content not allowed as a stringAsXml child".format(x)
)
}
e.asInstanceOf[Elem].copy(child = elemChildren)
}
case _ => {
val e = ns.asInstanceOf[Elem]
val children = e.child
val noMixedChildren =
if (children.exists(_.isInstanceOf[Elem])) {
children
.filter {
case Text(data) if data.matches("""\s*""") => false
case Text(data) =>
throw new Exception(
"Element %s contains mixed data: %s".format(e.label, data)
)
case _ => true
}
.map(removeMixedWhitespace)
} else {
children.filter {
//
// So this is a bit strange, but we're dropping nodes that are Empty String.
//
// In XML we cannot tell <foo></foo> where there is a Text("") child, from <foo></foo> with Nil children
//
case Text("") => false // drop empty strings
case _ => true
}
}

val res =
if (noMixedChildren eq children) e
else e.copy(child = noMixedChildren)
res
}
}

val res =
if (noMixedChildren eq children) e
else e.copy(child = noMixedChildren)
res
}

/**
Expand Down Expand Up @@ -700,6 +741,15 @@ object XMLUtils {
): NodeSeq = {
val res = n match {

case e @ Elem(
null,
XMLTextInfoset.stringAsXml,
Null,
NamespaceBinding(null, null | "", _),
_*
) =>
e

case e @ Elem(prefix, label, attributes, scope, children*) => {

val filteredScope = if (ns.length > 0) filterScope(scope, ns) else xml.TopScope
Expand Down Expand Up @@ -973,6 +1023,15 @@ Differences were (path, expected, actual):
} else if (checkPrefixes && prefixA != prefixB) {
// different prefix
List((zPath + "/" + labelA + "@prefix", prefixA, prefixB))
} else if (checkPrefixes && a.scope.getURI(prefixA) != b.scope.getURI(prefixB)) {
// prefixes doesn't resolve to same namespace
List(
(
zPath + "/" + labelA + "@prefix-namespace",
a.scope.getURI(prefixA),
b.scope.getURI(prefixB)
)
)
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for these new checks to fail? I thought we validated the expected infoset to make sure it was valid, which I think should check to make sure namespaces resolve?

Copy link
Copy Markdown
Contributor Author

@olabusayoT olabusayoT Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we validate the expected infoset, it looks like it's still an open ticket

https://issues.apache.org/jira/projects/DAFFODIL/issues/DAFFODIL-288

That being said I don't think it can actually fail, since a.getNamespace(prefixA) is usually equal to nsbA.getURI(prefixA). I cannot think of a failing elem example

} else if (checkNamespaces && mappingsA != mappingsB) {
// different namespace bindings
List((zPath + "/" + labelA + "@xmlns", mappingsA, mappingsB))
Expand Down Expand Up @@ -1055,6 +1114,28 @@ Differences were (path, expected, actual):
computeTextDiff(zPath, tA, tB, maybeType, maybeFloatEpsilon, maybeDoubleEpsilon)
thisDiff
}
case (cA: Comment, cB: Comment) => {
val thisDiff = computeTextDiff(
zPath,
cA.toString,
cB.toString,
maybeType,
maybeFloatEpsilon,
maybeDoubleEpsilon
)
thisDiff
}
case (pcA: PCData, pcB: PCData) => {
val thisDiff = computeTextDiff(
zPath,
pcA.toString,
pcB.toString,
maybeType,
maybeFloatEpsilon,
maybeDoubleEpsilon
)
thisDiff
}
case (pA: ProcInstr, pB: ProcInstr) => {
val ProcInstr(tA1label, tA1content) = pA
val ProcInstr(tB1label, tB1content) = pB
Expand Down
25 changes: 3 additions & 22 deletions ...l-core/src/main/scala/org/apache/daffodil/runtime1/infoset/ScalaXMLInfosetOutputter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ package org.apache.daffodil.runtime1.infoset

import scala.collection.mutable.ListBuffer
import scala.xml.MetaData
import scala.xml.NamespaceBinding
import scala.xml.Null
import scala.xml.PrefixedAttribute
import scala.xml.UnprefixedAttribute

import org.apache.daffodil.api.DFDLPrimType
Expand Down Expand Up @@ -56,16 +54,6 @@ class ScalaXMLInfosetOutputter(showFreedInfo: Boolean = false)
resultNode = Maybe(root(0))
}

private def getScope(diElem: DIElement): NamespaceBinding = {
val minScope = diElem.metadata.minimizedScope
// if including xsi:type is enabled, ensure the xsi namespace is defined on the root element
if (getIncludeDataType() && stack.length == 1 && minScope.getURI("xsi") == null) {
NamespaceBinding("xsi", XMLUtils.XSI_NAMESPACE, minScope)
} else {
minScope
}
}

private def getAttributes(diElem: DIElement): MetaData = {
val nilAttr = if (diElem.isNilled) XMLUtils.xmlNilAttribute else Null
val freedAttr =
Expand All @@ -92,14 +80,7 @@ class ScalaXMLInfosetOutputter(showFreedInfo: Boolean = false)
} else {
nilAttr
}
val typedAttr =
if (getIncludeDataType() && diElem.isSimple) {
val primName = diElem.erd.optPrimType.get.name
new PrefixedAttribute("xsi", "type", "xs:" + primName, freedAttr)
} else {
freedAttr
}
typedAttr
freedAttr
}

override def startSimple(se: InfosetSimpleElement): Unit = {
Expand All @@ -124,7 +105,7 @@ class ScalaXMLInfosetOutputter(showFreedInfo: Boolean = false)
diSimple.metadata.prefix,
diSimple.metadata.name,
attributes,
getScope(diSimple),
diSimple.metadata.minimizedScope,
minimizeEmpty = true,
children*
)
Expand All @@ -149,7 +130,7 @@ class ScalaXMLInfosetOutputter(showFreedInfo: Boolean = false)
diComplex.metadata.prefix,
diComplex.metadata.name,
attributes,
getScope(diComplex),
diComplex.metadata.minimizedScope,
minimizeEmpty = true,
children*
)
Expand Down
Loading
Loading