You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2023/06/12 20:05:41 UTC
[GitHub] [daffodil] stevedlawrence opened a new pull request, #1032: Modify schematron validator to support relative imports
stevedlawrence opened a new pull request, #1032:
URL: https://github.com/apache/daffodil/pull/1032
The current schematron validator uses a custom ClassPathUriResolver which only looks on the classpath to resolve includes/imports. However, it is not uncommon for includes/imports to use relative paths, especially in the case of DFDL schemas, which fails to resolve because they are not on the classpath. This is particularly of issue when schematron rules are embedded in a DFDL schema that uses relative paths, but does also affect .sch files if they use relative paths.
To support relative paths, this modifies the schematron validator to use the same DFDLCatalogResolver that the rest of Daffodil uses for resolving includes and imports, for both .sch files and schematron embedded in DFDL files. In some places this requires changes to the logic so we can call setSystemId, which is information needed during resolution in the DFDLCatalogResolver.
Note that transformers use the URIResolver interface instead of the EntityResolver interface, so this modifies the DFDLCatalogResolver to implement that interface and add the needed "resolve" function.
The custom ClassPathUriResolver is no longer needed and is removed. This does lose the ability for a SchematronValidator to use a custom resolver, however that adds extra complexity for functionality that shouldn't be needed now that it uses the DFDLCatalogResolver, which supports classpath, catalogs, relative paths, etc.
DAFFODIL-2813
--
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 diff in pull request #1032: Modify schematron validator to support relative imports
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1032:
URL: https://github.com/apache/daffodil/pull/1032#discussion_r1228588555
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala:
##########
@@ -172,14 +174,25 @@ class DFDLCatalogResolver private ()
}
def resolveURI(uri: String): String = {
- init
val optURI = resolveCommon(uri, null, null)
optURI match {
case None => null
case Some(uri) => uri.toString
}
}
+ override def resolve(href: String, base: String): Source = {
+ val optURI = resolveCommon(null, href, base)
+ optURI match {
+ case None => null
Review Comment:
Done, we now have full coverage of the new resolve function.
--
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 merged pull request #1032: Modify schematron validator to support relative imports
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence merged PR #1032:
URL: https://github.com/apache/daffodil/pull/1032
--
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 diff in pull request #1032: Modify schematron validator to support relative imports
Posted by "tuxji (via GitHub)" <gi...@apache.org>.
tuxji commented on code in PR #1032:
URL: https://github.com/apache/daffodil/pull/1032#discussion_r1227248329
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala:
##########
@@ -172,14 +174,25 @@ class DFDLCatalogResolver private ()
}
def resolveURI(uri: String): String = {
- init
Review Comment:
For other reviewers: the reason why the `init` call is not needed at this line is because the next line calls `resolveCommon` which also calls `init` too.
--
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 diff in pull request #1032: Modify schematron validator to support relative imports
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1032:
URL: https://github.com/apache/daffodil/pull/1032#discussion_r1228530702
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala:
##########
@@ -172,14 +174,25 @@ class DFDLCatalogResolver private ()
}
def resolveURI(uri: String): String = {
- init
val optURI = resolveCommon(uri, null, null)
optURI match {
case None => null
case Some(uri) => uri.toString
}
}
+ override def resolve(href: String, base: String): Source = {
+ val optURI = resolveCommon(null, href, base)
+ optURI match {
+ case None => null
Review Comment:
I would then suppress the codeCov thing with their suppression comments and change the code to do Assert.invariantFailed(...) on the None return.
--
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 diff in pull request #1032: Modify schematron validator to support relative imports
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1032:
URL: https://github.com/apache/daffodil/pull/1032#discussion_r1228521480
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala:
##########
@@ -172,14 +174,25 @@ class DFDLCatalogResolver private ()
}
def resolveURI(uri: String): String = {
- init
val optURI = resolveCommon(uri, null, null)
optURI match {
case None => null
case Some(uri) => uri.toString
}
}
+ override def resolve(href: String, base: String): Source = {
+ val optURI = resolveCommon(null, href, base)
+ optURI match {
+ case None => null
Review Comment:
I've added a test to show resolution failing, but it still does not hit this `None` case. Baed on a comment and the code, it looks like `resolveCommon` only returns `None` in certain unit tests. In most other cases it just throws an exception that is handled elsewhere.
https://github.com/apache/daffodil/blob/main/daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala#L250-L254
So I think this None case is just dead code (some other resolve functions have this same dead code, but some do hit it), but is maybe good to keep it just so it is consistent the the other resolve functions or in case we every change the behavior of resolveCommon?
--
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 diff in pull request #1032: Modify schematron validator to support relative imports
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1032:
URL: https://github.com/apache/daffodil/pull/1032#discussion_r1228370009
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/xml/DaffodilXMLLoader.scala:
##########
@@ -172,14 +174,25 @@ class DFDLCatalogResolver private ()
}
def resolveURI(uri: String): String = {
- init
val optURI = resolveCommon(uri, null, null)
optURI match {
case None => null
case Some(uri) => uri.toString
}
}
+ override def resolve(href: String, base: String): Source = {
+ val optURI = resolveCommon(null, href, base)
+ optURI match {
+ case None => null
Review Comment:
We are missing a negative test where resolveCommon returns None.
It is worth it to add a test that covers 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