0

When I fetch a "Users" info for their profile page. I'm fetching multiple child entity collections from the "User" (parent) entity to either get info from it (ex. images) or I'm fetching them just to get a count (ex. Likes)

Problem - When fetching all the "Yogaband" children entities it's killing the time it takes to fetch (~12 secs) because, I assume, each "Yogaband" entity has many child entities and even the children have children, so there is a ton of data or the underlying query is complex (haven't looked) despite having roughly only ~15 "Yogaband" entities in the DB.

What I need - All I need is the count of "Yogabands" from the "User" and a few other counts of child collections but I'm not sure how to modify my specification pattern and generic repo to just fetch a count. The code now only fetches the entire entity (ex. AddInclude()) You can see in the automapper mapping code below I have YogabandCount, which is my attempt at keeping a numerical count of the "Yogabands" but that could end up getting disconnected from the actual count and probably isn't the best way to handle this problem. What I need is a better way to fetch the "Yogaband" count without getting the entire entity.

I'd like to have something like this below to the specification pattern

AddCount()

I'll start with my controller to fetch the "User"

[HttpGet("{username}", Name = "GetMember")]
public async Task<IActionResult> GetMember(string username)
{
    var spec = new MembersWithTypesSpecification(username);
    var user = await _unitOfWork.Repository<User>().GetEntityWithSpec(spec);
    if (user == null) return NotFound(new ApiResponse(404));
    var userToReturn = _mapper.Map<MemberForDetailDto>(user);
    return Ok(userToReturn);
}

Here is MembersWithTypesSpecification for creating everything I want

public class MembersWithTypesSpecification : BaseSpecification<User>
{
    public MembersWithTypesSpecification(string userName) 
        : base(x => x.UserName == userName) 
    {
        AddInclude(x => x.UserPhotos);
        AddInclude(x => x.PracticedStyles);
        AddInclude(x => x.PracticedPoses);
        AddInclude(x => x.InstructedStyles);
        AddInclude(x => x.InstructedPoses);
        AddInclude(x => x.InstructorPrograms);
        AddInclude(x => x.Yogabands);
        AddInclude(x => x.ReceivedReviews);
        AddInclude(x => x.Likers);     
    }
}

In the BaseSpecification file I have this below for AddInclude

public class BaseSpecification<T> : ISpecification<T>
{
    public BaseSpecification() {}
    public BaseSpecification(Expression<Func<T, bool>> criteria)
    {
        Criteria = criteria;
    }
    public Expression<Func<T, bool>> Criteria { get; }
    public List<Expression<Func<T, object>>> Includes { get; } = new List<Expression<Func<T, object>>>();
    public List<string> IncludeStrings { get; } = new List<string>();
    public Expression<Func<T, object>> OrderBy { get; private set; }
    public Expression<Func<T, object>> OrderByDescending { get; private set; }
    public Expression<Func<T, object>> GroupBy { get; private set; }
    public int Take { get; private set; }
    public int Skip { get; private set; }
    public bool IsPagingEnabled { get; private set; }
    protected void AddInclude(Expression<Func<T, object>> includeExpression)
    {
        Includes.Add(includeExpression);
    }
    protected void AddInclude(string includeString)
    {
        IncludeStrings.Add(includeString);
    }
    protected void AddOrderBy(Expression<Func<T, object>> orderByExpression)
    {
        OrderBy = orderByExpression;
    }
    protected void AddOrderByDescending(Expression<Func<T, object>> orderByDescExpression)
    {
        OrderByDescending = orderByDescExpression;
    }
    protected void AddGroupBy(Expression<Func<T, object>> groupByExpression)
    {
        GroupBy = groupByExpression;
    }
    protected void ApplyPaging(int skip, int take)
    {
        Skip = skip;
        Take = take;
        IsPagingEnabled = true;
    }
}

Here is GetEntityWithSpec() from my generic repo

public class GenericRepository<T> : IGenericRepository<T> where T : class
{
    private readonly DataContext _context;
    public GenericRepository(DataContext context)
    {
        _context = context;
    }
    public async Task<T> GetByIdAsync(int id)
    {
        return await _context.Set<T>().FindAsync(id);
    }
    public async Task<IReadOnlyList<T>> ListAllAsync()
    {
        return await _context.Set<T>().ToListAsync();
    }
    public async Task<T> GetEntityWithSpec(ISpecification<T> spec)
    {
        return await ApplySpecification(spec).FirstOrDefaultAsync();
    }
    public async Task<IReadOnlyList<T>> ListAsync(ISpecification<T> spec)
    {
        return await ApplySpecification(spec).ToListAsync();
    }
    public async Task<int> CountAsync(ISpecification<T> spec)
    {
        return await ApplySpecification(spec).CountAsync();
    }
    private IQueryable<T> ApplySpecification(ISpecification<T> spec)
    {
        return SpecificationEvaluator<T>.GetQuery(_context.Set<T>().AsQueryable(), spec);
    }
    public void Add(T entity)
    {
        _context.Set<T>().Add(entity);
    }
    public void Update(T entity)
    {
        _context.Set<T>().Attach(entity);
        _context.Entry(entity).State = EntityState.Modified;
    }
    public void Delete(T entity)
    {
        _context.Set<T>().Remove(entity);
    }
    public async Task<bool> SaveChangesAsync()
    {
        return await _context.SaveChangesAsync() > 0;
    }
}

