Better distinguish between Ranch and socket options

A map should now be used when specifying transport options
that contain more than just socket options. It is still
possible to pass a list of socket options directly as a
convenience.

The ack_timeout is renamed to handshake_timeout when
specified as a map. This corresponds to the new function
ranch:handshake/1,2 that will be favored in Ranch 2.0.

Specifying Ranch-specific options via the proplist will
no longer be possible starting from Ranch 2.0.

4 files changed, 118 insertions, 62 deletions

diff --git a/src/ranch.erl b/src/ranch.erl
index 0edcd78..cab6e8f 100644
--- a/src/ranch.erl
+++ b/src/ranch.erl
@@ -48,6 +48,7 @@
 -type max_conns() :: non_neg_integer() | infinity.
 -export_type([max_conns/0]).
 
+%% This type is deprecated and will be removed in Ranch 2.0.
 -type opt() :: {ack_timeout, timeout()}
 	| {connection_type, worker | supervisor}
 	| {max_connections, max_conns()}
@@ -56,28 +57,33 @@
 	| {socket, any()}.
 -export_type([opt/0]).
 
+-type opts() :: any() | #{
+	connection_type => worker | supervisor,
+	handshake_timeout => timeout(),
+	max_connections => max_conns(),
+	num_acceptors => pos_integer(),
+	shutdown => timeout() | brutal_kill,
+	socket => any(),
+	socket_opts => any()
+}.
+-export_type([opts/0]).
+
 -type ref() :: any().
 -export_type([ref/0]).
 
--spec start_listener(ref(), module(), any(), module(), any())
-	-> supervisor:startchild_ret().
-start_listener(Ref, Transport, TransOpts, Protocol, ProtoOpts) ->
-	NumAcceptors = proplists:get_value(num_acceptors, TransOpts, 10),
-	start_listener(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts).
-
--spec start_listener(ref(), non_neg_integer(), module(), any(), module(), any())
+-spec start_listener(ref(), module(), opts(), module(), any())
 	-> supervisor:startchild_ret().
-start_listener(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts)
-		when is_integer(NumAcceptors) andalso is_atom(Transport)
-		andalso is_atom(Protocol) ->
+start_listener(Ref, Transport, TransOpts0, Protocol, ProtoOpts)
+		when is_atom(Transport), is_atom(Protocol) ->
+	TransOpts = normalize_opts(TransOpts0),
 	_ = code:ensure_loaded(Transport),
 	case erlang:function_exported(Transport, name, 0) of
 		false ->
 			{error, badarg};
 		true ->
