From d6a056265811235f578aea0d5d0ab8c62e8a4fd2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 10:24:37 +1100 Subject: [PATCH 01/14] Fix filter projection to avoid selecting all navigation fields --- .../GraphApi/EfGraphQLService_Single.cs | 7 +- .../IncludeAppender.cs | 27 ++-- .../SelectExpressionBuilder.cs | 2 + .../Graphs/Filtering/FilterBaseEntity.cs | 17 +++ .../Graphs/Filtering/FilterBaseGraphType.cs | 7 + .../Graphs/Filtering/FilterDerivedEntity.cs | 5 + .../Filtering/FilterDerivedGraphType.cs | 7 + .../Graphs/Filtering/FilterParentEntity.cs | 13 ++ .../Graphs/Filtering/FilterReferenceEntity.cs | 9 ++ .../Filtering/FilterReferenceGraphType.cs | 7 + .../IntegrationTests/IntegrationDbContext.cs | 11 ++ ..._select_all_navigation_fields.verified.txt | 21 +++ ..._should_not_select_all_fields.verified.txt | 23 +++ .../IntegrationTests.SchemaPrint.verified.txt | 53 +++++++ ...ationTests_named_type_filter_projection.cs | 137 ++++++++++++++++++ src/Tests/IntegrationTests/Query.cs | 16 ++ src/Tests/IntegrationTests/Schema.cs | 4 + 17 files changed, 350 insertions(+), 16 deletions(-) create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseEntity.cs create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseGraphType.cs create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedEntity.cs create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedGraphType.cs create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceEntity.cs create mode 100644 src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceGraphType.cs create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Filter_projection_should_not_select_all_navigation_fields.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_TPH_inheritance_should_not_select_all_fields.verified.txt diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs index 50410c42c..8377878f8 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs @@ -172,10 +172,11 @@ FieldType BuildSingleField( // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) - if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) + if (!typeof(TReturn).IsAbstract) { - // Merge filter-required scalar fields into projection (navigations are handled via Include above) + // Try to build projection even with abstract filter navigations + // The projection system may handle them (e.g., TPH inheritance) + // If projection build fails, we fall back to Include (which was already added above) if (includeAppender.TryGetProjectionExpressionWithFilters(context, allFilterFields, out var selectExpr)) { query = query.Select(selectExpr); diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index fafd11268..acfda2fcd 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -24,11 +24,8 @@ public IQueryable AddIncludesWithFilters( IQueryable query, IResolveFieldContext context, IReadOnlyDictionary>? allFilterFields) - where TItem : class - { - var (resultQuery, _) = AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - return resultQuery; - } + where TItem : class => + AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields).query; internal (IQueryable query, bool hasAbstractFilterNavigations) AddIncludesWithFiltersAndDetectNavigations( IQueryable query, @@ -89,17 +86,22 @@ public IQueryable AddIncludesWithFilters( } } - // Add Include for each filter-required navigation + // Only add Include for abstract filter-required navigations + // Non-abstract navigations are handled via the projection system in TryGetProjectionExpressionWithFilters + // which only selects the filter-required columns, not all columns var hasAbstractNavigations = false; foreach (var navName in filterNavigations) { if (navigationProperties.TryGetValue(navName, out var navMetadata)) { - query = query.Include(navName); if (navMetadata.Type.IsAbstract) { + // Abstract types cannot be projected (can't instantiate abstract class) + // Must use Include which loads all columns + query = query.Include(navName); hasAbstractNavigations = true; } + // Non-abstract navigations: projection system handles them efficiently } } @@ -273,17 +275,16 @@ FieldProjectionInfo MergeFilterFieldsIntoProjection( Projection = updatedProjection }; } - else if (!navType.IsAbstract) + else { - // Create navigation projection for filter-only navigations (unless abstract) - // Abstract navigations are handled via AddIncludesWithFilters() which uses EF Include + // Create navigation projection for filter-only navigations + // Note: For abstract types, we still create the projection here + // If SelectExpressionBuilder can't handle it, TryBuild will return false + // and the Include added by AddFilterIncludes will be used instead var navProjection = new FieldProjectionInfo(requiredProps, null, null, null); navProjection = MergeFilterFieldsIntoProjection(navProjection, allFilterFields, navType); mergedNavigations[navName] = new(navType, navMetadata.IsCollection, navProjection); } - // Note: Abstract filter-required navigations are handled via AddIncludesWithFilters() which uses EF Include. - // We skip creating projections for them to prevent SelectExpressionBuilder from failing, - // which would cause it to fall back to loading all entity properties. } // Recursively process existing navigations diff --git a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs index a06044228..1a582ae79 100644 --- a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs +++ b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs @@ -142,6 +142,7 @@ public static bool TryBuild( var navType = navProjection.EntityType; // Can't create NewExpression for abstract types + // (EF Core can't compile "new AbstractType { ... }") if (navType.IsAbstract) { return null; @@ -190,6 +191,7 @@ public static bool TryBuild( var navType = navProjection.EntityType; // Can't create NewExpression for abstract types + // (EF Core can't compile "new AbstractType { ... }") if (navType.IsAbstract) { return null; diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseEntity.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseEntity.cs new file mode 100644 index 000000000..093af5e1b --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseEntity.cs @@ -0,0 +1,17 @@ +public class FilterBaseEntity +{ + public Guid Id { get; set; } = Guid.NewGuid(); + public string? CommonProperty { get; set; } + + // Many additional properties to simulate a "rich" entity + public string? Field1 { get; set; } + public string? Field2 { get; set; } + public string? Field3 { get; set; } + public int Field4 { get; set; } + public int Field5 { get; set; } + public DateTime? Field6 { get; set; } + public DateTime? Field7 { get; set; } + public bool Field8 { get; set; } + public bool Field9 { get; set; } + public Guid? Field10 { get; set; } +} diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseGraphType.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseGraphType.cs new file mode 100644 index 000000000..995c6c32a --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterBaseGraphType.cs @@ -0,0 +1,7 @@ +public class FilterBaseGraphType : + EfObjectGraphType +{ + public FilterBaseGraphType(IEfGraphQLService graphQlService) : + base(graphQlService) => + AutoMap(); +} diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedEntity.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedEntity.cs new file mode 100644 index 000000000..5cb9e3560 --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedEntity.cs @@ -0,0 +1,5 @@ +public class FilterDerivedEntity : FilterBaseEntity +{ + public string? DerivedProperty { get; set; } + public IList References { get; set; } = []; +} diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedGraphType.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedGraphType.cs new file mode 100644 index 000000000..624233925 --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterDerivedGraphType.cs @@ -0,0 +1,7 @@ +public class FilterDerivedGraphType : + EfObjectGraphType +{ + public FilterDerivedGraphType(IEfGraphQLService graphQlService) : + base(graphQlService) => + AutoMap(); +} diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterParentEntity.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterParentEntity.cs index d0784eab5..942d3b3cb 100644 --- a/src/Tests/IntegrationTests/Graphs/Filtering/FilterParentEntity.cs +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterParentEntity.cs @@ -2,5 +2,18 @@ { public Guid Id { get; set; } = Guid.NewGuid(); public string? Property { get; set; } + + // Additional properties to simulate a "rich" entity like TravelRequest + public string? Field1 { get; set; } + public string? Field2 { get; set; } + public string? Field3 { get; set; } + public int Field4 { get; set; } + public int Field5 { get; set; } + public DateTime? Field6 { get; set; } + public DateTime? Field7 { get; set; } + public bool Field8 { get; set; } + public bool Field9 { get; set; } + public Guid? Field10 { get; set; } + public IList Children { get; set; } = []; } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceEntity.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceEntity.cs new file mode 100644 index 000000000..8ca02d6aa --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceEntity.cs @@ -0,0 +1,9 @@ +public class FilterReferenceEntity +{ + public Guid Id { get; set; } = Guid.NewGuid(); + public string? Property { get; set; } + + // Navigation to base type (like Accommodation -> TravelRequest) + public Guid BaseEntityId { get; set; } + public FilterBaseEntity? BaseEntity { get; set; } +} diff --git a/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceGraphType.cs b/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceGraphType.cs new file mode 100644 index 000000000..7471d493d --- /dev/null +++ b/src/Tests/IntegrationTests/Graphs/Filtering/FilterReferenceGraphType.cs @@ -0,0 +1,7 @@ +public class FilterReferenceGraphType : + EfObjectGraphType +{ + public FilterReferenceGraphType(IEfGraphQLService graphQlService) : + base(graphQlService) => + AutoMap(); +} diff --git a/src/Tests/IntegrationTests/IntegrationDbContext.cs b/src/Tests/IntegrationTests/IntegrationDbContext.cs index 72769a8c4..b369eb4b1 100644 --- a/src/Tests/IntegrationTests/IntegrationDbContext.cs +++ b/src/Tests/IntegrationTests/IntegrationDbContext.cs @@ -40,6 +40,9 @@ protected override void OnConfiguring(DbContextOptionsBuilder builder) => public DbSet FieldBuilderProjectionParentEntities { get; set; } = null!; public DbSet Departments { get; set; } = null!; public DbSet Employees { get; set; } = null!; + public DbSet FilterBaseEntities { get; set; } = null!; + public DbSet FilterDerivedEntities { get; set; } = null!; + public DbSet FilterReferenceEntities { get; set; } = null!; protected override void OnModelCreating(ModelBuilder modelBuilder) { @@ -112,5 +115,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .WithMany(_ => _.Employees) .HasForeignKey(_ => _.DepartmentId) .OnDelete(DeleteBehavior.Restrict); + + // Configure TPH inheritance for FilterBaseEntity -> FilterDerivedEntity + modelBuilder.Entity() + .OrderBy(_ => _.CommonProperty); + modelBuilder.Entity() + .HasBaseType(); + modelBuilder.Entity() + .OrderBy(_ => _.Property); } } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_should_not_select_all_navigation_fields.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_should_not_select_all_navigation_fields.verified.txt new file mode 100644 index 000000000..69d7c3504 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_should_not_select_all_navigation_fields.verified.txt @@ -0,0 +1,21 @@ +{ + target: +{ + "data": { + "filterChildEntity": { + "id": "Guid_1" + } + } +}, + sql: { + Text: +select top (2) f.Id, + case when f0.Id is null then cast (1 as bit) else cast (0 as bit) end, + f0.Property +from FilterChildEntities as f + left outer join + FilterParentEntities as f0 + on f.ParentId = f0.Id +where f.Id = 'Guid_1' + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_TPH_inheritance_should_not_select_all_fields.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_TPH_inheritance_should_not_select_all_fields.verified.txt new file mode 100644 index 000000000..923918f42 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_TPH_inheritance_should_not_select_all_fields.verified.txt @@ -0,0 +1,23 @@ +{ + target: +{ + "data": { + "filterReferenceEntity": { + "id": "Guid_1" + } + } +}, + sql: { + Text: +select top (2) f.Id, + f.BaseEntityId, + cast (0 as bit), + f0.CommonProperty, + f0.Id +from FilterReferenceEntities as f + inner join + FilterBaseEntities as f0 + on f.BaseEntityId = f0.Id +where f.Id = 'Guid_1' + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt index 2b18b8838..cc70bd8a5 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt @@ -151,6 +151,10 @@ fieldBuilderProjectionParents(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FieldBuilderProjectionParent!]! departments(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [Department!]! employees(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [Employee!]! + filterChildEntities(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FilterChild!]! + filterChildEntity(id: ID, ids: [ID!], where: [WhereExpression!]): FilterChild! + filterReferenceEntities(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FilterReference!]! + filterReferenceEntity(id: ID, ids: [ID!], where: [WhereExpression!]): FilterReference! } type CustomType { @@ -412,6 +416,16 @@ type FilterParent { orderBy: [OrderBy!], ids: [ID!]): FilterChildConnection! children(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FilterChild!]! + field1: String + field10: ID + field2: String + field3: String + field4: Int! + field5: Int! + field6: DateTime + field7: DateTime + field8: Boolean! + field9: Boolean! id: ID! property: String } @@ -710,6 +724,28 @@ type Employee { name: String } +type FilterReference { + baseEntity: FilterBase + baseEntityId: ID! + id: ID! + property: String +} + +type FilterBase { + commonProperty: String + field1: String + field10: ID + field2: String + field3: String + field4: Int! + field5: Int! + field6: DateTime + field7: DateTime + field8: Boolean! + field9: Boolean! + id: ID! +} + type Mutation { parentEntityMutation(id: ID, ids: [ID!], where: [WhereExpression!]): Parent! } @@ -745,3 +781,20 @@ type DerivedWithNavigation implements BaseEntity { property: String status: String } + +type FilterDerived { + references(id: ID, ids: [ID!], where: [WhereExpression!], orderBy: [OrderBy!], skip: Int, take: Int): [FilterReference!]! + commonProperty: String + derivedProperty: String + field1: String + field10: ID + field2: String + field3: String + field4: Int! + field5: Int! + field6: DateTime + field7: DateTime + field8: Boolean! + field9: Boolean! + id: ID! +} diff --git a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs index dfc8876fa..0323b58cc 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs @@ -102,7 +102,144 @@ public async Task Filter_projection_with_abstract_navigation_should_use_include( await RunQuery(database, query, null, filters, false, [parent, child1, child2]); } + /// + /// BUG TEST: Verifies that filter projections with navigation property access + /// should only SELECT the required fields from the navigation, not all fields. + /// + /// This reproduces the bug where EntityFilters cause Include() to be added, + /// which loads ALL columns from the navigation entity instead of just the + /// filter-required columns. + /// + /// Expected behavior: SQL should only select Parent.Property (required by filter) + /// and Parent.Id (primary key), not all the extra fields (Field1-Field10). + /// + /// Actual behavior (BUG): SQL includes ALL parent fields because Include() is used. + /// + [Fact] + public async Task Filter_projection_should_not_select_all_navigation_fields() + { + var query = + """ + { + filterChildEntity(id: "{id}") + { + id + } + } + """; + + var parent = new FilterParentEntity + { + Property = "AllowedParent", + Field1 = "Extra1", + Field2 = "Extra2", + Field3 = "Extra3", + Field4 = 100, + Field5 = 200, + Field6 = DateTime.UtcNow, + Field7 = DateTime.UtcNow, + Field8 = true, + Field9 = false, + Field10 = Guid.NewGuid() + }; + var child = new FilterChildEntity + { + Property = "Child1", + Parent = parent + }; + + query = query.Replace("{id}", child.Id.ToString()); + + var filters = new Filters(); + + // Filter only accesses Parent.Property - should NOT load all parent fields + filters.For().Add( + projection: c => new FilterChildProjection( + c.Parent != null ? c.Parent.Property : null, + c.Id), + filter: (_, _, _, projection) => projection.ParentProperty == "AllowedParent"); + + await using var database = await sqlInstance.Build(); + + // BUG: The SQL will include Parent.Field1, Parent.Field2, etc. even though + // only Parent.Property is needed by the filter + await RunQuery(database, query, null, filters, false, [parent, child]); + } + + /// + /// BUG TEST: Reproduces the bug where TPH inheritance + entity filters on base type + /// cause ALL columns to be selected from the navigation entity. + /// + /// This matches the MinistersApi scenario: + /// - FilterReferenceEntity (like Accommodation) has a navigation to FilterBaseEntity + /// - FilterBaseEntity (like BaseRequest) is an abstract base with TPH inheritance + /// - FilterDerivedEntity (like TravelRequest) inherits from FilterBaseEntity + /// - Both FilterReferenceEntity and FilterBaseEntity have entity filters + /// + /// Expected: SQL should only select BaseEntity.CommonProperty (required by filter) + /// Actual (BUG): SQL includes ALL fields (Field1-Field10) due to Include() being used + /// + [Fact] + public async Task Filter_projection_with_TPH_inheritance_should_not_select_all_fields() + { + var query = + """ + { + filterReferenceEntity(id: "{id}") + { + id + } + } + """; + + var derived = new FilterDerivedEntity + { + CommonProperty = "Allowed", + DerivedProperty = "Derived1", + Field1 = "Extra1", + Field2 = "Extra2", + Field3 = "Extra3", + Field4 = 100, + Field5 = 200, + Field6 = DateTime.UtcNow, + Field7 = DateTime.UtcNow, + Field8 = true, + Field9 = false, + Field10 = Guid.NewGuid() + }; + var reference = new FilterReferenceEntity + { + Property = "Reference1", + BaseEntity = derived + }; + + query = query.Replace("{id}", reference.Id.ToString()); + + var filters = new Filters(); + + // Filter on FilterReferenceEntity accesses BaseEntity.CommonProperty + filters.For().Add( + projection: r => new FilterReferenceProjection( + r.BaseEntity != null ? r.BaseEntity.CommonProperty : null, + r.Id), + filter: (_, _, _, proj) => proj.BaseCommonProperty == "Allowed"); + + // Filter on base type (like BaseRequest in MinistersApi) + filters.For().Add( + projection: b => new FilterBaseProjection(b.CommonProperty, b.Id), + filter: (_, _, _, proj) => proj.CommonProperty != null); + + await using var database = await sqlInstance.Build(); + + // BUG: The SQL will include a subquery selecting ALL fields from FilterBaseEntity + // (Field1-Field10) even though only CommonProperty is needed + await RunQuery(database, query, null, filters, false, [derived, reference]); + } + // ReSharper disable NotAccessedPositionalProperty.Local record ChildFilterProjection(Guid? ParentId, string? ParentProperty, Guid ChildId); record AbstractNavFilterProjection(Guid? ParentId, string? ParentProperty, Guid ChildId); + record FilterChildProjection(string? ParentProperty, Guid ChildId); + record FilterReferenceProjection(string? BaseCommonProperty, Guid Id); + record FilterBaseProjection(string? CommonProperty, Guid Id); } diff --git a/src/Tests/IntegrationTests/Query.cs b/src/Tests/IntegrationTests/Query.cs index 9957846e1..2000c0e5f 100644 --- a/src/Tests/IntegrationTests/Query.cs +++ b/src/Tests/IntegrationTests/Query.cs @@ -336,5 +336,21 @@ public Query(IEfGraphQLService efGraphQlService) AddQueryField( name: "employees", resolve: _ => _.DbContext.Employees); + + AddQueryField( + name: "filterChildEntities", + resolve: _ => _.DbContext.FilterChildEntities); + + AddSingleField( + name: "filterChildEntity", + resolve: _ => _.DbContext.FilterChildEntities); + + AddQueryField( + name: "filterReferenceEntities", + resolve: _ => _.DbContext.FilterReferenceEntities); + + AddSingleField( + name: "filterReferenceEntity", + resolve: _ => _.DbContext.FilterReferenceEntities); } } diff --git a/src/Tests/IntegrationTests/Schema.cs b/src/Tests/IntegrationTests/Schema.cs index 216da019e..0988ca31a 100644 --- a/src/Tests/IntegrationTests/Schema.cs +++ b/src/Tests/IntegrationTests/Schema.cs @@ -39,9 +39,13 @@ public Schema(IServiceProvider resolver) : RegisterTypeMapping(typeof(FieldBuilderProjectionParentEntity), typeof(FieldBuilderProjectionParentGraphType)); RegisterTypeMapping(typeof(DepartmentEntity), typeof(DepartmentGraphType)); RegisterTypeMapping(typeof(EmployeeEntity), typeof(EmployeeGraphType)); + RegisterTypeMapping(typeof(FilterReferenceEntity), typeof(FilterReferenceGraphType)); + RegisterTypeMapping(typeof(FilterBaseEntity), typeof(FilterBaseGraphType)); + RegisterTypeMapping(typeof(FilterDerivedEntity), typeof(FilterDerivedGraphType)); Query = (Query)resolver.GetService(typeof(Query))!; Mutation = (Mutation)resolver.GetService(typeof(Mutation))!; RegisterType(typeof(DerivedGraphType)); RegisterType(typeof(DerivedWithNavigationGraphType)); + RegisterType(typeof(FilterDerivedGraphType)); } } \ No newline at end of file From 468bbfca3c6d504764098f3f4c2a1b10fae77d4e Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 10:38:23 +1100 Subject: [PATCH 02/14] . --- .../GraphApi/EfGraphQLService_First.cs | 7 ++++--- .../GraphApi/EfGraphQLService_Queryable.cs | 7 ++++--- .../GraphApi/EfGraphQLService_QueryableConnection.cs | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs index 9d7182040..e798daffc 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs @@ -172,10 +172,11 @@ FieldType BuildFirstField( // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) - if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) + if (!typeof(TReturn).IsAbstract) { - // Merge filter-required scalar fields into projection (navigations are handled via Include above) + // Try to build projection even with abstract filter navigations + // The projection system may handle them (e.g., TPH inheritance) + // If projection build fails, we fall back to Include (which was already added above) if (includeAppender.TryGetProjectionExpressionWithFilters(context, allFilterFields, out var selectExpr)) { query = query.Select(selectExpr); diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs index ce6753e50..719977ab9 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs @@ -121,10 +121,11 @@ FieldType BuildQueryField( // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) - if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) + if (!typeof(TReturn).IsAbstract) { - // Merge filter-required scalar fields into projection (navigations are handled via Include above) + // Try to build projection even with abstract filter navigations + // The projection system may handle them (e.g., TPH inheritance) + // If projection build fails, we fall back to Include (which was already added above) if (includeAppender.TryGetProjectionExpressionWithFilters(context, allFilterFields, out var selectExpr)) { query = query.Select(selectExpr); diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs index c00c83e2f..924839678 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs @@ -164,10 +164,11 @@ ConnectionBuilder AddQueryableConnection( // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) - if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) + if (!typeof(TReturn).IsAbstract) { - // Merge filter-required scalar fields into projection (navigations are handled via Include above) + // Try to build projection even with abstract filter navigations + // The projection system may handle them (e.g., TPH inheritance) + // If projection build fails, we fall back to Include (which was already added above) if (includeAppender.TryGetProjectionExpressionWithFilters(context, allFilterFields, out var selectExpr)) { query = query.Select(selectExpr); From 6f91b238910065694e38e6de2674764512e77bd4 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 12:58:06 +1100 Subject: [PATCH 03/14] . --- docs/analyzers/GQLEF007.md | 143 ++++++++ docs/filters.md | 99 +++++- docs/mdsource/filters.source.md | 99 +++++- ...avigationProjectionCodeFixProviderTests.cs | 162 +++++++++ .../FilterIdentityProjectionAnalyzerTests.cs | 201 ++++++++++- .../AnalyzerReleases.Unshipped.md | 1 + .../DiagnosticDescriptors.cs | 10 + .../FilterIdentityProjectionAnalyzer.cs | 107 +++++- ...ractNavigationProjectionCodeFixProvider.cs | 319 ++++++++++++++++++ .../Filters/FilterEntry.cs | 62 ++++ ...grationTests_abstract_filter_validation.cs | 125 +++++++ .../UnitTests/FilterEntryValidationTests.cs | 97 ++++++ 12 files changed, 1415 insertions(+), 10 deletions(-) create mode 100644 docs/analyzers/GQLEF007.md create mode 100644 src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs create mode 100644 src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs create mode 100644 src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs create mode 100644 src/Tests/UnitTests/FilterEntryValidationTests.cs diff --git a/docs/analyzers/GQLEF007.md b/docs/analyzers/GQLEF007.md new file mode 100644 index 000000000..1b96314d7 --- /dev/null +++ b/docs/analyzers/GQLEF007.md @@ -0,0 +1,143 @@ +# GQLEF007: Identity projection with abstract navigation access + +## Cause + +A filter uses identity projection (`_ => _` or 4-parameter syntax) but the filter lambda accesses properties through an abstract navigation type. + +## Problem + +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. + +This creates a significant performance problem: +- The filter only needs 1-2 fields from the navigation +- But EF Core loads all 30+ fields because of the Include fallback +- Query performance degrades and memory usage increases unnecessarily + +## Rule Details + +This diagnostic is reported when: +1. A filter uses identity projection (explicit `_ => _` or implicit via 4-parameter syntax) +2. The filter lambda accesses a property through a navigation property +3. That navigation property's type is abstract (has the `abstract` keyword) + +### Examples of violations + +```csharp +// ❌ Identity projection with abstract navigation access +public abstract class BaseEntity +{ + public string Property { get; set; } +} + +public class Child +{ + public Guid Id { get; set; } + public BaseEntity Parent { get; set; } +} + +// This triggers GQLEF007: +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, c) => c.Parent.Property == "test"); + +// This also triggers GQLEF007 (4-parameter syntax uses identity projection): +filters.For().Add( + filter: (_, _, _, c) => c.Parent.Property == "test"); +``` + +### Examples of correct code + +```csharp +// ✅ Explicit projection extracting required properties +filters.For().Add( + projection: c => new { c.Id, ParentProperty = c.Parent.Property }, + filter: (_, _, _, proj) => proj.ParentProperty == "test"); + +// ✅ Concrete (non-abstract) navigation is fine with identity projection +public class ConcreteParent // Not abstract +{ + public string Property { get; set; } +} + +public class Child +{ + public ConcreteParent Parent { get; set; } +} + +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent.Property == "test"); // OK - ConcreteParent is not abstract +``` + +## Solution + +Convert the identity projection to an explicit projection that extracts only the required properties from the abstract navigation. + +### Manual Fix + +1. Identify all properties accessed through the abstract navigation +2. Create an explicit projection that extracts those properties with flattened names +3. Update the filter to use the projected property names +4. Rename the filter parameter to `proj` for clarity + +**Before:** +```csharp +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent.Property == "test"); +``` + +**After:** +```csharp +filters.For().Add( + projection: c => new { c.Id, ParentProperty = c.Parent.Property }, + filter: (_, _, _, proj) => proj.ParentProperty == "test"); +``` + +### Automatic Fix + +The code fixer can automatically perform this transformation: +1. Place cursor on the diagnostic +2. Press `Ctrl+.` (or `Cmd+.` on Mac) +3. Select "Convert to explicit projection" + +The fixer will: +- Extract all accessed navigation properties +- Create an anonymous type projection with flattened property names +- Update the filter lambda to use the new property names +- Rename the filter parameter from entity name to `proj` + +## Performance Impact + +Using explicit projections instead of identity projections with abstract navigations can significantly improve performance: + +**Identity Projection (Inefficient):** +```sql +-- Loads all 34 columns from BaseRequest table +SELECT a.*, br.* +FROM Accommodation a +LEFT JOIN BaseRequest br ON a.TravelRequestId = br.Id +``` + +**Explicit Projection (Efficient):** +```sql +-- Loads only required columns +SELECT a.Id, br.GroupOwnerId, br.HighestStatusAchieved +FROM Accommodation a +LEFT JOIN BaseRequest br ON a.TravelRequestId = br.Id +``` + +## When to Suppress + +**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. + +## Related Rules + +- **GQLEF004** - Suggests using 4-parameter filter syntax for identity projections with key-only access +- **GQLEF005** - Prevents accessing non-key properties with 4-parameter filter syntax +- **GQLEF006** - Prevents accessing non-key properties with explicit identity projection + +## See Also + +- [Filter Projections Documentation](/docs/filters.md#abstract-type-navigations) +- [Understanding EF Core Projections](https://learn.microsoft.com/en-us/ef/core/querying/how-query-works) diff --git a/docs/filters.md b/docs/filters.md index da4579ebe..cc0306ece 100644 --- a/docs/filters.md +++ b/docs/filters.md @@ -258,7 +258,7 @@ EfGraphQLConventions.RegisterInContainer( Common nullable patterns: * **Has value check**: `quantity.HasValue && quantity.Value > 0` -* **Null check**: `!quantity.HasValue` +* **Null check**: `?quantity.HasValue` * **Exact match**: `isApproved == true` (not null or false) @@ -522,12 +522,12 @@ The simplified API is syntactic sugar for the identity projection pattern: ```csharp // Simplified API filters.For().Add( - filter: (_, _, _, a) => a.Id != Guid.Empty); + filter: (_, _, _, a) => a.Id ?= Guid.Empty); // Equivalent full API filters.For().Add( projection: _ => _, // Identity projection - filter: (_, _, _, a) => a.Id != Guid.Empty); + filter: (_, _, _, a) => a.Id ?= Guid.Empty); ``` ### Analyzer Support @@ -556,3 +556,96 @@ filters.For().Add( ``` The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior + +## Abstract Type Navigations + +When working with filters that access properties through abstract navigation properties, special care must be taken to avoid performance issues. + +### The Problem + +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. + +### Example + +```csharp +// Given: +public abstract class BaseRequest // Abstract class with many fields +{ + public Guid Id { get; set; } + public Guid GroupOwnerId { get; set; } + public RequestStatus HighestStatusAchieved { get; set; } + // ... 30+ more columns ... +} + +public class Accommodation +{ + public Guid Id { get; set; } + public Guid TravelRequestId { get; set; } + public BaseRequest? TravelRequest { get; set; } // Navigation to abstract type +} + +// ❌ INEFFICIENT - Loads all 34 columns from BaseRequest: +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.TravelRequest?.GroupOwnerId == groupId); + +// ✅ EFFICIENT - Only loads Id, GroupOwnerId, HighestStatusAchieved: +filters.For().Add( + projection: a => new { + a.Id, + RequestOwnerId = a.TravelRequest?.GroupOwnerId, + RequestStatus = a.TravelRequest?.HighestStatusAchieved + }, + filter: (_, _, _, proj) => proj.RequestOwnerId == groupId); +``` + +### Detection and Prevention + +The library provides both compile-time and runtime detection: + +**Compile-Time (Analyzer GQLEF007)** + +The analyzer detects when filters use identity projections with abstract navigation access: + +```csharp +// This will show GQLEF007 error in the IDE: +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.AbstractParent?.Property == "value"); +``` + +The code fixer can automatically convert this to an explicit projection. + +**Runtime (Exception)** + +If the analyzer is bypassed, runtime validation will throw an exception when the filter is registered: + +```csharp +// Throws InvalidOperationException: +// "Filter for 'Child' uses identity projection '_ => _' to access properties +// of abstract navigation 'Parent' (BaseEntity). This forces Include() to load +// all columns from BaseEntity. Extract only the required properties..." +``` + +### Best Practices + +1. **Always use explicit projections** when accessing abstract navigations +2. **Extract only required properties** from the abstract navigation +3. **Flatten navigation properties** in the projection (e.g., `ParentProperty` instead of nested access) +4. **Update the filter** to use the flattened property names + +### Concrete Navigations + +This issue only affects **abstract** navigation types. Concrete navigation types work fine with identity projections: + +```csharp +// ✅ WORKS - ConcreteParent is not abstract: +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.ConcreteParent?.Property == "value"); +``` + +### See Also + +* [GQLEF007 Diagnostic Documentation](/docs/analyzers/GQLEF007.md) +* [Identity Projection Filters](#simplified-filter-api) diff --git a/docs/mdsource/filters.source.md b/docs/mdsource/filters.source.md index ee91cd1c7..42d5cdbbe 100644 --- a/docs/mdsource/filters.source.md +++ b/docs/mdsource/filters.source.md @@ -86,7 +86,7 @@ snippet: nullable-value-type-projections Common nullable patterns: * **Has value check**: `quantity.HasValue && quantity.Value > 0` -* **Null check**: `!quantity.HasValue` +* **Null check**: `?quantity.HasValue` * **Exact match**: `isApproved == true` (not null or false) @@ -201,12 +201,12 @@ The simplified API is syntactic sugar for the identity projection pattern: ```csharp // Simplified API filters.For().Add( - filter: (_, _, _, a) => a.Id != Guid.Empty); + filter: (_, _, _, a) => a.Id ?= Guid.Empty); // Equivalent full API filters.For().Add( projection: _ => _, // Identity projection - filter: (_, _, _, a) => a.Id != Guid.Empty); + filter: (_, _, _, a) => a.Id ?= Guid.Empty); ``` ### Analyzer Support @@ -235,3 +235,96 @@ filters.For().Add( ``` The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior + +## Abstract Type Navigations + +When working with filters that access properties through abstract navigation properties, special care must be taken to avoid performance issues. + +### The Problem + +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. + +### Example + +```csharp +// Given: +public abstract class BaseRequest // Abstract class with many fields +{ + public Guid Id { get; set; } + public Guid GroupOwnerId { get; set; } + public RequestStatus HighestStatusAchieved { get; set; } + // ... 30+ more columns ... +} + +public class Accommodation +{ + public Guid Id { get; set; } + public Guid TravelRequestId { get; set; } + public BaseRequest? TravelRequest { get; set; } // Navigation to abstract type +} + +// ❌ INEFFICIENT - Loads all 34 columns from BaseRequest: +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.TravelRequest?.GroupOwnerId == groupId); + +// ✅ EFFICIENT - Only loads Id, GroupOwnerId, HighestStatusAchieved: +filters.For().Add( + projection: a => new { + a.Id, + RequestOwnerId = a.TravelRequest?.GroupOwnerId, + RequestStatus = a.TravelRequest?.HighestStatusAchieved + }, + filter: (_, _, _, proj) => proj.RequestOwnerId == groupId); +``` + +### Detection and Prevention + +The library provides both compile-time and runtime detection: + +**Compile-Time (Analyzer GQLEF007)** + +The analyzer detects when filters use identity projections with abstract navigation access: + +```csharp +// This will show GQLEF007 error in the IDE: +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.AbstractParent?.Property == "value"); +``` + +The code fixer can automatically convert this to an explicit projection. + +**Runtime (Exception)** + +If the analyzer is bypassed, runtime validation will throw an exception when the filter is registered: + +```csharp +// Throws InvalidOperationException: +// "Filter for 'Child' uses identity projection '_ => _' to access properties +// of abstract navigation 'Parent' (BaseEntity). This forces Include() to load +// all columns from BaseEntity. Extract only the required properties..." +``` + +### Best Practices + +1. **Always use explicit projections** when accessing abstract navigations +2. **Extract only required properties** from the abstract navigation +3. **Flatten navigation properties** in the projection (e.g., `ParentProperty` instead of nested access) +4. **Update the filter** to use the flattened property names + +### Concrete Navigations + +This issue only affects **abstract** navigation types. Concrete navigation types work fine with identity projections: + +```csharp +// ✅ WORKS - ConcreteParent is not abstract: +filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.ConcreteParent?.Property == "value"); +``` + +### See Also + +* [GQLEF007 Diagnostic Documentation](/docs/analyzers/GQLEF007.md) +* [Identity Projection Filters](#simplified-filter-api) diff --git a/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs new file mode 100644 index 000000000..723822b8b --- /dev/null +++ b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs @@ -0,0 +1,162 @@ +public class AbstractNavigationProjectionCodeFixProviderTests +{ + [Fact] + public async Task ConvertsIdentityProjectionToExplicitProjection() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public abstract class BaseEntity + { + public Guid Id { get; set; } + public string Property { get; set; } = ""; + } + + public class ChildEntity + { + public Guid Id { get; set; } + public BaseEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent!.Property == "test"); + } + } + """; + + var result = await ApplyCodeFixAsync(source); + await Verify(result); + } + + static async Task ApplyCodeFixAsync(string source) + { + var document = CreateDocument(source); + var compilation = await document.Project.GetCompilationAsync(); + if (compilation == null) + { + throw new("Compilation failed"); + } + + // Get diagnostics from analyzer + var analyzer = new FilterIdentityProjectionAnalyzer(); + var compilationWithAnalyzers = compilation.WithAnalyzers([analyzer]); + var allDiagnostics = await compilationWithAnalyzers.GetAllDiagnosticsAsync(); + + var diagnostic = allDiagnostics.FirstOrDefault(_ => _.Id == "GQLEF007"); + if (diagnostic == null) + { + throw new("No GQLEF007 diagnostic found"); + } + + // Apply code fix + var codeFixProvider = new AbstractNavigationProjectionCodeFixProvider(); + var actions = new List(); + + var context = new CodeFixContext( + document, + diagnostic, + (action, _) => actions.Add(action), + default); + + await codeFixProvider.RegisterCodeFixesAsync(context); + + if (actions.Count == 0) + { + throw new($"No code fixes were registered. Diagnostic: {diagnostic}"); + } + + var operations = await actions[0].GetOperationsAsync(default); + if (operations.IsEmpty) + { + throw new("Code action returned no operations"); + } + + var operation = operations.OfType().FirstOrDefault(); + if (operation == null) + { + throw new($"No ApplyChangesOperation found"); + } + + var changedSolution = operation.ChangedSolution; + var changedDocument = changedSolution.GetDocument(document.Id); + if (changedDocument == null) + { + throw new("Failed to get changed document"); + } + + var root = await changedDocument.GetSyntaxRootAsync(); + return root?.ToFullString() ?? ""; + } + + static Document CreateDocument(string source) + { + var projectId = ProjectId.CreateNewId(); + var documentId = DocumentId.CreateNewId(projectId); + + var references = new List + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + MetadataReference.CreateFromFile(typeof(DbContext).Assembly.Location), + MetadataReference.CreateFromFile(typeof(Filters<>).Assembly.Location), + }; + + var requiredAssemblies = new[] + { + typeof(Guid).Assembly, + Assembly.Load("System.Runtime"), + typeof(IQueryable<>).Assembly, + Assembly.Load("System.Security.Claims"), + }; + + foreach (var assembly in requiredAssemblies) + { + if (!string.IsNullOrEmpty(assembly.Location)) + { + references.Add(MetadataReference.CreateFromFile(assembly.Location)); + } + } + + foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) + { + if (!assembly.IsDynamic && + !string.IsNullOrEmpty(assembly.Location) && + references.All(_ => _.Display != assembly.Location)) + { + var name = assembly.GetName().Name ?? ""; + if ((!name.StartsWith("System.") && + !name.StartsWith("Microsoft.")) || + name.Contains("xunit", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + try + { + references.Add(MetadataReference.CreateFromFile(assembly.Location)); + } + catch + { + // Ignore assemblies that can't be referenced + } + } + } + + var solution = new AdhocWorkspace() + .CurrentSolution + .AddProject(projectId, "TestProject", "TestProject", LanguageNames.CSharp) + .WithProjectCompilationOptions(projectId, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .AddMetadataReferences(projectId, references) + .AddDocument(documentId, "TestDocument.cs", source); + + return solution.GetDocument(documentId)!; + } +} diff --git a/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs b/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs index 0d3ca3a01..3524dfcd0 100644 --- a/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs +++ b/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs @@ -602,9 +602,206 @@ static async Task GetDiagnosticsAsync(string source) throw new($"Compilation errors:\n{errorMessages}"); } - // Filter to only GQLEF004, GQLEF005, and GQLEF006 diagnostics + // Filter to only GQLEF004, GQLEF005, GQLEF006, and GQLEF007 diagnostics return allDiagnostics - .Where(_ => _.Id == "GQLEF004" || _.Id == "GQLEF005" || _.Id == "GQLEF006") + .Where(_ => _.Id == "GQLEF004" || _.Id == "GQLEF005" || _.Id == "GQLEF006" || _.Id == "GQLEF007") .ToArray(); } + + // GQLEF007 Tests - Identity projection with abstract navigation access + + [Fact] + public async Task GQLEF007_Reports_IdentityProjection_WithAbstractNavigationAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public abstract class BaseEntity + { + public Guid Id { get; set; } + public string Property { get; set; } = ""; + } + + public class DerivedEntity : BaseEntity { } + + public class ChildEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + public BaseEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent!.Property == "test"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF007", diagnostics[0].Id); + Assert.Contains("Parent", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task GQLEF007_Reports_FourParamFilter_WithAbstractNavigationAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public abstract class BaseEntity + { + public Guid Id { get; set; } + public string Status { get; set; } = ""; + } + + public class ChildEntity + { + public Guid Id { get; set; } + public BaseEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + filter: (_, _, _, c) => c.Parent!.Status == "Active"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF007", diagnostics[0].Id); + } + + [Fact] + public async Task GQLEF007_NoReport_ConcreteNavigationAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class ConcreteParent + { + public Guid Id { get; set; } + public string Property { get; set; } = ""; + } + + public class ChildEntity + { + public Guid Id { get; set; } + public ConcreteParent? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent!.Property == "test"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + // Should report GQLEF006 (non-key property access) but not GQLEF007 + Assert.DoesNotContain(diagnostics, d => d.Id == "GQLEF007"); + } + + [Fact] + public async Task GQLEF007_NoReport_ExplicitProjection_WithAbstractNavigation() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public abstract class BaseEntity + { + public Guid Id { get; set; } + public string Property { get; set; } = ""; + } + + public class ChildEntity + { + public Guid Id { get; set; } + public BaseEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: c => new { c.Id, ParentProp = c.Parent!.Property }, + filter: (_, _, _, proj) => proj.ParentProp == "test"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + // Explicit projection - analyzer doesn't catch this (runtime does) + Assert.Empty(diagnostics); + } + + [Fact] + public async Task GQLEF007_Reports_NestedAbstractNavigationAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + using System.Collections.Generic; + + public abstract class BaseEntity + { + public Guid Id { get; set; } + public List Children { get; set; } = new(); + } + + public class ChildEntity + { + public Guid Id { get; set; } + public BaseEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, c) => c.Parent!.Children.Count > 0); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF007", diagnostics[0].Id); + Assert.Contains("Parent", diagnostics[0].GetMessage()); + } } diff --git a/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md b/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md index 4c5e96d78..2ddac229b 100644 --- a/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md @@ -10,3 +10,4 @@ GQLEF003 | Usage | Error | Identity projection is not allowed in projection-base GQLEF004 | Usage | Info | Suggests using 4-parameter filter syntax instead of explicit identity projection GQLEF005 | Usage | Error | Prevents accessing non-key properties with 4-parameter filter syntax GQLEF006 | Usage | Error | Prevents accessing non-key properties with explicit identity projection +GQLEF007 | Usage | Error | Identity projection with abstract navigation access diff --git a/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs b/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs index bac59381c..e0c24a138 100644 --- a/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs +++ b/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs @@ -49,4 +49,14 @@ public static class DiagnosticDescriptors isEnabledByDefault: true, description: "When using identity projection '_ => _', only primary keys and foreign keys are guaranteed to be loaded. Either project specific properties needed or use the simplified API if you only need key properties.", helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#identity-projection-filters"); + + public static readonly DiagnosticDescriptor GQLEF007 = new( + id: "GQLEF007", + title: "Identity projection with abstract navigation access", + messageFormat: "Filter for '{0}' uses identity projection but accesses abstract navigation '{1}'. This forces Include() to load all columns. Use explicit projection to load only required properties.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "Abstract type navigations cannot use SQL projections and fall back to Include() which loads all columns. Use explicit projection to extract only required properties from abstract navigations.", + helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#abstract-navigation-projections"); } diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index bf32b23e5..165348906 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -7,7 +7,8 @@ public class FilterIdentityProjectionAnalyzer : DiagnosticAnalyzer [ DiagnosticDescriptors.GQLEF004, DiagnosticDescriptors.GQLEF005, - DiagnosticDescriptors.GQLEF006 + DiagnosticDescriptors.GQLEF006, + DiagnosticDescriptors.GQLEF007 ]; public override void Initialize(AnalysisContext context) @@ -45,6 +46,21 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) } // Identity projection detected + // First check for abstract navigation access (GQLEF007) + var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel, entityType); + if (abstractNav != null) + { + // Error: Identity projection with abstract navigation access + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF007, + invocation.GetLocation(), + entityType ?? "Entity", + abstractNav); + context.ReportDiagnostic(diagnostic); + return; + } + + // Then check for non-key property access (GQLEF006) var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda, context.SemanticModel); if (nonKeyProperty != null) { @@ -75,7 +91,20 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) if (is4ParamFilter) { - // Validate that filter only accesses PK/FK properties + // First check for abstract navigation access (GQLEF007) + var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel, entityType); + if (abstractNav != null) + { + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF007, + invocation.GetLocation(), + entityType ?? "Entity", + abstractNav); + context.ReportDiagnostic(diagnostic); + return; + } + + // Then validate that filter only accesses PK/FK properties var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda, context.SemanticModel); if (nonKeyProperty != null) { @@ -409,4 +438,78 @@ static int GetParameterIndex(ImmutableArray parameters, IParam } return -1; } + + static string? FindAbstractNavigationAccess( + LambdaExpressionSyntax lambda, + SemanticModel semanticModel, + string? entityTypeName) + { + var body = lambda.Body; + + // Get the filter lambda parameter name (last parameter in the filter signature) + string? filterParameterName = null; + if (lambda is SimpleLambdaExpressionSyntax simpleLambda) + { + filterParameterName = simpleLambda.Parameter.Identifier.Text; + } + else if (lambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + { + var parameterCount = parenthesizedLambda.ParameterList.Parameters.Count; + if (parameterCount > 0) + { + filterParameterName = parenthesizedLambda.ParameterList.Parameters[parameterCount - 1].Identifier.Text; + } + } + + if (filterParameterName == null) + { + return null; + } + + // Find all member access expressions in the lambda + var memberAccesses = body.DescendantNodesAndSelf() + .OfType(); + + foreach (var memberAccess in memberAccesses) + { + // Check for navigation property access pattern: e.Parent.Property + // We want to detect when `e.Parent` is a navigation to an abstract type + if (memberAccess.Expression is MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax identifier } nestedAccess && + identifier.Identifier.Text == filterParameterName) + { + // This is e.Parent.Property - check if Parent is abstract + var navigationSymbol = semanticModel.GetSymbolInfo(nestedAccess).Symbol as IPropertySymbol; + if (navigationSymbol != null) + { + var navType = navigationSymbol.Type; + if (navType.IsAbstract) + { + return navigationSymbol.Name; + } + } + } + // Also check direct navigation access: e.Parent (without further property access) + else if (memberAccess.Expression is IdentifierNameSyntax directIdentifier && + directIdentifier.Identifier.Text == filterParameterName) + { + var propertySymbol = semanticModel.GetSymbolInfo(memberAccess).Symbol as IPropertySymbol; + if (propertySymbol != null) + { + var propType = propertySymbol.Type; + // Check if this is a reference type (not a primitive) and is abstract + // This indicates it's likely a navigation property to an abstract entity + if (propType is { IsAbstract: true, TypeKind: TypeKind.Class }) + { + // But make sure it's not a primitive key property or known scalar + if (!IsPrimaryKeyProperty(propertySymbol) && !IsForeignKeyProperty(propertySymbol)) + { + return propertySymbol.Name; + } + } + } + } + } + + return null; + } } diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs new file mode 100644 index 000000000..fbe73790a --- /dev/null +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -0,0 +1,319 @@ +using Microsoft.CodeAnalysis.CSharp; + +namespace GraphQL.EntityFramework.CodeFixes; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AbstractNavigationProjectionCodeFixProvider))] +[Shared] +public class AbstractNavigationProjectionCodeFixProvider : CodeFixProvider +{ + const string title = "Convert to explicit projection"; + + public override ImmutableArray FixableDiagnosticIds { get; } = + [DiagnosticDescriptors.GQLEF007.Id]; + + public override FixAllProvider GetFixAllProvider() => + WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken); + if (root == null) + { + return; + } + + var diagnostic = context.Diagnostics[0]; + var diagnosticSpan = diagnostic.Location.SourceSpan; + + var node = root.FindNode(diagnosticSpan); + var invocation = node as InvocationExpressionSyntax ?? node.FirstAncestorOrSelf(); + + if (invocation == null) + { + return; + } + + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken); + if (semanticModel == null) + { + return; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: title, + createChangedDocument: c => ConvertToExplicitProjectionAsync(context.Document, invocation, semanticModel, c), + equivalenceKey: title), + diagnostic); + } + + static async Task ConvertToExplicitProjectionAsync( + Document document, + InvocationExpressionSyntax invocation, + SemanticModel semanticModel, + Cancel cancel) + { + var root = await document.GetSyntaxRootAsync(cancel); + if (root == null) + { + return document; + } + + // Find projection and filter arguments + var arguments = invocation.ArgumentList.Arguments; + LambdaExpressionSyntax? projectionLambda = null; + LambdaExpressionSyntax? filterLambda = null; + var projectionArgumentIndex = -1; + var filterArgumentIndex = -1; + + for (var i = 0; i < arguments.Count; i++) + { + var arg = arguments[i]; + if (arg.NameColon?.Name.Identifier.Text == "projection" && + arg.Expression is LambdaExpressionSyntax projLambda) + { + projectionLambda = projLambda; + projectionArgumentIndex = i; + } + else if (arg.NameColon?.Name.Identifier.Text == "filter" && + arg.Expression is LambdaExpressionSyntax filtLambda) + { + filterLambda = filtLambda; + filterArgumentIndex = i; + } + } + + // If no explicit projection argument, this is 4-parameter syntax + var is4ParamSyntax = projectionArgumentIndex == -1; + + if (filterLambda == null) + { + return document; + } + + // Extract accessed properties from filter + var accessedProperties = ExtractAccessedProperties(filterLambda, semanticModel); + if (accessedProperties.Count == 0) + { + return document; + } + + // Get the entity parameter name from the filter (last parameter) + string entityParamName; + if (filterLambda is SimpleLambdaExpressionSyntax simpleLambda) + { + entityParamName = simpleLambda.Parameter.Identifier.Text; + } + else if (filterLambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + { + var paramCount = parenthesizedLambda.ParameterList.Parameters.Count; + entityParamName = paramCount > 0 + ? parenthesizedLambda.ParameterList.Parameters[paramCount - 1].Identifier.Text + : "entity"; + } + else + { + return document; + } + + // Build new projection lambda: e => new { e.Id, Prop1 = e.Nav.Prop1, ... } + var newProjectionLambda = BuildProjectionLambda(entityParamName, accessedProperties); + + // Build new filter lambda with renamed parameter: (_, _, _, proj) => proj.Prop1 == value + var newFilterLambda = BuildFilterLambda(filterLambda, entityParamName, accessedProperties); + + // Replace arguments + SyntaxNode newInvocation; + if (is4ParamSyntax) + { + // Add projection argument + var projectionArg = SyntaxFactory.Argument(newProjectionLambda) + .WithNameColon(SyntaxFactory.NameColon("projection")) + .WithLeadingTrivia(SyntaxFactory.Whitespace("\n ")); + + var filterArg = arguments[0].WithExpression(newFilterLambda); + + var newArguments = SyntaxFactory.SeparatedList([projectionArg, filterArg], + [SyntaxFactory.Token(SyntaxKind.CommaToken).WithTrailingTrivia(SyntaxFactory.Whitespace(" "))]); + + newInvocation = invocation.WithArgumentList( + invocation.ArgumentList.WithArguments(newArguments)); + } + else + { + // Replace projection and filter arguments + var newArguments = arguments; + + if (projectionArgumentIndex >= 0) + { + newArguments = newArguments.Replace( + newArguments[projectionArgumentIndex], + newArguments[projectionArgumentIndex].WithExpression(newProjectionLambda)); + } + + if (filterArgumentIndex >= 0) + { + newArguments = newArguments.Replace( + newArguments[filterArgumentIndex], + newArguments[filterArgumentIndex].WithExpression(newFilterLambda)); + } + + newInvocation = invocation.WithArgumentList( + invocation.ArgumentList.WithArguments(newArguments)); + } + + var newRoot = root.ReplaceNode(invocation, newInvocation); + return document.WithSyntaxRoot(newRoot); + } + + static List ExtractAccessedProperties( + LambdaExpressionSyntax filterLambda, + SemanticModel semanticModel) + { + var properties = new List(); + var body = filterLambda.Body; + + // Get filter parameter name + string? paramName = null; + if (filterLambda is SimpleLambdaExpressionSyntax simpleLambda) + { + paramName = simpleLambda.Parameter.Identifier.Text; + } + else if (filterLambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + { + var paramCount = parenthesizedLambda.ParameterList.Parameters.Count; + if (paramCount > 0) + { + paramName = parenthesizedLambda.ParameterList.Parameters[paramCount - 1].Identifier.Text; + } + } + + if (paramName == null) + { + return properties; + } + + // Find all member accesses + var memberAccesses = body.DescendantNodesAndSelf() + .OfType(); + + foreach (var memberAccess in memberAccesses) + { + // Look for patterns like: e.Parent.Property + if (memberAccess.Expression is MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax identifier } nestedAccess && + identifier.Identifier.Text == paramName) + { + // e.Parent.Property - extract as "Parent.Property" + var navName = nestedAccess.Name.Identifier.Text; + var propName = memberAccess.Name.Identifier.Text; + var fullPath = $"{navName}.{propName}"; + var flatName = $"{navName}{propName}"; + + if (!properties.Any(p => p.FullPath == fullPath)) + { + properties.Add(new(fullPath, flatName, memberAccess)); + } + } + } + + return properties; + } + + static LambdaExpressionSyntax BuildProjectionLambda( + string paramName, + List properties) + { + // Build: e => new { e.Id, Prop1 = e.Nav.Prop1, ... } + var parameter = SyntaxFactory.Parameter(SyntaxFactory.Identifier(paramName)); + + List initializers = []; + + // Add Id property + initializers.Add( + SyntaxFactory.AnonymousObjectMemberDeclarator( + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName(paramName), + SyntaxFactory.IdentifierName("Id")))); + + // Add accessed properties with flattened names + foreach (var prop in properties) + { + var parts = prop.FullPath.Split('.'); + ExpressionSyntax expression = SyntaxFactory.IdentifierName(paramName); + + foreach (var part in parts) + { + expression = SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + expression, + SyntaxFactory.IdentifierName(part)); + } + + initializers.Add( + SyntaxFactory.AnonymousObjectMemberDeclarator( + SyntaxFactory.NameEquals(prop.FlatName), + expression)); + } + + var anonymousObject = SyntaxFactory.AnonymousObjectCreationExpression( + SyntaxFactory.SeparatedList(initializers)); + + return SyntaxFactory.SimpleLambdaExpression(parameter, anonymousObject); + } + + static LambdaExpressionSyntax BuildFilterLambda( + LambdaExpressionSyntax originalFilter, + string originalParamName, + List properties) + { + // Replace entity parameter references with proj and update property accesses + var newBody = originalFilter.Body; + + // Replace each property access with flattened name + foreach (var prop in properties) + { + // Replace e.Parent.Property with proj.ParentProperty + newBody = newBody.ReplaceNodes( + newBody.DescendantNodesAndSelf().Where(n => n == prop.OriginalAccess), + (_, __) => SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName("proj"), + SyntaxFactory.IdentifierName(prop.FlatName))); + } + + // Build new parameter list with "proj" as last parameter + if (originalFilter is SimpleLambdaExpressionSyntax) + { + return SyntaxFactory.SimpleLambdaExpression( + SyntaxFactory.Parameter(SyntaxFactory.Identifier("proj")), + newBody); + } + else if (originalFilter is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + { + var parameters = parenthesizedLambda.ParameterList.Parameters; + var newParameters = parameters.RemoveAt(parameters.Count - 1) + .Add(SyntaxFactory.Parameter(SyntaxFactory.Identifier("proj"))); + + return SyntaxFactory.ParenthesizedLambdaExpression( + SyntaxFactory.ParameterList(newParameters), + newBody); + } + + return originalFilter; + } + + class PropertyAccess + { + public PropertyAccess(string fullPath, string flatName, SyntaxNode originalAccess) + { + FullPath = fullPath; + FlatName = flatName; + OriginalAccess = originalAccess; + } + + public string FullPath { get; } + public string FlatName { get; } + public SyntaxNode OriginalAccess { get; } + } +} diff --git a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs index 1ba9f4ab1..57708b0cf 100644 --- a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs +++ b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs @@ -20,6 +20,7 @@ public FilterEntry( var compiled = projection.Compile(); compiledProjection = entity => compiled((TEntity)entity); RequiredPropertyNames = ProjectionAnalyzer.ExtractRequiredProperties(projection); + ValidateProjectionCompatibility(projection, RequiredPropertyNames); } } @@ -36,4 +37,65 @@ public Task ShouldIncludeWithProjection( : default!; return filter(userContext, data, userPrincipal, projectedData); } + + static void ValidateProjectionCompatibility( + Expression> projection, + IReadOnlySet requiredPropertyNames) + { + // Extract navigation paths (e.g., "Parent.Property" -> navigation "Parent") + var navigationPaths = requiredPropertyNames + .Where(p => p.Contains('.')) + .Select(p => p.Split('.')[0]) + .Distinct() + .ToList(); + + if (navigationPaths.Count == 0) + { + return; + } + + var entityType = typeof(TEntity); + var isIdentity = IsIdentityProjection(projection); + + // Check each navigation for abstract types + foreach (var navPath in navigationPaths) + { + var navProperty = entityType.GetProperty(navPath); + if (navProperty == null) + { + continue; + } + + var navType = navProperty.PropertyType; + + // For collections, get the element type + if (navType.IsGenericType) + { + var genericDef = navType.GetGenericTypeDefinition(); + if (genericDef == typeof(ICollection<>) || + genericDef == typeof(IList<>) || + genericDef == typeof(IEnumerable<>) || + genericDef == typeof(List<>)) + { + navType = navType.GetGenericArguments()[0]; + } + } + + if (navType.IsAbstract) + { + var projectionType = isIdentity ? "identity projection '_ => _'" : "projection that accesses abstract navigation"; + throw new InvalidOperationException( + $"Filter for '{entityType.Name}' uses {projectionType} " + + $"to access properties of abstract navigation '{navPath}' ({navType.Name}). " + + $"This forces Include() to load all columns from {navType.Name}. " + + $"Abstract types cannot be projected. Extract only the required properties: " + + $"projection: e => new {{ e.Id, {navPath}Property = e.{navPath}.PropertyName }}, " + + $"filter: (_, _, _, proj) => proj.{navPath}Property == value"); + } + } + } + + // Detect: x => x (where body == parameter) + static bool IsIdentityProjection(Expression> projection) => + projection.Body == projection.Parameters[0]; } diff --git a/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs b/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs new file mode 100644 index 000000000..9dee370bd --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs @@ -0,0 +1,125 @@ +public partial class IntegrationTests +{ + /// + /// Test that explicit projection accessing properties through abstract navigation throws. + /// DerivedChildEntity.Parent is BaseEntity (abstract), so projecting Parent.Property + /// should throw because abstract types cannot be projected by EF Core. + /// + [Fact] + public void Explicit_projection_with_abstract_navigation_property_throws() + { + var filters = new Filters(); + + var exception = Assert.Throws(() => + { + // Projection accesses Parent.Property where Parent is BaseEntity (abstract) + filters.For().Add( + projection: c => c.Parent!.Property, + filter: (_, _, _, prop) => prop == "test"); + }); + + Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Parent", exception.Message); + } + + /// + /// Test that anonymous type projection with abstract navigation also throws. + /// Even with anonymous types, we cannot project from abstract navigations. + /// + [Fact] + public void Anonymous_type_projection_with_abstract_navigation_throws() + { + var filters = new Filters(); + + var exception = Assert.Throws(() => + { + // Even with anonymous type, projecting from abstract Parent throws + filters.For().Add( + projection: c => new { c.Id, ParentProperty = c.Parent!.Property }, + filter: (_, _, _, proj) => proj.ParentProperty == "test"); + }); + + Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Parent", exception.Message); + } + + /// + /// Test that explicit projection with concrete (non-abstract) navigation succeeds. + /// DerivedChildEntity.TypedParent is DerivedWithNavigationEntity (concrete), + /// so projection through it should be allowed. + /// + [Fact] + public void Explicit_projection_with_concrete_navigation_succeeds() + { + var filters = new Filters(); + + // DerivedWithNavigationEntity is concrete - should work fine + filters.For().Add( + projection: c => c.TypedParent!.Property, + filter: (_, _, _, prop) => prop == "test"); + + // Verify the filter was added successfully + var requiredProps = filters.GetRequiredFilterProperties(); + Assert.Contains("TypedParent.Property", requiredProps); + } + + /// + /// Test that identity projection without navigation access succeeds. + /// Identity projections are fine when the filter only accesses scalar properties. + /// Runtime validation doesn't analyze the filter, so this succeeds. + /// + [Fact] + public void Identity_projection_without_navigation_succeeds() + { + var filters = new Filters(); + + // Identity projection with filter that only accesses scalars + // (Runtime doesn't validate filter, so this is allowed) + filters.For().Add( + projection: c => c, + filter: (_, _, _, c) => c.Property == "test" && c.ParentId != null); + + // For identity projection, RequiredPropertyNames is empty + var requiredProps = filters.GetRequiredFilterProperties(); + Assert.Empty(requiredProps); + } + + /// + /// Test that the 4-parameter filter syntax (which uses identity projection internally) + /// does NOT throw at runtime. The analyzer will catch filter accesses at compile time. + /// Runtime validation only catches explicit projections through abstract navigations. + /// + [Fact] + public void Four_parameter_filter_with_abstract_navigation_allowed_at_runtime() + { + var filters = new Filters(); + + // Runtime doesn't analyze the filter body, so this is allowed + // (The analyzer will catch this at compile time) + filters.For().Add( + filter: (_, _, _, c) => c.ParentId != null); + + var requiredProps = filters.GetRequiredFilterProperties(); + Assert.Empty(requiredProps); // Identity projection extracts no properties + } + + /// + /// Test projecting from nested abstract navigation property. + /// + [Fact] + public void Projection_with_nested_abstract_navigation_property_throws() + { + var filters = new Filters(); + + var exception = Assert.Throws(() => + { + // Accessing nested property through abstract parent + filters.For().Add( + projection: c => c.Parent!.Status, + filter: (_, _, _, status) => status == "Active"); + }); + + Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Parent", exception.Message); + } +} diff --git a/src/Tests/UnitTests/FilterEntryValidationTests.cs b/src/Tests/UnitTests/FilterEntryValidationTests.cs new file mode 100644 index 000000000..3a09cea41 --- /dev/null +++ b/src/Tests/UnitTests/FilterEntryValidationTests.cs @@ -0,0 +1,97 @@ +using GraphQL.EntityFramework; + +public class FilterEntryValidationTests +{ + class TestEntity + { + public Guid Id { get; set; } + public string? Property { get; set; } + public Guid? ParentId { get; set; } + public AbstractParent? Parent { get; set; } + public ConcreteParent? ConcreteParent { get; set; } + } + + abstract class AbstractParent + { + public Guid Id { get; set; } + public string? Property { get; set; } + } + + class ConcreteParent + { + public Guid Id { get; set; } + public string? Property { get; set; } + } + + class TestDbContext : DbContext + { + } + + [Fact] + public void Explicit_projection_with_abstract_navigation_property_throws() + { + var filters = new Filters(); + + // This should throw - projection directly accesses abstract navigation property + var exception = Assert.Throws(() => + { + filters.For().Add( + projection: e => e.Parent!.Property, + filter: (_, _, _, prop) => prop == "test"); + }); + + Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Parent", exception.Message); + } + + [Fact] + public void Concrete_navigation_allows_identity_projection() + { + var filters = new Filters(); + + // Should not throw - concrete navigation + filters.For().Add( + projection: e => e, + filter: (_, _, _, e) => e.ConcreteParent!.Property == "test"); + } + + [Fact] + public void Anonymous_type_projection_with_abstract_navigation_throws() + { + var filters = new Filters(); + + // This should also throw - even with anonymous type, we're projecting from abstract navigation + var exception = Assert.Throws(() => + { + filters.For().Add( + projection: e => new { e.Id, ParentProp = e.Parent!.Property }, + filter: (_, _, _, proj) => proj.ParentProp == "test"); + }); + + Assert.Contains("abstract navigation", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("Parent", exception.Message); + } + + [Fact] + public void Projection_without_abstract_navigation_succeeds() + { + var filters = new Filters(); + + // Should not throw - only accessing scalar properties + filters.For().Add( + projection: e => new { e.Id, e.Property }, + filter: (_, _, _, proj) => proj.Property == "test"); + } + + [Fact] + public void Identity_projection_without_navigation_succeeds() + { + var filters = new Filters(); + + // Should not throw - identity projection is fine if not accessing navigations + // (the analyzer will catch if the filter accesses navigations) + filters.For().Add( + projection: e => e, + filter: (_, _, _, e) => e.Property == "test"); + } +} From 3c99884a4bcc7b114b963ceb2a4cc1e7e1b6d8ba Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:01:43 +1100 Subject: [PATCH 04/14] . --- .../GraphApi/EfGraphQLService_First.cs | 3 +- .../GraphApi/EfGraphQLService_Queryable.cs | 3 +- .../EfGraphQLService_QueryableConnection.cs | 3 +- .../GraphApi/EfGraphQLService_Single.cs | 3 +- .../IncludeAppender.cs | 38 ++++++------------- 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs index e798daffc..e12002ee4 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs @@ -166,8 +166,7 @@ FieldType BuildFirstField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - var (queryWithIncludes, hasAbstractFilterNavigations) = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - query = queryWithIncludes; + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs index 719977ab9..ecb91f91f 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs @@ -112,8 +112,7 @@ FieldType BuildQueryField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - var (queryWithIncludes, hasAbstractFilterNavigations) = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - query = queryWithIncludes; + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); if (!omitQueryArguments) { query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs index 924839678..6000e0d16 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs @@ -158,8 +158,7 @@ ConnectionBuilder AddQueryableConnection( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - var (queryWithIncludes, hasAbstractFilterNavigations) = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - query = queryWithIncludes; + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); // Apply column projection based on requested GraphQL fields diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs index 8377878f8..42a67b873 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs @@ -166,8 +166,7 @@ FieldType BuildSingleField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - var (queryWithIncludes, hasAbstractFilterNavigations) = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - query = queryWithIncludes; + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index acfda2fcd..37f819037 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -25,9 +25,9 @@ public IQueryable AddIncludesWithFilters( IResolveFieldContext context, IReadOnlyDictionary>? allFilterFields) where TItem : class => - AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields).query; + AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); - internal (IQueryable query, bool hasAbstractFilterNavigations) AddIncludesWithFiltersAndDetectNavigations( + internal IQueryable AddIncludesWithFiltersAndDetectNavigations( IQueryable query, IResolveFieldContext context, IReadOnlyDictionary>? allFilterFields) @@ -37,20 +37,19 @@ public IQueryable AddIncludesWithFilters( query = AddIncludes(query, context); // Then add includes for filter-required navigations - var hasAbstractFilterNavigations = false; if (allFilterFields is { Count: > 0 }) { var type = typeof(TItem); if (navigations.TryGetValue(type, out var navigationProperties)) { - (query, hasAbstractFilterNavigations) = AddFilterIncludes(query, allFilterFields, type, navigationProperties); + query = AddFilterIncludes(query, allFilterFields, type, navigationProperties); } } - return (query, hasAbstractFilterNavigations); + return query; } - static (IQueryable query, bool hasFilterNavigations) AddFilterIncludes( + static IQueryable AddFilterIncludes( IQueryable query, IReadOnlyDictionary> allFilterFields, Type entityType, @@ -72,7 +71,7 @@ public IQueryable AddIncludesWithFilters( if (relevantFilterFields.Count == 0) { - return (query, false); + return query; } // Extract navigation names from filter fields (e.g., "TravelRequest.GroupOwnerId" -> "TravelRequest") @@ -86,26 +85,13 @@ public IQueryable AddIncludesWithFilters( } } - // Only add Include for abstract filter-required navigations - // Non-abstract navigations are handled via the projection system in TryGetProjectionExpressionWithFilters - // which only selects the filter-required columns, not all columns - var hasAbstractNavigations = false; - foreach (var navName in filterNavigations) - { - if (navigationProperties.TryGetValue(navName, out var navMetadata)) - { - if (navMetadata.Type.IsAbstract) - { - // Abstract types cannot be projected (can't instantiate abstract class) - // Must use Include which loads all columns - query = query.Include(navName); - hasAbstractNavigations = true; - } - // Non-abstract navigations: projection system handles them efficiently - } - } + // Note: Abstract filter navigations are now prevented by FilterEntry validation + // All filter-required navigations here should be from explicit projections + // that extract specific properties (not identity projections) + // These are handled via the projection system in TryGetProjectionExpressionWithFilters + // which only selects the required columns, not all columns - return (query, hasAbstractNavigations); + return query; } public FieldProjectionInfo? GetProjection(IResolveFieldContext context) From ca115f2d608d2771a98e67cd3d00bbf8b78017e0 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:05:25 +1100 Subject: [PATCH 05/14] . --- ...avigationProjectionCodeFixProviderTests.cs | 2 +- .../IncludeAppender.cs | 59 ++----------------- ...grationTests_abstract_filter_validation.cs | 2 +- .../UnitTests/FilterEntryValidationTests.cs | 6 +- 4 files changed, 8 insertions(+), 61 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs index 723822b8b..f50f0b427 100644 --- a/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs +++ b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.cs @@ -83,7 +83,7 @@ static async Task ApplyCodeFixAsync(string source) var operation = operations.OfType().FirstOrDefault(); if (operation == null) { - throw new($"No ApplyChangesOperation found"); + throw new("No ApplyChangesOperation found"); } var changedSolution = operation.ChangedSolution; diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index 37f819037..f6d96647e 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -33,66 +33,17 @@ internal IQueryable AddIncludesWithFiltersAndDetectNavigations( IReadOnlyDictionary>? allFilterFields) where TItem : class { - // First add includes from GraphQL query + // Add includes from GraphQL query query = AddIncludes(query, context); - // Then add includes for filter-required navigations - if (allFilterFields is { Count: > 0 }) - { - var type = typeof(TItem); - if (navigations.TryGetValue(type, out var navigationProperties)) - { - query = AddFilterIncludes(query, allFilterFields, type, navigationProperties); - } - } + // Note: Filter-required navigations are now handled entirely by the projection system + // in TryGetProjectionExpressionWithFilters, which builds projections that include + // filter-required fields. Abstract navigation access is prevented by FilterEntry validation. + // No additional includes are needed here. return query; } - static IQueryable AddFilterIncludes( - IQueryable query, - IReadOnlyDictionary> allFilterFields, - Type entityType, - IReadOnlyDictionary navigationProperties) - where TItem : class - { - // Get filter fields for this entity type and its base types - var relevantFilterFields = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var (filterType, filterFields) in allFilterFields) - { - if (filterType.IsAssignableFrom(entityType)) - { - foreach (var field in filterFields) - { - relevantFilterFields.Add(field); - } - } - } - - if (relevantFilterFields.Count == 0) - { - return query; - } - - // Extract navigation names from filter fields (e.g., "TravelRequest.GroupOwnerId" -> "TravelRequest") - var filterNavigations = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var field in relevantFilterFields) - { - if (field.Contains('.')) - { - var navName = field.Split('.', 2)[0]; - filterNavigations.Add(navName); - } - } - - // Note: Abstract filter navigations are now prevented by FilterEntry validation - // All filter-required navigations here should be from explicit projections - // that extract specific properties (not identity projections) - // These are handled via the projection system in TryGetProjectionExpressionWithFilters - // which only selects the required columns, not all columns - - return query; - } public FieldProjectionInfo? GetProjection(IResolveFieldContext context) where TItem : class diff --git a/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs b/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs index 9dee370bd..45c825bf9 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs @@ -77,7 +77,7 @@ public void Identity_projection_without_navigation_succeeds() // (Runtime doesn't validate filter, so this is allowed) filters.For().Add( projection: c => c, - filter: (_, _, _, c) => c.Property == "test" && c.ParentId != null); + filter: (_, _, _, c) => c is { Property: "test", ParentId: not null }); // For identity projection, RequiredPropertyNames is empty var requiredProps = filters.GetRequiredFilterProperties(); diff --git a/src/Tests/UnitTests/FilterEntryValidationTests.cs b/src/Tests/UnitTests/FilterEntryValidationTests.cs index 3a09cea41..9cf21089f 100644 --- a/src/Tests/UnitTests/FilterEntryValidationTests.cs +++ b/src/Tests/UnitTests/FilterEntryValidationTests.cs @@ -1,5 +1,3 @@ -using GraphQL.EntityFramework; - public class FilterEntryValidationTests { class TestEntity @@ -23,9 +21,7 @@ class ConcreteParent public string? Property { get; set; } } - class TestDbContext : DbContext - { - } + class TestDbContext : DbContext; [Fact] public void Explicit_projection_with_abstract_navigation_property_throws() From b16ea6f59fa6e2c4ff3a9896eeffbb65afc7b277 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:09:27 +1100 Subject: [PATCH 06/14] Update FilterIdentityProjectionAnalyzer.cs --- .../FilterIdentityProjectionAnalyzer.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index 165348906..d227136ff 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -47,7 +47,7 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) // Identity projection detected // First check for abstract navigation access (GQLEF007) - var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel, entityType); + var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel); if (abstractNav != null) { // Error: Identity projection with abstract navigation access @@ -92,7 +92,7 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) if (is4ParamFilter) { // First check for abstract navigation access (GQLEF007) - var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel, entityType); + var abstractNav = FindAbstractNavigationAccess(filterLambda, context.SemanticModel); if (abstractNav != null) { var diagnostic = Diagnostic.Create( @@ -441,8 +441,7 @@ static int GetParameterIndex(ImmutableArray parameters, IParam static string? FindAbstractNavigationAccess( LambdaExpressionSyntax lambda, - SemanticModel semanticModel, - string? entityTypeName) + SemanticModel semanticModel) { var body = lambda.Body; @@ -478,22 +477,16 @@ static int GetParameterIndex(ImmutableArray parameters, IParam identifier.Identifier.Text == filterParameterName) { // This is e.Parent.Property - check if Parent is abstract - var navigationSymbol = semanticModel.GetSymbolInfo(nestedAccess).Symbol as IPropertySymbol; - if (navigationSymbol != null) + if (semanticModel.GetSymbolInfo(nestedAccess).Symbol is IPropertySymbol { Type.IsAbstract: true } navigationSymbol) { - var navType = navigationSymbol.Type; - if (navType.IsAbstract) - { - return navigationSymbol.Name; - } + return navigationSymbol.Name; } } // Also check direct navigation access: e.Parent (without further property access) else if (memberAccess.Expression is IdentifierNameSyntax directIdentifier && directIdentifier.Identifier.Text == filterParameterName) { - var propertySymbol = semanticModel.GetSymbolInfo(memberAccess).Symbol as IPropertySymbol; - if (propertySymbol != null) + if (semanticModel.GetSymbolInfo(memberAccess).Symbol is IPropertySymbol propertySymbol) { var propType = propertySymbol.Type; // Check if this is a reference type (not a primitive) and is abstract From d775e6c717abebacadf27a4ee28c204875860629 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:15:55 +1100 Subject: [PATCH 07/14] Update AbstractNavigationProjectionCodeFixProvider.cs --- .../AbstractNavigationProjectionCodeFixProvider.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs index fbe73790a..674d86351 100644 --- a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -61,7 +61,6 @@ static async Task ConvertToExplicitProjectionAsync( // Find projection and filter arguments var arguments = invocation.ArgumentList.Arguments; - LambdaExpressionSyntax? projectionLambda = null; LambdaExpressionSyntax? filterLambda = null; var projectionArgumentIndex = -1; var filterArgumentIndex = -1; @@ -69,10 +68,8 @@ static async Task ConvertToExplicitProjectionAsync( for (var i = 0; i < arguments.Count; i++) { var arg = arguments[i]; - if (arg.NameColon?.Name.Identifier.Text == "projection" && - arg.Expression is LambdaExpressionSyntax projLambda) + if (arg.NameColon?.Name.Identifier.Text == "projection") { - projectionLambda = projLambda; projectionArgumentIndex = i; } else if (arg.NameColon?.Name.Identifier.Text == "filter" && From 57a342225924fb751a0da412f43cd2dd746aafc6 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:22:54 +1100 Subject: [PATCH 08/14] . --- ...rojectionToExplicitProjection.verified.txt | 27 +++++++++++++ ...ractNavigationProjectionCodeFixProvider.cs | 38 ++++++++++++++----- 2 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.ConvertsIdentityProjectionToExplicitProjection.verified.txt diff --git a/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.ConvertsIdentityProjectionToExplicitProjection.verified.txt b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.ConvertsIdentityProjectionToExplicitProjection.verified.txt new file mode 100644 index 000000000..81d1ef942 --- /dev/null +++ b/src/GraphQL.EntityFramework.Analyzers.Tests/AbstractNavigationProjectionCodeFixProviderTests.ConvertsIdentityProjectionToExplicitProjection.verified.txt @@ -0,0 +1,27 @@ +using GraphQL.EntityFramework; +using Microsoft.EntityFrameworkCore; +using System; + +public abstract class BaseEntity +{ + public Guid Id { get; set; } + public string Property { get; set; } = ""; +} + +public class ChildEntity +{ + public Guid Id { get; set; } + public BaseEntity? Parent { get; set; } +} + +public class TestDbContext : DbContext { } + +public class TestClass +{ + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: c => new { c.Id, ParentProperty = c.Parent.Property }, + filter: (_, _, _, proj) => proj.ParentProperty == "test"); + } +} \ No newline at end of file diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs index 674d86351..2012ef1bd 100644 --- a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -196,19 +196,37 @@ static List ExtractAccessedProperties( foreach (var memberAccess in memberAccesses) { - // Look for patterns like: e.Parent.Property - if (memberAccess.Expression is MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax identifier } nestedAccess && - identifier.Identifier.Text == paramName) + // Look for patterns like: e.Parent.Property or e.Parent!.Property (with null-forgiving operator) + // The null-forgiving operator creates a PostfixUnaryExpressionSyntax wrapper + var innerExpression = memberAccess.Expression; + + // Unwrap null-forgiving operator if present: e.Parent! -> e.Parent + if (innerExpression is PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } postfix) + { + innerExpression = postfix.Operand; + } + + if (innerExpression is MemberAccessExpressionSyntax nestedAccess) { - // e.Parent.Property - extract as "Parent.Property" - var navName = nestedAccess.Name.Identifier.Text; - var propName = memberAccess.Name.Identifier.Text; - var fullPath = $"{navName}.{propName}"; - var flatName = $"{navName}{propName}"; + // Check if the root is our parameter (possibly with null-forgiving) + var rootExpr = nestedAccess.Expression; + if (rootExpr is PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.SuppressNullableWarningExpression } rootPostfix) + { + rootExpr = rootPostfix.Operand; + } - if (!properties.Any(p => p.FullPath == fullPath)) + if (rootExpr is IdentifierNameSyntax identifier && identifier.Identifier.Text == paramName) { - properties.Add(new(fullPath, flatName, memberAccess)); + // e.Parent.Property - extract as "Parent.Property" + var navName = nestedAccess.Name.Identifier.Text; + var propName = memberAccess.Name.Identifier.Text; + var fullPath = $"{navName}.{propName}"; + var flatName = $"{navName}{propName}"; + + if (!properties.Any(p => p.FullPath == fullPath)) + { + properties.Add(new(fullPath, flatName, memberAccess)); + } } } } From 65231682315460956716b71959bb4737d3529ae8 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:26:17 +1100 Subject: [PATCH 09/14] Update AbstractNavigationProjectionCodeFixProvider.cs --- ...AbstractNavigationProjectionCodeFixProvider.cs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs index 2012ef1bd..93580da59 100644 --- a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -33,16 +33,10 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken); - if (semanticModel == null) - { - return; - } - context.RegisterCodeFix( CodeAction.Create( title: title, - createChangedDocument: c => ConvertToExplicitProjectionAsync(context.Document, invocation, semanticModel, c), + createChangedDocument: c => ConvertToExplicitProjectionAsync(context.Document, invocation, c), equivalenceKey: title), diagnostic); } @@ -50,7 +44,6 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) static async Task ConvertToExplicitProjectionAsync( Document document, InvocationExpressionSyntax invocation, - SemanticModel semanticModel, Cancel cancel) { var root = await document.GetSyntaxRootAsync(cancel); @@ -89,7 +82,7 @@ static async Task ConvertToExplicitProjectionAsync( } // Extract accessed properties from filter - var accessedProperties = ExtractAccessedProperties(filterLambda, semanticModel); + var accessedProperties = ExtractAccessedProperties(filterLambda); if (accessedProperties.Count == 0) { return document; @@ -163,9 +156,7 @@ static async Task ConvertToExplicitProjectionAsync( return document.WithSyntaxRoot(newRoot); } - static List ExtractAccessedProperties( - LambdaExpressionSyntax filterLambda, - SemanticModel semanticModel) + static List ExtractAccessedProperties(LambdaExpressionSyntax filterLambda) { var properties = new List(); var body = filterLambda.Body; From cf77b1948f8dc9bd2a2e74906caf814a5bf2802f Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:34:04 +1100 Subject: [PATCH 10/14] . --- .../FilterIdentityProjectionAnalyzer.cs | 28 +++++++++++-------- ...ractNavigationProjectionCodeFixProvider.cs | 8 +++--- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index d227136ff..7e3dc64b5 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -486,19 +486,23 @@ static int GetParameterIndex(ImmutableArray parameters, IParam else if (memberAccess.Expression is IdentifierNameSyntax directIdentifier && directIdentifier.Identifier.Text == filterParameterName) { - if (semanticModel.GetSymbolInfo(memberAccess).Symbol is IPropertySymbol propertySymbol) + if (semanticModel.GetSymbolInfo(memberAccess).Symbol is not IPropertySymbol propertySymbol) { - var propType = propertySymbol.Type; - // Check if this is a reference type (not a primitive) and is abstract - // This indicates it's likely a navigation property to an abstract entity - if (propType is { IsAbstract: true, TypeKind: TypeKind.Class }) - { - // But make sure it's not a primitive key property or known scalar - if (!IsPrimaryKeyProperty(propertySymbol) && !IsForeignKeyProperty(propertySymbol)) - { - return propertySymbol.Name; - } - } + continue; + } + + var propType = propertySymbol.Type; + // Check if this is a reference type (not a primitive) and is abstract + // This indicates it's likely a navigation property to an abstract entity + if (propType is not { IsAbstract: true, TypeKind: TypeKind.Class }) + { + continue; + } + + // But make sure it's not a primitive key property or known scalar + if (!IsPrimaryKeyProperty(propertySymbol) && !IsForeignKeyProperty(propertySymbol)) + { + return propertySymbol.Name; } } } diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs index 93580da59..49b9f10a7 100644 --- a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -110,7 +110,7 @@ static async Task ConvertToExplicitProjectionAsync( var newProjectionLambda = BuildProjectionLambda(entityParamName, accessedProperties); // Build new filter lambda with renamed parameter: (_, _, _, proj) => proj.Prop1 == value - var newFilterLambda = BuildFilterLambda(filterLambda, entityParamName, accessedProperties); + var newFilterLambda = BuildFilterLambda(filterLambda, accessedProperties); // Replace arguments SyntaxNode newInvocation; @@ -270,7 +270,6 @@ static LambdaExpressionSyntax BuildProjectionLambda( static LambdaExpressionSyntax BuildFilterLambda( LambdaExpressionSyntax originalFilter, - string originalParamName, List properties) { // Replace entity parameter references with proj and update property accesses @@ -282,7 +281,7 @@ static LambdaExpressionSyntax BuildFilterLambda( // Replace e.Parent.Property with proj.ParentProperty newBody = newBody.ReplaceNodes( newBody.DescendantNodesAndSelf().Where(n => n == prop.OriginalAccess), - (_, __) => SyntaxFactory.MemberAccessExpression( + (_, _) => SyntaxFactory.MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("proj"), SyntaxFactory.IdentifierName(prop.FlatName))); @@ -295,7 +294,8 @@ static LambdaExpressionSyntax BuildFilterLambda( SyntaxFactory.Parameter(SyntaxFactory.Identifier("proj")), newBody); } - else if (originalFilter is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + + if (originalFilter is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) { var parameters = parenthesizedLambda.ParameterList.Parameters; var newParameters = parameters.RemoveAt(parameters.Count - 1) From 4fe307bc8824c4181dd88c27ebe1ba238ed5e24f Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:36:40 +1100 Subject: [PATCH 11/14] . --- .../GraphApi/EfGraphQLService_First.cs | 4 ++-- .../GraphApi/EfGraphQLService_Queryable.cs | 4 ++-- .../GraphApi/EfGraphQLService_QueryableConnection.cs | 4 ++-- .../GraphApi/EfGraphQLService_Single.cs | 4 ++-- src/GraphQL.EntityFramework/IncludeAppender.cs | 9 +++------ 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs index e12002ee4..ddf49a9fa 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs @@ -166,7 +166,7 @@ FieldType BuildFirstField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -248,4 +248,4 @@ await fieldContext.Filters.ShouldInclude(context.UserContext, fieldContext.DbCon TReturn? ReturnNullable(IQueryable? query = null) => nullable ? null : throw new FirstEntityNotFoundException(query?.SafeToQueryString()); } -} \ No newline at end of file +} diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs index ecb91f91f..16f230712 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs @@ -112,7 +112,7 @@ FieldType BuildQueryField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context); if (!omitQueryArguments) { query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); @@ -198,4 +198,4 @@ static Type MakeListGraphType(Type? itemGraphType) keyNames.TryGetValue(typeof(TSource), out var names); return names; } -} \ No newline at end of file +} diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs index 6000e0d16..595830af0 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs @@ -158,7 +158,7 @@ ConnectionBuilder AddQueryableConnection( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context); query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -219,4 +219,4 @@ ConnectionBuilder AddQueryableConnection( field.AddWhereArgument(hasId); return builder; } -} \ No newline at end of file +} diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs index 42a67b873..f6fe30be1 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs @@ -166,7 +166,7 @@ FieldType BuildSingleField( // Get filter-required fields early so we can add filter-required navigations via Include var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); - query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + query = includeAppender.AddIncludesWithFiltersAndDetectNavigations(query, context); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -248,4 +248,4 @@ await fieldContext.Filters.ShouldInclude(context.UserContext, fieldContext.DbCon TReturn? ReturnNullable(IQueryable? query = null) => nullable ? null : throw new SingleEntityNotFoundException(query?.SafeToQueryString()); } -} \ No newline at end of file +} diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index f6d96647e..581ab779b 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -25,12 +25,11 @@ public IQueryable AddIncludesWithFilters( IResolveFieldContext context, IReadOnlyDictionary>? allFilterFields) where TItem : class => - AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + AddIncludesWithFiltersAndDetectNavigations(query, context); internal IQueryable AddIncludesWithFiltersAndDetectNavigations( IQueryable query, - IResolveFieldContext context, - IReadOnlyDictionary>? allFilterFields) + IResolveFieldContext context) where TItem : class { // Add includes from GraphQL query @@ -44,7 +43,6 @@ internal IQueryable AddIncludesWithFiltersAndDetectNavigations( return query; } - public FieldProjectionInfo? GetProjection(IResolveFieldContext context) where TItem : class { @@ -497,8 +495,7 @@ void ProcessProjectionExpression( // Add any specific fields from the projection expression foreach (var nestedPath in nestedPaths) { - if (!nestedPath.Contains('.') && - !nestedProjection.ScalarFields.Contains(nestedPath)) + if (!nestedPath.Contains('.')) { nestedProjection.ScalarFields.Add(nestedPath); } From 9e6467f007cc753ffed836dcc33be7b7d3afd5f2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:52:18 +1100 Subject: [PATCH 12/14] . --- .../FilterIdentityProjectionAnalyzer.cs | 4 ++-- .../AbstractNavigationProjectionCodeFixProvider.cs | 4 ++-- src/GraphQL.EntityFramework/Filters/FilterEntry.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index 7e3dc64b5..a1a9e3117 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -178,8 +178,8 @@ static bool IsFilterBuilderAdd( } // Find the projection and filter parameters - var projectionParameter = methodSymbol.Parameters.FirstOrDefault(p => p.Name == "projection"); - var filterParameter = methodSymbol.Parameters.FirstOrDefault(p => p.Name == "filter"); + var projectionParameter = methodSymbol.Parameters.FirstOrDefault(_ => _.Name == "projection"); + var filterParameter = methodSymbol.Parameters.FirstOrDefault(_ => _.Name == "filter"); if (projectionParameter == null && filterParameter == null) { diff --git a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs index 49b9f10a7..9bbc3bd78 100644 --- a/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs +++ b/src/GraphQL.EntityFramework.CodeFixes/AbstractNavigationProjectionCodeFixProvider.cs @@ -229,7 +229,7 @@ static LambdaExpressionSyntax BuildProjectionLambda( string paramName, List properties) { - // Build: e => new { e.Id, Prop1 = e.Nav.Prop1, ... } + // Build: _ => new { _.Id, Prop1 = _.Nav.Prop1, ... } var parameter = SyntaxFactory.Parameter(SyntaxFactory.Identifier(paramName)); List initializers = []; @@ -280,7 +280,7 @@ static LambdaExpressionSyntax BuildFilterLambda( { // Replace e.Parent.Property with proj.ParentProperty newBody = newBody.ReplaceNodes( - newBody.DescendantNodesAndSelf().Where(n => n == prop.OriginalAccess), + newBody.DescendantNodesAndSelf().Where(_ => _ == prop.OriginalAccess), (_, _) => SyntaxFactory.MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("proj"), diff --git a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs index 57708b0cf..60ba19d5a 100644 --- a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs +++ b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs @@ -44,8 +44,8 @@ static void ValidateProjectionCompatibility( { // Extract navigation paths (e.g., "Parent.Property" -> navigation "Parent") var navigationPaths = requiredPropertyNames - .Where(p => p.Contains('.')) - .Select(p => p.Split('.')[0]) + .Where(_ => _.Contains('.')) + .Select(_ => _.Split('.')[0]) .Distinct() .ToList(); From 8cf0c47050636db122de310e0eff04eee11390ef Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:53:13 +1100 Subject: [PATCH 13/14] Update SelectExpressionBuilder.cs --- .../SelectProjection/SelectExpressionBuilder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs index 1a582ae79..24656a058 100644 --- a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs +++ b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs @@ -161,7 +161,7 @@ public static bool TryBuild( var innerMemberInit = Expression.MemberInit(navMetadata.NewInstance, innerBindings); var innerLambda = Expression.Lambda(innerMemberInit, navParam); - // Build: x.Children.OrderBy(n => n.Key) to ensure deterministic ordering + // Build: x.Children.OrderBy(_ => _.Key) to ensure deterministic ordering Expression orderedCollection = navAccess; if (keyNames.TryGetValue(navType, out var keys) && keys.Count > 0) { @@ -335,7 +335,7 @@ static bool TryBuildNestedNavigationBinding( var innerMemberInit = Expression.MemberInit(navMetadata.NewInstance, innerBindings); var innerLambda = Expression.Lambda(innerMemberInit, navParam); - // .OrderBy(o => o.Key) to ensure deterministic ordering + // .OrderBy(_ => _.Key) to ensure deterministic ordering Expression orderedCollection = navAccess; if (keyNames.TryGetValue(navType, out var keys) && keys.Count > 0) { @@ -348,7 +348,7 @@ static bool TryBuildNestedNavigationBinding( } } - // .Select(n => new Child { ... }) + // .Select(_ => new Child { ... }) var selectCall = Expression.Call(null, navMetadata.SelectMethod, orderedCollection, innerLambda); // .ToList() From c461478f8162304666cd035445bcfe445069d997 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Mon, 2 Feb 2026 13:53:42 +1100 Subject: [PATCH 14/14] Update Directory.Build.props --- src/Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index bbf01b707..467fcb884 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU5104;CS1573;CS9107;NU1608;NU1109 - 34.1.4 + 34.1.5 preview 1.0.0 EntityFrameworkCore, EntityFramework, GraphQL