rework of ejabberd_hooks module

src/ejabberd_hooks.erl

@@ -34,10 +34,10 @@
          add/5,
          delete/4,
          delete/5,
-         run/2,
-         run/3,
-         run_fold/3,
-         run_fold/4]).
+         run_global/2,
+         run_global/3,
+         run_for_host_type/3,
+         run_for_host_type/4]).
 
 %% gen_server callbacks
 -export([init/1,
@@ -54,7 +54,18 @@
 
 -include("mongoose.hrl").
 
--type hook() :: {atom(), jid:server() | global, module(), fun() | atom(), integer()}.
+-type hook() :: {HookName :: atom(),
+                 HostType :: binary() | global,
+                 Module :: module() | undefined,
+                 Fn :: fun() | atom(),
+                 Priority:: integer()}.
+
+-type key() :: {HookName :: atom(),
+                HostType :: binary() | global}.
+
+-type element() :: {Priority:: integer(), %% must be first to ensure proper sorting
+                    Module :: module() | undefined,
+                    Fn :: fun() | atom()}.
 
 -record(state, {}).
 
@@ -67,94 +78,93 @@ start_link() ->
     gen_server:start_link({local, ejabberd_hooks}, ejabberd_hooks, [], []).
 
 %% @doc Add a fun to the given hook.
-%% The integer sequence is used to sort the calls:
+%% The integer Priority is used to sort the calls:
 %% low numbers are executed before high numbers.
--spec add(Hook :: atom(),
-          Host :: jid:server() | global,
+-spec add(HookName :: atom(),
+          HostType :: binary() | global,
           Function :: fun() | atom(),
-          Seq :: integer()) -> ok.
-add(Hook, Host, Function, Seq) when is_function(Function) ->
-    add(Hook, Host, undefined, Function, Seq).
+          Priority :: integer()) -> ok.
+add(HookName, HostType, Function, Priority) when is_function(Function) ->
+    add_hook({HookName, HostType, undefined, Function, Priority}).
 
 %% @doc Add a module and function to the given hook.
-%% The integer sequence is used to sort the calls:
+%% The integer Priority is used to sort the calls:
 %% low numbers are executed before high numbers.
--spec add(Hook :: atom(),
-          Host :: jid:server() | global,
+-spec add(HookName :: atom(),
+          HostType :: binary() | global,
           Module :: atom(),
           Function :: fun() | atom(),
-          Seq :: integer()) -> ok.
-add(Hook, Host, Module, Function, Seq) ->
-    gen_server:call(ejabberd_hooks, {add, Hook, Host, Module, Function, Seq}).
+          Priority :: integer()) -> ok.
+add(HookName, HostType, Module, Function, Priority) ->
+    add_hook({HookName, HostType, Module, Function, Priority}).
 
 -spec add([hook()]) -> ok.
 add(Hooks) when is_list(Hooks) ->
-    add_or_del(add, Hooks).
-
--spec add_or_del(add | delete, [hook()]) -> ok.
-add_or_del(AddOrDel, Hooks) ->
-    [erlang:apply(?MODULE, AddOrDel, tuple_to_list(Hook)) ||
-     Hook <- Hooks],
+    [add_hook(Hook) || Hook <- Hooks],
     ok.
 
+-spec add_hook(hook()) -> ok.
+add_hook(Hook) ->
+    gen_server:call(ejabberd_hooks, {add, Hook}).
+
 %% @doc Delete a module and function from this hook.
 %% It is important to indicate exactly the same information than when the call was added.
--spec delete(Hook :: atom(),
-             Host :: jid:server() | global,
+-spec delete(HookName :: atom(),
+             HostType :: binary() | global,
              Function :: fun() | atom(),
-             Seq :: integer()) -> ok.
-delete(Hook, Host, Function, Seq) when is_function(Function) ->
-    delete(Hook, Host, undefined, Function, Seq).
+             Priority :: integer()) -> ok.
+delete(HookName, HostType, Function, Priority) when is_function(Function) ->
+    delete_hook({HookName, HostType, undefined, Function, Priority}).
 
--spec delete(Hook :: atom(),
-             Host :: jid:server() | global,
+-spec delete(HookName :: atom(),
+             HostType :: binary() | global,
              Module :: atom(),
              Function :: fun() | atom(),
-             Seq :: integer()) -> ok.
-delete(Hook, Host, Module, Function, Seq) ->
-    gen_server:call(ejabberd_hooks, {delete, Hook, Host, Module, Function, Seq}).
+             Priority :: integer()) -> ok.
+delete(HookName, HostType, Module, Function, Priority) ->
+    delete_hook({HookName, HostType, Module, Function, Priority}).
 
 -spec delete([hook()]) -> ok.
 delete(Hooks) when is_list(Hooks) ->
-    add_or_del(delete, Hooks).
+    [delete_hook(Hook) || Hook <- Hooks],
+    ok.
+
+-spec delete_hook(hook()) -> ok.
+delete_hook(Hook) ->
+    gen_server:call(ejabberd_hooks, {delete, Hook}).
 
 %% @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.
--spec run(Hook :: atom(),
-          Args :: [any()]) -> ok.
-run(Hook, Args) ->
-    run(Hook, global, Args).
-
--spec run(Hook :: atom(),
-          Host :: jid:server() | global,
-          Args :: [any()]) -> ok.
-run(Hook, Host, Args) ->
+-spec run_global(HookName :: atom(),
+                   Args :: [any()]) -> ok.
+run_global(HookName, Args) ->
+    run_fold(HookName, global, ok, Args).
+
+-spec run_for_host_type(HookName :: atom(),
+                        HostType :: binary(),
+                        Args :: [any()]) -> ok.
+run_for_host_type(HookName, HostType, Args) ->
     %% We don't provide mongoose_acc here because we can't create a valid one.
-    run_fold(Hook, Host, ok, Args).
+    run_fold(HookName, HostType, ok, Args).
 
-%% @spec (Hook::atom(), Val, Args) -> Val | stopped | NewVal
+%% @spec (HookName::atom(), Acc, Args) -> Val | stopped | NewVal
 %% @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.
+%% The arguments passed to the function are: [Acc | Args].
+%% The result of a call is used as Acc for the next call.
 %% If a call returns 'stop', no more calls are performed and 'stopped' is returned.
-%% If a call returns {stop, NewVal}, no more calls are performed and NewVal is returned.
--spec run_fold(Hook :: atom(), Val :: term(), Args :: [term()]) -> term() | stopped.
-run_fold(Hook, Val, Args) ->
-    run_fold(Hook, global, Val, Args).
-
--spec run_fold(Hook :: atom(),
-               Host :: global | jid:server(),
-               Val :: term(),
-               Args :: [term()]) ->
-    term() | stopped.
-run_fold(Hook, Host, Val, Args) ->
-    case ets:lookup(hooks, {Hook, Host}) of
-        [{_, Ls}] ->
-            mongoose_metrics:increment_generic_hook_metric(Host, Hook),
-            run_fold1(Ls, Hook, Val, Args);
-        [] ->
-            Val
-    end.
+%% If a call returns {stop, NewAcc}, no more calls are performed and NewAcc is returned.
+-spec run_global(HookName :: atom(), Acc :: term(), Args :: [term()]) ->
+    NewAcc :: term() | stopped.
+run_global(HookName, Acc, Args) ->
+    run_fold(HookName, global, Acc, Args).
+
+-spec run_for_host_type(HookName :: atom(),
+                        HostType :: binary(),
+                        Acc :: term(),
+                        Args :: [term()]) ->
+    NewAcc :: term() | stopped.
+run_for_host_type(HookName, HostType, Acc, Args) ->
+    run_fold(HookName, HostType, Acc, Args).
 
 %%%----------------------------------------------------------------------
 %%% Callback functions from gen_server
@@ -180,31 +190,33 @@ init([]) ->
 %%          {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) ->
-    Reply = case ets:lookup(hooks, {Hook, Host}) of
+handle_call({add, Hook}, _From, State) ->
+    {Key, El} = get_key_and_el(Hook),
+    Reply = case ets:lookup(hooks, Key) of
                 [{_, Ls}] ->
-                    El = {Seq, Module, Function},
                     case lists:member(El, Ls) of
                         true ->
                             ok;
                         false ->
+                            %% NB: lists:merge/2 returns sorted list!
                             NewLs = lists:merge(Ls, [El]),
-                            ets:insert(hooks, {{Hook, Host}, NewLs}),
+                            ets:insert(hooks, {Key, NewLs}),
                             ok
                     end;
                 [] ->
-                    NewLs = [{Seq, Module, Function}],
-                    ets:insert(hooks, {{Hook, Host}, NewLs}),
-                    mongoose_metrics:create_generic_hook_metric(Host, Hook),
+                    NewLs = [El],
+                    ets:insert(hooks, {Key, NewLs}),
+                    create_hook_metric(Key),
                     ok
             end,
     {reply, Reply, State};
 
-handle_call({delete, Hook, Host, Module, Function, Seq}, _From, State) ->
-    Reply = case ets:lookup(hooks, {Hook, Host}) of
+handle_call({delete, Hook}, _From, State) ->
+    {Key, El} = get_key_and_el(Hook),
+    Reply = case ets:lookup(hooks, Key) of
                 [{_, Ls}] ->
-                    NewLs = lists:delete({Seq, Module, Function}, Ls),
-                    ets:insert(hooks, {{Hook, Host}, NewLs}),
+                    NewLs = lists:delete(El, Ls),
+                    ets:insert(hooks, {Key, NewLs}),
                     ok;
                 [] ->
                     ok
@@ -249,29 +261,53 @@ code_change(_OldVsn, State, _Extra) ->
 %%%----------------------------------------------------------------------
 %%% Internal functions
 %%%----------------------------------------------------------------------
+-spec run_fold(HookName :: atom(),
+               HostType :: global | binary(),
+               Acc :: term(),
+               Args :: [term()]) ->
+                  term() | stopped.
+run_fold(HookName, HostType, Acc, Args) ->
+    Key = {HookName, HostType},
+    case ets:lookup(hooks, Key) of
+        [{_, Ls}] ->
+            mongoose_metrics:increment_generic_hook_metric(HostType, HookName),
+            run_hook(Ls, Key, Acc, Args);
+        [] ->
+            Acc
+    end.
 
-run_fold1([], _Hook, Val, _Args) ->
+run_hook([], _Key, Val, _Args) ->
     Val;
-run_fold1([{_Seq, Module, Function} | Ls], Hook, Val, Args) ->
-    Res = hook_apply_function(Module, Function, Val, Args),
+run_hook([{_Priority, Module, Function} | Ls], Key, Acc, Args) ->
+    Res = hook_apply_function(Module, Function, Acc, Args),
     case Res of
         {'EXIT', Reason} ->
-            error_running_hook(Reason, Hook, Args),
-            run_fold1(Ls, Hook, Val, Args);
+            error_running_hook(Reason, Key, Args),
+            run_hook(Ls, Key, Acc, Args);
         stop ->
             stopped;
-        {stop, NewVal} ->
-            NewVal;
-        NewVal ->
-            run_fold1(Ls, Hook, NewVal, Args)
+        {stop, NewAcc} ->
+            NewAcc;
+        NewAcc ->
+            run_hook(Ls, Key, NewAcc, Args)
     end.
 
-hook_apply_function(_Module, Function, Val, Args) when is_function(Function) ->
-    safely:apply(Function, [Val | Args]);
-hook_apply_function(Module, Function, Val, Args) ->
-    safely:apply(Module, Function, [Val | Args]).
+hook_apply_function(_Module, Function, Acc, Args) when is_function(Function) ->
+    safely:apply(Function, [Acc | Args]);
+hook_apply_function(Module, Function, Acc, Args) ->
+    safely:apply(Module, Function, [Acc | Args]).
 
-error_running_hook(Reason, Hook, Args) ->
+error_running_hook(Reason, Key, Args) ->
     ?LOG_ERROR(#{what => hook_failed,
                  text => <<"Error running hook">>,
-                 hook => Hook, args => Args, reason => Reason}).
+                 hook_key => Key, args => Args, reason => Reason}).
+
+-spec get_key_and_el(hook()) -> {key(), element()}.
+get_key_and_el({HookName, HostType, Module, Function, Priority}) ->
+    Key = {HookName, HostType},
+    El = {Priority, Module, Function},
+    {Key, El}.
+
+-spec create_hook_metric(Key :: key()) -> any().
+create_hook_metric({HookName, HostType}) ->
+    mongoose_metrics:create_generic_hook_metric(HostType, HookName).

src/mongoose_commands.erl

@@ -334,7 +334,7 @@ register_commands(Commands) ->
         fun(Command) ->
             check_registration(Command), %% may throw
             ets:insert_new(mongoose_commands, Command),
-            mongoose_hooks:register_command(global, Command),
+            mongoose_hooks:register_command(Command),
             ok
         end,
         Commands).
@@ -344,7 +344,7 @@ unregister_commands(Commands) ->
     lists:foreach(
         fun(Command) ->
             ets:delete_object(mongoose_commands, Command),
-            mongoose_hooks:unregister_command(global, Command)
+            mongoose_hooks:unregister_command(Command)
         end,
         Commands).