Skip to content

Commit

Permalink
allowing only external function references in gen_hook interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
@DenysGonchar
DenysGonchar committed Jul 8, 2021
1 parent 502c2db commit ce84334
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 65 deletions.
6 changes: 4 additions & 2 deletions src/ejabberd_hooks.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
-export([add/1,
delete/1]).

-export([gen_hook_fn_wrapper/3]).

-include("mongoose.hrl").

-type hook() :: {HookName :: atom(),
Expand Down Expand Up @@ -96,14 +98,14 @@ run_fold(HookName, HostType, Acc, Args) when is_binary(HostType); HostType =:= g
-spec add_hook(hook()) -> ok.
add_hook({HookName, HostType, Module, Function, Priority}) when is_atom(Function) ->
gen_hook:add_handler(HookName, HostType,
fun gen_hook_fn_wrapper/3,
fun ?MODULE:gen_hook_fn_wrapper/3,
#{module => Module, function =>Function},
Priority).

-spec delete_hook(hook()) -> ok.
delete_hook({HookName, HostType, Module, Function, Priority}) when is_atom(Function) ->
gen_hook:delete_handler(HookName, HostType,
fun gen_hook_fn_wrapper/3,
fun ?MODULE:gen_hook_fn_wrapper/3,
#{module => Module, function => Function},
Priority).

Expand Down
147 changes: 87 additions & 60 deletions src/gen_hook.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,24 @@
-type key() :: {HookName :: atom(),
Tag :: any()}.

-type hook_tuple() :: {HookName :: hook_name(),
Tag :: hook_tag(),
Function :: hook_fn(),
Extra :: hook_extra(),
Priority :: pos_integer()}.

-type hook_list() :: [hook_tuple()].

-export_type([hook_fn/0, hook_list/0]).

-record(hook_handler, {key :: key(),
%% 'prio' field must go right after the 'key',
%% this is required for the proper sorting.
prio :: pos_integer(),
fn :: hook_fn(),
module :: module(),
function :: atom(), %% function name
extra :: map()}).

-export_type([hook_fn/0]).

-define(TABLE, ?MODULE).
%%%----------------------------------------------------------------------
%%% API
Expand All @@ -63,16 +72,17 @@ start_link() ->
Function :: hook_fn(),
Extra :: hook_extra(),
Priority :: pos_integer()) -> ok.
add_handler(HookName, Tag, Function, Extra, Priority) when is_atom(HookName),
is_function(Function, 3),
is_map(Extra),
is_integer(Priority),
Priority > 0 ->
NewExtra = extend_extra(HookName, Tag, Extra),
Handler = #hook_handler{key = hook_key(HookName, Tag),
prio = Priority,
fn = Function,
extra = NewExtra},
add_handler(HookName, Tag, Function, Extra, Priority) ->
add_handler({HookName, Tag, Function, Extra, Priority}).

-spec add_handlers(hook_list()) -> ok.
add_handlers(List) ->
[add_handler(HookTuple) || HookTuple <- List],
ok.

-spec add_handler(hook_tuple()) -> ok.
add_handler(HookTuple) ->
Handler = make_hook_handler(HookTuple),
gen_server:call(?MODULE, {add_handler, Handler}).

%% @doc Delete a hook handler.
Expand All @@ -82,46 +92,19 @@ add_handler(HookName, Tag, Function, Extra, Priority) when is_atom(HookName),
Function :: hook_fn(),
Extra :: hook_extra(),
Priority :: pos_integer()) -> ok.
delete_handler(HookName, Tag, Function, Extra, Priority) when is_atom(HookName),
is_function(Function, 3),
is_map(Extra),
is_integer(Priority),
Priority > 0 ->
NewExtra = extend_extra(HookName, Tag, Extra),
Handler = #hook_handler{key = hook_key(HookName, Tag),
prio = Priority,
fn = Function,
extra = NewExtra},
gen_server:call(?MODULE, {delete_handler, Handler}).
delete_handler(HookName, Tag, Function, Extra, Priority) ->
delete_handler({HookName, Tag, Function, Extra, Priority}).

-spec add_handlers([{HookName :: hook_name(),
Tag :: hook_tag(),
Function :: hook_fn(),
Extra :: hook_extra(),
Priority :: pos_integer()}]) -> ok.
add_handlers(List) ->
[add_handler(HookName, Tag, Function, Extra, Priority) ||
{HookName, Tag, Function, Extra, Priority} <- List, is_atom(HookName),
is_function(Function, 3),
is_map(Extra),
is_integer(Priority),
Priority > 0],
ok.

-spec delete_handlers([{HookName :: hook_name(),
Tag :: hook_tag(),
Function :: hook_fn(),
Extra :: hook_extra(),
Priority :: pos_integer()}]) -> ok.
-spec delete_handlers(hook_list()) -> ok.
delete_handlers(List) ->
[delete_handler(HookName, Tag, Function, Extra, Priority) ||
{HookName, Tag, Function, Extra, Priority} <- List, is_atom(HookName),
is_function(Function, 3),
is_map(Extra),
is_integer(Priority),
Priority > 0],
[delete_handler(HookTuple) || HookTuple <- List],
ok.

-spec delete_handler(hook_tuple()) -> ok.
delete_handler(HookTuple) ->
Handler = make_hook_handler(HookTuple),
gen_server:call(?MODULE, {delete_handler, Handler}).

