test(exhook): refine tests

apps/emqx_exhook/include/emqx_exhook.hrl

@@ -25,7 +25,7 @@
       , {'client.connected',    {emqx_exhook_handler, on_client_connected,     []}}
       , {'client.disconnected', {emqx_exhook_handler, on_client_disconnected,  []}}
       , {'client.authenticate', {emqx_exhook_handler, on_client_authenticate,  []}}
-      , {'client.check_acl',    {emqx_exhook_handler, on_client_check_acl,     []}}
+      , {'client.authorize',    {emqx_exhook_handler, on_client_authorize,     []}}
       , {'client.subscribe',    {emqx_exhook_handler, on_client_subscribe,     []}}
       , {'client.unsubscribe',  {emqx_exhook_handler, on_client_unsubscribe,   []}}
       , {'session.created',     {emqx_exhook_handler, on_session_created,      []}}

apps/emqx_exhook/priv/emqx_exhook.schema

@@ -1,38 +0,0 @@
-%%-*- mode: erlang -*-
-
-{mapping, "exhook.server.$name.url", "emqx_exhook.servers", [
-  {datatype, string}
-]}.
-
-{mapping, "exhook.server.$name.ssl.cacertfile", "emqx_exhook.servers", [
-  {datatype, string}
-]}.
-
-{mapping, "exhook.server.$name.ssl.certfile", "emqx_exhook.servers", [
-  {datatype, string}
-]}.
-
-{mapping, "exhook.server.$name.ssl.keyfile", "emqx_exhook.servers", [
-  {datatype, string}
-]}.
-
-{translation, "emqx_exhook.servers", fun(Conf) ->
-  Filter  = fun(Opts) -> [{K, V} || {K, V} <- Opts, V =/= undefined] end,
-  ServerOptions = fun(Prefix) ->
-                      case http_uri:parse(cuttlefish:conf_get(Prefix ++ ".url", Conf)) of
-                          {ok, {http, _, Host, Port, _, _}} ->
-                              [{scheme, http}, {host, Host}, {port, Port}];
-                          {ok, {https, _, Host, Port, _, _}} ->
-                              [{scheme, https}, {host, Host}, {port, Port},
-                               {ssl_options,
-                                 Filter([{ssl, true},
-                                         {certfile, cuttlefish:conf_get(Prefix ++ ".ssl.certfile", Conf, undefined)},
-                                         {keyfile, cuttlefish:conf_get(Prefix ++ ".ssl.keyfile", Conf, undefined)},
-                                         {cacertfile, cuttlefish:conf_get(Prefix ++ ".ssl.cacertfile", Conf, undefined)}
-                                        ])}];
-                          _ -> error(invalid_server_options)
-                      end
-                  end,
-  [{list_to_atom(Name), ServerOptions("exhook.server." ++ Name)}
-   || {["exhook", "server", Name, "url"], _} <- cuttlefish_variable:filter_by_prefix("exhook.server", Conf)]
-end}.

apps/emqx_exhook/rebar.config

@@ -39,11 +39,3 @@
 {cover_excl_mods, [emqx_exhook_pb,
                    emqx_exhook_v_1_hook_provider_bhvr,
                    emqx_exhook_v_1_hook_provider_client]}.
-
-{profiles,
- [{test,
-   [{deps,
-      [{emqx_ct_helper, {git, "https://github.com/emqx/emqx-ct-helpers", {tag, "v1.3.1"}}}
-      ]}
-    ]}
-]}.

apps/emqx_exhook/src/emqx_exhook_cli.erl

@@ -30,7 +30,7 @@ cli(["server", "list"]) ->
 cli(["server", "enable", Name0]) ->
     if_enabled(fun() ->
         Name = list_to_atom(Name0),
-        case proplists:get_value(Name, application:get_env(?APP, servers, [])) of
+        case maps:get(Name, emqx_config:get([exhook, server]), undefined) of
             undefined ->
                 emqx_ctl:print("not_found~n");
             Opts ->

apps/emqx_exhook/src/emqx_exhook_handler.erl

@@ -87,7 +87,6 @@ on_client_disconnected(ClientInfo, Reason, _ConnInfo) ->
            },
     cast('client.disconnected', Req).
 
-%% FIXME: `AuthResult`
 on_client_authenticate(ClientInfo, AuthResult) ->
     %% XXX: Bool is missing more information about the atom of the result
     %%      So, the `Req` has missed detailed info too.
@@ -95,7 +94,7 @@ on_client_authenticate(ClientInfo, AuthResult) ->
     %%      The return value of `call_fold` just a bool, that has missed
     %%      detailed info too.
     %%
-    Bool = maps:get(auth_result, AuthResult, undefined) == success,
+    Bool = AuthResult == ok,
     Req = #{clientinfo => clientinfo(ClientInfo),
             result => Bool
            },
