Change TDMLRunner to use XMLTextInfosetInputter/Outputter as default #1650
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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*) | ||
|
|
@@ -644,34 +653,66 @@ object XMLUtils { | |
|
|
||
| private def removeMixedWhitespace(ns: Node): Node = { | ||
| if (!ns.isInstanceOf[Elem]) return ns | ||
|
|
||
| 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 | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -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 | ||
|
|
@@ -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) | ||
| ) | ||
| ) | ||
|
Member
There was a problem hiding this comment. 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?
Contributor
Author
There was a problem hiding this comment. 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)) | ||
|
|
@@ -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, | ||
|
Member
There was a problem hiding this comment. It might be worth adding something to zpath to make it clear this is a comment that is a child of whatever zpath is. Maybe somethin like |
||
| cA.toString, | ||
| cB.toString, | ||
| maybeType, | ||
| maybeFloatEpsilon, | ||
| maybeDoubleEpsilon | ||
|
Member
There was a problem hiding this comment. Should we pass in None for type, and epsilons for comments and PCData? I'm not even really sure how maybeType coul dbe defined here. Maybe if stringAsXml contained something like this: <stringAsXml xmlns="">
<foo xsi:type="xs:int">
<!-- inline comment -->
5
</foo>
</stringAsXml>I think in that case we might e usuing xs:int for type aware comparisons? And we'll try to compare the comment as if it were an int, which might break? I'm not postive, but I think we can avoid this all if we just pass in None for these to disable type awareness.
Contributor
Author
There was a problem hiding this comment. So I tested this and inline comment ends up having its own type who is None. foo is a separate iteration of computeDiff and is what actually carries the type. But for extra precaution, will do! |
||
| ) | ||
| 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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on changing these and the below generic Exception throws to asserts? Does having it be an exception allow it to be caught or converted into a ParseError later on? Not sure how these sort of failure cases are supposed to be handled with the stringAsXML feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this wants to be an assert since the input could be an expected infoset in a TDML file. So if user provides invalid XML in the infoset, instead of an assert we probably want this to be some kind of exception.
Maybe we want a custom exception (maybe something like
InvalidInfosetException?), then users and the TDML runner can handle this however they want. The TDMLRunner an just convert it to a TDMLException like it does most things.