Skip to content

Commit 83976ab

Browse files
committed
Allow explicit projections through abstract navigations
1 parent 37ea8a1 commit 83976ab

4 files changed

Lines changed: 111 additions & 60 deletions

File tree

src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
4242
// Pattern 2: filters.For<T>().Add(projection: ..., filter: ...)
4343
if (!IsIdentityProjection(projectionLambda))
4444
{
45+
// Explicit projections that extract specific properties from abstract navigations
46+
// are ALLOWED - they produce efficient SQL. Only identity projections are problematic.
4547
return;
4648
}
4749

src/GraphQL.EntityFramework/Filters/FilterEntry.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ static void ValidateProjectionCompatibility(
4242
Expression<Func<TEntity, TProjection>> projection,
4343
IReadOnlySet<string> requiredPropertyNames)
4444
{
45+
// Only validate identity projections (x => x)
46+
// Explicit projections that extract specific properties from abstract navigations are ALLOWED
47+
// because EF Core can project scalar properties through abstract navigations efficiently.
48+
// The problem only occurs with identity projections where the filter accesses abstract nav properties,
49+
// which forces Include() to load all columns.
50+
if (!IsIdentityProjection(projection))
51+
{
52+
return;
53+
}
54+
4555
// Extract navigation paths (e.g., "Parent.Property" -> navigation "Parent")
4656
var navigationPaths = requiredPropertyNames
4757
.Where(_ => _.Contains('.'))
@@ -55,7 +65,6 @@ static void ValidateProjectionCompatibility(
5565
}
5666

5767
var entityType = typeof(TEntity);
58-
var isIdentity = IsIdentityProjection(projection);
5968

6069
// Check each navigation for abstract types
6170
foreach (var navPath in navigationPaths)
@@ -83,12 +92,11 @@ static void ValidateProjectionCompatibility(
8392

8493
if (navType.IsAbstract)
8594
{
86-
var projectionType = isIdentity ? "identity projection '_ => _'" : "projection that accesses abstract navigation";
8795
throw new InvalidOperationException(
88-
$"Filter for '{entityType.Name}' uses {projectionType} " +
96+
$"Filter for '{entityType.Name}' uses identity projection '_ => _' " +
8997
$"to access properties of abstract navigation '{navPath}' ({navType.Name}). " +
9098
$"This forces Include() to load all columns from {navType.Name}. " +
91-
$"Abstract types cannot be projected. Extract only the required properties: " +
99+
$"Extract only the required properties in an explicit projection: " +
92100
$"projection: e => new {{ e.Id, {navPath}Property = e.{navPath}.PropertyName }}, " +
93101
$"filter: (_, _, _, proj) => proj.{navPath}Property == value");
94102
}

src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,47 @@
11
public partial class IntegrationTests
22
{
33
/// <summary>
4-
/// Test that explicit projection accessing properties through abstract navigation throws.
5-
/// DerivedChildEntity.Parent is BaseEntity (abstract), so projecting Parent.Property
6-
/// should throw because abstract types cannot be projected by EF Core.
4+
/// Test that explicit projection accessing properties through abstract navigation succeeds.
5+
/// DerivedChildEntity.Parent is BaseEntity (abstract), but explicit projections that extract
6+
/// specific scalar properties are ALLOWED because EF Core can project scalar properties
7+
/// through abstract navigations efficiently. Only identity projections are problematic.
78
/// </summary>
89
[Fact]
9-
public void Explicit_projection_with_abstract_navigation_property_throws()
10+
public void Explicit_projection_with_abstract_navigation_property_succeeds()
1011
{
1112
var filters = new Filters<IntegrationDbContext>();
1213

13-
var exception = Assert.Throws<InvalidOperationException>(() =>
14-
{
15-
// Projection accesses Parent.Property where Parent is BaseEntity (abstract)
16-
filters.For<DerivedChildEntity>().Add(
17-
projection: c => c.Parent!.Property,
18-
filter: (_, _, _, prop) => prop == "test");
19-
});
14+
// Projection accesses Parent.Property where Parent is BaseEntity (abstract)
15+
// This is ALLOWED - EF Core generates efficient JOIN to get only the needed column
16+
filters.For<DerivedChildEntity>().Add(
17+
projection: c => c.Parent!.Property,
18+
filter: (_, _, _, prop) => prop == "test");
2019

21-
Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase);
22-
Assert.Contains("Parent", exception.Message);
20+
// Verify the filter was registered with the expected property
21+
var requiredProps = filters.GetRequiredFilterProperties<DerivedChildEntity>();
22+
Assert.Contains("Parent.Property", requiredProps);
2323
}
2424

2525
/// <summary>
26-
/// Test that anonymous type projection with abstract navigation also throws.
27-
/// Even with anonymous types, we cannot project from abstract navigations.
26+
/// Test that anonymous type projection with abstract navigation succeeds.
27+
/// Explicit projections (including anonymous types) that extract specific properties
28+
/// through abstract navigations are ALLOWED - EF Core handles this efficiently.
2829
/// </summary>
2930
[Fact]
30-
public void Anonymous_type_projection_with_abstract_navigation_throws()
31+
public void Anonymous_type_projection_with_abstract_navigation_succeeds()
3132
{
3233
var filters = new Filters<IntegrationDbContext>();
3334

34-
var exception = Assert.Throws<InvalidOperationException>(() =>
35-
{
36-
// Even with anonymous type, projecting from abstract Parent throws
37-
filters.For<DerivedChildEntity>().Add(
38-
projection: c => new { c.Id, ParentProperty = c.Parent!.Property },
39-
filter: (_, _, _, proj) => proj.ParentProperty == "test");
40-
});
35+
// Anonymous type projection extracting properties through abstract Parent
36+
// This is ALLOWED - EF Core generates efficient SQL with JOINs
37+
filters.For<DerivedChildEntity>().Add(
38+
projection: c => new { c.Id, ParentProperty = c.Parent!.Property },
39+
filter: (_, _, _, proj) => proj.ParentProperty == "test");
4140

42-
Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase);
43-
Assert.Contains("Parent", exception.Message);
41+
// Verify the filter was registered with expected properties
42+
var requiredProps = filters.GetRequiredFilterProperties<DerivedChildEntity>();
43+
Assert.Contains("Id", requiredProps);
44+
Assert.Contains("Parent.Property", requiredProps);
4445
}
4546

4647
/// <summary>
@@ -104,22 +105,43 @@ public void Four_parameter_filter_with_abstract_navigation_allowed_at_runtime()
104105
}
105106

