AutoMapper / AutoMapper.Extensions.OData

Creates LINQ expressions from ODataQueryOptions and executes the query.
MIT License
140 stars 38 forks source link

GetQueryAsync does not expand 1:1 optional relationship #150

Closed FelipeMathieu closed 2 years ago

FelipeMathieu commented 2 years ago

Hey all. I've been working with Odata and using DTOs for a quite long time and I've noticed this behavior before with old versions. However, with the new "GetQueryAsync" most of the bugs I used to face have gone. With that said, I'm working on the newest version of Automapper and Odata and I faced a scenario in that I've got a 1:1 relationship in DB mapped on a DTO that expands and works perfectly when expanding a collection inside, but when expanding the optional relationship it doesn't. It's important to mention I'm using ODataQueryOptions as required in the documentation, which works fine too. I'm gonna place my DB Entity, my EDM model config, and the DTOs. Entity: ` public class Category : BaseWithImage, ITrackable, IActivable {

    public string Name { get; set; }
    public long? ParentId { get; set; }
    public Track Track { get; set; } = new();
    public Active Active { get; set; }

    public Category ParentCategory { get; set; }
    public IEnumerable<Category> ChildrenCategories { get; set; }
    public IEnumerable<ProductType> ProductTypes { get; set; }
    public ICollection<Segment> Segments { get; set; }

} `

DB mapping: ` public class CategoryMapping : IEntityTypeConfiguration

{

    public void Configure(EntityTypeBuilder<Category> builder)
    {

        builder.HasKey(p => p.Id);
        builder.Property(p => p.Id)
            .ValueGeneratedOnAdd();

        builder.HasIndex(p => p.Name);

        builder.OwnsOne(p => p.Active, options =>
        {
            options.Property(t => t.IsActive)
                .HasColumnName("Active");
        });

        builder.OwnsOne(p => p.Track, options =>
        {
            options.Property(t => t.CreationDate)
                .HasColumnName("CreationDate");
            options.Property(t => t.CreatorId)
                .HasColumnName("CreatorId");
            options.Property(t => t.UpdateDate)
                .HasColumnName("UpdateDate");
            options.Property(t => t.UpdaterId)
                .HasColumnName("UpdaterId");
        });

        builder.HasOne(p => p.ParentCategory)
            .WithMany(p => p.ChildrenCategories)
            .HasForeignKey(p => p.ParentId)
            .OnDelete(DeleteBehavior.Restrict);
    }

}

`

EDM: ` public static void ConfigCategory(this ODataConventionModelBuilder modelBuilder)

    {

        var entitySet = modelBuilder.EntitySet<CategoryOVM>("Categories");

    }

`

Automapper Profile: ` public CategoryProfile()

    {

        CreateMap<Category, CategoryOVM>()
            .ForMember(p => p.UpdateDate, o => o.MapFrom(x => x.Track.UpdateDate))
            .ForMember(p => p.UpdaterId, o => o.MapFrom(x => x.Track.UpdaterId))
            .ForMember(p => p.CreationDate, o => o.MapFrom(x => x.Track.CreationDate))
            .ForMember(p => p.CreatorId, o => o.MapFrom(x => x.Track.CreatorId))
            .ForMember(p => p.Active, o => o.MapFrom(x => x.Active.IsActive))
            .ForAllMembers(o => o.ExplicitExpansion());

    }

`

CategoryService: ` public async Task GetAllAsync(ODataQueryOptions options, CancellationToken cancellationToken)

    {

        var result = await _categoryRepository.GetAllAsNoTracking().GetQueryAsync(Mapper, options, new QuerySettings()
        {
            AsyncSettings = new AsyncSettings()
            {
                CancellationToken = cancellationToken
            },
        });

        return CreateOkResult(result);

    }

`

The API request I make is: {{url-odata}}/categories?$select=name,parentCategory&$filter=parentId ne null&$expand=parentCategory

The response for this call using "GetQueryAsync()" is: { "@odata.context": "https://localhost:44308/odata/$metadata#Categories(id,name,parentId,parentCategory,parentCategory())", "value": [ { "id": 3, "name": "Categoria 3", "parentId": 1, "parentCategory": null } ] } and the query made is: SELECT c."Id", c."Name", c."ParentId" FROM "Categories" AS c WHERE (c."ParentId" IS NOT NULL)

