diff --git a/JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs b/JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs index 488ea11..7a3e72e 100644 --- a/JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs +++ b/JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs @@ -65,13 +65,18 @@ public void ApplyFilteredIncludes_WithSimpleIncludeFilter_BuildsCorrectExpressio new() { RelationshipPath = "comments", - FieldPath = "status", - Filter = new FilterParameter + FilterGroup = new FilterGroup { - Field = "status", - Operator = FilterOperator.Eq, - Value = "approved", - }, + Filters = new List + { + new() + { + Field = "status", + Operator = FilterOperator.Eq, + Value = "approved", + } + } + } }, }; @@ -94,24 +99,24 @@ public void ApplyFilteredIncludes_WithMultipleFiltersOnSameRelationship_Combines new() { RelationshipPath = "comments", - FieldPath = "status", - Filter = new FilterParameter + FilterGroup = new FilterGroup { - Field = "status", - Operator = FilterOperator.Eq, - Value = "approved", - }, - }, - new() - { - RelationshipPath = "comments", - FieldPath = "priority", - Filter = new FilterParameter - { - Field = "priority", - Operator = FilterOperator.Gt, - Value = "5", - }, + Filters = new List + { + new() + { + Field = "status", + Operator = FilterOperator.Eq, + Value = "approved", + }, + new() + { + Field = "priority", + Operator = FilterOperator.Gt, + Value = "5", + } + } + } }, }; @@ -148,13 +153,18 @@ public void ApplyFilteredIncludes_WithMixedFilteredAndUnfilteredIncludes_Handles new() { RelationshipPath = "comments", - FieldPath = "status", - Filter = new FilterParameter + FilterGroup = new FilterGroup { - Field = "status", - Operator = FilterOperator.Eq, - Value = "approved", - }, + Filters = new List + { + new() + { + Field = "status", + Operator = FilterOperator.Eq, + Value = "approved", + } + } + } }, // tags and author have no filters }; diff --git a/JsonApiToolkit.Tests/Extensions/IncludeFilterParserTests.cs b/JsonApiToolkit.Tests/Extensions/IncludeFilterParserTests.cs index 714a177..2fe1c36 100644 --- a/JsonApiToolkit.Tests/Extensions/IncludeFilterParserTests.cs +++ b/JsonApiToolkit.Tests/Extensions/IncludeFilterParserTests.cs @@ -98,8 +98,9 @@ public void SeparateIncludeFilters_WithSimpleIncludeFilter_SeparatesCorrectly() Assert.Single(includeFilters); Assert.Equal("comments", includeFilters[0].RelationshipPath); - Assert.Equal("status", includeFilters[0].FieldPath); - Assert.Equal("approved", includeFilters[0].Filter.Value); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("status", includeFilters[0].FilterGroup.Filters[0].Field); + Assert.Equal("approved", includeFilters[0].FilterGroup.Filters[0].Value); } [Fact] @@ -129,7 +130,8 @@ public void SeparateIncludeFilters_WithKebabCaseInclude_HandlesCorrectly() // Assert Assert.Single(includeFilters); Assert.Equal("cveComments", includeFilters[0].RelationshipPath); - Assert.Equal("companyCode", includeFilters[0].FieldPath); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("companyCode", includeFilters[0].FilterGroup.Filters[0].Field); } [Fact] @@ -159,7 +161,8 @@ public void SeparateIncludeFilters_WithNestedIncludeFilter_SeparatesCorrectly() // Assert Assert.Single(includeFilters); Assert.Equal("comments.author", includeFilters[0].RelationshipPath); - Assert.Equal("department", includeFilters[0].FieldPath); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("department", includeFilters[0].FilterGroup.Filters[0].Field); } [Fact] @@ -194,9 +197,11 @@ public void SeparateIncludeFilters_WithComplexOrFilter_HandlesCorrectly() ); // Assert - Assert.Equal(2, includeFilters.Count); - Assert.All(includeFilters, f => Assert.Equal("comments", f.RelationshipPath)); - Assert.All(includeFilters, f => Assert.Equal("companyCode", f.FieldPath)); + Assert.Single(includeFilters); + Assert.Equal("comments", includeFilters[0].RelationshipPath); + Assert.Equal(LogicalOperator.Or, includeFilters[0].FilterGroup.LogicalOperator); + Assert.Equal(2, includeFilters[0].FilterGroup.Filters.Count); + Assert.All(includeFilters[0].FilterGroup.Filters, f => Assert.Equal("companyCode", f.Field)); } [Fact] @@ -334,7 +339,8 @@ public void SeparateIncludeFilters_WithMixedMainAndIncludeFilters_SeparatesCorre Assert.Single(includeFilters); Assert.Equal("comments", includeFilters[0].RelationshipPath); - Assert.Equal("approved", includeFilters[0].FieldPath); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("approved", includeFilters[0].FilterGroup.Filters[0].Field); } [Fact] @@ -389,9 +395,12 @@ public void SeparateIncludeFilters_WithNestedGroups_HandlesCorrectly() Assert.Single(mainFilters.Filters); Assert.Equal("title", mainFilters.Filters[0].Field); - Assert.Equal(2, includeFilters.Count); - Assert.All(includeFilters, f => Assert.Equal("comments", f.RelationshipPath)); - Assert.All(includeFilters, f => Assert.Equal("status", f.FieldPath)); + Assert.Single(includeFilters); + Assert.Equal("comments", includeFilters[0].RelationshipPath); + // The OR group should be used directly (not wrapped) + Assert.Equal(LogicalOperator.Or, includeFilters[0].FilterGroup.LogicalOperator); + Assert.Equal(2, includeFilters[0].FilterGroup.Filters.Count); + Assert.All(includeFilters[0].FilterGroup.Filters, f => Assert.Equal("status", f.Field)); } [Fact] @@ -422,8 +431,9 @@ public void SeparateIncludeFilters_WithDeepNestedIncludeFilterUsingLeafName_Sepa Assert.Null(mainFilters); // No main filters expected Assert.Single(includeFilters); Assert.Equal("cve.cvecomments", includeFilters[0].RelationshipPath); - Assert.Equal("companyCode", includeFilters[0].FieldPath); - Assert.Equal("AA", includeFilters[0].Filter.Value); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("companyCode", includeFilters[0].FilterGroup.Filters[0].Field); + Assert.Equal("AA", includeFilters[0].FilterGroup.Filters[0].Value); } [Fact] @@ -455,7 +465,8 @@ public void SeparateIncludeFilters_WithDeepNestedIncludeFilterUsingKebabCase_Sep Assert.Single(includeFilters); // The relationship path is returned as the matched normalized path Assert.Equal("cve.cveComments", includeFilters[0].RelationshipPath); - Assert.Equal("companyCode", includeFilters[0].FieldPath); + Assert.Single(includeFilters[0].FilterGroup.Filters); + Assert.Equal("companyCode", includeFilters[0].FilterGroup.Filters[0].Field); } [Fact] @@ -492,12 +503,16 @@ public void SeparateIncludeFilters_WithMultipleDeepNestedFilters_SeparatesCorrec Assert.Null(mainFilters); Assert.Equal(2, includeFilters.Count); - var authorFilter = includeFilters.First(f => f.FieldPath == "name"); + var authorFilter = includeFilters.First(f => f.RelationshipPath == "posts.author"); Assert.Equal("posts.author", authorFilter.RelationshipPath); - Assert.Equal("John", authorFilter.Filter.Value); + Assert.Single(authorFilter.FilterGroup.Filters); + Assert.Equal("name", authorFilter.FilterGroup.Filters[0].Field); + Assert.Equal("John", authorFilter.FilterGroup.Filters[0].Value); - var commentFilter = includeFilters.First(f => f.FieldPath == "status"); + var commentFilter = includeFilters.First(f => f.RelationshipPath == "posts.comments"); Assert.Equal("posts.comments", commentFilter.RelationshipPath); - Assert.Equal("approved", commentFilter.Filter.Value); + Assert.Single(commentFilter.FilterGroup.Filters); + Assert.Equal("status", commentFilter.FilterGroup.Filters[0].Field); + Assert.Equal("approved", commentFilter.FilterGroup.Filters[0].Value); } } diff --git a/JsonApiToolkit/Extensions/Querying/Filtering/FilterExpressionBuilder.cs b/JsonApiToolkit/Extensions/Querying/Filtering/FilterExpressionBuilder.cs index e7eb41c..51c6bb5 100644 --- a/JsonApiToolkit/Extensions/Querying/Filtering/FilterExpressionBuilder.cs +++ b/JsonApiToolkit/Extensions/Querying/Filtering/FilterExpressionBuilder.cs @@ -18,6 +18,19 @@ public static class FilterExpressionBuilder ParameterExpression parameter, ILogger? logger = null ) + { + return BuildFilterExpression(group, parameter, typeof(T), logger); + } + + /// + /// Builds a composite filter expression from filter conditions and nested groups (non-generic overload). + /// + public static Expression? BuildFilterExpression( + FilterGroup group, + ParameterExpression parameter, + Type entityType, + ILogger? logger = null + ) { var expressions = new List(); @@ -31,7 +44,7 @@ public static class FilterExpressionBuilder else { PropertyInfo? property = QueryHelpers.GetPropertyByJsonName( - typeof(T), + entityType, filter.Field ); if (property == null) @@ -39,7 +52,7 @@ public static class FilterExpressionBuilder logger?.LogWarning( "Property '{Field}' not found on {Type}, skipping filter", filter.Field, - typeof(T).Name + entityType.Name ); continue; } @@ -58,7 +71,7 @@ public static class FilterExpressionBuilder foreach (FilterGroup nestedGroup in group.Groups) { - Expression? nestedExpr = BuildFilterExpression(nestedGroup, parameter, logger); + Expression? nestedExpr = BuildFilterExpression(nestedGroup, parameter, entityType, logger); if (nestedExpr != null) expressions.Add(nestedExpr); } diff --git a/JsonApiToolkit/Extensions/Querying/Filtering/IncludeFilterParser.cs b/JsonApiToolkit/Extensions/Querying/Filtering/IncludeFilterParser.cs index bd8be3d..a78c46d 100644 --- a/JsonApiToolkit/Extensions/Querying/Filtering/IncludeFilterParser.cs +++ b/JsonApiToolkit/Extensions/Querying/Filtering/IncludeFilterParser.cs @@ -24,10 +24,25 @@ List includeFilters if (filters == null) return (null, new List()); - var includeFilters = new List(); var normalizedIncludePaths = NormalizeIncludePaths(includePaths ?? new List()); - var mainFilters = ExtractIncludeFilters(filters, normalizedIncludePaths, includeFilters); + // Dictionary to group filters by relationship path + var filtersByRelationship = new Dictionary(StringComparer.OrdinalIgnoreCase); + + var mainFilters = ExtractIncludeFilters( + filters, + normalizedIncludePaths, + filtersByRelationship + ); + + // Convert dictionary to list of IncludeFilters + var includeFilters = filtersByRelationship + .Select(kvp => new IncludeFilter + { + RelationshipPath = kvp.Key, + FilterGroup = kvp.Value + }) + .ToList(); ValidateIncludeFilters(includeFilters, normalizedIncludePaths); @@ -37,7 +52,7 @@ List includeFilters private static FilterGroup? ExtractIncludeFilters( FilterGroup group, HashSet normalizedIncludePaths, - List includeFilters + Dictionary filtersByRelationship ) { var newGroup = new FilterGroup { LogicalOperator = group.LogicalOperator }; @@ -50,6 +65,9 @@ List includeFilters ); } + // Track filters for each relationship in this group + var localIncludeFilters = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var filter in group.Filters) { if ( @@ -61,14 +79,24 @@ out var fieldPath ) ) { - includeFilters.Add( - new IncludeFilter + // Add to local filters for this group + if (!localIncludeFilters.ContainsKey(relationshipPath)) + { + localIncludeFilters[relationshipPath] = new FilterGroup { - RelationshipPath = relationshipPath, - FieldPath = fieldPath, - Filter = filter, - } - ); + LogicalOperator = group.LogicalOperator + }; + } + + // Create a filter with the field path relative to the relationship + var relativeFilter = new FilterParameter + { + Field = fieldPath, + Operator = filter.Operator, + Value = filter.Value + }; + + localIncludeFilters[relationshipPath].Filters.Add(relativeFilter); } else { @@ -76,12 +104,13 @@ out var fieldPath } } + // Process nested groups foreach (var nestedGroup in group.Groups) { var processedNestedGroup = ExtractIncludeFilters( nestedGroup, normalizedIncludePaths, - includeFilters + filtersByRelationship ); if ( @@ -93,6 +122,39 @@ out var fieldPath } } + // Merge local include filters into the global dictionary + foreach (var kvp in localIncludeFilters) + { + if (!filtersByRelationship.ContainsKey(kvp.Key)) + { + // First time seeing this relationship - use its logical operator directly + filtersByRelationship[kvp.Key] = kvp.Value; + } + else + { + // Already have filters for this relationship - need to merge + var existing = filtersByRelationship[kvp.Key]; + + // If both groups have the same operator and no nested groups, merge filters + if (existing.LogicalOperator == kvp.Value.LogicalOperator + && existing.Groups.Count == 0 + && kvp.Value.Groups.Count == 0) + { + existing.Filters.AddRange(kvp.Value.Filters); + } + else + { + // Otherwise, need to combine with AND logic + var combined = new FilterGroup + { + LogicalOperator = LogicalOperator.And, + Groups = new List { existing, kvp.Value } + }; + filtersByRelationship[kvp.Key] = combined; + } + } + } + // Return null if the group is empty after extraction if (newGroup.Filters.Count == 0 && newGroup.Groups.Count == 0) return null; @@ -241,6 +303,25 @@ HashSet normalizedIncludePaths $"Filtered includes beyond 2 levels are not supported. Include path '{includeFilter.RelationshipPath}' has {depth} levels. Maximum supported: 2 levels." ); } + + // Count total filters in the group + int totalFilters = CountFiltersInGroup(includeFilter.FilterGroup); + if (totalFilters > MaxIncludeFilters) + { + throw new JsonApiBadRequestException( + $"Too many filters for relationship '{includeFilter.RelationshipPath}'. Maximum allowed: {MaxIncludeFilters}" + ); + } + } + } + + private static int CountFiltersInGroup(FilterGroup group) + { + int count = group.Filters.Count; + foreach (var nestedGroup in group.Groups) + { + count += CountFiltersInGroup(nestedGroup); } + return count; } } diff --git a/JsonApiToolkit/Extensions/Querying/Includes/FilteredIncludeBuilder.cs b/JsonApiToolkit/Extensions/Querying/Includes/FilteredIncludeBuilder.cs index 8f38c97..c195bcc 100644 --- a/JsonApiToolkit/Extensions/Querying/Includes/FilteredIncludeBuilder.cs +++ b/JsonApiToolkit/Extensions/Querying/Includes/FilteredIncludeBuilder.cs @@ -26,8 +26,7 @@ public static IQueryable ApplyFilteredIncludes( return query; var filtersByRelationship = includeFilters - .GroupBy(f => f.RelationshipPath, StringComparer.OrdinalIgnoreCase) - .ToDictionary(g => g.Key, g => g.ToList(), StringComparer.OrdinalIgnoreCase); + .ToDictionary(f => f.RelationshipPath, f => f.FilterGroup, StringComparer.OrdinalIgnoreCase); var sortedPaths = includePaths.OrderBy(p => p.Count(c => c == '.')).ToList(); @@ -36,11 +35,11 @@ public static IQueryable ApplyFilteredIncludes( var segments = includePath.Split('.'); if ( - filtersByRelationship.TryGetValue(includePath, out var filters) - && filters.Count > 0 + filtersByRelationship.TryGetValue(includePath, out var filterGroup) + && (filterGroup.Filters.Count > 0 || filterGroup.Groups.Count > 0) ) { - query = ApplyFilteredIncludeChain(query, segments, filters, typeof(T), logger); + query = ApplyFilteredIncludeChain(query, segments, filterGroup, typeof(T), logger); } else { @@ -54,7 +53,7 @@ public static IQueryable ApplyFilteredIncludes( private static IQueryable ApplyFilteredIncludeChain( IQueryable query, string[] pathSegments, - List filters, + FilterGroup filterGroup, Type rootType, ILogger? logger ) @@ -64,10 +63,10 @@ private static IQueryable ApplyFilteredIncludeChain( return query; if (pathSegments.Length == 1) - return ApplyFilteredIncludeWithFilters(query, pathSegments[0], filters, rootType, logger); + return ApplyFilteredIncludeWithFilters(query, pathSegments[0], filterGroup, rootType, logger); if (pathSegments.Length == 2) - return ApplyTwoLevelFilteredInclude(query, pathSegments, filters, rootType, logger); + return ApplyTwoLevelFilteredInclude(query, pathSegments, filterGroup, rootType, logger); logger?.LogWarning( "Filtered includes beyond 2 levels are not supported. Include path '{Path}' will use unfiltered include. Filters will be ignored.", @@ -79,7 +78,7 @@ private static IQueryable ApplyFilteredIncludeChain( private static IQueryable ApplyTwoLevelFilteredInclude( IQueryable query, string[] pathSegments, - List filters, + FilterGroup filterGroup, Type rootType, ILogger? logger ) @@ -130,19 +129,14 @@ private static IQueryable ApplyTwoLevelFilteredInclude( var secondNavAccess = Expression.Property(navParam, secondProperty); var filterParam = Expression.Parameter(elementType, "item"); - Expression? filterExpr = null; - foreach (var filter in filters) - { - var singleFilterExpr = BuildSingleFilterExpression(filterParam, filter); - if (singleFilterExpr != null) - { - filterExpr = - filterExpr == null - ? singleFilterExpr - : Expression.OrElse(filterExpr, singleFilterExpr); - } - } + // Use FilterExpressionBuilder to build the filter expression with proper logical operators + var filterExpr = FilterExpressionBuilder.BuildFilterExpression( + filterGroup, + filterParam, + elementType, + logger + ); if (filterExpr != null) { @@ -203,7 +197,7 @@ private static IQueryable ApplyTwoLevelFilteredInclude( private static IQueryable ApplyFilteredIncludeWithFilters( IQueryable query, string navigationPath, - List filters, + FilterGroup filterGroup, Type entityType, ILogger? logger ) @@ -225,7 +219,8 @@ private static IQueryable ApplyFilteredIncludeWithFilters( entityType, navigationProperty, elementType, - filters + filterGroup, + logger ); query = EfCoreIncludeExpressions.ApplyIncludeExpression(query, includeExpression); @@ -251,27 +246,21 @@ private static IQueryable ApplyFilteredIncludeWithFilters( Type entityType, PropertyInfo navigationProperty, Type elementType, - List filters + FilterGroup filterGroup, + ILogger? logger ) { var entityParameter = Expression.Parameter(entityType, "e"); var navigationAccess = Expression.Property(entityParameter, navigationProperty); var elementParameter = Expression.Parameter(elementType, "x"); - Expression? filterExpression = null; - - foreach (var filter in filters) - { - var singleFilterExpr = BuildSingleFilterExpression(elementParameter, filter); - - if (singleFilterExpr != null) - { - filterExpression = - filterExpression == null - ? singleFilterExpr - : Expression.OrElse(filterExpression, singleFilterExpr); - } - } + // Use FilterExpressionBuilder to build the filter expression with proper logical operators + var filterExpression = FilterExpressionBuilder.BuildFilterExpression( + filterGroup, + elementParameter, + elementType, + logger + ); if (filterExpression == null) return null; @@ -290,25 +279,6 @@ List filters return includeLambda; } - private static Expression? BuildSingleFilterExpression( - ParameterExpression parameter, - IncludeFilter filter - ) - { - var property = GetPropertyExpression(parameter, filter.FieldPath, parameter.Type); - if (property == null) - return null; - - var filterParam = new FilterParameter - { - Field = filter.FieldPath, - Operator = filter.Filter.Operator, - Value = filter.Filter.Value, - }; - - return FilterExpressionBuilder.BuildSingleFilterExpression(parameter, filterParam); - } - private static MemberExpression? GetPropertyExpression( Expression parameter, string propertyPath, diff --git a/JsonApiToolkit/JsonApiToolkit.csproj b/JsonApiToolkit/JsonApiToolkit.csproj index c074562..2f64b8e 100644 --- a/JsonApiToolkit/JsonApiToolkit.csproj +++ b/JsonApiToolkit/JsonApiToolkit.csproj @@ -7,7 +7,7 @@ Intility.JsonApiToolkit - 1.1.7-rc3 + 1.1.8-rc1 Intility Intility A toolkit for implementing JSON:API specification in .NET applications diff --git a/JsonApiToolkit/Models/Querying/Filtering/IncludeFilter.cs b/JsonApiToolkit/Models/Querying/Filtering/IncludeFilter.cs index 55cccd9..c6f5a31 100644 --- a/JsonApiToolkit/Models/Querying/Filtering/IncludeFilter.cs +++ b/JsonApiToolkit/Models/Querying/Filtering/IncludeFilter.cs @@ -11,18 +11,8 @@ public class IncludeFilter public string RelationshipPath { get; set; } = string.Empty; /// - /// Field path within the relationship (e.g., "name"). + /// Filter group containing all conditions for this relationship path. + /// Preserves logical operator structure (AND/OR/NOT). /// - public string FieldPath { get; set; } = string.Empty; - - /// - /// Filter condition to apply. - /// - public FilterParameter Filter { get; set; } = new(); - - /// - /// Full path combining relationship and field (e.g., "author.name"). - /// - public string FullPath => - string.IsNullOrEmpty(FieldPath) ? RelationshipPath : $"{RelationshipPath}.{FieldPath}"; + public FilterGroup FilterGroup { get; set; } = new(); }