aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2017-06-28 17:38:17 +0200
committerLoïc Hoguin <[email protected]>2017-06-28 17:38:17 +0200
commitc22173037134becbac882533f80339f4127a8ad2 (patch)
tree666fc6e31c9e02766eb33ecadc5068dd54e18aa4 /src
parent3eb7693e4fd7e1172b55b30ab25c5656040d1fd2 (diff)
downloadcowboy-c22173037134becbac882533f80339f4127a8ad2.tar.gz
cowboy-c22173037134becbac882533f80339f4127a8ad2.tar.bz2
cowboy-c22173037134becbac882533f80339f4127a8ad2.zip
Improve the interface for constraints
There are two important changes in this commit. Constraints are now producing an error tuple. This error tuple in turn can be provided to a function for formatting a human readable error message. Both the error tuple and the formatting code are controlled by and part of the constraint function. Constraints now also implement the reverse operation. When constraint functions only validate, the reverse operation will be the same as the forward operation. When they also do some conversion then the reverse operation will reverse it. Since constraints are now performing 3 different operations (forward, reverse and format_error), they now take the form of a function accepting two separate arguments. The operation is the first argument. In addition, the return value was changed to take the form of {ok, Value} | {error, Reason}. The value must be returned as-is if it was not modified.
Diffstat (limited to 'src')
-rw-r--r--src/cowboy_constraints.erl175
-rw-r--r--src/cowboy_req.erl12
-rw-r--r--src/cowboy_router.erl12
3 files changed, 155 insertions, 44 deletions
diff --git a/src/cowboy_constraints.erl b/src/cowboy_constraints.erl
index 27c0e72..6b468fe 100644
--- a/src/cowboy_constraints.erl
+++ b/src/cowboy_constraints.erl
@@ -15,47 +15,160 @@
-module(cowboy_constraints).
-export([validate/2]).
+-export([reverse/2]).
+-export([format_error/1]).
-type constraint() :: int | nonempty | fun().
-export_type([constraint/0]).
--spec validate(binary(), [constraint()]) -> true | {true, any()} | false.
-validate(Value, [Constraint]) ->
- apply_constraint(Value, Constraint);
+-type reason() :: {constraint(), any(), any()}.
+-export_type([reason/0]).
+
+-spec validate(binary(), constraint() | [constraint()])
+ -> {ok, any()} | {error, reason()}.
validate(Value, Constraints) when is_list(Constraints) ->
- validate_list(Value, Constraints, original);
+ apply_list(forward, Value, Constraints);
validate(Value, Constraint) ->
- apply_constraint(Value, Constraint).
-
-validate_list(_, [], original) ->
- true;
-validate_list(Value, [], modified) ->
- {true, Value};
-validate_list(Value, [Constraint|Tail], State) ->
- case apply_constraint(Value, Constraint) of
- true ->
- validate_list(Value, Tail, State);
- {true, Value2} ->
- validate_list(Value2, Tail, modified);
- false ->
- false
+ apply_list(forward, Value, [Constraint]).
+
+-spec reverse(any(), constraint() | [constraint()])
+ -> {ok, binary()} | {error, reason()}.
+reverse(Value, Constraints) when is_list(Constraints) ->
+ apply_list(reverse, Value, Constraints);
+reverse(Value, Constraint) ->
+ apply_list(reverse, Value, [Constraint]).
+
+-spec format_error(reason()) -> iodata().
+format_error({Constraint, Reason, Value}) ->
+ apply_constraint(format_error, {Reason, Value}, Constraint).
+
+apply_list(_, Value, []) ->
+ {ok, Value};
+apply_list(Type, Value0, [Constraint|Tail]) ->
+ case apply_constraint(Type, Value0, Constraint) of
+ {ok, Value} ->
+ apply_list(Type, Value, Tail);
+ {error, Reason} ->
+ {error, {Constraint, Reason, Value0}}
end.
%% @todo {int, From, To}, etc.
-apply_constraint(Value, int) ->
- int(Value);
-apply_constraint(Value, nonempty) ->
- nonempty(Value);
-apply_constraint(Value, F) when is_function(F) ->
- F(Value).
+apply_constraint(Type, Value, int) ->
+ int(Type, Value);
+apply_constraint(Type, Value, nonempty) ->
+ nonempty(Type, Value);
+apply_constraint(Type, Value, F) when is_function(F) ->
+ F(Type, Value).
%% Constraint functions.
-int(Value) when is_binary(Value) ->
- try {true, binary_to_integer(Value)}
- catch _:_ -> false
- end.
+int(forward, Value) ->
+ try
+ {ok, binary_to_integer(Value)}
+ catch _:_ ->
+ {error, not_an_integer}
+ end;
+int(reverse, Value) ->
+ try
+ {ok, integer_to_binary(Value)}
+ catch _:_ ->
+ {error, not_an_integer}
+ end;
+int(format_error, {not_an_integer, Value}) ->
+ io_lib:format("The value ~p is not an integer.", [Value]).
+
+nonempty(Type, <<>>) when Type =/= format_error ->
+ {error, not_empty};
+nonempty(Type, Value) when Type =/= format_error, is_binary(Value) ->
+ {ok, Value};
+nonempty(format_error, {not_empty, Value}) ->
+ io_lib:format("The value ~p is not empty.", [Value]).
+
+-ifdef(TEST).
+
+validate_test() ->
+ F = fun(_, Value) ->
+ try
+ {ok, binary_to_atom(Value, latin1)}
+ catch _:_ ->
+ {error, not_a_binary}
+ end
+ end,
+ %% Value, Constraints, Result.
+ Tests = [
+ {<<>>, [], <<>>},
+ {<<"123">>, int, 123},
+ {<<"123">>, [int], 123},
+ {<<"123">>, [nonempty, int], 123},
+ {<<"123">>, [int, nonempty], 123},
+ {<<>>, nonempty, error},
+ {<<>>, [nonempty], error},
+ {<<"hello">>, F, hello},
+ {<<"hello">>, [F], hello},
+ {<<"123">>, [F, int], error},
+ {<<"123">>, [int, F], error},
+ {<<"hello">>, [nonempty, F], hello},
+ {<<"hello">>, [F, nonempty], hello}
+ ],
+ [{lists:flatten(io_lib:format("~p, ~p", [V, C])), fun() ->
+ case R of
+ error -> {error, _} = validate(V, C);
+ _ -> {ok, R} = validate(V, C)
+ end
+ end} || {V, C, R} <- Tests].
+
+reverse_test() ->
+ F = fun(_, Value) ->
+ try
+ {ok, atom_to_binary(Value, latin1)}
+ catch _:_ ->
+ {error, not_an_atom}
+ end
+ end,
+ %% Value, Constraints, Result.
+ Tests = [
+ {<<>>, [], <<>>},
+ {123, int, <<"123">>},
+ {123, [int], <<"123">>},
+ {123, [nonempty, int], <<"123">>},
+ {123, [int, nonempty], <<"123">>},
+ {<<>>, nonempty, error},
+ {<<>>, [nonempty], error},
+ {hello, F, <<"hello">>},
+ {hello, [F], <<"hello">>},
+ {123, [F, int], error},
+ {123, [int, F], error},
+ {hello, [nonempty, F], <<"hello">>},
+ {hello, [F, nonempty], <<"hello">>}
+ ],
+ [{lists:flatten(io_lib:format("~p, ~p", [V, C])), fun() ->
+ case R of
+ error -> {error, _} = reverse(V, C);
+ _ -> {ok, R} = reverse(V, C)
+ end
+ end} || {V, C, R} <- Tests].
+
+int_format_error_test() ->
+ {error, Reason} = validate(<<"string">>, int),
+ Bin = iolist_to_binary(format_error(Reason)),
+ true = is_binary(Bin),
+ ok.
+
+nonempty_format_error_test() ->
+ {error, Reason} = validate(<<>>, nonempty),
+ Bin = iolist_to_binary(format_error(Reason)),
+ true = is_binary(Bin),
+ ok.
+
+fun_format_error_test() ->
+ F = fun
+ (format_error, {test, <<"value">>}) ->
+ formatted;
+ (_, _) ->
+ {error, test}
+ end,
+ {error, Reason} = validate(<<"value">>, F),
+ formatted = format_error(Reason),
+ ok.
-nonempty(<<>>) -> false;
-nonempty(Value) when is_binary(Value) -> true.
-%% @todo Perhaps return true for any other type except empty list?
+-endif.
diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index ab77a68..3eca379 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -780,10 +780,10 @@ filter([Key|Tail], Map) ->
true = maps:is_key(Key, Map),
filter(Tail, Map).
-filter_constraints(Tail, Map, Key, Value, Constraints) ->
- case cowboy_constraints:validate(Value, Constraints) of
- true ->
- filter(Tail, Map);
- {true, Value2} ->
- filter(Tail, Map#{Key => Value2})
+filter_constraints(Tail, Map, Key, Value0, Constraints) ->
+ case cowboy_constraints:validate(Value0, Constraints) of
+ {ok, Value} ->
+ filter(Tail, Map#{Key => Value});
+ {error, Reason} ->
+ exit(Reason)
end.
diff --git a/src/cowboy_router.erl b/src/cowboy_router.erl
index 7e70025..53ead4a 100644
--- a/src/cowboy_router.erl
+++ b/src/cowboy_router.erl
@@ -277,14 +277,12 @@ check_constraints([Field|Tail], Bindings) when is_atom(Field) ->
check_constraints([Field|Tail], Bindings) ->
Name = element(1, Field),
case Bindings of
- #{Name := Value} ->
+ #{Name := Value0} ->
Constraints = element(2, Field),
- case cowboy_constraints:validate(Value, Constraints) of
- true ->
- check_constraints(Tail, Bindings);
- {true, Value2} ->
- check_constraints(Tail, Bindings#{Name => Value2});
- false ->
+ case cowboy_constraints:validate(Value0, Constraints) of
+ {ok, Value} ->
+ check_constraints(Tail, Bindings#{Name => Value});
+ {error, _} ->
nomatch
end;
_ ->