And finally here is how I map the data with Automapper after it's been fetched.

CreateMap<User, MemberForDetailDto>()
            .ForMember(d => d.YearsPracticing, o => o.MapFrom(s => System.DateTime.Now.Year - s.YearStarted))
            .ForMember(d => d.Age, o => o.MapFrom(d => d.DateOfBirth.CalculateAge()))
            .ForMember(d => d.Gender, o => o.MapFrom(d => (int)d.Gender))
            .ForMember(d => d.Photos, opt => opt.MapFrom(src => src.UserPhotos.OrderByDescending(p => p.IsMain)))
            .ForMember(d => d.Yogabands, opt => opt.MapFrom(source => source.Yogabands.Where(p => p.IsActive).Count()))
            // .ForMember(d => d.Yogabands, opt => opt.MapFrom(source => source.YogabandsCount))
            // .ForMember(d => d.Likers, opt => opt.MapFrom(source => source.Likers.Count()))
            .ForMember(d => d.Likers, opt => opt.MapFrom(source => source.LikersCount))
            .ForMember(d => d.Reviews, opt => {
                opt.PreCondition(source => (source.IsInstructor == true));
                opt.MapFrom(source => (source.ReceivedReviews.Count()));
            })
            // .ForMember(d => d.Reviews, opt => opt.MapFrom(source => source.ReceivedReviews.Count()))
            .ForMember(d => d.ExperienceLevel, opt => opt.MapFrom(source => source.ExperienceLevel.GetEnumName()))
            .ForMember(d => d.PracticedStyles, opt => opt.MapFrom(source => source.PracticedStyles.Count())) 
            .ForMember(d => d.PracticedPoses, opt => opt.MapFrom(source => source.PracticedPoses.Count())) 
            .ForMember(d => d.InstructedStyles, opt => opt.MapFrom(source => source.InstructedStyles.Count())) 
            .ForMember(d => d.InstructedPoses, opt => opt.MapFrom(source => source.InstructedPoses.Count()))
            .ForMember(d => d.InstructorPrograms, opt => opt.MapFrom(source => source.InstructorPrograms.Where(p => p.IsActive == true)));
chuckd
  • 13,460
  • 29
  • 152
  • 331
  • To do that efficiently you need to apply automapper _before_ executing database query. Now you first fetch user entity and then map it to something else. At this point it's too late to obtain count of child entities without fetching them all. – Evk Nov 03 '21 at 09:33
  • https://docs.automapper.org/en/latest/Queryable-Extensions.html – Lucian Bargaoanu Nov 03 '21 at 13:58
  • Hi Evk. Can post an example as an answer? – chuckd Nov 03 '21 at 18:22
  • Can't post a proper answer because I kind of got lost in your abstractions. Your can check that link in comment above, the basic idea is simple - map `User` to `MemberForDetailDto` before making database query (before executing `FirstOrDefault` etc). But since you layered more abstractions on top of abstractions already provided by Entity Framework - simple things became hard. – Evk Nov 04 '21 at 09:06
  • Hi Evk. Ya I think you're right. I'm currently working on removing generic repo, unit of work and specification patterns from my DB. I think it will be much easier moving forward if I just access the context and work from there. If I ever want to go back I'll still have it there to use later on. – chuckd Nov 05 '21 at 01:49

2 Answers2

2

I assume that if you use Specification Pattern, what you do is trying to do things "DDD like".

First of all, you should re-think why do you use repository-pattern when your repository methods returns IQueryable. To use specifications with EF you dont need repositories at all - especially when you use such a implementation.

Second thing, what you trying to achieve is responsibility of repository. So you should move all Include / Pagination etc. to the specialized repository methods. Your specifications should contain predicates without additional logic (should contain only domain logic).

So create specialized repository like UserRepository then use it together with specification pattern eg.

class UserRepository : RepositoryBase {
    int CountUsers(UserSpecification spec){
        var query = ApplySpecification(spec);
        return query.Count();
    } 
}

So you'll be able to evaluate things on DB's side.

Also... you should abvoid using AutoMapper in such a ugly way. In case of that many custom mappings, it's easier & safer to map it manually. You can also consider to use projections instead of mapping AFTER fetch - check ProjectTo query extension.

