Просмотр исходного кода

refactor: for better test coverage

Zaiming (Stone) Shi 3 лет назад
Родитель
Сommit
a9c650da66

+ 4 - 4
apps/emqx/src/emqx_config.erl

@@ -413,8 +413,8 @@ check_config(SchemaMod, RawConf, Opts0) ->
     try
         do_check_config(SchemaMod, RawConf, Opts0)
     catch
-        throw:{Schema, Errors} ->
-            compact_errors(Schema, Errors)
+        throw:Errors:Stacktrace ->
+            throw(emqx_hocon:compact_errors(Errors, Stacktrace))
     end.
 
 %% HOCON tries to be very informative about all the detailed errors
@@ -425,8 +425,8 @@ compact_errors(Schema, [Error0 | More]) when is_map(Error0) ->
         case length(More) of
             0 ->
                 Error0;
-            _ ->
-                Error0#{unshown_errors => length(More)}
+            N ->
+                Error0#{unshown_errors => N}
         end,
     Error =
         case is_atom(Schema) of

+ 34 - 2
apps/emqx/src/emqx_hocon.erl

@@ -20,6 +20,7 @@
 -export([
     format_path/1,
     check/2,
+    compact_errors/2,
     format_error/1,
     format_error/2,
     make_schema/1
@@ -43,8 +44,8 @@ check(SchemaModule, Conf) when is_map(Conf) ->
     try
         {ok, hocon_tconf:check_plain(SchemaModule, Conf, Opts)}
     catch
-        throw:Reason ->
-            {error, Reason}
+        throw:Errors:Stacktrace ->
+            compact_errors(Errors, Stacktrace)
     end;
 check(SchemaModule, HoconText) ->
     case hocon:binary(HoconText, #{format => map}) of
@@ -90,3 +91,34 @@ iol(L) when is_list(L) -> L.
 
 no_stacktrace(Map) ->
     maps:without([stacktrace], Map).
+
+%% @doc HOCON tries to be very informative about all the detailed errors
+%% it's maybe too much when reporting to the user
+-spec compact_errors(any(), Stacktrace :: list()) -> {error, any()}.
+compact_errors({SchemaModule, Errors}, Stacktrace) ->
+    compact_errors(SchemaModule, Errors, Stacktrace).
+
+compact_errors(SchemaModule, [Error0 | More], _Stacktrace) when is_map(Error0) ->
+    Error1 =
+        case length(More) of
+            0 ->
+                Error0;
+            N ->
+                Error0#{unshown_errors_count => N}
+        end,
+    Error =
+        case is_atom(SchemaModule) of
+            true ->
+                Error1#{schema_module => SchemaModule};
+            false ->
+                Error1
+        end,
+    {error, Error};
+compact_errors(SchemaModule, Error, Stacktrace) ->
+    %% unexpected, we need the stacktrace reported, hence error
+    %% if this happens i'ts a bug in hocon_tconf
+    {error, #{
+        schema_module => SchemaModule,
+        exception => Error,
+        stacktrace => Stacktrace
+    }}.

+ 1 - 1
apps/emqx_authn/src/emqx_authn.erl

@@ -69,7 +69,7 @@ do_check_config(#{<<"mechanism">> := Mec0} = Config, Opts) ->
         false ->
             throw(#{error => unknown_authn_provider, which => Key});
         {_, ProviderModule} ->
-            hocon_tconf:check_plain(
+            emqx_hocon:check(
                 ProviderModule,
                 #{?CONF_NS_BINARY => Config},
                 Opts#{atom_key => true}

+ 3 - 6
apps/emqx_authn/src/emqx_authn_schema.erl

@@ -105,13 +105,10 @@ select_union_member(Value, _Providers) ->
     throw(#{reason => "not_a_struct", value => Value}).
 
 try_select_union_member(Module, Value) ->
-    %% some modules have refs/1 exported to help selectin the sub-types
+    %% some modules have union_member_selector/1 exported to help selectin the sub-types
     %% emqx_authn_http, emqx_authn_jwt, emqx_authn_mongodb and emqx_authn_redis
-    try Module:refs(Value) of
-        {ok, Type} ->
-            [Type];
-        {error, Reason} ->
-            throw(Reason)
+    try
+        Module:union_member_selector({value, Value})
     catch
         error:undef ->
             %% otherwise expect only one member from this module

+ 11 - 6
apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -40,7 +40,7 @@
 
 -export([
     refs/0,
-    refs/1,
+    union_member_selector/1,
     create/2,
     update/2,
     authenticate/2,
@@ -60,7 +60,7 @@ roots() ->
     [
         {?CONF_NS,
             hoconsc:mk(
-                hoconsc:union(refs()),
+                hoconsc:union(fun union_member_selector/1),
                 #{}
             )}
     ].
@@ -160,15 +160,20 @@ refs() ->
         hoconsc:ref(?MODULE, post)
     ].
 
+union_member_selector(all_union_members) ->
+    refs();
+union_member_selector({value, Value}) ->
+    refs(Value).
+
 refs(#{<<"method">> := <<"get">>}) ->
-    {ok, hoconsc:ref(?MODULE, get)};
+    [hoconsc:ref(?MODULE, get)];
 refs(#{<<"method">> := <<"post">>}) ->
-    {ok, hoconsc:ref(?MODULE, post)};
+    [hoconsc:ref(?MODULE, post)];
 refs(_) ->
-    {error, #{
+    throw(#{
         field_name => method,
         expected => "get | post"
-    }}.
+    }).
 
 create(_AuthenticatorID, Config) ->
     create(Config).

+ 10 - 8
apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl

@@ -32,7 +32,7 @@
 
 -export([
     refs/0,
-    refs/1,
+    union_member_selector/1,
     create/2,
     update/2,
     authenticate/2,
@@ -52,7 +52,7 @@ roots() ->
     [
         {?CONF_NS,
             hoconsc:mk(
-                hoconsc:union(refs()),
+                hoconsc:union(fun union_member_selector/1),
                 #{}
             )}
     ].
@@ -169,7 +169,9 @@ refs() ->
         hoconsc:ref(?MODULE, 'jwks')
     ].
 
-refs(#{<<"mechanism">> := <<"jwt">>} = V) ->
+union_member_selector(all_union_members) ->
+    refs();
+union_member_selector({value, V}) ->
     UseJWKS = maps:get(<<"use_jwks">>, V, undefined),
     select_ref(boolean(UseJWKS), V).
 
@@ -181,16 +183,16 @@ boolean(<<"false">>) -> false;
 boolean(Other) -> Other.
 
 select_ref(true, _) ->
-    {ok, hoconsc:ref(?MODULE, 'jwks')};
+    [hoconsc:ref(?MODULE, 'jwks')];
 select_ref(false, #{<<"public_key">> := _}) ->
-    {ok, hoconsc:ref(?MODULE, 'public-key')};
+    [hoconsc:ref(?MODULE, 'public-key')];
 select_ref(false, _) ->
-    {ok, hoconsc:ref(?MODULE, 'hmac-based')};
+    [hoconsc:ref(?MODULE, 'hmac-based')];
 select_ref(_, _) ->
-    {error, #{
+    throw(#{
         field_name => use_jwks,
         expected => "true | false"
-    }}.
+    }).
 
 create(_AuthenticatorID, Config) ->
     create(Config).

+ 19 - 14
apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl

@@ -33,7 +33,7 @@
 
 -export([
     refs/0,
-    refs/1,
+    union_member_selector/1,
     create/2,
     update/2,
     authenticate/2,
@@ -53,7 +53,7 @@ roots() ->
     [
         {?CONF_NS,
             hoconsc:mk(
-                hoconsc:union(refs()),
+                hoconsc:union(fun union_member_selector/1),
                 #{}
             )}
     ].
@@ -131,18 +131,6 @@ refs() ->
         hoconsc:ref(?MODULE, 'sharded-cluster')
     ].
 
-refs(#{<<"mongo_type">> := <<"single">>}) ->
-    {ok, hoconsc:ref(?MODULE, standalone)};
-refs(#{<<"mongo_type">> := <<"rs">>}) ->
-    {ok, hoconsc:ref(?MODULE, 'replica-set')};
-refs(#{<<"mongo_type">> := <<"sharded">>}) ->
-    {ok, hoconsc:ref(?MODULE, 'sharded-cluster')};
-refs(_) ->
-    {error, #{
-        field_name => mongo_type,
-        expected => "single | rs | sharded"
-    }}.
-
 create(_AuthenticatorID, Config) ->
     create(Config).
 
@@ -259,3 +247,20 @@ is_superuser(Doc, #{is_superuser_field := IsSuperuserField}) ->
     emqx_authn_utils:is_superuser(#{<<"is_superuser">> => IsSuperuser});
 is_superuser(_, _) ->
     emqx_authn_utils:is_superuser(#{<<"is_superuser">> => false}).
+
+union_member_selector(all_union_members) ->
+    refs();
+union_member_selector({value, Value}) ->
+    refs(Value).
+
+refs(#{<<"mongo_type">> := <<"single">>}) ->
+    [hoconsc:ref(?MODULE, standalone)];
+refs(#{<<"mongo_type">> := <<"rs">>}) ->
+    [hoconsc:ref(?MODULE, 'replica-set')];
+refs(#{<<"mongo_type">> := <<"sharded">>}) ->
+    [hoconsc:ref(?MODULE, 'sharded-cluster')];
+refs(_) ->
+    throw(#{
+        field_name => mongo_type,
+        expected => "single | rs | sharded"
+    }).

+ 12 - 7
apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl

@@ -33,7 +33,7 @@
 
 -export([
     refs/0,
-    refs/1,
+    union_member_selector/1,
     create/2,
     update/2,
     authenticate/2,
@@ -53,7 +53,7 @@ roots() ->
     [
         {?CONF_NS,
             hoconsc:mk(
-                hoconsc:union(refs()),
+                hoconsc:union(fun union_member_selector/1),
                 #{}
             )}
     ].
@@ -98,17 +98,22 @@ refs() ->
         hoconsc:ref(?MODULE, sentinel)
     ].
 
+union_member_selector(all_union_members) ->
+    refs();
+union_member_selector({value, Value}) ->
+    refs(Value).
+
 refs(#{<<"redis_type">> := <<"single">>}) ->
-    {ok, hoconsc:ref(?MODULE, standalone)};
+    [hoconsc:ref(?MODULE, standalone)];
 refs(#{<<"redis_type">> := <<"cluster">>}) ->
-    {ok, hoconsc:ref(?MODULE, cluster)};
+    [hoconsc:ref(?MODULE, cluster)];
 refs(#{<<"redis_type">> := <<"sentinel">>}) ->
-    {ok, hoconsc:ref(?MODULE, sentinel)};
+    [hoconsc:ref(?MODULE, sentinel)];
 refs(_) ->
-    {error, #{
+    throw(#{
         field_name => redis_type,
         expected => "single | cluster | sentinel"
-    }}.
+    }).
 
 create(_AuthenticatorID, Config) ->
     create(Config).

+ 135 - 0
apps/emqx_authn/test/emqx_authn_schema_tests.erl

@@ -0,0 +1,135 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_authn_schema_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% schema error
+-define(ERR(Reason), {error, Reason}).
+
+union_member_selector_mongo_test_() ->
+    Check = fun(Txt) -> check(emqx_authn_mongodb, Txt) end,
+    [
+        {"unknown", fun() ->
+            ?assertMatch(
+                ?ERR(#{field_name := mongo_type, expected := _}),
+                Check("{mongo_type: foobar}")
+            )
+        end},
+        {"single", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-mongodb:standalone"}),
+                Check("{mongo_type: single}")
+            )
+        end},
+        {"replica-set", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-mongodb:replica-set"}),
+                Check("{mongo_type: rs}")
+            )
+        end},
+        {"sharded", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-mongodb:sharded-cluster"}),
+                Check("{mongo_type: sharded}")
+            )
+        end}
+    ].
+
+union_member_selector_jwt_test_() ->
+    Check = fun(Txt) -> check(emqx_authn_jwt, Txt) end,
+    [
+        {"unknown", fun() ->
+            ?assertMatch(
+                ?ERR(#{field_name := use_jwks, expected := "true | false"}),
+                Check("{use_jwks = 1}")
+            )
+        end},
+        {"jwks", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-jwt:jwks"}),
+                Check("{use_jwks = true}")
+            )
+        end},
+        {"publick-key", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-jwt:public-key"}),
+                Check("{use_jwks = false, public_key = 1}")
+            )
+        end},
+        {"hmac-based", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-jwt:hmac-based"}),
+                Check("{use_jwks = false}")
+            )
+        end}
+    ].
+
+union_member_selector_redis_test_() ->
+    Check = fun(Txt) -> check(emqx_authn_redis, Txt) end,
+    [
+        {"unknown", fun() ->
+            ?assertMatch(
+                ?ERR(#{field_name := redis_type, expected := _}),
+                Check("{redis_type = 1}")
+            )
+        end},
+        {"single", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-redis:standalone"}),
+                Check("{redis_type = single}")
+            )
+        end},
+        {"cluster", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-redis:cluster"}),
+                Check("{redis_type = cluster}")
+            )
+        end},
+        {"sentinel", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-redis:sentinel"}),
+                Check("{redis_type = sentinel}")
+            )
+        end}
+    ].
+
+union_member_selector_http_test_() ->
+    Check = fun(Txt) -> check(emqx_authn_http, Txt) end,
+    [
+        {"unknown", fun() ->
+            ?assertMatch(
+                ?ERR(#{field_name := method, expected := _}),
+                Check("{method = 1}")
+            )
+        end},
+        {"get", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-http:get"}),
+                Check("{method = get}")
+            )
+        end},
+        {"post", fun() ->
+            ?assertMatch(
+                ?ERR(#{matched_type := "authn-http:post"}),
+                Check("{method = post}")
+            )
+        end}
+    ].
+
+check(Module, HoconConf) ->
+    emqx_hocon:check(Module, ["authentication= ", HoconConf]).