Codegen: Possibility to combine method arguments into a single object

[akamyshanov]

For example, the C# code below

public class FooQuery
{
  public string Bar { get; set; }
  public string Baz { get; set; }
  public string FooBar { get; set; }
  public string FooBaz { get; set; }
  public int? FooBarBaz { get; set; }
}

public class FooController
{
  [HttpGet, Route]
  public ICollection<Foo> ReadAll([FromUri]FooQuery query = null)
  {
    // ...
  }
}

generates the following TypeScript method:

export class FooApi {
  readAll(bar: string | null, baz: string | null, fooBar: string | null, fooBaz: string | null, fooBarBaz: number | null): Observable<Foo | null> {
    // ...
  }    
}

Clearly, the generated method is cumbersome and error-prone to use and code using it will break if we add another property to FooQuery.

My proposal is to generate a FooApiReadAllQuery class when there are >1 arguments and use that class as an argument to the method:

export class FooApiReadAllArgs {
  bar: string | null;
  baz: string | null;
  fooBar: string | null;
  fooBaz: string | null;
  fooBarBaz: number | null;
}

export class FooApi {
  readAll(query: FooApiReadAllArgs): Observable<Foo | null> {
    // ...
  }
}

This will:

• Make adding new arguments to the API controller backward-compatible with existing code
• Make such methods easier and safer to use

Is such an implementation welcome as a pull request in this repo? It seems that it's relatively easy to this to the templates and configuration.

[akamyshanov]

Okay, write first, search second 😄

Found this: https://github.com/RSuter/NSwag/pull/589

If configuring the behaviour is implemented to avoid breaking changes, would that pull request be merged?

Upd If I am not mistaken, the pull request above is not as flexible as creating a separate *Args class due to the fact that the consuming code would still have to specify all the arguments.

[RicoSuter]

Generally, I think this is a good idea. However we need to implement this in a way that it does not break existing code and that it is as flexible as possible... And if possible, for CSharp and TypeScript.

[RicoSuter]

The main problem with complex FromUri parameters is that we lose the query class information in swagger (its not supported). We also could add a custom property to the swagger parameter (eg "x-group": "FooQuery") so that we can group this in the client generator and generate a group/query class for the same parameters and with the correct name. This will only work if the spec is generated with nswag...

[akamyshanov]

However we need to implement this in a way that it does not break existing code and that it is as flexible as possible

Of course, this should be a configuration switch and disabled by default.

The main problem with complex FromUri parameters is that we lose the query class information in swagger (its not supported)

I do not think that the query class information is needed. NSwag should just take all the method's parameters and wrap them in a generated class. Regardless whether the controller method wraps its arguments or not.

I will try to take a look at that pull request shortly.

[RicoSuter]

And what will the name of the argument/query class be? This should also be configurable, eg "{operation}Request"

[RicoSuter]

Another problem: This is a feature which currently must be implemented in all TS templates *ClientTemplate.tt (change parameters) and the RequestUrlTemplate.tt (load data from obj instead of param)... We have to keep the templates as simple as possible.

I try to consolidate and improve the templates in the future and also to switch to another template engine (DotLiquid?) for better extensibility (more smaller templates which can be exchanged if needed), but adding more and more features makes everything more complex. Nonetheless I think this is a good feature.

I think its important to discuss the implementation for this feature first before we begin to implement it.

[akamyshanov]

And what will the name of the argument/query class be?

My proposal is concatenation of:

• Client class name
• Method name
• "Args"

Very doubtful that there could be a naming conflict this way.

Example:

[mikoskinen]

This would be great and the way @akamyshanov describes it should work just nicely. Does there happen to be any plans/roadmap for this?

[feafarot]

Any updates on this feature? It's really painful to manage 20+ parameters.

[goldsam]

This would really be a great feature when implementing collection endpoints with lots of filter parameters.

[k290]

Bumping because of the amount of times we've added a new parameter and everywhere its used breaks at runtime.

[csutorasr]

This should be added as an option. Then nothing breaks and everyone caan use the new feature.

[DevonAleshire]

Need this feature implemented to help create more generic TS functions when using NSwag generated API clients.

Would love to see this added as an option.

[fabich]

@akamyshanov @RicoSuter any updates on this?

[LowieDelneste]

@RicoSuter Any news on this?

[bsssshhhhhhh]

+1, this would be a profound improvement to an otherwise great tool.

For now I ended up writing wrapper functions that take in the params as an object so I can use the NSwag-generated TypeScript client without having to update every place the method is called every time a new parameter is added.

[vdurante]

Any news on this?

[RicoSuter]

This is quite some work and currently i do not have the requirement in my professional work...

The idea should be to add a new setting to choose between the current behavior (default) and generating a wrapper class per operation and generate it with a single parameter.

[jeremyVignelles]

What if someone what this behavior per-operation?

