AutoMapper / AutoMapper.Extensions.OData

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

Many To Many Expand Error on GetAsync() #57

Closed adixon501 closed 4 years ago

adixon501 commented 4 years ago

Info

The app type is a Hosted Blazor web-assembly. And below are the versions of the nuget packages I am using. There is an error that occurs when trying to expand a navigation property that is a many-to-many relationship. The classes are mapped to DTO classes that flattens the middle relationship class.

To run this repo, you will need the free version of SQL Server or better

Set the EfCoreAutomapperOdata.Server project as the startup project and navigate to the Courses page (https://localhost:44374/courses) and click on either course. This will throw the following error:

System.InvalidOperationException: No generic method 'Include' on type 'Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions' is compatible with the supplied type arguments and arguments. No type arguments should be provided if the method is non-generic. at System.Linq.Expressions.Expression.FindMethod(Type type, String methodName, Type[] typeArgs, Expression[] args, BindingFlags flags)...

Models

See here - Entity Models and here - Dto Models for class definition

Automapper Config
    public class AutomapperConfig : Profile
    {
        public AutomapperConfig()
        {
            CreateMap<Instructor, InstructorDto>();
            CreateMap<InstructorDto, Instructor>();

            CreateMap<Course, CourseDto>()
                .ForMember(dto => dto.Students, opt => {
                    opt.MapFrom(_ => _.Students.Select(y => y.Student));
                });
            CreateMap<CourseDto, Course>()
                .ForMember(ent => ent.Students, ex => ex
                    .MapFrom(x => x.Students.Select(y => new CourseStudent {
                        CourseId = x.Id,
                        StudentId = y.Id
                    })));

            CreateMap<Student, StudentDto>()
                .ForMember(dto => dto.Courses, opt => {
                    opt.MapFrom(x => x.Courses.Select(y => y.Course));
                })
                .ForMember(dto => dto.Friends, opt => {
                    opt.MapFrom(x => x.Friends.Select(y => y.Friend));
                });
            CreateMap<StudentDto, Student>()
                .ForMember(ent => ent.Courses, ex => ex
                    .MapFrom(x => x.Courses.Select(y => new CourseStudent
                    {
                        StudentId = x.Id,
                        CourseId = y.Id
                    })));
        }
    }
Startup
    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

        public void ConfigureServices(IServiceCollection services)
        {
            // ------ Some code removed for brevity ------

            services.AddOData();
            services.AddAutoMapper(cfg => { cfg.AddExpressionMapping(); },typeof(AutomapperConfig));

            // ------ Some code removed for brevity ------
        }

        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            // ------ Some code removed for brevity ------

            app.UseHttpsRedirection();
            app.UseBlazorFrameworkFiles();
            app.UseStaticFiles();

            app.UseRouting();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapRazorPages();
                endpoints.MapControllers();
                endpoints.EnableDependencyInjection();
                endpoints.Select().Filter().OrderBy().Count().Expand().MaxTop(1000);
                endpoints.MapODataRoute("odata", "odata", GetEdmModel());
                endpoints.MapFallbackToFile("index.html");
            });
        }

        private IEdmModel GetEdmModel()
        {
            var builder = new ODataConventionModelBuilder();
            builder.EntitySet<CourseDto>("Courses");
            builder.EntitySet<InstructorDto>("Instructors");
            builder.EntitySet<StudentDto>("Students");

            return builder.GetEdmModel();
        }
    }
Course Controller
    public class CourseController : ODataController
    {
        protected readonly BlazorContext _context;
        protected readonly IMapper _mapper;

        public CourseController(BlazorContext context, IMapper mapper)
        {
            _context = context;
            _mapper = mapper;
        }

        [HttpGet]
        [ODataRoute("Courses")]
        public async Task<IActionResult> Get(ODataQueryOptions<CourseDto> options)
        {
            return Ok(await _context.Course.GetAsync(_mapper, options));
        }

        [HttpGet]
        [ODataRoute("Courses({id})")]
        public async Task<IActionResult> Get([FromODataUri] int id, ODataQueryOptions<CourseDto> options)
        {
            return Ok((await _context.Course.GetAsync(_mapper, options)).Where(s => s.Id == id).ToList());
        }
    }
Sample odata api query that fails

/odata/Courses?$expand=Students

Repro

I have built demo Blazor WASM app for this issue to reproduce

Repository

BlaiseD commented 4 years ago

You are correct:

            //Works
            Expression<Func<CourseDto, ICollection<StudentDto>>> selection = s => s.Students;
            Expression<Func<Course, ICollection<CourseStudent>>> selectionMapped = _mapper.MapExpressionAsInclude<Expression<Func<Course, ICollection<CourseStudent>>>>(selection);

            //Fails
            //Expression<Func<CourseDto, object>> selection = s => s.Students;
            //Expression<Func<Course, object>> selectionMapped = _mapper.MapExpressionAsInclude<Expression<Func<Course, object>>>(selection);

            List<Course> students = _context.Course.Include(selectionMapped).ToList();

Looks like the conversion to object doesn't work for EF. We should leave this open.

GetQueryAsync() should work in the meantime.

adixon501 commented 4 years ago

@BlaiseD I have noticed that GetQueryAsync() works in this case. But in my actual project (configured the same as this, just with a lot more tables), when I use GetQueryAsync() it sends a massive query to the database (800 lines and over 160,000 characters). It eventually times out even though the odata string is simply /odata/Customers(1). The query is trying to expand every navigational property in the database. This is why I was forced to use GetAsync()

BlaiseD commented 4 years ago

Neither function supports the entity key query. How about $filter?

adixon501 commented 4 years ago

@BlaiseD $filter is for applying a condition for the results, whereas $expand is for including child properties which is what I am needing to accomplish

adixon501 commented 4 years ago

Since I am sure this falls under the same issue, I have encountered the same bug when using the any odata query operator

/odata/Courses?$filter=Students/any(c:c/Id eq 3)

System.InvalidOperationException: No generic method 'Any' on type 'System.Linq.Enumerable' is compatible with the supplied type arguments and arguments. No type arguments should be provided if the method is non-generic.
BlaiseD commented 4 years ago

There are several tests in the repo showing filtering and expansion.

You might want to pull down the code, isolate the problem and create a PR where there's an issue.

BlaiseD commented 4 years ago

The "conversion when mapping includes" problem has been addressed in the expression mapping repo.

The problem here is not what I thought it was. The following code (which is what the library is doing) works fine (once you get the configuration right).

            ICollection<Expression<Func<IQueryable<CourseDto>, IIncludableQueryable<CourseDto, object>>>> includeProperties = new List<Expression<Func<IQueryable<CourseDto>, IIncludableQueryable<CourseDto, object>>>>
            {
                q => q.Include(s => s.Students)
            };

            ICollection<Expression<Func<IQueryable<Course>, IIncludableQueryable<Course, object>>>> includePropertiesMapped = _mapper.MapIncludesList<Expression<Func<IQueryable<Course>, IIncludableQueryable<Course, object>>>>(includeProperties).ToList();

            IQueryable<Course> queryable = _context.Course;
            queryable = includePropertiesMapped.Select(i => i.Compile()).Aggregate(queryable, (q, next) => q = next(q));
            List<Course> students = queryable.ToList();
adixon501 commented 4 years ago

Is this in AutoMapper.Extensions.ExpressionMapping 4.0.1 or 4.0.2?

Edit: Never mind, I see it was PR from 402Preview03

adixon501 commented 4 years ago

@BlaiseD , will AutoMapper.Extensions.OData be updated to use 4.0.2 when its released?

BlaiseD commented 4 years ago

Yes we're using the latest from AutoMapper.Extensions.ExpressionMapping.

adixon501 commented 4 years ago

@BlaiseD I updated (on my local branch) the linked repro to use the Preview packages. And using GetAsync(), still throws the aforementioned error. Do I need to open a new issue on the expression mapping repo?

BlaiseD commented 4 years ago

I mentioned above that your error was not related to the includes mapping fix.

Parts of the configuration were missing/incorrect:

            //CreateMap<Course, CourseDto>()
            //    .ForMember(dto => dto.Students, opt => {
            //        opt.MapFrom(src => src.Students.Select(y => y.Student));
            //    });

            CreateMap<Course, CourseDto>()
                .ForMember(dto => dto.Students, opt => {
                    opt.MapFrom(src => src.Students);
                });

            //CreateMap<CourseDto, Course>()
            //    .ForMember(ent => ent.Students, ex => ex
            //        .MapFrom(x => x.Students.Select(y => new CourseStudent {
            //            CourseId = x.Id,
            //            StudentId = y.Id
            //        })));
            CreateMap<CourseDto, Course>()
                .ForMember(ent => ent.Students, ex => ex
                    .MapFrom(x => x.Students));

            CreateMap<CourseStudent, StudentDto>()
                .ForMember(dto => dto.Id, opt => {
                    opt.MapFrom(x => x.StudentId);
                })
                .ForMember(dto => dto.Name, opt => {
                    opt.MapFrom(x => x.Student.Name);
                });