[csharp-netcore] Generic Host: Generated Model ctor has more Parameters than the other Library types

ikill69 commented 2 years ago

Bug Report Checklist

[ ] Have you provided a full/minimal spec to reproduce the issue?
[ ] Have you validated the input using an OpenAPI validator (example)?
[ ] Have you tested with the latest master to confirm the issue still exists?
[ ] Have you searched for related issues/PRs?
[ ] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

I have hit a problem with a hard limit in the MaxParameterCount in the System.Text.Json class. See issue 12792 for details

After looking closely at the issue I have noticed that when I run the generator with the "--library=generichost" set i get 68 Parameters in the cstor. However when I run it with the "restsharp" library set I only get 32

Both of these instances are using the exact same Swagger file.

It looks like it is something to do with "format" field in the JSON. When "restsharp" is used these fields are excluded but when "generichost" is used they are all included. I thought at first it was "Readonly" but that does not seem to match up with what is generated.

This is what the Swagger looks like:

restsharp ctor

generichost ctor

openapi-generator version

docker run --rm -v C:\Dev\OpenAPI_CodeGen:/local openapitools/openapi-generator-cli:v6.1.0 generate -i /local/My_swagger.json -g csharp-netcore -o /local/csharp/OpenApi --skip-validate-spec --additional-properties=netCoreProjectFile=true,targetFramework=net6.0,packageName=MyClient.SDK --library=generichost

OpenAPI declaration file content or url

Unfortunately I can not post the exact Swagger

Steps to reproduce

You need a Swagger file that has more properties than 64 on a particular object and have some of the properties setup using the format property.

docker run --rm -v C:\Dev\OpenAPI_CodeGen:/local openapitools/openapi-generator-cli:v6.1.0 generate -i /local/My_swagger.json -g csharp-netcore -o /local/csharp/OpenApi --skip-validate-spec --additional-properties=netCoreProjectFile=true,targetFramework=net6.0,packageName=MyClient.SDK --library=generichost

docker run --rm -v C:\Dev\OpenAPI_CodeGen:/local openapitools/openapi-generator-cli:v6.1.0 generate -i /local/My_swagger.json -g csharp-netcore -o /local/csharp/OpenApi --skip-validate-spec --additional-properties=netCoreProjectFile=true,targetFramework=net6.0,packageName=MyClient.SDK --library=restsharp

Related issues/PRs

Suggest a fix

Suggest a Temporary Work Around

Replace all the "private set;" with "init;" in the affected classes. (So that the JsonSerializer.Deserialize can work with them properly. and they can still be set on create of the object)
You can also copy the constructor from the affected "restsharp" generated classes into the "generichost" classes. (This may not match exactly the readonly fields so I suggest the following step above as well if your going to do this)

wing328 commented 2 years ago

thanks for reporting the issue

cc @devhl-labs

devhl-labs commented 2 years ago

How many parameters does the spec say there are? I suspect the problem is not about the format, but rather the readOnly. In either case, I'm not sure why the RestSharp library would be ignoring properties. Can you shed some light on it? FYI, I have a lot of changes pending and I'm working with wing to get it merged.

ikill69 commented 2 years ago

@devhl-labs You are absolutely right.

I did another diff and compared to the swagger and it is the readOnly. When I did my first check I thought I had some readonly ones in the restsharp ctor but I don't.

devhl-labs commented 2 years ago

Why should readOnly be excluded? If it is excluded, how do the properties ever get populated?

ikill69 commented 2 years ago

@devhl-labs good point. That brings it back to "format" being the issue again. It is the only other property that mirrors the output.

This is the section of the swagger for user, it is one of the objects having a problem

Swagger User

