Skip to content

Fix response isArray, isMap flag - #24566

Open
wing328 wants to merge 9 commits into
masterfrom
fix-response-flags
Open

Fix response isArray, isMap flag#24566
wing328 wants to merge 9 commits into
masterfrom
fix-response-flags

Conversation

@wing328

@wing328 wing328 commented Aug 2, 2026

Copy link
Copy Markdown
Member

Fix response isArray, isMap flags
Commented tests will be updated later when we fix the isMapSchema
Add patches to Elm, Protobuf, WSDL generators to keep the old (incorrect) behavior (pending template update to fully work with the fix)

@OpenAPITools/generator-core-team

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

Fixes response schema classification in org.openapitools.codegen.DefaultCodegen#fromResponse so array, map, and free‑form object responses set isArray, isMap, and isFreeFormObject correctly and propagate containerType/containerTypeMapped, instead of being treated as models. Adds patches in ElmClientCodegen, ProtobufSchemaCodegen, and WsdlSchemaCodegen to keep object response handling stable pending template updates, updates Java CXF and C# tests, disables two spec-based tests tied to old behavior, and updates WSDL samples.

Written for commit f285ddb. Summary will update on new commits.

Review in cubic

@wing328 wing328 changed the title fix response isArray, isMap flag Fix response isArray, isMap flag Aug 2, 2026
@wing328
wing328 marked this pull request as ready for review August 2, 2026 16:18

@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.

5 issues found across 7 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/languages/ElmClientCodegen.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ElmClientCodegen.java:307">
P2: Typed map responses are again marked as models. A `type: object` response with `additionalProperties: { type: string }` reaches this branch and ends with both `isMap` and `isModel` true; preserve the superclass map classification by handling `isMapSchema` before setting `isModel`.</violation>
</file>

<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java:5002">
P2: Map and free-form responses now lose `additionalProperties` metadata because these early branches bypass `addVarsRequiredVarsAdditionalProps`. Populate vars/additional properties for both branches so generators consuming `response.additionalProperties` retain the map value schema.</violation>
</file>

<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ProtobufSchemaCodegen.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/ProtobufSchemaCodegen.java:682">
P2: Map responses remain marked as models because this `else` also matches `isMapSchema` responses. Preserve `isModel == false` for maps so response flags match the intended map classification.</violation>
</file>

<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java">

<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java:276">
P2: The new testIsFreeFormObjectFromSpec is disabled via @Test(enabled = false), so it never runs and adds zero coverage for the isFreeFormObject/isMapSchema classification that this PR is meant to verify. Either enable it (fixing the disabled status so it actually guards the behavior) or remove it — shipping dead, disabled test code gives a false impression that these cases are covered. If it was disabled because an assertion fails (e.g. the additionalProperties: false case), address that assertion rather than leaving the test disabled.</violation>
</file>

<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java">

<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java:3172">
P2: This PR silently disables testAdditionalPropertiesPresentInResponses while editing its assertions to assert isFreeFormObject. Because the test runs with enabled=false, none of the new checks execute and the array/map/free-form regression coverage being added by the fromResponse change is effectively dropped from CI. Please re-enable the test and update its assertions to match the corrected fromResponse behavior (or add equivalent running coverage) so the new flags are actually verified.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread samples/schema/petstore/wsdl-schema/service.wsdl Outdated
if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
cr.isFreeFormObject = true;
} else {
cr.isModel = true;

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: Typed map responses are again marked as models. A type: object response with additionalProperties: { type: string } reaches this branch and ends with both isMap and isModel true; preserve the superclass map classification by handling isMapSchema before setting isModel.

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/languages/ElmClientCodegen.java, line 307:

<comment>Typed map responses are again marked as models. A `type: object` response with `additionalProperties: { type: string }` reaches this branch and ends with both `isMap` and `isModel` true; preserve the superclass map classification by handling `isMapSchema` before setting `isModel`.</comment>

<file context>
@@ -286,6 +287,34 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert
+            if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
+                cr.isFreeFormObject = true;
+            } else {
+                cr.isModel = true;
+            }
+            cr.simpleType = false;
</file context>

if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
cr.isFreeFormObject = true;
} else {
cr.isModel = true;

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: Map responses remain marked as models because this else also matches isMapSchema responses. Preserve isModel == false for maps so response flags match the intended map classification.

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/languages/ProtobufSchemaCodegen.java, line 682:

<comment>Map responses remain marked as models because this `else` also matches `isMapSchema` responses. Preserve `isModel == false` for maps so response flags match the intended map classification.</comment>

<file context>
@@ -661,6 +662,34 @@ public String getNameFromDataType(CodegenProperty property) {
+            if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
+                cr.isFreeFormObject = true;
+            } else {
+                cr.isModel = true;
+            }
+            cr.simpleType = false;
</file context>
Suggested change
cr.isModel = true;
cr.isModel = !ModelUtils.isMapSchema(responseSchema);

Assert.assertFalse(ModelUtils.isFreeFormObject(null, openAPI));
}

