Use new Microsoft.Extensions.ApiDescription.Server package #2226

dougbu · 2019-06-10T23:40:55Z

provides document download on build and aligns with Microsoft.Extensions.ApiDescription.Client p2p references
- this feature is on by default
- can be disabled using <OpenApiRetrieveDocuments>false</OpenApiRetrieveDocuments>
- timing can be changed using <OpenApiRetrieveDocumentsAfterBuild>false</OpenApiRetrieveDocumentsAfterBuild>
  - then, depend on the RetrieveOpenApiDocuments target from somewhere else
add GetDocumentNames() to IDocumentProvider and AspNetCoreOpenApiDocumentGenerator
rename IDocumentProvider namespace Microsoft.Extensions.ApiDescription -> Microsoft.Extensions.ApiDescriptions
- avoid conflict with Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription type
depend on Microsoft.Extensions.ApiDescription.Server package from NSwag.AspNetCore package
- restore and update the previously-unused (then deleted) NSwag.AspNetCore.nuspec file

nits:

remove and sort usings
take VS suggestions in NSwagServiceCollectionExtensions
add missing NSwagServiceCollectionExtensions copyright header
correct spelling in AspNetCoreOpenApiDocumentGenerator

dougbu · 2019-06-11T02:16:40Z

src/NSwag.AspNetCore/NSwag.AspNetCore.nuspec

+        <description>$description$</description>
+        <tags>Swagger Documentation WebApi AspNet TypeScript CodeGen</tags>
+        <projectUrl>https://github.com/NSwag/NSwag</projectUrl>
+        <licenseUrl>https://github.com/NSwag/NSwag/blob/master/LICENSE.md</licenseUrl>


@RicoSuter does it matter (much) that I'm using old GitHub locations for this repo i.e. should projectUrl be https://github.com/RicoSuter/NSwag ?

We should use the correct URL, why using the old one?

This matches what I see in your released packages. I'll update.

dougbu · 2019-06-11T16:38:01Z

@RicoSuter please let me know if this needs to change at all.

FYI our 3.0 Preview 6 release should be soon.

RicoSuter · 2019-06-11T16:42:08Z

@dougbu does this have to be part of the v13.0.0 release? We can push this later, right? This all is WiP anyway, correct?

dougbu · 2019-06-11T16:55:35Z

@RicoSuter this will only be WIP for a few more days (I think) and I'd really like to see it in your next release.

RicoSuter · 2019-06-11T16:58:31Z

@dougbu is it ok to do a patch release (v13.0.1) with this in a few days? I really want to get this version out :-)

dougbu · 2019-06-11T17:02:16Z

Sure❕ I wasn't directing, just asking 😺

RicoSuter · 2019-06-11T18:51:10Z

Do we really need to use a nuspec file? I dont like this as it’s quite bit harder to maintain deps etc..

dougbu · 2019-06-11T18:53:15Z

@RicoSuter it's not possible to add a dev dependency to a shipped package without the .nuspec file. W.r.t. maintaining dependencies, I tried to minimize the issues by keeping all of the versions in the .csproj file.

RicoSuter · 2019-06-11T19:26:49Z

provides document download on build and aligns with Microsoft.Extensions.ApiDescription.Client p2p references

Does this mean that the spec is generated on build by default when the NSwag.AspNetCore package is installed.
This might be a bit dangerous to enable by default because the generation might take quite some time for bigger apis.

rename IDocumentProvider namespace Microsoft.Extensions.ApiDescription -> Microsoft.Extensions.ApiDescriptions

Does this also work when multiple IDocumentProviders are registered, e.g. also from Swashbuckle or another document generator like GRPC or AsyncApi?

depend on Microsoft.Extensions.ApiDescription.Server package from NSwag.AspNetCore package

Isn't this a breaking change, e.g. for "older" target frameworks?

dougbu · 2019-06-11T22:06:57Z

Does this mean that the spec is generated on build by default when the NSwag.AspNetCore package is installed.

Yes, it's on by default.

This might be a bit dangerous to enable by default because the generation might take quite some time for bigger apis.

Can be disabled using <OpenApiGenerateDocuments>false</OpenApiGenerateDocuments>.

Does this also work when multiple IDocumentProviders are registered, e.g. also from Swashbuckle or another document generator like GRPC or AsyncApi?

IDocumentProviders are limited to OpenAPI document providers i.e. will only available in the NSwag.AspNetCore and Swashbuckle.AspNetCore packages. If a project uses both of those packages, the dotnet-getdocument tool in Microsoft.Extensions.ApiDescription.Server will choose the first assembly it finds in the application containing an IDocumentProvider definition. This is silent because it seems to be a test-only scenario.

Isn't this a breaking change, e.g. for "older" target frameworks?

