ztkmkoo / dss

Asynchronous distributed server system (dss)
MIT License
19 stars 13 forks source link

[#52] Add websocket handler #133

Closed Doyuni closed 4 years ago

Doyuni commented 4 years ago

I added websocket handler first. I will add service interface later.

Websocket Process

  1. Open Handshake : It occurs in DssHttpRequestHandler.java
    • Client's handshake request : HTTP Upgrade Request using only GET method
GET /chat HTTP/1.1
Host: dssserver.example.com
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Key: ScmyStFFNlK9TVHteROMKA==
Origin: <http://example.com>
Sec-WebSocket-Version: 13
HTTP/1.1 101 Switching Protocols
upgrade: websocket
connection: upgrade
sec-websocket-accept: t3gjBK9ILNyvtnyC+vneMlV98g4=
  1. Sending and Receiving Data Frame (Current: Echo message) : It occurs in DssWebSocketHandler.java

    • If client sends message to dss server, dss server receives message called Websocket Frame.
  2. Close connection


I tested it works fine using Jetty websocket client or Echo Test (https://www.websocket.org/echo.html)

Here is test logs.

[INFO ] [DssWebSocketServer.java]start(66) : Server Started
[INFO ] [DssWebSocketChannelInitializer.java]initChannel(20) : Try to initChannel
[INFO ] [DssHttpRequestHandler.java]handleHttpRequest(47) : Request URI: /websocket
[INFO ] [DssHttpRequestHandler.java]handleHttpRequest(65) : Request WebSocket Protocol
[INFO ] [DssHttpRequestHandler.java]handleHandshake(95) : Handshake Start
[INFO ] [DssHttpRequestHandler.java]handleHandshake(98) : ws://localhost:8181/websocket
[INFO ] [DssHttpRequestHandler.java]handleHandshake(106) : Handshake Success
[INFO ] [DssWebSocketHandler.java]channelRead(17) : Client Channel : [id: 0x70e76c8c, L:/127.0.0.1:8181 - R:/127.0.0.1:55702]
[INFO ] [DssWebSocketHandler.java]channelRead(23) : TextWebSocketFrame Received
[INFO ] [DssWebSocketHandler.java]channelRead(28) : Received Message : Hello! Dss Server
[INFO ] [DssWebSocketHandler.java]channelRead(17) : Client Channel : [id: 0x70e76c8c, L:/127.0.0.1:8181 - R:/127.0.0.1:55702]
[INFO ] [DssWebSocketHandler.java]channelRead(36) : CloseWebSocketFrame Received
[INFO ] [DssWebSocketHandler.java]channelRead(39) : ReasonText : 
[INFO ] [DssWebSocketHandler.java]channelRead(40) : StatusCode : 1000
[INFO ] [DssWebSocketChannelInitializer.java]initChannel(20) : Try to initChannel
[INFO ] [DssHttpRequestHandler.java]handleHttpRequest(47) : Request URI: /websocket?encoding=text
[INFO ] [DssHttpRequestHandler.java]handleHttpRequest(65) : Request WebSocket Protocol
[INFO ] [DssHttpRequestHandler.java]handleHandshake(95) : Handshake Start
[INFO ] [DssHttpRequestHandler.java]handleHandshake(98) : ws://127.0.0.1:8181/websocket
[INFO ] [DssHttpRequestHandler.java]handleHandshake(106) : Handshake Success
[INFO ] [DssWebSocketHandler.java]channelRead(17) : Client Channel : [id: 0xfed0f797, L:/127.0.0.1:8181 - R:/127.0.0.1:55731]
[INFO ] [DssWebSocketHandler.java]channelRead(23) : TextWebSocketFrame Received
[INFO ] [DssWebSocketHandler.java]channelRead(28) : Received Message : Hello

Code Review

Thank you for reading.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-1.8%) to 84.057% when pulling 8060501065619efe4eadb4afa5e06b738261ddd4 on Doyuni:feature/52_add_websocket_handler_and_its_service_interface into b228e300d81c4ddc253a98052d8bbb00b523716b on ztkmkoo:develop.

ztkmkoo commented 4 years ago

Thank you @Doyuni

ztkmkoo commented 4 years ago
  • I want to handle websocket endpoint well. ex) URI: "/websocket" or "/websocket?encoding=text" or "/websocket/example" These endpoints all mean "request websocket"?