``` "User": { "example": { "firstName": "John", "lastName": "Smith", "userId": "JohnSmith147" }, "properties": { "href": { "description": "Bla Bla", "type": "string", "readOnly": true }, "id": { "description": "Bla Bla", "type": "string", "x-null-on-create": true, "readOnly": true }, "accountUnlockedCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "addressLine1": { "description": "Bla Bla", "maxLength": 200, "type": "string" }, "addressLine2": { "description": "Bla Bla", "maxLength": 200, "type": "string" }, "adosFaceFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "adosFaceLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "adosFaceUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "adosPasscodeFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "adosPasscodeLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "adosPasscodeUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "adosVoiceFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "adosVoiceLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "adosVoiceUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "allowedPermissions": { "description": "Bla Bla", "type": "string", "readOnly": true }, "alternatePhone": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "alternatePhoneCountryCode": { "description": "Bla Bla", "maxLength": 10, "type": "string" }, "archived": { "description": "Bla Bla", "format": "dateTime", "type": "string", "x-null-on-create": true, "readOnly": true }, "city": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "country": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "created": { "description": "Bla Bla", "format": "dateTime", "type": "string", "x-null-on-create": true, "readOnly": true, "x-sortable": true }, "directOtpFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "directOtpLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "directOtpUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "email": { "description": "Bla Bla", "maxLength": 100, "type": "string" }, "externalName": { "description": "Bla Bla", "maxLength": 100, "type": "string" }, "face": { "$ref": "#/definitions/FaceData", "description": "Bla Bla", "readOnly": true }, "faceEnrolled": { "description": "Bla Bla", "type": "boolean", "readOnly": true }, "failedVerificationCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "firstName": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "keystrokesEnrolled": { "description": "Bla Bla", "type": "boolean", "readOnly": true }, "keystrokesFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "keystrokesLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "keystrokesUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "language": { "description": "Bla Bla", "type": "string" }, "lastName": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "lockedUntil": { "description": "Bla Bla", "format": "dateTime", "type": "string", "x-null-on-create": true, "readOnly": true }, "middleName": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "nextPossibleDirectOtpDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "otpFailedAttempts": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "otpLockedUntilDtm": { "description": "Bla Bla", "format": "dateTime", "type": "string", "readOnly": true }, "otpUnlockCount": { "description": "Bla Bla", "format": "int64", "type": "integer", "readOnly": true }, "pin": { "description": "Bla Bla", "type": "string", "x-writeonly": true }, "pinEnrolled": { "description": "Bla Bla", "type": "boolean", "readOnly": true }, "postalCode": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "prefix": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "primaryPhone": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "primaryPhoneCountryCode": { "description": "Bla Bla", "maxLength": 10, "type": "string" }, "secretQuestion": { "description": "Bla Bla", "items": { "$ref": "#/definitions/SecretQuestion" }, "type": "array" }, "speakerId": { "description": "Bla Bla", "type": "string" }, "state": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "status": { "description": "Bla Bla", "enum": [ "ACTIVE", "ARCHIVED", "BLOCKED", "SUSPENDED" ], "type": "string", "x-null-on-create": true, "readOnly": true }, "suffix": { "description": "Bla Bla", "maxLength": 50, "type": "string" }, "updated": { "description": "Bla Bla", "format": "dateTime", "type": "string", "x-null-on-create": true, "readOnly": true, "x-sortable": true }, "userId": { "description": "Bla Bla", "maxLength": 50, "type": "string", "x-mandatory-on-create": true, "x-sortable": true }, "voiceDigitsEnrolled": { "description": "Bla Bla", "type": "boolean", "readOnly": true }, "voiceEnrolled": { "description": "Bla Bla", "type": "boolean", "readOnly": true }, "voiceTextDependent": { "description": "Bla Bla", "items": { "$ref": "#/definitions/VoiceData" }, "type": "array", "readOnly": true }, "voiceTextPromptedDigits": { "description": "Bla Bla", "items": { "$ref": "#/definitions/VoiceData" }, "type": "array", "readOnly": true }, "applications": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "authenticationRequests": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "authenticators": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "enrollments": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "readOnly": true }, "registrationChallenges": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "registrations": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "samples": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "readOnly": true }, "sponsorships": { "$ref": "#/definitions/PageOfResourcesLink", "description": "Bla Bla", "x-expandable-in-instance": true, "readOnly": true }, "tenant": { "$ref": "#/definitions/ResourceLink", "description": "Bla Bla", "readOnly": true } }, "required": [ "userId" ] }, ```

restsharp ctor User

