From aa4d86b81f6095316813c599659014c15bf9b935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Wed, 24 Sep 2014 14:39:17 +0300 Subject: Remove the onrequest hook It was redundant with middlewares. Allows us to save a few operations for every incoming requests. --- ROADMAP.md | 3 - doc/src/guide/hooks.ezdoc | 48 +--------- doc/src/guide/http_req_resp.png | Bin 33228 -> 28370 bytes doc/src/guide/http_req_resp.svg | 174 ++++++++++++++--------------------- doc/src/manual/cowboy.ezdoc | 9 -- doc/src/manual/cowboy_protocol.ezdoc | 5 - doc/src/manual/cowboy_req.ezdoc | 6 +- doc/src/manual/cowboy_spdy.ezdoc | 5 - src/cowboy.erl | 3 - src/cowboy_protocol.erl | 22 +---- src/cowboy_spdy.erl | 27 ++---- test/http_SUITE.erl | 40 -------- 12 files changed, 84 insertions(+), 258 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 5a2f95c..ad684ec 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -45,9 +45,6 @@ the halt tuple. The error tuple will therefore be removed. ### Hooks -The `onrequest` hook will be removed. It can easily be replaced -by a middleware. - The interface of the `onresponse` hook will change. There has been a number of issues and added complexity with the current interface that warrant fixing. The main problem is that the diff --git a/doc/src/guide/hooks.ezdoc b/doc/src/guide/hooks.ezdoc index e835a6f..1c19648 100644 --- a/doc/src/guide/hooks.ezdoc +++ b/doc/src/guide/hooks.ezdoc @@ -1,45 +1,7 @@ ::: Hooks -Cowboy provides two hooks. `onrequest` is called once the request -line and headers have been received. `onresponse` is called just -before sending the response. - -:: Onrequest - -The `onrequest` hook is called as soon as Cowboy finishes fetching -the request headers. It occurs before any other processing, including -routing. It can be used to perform any modification needed on the -request object before continuing with the processing. If a reply is -sent inside this hook, then Cowboy will move on to the next request, -skipping any subsequent handling. - -This hook is a function that takes a request object as argument, -and returns a request object. This function MUST NOT crash. Cowboy -will not send any reply if a crash occurs in this function. - -You can specify the `onrequest` hook when creating the listener, -inside the request options. - -``` erlang -cowboy:start_http(my_http_listener, 100, - [{port, 8080}], - [ - {env, [{dispatch, Dispatch}]}, - {onrequest, fun ?MODULE:debug_hook/1} - ] -). -``` - -The following hook function prints the request object everytime a -request is received. This can be useful for debugging, for example. - -``` erlang -debug_hook(Req) -> - erlang:display(Req), - Req. -``` - -Make sure to always return the last request object obtained. +Hooks allow the user to customize Cowboy's behavior during specific +operations. :: Onresponse @@ -48,9 +10,9 @@ to the socket. It can be used for the purposes of logging responses, or for modifying the response headers or body. The best example is providing custom error pages. -Note that like the `onrequest` hook, this function MUST NOT crash. -Cowboy may or may not send a reply if this function crashes. If a reply -is sent, the hook MUST explicitly provide all headers that are needed. +Note that this function MUST NOT crash. Cowboy may or may not send a +reply if this function crashes. If a reply is sent, the hook MUST +explicitly provide all headers that are needed. You can specify the `onresponse` hook when creating the listener. diff --git a/doc/src/guide/http_req_resp.png b/doc/src/guide/http_req_resp.png index e38935f..8c9cae9 100644 Binary files a/doc/src/guide/http_req_resp.png and b/doc/src/guide/http_req_resp.png differ diff --git a/doc/src/guide/http_req_resp.svg b/doc/src/guide/http_req_resp.svg index 0cfa0ae..d1e7f78 100644 --- a/doc/src/guide/http_req_resp.svg +++ b/doc/src/guide/http_req_resp.svg @@ -65,15 +65,15 @@ inkscape:pageopacity="1" inkscape:pageshadow="2" inkscape:zoom="1.4142136" - inkscape:cx="229.71447" + inkscape:cx="82.28271" inkscape:cy="764.83183" inkscape:document-units="px" inkscape:current-layer="layer1" showgrid="false" - inkscape:window-width="1920" - inkscape:window-height="1014" + inkscape:window-width="2560" + inkscape:window-height="1402" inkscape:window-x="0" - inkscape:window-y="33" + inkscape:window-y="38" inkscape:window-maximized="1" inkscape:snap-global="true" showguides="true"> @@ -93,7 +93,7 @@ image/svg+xml - + @@ -101,42 +101,28 @@ inkscape:label="Layer 1" inkscape:groupmode="layer" id="layer1"> - - - + + inkscape:export-filename="/home/essen/extend/cowboy/guide/http_req_resp.png" + inkscape:connector-curvature="0" + id="use5777" + d="m 178.49877,231.1517 203.00246,0.045" + style="fill:none;stroke:#6d8e41;stroke-width:1;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-opacity:1;stroke-dasharray:1.99999999, 3.99999998;stroke-dashoffset:0" /> - - + transform="translate(205.03261,-207.5)"> + + + - - + style="fill:#d1f2a5;fill-opacity:1;fill-rule:nonzero;stroke:#a9ca7d;stroke-width:3;stroke-linecap:butt;stroke-linejoin:round;stroke-miterlimit:4;stroke-opacity:1;stroke-dasharray:none;stroke-dashoffset:0" /> router + y="323.0921">router some text - onrequest handler + y="410.38519">handler middlewares reply + y="236.73991">reply onresponse + y="323.09195">onresponse diff --git a/doc/src/manual/cowboy.ezdoc b/doc/src/manual/cowboy.ezdoc index 209d473..c26ed37 100644 --- a/doc/src/manual/cowboy.ezdoc +++ b/doc/src/manual/cowboy.ezdoc @@ -25,15 +25,6 @@ A binary status can be used to set a custom message. HTTP version. -: onrequest_fun() = fun((cowboy_req:req()) -> cowboy_req:req()) - -Fun called immediately after receiving a request. - -It can perform any operation on the Req object, including -reading the request body or replying. If a reply is sent, -the processing of the request ends here, before any middleware -is executed. - : onresponse_fun() = fun((http_status(), http_headers(), iodata(), cowboy_req:req()) -> cowboy_req:req()) diff --git a/doc/src/manual/cowboy_protocol.ezdoc b/doc/src/manual/cowboy_protocol.ezdoc index 6813295..335f2ff 100644 --- a/doc/src/manual/cowboy_protocol.ezdoc +++ b/doc/src/manual/cowboy_protocol.ezdoc @@ -14,7 +14,6 @@ as a Ranch protocol. | {max_keepalive, non_neg_integer()} | {max_request_line_length, non_neg_integer()} | {middlewares, [module()]} - | {onrequest, cowboy:onrequest_fun()} | {onresponse, cowboy:onresponse_fun()} | {timeout, timeout()}] @@ -67,10 +66,6 @@ Maximum length of the request line. List of middlewares to execute for every requests. -: onrequest (undefined) - -Fun called every time a request is received. - : onresponse (undefined) Fun called every time a response is sent. diff --git a/doc/src/manual/cowboy_req.ezdoc b/doc/src/manual/cowboy_req.ezdoc index 94556b2..bcec9b9 100644 --- a/doc/src/manual/cowboy_req.ezdoc +++ b/doc/src/manual/cowboy_req.ezdoc @@ -126,8 +126,7 @@ Types: Return the requested URL excluding the path component. This function will always return `undefined` until the -`cowboy_router` middleware has been executed. This includes -the `onrequest` hook. +`cowboy_router` middleware has been executed. : match_cookies(Req, Fields) -> Map @@ -371,8 +370,7 @@ Types: Return the requested URL. This function will always return `undefined` until the -`cowboy_router` middleware has been executed. This includes -the `onrequest` hook. +`cowboy_router` middleware has been executed. : version(Req) -> Version diff --git a/doc/src/manual/cowboy_spdy.ezdoc b/doc/src/manual/cowboy_spdy.ezdoc index 51a2110..534434c 100644 --- a/doc/src/manual/cowboy_spdy.ezdoc +++ b/doc/src/manual/cowboy_spdy.ezdoc @@ -6,7 +6,6 @@ The `cowboy_spdy` module implements SPDY/3 as a Ranch protocol. : opts() = [{env, cowboy_middleware:env()} | {middlewares, [module()]} - | {onrequest, cowboy:onrequest_fun()} | {onresponse, cowboy:onresponse_fun()}] Configuration for the SPDY protocol handler. @@ -30,10 +29,6 @@ Initial middleware environment. List of middlewares to execute for every requests. -: onrequest (undefined) - -Fun called every time a request is received. - : onresponse (undefined) Fun called every time a response is sent. diff --git a/src/cowboy.erl b/src/cowboy.erl index 8e9232f..af9f1b3 100644 --- a/src/cowboy.erl +++ b/src/cowboy.erl @@ -34,9 +34,6 @@ -type http_version() :: 'HTTP/1.1' | 'HTTP/1.0'. -export_type([http_version/0]). --type onrequest_fun() :: fun((Req) -> Req). --export_type([onrequest_fun/0]). - -type onresponse_fun() :: fun((http_status(), http_headers(), iodata(), Req) -> Req). -export_type([onresponse_fun/0]). diff --git a/src/cowboy_protocol.erl b/src/cowboy_protocol.erl index 1026d28..a5873ea 100644 --- a/src/cowboy_protocol.erl +++ b/src/cowboy_protocol.erl @@ -32,7 +32,6 @@ | {max_keepalive, non_neg_integer()} | {max_request_line_length, non_neg_integer()} | {middlewares, [module()]} - | {onrequest, cowboy:onrequest_fun()} | {onresponse, cowboy:onresponse_fun()} | {timeout, timeout()}]. -export_type([opts/0]). @@ -43,7 +42,6 @@ middlewares :: [module()], compress :: boolean(), env :: cowboy_middleware:env(), - onrequest :: undefined | cowboy:onrequest_fun(), onresponse = undefined :: undefined | cowboy:onresponse_fun(), max_empty_lines :: non_neg_integer(), req_keepalive = 1 :: non_neg_integer(), @@ -85,7 +83,6 @@ init(Ref, Socket, Transport, Opts) -> MaxRequestLineLength = get_value(max_request_line_length, Opts, 4096), Middlewares = get_value(middlewares, Opts, [cowboy_router, cowboy_handler]), Env = [{listener, Ref}|get_value(env, Opts, [])], - OnRequest = get_value(onrequest, Opts, undefined), OnResponse = get_value(onresponse, Opts, undefined), Timeout = get_value(timeout, Opts, 5000), ok = ranch:accept_ack(Ref), @@ -95,8 +92,7 @@ init(Ref, Socket, Transport, Opts) -> max_request_line_length=MaxRequestLineLength, max_header_name_length=MaxHeaderNameLength, max_header_value_length=MaxHeaderValueLength, max_headers=MaxHeaders, - onrequest=OnRequest, onresponse=OnResponse, - timeout=Timeout, until=until(Timeout)}, 0). + onresponse=OnResponse, timeout=Timeout, until=until(Timeout)}, 0). -spec until(timeout()) -> non_neg_integer() | infinity. until(infinity) -> @@ -410,26 +406,12 @@ request(Buffer, State=#state{socket=Socket, transport=Transport, Req = cowboy_req:new(Socket, Transport, Peer, Method, Path, Query, Version, Headers, Host, Port, Buffer, ReqKeepalive < MaxKeepalive, Compress, OnResponse), - onrequest(Req, State); + execute(Req, State); {error, _} -> %% Couldn't read the peer address; connection is gone. terminate(State) end. -%% Call the global onrequest callback. The callback can send a reply, -%% in which case we consider the request handled and move on to the next -%% one. Note that since we haven't dispatched yet, we don't know the -%% handler, host_info, path_info or bindings yet. --spec onrequest(cowboy_req:req(), #state{}) -> ok. -onrequest(Req, State=#state{onrequest=undefined}) -> - execute(Req, State); -onrequest(Req, State=#state{onrequest=OnRequest}) -> - Req2 = OnRequest(Req), - case cowboy_req:get(resp_state, Req2) of - waiting -> execute(Req2, State); - _ -> next_request(Req2, State, ok) - end. - -spec execute(cowboy_req:req(), #state{}) -> ok. execute(Req, State=#state{middlewares=Middlewares, env=Env}) -> execute(Req, State, Env, Middlewares). diff --git a/src/cowboy_spdy.erl b/src/cowboy_spdy.erl index 8da9613..5b89c48 100644 --- a/src/cowboy_spdy.erl +++ b/src/cowboy_spdy.erl @@ -24,7 +24,7 @@ -export([system_code_change/4]). %% Internal request process. --export([request_init/11]). +-export([request_init/10]). -export([resume/5]). -export([reply/4]). -export([stream_reply/3]). @@ -59,7 +59,6 @@ buffer = <<>> :: binary(), middlewares, env, - onrequest, onresponse, peer, zdef, @@ -70,7 +69,6 @@ -type opts() :: [{env, cowboy_middleware:env()} | {middlewares, [module()]} - | {onrequest, cowboy:onrequest_fun()} | {onresponse, cowboy:onresponse_fun()}]. -export_type([opts/0]). @@ -97,13 +95,12 @@ init(Parent, Ref, Socket, Transport, Opts) -> {ok, Peer} = Transport:peername(Socket), Middlewares = get_value(middlewares, Opts, [cowboy_router, cowboy_handler]), Env = [{listener, Ref}|get_value(env, Opts, [])], - OnRequest = get_value(onrequest, Opts, undefined), OnResponse = get_value(onresponse, Opts, undefined), Zdef = cow_spdy:deflate_init(), Zinf = cow_spdy:inflate_init(), ok = ranch:accept_ack(Ref), loop(#state{parent=Parent, socket=Socket, transport=Transport, - middlewares=Middlewares, env=Env, onrequest=OnRequest, + middlewares=Middlewares, env=Env, onresponse=OnResponse, peer=Peer, zdef=Zdef, zinf=Zinf}). loop(State=#state{parent=Parent, socket=Socket, transport=Transport, @@ -257,11 +254,11 @@ handle_frame(State, {syn_stream, StreamID, AssocToStreamID, %% Erlang does not allow us to control the priority of processes %% so we ignore that value entirely. handle_frame(State=#state{middlewares=Middlewares, env=Env, - onrequest=OnRequest, onresponse=OnResponse, peer=Peer}, + onresponse=OnResponse, peer=Peer}, {syn_stream, StreamID, _, IsFin, _, _, Method, _, Host, Path, Version, Headers}) -> Pid = spawn_link(?MODULE, request_init, [ - {self(), StreamID}, Peer, OnRequest, OnResponse, + {self(), StreamID}, Peer, OnResponse, Env, Middlewares, Method, Host, Path, Version, Headers ]), new_child(State, StreamID, Pid, IsFin); @@ -385,11 +382,10 @@ delete_child(Pid, State=#state{children=Children}) -> %% Request process. -spec request_init(socket(), {inet:ip_address(), inet:port_number()}, - cowboy:onrequest_fun(), cowboy:onresponse_fun(), - cowboy_middleware:env(), [module()], + cowboy:onresponse_fun(), cowboy_middleware:env(), [module()], binary(), binary(), binary(), binary(), [{binary(), binary()}]) -> ok. -request_init(FakeSocket, Peer, OnRequest, OnResponse, +request_init(FakeSocket, Peer, OnResponse, Env, Middlewares, Method, Host, Path, Version, Headers) -> {Host2, Port} = cow_http:parse_fullhost(Host), {Path2, Qs} = cow_http:parse_fullpath(Path), @@ -397,16 +393,7 @@ request_init(FakeSocket, Peer, OnRequest, OnResponse, Req = cowboy_req:new(FakeSocket, ?MODULE, Peer, Method, Path2, Qs, Version2, Headers, Host2, Port, <<>>, true, false, OnResponse), - case OnRequest of - undefined -> - execute(Req, Env, Middlewares); - _ -> - Req2 = OnRequest(Req), - case cowboy_req:get(resp_state, Req2) of - waiting -> execute(Req2, Env, Middlewares); - _ -> ok - end - end. + execute(Req, Env, Middlewares). -spec execute(cowboy_req:req(), cowboy_middleware:env(), [module()]) -> ok. diff --git a/test/http_SUITE.erl b/test/http_SUITE.erl index 1bc13c1..e98ce1b 100644 --- a/test/http_SUITE.erl +++ b/test/http_SUITE.erl @@ -34,7 +34,6 @@ all() -> {group, https}, {group, http_compress}, {group, https_compress}, - {group, onrequest}, {group, onresponse}, {group, onresponse_capitalize}, {group, parse_host}, @@ -43,7 +42,6 @@ all() -> groups() -> Tests = cowboy_test:all(?MODULE) -- [ - onrequest, onrequest_reply, onrequest_hook, onresponse_crash, onresponse_reply, onresponse_capitalize, parse_host, set_env_dispatch ], @@ -52,10 +50,6 @@ groups() -> {https, [parallel], Tests}, {http_compress, [parallel], Tests}, {https_compress, [parallel], Tests}, - {onrequest, [parallel], [ - onrequest, - onrequest_reply - ]}, {onresponse, [parallel], [ onresponse_crash, onresponse_reply @@ -98,15 +92,6 @@ init_per_group(Name = https_compress, Config) -> {compress, true} ], Config); %% Most, if not all of these, should be in separate test suites. -init_per_group(onrequest, Config) -> - {ok, _} = cowboy:start_http(onrequest, 100, [{port, 0}], [ - {env, [{dispatch, init_dispatch(Config)}]}, - {max_keepalive, 50}, - {onrequest, fun do_onrequest_hook/1}, - {timeout, 500} - ]), - Port = ranch:get_port(onrequest), - [{type, tcp}, {port, Port}, {opts, []}|Config]; init_per_group(onresponse, Config) -> {ok, _} = cowboy:start_http(onresponse, 100, [{port, 0}], [ {env, [{dispatch, init_dispatch(Config)}]}, @@ -574,31 +559,6 @@ nc_rand(Config) -> nc_zero(Config) -> do_nc(Config, "/dev/zero"). -onrequest(Config) -> - ConnPid = gun_open(Config), - Ref = gun:get(ConnPid, "/"), - {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), - {<<"server">>, <<"Serenity">>} = lists:keyfind(<<"server">>, 1, Headers), - {ok, <<"http_handler">>} = gun:await_body(ConnPid, Ref), - ok. - -onrequest_reply(Config) -> - ConnPid = gun_open(Config), - Ref = gun:get(ConnPid, "/?reply=1"), - {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), - {<<"server">>, <<"Cowboy">>} = lists:keyfind(<<"server">>, 1, Headers), - {ok, <<"replied!">>} = gun:await_body(ConnPid, Ref), - ok. - -%% Hook for the above onrequest tests. -do_onrequest_hook(Req) -> - case cowboy_req:match_qs(Req, [{reply, [], noreply}]) of - #{reply := noreply} -> - cowboy_req:set_resp_header(<<"server">>, <<"Serenity">>, Req); - _ -> - cowboy_req:reply(200, [], <<"replied!">>, Req) - end. - onresponse_capitalize(Config) -> Client = raw_open(Config), ok = raw_send(Client, "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"), -- cgit v1.2.3