@Doyuni Hi, I do not understand I want to handle endpoint well

ztkmkoo commented 4 years ago
  • I want to add WebSocketServerProtocolHandler but It can not handle http message in Echo Test (https://www.websocket.org/echo.html). Because this handler replaces HttpServerCodec into WebSocketFrameCodec. If I use this handler, I don't need to write handshake method.

@Doyuni WebSocketServerProtocolHandler check if handshake conditions:

String websocketPath = serverConfig.websocketPath();
// serverConfig.checkStartsWith() ==> default value is false
//  ==> your echo test page send url like websocket?encoding=text  ==> skip handshake ==> return 404
return serverConfig.checkStartsWith() ? !req.uri().startsWith(websocketPath) : !req.uri().equals(websocketPath);

If want using WebSocketServerProtocolHandler, following the codes in DssWebSocketChannelInitializer

@Override
    protected void initChannel(SocketChannel ch) throws Exception {
        logger.info("Try to initChannel");

        final ChannelPipeline p = ch.pipeline();

        p.addLast(new HttpServerCodec());
        p.addLast(new HttpObjectAggregator(65536));
        p.addLast(new WebSocketServerProtocolHandler(WEBSOCKET_PATH, true)); // set serverConfig.checkStartsWith() as true
        p.addLast(new DssHttpRequestHandler(WEBSOCKET_PATH));
        p.addLast(new DssWebSocketHandler());
    }

At last modify DssHttpRequestHandler channelRead0, you can pass the tests.

ztkmkoo commented 4 years ago

Hi @Doyuni , After pass the coveralls test coverage, i will merge this pr. Thx.

Doyuni commented 4 years ago
  • I want to handle websocket endpoint well. ex) URI: "/websocket" or "/websocket?encoding=text" or "/websocket/example" These endpoints all mean "request websocket"?

@Doyuni Hi, I do not understand I want to handle endpoint well

It means how do I check websocket uri well.

I use this way.

int baseIndex = uri.indexOf("?");
boolean checkEndPoint = baseIndex == -1 ? WEBSOCKET_PATH.equals(uri) : WEBSOCKET_PATH.equals(uri.substring(0, baseIndex));

So, It recognizes follow uri : /websocket or /websocket?encoding=text or /websocket?userId=doyuni not /websocketabc or /websocket/hello.

Doyuni commented 4 years ago
  • I want to add WebSocketServerProtocolHandler but It can not handle http message in Echo Test (https://www.websocket.org/echo.html). Because this handler replaces HttpServerCodec into WebSocketFrameCodec. If I use this handler, I don't need to write handshake method.

@Doyuni WebSocketServerProtocolHandler check if handshake conditions:

String websocketPath = serverConfig.websocketPath();
// serverConfig.checkStartsWith() ==> default value is false
//  ==> your echo test page send url like websocket?encoding=text  ==> skip handshake ==> return 404
return serverConfig.checkStartsWith() ? !req.uri().startsWith(websocketPath) : !req.uri().equals(websocketPath);

If want using WebSocketServerProtocolHandler, following the codes in DssWebSocketChannelInitializer

@Override
    protected void initChannel(SocketChannel ch) throws Exception {
        logger.info("Try to initChannel");

        final ChannelPipeline p = ch.pipeline();

        p.addLast(new HttpServerCodec());
        p.addLast(new HttpObjectAggregator(65536));
        p.addLast(new WebSocketServerProtocolHandler(WEBSOCKET_PATH, true)); // set serverConfig.checkStartsWith() as true
        p.addLast(new DssHttpRequestHandler(WEBSOCKET_PATH));
        p.addLast(new DssWebSocketHandler());
    }

At last modify DssHttpRequestHandler channelRead0, you can pass the tests.

Thank you for reviewing.

I'm wondering which one to choose. (use or not that handler) If use this handler, It supports handshake and benefits performance a little bit. But It recognizes follow uri too : /websocket/ or /websocketabcd or/websocket?encoding=text. Because It checks only startWith.

Doyuni commented 4 years ago

Hi @Doyuni , After pass the coveralls test coverage, i will merge this pr. Thx.

Okay, I'm writing test code.