Skip to content

Commit

Permalink
Change the type of bindings from a list to a map
Browse files Browse the repository at this point in the history
Maps make more sense because the keys are unique.
  • Loading branch information
essen committed Feb 19, 2017
1 parent 91ae70b commit 9255cdf
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 33 deletions.
7 changes: 1 addition & 6 deletions doc/src/guide/req.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions doc/src/manual/cowboy_req.bindings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion doc/src/manual/cowboy_router.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ a 404 response otherwise.

[source,erlang]
----
bindings() :: [{atom(), binary()}]
bindings() :: #{atom() => any()}
----

Bindings found during routing.
Expand Down
10 changes: 5 additions & 5 deletions src/cowboy_req.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down
30 changes: 14 additions & 16 deletions src/cowboy_router.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -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} ->
Expand Down Expand Up @@ -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().
Expand Down Expand Up @@ -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) ->
Expand Down
3 changes: 1 addition & 2 deletions test/req_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down

0 comments on commit 9255cdf

Please sign in to comment.