``` public User(string addressLine1 = default(string), string addressLine2 = default(string), string alternatePhone = default(string), string alternatePhoneCountryCode = default(string), string city = default(string), string country = default(string), string email = default(string), string externalName = default(string), FaceData face = default(FaceData), string firstName = default(string), string language = default(string), string lastName = default(string), string middleName = default(string), string pin = default(string), string postalCode = default(string), string prefix = default(string), string primaryPhone = default(string), string primaryPhoneCountryCode = default(string), List secretQuestion = default(List), string speakerId = default(string), string state = default(string), string suffix = default(string), string userId = default(string), PageOfResourcesLink applications = default(PageOfResourcesLink), PageOfResourcesLink authenticationRequests = default(PageOfResourcesLink), PageOfResourcesLink authenticators = default(PageOfResourcesLink), PageOfResourcesLink enrollments = default(PageOfResourcesLink), PageOfResourcesLink registrationChallenges = default(PageOfResourcesLink), PageOfResourcesLink registrations = default(PageOfResourcesLink), PageOfResourcesLink samples = default(PageOfResourcesLink), PageOfResourcesLink sponsorships = default(PageOfResourcesLink), ResourceLink tenant = default(ResourceLink)) { // to ensure "userId" is required (not null) if (userId == null) { throw new ArgumentNullException("userId is a required property for User and cannot be null"); } this.UserId = userId; this.AddressLine1 = addressLine1; this.AddressLine2 = addressLine2; this.AlternatePhone = alternatePhone; this.AlternatePhoneCountryCode = alternatePhoneCountryCode; this.City = city; this.Country = country; this.Email = email; this.ExternalName = externalName; this.Face = face; this.FirstName = firstName; this.Language = language; this.LastName = lastName; this.MiddleName = middleName; this.Pin = pin; this.PostalCode = postalCode; this.Prefix = prefix; this.PrimaryPhone = primaryPhone; this.PrimaryPhoneCountryCode = primaryPhoneCountryCode; this.SecretQuestion = secretQuestion; this.SpeakerId = speakerId; this.State = state; this.Suffix = suffix; this.Applications = applications; this.AuthenticationRequests = authenticationRequests; this.Authenticators = authenticators; this.Enrollments = enrollments; this.RegistrationChallenges = registrationChallenges; this.Registrations = registrations; this.Samples = samples; this.Sponsorships = sponsorships; this.Tenant = tenant; } ```

generichost ctor User