106107
/// <summary>
107-
/// Test projecting from nested abstract navigation property.
108+
/// Test projecting from nested abstract navigation property succeeds.
109+
/// Explicit projections that extract specific scalar properties are allowed.
110+
/// </summary>
111+
[Fact]
112+
public void Projection_with_nested_abstract_navigation_property_succeeds()
113+
{
114+
var filters = new Filters<IntegrationDbContext>();
115+
116+
// Accessing nested property through abstract parent is allowed for explicit projections
117+
filters.For<DerivedChildEntity>().Add(
118+
projection: c => c.Parent!.Status,
119+
filter: (_, _, _, status) => status == "Active");
120+
121+
var requiredProps = filters.GetRequiredFilterProperties<DerivedChildEntity>();
122+
Assert.Contains("Parent.Status", requiredProps);
123+
}
124+
125+
/// <summary>
126+
/// Test that identity projection with filter accessing abstract navigation is NOT caught at runtime.
127+
/// The runtime cannot analyze the filter delegate. The GQLEF007 analyzer catches this at compile time.
108128
/// </summary>
109129
[Fact]
110-
public void Projection_with_nested_abstract_navigation_property_throws()
130+
public void Identity_projection_with_abstract_navigation_in_filter_not_caught_at_runtime()
111131
{
112132
var filters = new Filters<IntegrationDbContext>();
113133

114-
var exception = Assert.Throws<InvalidOperationException>(() =>
115-
{
116-
// Accessing nested property through abstract parent
117-
filters.For<DerivedChildEntity>().Add(
118-
projection: c => c.Parent!.Status,
119-
filter: (_, _, _, status) => status == "Active");
120-
});
134+
// Identity projection where filter accesses abstract Parent
135+
// This is NOT caught at runtime because:
136+
// 1. The projection `c => c` has no property accesses to analyze
137+
// 2. The filter is a compiled delegate, not an expression tree
138+
// The GQLEF007 analyzer catches this at compile time instead.
139+
filters.For<DerivedChildEntity>().Add(
140+
projection: c => c,
141+
filter: (_, _, _, c) => c.Parent!.Property == "test");
121142

122-
Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase);
123-
Assert.Contains("Parent", exception.Message);
143+
// Identity projection extracts no properties
144+
var requiredProps = filters.GetRequiredFilterProperties<DerivedChildEntity>();
145+
Assert.Empty(requiredProps);
124146
}
125147
}

