Browse Source

Change the type of bindings from a list to a map

Maps make more sense because the keys are unique.
Loïc Hoguin 8 years ago
parent
commit
9255cdf1d7

+ 1 - 6
doc/src/guide/req.asciidoc

@@ -197,8 +197,6 @@ And any other combination.
 
 === Bindings
 
-// @todo Bindings should probably be a map themselves.
-
 Bindings are the host and path components that you chose
 to extract when defining the routes of your application.
 They are only available after the routing.
@@ -222,10 +220,7 @@ To retrieve everything that was bound:
 [source,erlang]
 Bindings = cowboy_req:bindings(Req).
 
-They are returned as a list of key/value pairs, with
-keys being atoms.
-
-// ...
+They are returned as a map, with keys being atoms.
 
 The Cowboy router also allows you to capture many host
 or path segments at once using the `...` qualifier.

+ 3 - 3
doc/src/manual/cowboy_req.bindings.asciidoc

@@ -8,10 +8,10 @@ cowboy_req:bindings - Access all values bound from the route
 
 [source,erlang]
 ----
-bindings(Req :: cowboy_req:req()) -> [{Name :: atom(), any()}]
+bindings(Req :: cowboy_req:req()) -> cowboy_router:bindings()
 ----
 
-Return all bindings as a list of key/value pairs.
+Return a map containing all bindings.
 
 == Arguments
 
@@ -27,7 +27,7 @@ automatically converting numbers to integer).
 
 == Changelog
 
-* *2.0*: Only the values are returned, it is no longer wrapped in a tuple.
+* *2.0*: Only the values are returned, they are no longer wrapped in a tuple.
 * *1.0*: Function introduced.
 
 == Examples

+ 1 - 1
doc/src/manual/cowboy_router.asciidoc

@@ -33,7 +33,7 @@ a 404 response otherwise.
 
 [source,erlang]
 ----
-bindings() :: [{atom(), binary()}]
+bindings() :: #{atom() => any()}
 ----
 
 Bindings found during routing.

+ 5 - 5
src/cowboy_req.erl

@@ -326,18 +326,18 @@ binding(Name, Req) ->
 
 -spec binding(atom(), req(), Default) -> any() | Default when Default::any().
 binding(Name, #{bindings := Bindings}, Default) when is_atom(Name) ->
-	case lists:keyfind(Name, 1, Bindings) of
-		{_, Value} -> Value;
-		false -> Default
+	case Bindings of
+		#{Name := Value} -> Value;
+		_ -> Default
 	end;
 binding(Name, _, Default) when is_atom(Name) ->
 	Default.
 
--spec bindings(req()) -> [{atom(), any()}].
+-spec bindings(req()) -> cowboy_router:bindings().
 bindings(#{bindings := Bindings}) ->
 	Bindings;
 bindings(_) ->
-	[].
+	#{}.
 
 -spec header(binary(), req()) -> binary() | undefined.
 header(Name, Req) ->

+ 14 - 16
src/cowboy_router.erl

@@ -28,7 +28,7 @@
 -export([compile/1]).
 -export([execute/2]).
 
--type bindings() :: [{atom(), binary()}].
+-type bindings() :: #{atom() => any()}.
 -type tokens() :: [binary()].
 -export_type([bindings/0]).
 -export_type([tokens/0]).
@@ -218,10 +218,10 @@ match([], _, _) ->
 	{error, notfound, host};
 %% If the host is '_' then there can be no constraints.
 match([{'_', [], PathMatchs}|_Tail], _, Path) ->
-	match_path(PathMatchs, undefined, Path, []);
+	match_path(PathMatchs, undefined, Path, #{});
 match([{HostMatch, Fields, PathMatchs}|Tail], Tokens, Path)
 		when is_list(Tokens) ->
-	case list_match(Tokens, HostMatch, []) of
+	case list_match(Tokens, HostMatch, #{}) of
 		false ->
 			match(Tail, Tokens, Path);
 		{true, Bindings, HostInfo} ->
@@ -276,21 +276,19 @@ check_constraints([Field|Tail], Bindings) when is_atom(Field) ->
 	check_constraints(Tail, Bindings);
 check_constraints([Field|Tail], Bindings) ->
 	Name = element(1, Field),
-	case lists:keyfind(Name, 1, Bindings) of
-		false ->
-			check_constraints(Tail, Bindings);
-		{_, Value} ->
+	case Bindings of
+		#{Name := Value} ->
 			Constraints = element(2, Field),
 			case cowboy_constraints:validate(Value, Constraints) of
 				true ->
 					check_constraints(Tail, Bindings);
 				{true, Value2} ->
-					Bindings2 = lists:keyreplace(Name, 1, Bindings,
-						{Name, Value2}),
-					check_constraints(Tail, Bindings2);
+					check_constraints(Tail, Bindings#{Name => Value2});
 				false ->
 					nomatch
-			end
+			end;
+		_ ->
+			check_constraints(Tail, Bindings)
 	end.
 
 -spec split_host(binary()) -> tokens().
@@ -369,13 +367,13 @@ list_match([E|Tail], [E|TailMatch], Binds) ->
 %% Bind E to the variable name V and continue,
 %% unless V was already defined and E isn't identical to the previous value.
 list_match([E|Tail], [V|TailMatch], Binds) when is_atom(V) ->
-	case lists:keyfind(V, 1, Binds) of
-		{_, E} ->
+	case Binds of
+		#{V := E} ->
 			list_match(Tail, TailMatch, Binds);
-		{_, _} ->
+		#{V := _} ->
 			false;
-		false ->
-			list_match(Tail, TailMatch, [{V, E}|Binds])
+		_ ->
+			list_match(Tail, TailMatch, Binds#{V => E})
 	end;
 %% Match complete.
 list_match([], [], Binds) ->

+ 1 - 2
test/req_SUITE.erl

@@ -114,10 +114,9 @@ binding(Config) ->
 	<<"default">> = do_get_body("/args/binding/undefined/default", Config),
 	ok.
 
-%% @todo Do we really want a key/value list here instead of a map?
 bindings(Config) ->
 	doc("Values bound from request URI path."),
-	<<"[{key,<<\"bindings\">>}]">> = do_get_body("/bindings", Config),
+	<<"#{key => <<\"bindings\">>}">> = do_get_body("/bindings", Config),
 	ok.
 
 header(Config) ->