Skip to content

Commit 37ea8a1

Browse files
SimonCroppSimonCropp
authored
Fix filter projection to avoid selecting all navigation fields (#1226)
1 parent 1131106 commit 37ea8a1

34 files changed

Lines changed: 1819 additions & 121 deletions

‎docs/analyzers/GQLEF007.md‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# GQLEF007: Identity projection with abstract navigation access
2+
3+
## Cause
4+
5+
A filter uses identity projection (`_ => _` or 4-parameter syntax) but the filter lambda accesses properties through an abstract navigation type.
6+
7+
## Problem
8+
9+
Abstract types cannot be instantiated in SQL projections. When Entity Framework Core encounters a projection that would require instantiating an abstract class, it cannot generate the SQL expression. Instead, EF Core falls back to using `Include()` which loads **all columns** from the related table.
10+
11+
This creates a significant performance problem:
12+
- The filter only needs 1-2 fields from the navigation
13+
- But EF Core loads all 30+ fields because of the Include fallback
14+
- Query performance degrades and memory usage increases unnecessarily
15+
16+
## Rule Details
17+
18+
This diagnostic is reported when:
19+
1. A filter uses identity projection (explicit `_ => _` or implicit via 4-parameter syntax)
20+
2. The filter lambda accesses a property through a navigation property
21+
3. That navigation property's type is abstract (has the `abstract` keyword)
22+
23+
### Examples of violations
24+
25+
```csharp
26+
// ❌ Identity projection with abstract navigation access
27+
public abstract class BaseEntity
28+
{
29+
public string Property { get; set; }
30+
}
31+
32+
public class Child
33+
{
34+
public Guid Id { get; set; }
35+
public BaseEntity Parent { get; set; }
36+
}
37+
38+
// This triggers GQLEF007:
39+
filters.For<Child>().Add(
40+
projection: _ => _, // Identity projection
41+
filter: (_, _, _, c) => c.Parent.Property == "test");
42+
43+
// This also triggers GQLEF007 (4-parameter syntax uses identity projection):
44+
filters.For<Child>().Add(
45+
filter: (_, _, _, c) => c.Parent.Property == "test");
46+
```
47+
48+
### Examples of correct code
49+
50+
```csharp
51+
// ✅ Explicit projection extracting required properties
52+
filters.For<Child>().Add(
53+
projection: c => new { c.Id, ParentProperty = c.Parent.Property },
54+
filter: (_, _, _, proj) => proj.ParentProperty == "test");
55+
56+
// ✅ Concrete (non-abstract) navigation is fine with identity projection
57+
public class ConcreteParent // Not abstract
58+
{
59+
public string Property { get; set; }
60+
}
61+
62+
public class Child
63+
{
64+
public ConcreteParent Parent { get; set; }
65+
}
66+
67+
filters.For<Child>().Add(
68+
projection: _ => _,
69+
filter: (_, _, _, c) => c.Parent.Property == "test"); // OK - ConcreteParent is not abstract
70+
```
71+
72+
## Solution
73+
74+
Convert the identity projection to an explicit projection that extracts only the required properties from the abstract navigation.
75+
76+
### Manual Fix
77+
78+
1. Identify all properties accessed through the abstract navigation
79+
2. Create an explicit projection that extracts those properties with flattened names
80+
3. Update the filter to use the projected property names
81+
4. Rename the filter parameter to `proj` for clarity
82+
83+
**Before:**
84+
```csharp
85+
filters.For<Child>().Add(
86+
projection: _ => _,
87+
filter: (_, _, _, c) => c.Parent.Property == "test");
88+
```
89+
90+
**After:**
91+
```csharp
92+
filters.For<Child>().Add(
93+
projection: c => new { c.Id, ParentProperty = c.Parent.Property },
94+
filter: (_, _, _, proj) => proj.ParentProperty == "test");
95+
```
96+
97+
### Automatic Fix
98+
99+
The code fixer can automatically perform this transformation:
100+
1. Place cursor on the diagnostic
101+
2. Press `Ctrl+.` (or `Cmd+.` on Mac)
102+
3. Select "Convert to explicit projection"
103+
104+
The fixer will:
105+
- Extract all accessed navigation properties
106+
- Create an anonymous type projection with flattened property names
107+
- Update the filter lambda to use the new property names
108+
- Rename the filter parameter from entity name to `proj`
109+
110+
## Performance Impact
111+
112+
Using explicit projections instead of identity projections with abstract navigations can significantly improve performance:
113+
114+
**Identity Projection (Inefficient):**
115+
```sql
116+
-- Loads all 34 columns from BaseRequest table
117+
SELECT a.*, br.*
118+
FROM Accommodation a
119+
LEFT JOIN BaseRequest br ON a.TravelRequestId = br.Id
120+
```
121+
122+
**Explicit Projection (Efficient):**
123+
```sql
124+
-- Loads only required columns
125+
SELECT a.Id, br.GroupOwnerId, br.HighestStatusAchieved
126+
FROM Accommodation a
127+
LEFT JOIN BaseRequest br ON a.TravelRequestId = br.Id
128+
```
129+
130+
## When to Suppress
131+
132+
**Never.** This diagnostic indicates a real performance issue that should be fixed. There is no valid scenario where loading all columns from an abstract navigation is preferred over loading only the required columns.
133+
134+
## Related Rules
135+
136+
- **GQLEF004** - Suggests using 4-parameter filter syntax for identity projections with key-only access
137+
- **GQLEF005** - Prevents accessing non-key properties with 4-parameter filter syntax
138+
- **GQLEF006** - Prevents accessing non-key properties with explicit identity projection
139+
140+
## See Also
141+
142+
- [Filter Projections Documentation](/docs/filters.md#abstract-type-navigations)
143+
- [Understanding EF Core Projections](https://learn.microsoft.com/en-us/ef/core/querying/how-query-works)

‎docs/filters.md‎

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ EfGraphQLConventions.RegisterInContainer<MyDbContext>(
258258
Common nullable patterns:
259259

260260
* **Has value check**: `quantity.HasValue && quantity.Value > 0`
261-
* **Null check**: `!quantity.HasValue`
261+
* **Null check**: `?quantity.HasValue`
262262
* **Exact match**: `isApproved == true` (not null or false)
263263

264264

@@ -522,12 +522,12 @@ The simplified API is syntactic sugar for the identity projection pattern:
522522
```csharp
523523
// Simplified API
524524
filters.For<Accommodation>().Add(
525-
filter: (_, _, _, a) => a.Id != Guid.Empty);
525+
filter: (_, _, _, a) => a.Id ?= Guid.Empty);
526526

527527
// Equivalent full API
528528
filters.For<Accommodation>().Add(
529529
projection: _ => _, // Identity projection
530-
filter: (_, _, _, a) => a.Id != Guid.Empty);
530+
filter: (_, _, _, a) => a.Id ?= Guid.Empty);
531531
```
532532

533533
### Analyzer Support
@@ -556,3 +556,96 @@ filters.For<Product>().Add(
556556
```
557557

558558
The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior
559+
560+
## Abstract Type Navigations
561+
562+
When working with filters that access properties through abstract navigation properties, special care must be taken to avoid performance issues.
563+
564+
### The Problem
565+
566+
Abstract types cannot be instantiated in SQL projections. When EF Core encounters an abstract navigation in a projection, it falls back to using `Include()` which loads **all columns** from the navigation table, even when only one or two fields are required.
567+
568+
### Example
569+
570+
```csharp
571+
// Given:
572+
public abstract class BaseRequest // Abstract class with many fields
573+
{
574+
public Guid Id { get; set; }
575+
public Guid GroupOwnerId { get; set; }
576+
public RequestStatus HighestStatusAchieved { get; set; }
577+
// ... 30+ more columns ...
578+
}
579+
580+
public class Accommodation
581+
{
582+
public Guid Id { get; set; }
583+
public Guid TravelRequestId { get; set; }
584+
public BaseRequest? TravelRequest { get; set; } // Navigation to abstract type
585+
}
586+
587+
// ❌ INEFFICIENT - Loads all 34 columns from BaseRequest:
588+
filters.For<Accommodation>().Add(
589+
projection: _ => _, // Identity projection
590+
filter: (_, _, _, a) => a.TravelRequest?.GroupOwnerId == groupId);
591+
592+
// ✅ EFFICIENT - Only loads Id, GroupOwnerId, HighestStatusAchieved:
593+
filters.For<Accommodation>().Add(
594+
projection: a => new {
595+
a.Id,
596+
RequestOwnerId = a.TravelRequest?.GroupOwnerId,
597+
RequestStatus = a.TravelRequest?.HighestStatusAchieved
598+
},
599+
filter: (_, _, _, proj) => proj.RequestOwnerId == groupId);
600+
```
601+
602+
### Detection and Prevention
603+
604+
The library provides both compile-time and runtime detection:
605+
606+
**Compile-Time (Analyzer GQLEF007)**
607+
608+
The analyzer detects when filters use identity projections with abstract navigation access:
609+
610+
```csharp
611+
// This will show GQLEF007 error in the IDE:
612+
filters.For<Child>().Add(
613+
projection: _ => _,
614+
filter: (_, _, _, c) => c.AbstractParent?.Property == "value");
615+
```
616+
617+
The code fixer can automatically convert this to an explicit projection.
618+
619+
**Runtime (Exception)**
620+
621+
If the analyzer is bypassed, runtime validation will throw an exception when the filter is registered:
622+
623+
```csharp
624+
// Throws InvalidOperationException:
625+
// "Filter for 'Child' uses identity projection '_ => _' to access properties
626+
// of abstract navigation 'Parent' (BaseEntity). This forces Include() to load
627+
// all columns from BaseEntity. Extract only the required properties..."
628+
```
629+
630+
### Best Practices
631+
632+
1. **Always use explicit projections** when accessing abstract navigations
633+
2. **Extract only required properties** from the abstract navigation
634+
3. **Flatten navigation properties** in the projection (e.g., `ParentProperty` instead of nested access)
635+
4. **Update the filter** to use the flattened property names
636+
637+
### Concrete Navigations
638+
639+
This issue only affects **abstract** navigation types. Concrete navigation types work fine with identity projections:
640+
641+
```csharp
642+
// ✅ WORKS - ConcreteParent is not abstract:
643+
filters.For<Child>().Add(
644+
projection: _ => _,
645+
filter: (_, _, _, c) => c.ConcreteParent?.Property == "value");
646+
```
647+
648+
### See Also
649+
650+
* [GQLEF007 Diagnostic Documentation](/docs/analyzers/GQLEF007.md)
651+
* [Identity Projection Filters](#simplified-filter-api)

‎docs/mdsource/filters.source.md‎

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ snippet: nullable-value-type-projections
8686
Common nullable patterns:
8787

8888
* **Has value check**: `quantity.HasValue && quantity.Value > 0`
89-
* **Null check**: `!quantity.HasValue`
89+
* **Null check**: `?quantity.HasValue`
9090
* **Exact match**: `isApproved == true` (not null or false)
9191

9292

@@ -201,12 +201,12 @@ The simplified API is syntactic sugar for the identity projection pattern:
201201
```csharp
202202
// Simplified API
203203
filters.For<Accommodation>().Add(
204-
filter: (_, _, _, a) => a.Id != Guid.Empty);
204+
filter: (_, _, _, a) => a.Id ?= Guid.Empty);
205205

206206
// Equivalent full API
207207
filters.For<Accommodation>().Add(
208208
projection: _ => _, // Identity projection
209-
filter: (_, _, _, a) => a.Id != Guid.Empty);
209+
filter: (_, _, _, a) => a.Id ?= Guid.Empty);
210210
```
211211

212212
### Analyzer Support
@@ -235,3 +235,96 @@ filters.For<Product>().Add(
235235
```
236236

237237
The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior
238+
239+
## Abstract Type Navigations
240+
241+
When working with filters that access properties through abstract navigation properties, special care must be taken to avoid performance issues.
242+
243+
### The Problem
244+
245+
Abstract types cannot be instantiated in SQL projections. When EF Core encounters an abstract navigation in a projection, it falls back to using `Include()` which loads **all columns** from the navigation table, even when only one or two fields are required.
246+
247+
### Example
248+
249+
```csharp
250+
// Given:
251+
public abstract class BaseRequest // Abstract class with many fields
252+
{
253+
public Guid Id { get; set; }
254+
public Guid GroupOwnerId { get; set; }
255+
public RequestStatus HighestStatusAchieved { get; set; }
256+
// ... 30+ more columns ...
257+
}
258+
259+
public class Accommodation
260+
{
261+
public Guid Id { get; set; }
262+
public Guid TravelRequestId { get; set; }
263+
public BaseRequest? TravelRequest { get; set; } // Navigation to abstract type
264+
}
265+
266+
// ❌ INEFFICIENT - Loads all 34 columns from BaseRequest:
267+
filters.For<Accommodation>().Add(
268+
projection: _ => _, // Identity projection
269+
filter: (_, _, _, a) => a.TravelRequest?.GroupOwnerId == groupId);
270+
271+
// ✅ EFFICIENT - Only loads Id, GroupOwnerId, HighestStatusAchieved:
272+
filters.For<Accommodation>().Add(
273+
projection: a => new {
274+
a.Id,
275+
RequestOwnerId = a.TravelRequest?.GroupOwnerId,
276+
RequestStatus = a.TravelRequest?.HighestStatusAchieved
277+
},
278+
filter: (_, _, _, proj) => proj.RequestOwnerId == groupId);
279+
```
280+
281+
### Detection and Prevention
282+
283+
The library provides both compile-time and runtime detection:
284+
285+
**Compile-Time (Analyzer GQLEF007)**
286+
287+
The analyzer detects when filters use identity projections with abstract navigation access:
288+
289+
```csharp
290+
// This will show GQLEF007 error in the IDE:
291+
filters.For<Child>().Add(
292+
projection: _ => _,
293+
filter: (_, _, _, c) => c.AbstractParent?.Property == "value");
294+
```
295+
296+
The code fixer can automatically convert this to an explicit projection.
297+
298+
**Runtime (Exception)**
299+
300+
If the analyzer is bypassed, runtime validation will throw an exception when the filter is registered:
301+
302+
```csharp
303+
// Throws InvalidOperationException:
304+
// "Filter for 'Child' uses identity projection '_ => _' to access properties
305+
// of abstract navigation 'Parent' (BaseEntity). This forces Include() to load
306+
// all columns from BaseEntity. Extract only the required properties..."
307+
```
308+
309+
### Best Practices
310+
311+
1. **Always use explicit projections** when accessing abstract navigations
312+
2. **Extract only required properties** from the abstract navigation
313+
3. **Flatten navigation properties** in the projection (e.g., `ParentProperty` instead of nested access)
314+
4. **Update the filter** to use the flattened property names
315+
316+
### Concrete Navigations
317+
318+
This issue only affects **abstract** navigation types. Concrete navigation types work fine with identity projections:
319+
320+
```csharp
321+
// ✅ WORKS - ConcreteParent is not abstract:
322+
filters.For<Child>().Add(
323+
projection: _ => _,
324+
filter: (_, _, _, c) => c.ConcreteParent?.Property == "value");
325+
```
326+
327+
### See Also
328+
329+
* [GQLEF007 Diagnostic Documentation](/docs/analyzers/GQLEF007.md)
330+
* [Identity Projection Filters](#simplified-filter-api)

‎src/Directory.Build.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<Project>
33
<PropertyGroup>
44
<NoWarn>CS1591;NU5104;CS1573;CS9107;NU1608;NU1109</NoWarn>
5-
<Version>34.1.4</Version>
5+
<Version>34.1.5</Version>
66
<LangVersion>preview</LangVersion>
77
<AssemblyVersion>1.0.0</AssemblyVersion>
88
<PackageTags>EntityFrameworkCore, EntityFramework, GraphQL</PackageTags>

0 commit comments

Comments
 (0)