Skip to content

Commit ff588a5

Browse files
committed
Improve code quality: serializer safety, FieldValue consistency, and test assertions
- Make LazyDocument require ITextSerializer (no hardcoded default) - Add serializer parameter to TopHitsAggregate.Documents<T>() for round-trip path - Use explicit FieldValue factory methods in FieldConditionsQueryBuilder.ToFieldValue with decimal/DateTime/DateTimeOffset cases (aligns with PageableQueryBuilder) - Add readonly to FindHitExtensions._options static field - Cache JsonSerializerOptions in AggregationsSystemTextJsonConverter.Write - Use ToLowerInvariant() instead of ToLower() in BucketsSystemTextJsonConverter - Remove no-op b.Pipeline(DefaultPipeline) in bulk patch loop (unreachable when set) - Split Assert.NotNull(x?.Id) patterns and add null guards in test files - Remove duplicate identity1 declaration (merge artifact) Made-with: Cursor
1 parent 4d5c826 commit ff588a5

File tree

13 files changed

+50
-31
lines changed

13 files changed

+50
-31
lines changed

src/Foundatio.Repositories.Elasticsearch/Extensions/FindHitExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace Foundatio.Repositories.Elasticsearch.Extensions;
1212

1313
public static class FindHitExtensions
1414
{
15-
private static JsonSerializerOptions _options;
15+
private static readonly JsonSerializerOptions _options;
1616
static FindHitExtensions()
1717
{
1818
_options = new JsonSerializerOptions { PropertyNameCaseInsensitive = true };

src/Foundatio.Repositories.Elasticsearch/Queries/Builders/FieldConditionsQueryBuilder.cs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,17 @@ private static FieldValue ToFieldValue(object value)
289289
{
290290
return value switch
291291
{
292-
null => null,
293-
string s => s,
294-
long l => l,
295-
int i => i,
296-
double d => d,
297-
float f => f,
298-
bool b => b,
299-
_ => value.ToString()
292+
null => FieldValue.Null,
293+
string s => FieldValue.String(s),
294+
bool b => FieldValue.Boolean(b),
295+
long l => FieldValue.Long(l),
296+
int i => FieldValue.Long(i),
297+
double d => FieldValue.Double(d),
298+
float f => FieldValue.Double(f),
299+
decimal m => FieldValue.Double((double)m),
300+
DateTime dt => FieldValue.String(dt.ToString("o")),
301+
DateTimeOffset dto => FieldValue.String(dto.ToString("o")),
302+
_ => FieldValue.String(value.ToString())
300303
};
301304
}
302305
}

src/Foundatio.Repositories.Elasticsearch/Repositories/ElasticRepositoryBase.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -478,8 +478,6 @@ public virtual async Task PatchAsync(Ids ids, IPatchOperation operation, IComman
478478
b.Refresh(options.GetRefreshMode(DefaultConsistency));
479479
foreach (var id in ids)
480480
{
481-
b.Pipeline(DefaultPipeline);
482-
483481
if (scriptOperation != null)
484482
b.Update<T>(u =>
485483
{

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Linq;
34
using System.Text;
5+
using Foundatio.Serializer;
46

57
namespace Foundatio.Repositories.Models;
68

@@ -23,19 +25,21 @@ public TopHitsAggregate(IList<ILazyDocument> hits)
2325

2426
public TopHitsAggregate() { }
2527

26-
public IReadOnlyCollection<T> Documents<T>() where T : class
28+
public IReadOnlyCollection<T> Documents<T>(ITextSerializer serializer = null) where T : class
2729
{
2830
if (_hits != null && _hits.Count > 0)
2931
return _hits.Select(h => h.As<T>()).ToList();
3032

3133
if (Hits != null && Hits.Count > 0)
3234
{
35+
ArgumentNullException.ThrowIfNull(serializer);
36+
3337
return Hits
3438
.Select(json =>
3539
{
3640
if (string.IsNullOrEmpty(json))
3741
return null;
38-
var lazy = new LazyDocument(Encoding.UTF8.GetBytes(json));
42+
var lazy = new LazyDocument(Encoding.UTF8.GetBytes(json), serializer);
3943
return lazy.As<T>();
4044
})
4145
.Where(d => d != null)

src/Foundatio.Repositories/Models/LazyDocument.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,12 @@ public class LazyDocument : ILazyDocument
3939
/// Initializes a new instance of the <see cref="LazyDocument"/> class.
4040
/// </summary>
4141
/// <param name="data">The raw document data.</param>
42-
/// <param name="serializer">The serializer to use for deserialization. Defaults to JSON.</param>
43-
public LazyDocument(byte[] data, ITextSerializer serializer = null)
42+
/// <param name="serializer">The serializer to use for deserialization.</param>
43+
public LazyDocument(byte[] data, ITextSerializer serializer)
4444
{
45+
ArgumentNullException.ThrowIfNull(serializer);
4546
_data = data;
46-
_serializer = serializer ?? new JsonNetSerializer();
47+
_serializer = serializer;
4748
}
4849

4950
/// <inheritdoc/>

src/Foundatio.Repositories/Utility/AggregationsSystemTextJsonConverter.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Runtime.CompilerServices;
34
using System.Text.Json;
45
using Foundatio.Repositories.Models;
56

67
namespace Foundatio.Repositories.Utility;
78

89
public class AggregationsSystemTextJsonConverter : System.Text.Json.Serialization.JsonConverter<IAggregate>
910
{
11+
private static readonly ConditionalWeakTable<JsonSerializerOptions, JsonSerializerOptions> _writeOptionsCache = new();
12+
1013
public override bool CanConvert(Type type)
1114
{
1215
return typeof(IAggregate).IsAssignableFrom(type);
@@ -36,10 +39,8 @@ public override IAggregate Read(ref Utf8JsonReader reader, Type typeToConvert, J
3639

3740
public override void Write(Utf8JsonWriter writer, IAggregate value, JsonSerializerOptions options)
3841
{
39-
var serializerOptions = new JsonSerializerOptions(options)
40-
{
41-
Converters = { new DoubleSystemTextJsonConverter() }
42-
};
42+
var serializerOptions = _writeOptionsCache.GetValue(options, static o =>
43+
new JsonSerializerOptions(o) { Converters = { new DoubleSystemTextJsonConverter() } });
4344

4445
JsonSerializer.Serialize(writer, value, value.GetType(), serializerOptions);
4546
}

src/Foundatio.Repositories/Utility/BucketsSystemTextJsonConverter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public override void Write(Utf8JsonWriter writer, IBucket value, JsonSerializerO
7777
if (element.TryGetProperty(propertyName, out var dataElement))
7878
return dataElement;
7979

80-
if (element.TryGetProperty(propertyName.ToLower(), out dataElement))
80+
if (element.TryGetProperty(propertyName.ToLowerInvariant(), out dataElement))
8181
return dataElement;
8282

8383
return null;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
using System.Collections.Generic;
33
using System.IO;
44
using System.Linq;
5+
using System.Text.Json;
56
using System.Threading.Tasks;
67
using Elastic.Clients.Elasticsearch.Core.Search;
78
using Exceptionless.DateTimeExtensions;
89
using Foundatio.Repositories.Elasticsearch.Extensions;
910
using Foundatio.Repositories.Elasticsearch.Tests.Repositories.Models;
1011
using Foundatio.Repositories.Models;
12+
using Foundatio.Serializer;
1113
using Microsoft.Extensions.Time.Testing;
1214
using Newtonsoft.Json;
1315
using Xunit;
@@ -532,7 +534,8 @@ public async Task GetTermAggregationsWithTopHitsAsync()
532534

533535
tophits = bucket.Aggregations.TopHits();
534536
Assert.NotNull(tophits);
535-
employees = tophits.Documents<Employee>();
537+
var serializer = new SystemTextJsonSerializer(new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
538+
employees = tophits.Documents<Employee>(serializer);
536539
Assert.Single(employees);
537540
Assert.Equal(19, employees.First().Age);
538541
Assert.Equal(1, employees.First().YearsEmployed);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ public async Task CanQueryByParent()
108108
{
109109
var parent = ParentGenerator.Default;
110110
parent = await _parentRepository.AddAsync(parent, o => o.ImmediateConsistency());
111-
Assert.NotNull(parent?.Id);
111+
Assert.NotNull(parent);
112+
Assert.NotNull(parent.Id);
112113

113114
await _parentRepository.AddAsync(ParentGenerator.Generate(), o => o.ImmediateConsistency());
114115

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@ public async Task AddAsync_WithLowercasePipeline_LowercasesName()
4343
employee = await _employeeRepository.AddAsync(employee, o => o.ImmediateConsistency());
4444

4545
// Assert
46-
Assert.NotNull(employee?.Id);
46+
Assert.NotNull(employee);
47+
Assert.NotNull(employee.Id);
4748
var result = await _employeeRepository.GetByIdAsync(employee.Id);
49+
Assert.NotNull(result);
4850
Assert.Equal(" blake ", result.Name);
4951
}
5052

@@ -98,6 +100,7 @@ public async Task JsonPatchAsync_WithLowercasePipeline_LowercasesName()
98100

99101
// Assert
100102
employee = await _employeeRepository.GetByIdAsync(employee.Id);
103+
Assert.NotNull(employee);
101104
Assert.Equal(EmployeeGenerator.Default.Age, employee.Age);
102105
Assert.Equal("patched", employee.Name);
103106
}
@@ -134,6 +137,7 @@ public async Task PartialPatchAsync_WithLowercasePipeline_LowercasesName()
134137

135138
// Assert
136139
employee = await _employeeRepository.GetByIdAsync(employee.Id);
140+
Assert.NotNull(employee);
137141
Assert.Equal(EmployeeGenerator.Default.Age, employee.Age);
138142
Assert.Equal("patched", employee.Name);
139143
}
@@ -169,6 +173,7 @@ public async Task ScriptPatchAsync_WithLowercasePipeline_LowercasesName()
169173

170174
// Assert
171175
employee = await _employeeRepository.GetByIdAsync(employee.Id);
176+
Assert.NotNull(employee);
172177
Assert.Equal(EmployeeGenerator.Default.Age, employee.Age);
173178
Assert.Equal("patched", employee.Name);
174179
}

0 commit comments

Comments
 (0)