mapperly icon indicating copy to clipboard operation
mapperly copied to clipboard

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

Open Tvde1 opened this issue 1 year ago • 7 comments

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.

Tvde1 avatar Sep 07 '23 09:09 Tvde1

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.

latonz avatar Sep 08 '23 08:09 latonz

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

TimothyMakkison avatar Sep 08 '23 13:09 TimothyMakkison

nvm might be a bit trickier than I thought, sorry

TimothyMakkison avatar Sep 08 '23 20:09 TimothyMakkison

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

Tvde1 avatar Sep 09 '23 16:09 Tvde1

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 😆

TimothyMakkison avatar Sep 09 '23 16:09 TimothyMakkison

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).

ranma42 avatar Nov 22 '23 22:11 ranma42

@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.

latonz avatar Nov 24 '23 18:11 latonz