From 11f995897ab56fa7608e4d2164aa9c58b0fab164 Mon Sep 17 00:00:00 2001 From: Scott Fauerbach Date: Fri, 12 May 2023 09:09:49 -0400 Subject: [PATCH] Fixed GetChanges for backoff and metadata (#769) * Fixed GetChanges for backoff and metadata * fix uses of obsolete --- src/NATS.Client/Internals/JsonUtils.cs | 14 +++- src/NATS.Client/Internals/Validator.cs | 20 ++++- .../JetStream/ConsumerConfiguration.cs | 75 ++++++++++++------- .../JetStream/StreamConfiguration.cs | 2 +- src/NATS.Client/Service/Endpoint.cs | 2 +- src/NATS.Client/Service/ServiceResponse.cs | 2 +- 6 files changed, 80 insertions(+), 35 deletions(-) diff --git a/src/NATS.Client/Internals/JsonUtils.cs b/src/NATS.Client/Internals/JsonUtils.cs index 20e9b2393..a0bf9c723 100644 --- a/src/NATS.Client/Internals/JsonUtils.cs +++ b/src/NATS.Client/Internals/JsonUtils.cs @@ -68,7 +68,7 @@ public static List StringList(JSONNode node, string field) return list; } - public static List DurationList(JSONNode node, string field) + public static List DurationList(JSONNode node, string field, bool nullIfEmpty = false) { List list = new List(); foreach (var child in node[field].Children) @@ -76,7 +76,7 @@ public static List DurationList(JSONNode node, string field) list.Add(Duration.OfNanos(child.AsLong)); } - return list; + return list.Count == 0 && nullIfEmpty ? null : list; } public static List OptionalStringList(JSONNode node, string field) @@ -84,8 +84,14 @@ public static List OptionalStringList(JSONNode node, string field) List list = StringList(node, field); return list.Count == 0 ? null : list; } - + + [Obsolete("Method name replaced with proper spelling, 'StringStringDictionary'")] public static Dictionary StringStringDictionay(JSONNode node, string field) + { + return StringStringDictionary(node, field, false); + } + + public static Dictionary StringStringDictionary(JSONNode node, string field, bool nullIfEmpty = false) { Dictionary temp = new Dictionary(); JSONNode meta = node[field]; @@ -93,7 +99,7 @@ public static Dictionary StringStringDictionay(JSONNode node, st { temp[key] = meta[key]; } - return temp; + return temp.Count == 0 && nullIfEmpty ? null : temp; } public static MsgHeader AsHeaders(JSONNode node, string field) diff --git a/src/NATS.Client/Internals/Validator.cs b/src/NATS.Client/Internals/Validator.cs index bf40bf2b0..54d0f507b 100644 --- a/src/NATS.Client/Internals/Validator.cs +++ b/src/NATS.Client/Internals/Validator.cs @@ -13,6 +13,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Text; using System.Text.RegularExpressions; @@ -637,6 +638,21 @@ public static bool IsSemVer(String s) return Regex.IsMatch(s, SemVerPattern); } + public static bool SequenceEqual(IList l1, IList l2, bool nullSecondEqualsEmptyFirst) + { + if (l1 == null) + { + return l2 == null; + } + + if (l2 == null) + { + return nullSecondEqualsEmptyFirst && l1.Count == 0; + } + + return l1.SequenceEqual(l2); + } + public static bool DictionariesEqual(IDictionary d1, IDictionary d2) { if (d1 == d2) @@ -651,9 +667,7 @@ public static bool DictionariesEqual(IDictionary d1, IDictionary foreach (KeyValuePair pair in d1) { - string value; - if (!d2.TryGetValue(pair.Key, out value) || - !pair.Value.Equals(value)) + if (!d2.TryGetValue(pair.Key, out var value) || !pair.Value.Equals(value)) { return false; } diff --git a/src/NATS.Client/JetStream/ConsumerConfiguration.cs b/src/NATS.Client/JetStream/ConsumerConfiguration.cs index aaf07d0d7..ff070b84e 100644 --- a/src/NATS.Client/JetStream/ConsumerConfiguration.cs +++ b/src/NATS.Client/JetStream/ConsumerConfiguration.cs @@ -55,6 +55,8 @@ public sealed class ConsumerConfiguration : JsonSerializable internal bool? _flowControl; internal bool? _headersOnly; internal bool? _memStorage; + internal IList _backoff; + internal Dictionary _metadata; public DeliverPolicy DeliverPolicy => _deliverPolicy ?? DeliverPolicy.All; public AckPolicy AckPolicy => _ackPolicy ?? AckPolicy.Explicit; @@ -85,8 +87,8 @@ public sealed class ConsumerConfiguration : JsonSerializable public bool FlowControl => _flowControl ?? false; public bool HeadersOnly => _headersOnly ?? false; public bool MemStorage => _memStorage ?? false; - public IList Backoff { get; } - public Dictionary Metadata { get; } + public IList Backoff => _backoff ?? new List(); + public Dictionary Metadata => _metadata ?? new Dictionary(); internal ConsumerConfiguration(string json) : this(JSON.Parse(json)) {} @@ -122,8 +124,8 @@ internal ConsumerConfiguration(JSONNode ccNode) _headersOnly = ccNode[ApiConstants.HeadersOnly].AsBool; _memStorage = ccNode[ApiConstants.MemStorage].AsBool; - Backoff = DurationList(ccNode, ApiConstants.Backoff); - Metadata = JsonUtils.StringStringDictionay(ccNode, ApiConstants.Metadata); + _backoff = DurationList(ccNode, ApiConstants.Backoff, true); + _metadata = StringStringDictionary(ccNode, ApiConstants.Metadata, true); } private ConsumerConfiguration(ConsumerConfigurationBuilder builder) @@ -158,8 +160,8 @@ private ConsumerConfiguration(ConsumerConfigurationBuilder builder) _maxBytes = builder._maxBytes; _numReplicas = builder._numReplicas; - Backoff = builder._backoff; - Metadata = builder._metadata; + _backoff = builder._backoff; + _metadata = builder._metadata; } public override JSONNode ToJsonNode() @@ -230,13 +232,9 @@ internal IList GetChanges(ConsumerConfiguration server) RecordWouldBeChange(SampleFrequency, server.SampleFrequency, "SampleFrequency", changes); RecordWouldBeChange(DeliverSubject, server.DeliverSubject, "DeliverSubject", changes); RecordWouldBeChange(DeliverGroup, server.DeliverGroup, "DeliverGroup", changes); - - if (!Backoff.SequenceEqual(server.Backoff)) { changes.Add("Backoff"); } - if (!Validator.DictionariesEqual(Metadata, server.Metadata)) - { - changes.Add("Metadata"); - } + if (_backoff != null && !SequenceEqual(_backoff, server._backoff, true)) { changes.Add("Backoff"); } + if (_metadata != null && !DictionariesEqual(_metadata, server._metadata)) { changes.Add("Metadata"); } return changes; } @@ -334,8 +332,8 @@ public sealed class ConsumerConfigurationBuilder internal bool? _flowControl; internal bool? _headersOnly; internal bool? _memStorage; - internal IList _backoff = new List(); - internal Dictionary _metadata = new Dictionary(); + internal IList _backoff; + internal Dictionary _metadata; public ConsumerConfigurationBuilder() {} @@ -372,10 +370,18 @@ public ConsumerConfigurationBuilder(ConsumerConfiguration cc) _flowControl = cc._flowControl; _headersOnly = cc._headersOnly; _memStorage = cc._memStorage; - _backoff = new List(cc.Backoff); - foreach (string key in cc.Metadata.Keys) + + if (cc._backoff != null) { - _metadata[key] = cc.Metadata[key]; + _backoff = new List(cc._backoff); + } + + if (cc._metadata != null) + { + foreach (string key in cc.Metadata.Keys) + { + _metadata[key] = cc.Metadata[key]; + } } } @@ -775,20 +781,29 @@ public ConsumerConfigurationBuilder WithMemStorage(bool? memStorage) { /// /// zero or more backoff durations or an array of backoffs /// The ConsumerConfigurationBuilder - public ConsumerConfigurationBuilder WithBackoff(params Duration[] backoffs) { - _backoff.Clear(); - if (backoffs != null) { + public ConsumerConfigurationBuilder WithBackoff(params Duration[] backoffs) + { + if (backoffs == null || (backoffs.Length == 1 && backoffs[0] == null)) + { + _backoff = null; + } + else + { + _backoff = new List(); foreach (Duration d in backoffs) { - if (d != null) { + if (d != null) + { if (d.Nanos < DurationMinLong) { throw new ArgumentException($"Backoff cannot be less than {DurationMinLong}"); } + _backoff.Add(d); } } } + return this; } @@ -798,8 +813,13 @@ public ConsumerConfigurationBuilder WithBackoff(params Duration[] backoffs) { /// zero or more backoff in millis or an array of backoffsMillis /// The ConsumerConfigurationBuilder public ConsumerConfigurationBuilder WithBackoff(params long[] backoffsMillis) { - _backoff.Clear(); - if (backoffsMillis != null) { + if (backoffsMillis == null) + { + _backoff = null; + } + else + { + _backoff = new List(); foreach (long ms in backoffsMillis) { if (ms < DurationMinLong) { @@ -808,6 +828,7 @@ public ConsumerConfigurationBuilder WithBackoff(params long[] backoffsMillis) { _backoff.Add(Duration.OfMillis(ms)); } } + return this; } @@ -817,9 +838,13 @@ public ConsumerConfigurationBuilder WithBackoff(params long[] backoffsMillis) { /// the metadata dictionary /// The ConsumerConfigurationBuilder public ConsumerConfigurationBuilder WithMetadata(Dictionary metadata) { - _metadata.Clear(); - if (metadata != null) + if (metadata == null) + { + _metadata = null; + } + else { + _metadata = new Dictionary(); foreach (string key in metadata.Keys) { _metadata[key] = metadata[key]; diff --git a/src/NATS.Client/JetStream/StreamConfiguration.cs b/src/NATS.Client/JetStream/StreamConfiguration.cs index e87bc3a43..f90667390 100644 --- a/src/NATS.Client/JetStream/StreamConfiguration.cs +++ b/src/NATS.Client/JetStream/StreamConfiguration.cs @@ -84,7 +84,7 @@ internal StreamConfiguration(JSONNode scNode) DenyDelete = scNode[ApiConstants.DenyDelete].AsBool; DenyPurge = scNode[ApiConstants.DenyPurge].AsBool; DiscardNewPerSubject = scNode[ApiConstants.DiscardNewPerSubject].AsBool; - Metadata = JsonUtils.StringStringDictionay(scNode, ApiConstants.Metadata); + Metadata = JsonUtils.StringStringDictionary(scNode, ApiConstants.Metadata); } private StreamConfiguration(StreamConfigurationBuilder builder) diff --git a/src/NATS.Client/Service/Endpoint.cs b/src/NATS.Client/Service/Endpoint.cs index f02944b5e..69a2be8e1 100644 --- a/src/NATS.Client/Service/Endpoint.cs +++ b/src/NATS.Client/Service/Endpoint.cs @@ -48,7 +48,7 @@ internal Endpoint(JSONNode node) Name = node[ApiConstants.Name]; Subject = node[ApiConstants.Subject]; Schema = Schema.OptionalInstance(node[ApiConstants.Schema]); - Metadata = JsonUtils.StringStringDictionay(node, ApiConstants.Metadata); + Metadata = JsonUtils.StringStringDictionary(node, ApiConstants.Metadata); } private Endpoint(EndpointBuilder b) diff --git a/src/NATS.Client/Service/ServiceResponse.cs b/src/NATS.Client/Service/ServiceResponse.cs index d1b8e2d9e..3c1e4065b 100644 --- a/src/NATS.Client/Service/ServiceResponse.cs +++ b/src/NATS.Client/Service/ServiceResponse.cs @@ -48,7 +48,7 @@ internal ServiceResponse(string type, JSONNode node) Id = Validator.Required(node[ApiConstants.Id], "Id"); Name = Validator.Required(node[ApiConstants.Name], "Name"); Version = Validator.ValidateSemVer(node[ApiConstants.Version], "Version", true); - Metadata = JsonUtils.StringStringDictionay(node, ApiConstants.Metadata); + Metadata = JsonUtils.StringStringDictionary(node, ApiConstants.Metadata); } protected JSONObject BaseJsonObject()