AutoMapper / AutoMapper.Extensions.OData

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

Unnecessary Left Joins Being Generated In Underlying Sql - Possible Solution Provided In Second Comment #161

Closed CShelton11 closed 1 year ago

CShelton11 commented 1 year ago

Discussed in https://github.com/AutoMapper/AutoMapper.Extensions.OData/discussions/160

Originally posted by **CShelton11** November 8, 2022 Sorry for the lengthy post, but trying to be detailed. I'm experiencing an issue where unnecessary left joins are being generated to child tables in underlying sql being generated. My setup is outlined below: Issue By Example: ******************************************************** If I were to invoke the PolicyCase endpoint on my api with the following url: https://site.com/odata/policyCase?$top=1 I get a response back from the server like the following: ``` { "PolicyCaseId": 1, "TicketDetails": [ { "TicketDetailId": 1, etc... }, { "TicketDetailId": 2, etc... } ] } ``` I didn't request the TicketDetails information in my query, but it is being provided anyway. I can't see that I have configured anything incorrectly, and I can't believe that this isn't something that has been considered already. Entity Classes: ******************************************************** ``` public class PolicyCase { public Int64 PolicyCaseId { get; set; } public virtual ICollection TicketDetails { get; set; } } public class TicketDetail { public Int64 TicketDetailId { get; set; } public virtual PolicyCase PolicyCase { get; set; } } ``` Dto Classes: ******************************************************** ``` public class PolicyCaseDto { public Int64 PolicyCaseId { get; set; } public ICollection TicketDetails { get; set; } } public class TicketDetailDto { public Int64 TicketDetailId { get; set; } public PolicyCaseDto PolicyCase { get; set; } } ``` Profiles: ******************************************************** ``` public class PolicyCaseProfile : Profile { public PolicyCaseProfile() { CreateMap(MemberList.Destination).ReverseMap(); } } public class TicketDetailProfile : Profile { public TicketDetailProfile() { CreateMap(MemberList.Destination).ReverseMap(); } } ``` Mappers: ******************************************************** `_mapper = new MapperConfiguration(a => a.AddMaps(typeof(PolicyCaseProfile))).CreateMapper();` Extension Method: ******************************************************** ``` public static IQueryable ToDtoQueryable(this IQueryable queryable, ODataQueryOptions options) { return queryable.GetQuery(_mapper, options); } ``` Note - If I were to do the following: queryable.GetQuery(_mapper, options).ToList(); It returns all of the TicketDetails for the 1 record I filtered for. I'm guessing it has something to do with the projection including a ToList in there somewhere, but I have to believe that this has been considered already (Like an option telling the projection to issue a Select statement instead of a ToList???). How do I avoid this issue???? Any help is greatly appreciated...
CShelton11 commented 1 year ago

Alright, finally got through this today. I'm not 100% sure that it works as expected in all scenarios, but I wanted to be able to get this out there so you could have a look. I'll continue testing on my end to see how I should continue.


To begin with, The nature of odata makes it such that we can't just fire up a new profile with custom mappings every time a query is executed (It's too expensive). Understanding that, I started by creating a caching mechanism to hold the generated Profiles (Each specific to the expansions and selections provided in the queries).



using AutoMapper;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace AutoMapper.Caches
{
    public static class AutoMapperODataCache
    {
        private static ConcurrentDictionary<String, IMapper> _mappers;

        static AutoMapperODataCache()
        {
            _mappers = new ConcurrentDictionary<String, IMapper>();
        }

        public static void Add(String type, String query, IMapper mapper)
        {
            _mappers.TryAdd(type.ToLower() + "-" + query.ToLower(), mapper);
        }

        public static IMapper Get(String type, String query)
        {
            var key = type.ToLower() + "-" + query.ToLower();
            var keys = _mappers.Keys;
            var exists = keys.Where(a => a.ToLower() == key).Count() > 0;
            return exists == true ? _mappers[key] : null;
        }
    }
}

I then created 2 new extension methods.

