Bugfix/ignore schema mappings and import mappings for forced generate schemas - #24771
Conversation
…as as isolated shadow models
…mas as isolated shadow models
There was a problem hiding this comment.
7 issues found across 39 files
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/main/java/org/openapitools/codegen/DefaultGenerator.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java:625">
P2: When `generateRecursiveDependentModels` is enabled, forcing a mapped schema silently drops its dependent/child/oneOf models. Phase 1 defers forced shadow schemas, and Phases 2 adds `&& restrictModelEmissionTo == null` to skip the recursive-dependent block entirely during the forced pass, so no pass discovers the forced schema's dependents. Generate those dependent models (or run the recursive-dependent block) during the forced-schema pass instead of disabling it.</violation>
</file>
<file name="samples/server/petstore/kotlin-springboot-3/src/main/kotlin/com/example/mapped/Category.kt">
<violation number="1" location="samples/server/petstore/kotlin-springboot-3/src/main/kotlin/com/example/mapped/Category.kt:14">
P3: Category.kt implements `java.io.Serializable` but never declares a `serialVersionUID`, which triggers JVM serialization/lint warnings and leaves the serialized class identity unspecified. The generated shadow `org.openapitools.model.Category` declares `serialVersionUID` in a companion object; mirror it here, or drop `: Serializable` (the Java mapped model implements nothing).</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/AbstractKotlinCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/AbstractKotlinCodegenTest.java:38">
P3: Removing the first Javadoc line leaves the comment as a fragment: "using @BeforeMethod to have a fresh codegen mock for each test" now has no subject or context. Either keep the first line or reword the remaining line into a complete sentence.</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/ForcedGenerateSchemasCSharpTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/ForcedGenerateSchemasCSharpTest.java:72">
P3: The first test forces 5 schemas but only asserts 4 stock files exist, silently dropping ApiShape from the verification despite the comment claiming 'The forced+mapped schemas are emitted as stock ApiXxx classes'. It also never asserts that the mapped FQN (Com.Example.Mapped.) is absent from the forced files, which is the exact regression this PR guards against. Add ApiShape to the existence list and assert assertFileNotContains(..., "Com.Example.Mapped.") for the forced ApiXxx files, mirroring ForcedGenerateSchemasKotlinTest.</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultGeneratorTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultGeneratorTest.java:547">
P2: Part 4 verifies only that the stock Category.java file is produced, without any negative control showing that typeMapping+importMapping alone suppresses Category in Phase 1. If the type/import mapping did not actually suppress the model, this assertion would pass trivially and the new forced-generation override for type/import mappings would never be exercised. Add the no-force control (same type/import mappings, no addForcedGenerateSchema) asserting Category.java is NOT generated, mirroring Part 1's role for schemaMapping.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3/src/main/java/com/example/mapped/Category.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3/src/main/java/com/example/mapped/Category.java:9">
P2: When a mapped `Category` has a null field, Jackson serializes that field as `null`, unlike the generated `Category` and configured `NON_NULL` contract. Add class-level `@JsonInclude(NON_NULL)` so mapped and generated categories have the same JSON wire shape.</violation>
<violation number="2" location="samples/openapi3/server/petstore/springboot-3/src/main/java/com/example/mapped/Category.java:31">
P2: Requests containing an invalid `category.name` pass validation because `Pet.getCategory()` cascades into this mapped class, which exposes no `@Pattern` constraint. Add the schema regex to `getName()` so mapped `Category` preserves the OpenAPI validation contract.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| allProcessedModels = config.postProcessAllModels(allProcessedModels); | ||
|
|
||
| if (generateRecursiveDependentModels) { | ||
| if (generateRecursiveDependentModels && restrictModelEmissionTo == null) { |
There was a problem hiding this comment.
P2: When generateRecursiveDependentModels is enabled, forcing a mapped schema silently drops its dependent/child/oneOf models. Phase 1 defers forced shadow schemas, and Phases 2 adds && restrictModelEmissionTo == null to skip the recursive-dependent block entirely during the forced pass, so no pass discovers the forced schema's dependents. Generate those dependent models (or run the recursive-dependent block) during the forced-schema pass instead of disabling it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java, line 625:
<comment>When `generateRecursiveDependentModels` is enabled, forcing a mapped schema silently drops its dependent/child/oneOf models. Phase 1 defers forced shadow schemas, and Phases 2 adds `&& restrictModelEmissionTo == null` to skip the recursive-dependent block entirely during the forced pass, so no pass discovers the forced schema's dependents. Generate those dependent models (or run the recursive-dependent block) during the forced-schema pass instead of disabling it.</comment>
<file context>
@@ -537,7 +622,7 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
allProcessedModels = config.postProcessAllModels(allProcessedModels);
- if (generateRecursiveDependentModels) {
+ if (generateRecursiveDependentModels && restrictModelEmissionTo == null) {
for (ModelsMap modelsMap : allProcessedModels.values()) {
for (ModelMap mm : modelsMap.getModels()) {
</file context>
|
|
||
| List<File> files = generator.opts(configurator.toClientOptInput()).generate(); | ||
|
|
||
| Assert.assertTrue( |
There was a problem hiding this comment.
P2: Part 4 verifies only that the stock Category.java file is produced, without any negative control showing that typeMapping+importMapping alone suppresses Category in Phase 1. If the type/import mapping did not actually suppress the model, this assertion would pass trivially and the new forced-generation override for type/import mappings would never be exercised. Add the no-force control (same type/import mappings, no addForcedGenerateSchema) asserting Category.java is NOT generated, mirroring Part 1's role for schemaMapping.
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/DefaultGeneratorTest.java, line 547:
<comment>Part 4 verifies only that the stock Category.java file is produced, without any negative control showing that typeMapping+importMapping alone suppresses Category in Phase 1. If the type/import mapping did not actually suppress the model, this assertion would pass trivially and the new forced-generation override for type/import mappings would never be exercised. Add the no-force control (same type/import mappings, no addForcedGenerateSchema) asserting Category.java is NOT generated, mirroring Part 1's role for schemaMapping.</comment>
<file context>
@@ -498,24 +505,54 @@ public void forcedGenerateSchemaOverridesSchemaMappingSkip() throws IOException
+
+ List<File> files = generator.opts(configurator.toClientOptInput()).generate();
+
+ Assert.assertTrue(
+ files.stream().anyMatch(f -> f.getPath().replace('\\', '/').endsWith(originalModelRelPath)),
+ "Category.java MUST be generated when forced generation overrides type/import mapping suppression");
</file context>
| /** | ||
| * Handwritten production model used through the Category schema mapping. | ||
| */ | ||
| public class Category { |
There was a problem hiding this comment.
P2: When a mapped Category has a null field, Jackson serializes that field as null, unlike the generated Category and configured NON_NULL contract. Add class-level @JsonInclude(NON_NULL) so mapped and generated categories have the same JSON wire shape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3/src/main/java/com/example/mapped/Category.java, line 9:
<comment>When a mapped `Category` has a null field, Jackson serializes that field as `null`, unlike the generated `Category` and configured `NON_NULL` contract. Add class-level `@JsonInclude(NON_NULL)` so mapped and generated categories have the same JSON wire shape.</comment>
<file context>
@@ -0,0 +1,38 @@
+/**
+ * Handwritten production model used through the Category schema mapping.
+ */
+public class Category {
+ private @Nullable Long id;
+ private @Nullable String name;
</file context>
| public class Category { | |
| @com.fasterxml.jackson.annotation.JsonInclude(com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL) | |
| public class Category { |
| } | ||
|
|
||
| @JsonProperty("name") | ||
| public @Nullable String getName() { |
There was a problem hiding this comment.
P2: Requests containing an invalid category.name pass validation because Pet.getCategory() cascades into this mapped class, which exposes no @Pattern constraint. Add the schema regex to getName() so mapped Category preserves the OpenAPI validation contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3/src/main/java/com/example/mapped/Category.java, line 31:
<comment>Requests containing an invalid `category.name` pass validation because `Pet.getCategory()` cascades into this mapped class, which exposes no `@Pattern` constraint. Add the schema regex to `getName()` so mapped `Category` preserves the OpenAPI validation contract.</comment>
<file context>
@@ -0,0 +1,38 @@
+ }
+
+ @JsonProperty("name")
+ public @Nullable String getName() {
+ return name;
+ }
</file context>
| public @Nullable String getName() { | |
| @jakarta.validation.constraints.Pattern(regexp = "^[a-zA-Z0-9]+[a-zA-Z0-9\\.\\-_]*[a-zA-Z0-9]+$") | |
| public @Nullable String getName() { |
| val id: Long? = null, | ||
| @get:JsonProperty("name") | ||
| val name: String? = null | ||
| ) : Serializable |
There was a problem hiding this comment.
P3: Category.kt implements java.io.Serializable but never declares a serialVersionUID, which triggers JVM serialization/lint warnings and leaves the serialized class identity unspecified. The generated shadow org.openapitools.model.Category declares serialVersionUID in a companion object; mirror it here, or drop : Serializable (the Java mapped model implements nothing).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/kotlin-springboot-3/src/main/kotlin/com/example/mapped/Category.kt, line 14:
<comment>Category.kt implements `java.io.Serializable` but never declares a `serialVersionUID`, which triggers JVM serialization/lint warnings and leaves the serialized class identity unspecified. The generated shadow `org.openapitools.model.Category` declares `serialVersionUID` in a companion object; mirror it here, or drop `: Serializable` (the Java mapped model implements nothing).</comment>
<file context>
@@ -0,0 +1,14 @@
+ val id: Long? = null,
+ @get:JsonProperty("name")
+ val name: String? = null
+) : Serializable
</file context>
| private AbstractKotlinCodegen codegen; | ||
|
|
||
| /** | ||
| * In TEST-NG, test class (and its fields) is only constructed once (vs. for every test in Jupiter), |
There was a problem hiding this comment.
P3: Removing the first Javadoc line leaves the comment as a fragment: "using @BeforeMethod to have a fresh codegen mock for each test" now has no subject or context. Either keep the first line or reword the remaining line into a complete sentence.
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/kotlin/AbstractKotlinCodegenTest.java, line 38:
<comment>Removing the first Javadoc line leaves the comment as a fragment: "using @BeforeMethod to have a fresh codegen mock for each test" now has no subject or context. Either keep the first line or reword the remaining line into a complete sentence.</comment>
<file context>
@@ -35,7 +35,6 @@ public class AbstractKotlinCodegenTest {
/**
- * In TEST-NG, test class (and its fields) is only constructed once (vs. for every test in Jupiter),
* using @BeforeMethod to have a fresh codegen mock for each test
*/
@BeforeMethod
</file context>
| File modelDir = generate(output, "Widget", "Group", "Shape", "Circle", "Square"); | ||
|
|
||
| // The forced+mapped schemas are emitted as stock ApiXxx classes despite the FQN mapping. | ||
| for (String name : Arrays.asList("ApiWidget", "ApiGroup", "ApiCircle", "ApiSquare")) { |
There was a problem hiding this comment.
P3: The first test forces 5 schemas but only asserts 4 stock files exist, silently dropping ApiShape from the verification despite the comment claiming 'The forced+mapped schemas are emitted as stock ApiXxx classes'. It also never asserts that the mapped FQN (Com.Example.Mapped.) is absent from the forced files, which is the exact regression this PR guards against. Add ApiShape to the existence list and assert assertFileNotContains(..., "Com.Example.Mapped.") for the forced ApiXxx files, mirroring ForcedGenerateSchemasKotlinTest.
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/csharpnetcore/ForcedGenerateSchemasCSharpTest.java, line 72:
<comment>The first test forces 5 schemas but only asserts 4 stock files exist, silently dropping ApiShape from the verification despite the comment claiming 'The forced+mapped schemas are emitted as stock ApiXxx classes'. It also never asserts that the mapped FQN (Com.Example.Mapped.) is absent from the forced files, which is the exact regression this PR guards against. Add ApiShape to the existence list and assert assertFileNotContains(..., "Com.Example.Mapped.") for the forced ApiXxx files, mirroring ForcedGenerateSchemasKotlinTest.</comment>
<file context>
@@ -0,0 +1,96 @@
+ File modelDir = generate(output, "Widget", "Group", "Shape", "Circle", "Square");
+
+ // The forced+mapped schemas are emitted as stock ApiXxx classes despite the FQN mapping.
+ for (String name : Arrays.asList("ApiWidget", "ApiGroup", "ApiCircle", "ApiSquare")) {
+ assertTrue(new File(modelDir, name + ".cs").exists(), name + ".cs must be generated");
+ }
</file context>
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
Forced schema generation now bypasses schema/import mappings and emits “shadow” models under their stock (unmapped) names, while normal generation continues to use mapped classes. This fixes mapped schemas being skipped or emitted under invalid names and keeps APIs and supporting files unchanged.
Review notes:
typeMappingstill applies. Wildcard*includes all mapping-suppressed schemas.CodegenConfig.clearModelNameCache()and implemented it in major generators to invalidate model-name caches between phases.forcedGenerateSchemasto describe shadow-model behavior.schemaMappingsforCategoryandforcedGenerateSchemasinspring-boot-3.yamlandkotlin-spring-boot-3.yaml; samples now importcom.example.mapped.Category; added a JSON equivalence test.module-info.class.Migration:
forcedGenerateSchemas.Category.javainstead of a mappedExternalCategory.java), while generated APIs and non-forced models continue to reference the mapped class (for example,com.example.mapped.Category).Written for commit 0a194d3. Summary will update on new commits.