[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1032: Modify schematron validator to support relative imports

["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