Skip to content

Commit

Permalink
Be stricter on Elvis analysis and fix issues
Browse files Browse the repository at this point in the history
  • Loading branch information
paulo-ferraz-oliveira committed Jul 27, 2023
1 parent 3e0948b commit 8a7b68c
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 41 deletions.
9 changes: 4 additions & 5 deletions elvis.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
{config,
[#{dirs => ["src", "test"],
filter => "*.erl",
ruleset => erl_files
},
#{dirs => ["."],
filter => "Makefile",
ruleset => makefiles
ruleset => erl_files,
rules => [{elvis_style, no_throw, disable},
{elvis_style, invalid_dynamic_call, disable},
{elvis_style, dont_repeat_yourself, disable}]
},
#{dirs => ["."],
filter => "rebar.config",
Expand Down
14 changes: 7 additions & 7 deletions src/elli.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

%% @type req(). A record representing an HTTP request.
-type req() :: #req{}.
-elvis([{elvis_style, private_data_types, disable}]).

%% @type http_method(). An uppercase atom representing a known HTTP verb or a
%% binary for other verbs.
Expand All @@ -37,7 +38,7 @@
%% @type body(). A binary or iolist.
-type body() :: binary() | iolist().

-type header() :: {Key::binary(), Value::binary() | string()}.
-type header() :: {Key :: binary(), Value :: binary() | string()}.
-type headers() :: [header()].

-type response_code() :: 100..999.
Expand Down Expand Up @@ -162,19 +163,18 @@ init([Opts]) ->
| SSLSockOpts]),

Acceptors = ets:new(acceptors, [private, set]),
[begin
Pid = elli_http:start_link(self(), Socket, Options,
{Callback, CallbackArgs}),
ets:insert(Acceptors, {Pid})
end
|| _ <- lists:seq(1, MinAcceptors)],
[http_start(Socket, Options, Callback, CallbackArgs, Acceptors)
|| _ <- lists:seq(1, MinAcceptors)],

{ok, #state{socket = Socket,
acceptors = Acceptors,
open_reqs = 0,
options = Options,
callback = {Callback, CallbackArgs}}}.

http_start(Socket, Options, Callback, CallbackArgs, Acceptors) ->
Pid = elli_http:start_link(self(), Socket, Options, {Callback, CallbackArgs}),
ets:insert(Acceptors, {Pid}).

%% @hidden
-spec handle_call(get_acceptors, {pid(), _Tag}, state()) ->
Expand Down
2 changes: 1 addition & 1 deletion src/elli_example_callback.erl
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ chunk_loop(Ref, N) ->
{error, Reason} -> ?LOG_ERROR("error in sending chunk: ~p~n", [Reason])
end,

chunk_loop(Ref, N-1).
chunk_loop(Ref, N - 1).


%%
Expand Down
8 changes: 5 additions & 3 deletions src/elli_http.erl
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
-define(CONNECTION_HEADER, <<"connection">>).
-define(TRANSFER_ENCODING_HEADER, <<"Transfer-Encoding">>).

-elvis([{elvis_style, max_function_arity, disable}]).

%% TODO: use this
%% -type connection_token() :: keep_alive | close.

Expand All @@ -58,7 +60,7 @@ start_link(Server, ListenSocket, Options, Callback) ->
Options :: proplists:proplist(),
Callback :: elli_handler:callback().
accept(Server, ListenSocket, Options, Callback) ->
case catch elli_tcp:accept(ListenSocket, Server, accept_timeout(Options)) of
case elli_tcp:accept(ListenSocket, Server, accept_timeout(Options)) of
{ok, Socket} ->
t(accepted),
?MODULE:keepalive_loop(Socket, Options, Callback);
Expand Down Expand Up @@ -697,7 +699,7 @@ connection(Req, UserHeaders) ->
[]
end.

content_length(Headers, Body)->
content_length(Headers, Body) ->
?IF(is_header_defined(?CONTENT_LENGTH_HEADER, Headers), [],
{?CONTENT_LENGTH_HEADER, iolist_size(Body)}).

