docs: clarify that supportingFiles is an allow-list (#22238) - #25075
brbousnguar wants to merge 1 commit into
Conversation
OpenAPITools#22238 reports that the restclient ApiClient references ServerConfiguration, ServerVariable and ExceptionProvider without those classes being available. They are registered as supporting files and live in the same package as ApiClient, so a default run emits all of them and there is nothing to import. The reported build fails because its <supportingFilesToGenerate> list predates the ApiClient gaining those references, and DefaultGenerator treats the list as a fixed allow-list, so the three files are skipped. Document that trap where it is read: a paragraph in the Selective generation section of docs/customization.md with the restclient example and a pointer to .openapi-generator-ignore, and a cross-reference from docs/global-properties.md. Correct the Javadoc on CodeGenMojo#supportingFilesToGenerate, which described modelsToGenerate instead, and add the same caveat to the Maven plugin README. Add two tests to JavaClientCodegenTest: one reproducing the reported configuration, where setting the supportingFiles global property to ApiClient.java emits ApiClient.java while skipping the three companions it references, and one pinning the premise the issue assumed had regressed, that a default restclient run emits all four files side by side. No generator behaviour change, so samples are untouched.
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/global-properties.md">
<violation number="1" location="docs/global-properties.md:26">
P3: The added sentence is hard to parse: "files a newer version of a generator added are not generated" is missing a relative pronoun. Since this PR's purpose is doc clarity, rephrase, e.g. "files added by a newer version of a generator are not generated unless the list mentions them".</violation>
<violation number="2" location="docs/global-properties.md:26">
P2: This applies the supporting-file upgrade warning to `models` and `apis`, whose lists filter model/API names rather than supporting-file outputs. Separate those name filters from the warning that newer supporting files are omitted only when `supportingFiles` is restricted.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| | modelTests | Allows the user to define if model tests will be generated. Prefer using the more robust `.openapi-generator-ignore`. | `true` or `false` | | ||
| | splitOperationsByContentType | Generates one operation per request/response content-type when an operation exposes several with different schemas | `true` or `false` | | ||
|
|
||
| Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation). |
There was a problem hiding this comment.
P2: This applies the supporting-file upgrade warning to models and apis, whose lists filter model/API names rather than supporting-file outputs. Separate those name filters from the warning that newer supporting files are omitted only when supportingFiles is restricted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/global-properties.md, line 26:
<comment>This applies the supporting-file upgrade warning to `models` and `apis`, whose lists filter model/API names rather than supporting-file outputs. Separate those name filters from the warning that newer supporting files are omitted only when `supportingFiles` is restricted.</comment>
<file context>
@@ -23,6 +23,8 @@ title: Global Properties
| modelTests | Allows the user to define if model tests will be generated. Prefer using the more robust `.openapi-generator-ignore`. | `true` or `false` |
| splitOperationsByContentType | Generates one operation per request/response content-type when an operation exposes several with different schemas | `true` or `false` |
+Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation).
+
</file context>
| Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation). | |
| Note that `supportingFiles` is an allow-list: supporting files added by a newer generator version are not generated unless the list mentions them, which can break the generated code. `models` and `apis` also accept allow-lists of model and API names, respectively — see [Selective generation](./customization.md#selective-generation). |
| | modelTests | Allows the user to define if model tests will be generated. Prefer using the more robust `.openapi-generator-ignore`. | `true` or `false` | | ||
| | splitOperationsByContentType | Generates one operation per request/response content-type when an operation exposes several with different schemas | `true` or `false` | | ||
|
|
||
| Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation). |
There was a problem hiding this comment.
P3: The added sentence is hard to parse: "files a newer version of a generator added are not generated" is missing a relative pronoun. Since this PR's purpose is doc clarity, rephrase, e.g. "files added by a newer version of a generator are not generated unless the list mentions them".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/global-properties.md, line 26:
<comment>The added sentence is hard to parse: "files a newer version of a generator added are not generated" is missing a relative pronoun. Since this PR's purpose is doc clarity, rephrase, e.g. "files added by a newer version of a generator are not generated unless the list mentions them".</comment>
<file context>
@@ -23,6 +23,8 @@ title: Global Properties
| modelTests | Allows the user to define if model tests will be generated. Prefer using the more robust `.openapi-generator-ignore`. | `true` or `false` |
| splitOperationsByContentType | Generates one operation per request/response content-type when an operation exposes several with different schemas | `true` or `false` |
+Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation).
+
</file context>
| Note that `supportingFiles`, `models` and `apis` take an allow-list: files a newer version of a generator added are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation). | |
| Note that `supportingFiles`, `models` and `apis` take an allow-list: files added by a newer version of a generator are not generated unless the list mentions them, which can break the generated code — see [Selective generation](./customization.md#selective-generation). |
Closes #22238.
What is actually going on
#22238 reports that the
restclientlibrary generates anApiClientwhichreferences
ServerConfiguration/ServerVariablewithout those classes beingavailable, and suggests adding imports. Two things about that:
ApiClient(invokerPackage), so animport would not help — and would not compile.
[java] Support templated servers #4998:
JavaClientCodegen.java:572-573addsServerConfiguration.mustacheand
ServerVariable.mustache, and:764addsExceptionProvider.mustachefor
restclient. A default run emits all of them next toApiClient.java.The reporter's build fails because of their own Maven configuration:
That maps to the
supportingFilesglobal property, whichDefaultGeneratortreats as a fixed allow-list:
The list was written before #21699 taught the
restclientApiClientaboutservers, so
ServerConfiguration.java,ServerVariable.javaandExceptionProvider.javaare now skipped while the emittedApiClientstillreferences them. Reproduced with the CLI equivalent on a minimal spec: with
--global-property supportingFiles="ApiClient.java:..."exactly those threefiles are missing from the output; without it, they are all generated and also
listed in
.openapi-generator/FILES. @BobLuursema reached the same conclusionin the issue thread.
So there is no generation bug to fix — but the trap is undocumented in the
places a user actually reads, and the Maven plugin's own Javadoc for the
option describes the wrong thing. This PR fixes that.
Changes
docs/customization.md— new paragraph in Selective generation noting thatan explicit list replaces the full set rather than adding to it, that
supporting files reference one another (with the
restclientApiClientexample), that each skip is logged at
INFOlevel, and that.openapi-generator-ignoreis the upgrade-safe alternative.docs/global-properties.md— one-line cross-reference to that caveat, sincethis is where
supportingFilesis documented as a global property and where auser building the list reads first.
CodeGenMojo.java— the Javadoc onsupportingFilesToGenerateread "A commaseparated list of models to generate. All models is the default.", copy-pasted
from
modelsToGenerate. Corrected, with the same caveat.modules/openapi-generator-maven-plugin/README.md— caveat added to thesupportingFilesToGeneraterow.JavaClientCodegenTest— two tests:testRestClientSupportingFilesAllowListSkipsApiClientCompanions_issue_22238reproduces the reported configuration: with the
supportingFilesglobalproperty set to
ApiClient.java,ApiClient.javais still emitted while theServerConfiguration,ServerVariableandExceptionProviderit referencesare skipped. This pins and documents the footgun itself.
testRestClientDefaultGenerationIncludesCompanionFilespins the premise theissue assumed had regressed — a default
restclientrun emits all four filesside by side — so the registration cannot silently disappear.
No generator behaviour changes, so
samples/is untouched.Testing
All green: the core library reports
Tests run: 5270, Failures: 0, Errors: 0, Skipped: 14withJavaClientCodegenTestat 299/0, the Maven plugin modulereports 27/0, and checkstyle passes with test sources included.
I also checked the default-generation test is meaningful: commenting out the
ServerConfiguration.mustacheregistration inJavaClientCodegenmakes it failwith
File does not exist when it should: .../ServerConfiguration.java.One note for anyone reproducing locally, since it cost me a round: the
cleanisload-bearing. This repo enables the Develocity Maven extension with the local
build cache on, and a cached
target/classesfrom before #24783 still containedthe ~30 mustache templates that commit moved from
cpp-boost-beast-client/tocpp-boost-beast-common/. Maven's resources plugin never deletes removed files,so the template locator resolved the stale copies and three unrelated tests
failed (
cppboostbeast.ModelApiSurfaceTestand bothtemplating.GeneratorTemplateContentLocatorAdditionalDirsTestmethods). Theypass on a clean build. Relatedly,
mvn ... test compilecannot be used here atall: the second lifecycle pass recompiles the antlr4-generated
KotlinLexer/KotlinParseragainst the compile classpath, whereantlr4-runtimeis test-scoped, and it fails before the reactor reaches theMaven plugin module.
PR checklist
(documentation and tests only), so there is nothing to regenerate.
the docs is the
restclientApiClientyou maintain; happy to reword.Summary by cubic
Clarifies that the
supportingFilesoption is a fixed allow-list, not an addition, and fixes the docs and Maven plugin Javadoc that described it incorrectly.Issue #22238 reported that the
restclientApiClientreferencesServerConfiguration,ServerVariable, andExceptionProviderwithout generating them. A default run does generate them; the reporter's build failed because their explicit list predates those references, and the allow-list skips anything not listed. This PR documents that trap instead of changing generation.docs/global-properties.md, and a recommendation to use.openapi-generator-ignoreas the upgrade-safe alternative.CodeGenMojoJavadoc forsupportingFilesToGenerate(previously copy-pasted frommodelsToGenerate) and the corresponding Maven plugin README row.restclientrun emits all referenced files.No generator behavior changes;
samples/is untouched.Written for commit 72b9504. Summary will update on new commits.