OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 158 forks source link

.NET6 / OData 8 not serializing OData responses properly #424

Open gordon-matt opened 2 years ago

gordon-matt commented 2 years ago

I originally opened this issue in the WebApi repo (not realizing there's this new repo specifically for .NET Core).. The full details of the issue can be found here:

https://github.com/OData/WebApi/issues/2602

But basically I am seeing this error in .NET 6 (where everything was fine in .NET 5):

Uexpected End

I guess the guy who was going to look into it got busy, so I decided to look into this further myself by debugging with your source code.. and I've discovered that this issue happens when one of the properties is an object[]. In my case:

public object[] KeyValues
{
    get { return new object[] { Id }; }
}

There seems to be an error happening in Microsoft.AspNetCore.OData.Formatter.Serialization.CreateStructuralPropertyBag() for that one property. No error message thrown.. it just breaks right where it was silently and I get a broken JSON response.

Breaks Here

Hoping this now helps you guys find and fix the issue.

gordon-matt commented 2 years ago

Okay, so I was playing with the code and tried this:

foreach (IEdmStructuralProperty structuralProperty in structuralProperties)
{
    if (structuralProperty.Type != null && structuralProperty.Type.IsStream())
    {
        // skip the stream property, the stream property is written in its own logic
        continue;
    }

    // ========================================================================
    // New Code
    // ========================================================================
    if (structuralProperty.Type.Definition.TypeKind == EdmTypeKind.Collection &&
        structuralProperty.Type.GetElementType().TypeKind == EdmTypeKind.Untyped)
    {
        continue;
    }
    // ========================================================================
    // END: New Code
    // ========================================================================

    ODataProperty property = CreateStructuralProperty(structuralProperty, resourceContext);
    if (property != null)
    {
        properties.Add(property);
    }
}

This works, but I am not sure it's a good idea.. Then I realized I should probably be able to use either a [JsonIgnore] attribute or an [IgnoreDataMember] attribute.

Neither of these seem to work:

[Newtonsoft.Json.JsonIgnore]
[System.Text.Json.Serialization.JsonIgnore]

Maybe consider supporting them to ignore certain properties from being serialized..

Then I tried [IgnoreDataMember]. Unfortunately it did not work when used on the IEntity interface that I have, but when using a base class I created (BaseEntity<TKey>) and applying [IgnoreDataMember] to my KeyValues property there (and of course ensuring that my entities inherit that instead of the interface), then it works.

So.. I will need to modify tons of entities in multiple apps to inherit from the BaseEntity instead of directly inheriting the interface. It's not ideal, but at least I have a workaround that's not a mess.

I have some questions though:

  1. Why was this not an issue prior to v8?

  2. What if someone wants to ignore a property in OData serialization, but not use [IgnoreDataMember] because they want to still use it in Entity Framework? Perhaps an [ODataIgnore] attribute would be helpful? In my case, it's no issue, as it's a readonly field returning the key values (Maybe OData should not serialize readonly properties by default?).

  3. Are there any better solutions to this scenario?

stevendarby commented 2 years ago

Have you tried installing the OData newtonsoft package and called the extension it provides?

https://www.nuget.org/packages/Microsoft.AspNetCore.OData.NewtonsoftJson/

They switched from newtonsoft to system.text.json so this package may help restore support for newtonsoft.

ramaseshulukka commented 1 year ago

Okay, so I was playing with the code and tried this:

foreach (IEdmStructuralProperty structuralProperty in structuralProperties)
{
    if (structuralProperty.Type != null && structuralProperty.Type.IsStream())
    {
        // skip the stream property, the stream property is written in its own logic
        continue;
    }

    // ========================================================================
    // New Code
    // ========================================================================
    if (structuralProperty.Type.Definition.TypeKind == EdmTypeKind.Collection &&
        structuralProperty.Type.GetElementType().TypeKind == EdmTypeKind.Untyped)
    {
        continue;
    }
    // ========================================================================
    // END: New Code
    // ========================================================================

    ODataProperty property = CreateStructuralProperty(structuralProperty, resourceContext);
    if (property != null)
    {
        properties.Add(property);
    }
}

This works, but I am not sure it's a good idea.. Then I realized I should probably be able to use either a [JsonIgnore] attribute or an [IgnoreDataMember] attribute.

Neither of these seem to work:

[Newtonsoft.Json.JsonIgnore]
[System.Text.Json.Serialization.JsonIgnore]

Maybe consider supporting them to ignore certain properties from being serialized..

Then I tried [IgnoreDataMember]. Unfortunately it did not work when used on the IEntity interface that I have, but when using a base class I created (BaseEntity<TKey>) and applying [IgnoreDataMember] to my KeyValues property there (and of course ensuring that my entities inherit that instead of the interface), then it works.

So.. I will need to modify tons of entities in multiple apps to inherit from the BaseEntity instead of directly inheriting the interface. It's not ideal, but at least I have a workaround that's not a mess.

I have some questions though:

  1. Why was this not an issue prior to v8?
  2. What if someone wants to ignore a property in OData serialization, but not use [IgnoreDataMember] because they want to still use it in Entity Framework? Perhaps an [ODataIgnore] attribute would be helpful? In my case, it's no issue, as it's a readonly field returning the key values (Maybe OData should not serialize readonly properties by default?).
  3. Are there any better solutions to this scenario?

@gordon-matt Were you able to fix these i am facing similar issue below Could you check that https://github.com/OData/AspNetCoreOData/issues/739

gordon-matt commented 1 year ago

@ramaseshulukka I am still using the workaround I mentioned - an abstract base class with [IgnoreDataMember] decorating the property I need ignored. You can see my implementation here: BaseEntity.cs

ramaseshulukka commented 1 year ago

@gordon-matt ok thanks . May be your scenario is different incase if you have any similarity with my issue i was able to fix

https://github.com/OData/AspNetCoreOData/issues/739