getting rid of ejabberd_hooks:run_fold/4 interface and converting a f…

…ew hooks used in testing
esl · Jul 12, 2021 · 75b93b1 · 75b93b1
1 parent b2404f2
commit 75b93b1
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 42 deletions.
diff --git a/big_tests/tests/mod_ping_SUITE.erl b/big_tests/tests/mod_ping_SUITE.erl
@@ -116,7 +116,7 @@ setup_pong_hook(HostType, Pid) ->
                          #{pid => Pid}, 50).
 
 pong_hook_handler(Acc,
-                  #{args := [_HostType, JID, _Response, _TDelta]} = _Params,
+                  #{jid := JID} = _Params,
                   #{pid := Pid} = _Extra) ->
     Pid ! {pong, jid:to_binary(jid:to_lower(JID))},
     {ok, Acc}.

diff --git a/big_tests/tests/push_SUITE.erl b/big_tests/tests/push_SUITE.erl
@@ -763,7 +763,7 @@ rpc_start_hook_handler(TestCasePid, PubSubJID) ->
                          #{pid => TestCasePid, jid => PubSubJID}, 50).
 
 hook_handler_fn(Acc,
-                #{args := [_Host, [PayloadMap], OptionMap]} = _Params,
+                #{notification_forms := [PayloadMap], options := OptionMap} = _Params,
                 #{pid := TestCasePid, jid := PubSubJID} = _Extra) ->
     try jid:to_binary(mongoose_acc:get(push_notifications, pubsub_jid, Acc)) of
         PubSubJIDBin when PubSubJIDBin =:= PubSubJID ->

diff --git a/src/ejabberd_hooks.erl b/src/ejabberd_hooks.erl
@@ -29,7 +29,7 @@
 %% External exports
 -export([add/5,
          delete/5,
-         run_fold/4]).
+         add_args/2]).
 
 -export([add/1,
          delete/1]).
@@ -80,16 +80,10 @@ delete(Hooks) when is_list(Hooks) ->
     [delete_hook(Hook) || Hook <- Hooks],
     ok.
 
-
-%% @doc run the hook.
--spec run_fold(HookName :: atom(),
-               HostType :: mongooseim:host_type() | global,
-               Acc :: term(),
-               Args :: [term()]) ->
-    NewAcc :: term() | stopped.
-run_fold(HookName, HostType, Acc, Args) when is_binary(HostType); HostType =:= global ->
-    {_, RetValue} = gen_hook:run_fold(HookName, HostType, Acc, #{args => Args}),
-    RetValue.
+-spec add_args(HookParams :: map(), LegacyArgsList :: [term()]) ->
+    HookParamsWithArgs :: map().
+add_args(HookParams, LegacyArgsList) ->
+    HookParams#{args => LegacyArgsList}.
 
 %%%----------------------------------------------------------------------
 %%% Internal functions

diff --git a/src/mongoose_hooks.erl b/src/mongoose_hooks.erl
@@ -225,7 +225,10 @@ remove_domain(HostType, Domain) ->
 
 -spec node_cleanup(Node :: node()) -> Acc :: map().
 node_cleanup(Node) ->
-    run_global_hook(node_cleanup, #{}, [Node]).
+    Params = #{node => Node},
+    Args = [Node],
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args),
+    run_global_hook(node_cleanup, #{}, ParamsWithLegacyArgs).
 
 -spec ejabberd_ctl_process(Acc, Args) -> Result when
     Acc :: any(),
@@ -306,8 +309,11 @@ presence_probe_hook(HostType, Acc, From, To, Pid) ->
     Options :: #{atom() => binary()},
     Result :: ok | {error, any()}.
 push_notifications(Server, Acc, NotificationForms, Options) ->
-    run_hook_for_host_type(push_notifications, Server, Acc,
-                           [Server, NotificationForms, Options]).
+    Params = #{server => Server, options => Options,
+               notification_forms => NotificationForms},
+    Args = [Server, NotificationForms, Options],
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args),
+    run_hook_for_host_type(push_notifications, Server, Acc, ParamsWithLegacyArgs).
 
 %%% @doc The `register_command' hook is called when a command
 %%% is registered in `mongoose_commands'.
@@ -395,7 +401,10 @@ set_vcard(LServer, User, VCard) ->
     JID :: jid:jid(),
     Result :: mongoose_acc:t().
 unacknowledged_message(HostType, Acc, JID) ->
