Remove Newtonsoft.Json dependency and enhance FieldValueHelper enum resolution#274
Merged
Conversation
…esolution Remove the transitive Newtonsoft.Json dependency that was unnecessarily pulled in via Foundatio.JsonNet, completing the System.Text.Json migration: - Delete AggregationsNewtonsoftJsonConverter and BucketsNewtonsoftJsonConverter - Remove all [Newtonsoft.Json.*] attributes from IAggregate, IBucket, FindResults, FindHit, and CountResult models (STJ equivalents already in place) - Remove Foundatio.JsonNet PackageReference/ProjectReference from csproj - Update SerializerTestHelper to STJ-only Additionally, enhance FieldValueHelper.GetEnumStringValue to resolve enum values using [EnumMember(Value)] as a fallback after [JsonStringEnumMemberName], fixing silent Elasticsearch query bugs when custom enum serialization names are used. Add comprehensive FieldValueHelper unit tests covering enum attribute resolution and all primitive type conversions.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the library’s Json.NET (Newtonsoft.Json) integration to standardize on System.Text.Json across the public API surface, and improves Elasticsearch query value handling by resolving enum string values from serialization-related attributes.
Changes:
- Removed the
Foundatio.JsonNetreference and deleted the Json.NET aggregation/bucket converters. - Removed Newtonsoft.Json attributes from aggregation and result models, leaving System.Text.Json attributes/converters in place.
- Enhanced
FieldValueHelper.ToFieldValueto resolve enum values via[JsonStringEnumMemberName]and[EnumMember(Value=...)], and added tests for enum + primitive conversions.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Foundatio.Repositories.Tests/Serialization/SerializerTestHelper.cs | Removes Json.NET serializer from the serializer matrix so tests validate STJ-only. |
| tests/Foundatio.Repositories.Elasticsearch.Tests/FieldValueHelperTests.cs | Adds test coverage for enum attribute resolution and primitive conversions to FieldValue. |
| src/Foundatio.Repositories/Serialization/BucketsNewtonsoftJsonConverter.cs | Deletes Json.NET bucket converter implementation. |
| src/Foundatio.Repositories/Serialization/AggregationsNewtonsoftJsonConverter.cs | Deletes Json.NET aggregation converter implementation. |
| src/Foundatio.Repositories/Models/FindResults.cs | Removes Newtonsoft.Json constructor/property attributes from result models. |
| src/Foundatio.Repositories/Models/Aggregations/IBucket.cs | Removes Newtonsoft.Json converter attribute from IBucket (STJ converter remains). |
| src/Foundatio.Repositories/Models/Aggregations/IAggregate.cs | Removes Newtonsoft.Json converter attribute from IAggregate (STJ converter remains). |
| src/Foundatio.Repositories/Foundatio.Repositories.csproj | Drops Foundatio.JsonNet dependency. |
| src/Foundatio.Repositories.Elasticsearch/Utility/FieldValueHelper.cs | Adds enum handling with attribute-based string resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace all JsonConvert usage in ES tests with System.Text.Json.JsonSerializer - Add ConcurrentDictionary cache for enum string resolution in FieldValueHelper - Use GetField instead of GetMember for more efficient reflection
Caches attribute resolution per enum Type (not per value), so growth is bounded by the number of distinct enum types in the app. Flags combinations and undefined values fall through to ToString() without polluting the cache.
niemyjski
commented
May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan