OData / AspNetCoreOData

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

$expand fails with DTO - EF cannot translate the query expression #1245

Open haharoi opened 3 months ago

haharoi commented 3 months ago

Assemblies affected ASP.NET Core OData 8.2.5

Describe the bug Expanding collection fails as the underlying EF Core cannot translate the expression constructed by OData.

The data model consits of two entities, UserDto and RoleDto.

public class UserDto
{
    public Guid Id { get; set; }
    public string UserName { get; set; }
    public IList<RoleDto> Roles { get; set; } = new List<RoleDto>();
}
public class RoleDto
{
    public Guid Id { get; set; }
    public string RoleName { get; set; }
}

Db entities and projections are:

public class User
{
    public Guid Id { get; set; }
    public string UserName { get; set; }

    public virtual IList<Role> Roles { get; set; } = new List<Role>();

    public static Expression<Func<User, UserDto>> toDto = u => u == null ? null : new UserDto
    {
        Id = u.Id,
        UserName = u.UserName,
        Roles = u.Roles.AsQueryable().Select(Role.toDto).ToList()
    };
}
public class Role
{
    public Guid Id { get; set; }
    public string RoleName { get; set; }

    public virtual IList<User> Users { get; set; } = new List<User>();

    public static Expression<Func<Role, RoleDto>> toDto = r => r == null ? null : new RoleDto
    { 
        Id = r.Id,
        RoleName = r.RoleName,
    };
}

The underlying DB provider is Npgsql.EntityFrameworkCore.PostgreSQL. but I think it does not really matter. When trying to get list of Users with expanded Roles with http://localhost:5070/Users?$expand=Roles, EF Core tries to compile the expression

DbSet<User>()
          .Select(u => u == null ? null : new UserDto{
              Id = u.Id,
              UserName = u.UserName,
              Roles = u.Roles
                  .AsQueryable()
                  .Select(r => r == null ? null : new RoleDto{
                      Id = r.Id,
                      RoleName = r.RoleName
                  }
                  )
                  .ToList()
          })
          .Select($it => new SelectAllAndExpand<UserDto>{
              Model = __TypedProperty_0,
              Instance = $it,
              UseInstanceForProperties = True,
              Container = new NamedProperty<IEnumerable<SelectAll<RoleDto>>>{
                  Name = "Roles",
                  Value = $it.Roles
                      .Select($it => new SelectAll<RoleDto>{
                          Model = __TypedProperty_1,
                          Instance = $it,
                          UseInstanceForProperties = True
                      }
                      )
              }
          })

and fails with

System.InvalidOperationException: The LINQ expression '$it => new SelectAll<RoleDto>{
          Model = __TypedProperty_1,
          Instance = $it,
          UseInstanceForProperties = True
      }
      ' could not be translated.

Expected behavior A list of UserDto entities, with Roles property

Additional context If the null check in User.toDto is removed, everything works in this particular case. By the null check cannot be removed in my actual application as the expression is used throughout the app and in some cases it is evaluated on client (not in database) and the null-check is required. Moreover, there are a number of similar expressions for other projections (hundreds of them).

Here is a sample project with application that demonstrates the described behavior pr01ODataModels.zip.

EDM (CSDL) Model

<edmx:Edmx xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx" Version="4.0">
    <edmx:DataServices>
        <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="pr01">
            <EntityType Name="UserDto">
                <Key>
                    <PropertyRef Name="Id"/>
                </Key>
                <Property Name="Id" Type="Edm.Guid" Nullable="false"/>
                <Property Name="UserName" Type="Edm.String"/>
                <NavigationProperty Name="Roles" Type="Collection(pr01.RoleDto)"/>
            </EntityType>
            <EntityType Name="RoleDto">
                <Key>
                    <PropertyRef Name="Id"/>
                </Key>
                <Property Name="Id" Type="Edm.Guid" Nullable="false"/>
                <Property Name="RoleName" Type="Edm.String"/>
            </EntityType>
        </Schema>
        <Schema xmlns="http://docs.oasis-open.org/odata/ns/edm" Namespace="Default">
            <EntityContainer Name="Container">
                <EntitySet Name="Users" EntityType="pr01.UserDto">
                    <NavigationPropertyBinding Path="Roles" Target="Roles"/>
                </EntitySet>
                <EntitySet Name="Roles" EntityType="pr01.RoleDto"/>
            </EntityContainer>
        </Schema>
    </edmx:DataServices>
