You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As a maintainer, understanding how a JSON:API filter like filter[author.posts.title][like]=foo becomes a LINQ expression requires reading five small modules, FilterExpressionBuilder, NestedPropertyNavigator, PropertyFilterBuilder, CollectionFilterBuilder, FilterOperatorExpressions, which call into each other in cycles (the navigator calls back into the filter builder for the leaf; the filter builder delegates collection branches back to the collection builder). Each piece has its own unit tests that use LINQ-to-Objects, so EF Core translation regressions only show up in controller-level integration tests. The deletion test on most of these modules fails: removing any one of them would scatter, not concentrate, complexity.
Solution
Collapse the five filter modules into a single deep FilterExpressionComposer. The composer exposes one public method that takes a FilterGroup and an entity type and returns the Expression<Func<T, bool>> ready to apply to an IQueryable<T>. The state machine for walking the property chain (scalar leaf, nested scalar via dot-path, collection navigation with Any(), collection-of-collection) becomes an internal detail. Operator semantics (Eq, Ne, Gt, Gte, Lt, Lte, Like, In, Nin) become a private lookup table. Callers (JsonApiController and FilteredIncludeBuilder) route through the composer; the existing helper classes are absorbed and deleted.
Behavior is preserved end-to-end. The change ships as a single PR with no coexistence period; existing integration tests act as the safety net.
User Stories
As a contributor, I want to read one module to understand how a filter expression is built, so that I don't have to trace through five files to follow the work.
As a maintainer, I want filter null-safety, recursion guards, collection-navigation, and operator semantics to live in one place, so that fixes don't have to be duplicated across helpers.
As a maintainer, I want a single test surface for filter composition, so that any filter regression has one obvious place for a failing test.
As a maintainer, I want EF Core integration tests for filter composition, so that IQueryable->SQL translation regressions are caught at the composer seam instead of surfacing only through controller integration tests.
As a contributor, I want the composer's interface to expose only what callers need, so that I'm not tempted to build new code against internal walker classes.
As a consumer of the toolkit, I want every existing filter operator (Eq, Ne, Gt, Gte, Lt, Lte, Like, In, Nin) to produce identical results, so that my API contracts continue to work.
As a consumer, I want dot-notation filters (filter[author.name]=John) to navigate relationships exactly as they do today, so that no existing client query breaks.
As a consumer, I want collection-navigation filters (filter[posts.title]=foo) to continue translating to Any() in the generated query, so that my SQL plan doesn't change.
As a consumer, I want null relationships mid-chain to short-circuit safely, so that requests don't fail with NullReferenceException.
As a consumer, I want And, Or, Not group composition to behave identically, so that complex conditions remain expressible.
As a consumer, I want sparse fieldsets, includes, and pagination to behave identically after the refactor, so that the upgrade is invisible.
As a maintainer, I want FilteredIncludeBuilder to use the same composer as the primary-resource filter pipeline, so that included-resource and primary-resource filter semantics can never silently diverge.
As a contributor, I want operator-to-expression mapping to be a single private table, so that adding a new operator is one localized change.
As a maintainer, I want the deletion test on the composer to pass, deleting it would force every caller to re-derive path-walking and operator-mapping, so that the seam clearly earns its place.
As a maintainer, I want the existing per-helper tests replaced by tests at the composer's public seam, so that test maintenance doesn't drag behind future internal refactors.
As a maintainer, I want the integration tests to use Microsoft.EntityFrameworkCore.InMemory consistent with JsonApiQueryAsyncTests, so that the new tests follow established prior art.
As a contributor, I want the composer to be public so it can be reused by advanced consumers, but the walker internals to remain internal, so the supported API surface stays small.
As a maintainer, I want this refactor to leave room for a future name-resolver module (candidate Nested relationships are not returned #6) to drop in, so that the next deepening doesn't require rewiring this one.
Implementation Decisions
Introduce FilterExpressionComposer with one public entry point: Compose<T>(FilterGroup) -> Expression<Func<T, bool>>. A non-generic overload taking Type entityType exists for callers that already work in Type-erased contexts (e.g. the controller's reflection paths).
The composer holds an internal state machine covering four path shapes: scalar leaf, nested scalar via dot-path, collection navigation with single Any(), collection-of-collection with chained Any().
Operator semantics consolidate into a private operator table keyed by FilterOperator. Each entry maps (memberAccess, value, propertyType) to an Expression. The Ne-with-null special case lives in this table, not in the path walker.
Null-safety rule: each navigation step inside a chain emits a != null guard. The current per-helper guards are unified into one rule.
Recursion-depth limit is preserved as a constant inside the composer.
Property name resolution continues to use QueryHelpers.GetPropertyByJsonName, but the composer accepts the resolver as a delegate so the future name-resolver module can replace it without touching callers.
Callers updated: JsonApiController.ApplyFilters, JsonApiController.ApplyFiltersAndIncludes, Extensions/FilteredIncludeBuilder. All other call sites of the deleted helpers are removed.
Public surface: FilterExpressionComposer is public sealed; absorbed helpers become internal types within the composer's file or are deleted entirely.
No changes to FilterParameter, FilterGroup, JsonApiFilterParser, or IncludeFilterParser. Those are scoped to other PRDs.
Single PR. No feature flag, no parallel-implementations period. The integration suite gates the change.
Testing Decisions
Good tests target the composer's public Compose method and assert the behavior of the resulting expression when applied to data. They do not assert the shape of the expression tree or reference internal walker types.
Two test surfaces:
Composer unit tests, replace FilterExpressionBuilderTests, NestedPropertyNavigatorTests, and the existing collection-builder coverage. Use LINQ-to-Objects against in-memory entity graphs to cover the matrix: each operator (Eq, Ne, Gt, Gte, Lt, Lte, Like, In, Nin) x each path shape (scalar, dot-path, collection-Any, collection-of-collection) x each combinator (And, Or, Not). Add explicit tests for null-relationship short-circuiting and recursion-depth enforcement.
Composer EF Core integration tests, new. Use Microsoft.EntityFrameworkCore.InMemory (already a project dependency). For each operator and path shape, build a small DbContext, apply the composer's expression, and assert the materialized results are correct. The goal is to catch translation issues that LINQ-to-Objects masks.
Prior art:
JsonApiToolkit.Tests/Integration/JsonApiQueryAsyncTests.cs, existing pattern for EF Core InMemory integration tests with WebApplicationFactory. New composer integration tests follow the same in-memory provider pattern but skip the WebApplicationFactory layer (the composer is testable directly).
JsonApiToolkit.Tests/Integration/AllowedIncludesIntegrationTests.cs, entity-graph fixture style to mirror.
JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs, reference for expression-against-IQueryable assertions.
The existing controller-level integration tests (JsonApiQueryAsyncTests, AllowedIncludesIntegrationTests, StrictPaginationIntegrationTests) act as the safety net during migration and must pass unchanged.
Out of Scope
Splitting FilterParameter into primary-resource and included-resource types (deepening candidate Add more responsetypes #2).
Any change to JsonApiFilterParser or the wire format of FilterParameter / FilterGroup.
New filter operators or new supported path shapes.
Performance optimization beyond preserving current behavior.
Further Notes
This refactor is a precondition for cleanly extracting JsonApiQueryPipeline later (candidate Disallow invalid page numbers #3): once filter application is a one-liner against the composer, the pipeline's applyFilters step becomes trivial.
Recent context: PR refactor(filters): split NestedPropertyNavigator into focused classes #130 ("split NestedPropertyNavigator into focused classes") moved in the opposite direction, breaking one walker into smaller helpers for testability of internals. That choice maximized internal mockability at the cost of locality. This PRD argues the better deepening is to consolidate at the outer seam (the composer) and keep the internals private.
Because observable behavior is preserved, no version bump beyond a normal minor release is required and no consumer migration guide is needed.
Problem Statement
As a maintainer, understanding how a JSON:API filter like
filter[author.posts.title][like]=foobecomes a LINQ expression requires reading five small modules,FilterExpressionBuilder,NestedPropertyNavigator,PropertyFilterBuilder,CollectionFilterBuilder,FilterOperatorExpressions, which call into each other in cycles (the navigator calls back into the filter builder for the leaf; the filter builder delegates collection branches back to the collection builder). Each piece has its own unit tests that use LINQ-to-Objects, so EF Core translation regressions only show up in controller-level integration tests. The deletion test on most of these modules fails: removing any one of them would scatter, not concentrate, complexity.Solution
Collapse the five filter modules into a single deep
FilterExpressionComposer. The composer exposes one public method that takes aFilterGroupand an entity type and returns theExpression<Func<T, bool>>ready to apply to anIQueryable<T>. The state machine for walking the property chain (scalar leaf, nested scalar via dot-path, collection navigation withAny(), collection-of-collection) becomes an internal detail. Operator semantics (Eq,Ne,Gt,Gte,Lt,Lte,Like,In,Nin) become a private lookup table. Callers (JsonApiControllerandFilteredIncludeBuilder) route through the composer; the existing helper classes are absorbed and deleted.Behavior is preserved end-to-end. The change ships as a single PR with no coexistence period; existing integration tests act as the safety net.
User Stories
Eq,Ne,Gt,Gte,Lt,Lte,Like,In,Nin) to produce identical results, so that my API contracts continue to work.filter[author.name]=John) to navigate relationships exactly as they do today, so that no existing client query breaks.filter[posts.title]=foo) to continue translating toAny()in the generated query, so that my SQL plan doesn't change.And,Or,Notgroup composition to behave identically, so that complex conditions remain expressible.FilteredIncludeBuilderto use the same composer as the primary-resource filter pipeline, so that included-resource and primary-resource filter semantics can never silently diverge.Microsoft.EntityFrameworkCore.InMemoryconsistent withJsonApiQueryAsyncTests, so that the new tests follow established prior art.publicso it can be reused by advanced consumers, but the walker internals to remaininternal, so the supported API surface stays small.Implementation Decisions
FilterExpressionComposerwith one public entry point:Compose<T>(FilterGroup) -> Expression<Func<T, bool>>. A non-generic overload takingType entityTypeexists for callers that already work inType-erased contexts (e.g. the controller's reflection paths).Any(), collection-of-collection with chainedAny().FilterOperator. Each entry maps(memberAccess, value, propertyType)to anExpression. TheNe-with-null special case lives in this table, not in the path walker.!= nullguard. The current per-helper guards are unified into one rule.QueryHelpers.GetPropertyByJsonName, but the composer accepts the resolver as a delegate so the future name-resolver module can replace it without touching callers.JsonApiController.ApplyFilters,JsonApiController.ApplyFiltersAndIncludes,Extensions/FilteredIncludeBuilder. All other call sites of the deleted helpers are removed.FilterExpressionBuilder,NestedPropertyNavigator(and any siblingPropertyNavigator),PropertyFilterBuilder,CollectionFilterBuilder,FilterOperatorExpressions.FilterExpressionComposerispublic sealed; absorbed helpers becomeinternaltypes within the composer's file or are deleted entirely.FilterParameter,FilterGroup,JsonApiFilterParser, orIncludeFilterParser. Those are scoped to other PRDs.Testing Decisions
Good tests target the composer's public
Composemethod and assert the behavior of the resulting expression when applied to data. They do not assert the shape of the expression tree or reference internal walker types.Two test surfaces:
Composer unit tests, replace
FilterExpressionBuilderTests,NestedPropertyNavigatorTests, and the existing collection-builder coverage. Use LINQ-to-Objects against in-memory entity graphs to cover the matrix: each operator (Eq,Ne,Gt,Gte,Lt,Lte,Like,In,Nin) x each path shape (scalar, dot-path, collection-Any, collection-of-collection) x each combinator (And,Or,Not). Add explicit tests for null-relationship short-circuiting and recursion-depth enforcement.Composer EF Core integration tests, new. Use
Microsoft.EntityFrameworkCore.InMemory(already a project dependency). For each operator and path shape, build a small DbContext, apply the composer's expression, and assert the materialized results are correct. The goal is to catch translation issues that LINQ-to-Objects masks.Prior art:
JsonApiToolkit.Tests/Integration/JsonApiQueryAsyncTests.cs, existing pattern for EF Core InMemory integration tests with WebApplicationFactory. New composer integration tests follow the same in-memory provider pattern but skip the WebApplicationFactory layer (the composer is testable directly).JsonApiToolkit.Tests/Integration/AllowedIncludesIntegrationTests.cs, entity-graph fixture style to mirror.JsonApiToolkit.Tests/Extensions/FilteredIncludeBuilderTests.cs, reference for expression-against-IQueryable assertions.The existing controller-level integration tests (
JsonApiQueryAsyncTests,AllowedIncludesIntegrationTests,StrictPaginationIntegrationTests) act as the safety net during migration and must pass unchanged.Out of Scope
FilterParameterinto primary-resource and included-resource types (deepening candidate Add more responsetypes #2).JsonApiQueryPipelinefrom the controller (candidate Disallow invalid page numbers #3).JsonApiFilterParseror the wire format ofFilterParameter/FilterGroup.Further Notes
JsonApiQueryPipelinelater (candidate Disallow invalid page numbers #3): once filter application is a one-liner against the composer, the pipeline'sapplyFiltersstep becomes trivial.NestedPropertyNavigatorinto focused classes #130 ("splitNestedPropertyNavigatorinto focused classes") moved in the opposite direction, breaking one walker into smaller helpers for testability of internals. That choice maximized internal mockability at the cost of locality. This PRD argues the better deepening is to consolidate at the outer seam (the composer) and keep the internals private.