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 2022/02/01 18:09:06 UTC
[GitHub] [daffodil] mbeckerle opened a new pull request #744: SDE if include/import schemaLocation not found.
mbeckerle opened a new pull request #744:
URL: https://github.com/apache/daffodil/pull/744
DAFFODIL-2642
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #744: SDE if include/import schemaLocation not found.
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #744:
URL: https://github.com/apache/daffodil/pull/744#discussion_r796910537
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/DaffodilXMLLoader.scala
##########
@@ -215,18 +215,37 @@ class DFDLCatalogResolver private ()
val result = (resolvedId, systemId) match {
case (null, null) => {
- // This happens now in some unit tests.
- // Assert.invariantFailed("resolvedId and systemId were null.")
- Logger.log.debug(s"Unable to resolve.")
+ // This happens in numerous unit tests.
+ //
+ // It seems that in some situations the resolver is called
+ // to attempt to resolve things certain ways. Such as
+ // providing just the namespace URI, without the systemId.
+ //
+ // So the inability to resolve, in this case anyway, is not an error.
+ //
None
}
case (null, sysId) =>
{
+ // We did not get a catalog resolution of the nsURI, nor
+ // a straight file resolution of the systemId, so we
+ // use the systemId (which comes from the schemaLocation attribute)
+ // and the classpath.
val baseURI = if (baseURIString == null) None else Some(new URI(baseURIString))
val optURI = Misc.getResourceRelativeOption(sysId, baseURI)
optURI match {
case Some(uri) => Logger.log.debug(s"Found on classpath: ${uri}.")
- case None => Logger.log.info(s"Unable to resolve ${sysId} in ${baseURI}")
+ case None => {
+ //
+ // We have to explicitly throw this, because returning with
+ // a no-resolve does not cause Xerces to report an error.
+ // Instead you just get later errors about symbols that can't
+ // be resolved, but it never mentions that an include/import didn't
+ // work.
+ lazy val e = new SAXParseException(
+ s"""DaffodilXMLLoader: Unable to resolve schemaLocation='$systemId'.""", null)
+ throw e
Review comment:
Will fix. This was a leftover from when I was using it in a log message, when the lazy val prevents construting when the logging is not being performed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #744: SDE if include/import schemaLocation not found.
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #744:
URL: https://github.com/apache/daffodil/pull/744#discussion_r797563695
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/DaffodilXMLLoader.scala
##########
@@ -215,18 +215,37 @@ class DFDLCatalogResolver private ()
val result = (resolvedId, systemId) match {
case (null, null) => {
- // This happens now in some unit tests.
- // Assert.invariantFailed("resolvedId and systemId were null.")
- Logger.log.debug(s"Unable to resolve.")
+ // This happens in numerous unit tests.
+ //
+ // It seems that in some situations the resolver is called
+ // to attempt to resolve things certain ways. Such as
+ // providing just the namespace URI, without the systemId.
+ //
+ // So the inability to resolve, in this case anyway, is not an error.
+ //
None
}
case (null, sysId) =>
{
+ // We did not get a catalog resolution of the nsURI, nor
+ // a straight file resolution of the systemId, so we
+ // use the systemId (which comes from the schemaLocation attribute)
+ // and the classpath.
val baseURI = if (baseURIString == null) None else Some(new URI(baseURIString))
val optURI = Misc.getResourceRelativeOption(sysId, baseURI)
optURI match {
case Some(uri) => Logger.log.debug(s"Found on classpath: ${uri}.")
- case None => Logger.log.info(s"Unable to resolve ${sysId} in ${baseURI}")
+ case None => {
+ //
+ // We have to explicitly throw this, because returning with
+ // a no-resolve does not cause Xerces to report an error.
+ // Instead you just get later errors about symbols that can't
+ // be resolved, but it never mentions that an include/import didn't
+ // work.
+ val e = new SAXParseException(
+ s"""DaffodilXMLLoader: Unable to resolve schemaLocation='$systemId'.""", null)
+ throw e
Review comment:
Is this a bug in Xerces where it doesn't consider faling to resolve `schemaLocation` an error? Or maybe there's some configuration we aren't setting correctly to causes this to be an errors? Seems odd that a failure to resolve this specific thing isn't considered an error when all other failures are handled by Xerces correctly.
Also, I wonder if PR #745, which switches a different xml resolver, would have any affect on this?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #744: SDE if include/import schemaLocation not found.
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #744:
URL: https://github.com/apache/daffodil/pull/744#discussion_r796868231
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/DaffodilXMLLoader.scala
##########
@@ -215,18 +215,34 @@ class DFDLCatalogResolver private ()
val result = (resolvedId, systemId) match {
case (null, null) => {
- // This happens now in some unit tests.
- // Assert.invariantFailed("resolvedId and systemId were null.")
- Logger.log.debug(s"Unable to resolve.")
+ // This happens in numerous unit tests.
+ //
+ // It seems that in some situations the resolver is called
+ // to attempt to resolve things certain ways. Such as
+ // providing just the namespace URI, without the systemId.
+ //
+ // So the inability to resolve, in this case anyway, is not an error.
+ //
None
}
case (null, sysId) =>
{
+ // We did not get a catalog resolution of the nsURI, nor
+ // a straight file resolution of the systemId, so we
+ // use the systemId (which comes from the schemaLocation attribute)
+ // and the classpath.
val baseURI = if (baseURIString == null) None else Some(new URI(baseURIString))
val optURI = Misc.getResourceRelativeOption(sysId, baseURI)
optURI match {
case Some(uri) => Logger.log.debug(s"Found on classpath: ${uri}.")
- case None => Logger.log.info(s"Unable to resolve ${sysId} in ${baseURI}")
+ case None => {
Review comment:
Add more doc of why we have to throw this. (Because Xerces won't create an error if we just return without resolving. )
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle merged pull request #744: SDE if include/import schemaLocation not found.
Posted by GitBox <gi...@apache.org>.
mbeckerle merged pull request #744:
URL: https://github.com/apache/daffodil/pull/744
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #744: SDE if include/import schemaLocation not found.
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #744:
URL: https://github.com/apache/daffodil/pull/744#discussion_r796889570
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/xml/DaffodilXMLLoader.scala
##########
@@ -215,18 +215,37 @@ class DFDLCatalogResolver private ()
val result = (resolvedId, systemId) match {
case (null, null) => {
- // This happens now in some unit tests.
- // Assert.invariantFailed("resolvedId and systemId were null.")
- Logger.log.debug(s"Unable to resolve.")
+ // This happens in numerous unit tests.
+ //
+ // It seems that in some situations the resolver is called
+ // to attempt to resolve things certain ways. Such as
+ // providing just the namespace URI, without the systemId.
+ //
+ // So the inability to resolve, in this case anyway, is not an error.
+ //
None
}
case (null, sysId) =>
{
+ // We did not get a catalog resolution of the nsURI, nor
+ // a straight file resolution of the systemId, so we
+ // use the systemId (which comes from the schemaLocation attribute)
+ // and the classpath.
val baseURI = if (baseURIString == null) None else Some(new URI(baseURIString))
val optURI = Misc.getResourceRelativeOption(sysId, baseURI)
optURI match {
case Some(uri) => Logger.log.debug(s"Found on classpath: ${uri}.")
- case None => Logger.log.info(s"Unable to resolve ${sysId} in ${baseURI}")
+ case None => {
+ //
+ // We have to explicitly throw this, because returning with
+ // a no-resolve does not cause Xerces to report an error.
+ // Instead you just get later errors about symbols that can't
+ // be resolved, but it never mentions that an include/import didn't
+ // work.
+ lazy val e = new SAXParseException(
+ s"""DaffodilXMLLoader: Unable to resolve schemaLocation='$systemId'.""", null)
+ throw e
Review comment:
I'm surprised you're creating a lazy val and throwing it immediately (therefore forcing it to be instantiated right away instead of lazily when it's needed). Are you using lazy to perform a static one-time-only initialization? I'm not sure it works that way, and it wouldn't allow different values of `systemId`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org