From 293cf33702c8cad471989c1e08ce05323baadaf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Tue, 19 Jul 2011 12:12:25 +0200 Subject: Separate message and packet handling for websockets Improves the readability of websocket handler code by having two functions: websocket_handle/3 handles the packets received from the socket, removing the tuple construct that was otherwise needed, so only websocket_handle(Data, Req, State) is needed now; websocket_info/3 handles the messages that the websocket handler process received, as websocket_info(Info, Req, State). Both functions return values are handled identically by Cowboy so nothing changes on that end. --- README.md | 12 ++++++++---- src/cowboy_http_websocket.erl | 28 ++++++++++++++++------------ src/cowboy_http_websocket_handler.erl | 35 +++++++++++++++++++++-------------- test/websocket_handler.erl | 12 ++++++++---- 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index f9fb6ec..5933481 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,8 @@ Websocket would look like this: -behaviour(cowboy_http_handler). -behaviour(cowboy_http_websocket_handler). -export([init/3, handle/2, terminate/2]). --export([websocket_init/3, websocket_handle/3, websocket_terminate/3]). +-export([websocket_init/3, websocket_handle/3, + websocket_info/3, websocket_terminate/3]). init({tcp, http}, Req, Opts) -> {upgrade, protocol, cowboy_http_websocket}. @@ -194,11 +195,14 @@ websocket_init(TransportName, Req, _Opts) -> erlang:start_timer(1000, self(), <<"Hello!">>), {ok, Req, undefined_state}. -websocket_handle({timeout, _Ref, Msg}, Req, State) -> +websocket_handle(Msg, Req, State) -> + {reply, << "That's what she said! ", Msg/binary >>, Req, State}. + +websocket_info({timeout, _Ref, Msg}, Req, State) -> erlang:start_timer(1000, self(), <<"How' you doin'?">>), {reply, Msg, Req, State}; -websocket_handle({websocket, Msg}, Req, State) -> - {reply, << "That's what she said! ", Msg/binary >>, Req, State}. +websocket_info(_Info, Req, State) -> + {ok, Req, State}. websocket_terminate(_Reason, _Req, _State) -> ok. diff --git a/src/cowboy_http_websocket.erl b/src/cowboy_http_websocket.erl index fa3e50f..66187f0 100644 --- a/src/cowboy_http_websocket.erl +++ b/src/cowboy_http_websocket.erl @@ -157,7 +157,7 @@ handler_loop(State=#state{messages={OK, Closed, Error}, timeout=Timeout}, handler_terminate(State, Req, HandlerState, {error, Reason}); Message -> handler_call(State, Req, HandlerState, - SoFar, Message, fun handler_before_loop/4) + SoFar, websocket_info, Message, fun handler_before_loop/4) after Timeout -> websocket_close(State, Req, HandlerState, {normal, timeout}) end. @@ -178,7 +178,7 @@ websocket_frame(State=#state{eop=EOP}, Req, HandlerState, Data, 0) -> Pos2 = Pos - 1, << 0, Frame:Pos2/binary, 255, Rest/bits >> = Data, handler_call(State, Req, HandlerState, - Rest, {websocket, Frame}, fun websocket_data/4); + Rest, websocket_handle, Frame, fun websocket_data/4); nomatch -> %% @todo We probably should allow limiting frame length. handler_before_loop(State, Req, HandlerState, Data) @@ -186,10 +186,11 @@ websocket_frame(State=#state{eop=EOP}, Req, HandlerState, Data, 0) -> websocket_frame(State, Req, HandlerState, _Data, _FrameType) -> websocket_close(State, Req, HandlerState, {error, badframe}). --spec handler_call(#state{}, #http_req{}, any(), binary(), any(), fun()) -> ok. +-spec handler_call(#state{}, #http_req{}, any(), binary(), + atom(), any(), fun()) -> ok. handler_call(State=#state{handler=Handler, opts=Opts}, Req, HandlerState, - RemainingData, Message, NextState) -> - try Handler:websocket_handle(Message, Req, HandlerState) of + RemainingData, Callback, Message, NextState) -> + try Handler:Callback(Message, Req, HandlerState) of {ok, Req2, HandlerState2} -> NextState(State, Req2, HandlerState2, RemainingData); {ok, Req2, HandlerState2, hibernate} -> @@ -242,16 +243,19 @@ handler_terminate(#state{handler=Handler, opts=Opts}, HandlerState, Req, erlang:get_stacktrace()]) end. -%% eunit +%% Tests. -ifdef(TEST). websocket_location_test() -> - ?assertEqual(<<"ws://localhost/path">>, websocket_location(other, <<"localhost">>, 80, <<"/path">>)), - ?assertEqual(<<"ws://localhost:8080/path">>, websocket_location(other, <<"localhost">>, 8080, <<"/path">>)), - ?assertEqual(<<"wss://localhost/path">>, websocket_location(ssl, <<"localhost">>, 443, <<"/path">>)), - ?assertEqual(<<"wss://localhost:8443/path">>, websocket_location(ssl, <<"localhost">>, 8443, <<"/path">>)), - ok. + ?assertEqual(<<"ws://localhost/path">>, + websocket_location(other, <<"localhost">>, 80, <<"/path">>)), + ?assertEqual(<<"ws://localhost:8080/path">>, + websocket_location(other, <<"localhost">>, 8080, <<"/path">>)), + ?assertEqual(<<"wss://localhost/path">>, + websocket_location(ssl, <<"localhost">>, 443, <<"/path">>)), + ?assertEqual(<<"wss://localhost:8443/path">>, + websocket_location(ssl, <<"localhost">>, 8443, <<"/path">>)), + ok. -endif. - diff --git a/src/cowboy_http_websocket_handler.erl b/src/cowboy_http_websocket_handler.erl index b02c28d..90cf7ac 100644 --- a/src/cowboy_http_websocket_handler.erl +++ b/src/cowboy_http_websocket_handler.erl @@ -14,12 +14,13 @@ %% @doc Handler for HTTP WebSocket requests. %% -%% WebSocket handlers must implement three callbacks: websocket_init/3, -%% websocket_handle/3 and websocket_terminate/3. These -%% callbacks will only be called if the connection is upgraded to WebSocket -%% in the HTTP handler's init/3 callback. They are then called in that -%% order, although websocket_handle/3 will be called multiple time, -%% one time for each message or packet received. +%% WebSocket handlers must implement four callbacks: websocket_init/3, +%% websocket_handle/3, websocket_info/3 and +%% websocket_terminate/3. These callbacks will only be called if the +%% connection is upgraded to WebSocket in the HTTP handler's init/3 +%% callback. They are then called in that order, although +%% websocket_handle/3 will be called for each packet received, +%% and websocket_info for each message received. %% %% websocket_init/3 is meant for initialization. It receives %% information about the transport and protocol used, along with the handler @@ -27,11 +28,15 @@ %% If you are going to want to compact the request, you should probably do it %% here. %% -%% websocket_handle/3 receives messages sent to the process and -%% also the data sent to the socket. In the later case the information is -%% given as a tuple {websocket, Data}. It can reply something, do -%% nothing or close the connection. You can choose to hibernate the process -%% by returning hibernate to save memory and CPU. +%% websocket_handle/3 receives the data from the socket. It can reply +%% something, do nothing or close the connection. You can choose to hibernate +%% the process by returning hibernate to save memory and CPU. +%% +%% websocket_info/3 receives messages sent to the process. It has +%% the same reply format as websocket_handle/3 described above. Note +%% that unlike in a gen_server, when websocket_info/3 +%% replies something, it is always to the socket, not to the process that +%% originated the message. %% %% websocket_terminate/3 is meant for cleaning up. It also receives %% the request and the state previously defined, along with a reason for @@ -41,9 +46,11 @@ -export([behaviour_info/1]). %% @private --spec behaviour_info(_) -> undefined | [{websocket_handle, 3} - | {websocket_init, 3} | {websocket_terminate, 3}, ...]. +-spec behaviour_info(_) + -> undefined | [{websocket_handle, 3} | {websocket_info, 3} + | {websocket_init, 3} | {websocket_terminate, 3}, ...]. behaviour_info(callbacks) -> - [{websocket_init, 3}, {websocket_handle, 3}, {websocket_terminate, 3}]; + [{websocket_init, 3}, {websocket_handle, 3}, + {websocket_info, 3}, {websocket_terminate, 3}]; behaviour_info(_Other) -> undefined. diff --git a/test/websocket_handler.erl b/test/websocket_handler.erl index 8e6915b..d06bfee 100644 --- a/test/websocket_handler.erl +++ b/test/websocket_handler.erl @@ -4,7 +4,8 @@ -behaviour(cowboy_http_handler). -behaviour(cowboy_http_websocket_handler). -export([init/3, handle/2, terminate/2]). --export([websocket_init/3, websocket_handle/3, websocket_terminate/3]). +-export([websocket_init/3, websocket_handle/3, + websocket_info/3, websocket_terminate/3]). init(_Any, _Req, _Opts) -> {upgrade, protocol, cowboy_http_websocket}. @@ -19,11 +20,14 @@ websocket_init(_TransportName, Req, _Opts) -> erlang:start_timer(1000, self(), <<"websocket_init">>), {ok, Req, undefined}. -websocket_handle({timeout, _Ref, Msg}, Req, State) -> +websocket_handle(Data, Req, State) -> + {reply, Data, Req, State}. + +websocket_info({timeout, _Ref, Msg}, Req, State) -> erlang:start_timer(1000, self(), <<"websocket_handle">>), {reply, Msg, Req, State}; -websocket_handle({websocket, Data}, Req, State) -> - {reply, Data, Req, State}. +websocket_info(_Info, Req, State) -> + {ok, Req, State}. websocket_terminate(_Reason, _Req, _State) -> ok. -- cgit v1.2.3