WebApi v7 throws `ResourceTypeAnnotationNotFirst` error when payload has both @removed and @odata.type

vasrinwork commented 2 months ago

Webapiv7 throws ResourceTypeAnnotationNotFirst error when payload has both @removed and @odata.type.

Our clients seem to be reusing the same class structure for both normal OData payloads for POST and Delta payloads. This issue was not encountered by our clients in the older OData versions, possibly because the concept of @removed did not exist in older libraries.

The library seems to be reordering the json payload in this order: @odata.context > @removed > @odata.type. So, when @removed property is encountered, it skips checking for @odata.type property. However, the logic later in the ReadEntryInstanceAnnotation() method in ODataJsonLightResourceDeserializer.cs assumes that @odata.type was already encountered in ReadResourceStart and hence considers the occurrence as duplicate or occurring in an incorrect order.

Assemblies affected

OData WebApi lib 7x.

Reproduce steps

Consider the following payload sent to the POST planner/plans endpoint to create a new plan

{
  "@odata.type": "#microsoft.taskServices.plan",
  "title": "The Plan",
  "@removed": false,
  "owner": "abcd"
}

Expected result

201 Created

{"@odata.context":"http://localhost/V3.0/$metadata#plans/$entity","owner":"abcd","title":"The Plan"}

Actual result

400 BadRequest

"error":{"code":"","message":"The request is invalid:\r\nThe 'odata.type' instance annotation in a resource object is preceded by an invalid property. In OData, the 'odata.type' instance annotation must be either the first property in the JSON object or the second if the 'odata.context' instance annotation is present."

Additional detail

We have upgraded odata to v7 and deployed the changes to Prod and are receiving incidents from our clients due to this issue. We are blocked from deploying to further regions.

xuzhg commented 2 months ago

OData Lib reorders the JSON payload to get better performance for 'stream' reading.

Here's the reordering: https://github.com/OData/odata.net/blob/release-7.x/src/Microsoft.OData.Core/JsonLight/ReorderingJsonReader.cs#L627C1-L628C77

You may noticed that '@odata.removed' is reordered before the '@odata.type'.

However, OData Lib has the following codes:

https://github.com/OData/odata.net/blob/release-7.x/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs#L790-L791

It assumes the '@odata.type' should be the first property or second property if it contains '@odata.context'.

So, there's a conflict and don't know how to resolve it. I'd like to invite @mikepizzo to share his inputs to help.

xuzhg commented 1 month ago

From an OData Library perspective:

1) The ordering reader should reorder as @context, @removed, @type, @id 2) When reading, we should support reading @context, @removed, @type, @id in any order.

However, I don't think that fixes the issue. What I believe would fix the issue is for 3) The library should ignore @removed if its value is null, or anything other than a JSON object.

OData / odata.net