Not really. The dotnet-getdocument tool supports all supported .NET Core versions (2.1 and greater). If users choose to use the latest NSwag.AspNetCore in older projects, they also can use <OpenApiGenerateDocuments>false</OpenApiGenerateDocuments>. I'll auto-disable this feature for downlevel releases in our next 3.0 preview to save them the minor trouble. Will also document this property for exactly this use case in the meantime.

dougbu · 2019-06-12T21:07:55Z

3.0 Preview 6 packages are now public. This PR can be safely merged, perhaps with a release note mentioning <OpenApiRetrieveDocuments>false</OpenApiRetrieveDocuments> for .NET Core 2.0 and earlier web projects using the latest NSwag.AspNetCore

RicoSuter · 2019-06-13T07:44:07Z

If I only install nswag.aspnetcore, will it still auto gen the specs even if they are not used anywhere? Or what is this feature for?

dougbu · 2019-06-13T16:12:03Z

Yes, it will get the documents and save them in obj/ by default. The feature is for both API compatibility checks (when default doc location is overridden and user chooses to check the files in) and consumption in client projects using @(OpenApiProjectReference) items.

dougbu · 2019-06-13T16:45:30Z

🆙📅 to rebase on 'master' and change all GitHub URLs to avoid the extra redirects

RicoSuter · 2019-06-13T16:52:31Z

Yes, it will get the documents and save them in obj/ by default

I don't like this as it means that I still pay the performance impact even if I'm only interested in the HTTP middlewares (specs + web UI) and do not store the documents in the filesystem or use any @(OpenApiProjectReference)... Can't we disable it by default and only enable it if the output path is set (i.e. it should end up in the filesystem) or if a project uses @(OpenApiProjectReference) (probably harder to find out)?

The feature is for both API compatibility checks (when default doc location is overridden and user chooses to check the files in)

Compatibility checks = doing a diff in the PR?

dougbu · 2019-06-13T17:01:07Z

Can't we disable it by default and only enable it if the output path is set (i.e. it should end up in the filesystem) or if a project uses @(OpenApiProjectReference) (probably harder to find out)?

That's not viable because @(OpenApiProjectReference) will silently fail if the feature is opt-in. I could add an error if the feature is disabled when the client project calls the OpenApiGetDocuments target but that won't happen until preview 7 is released and feels user-violent.

Users concerned about performance or just don't want the docs can use <OpenApiGenerateDocuments>false</OpenApiGenerateDocuments>. They can also use <OpenApiGenerateDocumentsOnBuild>false</OpenApiGenerateDocumentsOnBuild> if they only want to update the documents explicitly or as part of some other target.

Compatibility checks = doing a diff in the PR?

Yes

RicoSuter · 2019-06-13T19:28:18Z

... and feels user-violent.

For me it feels user-violent that when you install a package like NSwag.AspNetCore the build immediately takes much longer (I have big APIs which take > 20s) and you are not even using a feature of it. I think 80% of the users only want the Swagger UI + spec server (only generated on demand when accessing /swagger) and will not use spec generation to file or service project references... With this you immediately get the penalty (because no user will know of this new option) and the feature is not even usable...

Wouldnt it be better if we create a new package, e.g. NSwag.AspNetCore.Build (or whatever?) which has this enabled etc. and which references the original NSwag.AspNetCore package?

dougbu · 2019-06-13T19:33:51Z

