[GitHub] [rocketmq] coder-zzzz commented on a change in pull request #1985: [ISSUE #1976]System topic should add permission checking globally

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

coder-zzzz commented on a change in pull request #1985:
URL: https://github.com/apache/rocketmq/pull/1985#discussion_r430138716



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -285,7 +273,19 @@ private synchronized RemotingCommand updateAndCreateTopic(ChannelHandlerContext
 
         this.brokerController.registerIncrementBrokerData(topicConfig, this.brokerController.getTopicConfigManager().getDataVersion());
 
-        return null;
+        response.setCode(ResponseCode.SUCCESS);
+        return response;
+    }
+
+    private boolean isSystemTopic(RemotingCommand response, String topic) {
+        if (this.brokerController.getTopicConfigManager().isSystemTopic(topic)) {
+            String errorMsg = "The topic[" + topic + "] is conflict with system reserved words.";

Review comment:
       ok

##########
File path: broker/src/main/java/org/apache/rocketmq/broker/topic/TopicConfigManager.java
##########
@@ -124,6 +124,14 @@ public TopicConfigManager(BrokerController brokerController) {
             topicConfig.setWriteQueueNums(1);
             this.topicConfigTable.put(topicConfig.getTopicName(), topicConfig);
         }
+        {
+            String topic = MixAll.SCHEDULE_TOPIC;
+            TopicConfig topicConfig = new TopicConfig(topic);

Review comment:
       Yeah, it is a special system topic used in delay msg. When producer send a delay  msg, the msg will be send to SCHEDULE_TOPIC, then send to the real topic when the delay time comes.
   Do u meaning  add prefix to system topic , or remove system topics to a independent class?

##########
File path: broker/src/main/java/org/apache/rocketmq/broker/topic/TopicConfigManager.java
##########
@@ -124,6 +124,14 @@ public TopicConfigManager(BrokerController brokerController) {
             topicConfig.setWriteQueueNums(1);
             this.topicConfigTable.put(topicConfig.getTopicName(), topicConfig);
         }
+        {
+            String topic = MixAll.SCHEDULE_TOPIC;
+            TopicConfig topicConfig = new TopicConfig(topic);
+            this.systemTopicList.add(topic);
+            topicConfig.setReadQueueNums(18);
+            topicConfig.setWriteQueueNums(18);

Review comment:
       I will do it.

##########
File path: common/src/main/java/org/apache/rocketmq/common/MixAll.java
##########
@@ -56,6 +56,7 @@
     //http://jmenv.tbsite.net:8080/rocketmq/nsaddr
     //public static final String WS_ADDR = "http://" + WS_DOMAIN_NAME + ":8080/rocketmq/" + WS_DOMAIN_SUBGROUP;
     public static final String AUTO_CREATE_TOPIC_KEY_TOPIC = "TBW102"; // Will be created at broker when isAutoCreateTopicEnable
+    public static final String SCHEDULE_TOPIC = "SCHEDULE_TOPIC_XXXX";

Review comment:
       It's a good way to make code cleaner.I'will try my best to do it




----------------------------------------------------------------
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.

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