Skip to content

Commit

Permalink
Rework module opt getters in gen_mod
Browse files Browse the repository at this point in the history
- Add lookup_module_opts to gen_mod
  It is less error-prone than plugging in 'undefined' as default

- Use mongoose_config directly in gen_mod getters

Also: improve test coverage
  • Loading branch information
chrzaszcz committed Apr 7, 2022
1 parent 4d33aa4 commit b220bc2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
24 changes: 17 additions & 7 deletions src/gen_mod.erl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
% Get/set opts by host or from a list
get_opt/2,
get_opt/3,
lookup_module_opt/3,
get_module_opt/3,
get_module_opt/4,
get_module_opts/2,
Expand Down Expand Up @@ -247,24 +248,33 @@ get_opt(Path, Opts, Default) ->
error:{badkey, _} -> Default
end.

-spec lookup_module_opt(mongooseim:host_type(), module(), opt_key() | key_path()) ->
{ok, opt_value()} | {error, not_found}.
lookup_module_opt(HostType, Module, Key) ->
mongoose_config:lookup_opt(config_path(HostType, Module, Key)).

-spec get_module_opt(mongooseim:host_type(), module(), opt_key() | key_path(), opt_value()) ->
opt_value().
get_module_opt(HostType, Module, Opt, Default) ->
get_module_opt(HostType, Module, Key, Default) ->
%% Fail in dev builds.
%% It protects against passing something weird as a Module argument
%% or against wrong argument order.
?ASSERT_MODULE(Module),
ModuleOpts = get_module_opts(HostType, Module),
get_opt(Opt, ModuleOpts, Default).
mongoose_config:get_opt(config_path(HostType, Module, Key), Default).

-spec get_module_opt(mongooseim:host_type(), module(), opt_key() | key_path()) -> opt_value().
get_module_opt(HostType, Module, Opt) ->
?ASSERT_MODULE(Module),
ModuleOpts = get_loaded_module_opts(HostType, Module),
get_opt(Opt, ModuleOpts).
get_module_opt(HostType, Module, Key) ->
mongoose_config:get_opt(config_path(HostType, Module, Key)).

-spec config_path(mongooseim:host_type(), module(), opt_key() | key_path()) -> key_path().
config_path(HostType, Module, Path) when is_list(Path) ->
[{modules, HostType}, Module] ++ Path;
config_path(HostType, Module, Key) when is_atom(Key) ->
[{modules, HostType}, Module, Key].

-spec get_module_opts(mongooseim:host_type(), module()) -> module_opts().
get_module_opts(HostType, Module) ->
?ASSERT_MODULE(Module),
mongoose_config:get_opt([{modules, HostType}, Module], #{}).

-spec get_loaded_module_opts(mongooseim:host_type(), module()) -> module_opts().
Expand Down
6 changes: 3 additions & 3 deletions src/mod_register.erl
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@ verify_password_and_register(HostType, #jid{} = JID, Password, SourceRaw, Lang)
end.

send_welcome_message(HostType, #jid{lserver = Server} = JID) ->
case gen_mod:get_module_opt(HostType, ?MODULE, welcome_message, undefined) of
undefined ->
case gen_mod:lookup_module_opt(HostType, ?MODULE, welcome_message) of
{error, not_found} ->
ok;
{Subj, Body} ->
{ok, {Subj, Body}} ->
ejabberd_router:route(
jid:make_noprep(<<>>, Server, <<>>),
JID,
Expand Down
14 changes: 14 additions & 0 deletions test/gen_mod_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ all() ->
stop_error,
loaded_modules,
loaded_modules_with_opts,
get_module_opt,
lookup_module_opt,
hosts_with_module,
hosts_and_opts_with_module].

Expand Down Expand Up @@ -80,6 +82,18 @@ loaded_modules_with_opts(_Config) ->
?assertEqual(MB, gen_mod:loaded_modules_with_opts(host(b))),
?assertEqual(#{host(a) => MA, host(b) => MB}, gen_mod:loaded_modules_with_opts()).

get_module_opt(_Config) ->
?assertEqual(v, gen_mod:get_module_opt(host(b), b_module, k)),
?assertError({badkey, k}, gen_mod:get_module_opt(host(a), a_module, k)),
?assertError({badkey, b_module}, gen_mod:get_module_opt(host(a), b_module, k)),
?assertEqual(default, gen_mod:get_module_opt(host(a), a_module, k, default)),
?assertEqual(default, gen_mod:get_module_opt(host(a), b_module, k, default)).

lookup_module_opt(_Config) ->
?assertEqual({ok, v}, gen_mod:lookup_module_opt(host(b), b_module, k)),
?assertEqual({error, not_found}, gen_mod:lookup_module_opt(host(a), a_module, k)),
?assertEqual({error, not_found}, gen_mod:lookup_module_opt(host(a), b_module, k)).

hosts_with_module(_Config) ->
?assertEqual([host(a)], gen_mod:hosts_with_module(a_module)),
?assertEqual([host(b)], gen_mod:hosts_with_module(b_module)).
Expand Down

0 comments on commit b220bc2

Please sign in to comment.