The idea is to enable the document generation by default exactly so that code generation works end-to-end. There’s no way to know if there’s another project depending on this one until `OpenApiGetDocuments` is invoked. * the feature is not even usable What do you mean? With the changes in this PR, everything works end-to-end. From: Rico Suter <notifications@github.com> Sent: June 13, 2019 12:28 To: RicoSuter/NSwag <NSwag@noreply.github.com> Cc: Doug Bunting (AAPT) <Douglas.R.Bunting@microsoft.com>; Mention <mention@noreply.github.com> Subject: Re: [RicoSuter/NSwag] Use new Microsoft.Extensions.ApiDescription.Server package (#2226) ... and feels user-violent. For me it feels user-violent that when you install a package like NSwag.AspNetCore the build immediately takes much longer (I have big APIs which take > 20s) and you are not even using a feature of it. I think 80% of the users only want the Swagger UI + spec server (only generated on demand when accessing /swagger) and will not use spec generation to file or service project references... With this you immediately get the penalty (because no user will know of this new option) and the feature is not even usable... Wouldnt it be better if we create a new package, e.g. NSwag.AspNetCore.Build (or whatever?) which has this enabled etc. and which references the original NSwag.AspNetCore package? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FRicoSuter%2FNSwag%2Fpull%2F2226%3Femail_source%3Dnotifications%26email_token%3DABRCFPOGA46T55ZXDIVM2VLP2KNVRA5CNFSM4HWZMRNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXUZE2I%23issuecomment-501846633&data=02%7C01%7CDouglas.R.Bunting%40microsoft.com%7C5c93f4afc6134615747f08d6f0354e0e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636960509071693159&sdata=07tkRMAFfycREFfP6FwAVDhzgIOtJf0c4mFjNHFK6G0%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABRCFPJO4WKL4I5ETY7S723P2KNVRANCNFSM4HWZMRNA&data=02%7C01%7CDouglas.R.Bunting%40microsoft.com%7C5c93f4afc6134615747f08d6f0354e0e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636960509071693159&sdata=Pog5NhT0ew3jQhvY17IRsNshm37XZn5aIY%2FSkKIBWYY%3D&reserved=0>.

RicoSuter · 2019-06-19T18:01:05Z

NSwag.AspNetCore will then depend on a preview package, isn't this a problem?

dougbu · 2019-06-19T18:05:55Z

NSwag.AspNetCore will then depend on a preview package, isn't this a problem?

No more than NSwag.ApiDescription.Client depending on a preview package -- just leads to another warning in your build that can be ignored. In any case, the feature is code compete at this point though it has a couple of rough edges we'll fix in 3.0 preview 7.

@glennc and I'll discuss moving from 0.3.0 to 3.0.0 for these packages in preview 7 (which might be closer to release candidate).

RicoSuter · 2019-06-19T18:14:22Z

I just want to ensure that this does not break existing users - not everyone is using the latest versions of .NET Core 3...

dougbu · 2019-06-19T18:15:54Z

No, it won't break existing users, especially because we're switching the feature to be opt-in as you requested. Will update this PR to that end soon.

RicoSuter · 2019-06-19T18:16:53Z

especially because we're switching the feature to be opt-in as you requested.

This all is preview at the moment anyway - so enabling it by default should be discussed later I think...

RicoSuter · 2019-06-20T18:54:08Z

I’ve changed the versions to v13.0.2 and as soon as you updated the pr i’ll merge and release it.

dougbu · 2019-06-21T00:17:57Z

Thanks @RicoSuter! I've been laid up since yesterday and might not get to this 'til early next week.

dougbu · 2019-06-23T20:38:56Z

🆙📅 to rebase on 'master' (removing conflicts) and change doc generation to opt-in

RicoSuter · 2019-06-23T20:46:12Z

This pr might interfere with #2261

dougbu · 2019-06-23T20:50:04Z

@RicoSuter yes it will conflict. If you and @mkArtakMSFT like, I could add <IncludeOpenAPIAnalyzers>true</IncludeOpenAPIAnalyzers> in this PR. The alternative is to pare #2261 down to just that addition.

mkArtakMSFT · 2019-06-23T21:42:28Z

We can absolutely simplify this by including the flag in this PR.
@RicoSuter it's up to you!
I vote for simplicity.

dougbu · 2019-06-23T23:08:50Z

@RicoSuter I added the property setting to this PR. I'll remove this last commit if you prefer to separate the concerns.

- provides document download on build and aligns with Microsoft.Extensions.ApiDescription.Client p2p references - this feature is off by default (though Microsoft.Extensions.ApiDescription.Server features are normally opt-out) - can be enabled using `<OpenApiRetrieveDocuments>true</OpenApiRetrieveDocuments>` - changes to `$(OpenApiDocumentsDirectory)` also enable the feature - timing can be changed using `<OpenApiRetrieveDocumentsAfterBuild>false</OpenApiRetrieveDocumentsAfterBuild>` - then, depend on the `RetrieveOpenApiDocuments` target from somewhere else - add `GetDocumentNames()` to `IDocumentProvider` and `AspNetCoreOpenApiDocumentGenerator` - rename `IDocumentProvider` namespace Microsoft.Extensions.ApiDescription -> Microsoft.Extensions.ApiDescriptions - avoid conflict with `Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription` type - depend on Microsoft.Extensions.ApiDescription.Server package from NSwag.AspNetCore package - restore and update the previously-unused (then deleted) NSwag.AspNetCore.nuspec file nits: - remove and sort `using`s - update remaining 13.0.0 version - take VS suggestions in `NSwagServiceCollectionExtensions` - add missing `NSwagServiceCollectionExtensions` copyright header - correct spelling in `AspNetCoreOpenApiDocumentGenerator`

- https://github.com/NSwag and https://github.com/RSuter just add an extra hop to https://github.com/RicoSuter

- see also RicoSuter#2261

dougbu · 2019-06-23T23:19:37Z

🆙📅 again to split concerns more clearly into 3 commits

RicoSuter · 2019-07-11T11:28:32Z

@dougbu Can you point me to the docs with the available msbuild tags, e.g. OpenApiRetrieveDocuments or OpenApiDocumentsDirectory - is there a ways to also specify the file name, e.g. rename it to openapi.json?

dougbu · 2019-07-11T15:37:46Z

Document filenames are intentionally not controllable. They following the format {project name}_{logical document name}.json though _{logical document name} is left out for the common name "v1". This makes the filenames unique enough to avoid extra machinations in projects referencing the documents. It also maintains the Startup configuration as the source of truth for names.

Of course, this is all MSBuild. Users could for example leave the documents in obj/ (a default that may be overridden) but use the Copy task to move them elsewhere and rename them for other purposes.

More generally, what is the main impetus for changing the document filenames (especially for files that land in obj/ by default)?

Documentation is currently minimal. Most of the information is in Microsoft.Extensions.ApiDescription.Server.props.

RicoSuter · 2019-07-11T16:26:11Z

So there's no easy way to generate an openapi.json for a given document name (e.g. "v1") in the web project dir?

dougbu · 2019-07-11T16:59:10Z

In a target that runs after GenerateOpenApiDocuments:

<Copy SourceFiles="$(OpenApiDocumentsDirectory)/$(MSBuildProjectName).json"
    DestinationFiles="$(MSBuildProjectDirectory)openapi.json" />

dougbu commented Jun 11, 2019

View reviewed changes

dougbu changed the title **Do not merge yet** Use new Microsoft.Extensions.ApiDescription.Server package Use new Microsoft.Extensions.ApiDescription.Server package Jun 12, 2019

dougbu force-pushed the dougbu/add.doc.autogen branch from 1cf2396 to 73dcaef Compare June 13, 2019 16:44

dougbu mentioned this pull request Jun 18, 2019

Can I use NSwag.ApiDescription.Client? #2251

Open

dougbu force-pushed the dougbu/add.doc.autogen branch from 73dcaef to 7151494 Compare June 23, 2019 20:37

dougbu added 3 commits June 23, 2019 16:13

Use latest GitHub URLs

4e94b9d

- https://github.com/NSwag and https://github.com/RSuter just add an extra hop to https://github.com/RicoSuter

Set $(IncludeOpenAPIAnalyzers) when targeting netcoreapp3.0

b147cad

- see also RicoSuter#2261

dougbu force-pushed the dougbu/add.doc.autogen branch from 016ecbf to b147cad Compare June 23, 2019 23:18

mkArtakMSFT mentioned this pull request Jun 25, 2019

Send PRs to partner project packages to enable API analyzer dotnet/aspnetcore#8418

Closed

dougbu referenced this pull request in dotnet/aspnetcore Jun 26, 2019

Take new NSwag.ApiDescription.Client version

b70fd59

RicoSuter merged commit 85ae862 into RicoSuter:master Jun 27, 2019

dougbu deleted the dougbu/add.doc.autogen branch July 11, 2019 15:33

Use new Microsoft.Extensions.ApiDescription.Server package #2226

Use new Microsoft.Extensions.ApiDescription.Server package #2226

Conversation

dougbu commented Jun 10, 2019 • edited Loading

dougbu Jun 11, 2019

Choose a reason for hiding this comment

RicoSuter Jun 11, 2019

Choose a reason for hiding this comment

dougbu Jun 11, 2019

Choose a reason for hiding this comment

dougbu commented Jun 11, 2019

RicoSuter commented Jun 11, 2019 • edited Loading

dougbu commented Jun 11, 2019

RicoSuter commented Jun 11, 2019

dougbu commented Jun 11, 2019

RicoSuter commented Jun 11, 2019

dougbu commented Jun 11, 2019

RicoSuter commented Jun 11, 2019

dougbu commented Jun 11, 2019

dougbu commented Jun 12, 2019

RicoSuter commented Jun 13, 2019

dougbu commented Jun 13, 2019

dougbu commented Jun 13, 2019

RicoSuter commented Jun 13, 2019

dougbu commented Jun 13, 2019

RicoSuter commented Jun 13, 2019

dougbu commented Jun 13, 2019 via email

RicoSuter commented Jun 19, 2019

dougbu commented Jun 19, 2019

RicoSuter commented Jun 19, 2019

dougbu commented Jun 19, 2019

RicoSuter commented Jun 19, 2019

RicoSuter commented Jun 20, 2019

dougbu commented Jun 21, 2019

dougbu commented Jun 23, 2019

RicoSuter commented Jun 23, 2019

dougbu commented Jun 23, 2019

mkArtakMSFT commented Jun 23, 2019

dougbu commented Jun 23, 2019

dougbu commented Jun 23, 2019

RicoSuter commented Jul 11, 2019

dougbu commented Jul 11, 2019

RicoSuter commented Jul 11, 2019

dougbu commented Jul 11, 2019

dougbu commented Jun 10, 2019 •

edited

Loading

RicoSuter commented Jun 11, 2019 •

edited

Loading