It's important to mention that the "GetAllAsNoTracking()" just returns the DBset.AsNoTracking().

Here I've read a lot and didn't see anybody mentioning that than I made some assumptions and had to make a change to make what I want work. So, feel free to correct me if I'm wrong here. As far as I understood, because of a limitation that the EF has, with not having any filter clausing as where in .Include(), it won't have LEFT joins. So, we need to make projections in .Select() for that behavior works. However, I believe once I have a nullable 1:1 relationship based on ParentId?: long, it seems that this projection under the hood it's not prepared to handle that.

So, as I mentioned before, I had to make a change to make it work. I switched the "GetQueryAsync()", which I believe makes the projection and then the filter/expand, to the "GetAsync()", which I believe first makes the filter/expand and then the projection, and therefore, it works.

So, switching it to "GetAsync()" I have these returns for the same request;

The new Category Service: ` public async Task GetAllAsync(ODataQueryOptions options, CancellationToken cancellationToken)

    {

        var result = await _categoryRepository.GetAllTracked().GetAsync(Mapper, options, new QuerySettings()
        {
            AsyncSettings = new AsyncSettings()
            {
                CancellationToken = cancellationToken
            },
        });

        return CreateOkResult(result);
    }

`

The response for this call using "GetAsync()" is: { "@odata.context": "https://localhost:44308/odata/$metadata#Categories(id,name,parentId,parentCategory,parentCategory())", "value": [ { "id": 3, "name": "Categoria 3", "parentId": 1, "parentCategory": { "id": 1, "name": "Categoria 1", "creationDate": "2022-02-02T00:00:00Z", "creatorId": 1, "updateDate": "2022-09-08T19:12:56.853852Z", "updaterId": 3, "active": true, "parentId": null } } ] }

and the query made is: SELECT c."Id", c."ImageUrl", c."Name", c."ParentId", c."Active", c."CreationDate", c."CreatorId", c."UpdateDate", c."UpdaterId", c0."Id", c0."ImageUrl", c0."Name", c0."ParentId", c0."Active", c0."CreationDate", c0."CreatorId", c0."UpdateDate", c0."UpdaterId" FROM "Categories" AS c LEFT JOIN "Categories" AS c0 ON c."ParentId" = c0."Id" WHERE (c."ParentId" IS NOT NULL)

And here I had the behavior that made me understand that the "GetQueryAsync()" makes the filter/expand before the projection and it's falling to handle the left join with the 1:1 optional relationship. And the "GetAsync()" first makes the project and then resolves the filter/expand. However, here I need to point out another issue I noticed. Once the filter/expand/select are made after it, in the SQL all the fields are being retrieved to be resolved on some "ToListAsync()" under the hood. This is bad, because of performance and it's not the expected behavior.

So, on one hand with "GetQueryAsync()" I don't have the ability to expand an optional 1:1 relationship, and on the other hand I've got "GetAsync()" that brings me the expected result, but with the wrong SQL handle. As I mentioned before, I believe that happens because of the limitation we have in EF that doesn't allow us to have filtered clauses in .Include() and force us to use selection, but I think the optional relationship can be handled correctly in "GetQueryAsync()", and the "GetAsync()" could handle the selections correctly before resolving them.

One last note. For "GetAsync()" I had to use the tracked DBSet, because if I also expand the childrenCategories inside of the parentCategory expansion, like that: "$expand=parentCategory($expand=childrenCategories)", it throws me an error about circular dependency. Wouldn't be possible to have it also as NoTracking?

Pretty long issue, but I believe it could bring good improvements to this wonderful lib.

Thanks a lot.

BlaiseD commented 2 years ago

The tests here show 1:1 expansions working.

A breakpoint here will show you what the expansions are for your scenario i.e. you can confirm whether or not there is a bug.

FelipeMathieu commented 2 years ago

Have you tested with GetQueryAsync and an optional relationship, like "long parentId? {get;set;}"?

BlaiseD commented 2 years ago

When there's a bug it's really up you to:

igorgy commented 4 months ago

I was able to reproduce the issue, but thing is it is not an 1:1 optional requirement issue, but optional 1:1 with the table self referencing, as in the original example Category has a FK on itself, it works fine when FK points to a different table, and workaround in my case as well was - switch to "GetAsync()".