Skip to content

Commit 38f6b9c

Browse files
SimonCroppSimonCropp
authored
Add include fallback for abstract navigation filters (#1227)
* Add Include fallback for abstract navigation filters Fixes an issue where filter projections accessing properties through abstract navigation types could not be projected, resulting in null values. Now, when a filter requires properties from an abstract navigation, an Include is added as a fallback to ensure the navigation data is loaded. Adds integration tests to verify correct behavior for abstract and mixed navigation filters. * Update Directory.Build.props
1 parent e041bb6 commit 38f6b9c

12 files changed

Lines changed: 563 additions & 11 deletions

‎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.6</Version>
5+
<Version>34.1.7</Version>
66
<LangVersion>preview</LangVersion>
77
<AssemblyVersion>1.0.0</AssemblyVersion>
88
<PackageTags>EntityFrameworkCore, EntityFramework, GraphQL</PackageTags>

‎src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ FieldType BuildFirstField<TSource, TReturn>(
166166
// Get filter-required fields early so we can add filter-required navigations via Include
167167
var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties();
168168

169-
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context);
169+
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields);
170170
query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments);
171171

172172
// Apply column projection based on requested GraphQL fields

‎src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ FieldType BuildQueryField<TSource, TReturn>(
112112
// Get filter-required fields early so we can add filter-required navigations via Include
113113
var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties();
114114

115-
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context);
115+
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields);
116116
if (!omitQueryArguments)
117117
{
118118
query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments);

‎src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ ConnectionBuilder<TSource> AddQueryableConnection<TSource, TGraph, TReturn>(
158158
// Get filter-required fields early so we can add filter-required navigations via Include
159159
var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties();
160160

161-
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context);
161+
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields);
162162
query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments);
163163

164164
// Apply column projection based on requested GraphQL fields

‎src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ FieldType BuildSingleField<TSource, TReturn>(
166166
// Get filter-required fields early so we can add filter-required navigations via Include
167167
var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties();
168168

169-
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context);
169+
query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields);
170170
query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments);
171171

172172
// Apply column projection based on requested GraphQL fields

‎src/GraphQL.EntityFramework/IncludeAppender.cs‎

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,94 @@ public IQueryable<TItem> AddIncludesWithFilters<TItem>(
2525
IResolveFieldContext context,
2626
IReadOnlyDictionary<Type, IReadOnlySet<string>>? allFilterFields)
2727
where TItem : class =>
28-
AddIncludesWithFiltersAndDetectNavigations(query, context);
28+
AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields);
2929

3030
internal IQueryable<TItem> AddIncludesWithFiltersAndDetectNavigations<TItem>(
3131
IQueryable<TItem> query,
32-
IResolveFieldContext context)
32+
IResolveFieldContext context,
33+
IReadOnlyDictionary<Type, IReadOnlySet<string>>? allFilterFields = null)
3334
where TItem : class
3435
{
3536
// Add includes from GraphQL query
3637
query = AddIncludes(query, context);
3738

38-
// Note: Filter-required navigations are now handled entirely by the projection system
39-
// in TryGetProjectionExpressionWithFilters, which builds projections that include
40-
// filter-required fields. Abstract navigation access is prevented by FilterEntry validation.
41-
// No additional includes are needed here.
39+
// Add includes for filter-required navigations.
40+
// While projection handles most cases, abstract navigation types cannot be projected
41+
// (can't do "new AbstractType { ... }"). In those cases, Include is needed as fallback.
42+
if (allFilterFields is { Count: > 0 })
43+
{
44+
query = AddFilterNavigationIncludes(query, allFilterFields, typeof(TItem));
45+
}
46+
47+
return query;
48+
}
49+
50+
IQueryable<TItem> AddFilterNavigationIncludes<TItem>(
51+
IQueryable<TItem> query,
52+
IReadOnlyDictionary<Type, IReadOnlySet<string>> allFilterFields,
53+
Type entityType)
54+
where TItem : class
55+
{
56+
// Get filter fields for this entity type and its base types
57+
var relevantFilterFields = new List<string>();
58+
foreach (var (filterType, filterFields) in allFilterFields)
59+
{
60+
if (filterType.IsAssignableFrom(entityType))
61+
{
62+
relevantFilterFields.AddRange(filterFields);
63+
}
64+
}
65+
66+
if (relevantFilterFields.Count == 0)
67+
{
68+
return query;
69+
}
70+
71+
// Get navigation metadata for this entity type
72+
if (!navigations.TryGetValue(entityType, out var navigationProperties))
73+
{
74+
return query;
75+
}
76+
77+
// Extract unique navigation names from filter fields (paths like "TravelRequest.Status" -> "TravelRequest")
78+
var navigationNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
79+
foreach (var field in relevantFilterFields)
80+
{
81+
if (field.Contains('.'))
82+
{
83+
var navName = field[..field.IndexOf('.')];
84+
navigationNames.Add(navName);
85+
}
86+
}
87+
88+
// Add Include for each filter-required navigation that has an abstract type
89+
// (Non-abstract navigations can be handled by projection)
90+
foreach (var navName in navigationNames)
91+
{
92+
// Find navigation in metadata (case-insensitive)
93+
Navigation? navMetadata = null;
94+
string? actualNavName = null;
95+
foreach (var (key, value) in navigationProperties)
96+
{
97+
if (string.Equals(key, navName, StringComparison.OrdinalIgnoreCase))
98+
{
99+
navMetadata = value;
100+
actualNavName = key;
101+
break;
102+
}
103+
}
104+
105+
if (navMetadata == null || actualNavName == null)
106+
{
107+
continue;
108+
}
109+
110+
// Only add Include for abstract types - concrete types can use projection
111+
if (navMetadata.Type.IsAbstract)
112+
{
113+
query = query.Include(navMetadata.Name);
114+
}
115+
}
42116

43117
return query;
44118
}

