| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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/main/resources/csharp/libraries/generichost/JsonConverter.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache:386">
P2: For primitive oneOf members the new code serializes the value as a bare JSON primitive and `return`s before `writer.WriteStartObject()` is reached. This path is asymmetric with the referenced-object path it sits next to: the object path preserves the parent's sibling properties via WriteProperties, but the primitive path writes only the bare scalar and drops every sibling property (and never emits an object). For a composed model that mixes a primitive oneOf member with sibling properties (e.g. `oneOf: [string, Apple]` plus a `color` sibling), serializing when the primitive is selected silently omits the sibling data, so the output won't round-trip. Consider writing the primitive into the object (or documenting that a primitive oneOf cannot be combined with siblings).</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache:403">
P1: `oneOf` alternatives that are arrays/maps now generate a `<baseType>JsonConverter` lookup and `WriteProperties` call, so their clients fail to compile or serialize; limit this path to model references and serialize collection alternatives directly before starting an object.</violation>
<violation number="3" location="modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache:404">
P2: The referenced oneOf model's properties are written directly into the parent's already-open JSON object (flattened), followed by the parent's own WriteProperties. If a oneOf child schema shares a JSON property name with a sibling property on the parent (or with the discriminator/additionalProperties), the serialized output will contain duplicate keys for that name. The included Fruit sample avoids this only by luck (child keys kind/count/sweet vs parent key color); a child such as `apple` defining a `color` property alongside Fruit's `color` sibling would produce duplicate "color" entries, and on deserialization the parent's value silently wins because it is written last. Consider guarding against name collisions (e.g. wrapper/discriminator object or a name-overlap check) or at least documenting this flattening limitation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
| {{^isPrimitiveType}} | ||
| if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null) | ||
| { | ||
| {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = ({{baseType}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}.GetType())); |
There was a problem hiding this comment.
P1: oneOf alternatives that are arrays/maps now generate a <baseType>JsonConverter lookup and WriteProperties call, so their clients fail to compile or serialize; limit this path to model references and serialize collection alternatives directly before starting an object.
Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache, line 403:
<comment>`oneOf` alternatives that are arrays/maps now generate a `<baseType>JsonConverter` lookup and `WriteProperties` call, so their clients fail to compile or serialize; limit this path to model references and serialize collection alternatives directly before starting an object.</comment>
<file context>
@@ -376,7 +376,39 @@
+ {{^isPrimitiveType}}
+ if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null)
+ {
+ {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = ({{baseType}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}.GetType()));
+ {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}}.WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions);
+ }
</file context>
Sorry, something went wrong.
| if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null) | ||
| { | ||
| {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = ({{baseType}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}.GetType())); | ||
| {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}}.WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions); |
There was a problem hiding this comment.
P2: The referenced oneOf model's properties are written directly into the parent's already-open JSON object (flattened), followed by the parent's own WriteProperties. If a oneOf child schema shares a JSON property name with a sibling property on the parent (or with the discriminator/additionalProperties), the serialized output will contain duplicate keys for that name. The included Fruit sample avoids this only by luck (child keys kind/count/sweet vs parent key color); a child such as apple defining a color property alongside Fruit's color sibling would produce duplicate "color" entries, and on deserialization the parent's value silently wins because it is written last. Consider guarding against name collisions (e.g. wrapper/discriminator object or a name-overlap check) or at least documenting this flattening limitation.
Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache, line 404:
<comment>The referenced oneOf model's properties are written directly into the parent's already-open JSON object (flattened), followed by the parent's own WriteProperties. If a oneOf child schema shares a JSON property name with a sibling property on the parent (or with the discriminator/additionalProperties), the serialized output will contain duplicate keys for that name. The included Fruit sample avoids this only by luck (child keys kind/count/sweet vs parent key color); a child such as `apple` defining a `color` property alongside Fruit's `color` sibling would produce duplicate "color" entries, and on deserialization the parent's value silently wins because it is written last. Consider guarding against name collisions (e.g. wrapper/discriminator object or a name-overlap check) or at least documenting this flattening limitation.</comment>
<file context>
@@ -376,7 +376,39 @@
+ if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null)
+ {
+ {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = ({{baseType}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}.GetType()));
+ {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}}.WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions);
+ }
+
</file context>
Sorry, something went wrong.
| {{#isPrimitiveType}} | ||
| if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null) | ||
| { | ||
| JsonSerializer.Serialize(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions); |
There was a problem hiding this comment.
P2: For primitive oneOf members the new code serializes the value as a bare JSON primitive and returns before writer.WriteStartObject() is reached. This path is asymmetric with the referenced-object path it sits next to: the object path preserves the parent's sibling properties via WriteProperties, but the primitive path writes only the bare scalar and drops every sibling property (and never emits an object). For a composed model that mixes a primitive oneOf member with sibling properties (e.g. oneOf: [string, Apple] plus a color sibling), serializing when the primitive is selected silently omits the sibling data, so the output won't round-trip. Consider writing the primitive into the object (or documenting that a primitive oneOf cannot be combined with siblings).
Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache, line 386:
<comment>For primitive oneOf members the new code serializes the value as a bare JSON primitive and `return`s before `writer.WriteStartObject()` is reached. This path is asymmetric with the referenced-object path it sits next to: the object path preserves the parent's sibling properties via WriteProperties, but the primitive path writes only the bare scalar and drops every sibling property (and never emits an object). For a composed model that mixes a primitive oneOf member with sibling properties (e.g. `oneOf: [string, Apple]` plus a `color` sibling), serializing when the primitive is selected silently omits the sibling data, so the output won't round-trip. Consider writing the primitive into the object (or documenting that a primitive oneOf cannot be combined with siblings).</comment>
<file context>
@@ -376,7 +376,39 @@
+ {{#isPrimitiveType}}
+ if ({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} != null)
+ {
+ JsonSerializer.Serialize(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions);
+ return;
+ }
</file context>
Sorry, something went wrong.
|
To better showcase that the introduced functionality handles the scenario in the issue I would suggest that the specification is modified/extended to have an inline oneOf. The currently generated code seems to rely a lot on the fact that Fruit binds the oneOfs to each other through the model structure as per components:
schemas:
fruit:
example:
color: color
oneOf:
- $ref: "#/components/schemas/apple"
- $ref: "#/components/schemas/banana"
- $ref: "#/components/schemas/orange"while the example schema provided has it as an inline oneOf for a lone property. I would also argue that it would be beneficial to have a test case for the Union too, since I would not be surprised if there was some Codegen logic that heavily modifies these types of oneOf to make it possible to handle them more easily (from what I know too it is extremely rare for a generator to support it, so it might be that it is more complex than currently thought). The CSharp generator is also currently documented as not supporting Union nor oneOf, should that be updated with this? To my understanding the current oneOf logic existing for the generator is purely designed towards having a clear discriminator, and thus why the feature is not marked as implemented. |
Sorry, something went wrong.
|
I'll review when the samples are up to date. |
Sorry, something went wrong.
|
FYI the issue #24398 And also thanks for working on this! |
Sorry, something went wrong.
|
Thank you wing for building the samples. @madhus1218 Can you make two corrections? Particularly the first of these two issues before we merge. Thanks for the fix — the overall approach of splitting primitive and model oneOf serialization around WriteStartObject() is correct. Two concerns: 1. Converter instantiation is inconsistent with the rest of the template The new model path uses new {{baseType}}JsonConverter(): {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = new {{baseType}}JsonConverter();Every other non-primitive path in this template retrieves the converter from the registered JsonSerializerOptions instead: {{baseType}}JsonConverter {{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}} = ({{baseType}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}.GetType()));In the generichost pattern, converters are registered through DI and may carry constructor dependencies or configured options. Calling new bypasses that — it will produce incorrect behavior or a runtime error for any converter that lacks a parameterless constructor. Please align with the existing pattern. 2. Container oneOf variants are silently skipped The {{^isContainer}} guard correctly excludes arrays/maps from the WriteProperties delegation, but there is no fallback for them. If a oneOf schema includes a container variant (e.g. oneOf: [array<string>, SomeModel]), that variant's value will be dropped during serialization with no error. This is likely an edge case, but it would be good to either handle it or leave a comment acknowledging the limitation. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
What does this PR do?
Fixes C# GenericHost serialization for oneOf schemas that contain referenced model types.
Previously, when a oneOf value was a referenced object, its properties were not written into the serialized JSON output. This update adds handling in JsonConverter.mustache so referenced oneOf models delegate to their generated WriteProperties(...) method while preserving sibling properties on the parent model.
Testing
Added regression coverage for serializing a referenced oneOf model.
Added coverage for a referenced oneOf model with a sibling property.
Regenerated the C# GenericHost OneOf sample.
Verified the generated .NET 8 sample tests:
Verified CSharpClientCodegenTest:
Maven reactor build completed successfully.
Fixes #24398
PR checklist
Summary by cubic
Fixes C# GenericHost JSON serialization for oneOf when the selected type is a referenced model or a primitive. Referenced oneOf objects now write their properties, primitives serialize as raw values, and sibling properties are preserved.
Written for commit e55db64. Summary will update on new commits.