Add Microsoft.Extensions.ApiDesription.Server project and package #10669

dougbu · 2019-05-30T22:17:37Z

Add Web Application project support for build-time OpenAPI document generation #8242 2 of 2

dougbu · 2019-05-30T22:19:58Z

Also addresses #4912 because I didn't restore the GetProjectReferenceMetadata task

dougbu · 2019-05-30T22:20:33Z

@natemcmaster I'd of course appreciate any input you have on the MSBuild changes

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props

eng/ProjectReferences.props

src/Mvc/Extensions.ApiDescription.Server/src/Microsoft.Extensions.ApiDescription.Server.nuspec

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props

dougbu · 2019-05-31T16:41:47Z

Bit more overlap with #10667 but resolution won't be too difficult

JunTaoLuo · 2019-05-31T17:13:59Z

/azp run

azure-pipelines · 2019-05-31T17:14:31Z

Azure Pipelines successfully started running 3 pipeline(s).

natemcmaster · 2019-05-31T17:17:52Z

src/Mvc/Extensions.ApiDescription.Server/src/Microsoft.Extensions.ApiDescription.Server.csproj

+    <NoPackageAnalysis>true</NoPackageAnalysis>
+
+    <!-- Included primarily to ensure dotnet-getdocument and GetDocument.Insider can be referenced. -->
+    <TargetFrameworks>netcoreapp2.1;net461</TargetFrameworks>


Have you tested this tool when only a .NET Core 3.0 SDK is installed? I didn't see anything which configures rollforward, so I would expect the dotnet get-document.dll command to fail with a "Could not find .NET Core 2.1..." error.

See #10667. That adds runtimeconfig.template.json files for dotnet-getdocument and GetDocument.Insider

Oh, I also confirmed this works fine in a 3.0-only environment with 3.0 projects

natemcmaster · 2019-05-31T17:20:36Z

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props

+      Options added to the Open API retrieval tool ('dotnet-getdocument') command line. Available options control
+      console output: 'no-color', 'prefix-output' and 'verbose'. All require a double-dash prefix.
+    -->
+    <OpenApiGetDocumentOptions Condition=" '$(OpenApiGetDocumentOptions)' == '' "></OpenApiGetDocumentOptions>


This line doesn't do anything. "If OpenApiGetDocumentOptions is empty, assign OpenApiGetDocumentOptions to empty."

Good catch! This property should be true by default

Oh, wait. This is here only for documentation purposes. Should be empty by default.

natemcmaster · 2019-05-31T17:28:03Z

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props

+    <!--
+      If 'true' (the default), enable retrieval of Open API documents. Otherwise, this feature is completely disabled.
+    -->
+    <OpenApiRetrieveDocuments Condition=" '$(OpenApiRetrieveDocuments)' == '' ">true</OpenApiRetrieveDocuments>


Why would I install this package and then disable it's functionality completely? Seems like it would be simpler to say "if you want to disable the feature, uninstall the package"

Users are not usually aware of this package at all. We'll be working with NSwag and SwashBuckle to ship with a dependency on it in their existing NSwag.AspNetCore and SwashBuckle.AspNetCore. Those packages have a lot of features and some users may wish to disable this particular one.

That is, we decided on opt-out for saving Open API docs after build.

dougbu · 2019-05-31T19:17:59Z

@natemcmaster @JunTaoLuo builds are looking much better now 😺

rynowak · 2019-05-31T22:24:12Z

...xtensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.targets

+  </ItemGroup>
+
+  <Target Name="OpenApiGetDocuments" Returns="@(_OpenApiProjectDocuments)">
+    <ReadLinesFromFile File="$(_OpenApiDocumentsCache)">


Is this actually a cache? or is this just the output of generating the documents?

TL;DR

Is your question aimed at my naming (and life) choices? I don't see much consistency in how similar MSBuild properties are named and chose something that fit.

Make a suggestion if you feel strongly please.

Details

This file is many things…

It's a cache in the sense seen in lots of MSBuild code: It's a way to avoid redoing an action (calling a task, executing a too, whatever). In particular, this file avoids reopening the app whenever a client project calls OpenApiGetDocuments.

Yes, as an implementation detail, this file is one of the files written as a result of running the tool. It lists the document files.

The file is also like many used in MSBuild code in that it summarizes multiple inputs or outputs of a task or tool. For example, I could do the Inputs=$(TargetPath) … Outputs=$(_OpenApiDocumentsCache) batching trick to avoid running dotnet-getdocument even if there are multiple document files.

Finally, this file is the communication mechanism between the GenerateOpenApiDocuments and OpenApiGetDocuments targets.

rynowak · 2019-05-31T22:24:44Z

...xtensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.targets

+
+    <Message Importance="high" Text="%0ARetrieveOpenApiDocuments:" />
+    <Message Importance="high" Text="  $(_Command)" />
+    <Exec Command="$(_Command)" />


What are the caching expectations of this? Should we be adding some things to FileWrites so they can be removed by incremental clean?

I have been thinking about FileWrites both here and in the client-side glue. But, every time I start making those changes, I hit the "What if the user plans to check these files in?" question. These files aren't always throw-away. But, I could probably complicate things enough to avoid that concern e.g. with conditions on $(OpenApiDocumentsDirectory) matching the default.

I'll add a note to #4923 reminding me to work through the implications and do something.

Discussed in person. Right now there is no input/output checking and this command will re-run every time you build, even if the assembly does not change.

This is tracked separately and will be improved in the next preview.

dougbu · 2019-06-01T05:52:23Z

Fair number of comments from a fair number of people but I'm not sure whether anything is a blocking concern. @rynowak (for example), do you approve of leaving things as-is until I address the #4923 list?

- #8242 2 of 2

@JunTaoLuo

