feat: honor thread context classloader when loading custom normalizers and generators - #24617
feat: honor thread context classloader when loading custom normalizers and generators#24617Picazsoo wants to merge 26 commits into
Conversation
…t classloader support
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Keep core error message tool-agnostic (no Gradle-specific terms); Gradle-specific guidance remains only in the plugin README/docs. - Add missing test coverage for generatorClasspath + classloader isolation. - Clean up temp directories created by classloader fallback tests to avoid leaking compiled fixture files across test runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lizer KDoc Cross-reference generatorClasspath/openApiGeneratorExtra directly on the openapiNormalizer property (both extension and task) so the requirement is discoverable from IDE tooltips/KDoc, not just the README. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Split ClassNotFoundException (classpath guidance) from other ReflectiveOperationException cases (constructor/instantiation failures) in OpenAPINormalizer.createNormalizer so the error message matches the actual failure. - Replace redundant expectedExceptions + manual catch/rethrow in OpenAPINormalizerTest with expectThrows. - Clean up normalizer fixture temp directories after each test in GeneratorClasspathIsolationTest (AfterMethod deleteRecursively). - Assert on the wrapped classpath-guidance log message in addition to the raw ClassNotFoundException text, reducing coupling to log format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Skip (not NPE) the TCCL-fixture test when running on a JRE without a system Java compiler (ToolProvider.getSystemJavaCompiler() returns null). - Assert that positive GeneratorClasspathIsolationTest cases show no NORMALIZER_CLASS load failure/ClassNotFoundException in the build log, in addition to TaskOutcome.SUCCESS, since DefaultGenerator logs but does not fail the build on a normalizer load failure. Without this, a regression dropping the forwarded classpath would pass silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fixture normalizer now overrides normalize() to write a marker file (path passed via a MARKER_FILE inputRule) before delegating to super, giving direct proof the custom NORMALIZER_CLASS ran under the worker - across a forked JVM in 'process' isolation - rather than relying only on the build succeeding and no failure text appearing in the log. The negative-control test asserts the marker is absent when the class fails to load, corroborating that normalize() never executed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…fixture Mirrors the guard already present in OpenAPINormalizerTest's compileNormalizerFixture: skip with a clear SkipException instead of an opaque NullPointerException when running on a JRE without a system Java compiler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ove error messages
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…or class loading issues
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…loading exceptions
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…n failures during class loading
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…e loading exceptions
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/CodegenConfigLoaderTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/CodegenConfigLoaderTest.java:223">
P2: This test locks in a behavior that contradicts the PR's stated goal: a broken provider on the first line of a service file silently hides every valid generator listed after it in that same file, so a user with one malformed entry in their custom-generators jar loses functioning generators. Consider having getAll() continue enumeration past per-provider class-not-found errors (only break on the resource-location/advancement error that could spin), and flip this test to assert AfterMalformedEntryCodegen IS discovered rather than documenting the lossy outcome as intentional.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
|
|
||
| @Test | ||
| public void testEnumerationStopsAtMalformedEntryAndSkipsLaterEntriesInSameServiceFile() throws Exception { |
There was a problem hiding this comment.
P2: This test locks in a behavior that contradicts the PR's stated goal: a broken provider on the first line of a service file silently hides every valid generator listed after it in that same file, so a user with one malformed entry in their custom-generators jar loses functioning generators. Consider having getAll() continue enumeration past per-provider class-not-found errors (only break on the resource-location/advancement error that could spin), and flip this test to assert AfterMalformedEntryCodegen IS discovered rather than documenting the lossy outcome as intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/java/org/openapitools/codegen/CodegenConfigLoaderTest.java, line 223:
<comment>This test locks in a behavior that contradicts the PR's stated goal: a broken provider on the first line of a service file silently hides every valid generator listed after it in that same file, so a user with one malformed entry in their custom-generators jar loses functioning generators. Consider having getAll() continue enumeration past per-provider class-not-found errors (only break on the resource-location/advancement error that could spin), and flip this test to assert AfterMalformedEntryCodegen IS discovered rather than documenting the lossy outcome as intentional.</comment>
<file context>
@@ -218,6 +219,47 @@ public void testMalformedSpiEntryDoesNotPreventDirectConfigLoading() throws Exce
}
+ @Test
+ public void testEnumerationStopsAtMalformedEntryAndSkipsLaterEntriesInSameServiceFile() throws Exception {
+ // Documents the deliberate behavior of getAll(): a failure while *advancing* the ServiceLoader
+ // iterator (here a non-existent provider class named on the first line of a service file) stops
</file context>
This reverts commit ce9ee8e.
A custom
NORMALIZER_CLASSnormalizer (and, more generally, any custom class referenced by name in generator options, such as a custom generator selected bygeneratorName/FQCN) could fail to load, especially under the Gradle plugin'sprocessworker isolation:OpenAPINormalizer.createNormalizerused bareClass.forName(String), andCodegenConfigLoaderusedServiceLoader.load(..., CodegenConfig.class.getClassLoader())/ bareClass.forName(String), all of which resolve against the classloader that loadedopenapi-generatorcore, not any user-supplied classpath.classpath.from(...)in either isolation branch. Underprocessisolation this is fatal: the forked JVM has zero visibility into anything not explicitly forwarded. Underclassloaderisolation it was merely fragile: since the worker runs in the same JVM, a custom normalizer could work by accident if it happened to already be reachable through the existing classloader hierarchy (e.g. stuck on the buildscript classpath), but this was never a supported or documented mechanism.Changes
Core (
OpenAPINormalizer.java)createNormalizernow resolves the class via the current thread's context classloader (TCCL) first (which Gradle's Worker API sets to include user classpath), falling back to the original defining classloader for backward compatibility.ClassNotFoundExceptiongets classpath guidance, other reflective failures (bad constructor, instantiation errors) get a distinct, accurate message.Core (
CodegenConfigLoader.java)getAll) and directClass.forName(forName) now consult the TCCL first, falling back to the defining classloader, so custom generators on the worker classpath are found while existing generators keep loading unchanged.getAll()discovers providers across both classloaders, dedupes by class name, and tolerates a broken/unrelated SPI provider (logged and skipped rather than aborting discovery).forNamefailure messages are split by cause: not found on classpath, static-initializer failure, incompatible Java version, other linkage errors, and "found but not a validCodegenConfig/ missing public no-arg constructor".Gradle plugin
generatorClasspath(file collection) and anopenApiGeneratorExtraresolvable configuration, both forwarded to the worker inprocessIsolationandclassLoaderIsolation. Users can now supply a custom normalizer or custom generator jar via normal Gradle dependencies or file collections.openapiNormalizerKDoc, README, and customization docs.Tests
OpenAPINormalizerTest) and the generator loader (CodegenConfigLoaderTest; the latter also covers the not-a-CodegenConfig, missing-constructor, linkage-error, failing-static-initializer, and broken/malformed-SPI cases).GeneratorClasspathIsolationTestcovering both classpath mechanisms under both isolation modes, plus a negative control. Positive tests assert a marker file written by the fixture normalizer'snormalize()to directly prove it ran, not just that the build succeeded.Compatibility
Fully backward compatible. No
NORMALIZER_CLASSset, or normalizer already visible on the default classpath: unchanged behavior. New properties default to empty/unset. One intentional behavior change: a broken third-partyCodegenConfigSPI provider that previously aborted generator discovery is now logged atWARNand skipped.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Honor the thread context classloader when loading custom normalizers and generators, and forward user classpaths in the Gradle plugin for both worker isolation modes. Adds clear, tool-agnostic errors, resilient SPI discovery, updated docs, and tests that prove a normalizer and a generator actually run.
New Features
OpenAPINormalizerprefers the thread context classloader with fallback; splits classpath/not-found vs constructor errors.CodegenConfigLoaderprefers the context classloader;getAll()merges/dedupes across classloaders and skips broken/malformed SPI providers and resource I/O errors with warnings;forName()returns precise errors for classpath/not-found (incl.NoClassDefFoundErrorafter a failing initializer), incompatible Java version, static initializer failures, linkage errors, and bad constructors/implementations. Tests cover TCCL resolution, fallbacks, SPI edge cases, and error messaging for both normalizers and generators.openApiGeneratorExtraand ageneratorClasspathproperty, both forwarded to the worker classpath forworkerIsolationprocessandclassloader. README/KDoc/docs explain classpath requirements forNORMALIZER_CLASSand custom generators (by name/FQCN). Tests cover both isolation modes and both classpath mechanisms, generator by FQCN, negative controls, and execution proven via marker files.Migration
NORMALIZER_CLASSor custom generator (by name/FQCN) with the Gradle plugin, add it toopenApiGeneratorExtra(preferred) orgeneratorClasspath, e.g.:dependencies { openApiGeneratorExtra("com.acme:my-normalizer:1.0.0") }openApiGenerate { generatorClasspath.from(files("libs/my-generator.jar")) }Written for commit 920f6a7. Summary will update on new commits.