-    run_hook_for_host_type(unacknowledged_message, HostType, Acc, [JID]).
+    Params = #{jid => JID},
+    Args = [JID],
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args),
+    run_hook_for_host_type(unacknowledged_message, HostType, Acc, ParamsWithLegacyArgs).
 
 %%% @doc The `unregister_command' hook is called when a command
 %%% is unregistered from `mongoose_commands'.
@@ -430,8 +439,10 @@ user_available_hook(HostType, Acc, JID) ->
     TDelta :: non_neg_integer(),
     Result :: mongoose_acc:t().
 user_ping_response(HostType, Acc, JID, Response, TDelta) ->
-    run_hook_for_host_type(user_ping_response, HostType, Acc,
-                           [HostType, JID, Response, TDelta]).
+    Params = #{jid => JID, response => Response, time_delta => TDelta},
+    Args =  [HostType, JID, Response, TDelta],
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, Args),
+    run_hook_for_host_type(user_ping_response, HostType, Acc, ParamsWithLegacyArgs).
 
 %%% @doc The `user_ping_timeout' hook is called when there is a timeout
 %%% when waiting for a ping response from a user.
@@ -1442,22 +1453,25 @@ mod_global_distrib_unknown_recipient(GlobalHost, Info) ->
 %%%----------------------------------------------------------------------
 %%% Internal functions
 %%%----------------------------------------------------------------------
-
-%% run_global_hook(HookName, Acc, Params) when is_map(Params) ->
-%%     {_, RetValue} = gen_hook:run_fold(HookName, global, Acc, Params),
-%%     RetValue;
+run_global_hook(HookName, Acc, Params) when is_map(Params) ->
+    run_fold(HookName, global, Acc, Params);
 run_global_hook(HookName, Acc, Args) when is_list(Args) ->
-    ejabberd_hooks:run_fold(HookName, global, Acc, Args).
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(#{}, Args),
+    run_fold(HookName, global, Acc, ParamsWithLegacyArgs).
 
 run_hook_for_host_type(HookName, undefined, Acc, Args) ->
     ?LOG_ERROR(#{what => undefined_host_type,
                  text => <<"Running hook for an undefined host type">>,
                  hook_name => HookName, hook_acc => Acc, hook_args => Args}),
     Acc;
-%% run_hook_for_host_type(HookName, HostType, Acc, Params) when is_binary(HostType),
-%%                                                              is_map(Params) ->
-%%     {_, RetValue} = gen_hook:run_fold(HookName, HostType, Acc, Params),
-%%     RetValue;
+run_hook_for_host_type(HookName, HostType, Acc, Params) when is_binary(HostType),
+                                                             is_map(Params) ->
+    run_fold(HookName, HostType, Acc, Params);
 run_hook_for_host_type(HookName, HostType, Acc, Args) when is_binary(HostType),
                                                            is_list(Args) ->
-    ejabberd_hooks:run_fold(HookName, HostType, Acc, Args).
+    ParamsWithLegacyArgs = ejabberd_hooks:add_args(#{}, Args),
+    run_fold(HookName, HostType, Acc, ParamsWithLegacyArgs).
+
+run_fold(HookName, HostType, Acc, Params) when is_map(Params) ->
+    {_, RetValue} = gen_hook:run_fold(HookName, HostType, Acc, Params),
+    RetValue.
diff --git a/test/ejabberd_hooks_SUITE.erl b/test/ejabberd_hooks_SUITE.erl
@@ -67,7 +67,7 @@ hooks_run_launches_nullary_fun(_) ->
     given_hook_added(test_run_hook, hook_mod, fun_nullary, 1),
 
     %% when
-    ejabberd_hooks:run_fold(test_run_hook, ?HOST, ok, []),
+    run_fold(test_run_hook, ?HOST, ok, []),
 
     %% then
     H = meck:history(hook_mod),
@@ -79,7 +79,7 @@ hooks_run_launches_unary_fun(_) ->
     given_hook_added(test_run_hook, hook_mod, fun_onearg, 1),
 
     %% when
-    ejabberd_hooks:run_fold(test_run_hook, ?HOST, ok, [oneval]),
+    run_fold(test_run_hook, ?HOST, ok, [oneval]),
 
     %% then
     [{_,{hook_mod,fun_onearg,[ok, oneval]}, oneval}] = meck:history(hook_mod).
@@ -93,7 +93,7 @@ hooks_run_ignores_different_arity_funs(_) ->
     given_hook_added(test_run_hook, hook_mod, fun_twoarg, 1),
 
     %% when
-    ejabberd_hooks:run_fold(test_run_hook, ?HOST, ok, [one, two]),
+    run_fold(test_run_hook, ?HOST, ok, [one, two]),
 
     %% then
     [{_,{hook_mod, fun_twoarg, [ok, one, two]}, success2}] = meck:history(hook_mod).
@@ -107,7 +107,7 @@ hooks_run_stops_when_fun_returns_stop(_) ->
     given_hook_added(test_run_hook, hook_mod, another_fun, 2),
 
     %% when
-    ejabberd_hooks:run_fold(test_run_hook, ?HOST, ok, []),
+    run_fold(test_run_hook, ?HOST, ok, []),
 
     %% then
     [{_,{hook_mod,a_fun,[ok]}, stop}] = meck:history(hook_mod).
@@ -119,7 +119,7 @@ hooks_run_fold_folds_with_unary_fun(_) ->
     given_hook_added(test_fold_hook, hook_mod, unary_folder, 1),
 
     %% when
-    ejabberd_hooks:run_fold(test_fold_hook, ?HOST, initial, []),
+    run_fold(test_fold_hook, ?HOST, initial, []),
 
     %% then
     [{_,{hook_mod,unary_folder,[initial]}, done}] = meck:history(hook_mod).
@@ -130,7 +130,7 @@ hooks_run_fold_folds_with_binary_fun(_) ->
     given_hook_added(test_fold_hook, hook_mod, binary_folder, 1),
 
     %% when
-    ejabberd_hooks:run_fold(test_fold_hook, ?HOST, initial, [arg1]),
+    run_fold(test_fold_hook, ?HOST, initial, [arg1]),
 
     %% then
     [{_,{hook_mod,binary_folder,[initial, arg1]}, done}] = meck:history(hook_mod).
@@ -144,7 +144,7 @@ hooks_run_fold_passes_acc_along(_) ->
     given_hook_added(test_fold_hook, hook_mod2, second_folder, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, 0, [10]),
+    R = run_fold(test_fold_hook, ?HOST, 0, [10]),
 
     %% then
     -10 = R.
@@ -158,7 +158,7 @@ hooks_run_fold_stops_when_fun_returns_stop(_) ->
     given_hook_added(test_fold_hook, hook_mod2, folder, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, continue, []),
+    R = run_fold(test_fold_hook, ?HOST, continue, []),
 
     %% then
     [{_,{hook_mod1,stopper,[continue]}, stop}] = meck:history(hook_mod1),
@@ -175,7 +175,7 @@ hooks_run_fold_preserves_order(_) ->
     given_hook_added(test_fold_hook, hook_mod2, second_folder, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, 0, []),
+    R = run_fold(test_fold_hook, ?HOST, 0, []),
 
     %% then
     2 = R.
@@ -191,7 +191,7 @@ error_in_run_fold_is_ignored(_) ->
     given_hook_added(test_fold_hook, working_mod, good, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, initial, []),
+    R = run_fold(test_fold_hook, ?HOST, initial, []),
 
     %% then
     i_was_run = R,
@@ -209,7 +209,7 @@ throw_in_run_fold_is_ignored(_) ->
     given_hook_added(test_fold_hook, working_mod, good, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, initial, []),
+    R = run_fold(test_fold_hook, ?HOST, initial, []),
 
     %% then
     initial = R,
@@ -228,7 +228,7 @@ exit_in_run_fold_is_ignored(_) ->
     given_hook_added(test_fold_hook, working_mod, good, 2),
 
     %% when
-    R = ejabberd_hooks:run_fold(test_fold_hook, ?HOST, initial, []),
+    R = run_fold(test_fold_hook, ?HOST, initial, []),
 
     %% then
     initial = R,
@@ -258,6 +258,10 @@ given_module(ModName, FunName, Fun) ->
 given_fun(ModName, FunName, Fun) ->
     meck:expect(ModName, FunName, Fun).
 
+run_fold(HookName, HostType, Acc, Args) ->
+    Params = ejabberd_hooks:add_args(#{}, Args),
+    {_, RetValue} = gen_hook:run_fold(HookName, HostType, Acc, Params),
+    RetValue.
 
 get_hooks() ->
     ets:tab2list(gen_hook).
diff --git a/test/mongoose_cleanup_SUITE.erl b/test/mongoose_cleanup_SUITE.erl
@@ -80,7 +80,7 @@ cleaner_runs_hook_on_nodedown(_Config) ->
     ?assertEqual(false, meck:called(gen_hook, error_running_hook,
                                     ['_', '_', '_', '_'])).
 
-notify_self_hook(Acc, #{args := [Node]}, #{self := Self}) ->
+notify_self_hook(Acc, #{node := Node}, #{self := Self}) ->
     Self ! {got_nodedown, Node},
     {ok, Acc}.