It doesn't make sense to resolve untyped integer value to Edm.Decimal

[xuzhg]

Short summary (3-5 sentences) describing the issue.

Assemblies affected

Which assemblies and versions are known to be affected e.g. OData .Net lib 7.x

Reproduce steps

Supposed I have the dynamic property or (untyped property) value as below:

{
    ....
    "Data": 42
}

Expected result

*The property value of 'Data' should be a "System.Int32".

Actual result

*The property value of 'Data' is a "System.Decimal"

Additional detail

• It's weird, because '42' looks like an integer, not a decimal.

Here's the codes: https://github.com/OData/odata.net/blob/master/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertyAndValueDeserializer.cs#L265-L276

In my opinion, we should add extra 'else if' to identify other 'common' types.

[xuzhg]

Interesting thing:
If we have the following untyped value: (Be noted, put the value into a untyped collection)

{
   "Data": [
        42
   ]
}

ODL reads it as 'Edm.Int32'.

[habbes]

Would this be a breaking change? Or was it just a bug?

[xuzhg]

Would this be a breaking change? Or was it just a bug?

I don't it's a breaking change. What do you think @mikepizzo ?

[mikepizzo]

Yes; this would be a breaking change. No, we should not change it.

Per protocol, if there is no type information (it's not a defined property and there's not @odata.type annotation) then we have to determine the type heuristically, not based on value. See http://docs.oasis-open.org/odata/odata-json-format/v4.01/odata-json-format-v4.01.html#sec_ControlInformationtypeodatatype.

Note that, per spec, we should be interpreting this as Edm.Double, not Edm.Decimal but, again, it would be a breaking change to change the current behavior.

[gathogojr]

In a case like this where an untyped numeric value is read as a decimal, there could be customers depending on this behaviour and changing that could lead to a hidden semantics break that would be hard for the customer to discover.

If we change the logic such that an untyped numeric value is read as an Edm.Double instead of Edm.Decimal, we would need to make a similar change when writing dynamic properties such that double values are written without the @odata.type annotation while everything else is written with the @odata.type annotation. That change would need to be behind a feature flag.

We could introduce the new/correct behaviour behind a feature flag and retain the old/incorrect behaviour as the default. In a subsequent major release, we can switch the default to be the correct behaviour and retain the feature flag indefinitely for customers who may prefer to retain the old behaviour.

Changes that affect what's written on the wire should be behind a feature flag to make it possible for the customer to toggle their desired behaviour. Such a flag would be supporting indefinitely for the writing scenarios. When it comes to reading, it'd be easier for services to adjust their behaviour so we can deprecate the flag with time.

We could have bit mask and that bit mask could have bits for different compatibility flags. Someone who wanted to preserve backwards compatibility could set all bits. The challenge with that approach would be if the flags exceeded the number than can fit into Int64

[adzhiljano]

Hey guys, we're currently trying to migrate our OData based project from .NET 4.8 to .NET 8 and we stumbled upon this problem. We have an OData Action that has one open type parameter like so:

<Action Name="SomeAction">
    <Parameter Name="SomeActionRequest" Type="SomeActionRequestType" Nullable="false"/>
    <ReturnType Type="SomeActionResponseType" Nullable="false"/>
</Action>

<ComplexType Name="SomeActionRequestType" OpenType="true">
    <Property Name="SomeStringProperty" Type="Edm.String"/>
</ComplexType>

We are expecting some "well-known" numeric dynamic properties and with Microsoft.AspNet.OData (7.5.14) they are deserialized as System.Int32, but with Microsoft.AspNetCore.OData (8.2.5) they are deserialized as System.Decimal.

We haven't tried updating Microsoft.AspNet.OData to latest and try it out again, but we're guessing the result would be the same. We did some debugging and found out the following difference regarding the mentioned numeric dynamic properties:

In Microsoft.AspNet.OData the deserialized ODataProperty.Value is of type ODataUntypedValue and goes through the following conversion logic: https://github.com/OData/WebApi/blob/a5c4041d07e8d4bde06d7374f49e0e8c3a62b7da/src/Microsoft.AspNet.OData.Shared/Formatter/Deserialization/DeserializationHelpers.cs#L419 In Microsoft.AspNetCore.OData the deserialized ODataProperty.Value is of type ODataPrimitiveValue and does NOT go through the following conversion logic (which seems to be pretty close to the old version one): https://github.com/OData/AspNetCoreOData/blob/main/src/Microsoft.AspNetCore.OData/Formatter/Deserialization/DeserializationHelper.cs#L299

It feels like a breaking change in the ODL libraries. Our best guess is that there is a difference between the sync and async deserialization logic.

Thanks!

[xuzhg]

@adzhiljano Microsoft.AspNetCore.OData 8.x by default configs to read the untyped value as primitive value if it's 'primitive value' literal. You can see the settings at: https://github.com/OData/AspNetCoreOData/blob/599dc1bdca9edc2cf88935ed54fe46ad5478da5b/src/Microsoft.AspNetCore.OData/Abstracts/ContainerBuilderExtensions.cs#L46-L48

If you want to get the 'ODataUntypedValue', you can config ReadUntypedAsString=true for ODataMessageReaderSetting.

Be noted, as the comments ( in the above link) mentioned, 'ReadUntypedAsString' could be deleted in the next ODL major version (8.x).

https://github.com/OData/odata.net/blob/main/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs#L134 is recommended to resolve the untyped primitive type.

[adzhiljano]

Hey @xuzhg, thank you very much for the quick response and guidance! Nice to know that this was controlled by a configuration, which in the Microsoft.AspNet.OData (7.5.14) case was set to true by default. We'll adjust accordingly to the default/future behaviour of Microsoft.AspNetCore.OData (8.*).