OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
854 stars 476 forks source link

Nested $select on optional complex property throws exception when complex property is null #2413

Open UmairB opened 3 years ago

UmairB commented 3 years ago

When an entity has a nullable complex property (which is implemented as an owned type in entity framework), a nested $select on the complex property will throw an error whenever the complex property is null.

Assemblies affected

Tried on Microsoft.AspNetCore.OData version 7.5.1 and the 8 beta, being used with Microsoft.EntityFrameworkCore.SqlServer v5.0.2

Reproduce steps

Setup the following classes

public class EntityWithOwnedType
{
    public int Id { get; set; }

    public SingleOwnedType SingleOwnedType { get; set; }
}

public class SingleOwnedType
{
    public int SomeId { get; set; }

    public string Name { get; set; }
}

with the following entity framework core configuration

builder.HasKey(e => e.Id);
builder.Property(e => e.Id).UseIdentityColumn();

builder.OwnsOne(e => e.SingleOwnedType, b =>
{
    b.WithOwner();
});

and the following odata configuration

var entityType = builder.EntitySet<EntityWithOwnedType>("EntityWithOwnedTypes").EntityType;
entityType.ComplexProperty(e => e.SingleOwnedType).IsNullable();

Expected result

The following endpoint /entityWithOwnedTypes?$select=Id,SingleOwnedType($select=SomeId) should return

{
    "@odata.context": "https://localhost:5001/odata/$metadata#EntityWithOwnedTypes(Id,SingleOwnedType/SomeId)",
    "value": [
        {
            "Id": 1,
            "SingleOwnedType": null
        }
    ]
}

if SingleOwnedType is null on EntityWithOwnedType

Actual result

Invalid, partial json is returned with the error "Microsoft.OData.ODataException: The property 'SomeId[Nullable=False]' of type 'Edm.Int32' has a null value, which is not allowed."

Additional detail

Not quite sure what the expected behaviour is here. The error occurs for SingleOwnedType($select=SomeId) but for SingleOwnedType($select=Name) it returns the following valid json:

{
    "@odata.context": "https://localhost:5001/odata/$metadata#EntityWithOwnedTypes(Id,SingleOwnedType/Name)",
    "value": [
        {
            "Id": 1,
            "SingleOwnedType": {
                "Name": null
            }
        }
    ]
}

This is actually not correct from the database perspective since "SingleOwnedType" is actually null. I know why it happens: SomeId property on SingleOwnedType is not optional, but the way it is configured and stored in EF Core, yes it is not optional but the owned entity/complex type itself is. This is a mismatch in how EF Core is storing and handling the data vs in how odata is interpreting the data. Is the solution here to update the odata configuration to make the SomeId optional even though they are not? Because if that is the case that would be a shame as it's not a good reflection on the data.

What other options do I have here?

On another note, the following query /entityWithOwnedTypes?$select=Id,SingleOwnedType produces the following projection in odata:

.Lambda #Lambda1<System.Func`2[ODataIssue.Models.EntityWithOwnedType,Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[ODataIssue.Models.EntityWithOwnedType]]>(ODataIssue.Models.EntityWithOwnedType $$it)
{
    .New Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[ODataIssue.Models.EntityWithOwnedType](){
        Model = .Constant<Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]>(Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]).TypedProperty,
        Container = .New Microsoft.AspNetCore.OData.Query.Container.PropertyContainer+NamedPropertyWithNext0`1[System.Nullable`1[System.Int32]]()
        {
            Name = "Id",
            Value = (System.Nullable`1[System.Int32])$$it.Id,
            Next0 = .New Microsoft.AspNetCore.OData.Query.Container.SingleExpandedProperty`1[Microsoft.AspNetCore.OData.Query.Wrapper.SelectAll`1[ODataIssue.Models.SingleOwnedType]]()
            {
                Name = "SingleOwnedType",
                Value = .New Microsoft.AspNetCore.OData.Query.Wrapper.SelectAll`1[ODataIssue.Models.SingleOwnedType](){
                    Model = .Constant<Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]>(Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]).TypedProperty,
                    Instance = $$it.SingleOwnedType,
                    UseInstanceForProperties = True
                },
                IsNull = $$it.SingleOwnedType == null
            }
        }
    }
}

What's interesting here is that odata is performing a IsNull = $$it.SingleOwnedType == null check at the bottom. So it already has code to check if the SingleOwnedType is null or not. Why does it do it for /entityWithOwnedTypes?$select=Id,SingleOwnedType but for nested select's on SingleOwnedType?

xuzhg commented 3 years ago

If we expect to have the following payload, it seems not consistent with the context uri. @mikepizzo

{
    "@odata.context": "https://localhost:5001/odata/$metadata#EntityWithOwnedTypes(Id,SingleOwnedType/SomeId)",
    "value": [
        {
            "Id": 1,
            "SingleOwnedType": null
        }
    ]
}
UmairB commented 3 years ago

Then what is the expectation for the following query /entityWithOwnedTypes?$select=Id,SingleOwnedType($select=SomeId) where SingleOwnedType is an optional/nullable complex type with a required property SomeId when SingleOwnedType is null? At the moment it throws an odata error and returns malformed json which obviously isn't right...

mikepizzo commented 1 year ago

If "SingleOwnedType" is nullable, and has a null value, then that property should be returned with a null value; we should not make it a non-null object in order to return a nested property.