From 551a515bf2fe65582c70300cf6c6739b8f0003f8 Mon Sep 17 00:00:00 2001 From: Stuart Cam Date: Mon, 18 Mar 2019 21:24:48 +1100 Subject: [PATCH 1/3] Ensure GetHashcode() returns same value for out of order lists. Prefer empty lists over nulls for internal _actionIds collection. --- src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs index 478ad5a865e..bbe6c140d19 100644 --- a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs +++ b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs @@ -34,13 +34,13 @@ public bool Equals(ActionIds other) string IUrlParameter.GetString(IConnectionConfigurationValues settings) => string.Join(",", _actionIds); public static implicit operator ActionIds(string actionIds) => - actionIds.IsNullOrEmptyCommaSeparatedList(out var list) ? null : new ActionIds(list); + actionIds.IsNullOrEmptyCommaSeparatedList(out var list) ? new ActionIds(null) : new ActionIds(list); - public static implicit operator ActionIds(string[] actionIds) => actionIds.IsEmpty() ? null : new ActionIds(actionIds); + public static implicit operator ActionIds(string[] actionIds) => actionIds.IsEmpty() ? new ActionIds(null) : new ActionIds(actionIds); public override bool Equals(object obj) => obj is ActionIds other && Equals(other); - public override int GetHashCode() => _actionIds.GetHashCode(); + public override int GetHashCode() => _actionIds.OrderBy(s => s).GetHashCode(); public static bool operator ==(ActionIds left, ActionIds right) => Equals(left, right); From d42b2b6c71dba02ea65dda90371feb4a2fc25633 Mon Sep 17 00:00:00 2001 From: Stuart Cam Date: Mon, 18 Mar 2019 21:29:07 +1100 Subject: [PATCH 2/3] Fix ambiguous method invocation. --- src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs index bbe6c140d19..1e1fbcfa4af 100644 --- a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs +++ b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs @@ -34,9 +34,10 @@ public bool Equals(ActionIds other) string IUrlParameter.GetString(IConnectionConfigurationValues settings) => string.Join(",", _actionIds); public static implicit operator ActionIds(string actionIds) => - actionIds.IsNullOrEmptyCommaSeparatedList(out var list) ? new ActionIds(null) : new ActionIds(list); + actionIds.IsNullOrEmptyCommaSeparatedList(out var list) ? new ActionIds((string)null) : new ActionIds(list); - public static implicit operator ActionIds(string[] actionIds) => actionIds.IsEmpty() ? new ActionIds(null) : new ActionIds(actionIds); + public static implicit operator ActionIds(string[] actionIds) => + actionIds.IsEmpty() ? new ActionIds((string)null) : new ActionIds(actionIds); public override bool Equals(object obj) => obj is ActionIds other && Equals(other); From 7e8b4e425b8f7a93500e1ccb14bd6c1d5c36bcae Mon Sep 17 00:00:00 2001 From: russcam Date: Tue, 19 Mar 2019 11:18:08 +1000 Subject: [PATCH 3/3] Fix ActionIds equality and hashcodes --- .../Infer/ActionIds/ActionIds.cs | 41 ++++++++++++------- .../Tests/CommonOptions/ActionIdsTests.cs | 27 ++++++++++++ 2 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 src/Tests/Tests/CommonOptions/ActionIdsTests.cs diff --git a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs index 1e1fbcfa4af..9eeed7110f7 100644 --- a/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs +++ b/src/Nest/CommonAbstractions/Infer/ActionIds/ActionIds.cs @@ -11,37 +11,48 @@ public class ActionIds : IUrlParameter, IEquatable { private readonly List _actionIds; - public ActionIds(IEnumerable actionIds) => _actionIds = actionIds?.ToList() ?? new List(); + public ActionIds(IEnumerable actionIds) => _actionIds = actionIds?.ToList(); - public ActionIds(string actionIds) => _actionIds = actionIds.IsNullOrEmpty() - ? new List() - : actionIds.Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries) - .Select(s => s.Trim()) - .ToList(); - - internal IReadOnlyList Ids => _actionIds; + public ActionIds(string actionIds) + { + if (!actionIds.IsNullOrEmptyCommaSeparatedList(out var arr)) + _actionIds = arr.ToList(); + } private string DebugDisplay => ((IUrlParameter)this).GetString(null); public bool Equals(ActionIds other) { - if (Ids == null && other.Ids == null) return true; - if (Ids == null || other.Ids == null) return false; + if (other == null) return false; + if (_actionIds == null && other._actionIds == null) return true; + if (_actionIds == null || other._actionIds == null) return false; - return Ids.Count == other.Ids.Count && !Ids.Except(other.Ids).Any(); + return _actionIds.Count == other._actionIds.Count && + _actionIds.OrderBy(id => id).SequenceEqual(other._actionIds.OrderBy(id => id)); } - string IUrlParameter.GetString(IConnectionConfigurationValues settings) => string.Join(",", _actionIds); + string IUrlParameter.GetString(IConnectionConfigurationValues settings) => + string.Join(",", _actionIds ?? Enumerable.Empty()); public static implicit operator ActionIds(string actionIds) => - actionIds.IsNullOrEmptyCommaSeparatedList(out var list) ? new ActionIds((string)null) : new ActionIds(list); + actionIds.IsNullOrEmptyCommaSeparatedList(out var arr) ? null : new ActionIds(arr); public static implicit operator ActionIds(string[] actionIds) => - actionIds.IsEmpty() ? new ActionIds((string)null) : new ActionIds(actionIds); + actionIds.IsEmpty() ? null : new ActionIds(actionIds); public override bool Equals(object obj) => obj is ActionIds other && Equals(other); - public override int GetHashCode() => _actionIds.OrderBy(s => s).GetHashCode(); + public override int GetHashCode() + { + if (_actionIds == null) return 0; + unchecked + { + var hc = 0; + foreach (var id in _actionIds.OrderBy(id => id)) + hc = hc * 17 + id.GetHashCode(); + return hc; + } + } public static bool operator ==(ActionIds left, ActionIds right) => Equals(left, right); diff --git a/src/Tests/Tests/CommonOptions/ActionIdsTests.cs b/src/Tests/Tests/CommonOptions/ActionIdsTests.cs new file mode 100644 index 00000000000..1b26dd49ec9 --- /dev/null +++ b/src/Tests/Tests/CommonOptions/ActionIdsTests.cs @@ -0,0 +1,27 @@ +using Elastic.Xunit.XunitPlumbing; +using FluentAssertions; +using Nest; + +namespace Tests.CommonOptions +{ + public class ActionIdsTests + { + [U] public void Equal() + { + var actionIds1 = new ActionIds("1,2,3"); + var actionIds2 = new ActionIds(new [] { "3", "2", "1" }); + + actionIds1.Should().Be(actionIds2); + actionIds1.GetHashCode().Should().Be(actionIds2.GetHashCode()); + } + + [U] public void NotEqual() + { + var actionIds1 = new ActionIds("1,2,3,3"); + var actionIds2 = new ActionIds(new [] { "3", "2", "1" }); + + actionIds1.Should().NotBe(actionIds2); + actionIds1.GetHashCode().Should().NotBe(actionIds2.GetHashCode()); + } + } +}