@Test(enabled = false)

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: The new testIsFreeFormObjectFromSpec is disabled via @test(enabled = false), so it never runs and adds zero coverage for the isFreeFormObject/isMapSchema classification that this PR is meant to verify. Either enable it (fixing the disabled status so it actually guards the behavior) or remove it — shipping dead, disabled test code gives a false impression that these cases are covered. If it was disabled because an assertion fails (e.g. the additionalProperties: false case), address that assertion rather than leaving the test disabled.

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/utils/ModelUtilsTest.java, line 276:

<comment>The new testIsFreeFormObjectFromSpec is disabled via @Test(enabled = false), so it never runs and adds zero coverage for the isFreeFormObject/isMapSchema classification that this PR is meant to verify. Either enable it (fixing the disabled status so it actually guards the behavior) or remove it — shipping dead, disabled test code gives a false impression that these cases are covered. If it was disabled because an assertion fails (e.g. the additionalProperties: false case), address that assertion rather than leaving the test disabled.</comment>

<file context>
@@ -273,6 +273,27 @@ public void testIsFreeFormObject() {
         Assert.assertFalse(ModelUtils.isFreeFormObject(null, openAPI));
     }
 
+    @Test(enabled = false)
+    public void testIsFreeFormObjectFromSpec() {
+        final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/issue_7613.yaml");
</file context>

}

@Test
@Test(enabled = false)

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: This PR silently disables testAdditionalPropertiesPresentInResponses while editing its assertions to assert isFreeFormObject. Because the test runs with enabled=false, none of the new checks execute and the array/map/free-form regression coverage being added by the fromResponse change is effectively dropped from CI. Please re-enable the test and update its assertions to match the corrected fromResponse behavior (or add equivalent running coverage) so the new flags are actually verified.

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/DefaultCodegenTest.java, line 3172:

<comment>This PR silently disables testAdditionalPropertiesPresentInResponses while editing its assertions to assert isFreeFormObject. Because the test runs with enabled=false, none of the new checks execute and the array/map/free-form regression coverage being added by the fromResponse change is effectively dropped from CI. Please re-enable the test and update its assertions to match the corrected fromResponse behavior (or add equivalent running coverage) so the new flags are actually verified.</comment>

<file context>
@@ -3169,7 +3169,7 @@ public void testAdditionalPropertiesPresentInParameters() {
     }
 
-    @Test
+    @Test(enabled = false)
     public void testAdditionalPropertiesPresentInResponses() {
         final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/issue_7613.yaml");
</file context>

} else if (ModelUtils.isDoubleSchema(responseSchema)) { // double
r.isDouble = Boolean.TRUE;
}
} else if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {

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: Map and free-form responses now lose additionalProperties metadata because these early branches bypass addVarsRequiredVarsAdditionalProps. Populate vars/additional properties for both branches so generators consuming response.additionalProperties retain the map value schema.

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/DefaultCodegen.java, line 5002:

<comment>Map and free-form responses now lose `additionalProperties` metadata because these early branches bypass `addVarsRequiredVarsAdditionalProps`. Populate vars/additional properties for both branches so generators consuming `response.additionalProperties` retain the map value schema.</comment>

<file context>
@@ -4999,12 +4999,26 @@ public CodegenResponse fromResponse(String responseCode, ApiResponse response) {
             } else if (ModelUtils.isDoubleSchema(responseSchema)) { // double
                 r.isDouble = Boolean.TRUE;
             }
+        } else if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
+            r.isFreeFormObject = true;
+            r.simpleType = false;
</file context>

@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.

1 issue found across 1 file (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/main/java/org/openapitools/codegen/languages/WsdlSchemaCodegen.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/WsdlSchemaCodegen.java:405">
P1: Map responses now generate `type="xs:Map"`, which is not an XML Schema built-in and makes the generated WSDL invalid for the bundled inventory response. Preserve the fixed map state (`isModel = false`) rather than restoring `isModel` for map schemas.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

responseSchema = ModelUtils.getSchemaFromResponse(openAPI, response);
}

if (ModelUtils.isTypeObjectSchema(responseSchema)) {

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.

P1: Map responses now generate type="xs:Map", which is not an XML Schema built-in and makes the generated WSDL invalid for the bundled inventory response. Preserve the fixed map state (isModel = false) rather than restoring isModel for map schemas.

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/languages/WsdlSchemaCodegen.java, line 405:

<comment>Map responses now generate `type="xs:Map"`, which is not an XML Schema built-in and makes the generated WSDL invalid for the bundled inventory response. Preserve the fixed map state (`isModel = false`) rather than restoring `isModel` for map schemas.</comment>

<file context>
@@ -385,4 +388,34 @@ public String escapeUnsafeCharacters(String input) {
+            responseSchema = ModelUtils.getSchemaFromResponse(openAPI, response);
+        }
+
+        if (ModelUtils.isTypeObjectSchema(responseSchema)) {
+            CodegenProperty cp = fromProperty("response", responseSchema, false);
+
</file context>

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant