From a73004e966a45b78d81168bb03c68acdd7bfea65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 10 Oct 2019 15:53:26 +0200 Subject: Fix a number of low hanging todos --- doc/src/manual/cowboy_middleware.asciidoc | 2 ++ src/cowboy_http.erl | 6 ++---- src/cowboy_req.erl | 1 - src/cowboy_stream_h.erl | 22 +++++----------------- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/doc/src/manual/cowboy_middleware.asciidoc b/doc/src/manual/cowboy_middleware.asciidoc index 11aa3e8..180fb0a 100644 --- a/doc/src/manual/cowboy_middleware.asciidoc +++ b/doc/src/manual/cowboy_middleware.asciidoc @@ -47,6 +47,8 @@ Cowboy will stop middleware execution. No other middleware will be executed. This effectively ends the processing of the request. +// @todo No need to return the Req when stopping. Fix in 3.0. + == Types === env() diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 8ff8ae2..f128a44 100644 --- a/src/cowboy_http.erl +++ b/src/cowboy_http.erl @@ -78,8 +78,8 @@ -record(ps_body, { length :: non_neg_integer() | undefined, received = 0 :: non_neg_integer(), - transfer_decode_fun :: fun(), %% @todo better type - transfer_decode_state :: any() %% @todo better type + transfer_decode_fun :: fun((binary(), cow_http_te:state()) -> cow_http_te:decode_ret()), + transfer_decode_state :: cow_http_te:state() }). -record(stream, { @@ -1275,8 +1275,6 @@ stream_terminate(State0=#state{opts=Opts, in_streamid=InStreamID, in_state=InSta NextOutStreamID = OutStreamID + 1, case lists:keyfind(NextOutStreamID, #stream.id, Streams) of false -> - %% @todo This is clearly wrong, if the stream is gone we need to check if - %% there used to be such a stream, and if there was to send an error. State#state{out_streamid=NextOutStreamID, out_state=wait}; #stream{queue=Commands} -> %% @todo Remove queue from the stream. diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index 0ec7902..ea61194 100644 --- a/src/cowboy_req.erl +++ b/src/cowboy_req.erl @@ -107,7 +107,6 @@ %% While sendfile allows a Len of 0 that means "everything past Offset", %% Cowboy expects the real length as it is used as metadata. -%% @todo We should probably explicitly reject it. -type resp_body() :: iodata() | {sendfile, non_neg_integer(), non_neg_integer(), file:name_all()}. -export_type([resp_body/0]). diff --git a/src/cowboy_stream_h.erl b/src/cowboy_stream_h.erl index 9f0e745..71b2948 100644 --- a/src/cowboy_stream_h.erl +++ b/src/cowboy_stream_h.erl @@ -26,7 +26,6 @@ -export([early_error/5]). -export([request_process/3]). --export([execute/3]). -export([resume/5]). -record(state, { @@ -45,10 +44,6 @@ stream_body_status = normal :: normal | blocking | blocked }). -%% @todo For shutting down children we need to have a timeout before we terminate -%% the stream like supervisors do. So here just send a message to yourself first, -%% and then decide what to do when receiving this message. - -spec init(cowboy_stream:streamid(), cowboy_req:req(), cowboy:opts()) -> {[{spawn, pid(), timeout()}], #state{}}. init(StreamID, Req=#{ref := Ref}, Opts) -> @@ -116,7 +111,6 @@ data(StreamID, IsFin=nofin, Data, State=#state{ data(StreamID, IsFin, Data, State=#state{read_body_pid=Pid, read_body_ref=Ref, read_body_timer_ref=TRef, read_body_buffer=Buffer, body_length=BodyLen0}) -> BodyLen = BodyLen0 + byte_size(Data), - %% @todo Handle the infinity case where no TRef was defined. ok = erlang:cancel_timer(TRef, [{async, true}, {info, false}]), send_request_body(Pid, Ref, IsFin, BodyLen, <>), do_data(StreamID, IsFin, Data, [], State#state{ @@ -191,7 +185,6 @@ info(StreamID, Info={read_body, Pid, Ref, Length, Period}, State=#state{expect=E continue -> [{inform, 100, #{}}, {flow, Length}]; undefined -> [{flow, Length}] end, - %% @todo Handle the case where Period =:= infinity. TRef = erlang:send_after(Period, self(), {{self(), StreamID}, {read_body_timeout, Ref}}), do_info(StreamID, Info, Commands, State#state{ read_body_pid=Pid, @@ -302,7 +295,7 @@ send_request_body(Pid, Ref, fin, BodyLen, Data) -> %% just for errors and throws. %% %% @todo Better spec. --spec request_process(_, _, _) -> _. +-spec request_process(cowboy_req:req(), cowboy_middleware:env(), [module()]) -> ok. request_process(Req, Env, Middlewares) -> OTP = erlang:system_info(otp_release), try @@ -321,12 +314,8 @@ request_process(Req, Env, Middlewares) -> erlang:raise(Class, Reason, erlang:get_stacktrace()) end. -%% @todo -%-spec execute(cowboy_req:req(), #state{}, cowboy_middleware:env(), [module()]) -% -> ok. --spec execute(_, _, _) -> _. execute(_, _, []) -> - ok; %% @todo Maybe error reason should differ here and there. + ok; execute(Req, Env, [Middleware|Tail]) -> case Middleware:execute(Req, Env) of {ok, Req2, Env2} -> @@ -334,11 +323,10 @@ execute(Req, Env, [Middleware|Tail]) -> {suspend, Module, Function, Args} -> proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module, Function, Args]); {stop, _Req2} -> - ok %% @todo Maybe error reason should differ here and there. + ok end. --spec resume(cowboy_middleware:env(), [module()], - module(), module(), [any()]) -> ok. +-spec resume(cowboy_middleware:env(), [module()], module(), atom(), [any()]) -> ok. resume(Env, Tail, Module, Function, Args) -> case apply(Module, Function, Args) of {ok, Req2, Env2} -> @@ -346,5 +334,5 @@ resume(Env, Tail, Module, Function, Args) -> {suspend, Module2, Function2, Args2} -> proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module2, Function2, Args2]); {stop, _Req2} -> - ok %% @todo Maybe error reason should differ here and there. + ok end. -- cgit v1.2.3