I think it would only increase complexity, with the bugs that come along.

At some point, I think that we would need some kind of "generator processor" that one would be able to write and generate what they want? I can't think of how that could work, but adding more and more options raises complexity exponentially...

[RicoSuter]

At some point, I think that we would need some kind of "generator processor" that one would be able to write and generate what they want? I can't think of how that could work, but adding more and more options raises complexity exponentially...

I agree. I think with own templates you can already achieve that if you need it... In most cases having more than 2-3 params is bad API design and you should consider using a JSON POST instead anyway...

[jeremyVignelles]

Templates are good, but it's hard to change something in a future-proof way (what if the behavior of NSwag's default template change? you would need to port the changes in your template, and that could be quite tedious...)

having some kind of AST that you could manipulate would be a better way to "fix" things without interfering with the NSwag updates most of the time.

[RicoSuter]

having some kind of AST

Yep, like using Roslyn to generate code instead of liquid templates? Cool idea because it would potentially give you many extension points but would be an essential rewrite of the code gen :-)

[jeremyVignelles]

Indeed. Roslyn would be cool, but I don't know how we could do with Typescript

[RicoSuter]

but I don't know how we could do with Typescript

They also have some AST compiler, but we'd need to write it in TS, etc... This would be huge and would need another technology per output language :-)

[jeremyVignelles]

Isn't clang able to gen them all? That won't be for today anyway, just a possibility.

[fncap]

My 2 cents on that ... Actually generators already create a Wrapper Class for HTTP POST endpoints. The solution could be a check like following:

If endpoint is an HTTP GET and arguments are more than one, then use same generator as per HTTP POST endpoint, but leaving it as a POST verb; instead, if endpoint is an HTTP GET and had only one argument, then use the default generator.

Make sense for you?

[johan-v-r]

Seems like the original feature was to wrap only query parameters in an object. That's applicable to all HTTP verbs right.

Leaving route & body parameters as individual function arguments (like what they currently are) should relieve some complexity, as there's a clear distinction between those types.

Generally you wouldn't have loads of route params anyway, and they rarely ever change - instead a change would indicate a completely new route in my mind. Query params on the other hand are often optional and can easily expand on existing routes. This where 90% of the pain comes in.

[terrencejames]

Any updates on this?

[pinkfloydx33]

This is sorely needed for us. We have search/list endpoints that support a variety of search/filtering query parameters. I don't think that it's appropriate for that to be a POST request nor do I think there's a "right" number of parameters.

An object generated for query parameters would:

• make calling the method easier with many arguments.
• would be (more) future proof

The last point is important. Currently, if you reorder the properties on your C# query DTO, the generated search parameters will be in a different order. Adding a new parameter in the middle could break consuming code entirely (no compilation). What's worse is if the reordered parameters have the same underlying type--you could introduce a silent breaking change that would still compile. (We use swagger to generate the API spec fed into nswag, perhaps this isn't an issue if using nswag outright)

[Macrox14]

No updates?

[SudoZachCampbell]

Though it might seem like an unimportant issue (given it hasn't been solved in nearly 6 years), this blocked our potential implementation of NSwag when this was noticed during the initial spike. This adds a serious domino effect to the API for every endpoint that consumes the query params, and that level of maintenance didn't fly so until it can be solved our potential commercial usage has been dropped.

[luca-domenichini]

@SudoZachCampbell just for my information, which alternative/workaround did you use?

[SudoZachCampbell]

We currently use an in-house type generator that fulfils our requirements but isn't fully openapi spec compliant, hence the search for an alternative vs reworking the tool to be compliant instead

[pinkfloydx33]

I ended up writing a custom MSBuild task that's run after the nswag generator msbuild task. As arguments, it's fed a list of operationIds and it parses the nswag generated output to find the corresponding methods. It then turns all of the arguments (sans cancel token) into a single parameter along with a type to go with it (auto named, but also configurable). It works for our scenario and I'm happy to share it if anyone is interested.

[luca-domenichini]

@pinkfloydx33 yes, I would really appreciate!

[HanMoeHtet]

Currently if I change positions in Query class in C#, in Angular client, parameter positions will change, which is very hard to notice since we won't get any compiler errors. Can't believe this critical feature is missing in a code generator.

[eulergkb]

Looks like @RicoSuter hasn't made any plans to do it so kindly use this package: https://www.npmjs.com/package/swagger-typescript-api instead for generating ts client

[Matster2]

I ended up writing a custom MSBuild task that's run after the nswag generator msbuild task. As arguments, it's fed a list of operationIds and it parses the nswag generated output to find the corresponding methods. It then turns all of the arguments (sans cancel token) into a single parameter along with a type to go with it (auto named, but also configurable). It works for our scenario and I'm happy to share it if anyone is interested.

Could you share the code? any work around for this issue would be massive. All of our query functions are huge right now with so many filter params