Skip to content

Commit 1e337ae

Browse files
authored
Remove Newtonsoft.Json dependency and enhance FieldValueHelper enum resolution (#274)
* Remove Newtonsoft.Json dependency and enhance FieldValueHelper enum resolution 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. * Migrate test serialization from Newtonsoft to STJ and cache enum lookups - 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 * Remove enum cache — reflection cost is negligible for query-builder call site * Add bounded per-type enum lookup cache for GetEnumStringValue 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. * Add edge-case tests and remove redundant deserialization blocks * Apply suggestion from @niemyjski
1 parent 93b97ab commit 1e337ae

12 files changed

Lines changed: 283 additions & 212 deletions

File tree

src/Foundatio.Repositories.Elasticsearch/Utility/FieldValueHelper.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
using System;
2+
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
4+
using System.Reflection;
5+
using System.Runtime.Serialization;
6+
using System.Text.Json.Serialization;
27
using Elastic.Clients.Elasticsearch;
38

49
namespace Foundatio.Repositories.Elasticsearch.Utility;
510

611
public static class FieldValueHelper
712
{
13+
private static readonly ConcurrentDictionary<Type, Dictionary<string, string>> _enumTypeLookup = new();
14+
815
public static FieldValue ToFieldValue(object? value)
916
{
1017
return value switch
@@ -25,7 +32,34 @@ public static FieldValue ToFieldValue(object? value)
2532
decimal m => FieldValue.Double((double)m),
2633
DateTime dt => FieldValue.String(dt.ToString("o")),
2734
DateTimeOffset dto => FieldValue.String(dto.ToString("o")),
35+
Enum e => FieldValue.String(GetEnumStringValue(e)),
2836
_ => FieldValue.String(value.ToString()!)
2937
};
3038
}
39+
40+
private static string GetEnumStringValue(Enum value)
41+
{
42+
var lookup = _enumTypeLookup.GetOrAdd(value.GetType(), static type =>
43+
{
44+
var map = new Dictionary<string, string>();
45+
foreach (var field in type.GetFields(BindingFlags.Public | BindingFlags.Static))
46+
{
47+
var jsonAttr = field.GetCustomAttribute<JsonStringEnumMemberNameAttribute>();
48+
if (jsonAttr is not null)
49+
{
50+
map[field.Name] = jsonAttr.Name;
51+
continue;
52+
}
53+
54+
var enumMemberAttr = field.GetCustomAttribute<EnumMemberAttribute>();
55+
if (enumMemberAttr?.Value is not null)
56+
map[field.Name] = enumMemberAttr.Value;
57+
}
58+
59+
return map;
60+
});
61+
62+
var name = value.ToString();
63+
return lookup.TryGetValue(name, out var resolved) ? resolved : name;
64+
}
3165
}

src/Foundatio.Repositories/Foundatio.Repositories.csproj

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,5 @@
22
<ItemGroup>
33
<PackageReference Include="Foundatio" Version="13.0.1" Condition="'$(ReferenceFoundatioSource)' == '' OR '$(ReferenceFoundatioSource)' == 'false'" />
44
<ProjectReference Include="$(FoundatioProjectsDir)Foundatio\src\Foundatio\Foundatio.csproj" Condition="'$(ReferenceFoundatioSource)' == 'true'" />
5-
6-
<PackageReference Include="Foundatio.JsonNet" Version="13.0.1" Condition="'$(ReferenceFoundatioSource)' == '' OR '$(ReferenceFoundatioSource)' == 'false'" />
7-
<ProjectReference Include="$(FoundatioProjectsDir)Foundatio\src\Foundatio.JsonNet\Foundatio.JsonNet.csproj" Condition="'$(ReferenceFoundatioSource)' == 'true'" />
85
</ItemGroup>
96
</Project>

