From f7ad25108cee68862f6f570c93e5d5565077ee0d Mon Sep 17 00:00:00 2001 From: Evgeny Khramtsov Date: Tue, 9 Jul 2019 01:26:48 +0300 Subject: [PATCH] Get rid of unused API functions in ejabberd_hooks Also improve code formatting and type specs --- src/ejabberd_hooks.erl | 197 +++++------------------------------------ 1 file changed, 23 insertions(+), 174 deletions(-) diff --git a/src/ejabberd_hooks.erl b/src/ejabberd_hooks.erl index 87a559d90..f329da4a8 100644 --- a/src/ejabberd_hooks.erl +++ b/src/ejabberd_hooks.erl @@ -22,10 +22,8 @@ %%% 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. %%% %%%---------------------------------------------------------------------- - -module(ejabberd_hooks). -author('alexey@process-one.net'). - -behaviour(gen_server). %% External exports @@ -33,21 +31,13 @@ add/3, add/4, add/5, - add_dist/5, - add_dist/6, delete/3, delete/4, delete/5, - delete_dist/5, - delete_dist/6, run/2, run/3, run_fold/3, - run_fold/4, - get_handlers/2]). - --export([delete_all_hooks/0]). - + run_fold/4]). %% gen_server callbacks -export([init/1, handle_call/3, @@ -60,22 +50,20 @@ -include("ejabberd_stacktrace.hrl"). -record(state, {}). --type local_hook() :: { Seq :: integer(), Module :: atom(), Function :: atom()}. --type distributed_hook() :: { Seq :: integer(), Node :: atom(), Module :: atom(), Function :: atom()}. +-type hook() :: {Seq :: integer(), Module :: atom(), Function :: atom() | fun()}. %%%---------------------------------------------------------------------- %%% API %%%---------------------------------------------------------------------- start_link() -> - gen_server:start_link({local, ejabberd_hooks}, ejabberd_hooks, [], []). - --spec add(atom(), fun(), number()) -> ok. + gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). +-spec add(atom(), fun(), integer()) -> ok. %% @doc See add/4. add(Hook, Function, Seq) when is_function(Function) -> add(Hook, global, undefined, Function, Seq). --spec add(atom(), HostOrModule :: binary() | atom(), fun() | atom() , number()) -> ok. +-spec add(atom(), HostOrModule :: binary() | atom(), fun() | atom() , integer()) -> ok. add(Hook, Host, Function, Seq) when is_function(Function) -> add(Hook, Host, undefined, Function, Seq); @@ -84,29 +72,16 @@ add(Hook, Host, Function, Seq) when is_function(Function) -> add(Hook, Module, Function, Seq) -> add(Hook, global, Module, Function, Seq). --spec add(atom(), binary() | global, atom(), atom() | fun(), number()) -> ok. - +-spec add(atom(), binary() | global, atom(), atom() | fun(), integer()) -> ok. add(Hook, Host, Module, Function, Seq) -> - gen_server:call(ejabberd_hooks, {add, Hook, Host, Module, Function, Seq}). - --spec add_dist(atom(), atom(), atom(), atom() | fun(), number()) -> ok. - -add_dist(Hook, Node, Module, Function, Seq) -> - gen_server:call(ejabberd_hooks, {add, Hook, global, Node, Module, Function, Seq}). - --spec add_dist(atom(), binary() | global, atom(), atom(), atom() | fun(), number()) -> ok. - -add_dist(Hook, Host, Node, Module, Function, Seq) -> - gen_server:call(ejabberd_hooks, {add, Hook, Host, Node, Module, Function, Seq}). - --spec delete(atom(), fun(), number()) -> ok. + gen_server:call(?MODULE, {add, Hook, Host, Module, Function, Seq}). +-spec delete(atom(), fun(), integer()) -> ok. %% @doc See del/4. delete(Hook, Function, Seq) when is_function(Function) -> delete(Hook, global, undefined, Function, Seq). --spec delete(atom(), binary() | atom(), atom() | fun(), number()) -> ok. - +-spec delete(atom(), binary() | atom(), atom() | fun(), integer()) -> ok. delete(Hook, Host, Function, Seq) when is_function(Function) -> delete(Hook, Host, undefined, Function, Seq); @@ -115,41 +90,17 @@ delete(Hook, Host, Function, Seq) when is_function(Function) -> delete(Hook, Module, Function, Seq) -> delete(Hook, global, Module, Function, Seq). --spec delete(atom(), binary() | global, atom(), atom() | fun(), number()) -> ok. - +-spec delete(atom(), binary() | global, atom(), atom() | fun(), integer()) -> ok. delete(Hook, Host, Module, Function, Seq) -> - gen_server:call(ejabberd_hooks, {delete, Hook, Host, Module, Function, Seq}). - --spec delete_dist(atom(), atom(), atom(), atom() | fun(), number()) -> ok. - -delete_dist(Hook, Node, Module, Function, Seq) -> - delete_dist(Hook, global, Node, Module, Function, Seq). - --spec delete_dist(atom(), binary() | global, atom(), atom(), atom() | fun(), number()) -> ok. - -delete_dist(Hook, Host, Node, Module, Function, Seq) -> - gen_server:call(ejabberd_hooks, {delete, Hook, Host, Node, Module, Function, Seq}). - --spec delete_all_hooks() -> true. - -%% @doc Primarily for testing / instrumentation -delete_all_hooks() -> - gen_server:call(ejabberd_hooks, {delete_all}). - --spec get_handlers(atom(), binary() | global) -> [local_hook() | distributed_hook()]. -%% @doc Returns currently set handler for hook name -get_handlers(Hookname, Host) -> - gen_server:call(ejabberd_hooks, {get_handlers, Hookname, Host}). + gen_server:call(?MODULE, {delete, Hook, Host, Module, Function, Seq}). -spec run(atom(), list()) -> ok. - %% @doc Run the calls of this hook in order, don't care about function results. %% If a call returns stop, no more calls are performed. run(Hook, Args) -> run(Hook, global, Args). -spec run(atom(), binary() | global, list()) -> ok. - run(Hook, Host, Args) -> try ets:lookup(hooks, {Hook, Host}) of [{_, Ls}] -> @@ -161,7 +112,6 @@ run(Hook, Host, Args) -> end. -spec run_fold(atom(), any(), list()) -> any(). - %% @doc Run the calls of this hook in order. %% The arguments passed to the function are: [Val | Args]. %% The result of a call is used as Val for the next call. @@ -171,7 +121,6 @@ run_fold(Hook, Val, Args) -> run_fold(Hook, global, Val, Args). -spec run_fold(atom(), binary() | global, any(), list()) -> any(). - run_fold(Hook, Host, Val, Args) -> try ets:lookup(hooks, {Hook, Host}) of [{_, Ls}] -> @@ -185,62 +134,23 @@ run_fold(Hook, Host, Val, Args) -> %%%---------------------------------------------------------------------- %%% Callback functions from gen_server %%%---------------------------------------------------------------------- - -%%---------------------------------------------------------------------- -%% Func: init/1 -%% Returns: {ok, State} | -%% {ok, State, Timeout} | -%% ignore | -%% {stop, Reason} -%%---------------------------------------------------------------------- init([]) -> _ = ets:new(hooks, [named_table, {read_concurrency, true}]), {ok, #state{}}. -%%---------------------------------------------------------------------- -%% Func: handle_call/3 -%% Returns: {reply, Reply, State} | -%% {reply, Reply, State, Timeout} | -%% {noreply, State} | -%% {noreply, State, Timeout} | -%% {stop, Reason, Reply, State} | (terminate/2 is called) -%% {stop, Reason, State} (terminate/2 is called) -%%---------------------------------------------------------------------- handle_call({add, Hook, Host, Module, Function, Seq}, _From, State) -> HookFormat = {Seq, Module, Function}, Reply = handle_add(Hook, Host, HookFormat), {reply, Reply, State}; -handle_call({add, Hook, Host, Node, Module, Function, Seq}, _From, State) -> - HookFormat = {Seq, Node, Module, Function}, - Reply = handle_add(Hook, Host, HookFormat), - {reply, Reply, State}; - handle_call({delete, Hook, Host, Module, Function, Seq}, _From, State) -> HookFormat = {Seq, Module, Function}, Reply = handle_delete(Hook, Host, HookFormat), {reply, Reply, State}; -handle_call({delete, Hook, Host, Node, Module, Function, Seq}, _From, State) -> - HookFormat = {Seq, Node, Module, Function}, - Reply = handle_delete(Hook, Host, HookFormat), - {reply, Reply, State}; +handle_call(Request, From, State) -> + ?WARNING_MSG("Unexpected call from ~p: ~p", [From, Request]), + {noreply, State}. -handle_call({get_handlers, Hook, Host}, _From, State) -> - Reply = case ets:lookup(hooks, {Hook, Host}) of - [{_, Handlers}] -> Handlers; - [] -> [] - end, - {reply, Reply, State}; - -handle_call({delete_all}, _From, State) -> - Reply = ets:delete_all_objects(hooks), - {reply, Reply, State}; - -handle_call(_Request, _From, State) -> - Reply = ok, - {reply, Reply, State}. - --spec handle_add(atom(), atom(), local_hook() | distributed_hook()) -> ok. -%% in-memory storage operation: Handle adding hook in ETS table +-spec handle_add(atom(), atom(), hook()) -> ok. handle_add(Hook, Host, El) -> case ets:lookup(hooks, {Hook, Host}) of [{_, Ls}] -> @@ -258,9 +168,7 @@ handle_add(Hook, Host, El) -> ok end. - --spec handle_delete(atom(), atom(), local_hook() | distributed_hook()) -> ok. -%% in-memory storage operation: Handle deleting hook from ETS table +-spec handle_delete(atom(), atom(), hook()) -> ok. handle_delete(Hook, Host, El) -> case ets:lookup(hooks, {Hook, Host}) of [{_, Ls}] -> @@ -271,29 +179,14 @@ handle_delete(Hook, Host, El) -> ok end. -%%---------------------------------------------------------------------- -%% Func: handle_cast/2 -%% Returns: {noreply, State} | -%% {noreply, State, Timeout} | -%% {stop, Reason, State} (terminate/2 is called) -%%---------------------------------------------------------------------- -handle_cast(_Msg, State) -> +handle_cast(Msg, State) -> + ?WARNING_MSG("Unexpected cast: ~p", [Msg]), {noreply, State}. -%%---------------------------------------------------------------------- -%% Func: handle_info/2 -%% Returns: {noreply, State} | -%% {noreply, State, Timeout} | -%% {stop, Reason, State} (terminate/2 is called) -%%---------------------------------------------------------------------- -handle_info(_Info, State) -> +handle_info(Info, State) -> + ?WARNING_MSG("Unexpected info: ~p", [Info]), {noreply, State}. -%%---------------------------------------------------------------------- -%% Func: terminate/2 -%% Purpose: Shutdown the server -%% Returns: any (ignored by gen_server) -%%---------------------------------------------------------------------- terminate(_Reason, _State) -> ok. @@ -303,33 +196,9 @@ code_change(_OldVsn, State, _Extra) -> %%%---------------------------------------------------------------------- %%% Internal functions %%%---------------------------------------------------------------------- - --spec run1([local_hook()|distributed_hook()], atom(), list()) -> ok. - +-spec run1([hook()], atom(), list()) -> ok. run1([], _Hook, _Args) -> ok; -%% Run distributed hook on target node. -%% It is not attempted again in case of failure. Next hook will be executed -run1([{_Seq, Node, Module, Function} | Ls], Hook, Args) -> - %% MR: Should we have a safe rpc, like we have a safe apply or is bad_rpc enough ? - case ejabberd_cluster:call(Node, Module, Function, Args) of - timeout -> - ?ERROR_MSG("Timeout on RPC to ~p~nrunning hook: ~p", - [Node, {Hook, Args}]), - run1(Ls, Hook, Args); - {badrpc, Reason} -> - ?ERROR_MSG("Bad RPC error to ~p: ~p~nrunning hook: ~p", - [Node, Reason, {Hook, Args}]), - run1(Ls, Hook, Args); - stop -> - ?INFO_MSG("~nThe process ~p in node ~p ran a hook in node ~p.~n" - "Stop.", [self(), node(), Node]), % debug code - ok; - Res -> - ?INFO_MSG("~nThe process ~p in node ~p ran a hook in node ~p.~n" - "The response is:~n~s", [self(), node(), Node, Res]), % debug code - run1(Ls, Hook, Args) - end; run1([{_Seq, Module, Function} | Ls], Hook, Args) -> Res = safe_apply(Hook, Module, Function, Args), case Res of @@ -341,30 +210,9 @@ run1([{_Seq, Module, Function} | Ls], Hook, Args) -> run1(Ls, Hook, Args) end. - +-spec run_fold1([hook()], atom(), T, list()) -> T | stopped. run_fold1([], _Hook, Val, _Args) -> Val; -run_fold1([{_Seq, Node, Module, Function} | Ls], Hook, Val, Args) -> - case ejabberd_cluster:call(Node, Module, Function, [Val | Args]) of - {badrpc, Reason} -> - ?ERROR_MSG("Bad RPC error to ~p: ~p~nrunning hook: ~p", - [Node, Reason, {Hook, Args}]), - run_fold1(Ls, Hook, Val, Args); - timeout -> - ?ERROR_MSG("Timeout on RPC to ~p~nrunning hook: ~p", - [Node, {Hook, Args}]), - run_fold1(Ls, Hook, Val, Args); - stop -> - stopped; - {stop, NewVal} -> - ?INFO_MSG("~nThe process ~p in node ~p ran a hook in node ~p.~n" - "Stop, and the NewVal is:~n~p", [self(), node(), Node, NewVal]), % debug code - NewVal; - NewVal -> - ?INFO_MSG("~nThe process ~p in node ~p ran a hook in node ~p.~n" - "The NewVal is:~n~p", [self(), node(), Node, NewVal]), % debug code - run_fold1(Ls, Hook, NewVal, Args) - end; run_fold1([{_Seq, Module, Function} | Ls], Hook, Val, Args) -> Res = safe_apply(Hook, Module, Function, [Val | Args]), case Res of @@ -378,6 +226,7 @@ run_fold1([{_Seq, Module, Function} | Ls], Hook, Val, Args) -> run_fold1(Ls, Hook, NewVal, Args) end. +-spec safe_apply(atom(), atom(), atom() | fun(), list()) -> any(). safe_apply(Hook, Module, Function, Args) -> ?DEBUG("Running hook ~p: ~p:~p/~B", [Hook, Module, Function, length(Args)]),