%% @doc Run hook handlers in order of priority (lower number means higher priority).
%% * if hook handler returns {ok, NewAcc}, the NewAcc value is used
%% as an accumulator parameter for the following hook handler.
Expand Down Expand Up @@ -208,8 +191,8 @@ code_change(_OldVsn, State, _Extra) ->
-spec run_hook([#hook_handler{}], hook_acc(), hook_params()) -> hook_fn_ret_value().
run_hook([], Acc, _Params) ->
{ok, Acc};
run_hook([#hook_handler{fn = Function, extra = Extra} = Handler | Ls], Acc, Params) ->
case apply_hook_function(Function, Acc, Params, Extra) of
run_hook([Handler | Ls], Acc, Params) ->
case apply_hook_function(Handler, Acc, Params) of
{'EXIT', Reason} ->
?MODULE:error_running_hook(Reason, Handler, Acc, Params),
run_hook(Ls, Acc, Params);
Expand All @@ -219,10 +202,11 @@ run_hook([#hook_handler{fn = Function, extra = Extra} = Handler | Ls], Acc, Para
run_hook(Ls, NewAcc, Params)
end.

-spec apply_hook_function(hook_fn(), hook_acc(), hook_params(), hook_extra()) ->
-spec apply_hook_function(#hook_handler{}, hook_acc(), hook_params()) ->
hook_fn_ret_value() | {'EXIT', Reason :: any()}.
apply_hook_function(Function, Acc, Params, Extra) ->
safely:apply(Function, [Acc, Params, Extra]).
apply_hook_function(#hook_handler{module = Module, function = Function, extra = Extra},
Acc, Params) ->
safely:apply(Module, Function, [Acc, Params, Extra]).

error_running_hook(Reason, Handler, Acc, Params) ->
?LOG_ERROR(#{what => hook_failed,
Expand All @@ -232,15 +216,58 @@ error_running_hook(Reason, Handler, Acc, Params) ->
params => Params,
reason => Reason}).

-spec hook_key(hook_name(), hook_tag()) -> key().
hook_key(HookName, Tag) -> {HookName, Tag}.
-spec make_hook_handler(hook_tuple()) -> #hook_handler{}.
make_hook_handler({HookName, _Tag, Function, Extra, Priority} = HookTuple)
when is_atom(HookName), is_function(Function, 3), is_map(Extra),
is_integer(Priority), Priority > 0 ->
NewExtra = extend_extra(HookTuple),
{Module, FunctionName} = check_hook_function(Function),
#hook_handler{key = hook_key(HookTuple),
prio = Priority,
module = Module,
function = FunctionName,
extra = NewExtra}.

-spec check_hook_function(hook_fn()) -> {module(), atom()}.
check_hook_function(Function) when is_function(Function, 3) ->
case erlang:fun_info(Function, type) of
{type, external} ->
{module, Module} = erlang:fun_info(Function, module),
{name, FunctionName} = erlang:fun_info(Function, name),
case code:ensure_loaded(Module) of
{module, Module} -> ok;
Error ->
throw_error(#{what => module_is_not_loaded,
module => Module, error => Error})
end,
case erlang:function_exported(Module, FunctionName, 3) of
true -> ok;
false ->
throw_error(#{what => function_is_not_exported,
function => Function})
end,
{Module, FunctionName};
{type, local} ->
throw_error(#{what => only_external_function_references_allowed,
function => Function})
end.

-spec throw_error(map()) -> no_return().
throw_error(ErrorMap) ->
?LOG_ERROR(ErrorMap),
error(ErrorMap).

-spec hook_key(hook_tuple()) -> key().
hook_key({HookName, Tag, _Function, _Extra, _Priority}) ->
{HookName, Tag}.

extend_extra(HookName, Tag, OriginalExtra) ->
ExtendedExtra = #{hook_name => HookName, hook_tag => Tag},
-spec extend_extra(hook_tuple()) -> hook_extra().
extend_extra({HookName, Tag, _Function, OriginalExtra, _Priority}) ->
ExtraExtension = #{hook_name => HookName, hook_tag => Tag},
%% KV pairs of the OriginalExtra map will remain unchanged,
%% only the new keys from the ExtendedExtra map will be added
%% only the new keys from the ExtraExtension map will be added
%% to the NewExtra map
maps:merge(ExtendedExtra, OriginalExtra).
maps:merge(ExtraExtension, OriginalExtra).

-spec create_hook_metric(Key :: key()) -> any().
create_hook_metric({HookName, Tag}) ->
Expand Down
5 changes: 3 additions & 2 deletions test/ejabberd_hooks_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ a_module_fun_can_be_added(_) ->

% then
[{{test_run_hook,<<"localhost">>},
[{hook_handler, {test_run_hook,<<"localhost">>},
1,_FN,#{function := fun_a, module := hook_mod}}]}] = get_hooks().
[{hook_handler, {test_run_hook,<<"localhost">>}, 1,
ejabberd_hooks, gen_hook_fn_wrapper,
#{function := fun_a, module := hook_mod}}]}] = get_hooks().

a_module_fun_can_be_removed(_) ->
given_hooks_started(),
Expand Down
5 changes: 4 additions & 1 deletion test/muc_light_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ codec_calls(_Config) ->
HandleFun = fun(_, _, _) -> count_call(handler) end,
gen_hook:add_handler(filter_room_packet,
<<"localhost">>,
fun(Acc, _Params, _Extra) -> count_call(hook), {ok, Acc} end,
fun ?MODULE:filter_room_packet_handler/3,
#{},
50),

Expand Down Expand Up @@ -154,6 +154,9 @@ codec_calls(_Config) ->
check_count(1, 2),
ok.

filter_room_packet_handler(Acc, _Params, _Extra) ->
count_call(hook),
{ok, Acc}.
%% ----------------- Room config schema ----------------------

simple_config_items_are_parsed(_Config) ->
Expand Down

0 comments on commit ce84334

Please sign in to comment.