diff --git a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs index bc321699..6ae1ed50 100644 --- a/src/GraphQL.EntityFramework/Filters/FilterEntry.cs +++ b/src/GraphQL.EntityFramework/Filters/FilterEntry.cs @@ -4,6 +4,7 @@ class FilterEntry : IFilterEntry { Func> filter; Func? compiledProjection; + IReadOnlySet requiredPropertyNames; public FilterEntry( Func> filter, @@ -13,18 +14,200 @@ public FilterEntry( if (projection is null) { compiledProjection = null; - RequiredPropertyNames = new HashSet(); + requiredPropertyNames = new HashSet(); } else { var compiled = projection.Compile(); compiledProjection = entity => compiled((TEntity)entity); - RequiredPropertyNames = ProjectionAnalyzer.ExtractRequiredProperties(projection); - ValidateProjectionCompatibility(projection, RequiredPropertyNames); + requiredPropertyNames = ProjectionAnalyzer.ExtractRequiredProperties(projection); + ValidateProjectionCompatibility(projection, requiredPropertyNames); } } - public IReadOnlySet RequiredPropertyNames { get; } + public IReadOnlySet RequiredPropertyNames => requiredPropertyNames; + + public FieldProjectionInfo AddRequirements( + FieldProjectionInfo projection, + IReadOnlyDictionary? navigationProperties) + { + if (requiredPropertyNames.Count == 0) + { + return projection; + } + + // Separate simple fields and navigation paths + var scalarFieldsToAdd = new List(); + var navigationPaths = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + foreach (var field in requiredPropertyNames) + { + if (field.Contains('.')) + { + // Navigation path like "Parent.Id" + var parts = field.Split('.', 2); + var navName = parts[0]; + var navProperty = parts[1]; + + if (!navigationPaths.TryGetValue(navName, out var properties)) + { + properties = new(StringComparer.OrdinalIgnoreCase); + navigationPaths[navName] = properties; + } + + // Only add if it doesn't contain further dots (single-level navigation) + if (!navProperty.Contains('.')) + { + properties.Add(navProperty); + } + } + else + { + // Simple field - check if it's a navigation property + var isNavigation = navigationProperties?.ContainsKey(field) == true; + + if (isNavigation || + projection.ScalarFields.Contains(field) || + projection.KeyNames?.Contains(field, StringComparer.OrdinalIgnoreCase) == true) + { + // Skip navigation names - they'll be handled via navigation paths + continue; + } + + scalarFieldsToAdd.Add(field); + } + } + + // Merge scalar fields + var mergedScalars = new HashSet(projection.ScalarFields, StringComparer.OrdinalIgnoreCase); + foreach (var field in scalarFieldsToAdd) + { + mergedScalars.Add(field); + } + + // Merge navigations + var infos = projection.Navigations; + Dictionary mergedNavigations; + if (infos == null) + { + mergedNavigations = []; + } + else + { + mergedNavigations = new(infos); + } + + // Process navigation paths from filter fields + foreach (var (navName, requiredProps) in navigationPaths) + { + // Skip if no navigation metadata available for this entity type + if (navigationProperties == null) + { + continue; + } + + // Try to find the navigation - use case-insensitive search + Navigation? navMetadata = null; + foreach (var (key, value) in navigationProperties) + { + if (string.Equals(key, navName, StringComparison.OrdinalIgnoreCase)) + { + navMetadata = value; + break; + } + } + + if (navMetadata == null) + { + continue; + } + + var navType = navMetadata.Type; + if (mergedNavigations.TryGetValue(navName, out var existingNav)) + { + // 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) + { + updatedScalars.Add(prop); + } + + var updatedProjection = existingNav.Projection with + { + ScalarFields = updatedScalars + }; + mergedNavigations[navName] = existingNav with + { + Projection = updatedProjection + }; + } + else + { + // Create navigation projection for filter-only navigations + // Note: For abstract types, SelectExpressionBuilder.TryBuild will return false, + // causing the entire projection to fail. This is intentional - it ensures + // Include (added in AddFilterNavigationIncludes) is used instead of Select. + // Don't include key/FK columns for filter-only navigations - the filter only + // needs the specific properties it accesses. + var navProjection = new FieldProjectionInfo(requiredProps, null, null, null); + mergedNavigations[navName] = new(navType, navMetadata.IsCollection, navProjection); + } + } + + return projection with + { + ScalarFields = mergedScalars, + Navigations = mergedNavigations + }; + } + + public IEnumerable GetAbstractNavigationIncludes( + IReadOnlyDictionary? navigationProperties) + { + if (navigationProperties == null) + { + yield break; + } + + // Extract navigation names from filter fields (paths like "Parent.Property" -> "Parent") + var navigationNames = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var field in requiredPropertyNames) + { + if (field.Contains('.')) + { + var navName = field[..field.IndexOf('.')]; + navigationNames.Add(navName); + } + } + + // Return only navigations that have abstract types + foreach (var navName in navigationNames) + { + // Find navigation in metadata (case-insensitive) + Navigation? navMetadata = null; + string? actualNavName = null; + foreach (var (key, value) in navigationProperties) + { + if (string.Equals(key, navName, StringComparison.OrdinalIgnoreCase)) + { + navMetadata = value; + actualNavName = key; + break; + } + } + + if (navMetadata == null || actualNavName == null) + { + continue; + } + + // Only return abstract types - concrete types can use projection + if (navMetadata.Type.IsAbstract) + { + yield return navMetadata.Name; + } + } + } public Task ShouldIncludeWithProjection( object userContext, diff --git a/src/GraphQL.EntityFramework/Filters/Filters.cs b/src/GraphQL.EntityFramework/Filters/Filters.cs index 0254a6c8..9325ac43 100644 --- a/src/GraphQL.EntityFramework/Filters/Filters.cs +++ b/src/GraphQL.EntityFramework/Filters/Filters.cs @@ -76,38 +76,36 @@ internal void Add( Dictionary> entries = []; - public IReadOnlySet GetRequiredFilterProperties() + /// + /// Get all filters that apply to the specified entity type (including base type filters). + /// + internal IEnumerable> GetFilters() where TEntity : class { - var result = new HashSet(StringComparer.OrdinalIgnoreCase); - var filterEntries = FindFilters(); - - foreach (var entry in filterEntries) - { - foreach (var prop in entry.RequiredPropertyNames) - { - result.Add(prop); - } - } - - return result; + var type = typeof(TEntity); + return entries + .Where(_ => _.Key.IsAssignableFrom(type)) + .Select(_ => _.Value); } - public IReadOnlyDictionary> GetAllRequiredFilterProperties() - { - var result = new Dictionary>(); + /// + /// Get all filters that apply to the specified entity type (including base type filters). + /// + internal IEnumerable> GetFilters(Type entityType) => + entries + .Where(_ => _.Key.IsAssignableFrom(entityType)) + .Select(_ => _.Value); - foreach (var (entityType, entry) in entries) - { - var props = entry.RequiredPropertyNames; - if (props.Count > 0) - { - result[entityType] = props; - } - } + /// + /// Get all registered filter entries. + /// + internal IEnumerable> GetAllFilters() => + entries.Values; - return result; - } + /// + /// Returns true if there are any filters registered. + /// + internal bool HasFilters => entries.Count > 0; internal virtual async Task> ApplyFilter( IEnumerable result, @@ -121,7 +119,7 @@ internal virtual async Task> ApplyFilter( return result; } - var filterEntries = FindFilters().ToList(); + var filterEntries = GetFilters().ToList(); if (filterEntries.Count == 0) { return result; @@ -175,7 +173,7 @@ internal virtual async Task ShouldInclude( return true; } - var filterEntries = FindFilters().ToList(); + var filterEntries = GetFilters().ToList(); if (filterEntries.Count == 0) { return true; @@ -183,14 +181,4 @@ internal virtual async Task ShouldInclude( return await ShouldIncludeItem(userContext, data, userPrincipal, item, filterEntries); } - - List> FindFilters() - where TEntity : class - { - var type = typeof(TEntity); - return entries - .Where(_ => _.Key.IsAssignableFrom(type)) - .Select(_ => _.Value) - .ToList(); - } } diff --git a/src/GraphQL.EntityFramework/Filters/IFilterEntry.cs b/src/GraphQL.EntityFramework/Filters/IFilterEntry.cs index 91ddef6e..b001607e 100644 --- a/src/GraphQL.EntityFramework/Filters/IFilterEntry.cs +++ b/src/GraphQL.EntityFramework/Filters/IFilterEntry.cs @@ -1,7 +1,26 @@ interface IFilterEntry where TDbContext : DbContext { - IReadOnlySet RequiredPropertyNames { get; } + /// + /// Add this filter's requirements to the projection. + /// Returns the updated projection with filter-required fields and navigations merged in. + /// + /// The current projection to merge requirements into. + /// Navigation property metadata for the entity type. + /// Updated projection with filter requirements included. + FieldProjectionInfo AddRequirements( + FieldProjectionInfo projection, + IReadOnlyDictionary? navigationProperties); + + /// + /// Get navigation names where the navigation type is abstract. + /// These navigations need Include() instead of projection because + /// abstract types cannot be instantiated in a Select expression. + /// + /// Navigation property metadata for the entity type. + /// Navigation names that require Include due to abstract types. + IEnumerable GetAbstractNavigationIncludes( + IReadOnlyDictionary? navigationProperties); Task ShouldIncludeWithProjection( object userContext, diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs index cbd91391..77e89468 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_First.cs @@ -163,10 +163,7 @@ FieldType BuildFirstField( query = query.AsNoTracking(); } - // 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, fieldContext.Filters); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -176,7 +173,7 @@ FieldType BuildFirstField( // 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)) + if (includeAppender.TryGetProjectionExpressionWithFilters(context, fieldContext.Filters, out var selectExpr)) { query = query.Select(selectExpr); } diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Navigation.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Navigation.cs index 8d87c15d..19798010 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Navigation.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Navigation.cs @@ -119,28 +119,36 @@ public FieldBuilder AddNavigationField( /// /// Gets the navigation paths required by filters for reloading entities. - /// Returns just the navigation parts (not prefixed with field name). + /// Returns navigation names that have abstract types (these need Include for reload). /// IReadOnlyList GetFilterRequiredNavPathsForReload() where TReturn : class { var filters = resolveFilters?.Invoke(null!); - if (filters == null) + if (filters is not {HasFilters: true}) { return []; } - var requiredProps = filters.GetRequiredFilterProperties(); - var navigationPaths = new HashSet(StringComparer.OrdinalIgnoreCase); + // Get navigation properties for TReturn + if (!Navigations.TryGetValue(typeof(TReturn), out var navigationProperties)) + { + return []; + } - foreach (var prop in requiredProps) + // Get all abstract navigation includes from filters that apply to TReturn + var relevantFilters = filters.GetFilters().ToList(); + if (relevantFilters.Count == 0) + { + return []; + } + + var navigationPaths = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var filter in relevantFilters) { - var lastDot = prop.LastIndexOf('.'); - if (lastDot > 0) + foreach (var include in filter.GetAbstractNavigationIncludes(navigationProperties)) { - // e.g., "TravelRequest.GroupOwnerId" -> "TravelRequest" - var navPath = prop[..lastDot]; - navigationPaths.Add(navPath); + navigationPaths.Add(include); } } diff --git a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs index 33397f33..5eba1e13 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Queryable.cs @@ -109,10 +109,7 @@ FieldType BuildQueryField( query = query.AsNoTracking(); } - // 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, fieldContext.Filters); if (!omitQueryArguments) { query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); @@ -125,7 +122,7 @@ FieldType BuildQueryField( // 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)) + if (includeAppender.TryGetProjectionExpressionWithFilters(context, fieldContext.Filters, 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 28ff9c8e..fe90a848 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_QueryableConnection.cs @@ -155,10 +155,7 @@ ConnectionBuilder AddQueryableConnection( query = query.AsNoTracking(); } - // 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, fieldContext.Filters); query = query.ApplyGraphQlArguments(context, names, true, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -168,7 +165,7 @@ ConnectionBuilder AddQueryableConnection( // 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)) + if (includeAppender.TryGetProjectionExpressionWithFilters(context, fieldContext.Filters, 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 fcc2bee1..051aaf93 100644 --- a/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs +++ b/src/GraphQL.EntityFramework/GraphApi/EfGraphQLService_Single.cs @@ -163,10 +163,7 @@ FieldType BuildSingleField( query = query.AsNoTracking(); } - // 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, fieldContext.Filters); query = query.ApplyGraphQlArguments(context, names, false, omitQueryArguments); // Apply column projection based on requested GraphQL fields @@ -176,7 +173,7 @@ FieldType BuildSingleField( // 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)) + if (includeAppender.TryGetProjectionExpressionWithFilters(context, fieldContext.Filters, out var selectExpr)) { query = query.Select(selectExpr); } diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index 2f2cd975..2674938f 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -1,4 +1,4 @@ -class IncludeAppender( +class IncludeAppender( IReadOnlyDictionary> navigations, IReadOnlyDictionary> keyNames, IReadOnlyDictionary> foreignKeys) @@ -20,17 +20,19 @@ public IQueryable AddIncludes(IQueryable query, IResolveFie return AddIncludes(query, context, navigationProperty); } - public IQueryable AddIncludesWithFilters( + public IQueryable AddIncludesWithFilters( IQueryable query, IResolveFieldContext context, - IReadOnlyDictionary>? allFilterFields) + Filters? filters) + where TDbContext : DbContext where TItem : class => - AddIncludesWithFiltersAndDetectNavigations(query, context, allFilterFields); + AddIncludesWithFiltersAndDetectNavigations(query, context, filters); - internal IQueryable AddIncludesWithFiltersAndDetectNavigations( + internal IQueryable AddIncludesWithFiltersAndDetectNavigations( IQueryable query, IResolveFieldContext context, - IReadOnlyDictionary>? allFilterFields = null) + Filters? filters = null) + where TDbContext : DbContext where TItem : class { // Include cannot be applied to queries that have already been projected (e.g., after Select). @@ -46,9 +48,9 @@ internal IQueryable AddIncludesWithFiltersAndDetectNavigations( // Add includes for filter-required navigations. // While projection handles most cases, abstract navigation types cannot be projected // (can't do "new AbstractType { ... }"). In those cases, Include is needed as fallback. - if (allFilterFields is { Count: > 0 }) + if (filters is { HasFilters: true }) { - query = AddFilterNavigationIncludes(query, allFilterFields, typeof(TItem)); + query = AddFilterNavigationIncludes(query, filters, typeof(TItem)); } return query; @@ -98,71 +100,40 @@ static bool HasSelectInQueryChain(Expression expression) return false; } - IQueryable AddFilterNavigationIncludes( + IQueryable AddFilterNavigationIncludes( IQueryable query, - IReadOnlyDictionary> allFilterFields, + Filters filters, Type entityType) + where TDbContext : DbContext where TItem : class { - // Get filter fields for this entity type and its base types - var relevantFilterFields = new List(); - foreach (var (filterType, filterFields) in allFilterFields) - { - if (filterType.IsAssignableFrom(entityType)) - { - relevantFilterFields.AddRange(filterFields); - } - } - - if (relevantFilterFields.Count == 0) + // Get navigation metadata for this entity type + if (!navigations.TryGetValue(entityType, out var navigationProperties)) { return query; } - // Get navigation metadata for this entity type - if (!navigations.TryGetValue(entityType, out var navigationProperties)) + // Get filters that apply to this entity type + var relevantFilters = filters.GetFilters(entityType).ToList(); + if (relevantFilters.Count == 0) { return query; } - // Extract unique navigation names from filter fields (paths like "TravelRequest.Status" -> "TravelRequest") - var navigationNames = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var field in relevantFilterFields) + // Collect all abstract navigation includes from filters + var abstractIncludes = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var filter in relevantFilters) { - if (field.Contains('.')) + foreach (var include in filter.GetAbstractNavigationIncludes(navigationProperties)) { - var navName = field[..field.IndexOf('.')]; - navigationNames.Add(navName); + abstractIncludes.Add(include); } } // Add Include for each filter-required navigation that has an abstract type - // (Non-abstract navigations can be handled by projection) - foreach (var navName in navigationNames) + foreach (var navName in abstractIncludes) { - // Find navigation in metadata (case-insensitive) - Navigation? navMetadata = null; - string? actualNavName = null; - foreach (var (key, value) in navigationProperties) - { - if (string.Equals(key, navName, StringComparison.OrdinalIgnoreCase)) - { - navMetadata = value; - actualNavName = key; - break; - } - } - - if (navMetadata == null || actualNavName == null) - { - continue; - } - - // Only add Include for abstract types - concrete types can use projection - if (navMetadata.Type.IsAbstract) - { - query = query.Include(navMetadata.Name); - } + query = query.Include(navName); } return query; @@ -184,10 +155,11 @@ IQueryable AddFilterNavigationIncludes( return GetProjectionInfo(context, navigationProperties, keys, fks); } - public bool TryGetProjectionExpressionWithFilters( + public bool TryGetProjectionExpressionWithFilters( IResolveFieldContext context, - IReadOnlyDictionary>? allFilterFields, + Filters? filters, [NotNullWhen(true)] out Expression>? expression) + where TDbContext : DbContext where TItem : class { expression = null; @@ -199,176 +171,59 @@ public bool TryGetProjectionExpressionWithFilters( } // Merge filter fields if provided (recursively for navigations) - if (allFilterFields is { Count: > 0 }) + if (filters is { HasFilters: true }) { - projection = MergeFilterFieldsIntoProjection(projection, allFilterFields, typeof(TItem)); + projection = MergeFilterFieldsIntoProjection(projection, filters, typeof(TItem)); } return SelectExpressionBuilder.TryBuild(projection, keyNames, out expression); } - FieldProjectionInfo MergeFilterFieldsIntoProjection( + FieldProjectionInfo MergeFilterFieldsIntoProjection( FieldProjectionInfo projection, - IReadOnlyDictionary> allFilterFields, + Filters filters, Type entityType) + where TDbContext : DbContext { - // Get filter fields for this entity type and its base types - var relevantFilterFields = new List(); - foreach (var (filterType, filterFields) in allFilterFields) - { - if (filterType.IsAssignableFrom(entityType)) - { - relevantFilterFields.AddRange(filterFields); - } - } - - if (relevantFilterFields.Count == 0) - { - return projection; - } - // Get navigation metadata for this entity type navigations.TryGetValue(entityType, out var navigationProperties); - // Separate simple fields and navigation paths - var scalarFieldsToAdd = new List(); - var navigationPaths = new Dictionary>(StringComparer.OrdinalIgnoreCase); - - foreach (var field in relevantFilterFields) - { - if (field.Contains('.')) - { - // Navigation path like "Parent.Id" - var parts = field.Split('.', 2); - var navName = parts[0]; - var navProperty = parts[1]; - - if (!navigationPaths.TryGetValue(navName, out var properties)) - { - properties = new(StringComparer.OrdinalIgnoreCase); - navigationPaths[navName] = properties; - } - - // Only add if it doesn't contain further dots (single-level navigation) - if (!navProperty.Contains('.')) - { - properties.Add(navProperty); - } - } - else - { - // Simple field - check if it's a navigation property - var isNavigation = navigationProperties?.ContainsKey(field) == true; - - if (isNavigation || - projection.ScalarFields.Contains(field) || - projection.KeyNames?.Contains(field, StringComparer.OrdinalIgnoreCase) == true) - { - // Skip navigation names - they'll be handled via navigation paths - continue; - } - - scalarFieldsToAdd.Add(field); - } - } - - // Merge scalar fields - var mergedScalars = new HashSet(projection.ScalarFields, StringComparer.OrdinalIgnoreCase); - foreach (var field in scalarFieldsToAdd) mergedScalars.Add(field); + // Get filters that apply to this entity type + var relevantFilters = filters.GetFilters(entityType).ToList(); - // Merge navigations - var infos = projection.Navigations; - Dictionary mergedNavigations; - if (infos == null) + // Apply each filter's requirements to the projection + foreach (var filter in relevantFilters) { - mergedNavigations = []; - } - else - { - mergedNavigations = new(infos); + projection = filter.AddRequirements(projection, navigationProperties); } - - // Process navigation paths from filter fields - foreach (var (navName, requiredProps) in navigationPaths) + // Recursively process existing navigations + if (projection.Navigations is { Count: > 0 }) { - // Skip if no navigation metadata available for this entity type - if (navigationProperties == null) - { - continue; - } + var updatedNavigations = new Dictionary(projection.Navigations); - // Try to find the navigation - use case-insensitive search - Navigation? navMetadata = null; - foreach (var (key, value) in navigationProperties) + foreach (var (navName, navProjection) in projection.Navigations) { - if (string.Equals(key, navName, StringComparison.OrdinalIgnoreCase)) + var updatedProjection = MergeFilterFieldsIntoProjection(navProjection.Projection, filters, navProjection.EntityType); + if (updatedProjection != navProjection.Projection) { - navMetadata = value; - break; + updatedNavigations[navName] = navProjection with + { + Projection = updatedProjection + }; } } - if (navMetadata == null) - { - continue; - } - - var navType = navMetadata.Type; - - if (mergedNavigations.TryGetValue(navName, out var existingNav)) + if (updatedNavigations != projection.Navigations) { - // 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) - { - updatedScalars.Add(prop); - } - - var updatedProjection = existingNav.Projection with - { - ScalarFields = updatedScalars - }; - updatedProjection = MergeFilterFieldsIntoProjection(updatedProjection, allFilterFields, navType); - mergedNavigations[navName] = existingNav with + projection = projection with { - Projection = updatedProjection + Navigations = updatedNavigations }; } - else - { - // Create navigation projection for filter-only navigations - // Note: For abstract types, SelectExpressionBuilder.TryBuild will return false, - // causing the entire projection to fail. This is intentional - it ensures - // Include (added in AddFilterNavigationIncludes) is used instead of Select. - var navProjection = new FieldProjectionInfo(requiredProps, null, null, null); - navProjection = MergeFilterFieldsIntoProjection(navProjection, allFilterFields, navType); - mergedNavigations[navName] = new(navType, navMetadata.IsCollection, navProjection); - } } - // Recursively process existing navigations - if (projection.Navigations != null) - { - foreach (var (navName, navProjection) in projection.Navigations) - { - if (!mergedNavigations.ContainsKey(navName)) - { - var updated = MergeFilterFieldsIntoProjection(navProjection.Projection, allFilterFields, navProjection.EntityType); - mergedNavigations[navName] = navProjection with - { - Projection = updated - }; - } - } - } - - return projection - with - { - ScalarFields = mergedScalars, - Navigations = mergedNavigations - }; + return projection; } FieldProjectionInfo GetProjectionInfo( @@ -460,48 +315,6 @@ void ProcessProjectionField( } } - // Check if this field has include metadata (fallback for abstract types, or legacy/obsolete approach) - if (TryGetIncludeMetadata(fieldInfo.FieldType, out var includeNames)) - { - // It's a navigation field - include ALL navigation properties from metadata - var addedAny = false; - foreach (var navName in includeNames) - { - Navigation? navigation = null; - navigationProperties?.TryGetValue(navName, out navigation); - - if (navigation != null && !navProjections.ContainsKey(navigation.Name)) - { - var navType = navigation.Type; - navigations.TryGetValue(navType, out var nestedNavProps); - keyNames.TryGetValue(navType, out var nestedKeys); - foreignKeys.TryGetValue(navType, out var nestedFks); - - // Only the first (primary) navigation gets the nested projection from the query - // Other navigations get empty projections (select all their keys and foreign keys) - var nestedProjection = addedAny - ? new([], nestedKeys ?? [], nestedFks ?? new HashSet(), []) - : GetNestedProjection( - fieldInfo.Field.SelectionSet, - nestedNavProps, - nestedKeys, - nestedFks, - context); - - navProjections[navigation.Name] = new( - navType, - navigation.IsCollection, - nestedProjection); - addedAny = true; - } - } - - if (addedAny) - { - return; - } - } - // Check if this field is a navigation property by name Navigation? navByName = null; navigationProperties?.TryGetValue(fieldName, out navByName); diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_using_anonymous_type_with_nested_navigation_property.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_using_anonymous_type_with_nested_navigation_property.verified.txt index 36d38b2b..f33ff800 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_using_anonymous_type_with_nested_navigation_property.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_using_anonymous_type_with_nested_navigation_property.verified.txt @@ -20,80 +20,32 @@ ] } }, - sql: [ - { - Text: + sql: { + Text: select f.Id, f.Property, - f0.Id, - f0.Property, - f0.Age, - f0.IsActive + s.Id, + s.Property, + s.Age, + s.IsActive, + s.c, + s.Property0, + s.Id0 from FilterParentEntities as f left outer join - FilterChildEntities as f0 - on f.Id = f0.ParentId -order by f.Property, f.Id, f0.Id - }, - { - Text: -select f.Id, - f.Age, - f.CreatedAt, - f.IsActive, - f.NullableAge, - f.NullableCreatedAt, - f.NullableIsActive, - f.ParentId, - f.Property, - f0.Id, - f0.Field1, - f0.Field10, - f0.Field2, - f0.Field3, - f0.Field4, - f0.Field5, - f0.Field6, - f0.Field7, - f0.Field8, - f0.Field9, - f0.Property -from FilterChildEntities as f - left outer join - FilterParentEntities as f0 - on f.ParentId = f0.Id -where f.Id in ('Guid_1', 'Guid_2', 'Guid_3') -order by f.Property - }, - { - Text: -select f.Id, - f.Age, - f.CreatedAt, - f.IsActive, - f.NullableAge, - f.NullableCreatedAt, - f.NullableIsActive, - f.ParentId, - f.Property, - f0.Id, - f0.Field1, - f0.Field10, - f0.Field2, - f0.Field3, - f0.Field4, - f0.Field5, - f0.Field6, - f0.Field7, - f0.Field8, - f0.Field9, - f0.Property -from FilterChildEntities as f - left outer join - FilterParentEntities as f0 - on f.ParentId = f0.Id -where f.Id = 'Guid_4' -order by f.Property - } - ] + (select f0.Id, + f0.Property, + f0.Age, + f0.IsActive, + case when f1.Id is null then cast (1 as bit) else cast (0 as bit) end as c, + f1.Property as Property0, + f1.Id as Id0, + f0.ParentId + from FilterChildEntities as f0 + left outer join + FilterParentEntities as f1 + on f0.ParentId = f1.Id) as s + on f.Id = s.ParentId +order by f.Property, f.Id, s.Id + } } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs b/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs deleted file mode 100644 index cac5ddc2..00000000 --- a/src/Tests/IntegrationTests/IntegrationTests_abstract_filter_validation.cs +++ /dev/null @@ -1,147 +0,0 @@ -public partial class IntegrationTests -{ - /// - /// Test that explicit projection accessing properties through abstract navigation succeeds. - /// DerivedChildEntity.Parent is BaseEntity (abstract), but explicit projections that extract - /// specific scalar properties are ALLOWED because EF Core can project scalar properties - /// through abstract navigations efficiently. Only identity projections are problematic. - /// - [Fact] - public void Explicit_projection_with_abstract_navigation_property_succeeds() - { - var filters = new Filters(); - - // Projection accesses Parent.Property where Parent is BaseEntity (abstract) - // This is ALLOWED - EF Core generates efficient JOIN to get only the needed column - filters.For().Add( - projection: c => c.Parent!.Property, - filter: (_, _, _, prop) => prop == "test"); - - // Verify the filter was registered with the expected property - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Contains("Parent.Property", requiredProps); - } - - /// - /// Test that anonymous type projection with abstract navigation succeeds. - /// Explicit projections (including anonymous types) that extract specific properties - /// through abstract navigations are ALLOWED - EF Core handles this efficiently. - /// - [Fact] - public void Anonymous_type_projection_with_abstract_navigation_succeeds() - { - var filters = new Filters(); - - // Anonymous type projection extracting properties through abstract Parent - // This is ALLOWED - EF Core generates efficient SQL with JOINs - filters.For().Add( - projection: c => new { c.Id, ParentProperty = c.Parent!.Property }, - filter: (_, _, _, proj) => proj.ParentProperty == "test"); - - // Verify the filter was registered with expected properties - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Contains("Id", requiredProps); - Assert.Contains("Parent.Property", requiredProps); - } - - /// - /// 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 is { Property: "test", ParentId: not 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 succeeds. - /// Explicit projections that extract specific scalar properties are allowed. - /// - [Fact] - public void Projection_with_nested_abstract_navigation_property_succeeds() - { - var filters = new Filters(); - - // Accessing nested property through abstract parent is allowed for explicit projections - filters.For().Add( - projection: c => c.Parent!.Status, - filter: (_, _, _, status) => status == "Active"); - - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Contains("Parent.Status", requiredProps); - } - - /// - /// Test that identity projection with filter accessing abstract navigation is NOT caught at runtime. - /// The runtime cannot analyze the filter delegate. The GQLEF007 analyzer catches this at compile time. - /// - [Fact] - public void Identity_projection_with_abstract_navigation_in_filter_not_caught_at_runtime() - { - var filters = new Filters(); - - // Identity projection where filter accesses abstract Parent - // This is NOT caught at runtime because: - // 1. The projection `c => c` has no property accesses to analyze - // 2. The filter is a compiled delegate, not an expression tree - // The GQLEF007 analyzer catches this at compile time instead. - filters.For().Add( - projection: c => c, - filter: (_, _, _, c) => c.Parent!.Property == "test"); - - // Identity projection extracts no properties - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Empty(requiredProps); - } -} diff --git a/src/Tests/UnitTests/FilterEntryValidationTests.cs b/src/Tests/UnitTests/FilterEntryValidationTests.cs deleted file mode 100644 index 0c5ece65..00000000 --- a/src/Tests/UnitTests/FilterEntryValidationTests.cs +++ /dev/null @@ -1,112 +0,0 @@ -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_succeeds() - { - var filters = new Filters(); - - // Explicit projections that extract specific properties through abstract navigations are ALLOWED - // because EF Core can project scalar properties through abstract navigations efficiently. - // Only identity projections are problematic. - filters.For().Add( - projection: e => e.Parent!.Property, - filter: (_, _, _, prop) => prop == "test"); - - // Verify the filter was registered with the expected property - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Contains("Parent.Property", requiredProps); - } - - [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_succeeds() - { - var filters = new Filters(); - - // Explicit projections (including anonymous types) that extract specific properties - // through abstract navigations are ALLOWED - EF Core handles this efficiently. - filters.For().Add( - projection: e => new { e.Id, ParentProp = e.Parent!.Property }, - filter: (_, _, _, proj) => proj.ParentProp == "test"); - - // Verify the filter was registered with expected properties - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Contains("Id", requiredProps); - Assert.Contains("Parent.Property", requiredProps); - } - - [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"); - } - - [Fact] - public void Identity_projection_with_abstract_navigation_in_filter_not_caught_at_runtime() - { - var filters = new Filters(); - - // Identity projection where filter accesses abstract navigation - // This is NOT caught at runtime because: - // 1. The projection `e => e` has no property accesses to analyze - // 2. The filter is a compiled delegate, not an expression tree - // The GQLEF007 analyzer catches this at compile time instead. - filters.For().Add( - projection: e => e, - filter: (_, _, _, e) => e.Parent!.Property == "test"); - - // Identity projection extracts no properties - var requiredProps = filters.GetRequiredFilterProperties(); - Assert.Empty(requiredProps); - } -}