From 4921c00a195d0070694a5f936d4e04cb920c49ca Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 12 Aug 2021 14:50:26 +0800 Subject: [PATCH] chore(exhook): change the name to binary type --- apps/emqx_exhook/src/emqx_exhook.erl | 4 ++-- apps/emqx_exhook/src/emqx_exhook_cli.erl | 15 ++++---------- apps/emqx_exhook/src/emqx_exhook_mngr.erl | 8 ++++--- apps/emqx_exhook/src/emqx_exhook_schema.erl | 16 +++++++++++++- apps/emqx_exhook/src/emqx_exhook_server.erl | 23 +++++++-------------- apps/emqx_exhook/src/emqx_exhook_sup.erl | 2 +- apps/emqx_exhook/test/emqx_exhook_SUITE.erl | 12 +++++------ 7 files changed, 40 insertions(+), 40 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook.erl b/apps/emqx_exhook/src/emqx_exhook.erl index 8558a91ad..c6b02e716 100644 --- a/apps/emqx_exhook/src/emqx_exhook.erl +++ b/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 diff --git a/apps/emqx_exhook/src/emqx_exhook_cli.erl b/apps/emqx_exhook/src/emqx_exhook_cli.erl index 40c6e2f9f..860499698 100644 --- a/apps/emqx_exhook/src/emqx_exhook_cli.erl +++ b/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. diff --git a/apps/emqx_exhook/src/emqx_exhook_mngr.erl b/apps/emqx_exhook/src/emqx_exhook_mngr.erl index cadd5eb37..8c15fc330 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mngr.erl +++ b/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} -> diff --git a/apps/emqx_exhook/src/emqx_exhook_schema.erl b/apps/emqx_exhook/src/emqx_exhook_schema.erl index 68ffb5735..852a210fe 100644 --- a/apps/emqx_exhook/src/emqx_exhook_schema.erl +++ b/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()} diff --git a/apps/emqx_exhook/src/emqx_exhook_server.erl b/apps/emqx_exhook/src/emqx_exhook_server.erl index 9bf9cbad3..94ac4df61 100644 --- a/apps/emqx_exhook/src/emqx_exhook_server.erl +++ b/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 diff --git a/apps/emqx_exhook/src/emqx_exhook_sup.erl b/apps/emqx_exhook/src/emqx_exhook_sup.erl index e9c405de0..ba92e3f96 100644 --- a/apps/emqx_exhook/src/emqx_exhook_sup.erl +++ b/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 diff --git a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl index 88964487c..d2cc78b47 100644 --- a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl +++ b/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(),