dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.6k stars 10.06k forks source link

dotnet-openapi produces uncompilable swaggerClient #21656

Open jrodenburg opened 4 years ago

jrodenburg commented 4 years ago

Describe the bug

Using the dotnet-openapi tool, a broken client (uncompilable) is generated from a swagger definition.

To Reproduce

  1. Create a basic console application
  2. Run dotnet-openapi against an openapi service definition
mkdir newapp
cd newapp
dotnet new console
dotnet openapi add url http://localhost:5000/swagger/v1/swagger.json -c NSwagCSharp
  1. Use swaggerClient in Program.cs
using System;
using System.Net.Http;

namespace newapp
{
    class Program
    {
        static void Main(string[] args)
        {
            HttpClient httpClient = new HttpClient {BaseAddress = new Uri("http://localhost:5000")};
            swaggerClient client = new swaggerClient(httpClient);
        }
    }
}
  1. Attempt to build the application
[ $ ~/newapp ] dotnet build
Microsoft (R) Build Engine version 16.5.0+d4cbfca49 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 214.29 ms for /projects/newapp/newapp.csproj.

  GenerateNSwagCSharp:
    dotnet --roll-forward-on-no-candidate-fx 2 /Users/jrodenburg/.nuget/packages/nswag.msbuild/13.0.5/build/../tools/NetCore21//dotnet-nswag.dll openapi2csclient /className:swaggerClient /namespace:newapp /input:/projects/newapp/swagger.json /output:obj/swaggerClient.cs
  NSwag command line tool for .NET Core NetCore21, toolchain v13.0.5.0 (NJsonSchema v10.0.22.0 (Newtonsoft.Json v11.0.0.0))
  Visit http://NSwag.org for more information.
  NSwag bin directory: /.nuget/packages/nswag.msbuild/13.0.5/tools/NetCore21
  Code has been successfully written to file.

  Duration: 00:00:00.9114557
