Browse Source

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.
Loïc Hoguin 10 years ago
parent
commit
25259671f5
3 changed files with 36 additions and 70 deletions
  1. 8 18
      doc/src/guide/routing.ezdoc
  2. 2 17
      doc/src/manual/cowboy_router.ezdoc
  3. 26 35
      src/cowboy_router.erl

+ 8 - 18
doc/src/guide/routing.ezdoc

@@ -191,26 +191,16 @@ HostMatch = "*".
 After the matching has completed, the resulting bindings can be tested
 After the matching has completed, the resulting bindings can be tested
 against a set of constraints. Constraints are only tested when the
 against a set of constraints. Constraints are only tested when the
 binding is defined. They run in the order you defined them. The match
 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
 :: Compilation
 
 

+ 2 - 17
doc/src/manual/cowboy_router.ezdoc

@@ -22,31 +22,16 @@ Environment output:
 
 
 List of bindings found during routing.
 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
 : dispatch_rules() - opaque to the user
 
 
 Rules for dispatching request used by Cowboy.
 Rules for dispatching request used by Cowboy.
 
 
-: routes() = [{Host, Paths} | {Host, constraints(), Paths}]
+: routes() = [{Host, Paths} | {Host, cowboy:fields(), Paths}]
 
 
 Types:
 Types:
 
 
 * Host = Path = '_' | iodata()
 * Host = Path = '_' | iodata()
-* Paths = [{Path, Handler, Opts} | {Path, constraints(), Handler, Opts}]
+* Paths = [{Path, Handler, Opts} | {Path, cowboy:fields(), Handler, Opts}]
 * Handler = module()
 * Handler = module()
 * Opts = any()
 * Opts = any()
 
 

+ 26 - 35
src/cowboy_router.erl

@@ -33,21 +33,17 @@
 -export_type([bindings/0]).
 -export_type([bindings/0]).
 -export_type([tokens/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_match() :: '_' | iodata().
 -type route_path() :: {Path::route_match(), Handler::module(), Opts::any()}
 -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()]}
 -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()].
 -type routes() :: [route_rule()].
 -export_type([routes/0]).
 -export_type([routes/0]).
 
 
 -type dispatch_match() :: '_' | <<_:8>> | [binary() | '_' | '...' | atom()].
 -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()].
 -opaque dispatch_rules() :: [dispatch_rule()].
 -export_type([dispatch_rules/0]).
 -export_type([dispatch_rules/0]).
 
 
@@ -59,15 +55,15 @@ compile([], Acc) ->
 	lists:reverse(Acc);
 	lists:reverse(Acc);
 compile([{Host, Paths}|Tail], Acc) ->
 compile([{Host, Paths}|Tail], 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
 	HostRules = case HostMatch of
 		'_' -> '_';
 		'_' -> '_';
 		_ -> compile_host(HostMatch)
 		_ -> compile_host(HostMatch)
 	end,
 	end,
 	PathRules = compile_paths(Paths, []),
 	PathRules = compile_paths(Paths, []),
 	Hosts = case HostRules of
 	Hosts = case HostRules of
-		'_' -> [{'_', Constraints, PathRules}];
-		_ -> [{R, Constraints, PathRules} || R <- HostRules]
+		'_' -> [{'_', Fields, PathRules}];
+		_ -> [{R, Fields, PathRules} || R <- HostRules]
 	end,
 	end,
 	compile(Tail, Hosts ++ Acc).
 	compile(Tail, Hosts ++ Acc).
 
 
@@ -80,16 +76,16 @@ compile_paths([], Acc) ->
 	lists:reverse(Acc);
 	lists:reverse(Acc);
 compile_paths([{PathMatch, Handler, Opts}|Tail], Acc) ->
 compile_paths([{PathMatch, Handler, Opts}|Tail], 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) ->
 		when is_list(PathMatch) ->
 	compile_paths([{iolist_to_binary(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) ->
 		Acc) ->
 	PathRules = compile_rules(PathMatch, $/, [], [], <<>>),
 	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(Tail, Paths ++ Acc);
 compile_paths([{PathMatch, _, _, _}|_], _) ->
 compile_paths([{PathMatch, _, _, _}|_], _) ->
 	error({badarg, "The following route MUST begin with a slash: "
 	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.
 %% If the host is '_' then there can be no constraints.
 match([{'_', [], PathMatchs}|_Tail], _, Path) ->
 match([{'_', [], PathMatchs}|_Tail], _, Path) ->
 	match_path(PathMatchs, undefined, Path, []);
 	match_path(PathMatchs, undefined, Path, []);
-match([{HostMatch, Constraints, PathMatchs}|Tail], Tokens, Path)
+match([{HostMatch, Fields, PathMatchs}|Tail], Tokens, Path)
 		when is_list(Tokens) ->
 		when is_list(Tokens) ->
 	case list_match(Tokens, HostMatch, []) of
 	case list_match(Tokens, HostMatch, []) of
 		false ->
 		false ->
@@ -230,7 +226,7 @@ match([{HostMatch, Constraints, PathMatchs}|Tail], Tokens, Path)
 				undefined -> undefined;
 				undefined -> undefined;
 				_ -> lists:reverse(HostInfo)
 				_ -> lists:reverse(HostInfo)
 			end,
 			end,
-			case check_constraints(Constraints, Bindings) of
+			case check_constraints(Fields, Bindings) of
 				{ok, Bindings2} ->
 				{ok, Bindings2} ->
 					match_path(PathMatchs, HostInfo2, Path, Bindings2);
 					match_path(PathMatchs, HostInfo2, Path, Bindings2);
 				nomatch ->
 				nomatch ->
@@ -251,15 +247,15 @@ match_path([], _, _, _) ->
 %% If the path is '_' then there can be no constraints.
 %% If the path is '_' then there can be no constraints.
 match_path([{'_', [], Handler, Opts}|_Tail], HostInfo, _, Bindings) ->
 match_path([{'_', [], Handler, Opts}|_Tail], HostInfo, _, Bindings) ->
 	{ok, Handler, Opts, Bindings, HostInfo, undefined};
 	{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};
 	{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) ->
 		Bindings) when is_list(Tokens) ->
 	case list_match(Tokens, PathMatch, Bindings) of
 	case list_match(Tokens, PathMatch, Bindings) of
 		false ->
 		false ->
 			match_path(Tail, HostInfo, Tokens, Bindings);
 			match_path(Tail, HostInfo, Tokens, Bindings);
 		{true, PathBinds, PathInfo} ->
 		{true, PathBinds, PathInfo} ->
-			case check_constraints(Constraints, PathBinds) of
+			case check_constraints(Fields, PathBinds) of
 				{ok, PathBinds2} ->
 				{ok, PathBinds2} ->
 					{ok, Handler, Opts, PathBinds2, HostInfo, PathInfo};
 					{ok, Handler, Opts, PathBinds2, HostInfo, PathInfo};
 				nomatch ->
 				nomatch ->
@@ -273,13 +269,16 @@ match_path(Dispatch, HostInfo, Path, Bindings) ->
 
 
 check_constraints([], Bindings) ->
 check_constraints([], Bindings) ->
 	{ok, 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
 	case lists:keyfind(Name, 1, Bindings) of
 		false ->
 		false ->
 			check_constraints(Tail, Bindings);
 			check_constraints(Tail, Bindings);
 		{_, Value} ->
 		{_, Value} ->
-			case check_constraint(Constraint, Value) of
+			Constraints = element(2, Field),
+			case cowboy_constraints:validate(Value, Constraints) of
 				true ->
 				true ->
 					check_constraints(Tail, Bindings);
 					check_constraints(Tail, Bindings);
 				{true, Value2} ->
 				{true, Value2} ->
@@ -291,13 +290,6 @@ check_constraints([Constraint|Tail], Bindings) ->
 			end
 			end
 	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().
 -spec split_host(binary()) -> tokens().
 split_host(Host) ->
 split_host(Host) ->
 	split_host(Host, []).
 	split_host(Host, []).
@@ -540,9 +532,8 @@ match_constraints_test() ->
 		<<"ninenines.eu">>, <<"/path/123/">>),
 		<<"ninenines.eu">>, <<"/path/123/">>),
 	{error, notfound, path} = match(Dispatch,
 	{error, notfound, path} = match(Dispatch,
 		<<"ninenines.eu">>, <<"/path/NaN/">>),
 		<<"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, []}]}],
 		match, []}]}],
 	{ok, _, [], [{username, <<"essen">>}], _, _} = match(Dispatch2,
 	{ok, _, [], [{username, <<"essen">>}], _, _} = match(Dispatch2,
 		<<"ninenines.eu">>, <<"/path/essen">>),
 		<<"ninenines.eu">>, <<"/path/essen">>),