Fix multiple slash command name issue in sentinel-transport-netty-http module (#817)

pom.xml

@@ -125,6 +125,11 @@
                 <artifactId>sentinel-transport-simple-http</artifactId>
                 <version>${project.version}</version>
             </dependency>
+            <dependency>
+                <groupId>com.alibaba.csp</groupId>
+                <artifactId>sentinel-transport-netty-http</artifactId>
+                <version>${project.version}</version>
+            </dependency>
             <dependency>
                 <groupId>com.alibaba.csp</groupId>
                 <artifactId>sentinel-transport-common</artifactId>

sentinel-transport/sentinel-transport-netty-http/src/main/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandler.java

@@ -221,11 +221,15 @@ public class HttpServerHandler extends SimpleChannelInboundHandler<Object> {
         if (StringUtil.isEmpty(uri)) {
             return "";
         }
-        String[] arr = uri.split("/");
-        if (arr.length < 2) {
-            return "";
+
+        // Remove the / of the uri as the target(command name)
+        // Usually the uri is start with /
+        int start = uri.indexOf('/');
+        if (start != -1) {
+            return uri.substring(start + 1);
         }
-        return arr[1];
+
+        return uri;
     }
 
     private CommandHandler getHandler(String commandName) {

sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/handler/MultipleSlashNameCommandTestHandler.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 1999-2018 Alibaba Group Holding Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.alibaba.csp.sentinel.transport.command.handler;
+
+import com.alibaba.csp.sentinel.command.CommandHandler;
+import com.alibaba.csp.sentinel.command.CommandRequest;
+import com.alibaba.csp.sentinel.command.CommandResponse;
+import com.alibaba.csp.sentinel.command.annotation.CommandMapping;
+
+/**
+ * @author cdfive
+ */
+@CommandMapping(name = "aa/bb/cc", desc = "a test handler with multiple / in its name")
+public class MultipleSlashNameCommandTestHandler implements CommandHandler<String> {
+    @Override
+    public CommandResponse<String> handle(CommandRequest request) {
+        return CommandResponse.ofSuccess("MultipleSlashNameCommandTestHandler result");
+    }
+}

sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerHandlerTest.java

@@ -23,6 +23,8 @@ import com.alibaba.csp.sentinel.slots.block.flow.FlowRule;
 import com.alibaba.csp.sentinel.slots.block.flow.FlowRuleManager;
 import com.alibaba.csp.sentinel.transport.CommandCenter;
 import com.alibaba.csp.sentinel.transport.command.NettyHttpCommandCenter;
+import com.alibaba.csp.sentinel.transport.command.handler.MultipleSlashNameCommandTestHandler;
+import com.alibaba.fastjson.JSON;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.embedded.EmbeddedChannel;
@@ -34,6 +36,7 @@ import org.junit.Test;
 import java.lang.reflect.Field;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -45,7 +48,6 @@ import static org.junit.Assert.assertEquals;
  * Test cases for {@link HttpServerHandler}.
  *
  * @author cdfive
- * @date 2018-12-17
  */
 public class HttpServerHandlerTest {
 
@@ -59,7 +61,7 @@ public class HttpServerHandlerTest {
 
     @BeforeClass
     public static void beforeClass() throws Exception {
-        // don't execute InitExecutor.doInit, to avoid CommandCenter SPI loaded
+        // Don't execute InitExecutor.doInit, to avoid CommandCenter SPI loaded
         Field[] declaredFields = InitExecutor.class.getDeclaredFields();
         for (Field declaredField : declaredFields) {
             if (declaredField.getName().equals("initialized")) {
@@ -68,23 +70,26 @@ public class HttpServerHandlerTest {
             }
         }
 
-        // create NettyHttpCommandCenter to create HttpServer
+        // Create NettyHttpCommandCenter to create HttpServer
         CommandCenter commandCenter = new NettyHttpCommandCenter();
-        // call beforeStart to register handlers
+        // Call beforeStart to register handlers
         commandCenter.beforeStart();
     }
 
     @Before
     public void before() {
-        // the same Handlers in order as the ChannelPipeline in HttpServerInitializer
+        // The same Handlers in order as the ChannelPipeline in HttpServerInitializer
         HttpRequestDecoder httpRequestDecoder = new HttpRequestDecoder();
         HttpObjectAggregator httpObjectAggregator = new HttpObjectAggregator(1024 * 1024);
         HttpResponseEncoder httpResponseEncoder = new HttpResponseEncoder();
 
         HttpServerHandler httpServerHandler = new HttpServerHandler();
 
-        // create new EmbeddedChannel every method call
+        // Create new EmbeddedChannel every method call
         embeddedChannel = new EmbeddedChannel(httpRequestDecoder, httpObjectAggregator, httpResponseEncoder, httpServerHandler);
+
+        // Clear flow rules
+        FlowRuleManager.loadRules(Collections.EMPTY_LIST);
     }
 
     @Test
@@ -92,9 +97,9 @@ public class HttpServerHandlerTest {
         String httpRequestStr = "GET / HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = "Invalid command";
+        String expectedBody = "Invalid command";
 
-        processError(httpRequestStr, body);
+        processError(httpRequestStr, expectedBody);
     }
 
     @Test
@@ -102,44 +107,49 @@ public class HttpServerHandlerTest {
         String httpRequestStr = "GET /aaa HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = String.format("Unknown command \"%s\"", "aaa");
+        String expectedBody = String.format("Unknown command \"%s\"", "aaa");
 
-        processError(httpRequestStr, body);
+        processError(httpRequestStr, expectedBody);
     }
 
+    /**
+     * {@link com.alibaba.csp.sentinel.command.handler.VersionCommandHandler}
+     */
     @Test
     public void testVersionCommand() {
         String httpRequestStr = "GET /version HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = Constants.SENTINEL_VERSION;
+        String expectedBody = Constants.SENTINEL_VERSION;
 
-        processSuccess(httpRequestStr, body);
+        processSuccess(httpRequestStr, expectedBody);
     }
 
+    /**
+     * {@link com.alibaba.csp.sentinel.command.handler.FetchActiveRuleCommandHandler}
+     */
     @Test
-    public void testGetRuleCommandInvalidType() {
+    public void testFetchActiveRuleCommandInvalidType() {
         String httpRequestStr = "GET /getRules HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = "invalid type";
+        String expectedBody = "invalid type";
 
-        processFailed(httpRequestStr, body);
+        processFailed(httpRequestStr, expectedBody);
     }
 
     @Test
-    public void testGetRuleCommandFlowEmptyRule() {
+    public void testFetchActiveRuleCommandEmptyRule() {
         String httpRequestStr = "GET /getRules?type=flow HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = "[]";
+        String expectedBody = "[]";
 
-        processSuccess(httpRequestStr, body);
+        processSuccess(httpRequestStr, expectedBody);
     }
 
-//    FIXME byteBuf.toString can't get body response now, need to find another way
-//    @Test
-    public void testGetRuleCommandFlowSomeRules() {
+    @Test
+    public void testFetchActiveRuleCommandSomeFlowRules() {
         List<FlowRule> rules = new ArrayList<FlowRule>();
         FlowRule rule1 = new FlowRule();
         rule1.setResource("key");
@@ -152,60 +162,86 @@ public class HttpServerHandlerTest {
         String httpRequestStr = "GET /getRules?type=flow HTTP/1.1" + CRLF
                               + "Host: localhost:8719" + CRLF
                               + CRLF;
-        String body = "";
 
-        processSuccess(httpRequestStr, body);
+        // body json
+        /*
+        String expectedBody = "[{\"clusterMode\":false,\"controlBehavior\":0,\"count\":20.0"
+                + ",\"grade\":1,\"limitApp\":\"default\",\"maxQueueingTimeMs\":500"
+                + ",\"resource\":\"key\",\"strategy\":0,\"warmUpPeriodSec\":10}]";
+        */
+        String expectedBody = JSON.toJSONString(rules);
+
+        processSuccess(httpRequestStr, expectedBody);
     }
 
-    private void processError(String httpRequestStr, String body) {
-        processError(httpRequestStr, BAD_REQUEST, body);
+    /**
+     * {@link MultipleSlashNameCommandTestHandler}
+     *
+     * Test command whose mapping path and command name contain multiple /
+     */
+    @Test
+    public void testMultipleSlashNameCommand() {
+        String httpRequestStr = "GET /aa/bb/cc HTTP/1.1" + CRLF
+                + "Host: localhost:8719" + CRLF
+                + CRLF;
+        String expectedBody = "MultipleSlashNameCommandTestHandler result";
+
+        processSuccess(httpRequestStr, expectedBody);
     }
 
-    private void processError(String httpRequestStr, HttpResponseStatus status, String body) {
+    private void processError(String httpRequestStr, String expectedBody) {
+        processError(httpRequestStr, BAD_REQUEST, expectedBody);
+    }
+
+    private void processError(String httpRequestStr, HttpResponseStatus status, String expectedBody) {
         String httpResponseStr = processResponse(httpRequestStr);
-        assertErrorStatusAndBody(status, body, httpResponseStr);
+        assertErrorStatusAndBody(status, expectedBody, httpResponseStr);
     }
 
-    private void processSuccess(String httpRequestStr, String body) {
-        process(httpRequestStr, OK, body);
+    private void processSuccess(String httpRequestStr, String expectedBody) {
+        process(httpRequestStr, OK, expectedBody);
     }
 
-    private void processFailed(String httpRequestStr, String body) {
-        process(httpRequestStr, BAD_REQUEST, body);
+    private void processFailed(String httpRequestStr, String expectedBody) {
+        process(httpRequestStr, BAD_REQUEST, expectedBody);
     }
 
-    private void process(String httpRequestStr, HttpResponseStatus status, String body) {
+    private void process(String httpRequestStr, HttpResponseStatus status, String expectedBody) {
         String responseStr = processResponse(httpRequestStr);
-        assertStatusAndBody(status, body, responseStr);
+        assertStatusAndBody(status, expectedBody, responseStr);
     }
 
     private String processResponse(String httpRequestStr) {
         embeddedChannel.writeInbound(Unpooled.wrappedBuffer(httpRequestStr.getBytes(SENTINEL_CHARSET)));
 
-        ByteBuf byteBuf = embeddedChannel.readOutbound();
+        StringBuilder sb = new StringBuilder();
+
+        ByteBuf byteBuf;
+        while ((byteBuf = embeddedChannel.readOutbound()) != null) {
+            sb.append(byteBuf.toString(SENTINEL_CHARSET));
+        }
 
-        String responseStr = byteBuf.toString(SENTINEL_CHARSET);
-        return responseStr;
+        return sb.toString();
     }
 
-    private void assertErrorStatusAndBody(HttpResponseStatus status, String body, String httpResponseStr) {
+    private void assertErrorStatusAndBody(HttpResponseStatus status, String expectedBody, String httpResponseStr) {
         StringBuilder text = new StringBuilder();
         text.append(HttpVersion.HTTP_1_1.toString()).append(' ').append(status.toString()).append(CRLF);
         text.append("Content-Type: text/plain; charset=").append(SENTINEL_CHARSET_NAME).append(CRLF);
         text.append(CRLF);
-        text.append(body);
+        text.append(expectedBody);
 
         assertEquals(text.toString(), httpResponseStr);
     }
 
-    private void assertStatusAndBody(HttpResponseStatus status, String body, String httpResponseStr) {
+    private void assertStatusAndBody(HttpResponseStatus status, String expectedBody, String httpResponseStr) {
         StringBuilder text = new StringBuilder();
         text.append(HttpVersion.HTTP_1_1.toString()).append(' ').append(status.toString()).append(CRLF);
         text.append("Content-Type: text/plain; charset=").append(SENTINEL_CHARSET_NAME).append(CRLF);
-        text.append("content-length: " + body.length()).append(CRLF);
+        text.append("content-length: " + expectedBody.length()).append(CRLF);
         text.append("connection: close").append(CRLF);
         text.append(CRLF);
-        text.append(body);
+        text.append(expectedBody);
 
         assertEquals(text.toString(), httpResponseStr);
     }

sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerInitializerTest.java

@@ -30,30 +30,29 @@ import static org.mockito.Mockito.*;
  * Test cases for {@link HttpServerInitializer}.
  *
  * @author cdfive
- * @date 2018-12-19
  */
 public class HttpServerInitializerTest {
 
     @Test
     public void testInitChannel() throws Exception {
-        // mock Objects
+        // Mock Objects
         HttpServerInitializer httpServerInitializer = mock(HttpServerInitializer.class);
         SocketChannel socketChannel = mock(SocketChannel.class);
         ChannelPipeline channelPipeline = mock(ChannelPipeline.class);
 
-        // mock SocketChannel#pipeline() method
+        // Mock SocketChannel#pipeline() method
         when(socketChannel.pipeline()).thenReturn(channelPipeline);
 
         // HttpServerInitializer#initChannel(SocketChannel) call real method
         doCallRealMethod().when(httpServerInitializer).initChannel(socketChannel);
 
-        // start test for HttpServerInitializer#initChannel(SocketChannel)
+        // Start test for HttpServerInitializer#initChannel(SocketChannel)
         httpServerInitializer.initChannel(socketChannel);
 
-        // verify 4 times calling ChannelPipeline#addLast() method
+        // Verify 4 times calling ChannelPipeline#addLast() method
         verify(channelPipeline, times(4)).addLast(any(ChannelHandler.class));
 
-        // verify the order of calling ChannelPipeline#addLast() method
+        // Verify the order of calling ChannelPipeline#addLast() method
         InOrder inOrder = inOrder(channelPipeline);
         inOrder.verify(channelPipeline).addLast(any(HttpRequestDecoder.class));
         inOrder.verify(channelPipeline).addLast(any(HttpObjectAggregator.class));

sentinel-transport/sentinel-transport-netty-http/src/test/java/com/alibaba/csp/sentinel/transport/command/netty/HttpServerTest.java

@@ -31,7 +31,6 @@ import static org.junit.Assert.*;
  * Test cases for {@link HttpServer}.
  *
  * @author cdfive
- * @date 2018-12-19
  */
 public class HttpServerTest {
 
@@ -39,20 +38,20 @@ public class HttpServerTest {
 
     @BeforeClass
     public static void beforeClass() {
-        // note: clear handlerMap first, as other test case may init HttpServer.handlerMap
+        // Note: clear handlerMap first, as other test case may init HttpServer.handlerMap
         // if not, run mvn test, the next assertEquals(0, HttpServer.handlerMap.size()) may fail
         HttpServer.handlerMap.clear();
 
-        // create new HttpServer
+        // Create new HttpServer
         httpServer = new HttpServer();
 
-        // no handler in handlerMap at first
+        // No handler in handlerMap at first
         assertEquals(0, HttpServer.handlerMap.size());
     }
 
     @Before
     public void before() {
-        // clear handlerMap every method call
+        // Clear handlerMap every method call
         HttpServer.handlerMap.clear();
     }
 
@@ -61,37 +60,37 @@ public class HttpServerTest {
         String commandName;
         CommandHandler handler;
 
-        // if commandName is null, no handler added in handlerMap
+        // If commandName is null, no handler added in handlerMap
         commandName = null;
         handler = new VersionCommandHandler();
         httpServer.registerCommand(commandName, handler);
         assertEquals(0, HttpServer.handlerMap.size());
 
-        // if commandName is "", no handler added in handlerMap
+        // If commandName is "", no handler added in handlerMap
         commandName = "";
         handler = new VersionCommandHandler();
         httpServer.registerCommand(commandName, handler);
         assertEquals(0, HttpServer.handlerMap.size());
 
-        // if handler is null, no handler added in handlerMap
+        // If handler is null, no handler added in handlerMap
         commandName = "version";
         handler = null;
         httpServer.registerCommand(commandName, handler);
         assertEquals(0, HttpServer.handlerMap.size());
 
-        // add one handler, commandName:version, handler:VersionCommandHandler
+        // Add one handler, commandName:version, handler:VersionCommandHandler
         commandName = "version";
         handler = new VersionCommandHandler();
         httpServer.registerCommand(commandName, handler);
         assertEquals(1, HttpServer.handlerMap.size());
 
-        // add the same name Handler, no handler added in handlerMap
+        // Add the same name Handler, no handler added in handlerMap
         commandName = "version";
         handler = new VersionCommandHandler();
         httpServer.registerCommand(commandName, handler);
         assertEquals(1, HttpServer.handlerMap.size());
 
-        // add another handler, commandName:basicInfo, handler:BasicInfoCommandHandler
+        // Add another handler, commandName:basicInfo, handler:BasicInfoCommandHandler
         commandName = "basicInfo";
         handler = new BasicInfoCommandHandler();
         httpServer.registerCommand(commandName, handler);
@@ -102,16 +101,16 @@ public class HttpServerTest {
     public void testRegisterCommands() {
         Map<String, CommandHandler> handlerMap = null;
 
-        // if handlerMap is null, no handler added in handlerMap
+        // If handlerMap is null, no handler added in handlerMap
         httpServer.registerCommands(handlerMap);
         assertEquals(0, HttpServer.handlerMap.size());
 
-        // add handler from CommandHandlerProvider
+        // Add handler from CommandHandlerProvider
         handlerMap = CommandHandlerProvider.getInstance().namedHandlers();
         httpServer.registerCommands(handlerMap);
-        // check same size
+        // Check same size
         assertEquals(handlerMap.size(), HttpServer.handlerMap.size());
-        // check not same reference
+        // Check not same reference
         assertTrue(handlerMap != HttpServer.handlerMap);
     }
 }

sentinel-transport/sentinel-transport-netty-http/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.command.CommandHandler

@@ -0,0 +1 @@
+com.alibaba.csp.sentinel.transport.command.handler.MultipleSlashNameCommandTestHandler

sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/heartbeat/SimpleHttpHeartbeatSender.java

@@ -100,7 +100,7 @@ public class SimpleHttpHeartbeatSender implements HeartbeatSender {
         try {
             String ipsStr = TransportConfig.getConsoleServer();
             if (StringUtil.isEmpty(ipsStr)) {
-                RecordLog.warn("[NettyHttpHeartbeatSender] Dashboard server address not configured");
+                RecordLog.warn("[SimpleHttpHeartbeatSender] Dashboard server address not configured");
                 return newAddrs;
             }