This is an automated email from the ASF dual-hosted git repository.

aaronai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/rocketmq-clients.git

commit 6e9fd521444c5d8c1151dfec1c0876462c9c7588
Author: Aaron Ai <[email protected]>
AuthorDate: Tue Feb 28 15:55:40 2023 +0800

    Add more limitation for Producer/Message/SimpleConsumer setter
---
 csharp/examples/SimpleConsumerExample.cs        |  2 +-
 csharp/rocketmq-client-csharp/Consumer.cs       |  2 ++
 csharp/rocketmq-client-csharp/Message.cs        |  4 ++++
 csharp/rocketmq-client-csharp/Producer.cs       | 12 ++++++++++--
 csharp/rocketmq-client-csharp/SimpleConsumer.cs | 21 ++++++++++++++++++---
 csharp/tests/MessageTest.cs                     | 16 +++++++++++++++-
 6 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/csharp/examples/SimpleConsumerExample.cs 
b/csharp/examples/SimpleConsumerExample.cs
index 924de61d..51d9c32d 100644
--- a/csharp/examples/SimpleConsumerExample.cs
+++ b/csharp/examples/SimpleConsumerExample.cs
@@ -46,7 +46,7 @@ namespace examples
             var subscription = new Dictionary<string, FilterExpression>
                 { { topic, new FilterExpression("*") } };
             // In most case, you don't need to create too many consumers, 
single pattern is recommended.
-            await using var simpleConsumer = new SimpleConsumer.Builder()
+            await using var simpleConsumer = await new SimpleConsumer.Builder()
                 .SetClientConfig(clientConfig)
                 .SetConsumerGroup(consumerGroup)
                 .SetAwaitDuration(TimeSpan.FromSeconds(15))
diff --git a/csharp/rocketmq-client-csharp/Consumer.cs 
b/csharp/rocketmq-client-csharp/Consumer.cs
index 925f1852..2ba1de1e 100644
--- a/csharp/rocketmq-client-csharp/Consumer.cs
+++ b/csharp/rocketmq-client-csharp/Consumer.cs
@@ -18,6 +18,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Google.Protobuf.WellKnownTypes;
 using Proto = Apache.Rocketmq.V2;
@@ -26,6 +27,7 @@ namespace Org.Apache.Rocketmq
 {
     public abstract class Consumer : Client
     {
+        internal static readonly Regex ConsumerGroupRegex = 
new("^[%a-zA-Z0-9_-]+$");
         protected readonly string ConsumerGroup;
 
         protected Consumer(ClientConfig clientConfig, string consumerGroup) : 
base(
diff --git a/csharp/rocketmq-client-csharp/Message.cs 
b/csharp/rocketmq-client-csharp/Message.cs
index 14d932a2..2430bbbd 100644
--- a/csharp/rocketmq-client-csharp/Message.cs
+++ b/csharp/rocketmq-client-csharp/Message.cs
@@ -17,11 +17,13 @@
 
 using System;
 using System.Collections.Generic;
+using System.Text.RegularExpressions;
 
 namespace Org.Apache.Rocketmq
 {
     public class Message
     {
+        internal static readonly Regex TopicRegex = new("^[%a-zA-Z0-9_-]+$"); 
         private Message(string topic, byte[] body, string tag, List<string> 
keys,
             Dictionary<string, string> properties, DateTime? 
deliveryTimestamp, string messageGroup)
         {
@@ -71,6 +73,7 @@ namespace Org.Apache.Rocketmq
             public Builder SetTopic(string topic)
             {
                 Preconditions.CheckArgument(null != topic, "topic should not 
be null");
+                Preconditions.CheckArgument(topic != null && 
TopicRegex.Match(topic).Success, $"topic does not match the regex 
{TopicRegex}");
                 _topic = topic;
                 return this;
             }
@@ -85,6 +88,7 @@ namespace Org.Apache.Rocketmq
             public Builder SetTag(string tag)
             {
                 Preconditions.CheckArgument(!string.IsNullOrWhiteSpace(tag), 
"tag should not be null or white space");
+                Preconditions.CheckArgument(tag != null && !tag.Contains("|"), 
"tag should not contain \"|\"");
                 _tag = tag;
                 return this;
             }
diff --git a/csharp/rocketmq-client-csharp/Producer.cs 
b/csharp/rocketmq-client-csharp/Producer.cs
index efdbdcbb..9b805a14 100644
--- a/csharp/rocketmq-client-csharp/Producer.cs
+++ b/csharp/rocketmq-client-csharp/Producer.cs
@@ -83,7 +83,7 @@ namespace Org.Apache.Rocketmq
             Shutdown().Wait();
             GC.SuppressFinalize(this);
         }
-        
+
         protected override async Task Shutdown()
         {
             try
@@ -177,6 +177,7 @@ namespace Org.Apache.Rocketmq
                         new KeyValuePair<string, 
object>(MetricConstant.ClientId, ClientId),
                         new KeyValuePair<string, 
object>(MetricConstant.InvocationStatus,
                             null == exception ? MetricConstant.True : 
MetricConstant.False));
+                    // TODO
                 }
             }
 
@@ -330,6 +331,7 @@ namespace Org.Apache.Rocketmq
 
             public Builder SetClientConfig(ClientConfig clientConfig)
             {
+                Preconditions.CheckArgument(null != clientConfig, 
"clientConfig should not be null");
                 _clientConfig = clientConfig;
                 return this;
             }
@@ -338,7 +340,10 @@ namespace Org.Apache.Rocketmq
             {
                 foreach (var topic in topics)
                 {
-                    _publishingTopics[topic] = true;
+                    Preconditions.CheckArgument(null != topic, "topic should 
not be null");
+                    Preconditions.CheckArgument(topic != null && 
Message.TopicRegex.Match(topic).Success,
+                        $"topic does not match the regex 
{Message.TopicRegex}");
+                    _publishingTopics[topic!] = true;
                 }
 
                 return this;
@@ -346,18 +351,21 @@ namespace Org.Apache.Rocketmq
 
             public Builder SetMaxAttempts(int maxAttempts)
             {
+                Preconditions.CheckArgument(maxAttempts > 0, "maxAttempts must 
be positive");
                 _maxAttempts = maxAttempts;
                 return this;
             }
 
             public Builder SetTransactionChecker(ITransactionChecker checker)
             {
+                Preconditions.CheckArgument(null != checker, "checker should 
not be null");
                 _checker = checker;
                 return this;
             }
 
             public async Task<Producer> Build()
             {
+                Preconditions.CheckArgument(null != _clientConfig, 
"clientConfig has not been set yet");
                 var producer = new Producer(_clientConfig, _publishingTopics, 
_maxAttempts, _checker);
                 await producer.Start();
                 return producer;
diff --git a/csharp/rocketmq-client-csharp/SimpleConsumer.cs 
b/csharp/rocketmq-client-csharp/SimpleConsumer.cs
index c91a0b7a..953de163 100644
--- a/csharp/rocketmq-client-csharp/SimpleConsumer.cs
+++ b/csharp/rocketmq-client-csharp/SimpleConsumer.cs
@@ -278,12 +278,16 @@ namespace Org.Apache.Rocketmq
 
             public Builder SetClientConfig(ClientConfig clientConfig)
             {
+                Preconditions.CheckArgument(null != clientConfig, 
"clientConfig should not be null");
                 _clientConfig = clientConfig;
                 return this;
             }
 
             public Builder SetConsumerGroup(string consumerGroup)
             {
+                Preconditions.CheckArgument(null != consumerGroup, 
"consumerGroup should not be null");
+                Preconditions.CheckArgument(consumerGroup != null && 
ConsumerGroupRegex.Match(consumerGroup).Success,
+                    $"topic does not match the regex {ConsumerGroupRegex}");
                 _consumerGroup = consumerGroup;
                 return this;
             }
@@ -296,13 +300,24 @@ namespace Org.Apache.Rocketmq
 
             public Builder SetSubscriptionExpression(Dictionary<string, 
FilterExpression> subscriptionExpressions)
             {
-                _subscriptionExpressions = new ConcurrentDictionary<string, 
FilterExpression>(subscriptionExpressions);
+                Preconditions.CheckArgument(null != subscriptionExpressions,
+                    "subscriptionExpressions should not be null");
+                Preconditions.CheckArgument(subscriptionExpressions!.Count != 
0,
+                    "subscriptionExpressions should not be empty");
+                _subscriptionExpressions = new ConcurrentDictionary<string, 
FilterExpression>(subscriptionExpressions!);
                 return this;
             }
 
-            public SimpleConsumer Build()
+            public async Task<SimpleConsumer> Build()
             {
-                return new SimpleConsumer(_clientConfig, _consumerGroup, 
_awaitDuration, _subscriptionExpressions);
+                Preconditions.CheckArgument(null != _clientConfig, 
"clientConfig has not been set yet");
+                Preconditions.CheckArgument(null != _consumerGroup, 
"consumerGroup has not been set yet");
+                Preconditions.CheckArgument(!_subscriptionExpressions!.IsEmpty,
+                    "subscriptionExpressions has not been set yet");
+                var simpleConsumer = new SimpleConsumer(_clientConfig, 
_consumerGroup, _awaitDuration,
+                    _subscriptionExpressions);
+                await simpleConsumer.Start();
+                return simpleConsumer;
             }
         }
     }
diff --git a/csharp/tests/MessageTest.cs b/csharp/tests/MessageTest.cs
index 59db04a6..7b28a6da 100644
--- a/csharp/tests/MessageTest.cs
+++ b/csharp/tests/MessageTest.cs
@@ -33,6 +33,14 @@ namespace Org.Apache.Rocketmq
             new Message.Builder().SetTopic(topic);
         }
 
+        [TestMethod]
+        [ExpectedException(typeof(ArgumentException))]
+        public void TestIllegalTopic1()
+        {
+            const string topic = "";
+            new Message.Builder().SetTopic(topic);
+        }
+
         [TestMethod]
         [ExpectedException(typeof(ArgumentException))]
         public void TestIllegalTag0()
@@ -61,6 +69,13 @@ namespace Org.Apache.Rocketmq
             new Message.Builder().SetTag("\t\n");
         }
 
+        [TestMethod]
+        [ExpectedException(typeof(ArgumentException))]
+        public void TestIllegalTag4()
+        {
+            new Message.Builder().SetTag("abc|cde");
+        }
+
         [TestMethod]
         [ExpectedException(typeof(ArgumentException))]
         public void TestIllegalMessageGroup0()
@@ -117,7 +132,6 @@ namespace Org.Apache.Rocketmq
             };
             Assert.AreEqual(1, message.Properties.Count);
             Assert.AreEqual(properties["a"], message.Properties["a"]);
-
         }
 
         [TestMethod]

Reply via email to