-
Notifications
You must be signed in to change notification settings - Fork 31
Fix EFCore.BulkExtensions compatibility: shadow _queryContextFactory in CustomQueryCompiler
#200
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
Merged
PhenX
merged 4 commits into
master
from
copilot/update-entityframeworkcore-projectables
Apr 11, 2026
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
38619af
Initial plan
Copilot 0d80766
Fix CustomQueryCompiler reflection compatibility with EFCore.BulkExte…
Copilot 68c6c6e
Address code review feedback: explicit private modifiers, helper meth…
Copilot cd26a77
Apply suggestions from code review
PhenX File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
105 changes: 105 additions & 0 deletions
105
tests/EntityFrameworkCore.Projectables.VendorTests/EFCoreBulkExtensionsCompatibilityTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| using EFCore.BulkExtensions; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using Xunit; | ||
|
|
||
| namespace EntityFrameworkCore.Projectables.VendorTests; | ||
|
|
||
| /// <summary> | ||
| /// Tests that verify Projectables is compatible with EFCore.BulkExtensions batch | ||
| /// delete/update operations. | ||
| /// | ||
| /// Background: EFCore.BulkExtensions' <c>BatchUtil.GetDbContext</c> discovers the | ||
| /// DbContext via reflection by accessing the IQueryCompiler instance stored inside | ||
| /// EntityQueryProvider and then reading its private <c>_queryContextFactory</c> field. | ||
| /// Because C# reflection does not surface private fields from base classes when | ||
| /// GetField is called on a derived type, without an explicit shadow field in | ||
| /// <c>CustomQueryCompiler</c> the lookup returns null and the next GetValue(null) | ||
| /// call throws a <c>TargetException</c> ("Non-static method requires a target"). | ||
| /// The shadow field added to <c>CustomQueryCompiler</c> fixes this. | ||
| /// </summary> | ||
| public class EFCoreBulkExtensionsCompatibilityTests : IDisposable | ||
| { | ||
| private readonly TestDbContext _context; | ||
|
|
||
| public EFCoreBulkExtensionsCompatibilityTests() | ||
| { | ||
| _context = new TestDbContext(); | ||
| _context.Database.EnsureCreated(); | ||
| _context.SeedData(); | ||
| } | ||
|
|
||
| public void Dispose() => _context.Dispose(); | ||
|
|
||
| [Fact] | ||
| public void GetDbContext_WithProjectablesEnabled_DoesNotThrow() | ||
| { | ||
| // Arrange | ||
| var query = _context.Set<Order>().Where(o => o.IsCompleted); | ||
|
|
||
| // Act – BatchUtil.GetDbContext is the method that was previously throwing | ||
| // "Non-static method requires a target" because _queryContextFactory was not | ||
| // discoverable via reflection on CustomQueryCompiler. | ||
| var exception = Record.Exception(() => BatchUtil.GetDbContext(query)); | ||
|
|
||
| // Assert | ||
| Assert.Null(exception); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void GetDbContext_WithProjectablesEnabled_ReturnsCorrectContext() | ||
| { | ||
| // Arrange | ||
| var query = _context.Set<Order>().Where(o => o.IsCompleted); | ||
|
|
||
| // Act | ||
| var dbContext = BatchUtil.GetDbContext(query); | ||
|
|
||
| // Assert – must return the same DbContext, not null | ||
| Assert.NotNull(dbContext); | ||
| Assert.Same(_context, dbContext); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task BatchDeleteAsync_WithProjectablesEnabled_DoesNotThrowTargetException() | ||
| { | ||
| // Arrange | ||
| var query = _context.Set<Order>().Where(o => o.IsCompleted); | ||
|
|
||
| // Act – previously this would throw TargetException with message | ||
| // "Non-static method requires a target" when Projectables 3.x was used. | ||
| #pragma warning disable CS0618 // BatchDeleteAsync is marked obsolete in favour of EF 7 ExecuteDeleteAsync, but we | ||
| // specifically need to test EFCore.BulkExtensions' own batch path. | ||
| var exception = await Record.ExceptionAsync( | ||
| () => query.BatchDeleteAsync(TestContext.Current.CancellationToken)); | ||
| #pragma warning restore CS0618 | ||
|
|
||
| // A TargetException means the reflection-based DbContext discovery inside | ||
| // EFCore.BulkExtensions failed. Other exceptions (e.g. SQL syntax differences | ||
| // on SQLite) are acceptable because they come from actual SQL execution, not | ||
| // from the broken reflection chain. | ||
| AssertNoTargetException(exception, "BatchDeleteAsync"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task BatchUpdateAsync_WithProjectablesEnabled_DoesNotThrowTargetException() | ||
| { | ||
| // Arrange | ||
| var query = _context.Set<Order>().Where(o => o.IsCompleted); | ||
|
|
||
| // Act | ||
| #pragma warning disable CS0618 // BatchUpdateAsync is marked obsolete in favour of EF 7 ExecuteUpdateAsync | ||
| var exception = await Record.ExceptionAsync( | ||
| () => query.BatchUpdateAsync( | ||
| o => new Order { Total = o.Total * 2 }, | ||
| cancellationToken: TestContext.Current.CancellationToken)); | ||
| #pragma warning restore CS0618 | ||
|
|
||
| AssertNoTargetException(exception, "BatchUpdateAsync"); | ||
| } | ||
|
|
||
| private static void AssertNoTargetException(Exception? exception, string operationName) | ||
| => Assert.False( | ||
| exception is System.Reflection.TargetException, | ||
| $"{operationName} threw TargetException (\"Non-static method requires a target\"). " + | ||
| $"This indicates that CustomQueryCompiler's _queryContextFactory shadow field is missing."); | ||
| } |
34 changes: 34 additions & 0 deletions
34
...rameworkCore.Projectables.VendorTests/EntityFrameworkCore.Projectables.VendorTests.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <!-- EFCore.BulkExtensions 8.0.4 only supports net8.0, so restrict to a single TFM | ||
| and override the global TargetFrameworks set in Directory.Build.props. --> | ||
| <TargetFrameworks>net8.0</TargetFrameworks> | ||
| <IsPackable>false</IsPackable> | ||
| <Nullable>enable</Nullable> | ||
| <!-- Suppress the LangVersion warning: Directory.Build.props sets LangVersion=12.0 globally | ||
| which is fine for this project. --> | ||
| <LangVersion>12.0</LangVersion> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="coverlet.collector"> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="EFCore.BulkExtensions" /> | ||
| <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="xunit.v3" /> | ||
| <PackageReference Include="xunit.runner.visualstudio"> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| </PackageReference> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\src\EntityFrameworkCore.Projectables.Generator\EntityFrameworkCore.Projectables.Generator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" /> | ||
| <ProjectReference Include="..\..\src\EntityFrameworkCore.Projectables\EntityFrameworkCore.Projectables.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
62 changes: 62 additions & 0 deletions
62
tests/EntityFrameworkCore.Projectables.VendorTests/TestContext.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| using Microsoft.Data.Sqlite; | ||
| using Microsoft.EntityFrameworkCore; | ||
|
|
||
| namespace EntityFrameworkCore.Projectables.VendorTests; | ||
|
|
||
| /// <summary>Order entity used in vendor-compatibility tests.</summary> | ||
| public class Order | ||
| { | ||
| public int Id { get; set; } | ||
| public string? CustomerName { get; set; } | ||
| public decimal Total { get; set; } | ||
| public bool IsCompleted { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// A computed projectable property. Having at least one [Projectable] read-only | ||
| /// property on the entity ensures that <c>CustomQueryCompiler</c> is exercised | ||
| /// (it expands the projectable reference and potentially adds a Select wrapper). | ||
| /// </summary> | ||
| [Projectable] | ||
| public bool IsLargeOrder => Total > 100; | ||
| } | ||
|
|
||
| public class TestDbContext : DbContext | ||
| { | ||
| // Keep the connection open for the lifetime of the context so the in-memory | ||
| // SQLite database is not destroyed between operations. | ||
| private readonly SqliteConnection _connection; | ||
|
|
||
| public TestDbContext() | ||
| { | ||
| _connection = new SqliteConnection("DataSource=:memory:"); | ||
| _connection.Open(); | ||
| } | ||
|
|
||
| public DbSet<Order> Orders => Set<Order>(); | ||
|
|
||
| protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) | ||
| { | ||
| optionsBuilder.UseSqlite(_connection); | ||
| optionsBuilder.UseProjectables(); | ||
| } | ||
|
|
||
| protected override void OnModelCreating(ModelBuilder modelBuilder) | ||
| { | ||
| modelBuilder.Entity<Order>(); | ||
| } | ||
|
|
||
| public void SeedData() | ||
| { | ||
| Orders.AddRange( | ||
| new Order { CustomerName = "Alice", Total = 50m, IsCompleted = false }, | ||
| new Order { CustomerName = "Bob", Total = 150m, IsCompleted = true }, | ||
| new Order { CustomerName = "Charlie", Total = 200m, IsCompleted = true }); | ||
| SaveChanges(); | ||
| } | ||
|
|
||
| public override void Dispose() | ||
| { | ||
| base.Dispose(); | ||
| _connection.Dispose(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.