Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Is it possible to generate a client method executing a GET request with a filter parameter having a complex data type? #3301

Open
ViRuSTriNiTy opened this issue Feb 12, 2021 · 12 comments

Comments

@ViRuSTriNiTy
Copy link

ViRuSTriNiTy commented Feb 12, 2021

Hi there,

my controller currently looks like this:

[Route("/api/[controller]")]
public class MyController
{
    ...
    [HttpGet]
    public async Task<ActionResult<...>> ListAsync([FromQuery] MyFilterModel filter)
    {
        ...
    }
}

The MyFilterModel is defined as

public class MyFilterModel 
{
    public string Name { get; set; }
    public string Description { get; set; }
}

Now when I generate a C# client from this controller via NSwag the resulting method signature looks like this

Task<...> ListAsync(string name, string description)

Is it possible to generate a method signature that uses the filter model class instead like

Task<...> ListAsync(MyFilterModel filter)

I am asking because currently calling ListAsync("foo", "bar") in multiple places has some drawbacks like you would need to revisit and change these calls when MyFilterModel is changed and its not pretty readable at the first glance. I would rather like to call it like ListAsync(new MyFilterModel { Name = "foo", Description = "bar" }).

Perhaps it is just not possible at the moment due to the still ongoing work for issue #1594 which is part of epic #945.

So lonG

Daniel

@ViRuSTriNiTy ViRuSTriNiTy changed the title Is it possible to generate a client method with complex Is it possible to generate a client method executing a GET request with a filter parameter having a complex data type? Feb 12, 2021
@RicoSuter
Copy link
Owner

This is currently not possible...

I think the main "problem" is that OpenAPI/Swagger has no notion to describe that directly - so we'd need some custom attributes to be able to generate complex objects from flat parameters.

@ViRuSTriNiTy
Copy link
Author

ViRuSTriNiTy commented Feb 15, 2021

Thanks for the reply. I was referring to the generated C# code, not the underlying Open API part. My hope is that the C# part can be changed somehow to provide a method signature with the complex datatype. The implementation of this method could still use the GET request as is, so it has to handle the mapping from complex data to query string used in the GET request a bit differently. I don't know if this is feasible but it would be nice to change the C# call to a more "friendly" version.

@jon-zu
Copy link

jon-zu commented Feb 15, 2021

@RicoSuter What do you think about the option to automatically generates a class If there are n(set via settings) or more query parameters.

@ViRuSTriNiTy
Copy link
Author

@JonasZ95 Can you provide an example? I'm not sure I can follow your suggestion.

@jon-zu
Copy link

jon-zu commented Feb 17, 2021