-			Res = supervisor:start_child(ranch_sup, child_spec(Ref, NumAcceptors,
+			Res = supervisor:start_child(ranch_sup, child_spec(Ref,
 					Transport, TransOpts, Protocol, ProtoOpts)),
-			Socket = proplists:get_value(socket, TransOpts),
+			Socket = maps:get(socket, TransOpts, undefined),
 			case Res of
 				{ok, Pid} when Socket =/= undefined ->
 					%% Give ownership of the socket to ranch_acceptors_sup
@@ -98,6 +104,59 @@ start_listener(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts)
 			maybe_started(Res)
 	end.
 
+-spec start_listener(ref(), non_neg_integer(), module(), opts(), module(), any())
+	-> supervisor:startchild_ret().
+start_listener(Ref, NumAcceptors, Transport, TransOpts0, Protocol, ProtoOpts)
+		when is_integer(NumAcceptors), is_atom(Transport), is_atom(Protocol) ->
+	TransOpts = normalize_opts(TransOpts0),
+	start_listener(Ref, Transport, TransOpts#{num_acceptors => NumAcceptors},
+		Protocol, ProtoOpts).
+
+normalize_opts(Map) when is_map(Map) ->
+	Map;
+normalize_opts(List0) when is_list(List0) ->
+	Map0 = #{},
+	{Map1, List1} = case take(ack_timeout, List0) of
+		{value, HandshakeTimeout, Tail0} ->
+			{Map0#{handshake_timeout => HandshakeTimeout}, Tail0};
+		false ->
+			{Map0, List0}
+	end,
+	{Map, List} = lists:foldl(fun(Key, {Map2, List2}) ->
+		case take(Key, List2) of
+			{value, ConnectionType, Tail2} ->
+				{Map2#{Key => ConnectionType}, Tail2};
+			false ->
+				{Map2, List2}
+		end
+	end, {Map1, List1}, [connection_type, max_connections, num_acceptors, shutdown, socket]),
+	if
+		Map =:= #{} ->
+			ok;
+		true ->
+			%% @todo This needs a test.
+			error_logger:warning_msg(
+				"Setting Ranch options together with socket options "
+				"is deprecated. Please use the new map syntax that allows "
+				"specifying socket options separately from other options.~n")
+	end,
+	case List of
+		[] -> Map;
+		_ -> Map#{socket_opts => List}
+	end;
+normalize_opts(Any) ->
+	#{socket_opts => Any}.
+
+take(Key, List) ->
+	take(Key, List, []).
+
+take(_, [], _) ->
+	false;
+take(Key, [{Key, Value}|Tail], Acc) ->
+	{value, Value, lists:reverse(Acc, Tail)};
+take(Key, [Value|Tail], Acc) ->
+	take(Key, Tail, [Value|Acc]).
+
 maybe_started({error, {{shutdown,
 		{failed_to_start_child, ranch_acceptors_sup,
 			{listen_error, _, Reason}}}, _}} = Error) ->
@@ -151,25 +210,26 @@ maybe_resumed({ok, _, _}) ->
 maybe_resumed(Res) ->
 	Res.
 
--spec child_spec(ref(), module(), any(), module(), any())
+-spec child_spec(ref(), module(), opts(), module(), any())
 	-> supervisor:child_spec().
-child_spec(Ref, Transport, TransOpts, Protocol, ProtoOpts) ->
-	NumAcceptors = proplists:get_value(num_acceptors, TransOpts, 10),
-	child_spec(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts).
-
--spec child_spec(ref(), non_neg_integer(), module(), any(), module(), any())
-	-> supervisor:child_spec().
-child_spec(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts)
-		when is_integer(NumAcceptors) andalso is_atom(Transport)
-		andalso is_atom(Protocol) ->
+child_spec(Ref, Transport, TransOpts0, Protocol, ProtoOpts) ->
+	TransOpts = normalize_opts(TransOpts0),
 	{{ranch_listener_sup, Ref}, {ranch_listener_sup, start_link, [
-		Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts
+		Ref, Transport, TransOpts, Protocol, ProtoOpts
 	]}, permanent, infinity, supervisor, [ranch_listener_sup]}.
 
+-spec child_spec(ref(), non_neg_integer(), module(), opts(), module(), any())
+	-> supervisor:child_spec().
+child_spec(Ref, NumAcceptors, Transport, TransOpts0, Protocol, ProtoOpts)
+		when is_integer(NumAcceptors), is_atom(Transport), is_atom(Protocol) ->
+	TransOpts = normalize_opts(TransOpts0),
+	child_spec(Ref, Transport, TransOpts#{num_acceptors => NumAcceptors},
+		Protocol, ProtoOpts).
+
 -spec accept_ack(ref()) -> ok.
 accept_ack(Ref) ->
-	receive {handshake, Ref, Transport, Socket, AckTimeout} ->
-		Transport:accept_ack(Socket, AckTimeout)
+	receive {handshake, Ref, Transport, Socket, HandshakeTimeout} ->
+		Transport:accept_ack(Socket, HandshakeTimeout)
 	end.
 
 -spec handshake(ref()) -> {ok, ranch_transport:socket()}.
@@ -178,8 +238,8 @@ handshake(Ref) ->
 
 -spec handshake(ref(), any()) -> {ok, ranch_transport:socket()}.
 handshake(Ref, Opts) ->
-	receive {handshake, Ref, Transport, Socket, AckTimeout} ->
-		Transport:handshake(Socket, Opts, AckTimeout)
+	receive {handshake, Ref, Transport, Socket, HandshakeTimeout} ->
+		Transport:handshake(Socket, Opts, HandshakeTimeout)
 	end.
 
 -spec remove_connection(ref()) -> ok.
@@ -220,8 +280,9 @@ set_max_connections(Ref, MaxConnections) ->
 get_transport_options(Ref) ->
 	ranch_server:get_transport_options(Ref).
 
--spec set_transport_options(ref(), any()) -> ok | {error, running}.
-set_transport_options(Ref, TransOpts) ->
+-spec set_transport_options(ref(), opts()) -> ok | {error, running}.
+set_transport_options(Ref, TransOpts0) ->
+	TransOpts = normalize_opts(TransOpts0),
 	case get_status(Ref) of
 		suspended ->
 			ok = ranch_server:set_transport_options(Ref, TransOpts);
@@ -229,7 +290,7 @@ set_transport_options(Ref, TransOpts) ->
 			{error, running}
 	end.
 
--spec get_protocol_options(ref()) -> any().
+-spec get_protocol_options(ref()) -> opts().
 get_protocol_options(Ref) ->
 	ranch_server:get_protocol_options(Ref).
 
@@ -248,7 +309,7 @@ info(Ref) ->
 	listener_info(Ref, Pid).
 
 listener_info(Ref, Pid) ->
-	[_, NumAcceptors, Transport, _, Protocol, _] = ranch_server:get_listener_start_args(Ref),
+	[_, Transport, _, Protocol, _] = ranch_server:get_listener_start_args(Ref),
 	ConnsSup = ranch_server:get_connections_sup(Ref),
 	Status = get_status(Ref),
 	{IP, Port} = get_addr(Ref),
@@ -260,7 +321,6 @@ listener_info(Ref, Pid) ->
 		{status, Status},
 		{ip, IP},
 		{port, Port},
-		{num_acceptors, NumAcceptors},
 		{max_connections, MaxConns},
 		{active_connections, ranch_conns_sup:active_connections(ConnsSup)},
 		{all_connections, proplists:get_value(active, supervisor:count_children(ConnsSup))},
diff --git a/src/ranch_acceptors_sup.erl b/src/ranch_acceptors_sup.erl
index 9e39f2d..9ec4abf 100644
--- a/src/ranch_acceptors_sup.erl
+++ b/src/ranch_acceptors_sup.erl
@@ -15,28 +15,24 @@
 -module(ranch_acceptors_sup).
 -behaviour(supervisor).
 
--export([start_link/3]).
+-export([start_link/2]).
 -export([init/1]).
 
--spec start_link(ranch:ref(), non_neg_integer(), module())
+-spec start_link(ranch:ref(), module())
 	-> {ok, pid()}.
-start_link(Ref, NumAcceptors, Transport) ->
-	supervisor:start_link(?MODULE, [Ref, NumAcceptors, Transport]).
+start_link(Ref, Transport) ->
+	supervisor:start_link(?MODULE, [Ref, Transport]).
 
-init([Ref, NumAcceptors, Transport]) ->
+init([Ref, Transport]) ->
 	ConnsSup = ranch_server:get_connections_sup(Ref),
 	TransOpts = ranch_server:get_transport_options(Ref),
-	LSocket = case proplists:get_value(socket, TransOpts) of
+	NumAcceptors = maps:get(num_acceptors, TransOpts, 10),
+	LSocket = case maps:get(socket, TransOpts, undefined) of
 		undefined ->
-			TransOpts2 = proplists:delete(ack_timeout,
-				proplists:delete(connection_type,
-				proplists:delete(max_connections,
-				proplists:delete(num_acceptors,
-				proplists:delete(shutdown,
-				proplists:delete(socket, TransOpts)))))),
-			case Transport:listen(TransOpts2) of
+			SocketOpts = maps:get(socket_opts, TransOpts, []),
+			case Transport:listen(SocketOpts) of
 				{ok, Socket} -> Socket;
-				{error, Reason} -> listen_error(Ref, Transport, TransOpts2, Reason)
+				{error, Reason} -> listen_error(Ref, Transport, SocketOpts, Reason)
 			end;
 		Socket ->
 			Socket
diff --git a/src/ranch_conns_sup.erl b/src/ranch_conns_sup.erl
index 72de2d7..ddf9a20 100644
--- a/src/ranch_conns_sup.erl
+++ b/src/ranch_conns_sup.erl
@@ -39,7 +39,7 @@
 	transport = undefined :: module(),
 	protocol = undefined :: module(),
 	opts :: any(),
-	ack_timeout :: timeout(),
+	handshake_timeout :: timeout(),
 	max_conns = undefined :: ranch:max_conns()
 }).
 
@@ -99,14 +99,14 @@ init(Parent, Ref, Transport, Protocol) ->
 	ok = ranch_server:set_connections_sup(Ref, self()),
 	MaxConns = ranch_server:get_max_connections(Ref),
 	TransOpts = ranch_server:get_transport_options(Ref),
-	ConnType = proplists:get_value(connection_type, TransOpts, worker),
-	Shutdown = proplists:get_value(shutdown, TransOpts, 5000),
-	AckTimeout = proplists:get_value(ack_timeout, TransOpts, 5000),
+	ConnType = maps:get(connection_type, TransOpts, worker),
+	Shutdown = maps:get(shutdown, TransOpts, 5000),
+	HandshakeTimeout = maps:get(handshake_timeout, TransOpts, 5000),
 	ProtoOpts = ranch_server:get_protocol_options(Ref),
 	ok = proc_lib:init_ack(Parent, {ok, self()}),
 	loop(#state{parent=Parent, ref=Ref, conn_type=ConnType,
 		shutdown=Shutdown, transport=Transport, protocol=Protocol,
-		opts=ProtoOpts, ack_timeout=AckTimeout, max_conns=MaxConns}, 0, 0, []).
+		opts=ProtoOpts, handshake_timeout=HandshakeTimeout, max_conns=MaxConns}, 0, 0, []).
 
 loop(State=#state{parent=Parent, ref=Ref, conn_type=ConnType,
 		transport=Transport, protocol=Protocol, opts=Opts,
@@ -219,11 +219,11 @@ loop(State=#state{parent=Parent, ref=Ref, conn_type=ConnType,
 			loop(State, CurConns, NbChildren, Sleepers)
 	end.
 
-handshake(State=#state{ref=Ref, transport=Transport, ack_timeout=AckTimeout, max_conns=MaxConns},
-		CurConns, NbChildren, Sleepers, To, Socket, SupPid, ProtocolPid) ->
+handshake(State=#state{ref=Ref, transport=Transport, handshake_timeout=HandshakeTimeout,
+		max_conns=MaxConns}, CurConns, NbChildren, Sleepers, To, Socket, SupPid, ProtocolPid) ->
 	case Transport:controlling_process(Socket, ProtocolPid) of
 		ok ->
-			ProtocolPid ! {handshake, Ref, Transport, Socket, AckTimeout},
+			ProtocolPid ! {handshake, Ref, Transport, Socket, HandshakeTimeout},
 			put(SupPid, active),
 			CurConns2 = CurConns + 1,
 			if CurConns2 < MaxConns ->
diff --git a/src/ranch_listener_sup.erl b/src/ranch_listener_sup.erl
index bd4ccc5..3853425 100644
--- a/src/ranch_listener_sup.erl
+++ b/src/ranch_listener_sup.erl
@@ -15,27 +15,27 @@
 -module(ranch_listener_sup).
 -behaviour(supervisor).
 
--export([start_link/6]).
+-export([start_link/5]).
 -export([init/1]).
 
--spec start_link(ranch:ref(), non_neg_integer(), module(), any(), module(), any())
+-spec start_link(ranch:ref(), module(), any(), module(), any())
 	-> {ok, pid()}.
-start_link(Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts) ->
-	MaxConns = proplists:get_value(max_connections, TransOpts, 1024),
+start_link(Ref, Transport, TransOpts, Protocol, ProtoOpts) ->
+	MaxConns = maps:get(max_connections, TransOpts, 1024),
 	ranch_server:set_new_listener_opts(Ref, MaxConns, TransOpts, ProtoOpts,
-		[Ref, NumAcceptors, Transport, TransOpts, Protocol, ProtoOpts]),
+		[Ref, Transport, TransOpts, Protocol, ProtoOpts]),
 	supervisor:start_link(?MODULE, {
-		Ref, NumAcceptors, Transport, Protocol
+		Ref, Transport, Protocol
 	}).
 
-init({Ref, NumAcceptors, Transport, Protocol}) ->
+init({Ref, Transport, Protocol}) ->
 	ok = ranch_server:set_listener_sup(Ref, self()),
 	ChildSpecs = [
 		{ranch_conns_sup, {ranch_conns_sup, start_link,
 				[Ref, Transport, Protocol]},
 			permanent, infinity, supervisor, [ranch_conns_sup]},
 		{ranch_acceptors_sup, {ranch_acceptors_sup, start_link,
-				[Ref, NumAcceptors, Transport]},
+				[Ref, Transport]},
 			permanent, infinity, supervisor, [ranch_acceptors_sup]}
 	],
 	{ok, {{rest_for_one, 1, 5}, ChildSpecs}}.