Pārlūkot izejas kodu

chore(exhook): change the name to binary type

JianBo He 4 gadi atpakaļ
vecāks
revīzija
4921c00a19

+ 2 - 2
apps/emqx_exhook/src/emqx_exhook.erl

@@ -33,7 +33,7 @@
 %% Mgmt APIs
 %%--------------------------------------------------------------------
 
--spec enable(atom()|string()) -> ok | {error, term()}.
+-spec enable(binary()) -> ok | {error, term()}.
 enable(Name) ->
     with_mngr(fun(Pid) -> emqx_exhook_mngr:enable(Pid, Name) end).
 
@@ -109,7 +109,7 @@ call_fold(Hookpoint, Req, FailedAction, AccFun, [ServerName|More]) ->
 %% XXX: Hard-coded the deny response
 deny_action_result('client.authenticate', _) ->
     #{result => false};
-deny_action_result('client.check_acl', _) ->
+deny_action_result('client.authorize', _) ->
     #{result => false};
 deny_action_result('message.publish', Msg) ->
     %% TODO: Not support to deny a message

+ 4 - 11
apps/emqx_exhook/src/emqx_exhook_cli.erl

@@ -28,12 +28,12 @@ cli(["server", "list"]) ->
 
 cli(["server", "enable", Name]) ->
     if_enabled(fun() ->
-        print(emqx_exhook:enable(list_to_existing_atom(Name)))
+        print(emqx_exhook:enable(iolist_to_binary(Name)))
     end);
 
 cli(["server", "disable", Name]) ->
     if_enabled(fun() ->
-        print(emqx_exhook:disable(list_to_existing_atom(Name)))
+        print(emqx_exhook:disable(iolist_to_binary(Name)))
     end);
 
 cli(["server", "stats"]) ->
@@ -52,14 +52,6 @@ print(ok) ->
 print({error, Reason}) ->
     emqx_ctl:print("~p~n", [Reason]).
 
