vernedeng commented on code in PR #9012:
URL: https://github.com/apache/inlong/pull/9012#discussion_r1353870480


##########
inlong-tubemq/tubemq-manager/src/main/java/org/apache/inlong/tubemq/manager/controller/topic/TopicWebController.java:
##########
@@ -61,47 +68,188 @@ public class TopicWebController {
     private TopicService topicService;
 
     /**
-     * broker method proxy
-     * divides the operation on broker to different method
+     * Broker method proxy.
+     * Divides the operation on the broker into different methods.
      */
     @RequestMapping(value = "")
     public @ResponseBody TubeMQResult topicMethodProxy(@RequestParam String 
method, @RequestBody String req)
             throws Exception {
+        // Log audit: Record the received method and req parameters
+        LOGGER.info("Received method for topicMethodProxy: {}", method);
+        LOGGER.info("Received req for topicMethodProxy: {}", req);
+
+        // Validate the 'method' parameter
+        if (!isValidMethod(method)) {
+            return handleInvalidMethod(method);
+        }
+
+        // Validate the 'req' parameter
+        if (!isValidJson(req)) {
+            return handleInvalidJson(req);
+        }
+
+        // Perform processing based on the 'method' parameter
         switch (method) {
             case TubeConst.ADD:
-                return masterService.baseRequestMaster(gson.fromJson(req, 
BatchAddTopicReq.class));
+                return handleAddTopicRequest(req);
             case TubeConst.CLONE:
-                return nodeService.cloneTopicToBrokers(gson.fromJson(req, 
CloneTopicReq.class));
+                return handleCloneTopicRequest(req);
             case TubeConst.AUTH_CONTROL:
-                return setAuthControl(gson.fromJson(req, 
SetAuthControlReq.class));
+                return handleAuthControlRequest(req);
             case TubeConst.MODIFY:
-                return masterService.baseRequestMaster(gson.fromJson(req, 
ModifyTopicReq.class));
+                return handleModifyTopicRequest(req);
             case TubeConst.DELETE:
             case TubeConst.REMOVE:
-                return masterService.baseRequestMaster(gson.fromJson(req, 
DeleteTopicReq.class));
+                return handleDeleteTopicRequest(req);
             case TubeConst.QUERY_CAN_WRITE:
-                return queryCanWrite(gson.fromJson(req, 
QueryCanWriteReq.class));
+                return handleQueryCanWriteRequest(req);
             case TubeConst.PUBLISH:
-                return masterService.baseRequestMaster(gson.fromJson(req, 
SetPublishReq.class));
+                return handlePublishRequest(req);
             case TubeConst.SUBSCRIBE:
-                return masterService.baseRequestMaster(gson.fromJson(req, 
SetSubscribeReq.class));
+                return handleSubscribeRequest(req);
             default:
-                return 
TubeMQResult.errorResult(TubeMQErrorConst.NO_SUCH_METHOD);
+                return handleInvalidMethod(method);
         }
     }
 
-    private TubeMQResult setAuthControl(SetAuthControlReq req) {
-        req.setMethod(TubeConst.SET_AUTH_CONTROL);
-        req.setType(TubeConst.OP_MODIFY);
-        req.setCreateUser(TubeConst.TUBEADMIN);
-        return masterService.baseRequestMaster(req);
+    /**
+     * Handles invalid 'method' parameter.
+     *
+     * @param method The invalid method value
+     * @return TubeMQResult indicating an error
+     */
+    private TubeMQResult handleInvalidMethod(String method) {
+        LOGGER.warn("Invalid method value received: {}", method);
+        return TubeMQResult.errorResult("Invalid method value.");
+    }
+
+    /**
+     * Handles invalid JSON format in 'req' parameter.
+     *
+     * @param req The invalid JSON format
+     * @return TubeMQResult indicating an error
+     */
+    private TubeMQResult handleInvalidJson(String req) {
+        LOGGER.warn("Invalid JSON format received: {}", req);
+        return TubeMQResult.errorResult("Invalid JSON format.");
+    }
+
+    /**
+     * Handles 'ADD' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleAddTopicRequest(String req) {
+        return masterService.baseRequestMaster(gson.fromJson(req, 
BatchAddTopicReq.class));
+    }
+
+    /**
+     * Handles 'CLONE' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleCloneTopicRequest(String req) throws Exception {
+        return nodeService.cloneTopicToBrokers(gson.fromJson(req, 
CloneTopicReq.class));
     }
 
-    private TubeMQResult queryCanWrite(QueryCanWriteReq req) {
-        if (!req.legal()) {
+    /**
+     * Handles 'AUTH_CONTROL' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleAuthControlRequest(String req) {
+        SetAuthControlReq setAuthControlReq = gson.fromJson(req, 
SetAuthControlReq.class);
+        setAuthControlReq.setMethod(TubeConst.SET_AUTH_CONTROL);
+        setAuthControlReq.setType(TubeConst.OP_MODIFY);
+        setAuthControlReq.setCreateUser(TubeConst.TUBEADMIN);
+        return masterService.baseRequestMaster(setAuthControlReq);
+    }
+
+    /**
+     * Handles 'MODIFY' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleModifyTopicRequest(String req) {
+        return masterService.baseRequestMaster(gson.fromJson(req, 
ModifyTopicReq.class));
+    }
+
+    /**
+     * Handles 'DELETE' and 'REMOVE' method requests.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleDeleteTopicRequest(String req) {
+        return masterService.baseRequestMaster(gson.fromJson(req, 
DeleteTopicReq.class));
+    }
+
+    /**
+     * Handles 'QUERY_CAN_WRITE' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleQueryCanWriteRequest(String req) {
+        QueryCanWriteReq queryCanWriteReq = gson.fromJson(req, 
QueryCanWriteReq.class);
+        if (!queryCanWriteReq.legal()) {
             return TubeMQResult.errorResult(TubeMQErrorConst.PARAM_ILLEGAL);
         }
-        return topicService.queryCanWrite(req.getTopicName(), 
req.getClusterId());
+        return topicService.queryCanWrite(queryCanWriteReq.getTopicName(), 
queryCanWriteReq.getClusterId());
+    }
+
+    /**
+     * Handles 'PUBLISH' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handlePublishRequest(String req) {
+        return masterService.baseRequestMaster(gson.fromJson(req, 
SetPublishReq.class));
+    }
+
+    /**
+     * Handles 'SUBSCRIBE' method request.
+     *
+     * @param req The JSON request
+     * @return TubeMQResult based on the request
+     */
+    private TubeMQResult handleSubscribeRequest(String req) {
+        return masterService.baseRequestMaster(gson.fromJson(req, 
SetSubscribeReq.class));
+    }
+
+    /**
+     * Checks if the given method is valid by comparing it against a list of 
allowed methods.
+     *
+     * @param method The method to validate.
+     * @return {@code true} if the method is valid, {@code false} otherwise.
+     */
+    private static boolean isValidMethod(String method) {
+        // Define a list of allowed methods
+        List<String> allowedMethods = Arrays.asList(
+                TubeConst.ADD, TubeConst.CLONE, TubeConst.AUTH_CONTROL, 
TubeConst.MODIFY,
+                TubeConst.DELETE, TubeConst.REMOVE, TubeConst.QUERY_CAN_WRITE, 
TubeConst.PUBLISH, TubeConst.SUBSCRIBE);
+        return allowedMethods.contains(method);
+    }
+
+    /**
+     * Validates whether the given JSON string has a valid format by 
attempting to parse it.
+     *
+     * @param json The JSON string to validate.
+     * @return {@code true} if the JSON format is valid, {@code false} 
otherwise.
+     */
+    private static boolean isValidJson(String json) {
+        // Use a JSON library or parser to validate the JSON format
+        try {
+            gson.fromJson(json, Object.class);
+            return true;
+        } catch (JsonSyntaxException e) {
+            return false;

Review Comment:
   Better to log these exceptions in debug, warn, or error level, depending on 
their importance.



-- 
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: commits-unsubscr...@inlong.apache.org

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

Reply via email to