Browse Source

Fix a number of low hanging todos

Loïc Hoguin 5 years ago
parent
commit
a73004e966
4 changed files with 9 additions and 22 deletions
  1. 2 0
      doc/src/manual/cowboy_middleware.asciidoc
  2. 2 4
      src/cowboy_http.erl
  3. 0 1
      src/cowboy_req.erl
  4. 5 17
      src/cowboy_stream_h.erl

+ 2 - 0
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
 will be executed. This effectively ends the processing of
 the request.
 the request.
 
 
+// @todo No need to return the Req when stopping. Fix in 3.0.
+
 == Types
 == Types
 
 
 === env()
 === env()

+ 2 - 4
src/cowboy_http.erl

@@ -78,8 +78,8 @@
 -record(ps_body, {
 -record(ps_body, {
 	length :: non_neg_integer() | undefined,
 	length :: non_neg_integer() | undefined,
 	received = 0 :: non_neg_integer(),
 	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, {
 -record(stream, {
@@ -1275,8 +1275,6 @@ stream_terminate(State0=#state{opts=Opts, in_streamid=InStreamID, in_state=InSta
 			NextOutStreamID = OutStreamID + 1,
 			NextOutStreamID = OutStreamID + 1,
 			case lists:keyfind(NextOutStreamID, #stream.id, Streams) of
 			case lists:keyfind(NextOutStreamID, #stream.id, Streams) of
 				false ->
 				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};
 					State#state{out_streamid=NextOutStreamID, out_state=wait};
 				#stream{queue=Commands} ->
 				#stream{queue=Commands} ->
 					%% @todo Remove queue from the stream.
 					%% @todo Remove queue from the stream.

+ 0 - 1
src/cowboy_req.erl

@@ -107,7 +107,6 @@
 
 
 %% While sendfile allows a Len of 0 that means "everything past Offset",
 %% While sendfile allows a Len of 0 that means "everything past Offset",
 %% Cowboy expects the real length as it is used as metadata.
 %% Cowboy expects the real length as it is used as metadata.
-%% @todo We should probably explicitly reject it.
 -type resp_body() :: iodata()
 -type resp_body() :: iodata()
 	| {sendfile, non_neg_integer(), non_neg_integer(), file:name_all()}.
 	| {sendfile, non_neg_integer(), non_neg_integer(), file:name_all()}.
 -export_type([resp_body/0]).
 -export_type([resp_body/0]).

+ 5 - 17
src/cowboy_stream_h.erl

@@ -26,7 +26,6 @@
 -export([early_error/5]).
 -export([early_error/5]).
 
 
 -export([request_process/3]).
 -export([request_process/3]).
--export([execute/3]).
 -export([resume/5]).
 -export([resume/5]).
 
 
 -record(state, {
 -record(state, {
@@ -45,10 +44,6 @@
 	stream_body_status = normal :: normal | blocking | blocked
 	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())
 -spec init(cowboy_stream:streamid(), cowboy_req:req(), cowboy:opts())
 	-> {[{spawn, pid(), timeout()}], #state{}}.
 	-> {[{spawn, pid(), timeout()}], #state{}}.
 init(StreamID, Req=#{ref := Ref}, Opts) ->
 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,
 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}) ->
 		read_body_timer_ref=TRef, read_body_buffer=Buffer, body_length=BodyLen0}) ->
 	BodyLen = BodyLen0 + byte_size(Data),
 	BodyLen = BodyLen0 + byte_size(Data),
-	%% @todo Handle the infinity case where no TRef was defined.
 	ok = erlang:cancel_timer(TRef, [{async, true}, {info, false}]),
 	ok = erlang:cancel_timer(TRef, [{async, true}, {info, false}]),
 	send_request_body(Pid, Ref, IsFin, BodyLen, <<Buffer/binary, Data/binary>>),
 	send_request_body(Pid, Ref, IsFin, BodyLen, <<Buffer/binary, Data/binary>>),
 	do_data(StreamID, IsFin, Data, [], State#state{
 	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}];
 		continue -> [{inform, 100, #{}}, {flow, Length}];
 		undefined -> [{flow, Length}]
 		undefined -> [{flow, Length}]
 	end,
 	end,
-	%% @todo Handle the case where Period =:= infinity.
 	TRef = erlang:send_after(Period, self(), {{self(), StreamID}, {read_body_timeout, Ref}}),
 	TRef = erlang:send_after(Period, self(), {{self(), StreamID}, {read_body_timeout, Ref}}),
 	do_info(StreamID, Info, Commands, State#state{
 	do_info(StreamID, Info, Commands, State#state{
 		read_body_pid=Pid,
 		read_body_pid=Pid,
@@ -302,7 +295,7 @@ send_request_body(Pid, Ref, fin, BodyLen, Data) ->
 %% just for errors and throws.
 %% just for errors and throws.
 %%
 %%
 %% @todo Better spec.
 %% @todo Better spec.
--spec request_process(_, _, _) -> _.
+-spec request_process(cowboy_req:req(), cowboy_middleware:env(), [module()]) -> ok.
 request_process(Req, Env, Middlewares) ->
 request_process(Req, Env, Middlewares) ->
 	OTP = erlang:system_info(otp_release),
 	OTP = erlang:system_info(otp_release),
 	try
 	try
@@ -321,12 +314,8 @@ request_process(Req, Env, Middlewares) ->
 			erlang:raise(Class, Reason, erlang:get_stacktrace())
 			erlang:raise(Class, Reason, erlang:get_stacktrace())
 	end.
 	end.
 
 
-%% @todo
-%-spec execute(cowboy_req:req(), #state{}, cowboy_middleware:env(), [module()])
-%	-> ok.
--spec execute(_, _, _) -> _.
 execute(_, _, []) ->
 execute(_, _, []) ->
-	ok; %% @todo Maybe error reason should differ here and there.
+	ok;
 execute(Req, Env, [Middleware|Tail]) ->
 execute(Req, Env, [Middleware|Tail]) ->
 	case Middleware:execute(Req, Env) of
 	case Middleware:execute(Req, Env) of
 		{ok, Req2, Env2} ->
 		{ok, Req2, Env2} ->
@@ -334,11 +323,10 @@ execute(Req, Env, [Middleware|Tail]) ->
 		{suspend, Module, Function, Args} ->
 		{suspend, Module, Function, Args} ->
 			proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module, Function, Args]);
 			proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module, Function, Args]);
 		{stop, _Req2} ->
 		{stop, _Req2} ->
-			ok %% @todo Maybe error reason should differ here and there.
+			ok
 	end.
 	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) ->
 resume(Env, Tail, Module, Function, Args) ->
 	case apply(Module, Function, Args) of
 	case apply(Module, Function, Args) of
 		{ok, Req2, Env2} ->
 		{ok, Req2, Env2} ->
@@ -346,5 +334,5 @@ resume(Env, Tail, Module, Function, Args) ->
 		{suspend, Module2, Function2, Args2} ->
 		{suspend, Module2, Function2, Args2} ->
 			proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module2, Function2, Args2]);
 			proc_lib:hibernate(?MODULE, resume, [Env, Tail, Module2, Function2, Args2]);
 		{stop, _Req2} ->
 		{stop, _Req2} ->
-			ok %% @todo Maybe error reason should differ here and there.
+			ok
 	end.
 	end.