Posio
  • 962
  • 4
  • 15
  • Hi Posio. Question - why am I using Automapper in an ugly way? Also, if I create a query just for the count, like your example, and now have to run multiple queries to fetch user data, wouldn't that be less efficient? – chuckd Nov 03 '21 at 17:09
  • "Second thing, what you trying to achieve is responsibility of repository. So you should move all Include / Pagination etc. to the specialized repository methods. Your specifications should contain predicates without additional logic (should contain only domain logic)." - do you have any project on Github that you can point me to where I can see what you are talking about??? – chuckd Nov 03 '21 at 17:15
  • 2
    1. Mapping shouldn't contain complicated logic - in case of errors, it's very hard to debug. Also, such complicated mappings will make projection impossible to evaluate. Read Automapper guidelines: https://jimmybogard.com/automapper-usage-guidelines/ 2. I think you should read some about specification pattern and it's purpose. Specification pattern is a "domain thing" which encapsulates portion of domain logic - in short words - you should'nt mix it with infrastructure (database specific) things. You can find A LOT about such pattern in net (eg. nice blog V.Khorikov) and books DDD by Evans. – Posio Nov 03 '21 at 20:15
  • ... about querying multiple counts. IMO you should create method in repository which will return dto with all things you need (you'll be able to apply spec pattern as well) Also manually selecting data to DTO will help you getting rid of automapper complicated mappings. – Posio Nov 03 '21 at 20:26
  • Hi Posio. Can you please explain why the automapper mappings are complicated? They seem pretty simple to me. – chuckd Nov 04 '21 at 01:33
  • @user1186050 Because in this particular case they look like a complicated way to write `Select((User s) => new MemberForDetailDto { ... }`. AM shines when most of the members are mapped automatically, i.e. the count of `ForMember` and similar configuration calls are much less than the total count of the destination members. Also, without using AM `ProjectTo` on top of the original `IQueryable` all the count "optimizations" won't have any effect, since `Map` executes on already executed and materialized in memory LINQ to Entities query. – Ivan Stoev Nov 04 '21 at 06:53
  • Please check out https://github.com/ardalis/CleanArchitecture and https://github.com/ardalis/Specification . Ardalis implemented specification in way similar to your needs and current implementation. About AutoMapper mappings, please check link i've already posted https://jimmybogard.com/automapper-usage-guidelines/ – Posio Nov 08 '21 at 17:30
1

I think it is an interesting problem, one whose origin is an intellectual hole i have found myself looking up from countless of times myself :) It's the strive to make generic solutions! Sometimes we get punished for that with Linq because when execution contains functions it often resolves the projected data before calling the given function. What i belive you need to do is to lower your generic ambition and inline your functions to allow EF/ Linq to work with the full queries and as you're reading also make sure to make it non-tracking.

Try benchmarking this one below against your generic-ness, for performance i propose you sacrifice generic structure for specific. So we're building our things into the repository when it need to be quick. I make a few assumtions about data for the example, please let me know if there are usefully corrections for clairity, as i do not see the dbcontext in your code.

public class UserDbContext : DbContext {
  DbSet<AppUser> Users { get; set; }
  DbSet<Yogabands> Yogabands { get; set; }

  protected override void OnModelCreating(ModelBuilder modelBuilder)
  {
    modelBuilder.Entity<AppUser>().HasKey(k => k.AppUserId);
    modelBuilder.Entity<Yogabands>().HasKey(k => k.YogabandsId);
    modelBuilder.Entity<AppUser>().HasMany(m => m.Yogabands).WithMany(n => n.Users);
  }

