[GitHub] [rocketmq-mqtt] ferrirW commented on a change in pull request #25: [ISSUE #19] fix '#' check in topicFilter

[GitBox <gi...@apache.org>]

ferrirW commented on a change in pull request #25:
URL: https://github.com/apache/rocketmq-mqtt/pull/25#discussion_r825640255



##########
File path: mqtt-ds/src/main/java/org/apache/rocketmq/mqtt/ds/meta/WildcardManager.java
##########
@@ -99,9 +99,7 @@ private void refreshWildcards(String firstTopic) {
             queueNames.add(pubTopic);
             Set<String> wildcards = matchWildcards(pubTopic);
             if (wildcards != null && !wildcards.isEmpty()) {

Review comment:
       I prefer not to removed. It's not expensive and could avoid problems that may arise from a change of matchWildcards()  in the future.

##########
File path: mqtt-common/src/test/java/org/apache/rocketmq/mqtt/common/test/TestTopicUtils.java
##########
@@ -0,0 +1,43 @@
+package org.apache.rocketmq.mqtt.common.test;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.rocketmq.mqtt.common.model.Trie;
+import org.apache.rocketmq.mqtt.common.util.TopicUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestTopicUtils {
+
+  @Test
+  public void testTopicMatch() {
+    String topic = "t/t1/t2/";
+    String topicFilter = "t/t1/t2/";
+    Assert.assertTrue(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/#/";
+    Assert.assertTrue(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/t1/+/";
+    Assert.assertTrue(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/+/#/";
+    Assert.assertTrue(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/#/t2/";
+    Assert.assertFalse(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/#/+/";
+    Assert.assertFalse(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/t1/t2/t3/";
+    Assert.assertFalse(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/t1/#/t3/";
+    Assert.assertFalse(TopicUtils.isMatch(topic, topicFilter));
+
+    topicFilter = "t/t1#/";

Review comment:
       > Will "t/t1#/" appear?
   
   Maybe misuse. Clients probably reject this type, but it's better for the server to check it itself.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org