obj/swaggerClient.cs(2761,26): error CS0102: The type 'swaggerClient' already contains a definition for 'ObjectResponseResult' [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3150,26): error CS0102: The type 'swaggerClient' already contains a definition for 'ObjectResponseResult' [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3433,26): error CS0102: The type 'swaggerClient' already contains a definition for 'ObjectResponseResult' [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3898,26): error CS0102: The type 'swaggerClient' already contains a definition for 'ObjectResponseResult' [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(4087,26): error CS0102: The type 'swaggerClient' already contains a definition for 'ObjectResponseResult' [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(2655,6): error CS0579: Duplicate 'System.CodeDom.Compiler.GeneratedCode' attribute [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(2854,6): error CS0579: Duplicate 'System.CodeDom.Compiler.GeneratedCode' attribute [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3243,6): error CS0579: Duplicate 'System.CodeDom.Compiler.GeneratedCode' attribute [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3526,6): error CS0579: Duplicate 'System.CodeDom.Compiler.GeneratedCode' attribute [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3991,6): error CS0579: Duplicate 'System.CodeDom.Compiler.GeneratedCode' attribute [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(2681,22): error CS0756: A partial method may not have multiple defining declarations [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(2880,22): error CS0756: A partial method may not have multiple defining declarations [/projects/newapp/newapp.csproj]
obj/swaggerClient.cs(3269,22): error CS0756: A partial method may not have multiple defining declarations [/projects/newapp/newapp.csproj]

{... further results elided ...}

Further technical details

[ $ ~ ] dotnet --info .NET Core SDK (reflecting any global.json): Version: 3.1.200 Commit: c5123d973b

Runtime Environment: OS Name: Mac OS X OS Version: 10.14 OS Platform: Darwin RID: osx.10.14-x64 Base Path: /usr/local/share/dotnet/sdk/3.1.200/

Host (useful for support): Version: 3.1.3 Commit: 4a9f85e9f8

.NET Core SDKs installed: 2.2.402 [/usr/local/share/dotnet/sdk] 3.0.100 [/usr/local/share/dotnet/sdk] 3.1.100 [/usr/local/share/dotnet/sdk] 3.1.103 [/usr/local/share/dotnet/sdk] 3.1.200 [/usr/local/share/dotnet/sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.2.7 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.2.7 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.2 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.16 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.7 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.2 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs: https://aka.ms/dotnet-download

mkArtakMSFT commented 4 years ago

@bradygaster is this something you're handline these days?

bradygaster commented 4 years ago

@mkArtakMSFT yes I think I would be involved, though it will require some attention on the part of @sayedihashimi as well given his old role is now his new role. :) @sayedihashimi - let's triage this together. Would either of you propose a method of being able to triage/tag all the Swagger/OAS-related issues? I'd like to start being on the hook for those.

bradygaster commented 4 years ago

@jrodenburg would you be willing/able to share the OAS doc?

jrodenburg commented 4 years ago

Including the generated swagger.json for the service. swagger1.txt

bradygaster commented 4 years ago

Did you expect it would be an Open API 3.0 doc? I used the swagger validator to validate it - looks okay from a document validity point of view. Would you mind trying it converted to a 2.0 doc? Curious if you'd omit the errors then. Also cc @ryanbrandenburg

ryanbrandenburg commented 4 years ago

This appears to be a problem with code-generation itself as opposed to the tool (which merely adds `

` to the csproj). When I ran the steps above locally I got errors very similar to the ones reported above. When I inspect `obj\swagger1Client.cs` I can see that multiple partials of `swagger1Client` have been generated, each containing definitions of _baseUrl, _httpClient,_settings, etc, as well as all having the `System.CodeDom.Compiler.GeneratedCode` attribute applied to them. [swagger1Client.txt](https://github.com/dotnet/aspnetcore/files/4799919/swagger1Client.txt)
jrodenburg commented 4 years ago

Did you expect it would be an Open API 3.0 doc? I used the swagger validator to validate it - looks okay from a document validity point of view. Would you mind trying it converted to a 2.0 doc? Curious if you'd omit the errors then. Also cc @ryanbrandenburg

yes we made this intentionally as OA3. Our codebase has morphed so getting it back to 2.0 will require hand-manipulation or a tool.

This appears to be a problem with code-generation itself as opposed to the tool (which merely adds <ItemGroup> <OpenApiReference Include="C:\Users\rybrande\Downloads\swagger1.json" /> </ItemGroup> to the csproj). When I ran the steps above locally I got errors very similar to the ones reported above.

When I inspect obj\swagger1Client.cs I can see that multiple partials of swagger1Client have been generated, each containing definitions of _baseUrl, _httpClient,_settings, etc, as well as all having the System.CodeDom.Compiler.GeneratedCode attribute applied to them. swagger1Client.txt

That's exactly the output I had. Multiple partial class definitions that duplicate definitions (hence the build errors.) Issue title is likely misleading.

bradygaster commented 4 years ago

@jrodenburg would you be open to another approach? I am getting up-to-speed with help from @ryanbrandenburg on how this set of the tools work so i'm not entirely familiar with what could be happening here. I have a bit of familiarity with an approach that uses AutoRest, however, and if you're open to that as an alternative for now until we can resolve this I'd be willing to set up some time with you to walk through it or write up a doc on how to set it up. AutoRest would have the single dependency of needing NPM on your dev machine as that's how AutoRest is acquired and how it acquires dependencies it uses.

jrodenburg commented 4 years ago

@bradygaster yes, i've actually used AutoRest previously (w/ dotnetcore v2.) Due to the node/npm dependency, can we bake this into a docker image, ala https://hub.docker.com/r/azuresdk/autorest?

bradygaster commented 4 years ago

@jrodenburg sorry for the late reply, i've actually been tinkering around with this a bunch. I presume you could do that, but wouldn't the Docker requirement be heavier than the NPM requirement?

jrodenburg commented 4 years ago

wouldn't the Docker requirement be heavier than the NPM requirement?

Docker is my preference. Doing so would make implementation in our build pipeline far easier and manageable.

Wouldn't any modification just be an update to https://github.com/Azure/autorest/blob/master/Dockerfile? Seems easy enough.....

bradygaster commented 4 years ago

@jrodenburg checking in on this - did you give the dockerized autorest a shot?

jrodenburg commented 4 years ago

I did, still no success. Here are the steps followed:

  1. Copy https://github.com/Azure/autorest/blob/master/Dockerfile to local file (Dockerfile).
  2. docker build -t autorest .
  3. Copy swagger1.txt from earlier in this read to local file (swagger.json)
  4. docker run -it --rm -v $(pwd):/clients autorest autorest --base-folder=/clients --input-file=swagger.json --output-folder=csharp --csharp --namespace=Whatever.Name.You.Want --v3

Received the following output:

[ $ ] docker run -it --rm -v $(pwd):/clients autorest autorest --base-folder=/clients --input-file=swagger.json --output-folder=csharp --csharp --namespace=random.namespace --v3
AutoRest code generation utility [cli version: 3.0.6247; node: v10.22.1, max-memory: 1280 gb]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@autorest_core@3.0.6320/node_modules/@autorest/core/dist' (3.0.6320)
   Installing AutoRest extension '@microsoft.azure/autorest.csharp' (~2.3.79)
   Installed AutoRest extension '@microsoft.azure/autorest.csharp' (~2.3.79->2.3.90)
   Installing AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.55)
   Installed AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.55->2.3.55)
  TypeError: Cannot delete property '17' of [object String]
GRMorgan commented 3 years ago

I've just ran into this bug myself. I found that removing "operationId" from the swagger file causes the problem to go away.

Though it might help with debugging and may help @jrodenburg move forward

tfabraham commented 3 years ago

I encountered this issue today using the Swagger doc provided by Thycotic's Secret Server Cloud found here.

I used NSwagStudio 13.10.8.0 to generate the client against the same Swagger doc, and it generates the same bad output.

Is anyone working with Rico Suter on this issue?

bradygaster commented 3 years ago

adding @RicoSuter here - I'd like to get his input on this. It would be great if this could be mitigated, but the reality is that no one generation author can support all the nuances of use-cases for folks building OpenAPI specs. Also curious if @darrelmiller has thoughts on this.

excelkobayashi commented 3 years ago

I was able to resolve this by specifying the class name inside of the OpenApiReference element:

<ClassName>{controller}Client</ClassName>

Using {controller} prevents the same class name from being generated multiple times.

RicoSuter commented 3 years ago

Is this resolved with @excelkobayashi fix?

psandgren commented 3 years ago

@excelkobayashi answer worked for me. However, I blame the API I'm working against, not following standards. In this case it was Swagger 2.0, and the json wasn't 100% valid.

TanayParikh commented 3 years ago

Thanks all, closing this out as there doesn't appear to be any direct action items remaining.

GRMorgan commented 3 years ago

I'm not sure this should have been closed. We have a work around, that doesn't fix the fact that clients that don't compile are being generated if you specify an operationId in the swagger definition.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

programatix commented 2 years ago

I found that the issue is caused by operationId where duplicated ID is used. The example of the following will cause this issue,

...
        "operationId": "getUser",
...
        "operationId": "getUser_1",

The [underscore] + [numeric] in the operationId seems to be truncated, thus causing the duplicate.

jasoncwik commented 2 years ago

Setting the option /OperationGenerationMode:SingleClientFromOperationId fixed it for me:

e.g.

    <OpenApiReference Include="connectv2.swagger.json">
      <ClassName>ConnectV2Client</ClassName>
      <Options>/OperationGenerationMode:SingleClientFromOperationId</Options>
    </OpenApiReference>

There's a good write-up with all the options here: https://stevetalkscode.co.uk/openapireference-commands

geometrikal commented 1 year ago

Setting the option /OperationGenerationMode:SingleClientFromOperationId fixed it for me:

e.g.

    <OpenApiReference Include="connectv2.swagger.json">
      <ClassName>ConnectV2Client</ClassName>
      <Options>/OperationGenerationMode:SingleClientFromOperationId</Options>
    </OpenApiReference>

There's a good write-up with all the options here: https://stevetalkscode.co.uk/openapireference-commands

Worked for me, but had to delete the obj folder first

datvm commented 6 months ago

I am using it for Apple Connect API (specs, grabbed from the doc), I still have this issue. I added <Options>/OperationGenerationMode:SingleClientFromOperationId</Options> but I received a new error from this faulty generated code:

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.0.7.0 (NJsonSchema v11.0.0.0 (Newtonsoft.Json v13.0.0.0))")]
    public enum SubscriptionStatusUrlVersion
    {

        [System.Runtime.Serialization.EnumMember(Value = @"V1")]
        V1 = 0,

        [System.Runtime.Serialization.EnumMember(Value = @"V2")]
        V2 = 1,

        [System.Runtime.Serialization.EnumMember(Value = @"v1")]
        V1 = 2,

        [System.Runtime.Serialization.EnumMember(Value = @"v2")]
        V2 = 3,

    }
The type 'SubscriptionStatusUrlVersion' already contains a definition for 'V1'
// And more