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

Epic: Use Asp.Net Core Api Explorer for Swagger generation #999

Closed
10 of 12 tasks
RicoSuter opened this issue Oct 14, 2017 · 18 comments
Closed
10 of 12 tasks

Epic: Use Asp.Net Core Api Explorer for Swagger generation #999

RicoSuter opened this issue Oct 14, 2017 · 18 comments

Comments

@RicoSuter
Copy link
Owner

RicoSuter commented Oct 14, 2017

Tasks:

  • Implement NSwag.SwaggerGeneration.AspNetCore project
  • Reference it and add new static methods in NSwag.AspNetCore (middlewares)
    • UseSwaggerCore() (experimental, with obsolete attribute) ?, or switch directly to new generator
    • Later: Remove NSwag.SwaggerGeneration.WebApi dependency (moved to v13, Epic: NSwag v13 #1700)
  • Implement new command “aspnetcore2swagger”
    • Same settings as webapi2swagger
    • Implement TestServer support, also with assembly loading etc.
    • Later: Automatically migrate nswag.json configs where IsAspNetCore == true to use this new command (must be done manually/explicitely)
  • Merge api-explorer into master
  • Documentation

Related:

PRs:

@RicoSuter
Copy link
Owner Author

cc @pranavkm \ @rynowak

@RicoSuter
Copy link
Owner Author

RicoSuter commented Nov 1, 2017

Open questions:

@rynowak
Copy link

rynowak commented Nov 1, 2017

Do we need to support this? #898

I'd suggest not supporting it.

We (ASP.NET Core team) strongly discourage anyone from using conventional routing MapRoute and friends for APIs. We don't support ApiExplorer/ApiDescription with actions that use conventional routing and we have no plans to.

And what about api versions? Is this exposed in ApiExplorer?

We currently have no built-in concept of API versioning, so we don't expose any information like that. We'd like to do something like this eventually but it keeps getting bumped down the priority list.

@RicoSuter
Copy link
Owner Author

Ok, then we leave it as is (reflection based).

@RicoSuter
Copy link
Owner Author

RicoSuter commented Nov 1, 2017

What about this?

I am thinking about these command line implementation - why cant we use TestServer to get the ApiExplorer/Description in the same process? If I understand you correctly, you want to start the application in another process and get the spec via HTTP? The problem with this approach is:

  1. You will use the NSwag version/generator installed in the web application and not the one of the command line application (users expect that if they install NSwag for example via NPM, then the generator binaries of the given NPM package version is used and not the one from the web app itself)
  2. What if the web application does not use NSwag (or Swashbuckle) at all? It would not be possible to generate a spec, but it should (this is a key feature of NSwag at the moment!)
  3. What if the web application has not registered the ApiExplorer and we cannot access it via dependency injection?

Using TestServer would solve all that, we could create a TestServer with the dynamically loaded Startup, register ApiExplorer if necessary, get the ApiDescriptions and feed it into a generator instance of the original process (i.e. the expected version).

@rynowak
Copy link

rynowak commented Nov 1, 2017

@pranavkm - can you answer the above? You are much more familiar with this CLI tools than I am

@pranavkm
Copy link
Contributor

pranavkm commented Nov 1, 2017

@RSuter the way TestServer works is that your unit test project references the application. Consequently, it transitively references the application's dependencies. This allows the test runner to execute the test in the context of the application's dependencies. We're essentially trying to do the same thing but without the benefit of having the user build \ compile the equivalent of a test application.

Here's how it's supposed to work:

  • The application references a build-time only package (commonly referred to as inside man) that will be the entry point for the command to call into. This entry point does the equivalent of the work TestHost does, setting up services and running NSwag.AspNetCore.SwaggerGeneration.
    Since the project references this package, NuGet \ dotnet has already reconciled it's dependencies in the context of the application. This means in .NET Core we don't have version conflicts and on desktop, the correct binding redirects have been generated.
  • Given the project, the aspnetcore2swagger command gathers metadata from the project. This includes the app's deps file and the location of the inside man. The command then does a dotnet exec on the inside man pointing to the app's deps file. This is the equivalent of saying running the application, but use the inside man as the entry point in lieu of the app's startup.

To answer your questions,

  • If I understand you correctly, you want to start the application in another process and get the spec via HTTP?

We wouldn't be launching a HTTP server, but passing in all the relevant arguments that are passed to the aspnetcore2swagger command to the inside man that executes as a console application.

  • What if the web application does not use NSwag (or Swashbuckle) at all? It would not be possible to generate a spec, but it should (this is a key feature of NSwag at the moment!)

I was currently prototyping a model where the application references the inside man package. This is the approach MvcPrecompilation, Scaffolding and dotnet-ef commands take. There might be a solution where the application does not have this reference (dotnet-xunit does this) but I'm not super familiar with the details of it's implementation. We can pursue this further if this scenario is important.

  • What if the web application has not registered the ApiExplorer and we cannot access it via dependency injection?

Having ApiExplorer registered would be a requirement for the tool. That said, in the standard scenarios where your app uses services.AddMvc(), it is registered by default.

  • You will use the NSwag version/generator installed in the application and not the one of the command line application (users expect that if they install NSwag for example via NPM, then the generator binaries of the given NPM package version is used and not the one from the web app itself)

I was attempting to do all of the work in the inside man, but one possibility would be to serialize the output of ApiExplorer to aspnetcore2swagger and have it generate the SwaggerDocument. I can play around with this approach if this is an important consideration.

@RicoSuter
Copy link
Owner Author

RicoSuter commented Nov 1, 2017

Thank you for your replies, I'm curious how this is implemented...

I think it is important that this works:

  1. If NSwag is not referenced at all in the web app
  2. With the version of the outer package/binary and not the one referenced by the project

...if this is even possible...

Also this should be easy to maintain and add extend in the future (e.g. add new parameters, etc.).

@RicoSuter
Copy link
Owner Author

You can implement the new command directly in NSwag.Commands - this package is usually not used directly in 3rd party apps but only in NSwag apps (eg nswag.npm, nswag.msbuild, nswagstudio, etc) and thus an added dependency should not make a big difference... it would be good if this serialization, communication infrastruction is independent of the command itself so that it could be reused in other projects (eg a signalr code generator, see https://github.com/RSuter/sigspec

@rynowak
Copy link

rynowak commented Nov 2, 2017

If NSwag is not referenced at all in the web app
With the version of the outer package/binary and not the one referenced by the project

We did a little brainstorming about this today and have a few ideas to try. We'll figure out if this is feasible and get back to you.

If this gets too far into the territory of implementing a totally custom loading behavior for dotnet, then I don't think this is something that either of us will want to live with long term. I suppose if we get there and find out this is too hard or fragile then we will want to have a discussion with you about alternatives.

@RicoSuter
Copy link
Owner Author

@rynowak Ok, this is fine. I'll let you experiment and will work on other parts of the project, so that everything is ready for the v12 release and all potential breaking changes can be incorporated in this release...

@pranavkm
Copy link
Contributor

pranavkm commented Nov 2, 2017

@RSuter I have a very WIP commit based on my initial design available as as PR #1034. I thought it might help since it's a little easier to visualize as code as opposed to set of bullet points. The bulk of the code that follows from the workflow is here - https://github.com/RSuter/NSwag/pull/1034/files#diff-61f8e53061cb1398e7b87f4177ccc8c2R147.

  • aspnetcore2swagger is part of NSwag.Commands and orchestrates launching the inside man.
  • In the current design, NSwag.Commands.AspNetCore is the inside man (the one referenced by the application) and does bulk of the work. The next thing I'm going to try out some of the ideas @rynowak and I discussed so this isn't referenced.
  • Much of the code is based on existing code in AspNetCore, so there's certainly room for cleanup and stylistic changes to match the repository's coding style. There's also commented out code for stuff I couldn't get immediately working, those will get fixed before I send a proper PR.

@RicoSuter
Copy link
Owner Author

Some sample Swagger 2.0 and OpenAPI 3.0 specifications can be compared here: https://github.com/RSuter/NSwag/tree/master/src/NSwag.Sample.NETCore20/Output

To update, just start the sample app and run Update-SwaggerSpecs.ps1.

@bugproof
Copy link

I used ApiExplorer based generator and it didn't generate the same output for generic controllers.

@RicoSuter
Copy link
Owner Author

@LeCoque can you provide a sample project?

@piaste
Copy link

piaste commented Oct 31, 2018

In the WebApiToSwaggerGenerator, it is possible to pass a list of controller types to the .UseSwagger(...) method, and only those controllers are passed to the Swagger generator and published.

This can be an important feature because you can have some controllers in the assembly that you only expose depending on some runtime options (using .AddControllersAsServices()). You don't want to expose a Swagger documentation for a controller that isn't actually online, and you can't use [<SwaggerIgnoreAttribute>] because the decision is made during initialization.

Would it be possible to have the .UseSwagger[..]WithApiExplorer() methods accept a similar list for runtime filtering purposes (optional, of course)? If there is no better approach, I could write and submit a PR that simply checks for such a list inside GenerateForControllerAsync,

@RicoSuter
Copy link
Owner Author

@piaste in the new generator you need to filter operations with an operation processor (return false to exclude an operation) and insert it at the beginning of the DocumentProcessors list setting (so it's filtered at the beginning). see https://github.com/RSuter/NSwag/wiki/Document-Processors-and-Operation-Processors

@piaste
Copy link

piaste commented Oct 31, 2018

@RSuter Thanks, that works. I didn't need to add it to DocumentProcessors either, OperationProcessors was enough.

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

No branches or pull requests

5 participants