Fix response isArray, isMap flag - #24566
Conversation
There was a problem hiding this comment.
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
| if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) { | ||
| cr.isFreeFormObject = true; | ||
| } else { | ||
| cr.isModel = true; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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>
| cr.isModel = true; | |
| cr.isModel = !ModelUtils.isMapSchema(responseSchema); |
| Assert.assertFalse(ModelUtils.isFreeFormObject(null, openAPI)); | ||
| } | ||
|
|
||
| @Test(enabled = false) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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)) { |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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)) { |
There was a problem hiding this comment.
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>
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
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
Fixes response schema classification in
org.openapitools.codegen.DefaultCodegen#fromResponseso array, map, and free‑form object responses setisArray,isMap, andisFreeFormObjectcorrectly and propagatecontainerType/containerTypeMapped, instead of being treated as models. Adds patches inElmClientCodegen,ProtobufSchemaCodegen, andWsdlSchemaCodegento 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.