OData / AspNetCoreOData

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

Allow casting Edm.Untyped to a primitive type for `$filter`ing when the other operand is also a primitive #1208

Open orty opened 7 months ago

orty commented 7 months ago

Assemblies affected ASP.NET Core OData 8.2.5

Describe the bug Hi, I encounter an issue with an entity in which a property can be of several primitive types :

This is caused by a TPH strategy implemented on the database layer, where an abstract base class is inherited by concrete ones which then hold the actual property value, with its proper type. I expose only one entity type on the API side through OData, the property is typed as object (Edm.Untyped in the model), allowing it to be dynamic, and a mapping layer handles the projection of the database models to the DTO.

Everything works fine when querying plain data, and a myProperty@odata.type: "#Guid" annotation is added as expected. Problems appear when I want to $filter the data:

This is caused by the following type check:

https://github.com/OData/AspNetCoreOData/blob/157e4ce2a9595c585bfd433ea88034aeea8e1b47/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinder.SingleValueFunctionCall.cs#L589C1-L590C126

    if ((!targetEdmTypeReference.IsPrimitive() && !targetEdmTypeReference.IsEnum()) ||
        (context.Model.GetEdmPrimitiveTypeReference(source.Type) == null && !TypeHelper.IsEnum(source.Type)))
    {
        // Cast fails and return null.
        return NullConstant;
    }

The source expression is evaluated as having the object CLR type, which is correct. But in my case, the concrete type (which I assume could be retrieved because of the very existence of the @odata.type annotation) would be a primitive type. Would it be possible to implement such a retrieval, in order to be able to, at least, try to cast to the destination primitive type and let the final try ... catch handle types mismatches ?

I would be glad to issue a pull request with a fix 👍 . Also, if I made a mistake in my model and there was an easier way to implement my business need, please let me know.

Many thanks.

julealgon commented 7 months ago

This is caused by a TPH strategy implemented on the database layer, where an abstract base class is inherited by concrete ones which then hold the actual property value, with its proper type. I expose only one entity type on the API side through OData, the property is typed as object

I don't necessarily have the solution to your issue, but I just wondered about this choice here. Why did you use TPH (which gives you distinct types for each variation) only to then "collapse" them all in a object instance? Why not just expose the same hierarchy of types to OData and use casts/etc when needed?

The reason I ask is that what you are doing feels like it adds complexity to the issue by switching something that doesn't need to be dynamic, to dynamic.

Keep me honest if I'm missing something obvious here.

orty commented 7 months ago

I understand your question, and the reason is that I first tried to do so (an abstract base type and concrete children) and I stumbled upon another issue because either EF, AutoMapper or OData was having a bad time when materializing the API response. The issue was that it tried to construct the abstract type itself, no matter what combination of mapping configuration I tried.

I am stuck with a lot of interdepencies between these layers because I need to maintain an end-to-end projectability between the DB and the API response (IQueryable and stuff...)

orty commented 7 months ago

The more I try to wrap my head around this issue, the more I realize how hard it would be to "infer" at expression binding time that the property registered as Edm.Untyped would be actually of a primitive type. The @odata.type data annotation is added on materialized values, for the $filter query binding the parser has basically no value to look for, yet.

I am trying to implement something different using a generic type for the enclosing entity (the one with the object Value {get; set;} property), and give it a TValue type but it adds a whole new complexity level to my modeling :/ (I am not even sure I can push this idea to a point where it just works, regardless of the query)

orty commented 7 months ago

I just thought of something, create a new custom type to hold the value, declare implicit conversion operators to and from the desired primitive types I need to implement, and finally declare this type as a custom value type in my model.

I see several discussions on this topic across SO and this repository, gonna give it a shot.

Angelinsky7 commented 1 month ago

I've got the same issue, i'm trying to to this $filter=cast(untypeValue, Edm.String) eq 'test' knowing that in that case this untypeValue is effectively a String and fail miserably because of

if ((!targetEdmTypeReference.IsPrimitive() && !targetEdmTypeReference.IsEnum()) ||
    (context.Model.GetEdmPrimitiveTypeReference(source.Type) == null && !TypeHelper.IsEnum(source.Type)))
{
    // Cast fails and return null.
    return NullConstant;
}

is there any good workaround (with keeping the Untype Value Type of course) ?

Thanks !