src/Tests/UnitTests/FilterEntryValidationTests.cs

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,20 @@ class ConcreteParent
2424
class TestDbContext : DbContext;
2525

2626
[Fact]
27-
public void Explicit_projection_with_abstract_navigation_property_throws()
27+
public void Explicit_projection_with_abstract_navigation_property_succeeds()
2828
{
2929
var filters = new Filters<TestDbContext>();
3030

31-
// This should throw - projection directly accesses abstract navigation property
32-
var exception = Assert.Throws<InvalidOperationException>(() =>
33-
{
34-
filters.For<TestEntity>().Add(
35-
projection: e => e.Parent!.Property,
36-
filter: (_, _, _, prop) => prop == "test");
37-
});
31+
// Explicit projections that extract specific properties through abstract navigations are ALLOWED
32+
// because EF Core can project scalar properties through abstract navigations efficiently.
33+
// Only identity projections are problematic.
34+
filters.For<TestEntity>().Add(
35+
projection: e => e.Parent!.Property,
36+
filter: (_, _, _, prop) => prop == "test");
3837

39-
Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase);
40-
Assert.Contains("Parent", exception.Message);
38+
// Verify the filter was registered with the expected property
39+
var requiredProps = filters.GetRequiredFilterProperties<TestEntity>();
40+
Assert.Contains("Parent.Property", requiredProps);
4141
}
4242

4343
[Fact]
@@ -52,20 +52,20 @@ public void Concrete_navigation_allows_identity_projection()
5252
}
5353

5454
[Fact]
55-
public void Anonymous_type_projection_with_abstract_navigation_throws()
55+
public void Anonymous_type_projection_with_abstract_navigation_succeeds()
5656
{
5757
var filters = new Filters<TestDbContext>();
5858

59-
// This should also throw - even with anonymous type, we're projecting from abstract navigation
60-
var exception = Assert.Throws<InvalidOperationException>(() =>
61-
{
62-
filters.For<TestEntity>().Add(
63-
projection: e => new { e.Id, ParentProp = e.Parent!.Property },
64-
filter: (_, _, _, proj) => proj.ParentProp == "test");
65-
});
59+
// Explicit projections (including anonymous types) that extract specific properties
60+
// through abstract navigations are ALLOWED - EF Core handles this efficiently.
61+
filters.For<TestEntity>().Add(
62+
projection: e => new { e.Id, ParentProp = e.Parent!.Property },
63+
filter: (_, _, _, proj) => proj.ParentProp == "test");
6664

67-
Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase);
68-
Assert.Contains("Parent", exception.Message);
65+
// Verify the filter was registered with expected properties
66+
var requiredProps = filters.GetRequiredFilterProperties<TestEntity>();
67+
Assert.Contains("Id", requiredProps);
68+
Assert.Contains("Parent.Property", requiredProps);
6969
}
7070

7171
[Fact]
@@ -90,4 +90,23 @@ public void Identity_projection_without_navigation_succeeds()
9090
projection: e => e,
9191
filter: (_, _, _, e) => e.Property == "test");
9292
}
93+
94+
[Fact]
95+
public void Identity_projection_with_abstract_navigation_in_filter_not_caught_at_runtime()
96+
{
97+
var filters = new Filters<TestDbContext>();
98+
99+
// Identity projection where filter accesses abstract navigation
100+
// This is NOT caught at runtime because:
101+
// 1. The projection `e => e` has no property accesses to analyze
102+
// 2. The filter is a compiled delegate, not an expression tree
103+
// The GQLEF007 analyzer catches this at compile time instead.
104+
filters.For<TestEntity>().Add(
105+
projection: e => e,
106+
filter: (_, _, _, e) => e.Parent!.Property == "test");
107+
108+
// Identity projection extracts no properties
109+
var requiredProps = filters.GetRequiredFilterProperties<TestEntity>();
110+
Assert.Empty(requiredProps);
111+
}
93112
}

0 commit comments

Comments
 (0)