@@ -103,8 +102,8 @@ on_client_authenticate(ClientInfo, AuthResult) ->
     case call_fold('client.authenticate', Req,
                    fun merge_responsed_bool/2) of
         {StopOrOk, #{result := Result0}} when is_boolean(Result0) ->
-            Result = case Result0 of true -> success; _ -> not_authorized end,
-            {StopOrOk, AuthResult#{auth_result => Result, anonymous => false}};
+            Result = case Result0 of true -> ok; _ -> {error, not_authorized} end,
+            {StopOrOk, Result};
         _ ->
             {ok, AuthResult}
     end.

apps/emqx_exhook/src/emqx_exhook_server.erl

@@ -122,7 +122,6 @@ to_list(Name) when is_list(Name) ->
 
 %% @private
 channel_opts(Opts = #{url := URL}) ->
-    io:format("~p~n", [Opts]),
     case uri_string:parse(URL) of
         #{scheme := <<"http">>, host := Host, port := Port} ->
             {format_http_uri("http", Host, Port), #{}};

apps/emqx_exhook/test/emqx_exhook_SUITE.erl

@@ -22,6 +22,10 @@
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
 
+-define(CONF_DEFAULT, <<"
+exhook: { server.default: { url: \"http://127.0.0.1:9000\" } }
+">>).
+
 %%--------------------------------------------------------------------
 %% Setups
 %%--------------------------------------------------------------------
@@ -30,35 +34,24 @@ all() -> emqx_ct:all(?MODULE).
 
 init_per_suite(Cfg) ->
     _ = emqx_exhook_demo_svr:start(),
-    emqx_ct_helpers:start_apps([emqx_exhook], fun set_special_cfgs/1),
+    ok = emqx_config:init_load(emqx_exhook_schema, ?CONF_DEFAULT),
+    emqx_ct_helpers:start_apps([emqx_exhook]),
     Cfg.
 
 end_per_suite(_Cfg) ->
     emqx_ct_helpers:stop_apps([emqx_exhook]),
     emqx_exhook_demo_svr:stop().
 
-set_special_cfgs(emqx) ->
-    application:set_env(emqx, allow_anonymous, false),
-    application:set_env(emqx, enable_acl_cache, false),
-    application:set_env(emqx, plugins_loaded_file, undefined),
-    application:set_env(emqx, modules_loaded_file, undefined);
-set_special_cfgs(emqx_exhook) ->
-    ok.
-
 %%--------------------------------------------------------------------
 %% Test cases
 %%--------------------------------------------------------------------
 
 t_noserver_nohook(_) ->
     emqx_exhook:disable(default),
-    ?assertEqual([], ets:tab2list(emqx_hooks)),
-
-    Opts = proplists:get_value(
-             default,
-             application:get_env(emqx_exhook, servers, [])
-            ),
+    ?assertEqual([], loaded_exhook_hookpoints()),
+    Opts = emqx_config:get([exhook, server, default]),
     ok = emqx_exhook:enable(default, Opts),
-    ?assertNotEqual([], ets:tab2list(emqx_hooks)).
+    ?assertNotEqual([], loaded_exhook_hookpoints()).
 
 t_cli_list(_) ->
     meck_print(),
@@ -94,3 +87,17 @@ meck_print() ->
 
 unmeck_print() ->
     meck:unload(emqx_ctl).
+
+loaded_exhook_hookpoints() ->
+    lists:filtermap(fun(E) ->
+        Name = element(2, E),
+        Callbacks = element(3, E),
+        case lists:any(fun is_exhook_callback/1, Callbacks) of
+            true -> {true, Name};
+            _ -> false
+        end
+    end, ets:tab2list(emqx_hooks)).
+
+is_exhook_callback(Cb) ->
+    Action = element(2, Cb),
+    emqx_exhook_handler == element(1, Action).

apps/emqx_exhook/test/emqx_exhook_demo_svr.erl

@@ -33,7 +33,7 @@
         , on_client_connected/2
         , on_client_disconnected/2
         , on_client_authenticate/2
-        , on_client_check_acl/2
+        , on_client_authorize/2
         , on_client_subscribe/2
         , on_client_unsubscribe/2
         , on_session_created/2
@@ -122,7 +122,7 @@ on_provider_loaded(Req, Md) ->
                      #{name => <<"client.connected">>},
                      #{name => <<"client.disconnected">>},
                      #{name => <<"client.authenticate">>},
-                     #{name => <<"client.check_acl">>},
+                     #{name => <<"client.authorize">>},
                      #{name => <<"client.subscribe">>},
                      #{name => <<"client.unsubscribe">>},
                      #{name => <<"session.created">>},
@@ -197,10 +197,10 @@ on_client_authenticate(#{clientinfo := #{username := Username}} = Req, Md) ->
             {ok, #{type => 'IGNORE'}, Md}
     end.
 
--spec on_client_check_acl(emqx_exhook_pb:client_check_acl_request(), grpc:metadata())
+-spec on_client_authorize(emqx_exhook_pb:client_authorize_request(), grpc:metadata())
     -> {ok, emqx_exhook_pb:valued_response(), grpc:metadata()}
      | {error, grpc_cowboy_h:error_response()}.
-on_client_check_acl(#{clientinfo := #{username := Username}} = Req, Md) ->
+on_client_authorize(#{clientinfo := #{username := Username}} = Req, Md) ->
     ?MODULE:in({?FUNCTION_NAME, Req}),
     %io:format("fun: ~p, req: ~0p~n", [?FUNCTION_NAME, Req]),
     %% some cases for testing

scripts/find-apps.sh

@@ -7,7 +7,7 @@ cd -P -- "$(dirname -- "$0")/.."
 
 find_app() {
     local appdir="$1"
-    find "${appdir}" -mindepth 1 -maxdepth 1 -type d | grep -vE "emqx_exhook|emqx_exproto|emqx_lwm2m|emqx_sn|emqx_coap|emqx_stomp|emqx_dashboard"
+    find "${appdir}" -mindepth 1 -maxdepth 1 -type d | grep -vE "emqx_dashboard"
 }
 
 find_app 'apps'