Allow a custom entity resolver to be registered with SPI by olabusayoT · Pull Request #1758 · apache/daffodil

olabusayoT · 2026-10-14T19:29:40Z

Add DaffodilEntityResolver, one interface covering the SAX, Xerces, DOM LS and URIResolver roles, and DaffodilEntityResolverFactory, an SPI that lets a user replace the default resolver.
Wrap the factory in PooledEntityResolver, which borrows a resolver from a ThreadSafePool for each call so custom resolvers need not be thread safe.
DaffodilXMLEntityResolver returns that resolver when one factory is registered and the per-thread DFDLCatalogResolver otherwise; registering more than one factory is an error.
SchemaSet, XercesValidator, the CLI EXI handler and the Schematron factory get the resolver from DaffodilXMLEntityResolver instead of using DFDLCatalogResolver directly.
Give DaffodilEntityResolver defaults for getExternalSubset and resolveEntity(publicId, systemId) so a custom resolver implements five methods.
Add CLI tests for the default, single factory and two factory cases.

- Add DaffodilEntityResolver, one interface covering the SAX, Xerces, DOM LS and URIResolver roles, and DaffodilEntityResolverFactory, an SPI that lets a user replace the default resolver. - Wrap the factory in PooledEntityResolver, which borrows a resolver from a ThreadSafePool for each call so custom resolvers need not be thread safe. - DaffodilXMLEntityResolver returns that resolver when one factory is registered and the per-thread DFDLCatalogResolver otherwise; registering more than one factory is an error. - SchemaSet, XercesValidator, the CLI EXI handler and the Schematron factory get the resolver from DaffodilXMLEntityResolver instead of using DFDLCatalogResolver directly. - Give DaffodilEntityResolver defaults for getExternalSubset and resolveEntity(publicId, systemId) so a custom resolver implements five methods. - Add CLI tests for the default, single factory and two factory cases. DAFFODIL-3098

stevedlawrence

A number of thoughts on possible API alternatives.

stevedlawrence · 2026-10-15T13:12:19Z

+   *
+   * @return the resolved URI, or null if it cannot be resolved
+   */
+  String resolveURI(String uri);


It would be nice if we didn't require that users implement this one Daffodil specific function. That way anything that implements the four resolvers will just work.

