-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
fix(java): don't emit unqualified enum default for discriminator ref-to-property #24934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7155,7 +7155,25 @@ public void updateCodegenPropertyEnum(CodegenProperty var) { | |
| } | ||
| } | ||
| if (enumName != null) { | ||
| var.defaultValue = toEnumDefaultValue(var, enumName); | ||
| if (var.isEnum || !languageSpecificPrimitives.contains(varDataType)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a referenced inline enum resolves to a mapped scalar such as Java Prompt for AI agents |
||
| var.defaultValue = toEnumDefaultValue(var, enumName); | ||
| } else { | ||
| // Not an inline nested enum (var.isEnum) and varDataType is a raw | ||
| // language-specific primitive (e.g. "String"): there's no enum class at | ||
| // this use site to qualify the value with (e.g. a discriminator property | ||
| // that is a `$ref` into another schema's property rather than a ref to the | ||
| // enum schema itself). allowableValues/enum matching still ran, but | ||
| // emitting a bare, unqualified token like `String.ARCHIVE` would not | ||
| // compile. Drop the default instead (see #24874). | ||
| // | ||
| // Note: this check is intentionally based on languageSpecificPrimitives | ||
| // rather than referencedSchema.isPresent() — referencedSchema's name match | ||
| // against the raw dataType string is brittle for codegens whose dataType is | ||
| // namespace/package-qualified (e.g. PHP's `OpenAPI\Server\Model\Foo`), which | ||
| // would never match `toModelName(...)` and incorrectly drop valid enum | ||
| // defaults for genuine named-enum refs. | ||
| var.defaultValue = null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The else branch sets Prompt for AI agents |
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1869,6 +1869,27 @@ public void testObjectDefaultWithEnumProperty_issue24298() { | |
| .contains("new OutputFormat().order(OutputFormat.OrderEnum.SIMILARITY).limit(10)"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDiscriminatorPropertyRefToEnumDoesNotEmitInvalidDefault_issue24874() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make it easier to write/review: |
||
| final Path output = newTempFolder(); | ||
| final CodegenConfigurator configurator = new CodegenConfigurator() | ||
| .setGeneratorName(JAVA_GENERATOR) | ||
| .setInputSpec("src/test/resources/bugs/issue_24874.yaml") | ||
| .setAdditionalProperties(Map.of(MODEL_NAME_PREFIX, "Stock")) | ||
| .setOutputDir(output.toString().replace("\\", "/")); | ||
|
|
||
| Map<String, File> files = new DefaultGenerator().opts(configurator.toClientOptInput()).generate().stream() | ||
| .collect(Collectors.toMap(File::getName, Function.identity())); | ||
|
|
||
| // CategoryEvent.category is a discriminator property that is a `$ref` to another schema's | ||
| // (CategorySource) inline enum property, not to a named enum schema and not an inline enum | ||
| // itself. There is no enum type at this use site to qualify the discriminator mapping value | ||
| // with, so no default must be emitted here; previously this rendered the uncompilable | ||
| // `this.category = String.ARCHIVE;` (see #24874). | ||
| JavaFileAssert.assertThat(files.get("StockArchiveCategoryEvent.java")) | ||
| .fileDoesNotContain("String.ARCHIVE"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testWebClientJsonCreatorWithNullable_issue12790() { | ||
| final Path output = newTempFolder(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| openapi: "3.1.0" | ||
| info: | ||
| title: Shared category discriminator | ||
| version: 1.0.0 | ||
| paths: {} | ||
| components: | ||
| schemas: | ||
| CategorySource: | ||
| type: object | ||
| required: | ||
| - category | ||
| properties: | ||
| category: | ||
| type: string | ||
| enum: | ||
| - ARCHIVE | ||
| CategoryEvent: | ||
| type: object | ||
| required: | ||
| - category | ||
| properties: | ||
| category: | ||
| description: Event category | ||
| $ref: "#/components/schemas/CategorySource/properties/category" | ||
| discriminator: | ||
| propertyName: category | ||
| mapping: | ||
| ARCHIVE: "#/components/schemas/ArchiveCategoryEvent" | ||
| ArchiveCategoryEvent: | ||
| allOf: | ||
| - $ref: "#/components/schemas/CategoryEvent" | ||
| - type: object | ||
| properties: | ||
| value: | ||
| type: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just update the invalid pojo.mustache.
Remove the line in
openapi-generator/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache
Line 89 in c83886d