-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Global query filters raise a "An item with the same key has already been added" exception #7
Comments
hi @dapnet2018, EntityFrameworkCore.Cacheable/EntityFrameworkCore.CacheableTests/CacheableExpressionTests.cs Lines 361 to 406 in bbf8ee7
Could you test it on you system? |
Thank you @SteffenMangold for taking the time to look into this issue - I really appreciate it. I couldn't recreate the issue using your updated unit tests either at first, but I have been able to recreate the issue with two minor changes: The first change is that the first time the BloggingContext is instantiated, the minBlogId value needs to be passed even though it's not needed yet to ensure that in the OnModelCreating method the global query filter is applied. OnModelCreating is only called the very first time the context is created. So in CacheableExpressionTests.cs, line 374 should change from: using (var initContext = new BloggingContext(options)) to: using (var initContext = new BloggingContext(options, minBlogId: 2)) The second change is that doing a Count() does not seem to cause this issue, but a ToList() or some other query with a predicate does, so if you add the below somewhere within the var allItems = constantContext.Blogs.ToList();
Assert.AreEqual(2, allItems.Count); For completeness, my updated version of the GlobalQueryFilterTest is as follows: /// <summary>
/// Testing null parameter query .
/// </summary>
[TestMethod]
public void GlobalQueryFilterTest()
{
MemoryCacheProvider.ClearCache();
var loggerProvider = new DebugLoggerProvider();
var loggerFactory = new LoggerFactory(new[] { loggerProvider });
var options = new DbContextOptionsBuilder<BloggingContext>()
.UseLoggerFactory(loggerFactory)
.UseInMemoryDatabase(databaseName: "GlobalQueryFilterTest")
.Options;
// create test entries
using (var initContext = new BloggingContext(options, minBlogId: 2))
{
initContext.Blogs.Add(new Blog { BlogId = 1, Url = "http://sample.com/cats" });
initContext.Blogs.Add(new Blog { BlogId = 2, Url = "http://sample.com/catfish" });
initContext.Blogs.Add(new Blog { BlogId = 3, Url = "http://sample.com/dogs" });
initContext.SaveChanges();
}
using (var constantContext = new BloggingContext(options, minBlogId: 2))
{
var allItems = constantContext.Blogs.ToList();
Assert.AreEqual(2, allItems.Count);
// shoud not hit cache, because no Cacheable call
var rawResult = constantContext.Blogs
.Count();
// shoud not hit cache, because first execution
var result = constantContext.Blogs
.Cacheable(TimeSpan.FromMinutes(5))
.Count();
// shoud hit cache, because second execution
var cachedResult = constantContext.Blogs
.Cacheable(TimeSpan.FromMinutes(5))
.Count();
Assert.AreEqual(result, cachedResult);
}
// find "cache hit" log entries
var logs = loggerProvider.Entries.Where(e => e.EventId == CacheableEventId.CacheHit);
// cache should hit one time
Assert.IsTrue(logs.Count() == 1);
} Thanks again for your assistance |
Hi seem that this error is a parameter name collision related to the global filter. Maybe @smitpatel can support us with this. |
That does seem to be the issue. One further thing is that I encounter this exception only if my DbContext makes a call to Any thoughts as to why that might be? |
No, sadly not. But it is a parameter name collision like in the mentioned bug. |
@dapnet2018 - Which line in Looking at the implementation, second level caching may not work properly with Query filters. QueryFilters have DbContext based property which can have different values for different execution. In order to use the query caching in EF Core, we compile the funcs inside compiled query to compute the value from current context and add it to parameters for the query. But the way second level cache is using cache key, it is looking at the external parameters but not query filter based parameters. (which it cannot see easily anyway). So if you run a query with second level cache, and run it in different context which has different values for filter properties, it will still give earlier result, which would be incorrect result. |
Hi @smitpatel, thanks for taking a look at this. I don't think Steffen has updated the code yet, so you will need to use my adjusted version of the GlobalQueryFilterTest I posted above, and then it will error on this line : var allItems = constantContext.Blogs.ToList(); The error is thrown in the However, based on what you've said I'm guessing I need to stop using the Second Level caching for any DbContexts where I'm also using Global Query Filters, as it sounds like it's not an easy fix to incorporate the Query Filter parameters into the cache key. Is that right? |
This is the culprit for this exception: EntityFrameworkCore.Cacheable/EntityFrameworkCore.Cacheable/CustomQueryCompiler.cs Line 188 in bbf8ee7
since Still cached results from second level cache will not reflect changed value in query filters if any. |
If it's the case that this library doesn't play well with Global Query Filters, it it possible to determine when filters have been defined for the query to skip the cache and log a warning? |
I get the same exception. I use a Global Query Filter to filter the results for single tenants in my app. Sadly, for me it looks like I have to remove these Extensions completely until this issue gets fixed. |
If an entity has Global Query Filters defined (e.g. in the OnModelCreating method of the DbContext executing modelBuilder.Entity().HasQueryFilter(...)) and the DbContext is using UseSecondLevelCache, then when trying to retrieve data from the DbContext an exception is thrown "An item with the same key has already been added".
Steps to reproduce
The below unit test shows the issue - it will error on line 103 "Assert.IsTrue(test1Context.Blogs.Count() == 3);"
This unit test is modelling a multi-tenant scenario, where each Entity has a MultiTenantId int property. The relevant MultiTenantID is passed to the DbContext in it's constructor and the Global Query Filter automatically applies a "MultiTenantID = X" predicate.
This unit test will succeed if the "optionsBuilder.UseSecondLevelCache();" is removed from the OnConfiguring method in the GlobalQueryFilterContext class
Further technical details
EntityFrameworkCore.Cacheable version: 2.0.0 (and also latest Master branch code from GitHub)
EF Core version: 2.2.0
IDE: Visual Studio 2017 15.9.6
The text was updated successfully, but these errors were encountered: