Merge pull request #831 from rabbitmq/rabbitmq-dotnet-client-827

Introduce PublicationAddress.TryParse and switch BasicProperties.ReplyToAddress to use it

Author: michaelklishin

projects/RabbitMQ.Client/client/api/IBasicProperties.cs

@@ -121,9 +121,9 @@ public interface IBasicProperties : IContentHeader
         string ReplyTo { get; set; }
 
         /// <summary>
-        /// Convenience property; parses <see cref="ReplyTo"/> property using <see cref="PublicationAddress.Parse"/>,
+        /// Convenience property; parses <see cref="ReplyTo"/> property using <see cref="PublicationAddress.TryParse"/>,
         /// and serializes it using <see cref="PublicationAddress.ToString"/>.
-        /// Returns null if <see cref="ReplyTo"/> property cannot be parsed by <see cref="PublicationAddress.Parse"/>.
+        /// Returns null if <see cref="ReplyTo"/> property cannot be parsed by <see cref="PublicationAddress.TryParse"/>.
         /// </summary>
         PublicationAddress ReplyToAddress { get; set; }
 

projects/RabbitMQ.Client/client/api/PublicationAddress.cs

@@ -116,6 +116,32 @@ public static PublicationAddress Parse(string uriLikeString)
             return null;
         }
 
+        public static bool TryParse(string uriLikeString, out PublicationAddress result)
+        {
+            // Callers such as IBasicProperties.ReplyToAddress
+            // expect null result for invalid input.
+            // The regex.Match() throws on null arguments so we perform explicit check here
+            if (uriLikeString == null)
+            {
+                result = null;
+                return false;
+            }
+            else
+            {
+                try
+                {
+                    var res = Parse(uriLikeString);
+                    result = res;
+                    return true;
+                }
+                catch
+                {
+                    result = null;
+                    return false;
+                }
+            }
+        }
+
         /// <summary>
         /// Reconstruct the "uri" from its constituents.
         /// </summary>

projects/RabbitMQ.Client/client/impl/BasicProperties.cs

@@ -109,13 +109,16 @@ public bool Persistent
         public abstract string ReplyTo { get; set; }
 
         /// <summary>
-        /// Convenience property; parses <see cref="ReplyTo"/> property using <see cref="PublicationAddress.Parse"/>,
+        /// Convenience property; parses <see cref="ReplyTo"/> property using <see cref="PublicationAddress.TryParse"/>,
         /// and serializes it using <see cref="PublicationAddress.ToString"/>.
-        /// Returns null if <see cref="ReplyTo"/> property cannot be parsed by <see cref="PublicationAddress.Parse"/>.
+        /// Returns null if <see cref="ReplyTo"/> property cannot be parsed by <see cref="PublicationAddress.TryParse"/>.
         /// </summary>
         public PublicationAddress ReplyToAddress
         {
-            get { return PublicationAddress.Parse(ReplyTo); }
+            get { 
+                PublicationAddress.TryParse(ReplyTo, out var result);
+                return result;
+            }
             set { ReplyTo = value.ToString(); }
         }
 

projects/Unit/APIApproval.Approve.verified.txt

@@ -483,6 +483,7 @@ namespace RabbitMQ.Client
         public string RoutingKey { get; }
         public override string ToString() { }
         public static RabbitMQ.Client.PublicationAddress Parse(string uriLikeString) { }
