models for inline response schemas collide when multiple operations on the same path item

[jchannon]

I have uploaded my modified openapi.json (with a txt extension because Github won't allow uploading json files) as a repro to show that the json model creates the incorrect C#

"schema": {
                  "type": "object",
                  "properties": {
                    "data": {
                      "type": "array",
                      "items": {
                        "type": "object",
                        "properties": {
                          "webhookId": {
                            "type": "number"
                          },
                          "callbackUrl": {
                            "type": "string",
                            "format": "uri",
                            "example": "https://myserver.com/send/callback/here"
                          }

I would expect a C# model to have a List<T> Data {get;set;} but instead it creates a type with the defined json schema properties as top level properties public double? WebhookId { get; set; }

I have similar models and the generated C# works with the expected List<T> but just not for this path.

Any ideas why?

hacked.json.txt

[baywet]

Thanks for reaching out. What version of kiota are you using? We've fixed a similar issue in 1.4.0 and I want to make sure this is the version you are using before we look into it further.

[jchannon]

Looks like it's 1.4.0

[baywet]

This is happening because your description has two inline response schemas for the same path item (GET POST) and they are different from one another. And because we're not including the operation name in the inline response model name we're generating. I'm not sure how we missed that, it's trivial, it shows in a number of unit tests, and we already do it for inline request bodies. I think this is most likely because we don't have a case like that in Microsoft Graph due to OData conventions.

How do we fix that?

• Just including the operation name (see #2953 ) would represent a breaking change for Go/Dotnet, leading to a major rev of Kiota (and of the Graph SDKs that depend on it)
• Not doing it propagates the problem to other languages that are soon going to be stable (PHP/Python/Java...)
• Doing it only when a collision occurs will introduce breaking changes over times (i.e. if a new operation gets added and the client is refreshes to include it).

ls .\src\Microsoft.Graph\Generated\ -Recurse -Filter *Response.cs | ? {$_.FullName -notlike "*Models*" } | measure

This on MS Graph dotnet v1 gives 435 results.

Let me take that discussion to the team...

[baywet]

As a workaround for the time being: you could move one of those as an component schema instead and you should get a normal behavior

[jchannon]

Ah ok thanks - look forward to the discussion.

Unfortunately I don't control the openapi generation, it's from an api we integrate with.

[darrelmiller]

Dammit Jonathan, can you play with anything without breaking it? Not only did you break it, but it is kinda embarassing too. :-P

@baywet I'm shocked we haven't run into this situation before. This is a likely enough scenario that we should seriously consider doing the v2 bump as you had suggested. I'd rather us solve these problems sooner rather keep pushing them off so that we can avoid the major bump.

[jchannon]

You're not the first to mention it, it's an innate ability I seem to have 😂

If you could have v2 on my desk by 9am tomorrow, that'd be great 😉

[baywet]

@darrelmiller this will require major bumps of service libs for dotnet and Go as well. Just want to make sure we're good with that CC @ddyett @maisarissi I'm going to create a v3 milestone and start moving blocked things in there to reduce the scope of v2 and see what breaking changes we can bundle together while at it.

[baywet]

I had an idea for this one to avoid shipping a breaking change while fixing the situation:

We could still emit the "wrong" type with the "wrong" name (and related executor method) as obsolete, and emit the "right" ones too. (just for dotnet and Go) Since this is a pattern we've implemented for indexers and a couple of other things, we could also have a flag so people can skip the obsolete (backward compatible) assets like "KIOTA_EXCLUDE_BACKWARD_COMPATIBLE" (default false) Reviewed during today's planning meeting.

[baywet]

PR #2953 is ready for review. We're introducing a new --exclude-backward-compatible switch to the generate command (which can also be set through environment variables) so people with a new client don't end up with the compatibility assets