And it looks like daffodil only calls this resolveURI funtion in one place, in Import.scala. I wonder if we could change it to call one of the resolve functions in the above? Maybe we could switch to using the URIResolver.resolveURI(...) and pass in a null base (or maybe there's an appropriate base we could pass in)?

XMLResourceIdentifier.resolveEntity was deemed best fit, as it Impl constructor takes a namespace directly.

stevedlawrence · 2026-10-15T13:14:24Z

+   */
+  default List<String> getCatalogFiles() {
+    return Collections.emptyList();
+  }


Suggest we don't provide defaults of these. Keeps the interface as small as possible and doesn't make any assumptions about how implementations work. That way all this DaffodilEntityResolver interface does is specify that you must implement these four other interfaces that Daffodil will use.

stevedlawrence · 2026-10-15T13:27:14Z

+ * from more than one thread at a time, so the resolvers need not be thread safe. The factory
+ * itself must be thread safe.
+ */
+public interface DaffodilEntityResolverFactory extends SimpleNamedLoadableService {


Thoughts on DaffodilEntityResolverProvider as a name. Matches our UDF provier naming convention and maybe makes it more clears its uses the Server Provider Interface?

stevedlawrence · 2026-10-15T13:30:44Z

+  /**
+   * @return a new entity resolver
+   */
+  DaffodilEntityResolver make();


Suggest we call this get() or getEntityResolver() or something, so it doesn't imply that implementations need to create a new instance. If they have a thread safe implementation of these interfaces then there is no need to create a new one every time this is called and they could keep returning the same instance.

Another thought is we could have multiple functions, e.g.:

public static XMLEntityResolver getXMLEntityResolver(); public static LSResourceResolver getLSResourceResolver(); ...

And then providers just need a way to provide different kinds of resolvers and we don't need the new DaffodilEntityResolver interface at all. Daffodil internally will request the type of resolver it needs for a specific operation and that does not necessarily need be the same resolver used for other operations. This solves the problem mentioned before about some resolver implemenations not having a single class that implements all different types of resolvers like the Daffodil one does. It does mean potentially creating excessive resolvers, but I imagine resolvers are relatively cheap to create and not needed very frequently, or at least not needed in critical sections.

It also means a provider could choose not to support a specific type of resolver by throwing an exception. For example, if an XMLEntityResolver is only by the XercesValidator and the user knows they'll never use Xerces then they don't need to support that specific resolver in their provider.

stevedlawrence · 2026-10-15T13:34:00Z

+ * <p>
+ * Daffodil calls {@link #make} whenever it needs another resolver and never uses a resolver
+ * from more than one thread at a time, so the resolvers need not be thread safe. The factory
+ * itself must be thread safe.


Is this true? Say someone compiled two schemas at the same time in separate threads, if the implementation return the same instance of this and it wasn't thread safe couldn't that cause issues?

YEs, that was because it was using ThreadSafePool.withInstance, which according to the validator code ensures whatever is returned from the withInstance is threadsafe, even if the original object istself was not.

stevedlawrence · 2026-10-15T14:09:34Z

  }

-  lazy val catalogFiles = cm.getCatalogFiles().asScala.toList.asInstanceOf[List[String]]
+  private lazy val _catalogFiles = cm.getCatalogFiles


I don' think we need this new private _catalogFiles. Nothing uses it except getCatalogFiles and that could use the existing catalogFiles member.

stevedlawrence · 2026-10-15T14:14:35Z

+ * A resolver that is safe to use from any thread, built from a factory of resolvers that
+ * are not. Each call runs on a resolver that no other thread is using at that moment.
+ */
+final class PooledEntityResolver(factory: DaffodilEntityResolverFactory)


For simplicity, I would suggest we don't worry about pooling entity resolvers and just let SPI's determine how they want to provide them. For example, if an SPI already has a thread safe resolver they can just return that every time and there's no need for pooling. Or if their resolvers are relatively cheap and fast to create, they can just allocate a new one every time one is needed. In fact, I wonder if that's what Daffodil should do instead of the ThreadLocal. I doubt these are that expensive to create and they should only be used during compilation outside any critical sections.

It does mean that we require that the entity resolver SPI returns a thread safe resolver, but I don't think that's too crazy of an ask, and in most cases they can just allocate a new instance.

I'll update documentation and remove the PooledEntityResolver. Now if a provider (which we ask to be threadsafe since it can be called at the same time on different thread) returns the same resolver to every caller, that resolver must be made thread safe...that choice belongs to the provider, and a provider that doesn't want to do that can just allocate a new instance like you said.

stevedlawrence · 2026-10-15T14:34:13Z

   */
-  public static EntityResolver getEntityResolver() {
-    return DFDLCatalogResolver.get();
+  public static DaffodilEntityResolver getEntityResolver() {


I beliee this is an API change that is not backwards compatible. I think this still needs to return an org.xml.sax.EntityResolver. But we can add a new function getDaffodilEntityResolver to get an instance of the new interface that supports all the different resolvers.

No worries, I already got rid of the DaffodilEntityResolver in favor of the specific resolvers (LS, URI, XML etc)

stevedlawrence · 2026-10-15T14:47:29Z

+ * single implementation serves all of these roles.
+ */
+public interface DaffodilEntityResolver
+    extends EntityResolver2, XMLEntityResolver, LSResourceResolver, URIResolver {


Do you know if an entity resolver for use by Daffodil really needs to implement all of these, or does Daffodils implementation just have happen to implement them for convenience and really only needs a subset?

My concern is that looking a org.xmlresolver.XMLResolver (a more modern implementation of resovers), it has a number of functions to get specific kinds of resolvers (e.g. XercesXniAdaptermm/xml getXMLEntityResolver(), URIResolver getURIResolver, SAX2Adapter getEntityResolver2). Each of these functions creates an adapter that wraps implementing the interface and wrapping the underlying resolver. So while you can get an instance of any kindof of interface, there isn't necessarily a single instance that implements all interfaces like DaffodilEntityResolver requires. Note that org.xmlresolver.Resolver exists, but it's deprecated and doesn't implement XMLEntityResolver (but does implment the others).

So this potentially means anyone wanting to use this needs to create a wrapper/adapter class. That's doable, but if Daffodil doesn't actually need all of these interfaces we could remove it and at least make adapters that much easier to write.

We don't need to implement all of these, I believe we do it for convenience. With the new specific methods, we call what we need across the codebase (ex lsresolver or xmlResolver). I left in the default DFDLCatalogResolver for convenience tho, and to make the diff smaller; should we refactor it?

stevedlawrence · 2026-10-15T15:11:13Z

+   */
+  def resolver(): DaffodilEntityResolver = custom match {
+    case Some(r) => r
+    case None => DFDLCatalogResolver.get


Is it worth implementing a provider for the DFDLCatalogResolver? If the result of loadClass contains more than one value then we skip the "daffodil" one and use the first one after that? And if there's only then we use that one (and it's proably the daffodil one). If there are zero it's likely some major error an we fail.

Note only would that provide an example of how to write an SPI for this, it also ensures we don't accidentally break things for other implementations since we're using the same API.

Sure, but we would need to create a new different Sevrice Loader, since SImpleNamedServiceLoader drops multiple loaded providers without using either. there is precedent for this as in the UDF service loader

Doesn't the SimpleNamedServiceLoader.loadClass create a Map where the key is the names of the service providers and the values are instances of the service providers? We could have an object that gets that Map and figures out which of the entires to actually use, and maybe stores that entry somewhere for future look ups.

- One DaffodilXMLLoader now serves all schema files of a SchemaSet, and withErrorHandler scopes each file's load errors to that file. - Add SchemaValidationCache, owned by a Compiler and shared by the SchemaSets it makes, holding one XercesValidator and one compiled schema per validation schema plus pools of XML and LS resolvers that are borrowed for the duration of one operation. - Remove the ThreadLocal from DFDLCatalogResolver; the default provider creates a resolver per request and the pools reuse them. - XercesValidator sets its resolver once when a pooled validator is allocated instead of on every validation. - SchemaSet.withXMLResolver replaces SchemaSet.resolver for namespace import lookups. - DaffodilEntityResolverProvider documents that resolvers are pooled for the life of the compiler and reused across operations. For a large schema this reduces the loaders, validators and resolvers created per compile from hundreds to a handful. DAFFODIL-3098

…tom provider - Create resolvers once per schema compile and drop the XSD re-entrancy flag so DFDLCatalogResolver is thread safe - Pass the error handler to the loader on each call instead of scoping it - Resolve Daffodil's own schemas and namespaces with a built-in-only resolver, never the provider - Wrap a custom provider so the built-in and user catalogs are consulted before it - Create catalog resolvers per request so user catalog settings are read each time - Document the resolution order in the provider Javadoc - Add unit tests and CLI tests for EXI, Schematron, a strict provider and user catalogs DAFFODIL-3098

…r provider - Add a test provider that returns the four resolvers of an XMLResolver reading its own catalog - Show with a forked CLI parse that the provider resolves a namespace Daffodil's catalogs do not map, and that the same schema fails without it DAFFODIL-3098

…the custom provider

olabusayoT requested review from jadams-tresys and stevedlawrence October 14, 2026 19:29

stevedlawrence reviewed Oct 15, 2026

View reviewed changes

olabusayoT added 5 commits October 15, 2026 13:10

fixup! Allow a custom entity resolver to be registered with SPI

046cc55

fixup! Resolve schemas through one loader and built-in catalog, then …

5f2f79b

…the custom provider

Conversation

olabusayoT commented Oct 14, 2026

Uh oh!

stevedlawrence left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants