Make routing constraints use the fields format

This makes routing more in line with the rest of Cowboy and
allows us to use cowboy_constraints directly.

3 files changed, 36 insertions, 70 deletions

diff --git a/doc/src/guide/routing.ezdoc b/doc/src/guide/routing.ezdoc
index e7b43f2..2482c12 100644
--- a/doc/src/guide/routing.ezdoc
+++ b/doc/src/guide/routing.ezdoc
@@ -191,26 +191,16 @@ HostMatch = "*".
 After the matching has completed, the resulting bindings can be tested
 against a set of constraints. Constraints are only tested when the
 binding is defined. They run in the order you defined them. The match
-will succeed only if they all succeed.
+will succeed only if they all succeed. If the match fails, then Cowboy
+tries the next route in the list.
 
-They are always given as a two or three elements tuple, where the first
-element is the name of the binding, the second element is the constraint's
-name, and the optional third element is the constraint's arguments.
+The format used for constraints is the same as match functions in
+`cowboy_req`: they are provided as a list of fields which may have
+one or more constraints. While the router accepts the same format,
+it will skip fields with no constraints and will also ignore default
+values, if any.
 
-The following constraints are currently defined:
-
-* {Name, int}
-* {Name, function, fun ((Value) -> true | {true, NewValue} | false)}
-
-The `int` constraint will check if the binding is a binary string
-representing an integer, and if it is, will convert the value to integer.
-
-The `function` constraint will pass the binding value to a user specified
-function that receives the binary value as its only argument and must
-return whether it fulfills the constraint, optionally modifying the value.
-The value thus returned can be of any type.
-
-Note that constraint functions SHOULD be pure and MUST NOT crash.
+Read more about ^constraints^.
 
 :: Compilation
 
diff --git a/doc/src/manual/cowboy_router.ezdoc b/doc/src/manual/cowboy_router.ezdoc
index f76acf6..8d45e67 100644
--- a/doc/src/manual/cowboy_router.ezdoc
+++ b/doc/src/manual/cowboy_router.ezdoc
@@ -22,31 +22,16 @@ Environment output:
 
 List of bindings found during routing.
 
-: constraints() = [IntConstraint | FunConstraint]
-
-Types:
-
-* IntConstraint = {atom(), int}
-* FunConstraint = {atom(), function, Fun}
-* Fun = fun((binary()) -> true | {true, any()} | false)
-
-List of constraints to apply to the bindings.
-
-The int constraint will convert the binding to an integer.
-The fun constraint allows writing custom code for checking
-the bindings. Returning a new value from that fun allows
-replacing the current binding with a new value.
-
 : dispatch_rules() - opaque to the user
 
 Rules for dispatching request used by Cowboy.
 
-: routes() = [{Host, Paths} | {Host, constraints(), Paths}]
+: routes() = [{Host, Paths} | {Host, cowboy:fields(), Paths}]
 
 Types:
 
 * Host = Path = '_' | iodata()
-* Paths = [{Path, Handler, Opts} | {Path, constraints(), Handler, Opts}]
+* Paths = [{Path, Handler, Opts} | {Path, cowboy:fields(), Handler, Opts}]
 * Handler = module()
 * Opts = any()
 
diff --git a/src/cowboy_router.erl b/src/cowboy_router.erl
index f5b2f26..3b71205 100644
--- a/src/cowboy_router.erl
+++ b/src/cowboy_router.erl
@@ -33,21 +33,17 @@
 -export_type([bindings/0]).
 -export_type([tokens/0]).
 
--type constraints() :: [{atom(), int}
-	| {atom(), function, fun ((binary()) -> true | {true, any()} | false)}].
--export_type([constraints/0]).
-
 -type route_match() :: '_' | iodata().
 -type route_path() :: {Path::route_match(), Handler::module(), Opts::any()}
-	| {Path::route_match(), constraints(), Handler::module(), Opts::any()}.
+	| {Path::route_match(), cowboy:fields(), Handler::module(), Opts::any()}.
 -type route_rule() :: {Host::route_match(), Paths::[route_path()]}
-	| {Host::route_match(), constraints(), Paths::[route_path()]}.
+	| {Host::route_match(), cowboy:fields(), Paths::[route_path()]}.
 -type routes() :: [route_rule()].
 -export_type([routes/0]).
 
 -type dispatch_match() :: '_' | <<_:8>> | [binary() | '_' | '...' | atom()].
--type dispatch_path() :: {dispatch_match(), module(), any()}.
--type dispatch_rule() :: {Host::dispatch_match(), Paths::[dispatch_path()]}.
+-type dispatch_path() :: {dispatch_match(), cowboy:fields(), module(), any()}.
+-type dispatch_rule() :: {Host::dispatch_match(), cowboy:fields(), Paths::[dispatch_path()]}.
 -opaque dispatch_rules() :: [dispatch_rule()].
 -export_type([dispatch_rules/0]).
 
@@ -59,15 +55,15 @@ compile([], Acc) ->
 	lists:reverse(Acc);
 compile([{Host, Paths}|Tail], Acc) ->
 	compile([{Host, [], Paths}|Tail], Acc);
-compile([{HostMatch, Constraints, Paths}|Tail], Acc) ->
+compile([{HostMatch, Fields, Paths}|Tail], Acc) ->
 	HostRules = case HostMatch of
 		'_' -> '_';
 		_ -> compile_host(HostMatch)
 	end,
 	PathRules = compile_paths(Paths, []),
 	Hosts = case HostRules of
-		'_' -> [{'_', Constraints, PathRules}];
-		_ -> [{R, Constraints, PathRules} || R <- HostRules]
+		'_' -> [{'_', Fields, PathRules}];
+		_ -> [{R, Fields, PathRules} || R <- HostRules]
 	end,
 	compile(Tail, Hosts ++ Acc).
 
@@ -80,16 +76,16 @@ compile_paths([], Acc) ->
 	lists:reverse(Acc);
 compile_paths([{PathMatch, Handler, Opts}|Tail], Acc) ->
 	compile_paths([{PathMatch, [], Handler, Opts}|Tail], Acc);
-compile_paths([{PathMatch, Constraints, Handler, Opts}|Tail], Acc)
+compile_paths([{PathMatch, Fields, Handler, Opts}|Tail], Acc)
 		when is_list(PathMatch) ->
 	compile_paths([{iolist_to_binary(PathMatch),
-		Constraints, Handler, Opts}|Tail], Acc);
-compile_paths([{'_', Constraints, Handler, Opts}|Tail], Acc) ->
-	compile_paths(Tail, [{'_', Constraints, Handler, Opts}] ++ Acc);
-compile_paths([{<< $/, PathMatch/binary >>, Constraints, Handler, Opts}|Tail],
+		Fields, Handler, Opts}|Tail], Acc);
+compile_paths([{'_', Fields, Handler, Opts}|Tail], Acc) ->
+	compile_paths(Tail, [{'_', Fields, Handler, Opts}] ++ Acc);
+compile_paths([{<< $/, PathMatch/binary >>, Fields, Handler, Opts}|Tail],
 		Acc) ->
 	PathRules = compile_rules(PathMatch, $/, [], [], <<>>),
-	Paths = [{lists:reverse(R), Constraints, Handler, Opts} || R <- PathRules],
+	Paths = [{lists:reverse(R), Fields, Handler, Opts} || R <- PathRules],
 	compile_paths(Tail, Paths ++ Acc);
 compile_paths([{PathMatch, _, _, _}|_], _) ->
 	error({badarg, "The following route MUST begin with a slash: "
@@ -220,7 +216,7 @@ match([], _, _) ->
 %% If the host is '_' then there can be no constraints.
 match([{'_', [], PathMatchs}|_Tail], _, Path) ->
 	match_path(PathMatchs, undefined, Path, []);
-match([{HostMatch, Constraints, PathMatchs}|Tail], Tokens, Path)
+match([{HostMatch, Fields, PathMatchs}|Tail], Tokens, Path)
 		when is_list(Tokens) ->
 	case list_match(Tokens, HostMatch, []) of
 		false ->
@@ -230,7 +226,7 @@ match([{HostMatch, Constraints, PathMatchs}|Tail], Tokens, Path)
 				undefined -> undefined;
 				_ -> lists:reverse(HostInfo)
 			end,
-			case check_constraints(Constraints, Bindings) of
+			case check_constraints(Fields, Bindings) of
 				{ok, Bindings2} ->
 					match_path(PathMatchs, HostInfo2, Path, Bindings2);
 				nomatch ->
@@ -251,15 +247,15 @@ match_path([], _, _, _) ->
 %% If the path is '_' then there can be no constraints.
 match_path([{'_', [], Handler, Opts}|_Tail], HostInfo, _, Bindings) ->
 	{ok, Handler, Opts, Bindings, HostInfo, undefined};
-match_path([{<<"*">>, _Constraints, Handler, Opts}|_Tail], HostInfo, <<"*">>, Bindings) ->
+match_path([{<<"*">>, _, Handler, Opts}|_Tail], HostInfo, <<"*">>, Bindings) ->
 	{ok, Handler, Opts, Bindings, HostInfo, undefined};
-match_path([{PathMatch, Constraints, Handler, Opts}|Tail], HostInfo, Tokens,
+match_path([{PathMatch, Fields, Handler, Opts}|Tail], HostInfo, Tokens,
 		Bindings) when is_list(Tokens) ->
 	case list_match(Tokens, PathMatch, Bindings) of
 		false ->
 			match_path(Tail, HostInfo, Tokens, Bindings);
 		{true, PathBinds, PathInfo} ->
-			case check_constraints(Constraints, PathBinds) of
+			case check_constraints(Fields, PathBinds) of
 				{ok, PathBinds2} ->
 					{ok, Handler, Opts, PathBinds2, HostInfo, PathInfo};
 				nomatch ->
@@ -273,13 +269,16 @@ match_path(Dispatch, HostInfo, Path, Bindings) ->
 
 check_constraints([], Bindings) ->
 	{ok, Bindings};
-check_constraints([Constraint|Tail], Bindings) ->
-	Name = element(1, Constraint),
+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 check_constraint(Constraint, Value) of
+			Constraints = element(2, Field),
+			case cowboy_constraints:validate(Value, Constraints) of
 				true ->
 					check_constraints(Tail, Bindings);
 				{true, Value2} ->
@@ -291,13 +290,6 @@ check_constraints([Constraint|Tail], Bindings) ->
 			end
 	end.
 
-check_constraint({_, int}, Value) ->
-	try {true, list_to_integer(binary_to_list(Value))}
-	catch _:_ -> false
-	end;
-check_constraint({_, function, Fun}, Value) ->
-	Fun(Value).
-
 -spec split_host(binary()) -> tokens().
 split_host(Host) ->
 	split_host(Host, []).
@@ -540,9 +532,8 @@ match_constraints_test() ->
 		<<"ninenines.eu">>, <<"/path/123/">>),
 	{error, notfound, path} = match(Dispatch,
 		<<"ninenines.eu">>, <<"/path/NaN/">>),
-	Dispatch2 = [{'_', [],
-		[{[<<"path">>, username], [{username, function,
-		fun(Value) -> Value =:= cowboy_bstr:to_lower(Value) end}],
+	Dispatch2 = [{'_', [], [{[<<"path">>, username],
+		[{username, fun(Value) -> Value =:= cowboy_bstr:to_lower(Value) end}],
 		match, []}]}],
 	{ok, _, [], [{username, <<"essen">>}], _, _} = match(Dispatch2,
 		<<"ninenines.eu">>, <<"/path/essen">>),