From d73af6c2a6e77e2b4651220ceb2f2b7399659cc8 Mon Sep 17 00:00:00 2001 From: Florian Bernd Date: Tue, 29 Oct 2024 09:05:10 +0100 Subject: [PATCH] Improve handcrafted types --- ...ic.Clients.Elasticsearch.Serverless.csproj | 3 + .../Elastic.Clients.Elasticsearch.csproj | 3 + .../_Shared/Core/Fields/FieldValue.cs | 5 +- .../_Shared/Core/Infer/Field/Field.cs | 77 ++++---- .../_Shared/Core/Infer/Fields/Fields.cs | 168 ++++++++++++------ .../Core/Infer/Fields/FieldsConverter.cs | 4 +- 6 files changed, 161 insertions(+), 99 deletions(-) diff --git a/src/Elastic.Clients.Elasticsearch.Serverless/Elastic.Clients.Elasticsearch.Serverless.csproj b/src/Elastic.Clients.Elasticsearch.Serverless/Elastic.Clients.Elasticsearch.Serverless.csproj index 4e00bb4e38c..9cf4c7ee04e 100644 --- a/src/Elastic.Clients.Elasticsearch.Serverless/Elastic.Clients.Elasticsearch.Serverless.csproj +++ b/src/Elastic.Clients.Elasticsearch.Serverless/Elastic.Clients.Elasticsearch.Serverless.csproj @@ -23,6 +23,9 @@ + + + diff --git a/src/Elastic.Clients.Elasticsearch/Elastic.Clients.Elasticsearch.csproj b/src/Elastic.Clients.Elasticsearch/Elastic.Clients.Elasticsearch.csproj index dcd3c9db9d8..794e77e3984 100644 --- a/src/Elastic.Clients.Elasticsearch/Elastic.Clients.Elasticsearch.csproj +++ b/src/Elastic.Clients.Elasticsearch/Elastic.Clients.Elasticsearch.csproj @@ -23,6 +23,9 @@ + + + diff --git a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Fields/FieldValue.cs b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Fields/FieldValue.cs index a2b89956e86..3959cc2dc2b 100644 --- a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Fields/FieldValue.cs +++ b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Fields/FieldValue.cs @@ -28,7 +28,8 @@ namespace Elastic.Clients.Elasticsearch; /// therefore cannot be specifically typed. /// [JsonConverter(typeof(FieldValueConverter))] -public readonly struct FieldValue : IEquatable +public readonly struct FieldValue : + IEquatable { internal FieldValue(ValueKind kind, object? value) { @@ -42,7 +43,7 @@ internal FieldValue(ValueKind kind, object? value) public readonly ValueKind Kind { get; } /// - /// The value contained within within this . + /// The value contained within this . /// public readonly object? Value { get; } diff --git a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Field/Field.cs b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Field/Field.cs index 2b0aef9bd91..b5f23d9cb42 100644 --- a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Field/Field.cs +++ b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Field/Field.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq.Expressions; using System.Reflection; @@ -15,7 +14,6 @@ #if ELASTICSEARCH_SERVERLESS namespace Elastic.Clients.Elasticsearch.Serverless; #else - namespace Elastic.Clients.Elasticsearch; #endif @@ -25,6 +23,10 @@ public sealed class Field : IEquatable, IUrlParameter { +#if !NET && !NETSTANDARD2_1_OR_GREATER + private static readonly char[] BoostSplit = ['^']; +#endif + private readonly object _comparisonValue; private readonly Type? _type; @@ -67,22 +69,10 @@ public sealed class Field : #region Constructors - public Field(string name) : this(name, null, null) - { - } - - public Field(string name, double boost) : this(name, boost, null) - { - } - - public Field(string name, string format) : this(name, null, format) - { - } - - public Field(string name, double? boost, string? format) + public Field(string name, double? boost = null, string? format = null) { if (string.IsNullOrEmpty(name)) - throw new ArgumentException($"{name} can not be null or empty.", nameof(name)); + throw new ArgumentException("Field name can not be null or empty.", nameof(name)); Name = ParseFieldName(name, out var b); Boost = b ?? boost; @@ -91,6 +81,16 @@ public Field(string name, double? boost, string? format) _comparisonValue = Name; } + public Field(string name, double boost) : + this(name, boost, null) + { + } + + public Field(string name, string format) : + this(name, null, format) + { + } + public Field(Expression expression, double? boost = null, string? format = null) { Expression = expression ?? throw new ArgumentNullException(nameof(expression)); @@ -117,21 +117,21 @@ public Field(PropertyInfo property, double? boost = null, string? format = null) #region Factory Methods - public static Field? FromString(string? name) => string.IsNullOrEmpty(name) ? null : new Field(name); + public static Field FromString(string name) => new(name); - public static Field? FromExpression(Expression? expression) => expression is null ? null : new Field(expression); + public static Field FromExpression(Expression expression) => new(expression); - public static Field? FromProperty(PropertyInfo? property) => property is null ? null : new Field(property); + public static Field FromProperty(PropertyInfo property) => new(property); #endregion Factory Methods #region Conversion Operators - public static implicit operator Field?(string? name) => FromString(name); + public static implicit operator Field(string name) => FromString(name); - public static implicit operator Field?(Expression? expression) => FromExpression(expression); + public static implicit operator Field(Expression expression) => FromExpression(expression); - public static implicit operator Field?(PropertyInfo? property) => FromProperty(property); + public static implicit operator Field(PropertyInfo property) => FromProperty(property); #endregion Conversion Operators @@ -203,15 +203,7 @@ public override bool Equals(object? obj) => _ => false }; - public override int GetHashCode() - { - unchecked - { - var hashCode = _comparisonValue?.GetHashCode() ?? 0; - hashCode = (hashCode * 397) ^ (_type?.GetHashCode() ?? 0); - return hashCode; - } - } + public override int GetHashCode() => HashCode.Combine(_comparisonValue, _type); #endregion Equality @@ -243,20 +235,29 @@ public override string ToString() => #endregion Debugging - [return: NotNullIfNotNull(nameof(name))] - private static string? ParseFieldName(string? name, out double? boost) + private static string ParseFieldName(string name, out double? boost) { boost = null; - if (name is null) - return null; - var caretIndex = name.IndexOf('^'); - if (caretIndex == -1) +#if NET || NETSTANDARD2_1_OR_GREATER + if (!name.Contains('^', StringComparison.Ordinal)) return name; +#else + if (name.IndexOf('^') == -1) + return name; +#endif - var parts = name.Split(new[] { '^' }, 2, StringSplitOptions.RemoveEmptyEntries); +#if NET || NETSTANDARD2_1_OR_GREATER + var parts = name.Split('^', 2, StringSplitOptions.RemoveEmptyEntries); +#else + var parts = name.Split(BoostSplit, 2, StringSplitOptions.RemoveEmptyEntries); +#endif name = parts[0]; boost = double.Parse(parts[1], CultureInfo.InvariantCulture); + + if (string.IsNullOrWhiteSpace(name)) + throw new ArgumentException("Field name can not be null or empty.", nameof(name)); + return name; } } diff --git a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/Fields.cs b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/Fields.cs index cf610735c72..50ff90fde15 100644 --- a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/Fields.cs +++ b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/Fields.cs @@ -24,114 +24,168 @@ public sealed class Fields : IEnumerable, IUrlParameter { - internal readonly List ListOfFields; + private readonly List _listOfFields; + + /// + /// The list of fields. + /// + public IReadOnlyList ListOfFields => _listOfFields; #region Constructors - internal Fields() => ListOfFields = []; + public Fields() => _listOfFields = []; - internal Fields(IEnumerable fields) + public Fields(IEnumerable fields) { if (fields is null) throw new ArgumentNullException(nameof(fields)); - ListOfFields = [.. fields.Where(f => f is not null)]; + _listOfFields = [.. fields]; + + if (_listOfFields.Any(x => x is null)) + throw new ArgumentException("Fields enumeration must not contain 'null' values."); } #endregion Constructors #region Factory Methods - public static Fields? FromField(Field? field) => field is null - ? null - : new Fields([field]); + public static Fields FromField(Field field) + { + if (field is null) + throw new ArgumentNullException(nameof(field)); + + return new Fields([field]); + } + + public static Fields FromFields(Field[] fields) + { + if (fields is null) + throw new ArgumentNullException(nameof(fields)); + + return new Fields(fields); + } + + public static Fields FromString(string name) + { + if (name is null) + throw new ArgumentNullException(nameof(name)); + + var split = name.Split(','); + + return new Fields(split.Select(f => new Field(f))); + } + + public static Fields FromStrings(string[] names) + { + if (names is null) + throw new ArgumentNullException(nameof(names)); + + return new Fields(names.Select(f => new Field(f))); + } + + public static Fields FromExpression(Expression expression) + { + if (expression is null) + throw new ArgumentNullException(nameof(expression)); + + return new Fields([new Field(expression)]); + } + + public static Fields FromExpressions(Expression[] expressions) + { + if (expressions is null) + throw new ArgumentNullException(nameof(expressions)); - public static Fields? FromFields(Field[]? fields) => fields.IsNullOrEmpty() - ? null - : new Fields(fields!); + return new Fields(expressions.Select(f => new Field(f))); + } - public static Fields? FromString(string? name) => name.IsNullOrEmptyCommaSeparatedList(out var split) - ? null - : new Fields(split.Select(f => new Field(f))); + public static Fields FromExpression(Expression> expression) + { + if (expression is null) + throw new ArgumentNullException(nameof(expression)); - public static Fields? FromStrings(string[]? names) => names.IsNullOrEmpty() - ? null - : new Fields(names!.Select(f => new Field(f))); + return new Fields([new Field(expression)]); + } - public static Fields? FromExpression(Expression? expression) => expression is null - ? null - : new Fields([new Field(expression)]); + public static Fields FromExpressions(Expression>[] expressions) + { + if (expressions is null) + throw new ArgumentNullException(nameof(expressions)); - public static Fields? FromExpressions(Expression[]? expressions) => expressions.IsNullOrEmpty() - ? null - : new Fields(expressions!.Select(f => new Field(f))); + return new Fields(expressions.Select(f => new Field(f))); + } - public static Fields? FromExpression(Expression>? expression) => expression is null - ? null - : new Fields([new Field(expression)]); + public static Fields FromProperty(PropertyInfo property) + { + if (property is null) + throw new ArgumentNullException(nameof(property)); - public static Fields? FromExpressions(Expression>[]? expressions) => expressions.IsNullOrEmpty() - ? null - : new Fields(expressions!.Select(f => new Field(f))); + return new Fields([property]); + } - public static Fields? FromProperty(PropertyInfo? property) => property is null - ? null - : new Fields([property]); + public static Fields FromProperties(PropertyInfo[] properties) + { + if (properties is null) + throw new ArgumentNullException(nameof(properties)); - public static Fields? FromProperties(PropertyInfo[]? properties) => properties.IsNullOrEmpty() - ? null - : new Fields(properties!.Select(f => new Field(f))); + return new Fields(properties.Select(f => new Field(f))); + } #endregion Factory Methods #region Conversion Operators - public static implicit operator Fields?(Field? field) => FromField(field); + public static implicit operator Fields(Field field) => FromField(field); - public static implicit operator Fields?(Field[]? fields) => FromFields(fields); + public static implicit operator Fields(Field[] fields) => FromFields(fields); - public static implicit operator Fields?(string? name) => FromString(name); + public static implicit operator Fields(string name) => FromString(name); - public static implicit operator Fields?(string[]? names) => FromStrings(names); + public static implicit operator Fields(string[] names) => FromStrings(names); - public static implicit operator Fields?(Expression? expression) => FromExpression(expression); + public static implicit operator Fields(Expression expression) => FromExpression(expression); - public static implicit operator Fields?(Expression[]? expressions) => FromExpressions(expressions); + public static implicit operator Fields(Expression[] expressions) => FromExpressions(expressions); - public static implicit operator Fields?(PropertyInfo? property) => FromProperty(property); + public static implicit operator Fields(PropertyInfo property) => FromProperty(property); - public static implicit operator Fields?(PropertyInfo[]? properties) => FromProperties(properties); + public static implicit operator Fields(PropertyInfo[] properties) => FromProperties(properties); #endregion Conversion Operators #region Combinator Methods - public Fields And(params Field?[] fields) + public Fields And(params Field[] fields) { if (fields is null) throw new ArgumentNullException(nameof(fields)); - ListOfFields.AddRange(fields.Where(f => f is not null)); + // ReSharper disable once ConditionIsAlwaysTrueOrFalse + if (fields.Any(x => x is null)) + throw new ArgumentException("Fields enumeration must not contain 'null' values."); + + _listOfFields.AddRange(fields); return this; } - public Fields And(params string?[] names) + public Fields And(params string[] names) { if (names is null) throw new ArgumentNullException(nameof(names)); - ListOfFields.AddRange(names.Where(f => f is not null).Select(f => new Field(f))); + _listOfFields.AddRange(names.Select(f => new Field(f))); return this; } - public Fields And(params Expression>?[] expressions) + public Fields And(params Expression>[] expressions) { if (expressions is null) throw new ArgumentNullException(nameof(expressions)); - ListOfFields.AddRange(expressions.Where(f => f is not null).Select(f => new Field(f))); + _listOfFields.AddRange(expressions.Select(f => new Field(f))); return this; } @@ -142,17 +196,17 @@ public Fields And(Expression> expression, double? boo if (expression is null) throw new ArgumentNullException(nameof(expression)); - ListOfFields.Add(new Field(expression, boost, format)); + _listOfFields.Add(new Field(expression, boost, format)); return this; } - public Fields And(params PropertyInfo?[] properties) + public Fields And(params PropertyInfo[] properties) { if (properties is null) throw new ArgumentNullException(nameof(properties)); - ListOfFields.AddRange(properties.Where(x => x is not null).Select(f => new Field(f))); + _listOfFields.AddRange(properties.Select(f => new Field(f))); return this; } @@ -180,9 +234,9 @@ public override bool Equals(object? obj) => _ => false }; - public override int GetHashCode() => ListOfFields.GetHashCode(); + public override int GetHashCode() => _listOfFields.GetHashCode(); - public bool Equals(Fields? other) => (ListOfFields, other?.ListOfFields) switch + public bool Equals(Fields? other) => (_listOfFields, other?._listOfFields) switch { (null, null) => true, ({ } a, { } b) => (a.Count == b.Count) && !a.Except(b).Any(), @@ -195,7 +249,7 @@ public override bool Equals(object? obj) => IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - public IEnumerator GetEnumerator() => ListOfFields.GetEnumerator(); + public IEnumerator GetEnumerator() => _listOfFields.GetEnumerator(); #endregion IEnumerable @@ -209,7 +263,7 @@ string IUrlParameter.GetString(ITransportConfiguration? settings) $"Can not resolve {nameof(Fields)} if no {nameof(IElasticsearchClientSettings)} is provided"); } - return string.Join(",", ListOfFields.Select(f => ((IUrlParameter)f).GetString(elasticsearchClientSettings))); + return string.Join(",", _listOfFields.Select(f => ((IUrlParameter)f).GetString(elasticsearchClientSettings))); } #endregion IUrlParameter @@ -217,8 +271,8 @@ string IUrlParameter.GetString(ITransportConfiguration? settings) #region Debugging public override string ToString() => - $"Count: {ListOfFields.Count} [" + - string.Join(",", ListOfFields.Select((t, i) => $"({i + 1}: {t?.DebuggerDisplay})")) + "]"; + $"Count: {_listOfFields.Count} [" + + string.Join(",", _listOfFields.Select((f, i) => $"({i + 1}: {f?.DebuggerDisplay})")) + "]"; private string DebuggerDisplay => ToString(); diff --git a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/FieldsConverter.cs b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/FieldsConverter.cs index 2e7c514c3b3..9ad9863782e 100644 --- a/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/FieldsConverter.cs +++ b/src/Elastic.Clients.Elasticsearch/_Shared/Core/Infer/Fields/FieldsConverter.cs @@ -31,7 +31,7 @@ internal sealed class FieldsConverter : JsonConverter } } - public override void Write(Utf8JsonWriter writer, Fields value, JsonSerializerOptions options) + public override void Write(Utf8JsonWriter writer, Fields? value, JsonSerializerOptions options) { if (value is null) { @@ -65,7 +65,7 @@ internal sealed class SingleOrManyFieldsConverter : JsonConverter } } - public override void Write(Utf8JsonWriter writer, Fields value, JsonSerializerOptions options) + public override void Write(Utf8JsonWriter writer, Fields? value, JsonSerializerOptions options) { if (value is null) {