Skip to content

Bugfix/ignore schema mappings and import mappings for forced generate schemas - #24771

Draft
Picazsoo wants to merge 6 commits into
OpenAPITools:masterfrom
Picazsoo:bugfix/ignore-schemaMappings-and-importMappings-for-forcedGenerateSchemas
Draft

Bugfix/ignore schema mappings and import mappings for forced generate schemas#24771
Picazsoo wants to merge 6 commits into
OpenAPITools:masterfrom
Picazsoo:bugfix/ignore-schemaMappings-and-importMappings-for-forcedGenerateSchemas

Conversation

@Picazsoo

@Picazsoo Picazsoo commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    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.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

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:

    • Generation runs in two phases. Phase 1 is unchanged. Phase 2 re-emits only forced schemas as stock models; forced-to-forced references use stock names; non-forced models keep mapped FQNs. typeMapping still applies. Wildcard * includes all mapping-suppressed schemas.
    • Added CodegenConfig.clearModelNameCache() and implemented it in major generators to invalidate model-name caches between phases.
    • Updated CLI/Gradle/Maven option docs for forcedGenerateSchemas to describe shadow-model behavior.
    • Samples/configs: added schemaMappings for Category and forcedGenerateSchemas in spring-boot-3.yaml and kotlin-spring-boot-3.yaml; samples now import com.example.mapped.Category; added a JSON equivalence test.
    • Tests: added end-to-end coverage for Java Spring, Kotlin Spring, and C#; updated existing tests to expect stock-name files when forced.
    • Build: the CLI shaded jar now excludes module-info.class.
  • Migration:

    • No action if you don’t use forcedGenerateSchemas.
    • If you relied on forced models being generated with mapped names, adjust: forced schemas now generate as stock names (for example, Category.java instead of a mapped ExternalCategory.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.

Review in cubic

@Picazsoo
Picazsoo marked this pull request as ready for review August 24, 2026 21:31
@Picazsoo
Picazsoo marked this pull request as draft August 24, 2026 21:32

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant