fix(normalizer): simplify oneOf/anyOf with a single inline const/enum… - #24821
fix(normalizer): simplify oneOf/anyOf with a single inline const/enum…#24821nagabalaji-b wants to merge 1 commit into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 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/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java:1730">
P3: The comment links to an incomplete issue URL that ends in "issues/" with no issue number, so the link is dead. Remove the placeholder URL or fill in the actual issue number referenced in the PR.</violation>
<violation number="2" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java:1746">
P3: These assertions pass the actual value in the expected position (assertEquals takes expected first). On a failed assertion JUnit will report the swapped values, e.g. "expected:<[this-is-my-only-value]> but was:<...>", which misleads when debugging. Swap to assertEquals(expected, actual), or use assertNull/assertTrue where clearer.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/EnumUtils.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/EnumUtils.java:131">
P2: In OpenAPI 3.1, this treats `$ref` schemas with sibling constraints as lone references, so single `oneOf`/`anyOf` branches combining a reference with `const` or `enum` never reach enum simplification. Distinguish a pure `$ref` from a reference with siblings, and handle the sibling constraints before deciding to skip it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // prevents named enum schemas from being converted to inline enum schemas. An inline | ||
| // const/enum sub-schema (e.g. `oneOf: [{const: foo}]`), however, is processed below so | ||
| // that single-value enums are simplified the same way as multi-value ones. | ||
| if (!(onlySubSchema instanceof Schema) || ((Schema) onlySubSchema).get$ref() != null) { |
There was a problem hiding this comment.
P2: In OpenAPI 3.1, this treats $ref schemas with sibling constraints as lone references, so single oneOf/anyOf branches combining a reference with const or enum never reach enum simplification. Distinguish a pure $ref from a reference with siblings, and handle the sibling constraints before deciding to skip 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/utils/EnumUtils.java, line 131:
<comment>In OpenAPI 3.1, this treats `$ref` schemas with sibling constraints as lone references, so single `oneOf`/`anyOf` branches combining a reference with `const` or `enum` never reach enum simplification. Distinguish a pure `$ref` from a reference with siblings, and handle the sibling constraints before deciding to skip it.</comment>
<file context>
@@ -118,11 +118,20 @@ public static Schema simplifyComposedSchemaWithEnums(Schema schema,
+ // prevents named enum schemas from being converted to inline enum schemas. An inline
+ // const/enum sub-schema (e.g. `oneOf: [{const: foo}]`), however, is processed below so
+ // that single-value enums are simplified the same way as multi-value ones.
+ if (!(onlySubSchema instanceof Schema) || ((Schema) onlySubSchema).get$ref() != null) {
+ return schema;
+ }
</file context>
|
|
||
| @Test | ||
| public void testOpenAPINormalizerSingleOneOfConstEnum31Spec() { | ||
| // reproduces https://github.com/OpenAPITools/openapi-generator/issues/ where a `oneOf` wrapping |
There was a problem hiding this comment.
P3: The comment links to an incomplete issue URL that ends in "issues/" with no issue number, so the link is dead. Remove the placeholder URL or fill in the actual issue number referenced in the PR.
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/OpenAPINormalizerTest.java, line 1730:
<comment>The comment links to an incomplete issue URL that ends in "issues/" with no issue number, so the link is dead. Remove the placeholder URL or fill in the actual issue number referenced in the PR.</comment>
<file context>
@@ -1725,6 +1725,30 @@ public void testOpenAPINormalizerSingleConstEnum31Spec() {
+ @Test
+ public void testOpenAPINormalizerSingleOneOfConstEnum31Spec() {
+ // reproduces https://github.com/OpenAPITools/openapi-generator/issues/ where a `oneOf` wrapping
+ // a single `const` sub-schema (as opposed to 2+ consts) was not simplified into a proper enum,
+ // and lost the parent's `type`/`default`/`x-omitempty` in the process.
</file context>
| // reproduces https://github.com/OpenAPITools/openapi-generator/issues/ where a `oneOf` wrapping | |
| // reproduces an issue where a `oneOf` wrapping |
| Schema normalizedTypeSchema = (Schema) schema2.getProperties().get("type"); | ||
| assertTrue(ModelUtils.isEnumSchema(normalizedTypeSchema)); | ||
| assertNull(normalizedTypeSchema.getOneOf()); | ||
| assertEquals(normalizedTypeSchema.getEnum(), List.of("this-is-my-only-value")); |
There was a problem hiding this comment.
P3: These assertions pass the actual value in the expected position (assertEquals takes expected first). On a failed assertion JUnit will report the swapped values, e.g. "expected:<[this-is-my-only-value]> but was:<...>", which misleads when debugging. Swap to assertEquals(expected, actual), or use assertNull/assertTrue where clearer.
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/OpenAPINormalizerTest.java, line 1746:
<comment>These assertions pass the actual value in the expected position (assertEquals takes expected first). On a failed assertion JUnit will report the swapped values, e.g. "expected:<[this-is-my-only-value]> but was:<...>", which misleads when debugging. Swap to assertEquals(expected, actual), or use assertNull/assertTrue where clearer.</comment>
<file context>
@@ -1725,6 +1725,30 @@ public void testOpenAPINormalizerSingleConstEnum31Spec() {
+ Schema normalizedTypeSchema = (Schema) schema2.getProperties().get("type");
+ assertTrue(ModelUtils.isEnumSchema(normalizedTypeSchema));
+ assertNull(normalizedTypeSchema.getOneOf());
+ assertEquals(normalizedTypeSchema.getEnum(), List.of("this-is-my-only-value"));
+ assertEquals(ModelUtils.getType(normalizedTypeSchema), "string");
+ assertEquals(normalizedTypeSchema.getDefault(), "this-is-my-only-value");
</file context>
| assertEquals(normalizedTypeSchema.getEnum(), List.of("this-is-my-only-value")); | |
| assertEquals(List.of("this-is-my-only-value"), normalizedTypeSchema.getEnum()); |
|
Could we elaborate on what is the background for having a pattern with a single Having the field be required while at the same time stating a default is also odd, since they are basically mutually exclusive (since a fallback value does not make sense if you tell the client that they always have to send it). |
… sub-schema
Previously EnumUtils.simplifyComposedSchemaWithEnums bailed out whenever a
oneOf/anyOf had fewer than 2 sub-schemas, regardless of whether the lone
sub-schema was a named \ (which should be preserved) or an inline
const/enum literal (e.g. oneOf: [{const: FIAttached}]).
For the inline case, the schema fell through to
simplifyOneOfAnyOfWithOnlyOneNonNullSubSchema, which collapses to the
sub-schema but does not convert const to enum and drops the parent's
type/default/extensions, producing a plain string field instead of a
generated enum (e.g. in C# generichost/netcore templates).
Only skip simplification now when the sole sub-schema is a \, preserving
reusable named enum schemas while correctly simplifying single-value inline
const/enum oneOf/anyOf schemas.
e2c23d0 to
5747fb8
Compare
Thanks for your quicker feedback @Mattias-Sehlstedt, got your point, i'll check with the team and get back. |
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 11 files (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/utils/EnumUtils.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/EnumUtils.java:131">
P2: For a valid `oneOf: [{const: null}]`, this new branch admits the child, but the loop below treats `subSchema.getConst() == null` as an absent const and returns the original oneOf. Preserve const presence separately from its value before using this single-schema path.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
| // prevents named enum schemas from being converted to inline enum schemas. An inline | ||
| // const/enum sub-schema (e.g. `oneOf: [{const: foo}]`), however, is processed below so | ||
| // that single-value enums are simplified the same way as multi-value ones. | ||
| if (!(onlySubSchema instanceof Schema) || ((Schema) onlySubSchema).get$ref() != null) { |
There was a problem hiding this comment.
P2: For a valid oneOf: [{const: null}], this new branch admits the child, but the loop below treats subSchema.getConst() == null as an absent const and returns the original oneOf. Preserve const presence separately from its value before using this single-schema path.
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/utils/EnumUtils.java, line 131:
<comment>For a valid `oneOf: [{const: null}]`, this new branch admits the child, but the loop below treats `subSchema.getConst() == null` as an absent const and returns the original oneOf. Preserve const presence separately from its value before using this single-schema path.</comment>
<file context>
@@ -118,11 +118,20 @@ public static Schema simplifyComposedSchemaWithEnums(Schema schema,
+ // prevents named enum schemas from being converted to inline enum schemas. An inline
+ // const/enum sub-schema (e.g. `oneOf: [{const: foo}]`), however, is processed below so
+ // that single-value enums are simplified the same way as multi-value ones.
+ if (!(onlySubSchema instanceof Schema) || ((Schema) onlySubSchema).get$ref() != null) {
+ return schema;
+ }
</file context>
2 participants
For OpenAPI 3.1 schemas where a property is a oneOf wrapping a single inline const (not a
$refto a named enum, not 2+ consts), OpenAPINormalizer failed to simplify it into an enum — dropping the parent's type, default, and vendor extensions (e.g.x-omitempty). Generators (e.g. C# generichost/netcore) emitted a plain string instead of an enum.Example:
Before: falls through unsimplified → generated as plain string, extensions lost.
After: normalized to enum: [this-is-my-only-value], type: string, default and x-omitempty preserved.
Root cause: `
``EnumUtils.simplifyComposedSchemaWithEnums
skipped simplification whenever **oneOf**/**anyOf** had fewer than 2 sub-schemas — treating a lone$ref``` (named enum, should be preserved) the same as a lone inline const/enum (should be simplified).Fix:
EnumUtils.java:118now only skips simplification when the single sub-schema is a$ref. Inline single-value const/enum sub-schemas are simplified like multi-value ones.Testing:
Added
testOpenAPINormalizerSingleOneOfConstEnum31Specin
OpenAPINormalizerTest.java:1728-1749+ new SingleValueOneOfConst_3_1 fixture inenum-single-value.yaml. Confirmed no regression for multi-value and
$ref-to-named-enum cases../mvnw clean package -DskipTestssucceeded; regenerated csharp-restsharp/httpclient samples and docs — no diffs.
Validate:
mvn test -pl modules/openapi-generator -Dtest=OpenAPINormalizerTest#testOpenAPINormalizerSingleOneOfConstEnum31SpecPR 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 the OpenAPI normalizer so
oneOf/anyOfwith a single inlineconstorenumsub-schema is simplified into a proper enum, preserving the parent'stype,default, and vendor extensions likex-omitempty. Previously these schemas fell through and generated a plain string instead of an enum.Details
EnumUtils.simplifyComposedSchemaWithEnumsnow skips simplification only when the lone sub-schema is a$refto a named schema; inline const/enum sub-schemas are processed like multi-value ones.oneOfconst case; existing multi-value and$ref-to-named-enum behavior is unchanged.Written for commit 5747fb8. Summary will update on new commits.