VahidN / EFSecondLevelCache.Core

Entity Framework Core Second Level Caching Library
Apache License 2.0
326 stars 51 forks source link

Cache incorrectly used when include is changed #39

Closed LeedsCode closed 5 years ago

LeedsCode commented 5 years ago

Summary of the issue

If you call the same query but only change the include, the cached value is used, ignoring the different include.

Environment

The in-use version: 2.3.0
Operating system: Windows 10
IDE: Visual Studio Code

Example code/Steps to reproduce:

public class LineItem {
    public string Id { get; set; }
    public string OrderId { get; set; }
    public virtual Order Order { get; set; }
}
public class Test {
    public override IEnumerable<LineItem> Get(Expression<Func<LineItem, bool>> filter = null, params Expression<Func<LineItem, object>>[] include)
    {
        IQueryable<LineItem> query = _dbSet;
        query = query.Cacheable(CacheExpirationMode.Absolute, TimeSpan.FromMinutes(5));

        if (filter != null)
            query = query.Where(filter);

        if (include != null)
        {
            foreach (var includeProperty in include.ToList())
                query = query.Include(includeProperty);
        }
        return query.ToList();
    }
    public void Test() {
        var lineItem1 = Get(x => x.Id == "abc");
        var lineItem2 = Get(x => x.Id == "abc", x => x.Order);
        Console.WriteLine(lineItem2.Order.Id);
    }
}

Output:

Exception message: object not set to an instance of an object
lineItem2.Order is null, if you comment out the Cacheable line lineItem2.Order is not null, so it appears a cached value is being returned even through the include has changed between lineItem1 and lineItem2.
VahidN commented 5 years ago

Have you tried adding Cacheable() before the ToList() method?

LeedsCode commented 5 years ago

That fixed it perfectly, actually!

Before:

    public override IEnumerable<LineItem> Get(Expression<Func<LineItem, bool>> filter = null, params Expression<Func<LineItem, object>>[] include)
    {
        IQueryable<LineItem> query = _dbSet;
        query = query.Cacheable(CacheExpirationMode.Absolute, TimeSpan.FromMinutes(5));

        if (filter != null)
            query = query.Where(filter);

        if (include != null)
        {
            foreach (var includeProperty in include.ToList())
                query = query.Include(includeProperty);
        }
        return query.ToList();
    }

That didn't work. This worked as expected:

    public override IEnumerable<LineItem> Get(Expression<Func<LineItem, bool>> filter = null, params Expression<Func<LineItem, object>>[] include)
    {
        IQueryable<LineItem> query = _dbSet;

        if (filter != null)
            query = query.Where(filter);

        if (include != null)
        {
            foreach (var includeProperty in include.ToList())
                query = query.Include(includeProperty);
        }
        query = query.Cacheable(CacheExpirationMode.Absolute, TimeSpan.FromMinutes(5));
        return query.ToList();
    }

The issue can be closed, but I guess I'm not sure why the call need to be done after the rest of it. Without looking in to the code, I would expect that either of the above methods would work the same. Maybe I'll be able to look at it sometime, but is it easy to make it work either way? I would assume the Cacheable call must overwrite the ToList() method if there's a matching cached version. A fix would be to overwrite the ToList() method by default to check the status of some new property like CachedValue. I'm just kind of guessing how it works at this point, tho.

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related problems.