@ViRuSTriNiTy my idea is that we add a new parameter to the code generator like MaxNonClassQueryParameters(like default to 3 or maybe null=disabled so we don't break existing configs) and then just generate a class for the query parameters. My idea is:

  1. Preprocess the ApiDocument such that you can generate a Set with all Operations which have n or more query parameters
  2. When the Operation is in the Set replace all Parameters by something like type={OperationId}Dto name={OperationIdParams}(https://github.com/RicoSuter/NSwag/blob/master/src/NSwag.CodeGeneration.TypeScript/Models/TypeScriptOperationModel.cs#L49) and use some prefix for using the parameters in the function.
  3. Iterate through the set and add a class + interface for each Operation Parameter List to the output

I think this will work fine(at least for Typescript) as long the Parameters are flat.

@ViRuSTriNiTy
Copy link
Author

ViRuSTriNiTy commented Feb 17, 2021

@JonasZ95 Thanks for the explanation, it guided me in the right direction. Btw my initial post was about C# code generation not TypeScript. But as far as I understand all classes inherit basic functionality from base classes and language specific stuff is implemented in parent classes, hence we have somewhat the same behaviour in the generation department for all supported languages.

Anyway, after digging in the source code I'm pretty sure your suggestion is doable but I think it would be easier to somehow replace the current implementation of NSwag.CodeGeneration.ClientGeneratorBase.GetOperations().

The current implementation creates one CSharpOperationModel per parameter stated in the OpenAPI document and since simple parameters are generated into the document for GET operations the resulting C# also contains simple parameters.

If we could somehow create a single CSharpOperationModel that describes all parameters of a OpenAPI operation in a case of a specific GET operation then passing around this single model would result in the same behaviour and code generation flow as for POST methods for example.

What do you say?

@ViRuSTriNiTy
Copy link
Author

ViRuSTriNiTy commented Feb 18, 2021

I have a prototype running and it works! 🎉

Customizing the ClientGeneratorBase does the trick. All I had to do is to replace ClientGeneratorBase.cs Line 1639

var operationModel = CreateOperationModel(tuple.Operation, BaseSettings);

with some code inspired by an existing code part in ControllerGenerationFormatTests.cs like

var openApiOperation = tuple.Operation;
if (openApiOperation.OperationId == "MyController_List")
{
    var complexTypeSchema = new JsonSchema();
    complexTypeSchema.Title = "MyFilterModel";
    foreach (var p in openApiOperation.ActualParameters)
    {
        complexTypeSchema.Properties[p.Name] = new JsonSchemaProperty {
            Type = p.ActualTypeSchema.Type, IsRequired = p.IsRequired };
    }

    var newOpenApiOperation = new OpenApiOperation
    {
        OperationId = openApiOperation.OperationId,
        Parameters = {
            new OpenApiParameter {
                Name = "filter",
                IsRequired = false,
                Kind = OpenApiParameterKind.Body,
                Type = JsonObjectType.Object,
                Reference = complexTypeSchema
            }
        }
    };

    foreach (var response in openApiOperation.Responses)
    {
        newOpenApiOperation.Responses.Add(response.Key, response.Value);
    }

    document.Paths["/" + tuple.Path][tuple.HttpMethod] = newOpenApiOperation;

    openApiOperation = newOpenApiOperation;
}

var operationModel = CreateOperationModel(openApiOperation, BaseSettings);

For sure this needs tweaking to support edge cases and it has to be configurable somehow. Any ideas?

@RicoSuter Is this a feature / change you would accept? How do I proceed to get this integrated?

@jon-zu
Copy link

jon-zu commented Feb 19, 2021

@ViRuSTriNiTy Can you show what code this generates? From what I see the newly added Parameter will be encoded as JSON body.

@Shaddix
Copy link

Shaddix commented Oct 9, 2021

I also faced the same issue, the main problem for me is that when you add new parameter to existing GET request, regenerated CSharp client is not compatible with previous code. So you need to go and change all previously written Client calls, which is really a pain, especially if newly added parameter is optional.

My guess is that it'd be great to always generate two overloads for GET methods: one with list of parameters (as it is now), and another one with a single parameter which is a class/struct with all parameters as properties.

That way the new versions will still be backwards compatible, but also the new possibility to use a class for GET requests would be clear.

@Shaddix
Copy link

Shaddix commented Nov 14, 2021

If anyone's interested, I implemented it using a custom liquid template.
You could take the folder, and run nswag pointing to it (/templateDirectory:PATH).

It will add another overload to Client for all GET requests having at least 1 parameter.
E.g. for initial method like this:
public Task<ResultDto> GetSomething(string searchQuery, bool? filter1, int filter2)
it will generate:

public Task<ResultDto> GetSomething(ControllerClientGetSomethingParametersDto parameters)
public class ControllerClientGetSomethingParametersDto  {
  public string searchQuery {get; set;}
  public bool filter1 {get; set;}
  public int filter2 {get; set;}
}

@luizfernando1996
Copy link

luizfernando1996 commented Apr 5, 2022

Hello @RicoSuter ,
I want this feature that is disponible in OpenApi 3.0. So, I guess I will implement this and create a pull request for you. However, can you help me saying some words that help me in implement this feature? I don't know nothing about nswag implementation and how I should send for you my pull request. I want this feature just for typescript client but if I implement I will should implement for all languages? In this project we have a pattern for pull requests or not?

Reference: #3301 (comment)

@ramax495
Copy link

Is there any solution with TypeScript ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants