Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[csharp][generichost] Remove discriminator property #18445

Merged
merged 10 commits into from
May 21, 2024
Merged

[csharp][generichost] Remove discriminator property #18445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d60eac2
fixed nullability
devhl-labs Apr 16, 2024
94fe9de
minor refactor to address a comment
devhl-labs Apr 20, 2024
eee1255
removed discriminator property
devhl-labs Apr 21, 2024
c8793f0
update more manual tests
devhl-labs Apr 21, 2024
f8c84d2
update more manual tests
devhl-labs Apr 21, 2024
d054f70
resolved conflicts
devhl-labs Apr 21, 2024
2ce3581
added tests and bug fixes
devhl-labs Apr 21, 2024
3b1f04d
fixed allof
devhl-labs May 5, 2024
3e03924
reworked manual tests to compare json instead of classes, reworked di…
devhl-labs May 19, 2024
4d20fc2
removed unneeded code
devhl-labs May 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
13 changes: 12 additions & 1 deletion modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,21 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
return objs;
}

/**
* A property is new if it is in a derived class and the derived property is different.
* This usually occurs when the data type is different.
* We can also consider discriminators as new because the derived class discriminator will have to be defined again
* to contain a new value. Doing so prevents having to include the discriminator in the constructor.
* @param model
* @param property
* @return
*/
private boolean codegenPropertyIsNew(CodegenModel model, CodegenProperty property) {
Copy link
Member

@wing328 wing328 May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should rename it to something like codegenPropertyIsNewOrDiscriminator but that's pretty minor.

fyi, later i may refactor this function into codegen model/property class instead to avoid too many function in default codegen class

return model.parentModel == null
? false
: model.parentModel.allVars.stream().anyMatch(p -> p.name.equals(property.name) && (p.dataType.equals(property.dataType) == false || p.datatypeWithEnum.equals(property.datatypeWithEnum) == false));
: model.parentModel.allVars.stream().anyMatch(p ->
p.name.equals(property.name) &&
(p.dataType.equals(property.dataType) == false || p.datatypeWithEnum.equals(property.datatypeWithEnum) == false || p.isDiscriminator));
}

/**
Expand Down
37 changes: 37 additions & 0 deletions ...es/openapi-generator/src/main/resources/csharp/libraries/generichost/ClientUtils.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,43 @@ using System.Runtime.CompilerServices;
return JsonRegex.IsMatch(mime) || mime.Equals("application/json-patch+json");
}

/// <summary>
/// Get the discriminator
/// </summary>
/// <param name="utf8JsonReader"></param>
/// <param name="discriminator"></param>
/// <returns></returns>
/// <exception cref="JsonException"></exception>
public static string{{nrt?}} GetDiscriminator(Utf8JsonReader utf8JsonReader, string discriminator)
{
int currentDepth = utf8JsonReader.CurrentDepth;

if (utf8JsonReader.TokenType != JsonTokenType.StartObject && utf8JsonReader.TokenType != JsonTokenType.StartArray)
throw new JsonException();

JsonTokenType startingTokenType = utf8JsonReader.TokenType;

while (utf8JsonReader.Read())
{
if (startingTokenType == JsonTokenType.StartObject && utf8JsonReader.TokenType == JsonTokenType.EndObject && currentDepth == utf8JsonReader.CurrentDepth)
break;

if (startingTokenType == JsonTokenType.StartArray && utf8JsonReader.TokenType == JsonTokenType.EndArray && currentDepth == utf8JsonReader.CurrentDepth)
break;

if (utf8JsonReader.TokenType == JsonTokenType.PropertyName && currentDepth == utf8JsonReader.CurrentDepth - 1)
{
string{{nrt?}} localVarJsonPropertyName = utf8JsonReader.GetString();
utf8JsonReader.Read();

if (localVarJsonPropertyName != null && localVarJsonPropertyName.Equals(discriminator))
return utf8JsonReader.GetString();
}
}

throw new JsonException("The specified discriminator was not found.");
}

/// <summary>
/// The base path of the API
/// </summary>
Expand Down
49 changes: 42 additions & 7 deletions .../openapi-generator/src/main/resources/csharp/libraries/generichost/JsonConverter.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@

{{/-last}}
{{/allVars}}
{{#discriminator}}
{{#children}}
{{#-first}}
string{{nrt?}} discriminator = ClientUtils.GetDiscriminator(utf8JsonReader, "{{discriminator.propertyBaseName}}");

{{/-first}}
if (discriminator != null && discriminator.Equals("{{name}}"))
return JsonSerializer.Deserialize<{{{name}}}>(ref utf8JsonReader, jsonSerializerOptions) ?? throw new JsonException("The result was an unexpected value.");

{{/children}}
{{/discriminator}}
{{#model.discriminator}}
{{#model.hasDiscriminatorWithNonEmptyMapping}}
{{#mappedModels}}
Expand Down Expand Up @@ -281,7 +292,7 @@
{{#model.hasDiscriminatorWithNonEmptyMapping}}
{{#mappedModels}}
if ({{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}} != null)
return new {{classname}}({{#lambda.joinWithComma}}{{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}.Value{{nrt!}}{{^isNullable}}{{#vendorExtensions.x-is-value-type}}.Value{{/vendorExtensions.x-is-value-type}}{{/isNullable}}{{/required}} {{/allVars}}{{/lambda.joinWithComma}});
return new {{classname}}({{#lambda.joinWithComma}}{{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{^isDiscriminator}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}.Value{{nrt!}}{{^isNullable}}{{#vendorExtensions.x-is-value-type}}.Value{{/vendorExtensions.x-is-value-type}}{{/isNullable}}{{/required}} {{/isDiscriminator}}{{/allVars}}{{/lambda.joinWithComma}});

{{#-last}}
throw new JsonException();
Expand All @@ -300,14 +311,14 @@

{{/-last}}
{{/required}}
return new {{classname}}({{#lambda.joinWithComma}}{{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}ParsedValue{{#required}}.Value{{#vendorExtensions.x-is-value-type}}{{nrt!}}.Value{{nrt!}}{{/vendorExtensions.x-is-value-type}}{{/required}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}.Value{{nrt!}}{{^isNullable}}{{#vendorExtensions.x-is-value-type}}.Value{{nrt!}}{{/vendorExtensions.x-is-value-type}}{{/isNullable}}{{/required}} {{/allVars}}{{/lambda.joinWithComma}});
return new {{classname}}({{#lambda.joinWithComma}}{{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}ParsedValue{{#required}}.Value{{#vendorExtensions.x-is-value-type}}{{nrt!}}.Value{{nrt!}}{{/vendorExtensions.x-is-value-type}}{{/required}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{^isDiscriminator}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}.Value{{nrt!}}{{^isNullable}}{{#vendorExtensions.x-is-value-type}}.Value{{nrt!}}{{/vendorExtensions.x-is-value-type}}{{/isNullable}}{{/required}} {{/isDiscriminator}}{{/allVars}}{{/lambda.joinWithComma}});
{{/composedSchemas.oneOf}}
{{^model.discriminator}}
{{#composedSchemas}}
{{#oneOf}}
{{^vendorExtensions.x-duplicated-data-type}}
if ({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}} != null)
return new {{classname}}({{#lambda.joinWithComma}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}.Value{{/vendorExtensions.x-is-value-type}} {{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}ParsedValue{{/required}} {{/allVars}}{{/lambda.joinWithComma}});
return new {{classname}}({{#lambda.joinWithComma}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}.Value{{/vendorExtensions.x-is-value-type}} {{#model.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#vendorExtensions.x-is-value-type}}{{^isNullable}}.Value{{/isNullable}}{{/vendorExtensions.x-is-value-type}} {{/model.composedSchemas.anyOf}}{{#allVars}}{{^isDiscriminator}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{#required}}ParsedValue{{/required}} {{/isDiscriminator}}{{/allVars}}{{/lambda.joinWithComma}});

{{/vendorExtensions.x-duplicated-data-type}}
{{#-last}}
Expand All @@ -331,6 +342,18 @@
public override void Write(Utf8JsonWriter writer, {{classname}} {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, JsonSerializerOptions jsonSerializerOptions)
{
{{#lambda.trimLineBreaks}}
{{#lambda.copy}}
{{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}
{{/lambda.copy}}
{{#discriminator}}
{{#children}}
if ({{#lambda.pasteLine}}{{/lambda.pasteLine}} is {{classname}} {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}){
JsonSerializer.Serialize<{{{name}}}>(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, jsonSerializerOptions);
return;
}

{{/children}}
{{/discriminator}}
writer.WriteStartObject();

{{#model.discriminator}}
Expand All @@ -339,7 +362,7 @@
{{^vendorExtensions.x-duplicated-data-type}}
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(ref writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions);
{{#lambda.camelcase_sanitize_param}}{{baseType}}JsonConverter{{/lambda.camelcase_sanitize_param}}.WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}, jsonSerializerOptions);
}

{{/vendorExtensions.x-duplicated-data-type}}
Expand All @@ -352,13 +375,13 @@
if ({{#lambda.joinWithAmpersand}}{{^required}}{{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}Option.IsSet {{/required}}{{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{^required}}Option.Value{{/required}} != null{{/lambda.joinWithAmpersand}})
{
{{datatypeWithEnum}}JsonConverter {{datatypeWithEnum}}JsonConverter = ({{datatypeWithEnum}}JsonConverter) jsonSerializerOptions.Converters.First(c => c.CanConvert({{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{^required}}Option.Value{{/required}}.GetType()));
{{datatypeWithEnum}}JsonConverter.WriteProperties(ref writer, {{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{^required}}Option.Value{{/required}}, jsonSerializerOptions);
{{datatypeWithEnum}}JsonConverter.WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{model.classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{^required}}Option.Value{{/required}}, jsonSerializerOptions);
}

{{/anyOf}}
{{/composedSchemas}}
{{/model.discriminator}}
WriteProperties(ref writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, jsonSerializerOptions);
WriteProperties(writer, {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, jsonSerializerOptions);
writer.WriteEndObject();
{{/lambda.trimLineBreaks}}
}
Expand All @@ -370,20 +393,31 @@
/// <param name="{{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}"></param>
/// <param name="jsonSerializerOptions"></param>
/// <exception cref="NotImplementedException"></exception>
public void WriteProperties(ref Utf8JsonWriter writer, {{classname}} {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, JsonSerializerOptions jsonSerializerOptions)
public void WriteProperties(Utf8JsonWriter writer, {{classname}} {{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}, JsonSerializerOptions jsonSerializerOptions)
{
{{#lambda.trimTrailingWithNewLine}}
{{#lambda.trimLineBreaks}}
{{#allVars}}
{{^isDiscriminator}}
{{^isNullable}}
{{#vendorExtensions.x-is-reference-type}}
if ({{^required}}{{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}Option.IsSet && {{/required}}{{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}} == null)
throw new ArgumentNullException(nameof({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}), "Property is required for class {{classname}}.");

{{/vendorExtensions.x-is-reference-type}}
{{/isNullable}}
{{/isDiscriminator}}
{{/allVars}}
{{#allVars}}
{{#isDiscriminator}}
{{^model.composedSchemas.anyOf}}
{{^model.composedSchemas.oneOf}}
writer.WriteString("{{baseName}}", {{^isEnum}}{{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{/isEnum}}{{#isNew}}{{#isEnum}}{{#isInnerEnum}}{{classname}}.{{{datatypeWithEnum}}}ToJsonValue{{/isInnerEnum}}{{^isInnerEnum}}{{{datatypeWithEnum}}}ValueConverter.ToJsonValue{{/isInnerEnum}}({{#lambda.camelcase_sanitize_param}}{{classname}}{{/lambda.camelcase_sanitize_param}}.{{name}}{{^required}}.Value{{/required}}){{/isEnum}}{{/isNew}});

{{/model.composedSchemas.oneOf}}
{{/model.composedSchemas.anyOf}}
{{/isDiscriminator}}
{{^isDiscriminator}}
{{#isString}}
{{^isMap}}
{{^isEnum}}
Expand Down Expand Up @@ -579,6 +613,7 @@
{{/isString}}
{{/isEnum}}
{{/isUuid}}
{{/isDiscriminator}}
{{/allVars}}
{{/lambda.trimLineBreaks}}
{{/lambda.trimTrailingWithNewLine}}
Expand Down
2 changes: 1 addition & 1 deletion ...api-generator/src/main/resources/csharp/libraries/generichost/ModelBaseSignature.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#parentModel.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{parent}}{{/lambda.camelcase_sanitize_param}}.{{#lambda.titlecase}}{{baseType}}{{#isArray}}{{{dataFormat}}}{{/isArray}}{{/lambda.titlecase}} {{/parentModel.composedSchemas.anyOf}}{{#allVars}}{{#isInherited}}{{^isNew}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{/isNew}}{{#isNew}}{{#isEnum}}{{#isInnerEnum}}{{classname}}.{{{datatypeWithEnum}}}ToJsonValue{{/isInnerEnum}}{{^isInnerEnum}}{{{datatypeWithEnum}}}ValueConverter.ToJsonValue{{/isInnerEnum}}({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^required}}.Value{{/required}}){{/isEnum}}{{^isEnum}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.ToString(){{/isEnum}}{{/isNew}} {{/isInherited}}{{/allVars}}
{{#parentModel.composedSchemas.anyOf}}{{#lambda.camelcase_sanitize_param}}{{parent}}{{/lambda.camelcase_sanitize_param}}.{{#lambda.titlecase}}{{baseType}}{{#isArray}}{{{dataFormat}}}{{/isArray}}{{/lambda.titlecase}} {{/parentModel.composedSchemas.anyOf}}{{#allVars}}{{^isDiscriminator}}{{#isInherited}}{{^isNew}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{/isNew}}{{#isNew}}{{#isEnum}}{{#isInnerEnum}}{{classname}}.{{{datatypeWithEnum}}}ToJsonValue{{/isInnerEnum}}{{^isInnerEnum}}{{{datatypeWithEnum}}}ValueConverter.ToJsonValue{{/isInnerEnum}}({{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}{{^required}}.Value{{/required}}){{/isEnum}}{{^isEnum}}{{#lambda.camelcase_sanitize_param}}{{name}}{{/lambda.camelcase_sanitize_param}}.ToString(){{/isEnum}}{{/isNew}} {{/isInherited}}{{/isDiscriminator}}{{/allVars}}
2 changes: 1 addition & 1 deletion ...openapi-generator/src/main/resources/csharp/libraries/generichost/ModelSignature.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#model.allVars}}{{^required}}Option<{{/required}}{{{datatypeWithEnum}}}{{>NullConditionalProperty}}{{^required}}>{{/required}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}{{#defaultValue}} = {{^required}}default{{/required}}{{#required}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{.}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/required}}{{/defaultValue}}{{^defaultValue}}{{#lambda.first}}{{#isNullable}} = default {{/isNullable}}{{^required}} = default {{/required}}{{/lambda.first}}{{/defaultValue}} {{/model.allVars}}
{{#model.allVars}}{{^isDiscriminator}}{{^required}}Option<{{/required}}{{{datatypeWithEnum}}}{{>NullConditionalProperty}}{{^required}}>{{/required}} {{#lambda.escape_reserved_word}}{{#lambda.camel_case}}{{name}}{{/lambda.camel_case}}{{/lambda.escape_reserved_word}}{{#defaultValue}} = {{^required}}default{{/required}}{{#required}}{{^isDateTime}}{{#isString}}{{^isEnum}}@{{/isEnum}}{{/isString}}{{{.}}}{{/isDateTime}}{{#isDateTime}}default{{/isDateTime}}{{/required}}{{/defaultValue}}{{^defaultValue}}{{#lambda.first}}{{#isNullable}} = default {{/isNullable}}{{^required}} = default {{/required}}{{/lambda.first}}{{/defaultValue}} {{/isDiscriminator}}{{/model.allVars}}
Loading
Loading