Skip to content

Commit aa783ed

Browse files
wiebrenclaude
wiebren
and
claude
authored
[csharp] fix: conditional serialization invents value-type defaults on all-optional models (#24877)
* fix: [csharp] conditionalSerialization must not flag value-typed properties the caller never set With conditionalSerialization=true, an all-optional model's public constructor took every value-typed property as a plain value type and then guarded the serialization flag with `if (this.AutoRenew != null)` — always true for a non-nullable bool/int/enum, so every value-typed property was marked as set the moment the model was constructed. A partial-update command built as `new UpdateThingCommand(note: "renamed")` went on the wire as {"autoRenew":false,"note":"renamed"}: an invented false on the exact operation where it does the most damage. (The C# compiler flags the old guard itself: CS0472 'the result of the expression is always true'.) Optional value-typed constructor parameters without a default value are now nullable, and the flag is only raised when the argument was actually provided: if (autoRenew != null) { this._AutoRenew = autoRenew.Value; this._flagAutoRenew = true; } Required properties, properties with a spec default, reference types, and the public property surface (plain value-typed properties with flag-raising setters) are all generated byte-for-byte as before; only the optional value-type constructor path changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxDNCjqJycKTfzVWg2SeTJ * test: [csharp] cover optional enums and nullable value types in the conditionalSerialization constructor Extends the conditional-serialization-value-types fixture with an optional inline enum, a nullable enum and a nullable integer. The test now asserts that enums take the guarded `.Value` path, that `nullable: true` value types keep their existing null check, and that no parameter type is emitted as `T??`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: [csharp] drop conditionalSerialization assertions the remaining ones already imply Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 8300f48 commit aa783ed

30 files changed

Lines changed: 309 additions & 165 deletions

‎modules/openapi-generator/src/main/resources/csharp/modelGeneric.mustache‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@
140140
{{#hasOnlyReadOnly}}
141141
[JsonConstructorAttribute]
142142
{{/hasOnlyReadOnly}}
143-
public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}} {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} = {{#defaultValue}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{defaultValue}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/defaultValue}}{{^defaultValue}}default{{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^-last}}, {{/-last}}{{/parentVars}}){{/parent}}
143+
public {{classname}}({{#readWriteVars}}{{{datatypeWithEnum}}}{{#isEnum}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}}{{/isEnum}}{{^isEnum}}{{#conditionalSerialization}}{{#vendorExtensions.x-csharp-value-type}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{/vendorExtensions.x-csharp-value-type}}{{/conditionalSerialization}}{{/isEnum}} {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} = {{#defaultValue}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{defaultValue}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/defaultValue}}{{^defaultValue}}default{{/defaultValue}}{{^-last}}, {{/-last}}{{/readWriteVars}}){{#parent}} : base({{#parentVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^-last}}, {{/-last}}{{/parentVars}}){{/parent}}
144144
{
145145
{{#vars}}
146146
{{^isInherited}}
@@ -196,11 +196,20 @@
196196
this.{{name}} = {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}};
197197
{{/conditionalSerialization}}
198198
{{#conditionalSerialization}}
199+
{{#vendorExtensions.x-csharp-value-type}}
200+
if ({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} != null)
201+
{
202+
this._{{name}} = {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.Value;
203+
this._flag{{name}} = true;
204+
}
205+
{{/vendorExtensions.x-csharp-value-type}}
206+
{{^vendorExtensions.x-csharp-value-type}}
199207
this._{{name}} = {{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}};
200208
if (this.{{name}} != null)
201209
{
202210
this._flag{{name}} = true;
203211
}
212+
{{/vendorExtensions.x-csharp-value-type}}
204213
{{/conditionalSerialization}}
205214
{{/defaultValue}}
206215
{{/required}}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/csharpnetcore/CSharpClientCodegenTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,78 @@ public void testMapResponse() throws Exception {
605605
Assert.assertTrue(cr1.isMap);
606606
}
607607

608+
@Test(description = "conditionalSerialization: an all-optional model's public constructor must not raise the serialization flag for value-typed properties the caller never passed")
609+
public void testConditionalSerializationValueTypeConstructorFlags() throws IOException {
610+
Map<String, File> files = generateConditionalSerializationValueTypeModels(true);
611+
612+
File updateCommand = files.get("UpdateThingCommand.cs");
613+
assertNotNull(updateCommand);
614+
// value-typed parameters become nullable so "not passed" is representable ...
615+
assertFileContains(updateCommand.toPath(),
616+
"public UpdateThingCommand(bool? autoRenew = default, int? period = default, int? graceDays = default, StatusEnum? status = default, RenewalModeEnum? renewalMode = default, string note = default)");
617+
// ... and the flag is only raised for arguments that were actually provided
618+
assertFileContains(updateCommand.toPath(),
619+
"if (autoRenew != null)\n" +
620+
" {\n" +
621+
" this._AutoRenew = autoRenew.Value;\n" +
622+
" this._flagAutoRenew = true;\n" +
623+
" }");
624+
// optional enums take the same guarded path
625+
assertFileContains(updateCommand.toPath(),
626+
"if (status != null)\n" +
627+
" {\n" +
628+
" this._Status = status.Value;\n" +
629+
" this._flagStatus = true;\n" +
630+
" }");
631+
// a nullable value type already had a meaningful null check and keeps it
632+
assertFileContains(updateCommand.toPath(),
633+
"this._GraceDays = graceDays;\n" +
634+
" if (this.GraceDays != null)\n" +
635+
" {\n" +
636+
" this._flagGraceDays = true;\n" +
637+
" }");
638+
// the public property surface is untouched: plain value types, flag-raising setters
639+
assertFileContains(updateCommand.toPath(), "public bool AutoRenew", "public int Period");
640+
assertFileNotContains(updateCommand.toPath(), "public bool? AutoRenew", "public int? Period");
641+
642+
// required value-typed properties keep their non-nullable parameter and unconditional assignment
643+
File createCommand = files.get("CreateThingCommand.cs");
644+
assertNotNull(createCommand);
645+
assertFileContains(createCommand.toPath(),
646+
"public CreateThingCommand(string name = default, bool autoRenew = default, string note = default)");
647+
assertFileContains(createCommand.toPath(), "this._AutoRenew = autoRenew;");
648+
assertFileNotContains(createCommand.toPath(), "autoRenew.Value");
649+
}
650+
651+
@Test(description = "without conditionalSerialization the constructor keeps plain value-typed parameters")
652+
public void testValueTypeConstructorUnchangedWithoutConditionalSerialization() throws IOException {
653+
Map<String, File> files = generateConditionalSerializationValueTypeModels(false);
654+
655+
File updateCommand = files.get("UpdateThingCommand.cs");
656+
assertNotNull(updateCommand);
657+
assertFileContains(updateCommand.toPath(),
658+
"public UpdateThingCommand(bool autoRenew = default, int period = default, int? graceDays = default, StatusEnum? status = default, RenewalModeEnum? renewalMode = default, string note = default)");
659+
assertFileNotContains(updateCommand.toPath(), "_flagAutoRenew");
660+
}
661+
662+
private Map<String, File> generateConditionalSerializationValueTypeModels(boolean conditionalSerialization) throws IOException {
663+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
664+
output.deleteOnExit();
665+
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/csharp/conditional-serialization-value-types.yaml");
666+
final DefaultGenerator defaultGenerator = new DefaultGenerator();
667+
final ClientOptInput clientOptInput = new ClientOptInput();
668+
clientOptInput.openAPI(openAPI);
669+
CSharpClientCodegen cSharpClientCodegen = new CSharpClientCodegen();
670+
cSharpClientCodegen.setLibrary("httpclient");
671+
cSharpClientCodegen.setOutputDir(output.getAbsolutePath());
672+
cSharpClientCodegen.additionalProperties().put(CodegenConstants.OPTIONAL_CONDITIONAL_SERIALIZATION, String.valueOf(conditionalSerialization));
673+
clientOptInput.config(cSharpClientCodegen);
674+
defaultGenerator.opts(clientOptInput);
675+
676+
return defaultGenerator.generate().stream()
677+
.collect(Collectors.toMap(File::getName, Function.identity(), (first, second) -> first));
678+
}
679+
608680
private Map<String, File> generateIssue23046Models(boolean nonPublicApi) throws IOException {
609681
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
610682
output.deleteOnExit();

‎modules/openapi-generator/src/test/resources/3_0/csharp/conditional-serialization-value-types.yaml‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
openapi: 3.0.3
2+
info:
3+
title: conditional serialization value-type flags
4+
version: 1.0.0
5+
paths:
6+
/things:
7+
post:
8+
operationId: createThing
9+
requestBody:
10+
required: true
11+
content:
12+
application/json:
13+
schema:
14+
$ref: '#/components/schemas/CreateThingCommand'
15+
responses:
16+
'200':
17+
description: created
18+
patch:
19+
operationId: updateThing
20+
requestBody:
21+
required: true
22+
content:
23+
application/json:
24+
schema:
25+
$ref: '#/components/schemas/UpdateThingCommand'
26+
responses:
27+
'200':
28+
description: updated
29+
components:
30+
schemas:
31+
UpdateThingCommand:
32+
type: object
33+
description: an all-optional partial-update command; its only constructor is the public one
34+
properties:
35+
autoRenew:
36+
type: boolean
37+
period:
38+
type: integer
39+
graceDays:
40+
type: integer
41+
nullable: true
42+
status:
43+
type: string
44+
enum: [active, suspended]
45+
renewalMode:
46+
type: string
47+
enum: [manual, automatic]
48+
nullable: true
49+
note:
50+
type: string
51+
CreateThingCommand:
52+
type: object
53+
description: required value type keeps its non-nullable constructor parameter
54+
required:
55+
- name
56+
- autoRenew
57+
properties:
58+
name:
59+
type: string
60+
autoRenew:
61+
type: boolean
62+
note:
63+
type: string

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/ApiResponse.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ public partial class ApiResponse : IEquatable<ApiResponse>, IValidatableObject
3838
/// <param name="code">code.</param>
3939
/// <param name="type">type.</param>
4040
/// <param name="message">message.</param>
41-
public ApiResponse(int code = default, string type = default, string message = default)
41+
public ApiResponse(int? code = default, string type = default, string message = default)
4242
{
43-
this._Code = code;
44-
if (this.Code != null)
43+
if (code != null)
4544
{
45+
this._Code = code.Value;
4646
this._flagCode = true;
4747
}
4848
this._Type = type;

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/AppleReq.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ protected AppleReq() { }
4242
/// </summary>
4343
/// <param name="cultivar">cultivar (required).</param>
4444
/// <param name="mealy">mealy.</param>
45-
public AppleReq(string cultivar = default, bool mealy = default)
45+
public AppleReq(string cultivar = default, bool? mealy = default)
4646
{
4747
// to ensure "cultivar" is required (not null)
4848
if (cultivar == null)
4949
{
5050
throw new ArgumentNullException("cultivar is a required property for AppleReq and cannot be null");
5151
}
5252
this._Cultivar = cultivar;
53-
this._Mealy = mealy;
54-
if (this.Mealy != null)
53+
if (mealy != null)
5554
{
55+
this._Mealy = mealy.Value;
5656
this._flagMealy = true;
5757
}
5858
}

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Banana.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ public partial class Banana : IEquatable<Banana>, IValidatableObject
3636
/// Initializes a new instance of the <see cref="Banana" /> class.
3737
/// </summary>
3838
/// <param name="lengthCm">lengthCm.</param>
39-
public Banana(decimal lengthCm = default)
39+
public Banana(decimal? lengthCm = default)
4040
{
41-
this._LengthCm = lengthCm;
42-
if (this.LengthCm != null)
41+
if (lengthCm != null)
4342
{
43+
this._LengthCm = lengthCm.Value;
4444
this._flagLengthCm = true;
4545
}
4646
this.AdditionalProperties = new Dictionary<string, object>();

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/BananaReq.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ protected BananaReq() { }
4242
/// </summary>
4343
/// <param name="lengthCm">lengthCm (required).</param>
4444
/// <param name="sweet">sweet.</param>
45-
public BananaReq(decimal lengthCm = default, bool sweet = default)
45+
public BananaReq(decimal lengthCm = default, bool? sweet = default)
4646
{
4747
this._LengthCm = lengthCm;
48-
this._Sweet = sweet;
49-
if (this.Sweet != null)
48+
if (sweet != null)
5049
{
50+
this._Sweet = sweet.Value;
5151
this._flagSweet = true;
5252
}
5353
}

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Cat.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ protected Cat()
4747
/// <param name="declawed">declawed.</param>
4848
/// <param name="className">className (required) (default to &quot;Cat&quot;).</param>
4949
/// <param name="color">color (default to &quot;red&quot;).</param>
50-
public Cat(bool declawed = default, string className = @"Cat", string color = @"red") : base(className, color)
50+
public Cat(bool? declawed = default, string className = @"Cat", string color = @"red") : base(className, color)
5151
{
52-
this._Declawed = declawed;
53-
if (this.Declawed != null)
52+
if (declawed != null)
5453
{
54+
this._Declawed = declawed.Value;
5555
this._flagDeclawed = true;
5656
}
5757
this.AdditionalProperties = new Dictionary<string, object>();

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/Category.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,17 @@ protected Category()
4545
/// </summary>
4646
/// <param name="id">id.</param>
4747
/// <param name="name">name (required) (default to &quot;default-name&quot;).</param>
48-
public Category(long id = default, string name = @"default-name")
48+
public Category(long? id = default, string name = @"default-name")
4949
{
5050
// to ensure "name" is required (not null)
5151
if (name == null)
5252
{
5353
throw new ArgumentNullException("name is a required property for Category and cannot be null");
5454
}
5555
this._Name = name;
56-
this._Id = id;
57-
if (this.Id != null)
56+
if (id != null)
5857
{
58+
this._Id = id.Value;
5959
this._flagId = true;
6060
}
6161
this.AdditionalProperties = new Dictionary<string, object>();

‎samples/client/petstore/csharp/restsharp/standard2.0/ConditionalSerialization/src/Org.OpenAPITools/Model/DateOnlyClass.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ public partial class DateOnlyClass : IEquatable<DateOnlyClass>, IValidatableObje
3636
/// Initializes a new instance of the <see cref="DateOnlyClass" /> class.
3737
/// </summary>
3838
/// <param name="dateOnlyProperty">dateOnlyProperty.</param>
39-
public DateOnlyClass(DateTime dateOnlyProperty = default)
39+
public DateOnlyClass(DateTime? dateOnlyProperty = default)
4040
{
41-
this._DateOnlyProperty = dateOnlyProperty;
42-
if (this.DateOnlyProperty != null)
41+
if (dateOnlyProperty != null)
4342
{
43+
this._DateOnlyProperty = dateOnlyProperty.Value;
4444
this._flagDateOnlyProperty = true;
4545
}
4646
this.AdditionalProperties = new Dictionary<string, object>();

0 commit comments

Comments
 (0)