src/Foundatio.Repositories/Models/Aggregations/IAggregate.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ namespace Foundatio.Repositories.Models;
1010
/// Aggregations provide summarized data about the documents matching a query, such as
1111
/// counts, averages, date histograms, and term frequencies.
1212
/// </remarks>
13-
[Newtonsoft.Json.JsonConverter(typeof(AggregationsNewtonsoftJsonConverter))]
1413
[System.Text.Json.Serialization.JsonConverter(typeof(AggregationsSystemTextJsonConverter))]
1514
public interface IAggregate
1615
{

src/Foundatio.Repositories/Models/Aggregations/IBucket.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ namespace Foundatio.Repositories.Models;
1010
/// Buckets group documents based on field values, ranges, or other criteria. Each bucket
1111
/// contains a subset of documents and can include nested aggregations.
1212
/// </remarks>
13-
[Newtonsoft.Json.JsonConverter(typeof(BucketsNewtonsoftJsonConverter))]
1413
[System.Text.Json.Serialization.JsonConverter(typeof(BucketsSystemTextJsonConverter))]
1514
public interface IBucket
1615
{

src/Foundatio.Repositories/Models/FindResults.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ public FindResults()
3939
/// <param name="aggregations">The aggregation results.</param>
4040
/// <param name="getNextPageFunc">A function to retrieve the next page of results.</param>
4141
/// <param name="data">Additional metadata.</param>
42-
[Newtonsoft.Json.JsonConstructor]
4342
public FindResults(IEnumerable<FindHit<T>>? hits = null, long total = 0, IReadOnlyDictionary<string, IAggregate>? aggregations = null, Func<FindResults<T>, Task<FindResults<T>>>? getNextPageFunc = null, IDictionary<string, object?>? data = null)
4443
: base(total, aggregations, data)
4544
{
@@ -81,14 +80,12 @@ protected set
8180
/// Gets the current page number (1-based).
8281
/// </summary>
8382
[JsonInclude]
84-
[Newtonsoft.Json.JsonProperty]
8583
public int Page { get; protected set; } = 1;
8684

8785
/// <summary>
8886
/// Gets whether there are more pages of results available.
8987
/// </summary>
9088
[JsonInclude]
91-
[Newtonsoft.Json.JsonProperty]
9289
public bool HasMore { get; protected set; }
9390

9491
int IFindResults<T>.Page
@@ -201,7 +198,6 @@ public class CountResult : IHaveData
201198
/// <param name="aggregations">The aggregation results.</param>
202199
/// <param name="data">Additional metadata.</param>
203200
[JsonConstructor]
204-
[Newtonsoft.Json.JsonConstructor]
205201
public CountResult(long total = 0, IReadOnlyDictionary<string, IAggregate>? aggregations = null, IDictionary<string, object?>? data = null)
206202
{
207203
Aggregations = aggregations ?? EmptyReadOnly<string, IAggregate>.Dictionary;
@@ -287,7 +283,6 @@ public class FindHit<T> : IHaveData
287283
/// <param name="routing">The routing value.</param>
288284
/// <param name="data">Additional metadata.</param>
289285
[JsonConstructor]
290-
[Newtonsoft.Json.JsonConstructor]
291286
public FindHit(string? id, T? document, double score, string? version = null, string? routing = null, IDictionary<string, object?>? data = null)
292287
{
293288
Id = id;

src/Foundatio.Repositories/Serialization/AggregationsNewtonsoftJsonConverter.cs

Lines changed: 0 additions & 64 deletions
This file was deleted.

src/Foundatio.Repositories/Serialization/BucketsNewtonsoftJsonConverter.cs

Lines changed: 0 additions & 83 deletions
This file was deleted.

tests/Foundatio.Repositories.Elasticsearch.Tests/AggregationQueryTests.cs

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using Foundatio.Repositories.Elasticsearch.Tests.Repositories.Models;
1010
using Foundatio.Repositories.Models;
1111
using Microsoft.Extensions.Time.Testing;
12-
using Newtonsoft.Json;
1312
using Xunit;
1413

1514
namespace Foundatio.Repositories.Elasticsearch.Tests;
@@ -498,8 +497,8 @@ public async Task GetTermAggregationsAsync()
498497
Assert.Equal(10, termsAge.Buckets.Count);
499498
Assert.Equal(1, termsAge.Buckets.First(f => f.Key == 19).Total);
500499

501-
string json = JsonConvert.SerializeObject(result);
502-
var roundTripped = JsonConvert.DeserializeObject<CountResult>(json);
500+
string json = System.Text.Json.JsonSerializer.Serialize(result);
501+
var roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(json);
503502
Assert.NotNull(roundTripped);
504503
Assert.Equal(10, roundTripped.Total);
505504
Assert.Single(roundTripped.Aggregations);
@@ -521,10 +520,10 @@ public async Task GetTermAggregationsAsync()
521520
Assert.NotNull(termsYears);
522521
Assert.Single(termsYears.Buckets);
523522

524-
json = JsonConvert.SerializeObject(result, Formatting.Indented);
525-
roundTripped = JsonConvert.DeserializeObject<CountResult>(json);
523+
json = System.Text.Json.JsonSerializer.Serialize(result, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
524+
roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(json);
526525
Assert.NotNull(roundTripped);
527-
string roundTrippedJson = JsonConvert.SerializeObject(roundTripped, Formatting.Indented);
526+
string roundTrippedJson = System.Text.Json.JsonSerializer.Serialize(roundTripped, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
528527
Assert.Equal(json, roundTrippedJson);
529528
Assert.Equal(10, roundTripped.Total);
530529
Assert.Single(roundTripped.Aggregations);
@@ -565,8 +564,8 @@ await _employeeRepository.AddAsync(new List<Employee> {
565564
oldestDate = oldestDate.AddDays(1);
566565
}
567566

568-
string json = JsonConvert.SerializeObject(result);
569-
var roundTripped = JsonConvert.DeserializeObject<CountResult>(json);
567+
string json = System.Text.Json.JsonSerializer.Serialize(result);
568+
var roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(json);
570569
Assert.NotNull(roundTripped);
571570

572571
dateHistogramAgg = roundTripped.Aggregations.DateHistogram("date_nextReview");
@@ -600,8 +599,8 @@ await _employeeRepository.AddAsync(new List<Employee> {
600599
Assert.NotNull(dateTermsAgg);
601600
Assert.Equal(utcToday.SubtractDays(2), dateTermsAgg.Value);
602601

603-
string json = JsonConvert.SerializeObject(result);
604-
var roundTripped = JsonConvert.DeserializeObject<CountResult>(json);
602+
string json = System.Text.Json.JsonSerializer.Serialize(result);
603+
var roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(json);
605604
Assert.NotNull(roundTripped);
606605

607606
dateTermsAgg = roundTripped.Aggregations.Min<DateTime>("min_nextReview");
@@ -631,8 +630,8 @@ public async Task GetTermAggregationsWithTopHitsAsync()
631630
Assert.Equal(19, employees.First().Age);
632631
Assert.Equal(1, employees.First().YearsEmployed);
633632

634-
string json = JsonConvert.SerializeObject(result);
635-
var roundTripped = JsonConvert.DeserializeObject<CountResult>(json);
633+
string json = System.Text.Json.JsonSerializer.Serialize(result);
634+
var roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(json);
636635
Assert.NotNull(roundTripped);
637636
Assert.Equal(10, roundTripped.Total);
638637
Assert.Single(roundTripped.Aggregations);
@@ -642,21 +641,6 @@ public async Task GetTermAggregationsWithTopHitsAsync()
642641
bucket = roundTrippedTermsAge.Buckets.First(f => f.Key == 19);
643642
Assert.Equal(1, bucket.Total);
644643

645-
string systemTextJson = System.Text.Json.JsonSerializer.Serialize(result);
646-
Assert.True(System.Text.Json.Nodes.JsonNode.DeepEquals(
647-
System.Text.Json.Nodes.JsonNode.Parse(json),
648-
System.Text.Json.Nodes.JsonNode.Parse(systemTextJson)),
649-
"Newtonsoft and System.Text.Json serialization should produce semantically equivalent JSON");
650-
roundTripped = System.Text.Json.JsonSerializer.Deserialize<CountResult>(systemTextJson);
651-
Assert.NotNull(roundTripped);
652-
Assert.Equal(10, roundTripped.Total);
653-
Assert.Single(roundTripped.Aggregations);
654-
var sysTermsAge = roundTripped.Aggregations.Terms<int>("terms_age");
655-
Assert.NotNull(sysTermsAge);
656-
Assert.Equal(10, sysTermsAge.Buckets.Count);
657-
bucket = sysTermsAge.Buckets.First(f => f.Key == 19);
658-
Assert.Equal(1, bucket.Total);
659-
660644
tophits = bucket.Aggregations.TopHits();
661645
Assert.NotNull(tophits);
662646
employees = tophits.Documents<Employee>(_serializer);

0 commit comments

Comments
 (0)