Avoid NPE in XMP schema registry on threads that did not seed the containers

[SAY-5]

The thread-local namespace and prefix maps in StaticXmpCoreContainers are only populated by clearAllContainers(), which the XMPSchemaRegistryImpl constructor calls on the constructing thread. Any other thread, such as a virtual thread spawned after the singleton registry is built, sees a null map and getNamespacePrefix() throws a NullPointerException before it can fall back to the standard namespace map. Initialising the thread-locals with ThreadLocal.withInitial(HashMap::new) gives every thread an empty map, so lookups fall through to the standard maps as intended. Fixes #1603.

Summary by CodeRabbit

• Tests

  • Added thread-safety verification tests for XMP schema registry operations.

• Refactor

  • Improved initialization of thread-local storage to ensure proper multi-threaded operation.

[coderabbitai]

📝 Walkthrough

Walkthrough

StaticXmpCoreContainers ThreadLocal maps are now auto-initialized per thread using ThreadLocal.withInitial(HashMap::new) instead of bare new ThreadLocal<>(), eliminating null dereferences when accessed from Java Virtual Threads. A new test confirms cross-thread safety.

Changes

Thread-safe XMP Schema Registry

Layer / File(s)	Summary
ThreadLocal initialization and thread-access verification xmp-core/src/main/java/org/verapdf/xmp/containers/StaticXmpCoreContainers.java, core/src/test/java/org/verapdf/model/impl/axl/XMPSchemaRegistryThreadTest.java	StaticXmpCoreContainers switches namespaceToPrefixMap and prefixToNamespaceMap from bare new ThreadLocal<>() to ThreadLocal.withInitial(HashMap::new) to ensure each thread gets its own initialized empty map. XMPSchemaRegistryThreadTest verifies that getNamespacePrefix() can be called safely from a worker thread without throwing NullPointerException.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through threading streams,
No longer chased by null-pointer dreams,
ThreadLocal maps now auto-seed,
Each virtual thread gets what it needs,
Thread-safe XMP, a fix so keen! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing NPE in XMP schema registry on threads that did not seed containers.
Linked Issues check	✅ Passed	The code changes directly address issue #1603 by initializing ThreadLocal maps with ThreadLocal.withInitial(HashMap::new) to prevent NPE on unseed threads, and adds a test to verify the fix works across threads.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the ThreadLocal NPE issue: ThreadLocal initialization in StaticXmpCoreContainers and a regression test for cross-thread access.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

core/src/test/java/org/verapdf/model/impl/axl/XMPSchemaRegistryThreadTest.java (1)

52-52: ⚡ Quick win

Surface the captured throwable to preserve the stack trace on failure.

If the worker thread fails, assertEquals(null, failure.get()) reports only the throwable's toString() and discards the stack trace, making the regression harder to diagnose. Rethrowing (or fail-ing) with the captured throwable keeps full context.

♻️ Proposed improvement

-        assertEquals(null, failure.get());
+        if (failure.get() != null) {
+            throw new AssertionError("Lookup on foreign thread failed", failure.get());
+        }
         assertEquals("rdf:", prefix.get());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@core/src/test/java/org/verapdf/model/impl/axl/XMPSchemaRegistryThreadTest.java`
at line 52, The test currently calls assertEquals(null, failure.get()) which
loses the original stack trace; change the assertion to rethrow or fail with the
captured throwable from failure.get() in XMPSchemaRegistryThreadTest so the
original stack trace is preserved (e.g., check failure.get() != null and throw
or call fail with that Throwable, or wrap it in a RuntimeException while
preserving the cause). Locate the failure AtomicReference/field used via
failure.get() and replace the assertEquals usage with the rethrow/fail approach
to preserve full context.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@core/src/test/java/org/verapdf/model/impl/axl/XMPSchemaRegistryThreadTest.java`:
- Line 52: The test currently calls assertEquals(null, failure.get()) which
loses the original stack trace; change the assertion to rethrow or fail with the
captured throwable from failure.get() in XMPSchemaRegistryThreadTest so the
original stack trace is preserved (e.g., check failure.get() != null and throw
or call fail with that Throwable, or wrap it in a RuntimeException while
preserving the cause). Locate the failure AtomicReference/field used via
failure.get() and replace the assertEquals usage with the rethrow/fail approach
to preserve full context.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d549174-0130-4d20-9b7a-0dd12312b6dc

📥 Commits

Reviewing files that changed from the base of the PR and between cb8e9d7 and 599c745.

📒 Files selected for processing (2)

• core/src/test/java/org/verapdf/model/impl/axl/XMPSchemaRegistryThreadTest.java
• xmp-core/src/main/java/org/verapdf/xmp/containers/StaticXmpCoreContainers.java

[MaximPlusov]

@ SAY-5 Thank you for contribution!