Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[csharp] Add a server generator for FastEndpoints #19690

Conversation

elendil-software
Copy link
Contributor

This pull request add a new generator for FastEndpoints.

It allows to generate an ASP.NET server project that use FastEnpoints.

Options allows to enable some FastEndpoints functionalities like :

Related issue : #19649

Technical committee mentions
@mandrean @shibayan @Blackclaws @lucamazzanti @iBicha

@elendil-software
Copy link
Contributor Author

Just noticed an error in record generation. When a parameter is optional it should be put in the body and not in the primary constructor.
I will fix that

@wing328
Copy link
Member

wing328 commented Sep 27, 2024

thanks for the PR

please take your time fixing the bug. let us know if you need help.

@@ -0,0 +1,14 @@
generatorName: aspnet-fastendpoints
outputDir: samples/server/petstore/aspnet/fastendpoints
Copy link
Member

Choose a reason for hiding this comment

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

what about adding the new folder to github workflow (e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-dotnet8-server.yaml) so that CI will test it moving forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
I also added configuration file for each option available in the generator

@elendil-software
Copy link
Contributor Author

elendil-software commented Sep 28, 2024

please take your time fixing the bug. let us know if you need help.

I have only one point I don't know how to solve a point. Here is the edited template :

{{#operation}}
public record {{operationId}}Request({{#allParams}}
    {{#isPathParam}}{{#required}}{{&dataType}}{{#isNullable}}?{{/isNullable}} {{nameInPascalCase}}{{/required}}{{/isPathParam}}
    {{#isQueryParam}}{{#required}}[property: QueryParam] {{&dataType}} {{nameInPascalCase}}{{/required}}{{/isQueryParam}}
    {{#isBodyParam}}{{#required}}[property: FromBody] {{&dataType}} {{paramName}}{{/required}}{{/isBodyParam}}
    {{#isFormParam}}{{#required}}{{&dataType}} {{nameInPascalCase}}{{/required}}{{/isFormParam}}
    {{#isHeaderParam}}{{#required}}[property: FromHeader] {{&dataType}} {{nameInPascalCase}}{{/required}}{{/isHeaderParam}}
    {{^-last}}{{^isCookieParam}}, {{/isCookieParam}}{{/-last}}{{/allParams}})
{
{{#allParams}}
    {{#isPathParam}}{{^required}}{{&dataType}}{{#isNullable}}?{{/isNullable}} {{nameInPascalCase}} {get; init; }{{/required}}{{/isPathParam}}
    {{#isQueryParam}}{{^required}}[property: QueryParam] {{&dataType}}{{#isNullable}}?{{/isNullable}} {{nameInPascalCase}} {get; init; }{{/required}}{{/isQueryParam}}
    {{#isBodyParam}}{{^required}}[property: FromBody] {{&dataType}}{{#isNullable}}?{{/isNullable}} {{paramName}} {get; init; }{{/required}}{{/isBodyParam}}
    {{#isFormParam}}{{^required}}{{&dataType}}{{#isNullable}}?{{/isNullable}} {{nameInPascalCase}} {get; init; }{{/required}}{{/isFormParam}}
    {{#isHeaderParam}}{{^required}}[property: FromHeader] {{&dataType}}{{#isNullable}}?{{/isNullable}} {{nameInPascalCase}} {get; init; }{{/required}}{{/isHeaderParam}}
{{/allParams}}
}
{{/operation}}
{{/operations}}

My problem is : how to avoid to insert a "," in the constructor if a parameter is the last parameter that is #required or if there is not #required parameter at all.

@wing328
Copy link
Member

wing328 commented Sep 28, 2024

My problem is : how to avoid to insert a "," in the constructor if a parameter is the last parameter that is #required or if there is not #required parameter at all.

what about using {{^-last}} ... {{/-last}}?

there are 2 more parameter lists for required and optional parameters:

would these help?

@devhl-labs
Copy link
Contributor

There is also a join with comma lambda.

@elendil-software elendil-software force-pushed the features/aspnet-fastendpoints-generator branch 2 times, most recently from a39a880 to fb5d45b Compare September 29, 2024 08:15
@elendil-software
Copy link
Contributor Author

My problem is : how to avoid to insert a "," in the constructor if a parameter is the last parameter that is #required or if there is not #required parameter at all.

what about using {{^-last}} ... {{/-last}}?

there are 2 more parameter lists for required and optional parameters:

* https://github.com/OpenAPITools/openapi-generator/blob/b01dd41ce354754421a2e0b9450d217eeaecf304/modules/openapi-generator/src/main/resources/elixir/api.mustache#L23

* https://github.com/OpenAPITools/openapi-generator/blob/b01dd41ce354754421a2e0b9450d217eeaecf304/modules/openapi-generator/src/main/resources/elixir/api.mustache#L27

would these help?

Perfect. Done.

@elendil-software
Copy link
Contributor Author

Not sure how to fix this error on action execution : https://github.com/OpenAPITools/openapi-generator/actions/runs/11091036118/job/30820196576#logs

@@ -1,7 +1,6 @@
 using FastEndpoints;
 
 using Org.OpenAPITools.Models;
-using Order = Org.OpenAPITools.Models.Order;
 
 namespace Org.OpenAPITools.Features;

To get the project built the line using Order = Org.OpenAPITools.Models.Order; need to be added manually. This is because both the model and FastEnpoint have a classe named Order

 -Project("{F0A8E8AF-E927-46D2-8D4C-2402A9559FBE}") = "Org.OpenAPITools", "Org.OpenAPITools\Org.OpenAPITools.csproj", "{11A46A2A-1367-4B2E-8FBA-121D75FB3192}"
+Project("{BB2B4493-3AE6-4AE7-BCC1-DA8D3C721BFC}") = "Org.OpenAPITools", "Org.OpenAPITools\Org.OpenAPITools.csproj", "{F25EF962-61BB-4618-82C7-FB72A5B7AB52}"

These GUID are created by the generator. I could give them a default value and let the user to choose if he want to generate new values

@wing328
Copy link
Member

wing328 commented Sep 30, 2024

FastEnpoint have a classe named Order

any convention in naming the FastEndpoint class?

In other server generators, we usually add "Api" as the suffix to avoid name collision with model names.

@elendil-software
Copy link
Contributor Author

FastEnpoint have a classe named Order

any convention in naming the FastEndpoint class?

In other server generators, we usually add "Api" as the suffix to avoid name collision with model names.

The convention are to add a "Request" suffix for the requests, which is what I have done. The "Response" suffix is used for the responses. That is what I have done too, but I revert it back. The reason is that when using the generator a model can also be used as the body of a request. And a request containing a property of type "SomethingResponse" didn't make any sense.

Maybe I could generate two models, one with a suffix "Response" and an other with, maybe, "Body" suffix. The drawback, I think, is that some classes could not be used at all.

@elendil-software
Copy link
Contributor Author

While checking again the required parameter and testing with a "real" open-api yaml file I noticed that some required parameters are not genereated as required.

I don't know if I made a mistake or miss something but looks like that parameter that are of form type are not included in the required parameters.

Here is a part of the yaml :

requestBody:
        content:
          application/x-www-form-urlencoded:
            schema:
              allOf:
                - $ref: '#/components/schemas/AlpacaRequest'
                - properties:
                    Action:
                      description: >-
                        A well known name that represents the action to be carried
                        out.
                      type: string
                    Parameters:
                      description: >-
                        List of required parameters or an Empty String if none are
                        required
                      type: string
                  required:
                    - Action
                    - Parameters

@elendil-software
Copy link
Contributor Author

FastEnpoint have a classe named Order

any convention in naming the FastEndpoint class?
In other server generators, we usually add "Api" as the suffix to avoid name collision with model names.

The convention are to add a "Request" suffix for the requests, which is what I have done. The "Response" suffix is used for the responses. That is what I have done too, but I revert it back. The reason is that when using the generator a model can also be used as the body of a request. And a request containing a property of type "SomethingResponse" didn't make any sense.

Maybe I could generate two models, one with a suffix "Response" and an other with, maybe, "Body" suffix. The drawback, I think, is that some classes could not be used at all.

I have tried, adding a suffix will do.
I only have a problem. How to handle the case when the model is used in a list.

if I do something like : public {{&dataType}}Model {{paramName}} { get; set; }
I get : public List<User>Model user { get; set; }
How can I get : public List<UserModel> user { get; set; } ?

@@ -152,3 +152,5 @@ org.openapitools.codegen.languages.TypeScriptRxjsClientCodegen
org.openapitools.codegen.languages.WsdlSchemaCodegen
org.openapitools.codegen.languages.XojoClientCodegen
org.openapitools.codegen.languages.ZapierClientCodegen

org.openapitools.codegen.languages.AspnetFastendpointsServerCodegen
Copy link
Member

Choose a reason for hiding this comment

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

please add this line based on alphabetical order

@wing328
Copy link
Member

wing328 commented Oct 3, 2024

I only have a problem. How to handle the case when the model is used in a list.

for model, i wouldn't recommend using a suffix by default

for endpoints, looks like adding a suffix like "Request" to avoid the naming collision with models if I understand it correctly.

@elendil-software
Copy link
Contributor Author

I only have a problem. How to handle the case when the model is used in a list.

for model, i wouldn't recommend using a suffix by default

for endpoints, looks like adding a suffix like "Request" to avoid the naming collision with models if I understand it correctly.

No, in this case, the collision is because both FastEndpoints and the generated project provide a type with the same name:

image

Generated requests and endpoints already have a suffix.

@wing328
Copy link
Member

wing328 commented Oct 4, 2024

ok. thanks for the details

2 ways to handle this kinda siutation:

  • use fully qualified name for FastEndpoints in the auto-generated code
  • add those names to reserved keyword list (e.g. Order will become ModelOrder, or users can use modelNameMapping option to name Order as something else, e.g. GeneralOrder)

@devhl-labs
Copy link
Contributor

devhl-labs commented Oct 4, 2024

Use a fully qualified name. We ought to be using fully qualified names everywhere, but we don't for some reason.

@elendil-software
Copy link
Contributor Author

ok. thanks for the details

2 ways to handle this kinda siutation:

* use fully qualified name for FastEndpoints in the auto-generated code

* add those names to reserved keyword list (e.g. Order will become ModelOrder, or users can use modelNameMapping option to name Order as something else, e.g. GeneralOrder)

I used a fully qualified name.
And I added option to set the GUID use in the solution file.

Hope everything works now.

@wing328 wing328 modified the milestones: 7.9.0, 7.10.0 Oct 8, 2024
@wing328
Copy link
Member

wing328 commented Oct 13, 2024

pull_request:
paths:
- samples/server/petstore/aspnetcore-8.0/**
- samples/server/petstore/aspnetcore-8.0-*/**
Copy link
Member

Choose a reason for hiding this comment

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

you will need to update the above 4 lines (path) referencing fastendpoints path mentioned below in order to trigger the CI to run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@elendil-software elendil-software force-pushed the features/aspnet-fastendpoints-generator branch from 94cf966 to 8bcd948 Compare October 13, 2024 17:59
@elendil-software
Copy link
Contributor Author

https://github.com/OpenAPITools/openapi-generator/actions/runs/11312510702/job/31462379731?pr=19690

please update the samples when you've time

Not sure how to solve that. Even after having updated the sample I still have 7.9.0-SNAPSHOTin VERSION files

This Guid need to be constant, it is related to project type
@elendil-software
Copy link
Contributor Author

https://github.com/OpenAPITools/openapi-generator/actions/runs/11312510702/job/31462379731?pr=19690
please update the samples when you've time

Not sure how to solve that. Even after having updated the sample I still have 7.9.0-SNAPSHOTin VERSION files

@wing328 Solved. It was an issue with my dev environment. OK after clearing the IDE cache.

@elendil-software
Copy link
Contributor Author

@wing328 Is there something more I need to correct on this PR ?

@wing328
Copy link
Member

wing328 commented Nov 7, 2024

https://github.com/OpenAPITools/openapi-generator/actions/runs/11392557217/job/32653087692?pr=19690

i'll fix it after merging this PR

UPDATE: done via 28e02fc

@wing328 wing328 merged commit 7deecdc into OpenAPITools:master Nov 7, 2024
17 of 19 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants