riok / mapperly

A .NET source generator for generating object mappings. No runtime reflection.
https://mapperly.riok.app
Apache License 2.0
2.85k stars 145 forks source link

Allow creating an Expression<Func<>> instead of a IQueryable Projection #733

Open Tvde1 opened 1 year ago

Tvde1 commented 1 year ago

It is possible to get a IQueryable<B> ProjectToB(IQueryable<A> query) which can be used as

context.ASet.ProjectToB().ToListAsync();

Is your feature request related to a problem? Please describe.

We have a generic class like follows:

class BaseRepository<TEntity, TModel>
{
    public BaseRepository(DbSet<TEntity> dbSet, Func<IQueryable<TEntity>, IQueryable<TModel>> projectToModel) { ... }

    public async Task<List<TModel>> GetAll()
    {
        return await _projectToModel(
                _dbSet.Where(x => x.SomeCondition)
            )
            .Where(x => x.IsSomething)
            .ToListAsync();
    }
}

class UserRepository : BaseRepository<UserEntity, UserModel>
{
    public UserRepository(DbContext c) : base(c.Users, UserMapper.ProjectToModel) {}
}

static partial class UserMapper
{
    public static partial IQueryable<UserModel> ProjectToModel(this IQueryable<UserEntity> q);
}

Describe the solution you'd like I would prefer being able to create an Expression<Func<A, B>> and give it to my base repository.

class BaseRepository<TEntity, TModel>
{
    public BaseRepository(DbSet<TEntity> dbSet, Expression<Func<TEntity, TModel>> projectToModel) { ... }

    public async Task<List<TModel>> GetAll()
    {
        return await _dbSet
            .Where(x => x.SomeCondition)
            .Select(_projectToModel)
            .Where(x => x.IsSomething)
            .ToListAsync();
    }
}

class UserRepository : BaseRepository<UserEntity, UserModel>
{
    public UserRepository(DbContext c) : base(c.Users, UserMapper.CreateToModelProjection()) {}
}

static partial class UserMapper
{
    public static partial Expression<Func<UserModel, UserEntity>> CreateToModelProjection()
}

This will make the query cleaner and easier to read.

Describe alternatives you've considered The aforementioned code works, but I'd prefer the new variant.

Additional context I'm happy to hear your opinions on this matter.

latonz commented 1 year ago

I understand your use case and I think this could probably be helpful in other scenarios. However, as I think this is an edge case, it is not a priority for us. But I would be happy to accept a PR that implements this.

TimothyMakkison commented 1 year ago

~I'd be happy to do this, should be easy to add~ 🤞

TimothyMakkison commented 1 year ago

nvm might be a bit trickier than I thought, sorry

Tvde1 commented 1 year ago

The Expression must already be created in order to use IQueryable<>, so I figured it might not be too difficult. If I find some free time this month I might dive into the code

TimothyMakkison commented 1 year ago

Yeah, I think the logic is identical to IQueryable. I'd naively thought it would be a quick, simple check for expressions and then write some tests. I hadn't registered that I'd need to edit the method extractor, maybe add a new object method builder and maybe change an interface.

It's possible to add, but not the quick, 15 minute change I wanted 😆

ranma42 commented 11 months ago

I started experimenting on this. I stumbled on some assumptions that currently seem to be baked in, but so far nothing major; in fact with some hacks I managed to get it somewhat working:

//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
    private partial global::System.Linq.IQueryable<global::B> Map(global::System.Linq.IQueryable<global::A> source)
    {
        return System.Linq.Queryable.Select(source, MapToExpression(42));
    }

    private global::System.Linq.Expressions.Expression<global::System.Func<global::A, global::B>> MapToExpression(int source)
    {
#nullable disable
        return x => new global::B()
        {
            Parent = x.Parent != null ? new global::B() : default,
        };
#nullable enable
    }
}

I'll try experimenting some more, but I would like to know if this direction looks somewhat reasonable. I am aiming for something like:

//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
    private partial global::System.Linq.IQueryable<global::B> Map(global::System.Linq.IQueryable<global::A> source)
    {
        return System.Linq.Queryable.Select(source, MapToExpression());
    }

    private global::System.Linq.Expressions.Expression<global::System.Func<global::A, global::B>> MapToExpression()
    {
#nullable disable
        return x => new global::B()
        {
            Parent = x.Parent != null ? new global::B() : default,
        };
#nullable enable
    }
}

(aka no hacked source). Later on I would like to memoize the expression, but that might not be really relevant (it would be a minor optimization for the allocations, definitely optional).

latonz commented 11 months ago

@ranma42 looks good so far 😊 Thank you for your efforts. IMO the externalized expression method only needs to be generated if there is a partial method definition for it:

[Mapper]
public partial class Mapper
{
    public partial IQueryable<B> Map(this IQueryable<A> source);
}

should generate

[Mapper]
public partial class Mapper
{
    public partial global::System.Linq.IQueryable<global::B> Map(this global::System.Linq.IQueryable<global::A> source)
    {
#nullable disable
          return System.Linq.Queryable.Select(source, x => ...);
#nullable enable
    }
}

but

[Mapper]
public partial class Mapper
{
    public partial Expression<Func<A, B>> Mapper();
}

should generate

[Mapper]
public partial class Mapper
{
    public partial Expression<Func<A, B>> Mapper()
    {
#nullable disable
          return x => ...;
#nullable enable
    }
}

If both are declared, the queryable method could call the expression builder, but IMO this would be optional for a first version and could be an optimisation later on.