Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Support IDocumentProvider methods named GenerateAsync #8593

Closed
dougbu opened this issue Oct 11, 2018 · 1 comment
Closed

Support IDocumentProvider methods named GenerateAsync #8593

dougbu opened this issue Oct 11, 2018 · 1 comment
Assignees
Labels
3 - Done cost: S Will take up to 2 days to complete enhancement feature-code-generation PRI: 0 - Critical Blocks a critical product path. Must be handled immediately
Milestone

Comments

@dougbu
Copy link
Member

dougbu commented Oct 11, 2018

Is this a Bug or Feature request?

Enhancement that's part of #7947. Adds support for a different IDocumentProvider.

Could also be considered a bug since the features/client.code.generation branch is inconsistent with what we're doing in the RicoSuter/NSwag#1658 PR.

Steps to reproduce (preferably a link to a GitHub repo with a repro project)

  1. Create new console project
  2. Add reference to manually-built NSwag.MSBuild.CodeGeneration package (relying on the Microsoft.Extensions.ApiDescription.Design package -- version 2.2.0-a-preview3-client-code-generation-19114 in my local NSwag build)
  3. Add the following to the .csproj file, referring to a server project that uses manually built NSwag.AspNetCore:
<ServiceProjectReference Include="../TrialNSwag/TrialNSwag.csproj"
    CodeGenerator="NSwagCSharp"
    DocumentPath="TrialNSwag1.json"
    GenerateNSwagCSharpOptions="/GenerateExceptionClasses:false" />
  1. dotnet build
Expected

Successful build

Actual
GenerateDefaultDocument:
    dotnet C:\Users\dougbu\.nuget\packages\microsoft.extensions.apidescription.design\2.2.0-preview3-t0071b0e8a\build\/../tools/dotnet-getdocument.dll --project C:\dd\Projects\CodeGeneration.II\TrialNSwag\TrialNSwag.csproj --framework netcoreapp2.1 --output C:\dd\Projects\CodeGeneration.II\ClientConsole\TrialNSwag1.json --configuration Debug
  Using document name 'v1'.
  Using method 'Generate'.
  Using service 'Microsoft.Extensions.ApiDescription.IDocumentProvider'.
  System.NullReferenceException: Object reference not set to an instance of an object.

Description of the problem

The dotnet-getdocument tool included in the Microsoft.Extensions.ApiDescription.Design package (actually its inside man) looks only for methods named Generate in the IDocumentProvider service. This doesn't match what @rynowak did in the RicoSuter/NSwag#1658 PR. (@RSuter's prefers the async pattern here.)

Fix is to try GenerateAsync first and Generate second. Support Task and Task<bool> if GenerateAsync is found; void and bool otherwise.

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All

See the features/client.code.generation branch. That work is intended for the 2.2 Preview 3 milestone.

@dougbu dougbu added enhancement 2 - Working cost: S Will take up to 2 days to complete PRI: 0 - Critical Blocks a critical product path. Must be handled immediately feature-code-generation labels Oct 11, 2018
@dougbu dougbu added this to the 2.2.0-preview3 milestone Oct 11, 2018
@dougbu dougbu self-assigned this Oct 11, 2018
dougbu added a commit that referenced this issue Oct 11, 2018
dougbu added a commit that referenced this issue Oct 14, 2018
- #8593
- default method name now `GenerateAsync`
- only supported signature is `public Task GenerateAsync(string, TextWriter)`

also:
- handle more error cases in the tool's inside man
- avoid an empty document file if `IDocumentProvider.GenerateAsync(...)` fails
- unwrap an `AggregateException`

nits:
- remove duplicate comments
- change `GetDocumentCommandWorker.TryProcess(...)` to return `false` on failure
  - minor because return value is currently ignored
- rename `GetDocumentCommandContext.Output` -> `OutputPath`
@mkArtakMSFT mkArtakMSFT modified the milestones: 2.2.0-preview3, 2.2.0 Oct 15, 2018
@mkArtakMSFT mkArtakMSFT modified the milestones: 2.2.0, ClientCodeGen Oct 23, 2018
dougbu added a commit that referenced this issue Oct 25, 2018
- #8593
- default method name now `GenerateAsync`
- only supported signature is `public Task GenerateAsync(string, TextWriter)`

also:
- handle more error cases in the tool's inside man
- avoid an empty document file if `IDocumentProvider.GenerateAsync(...)` fails
- unwrap an `AggregateException`

nits:
- remove duplicate comments
- change `GetDocumentCommandWorker.TryProcess(...)` to return `false` on failure
  - minor because return value is currently ignored
- rename `GetDocumentCommandContext.Output` -> `OutputPath`
dougbu added a commit that referenced this issue Oct 27, 2018
- #8593
- also find `IDocumentProvider` using a more-laborious process
  - `Type.GetType(string)` requires an assembly-qualified name and we don't know the assembly
- default method name now `GenerateAsync`
- only supported signature is `public Task GenerateAsync(string, TextWriter)`

also:
- handle more error cases in the tool's inside man
- avoid an empty document file if `IDocumentProvider.GenerateAsync(...)` fails
- unwrap an `AggregateException`

nits:
- remove duplicate comments
- change `GetDocumentCommandWorker.TryProcess(...)` to return `false` on failure
  - minor because return value is currently ignored
- rename `GetDocumentCommandContext.Output` -> `OutputPath`
- reflect recent change to `dotnet-getdocument`'s `Resources.resx` file in its designer file
dougbu added a commit that referenced this issue Oct 27, 2018
- #8593
- also find `IDocumentProvider` using a more-laborious process
  - `Type.GetType(string)` requires an assembly-qualified name and we don't know the assembly
- default method name now `GenerateAsync`
- only supported signature is `public Task GenerateAsync(string, TextWriter)`

also:
- handle more error cases in the tool's inside man
- avoid an empty document file if `IDocumentProvider.GenerateAsync(...)` fails
- unwrap an `AggregateException`

nits:
- remove duplicate comments
- change `GetDocumentCommandWorker.TryProcess(...)` to return `false` on failure
  - minor because return value is currently ignored
- rename `GetDocumentCommandContext.Output` -> `OutputPath`
- reflect recent change to `dotnet-getdocument`'s `Resources.resx` file in its designer file
@dougbu
Copy link
Member Author

dougbu commented Oct 28, 2018

37e5629

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done cost: S Will take up to 2 days to complete enhancement feature-code-generation PRI: 0 - Critical Blocks a critical product path. Must be handled immediately
Projects
None yet
Development

No branches or pull requests

2 participants