From 574eea58024bbdb078d68a9f4b8444d742d933d4 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 2 Oct 2026 09:13:36 +1000 Subject: [PATCH] Fix anti-pattern false positives on keyless AllData and EF's reload query - AllData no longer applies AsNoTracking to keyless entity types, which EF never tracks, so it threw under ThrowOnAntiPatterns on any model with a keyless view. - Queries EF builds itself for EntityEntry.Reload and GetDatabaseValues (AsNoTracking().IgnoreQueryFilters().Where(key).Select(object[])) are no longer checked, since the code under test can not change them. IgnoredQueryFiltersDetector threw on them for entity types without a query filter. --- .../AllDataTests.Keyless.verified.txt | 9 ++++ .../AllDataTests.cs | 50 +++++++++++++++++++ .../AntiPatternInterceptor.cs | 5 ++ src/Verify.EntityFramework/InternalQuery.cs | 42 ++++++++++++++++ .../VerifyEntityFramework.cs | 4 +- 5 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 src/Verify.EntityFramework.Tests/AllDataTests.Keyless.verified.txt create mode 100644 src/Verify.EntityFramework/InternalQuery.cs diff --git a/src/Verify.EntityFramework.Tests/AllDataTests.Keyless.verified.txt b/src/Verify.EntityFramework.Tests/AllDataTests.Keyless.verified.txt new file mode 100644 index 00000000..f72ce03a --- /dev/null +++ b/src/Verify.EntityFramework.Tests/AllDataTests.Keyless.verified.txt @@ -0,0 +1,9 @@ +[ + { + Id: 1, + Name: rex + }, + { + Name: rex + } +] \ No newline at end of file diff --git a/src/Verify.EntityFramework.Tests/AllDataTests.cs b/src/Verify.EntityFramework.Tests/AllDataTests.cs index 28dcb8cf..30c54239 100644 --- a/src/Verify.EntityFramework.Tests/AllDataTests.cs +++ b/src/Verify.EntityFramework.Tests/AllDataTests.cs @@ -49,6 +49,56 @@ public async Task CompositeKey() await Verify(data.AllData()); } + // AsNoTracking on a keyless entity type threw under ThrowOnAntiPatterns + [Test] + public async Task Keyless() + { + var builder = new DbContextOptionsBuilder(); + builder.UseInMemoryDatabase(nameof(AllDataTests) + nameof(Keyless)); + builder.ThrowOnAntiPatterns(); + await using var data = new KeylessDbContext(builder.Options); + data.Add(new Animal { Id = 1, Name = "rex" }); + await data.SaveChangesAsync(); + + await Verify(data.AllData()); + } + + // EF builds the query for Reload and GetDatabaseValues itself, with IgnoreQueryFilters, which threw under ThrowOnAntiPatterns + [Test] + public async Task Reload() + { + var builder = new DbContextOptionsBuilder(); + builder.UseInMemoryDatabase(nameof(AllDataTests) + nameof(Reload)); + builder.ThrowOnAntiPatterns(); + await using var data = new KeylessDbContext(builder.Options); + var animal = new Animal { Id = 1, Name = "rex" }; + data.Add(animal); + await data.SaveChangesAsync(); + + var entry = data.Entry(animal); + entry.Reload(); + await entry.ReloadAsync(); + await Assert.That(await entry.GetDatabaseValuesAsync()).IsNotNull(); + } + + public class KeylessDbContext(DbContextOptions options) : + DbContext(options) + { + public DbSet Animals { get; set; } = null!; + public DbSet AnimalNames { get; set; } = null!; + + protected override void OnModelCreating(ModelBuilder model) => + model + .Entity() + .HasNoKey() + .ToInMemoryQuery(() => Animals.Select(_ => new AnimalName { Name = _.Name })); + } + + public class AnimalName + { + public required string Name { get; set; } + } + static AllDataDbContext BuildData([CallerMemberName] string databaseName = "") { var builder = new DbContextOptionsBuilder(); diff --git a/src/Verify.EntityFramework/AntiPatternInterceptor.cs b/src/Verify.EntityFramework/AntiPatternInterceptor.cs index 17a56ce4..0c5ec14b 100644 --- a/src/Verify.EntityFramework/AntiPatternInterceptor.cs +++ b/src/Verify.EntityFramework/AntiPatternInterceptor.cs @@ -44,6 +44,11 @@ class AntiPatternInterceptor : public Expression QueryCompilationStarting(Expression query, QueryExpressionEventData data) { + if (InternalQuery.Is(query)) + { + return query; + } + DiscardedOrderByDetector.ThrowIfDiscarded(query); ConstantOrderingDetector.ThrowIfConstant(query); RedundantNullCheckDetector.ThrowIfRedundant(query); diff --git a/src/Verify.EntityFramework/InternalQuery.cs b/src/Verify.EntityFramework/InternalQuery.cs new file mode 100644 index 00000000..4f5165d6 --- /dev/null +++ b/src/Verify.EntityFramework/InternalQuery.cs @@ -0,0 +1,42 @@ +// A query that EF builds itself, which the code under test can not change, so is not checked for anti-patterns. +// EntityEntry.Reload and GetDatabaseValues query the row of an entry by its key, as +// root.AsNoTracking().IgnoreQueryFilters().Where(key).Select(_ => object[]), then FirstOrDefault. +static class InternalQuery +{ + public static bool Is(Expression query) + { + var expression = query; + while (expression is MethodCallExpression { Method.IsStatic: true, Arguments.Count: > 0 } call) + { + if (IsDatabaseValues(call)) + { + return true; + } + + expression = call.Arguments[0]; + } + + return false; + } + + static bool IsDatabaseValues(MethodCallExpression select) + { + if (!Is(select, typeof(Queryable), nameof(Queryable.Select)) || + select.Type != typeof(IQueryable) || + select.Arguments[0] is not MethodCallExpression where || + !Is(where, typeof(Queryable), nameof(Queryable.Where)) || + where.Arguments[0] is not MethodCallExpression ignore || + !Is(ignore, typeof(EntityFrameworkQueryableExtensions), nameof(EntityFrameworkQueryableExtensions.IgnoreQueryFilters)) || + ignore.Arguments[0] is not MethodCallExpression noTracking || + !Is(noTracking, typeof(EntityFrameworkQueryableExtensions), nameof(EntityFrameworkQueryableExtensions.AsNoTracking))) + { + return false; + } + + return noTracking.Arguments[0] is EntityQueryRootExpression; + } + + static bool Is(MethodCallExpression call, Type type, string name) => + call.Method.DeclaringType == type && + call.Method.Name == name; +} diff --git a/src/Verify.EntityFramework/VerifyEntityFramework.cs b/src/Verify.EntityFramework/VerifyEntityFramework.cs index 89bad923..befb9b59 100644 --- a/src/Verify.EntityFramework/VerifyEntityFramework.cs +++ b/src/Verify.EntityFramework/VerifyEntityFramework.cs @@ -43,11 +43,11 @@ static async Task> QueryEntities(DbContext data, IEntityType ent queryable = data.Set(); } - queryable = queryable.AsNoTracking(); - var key = entityType.FindPrimaryKey(); + // EF never tracks a keyless entity type, so AsNoTracking would be redundant if (key != null) { + queryable = queryable.AsNoTracking(); var method = nameof(Queryable.OrderBy); foreach (var property in key.Properties) {