[feature] Add ModuleFactory and IndexFactory SPI for auto-discovery of bundled modules and indexes by duncdrum · Pull Request #6551 · eXist-db/exist

duncdrum · 2026-07-06T18:38:57Z

Summary

ServiceLoader-based SPI for XQuery modules and index modules, so bundled implementations are auto-discovered without explicit conf.xml entries. New installs get a leaner default config; existing hand-maintained configs are unaffected — explicit entries always win over SPI, and enabled="no" (from #6550) suppresses an SPI entry without removing the JAR.

Closes #3062 — delivers both parts: the @enabled attribute (#6550) and the SPI/CDI-style autodiscovery this PR adds.

What changed

ModuleFactory SPI — new org.exist.xquery.ModuleFactory interface; Configuration.configureModules() scans ServiceLoader<ModuleFactory> before the conf.xml loop. All 27 bundled XQuery modules register via META-INF/services/.

IndexFactory SPI — new org.exist.indexing.IndexFactory interface; IndexManager registers SPI entries whose id has no live conf.xml entry. All bundled indexes (Lucene, ngram, range, sort, spatial) wired; index name defaults to the SPI id when there's no conf.xml element to supply one; guarded against a null/blank id from the factory.

Vector model registry — integrated with Configuration; @enabled on <vector-models> children; repeated WARN for unavailable models downgraded to DEBUG after the first occurrence.

exist-distribution/src/main/config/conf.xml — <builtin-modules> and index <modules> sections trimmed; the 27 bundled modules and 5 bundled indexes no longer listed individually.

Compatibility

Explicit conf.xml entries always take precedence over SPI discovery for the same namespace URI/index id — no upgrade action required. enabled="no" suppresses a bundled module/index without removing its JAR.

Test plan

mvn validate -pl exist-distribution — trimmed canonical conf.xml still validates
mvn test -pl exist-core — full unit suite passes
Manual: start eXist-db with the trimmed conf.xml; all 27 built-in modules available in eXide
Manual: start eXist-db; Lucene/ngram/range indexes active with no explicit conf.xml entries
Manual: enabled="no" on a module's conf.xml entry → not loaded despite SPI discovery

🤖 Generated with Claude Code

line-o

Resolving modules to load by default over SPI received pushback on the community call on 2026-07-06
Security concerns need to be addressed before we can consider pulling this in.

@dizzzz specific concern was: "I see autoloading modules that are not mentioned in the configuration as a risk."

My concern is: if bundled modules are left out of the configuration I have to know they are there in order to disable them again.

dizzzz · 2026-07-12T15:56:27Z

That said, I had similar ideas re module loading a long time ago. "plug the modules you only need".
Effectively nothing changes. If one adds a new module, it will be loaded, but the author can decide differently. In that respect I am happy with this change.

My original ideas can from a slightly different angle: will this make loading java modules from a XAR file more simple?

dizzzz · 2026-07-12T16:04:15Z

    private static final Logger LOG = LogManager.getLogger(Collection.class);
    private static final int SHALLOW_SIZE = 550;
    private static final int DOCUMENT_SIZE = 450;
+    private static final String XML_SCHEMA_NS = "http://www.w3.org/2001/XMLSchema";


I'd prefer to use and extend exist-core/src/main/java/org/exist/Namespaces.java om stead of adding it here. JAXP.java had the same definition too, which shall be combined.

Already addressed upstream — XSD_1_1_NS moved to Namespaces.java and the duplicate JAXP.java definition dropped in 0fe1766a96 (PR-C, #6530). This branch picked that up in today's rebase.

dizzzz · 2026-07-12T16:06:47Z

+     * attacker-influenced), so unlike {@link org.exist.validation.Xsd11SchemaDetection}'s
+     * location-driven cache, no per-Subject scoping/bounding is needed here.
+     */
+    private static final ConcurrentMap<String, Optional<Schema>> XSD11_SCHEMA_BY_NAMESPACE = new ConcurrentHashMap<>();


Shouldn't we use Cafeine for this, and make a shared/global cache for this? MutableCollection does not look a good place for this?

Done rather than deferred — split into two package-private classes in af2333f396: Xsd11SchemaCache (Caffeine-backed; no eviction policy, since the namespace set is finite and admin-controlled, which was your own point) holds the cache, and Xsd11ValidationHelper holds resolveXsd11SchemaForNamespace, the dynamic-discovery schema holder, validateWithXsd11Schema, and the lexical-handler forwarder. MutableCollection is down to a one-line delegating call. All 63 tests in the validation/XSD11 suite pass unchanged. This branch already picked up the fix in today's rebase.

dizzzz · 2026-07-12T16:09:10Z

            final AbstractIndex index = (AbstractIndex) clazz.newInstance();
            index.configure(pool, dataDir, config);
+            if (index.getIndexName() == null) {
+                index.setName(id);


what if if==null or "" or blank?

Handled — id != null && !id.isBlank() covers null, empty, and whitespace-only together; see the guard right above this line (21cd10a8cf, renumbered from 11d1c929d9 by today's rebase).

dizzzz

it looks that this PR combines 2 or 3 PRs (which makes the PR more difficult to review than strictly needed).

duncdrum · 2026-07-13T14:17:54Z

These are stacked PRs that all target develop as their GitHub base — so GitHub shows the full accumulated diff of the entire chain (schema governance → XSD 1.1 validation → fixture codegen → conf @enabled → module SPI) rather than just the incremental commits. The actual changes belonging to this PR are the commits in dp-module-spi above dp-conf-enabled-attr. The MutableCollection.java and JettyStart.java changes you see belong to dp-xsd11-validation (#6530) — they'll disappear from this diff once the earlier PRs merge.

MutableCollection.java:106 (Namespaces.java)
Good catch — XML_SCHEMA_NS was unused (callers already used XMLConstants directly), and XSD_1_1_NS is now Namespaces.XSD_1_1_NS (added to Namespaces.java). Both fixed in f7f3463460 on dp-xsd11-validation.

MutableCollection.java:116 (Caffeine / separate class)
The design concern is valid. The ConcurrentHashMap is correct here (the namespace set is finite and admin-controlled, so no eviction is needed), but moving the cache and XSD resolution logic out of MutableCollection into a dedicated class is worthwhile cleanup. Deferred to a follow-up PR so it doesn't block this chain.

IndexManager.java:146 (null/blank id)
Fixed in f52bf39b57. Configuration.configureIndexer() now validates factory.getDefaultId() before constructing an IndexModuleConfig entry (logs WARN and skips), with a belt-and-suspenders guard in IndexManager.initIndex() as well.

JettyStart.java:125 (different PR)
Agreed — split to standalone #6572. Only JettyStart.java is touched in that commit and the fix is fully independent of the XSD work.

On your question about XAR module loading:
Not directly — ServiceLoader.load(ModuleFactory.class, Configuration.class.getClassLoader()) only scans the main classpath, so a XAR's META-INF/services wouldn't be picked up automatically. But the abstraction is right: if XAR deployment registers factories with the EXistClassLoader (or a future Configuration.registerModuleFactory() hook), discovery would follow. This PR establishes the interface and wire-up pattern; XAR integration would be a natural extension.

dizzzz · 2026-07-13T18:47:57Z

            for (int i = 0; i < params.getLength(); i++) {
                final Element param = ((Element) params.item(i));

+                if ("no".equalsIgnoreCase(param.getAttribute("enabled"))) {


"no" or "false" or both?

"no" only — @enabled's type in conf.xsd is yes_no, an enumeration restricted to exactly yes/no (not xs:boolean), so "false" isn't a valid value to begin with. All 5 call sites (4 in Configuration.java, this one) check the same literal for that reason.

line-o · 2026-07-20T20:01:03Z

@dizzzz you approved this PR but have open questions on #6530 which this PR will close when merged.

…f MutableCollection MutableCollection carried a schema cache (plain ConcurrentHashMap) plus the namespace-resolution/validation logic that uses it -- neither is Collection state. Splits this into two package-private classes: - Xsd11SchemaCache: the per-namespace "needs XSD 1.1?" cache, now backed by Caffeine for consistency with the codebase's other caches (no eviction policy, since the namespace set is finite and admin-controlled -- this was never a correctness concern, just a style/testability one). - Xsd11ValidationHelper: resolveXsd11SchemaForNamespace, the dynamic-discovery schema holder, parseOrValidateXmlSource, validateWithXsd11Schema, and the Xsd11LexicalHandlerForwarder record -- store-time XSD 1.1 validation, not Collection bookkeeping. MutableCollection keeps only its public clearXsd11SchemaByNamespaceCache() facade (GrammarTooling.java's external call site) and a single delegating call from storeXmlDocument(); 9 imports that only the moved code needed are dropped. Behavior-preserving: all 63 tests across the validation/XSD11 suite pass unchanged. Addresses dizzzz's review comments on eXist-db#6530/eXist-db#6551.

… stylesheets conf-fixture.xsl and controller-config-fixture.xsl are new eXist-db-authored XSLT codegen tooling, not modifications of Adam Retter's original BSD-3 conf.xml/controller-config.xml. They fell outside the module's existing conf.xml/log4j2.xml excludes and so wrongly inherited the module-wide BSD-3 header. Excludes them from the BSD-3 licenseSet and adds a second licenseSet applying eXist-db's default LGPL-21 header to just these two files instead. Addresses review discussion between dizzzz, adamretter, and reinhapa on eXist-db#6551.

…X parser features in conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@enabled

…yes", scheduler jobs as live enabled="no" entries, @enabled on <parameter> and <property> in conf.xsd; bump schema to 2.5.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@SiZe

…2.6.0 - Add memorySizeType (pattern [0-9]+[KkMmGgTt]?|-1); apply to cacheSize, collectionCache, minDiskSpace, recovery/@SiZe - pool/@min, @max: xs:integer → xs:positiveInteger - lock-table/@trace-stack-depth: xs:int → xs:nonNegativeInteger - indexer/create/@type: xs:anySimpleType → xs:string - transformer/@caching: xs:string → yes_no with default="yes" - scheduler/job/@period: xs:string → xs:positiveInteger - vector-models/model/@Dimension: xs:integer → xs:positiveInteger - Fix swapped docs on raise-error-on-failed-retrieval / enforce-index-use - Fix "group-comit" typo in recovery/@group-commit doc - Fix rpc-server in-memory-size default 4196 → 4096 - Fill in TODO docs: indexer/create attrs, flushAfter, n, xquery module attrs (class/uri/src), xupdate/@growth-factor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@enabled

…ure triggers ParametersExtractor.parseProperties() used case-sensitive .equals("no") while every other enabled check in Configuration.java uses .equalsIgnoreCase(). An enabled="NO" parameter would be silently included instead of skipped. generate-conf-fixture.xsl stripped @enabled from kept module/index nodes but not from surviving triggers, so BouncyCastle and URLStreamHandler emitted an explicit enabled="yes" in generated fixtures — inconsistent with the module treatment and noise in fixture diffs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…nonical conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…abled

…abled to <vector-models> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…canonical conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…f.xml element AbstractIndex.configure() only sets name from config.getAttribute("id") when config != null. IndexFactory SPI registration passes config=null, leaving name null. IndexController.getWorkerByIndexName() matches on name, so SPI-registered indexes (range-index, ngram-index, sort-index, lucene-index) were never found, causing NPE at every range:index-keys-for-field() call. Fix: after configure(), if name is still null, call setName(id) with the configuration id that was used to key the indexers map. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…es covered by existing imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

factory.getDefaultId() is a third-party contract; a null or blank return would silently register the index under a useless key and leave its name unset. Validate at the Configuration.java SPI loop (primary gate) and add a belt-and-suspenders check in IndexManager.initIndex so the name is never set to null/blank. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-08-19T10:18:33Z

📊 XQTS result comparison

Comparison of this run against develop.

Warning

167 test cases were recorded in only one of the two runs (9 only in the previous run, 158 only in the current run). The runner's JUnit output is not fully deterministic (see eXist-db/exist-xqts-runner#74), so totals and per-category deltas include recording noise; the newly passing/failing lists count only tests recorded in both runs.

Metric	`develop`	this run	Change
🟢 Passed	28,814 (92.34%)	28,831 (91.96%)	+17 (-0.38 pp)
🔴 Failures	1,571	1,574	+3
➖ Errors	134	134	0
🔴 Skipped	685	814	+129
🧪 Total tests	31,204	31,353	+149

Relative to develop: 0 newly passing, 1 newly failing, 0 new errors, 0 newly skipped — counting only tests recorded in both runs whose outcome changed.

🔴 Newly failing tests (1)

Constr-inscope-1 (was passing)

⚪ Recorded only in this run (158)

casthcds12 (skipped)
CastAs-ListType-21 (skipped)
casthcds6 (skipped)
casthcds37 (skipped)
CastAs-UnionType-26 (skipped)
CastAs-ListType-1 (skipped)
casthcds3 (skipped)
user-defined-9 (skipped)
casthcds26 (skipped)
CastAs-UnionType-15 (skipped)
CastAs-UnionType-29 (skipped)
CastAs-UnionType-30 (skipped)
CastAs-ListType-4 (skipped)
CastAs-UnionType-19 (skipped)
user-defined-5 (skipped)
casthcds21 (skipped)
CastAs-ListType-13 (skipped)
CastAs-UnionType-2 (skipped)
casthcds40 (skipped)
casthcds10 (skipped)
CastAs-UnionType-36 (skipped)
CastAs-ListType-24 (skipped)
CastAs-ListType-8 (skipped)
notation-cast-2 (skipped)
user-defined-1 (skipped)
CastAs-ListType-28 (skipped)
casthcds29 (skipped)
CastAs-ListType-17 (skipped)
CastAs-UnionType-6 (skipped)
CastAs-ListType-6 (skipped)
CastAs-UnionType-14 (skipped)
CastAs-UnionType-20 (skipped)
user-defined-8 (skipped)
casthcds41 (skipped)
CastAs-ListType-19 (skipped)
casthcds9 (skipped)
CastAs-ListType-30 (skipped)
casthcds30 (skipped)
CastAs-UnionType-25 (skipped)
casthcds13 (skipped)
casthcds2 (skipped)
CastAs-ListType-5 (skipped)
CastAs-UnionType-31 (skipped)
casthcds33 (skipped)
casthcds16 (skipped)
CastAs-ListType-20 (skipped)
user-defined-6 (skipped)
user-defined-10 (skipped)
CastAs-ListType-9 (skipped)
CastAs-ListType-16 (skipped)
… and 108 more

⚪ Recorded only in `develop` (9)

fn-hours-from-time-16 (passing)
ArrayTest-060 (passing)
cbcl-fn-format-number-036 (passing)
eqname-913 (passing)
try-catch-fn-error-5 (passing)
Axes072-1 (passing)
fn-function-lookup-800 (passing)
fn-contains-token-60 (passing)
format-integer-067 (passing)

_{Runtime: 509.0s (+66.11s vs develop).}

duncdrum requested a review from a team as a code owner July 6, 2026 18:38

duncdrum added the needs documentation Signals issues or PRs that will require an update to the documentation repo label Jul 6, 2026

duncdrum force-pushed the dp-module-spi branch from b2fc02b to aae971b Compare July 6, 2026 19:12

duncdrum mentioned this pull request Jul 6, 2026

doc(configuration): adjust SPI conf.xml changes eXist-db/documentation#1268

Open

reinhapa requested changes Jul 7, 2026

View reviewed changes

Comment thread exist-core/src/main/java/org/exist/util/SaxonConfiguration.java Outdated

duncdrum force-pushed the dp-module-spi branch from 935b038 to 66fe2c5 Compare July 7, 2026 09:56

duncdrum mentioned this pull request Jul 7, 2026

[feature] Add @enabled attribute to conf.xml elements to disable entries without removing them #6550

Merged

4 tasks

duncdrum force-pushed the dp-module-spi branch 3 times, most recently from 1eece7a to afe8ce0 Compare July 7, 2026 13:57

line-o requested a review from a team July 8, 2026 12:23

line-o reviewed Jul 8, 2026

View reviewed changes

reinhapa requested changes Jul 9, 2026

View reviewed changes

Comment thread exist-core/src/test/java/org/exist/util/SchemaVersionSyncTest.java Outdated

dizzzz reviewed Jul 12, 2026

View reviewed changes

dizzzz requested changes Jul 12, 2026

View reviewed changes

Comment thread exist-core/src/main/java/org/exist/jetty/JettyStart.java

duncdrum force-pushed the dp-module-spi branch from afe8ce0 to f52bf39 Compare July 13, 2026 13:18

duncdrum force-pushed the dp-module-spi branch 2 times, most recently from 153a184 to 11d1c92 Compare July 13, 2026 15:07

reinhapa approved these changes Jul 13, 2026

View reviewed changes

dizzzz reviewed Jul 13, 2026

View reviewed changes

Comment thread extensions/exquery/restxq/src/test/resources-filtered/conf-fixture.xsl

dizzzz approved these changes Jul 13, 2026

View reviewed changes

duncdrum force-pushed the dp-module-spi branch from 11d1c92 to 21cd10a Compare July 26, 2026 15:01

duncdrum force-pushed the dp-module-spi branch from 21cd10a to 795cf70 Compare July 26, 2026 15:18

duncdrum force-pushed the dp-module-spi branch from 795cf70 to 9e0973c Compare August 18, 2026 09:16

duncdrum and others added 11 commits August 19, 2026 12:06

[feature] Add enabled="no" to disable modules, triggers, jobs, and SA…

a19e921

…X parser features in conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[feature] Complete @enabled coverage: startup triggers with enabled="…

9ec6fd7

…yes", scheduler jobs as live enabled="no" entries, @enabled on <parameter> and <property> in conf.xsd; bump schema to 2.5.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[feature] Add ModuleFactory SPI; wire all 27 bundled modules; trim ca…

f67b3a6

…nonical conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[feature] Integrate vector model registry with Configuration; add @en…

7f25cd0

…abled to <vector-models> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[bugfix] Suppress repeated WARN logging for unavailable vector models

cd0eac2

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[feature] Add IndexFactory SPI; wire all bundled index modules; trim …

2cda6d2

…canonical conf.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[ignore] Drop redundant FQNs in Factory inner classes; use simple nam…

e52b754

…es covered by existing imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

duncdrum force-pushed the dp-module-spi branch from 9e0973c to c03d824 Compare August 19, 2026 10:13

Uh oh!

Uh oh!

Conversation

duncdrum commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Compatibility

Related

Test plan

Uh oh!

Uh oh!

line-o left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dizzzz commented Jul 12, 2026

Uh oh!

dizzzz Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

duncdrum Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

dizzzz Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

duncdrum Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

dizzzz Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

duncdrum Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dizzzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

duncdrum commented Jul 13, 2026

Uh oh!

dizzzz Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

duncdrum Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

line-o commented Jul 20, 2026

Uh oh!

github-actions Bot commented Aug 19, 2026

📊 XQTS result comparison

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

duncdrum commented Jul 6, 2026 •

edited

Loading

duncdrum Jul 26, 2026 •

edited

Loading