OpenAPI document generation tool feedback #57044

martincostello · 2024-07-28T09:45:50Z

Playing around with the OpenAPI document CLI generation from Lint generated OpenAPI documents with Spectral I found a few issues and rough edges. Rather than create lots of small issues I figured I'd list them all here, and they can be spun off into dedicated issues based on feedback/triage.

OpenApiDocumentsDirectory is not resolved to a full path

If OpenApiDocumentsDirectory is specified as a relative path (e.g. dotnet build ./src/API /p:OpenApiGenerateDocuments=true /p:OpenApiDocumentsDirectory="./artifacts/openapi"), the path is not resolved to a full path, and then ends up as a path relative to the tool invocation, rather than relative to the current working directory of the command, so the document ends up in the bin\Debug etc. folder rather than the folder the user expected.

Seems like resolving the full path here in the MSBuild targets before invoking the tool would resolve that:

aspnetcore/src/Tools/Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.targets

Line 56 in 06155c0

    
           <_DotNetGetDocumentCommand>$(_DotNetGetDocumentCommand) --output "$(OpenApiDocumentsDirectory.TrimEnd('\'))" --project "$(MSBuildProjectName)"</_DotNetGetDocumentCommand>

"Invalid OpenAPI spec version" warning

If the target is run without explicitly an OpenAPI document version to generate, a warning is output:

Generating document named 'api'.
Using discovered `GenerateAsync` overload with version parameter.
Invalid OpenAPI spec version '' provided. Falling back to default: v3.0.
Writing document named 'api' to './artifacts/openapi\API_api.json'.

This comes from here:

aspnetcore/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs

Line 329 in 06155c0

    
           _reporter.WriteWarning(Resources.FormatInvalidOpenApiVersion(_context.OpenApiVersion));

If this Enum.TryParse() call fails to parse anything:

aspnetcore/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs

Line 323 in 06155c0

    
           if (Enum.TryParse<OpenApiSpecVersion>(_context.OpenApiVersion, out var version))

It looks like there's a few ways to resolve this:

Assign a default value for GetDocumentCommandContext.OpenApiVersion if no value is provided:

aspnetcore/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommand.cs

Line 141 in 06155c0

OpenApiVersion = _openApiVersion.Value(),
Don't emit the warning if GetDocumentCommandContext.OpenApiVersion is null/empty (rather than a value that doesn't parse)
Explicitly pass a value for --openapi-version through from the MSBuild target:

aspnetcore/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommand.cs

Line 33 in 06155c0

_openApiVersion = command.Option("--openapi-version <Version>", Resources.OpenApiVersionDescription);

No MSBuild property for specifying the OpenAPI version

It's possible to add extra options to work around the above using OpenApiGenerateDocumentsOptions (e.g. /p:OpenApiGenerateDocumentsOptions="--openapi-version OpenApi3_0"), but there isn't a specific MSBuild property for this.

To make usage easier a property could be defined that the user can specify to set the version without needing to explicitly set command line parameters that the MSBuild target could transform and pass through to the tool (e.g. /p:OpenApiGenerateDocumentVersion=3.0 into --openapi-version OpenApi3_0).

MSBuild properties not affecting "up-to-date-ness"

I noticed when the path wasn't being fully evaluated, if I re-ran the build with a different input property the document wouldn't be regenerated.

For example:

