Support header parameters for CSharp controller code generation #2033
Conversation
… - for now, should work in both core & .net framework generation (even though redundant in core).
|
I think it's imporant to first fix the web api vs asp.net core story and then generate which works for each framework... it seems to me that these custom attributes are web api only, right? There should be a check so that they are only generated for web api... |
|
I was working on the assumption that the Saying that, if we had a way to tell the template if we're using Core it would be pretty straightforward to switch the behaviour: this works fine using the built-in |
|
If you did decide to drop support for Web API, I wondered about making use of custom templates as a workaround for my use case... I've just been exploring the "Template Directory" setting (following your comment): providing an alternative version of Controller.liquid works as I would expect (despite the caution to only override empty/extension templates?). The only thing I would specifically need from this PR would be the extra properties on |
|
Sorry for the delay, see #1965 (comment) Will ping here when it’s ready |
|
Added PR with new ControllerTarget setting: #2064 |
|
Great - thanks for your work on this @RicoSuter . I'll hopefully get my PR revised over the next week or so! |
…ntroller generation (now we have IsAspNetCore available in template). Added test coverage.
…rGenerator now does this via a new template that renders once before all controllers are rendered
…ng (removed previous using alias)
…and now appears after all controllers (added some new properties to model & ClientGeneratorBase to enable this)
|
@RicoSuter: having figured out the concerns around multiple controller generation I've made a few changes to this PR in line with your comments:
|
…change controller generation option - now groups related actions together in separate controllers. Replaced current single implementation class with 4 separate ones. Controller.liquid template now largely follows default (@ v12.2.5) - except implemenation interfaces are renamed. Relies on changes to NSwag submitted here: RicoSuter/NSwag#2033
…rp-controller-code-generation
|
I've updated the code to use code artifacts (used for multiple file output later) |
|
Having digested your changes it looks like that approach fits much better with the general approach of the code: I hadn't fully appreciated the |
No worries - this whole CodeArtifact was not really incorporated yet - I just took this as an opportunity to add it and change it everywhere... Everything looks fine now except some additional line breaks etc. will merge soon. Is your feature still working as expected? |
|
I'll give it another try if you've finished integrating - at the last try it looked good. |
|
Ok. Now everything should be ready... with your go I'll merge |
|
I've got it building, the tests are passing and my code gen project is running OK - so fine by me: thanks for you help! |
|
v12.3.0 |
Work in progress - this fixes #1956 (and possibly #642?). I've not looked at test coverage yet, and have done a limited range of manual testing.
Essentially I'm creating a
FromHeaderAttributethat handles binding of header-based parameters. I've only tested in a framework web project so far, but in theory this should happily co-exist with the one available in ASP.NET Core for now. Looking at the discussion around how Core should be supported with the templates, I wonder whether this approach might be sufficient until that is resolved; maybe then only including the custom attribute/binding when the proposedIsAspNetCoresetting is false?