diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs index c631a9d5..9d718204 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs @@ -163,15 +163,19 @@ FieldType BuildFirstField( query = query.AsNoTracking(); } - query = includeAppender.AddIncludes(query, context); + // 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 = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - if (!typeof(TReturn).IsAbstract) + // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) + if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) { - // Get filter-required fields and merge into projection (for all entity types including navigations) - var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); + // Merge filter-required scalar fields into projection (navigations are handled via Include 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 90afa60f..ce6753e5 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs @@ -109,7 +109,11 @@ FieldType BuildQueryField( query = query.AsNoTracking(); } - query = includeAppender.AddIncludes(query, context); + // 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; if (!omitQueryArguments) { query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); @@ -117,10 +121,10 @@ FieldType BuildQueryField( // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - if (!typeof(TReturn).IsAbstract) + // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) + if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) { - // Get filter-required fields and merge into projection (for all entity types including navigations) - var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); + // Merge filter-required scalar fields into projection (navigations are handled via Include 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 f5ecfe23..c00c83e2 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs @@ -155,15 +155,19 @@ ConnectionBuilder AddQueryableConnection( query = query.AsNoTracking(); } - query = includeAppender.AddIncludes(query, context); + // 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 = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - if (!typeof(TReturn).IsAbstract) + // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) + if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) { - // Get filter-required fields and merge into projection (for all entity types including navigations) - var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); + // Merge filter-required scalar fields into projection (navigations are handled via Include above) if (includeAppender.TryGetProjectionExpressionWithFilters(context, allFilterFields, out var selectExpr)) { query = query.Select(selectExpr); diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs index 8133d777..50410c42 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs @@ -163,15 +163,19 @@ FieldType BuildSingleField( query = query.AsNoTracking(); } - query = includeAppender.AddIncludes(query, context); + // 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 = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields // Skip projection for abstract types as they cannot be instantiated - if (!typeof(TReturn).IsAbstract) + // Skip projection if abstract filter navigations are present (projection fails on abstract types, EF would drop includes) + if (!typeof(TReturn).IsAbstract && !hasAbstractFilterNavigations) { - // Get filter-required fields and merge into projection (for all entity types including navigations) - var allFilterFields = fieldContext.Filters?.GetAllRequiredFilterProperties(); + // Merge filter-required scalar fields into projection (navigations are handled via Include 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 b504ddfb..fafd1126 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -20,6 +20,92 @@ public IQueryable AddIncludes(IQueryable query, IResolveFie return AddIncludes(query, context, navigationProperty); } + public IQueryable AddIncludesWithFilters( + IQueryable query, + IResolveFieldContext context, + IReadOnlyDictionary>? allFilterFields) + where TItem : class + { + var (resultQuery, _) = AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + return resultQuery; + } + + internal (IQueryable query, bool hasAbstractFilterNavigations) AddIncludesWithFiltersAndDetectNavigations( + IQueryable query, + IResolveFieldContext context, + IReadOnlyDictionary>? allFilterFields) + where TItem : class + { + // First add includes from GraphQL query + 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); + } + } + + return (query, hasAbstractFilterNavigations); + } + + static (IQueryable query, bool hasFilterNavigations) 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, false); + } + + // 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); + } + } + + // Add Include for each filter-required navigation + var hasAbstractNavigations = false; + foreach (var navName in filterNavigations) + { + if (navigationProperties.TryGetValue(navName, out var navMetadata)) + { + query = query.Include(navName); + if (navMetadata.Type.IsAbstract) + { + hasAbstractNavigations = true; + } + } + } + + return (query, hasAbstractNavigations); + } + public FieldProjectionInfo? GetProjection(IResolveFieldContext context) where TItem : class { @@ -170,7 +256,7 @@ FieldProjectionInfo MergeFilterFieldsIntoProjection( if (mergedNavigations.TryGetValue(navName, out var existingNav)) { - // Navigation exists - add filter-required properties + // Navigation exists in GraphQL query - add filter-required properties to its projection var updatedScalars = new HashSet(existingNav.Projection.ScalarFields, StringComparer.OrdinalIgnoreCase); foreach (var prop in requiredProps) { @@ -187,21 +273,17 @@ FieldProjectionInfo MergeFilterFieldsIntoProjection( Projection = updatedProjection }; } - else + else if (!navType.IsAbstract) { - // Navigation doesn't exist - create it with filter-required properties - keyNames.TryGetValue(navType, out var navKeys); - foreignKeys.TryGetValue(navType, out var navFks); - - var navProjection = new FieldProjectionInfo( - new(requiredProps, StringComparer.OrdinalIgnoreCase), - navKeys, - navFks, - null); - + // Create navigation projection for filter-only navigations (unless abstract) + // Abstract navigations are handled via AddIncludesWithFilters() which uses EF Include + 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/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_use_include.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_use_include.verified.txt new file mode 100644 index 00000000..ae8b44d9 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_use_include.verified.txt @@ -0,0 +1,33 @@ +{ + target: +{ + "data": { + "derivedChildEntities": [ + { + "property": "Child1" + }, + { + "property": "Child2" + } + ] + } +}, + sql: { + Text: +select d.Id, + d.ParentId, + d.TypedParentId, + d.Property, + case when b0.Id is null then cast (1 as bit) else cast (0 as bit) end, + b0.Id, + b0.Property +from DerivedChildEntities as d + left outer join + (select b.Id, + b.Property + from BaseEntities as b + where b.Discriminator = N'DerivedWithNavigationEntity') as b0 + on d.TypedParentId = b0.Id +order by d.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests_Cancellation.cs b/src/Tests/IntegrationTests/IntegrationTests_Cancellation.cs index f695e807..510f5699 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_Cancellation.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_Cancellation.cs @@ -1,23 +1,5 @@ -public class IntegrationTests_Cancellation +public partial class IntegrationTests { - static SqlInstance sqlInstance; - - static IntegrationTests_Cancellation() => - sqlInstance = new( - buildTemplate: async data => - { - var database = data.Database; - await database.EnsureCreatedAsync(); - }, - constructInstance: builder => - { - builder.ConfigureWarnings(_ => - _.Ignore( - CoreEventId.NavigationBaseIncludeIgnored, - CoreEventId.ShadowForeignKeyPropertyCreated, - CoreEventId.CollectionWithoutComparer)); - return new(builder.Options); - }); [Fact] public async Task QueryConnection_WithCancelledToken_ThrowsOperationCanceledException() @@ -132,10 +114,4 @@ public async Task QueryConnection_WithOperationCancelled_ThrowsOperationCanceled await Assert.ThrowsAnyAsync(async () => await executer.ExecuteAsync(options)); } - - static IEnumerable GetGraphQlTypes() => - typeof(IntegrationTests_Cancellation) - .Assembly - .GetTypes() - .Where(_ => !_.IsAbstract && _.IsAssignableTo()); } diff --git a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs index 658f850d..dfc8876f 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs @@ -44,6 +44,65 @@ public async Task Named_record_filter_projection_should_select_minimal_columns() await RunQuery(database, query, null, filters, false, [parent1, parent2, entity1, entity2, entity3]); } + /// + /// Tests that filter projections accessing ABSTRACT navigation properties work correctly. + /// When a filter projection accesses properties from an abstract navigation: + /// 1. The navigation should be loaded via EF Include (not SELECT projection) + /// 2. The projection should be skipped to avoid SelectExpressionBuilder failures + /// 3. The full entity with all columns should be loaded + /// This prevents "Can't compile a NewExpression with a constructor declared on an abstract class" errors + /// + [Fact] + public async Task Filter_projection_with_abstract_navigation_should_use_include() + { + var query = + """ + { + derivedChildEntities + { + property + } + } + """; + + var parentId = Guid.NewGuid(); + var child1Id = Guid.NewGuid(); + var child2Id = Guid.NewGuid(); + + var parent = new DerivedWithNavigationEntity + { + Id = parentId, + Property = "Parent1" + }; + var child1 = new DerivedChildEntity + { + Id = child1Id, + Property = "Child1", + TypedParent = parent + }; + var child2 = new DerivedChildEntity + { + Id = child2Id, + Property = "Child2", + TypedParent = parent + }; + + var filters = new Filters(); + + // Filter projection accesses properties from abstract BaseEntity navigation + // This should use Include instead of projection to avoid abstract type construction errors + filters.For().Add( + projection: c => new AbstractNavFilterProjection( + c.TypedParent != null ? c.TypedParent.Id : null, + c.TypedParent != null ? c.TypedParent.Property : null, + c.Id), + filter: (_, _, _, projection) => projection.ParentId == parentId); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [parent, child1, child2]); + } + // ReSharper disable NotAccessedPositionalProperty.Local record ChildFilterProjection(Guid? ParentId, string? ParentProperty, Guid ChildId); + record AbstractNavFilterProjection(Guid? ParentId, string? ParentProperty, Guid ChildId); }