Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using automapper to return DTOs forces navigation properties enumeration #863

Open
doubret opened this issue Dec 1, 2016 · 10 comments
Open
Labels

Comments

@doubret
Copy link

doubret commented Dec 1, 2016

When using a mapper like automapper and not specifying $select in the url, the underlying IQueryable is not projected and the navigation properties of the model are fetched.

Could the underliying IQueryable be projected all times ?
This way the navigation properties could left untouched when there is no $select and be loaded only when specified in $expand.

Assemblies affected

System.Web.OData 6.0.0

Reproduce steps

  • 2 tables in my database : Parent, Child. Child has a foreign key to Parent.

  • using EntityFramework database first.

  • 2 models Parent2 and Child2 acting as DTOs (used in the EdmModel).

  • Child and Parent properties in the model are navigation properties, not "real" properties.

  • a simple odata action projecting the DbSet before returning it :
    [EnableQuery]
    public IQueryable Get()
    {
    var q = context.Child.AsNoTracking().Select(x => new Child2 { Id = x.Id, Name = x.Name, ParentId = x.ParentId, Parent = new Parent2 { Id = x.Parent.Id, Name = x.Parent.Name } });
    return q;
    }

Expected result

calling http://endpoint/odata/Child should not cause a sql join when querying the db as there is no $expand.

Actual result

a sql join is added to the query as the EnableQuery attribute doesn't limit the query to the real properties only.

SELECT
[Extent1].[Id] AS [Id],
[Extent1].[Name] AS [Name],
[Extent1].[ParentId] AS [ParentId],
[Extent2].[Name] AS [Name1]
FROM [dbo].[Child] AS [Extent1]
LEFT OUTER JOIN [dbo].[Parent] AS [Extent2] ON [Extent1].[ParentId] = [Extent2].[Id]

Additional details

in ODataQueryOptions.cs line 367 :

        if (SelectExpand != null)
        {
            var tempResult = ApplySelectExpand(result, querySettings);
            if (tempResult != default(IQueryable))
            {
                result = tempResult;
            }
        }

if SelectExpand is null, all properties pass through (not only real properties).

@doubret
Copy link
Author

doubret commented Dec 7, 2016

Are you going to reply to people ?

@TehWardy
Copy link

Use automappers own IQueryable extensions ...
https://github.com/AutoMapper/AutoMapper/wiki/Queryable-Extensions

They are designed to work with entities like this but it does project the whole set in the db (at least that's my understanding).

@doubret
Copy link
Author

doubret commented Dec 12, 2016

Automapper queryable extensions won't slob the problem here. At least not in a transparent way.

Automapper will enhance the query to reflect the projection from entity to dto so that ef can materialize the dto directly from the results of the query.

Now when the $select is empty, no additional projection is added by the enablequery attribute to filter out navigation properties. The navigation properties are not serialized though but the are still queried.
I have the feeling that it's sort of wrong and that the navigation properties should be queried only if requested instead of being queried but not serialized.

It would solve the problem of intermediary projection quite elegantly and provide an end to end projection chain.

@TehWardy
Copy link

Are you using EF proxying and lazy loading?
if not using lazy loading sometimes ive noticed OData can be bit a greedy with its queries but from what you've said it sounds like there might be a bug in there somewhere but is this really that bad if it pulls a bit extra data from the db (compared to some more critical bugs of course)?

@doubret
Copy link
Author

doubret commented Dec 13, 2016 via email

@TehWardy
Copy link

Now i'm curious to see how I got lazy loading working in my stack!

@doubret
Copy link
Author

doubret commented Dec 13, 2016

I'm curious too ! ;-)

@jcpc91
Copy link

jcpc91 commented Mar 22, 2017

looks like there is no way to integrate with dto's and need to expose your model to the public. i think is bad

@biaol-odata biaol-odata added the P3 label May 31, 2017
@ovidiubuligan
Copy link

@doubret it is quite easy to use Automapper DTO's with OData .
You will have to configure the dtos in automapper to use explicit expansion as in here https://stackoverflow.com/questions/36284142/using-dtos-with-odata-web-api/ .
You will also have to also manually specify the expanded members for ProjectTo as an array of strings , these could easily be extracted using the recursive ProcessExpands in your OData Get function. The code for process expands here is a bit lower in the same stackoverflow question :
https://stackoverflow.com/questions/36284142/using-dtos-with-odata-web-api/48585259#48585259

Then for returning the IQueriable<DTO> use:

var ret = myEFQueryable.ProjectTo<DTO>(AutomapperODataConfig.Config, 
                new Dictionary<string, object>(),
                membersToExpand // obtained with ProcessExpands from the odata query string
              );

@szv
Copy link

szv commented Apr 26, 2021

Hi,

I also struggled with this problem.
I came along with this solution...

...
public DbContext DbContext { get; private set; }

public IMapper Mapper { get; private set; }

[HttpGet]
[EnableQuery]
public async Task<ActionResult<ICollection<OutputDTO>>> Get(ODataQueryOptions<OutputDTO> queryOptions) =>
      this.Ok(await this.DbContext
          .Set<Entity>()
          .GetAsync<OutputDTO, Entity>(this.Mapper, queryOptions, null));


[HttpGet("{id}")]
[EnableQuery]
public async Task<ActionResult<OutputDTO>> Get(Guid id, ODataQueryOptions<OutputDTO> queryOptions)
{
    TOutput result = (await this.DbContext
        .Set<Entity>()
        .Where(x => x.Id == id)
        .GetAsync<OutputDTO, Entity>(this.Mapper, queryOptions, null))
        .FirstOrDefault();

    if (result == null)
        return this.NotFound();

    return result;
}

Hope it helps 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants