zijianhuang / openapiclientgen

Generate strongly typed C# and TypeScript client codes from Open API / Swagger definitions supporting jQuery, Angular, AXIOS, Fetch API, Aurelia and Angular Strictly Typed Forms
MIT License
64 stars 13 forks source link

Pull request and Testing information #26

Closed Kim-SSi closed 3 years ago

Kim-SSi commented 3 years ago

Hi, I am looking to create some pull request(s) and would like to know how you would like me to format them. Should each part be a separate request or should it be one request with multiple commits?

I am looking to add several options, what are you opinion on the following settings?

/// Use System.Text.Json instead of Newtonsoft.Json
public bool UseSystemTextJson { get; set; }

/// Generated data types will be decorated with JsonProperty with the PropertyName in C#.
public bool DecorateDataModelWithPropertyName { get; set; }

/// Disable property nullable by default unless set by the OpenApi v3 option nullable 
public bool DisableNullableByDefault { get; set; }

/// Create the Model classes only
public bool GenerateModelsOnly { get; set; }

/// Create arrays as List<T>
public bool ArrayAsList { get; set; }

/// Create arrays as ICollection<T>
public bool ArrayAsICollection { get; set; } 

How do you run the tests in this project? I would like to check my changes didn't break anything else.

Would you be open to having the openapi-directory directory be configurable or relative instead of hard coded? Should it be a git sub module or just a relative folder? If a relative folder then where would you like it? Beside the top level openapiclientgen directory or in the openapiclientgen\tests etc? And adding information in the ReadMe with all the dependencies and how to run the tests.

After doing a git clone, opening the project, building I get 2 errors where do I get these files from?

19>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(4919,5): error MSB3030: Could not copy the file "S:\Test\openapiclientgen\Tests\SwagTests\Results\Vsts\testResults.txt" because it was not found.
19>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(4919,5): error MSB3030: Could not copy the file "S:\Test\openapiclientgen\Tests\SwagTests\SwagMock\Vsts\testResults.json" because it was not found.

Regards, Kim

zijianhuang commented 3 years ago

Regarding to testing, here you are: https://github.com/zijianhuang/openapiclientgen/wiki/Testing

And for level 3 tests, you may ignore Medicare Online, which is an Australian government service with restricted access.

Regarding to replacing Newtonsoft.Json with System.Text.Json, this has been in radar, however, from my memory of developing WebApiClientGen, up to .NET 5, System.Text.Json is still a little weaker than Newtonsoft.json in some deserialization. Nevertheless, since what you purpose is just an option, obviously this is up to the client programmer to decide.

All options you suggested look good.

Regarding to the errors, I will document the test setup more, and likely adjust a little bit. Please stay tuned.

zijianhuang commented 3 years ago

Just added 2 missing files, while I am not sure why git on my dev machine ignore these 2 files.

And I had also refined https://github.com/zijianhuang/openapiclientgen/wiki/Testing,

zijianhuang commented 3 years ago

@Kim-SSi , presumably you can do #28 together, for UseSystemTextJson?

The option is good for generating codes for .NET Framework, which client apps may need Newtonsoft.Json, while .NET programmer may want only System.Text.Json.

Kim-SSi commented 3 years ago

@zijianhuang, Yes, I am looking into the enum conversion for System.Text.Json. I have not completed that yet.

Kim-SSi commented 3 years ago

@zijianhuang, I have added 2 pull requests. #30 and #31. The changes should handle #28 using the UseSystemTextJson setting. I have not run extensive testing. EnumToString setting is only basically tested for System.Text.Json.

Would it be a good idea to add SystemTextJson to GenerateCases? https://github.com/Kim-SSi/openapiclientgen/commit/3df02db339ebd4c38094d9285c6bed9f1d9426d3

return $@"
[Fact]
public void Test_{funcNameSuffix}()
{{
    helper.GenerateFromOpenApiAndBuild(@""..\..\..\..\openapi-directory\APIs\{d}"");
}}

[Fact]
public void Test_{funcNameSuffix}_SystemTextJson()
{{
    helper.GenerateFromOpenApiAndBuild(@""..\..\..\..\openapi-directory\APIs\{d}"", new Settings(){{UseSystemTextJson = true}});
}}";
zijianhuang commented 3 years ago

merged to master. And all tests pass. After further testing in more real world projects, changes will be released in v1.9

Kim-SSi commented 3 years ago

Thanks.

It would be worth adding to the Testing wiki page the changes and some more info: Change: Requires the extracted data in the openapi-directory in the Tests directory.

NPM dependencies for NG tests npm install -g @angular/cli npm install -g @angular-devkit/build-angular

zijianhuang commented 3 years ago

v1.9 released. Slight changes to initial PR by Kim-SSi:

/// Create arrays as List<T>
public bool ArrayAsList { get; set; }

/// Create arrays as ICollection<T>
public bool ArrayAsICollection { get; set; } 

is replaced by:

/// <summary>
/// By default, array type will be array in generated C#. You may generated IEnumerable and some of its derived types.
/// </summary>
public ArrayAsIEnumerableDerived ArrayAs { get; set; }

public enum ArrayAsIEnumerableDerived
{
    Array,
    IEnumable,
    IList,
    ICollection,
    IReadOnlyList,
    IReadOnlyCollection,

    List,
    Collection,
    ReadOnlyCollection,

}

And

        ///// <summary>not working well, the references to default value and api parameters not yet valid.
        ///// TitleCase Enum value names, while OpenApiClientGen by default use whatever defined in OpenApi definitions, commonly in camel casing.
        ///// </summary>
        //public bool TitleCaseEnumValueNames { get; set; }

and respective codes are commented out, because this breaks the parameters in API functions in which the enum type is used. Fixing this may require substantial works.

Also, I think respecting what enum member names defined in Open API definition may be more preferable. Overall this is probably not worthy. I will keep the commented-out codes for a while, and clean them upon next maintenance.