The first is call AddODataMaps. This method is to be used similarly to the AddMaps method that ships with AutoMapper. Example usage will be shown later) This method will will accept 3 arguments; 1 - typeOfProfileToAutoGenerate, This is the type of Profile that will need to be excluded from automatic Profile registration. The reason this is necessary is because inclusion of that profile will cause a Map to get included in the MapperConfiguration for the types that we will be dynamically creating a Profile for. 2 - options, The odata query options that were provided through the url 3 - cache, Indicated if the system should maintain a cache of the generated Mappers (Note, I didn't actually implement this, these methods are automatically caching in this code)

The second is a new AddProfile method.
This method is used to generate the IMappingExpression given the odata query options that were provided. This method accepts 1 argument. 1 - options, The odata query options



using AutoMapper;
using AutoMapper.AspNet.OData;
using AutoMapper.Caches;
using AutoMapper.Configuration;
using Microsoft.AspNetCore.OData.Query;
using Microsoft.OData.Edm;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Text;
using System.Threading.Tasks;

namespace AutoMapper.Extensions
{
    public static class AutoMapperODataExtensions
    {
        public static void AddODataMaps<TEntity, TDto>(this IMapperConfigurationExpression configuration, Type typeOfProfileToAutoGenerate, ODataQueryOptions options, Boolean cache)
        {
            var type = options.Context.ElementType.ToString();
            var query = options.SelectExpand == null ? "unexpandedandunselected" : ("$expand=" + (options.SelectExpand.RawExpand ?? "").ToString()) + ("$select=" + (options.SelectExpand.RawSelect ?? "").ToString());
            if (AutoMapperODataCache.Get(type, query) == null)
            { 
                var types = AppDomain.CurrentDomain.GetAssemblies().SelectMany(s => s.GetTypes()).Where(p => typeof(Profile).IsAssignableFrom(p)).ToList();
                var profiles = types.Where(a => a != typeOfProfileToAutoGenerate && a.FullName.ToLower() != "automapper.profile").Select(a => (Profile)Activator.CreateInstance(a)).ToArray();
                configuration.AddProfiles(profiles);
                configuration.AddProfile<TEntity, TDto>(options);
                var mapper = new MapperConfiguration(a => { a.AddProfiles(profiles); a.AddProfile<TEntity, TDto>(options); }).CreateMapper();
                AutoMapperODataCache.Add(type, query, mapper);
            }
        }

        public static IMappingExpression<T, J> AddProfile<T, J>(this IMapperConfigurationExpression configuration, ODataQueryOptions options)
        {
            var expression = configuration.CreateMap<T, J>(MemberList.Destination);
            var includes = options.SelectExpand.GetIncludes().ToList();
            var navigations = ((EdmStructuredType)options.Context.ElementType).NavigationProperties().Select(a => a.Name).ToList();
            var properties = typeof(J).GetProperties().Select(a => a.Name).ToList();

            for (var i = 0; i < includes.Count; i++)
            {
                var include = includes[i].Split((".").ToCharArray())[0].ToLower();
                var property = properties.FirstOrDefault(a => a.ToLower() == include);
                expression.ForMember(property, a => a.MapAtRuntime());
            }

            for (var i = 0; i < navigations.Count; i++)
            {
                var included = includes.FirstOrDefault(a => a.Split((".").ToCharArray())[0].ToLower() == navigations[i].ToLower()) != null;
                if (included == false)
                {
                    var property = properties.FirstOrDefault(a => a.ToLower() == navigations[i].ToLower());
                    expression.ForMember(property, a => a.Ignore());
                }
            }

            return expression;
        }
    }
}

Lastly I create a new class called AutoMapperODataConfiguration that extends MapperConfiguration. Its basically a wrapper for the MapperConfiguration that provides functionality in the constructor to extract needed information fro the odata query options. It also hides the base CreateMapper method so that it the map is returned from cache instead of beign created at this juncture.



using AutoMapper;
using AutoMapper.AspNet.OData;
using AutoMapper.Caches;
using AutoMapper.Extensions;
using Microsoft.AspNetCore.OData.Query;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Text;
using System.Threading.Tasks;

namespace Automapper.Configs
{
    public class AutoMapperODataConfiguration : MapperConfiguration
    {
        private String _type { get; set; }
        private String _query { get; set; }

        public AutoMapperODataConfiguration(Expression<Action<IMapperConfigurationExpression>> expression): base(expression.Compile()) 
        {
            Configure(expression);
        }

        private void Configure(Expression<Action<IMapperConfigurationExpression>> expression)
        {
            var parameters = ((MethodCallExpression)expression.Body).Arguments.Select(e => new { Type = e.GetType(), Expression = e }).ToArray();
            var parameter = parameters.FirstOrDefault(a => a.Type.Name.ToLower().Contains("fieldexpression"));
            var target = (MemberExpression)parameter.Expression;
            var method = (MethodCallExpression)expression.Body;
            var arguments = method.Arguments.ToList();
            var argument = arguments.FirstOrDefault(a => a.Type == typeof(ODataQueryOptions));
            var value = ((ConstantExpression)(((MemberExpression)arguments[2]).Expression)).Value;
            var options = (ODataQueryOptions)((System.Reflection.FieldInfo)(((MemberExpression)arguments[2]).Member)).GetValue(value);
            _type = options.Context.ElementType.ToString();
            _query = options.SelectExpand == null ? "unexpandedandunselected" : ("$expand=" + (options.SelectExpand.RawExpand ?? "").ToString()) + ("$select=" + (options.SelectExpand.RawSelect ?? "").ToString());
        }

        public new IMapper CreateMapper()
        {
            if (AutoMapperODataCache.Get(_type, _query) != null)
            {
                return AutoMapperODataCache.Get(_type, _query);
            }
            else
            {

                return null;
            }
        }
    }
}

To utilize this, you simply need to create the mapper like follows:

public static IQueryable<PolicyCaseDto> ToDtoQueryable(this IQueryable<PolicyCase> queryable, ODataQueryOptions<PolicyCaseDto> options)
{
    var mapper = new AutoMapperODataConfiguration(a => a.AddODataMaps<PolicyCase, PolicyCaseDto>(typeof(PolicyCaseProfile), options, true)).CreateMapper();
    var query = queryable.AsNoTracking().GetQuery(mapper, options);
    var sql = query.ToQueryString();
    return query;
}

Results 1 - Url query "https://site/odata/policyCases?$top=1" will result in a query without the left join in sql server. Results 2 - Url query "https://site/odata/policyCases?$top=1&$select=policyCaseId&$expand=ticketDetails" will result in a query with the proper left join in sql server.

There's still a lot to do with this, but I wanted to share b/c something like the above would be extremely helpful in the AutoMapper odata assembly. The one thing that I'd like to do is add a new type of profile that allows the Mapping to be exposed. We could then use this to retreive the Mapping information from the Profile that was excluded and automatically generated. This would let us generate the new profile using the same information that was in the original profile and thus enable the same ForMembers/Complex Mappings/Property Maps to be utilized in the automatically generated Profile.

CShelton11 commented 1 year ago

Just a note - this only does first level inclusions for now. Working through nested expansions/selects... Will hopefully have a solution for this soon

BlaiseD commented 1 year ago

Do the problems persist with explicit expansions in the configuration?

CShelton11 commented 1 year ago

That works like a charm! Thanks for the quick response...