Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vasrinwork opened this issue Aug 29, 2024 · 2 comments · May be fixed by #3069
Assignees

Comments

@vasrinwork
Copy link

vasrinwork commented Aug 29, 2024

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
Copy link
Member

xuzhg commented Sep 3, 2024

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 xuzhg transferred this issue from OData/WebApi Sep 19, 2024
@xuzhg
Copy link
Member

xuzhg commented Sep 19, 2024

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.

xuzhg added a commit that referenced this issue Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants