feat: implement sparse fieldsets (fields[type]) per JSON:API spec#82
Conversation
There was a problem hiding this comment.
Pull request overview
Implements JSON:API sparse fieldsets via fields[type] and propagates the parsed fieldsets through request parsing and JSON:API serialization to filter attributes for both primary and included resources.
Changes:
- Parse
fields[type]=a,b,cintoQueryParameters.Fieldsand surface it throughJsonApiQueryParserService. - Thread sparse fieldsets through controller → mapper → inclusion mapping and filter serialized attributes accordingly.
- Add documentation and unit/integration tests covering parsing and attribute filtering behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/docs/upgrade-guide.md | Marks sparse fieldsets as implemented and adds a release date entry. |
| docs/docs/querying.md | Documents fields[type] usage, examples, and behavior notes. |
| JsonApiToolkit/Services/JsonApiQueryParserService.cs | Adds a warning when fields... keys are present but no valid fieldsets were parsed. |
| JsonApiToolkit/Parsing/JsonApiQueryParser.cs | Adds parsing logic for fields[type] into a per-type dictionary. |
| JsonApiToolkit/Models/Querying/QueryParameters.cs | Introduces Fields to carry parsed sparse fieldsets through the pipeline. |
| JsonApiToolkit/Mapping/JsonApiMapper.cs | Filters mapped attributes based on sparse fieldsets and propagates fieldsets into include processing. |
| JsonApiToolkit/Mapping/InclusionMapper.cs | Propagates fieldsets into included-resource mapping so included resources are filtered too. |
| JsonApiToolkit/Controllers/JsonApiController.cs | Passes parsed Fields into mapping calls and adds fields count to debug logs. |
| JsonApiToolkit.Tests/Services/JsonApiQueryParserServiceTests.cs | Adds service-level test ensuring fieldsets are parsed through the service. |
| JsonApiToolkit.Tests/Parsing/JsonApiQueryParserTests.cs | Adds parser tests for single/multi fieldsets and malformed keys. |
| JsonApiToolkit.Tests/Mapping/JsonApiMapperTests.cs | Adds mapper tests validating attribute filtering (primary + included) and id/type behavior. |
| JsonApiToolkit.Tests/Integration/JsonApiQueryAsyncTests.cs | Adds end-to-end tests for sparse fieldsets combined with include/sort/page and invalid field names. |
| CLAUDE.md | Updates feature list to include sparse fieldsets support and behavior summary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| StringComparer.OrdinalIgnoreCase | ||
| ); | ||
|
|
||
| foreach (string? key in request.Query.Keys.Where(k => k.StartsWith("fields["))) |
There was a problem hiding this comment.
request.Query.Keys yields non-null strings, so foreach (string? key ...) is unnecessarily nullable. Using string here avoids implying key might be null (and prevents potential nullable-analysis noise around key.Length).
| foreach (string? key in request.Query.Keys.Where(k => k.StartsWith("fields["))) | |
| foreach (string key in request.Query.Keys.Where(k => k.StartsWith("fields["))) |
| List<string>? allowedFields = null; | ||
| fields?.TryGetValue(resourceType, out allowedFields); | ||
|
|
||
| foreach (PropertyInfo prop in EntityMapper.GetAttributeProperties(type)) | ||
| { | ||
| string camelName = prop.Name.ToCamelCase(); | ||
|
|
||
| if ( | ||
| allowedFields != null | ||
| && !allowedFields.Contains(camelName, StringComparer.OrdinalIgnoreCase) | ||
| ) | ||
| continue; |
There was a problem hiding this comment.
allowedFields.Contains(camelName, StringComparer.OrdinalIgnoreCase) does a linear scan for every mapped attribute. If sparse fieldsets are used heavily or entities have many attributes, consider normalizing allowedFields once per resource type into a HashSet<string> with an ordinal-ignore-case comparer to reduce per-attribute overhead.
| ILogger? logger = null, | ||
| Dictionary<string, List<string>>? fields = null | ||
| ) | ||
| { |
There was a problem hiding this comment.
Adding a new parameter to these public mapping methods changes the method signature and can be a binary-breaking change for consumers that load the new assembly without recompiling (even if the parameter is optional). If binary compatibility across minor versions matters for this package, consider keeping the existing signature and introducing an overload that takes fields, delegating to a shared implementation.
| ILogger? logger = null, | |
| Dictionary<string, List<string>>? fields = null | |
| ) | |
| { | |
| ILogger? logger = null) | |
| { | |
| return ToResourceObject( | |
| entity, | |
| resourceType, | |
| includedRelationships, | |
| logger, | |
| fields: null); | |
| } | |
| public static ResourceObject ToResourceObject( | |
| object entity, | |
| string resourceType, | |
| List<string>? includedRelationships, | |
| ILogger? logger, | |
| Dictionary<string, List<string>>? fields) | |
| { | |
| return ToResourceObjectCore( | |
| entity, | |
| resourceType, | |
| includedRelationships, | |
| logger, | |
| fields); | |
| } | |
| private static ResourceObject ToResourceObjectCore( | |
| object entity, | |
| string resourceType, | |
| List<string>? includedRelationships, | |
| ILogger? logger, | |
| Dictionary<string, List<string>>? fields) | |
| { |
| HashSet<string>? processedEntities = null, | ||
| Dictionary<string, List<string>>? fields = null | ||
| ) | ||
| { |
There was a problem hiding this comment.
Similar to JsonApiMapper, adding an optional parameter to this public method changes its signature and can be binary-breaking for precompiled consumers. If maintaining binary compatibility is a goal, consider introducing an overload that accepts fields while preserving the existing signature.
| HashSet<string>? processedEntities = null, | |
| Dictionary<string, List<string>>? fields = null | |
| ) | |
| { | |
| HashSet<string>? processedEntities = null | |
| ) | |
| { | |
| AddIncludedResources( | |
| entityOrCollection, | |
| includePaths, | |
| included, | |
| logger, | |
| processedEntities, | |
| null | |
| ); | |
| } | |
| public static void AddIncludedResources( | |
| object entityOrCollection, | |
| List<string> includePaths, | |
| List<ResourceObject> included, | |
| ILogger? logger, | |
| HashSet<string>? processedEntities, | |
| Dictionary<string, List<string>>? fields | |
| ) | |
| { |
| if ( | ||
| request.Query.Keys.Any(k => k.StartsWith("fields", StringComparison.OrdinalIgnoreCase)) | ||
| && queryParams.Fields == null | ||
| ) | ||
| { | ||
| _logger.LogWarning( | ||
| "Fields parameters detected but no valid fields parsed. Check syntax: fields[type]=field1,field2" | ||
| ); | ||
| } |
There was a problem hiding this comment.
The warning condition is very broad (StartsWith("fields")) and will also fire for keys like fieldsFoo or for valid-but-empty fields[type]= values, which can create noisy logs. Consider narrowing detection to fields[ (same shape the parser supports) and only warning when at least one fields[...] key is malformed or has a non-empty value that still fails to parse.
| if (string.IsNullOrWhiteSpace(value)) | ||
| continue; |
There was a problem hiding this comment.
fields[type]= (empty value) is currently treated as if no sparse fieldset was provided (it is skipped, leaving Fields null). That means the request will return all attributes instead of returning only id/type (or otherwise reflecting an explicit empty fieldset). Consider treating empty/whitespace values as an explicit empty fieldset for that type (store an empty list), or explicitly logging/handling it as invalid rather than silently ignoring it.
| if (string.IsNullOrWhiteSpace(value)) | |
| continue; | |
| if (string.IsNullOrWhiteSpace(value)) | |
| { | |
| // Treat an explicit empty fieldset (e.g. "fields[type]=") as an empty list | |
| // rather than ignoring it, so that only "id" and "type" are returned. | |
| fieldsDictionary[resourceType] = new List<string>(); | |
| continue; | |
| } |
| Assert.True(parameters.Fields.ContainsKey("articles")); | ||
| Assert.Equal(2, parameters.Fields["articles"].Count); | ||
| Assert.Contains("title", parameters.Fields["articles"]); | ||
| Assert.Contains("content", parameters.Fields["articles"]); |
There was a problem hiding this comment.
Inefficient use of 'ContainsKey' and indexer.
Inefficient use of 'ContainsKey' and indexer.
Inefficient use of 'ContainsKey' and indexer.
| Assert.True(parameters.Fields.ContainsKey("articles")); | |
| Assert.Equal(2, parameters.Fields["articles"].Count); | |
| Assert.Contains("title", parameters.Fields["articles"]); | |
| Assert.Contains("content", parameters.Fields["articles"]); | |
| Assert.True(parameters.Fields.TryGetValue("articles", out var articleFields)); | |
| Assert.Equal(2, articleFields.Count); | |
| Assert.Contains("title", articleFields); | |
| Assert.Contains("content", articleFields); |
🤖 I have created a release *beep* *boop* --- ## [1.7.0](v1.6.0...v1.7.0) (2026-02-10) ### Features * implement sparse fieldsets (`fields[type]`) per JSON:API spec ([#82](#82)) ([d03136b](d03136b)) ### Bug Fixes * update editorconfig ([#84](#84)) ([1931137](1931137)) ### Dependencies * **nuget:** Bump csharpier from 1.2.5 to 1.2.6 ([#81](#81)) ([5f63735](5f63735)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: intility-release-bot[bot] <175299729+intility-release-bot[bot]@users.noreply.github.com>
fields[type]=field1,field2) to reduce response payload sizeJsonApiQueryParserwith malformed key handling, whitespace trimming, and case-insensitive type matching