❯ dotnet build ./src/API /p:OpenApiGenerateDocuments=true /p:OpenApiDocumentsDirectory="${OutputDirectory}"
Restore complete (1.9s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  API succeeded (16.5s) → artifacts\bin\API\debug\API.dll

Build succeeded in 21.8s

api on  dotnet-nightly [$] via .NET v9.0.100-preview.7.24375.12 took 22s
❯ dotnet build ./src/API /p:OpenApiGenerateDocuments=true /p:OpenApiDocumentsDirectory="${OutputDirectory}2"
Restore complete (0.5s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  API succeeded (1.0s) → artifacts\bin\API\debug\API.dll

Build succeeded in 4.4s

api on  dotnet-nightly [$] via .NET v9.0.100-preview.7.24375.12 took 4s

The first command creates the API document at ${OutputDirectory} in ~20 seconds.

The second command with the output path set to ${OutputDirectory}2 doesn't emit anything and takes ~4 seconds.

It seems like the properties being passed through aren't affecting MSBuild up-to-date checks and causing the document not to be re-generated.

Output directory value isn't checked for

I made a mistake during playing with the tool and accidentally un-defined the variable for the output path. This then causes an ArgumentException to be thrown inside the tool (--output ""). Should something check a value is specified first?

❯ dotnet build ./src/API /p:OpenApiGenerateDocuments=true /p:OpenApiDocumentsDirectory="${OutputDirectory}" /p:OpenApiGenerateDocumentsOptions="--openapi-version OpenApi3_0"
Restore complete (0.6s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  API failed with 6 error(s) (12.4s) → artifacts\bin\API\debug\API.dll
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error : System.ArgumentException: The value cannot be an empty string. (Parameter 'path')
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.ArgumentException.ThrowNullOrEmptyException(String argument, String paramName)
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at System.IO.Directory.CreateDirectory(String path)
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescription.Tool.Commands.GetDocumentCommandWorker.GetDocuments(IServiceProvider services)
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error :    at Microsoft.Extensions.ApiDescription.Tool.Commands.GetDocumentCommandWorker.Process()
    C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\Microsoft.Extensions.ApiDescription.Server.targets(66,5): error MSB3073: The command "dotnet "C:\Users\Martin.Costello\.nuget\packages\microsoft.extensions.apidescription.server\9.0.0-preview.7.24375.8\build\../tools/dotnet-getdocument.dll" --assembly "C:\Coding\martincostello\api\artifacts\bin\API\debug\API.dll" --file-list "C:\Coding\martincostello\api\artifacts\obj\API\API.OpenApiFiles.cache" --framework ".NETCoreApp,Version=v9.0" --output "" --project "API" --assets-file "C:\Coding\martincostello\api\artifacts\obj\API\project.assets.json" --platform "AnyCPU" --openapi-version OpenApi3_0" exited with code 11.

The text was updated successfully, but these errors were encountered:

martincostello · 2024-07-28T10:48:03Z

Another fun thing I just bumped into that I figured I'd at least make a note of here.

If a project still has a reference to Swashbuckle.AspNetCore, this target will override OpenApiGenerateDocumentsOnBuild=false and then OpenApiGenerateDocuments=true won't seemingly do anything when attempting to use the CLI tool.

Explicitly setting OpenApiGenerateDocumentsOnBuild=true is required to get the tool to run in this case.

Cjewett · 2024-07-28T12:08:28Z

Another fun thing I just bumped into that I figured I'd at least make a note of here.

If a project still has a reference to Swashbuckle.AspNetCore, this target will override OpenApiGenerateDocumentsOnBuild=false and then OpenApiGenerateDocuments=true won't seemingly do anything when attempting to use the CLI tool.

Explicitly setting OpenApiGenerateDocumentsOnBuild=true is required to get the tool to run in this case.

Glad you were looking into this at the same time 😄. Can confirm this is an issue. Based on the other UI tools (ReDoc, Scalar) we're more than likely going to swap our OpenApi documentation generation to Microsoft's implementation but stick with SwaggerUI, so we would hope to continue referencing Swashbuckle.AspNetCore.

Also curious if <GenerateDocumentationFile> is supposed to enrich the generated OpenApi document or not? It seems like setting that to true isn't doing so.

martincostello · 2024-07-28T12:16:37Z

so we would hope to continue referencing Swashbuckle.AspNetCore

You could try removing the reference to Swashbuckle.AspNetCore (the meta-package which includes the generation and UI packages) and instead just referencing Swashbuckle.AspNetCore.SwaggerUI.

Also curious if is supposed to enrich the generated OpenApi document or not? It seems like setting that to true isn't doing so.

It's intended to eventually (see #39927 (comment)), but doesn't at the moment.

For now I've written a custom transformer that does as much as I need for my own purposes: #56318 (comment)

You could copy that and expand as-needed for your own needs.

Cjewett · 2024-07-28T16:24:03Z

Thanks! I'll watch those other threads and take a look at the custom transformer.

captainsafia · 2024-07-29T17:07:31Z

@martincostello Thanks for sharing this feedback!

So....I have a love/hate relationship with the strategy that we currently use for build-time OpenAPI document generation. 😅

For context, this functionality is supported by another package we ship (https://www.nuget.org/packages/Microsoft.Extensions.ApiDescription.Server) and was primarily implemented with the assumption that another package (like Swashbuckle or NSwag) would be installed alongside it. You'll observe this in the rather hokey use of the "pubternal" IDocumentProvider interface that both NSwag and Swashbuckle implement and the fact that they clobber over MSBuild properties defined in the package.

In the long-term, I'd like to move away from the use of this package for build-time document generation and move to something more built-in to the M.A.OpenApi package. It doesn't seem like the package is playing a good role being an abstraction layer for packages given that both NSwag and Swashbuckle implement their own CLIs anyways. The fact that this logic exists in a separate package also makes it hard to discover for users.

I've been conservative about the changes made to this package for .NET 9 in anticipation of exploring a possible rewrite for this in the future.

Here's a braindump of some of the things I've been thinking about here:

The current implementation uses an internal executable that is called via MSBuild, so there's massaging that needs to be done when mapping the MSBuild arguments to arguments for the CLI tool. IMO, we should ship either a tool or stick to MSBuild targets. I don't think there's much value in providing half-baked support for both.
The current implementation is intended to work with tools like Swashbuckle, which support a wider set of target frameworks that we intend to. Moving to a different approach would allow us to remove code/packaging logic related to unsupported frameworks (netfx, netstandard).
I'd like to standardize the configuration story for build-time document generation so there's an official schema exposed via options (similar to what you get with Aspire's config model) to improve the discoverability of these options.

So, all this to say, I agree that the tool is carrying a lot of cruft, but my current thinking is that exploring a full rewrite within M.A.OpenApi is a better strategy in the long-term.

Thoughts?

martincostello · 2024-07-30T13:34:03Z

I guess something that committed to one approach or the other (MSBuild or tools) would be better 😄

Separate to that, I can see why moving to something new would allow for greater agility with what the tool does, as it wouldn't be constrained to the duck-typed interface's constraints.

Are any of the specific items that are low-cost to address worth addressing for .NET 9 anyway? As it's still the documented approach for this kind of build-time generation, any low-hanging fruit that avoid any paper cuts would be beneficial IMHO.

I'd be happy to tackle any that are wanted (and can make it in time for .NET 9) - just let me know which ones. The first three items in the description seem like quick fixes to me.

For my own projects, the only reason I actually have for using the generator is to generate a static file for deploying with native AoT deployed apps in wwwroot. However, with M.A.OpenApi supporting native AoT, for my own needs that's now moot for .NET 9 as I can now dynamically generate the content 🥳

captainsafia · 2024-07-30T20:52:21Z

Are any of the specific items that are low-cost to address worth addressing for .NET 9 anyway?

@MauriceChocoSwiss opened #56974 because he was keen to be able to support modifying the name of the outputted file. This would be above my bar for things I'd feel OK including at this point given the friction point that was identified. It's also good signal for the kinds of things people care about customizing in a possible rewrite.

The OpenAPI version option was introduced in this release for a similar reason. There were some niche scenarios in VS where you needed to override the version configured in OpenApiOptions and always serialize to OpenAPI v2 to acount for some tools that didn't support v3. I intentionally avoided including an MSBuild option for it for the scenarios I mentioned above about the weirdness between shuffling between MSBuild and CLI args and because I'd really like to avoid exposing users to too many configurability points for the same value.

I'd be happy to tackle any that are wanted (and can make it in time for .NET 9) - just let me know which ones.

I think things like fully qualifying OpenApiDocumentsDirectory and fixing up the erroneous warnings on OpenAPI spec version seem worthwhile to do for .NET 9. As far as timelines, there's a little less than two weeks left for changes.

The other unfortunate thing about the tool is the testing story is a little subpar. I added some tests for it in #55823 but we definitely don't have the kind of coverage I'd like to see as we make changes...

martincostello · 2024-07-30T22:12:14Z

Ok cool - I'll try and do a PR for both of those tomorrow.

Which approach would you prefer for suppressing the console warning?

captainsafia · 2024-07-31T05:31:03Z

2. Don't emit the warning if GetDocumentCommandContext.OpenApiVersion is null/empty (rather than a value that doesn't parse)

I'm in favor of

Don't emit the warning if GetDocumentCommandContext.OpenApiVersion is null/empty (rather than a value that doesn't parse)

The warning is misleading. If you don't provide a value, it'll fallback to the value defined in OpenApiOptions which 99% of the time is OK since you want the document generated at build-time to match the document generated at compile-time.

Convert `$(OpenApiDocumentsDirectory)` to a full path before passing it to the `dotnet-getdocument` tool. Relates to dotnet#57044.

Do not emit warning about an invalid OpenAPI version in the `dotnet-getdocument` tool if no explicit value was passed to the tool via the `` argument. Relates to dotnet#57044.

martincostello · 2024-07-31T08:34:41Z

#57096

…57096) * Get full path of OpenApiDocumentsDirectory Convert `$(OpenApiDocumentsDirectory)` to a full path before passing it to the `dotnet-getdocument` tool. Relates to #57044. * Avoid warning about invalid OpenAPI version Do not emit warning about an invalid OpenAPI version in the `dotnet-getdocument` tool if no explicit value was passed to the tool via the `` argument. Relates to #57044. * Fix TargetTest tests Fix tests failing locally in Visual Studio due to test assets not being copied to the output directory. * Fix tests Fix assertions looking for relative paths instead of absolute paths.

captainsafia · 2024-07-31T18:31:07Z

Also curious if is supposed to enrich the generated OpenApi document or not? It seems like setting that to true isn't doing so.

@Cjewett To add to what @martincostello shared here, check out the comment I recently posted on the tracking issue.

pinkfloydx33 · 2024-08-03T06:23:27Z

For context, this functionality is supported by another package we ship (https://www.nuget.org/packages/Microsoft.Extensions.ApiDescription.Server)

Will this package continue to work if we do not switch to the new OpenApi generation? We plan to stick with Swashbuckle, at least until v10. It'd be very unfortunate if it stopped working. I assume OpenApi generation is going to be opt-in and not automatically pulled in by the SDK/SFX, thus making it so there's no conflict, but at this point I'm uncertain.

(We also use MS.E.ApiDescription.Client with NSwag.MsBuild to generate our API clients. It doesn't sound like this side of things would be impacted in any case)

martincostello · 2024-08-03T06:45:16Z

From my testing it's definitely still opt-in.

AFAIK, it would still work with Swashbuckle (or any other tool that implements the same interface to expose the generation).

martincostello added area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI feature-openapi labels Jul 28, 2024

captainsafia added this to the .NET 10 Planning milestone Jul 29, 2024

martincostello added a commit to martincostello/aspnetcore that referenced this issue Jul 31, 2024

Get full path of OpenApiDocumentsDirectory

5e9ec8d

Convert `$(OpenApiDocumentsDirectory)` to a full path before passing it to the `dotnet-getdocument` tool. Relates to dotnet#57044.

martincostello mentioned this issue Jul 31, 2024

Fixes for OpenAPI document generation with M.E.ApiDescription.Server #57096

Merged

captainsafia mentioned this issue Sep 19, 2024

Microsoft.Extensions.ApiDescription.Server Unable to find service type 'Microsoft.Extensions.ApiDescriptions.IDocumentProvider' in dependency injection container. #57939

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenAPI document generation tool feedback #57044

OpenAPI document generation tool feedback #57044

martincostello commented Jul 28, 2024

martincostello commented Jul 28, 2024

Cjewett commented Jul 28, 2024 •

edited

Loading

martincostello commented Jul 28, 2024

Cjewett commented Jul 28, 2024

captainsafia commented Jul 29, 2024

martincostello commented Jul 30, 2024

captainsafia commented Jul 30, 2024

martincostello commented Jul 30, 2024

captainsafia commented Jul 31, 2024

martincostello commented Jul 31, 2024

captainsafia commented Jul 31, 2024

pinkfloydx33 commented Aug 3, 2024

martincostello commented Aug 3, 2024

OpenAPI document generation tool feedback #57044

OpenAPI document generation tool feedback #57044

Comments

martincostello commented Jul 28, 2024

OpenApiDocumentsDirectory is not resolved to a full path

"Invalid OpenAPI spec version" warning

No MSBuild property for specifying the OpenAPI version

MSBuild properties not affecting "up-to-date-ness"

Output directory value isn't checked for

martincostello commented Jul 28, 2024

Cjewett commented Jul 28, 2024 • edited Loading

martincostello commented Jul 28, 2024

Cjewett commented Jul 28, 2024

captainsafia commented Jul 29, 2024

martincostello commented Jul 30, 2024

captainsafia commented Jul 30, 2024

martincostello commented Jul 30, 2024

captainsafia commented Jul 31, 2024

martincostello commented Jul 31, 2024

captainsafia commented Jul 31, 2024

pinkfloydx33 commented Aug 3, 2024

martincostello commented Aug 3, 2024

Cjewett commented Jul 28, 2024 •

edited

Loading