restricting hook_tag() type

esl · Jul 14, 2021 · 9e2d73c · 9e2d73c
1 parent 2ec4c5d
commit 9e2d73c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 9 deletions.
diff --git a/src/gen_hook.erl b/src/gen_hook.erl
@@ -24,7 +24,7 @@
 -include("mongoose.hrl").
 
 -type hook_name() :: atom().
--type hook_tag() :: any().
+-type hook_tag() :: mongoose:host_type() | global.
 
 %% while Accumulator is not limited to any type, it's recommended to use maps.
 -type hook_acc() :: any().
@@ -218,7 +218,8 @@ error_running_hook(Reason, Handler, Acc, Params) ->
 
 -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),
+        when is_atom(HookName), is_binary(Tag) or (Tag =:= global),
+             is_function(Function, 3), is_map(Extra),
              is_integer(Priority), Priority > 0 ->
     NewExtra = extend_extra(HookTuple),
     {Module, FunctionName} = check_hook_function(Function),
@@ -263,7 +264,12 @@ hook_key(HookName, Tag) ->
 
 -spec extend_extra(hook_tuple()) -> hook_extra().
 extend_extra({HookName, Tag, _Function, OriginalExtra, _Priority}) ->
-    ExtraExtension = #{hook_name => HookName, hook_tag => Tag},
+    ExtraExtension = case Tag of
+                         global -> #{hook_name => HookName, hook_tag => Tag};
+                         HostType when is_binary(HostType) ->
+                             #{hook_name => HookName, hook_tag => Tag,
+                               host_type => HostType}
+                     end,
     %% KV pairs of the OriginalExtra map will remain unchanged,
     %% only the new keys from the ExtraExtension map will be added
     %% to the NewExtra map

diff --git a/test/gen_hook_SUITE.erl b/test/gen_hook_SUITE.erl
@@ -3,8 +3,8 @@
 
 -include_lib("eunit/include/eunit.hrl").
 
--define(HOOK_TAG1, some_tag).
--define(HOOK_TAG2, another_tag).
+-define(HOOK_TAG1, global).
+-define(HOOK_TAG2, <<"some tag">>).
 
 -define(assertEqualLists(L1, L2), ?assertEqual(lists:sort(L1), lists:sort(L2))).
 
@@ -67,7 +67,8 @@ single_handler_can_be_added_and_removed(_) ->
     AllHandlers = [{{calculate, ?HOOK_TAG1}, Tag1Handlers},
                    {{calculate, ?HOOK_TAG2},
                     [{hook_handler, {calculate, ?HOOK_TAG2}, 1, mod1, plus,
-                      #{hook_name => calculate, hook_tag => ?HOOK_TAG2, id => 1}}]}],
+                      #{hook_name => calculate, hook_tag => ?HOOK_TAG2,
+                        host_type =>?HOOK_TAG2, id => 1}}]}],
     ?assertEqualLists(AllHandlers, get_handlers_for_all_hooks()),
     %% try to add some hook handler second time and check that nothing has changed
     ?assertEqual(ok, gen_hook:add_handler(calculate, ?HOOK_TAG1, MultiplyHandlerFn,
@@ -114,7 +115,8 @@ multiple_handlers_can_be_added_and_removed(_) ->
     AllHandlers = [{{calculate, ?HOOK_TAG1}, Tag1Handlers},
                    {{calculate, ?HOOK_TAG2},
                     [{hook_handler, {calculate, ?HOOK_TAG2}, 1, mod1, plus,
-                      #{hook_name => calculate, hook_tag => ?HOOK_TAG2, id => 1}}]}],
+                      #{hook_name => calculate, hook_tag => ?HOOK_TAG2,
+                        host_type =>?HOOK_TAG2, id => 1}}]}],
     ?assertEqualLists(AllHandlers, get_handlers_for_all_hooks()),
     %% try to add hook handlers second time and check that nothing has changed
     ?assertEqual(ok, gen_hook:add_handlers(HookHandlers)),
@@ -191,7 +193,8 @@ invalid_hook_handler_parameters_causes_error(_) ->
     HandlerFn = fun ?MODULE:hook_handler_stop/3,
     InvalidHookHandlers = [{calculate, ?HOOK_TAG1, HandlerFn, invalid_extra_param, 2},
                            {<<"invalid hook name">>, ?HOOK_TAG1, HandlerFn, #{}, 2},
-                           {calculate, ?HOOK_TAG1, HandlerFn, #{}, invalid_priority}],
+                           {calculate, ?HOOK_TAG1, HandlerFn, #{}, invalid_priority},
+                           {calculate, invalid_hook_tag, HandlerFn, #{}, 2}],
     [?assertError(function_clause, gen_hook:add_handlers([HookHandler]))
      || HookHandler <- InvalidHookHandlers],
     ?assertEqual([], get_handlers_for_all_hooks()).
@@ -286,7 +289,7 @@ errors_in_handlers_are_reported_but_ignored(_) ->
     ?assertEqual(true, meck:called(gen_hook, error_running_hook,
                                    [{some_error, '_'},
                                     {hook_handler, {calculate, ?HOOK_TAG1}, 3, mod1, error,
-                                     #{hook_name => calculate, hook_tag => some_tag}},
+                                     #{hook_name => calculate, hook_tag => ?HOOK_TAG1}},
                                     6, #{n => 2}])),
     %% check hook handlers execution sequence
     Self = self(),