-find_server_options(Name) ->
-    Ls = emqx_config:get([exhook, servers]),
-    case [ E || E = #{name := N} <- Ls, N =:= Name] of
-        [] -> undefined;
-        [Options] ->
-            maps:remove(name, Options)
-    end.
-
 %%--------------------------------------------------------------------
 %% Internal funcs
 %%--------------------------------------------------------------------
@@ -85,7 +77,8 @@ stats() ->
 format(Name) ->
     case emqx_exhook_mngr:server(Name) of
         undefined ->
-            io_lib:format("name=~s, hooks=#{}, active=false", [Name]);
+            lists:flatten(
+              io_lib:format("name=~s, hooks=#{}, active=false", [Name]));
         Server ->
             emqx_exhook_server:format(Server)
     end.

+ 5 - 3
apps/emqx_exhook/src/emqx_exhook_mngr.erl

@@ -84,11 +84,11 @@
 start_link(Servers, AutoReconnect, ReqOpts) ->
     gen_server:start_link(?MODULE, [Servers, AutoReconnect, ReqOpts], []).
 
--spec enable(pid(), atom()|string()) -> ok | {error, term()}.
+-spec enable(pid(), binary()) -> ok | {error, term()}.
 enable(Pid, Name) ->
     call(Pid, {load, Name}).
 
--spec disable(pid(), atom()|string()) -> ok | {error, term()}.
+-spec disable(pid(), binary()) -> ok | {error, term()}.
 disable(Pid, Name) ->
     call(Pid, {unload, Name}).
 
@@ -136,7 +136,9 @@ load_all_servers(Servers, ReqOpts) ->
     load_all_servers(Servers, ReqOpts, #{}, #{}).
 load_all_servers([], _Request, Waiting, Running) ->
     {Waiting, Running};
-load_all_servers([{Name, Options}|More], ReqOpts, Waiting, Running) ->
+load_all_servers([#{name := Name0} = Options0|More], ReqOpts, Waiting, Running) ->
+    Name = iolist_to_binary(Name0),
+    Options = Options0#{name => Name},
     {NWaiting, NRunning} =
         case emqx_exhook_server:load(Name, Options, ReqOpts) of
             {ok, ServerState} ->

+ 15 - 1
apps/emqx_exhook/src/emqx_exhook_schema.erl

@@ -26,10 +26,24 @@
 
 -behaviour(hocon_schema).
 
+-type duration() :: integer().
+
+-typerefl_from_string({duration/0, emqx_schema, to_duration}).
+
+-reflect_type([duration/0]).
+
 -export([structs/0, fields/1]).
+
 -export([t/1, t/3, t/4, ref/1]).
 
-structs() -> [servers].
+structs() -> [exhook].
+
+fields(exhook) ->
+    [ {request_failed_action, t(union([deny, ignore]), undefined, deny)}
+    , {request_timeout, t(duration(), undefined, "5s")}
+    , {auto_reconnect, t(union([false, duration()]), undefined, "60s")}
+    , {servers, t(hoconsc:array(ref(servers)), undefined, [])}
+    ];
 
 fields(servers) ->
     [ {name, string()}

+ 7 - 16
apps/emqx_exhook/src/emqx_exhook_server.erl

@@ -38,7 +38,7 @@
 
 -record(server, {
           %% Server name (equal to grpc client channel name)
-          name :: server_name(),
+          name :: binary(),
           %% The function options
           options :: map(),
           %% gRPC channel pid
@@ -49,7 +49,6 @@
           prefix :: list()
        }).
 
--type server_name() :: string().
 -type server() :: #server{}.
 
 -type hookpoint() :: 'client.connect'
@@ -84,9 +83,8 @@
 %% Load/Unload APIs
 %%--------------------------------------------------------------------
 
--spec load(atom(), options(), map()) -> {ok, server()} | {error, term()} .
-load(Name0, Opts0, ReqOpts) ->
-    Name = to_list(Name0),
+-spec load(binary(), options(), map()) -> {ok, server()} | {error, term()} .
+load(Name, Opts0, ReqOpts) ->
     {SvrAddr, ClientOpts} = channel_opts(Opts0),
     case emqx_exhook_sup:start_grpc_client_channel(
            Name,
@@ -112,20 +110,12 @@ load(Name0, Opts0, ReqOpts) ->
         {error, _} = E -> E
     end.
 
-%% @private
-to_list(Name) when is_atom(Name) ->
-    atom_to_list(Name);
-to_list(Name) when is_binary(Name) ->
-    binary_to_list(Name);
-to_list(Name) when is_list(Name) ->
-    Name.
-
 %% @private
 channel_opts(Opts = #{url := URL}) ->
     case uri_string:parse(URL) of
-        #{scheme := <<"http">>, host := Host, port := Port} ->
+        #{scheme := "http", host := Host, port := Port} ->
             {format_http_uri("http", Host, Port), #{}};
-        #{scheme := <<"https">>, host := Host, port := Port} ->
+        #{scheme := "https", host := Host, port := Port} ->
             SslOpts =
                 case maps:get(ssl, Opts, undefined) of
                     undefined -> [];
@@ -230,7 +220,8 @@ may_unload_hooks(HookSpecs) ->
     end, maps:keys(HookSpecs)).
 
 format(#server{name = Name, hookspec = Hooks}) ->
-    io_lib:format("name=~s, hooks=~0p, active=true", [Name, Hooks]).
+    lists:flatten(
+      io_lib:format("name=~s, hooks=~0p, active=true", [Name, Hooks])).
 
 %%--------------------------------------------------------------------
 %% APIs

+ 1 - 1
apps/emqx_exhook/src/emqx_exhook_sup.erl

@@ -58,7 +58,7 @@ request_options() ->
      }.
 
 env(Key, Def) ->
-    application:get_env(emqx_exhook, Key, Def).
+    emqx_config:get([exhook, Key], Def).
 
 %%--------------------------------------------------------------------
 %% APIs

+ 6 - 6
apps/emqx_exhook/test/emqx_exhook_SUITE.erl

@@ -53,13 +53,13 @@ end_per_suite(_Cfg) ->
 %%--------------------------------------------------------------------
 
 t_noserver_nohook(_) ->
-    emqx_exhook:disable(default),
+    emqx_exhook:disable(<<"default">>),
     ?assertEqual([], ets:tab2list(emqx_hooks)),
-    ok = emqx_exhook:enable(default),
+    ok = emqx_exhook:enable(<<"default">>),
     ?assertNotEqual([], ets:tab2list(emqx_hooks)).
 
 t_access_failed_if_no_server_running(_) ->
-    emqx_exhook:disable(default),
+    emqx_exhook:disable(<<"default">>),
     ClientInfo = #{clientid => <<"user-id-1">>,
                    username => <<"usera">>,
                    peerhost => {127,0,0,1},
@@ -67,16 +67,16 @@ t_access_failed_if_no_server_running(_) ->
                    protocol => mqtt,
                    mountpoint => undefined
                   },
-    ?assertMatch({stop, #{auth_result := not_authorized}},
+    ?assertMatch({stop, {error, not_authorized}},
                  emqx_exhook_handler:on_client_authenticate(ClientInfo, #{auth_result => success})),
 
     ?assertMatch({stop, deny},
-                 emqx_exhook_handler:on_client_check_acl(ClientInfo, publish, <<"t/1">>, allow)),
+                 emqx_exhook_handler:on_client_authorize(ClientInfo, publish, <<"t/1">>, allow)),
 
     Message = emqx_message:make(<<"t/1">>, <<"abc">>),
     ?assertMatch({stop, Message},
                  emqx_exhook_handler:on_message_publish(Message)),
-    emqx_exhook:enable(default).
+    emqx_exhook:enable(<<"default">>).
 
 t_cli_list(_) ->
     meck_print(),