+        public static bool TryParse(string uriLikeString, out RabbitMQ.Client.PublicationAddress result) { }
     }
     public class QueueDeclareOk
     {

projects/Unit/TestBasicProperties.cs

@@ -52,7 +52,6 @@ public void TestPersistentPropertyChangesDeliveryMode_PersistentTrueDelivery2()
             // Arrange
             var subject = new Framing.BasicProperties
             {
-
                 // Act
                 Persistent = true
             };
@@ -120,5 +119,35 @@ public void TestNullableProperties_CanWrite(
             Assert.AreEqual(isCorrelationIdPresent, propertiesFromStream.IsCorrelationIdPresent());
             Assert.AreEqual(isMessageIdPresent, propertiesFromStream.IsMessageIdPresent());
         }
+
+        [Test]
+        public void TestProperties_ReplyTo([Values(null, "foo_1", "fanout://name/key")] string replyTo)
+        {
+            // Arrange
+            var subject = new Framing.BasicProperties
+            {
+                // Act
+                ReplyTo = replyTo,
+            };
+
+            // Assert
+            bool isReplyToPresent = replyTo != null;
+            PublicationAddress result;
+            PublicationAddress.TryParse(replyTo, out result);
+            string replyToAddress = result?.ToString();
+            Assert.AreEqual(isReplyToPresent, subject.IsReplyToPresent());
+
+            var writer = new Impl.ContentHeaderPropertyWriter(new byte[1024]);
+            subject.WritePropertiesTo(ref writer);
+
+            // Read from Stream
+            var propertiesFromStream = new Framing.BasicProperties();
+            var reader = new Impl.ContentHeaderPropertyReader(writer.Memory.Slice(0, writer.Offset));
+            propertiesFromStream.ReadPropertiesFrom(ref reader);
+
+            Assert.AreEqual(replyTo, propertiesFromStream.ReplyTo);
+            Assert.AreEqual(isReplyToPresent, propertiesFromStream.IsReplyToPresent());
+            Assert.AreEqual(replyToAddress, propertiesFromStream.ReplyToAddress?.ToString());
+        }
     }
 }

projects/Unit/TestPropertiesClone.cs

@@ -68,10 +68,10 @@ private void TestBasicPropertiesClone(BasicProperties bp)
             bp.ContentType = "foo_1";
             bp.ContentEncoding = "foo_2";
             bp.Headers = new Dictionary<string, object>
-        {
-            { "foo_3", "foo_4" },
-            { "foo_5", "foo_6" }
-        };
+            {
+                { "foo_3", "foo_4" },
+                { "foo_5", "foo_6" }
+            };
             bp.DeliveryMode = 2;
             // Persistent also changes DeliveryMode's value to 2
             bp.Persistent = true;
@@ -123,6 +123,7 @@ private void TestBasicPropertiesClone(BasicProperties bp)
             Assert.AreEqual(12, bpClone.Priority);
             Assert.AreEqual("foo_7", bpClone.CorrelationId);
             Assert.AreEqual("foo_8", bpClone.ReplyTo);
+            Assert.AreEqual(null, bpClone.ReplyToAddress);
             Assert.AreEqual("foo_9", bpClone.Expiration);
             Assert.AreEqual("foo_10", bpClone.MessageId);
             Assert.AreEqual(new AmqpTimestamp(123), bpClone.Timestamp);
@@ -141,10 +142,10 @@ private void TestBasicPropertiesNoneClone(BasicProperties bp)
             bp.ContentType = "foo_1";
             bp.ContentEncoding = "foo_2";
             bp.Headers = new Dictionary<string, object>
-        {
-            { "foo_3", "foo_4" },
-            { "foo_5", "foo_6" }
-        };
+            {
+                { "foo_3", "foo_4" },
+                { "foo_5", "foo_6" }
+            };
             bp.DeliveryMode = 2;
             // Persistent also changes DeliveryMode's value to 2
             bp.Persistent = true;

projects/Unit/TestPublicationAddress.cs

@@ -57,9 +57,29 @@ public void TestParseOk()
         }
 
         [Test]
-        public void TestParseFail()
+        public void TestParseFailWithANE()
         {
-            Assert.IsNull(PublicationAddress.Parse("not a valid uri"));
+            Assert.That(()=> PublicationAddress.Parse(null), Throws.ArgumentNullException);
+        }
+
+        [Test]
+        public void TestParseFailWithUnparseableInput()
+        {
+            Assert.IsNull(PublicationAddress.Parse("not a valid URI"));
+        }
+
+        [Test]
+        public void TestTryParseFail()
+        {
+            PublicationAddress result;
+            PublicationAddress.TryParse(null, out result);
+            Assert.IsNull(result);
+
+            PublicationAddress.TryParse("not a valid URI", out result);
+            Assert.IsNull(result);
+
+            PublicationAddress.TryParse("}}}}}}}}", out result);
+            Assert.IsNull(result);
         }
 
         [Test]