OpenAPIGenerator upgrade to v7.16.0 - [2] Added new mustache templates #1208
Conversation
Kwok-he-Chu
changed the base branch from
main
to
v7-generator/1-add-core-classes
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant upgrade, replacing old templates with new ones for the generichost library to support OpenAPI Generator v7.16.0. The changes are extensive and introduce a new structure for C# code generation. My review of the new mustache templates has identified a few critical and high-severity issues related to null handling, incorrect logic in authentication flows, and attempts to instantiate abstract classes. These could lead to runtime exceptions or unexpected behavior in the generated code. I've provided detailed feedback and code suggestions to improve the robustness and correctness of the new templates.
templates-v7/csharp/libraries/generichost/HttpSigningConfiguration.mustache
Outdated
Show resolved
Hide resolved
templates-v7/csharp/libraries/generichost/HttpSigningConfiguration.mustache
Outdated
Show resolved
Hide resolved
Kwok-he-Chu
changed the title
Remove unused templates ApiTests, DependencyInjectionTests #1208 (comment)
|
@gcatanese The mustache templates include the generated templates for the classes in the |
Kwok-he-Chu
force-pushed
the
v7-generator/2-add-mustache-templates
branch
from
6d99b32 to
2e33dc4
Compare
Kwok-he-Chu
changed the base branch from
v7-generator/1-add-core-classes
to
v7-generator/development-branch
Remove old mustache templates
Kwok-he-Chu
force-pushed
the
v7-generator/2-add-mustache-templates
branch
from
2e33dc4 to
d49b160
Compare
templates-v7/csharp/libraries/generichost/ValidateRegex.mustache
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,41 @@ | |||
| \// <auto-generated> | |||
| if (DateOnly.TryParseExact(value, format, CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal, out DateOnly result)) | ||
| return result; | ||
|
|
||
| throw new NotSupportedException(); |
There was a problem hiding this comment.
I think this is out of sync with the generated file
| @@ -0,0 +1 @@ | |||
| {{#pathParams}}{{{dataType}}}{{>NullConditionalParameter}} {{paramName}}, {{/pathParams}}{{#queryParams}}{{#required}}{{{dataType}}}{{>NullConditionalParameter}} {{paramName}}, {{/required}}{{/queryParams}}{{#bodyParams}}{{{dataType}}}{{>NullConditionalParameter}} {{paramName}}, {{/bodyParams}}{{#queryParams}}{{^required}}Option<{{{dataType}}}{{>NullConditionalParameter}}> {{paramName}}{{#notRequiredOrIsNullable}} = default{{/notRequiredOrIsNullable}}, {{/required}}{{/queryParams}} RequestOptions? requestOptions = default, System.Threading.CancellationToken cancellationToken = default{{^netstandard20OrLater}}(global::System.Threading.CancellationToken){{/netstandard20OrLater}} No newline at end of file | |||
There was a problem hiding this comment.
I see that this version is a bit more complicated than the original. No need to change for now but we can create a follow up about maintainability later
There was a problem hiding this comment.
@gcatanese FYI, we decided to make these changes. The goal was to preserve the order of the operations and enforce mandatory body payloads
In terms of maintainability, this is the best I could given our mustache.template constraints, string manipulations are tedious yes :)
PR-2 of #1204
Description
This PR introduces the
mustache.templatesneeded for the upgrade of the OpenApiGenerator to v7.16.0 used in the Adyen sdk-automation to generate .NET models & services from the Adyen OpenAPI Specifications.