</edmx:Edmx>
julealgon commented 3 months ago

Instead of trying to do this mapping all manually, have you considered using something like AutoMapper.Extensions.OData?

I know this is not a direct answer to your question but hopefully it helps.

haharoi commented 3 months ago

Instead of trying to do this mapping all manually, have you considered using something like AutoMapper.Extensions.OData?

No, I have not yet. I'm porting an existing project from NET Framework+EF to NET+EF Core. The project does not use AutoMapper and utilizing it seems to be a lot of work.

julealgon commented 3 months ago

@haharoi

The underlying DB provider is Npgsql.EntityFrameworkCore.PostgreSQL. but I think it does not really matter.

This does matter quite a bit actually since the exception you are getting is coming from the provider not being able to translate the query.

Can you share the version of the Npgsql.EntityFrameworkCore.PostgreSQL package you are using? And, if it's an older version, could you try upgrading it and testing to see if the problem persists?

haharoi commented 3 months ago

Can you share the version of the Npgsql.EntityFrameworkCore.PostgreSQL package you are using? And, if it's an older version, could you try upgrading it and testing to see if the problem persists?

It is of the latest version, Npgsql.EntityFrameworkCore.PostgreSQL version 8.0.4

haharoi commented 3 months ago

This does matter quite a bit actually since the exception you are getting is coming from the provider not being able to translate the query.

I just checked with SQL Server:

  1. Added Microsoft.EntityFrameworkCore.SqlServer package (v8.0.5)
  2. Replaced UseNpgsql call with
    .UseSqlServer("data source=.;initial catalog=Users;persist security info=True;Integrated Security=True;TrustServerCertificate=True;multipleactiveresultsets=True")

    and got the same error.

julealgon commented 3 months ago

@haharoi you mentioned you are performing a migration right now. Did this same code work before with other versions of OData and EF? If so, could you also share that information?

haharoi commented 3 months ago

Yes, the same code worked with EntityFramework, Version=6.4.4 and Microsoft.AspNet.OData, Version=7.4.1.0

corranrogue9 commented 3 months ago

@xuzhg can you please loop in EF Core folks to help investigate? It seems the issue could be with the client evaluation changing from EF to EF Core

julealgon commented 3 months ago

@xuzhg can you please loop in EF Core folks to help investigate? It seems the issue could be with the client evaluation changing from EF to EF Core

@Roji any thoughts on this particular issue?

xuzhg commented 3 months ago

It could be related to https://github.com/dotnet/efcore/issues/27460. @maumar Any comments from your side.

maumar commented 3 months ago

The suggestion that worked there was to use 'HandleNullPropagation = false' - EF is generally good at handling null protection itself (there were some issues in the past and I think that's why OData tries to compensate). That would be my first suggestion.

https://github.com/dotnet/efcore/issues/27460#issuecomment-1043446872

haharoi commented 3 months ago

It could be related to dotnet/efcore#27460. @maumar Any comments from your side.

This seems to be the similar issue indeed, and I studied it before submitting the issue.

The suggestion that worked there was to use 'HandleNullPropagation = false' - EF is generally good at handling null protection itself (there were some issues in the past and I think that's why OData tries to compensate). That would be my first suggestion.

dotnet/efcore#27460 (comment)

Unfortunately, I cannot remove null checks completely, as I described above in the Additional context section in my initial post. The same User.toDto expression, for example, is used to project the optional property of other entities in the client code.