Fix C# GenericHost oneOf serialization for referenced models - #24674
Open
madhus1218 wants to merge 6 commits into
Open
Fix C# GenericHost oneOf serialization for referenced models#24674madhus1218 wants to merge 6 commits into
madhus1218 wants to merge 6 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
|
thanks for the PR cc @devhl-labs |
Contributor
|
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 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 I would also argue that it would be beneficial to have a test case for the The CSharp generator is also currently documented as not supporting |
Contributor
|
I'll review when the samples are up to date. |
|
FYI the issue #24398 And also thanks for working on this! |
Member
|
updated samples in #24735 |
Contributor
|
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 Two concerns: 1. Converter instantiation is inconsistent with the rest of the template The new model path uses {{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 {{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 2. Container The |
3 tasks
Contributor
There was a problem hiding this comment.
1 issue found across 100 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="samples/client/petstore/csharp/generichost/net10/OneOf/src/Org.OpenAPITools/Model/Fruit.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net10/OneOf/src/Org.OpenAPITools/Model/Fruit.cs:246">
P2: When more than one oneOf branch is assigned, these independent `if` statements serialize every branch, producing JSON that violates the oneOf contract and loses data when read back. Serialize only one selected branch or reject instances with multiple branches set.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| appleJsonConverter.WriteProperties(writer, fruit.Apple, jsonSerializerOptions); | ||
| } | ||
|
|
||
| if (fruit.Banana != null) |
Contributor
There was a problem hiding this comment.
P2: When more than one oneOf branch is assigned, these independent if statements serialize every branch, producing JSON that violates the oneOf contract and loses data when read back. Serialize only one selected branch or reject instances with multiple branches set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net10/OneOf/src/Org.OpenAPITools/Model/Fruit.cs, line 246:
<comment>When more than one oneOf branch is assigned, these independent `if` statements serialize every branch, producing JSON that violates the oneOf contract and loses data when read back. Serialize only one selected branch or reject instances with multiple branches set.</comment>
<file context>
@@ -237,6 +237,23 @@ public override Fruit Read(ref Utf8JsonReader utf8JsonReader, Type typeToConvert
+ appleJsonConverter.WriteProperties(writer, fruit.Apple, jsonSerializerOptions);
+ }
+
+ if (fruit.Banana != null)
+ {
+ BananaJsonConverter bananaJsonConverter = (BananaJsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert(fruit.Banana.GetType()));
</file context>
Contributor
There was a problem hiding this comment.
20 issues found across 26 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="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs:158">
P1: When a scalar `PolymorphicProperty` is nested in another JSON value, the final loop advances the original reader beyond the value because the scalar candidates used copied readers. Skip this container-scanning loop for scalar inputs so the converter returns with the reader on the scalar token.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs:174">
P1: When a primitive `oneOf` is nested in an object or array, this branch leaves `utf8JsonReader` on the scalar, then the unconditional scan below advances into the containing value. It can consume sibling properties before the parent converter reads them; return after selecting the scalar candidate or restrict that scan to object/array inputs.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/FruitReq.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/FruitReq.cs:119">
P2: When a scalar value is supplied for this model-only `oneOf`, both object deserializations fail and the final reader loop scans unrelated sibling JSON before throwing. Restrict this branch to `oneOf` schemas with a primitive alternative, or retain the initial object/array guard for this model.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs:174">
P1: When `MixedOneOfContent` contains a primitive and is nested in another object, this branch leaves the original reader on the primitive, but the cleanup loop then reads through the containing object. Return as soon as a primitive candidate succeeds, or skip that cleanup loop for non-container tokens, so the caller can process following properties.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs:158">
P1: When a scalar `PolymorphicProperty` is nested in another JSON value, this branch consumes the enclosing object or array before returning. Return the matched scalar from this branch, or run the shared scan only for container start tokens.</violation>
<violation number="2" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs:167">
P2: For a JSON number, `TryDeserialize<Object>` succeeds with a `JsonElement`, so `Read` returns an object variant even though the schema has no numeric `oneOf` branch. Do not attempt the object candidate in the scalar-token branch.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/FruitReq.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/FruitReq.cs:119">
P1: When this converter is generated for a scalar-capable oneOf, the new branch leaves `utf8JsonReader` on the scalar, but the trailing loop still consumes the parent’s remaining properties. Skip that object/array traversal for scalar tokens so nested deserialization does not lose siblings.</violation>
</file>
<file name="modules/openapi-generator/src/test/resources/3_0/oneOf.yaml">
<violation number="1" location="modules/openapi-generator/src/test/resources/3_0/oneOf.yaml:30">
P3: The two newly added schemas `inlineOneOfModel` and `union` are not referenced by any test. A repository-wide search shows the only consumers of this shared fixture (`DefaultCodegenTest`, `RubyClientCodegenTest`, `JavaClientCodegenTest`, `ProtobufSchemaCodegenTest`, `Swift6ClientCodegenTest`) all target only the pre-existing `fruit` schema, and no test or sample references `inlineOneOfModel` or `union`. They therefore add no regression coverage (the PR's referenced-model tests use `fruit`) and leave dead content in a shared test resource. Add a test that exercises these schemas, or drop them.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Mammal.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Mammal.cs:146">
P2: The regenerated Mammal.cs dropped the Read() token-type guard, but the current JsonConverter.mustache still emits it for oneOf models with no primitive members (Mammal's Pig/Whale/Zebra are all references, so oneOfHasPrimitiveType is false), and the sibling Animal.cs (lines 134-135) still has it. The sample no longer matches the template. Regenerate it; removing the guard also changes malformed-input handling from an early JsonException to a late ArgumentException("Property is required for class Mammal.").</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Shape.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Shape.cs:131">
P2: The removed token-type guard makes this regenerated sample inconsistent with JsonConverter.mustache, which still emits it for oneOf models with non-primitive types (Shape's oneOf is Triangle/Quadrilateral, so oneOfHasPrimitiveType is false and the guard is regenerated). Regenerating this sample would restore the guard, and without it the discriminator/main read loops never hit their StartObject/StartArray break conditions on malformed input, so they consume all remaining tokens instead of throwing a JsonException. Restore the guard here (or update the template to remove it for this case if the removal is intentional).</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Mammal.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Mammal.cs:146">
P2: This guard should not be removed for Mammal. Its oneOf members (whale, zebra, Pig) are all referenced model types, so JsonConverter.mustache computes oneOfHasPrimitiveType = false and still emits the token-type guard for model-only oneOf schemas. Without it, deserializing a non-object/array value for a model-only oneOf no longer throws the intended JsonException at the entry; it falls through the loops and fails later with a misleading 'Property is required for class Mammal' ArgumentException. Regenerate this sample from the updated template rather than deleting the guard.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Triangle.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Triangle.cs:146">
P2: Triangle.cs is now out of sync with JsonConverter.mustache. The template emits `bool oneOfHasPrimitiveType = false;` and `if (!oneOfHasPrimitiveType && utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray) throw new JsonException();` for every oneOf model, and Triangle's oneOf contains only model types, so regenerating this sample reintroduces the guard you just deleted. Regenerate the sample so the checked-in file matches the generator output (the maintainers already gate review on samples being up to date).</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Quadrilateral.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Quadrilateral.cs:131">
P2: This sample no longer matches the JsonConverter.mustache template, which still emits the StartObject/StartArray guard, and the sibling Animal.cs (same FormModels sample, same discriminated oneOf) still has it. Removing the guard also changes error behavior: scalar/non-object input no longer throws JsonException up front; with a scalar starting token the discriminator and main loops never hit their EndObject/EndArray break, so they run to exhaustion and the code falls through to a misleading `ArgumentException("Property is required for class Quadrilateral.")`. Either regenerate the sample from the current template so the guard is restored, or, if the guard removal is intentional, apply the same removal to the template so the generated code stays consistent.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/OneOfString.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/OneOfString.cs:106">
P2: After removing the StartObject/StartArray guard, Read() still has no way to parse the primitive value that Write() now emits (Write serializes a bare string for the oneOf string case). varString is never assigned, so Read() always throws JsonException, and when a primitive token appears nested inside a larger object the loop break conditions never match and Read() consumes the remainder of the document. JsonConverter.mustache already emits a oneOfHasPrimitiveType + TryDeserialize branch for this model; regenerate this sample so Read() can deserialize what Write() produces.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Shape.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Shape.cs:131">
P2: The removed guard does not match the current template: JsonConverter.mustache still emits the StartObject/StartArray check for Shape (a discriminator model with no composedSchemas.oneOf, `{{^composedSchemas.oneOf}}` branch), so regenerating with the restored template would re-add it. This leaves the regenerated OneOf sample inconsistent with the template and with its siblings — net9/Petstore, net9/NullReferenceTypes, and net10/net4.7 FormModels Shape.cs all retain the guard, while only net8/net9 FormModels dropped it. Re-add the guard (or regenerate the FormModels samples from the restored template). Without it, a non-object/array JSON value for Shape no longer throws a clean JsonException up front and instead falls through to an ArgumentException for the missing required `shapeType`.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Pig.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/Pig.cs:131">
P2: This removes the token-type guard from Pig's Read, but the current JsonConverter.mustache still emits that guard for any oneOf without a primitive type (Pig's oneOf is only BasquePig/DanishPig, so oneOfHasPrimitiveType stays false). The regenerated sample would put the guard back, and the sibling sample MixedOneOf.cs:116 still has it, so this hand-edit leaves the generated sample out of sync with the template and would be undone (or fail the sample-up-to-date check) on the next regeneration. Regenerate the C# GenericHost samples from the current template instead of editing Pig.cs directly.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Quadrilateral.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Quadrilateral.cs:131">
P2: The net8 sample's Read() guard removal does not match the JsonConverter.mustache template or the other framework samples. For a oneOf whose members are all models (Quadrilateral: SimpleQuadrilateral + ComplexQuadrilateral), oneOfHasPrimitiveType is false, so the template still emits `if (!oneOfHasPrimitiveType && TokenType != StartObject && TokenType != StartArray) throw new JsonException();`. net10, net4.8, and net4.7 all retain this guard; only net8/net9 dropped it. Regenerating net8 from the current template re-adds the guard, so this sample does not reflect the generated output. Re-add the guard (or regenerate the sample) so the sample is consistent with the template; without it, deserializing a non-object/array value now scans the entire stream and throws ArgumentException instead of JsonException.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/OneOfString.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/OneOfString.cs:106">
P2: Removing the token-type guard here is ineffective because the corresponding primitive-token handling is missing. OneOfString.Write emits a bare JSON string when String is set, but OneOfString.Read has no path to read one: the while loop only breaks on EndObject/EndArray, varString is never assigned from a primitive token, and the method always throws JsonException. The JsonConverter.mustache template pairs this guard removal with a ClientUtils.TryDeserialize branch for non-object/array tokens (see the regenerated PolymorphicProperty.cs, lines 151-169); this file only has the removal, so it is stale relative to the current template. Regenerate this sample from the current template so the primitive oneOf can round-trip.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/NullableShape.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/NullableShape.cs:131">
P2: The generated Read() no longer guards against non-object/non-array input, but JsonConverter.mustache still emits that guard for oneOf schemas whose members are all models (the `!oneOfHasPrimitiveType` branch). NullableShape's oneOf members (Triangle, Quadrilateral) are both models, so the template would still produce the guard and the `oneOfHasPrimitiveType` declaration, which this sample no longer contains. As a result, a malformed primitive token now falls through to `throw new ArgumentException("Property is required for class NullableShape.")` instead of a `JsonException`, and the sample is out of sync with the restored template. Regenerate this sample from the current template so the model-only oneOf keeps its input guard.</violation>
</file>
<file name="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Pig.cs">
<violation number="1" location="samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Pig.cs:131">
P2: Removing the StartObject/StartArray guard makes this generated sample diverge from the template that produces it. JsonConverter.mustache still emits `if (utf8JsonReader.TokenType != JsonTokenType.StartObject && ...) throw new JsonException();` for every `{{^composedSchemas.oneOf}}` model (line 62), and the identical Pig model in the sibling sample net8/Petstore/.../Pig.cs:131 still contains the guard. Since Pig is not a `composedSchemas.oneOf` model, regenerating this sample from the committed template would restore the removed check, so this edit has no lasting effect and the FormModels sample is out of sync with the template. Restore the guard to match the template, or if the guard should be dropped for referenced oneOf models, change the template and regenerate all samples.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader; | ||
| while (utf8JsonReaderOneOf.Read()) | ||
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P1: When a scalar PolymorphicProperty is nested in another JSON value, the final loop advances the original reader beyond the value because the scalar candidates used copied readers. Skip this container-scanning loop for scalar inputs so the converter returns with the reader on the scalar token.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs, line 158:
<comment>When a scalar `PolymorphicProperty` is nested in another JSON value, the final loop advances the original reader beyond the value because the scalar candidates used copied readers. Skip this container-scanning loop for scalar inputs so the converter returns with the reader on the scalar token.</comment>
<file context>
@@ -148,38 +148,52 @@ public override PolymorphicProperty Read(ref Utf8JsonReader utf8JsonReader, Type
- Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader;
- while (utf8JsonReaderOneOf.Read())
+ if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray)
{
- if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth)
</file context>
|
|
||
| Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader; | ||
| while (utf8JsonReaderOneOf.Read()) | ||
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P1: When a primitive oneOf is nested in an object or array, this branch leaves utf8JsonReader on the scalar, then the unconditional scan below advances into the containing value. It can consume sibling properties before the parent converter reads them; return after selecting the scalar candidate or restrict that scan to object/array inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs, line 174:
<comment>When a primitive `oneOf` is nested in an object or array, this branch leaves `utf8JsonReader` on the scalar, then the unconditional scan below advances into the containing value. It can consume sibling properties before the parent converter reads them; return after selecting the scalar candidate or restrict that scan to object/array inputs.</comment>
<file context>
@@ -174,31 +171,51 @@ public override MixedOneOfContent Read(ref Utf8JsonReader utf8JsonReader, Type t
- Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader;
- while (utf8JsonReaderOneOf.Read())
+ if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray)
{
- if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth)
</file context>
Comment on lines
+174
to
+190
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) | ||
| { | ||
| if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth) | ||
| break; | ||
| Utf8JsonReader utf8JsonReaderString = utf8JsonReader; | ||
| ClientUtils.TryDeserialize<string>(ref utf8JsonReaderString, jsonSerializerOptions, out varString); | ||
|
|
||
| if (startingTokenType == JsonTokenType.StartArray && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndArray && currentDepth == utf8JsonReaderOneOf.CurrentDepth) | ||
| break; | ||
| Utf8JsonReader utf8JsonReaderBool = utf8JsonReader; | ||
| ClientUtils.TryDeserialize<bool?>(ref utf8JsonReaderBool, jsonSerializerOptions, out varBool); | ||
|
|
||
| Utf8JsonReader utf8JsonReaderInt = utf8JsonReader; | ||
| ClientUtils.TryDeserialize<int?>(ref utf8JsonReaderInt, jsonSerializerOptions, out varInt); | ||
|
|
||
| if (utf8JsonReaderOneOf.TokenType == JsonTokenType.PropertyName && currentDepth == utf8JsonReaderOneOf.CurrentDepth - 1) | ||
| Utf8JsonReader utf8JsonReaderDecimal = utf8JsonReader; | ||
| ClientUtils.TryDeserialize<decimal?>(ref utf8JsonReaderDecimal, jsonSerializerOptions, out varDecimal); | ||
|
|
||
| Utf8JsonReader utf8JsonReaderMixedSubId = utf8JsonReader; | ||
| ClientUtils.TryDeserialize<MixedSubId>(ref utf8JsonReaderMixedSubId, jsonSerializerOptions, out mixedSubId); | ||
| } |
Contributor
There was a problem hiding this comment.
P1: When MixedOneOfContent contains a primitive and is nested in another object, this branch leaves the original reader on the primitive, but the cleanup loop then reads through the containing object. Return as soon as a primitive candidate succeeds, or skip that cleanup loop for non-container tokens, so the caller can process following properties.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/MixedOneOfContent.cs, line 174:
<comment>When `MixedOneOfContent` contains a primitive and is nested in another object, this branch leaves the original reader on the primitive, but the cleanup loop then reads through the containing object. Return as soon as a primitive candidate succeeds, or skip that cleanup loop for non-container tokens, so the caller can process following properties.</comment>
<file context>
@@ -174,31 +171,51 @@ public override MixedOneOfContent Read(ref Utf8JsonReader utf8JsonReader, Type t
- Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader;
- while (utf8JsonReaderOneOf.Read())
+ if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray)
{
- if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth)
</file context>
Suggested change
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) | |
| { | |
| if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth) | |
| break; | |
| Utf8JsonReader utf8JsonReaderString = utf8JsonReader; | |
| ClientUtils.TryDeserialize<string>(ref utf8JsonReaderString, jsonSerializerOptions, out varString); | |
| if (startingTokenType == JsonTokenType.StartArray && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndArray && currentDepth == utf8JsonReaderOneOf.CurrentDepth) | |
| break; | |
| Utf8JsonReader utf8JsonReaderBool = utf8JsonReader; | |
| ClientUtils.TryDeserialize<bool?>(ref utf8JsonReaderBool, jsonSerializerOptions, out varBool); | |
| Utf8JsonReader utf8JsonReaderInt = utf8JsonReader; | |
| ClientUtils.TryDeserialize<int?>(ref utf8JsonReaderInt, jsonSerializerOptions, out varInt); | |
| if (utf8JsonReaderOneOf.TokenType == JsonTokenType.PropertyName && currentDepth == utf8JsonReaderOneOf.CurrentDepth - 1) | |
| Utf8JsonReader utf8JsonReaderDecimal = utf8JsonReader; | |
| ClientUtils.TryDeserialize<decimal?>(ref utf8JsonReaderDecimal, jsonSerializerOptions, out varDecimal); | |
| Utf8JsonReader utf8JsonReaderMixedSubId = utf8JsonReader; | |
| ClientUtils.TryDeserialize<MixedSubId>(ref utf8JsonReaderMixedSubId, jsonSerializerOptions, out mixedSubId); | |
| } | |
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) | |
| { | |
| Utf8JsonReader utf8JsonReaderString = utf8JsonReader; | |
| if (ClientUtils.TryDeserialize<string>(ref utf8JsonReaderString, jsonSerializerOptions, out varString)) | |
| return new MixedOneOfContent(varString); | |
| Utf8JsonReader utf8JsonReaderBool = utf8JsonReader; | |
| if (ClientUtils.TryDeserialize<bool?>(ref utf8JsonReaderBool, jsonSerializerOptions, out varBool)) | |
| return new MixedOneOfContent(varBool.Value); | |
| Utf8JsonReader utf8JsonReaderInt = utf8JsonReader; | |
| if (ClientUtils.TryDeserialize<int?>(ref utf8JsonReaderInt, jsonSerializerOptions, out varInt)) | |
| return new MixedOneOfContent(varInt.Value); | |
| Utf8JsonReader utf8JsonReaderDecimal = utf8JsonReader; | |
| if (ClientUtils.TryDeserialize<decimal?>(ref utf8JsonReaderDecimal, jsonSerializerOptions, out varDecimal)) | |
| return new MixedOneOfContent(varDecimal.Value); | |
| Utf8JsonReader utf8JsonReaderMixedSubId = utf8JsonReader; | |
| if (ClientUtils.TryDeserialize<MixedSubId>(ref utf8JsonReaderMixedSubId, jsonSerializerOptions, out mixedSubId)) | |
| return new MixedOneOfContent(mixedSubId); | |
| throw new JsonException(); | |
| } |
|
|
||
| Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader; | ||
| while (utf8JsonReaderOneOf.Read()) | ||
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P1: When a scalar PolymorphicProperty is nested in another JSON value, this branch consumes the enclosing object or array before returning. Return the matched scalar from this branch, or run the shared scan only for container start tokens.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/PolymorphicProperty.cs, line 158:
<comment>When a scalar `PolymorphicProperty` is nested in another JSON value, this branch consumes the enclosing object or array before returning. Return the matched scalar from this branch, or run the shared scan only for container start tokens.</comment>
<file context>
@@ -148,38 +148,52 @@ public override PolymorphicProperty Read(ref Utf8JsonReader utf8JsonReader, Type
- Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader;
- while (utf8JsonReaderOneOf.Read())
+ if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray)
{
- if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth)
</file context>
|
|
||
| Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader; | ||
| while (utf8JsonReaderOneOf.Read()) | ||
| if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P1: When this converter is generated for a scalar-capable oneOf, the new branch leaves utf8JsonReader on the scalar, but the trailing loop still consumes the parent’s remaining properties. Skip that object/array traversal for scalar tokens so nested deserialization does not lose siblings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/FruitReq.cs, line 119:
<comment>When this converter is generated for a scalar-capable oneOf, the new branch leaves `utf8JsonReader` on the scalar, but the trailing loop still consumes the parent’s remaining properties. Skip that object/array traversal for scalar tokens so nested deserialization does not lose siblings.</comment>
<file context>
@@ -111,30 +111,38 @@ public override FruitReq Read(ref Utf8JsonReader utf8JsonReader, Type typeToConv
- Utf8JsonReader utf8JsonReaderOneOf = utf8JsonReader;
- while (utf8JsonReaderOneOf.Read())
+ if (startingTokenType != JsonTokenType.StartObject && startingTokenType != JsonTokenType.StartArray)
{
- if (startingTokenType == JsonTokenType.StartObject && utf8JsonReaderOneOf.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReaderOneOf.CurrentDepth)
</file context>
| { | ||
| int currentDepth = utf8JsonReader.CurrentDepth; | ||
|
|
||
| if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P2: The net8 sample's Read() guard removal does not match the JsonConverter.mustache template or the other framework samples. For a oneOf whose members are all models (Quadrilateral: SimpleQuadrilateral + ComplexQuadrilateral), oneOfHasPrimitiveType is false, so the template still emits if (!oneOfHasPrimitiveType && TokenType != StartObject && TokenType != StartArray) throw new JsonException();. net10, net4.8, and net4.7 all retain this guard; only net8/net9 dropped it. Regenerating net8 from the current template re-adds the guard, so this sample does not reflect the generated output. Re-add the guard (or regenerate the sample) so the sample is consistent with the template; without it, deserializing a non-object/array value now scans the entire stream and throws ArgumentException instead of JsonException.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Quadrilateral.cs, line 131:
<comment>The net8 sample's Read() guard removal does not match the JsonConverter.mustache template or the other framework samples. For a oneOf whose members are all models (Quadrilateral: SimpleQuadrilateral + ComplexQuadrilateral), oneOfHasPrimitiveType is false, so the template still emits `if (!oneOfHasPrimitiveType && TokenType != StartObject && TokenType != StartArray) throw new JsonException();`. net10, net4.8, and net4.7 all retain this guard; only net8/net9 dropped it. Regenerating net8 from the current template re-adds the guard, so this sample does not reflect the generated output. Re-add the guard (or regenerate the sample) so the sample is consistent with the template; without it, deserializing a non-object/array value now scans the entire stream and throws ArgumentException instead of JsonException.</comment>
<file context>
@@ -128,9 +128,6 @@ public override Quadrilateral Read(ref Utf8JsonReader utf8JsonReader, Type typeT
- if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray)
- throw new JsonException();
-
JsonTokenType startingTokenType = utf8JsonReader.TokenType;
Option<string> quadrilateralType = default;
</file context>
| { | ||
| int currentDepth = utf8JsonReader.CurrentDepth; | ||
|
|
||
| if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P2: Removing the token-type guard here is ineffective because the corresponding primitive-token handling is missing. OneOfString.Write emits a bare JSON string when String is set, but OneOfString.Read has no path to read one: the while loop only breaks on EndObject/EndArray, varString is never assigned from a primitive token, and the method always throws JsonException. The JsonConverter.mustache template pairs this guard removal with a ClientUtils.TryDeserialize branch for non-object/array tokens (see the regenerated PolymorphicProperty.cs, lines 151-169); this file only has the removal, so it is stale relative to the current template. Regenerate this sample from the current template so the primitive oneOf can round-trip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net9/FormModels/src/Org.OpenAPITools/Model/OneOfString.cs, line 106:
<comment>Removing the token-type guard here is ineffective because the corresponding primitive-token handling is missing. OneOfString.Write emits a bare JSON string when String is set, but OneOfString.Read has no path to read one: the while loop only breaks on EndObject/EndArray, varString is never assigned from a primitive token, and the method always throws JsonException. The JsonConverter.mustache template pairs this guard removal with a ClientUtils.TryDeserialize branch for non-object/array tokens (see the regenerated PolymorphicProperty.cs, lines 151-169); this file only has the removal, so it is stale relative to the current template. Regenerate this sample from the current template so the primitive oneOf can round-trip.</comment>
<file context>
@@ -103,9 +103,6 @@ public override OneOfString Read(ref Utf8JsonReader utf8JsonReader, Type typeToC
- if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray)
- throw new JsonException();
-
JsonTokenType startingTokenType = utf8JsonReader.TokenType;
string varString = default;
</file context>
| { | ||
| int currentDepth = utf8JsonReader.CurrentDepth; | ||
|
|
||
| if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P2: The generated Read() no longer guards against non-object/non-array input, but JsonConverter.mustache still emits that guard for oneOf schemas whose members are all models (the !oneOfHasPrimitiveType branch). NullableShape's oneOf members (Triangle, Quadrilateral) are both models, so the template would still produce the guard and the oneOfHasPrimitiveType declaration, which this sample no longer contains. As a result, a malformed primitive token now falls through to throw new ArgumentException("Property is required for class NullableShape.") instead of a JsonException, and the sample is out of sync with the restored template. Regenerate this sample from the current template so the model-only oneOf keeps its input guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/NullableShape.cs, line 131:
<comment>The generated Read() no longer guards against non-object/non-array input, but JsonConverter.mustache still emits that guard for oneOf schemas whose members are all models (the `!oneOfHasPrimitiveType` branch). NullableShape's oneOf members (Triangle, Quadrilateral) are both models, so the template would still produce the guard and the `oneOfHasPrimitiveType` declaration, which this sample no longer contains. As a result, a malformed primitive token now falls through to `throw new ArgumentException("Property is required for class NullableShape.")` instead of a `JsonException`, and the sample is out of sync with the restored template. Regenerate this sample from the current template so the model-only oneOf keeps its input guard.</comment>
<file context>
@@ -128,9 +128,6 @@ public override NullableShape Read(ref Utf8JsonReader utf8JsonReader, Type typeT
- if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray)
- throw new JsonException();
-
JsonTokenType startingTokenType = utf8JsonReader.TokenType;
Option<string> shapeType = default;
</file context>
| { | ||
| int currentDepth = utf8JsonReader.CurrentDepth; | ||
|
|
||
| if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray) |
Contributor
There was a problem hiding this comment.
P2: Removing the StartObject/StartArray guard makes this generated sample diverge from the template that produces it. JsonConverter.mustache still emits if (utf8JsonReader.TokenType != JsonTokenType.StartObject && ...) throw new JsonException(); for every {{^composedSchemas.oneOf}} model (line 62), and the identical Pig model in the sibling sample net8/Petstore/.../Pig.cs:131 still contains the guard. Since Pig is not a composedSchemas.oneOf model, regenerating this sample from the committed template would restore the removed check, so this edit has no lasting effect and the FormModels sample is out of sync with the template. Restore the guard to match the template, or if the guard should be dropped for referenced oneOf models, change the template and regenerate all samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/net8/FormModels/src/Org.OpenAPITools/Model/Pig.cs, line 131:
<comment>Removing the StartObject/StartArray guard makes this generated sample diverge from the template that produces it. JsonConverter.mustache still emits `if (utf8JsonReader.TokenType != JsonTokenType.StartObject && ...) throw new JsonException();` for every `{{^composedSchemas.oneOf}}` model (line 62), and the identical Pig model in the sibling sample net8/Petstore/.../Pig.cs:131 still contains the guard. Since Pig is not a `composedSchemas.oneOf` model, regenerating this sample from the committed template would restore the removed check, so this edit has no lasting effect and the FormModels sample is out of sync with the template. Restore the guard to match the template, or if the guard should be dropped for referenced oneOf models, change the template and regenerate all samples.</comment>
<file context>
@@ -128,9 +128,6 @@ public override Pig Read(ref Utf8JsonReader utf8JsonReader, Type typeToConvert,
- if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray)
- throw new JsonException();
-
JsonTokenType startingTokenType = utf8JsonReader.TokenType;
Option<string> className = default;
</file context>
| - $ref: '#/components/schemas/banana' | ||
| - $ref: '#/components/schemas/orange' | ||
|
|
||
| inlineOneOfModel: |
Contributor
There was a problem hiding this comment.
P3: The two newly added schemas inlineOneOfModel and union are not referenced by any test. A repository-wide search shows the only consumers of this shared fixture (DefaultCodegenTest, RubyClientCodegenTest, JavaClientCodegenTest, ProtobufSchemaCodegenTest, Swift6ClientCodegenTest) all target only the pre-existing fruit schema, and no test or sample references inlineOneOfModel or union. They therefore add no regression coverage (the PR's referenced-model tests use fruit) and leave dead content in a shared test resource. Add a test that exercises these schemas, or drop them.
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/resources/3_0/oneOf.yaml, line 30:
<comment>The two newly added schemas `inlineOneOfModel` and `union` are not referenced by any test. A repository-wide search shows the only consumers of this shared fixture (`DefaultCodegenTest`, `RubyClientCodegenTest`, `JavaClientCodegenTest`, `ProtobufSchemaCodegenTest`, `Swift6ClientCodegenTest`) all target only the pre-existing `fruit` schema, and no test or sample references `inlineOneOfModel` or `union`. They therefore add no regression coverage (the PR's referenced-model tests use `fruit`) and leave dead content in a shared test resource. Add a test that exercises these schemas, or drop them.</comment>
<file context>
@@ -1,46 +1,68 @@
+ - $ref: '#/components/schemas/banana'
+ - $ref: '#/components/schemas/orange'
+
+ inlineOneOfModel:
+ title: inlineOneOfModel
+ type: object
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes C# GenericHost serialization for
oneOfschemas that contain referenced model types.Previously, when a
oneOfvalue was a referenced object, its properties were not written into the serialized JSON output. This update adds handling inJsonConverter.mustacheso referencedoneOfmodels delegate to their generatedWriteProperties(...)method while preserving sibling properties on the parent model.Testing
Added regression coverage for serializing a referenced
oneOfmodel.Added coverage for a referenced
oneOfmodel with a sibling property.Regenerated the C# GenericHost
OneOfsample.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 handling for
oneOfwhen the selected type is a referenced model, a primitive, or a container. Serialized referencedoneOfobjects now write their properties, primitives and containers serialize as raw values, and sibling properties are preserved. Deserialization now accepts primitiveoneOfvalues instead of throwing for non-object or array input.oneOfmodels to theirWriteProperties(...)method.oneOfvalues without an object wrapper.oneOfvalues directly instead of rejecting non-object or array input.Written for commit f4118dc. Summary will update on new commits.