‎src/Tests/IntegrationTests/IntegrationTests.Filter_combining_abstract_and_concrete_navigations.verified.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
target:
3+
{
4+
"data": {
5+
"derivedChildEntities": [
6+
{
7+
"id": "Guid_1",
8+
"property": "Child1"
9+
}
10+
]
11+
}
12+
},
13+
sql: {
14+
Text:
15+
select d.Id,
16+
d.ParentId,
17+
d.Property,
18+
d.TypedParentId,
19+
b.Id,
20+
b.Discriminator,
21+
b.Property,
22+
b.Status
23+
from DerivedChildEntities as d
24+
left outer join
25+
BaseEntities as b
26+
on d.ParentId = b.Id
27+
order by d.Property
28+
}
29+
}

‎src/Tests/IntegrationTests/IntegrationTests.Filter_with_abstract_navigation_should_correctly_exclude_non_matching.verified.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
target:
3+
{
4+
"data": {
5+
"derivedChildEntities": [
6+
{
7+
"id": "Guid_1",
8+
"property": "Child1-Approved"
9+
}
10+
]
11+
}
12+
},
13+
sql: {
14+
Text:
15+
select d.Id,
16+
d.ParentId,
17+
d.Property,
18+
d.TypedParentId,
19+
b.Id,
20+
b.Discriminator,
21+
b.Property,
22+
b.Status
23+
from DerivedChildEntities as d
24+
left outer join
25+
BaseEntities as b
26+
on d.ParentId = b.Id
27+
order by d.Property
28+
}
29+
}

‎src/Tests/IntegrationTests/IntegrationTests.Filter_with_abstract_navigation_should_use_include_fallback.verified.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
target:
3+
{
4+
"data": {
5+
"derivedChildEntities": [
6+
{
7+
"id": "Guid_1",
8+
"property": "Child1"
9+
}
10+
]
11+
}
12+
},
13+
sql: {
14+
Text:
15+
select d.Id,
16+
d.ParentId,
17+
d.Property,
18+
d.TypedParentId,
19+
b.Id,
20+
b.Discriminator,
21+
b.Property,
22+
b.Status
23+
from DerivedChildEntities as d
24+
left outer join
25+
BaseEntities as b
26+
on d.ParentId = b.Id
27+
order by d.Property
28+
}
29+
}

‎src/Tests/IntegrationTests/IntegrationTests.Filter_with_multiple_properties_from_abstract_navigation.verified.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
target:
3+
{
4+
"data": {
5+
"derivedChildEntities": [
6+
{
7+
"id": "Guid_1",
8+
"property": "Child1"
9+
}
10+
]
11+
}
12+
},
13+
sql: {
14+
Text:
15+
select d.Id,
16+
d.ParentId,
17+
d.Property,
18+
d.TypedParentId,
19+
b.Id,
20+
b.Discriminator,
21+
b.Property,
22+
b.Status
23+
from DerivedChildEntities as d
24+
left outer join
25+
BaseEntities as b
26+
on d.ParentId = b.Id
27+
order by d.Property
28+
}
29+
}

0 commit comments

Comments
 (0)