Expand Down Expand Up @@ -729,7 +731,7 @@ parse_path({abs_path, FullPath}) ->
Query = maps:get(query, URIMap, <<>>),
Port = maps:get(port, URIMap, case Scheme of http -> 80; https -> 443; _ -> undefined end),
{ok, {Scheme, Host, Port}, {Path, split_path(Path), uri_string:dissect_query(Query)}};
parse_path({absoluteURI, Scheme, Host, Port, Path}) ->
parse_path({'absoluteURI', Scheme, Host, Port, Path}) ->
setelement(2, parse_path({abs_path, Path}), {Scheme, Host, Port});
parse_path(_) ->
{error, unsupported_uri}.
Expand Down
2 changes: 1 addition & 1 deletion src/elli_middleware.erl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ handle_event(Event, Args, Config) ->
Callbacks :: [elli_handler:callback()].
do_init(_, []) ->
{ok, standard};
do_init(Req, [{Mod, Args}|Mods]) ->
do_init(Req, [{Mod, Args} | Mods]) ->
?IF_NOT_EXPORTED(Mod, init, 2, do_init(Req, Mods),
case Mod:init(Req, Args) of
ignore -> do_init(Req, Mods);
Expand Down
15 changes: 8 additions & 7 deletions src/elli_request.erl
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@

-export_type([http_range/0]).

-type http_range() :: {First::non_neg_integer(), Last::non_neg_integer()}
| {offset, Offset::non_neg_integer()}
| {suffix, Length::pos_integer()}.
-type http_range() :: {First :: non_neg_integer(), Last :: non_neg_integer()}
| {offset, Offset :: non_neg_integer()}
| {suffix, Length :: pos_integer()}.

-elvis([{elvis_style, god_modules, disable}]).

%%
%% Helpers for working with a #req{}
Expand Down Expand Up @@ -184,7 +185,7 @@ get_range(#req{headers = Headers}) ->
end.


-spec parse_range_set(Bin::binary()) -> [http_range()] | parse_error.
-spec parse_range_set(Bin :: binary()) -> [http_range()] | parse_error.
parse_range_set(<<ByteRangeSet/binary>>) ->
RangeBins = binary:split(ByteRangeSet, <<",">>, [global]),
Parsed = [parse_range(remove_whitespace(RangeBin))
Expand All @@ -194,7 +195,7 @@ parse_range_set(<<ByteRangeSet/binary>>) ->
false -> Parsed
end.

-spec parse_range(Bin::binary()) -> http_range() | parse_error.
-spec parse_range(Bin :: binary()) -> http_range() | parse_error.
parse_range(<<$-, SuffixBin/binary>>) ->
%% suffix-byte-range
try {suffix, binary_to_integer(SuffixBin)}
Expand Down Expand Up @@ -295,5 +296,5 @@ uri_decode(<<C, Rest/binary>>, Acc) ->
-compile({inline, [hex_to_int/1]}).

hex_to_int(X) when X >= $0, X =< $9 -> X - $0;
hex_to_int(X) when X >= $a, X =< $f -> X - ($a-10);
hex_to_int(X) when X >= $A, X =< $F -> X - ($A-10).
hex_to_int(X) when X >= $a, X =< $f -> X - ($a - 10);
hex_to_int(X) when X >= $A, X =< $F -> X - ($A - 10).
1 change: 1 addition & 0 deletions src/elli_sendfile.erl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
-export([sendfile/5]).

-type sendfile_opts() :: [{chunk_size, non_neg_integer()}].
-export_type([sendfile_opts/0]).

%% @doc Send part of a file on a socket.
%%
Expand Down
6 changes: 3 additions & 3 deletions src/elli_util.erl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

-export_type([range/0]).

-type range() :: {Offset::non_neg_integer(), Length::non_neg_integer()}.
-type range() :: {Offset :: non_neg_integer(), Length :: non_neg_integer()}.

-spec normalize_range(RangeOrSet, Size) -> Normalized when
RangeOrSet :: any(),
Expand Down Expand Up @@ -42,8 +42,8 @@ normalize_range([], _Size) -> undefined;
normalize_range(_, _Size) -> invalid_range.


-spec encode_range(Range::range() | invalid_range,
Size::non_neg_integer()) -> ByteRange::iolist().
-spec encode_range(Range :: range() | invalid_range,
Size :: non_neg_integer()) -> ByteRange :: iolist().
%% @doc: Encode Range to a Content-Range value.
encode_range(Range, Size) ->
[<<"bytes ">>, encode_range_bytes(Range),
Expand Down
4 changes: 2 additions & 2 deletions test/elli_metrics_middleware.erl
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ postprocess(_Req, Res, _Args) ->
%% ELLI EVENT CALLBACKS
%%

handle_event(request_complete, [_Req,_C,_Hs,_B, {Timings, Sizes}], _) ->
handle_event(request_complete, [_Req, _C, _Hs, _B, {Timings, Sizes}], _) ->
ets:insert(elli_stat_table, {timings, Timings}),
ets:insert(elli_stat_table, {sizes, Sizes});
handle_event(chunk_complete, [_Req,_C,_Hs,_B, {Timings, Sizes}], _) ->
handle_event(chunk_complete, [_Req, _C, _Hs, _B, {Timings, Sizes}], _) ->
ets:insert(elli_stat_table, {timings, Timings}),
ets:insert(elli_stat_table, {sizes, Sizes});
handle_event(_Event, _Data, _Args) ->
Expand Down
8 changes: 4 additions & 4 deletions test/elli_middleware_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ hello_world() ->
?assertMatch(<<"Hello World!">>, body(Response)).

compress() ->
Url = "http://localhost:3002/compressed",
URL = "http://localhost:3002/compressed",
Headers = [{<<"Accept-Encoding">>, <<"gzip">>}],
Response = hackney:get(Url, Headers),
Response = hackney:get(URL, Headers),
?assertHeadersEqual([{<<"Connection">>, <<"Keep-Alive">>},
{<<"Content-Encoding">>, <<"gzip">>},
{<<"Content-Length">>, <<"41">>}],
Expand All @@ -43,9 +43,9 @@ compress() ->
headers(Response1)),
?assertEqual(iolist_to_binary(lists:duplicate(86, "Hello World!")),
body(Response1)),
Url2 = "http://localhost:3002/compressed-io_list",
URL2 = "http://localhost:3002/compressed-io_list",
Headers2 = [{<<"Accept-Encoding">>, <<"gzip">>}],
Response2 = hackney:get(Url2, Headers2),
Response2 = hackney:get(URL2, Headers2),
?assertMatch(200, status(Response2)),
?assertHeadersEqual([{<<"Connection">>, <<"Keep-Alive">>},
{<<"Content-Encoding">>, <<"gzip">>},
Expand Down
4 changes: 2 additions & 2 deletions test/elli_ssl_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ chunked() ->
?assertMatch(200, status(Response)),
?assertHeadersEqual([{<<"connection">>, <<"Keep-Alive">>},
{<<"content-type">>, <<"text/event-stream">>},
{<<"transfer-encoding">>,<<"chunked">>}], headers(Response)),
{<<"transfer-encoding">>, <<"chunked">>}], headers(Response)),
?assertMatch(Expected, body(Response)).

sendfile() ->
Expand Down Expand Up @@ -71,7 +71,7 @@ sendfile() ->
acceptor_leak_regression() ->
{ok, Before} = elli:get_acceptors(elli_under_test),
Opts = [{verify, verify_peer},
{verify_fun, {fun(_,_) -> {fail, 23} end, []}},
{verify_fun, {fun(_, _) -> {fail, 23} end, []}},
{reuse_sessions, false}],
{error, _} = ssl:connect("localhost", 3443, Opts),
{ok, After} = elli:get_acceptors(elli_under_test),
Expand Down
14 changes: 8 additions & 6 deletions test/elli_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ keep_alive_timings() ->

keep_alive_timings(Status, Headers, HCRef) ->
?assertMatch(200, Status),
?assertHeadersEqual([{<<"connection">>,<<"Keep-Alive">>},
{<<"content-length">>,<<"12">>}], Headers),
?assertHeadersEqual([{<<"connection">>, <<"Keep-Alive">>},
{<<"content-length">>, <<"12">>}], Headers),
?assertMatch({ok, <<"Hello World!">>}, hackney:body(HCRef)),
%% sizes
?assertMatch(63, get_size_value(resp_headers)),
Expand Down Expand Up @@ -472,7 +472,8 @@ sendfile_range() ->
?assertMatch(206, status(Response)),
?assertHeadersEqual([{<<"connection">>, <<"Keep-Alive">>},
{<<"content-length">>, <<"400">>},
{<<"Content-Range">>, iolist_to_binary(["bytes 300-699/", integer_to_binary(Size)])}],
{<<"Content-Range">>,
iolist_to_binary(["bytes 300-699/", integer_to_binary(Size)])}],
headers(Response)),
?assertEqual(Expected, body(Response)).

Expand Down Expand Up @@ -717,7 +718,7 @@ normalize_range_test_() ->


encode_range_test() ->
Expected = [<<"bytes ">>,<<"*">>,<<"/">>,<<"42">>],
Expected = [<<"bytes ">>, <<"*">>, <<"/">>, <<"42">>],
?assertMatch(Expected, elli_util:encode_range(invalid_range, 42)).

register_test() ->
Expand All @@ -735,7 +736,8 @@ register_test() ->
ok.

invalid_callback_test() ->
case catch elli:start_link([{callback, elli}]) of
E ->
try
elli:start_link([{callback, elli}])
catch throw:E ->
?assertMatch(invalid_callback, E)
end.

0 comments on commit 8a7b68c

Please sign in to comment.