You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/01/22 15:03:17 UTC
[GitHub] [incubator-daffodil] olabusayoT opened a new pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
olabusayoT opened a new pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478
To make XMLReader and OutputStream content handler aware of/handle appropriately the namespaces and namespace-prefixes features. Also fixed prefixesOrNull issue from DAFFODIL-2457
- More uniform handling of Attributes object in relation to attributes and prefixMappings
- force prefixMapping to be in the same order as the input, otherwise it gets reversed, affecting the output of getPrefix
- tests for false/false, false/true, true/true, and true/false (default)
for namespaces and namespace prefixes features
- Renamed prefix to lexicalPrefix
- added prefix function in ERD to do lookup of namedQName.namespace in erd.minimizedScope
- Replaced instances of prefixOrNull with a lookup using the current namespace, as well as the namespacebinding (either MinimizedScope or ElementBase.namespaces)
- Removed prefixOrNull
- Fixed XMLDifferenceException to include a non-stripped version of expected
DAFFODIL-2422 DAFFODIL-2457
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] olabusayoT commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
olabusayoT commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562827215
##########
File path: daffodil-core/.settings/org.eclipse.core.resources.prefs
##########
@@ -0,0 +1,3 @@
+#Generated by sbteclipse
Review comment:
This was actually committed in error but I created [DAFFODIL-2462](https://issues.apache.org/jira/browse/DAFFODIL-2462) to deal with it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] olabusayoT commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
olabusayoT commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562887621
##########
File path: daffodil-core/.settings/org.eclipse.core.resources.prefs
##########
@@ -0,0 +1,3 @@
+#Generated by sbteclipse
Review comment:
Ah the ticket was to deal with the the whole eclipse-project folder. I will undo these changes, but the ticket was just to capture our desire to clean up the IDE specific files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] olabusayoT commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
olabusayoT commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562829359
##########
File path: daffodil-core/src/test/resources/test/example_no_targetnamespace.dfdl.xsd
##########
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- copied from multi_C_02.dfdl.xsd -->
Review comment:
daffodil-core does not load daffodil-test files, since daffodil-test is not a dependency for it, so the reference will not be on the classpath. Since core is a dependency for test, and since it was just 1 file it didn't make sense to go the added dependency route
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] olabusayoT commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
olabusayoT commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562827988
##########
File path: daffodil-core/src/test/resources/test/example_nested_namespaces.dfdl.xsd
##########
@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- derived from multi_A_02.dfdl.xsd -->
Review comment:
Actually we can remove this comment. This was almost identical apart from the namespaces when I first created it, now it's completely different. Will do.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562705132
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/XMLUtils.scala
##########
@@ -826,11 +826,14 @@ object XMLUtils {
Comparison failed.
Expected (attributes stripped)
%s
+Excepted (attributes ignored for diff)
+ %s
Review comment:
I'm concerned this is going to make our already verbose TDML diff ouput even larger. Maybe we should only show this full expected output if we're finding the non-stripped attributes are useful, with the assumption that some parts of it will be ignored?
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseUnparseAPI.scala
##########
@@ -71,6 +82,15 @@ object TestSAXParseUnparseAPI {
}
dp
}
+
+ private def getResource(resPath: String): File = {
+ val f = try {
+ new File(this.getClass().getResource(resPath).toURI())
+ } catch {
+ case _: Throwable => null
+ }
+ f
+ }
Review comment:
We have a Misc.getResourceOption function that does something very similar, can we use that instead?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -67,12 +73,64 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
override def startPrefixMapping(prefix: String, uri: String): Unit = {
val _prefix = if (prefix == "") null else prefix
prefixMapping = NamespaceBinding(_prefix, uri, prefixMapping)
+ currentElementPrefixMapping = NamespaceBinding(_prefix, uri, currentElementPrefixMapping)
}
override def endPrefixMapping(prefix: String): Unit = {
// do nothing
}
+ /**
+ * Uses Attributes, which is passed in to the startElement callback, to extract prefix mappings
+ * and other attributes and return a tuple of an updated prefixMapping and a Seq of attribute=val
+ * String pairings
+ */
+ def processAttributes(atts: Attributes, prefixMapping: NamespaceBinding): (NamespaceBinding, Seq[String]) = {
+ var pm = prefixMapping
+ var attrPairings: Seq[String] = Seq()
+ var i = 0
+ while (i < atts.getLength) {
+ val qName = atts.getQName(i)
+ try {
+ val pre = qName.replace("xmlns:", "")
+ pm.getURI(pre)
+ // we make this call to check if the prefix already exists. If it doesn't exist, we get the
+ // npe so we can add it, but if it does exist, nothing happens and it doesn't get readded
+ // and we instead proceed to the next item in Attributes
+ } catch {
+ // prefix does not exist, so lookup resulted in npe, therefore
+ // proceed with scraping Attributes
+ case _: NullPointerException => {
+ val attrVal = atts.getValue(i)
+ if (qName == "xmlns") {
+ pm = NamespaceBinding(null, attrVal, pm)
+ } else if (qName.startsWith("xmlns:")) {
+ val prefix = qName.substring(6)
+ pm = NamespaceBinding(prefix, attrVal, pm)
+ } else {
+ // if not a namespace mapping, it is an attribute we're interested in
+ if (qName.nonEmpty) {
+ attrPairings :+= s" $qName=" + "\"" + s"${attrVal}" + "\""
Review comment:
We shoudl avoid this seq append, perhaps do a prepend and then reverse at the end if order matters.
Also, this might be cleaner using tripple quotes, e.g.
```scala
s""" ${qName}="${attrVal}""""
```
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -67,12 +73,64 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
override def startPrefixMapping(prefix: String, uri: String): Unit = {
val _prefix = if (prefix == "") null else prefix
prefixMapping = NamespaceBinding(_prefix, uri, prefixMapping)
+ currentElementPrefixMapping = NamespaceBinding(_prefix, uri, currentElementPrefixMapping)
}
override def endPrefixMapping(prefix: String): Unit = {
// do nothing
}
+ /**
+ * Uses Attributes, which is passed in to the startElement callback, to extract prefix mappings
+ * and other attributes and return a tuple of an updated prefixMapping and a Seq of attribute=val
+ * String pairings
+ */
+ def processAttributes(atts: Attributes, prefixMapping: NamespaceBinding): (NamespaceBinding, Seq[String]) = {
+ var pm = prefixMapping
+ var attrPairings: Seq[String] = Seq()
+ var i = 0
+ while (i < atts.getLength) {
+ val qName = atts.getQName(i)
+ try {
+ val pre = qName.replace("xmlns:", "")
+ pm.getURI(pre)
+ // we make this call to check if the prefix already exists. If it doesn't exist, we get the
+ // npe so we can add it, but if it does exist, nothing happens and it doesn't get readded
+ // and we instead proceed to the next item in Attributes
+ } catch {
+ // prefix does not exist, so lookup resulted in npe, therefore
+ // proceed with scraping Attributes
+ case _: NullPointerException => {
+ val attrVal = atts.getValue(i)
+ if (qName == "xmlns") {
+ pm = NamespaceBinding(null, attrVal, pm)
+ } else if (qName.startsWith("xmlns:")) {
+ val prefix = qName.substring(6)
+ pm = NamespaceBinding(prefix, attrVal, pm)
+ } else {
+ // if not a namespace mapping, it is an attribute we're interested in
+ if (qName.nonEmpty) {
+ attrPairings :+= s" $qName=" + "\"" + s"${attrVal}" + "\""
+ } else {
+ val uri = atts.getURI(i)
+ val ln = atts.getLocalName(i)
+ try {
+ val pre = pm.getPrefix(uri)
+ if (pre.nonEmpty) {
+ attrPairings :+= s" $pre:$ln=" + "\"" + s"${attrVal}" + "\""
Review comment:
Same comment as above.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -107,6 +183,36 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
outputNewlineStack.push(false)
}
+ private def outputTagName(uri: String, localName: String, qName: String, atts: Maybe[Attributes] = Nope): Unit = {
+ val tagName = {
+ if (qName.nonEmpty) {
+ qName
+ } else {
+ // use attributes
+ lazy val attrs = atts.get
+ lazy val in = attrs.getIndex(uri, localName)
+ if (atts.nonEmpty && attrs.getLength != 0 && in >= 0) {
+ val qn = attrs.getQName(in)
+ qn
+ } else {
+ // use prefixMapping
+ val sanitizedUri = if (uri.isEmpty) null else uri
+ try {
+ val pre = prefixMapping.getPrefix(sanitizedUri)
+ if (pre.nonEmpty) {
+ s"$pre:$localName"
+ } else {
+ localName
+ }
+ } catch {
+ case _: NullPointerException => localName
Review comment:
Again, we should try to avoid this NPE.
##########
File path: daffodil-core/.settings/org.scala-ide.sdt.core.prefs
##########
@@ -0,0 +1,7 @@
+#Generated by sbteclipse
+#Fri Nov 01 00:37:11 EDT 2019
+scala.compiler.additionalParams=-feature -language\:experimental.macros -Xxml\:-coalescing
+deprecation=true
+Xfatal-warnings=true
+unchecked=true
+scala.compiler.useProjectSettings=true
Review comment:
Since these .settings files are eclipse specific, they shouldn't be added. Might want to add .settings to a global gitignore file on yoru system.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -175,26 +180,99 @@ class SAXInfosetOutputter(xmlReader: DFDL.DaffodilParseXMLReader)
}
}
- private def doStartElement(diElem: DIElement, contentHandler: ContentHandler): Unit = {
- val (ns: String, elemName: String, qName: String) = getNameSpaceElemNameAndQName(diElem)
- doStartPrefixMapping(diElem, contentHandler)
+ private def doAttributesPrefixMapping(diElem: DIElement, attrs: AttributesImpl): AttributesImpl = {
+ val (nsbStart: NamespaceBinding, nsbEnd: NamespaceBinding) = getNsbStartAndEnd(diElem)
+ var n = nsbStart
+ while (n != nsbEnd && n != null && n != scala.xml.TopScope) {
+ val prefix = if (n.prefix == null) "xmlns" else s"xmlns:${n.prefix}"
+ val uri = if (n.uri == null) "" else n.uri
+ attrs.addAttribute("", "", prefix, "CDATA", uri)
+ n = n.parent
+ }
+ attrs
+ }
- val attrs = if (isNilled(diElem)) {
- createNilAttribute()
+ private def getNsbStartAndEnd(diElem: DIElement) = {
+ val nsbStart = diElem.erd.minimizedScope
+ val nsbEnd = if (diElem.isRoot) {
+ scala.xml.TopScope
} else {
- new AttributesImpl()
+ diElem.diParent.erd.minimizedScope
+ }
+ (nsbStart, nsbEnd)
+ }
+
+ private def doStartElement(diElem: DIElement, contentHandler: ContentHandler): Unit = {
+ val (ns: String, localName: String, qName: String) = getNamespaceLocalNameAndQName(diElem)
+ val attrs = new AttributesImpl()
+ var elemUri: String = ""
+ var elemLocalName: String = ""
+ var elemQname: String = ""
+
+ if (namespacesFeature) {
Review comment:
I think a more scala-y way to do this and avoid the var's is something like:
```scala
val (elemUri, elemLocalName) =
if (namespacesFeature) {
...
(ns, localName)
} else {
("", "")
}
```
Same thing for elemQName below.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -144,29 +153,25 @@ class SAXInfosetOutputter(xmlReader: DFDL.DaffodilParseXMLReader)
}
private def doStartPrefixMapping(diElem: DIElement, contentHandler: ContentHandler): Unit = {
- val nsbStart = diElem.erd.minimizedScope
- val nsbEnd = if (diElem.isRoot) {
- scala.xml.TopScope
- } else {
- diElem.diParent.erd.minimizedScope
- }
+ val (nsbStart: NamespaceBinding, nsbEnd: NamespaceBinding) = getNsbStartAndEnd(diElem)
var n = nsbStart
+ var mappingsList: Seq[(String, String)] = Seq()
while (n != nsbEnd && n != null && n != scala.xml.TopScope) {
val prefix = if (n.prefix == null) "" else n.prefix
val uri = if (n.uri == null) "" else n.uri
- contentHandler.startPrefixMapping(prefix, uri)
+ mappingsList :+= (prefix, uri)
Review comment:
This appends to the list, which requires a copy and is going to be quite slow. Instead, I think we can prepend to the list (which doesn't require a copy), and then the list is already in the reversed order, so we don't have to reverse it below.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -33,6 +36,9 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
extends ContentHandler with Indentable with XMLInfosetOutputter {
private val writer = new OutputStreamWriter(out)
private var prefixMapping: NamespaceBinding = null
+ private var currentElementPrefixMapping: NamespaceBinding = null
+ private var currentElementAttributes: Seq[String] = Seq()
Review comment:
I think some documentation could be useful here to describe the different between prefixMapping, currentElemenetPrefixMapping, prefixMappingCOntextStack, etc.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -67,12 +73,64 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
override def startPrefixMapping(prefix: String, uri: String): Unit = {
val _prefix = if (prefix == "") null else prefix
prefixMapping = NamespaceBinding(_prefix, uri, prefixMapping)
+ currentElementPrefixMapping = NamespaceBinding(_prefix, uri, currentElementPrefixMapping)
}
override def endPrefixMapping(prefix: String): Unit = {
// do nothing
}
+ /**
+ * Uses Attributes, which is passed in to the startElement callback, to extract prefix mappings
+ * and other attributes and return a tuple of an updated prefixMapping and a Seq of attribute=val
+ * String pairings
+ */
+ def processAttributes(atts: Attributes, prefixMapping: NamespaceBinding): (NamespaceBinding, Seq[String]) = {
+ var pm = prefixMapping
+ var attrPairings: Seq[String] = Seq()
+ var i = 0
+ while (i < atts.getLength) {
+ val qName = atts.getQName(i)
+ try {
+ val pre = qName.replace("xmlns:", "")
+ pm.getURI(pre)
+ // we make this call to check if the prefix already exists. If it doesn't exist, we get the
+ // npe so we can add it, but if it does exist, nothing happens and it doesn't get readded
+ // and we instead proceed to the next item in Attributes
+ } catch {
Review comment:
Is there no way to tell if it exists without NPE? Exceptions are fairly heavyweight, would be much quicker if there's an alterantive. Might be worth implementing our own getUriOption to avoid this.
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseAPI.scala
##########
@@ -174,4 +190,46 @@ class TestSAXParseAPI {
assertTrue(pr.isError)
assertTrue(spe.getMessage.contains("Insufficient bits in data"))
}
+
+ @Test def testDaffodilParseXMLReader_parse_features_prefixes_only(): Unit = {
+ val xmlReader = dpNamespaces.newXMLReaderInstance
+ val baos = new ByteArrayOutputStream()
+ val parseOutputStreamContentHandler = new DaffodilParseOutputStreamContentHandler(baos)
+ xmlReader.setContentHandler(parseOutputStreamContentHandler)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACES_FEATURE, false)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE, true)
+ val inArray = testNamespacesData.getBytes()
+ xmlReader.parse(inArray)
+ val pr = xmlReader.getProperty(XMLUtils.DAFFODIL_SAX_URN_PARSERESULT).asInstanceOf[ParseResult]
+ assertTrue(!pr.isError)
+ assertEquals(testNamespacesInfoset, scala.xml.XML.loadString(baos.toString))
+ }
+
+ @Test def testDaffodilParseXMLReader_parse_features_only(): Unit = {
+ val xmlReader = dpNamespaces.newXMLReaderInstance
+ val baos = new ByteArrayOutputStream()
+ val parseOutputStreamContentHandler = new DaffodilParseOutputStreamContentHandler(baos)
+ xmlReader.setContentHandler(parseOutputStreamContentHandler)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACES_FEATURE, true)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE, false)
+ val inArray = testNamespacesData.getBytes()
+ xmlReader.parse(inArray)
+ val pr = xmlReader.getProperty(XMLUtils.DAFFODIL_SAX_URN_PARSERESULT).asInstanceOf[ParseResult]
+ assertTrue(!pr.isError)
+ assertEquals(testNamespacesInfoset, scala.xml.XML.loadString(baos.toString))
+ }
+
+ @Test def testDaffodilParseXMLReader_parse_features_and_prefixes(): Unit = {
+ val xmlReader = dpNamespaces.newXMLReaderInstance
+ val baos = new ByteArrayOutputStream()
+ val parseOutputStreamContentHandler = new DaffodilParseOutputStreamContentHandler(baos)
+ xmlReader.setContentHandler(parseOutputStreamContentHandler)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACES_FEATURE, true)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE, true)
+ val inArray = testNamespacesData.getBytes()
+ xmlReader.parse(inArray)
+ val pr = xmlReader.getProperty(XMLUtils.DAFFODIL_SAX_URN_PARSERESULT).asInstanceOf[ParseResult]
+ assertTrue(!pr.isError)
+ assertEquals(testNamespacesInfoset, scala.xml.XML.loadString(baos.toString))
+ }
Review comment:
One thing about these tests is that they don't really test that the our XMLReader is creating the the correct sax events with the correct parameters, and that they different based on the featuers. All these test really verify is that different features give the same result. For example, we could be completely ignoring the features, and that would give the same result and the tests would still pass.
Ideally, we would have something like a special Test ContentHandler that would have a list of expected SAX events/parameters and ensure we got the right ones, and that they are different with different features set. Though, I'm not sure if the effort is worth that. We know that the new features are implemented, and they all result in the same infoset, so we can probably assume that the right SAX events are created and handled correctly.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -107,6 +183,36 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
outputNewlineStack.push(false)
}
+ private def outputTagName(uri: String, localName: String, qName: String, atts: Maybe[Attributes] = Nope): Unit = {
+ val tagName = {
+ if (qName.nonEmpty) {
+ qName
+ } else {
+ // use attributes
+ lazy val attrs = atts.get
+ lazy val in = attrs.getIndex(uri, localName)
+ if (atts.nonEmpty && attrs.getLength != 0 && in >= 0) {
Review comment:
Wy are the ``atts.nonEmpty`` && ``attrs.getLength`` checks needed? If ``in >= 0`` that means we found the index and getQName should work, right?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -27,6 +30,12 @@ import org.xml.sax.helpers.AttributesImpl
class SAXInfosetOutputter(xmlReader: DFDL.DaffodilParseXMLReader)
extends InfosetOutputter
with XMLInfosetOutputter {
+
+ lazy val namespacesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACES_FEATURE)
+ lazy val namespacePrefixesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE)
+ Assert.invariant(namespacesFeature || namespacePrefixesFeature,
+ s"Features ${XMLUtils.SAX_NAMESPACES_FEATURE} and ${XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE} cannot both be false")
Review comment:
I'm not sure an Assert is correct here. Asserts are usually for things that should be impossible, but it's very posible that a user will do the wrong thing and set these features so that they conflict. This should probably just be some sort of SAX exception.
Another question is if this is the right place for this? This makes it so the features are validited when this SAXInfosetOutputter is created, but can't the user change those features after it's created? At want point to we consider these features frozen and cannot be changed?
Looking at Xerces for guidance: https://xerces.apache.org/xerces2-j/features.html. This says about these two features:
> Access: (parsing) read-only; (not parsing) read-write;
This seems reasonable and probably something we should implement? So we maybe should only allow these to be changed during parsing, and should validate them everytime parse() is called. And this SAX InfosetOutputter needs to consider that these feautures may change and must somehow get the latest values when parse is called. So maybe it just always calls xmlReader.getFeature when it needs a feature, or maybe there's some var's that are changed when startDocument is called?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -67,12 +73,64 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
override def startPrefixMapping(prefix: String, uri: String): Unit = {
val _prefix = if (prefix == "") null else prefix
prefixMapping = NamespaceBinding(_prefix, uri, prefixMapping)
+ currentElementPrefixMapping = NamespaceBinding(_prefix, uri, currentElementPrefixMapping)
}
override def endPrefixMapping(prefix: String): Unit = {
// do nothing
}
+ /**
+ * Uses Attributes, which is passed in to the startElement callback, to extract prefix mappings
+ * and other attributes and return a tuple of an updated prefixMapping and a Seq of attribute=val
+ * String pairings
+ */
+ def processAttributes(atts: Attributes, prefixMapping: NamespaceBinding): (NamespaceBinding, Seq[String]) = {
+ var pm = prefixMapping
+ var attrPairings: Seq[String] = Seq()
+ var i = 0
+ while (i < atts.getLength) {
+ val qName = atts.getQName(i)
+ try {
+ val pre = qName.replace("xmlns:", "")
Review comment:
Can we do atts.getLocalName if we're just going to strip off the xmlns prefix? Also, what if this attribute isn't a xmlns attribute and is something like xsi:nil? Won't we try to find "xsi:nil" as if it were a prefix?
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseXMLReader.scala
##########
@@ -43,35 +41,34 @@ class DaffodilParseXMLReader(dp: DataProcessor) extends DFDL.DaffodilParseXMLRea
private var errorHandler: ErrorHandler = _
private var dtdHandler: DTDHandler = _
private var entityResolver: EntityResolver = _
- var saxParseResultPropertyValue: ParseResult = _
- var saxBlobDirectoryPropertyValue: Path = Paths.get(System.getProperty("java.io.tmpdir"))
- var saxBlobPrefixPropertyValue: String = "daffodil-sax-"
- var saxBlobSuffixPropertyValue: String = ".blob"
-
- private val featureMap = mutable.Map[String, Boolean](
- XMLUtils.SAX_NAMESPACES_FEATURE -> false,
- XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE -> false
- )
+ private var saxParseResultPropertyValue: ParseResult = _
+ private var saxBlobDirectoryPropertyValue: Path = Paths.get(System.getProperty("java.io.tmpdir"))
+ private var saxBlobPrefixPropertyValue: String = "daffodil-sax-"
+ private var saxBlobSuffixPropertyValue: String = ".blob"
+ private var saxNamespaceFeatureValue: Boolean = true
+ private var saxNamespacePrefixesFeatureValue: Boolean = false
override def getFeature(name: String): Boolean = {
- if (name == XMLUtils.SAX_NAMESPACES_FEATURE ||
- name == XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE) {
- featureMap(name)
- } else {
- throw new SAXNotRecognizedException("Feature unsupported: " + name + ".\n" +
- "Supported features are: " + featureMap.keys.mkString(", ")
- )
+ name match {
+ case XMLUtils.SAX_NAMESPACES_FEATURE => saxNamespaceFeatureValue
+ case XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE => saxNamespacePrefixesFeatureValue
+ case _ =>
+ throw new SAXNotRecognizedException("Feature not found: " + name + ".\n" +
+ "Supported features are: " +
+ Seq(XMLUtils.SAX_NAMESPACES_FEATURE,
+ XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE).mkString(", "))
}
}
override def setFeature(name: String, value: Boolean): Unit = {
- if (name == XMLUtils.SAX_NAMESPACES_FEATURE ||
- name == XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE) {
- featureMap(name) = value
- } else {
- throw new SAXNotRecognizedException("Feature unsupported: " + name + ".\n" +
- "Supported features are: " + featureMap.keys.mkString(", ")
- )
+ name match {
+ case XMLUtils.SAX_NAMESPACES_FEATURE => saxNamespaceFeatureValue = value
+ case XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE => saxNamespacePrefixesFeatureValue = value
+ case _ =>
+ throw new SAXNotRecognizedException("Feature unsupported: " + name + ".\n" +
+ "Supported features are: " +
+ Seq(XMLUtils.SAX_NAMESPACES_FEATURE,
+ XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE).mkString(", "))
Review comment:
Suggest we have a function that throws this exception so we don't have to duplicate this logic.
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DaffodilParseOutputStreamContentHandler.scala
##########
@@ -67,12 +73,64 @@ class DaffodilParseOutputStreamContentHandler(out: OutputStream, pretty: Boolean
override def startPrefixMapping(prefix: String, uri: String): Unit = {
val _prefix = if (prefix == "") null else prefix
prefixMapping = NamespaceBinding(_prefix, uri, prefixMapping)
+ currentElementPrefixMapping = NamespaceBinding(_prefix, uri, currentElementPrefixMapping)
}
override def endPrefixMapping(prefix: String): Unit = {
// do nothing
}
+ /**
+ * Uses Attributes, which is passed in to the startElement callback, to extract prefix mappings
+ * and other attributes and return a tuple of an updated prefixMapping and a Seq of attribute=val
+ * String pairings
+ */
+ def processAttributes(atts: Attributes, prefixMapping: NamespaceBinding): (NamespaceBinding, Seq[String]) = {
+ var pm = prefixMapping
+ var attrPairings: Seq[String] = Seq()
+ var i = 0
+ while (i < atts.getLength) {
+ val qName = atts.getQName(i)
+ try {
+ val pre = qName.replace("xmlns:", "")
+ pm.getURI(pre)
+ // we make this call to check if the prefix already exists. If it doesn't exist, we get the
+ // npe so we can add it, but if it does exist, nothing happens and it doesn't get readded
+ // and we instead proceed to the next item in Attributes
+ } catch {
+ // prefix does not exist, so lookup resulted in npe, therefore
+ // proceed with scraping Attributes
+ case _: NullPointerException => {
+ val attrVal = atts.getValue(i)
+ if (qName == "xmlns") {
+ pm = NamespaceBinding(null, attrVal, pm)
+ } else if (qName.startsWith("xmlns:")) {
+ val prefix = qName.substring(6)
+ pm = NamespaceBinding(prefix, attrVal, pm)
+ } else {
+ // if not a namespace mapping, it is an attribute we're interested in
+ if (qName.nonEmpty) {
+ attrPairings :+= s" $qName=" + "\"" + s"${attrVal}" + "\""
+ } else {
+ val uri = atts.getURI(i)
+ val ln = atts.getLocalName(i)
+ try {
+ val pre = pm.getPrefix(uri)
+ if (pre.nonEmpty) {
+ attrPairings :+= s" $pre:$ln=" + "\"" + s"${attrVal}" + "\""
+ }
+ } catch {
+ case _: NullPointerException => // couldn't find prefix for attribute; do nothing
Review comment:
Again, we should really avoid NPE's here. That's usuallya sign of a bug.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562763128
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -27,6 +30,12 @@ import org.xml.sax.helpers.AttributesImpl
class SAXInfosetOutputter(xmlReader: DFDL.DaffodilParseXMLReader)
extends InfosetOutputter
with XMLInfosetOutputter {
+
+ lazy val namespacesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACES_FEATURE)
+ lazy val namespacePrefixesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE)
+ Assert.invariant(namespacesFeature || namespacePrefixesFeature,
+ s"Features ${XMLUtils.SAX_NAMESPACES_FEATURE} and ${XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE} cannot both be false")
Review comment:
The XMLReader API says parse() can throw a generic SAXException or IOException. So assuming we check this during parse when these features become read-only, I would guess a SAXException of some kind is what most users might expect to get. It also means things using this via SAX might not have to change anything to catch or Assert exception.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562697796
##########
File path: daffodil-core/src/test/resources/test/example_nested_namespaces.dfdl.xsd
##########
@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- derived from multi_A_02.dfdl.xsd -->
Review comment:
Can you explain what's different in this derivation?
Also, the attributes of this xs:schema element below are important, so putting them 1 per line will make it much easier to spot the xmlns bindings which are the important part I believe.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/QNameBase.scala
##########
@@ -224,8 +224,7 @@ trait QNameBase extends Serializable {
* must show up in diagnostic messages. Mistakes by having the wrong prefix
* or by omitting one, are very common.
*/
- def prefix: Option[String]
- def prefixOrNull: String = prefix.orNull
+ def lexicalPrefix: Option[String]
Review comment:
Add scaladoc of what you mean by lexicalPrefix.
The term lexical is often used to mean "due to surrounding textual context", but I think your intent here is to be clear when we're dealing with the prefix part of a QName, from a prefix that is somehow implied by things elsewhere, like lexically enclosing xmlns default namespace declarations when there is also a prefix for that same namespace, or elementFormDefault on a schema so an element's name will have a namespace and that could have a lexically surrounding xmlns binding, etc.
Either of these concepts can be considered "lexical", so ... scaladoc.
##########
File path: daffodil-core/.settings/org.eclipse.core.resources.prefs
##########
@@ -0,0 +1,3 @@
+#Generated by sbteclipse
Review comment:
Not necessarily in this commit, but we're striving to get IDE-specific files out of the tree.
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseAPI.scala
##########
@@ -174,4 +190,46 @@ class TestSAXParseAPI {
assertTrue(pr.isError)
assertTrue(spe.getMessage.contains("Insufficient bits in data"))
}
+
+ @Test def testDaffodilParseXMLReader_parse_features_prefixes_only(): Unit = {
+ val xmlReader = dpNamespaces.newXMLReaderInstance
+ val baos = new ByteArrayOutputStream()
+ val parseOutputStreamContentHandler = new DaffodilParseOutputStreamContentHandler(baos)
+ xmlReader.setContentHandler(parseOutputStreamContentHandler)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACES_FEATURE, false)
+ xmlReader.setFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE, true)
+ val inArray = testNamespacesData.getBytes()
+ xmlReader.parse(inArray)
+ val pr = xmlReader.getProperty(XMLUtils.DAFFODIL_SAX_URN_PARSERESULT).asInstanceOf[ParseResult]
+ assertTrue(!pr.isError)
+ assertEquals(testNamespacesInfoset, scala.xml.XML.loadString(baos.toString))
Review comment:
Coding for debug guidelines say you should have
```
val actualInfoset = scala.xml.XML.loadString(baos.toString)
```
If these tests fail that's first thing I want to look at. Also makes this a little bit more obvious what's going on.
Also lines 196 to 204 are the same I think in all 4 tests, save the two booleans on lines 199, 200, so factoring that out would make the fact that these booleans are really the only difference in the way the tests work clear. I think you want
```
val actualInfoset = saxParseWithFeatures(false, true)
```
where that method captures lines 195 to 204.
##########
File path: daffodil-core/src/test/resources/test/example_no_targetnamespace.dfdl.xsd
##########
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- copied from multi_C_02.dfdl.xsd -->
Review comment:
If this is a copy, why not just use the original one by reference?
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseAPI.scala
##########
@@ -174,4 +190,46 @@ class TestSAXParseAPI {
assertTrue(pr.isError)
assertTrue(spe.getMessage.contains("Insufficient bits in data"))
}
+
+ @Test def testDaffodilParseXMLReader_parse_features_prefixes_only(): Unit = {
Review comment:
Despite the long names, I still don't grok really what is being tested.
Or are there really just 4 combinations of the two prefix/namespace "features" 3 of which are legal, and you just want to test all combinations?
Some scaladoc on these tests to explain their specifics would be helpful.
##########
File path: daffodil-core/src/test/resources/test/example_a02_targetnamespace.dfdl.xsd
##########
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<!-- derived from multi_C_02.dfdl.xsd -->
+<xs:schema targetNamespace="http://a02.com" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/">
+
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+
+ <xs:annotation>
+ <xs:appinfo source="http://www.ogf.org/dfdl/">
+ <dfdl:format ref="GeneralFormat" separator="" initiator="" separatorPosition="infix" ignoreCase="no" separatorSuppressionPolicy="anyEmpty" terminator="" occursCountKind="parsed" initiatedContent="no" representation="text" textNumberRep="standard" encoding="ASCII" textTrimKind="none" leadingSkip='0'/>
Review comment:
line too long
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseUnparseAPI.scala
##########
@@ -49,11 +50,21 @@ object TestSAXParseUnparseAPI {
</xs:sequence>
</xs:complexType>
)
- val testInfoset: Elem = <list xmlns="http://example.com"><w>9</w><w>1</w><w>0</w></list>
- val testInfosetString: String = testInfoset.toString()
- val testData = "910"
+ lazy val testInfoset: Elem = <list xmlns="http://example.com"><w>9</w><w>1</w><w>0</w></list>
+ lazy val testInfosetString: String = testInfoset.toString()
+ lazy val testData: String = "910"
+
+ lazy val schemaFile: File = getResource("/test/example_nested_namespaces.dfdl.xsd")
+ lazy val testNamespacesSchema: Elem = scala.xml.XML.loadFile(schemaFile)
+
+ lazy val testNamespacesInfoset: Elem =
+ <b02:seq xmlns:b02="http://b02.com" xmlns:a02="http://a02.com"><b02:seq2><a02:inty>3</a02:inty></b02:seq2><b02:seq2><b02:inty>4</b02:inty></b02:seq2><b02:seq2><a02:inty>2</a02:inty></b02:seq2><b02:seq2><b02:inty>1</b02:inty></b02:seq2><b02:seq2><b02:inty>44</b02:inty></b02:seq2><b02:seq2><b02:inty>643</b02:inty></b02:seq2><b02:seq2><a02:inty>3</a02:inty></b02:seq2><b02:seq2><a02:inty>5</a02:inty></b02:seq2><b02:seq2><b02:inty>1</b02:inty></b02:seq2></b02:seq>
Review comment:
line too long. Needs indentation.
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseUnparseAPI.scala
##########
@@ -71,6 +82,15 @@ object TestSAXParseUnparseAPI {
}
dp
}
+
+ private def getResource(resPath: String): File = {
+ val f = try {
+ new File(this.getClass().getResource(resPath).toURI())
+ } catch {
+ case _: Throwable => null
Review comment:
Do you really depend on this catch? Are there places where your testing needs a null back from this?
If not I suggest just removing the try/catch here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562888119
##########
File path: daffodil-core/.settings/org.eclipse.core.resources.prefs
##########
@@ -0,0 +1,3 @@
+#Generated by sbteclipse
Review comment:
I don't think we have an eclipse project folder anymore, maybe you just have old files sticking around?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562843955
##########
File path: daffodil-core/.settings/org.eclipse.core.resources.prefs
##########
@@ -0,0 +1,3 @@
+#Generated by sbteclipse
Review comment:
We should just fix this before it's ever merged. No need to tickets for uncomitted changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] olabusayoT commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
olabusayoT commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562831961
##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/processor/TestSAXParseAPI.scala
##########
@@ -174,4 +190,46 @@ class TestSAXParseAPI {
assertTrue(pr.isError)
assertTrue(spe.getMessage.contains("Insufficient bits in data"))
}
+
+ @Test def testDaffodilParseXMLReader_parse_features_prefixes_only(): Unit = {
Review comment:
Will add scaladoc, but yes. We just wanted to test all the combos of the below
| namespace | namespace-prefixes | legality | notes |
|-----------|--------------------|----------|---------|
| false | false | illegal | |
| true | false | legal | default |
| true | true | legal | |
| false | true | legal | |
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #478: Feature Aware DaffodilParseXMLReader and Prefixes Fixes
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #478:
URL: https://github.com/apache/incubator-daffodil/pull/478#discussion_r562759852
##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -27,6 +30,12 @@ import org.xml.sax.helpers.AttributesImpl
class SAXInfosetOutputter(xmlReader: DFDL.DaffodilParseXMLReader)
extends InfosetOutputter
with XMLInfosetOutputter {
+
+ lazy val namespacesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACES_FEATURE)
+ lazy val namespacePrefixesFeature: Boolean = xmlReader.getFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE)
+ Assert.invariant(namespacesFeature || namespacePrefixesFeature,
+ s"Features ${XMLUtils.SAX_NAMESPACES_FEATURE} and ${XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE} cannot both be false")
Review comment:
This is API, so Assert.usage(test, message) lays blame for the problem on the API caller, which seems more reasonable unless there is a more appropriate SAX-specific exception related to these feature settings being inconsistent.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org