VahidN / EFSecondLevelCache.Core

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

Invalid result from Cacheable when using FirstOrDefaultAsync #38

Closed MoienTajik closed 5 years ago

MoienTajik commented 5 years ago

Summary of the issue

When an argument passes to the FirstOrDefaultAsync method ( and this issue may exist for other methods like ToListAsync, SingleAsync, ... ), the Cacheable caches the results correctly, but when another argument with different value passes to it, it returns the previous results again.

Seems like the Cacheable method only differs and watch for different argument values just in the Where clause, not the FirstOrDefaultAsync, SingleOrDefaultAsync and other EntityFramework.Core methods, because different argument values work correctly by using Where method.

I know that the Cacheable is written on top of IQueryable and it caches the result based on the Query, but with that, we're unable to use the Microsoft.EntityFramework.Core queryable extensions anymore.

Environment

The in-use version: 
EFSecondLevelCache.Core v1.8.1
CacheManager.Microsoft.Extensions.Caching.Memory v1.2.0

Example code/Steps to reproduce:

// Cache userA correctly here
var userA = await context.Users
    .Cacheable()
    .FirstOrDefaultAsync(user => user.Username == "ABC", cancellationToken);

// In this query Cacheable returns previous result (userA) again, But the username is different now!
var userB = await context.Users
    .Cacheable()
    .FirstOrDefaultAsync(user => user.Username == "EFG", cancellationToken);

// By using Where clause, arguments cache properly.

// Returns UserA based on "ABC" username.
var userA = await context.Users
    .Where(user => user.Username == "ABC")
    .Cacheable()
    .FirstOrDefaultAsync(cancellationToken);

// Returns UserB based on "EFG" username.
var userB = await context.Users
    .Where(user => user.Username == "EFG")
    .Cacheable()
    .FirstOrDefaultAsync(cancellationToken);
VahidN commented 5 years ago

True. It doesn't see anything after the Cacheable() method. For these cases (FirstOrDefaultAsync/ SingleOrDefaultAsync()), add a Where() method before the .Cacheable() method:

var ad = context.Analytics.Cacheable().FirstOrDefault(x => x.ID == 5);
// change it to
var ac = context.Analytics.Where(x => x.ID == 5).Cacheable().FirstOrDefault();
MoienTajik commented 5 years ago

This could be better at least by writing a Roslyn Analyzer for queries which are using Cacheable to get a warning/error at compile time. I can write that and open a pull request if you're ok with it.

VahidN commented 5 years ago

Fixed it via #https://github.com/VahidN/EFSecondLevelCache.Core/commit/17a44e4da90fed372cfd12e7d797683793120d8a. Now it sees the whole expression tree.

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.