- remove incorrect `$(IsImplementationProject)` settings and re-run .\eng\scripts\GenerateProjectList.ps1 - address PR comments - correct extra word in a comment that @JunTaoLuo noticed nits: - minor formatting changes in Microsoft.Extensions.ApiDescription.Server.props - remove the project file's extension from the file list cache filename

- rename targets and the project capability for consistency - left the `OpenApiGetDocuments` target alone because it's used from client projects and isn't about generation

…osoft.Extesions.ApiDescription.Client.targets nits: - more s/retrieve/generate/

dougbu · 2019-06-03T03:18:10Z

🆙📅 for smaller fixes and a rebase on 'master'. Could someone please approve?

dougbu · 2019-06-03T05:35:57Z

Bump @rynowak or @pranavkm

dougbu · 2019-06-03T18:24:23Z

Retargeted to preview 6 branch. PR validation begun again.

/cc @mkArtakMSFT

dougbu · 2019-06-04T01:53:29Z

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=209867

dougbu · 2019-06-04T01:53:50Z

Just another Helix hang unrelated to anything in this PR. Merging…

aspnet-hello · 2019-06-04T01:57:35Z

This comment was made automatically. If there is a problem contact [email protected].

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/2521

dougbu requested review from pranavkm, rynowak, natemcmaster and a team May 30, 2019 22:17

dougbu closed this May 30, 2019

dougbu reopened this May 30, 2019

dougbu commented May 30, 2019

View reviewed changes

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props Show resolved Hide resolved

Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 30, 2019

JunTaoLuo mentioned this pull request May 31, 2019

Produce Microsoft.AspNetCore.Authorization as a package #10684

Merged

JunTaoLuo reviewed May 31, 2019

View reviewed changes

eng/ProjectReferences.props Outdated Show resolved Hide resolved

JunTaoLuo reviewed May 31, 2019

View reviewed changes

src/Mvc/Extensions.ApiDescription.Server/src/Microsoft.Extensions.ApiDescription.Server.nuspec Show resolved Hide resolved

JunTaoLuo reviewed May 31, 2019

View reviewed changes

.../Extensions.ApiDescription.Server/src/build/Microsoft.Extensions.ApiDescription.Server.props Outdated Show resolved Hide resolved

dougbu force-pushed the dougbu/add.server-side.8242.2 branch from 096bb75 to 1c0d301 Compare May 31, 2019 16:04

natemcmaster reviewed May 31, 2019

View reviewed changes

rynowak reviewed May 31, 2019

View reviewed changes

dougbu mentioned this pull request Jun 1, 2019

Rewrite dotnet-getdocument and GetDocument.Insider to retrieve all documents in one go #10667

Merged

dougbu added 6 commits June 2, 2019 20:00

Add Microsoft.Extensions.ApiDesription.Server project and package

125ba48

- #8242 2 of 2

Add missing project reference provider

f6a580d

!fixup! GetDocumentCommand was still using .NET Core App 2.0 define

1400926

!fixup! Close an element

cbde9cd

Rename zee properties

0819b5c

- rename targets and the project capability for consistency - left the `OpenApiGetDocuments` target alone because it's used from client projects and isn't about generation

dougbu added 2 commits June 2, 2019 20:03

!fixup! Avoid ending a quoted command-line argument with \"

52a5795

Correct OpenApiGetDocuments target name in buildMultiTargeting/Micr…

2de02fa

…osoft.Extesions.ApiDescription.Client.targets nits: - more s/retrieve/generate/

dougbu force-pushed the dougbu/add.server-side.8242.2 branch from 57d669c to 2de02fa Compare June 3, 2019 03:05

dougbu mentioned this pull request Jun 3, 2019

Clean up TODOs and duplicated code in client code generation #4923

Closed

3 tasks

Merge branch 'master' into dougbu/add.server-side.8242.2

1ab0931

rynowak approved these changes Jun 3, 2019

View reviewed changes

dougbu changed the base branch from master to release/3.0-preview6 June 3, 2019 18:23

dougbu merged commit b3ddd65 into release/3.0-preview6 Jun 4, 2019

ghost deleted the dougbu/add.server-side.8242.2 branch June 4, 2019 01:57

dougbu mentioned this pull request Jun 19, 2019

Address some rough edges in service reference feature #11393

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Microsoft.Extensions.ApiDesription.Server project and package #10669

Add Microsoft.Extensions.ApiDesription.Server project and package #10669

dougbu commented May 30, 2019

dougbu commented May 30, 2019

dougbu commented May 30, 2019

dougbu commented May 31, 2019

JunTaoLuo commented May 31, 2019

azure-pipelines bot commented May 31, 2019

natemcmaster May 31, 2019

dougbu May 31, 2019

dougbu Jun 3, 2019

natemcmaster May 31, 2019

dougbu May 31, 2019

dougbu May 31, 2019

natemcmaster May 31, 2019

dougbu May 31, 2019

dougbu commented May 31, 2019

rynowak May 31, 2019

dougbu Jun 1, 2019

rynowak May 31, 2019

dougbu Jun 1, 2019

rynowak Jun 3, 2019

dougbu commented Jun 1, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 4, 2019

dougbu commented Jun 4, 2019

aspnet-hello commented Jun 4, 2019

Add Microsoft.Extensions.ApiDesription.Server project and package #10669

Add Microsoft.Extensions.ApiDesription.Server project and package #10669

Conversation

dougbu commented May 30, 2019

dougbu commented May 30, 2019

dougbu commented May 30, 2019

dougbu commented May 31, 2019

JunTaoLuo commented May 31, 2019

azure-pipelines bot commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TL;DR

Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Jun 1, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 3, 2019

dougbu commented Jun 4, 2019

dougbu commented Jun 4, 2019

aspnet-hello commented Jun 4, 2019