  public int CountYogaBands(Guid appUserId)
  {
    return Users.AsNoTracking()
            .Include(u => u.Yogabands).AsNoTracking()
            .Where(x => x.AppUserId == appUserId)
            .Select(y => y.Yogabands.Count())
            .FirstOrDefault();
  }

So since what you're asking is how do I do this fast, my suggestion is to push the query to the dbcontext and make it specific to avoid the functional feature of when using functions they get resolved after, will it need N accessor methods instead of 1, yes, but consider if that couldn't be ok, after all you could be returning multiple counts into a new object, like

...Select(y => new {
     Yogabands = y.Yogabands.Count(),
     Likers = y.Likers.Count()
   })
   .FirstOrDefault();

Though you'd propably prefer a defined type to a generic to more easily be able to work with the counts result.

My personal conclusion at least, and I stand to be corrected obviously, is that EF Core is not yet mature enough to incorporate dynamic functions into the Linq resolving the ultimate query, nor have I ever seen a version of EF which is fully able, you have to 'denormalize' and cannot fully pursue the DRY principle.

Still EF Core does a banger of a job when you give it the entire linq expression up front and we should favor that for performance IMHO.

T. Nielsen
  • 835
  • 5
  • 18
  • Thanks T. Nielson for the answer. As of last night, I've re-injected my context into my controller and now I'm just running the query off the context and all is good(fast) again. I still have the generic repo and unit of work and specification pattern still there if I need to use it, but it looks like I might just start using the context more and more as I look through all my other controllers. If nobody else chimes in with any answer I'll give you the points. Also let me know if there's something else (code) I can post for you? – chuckd Nov 05 '21 at 21:03
  • I just want to add that in the proposed implementation of `CountYogaBands` neither `Include` nor `AsNoTracking` are necessary as non of the objects are loaded anyway (the query will be evaluated on the database). Assuming that `appUserId` is unique an easier way to write it would be `Users. .Where(x => x.AppUserId == appUserId).SelectMany(y => y.Yogabands).Count();` – Nannanas Nov 06 '21 at 17:27
  • EF Core has nothing to do with OP issue, and your conclusion about it is generally wrong. It has its drawbacks, but the main problem is with people which hide EF Core behind (anti) "patterns", thus losing EFC querying and/or tracking services. – Ivan Stoev Nov 06 '21 at 17:54
  • @Nannanas Your suggestion will work if there is only one count, so the proposition will work with many counts which is why I suggested like that. AsNoTracking then makes sure your EF doesn't track changes to entities, then of cause if the count can be pushed to the server and only count returned it will be even better, I trust EF does this for me here but haven't verified, worth doing though the important is to give it the entire query, which solved the problem for the member – T. Nielsen Nov 09 '21 at 10:33
  • @IvanStoev I do not understand completely. I haven't mentioned object oriented programming myself, nor did the member with the problem, the problem is that this pretty generalized implementation performed poorly. which hide "EF Core behind (anti) patterns" would you explain? How is the conclusion wrong, which to sum up is: Give EF the entire query, don't use function delegate and drop the tracking when you do not need it – T. Nielsen Nov 09 '21 at 10:41
  • I meant the part starting with *"My personal conclusion at least..."* and talking about EF Core maturity or ability to produce queries. – Ivan Stoev Nov 09 '21 at 11:17
  • @IvanStoev You think it is wrong to conclude that EF cannot yet inject function delegates into query in an optimized way instead of first resolving the entire dataset and then running the function? Well i hope they will be able to eventually, because it is an easy thing to miss and easy to use and get to somewhere else in performance that anybody really would want. Is it wrong that i think that they will eventually fix it (Become mature) or more interestingly did you find that it does not delay execution of query? See first comment, member was able to get performance after changes made. – T. Nielsen Nov 09 '21 at 11:53
  • All you are talking about is not EF fault. EF *can* do all that efficiently. The patterns/code used originally and hiding EF query capabilities are causing their problem. EF perfectly can handle count and other projections when done property (using EF queryable and not enumerable after taking the control from EF and giving to the LINQ to Objects, i.e. in memory after materializing the EF query). What about "function delegates", I don't know what do you mean by that, may be you know better than me and EF a way to inject C# **code** inside SQL and make it run server side. – Ivan Stoev Nov 09 '21 at 12:06
  • @IvanStoev i found an article with somebody writing about functional delegate problem here: https://coderethinked.com/don-t-use-func-delegate-on-the-ef-entities/ I understand now, yes you can absolutely make data interaction efficient with EF. Just well it is not very well documentated that if you use the delegate it will work like that, there should be huge big warning on the Func overload imo. – T. Nielsen Nov 09 '21 at 12:14
  • If you have in mind composing LINQ query expression tree from reusable expression parts, that's another story. Still it's not EF fault, but C# not providing a syntax for that. Otherwise, there are many libraries which allow lambda injection / expression composing, but not naturally as it was supported by the language. AutoMapper projections are one of them. Anyway, to end up this discusion, I would say the same as in the beginning - EF has its limitations/drawbacks/bugs etc., but it is not always guilty for everything, and here is one of the case it is not. – Ivan Stoev Nov 09 '21 at 12:15
  • What about `Func<...>` vs `Expression>` overloads, it was LINQ decision to make them distinguishable only by the type, since `IQueryable` inherits `IEnurable` (second arbitrary decision) and `Queryable` or `Enumerable` extension method with the same name is selected *by C# compiler* method overload rules. It can't give you warnings for pretty valid C# code. – Ivan Stoev Nov 09 '21 at 12:22
  • Agreed and that decision cause for it to be really easy to make valid code which performs poorly, so even if for instance EF core is the best version yet of EF imo, it is still too easy to write something which isn't wrong but doesn't perform well. Like difference run to shop, get 2 bags of carrots from shelve as opposed to run to shop get shelve, run back get 2 carrots from shelve, throw away the rest. the func sort of gets the shelves which is because it doesn't get resolved as part of the expression. It is possible you can make it do so, but i haven't come across the good way yet. – T. Nielsen Nov 09 '21 at 12:44