Azure / autorest.csharp

Extension for AutoRest (https://github.com/Azure/autorest) that generates C# code
MIT License
142 stars 166 forks source link

The generator crashes when a model gets properties with the same name from multiple base models (in swagger) #4127

Open KotanaSai21 opened 10 months ago

KotanaSai21 commented 10 months ago

Migrating a C# SDK application from Microsoft.Rest.ClientRuntime to Azure.Core. While regenerating the client files using v3 of the AutoRest tooling getting this error.

image

These are the versions of autorest we're using.

info    | AutoRest core version selected from configuration: 3.10.1.
info    |    Loading AutoRest core      'C:\Users\.autorest\@autorestcore@3.10.1\nodemodules\@autorest\core\dist' (3.10.1)
info    |    Loading AutoRest extension '@microsoft.azure/autorest.csharp' (2.2.57->2.2.57)
info    |    Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.43->2.3.43)

Here is my swagger file. Could you please let me know how to resolve this error ?

timotheeguerin commented 10 months ago

Hey @KotanaSai21 this error is coming from some autorest v2 plugins which is not maintained anymore. Can you share the command you tried to run?

KotanaSai21 commented 10 months ago

Thanks for the reply @timotheeguerin. This is the generate.bat script we run to autogenerate the client files. We specified. We updated the autorest version call AutoRest --version=3.10.1 powerbi.md. In the powerbi.md file, we also updated autorest.csharp version to latest version 2.3.102.

I think I missed removing the Microsoft.Rest dependency from this file. Will remove the entire dependency of Microsoft.Rest and try to regenerate the client files using the latest version.

Please let me know if there is a gap in the command I'm trying to run to generate files.

timotheeguerin commented 10 months ago

That would be the problem that version of csharp is not the latest but the one that worked with autorest v2.

latest is 3.0.0-beta.20240109.1

they do have a weird naming.

KotanaSai21 commented 10 months ago

Thank you so much @timotheeguerin. We will try using this version. We will work on this and reach out to you if we have any other issues in future.

KotanaSai21 commented 10 months ago

Hy @timotheeguerin , Updated the version. Here is the snapshot of autorest version I'm using.

image

But now I'm facing another issue.

image

Could you please help me to fix this issue? Unable to find any doc or reason why it is failing. Found one github issue but didn't understand how to resolve this. Thank you :)

timotheeguerin commented 10 months ago

Do you have a single value enum in your swagger somewhere with the value int32 but you have no type of that definition?

can you share the swagger ptherwise

KotanaSai21 commented 10 months ago

Thanks again @timotheeguerin . Yes, we have an Enum and was Able to resolve that issue and was able to generate the files using auto rest v3 successfully.

But we have model class files, and I was unable to generate the model class files using v3. Found this GitHub Issue on how to generate model class files.

Here is my config code.

input-file: swaggers\swagger.json
namespace: Microsoft.PowerBI.Api
csharp: true
output-folder: PowerBI.Api\Source
clear-output-folder: true
override-client-name: PowerBIClient
generation1-convenience-client: true
add-credentials: true
model-name: PowerBIClient 

While generating the files, getting this error. image

Could you please help me out in resolving this issue? Thank You :)

timotheeguerin commented 10 months ago

Transferred to the csharp extension repo as this now seems to be just the problem crashing in it

ArcturusZhang commented 10 months ago

Hi @KotanaSai21 use the swagger I found here, I have a produce issue, such as:

error   |   Error: Operation response '/paths/~1v1.0~1myorg~1datasets~1{datasetId}~1tables~1{tableName}/put/responses/200' produces type couldn't be resolved. Operation is probably is missing a produces field and there isn't any global value. Please add "produces": [<contentType>]"

which is a minor issue, I replaced every "produce": [] with "produce": ["application/json"] and I am able to reproduce your issue to this: image which looks like an issue Timothee has answered.

Am I using the out-dated swagger? If possible, since I saw your project is on github as well, please create a draft PR in your repo and let me know the command you are running with the exception, so that I could trouble shoot.

KotanaSai21 commented 10 months ago

Hi @ArcturusZhang , Thanks for looking into this. I forked the repository here and made necessary changes.

Resolved the issues with the swagger and find the updated swagger file here.

I was able to generate the files but when adding generation1-convenience-client: true to generate the model files facing the above mentioned error.

ArcturusZhang commented 10 months ago

Here is the issue: https://github.com/KotanaSai21/PowerBI-CSharp/blob/master/sdk/swaggers/swagger.json#L9878 An enum is defined, with a format of uuid, but its values are not uuid at all, therefore the generator throws the exception trying to parse those values (default) Well removing the uuid gives me a new exception - I need to investigate that as well

ArcturusZhang commented 10 months ago

OK the second issue happens in your swagger as well: This model WorkspaceInfoDataset has quite a few allOf, which is fine, but two of its allOf (UpstreamDatasetsProperties and WorkspaceInfoDataflowProperties), have the same property: upstreamDatamarts We currently do not support this (we should) As a workaround, I suggest you to modify your models so that we do not have this kind of cases.

ArcturusZhang commented 10 months ago

In the meantime, I suggest we could try to write your spec using typespec and use our latest generator to generate which produces better code, and as well as making your spec more intuitive using typespec.

KotanaSai21 commented 9 months ago

Since it is not currently supported to have the same property, created a duplicate property and updated my swagger file as a workaround for now. Now I am able to generate the Models files.

Thanks so much @ArcturusZhang , @timotheeguerin for helping me in generating the files.

ArcturusZhang commented 9 months ago

It is great that the workaround works for you. I will change the title of your issue to track the real issue: deduplicate the property when we have multiple with the same name. Fixing it might be relatively low priority because we are focusing on typespec generation now and usually we do not have swaggers with this issue.

KotanaSai21 commented 8 months ago

When we generated the files using auto rest, I was facing issue with the models property. Opened a GitHub issue https://github.com/Azure/autorest.csharp/issues/4294. But no one replied.

@ArcturusZhang , @timotheeguerin Could you guys help me out with the issue or guide me to correct place where I can get response.