OpenAPI document generation tool feedback

[martincostello]

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:

1. 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(),

2. Don't emit the warning if GetDocumentCommandContext.OpenApiVersion is null/empty (rather than a value that doesn't parse)
3. 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.

[martincostello]

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]

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]

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]

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

[captainsafia]

@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]

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]

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]

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]

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.