``` public User(string userId, string? href = default, string? id = default, long? accountUnlockedCount = default, string? addressLine1 = default, string? addressLine2 = default, long? adosFaceFailedAttempts = default, string? adosFaceLockedUntilDtm = default, long? adosFaceUnlockCount = default, long? adosPasscodeFailedAttempts = default, string? adosPasscodeLockedUntilDtm = default, long? adosPasscodeUnlockCount = default, long? adosVoiceFailedAttempts = default, string? adosVoiceLockedUntilDtm = default, long? adosVoiceUnlockCount = default, string? allowedPermissions = default, string? alternatePhone = default, string? alternatePhoneCountryCode = default, string? archived = default, string? city = default, string? country = default, string? created = default, long? directOtpFailedAttempts = default, string? directOtpLockedUntilDtm = default, long? directOtpUnlockCount = default, string? email = default, string? externalName = default, FaceData? face = default, bool? faceEnrolled = default, long? failedVerificationCount = default, string? firstName = default, bool? keystrokesEnrolled = default, long? keystrokesFailedAttempts = default, string? keystrokesLockedUntilDtm = default, long? keystrokesUnlockCount = default, string? language = default, string? lastName = default, string? lockedUntil = default, string? middleName = default, string? nextPossibleDirectOtpDtm = default, long? otpFailedAttempts = default, string? otpLockedUntilDtm = default, long? otpUnlockCount = default, string? pin = default, bool? pinEnrolled = default, string? postalCode = default, string? prefix = default, string? primaryPhone = default, string? primaryPhoneCountryCode = default, List? secretQuestion = default, string? speakerId = default, string? state = default, StatusEnum? status = default, string? suffix = default, string? updated = default, bool? voiceDigitsEnrolled = default, bool? voiceEnrolled = default, List? voiceTextDependent = default, List? voiceTextPromptedDigits = default, PageOfResourcesLink? applications = default, PageOfResourcesLink? authenticationRequests = default, PageOfResourcesLink? authenticators = default, PageOfResourcesLink? enrollments = default, PageOfResourcesLink? registrationChallenges = default, PageOfResourcesLink? registrations = default, PageOfResourcesLink? samples = default, PageOfResourcesLink? sponsorships = default, ResourceLink? tenant = default) { if (userId == null) throw new ArgumentNullException("userId is a required property for User and cannot be null."); UserId = userId; Href = href; Id = id; AccountUnlockedCount = accountUnlockedCount; AddressLine1 = addressLine1; AddressLine2 = addressLine2; AdosFaceFailedAttempts = adosFaceFailedAttempts; AdosFaceLockedUntilDtm = adosFaceLockedUntilDtm; AdosFaceUnlockCount = adosFaceUnlockCount; AdosPasscodeFailedAttempts = adosPasscodeFailedAttempts; AdosPasscodeLockedUntilDtm = adosPasscodeLockedUntilDtm; AdosPasscodeUnlockCount = adosPasscodeUnlockCount; AdosVoiceFailedAttempts = adosVoiceFailedAttempts; AdosVoiceLockedUntilDtm = adosVoiceLockedUntilDtm; AdosVoiceUnlockCount = adosVoiceUnlockCount; AllowedPermissions = allowedPermissions; AlternatePhone = alternatePhone; AlternatePhoneCountryCode = alternatePhoneCountryCode; Archived = archived; City = city; Country = country; Created = created; DirectOtpFailedAttempts = directOtpFailedAttempts; DirectOtpLockedUntilDtm = directOtpLockedUntilDtm; DirectOtpUnlockCount = directOtpUnlockCount; Email = email; ExternalName = externalName; Face = face; FaceEnrolled = faceEnrolled; FailedVerificationCount = failedVerificationCount; FirstName = firstName; KeystrokesEnrolled = keystrokesEnrolled; KeystrokesFailedAttempts = keystrokesFailedAttempts; KeystrokesLockedUntilDtm = keystrokesLockedUntilDtm; KeystrokesUnlockCount = keystrokesUnlockCount; Language = language; LastName = lastName; LockedUntil = lockedUntil; MiddleName = middleName; NextPossibleDirectOtpDtm = nextPossibleDirectOtpDtm; OtpFailedAttempts = otpFailedAttempts; OtpLockedUntilDtm = otpLockedUntilDtm; OtpUnlockCount = otpUnlockCount; Pin = pin; PinEnrolled = pinEnrolled; PostalCode = postalCode; Prefix = prefix; PrimaryPhone = primaryPhone; PrimaryPhoneCountryCode = primaryPhoneCountryCode; SecretQuestion = secretQuestion; SpeakerId = speakerId; State = state; Status = status; Suffix = suffix; Updated = updated; VoiceDigitsEnrolled = voiceDigitsEnrolled; VoiceEnrolled = voiceEnrolled; VoiceTextDependent = voiceTextDependent; VoiceTextPromptedDigits = voiceTextPromptedDigits; Applications = applications; AuthenticationRequests = authenticationRequests; Authenticators = authenticators; Enrollments = enrollments; RegistrationChallenges = registrationChallenges; Registrations = registrations; Samples = samples; Sponsorships = sponsorships; Tenant = tenant; } ```

ikill69 commented 2 years ago

They are making some movement in the issue System.Text.Json can't deserialize type with more than 64 ctor parameters. see the PR here https://github.com/dotnet/runtime/pull/75982

So limiting the number of parameters in the constructor may not be as important soon.

@devhl-labs perhaps simply changing the output for each property that is readonly from " { get; private set; }" to " { get; init; }" could be part of the solution. I have added this as a temporary workaround for now to the issue.

devhl-labs commented 2 years ago

Actually I saw this. As far as I can tell, the issue here is not that generic-host is making too many parameters, it's that other libraries don't have enough, plus that STJ issue you just posted.

init is a newer language feature, so I can't use it right now, but I do plan on doing something to use new language features. After I get my pending work merged, I will consider how to do that.

ikill69 commented 2 years ago

@devhl-labs I came to the exact same conclusion. The other library is doing it differently and I think it is to do with Newtonsoft being able to set Private properties.

This is what they are using in the "restsharp" output as the constructor and I debugged the code in both applications and watch the Deserialize set a property that is marked as private.

The particular property that I am taking about is in fact the "ID" so this is extremely important and is required almost all of the time after it is created. And as you can see in my comment above https://github.com/OpenAPITools/openapi-generator/issues/13480#issuecomment-1253256791 The "restsharp" constructor does not include it either and yet it is still able to be set even thought it is "private set;".

OpenAPITools / openapi-generator