Handling of integer number types with int16 format.

[andrueastman]

As a follow up to microsoft/OpenAPI.NET.OData#593

The integer types were replaced in the openapi description with number types which presented the a inconsistency in the generation. Due to the case at the line below.

https://github.com/microsoft/kiota/blob/a14fb8130851192316238393e4958844069967b8/src/Kiota.Builder/KiotaBuilder.cs#L1159C19-L1159C26

Properties with the type as integer and format as int16 would be projected as type integer in the codeDom.

However changing the type to integer results in the type being projected as double as opposed to integer before as there is no case to handle the integer with int16 format.

[andrueastman]

Taking a look at the table over at https://learn.microsoft.com/en-us/openapi/kiota/serialization?tabs=csharp#types-mapping, looks like we also do not have a case/mapping for the openapi int16 format.

As we can use short in C# and int16 in Golang would it make sense to update mappings? Any chance this was intentionally not catered for @baywet

From the perspective of previously generated SDKs, the change from properties from int to short may still be breaking though.

[baywet]

Thanks for starting this discussion. Happy to see that the mapping table is being useful here.
Adding proper support for int16 would mean adding a new method to the parse node and serialization writer interfaces, which means a breaking change.
Given it's not widely used, I don't think we could mandate that major rev just for this change. And given it's a regression, we don't have the luxury to wait for other breaking changes to be implemented, and bundled with this one.

We could probably map it to int32 without getting too much backlash on this one. Yes it uses a bit more memory, but it context it's not what matters the most. The main drawback being people trying to send numbers that are too large in the request body/query parameter and having the API yell at them. Not the greatest experience instead of having the compiler tell you. If we get a lot of feedback this is a problem, we can justify a breaking change. And I believe that int16 is being mapped to int32 when the type is integer (which doesn't mean anything spec-wise), which nobody has complained about to date.

The only thing that worries me with this approach is people who were using number/int16 before were getting a double. And that'll be a breaking change. It's most likely better than what they currently had since now they are not risking passing floating point numbers. Maybe we need to add this behaviour behind ecb? (which is going to remain a problem for Microsoft Graph).

Let me know what you think with this additional information.

[andrueastman]

And I believe that int16 is being mapped to int32 when the type is integer (which doesn't mean anything spec-wise), which nobody has complained about to date.

This is correct. As this was the behavior before the metadata change.

The only thing that worries me with this approach is people who were using number/int16 before were getting a double. And that'll be a breaking change. It's most likely better than what they currently had since now they are not risking passing floating point numbers. Maybe we need to add this behaviour behind ecb? (which is going to remain a problem for Microsoft Graph).

In my head, this behavior is more incorrect, and it could arguably be justified as a fix as mapping an int16 as a double is arguably more incorrect vs mapping it as an integer.

Adding proper support for int16 would mean adding a new method to the parse node and serialization writer interfaces, which means a breaking change.

Thanks for the context here.

What it looks like is we could update the mapping to map the int16 as a integer(treating it a fix). And follow up with a Kiota 2.0 issue to add support for the int16 types for the serializer/deserializer(Arguably this would be for Go,C#,Java as the other languages map to a the same type).

[baywet]

Alright, thanks for chiming in. Let's proceed with this fix and no EBC flag.

[andrueastman]

2.0 milestone item created at #5615