[csharp-netcore] Update to generate parameterless constructor by default for Models

[prajon84]

This PR addresses fixes the issue for generating parameterless constructor for csharp-netcore generator.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (5.3.0), 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@wing328 @devhl-labs @Blackclaws @GauravQ @pkunze @jfeltesse-mdsol @bgong-mdsol

[devhl-labs]

I disagree with adding parameterless constructors when there are in fact fields and properties that require values.

[jfeltesse-mdsol]

@devhl-labs the problem though is that without a paramerterless constructor the propery names have to match perfectly and when those are reserved keywords there's just no way to have it work: the generator would make up different names so the code compiles but then it breaks the arguments/property names mapping. At least that's my understanding of it (I'm not a C# hacker by trade).

[devhl-labs]

What is breaking and where?

[prajon84]

What is breaking and where?

For e.g. if openapi.yaml file is this:

openapi: 3.0.3
info:
  title: Test openapi spec file for parameterless constructor issue
  version: v1
paths:
  '/hello':
    get:
      responses:
        '200':
          description: Success
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TestModel'

components:
  schemas:
    TestModel:
      description: Model information
      properties:
        active:
          description: active description
          type: boolean
        model_uuid:
          description: model uuid description
          type: string
          format: uuid
        internal:
          description: internal value
          type: boolean

Produces Model as:

/// <summary>
/// Initializes a new instance of the <see cref="TestModel" /> class.
/// </summary>
/// <param name="active">active description.</param>
 /// <param name="modelUuid">model uuid description.</param>
 /// <param name="_internal">internal value.</param>
public TestModel(bool active = default(bool), Guid modelUuid = default(Guid), bool _internal = default(bool))
{
    this.Active = active;
    this.ModelUuid = modelUuid;
    this.Internal = _internal;
}

And, the problem here is:

SpecFlow (integration tests) will then try to get the properties of the type and will try to match those with the parameters of the constructor.
The problem for this particular type is the Internal property is declared as _internal parameter hence the parameter name doesn't match.
SpecFlow couldn't get any constructor to instantiate the object and it blows up.

@hkurniawan-mdsol : Am I correct with the issue?

[devhl-labs]

More than likely, if you use a parameterless constructor, the _internal parameter simply does not ever get populated nor tested. It looks like you want the TableAliases property attribute. To add this property I would recommend using your own template directory. If that fixes the issue, then I would consider a PR on specflow to consider DataMember and JsonPropertyName as aliases.

https://docs.specflow.org/projects/specflow/en/latest/Bindings/SpecFlow-Assist-Helpers.html#aliasing

[h-kurniawan]

Am I correct with the issue?

Yes. Property that happens to be a reserved C# keyword when lower cased is especially bad if that model is used in CRUD scenario.

Say we have the following model

public class Pizza
{
    public Pizza(int id = default, string name = default, string description = default, bool _internal = default)
    {
        Id = id;
        Name = name;
        Description = description;
        Internal = _internal;
    }

    public int Id { get; set; }
    public string Name { get; set; }
    public string Description { get; set; }
    public bool Internal { get; set; }
}

and the following app

using Microsoft.EntityFrameworkCore;
using WebApplication2;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddDbContext<PizzaDb>(options => options.UseInMemoryDatabase("Dominos")); ;

var app = builder.Build();

app.MapGet("/pizzas", async (PizzaDb db) => await db.Pizzas.ToListAsync());

app.MapPost("/pizza-nomodel", async (PizzaDb db) =>
{
    var pizza = new Pizza(name: "Margherita", description:"Tomato, mozzarella, basil");
    await db.Set<Pizza>().AddAsync(pizza);
    await db.SaveChangesAsync();
    return Results.Created($"/pizza/{pizza.Id}", pizza);
});

app.MapPost("/pizza-withmodel", (PizzaDb db, Pizza pizza) =>
{
    // Doesn't even reach here.
    return Results.Ok(pizza);
});

app.Run();

When

• /pizzas and /pizza-nomodel is called we got the following error

  InvalidOperationException: No suitable constructor was found for entity type 'Pizza'. The following constructors had parameters that could not be bound to properties of the entity type: cannot bind '_internal' in 'Pizza(int id, string name, string description, bool _internal)'.
  

• /pizza-withmodel is called we got the following error

  System.InvalidOperationException: Each parameter in the deserialization constructor on type 'WebApplication2.Pizza' must bind to an object property or field on deserialization. Each parameter name must match with a property or field on the object. The match can be case-insensitive.'.
  

  Error during model binding.

In all cases looks like there's an issue trying to instantiate Pizza dynamically due to the mismatch of the property name Internal with _internal.

[devhl-labs]

In the model, you should have attributes to handle the mapping, but there are none here. If you're using RestSharp or HttpClient libraries you should see DataMember. If you're using generichost library, you should see JsonPropertyName.

Also, prefix a method's argument with an underscore is unusual. I believe that will still cause an issue, even if you have the proper attribute in place. But making a parameterless constructor is not the fix.

[h-kurniawan]

Those exceptions thrown not due to the incorrect mapping of properties between the payload and model, but the CLR (probably) inability to instantiate the object.

Also the generated parameterized constructor does not enforce properties to have a value since all the parameters are optional.
You can just instantiate the object like var pizza = new Pizza(); and all the properties will have default value.
Hence the bigger question is, what is the benefit of this parameterized constructor?

[devhl-labs]

There are two problems here. One is that you're not providing any mapping. To fix that you need DataMember attribute on the Instance property. This is handled by the template, so I'm not sure why it's not shown in your Pizza class above. The second is that Instance and _instance do not have the same naming convention. This may require a PR to fix.

A class with required properties should not have a parameterless constructor. The arguments all having default values is a flaw, and one that I addressed in my generichost library.

[devhl-labs]

@hkurniawan-mdsol I submitted a PR to fix the issue of _internal having an underscore and Internal not. That will fix most of this issue. However, you will still need a mapping attribute. From the error you posted, I think you are using System.Text.Json. The restsharp library does not support this, but you can try my generichost library, which does.

[prajon84]

@devhl-labs : Thank you for the PR. Besides, we are using -g csharp-netcore --library httpclient, how can we benefit from generichost library ?

[devhl-labs]

Httpclient also does not support System.Text.Json. if you want that youll need to use generichost for the library argument. Be aware it is quite different and very new. The readme it creates will show the intended way to use it. If you switch, its best to completely delete the old generated content since generichost has a different set of files than httpclient.