From 6cd6a6e99255bc99e30d19e6c7eec2c10701b411 Mon Sep 17 00:00:00 2001 From: Janusz Jakubiec Date: Thu, 17 Feb 2022 11:10:28 +0100 Subject: [PATCH 1/3] Put mod_private opts in a map with defaults and updating tests --- big_tests/tests/private_SUITE.erl | 5 ++++- src/mod_private.erl | 16 +++++++++++----- src/mod_private_backend.erl | 4 ++-- src/mod_private_riak.erl | 2 +- test/common/config_parser_helper.erl | 6 ++++-- test/config_parser_SUITE.erl | 11 ++++++----- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/big_tests/tests/private_SUITE.erl b/big_tests/tests/private_SUITE.erl index 7dab2f58691..d3c1a39e0bf 100644 --- a/big_tests/tests/private_SUITE.erl +++ b/big_tests/tests/private_SUITE.erl @@ -48,10 +48,13 @@ init_per_suite(Config0) -> HostType = domain_helper:host_type(), Config1 = dynamic_modules:save_modules(HostType, Config0), Backend = mongoose_helper:get_backend_mnesia_rdbms_riak(HostType), - ModConfig = mongoose_helper:backend_for_module(mod_private, Backend), + ModConfig = create_config(Backend), dynamic_modules:ensure_modules(HostType, ModConfig), escalus:init_per_suite([{backend, Backend} | Config1]). +create_config(Backend) -> + [{mod_private, #{backend => Backend, iqdisc => one_queue, riak => <<"riak">>}}]. + end_per_suite(Config) -> dynamic_modules:restore_modules(Config), escalus:end_per_suite(Config). diff --git a/src/mod_private.erl b/src/mod_private.erl index 49da5c511d5..f35891920b3 100644 --- a/src/mod_private.erl +++ b/src/mod_private.erl @@ -69,16 +69,18 @@ get_personal_data(Acc, HostType, #jid{ luser = LUser, lserver = LServer }) -> %% ------------------------------------------------------------------ %% gen_mod callbacks -start(HostType, Opts) -> +-spec start(HostType :: mongooseim:host_type(), Opts :: gen_mod:module_opts()) -> ok. +start(HostType, #{iqdisc := IQDisc} = Opts) -> mod_private_backend:init(HostType, Opts), - IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue), ejabberd_hooks:add(hooks(HostType)), gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm, fun ?MODULE:process_iq/5, #{}, IQDisc). +-spec stop(HostType :: mongooseim:host_type()) -> ok. stop(HostType) -> ejabberd_hooks:delete(hooks(HostType)), - gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm). + gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm), + ok. supported_features() -> [dynamic_domains]. @@ -93,7 +95,10 @@ config_spec() -> items = #{<<"iqdisc">> => mongoose_config_spec:iqdisc(), <<"backend">> => #option{type = atom, validate = {module, mod_private}}, - <<"riak">> => riak_config_spec()} + <<"riak">> => riak_config_spec()}, + defaults = #{<<"iqdisc">> => one_queue, + <<"backend">> => rdbms}, + format_items = map }. riak_config_spec() -> @@ -101,7 +106,8 @@ riak_config_spec() -> items = #{<<"bucket_type">> => #option{type = binary, validate = non_empty} }, - wrap = none + defaults = #{<<"bucket_type">> => <<"private">>}, + format_items = map }. %% ------------------------------------------------------------------ diff --git a/src/mod_private_backend.erl b/src/mod_private_backend.erl index eb73f207bdd..3a88a8922e2 100644 --- a/src/mod_private_backend.erl +++ b/src/mod_private_backend.erl @@ -19,7 +19,7 @@ -callback init(HostType, Opts) -> ok when HostType :: mongooseim:host_type(), - Opts :: list(). + Opts :: gen_mod:module_opts(). -callback multi_set_data(HostType, LUser, LServer, NS2XML) -> Result when HostType :: mongooseim:host_type(), @@ -58,7 +58,7 @@ -spec init(HostType, Opts) -> ok when HostType :: mongooseim:host_type(), - Opts :: list(). + Opts :: gen_mod:module_opts(). init(HostType, Opts) -> TrackedFuns = [multi_get_data, multi_set_data], mongoose_backend:init(HostType, ?MAIN_MODULE, TrackedFuns, Opts), diff --git a/src/mod_private_riak.erl b/src/mod_private_riak.erl index 66ca3473585..e53e3568830 100644 --- a/src/mod_private_riak.erl +++ b/src/mod_private_riak.erl @@ -86,7 +86,7 @@ get_private_data(LUser, LServer, NS, Default) -> end. bucket_type(LServer) -> - {gen_mod:get_module_opt(LServer, mod_private, bucket_type, <<"private">>), LServer}. + {gen_mod:get_module_opt(LServer, mod_private, [riak, bucket_type]), LServer}. key(LUser, NS) -> <>. diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 2192d504df2..cfe9dcf7dbc 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -655,7 +655,7 @@ pgsql_modules() -> mod_muc_commands => [], mod_muc_light_commands => [], mod_offline => [{backend, rdbms}], mod_privacy => [{backend, rdbms}], - mod_private => [{backend, rdbms}], + mod_private => default_mod_config(mod_private), mod_register => [{access, register}, {ip_access, [{allow, "127.0.0.0/8"}, {deny, "0.0.0.0/0"}]}, @@ -814,6 +814,8 @@ mod_config(Module, ExtraOpts) -> default_mod_config(mod_adhoc) -> #{iqdisc => one_queue, report_commands_node => false}; +default_mod_config(mod_private) -> + #{iqdisc => one_queue, backend => rdbms}; default_mod_config(mod_auth_token) -> #{backend => rdbms, iqdisc => no_queue, validity_period => #{access => #{unit => hours, value => 1}, @@ -832,4 +834,4 @@ default_mod_config(mod_inbox) -> aff_changes => true, remove_on_kicked => true, reset_markers => [<<"displayed">>], - iqdisc => no_queue}. + iqdisc => no_queue}. \ No newline at end of file diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 8929af63ba7..226f6677d10 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2587,15 +2587,16 @@ mod_privacy(_Config) -> ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})). mod_private(_Config) -> + check_iqdisc_map(mod_private), + check_module_defaults(mod_private), T = fun(Opts) -> #{<<"modules">> => #{<<"mod_private">> => Opts}} end, - M = fun(Cfg) -> modopts(mod_private, Cfg) end, - ?cfgh(M([{backend, riak}]), + P = [modules, mod_private], + ?cfgh(P ++ [backend], riak, T(#{<<"backend">> => <<"riak">>})), - ?cfgh(M([{bucket_type, <<"private_stuff">>}]), + ?cfgh(P ++ [riak, bucket_type], <<"private_stuff">>, T(#{<<"riak">> => #{<<"bucket_type">> => <<"private_stuff">>}})), ?errh(T(#{<<"backend">> => <<"mssql">>})), - ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})), - check_iqdisc(mod_private). + ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})). mod_pubsub(_Config) -> check_iqdisc(mod_pubsub), From 5def8ddcefcb4062cc7c3faf1cb012f16b32d4ee Mon Sep 17 00:00:00 2001 From: Janusz Jakubiec Date: Thu, 17 Feb 2022 11:40:39 +0100 Subject: [PATCH 2/3] Small fix in private_SUITE --- big_tests/tests/gdpr_SUITE.erl | 7 ++++++- big_tests/tests/private_SUITE.erl | 6 +++++- src/mod_private.erl | 3 ++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/big_tests/tests/gdpr_SUITE.erl b/big_tests/tests/gdpr_SUITE.erl index 940c10e76f7..4842a13f530 100644 --- a/big_tests/tests/gdpr_SUITE.erl +++ b/big_tests/tests/gdpr_SUITE.erl @@ -429,7 +429,12 @@ offline_started() -> dynamic_modules:ensure_modules(host_type(), offline_required_modules()). private_required_modules() -> - [{mod_private, [{backend, pick_enabled_backend()}]}]. + [{mod_private, create_private_config(pick_enabled_backend())}]. + +create_private_config(riak) -> + #{backend => riak, iqdisc => one_queue, riak => #{bucket_type => <<"private">>}}; +create_private_config(Backend) -> + #{backend => Backend, iqdisc => one_queue}. private_started() -> dynamic_modules:ensure_modules(host_type(), private_required_modules()). diff --git a/big_tests/tests/private_SUITE.erl b/big_tests/tests/private_SUITE.erl index d3c1a39e0bf..5f1fae77431 100644 --- a/big_tests/tests/private_SUITE.erl +++ b/big_tests/tests/private_SUITE.erl @@ -52,8 +52,12 @@ init_per_suite(Config0) -> dynamic_modules:ensure_modules(HostType, ModConfig), escalus:init_per_suite([{backend, Backend} | Config1]). +create_config(riak) -> + [{mod_private, #{backend => riak, + iqdisc => one_queue, + riak => #{bucket_type => <<"private">>}}}]; create_config(Backend) -> - [{mod_private, #{backend => Backend, iqdisc => one_queue, riak => <<"riak">>}}]. + [{mod_private, #{backend => Backend, iqdisc => one_queue}}]. end_per_suite(Config) -> dynamic_modules:restore_modules(Config), diff --git a/src/mod_private.erl b/src/mod_private.erl index f35891920b3..ac7c30040a7 100644 --- a/src/mod_private.erl +++ b/src/mod_private.erl @@ -70,7 +70,8 @@ get_personal_data(Acc, HostType, #jid{ luser = LUser, lserver = LServer }) -> %% gen_mod callbacks -spec start(HostType :: mongooseim:host_type(), Opts :: gen_mod:module_opts()) -> ok. -start(HostType, #{iqdisc := IQDisc} = Opts) -> +start(HostType, Opts) -> + IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue), mod_private_backend:init(HostType, Opts), ejabberd_hooks:add(hooks(HostType)), gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm, From dca661236b0c76af7f21ec9568ff50ec3d4aeb05 Mon Sep 17 00:00:00 2001 From: Janusz Jakubiec Date: Fri, 18 Feb 2022 14:33:57 +0100 Subject: [PATCH 3/3] Fixing issues mentioned in PR commands --- big_tests/tests/domain_removal_SUITE.erl | 2 +- big_tests/tests/gdpr_SUITE.erl | 4 ++-- big_tests/tests/mongooseimctl_SUITE.erl | 4 +++- src/mod_private.erl | 22 +++++++++++++--------- test/common/config_parser_helper.erl | 6 +++--- test/config_parser_SUITE.erl | 8 ++++---- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/big_tests/tests/domain_removal_SUITE.erl b/big_tests/tests/domain_removal_SUITE.erl index f6ad8e024e6..23952fc6e64 100644 --- a/big_tests/tests/domain_removal_SUITE.erl +++ b/big_tests/tests/domain_removal_SUITE.erl @@ -93,7 +93,7 @@ group_to_modules(muc_removal) -> group_to_modules(inbox_removal) -> [{mod_inbox, inbox_helper:inbox_opts()}]; group_to_modules(private_removal) -> - [{mod_private, [{backend, rdbms}]}]; + [{mod_private, #{iqdisc => one_queue, backend => rdbms}}]; group_to_modules(roster_removal) -> [{mod_roster, [{backend, rdbms}]}]; group_to_modules(offline_removal) -> diff --git a/big_tests/tests/gdpr_SUITE.erl b/big_tests/tests/gdpr_SUITE.erl index 4842a13f530..33f762d4a77 100644 --- a/big_tests/tests/gdpr_SUITE.erl +++ b/big_tests/tests/gdpr_SUITE.erl @@ -432,9 +432,9 @@ private_required_modules() -> [{mod_private, create_private_config(pick_enabled_backend())}]. create_private_config(riak) -> - #{backend => riak, iqdisc => one_queue, riak => #{bucket_type => <<"private">>}}; + config_parser_helper:mod_config(mod_private, #{backend => riak, riak => #{bucket_type => <<"private">>}}); create_private_config(Backend) -> - #{backend => Backend, iqdisc => one_queue}. + config_parser_helper:mod_config(mod_private, #{backend => Backend}). private_started() -> dynamic_modules:ensure_modules(host_type(), private_required_modules()). diff --git a/big_tests/tests/mongooseimctl_SUITE.erl b/big_tests/tests/mongooseimctl_SUITE.erl index ddbb74c34b0..dfa3231fd10 100644 --- a/big_tests/tests/mongooseimctl_SUITE.erl +++ b/big_tests/tests/mongooseimctl_SUITE.erl @@ -219,7 +219,9 @@ init_per_group(basic, Config) -> dynamic_modules:ensure_modules(domain_helper:host_type(), [{mod_offline, []}]), Config; init_per_group(private, Config) -> - dynamic_modules:ensure_modules(domain_helper:host_type(), [{mod_private, []}]), + dynamic_modules:ensure_modules(domain_helper:host_type(), + [{mod_private, #{iqdisc => one_queue}}] + ), Config; init_per_group(vcard, Config) -> case rpc(mim(), gen_mod, get_module_opt, [host_type(), mod_vcard, backend, mnesia]) of diff --git a/src/mod_private.erl b/src/mod_private.erl index ac7c30040a7..93f07944458 100644 --- a/src/mod_private.erl +++ b/src/mod_private.erl @@ -35,7 +35,8 @@ config_spec/0, process_iq/5, remove_user/3, - remove_domain/3]). + remove_domain/3, + remove_unused_backend_opts/1]). -export([get_personal_data/3]). @@ -69,19 +70,17 @@ get_personal_data(Acc, HostType, #jid{ luser = LUser, lserver = LServer }) -> %% ------------------------------------------------------------------ %% gen_mod callbacks --spec start(HostType :: mongooseim:host_type(), Opts :: gen_mod:module_opts()) -> ok. -start(HostType, Opts) -> - IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue), +-spec start(HostType :: mongooseim:host_type(), Opts :: gen_mod:module_opts()) -> ok | {error, atom()}. +start(HostType, #{iqdisc := IQDisc} = Opts) -> mod_private_backend:init(HostType, Opts), ejabberd_hooks:add(hooks(HostType)), gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm, fun ?MODULE:process_iq/5, #{}, IQDisc). --spec stop(HostType :: mongooseim:host_type()) -> ok. +-spec stop(HostType :: mongooseim:host_type()) -> ok | {error, not_registered}. stop(HostType) -> ejabberd_hooks:delete(hooks(HostType)), - gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm), - ok. + gen_iq_handler:remove_iq_handler_for_domain(HostType, ?NS_PRIVATE, ejabberd_sm). supported_features() -> [dynamic_domains]. @@ -99,8 +98,12 @@ config_spec() -> <<"riak">> => riak_config_spec()}, defaults = #{<<"iqdisc">> => one_queue, <<"backend">> => rdbms}, - format_items = map - }. + format_items = map, + process = fun ?MODULE:remove_unused_backend_opts/1 + }. + +remove_unused_backend_opts(Opts = #{backend := riak}) -> Opts; +remove_unused_backend_opts(Opts) -> maps:remove(riak, Opts). riak_config_spec() -> #section{ @@ -108,6 +111,7 @@ riak_config_spec() -> validate = non_empty} }, defaults = #{<<"bucket_type">> => <<"private">>}, + include = always, format_items = map }. diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index cfe9dcf7dbc..ad52e4fafaf 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -814,8 +814,6 @@ mod_config(Module, ExtraOpts) -> default_mod_config(mod_adhoc) -> #{iqdisc => one_queue, report_commands_node => false}; -default_mod_config(mod_private) -> - #{iqdisc => one_queue, backend => rdbms}; default_mod_config(mod_auth_token) -> #{backend => rdbms, iqdisc => no_queue, validity_period => #{access => #{unit => hours, value => 1}, @@ -834,4 +832,6 @@ default_mod_config(mod_inbox) -> aff_changes => true, remove_on_kicked => true, reset_markers => [<<"displayed">>], - iqdisc => no_queue}. \ No newline at end of file + iqdisc => no_queue}; +default_mod_config(mod_private) -> + #{iqdisc => one_queue, backend => rdbms}. diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 226f6677d10..f768c4beaf8 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2591,10 +2591,10 @@ mod_private(_Config) -> check_module_defaults(mod_private), T = fun(Opts) -> #{<<"modules">> => #{<<"mod_private">> => Opts}} end, P = [modules, mod_private], - ?cfgh(P ++ [backend], riak, - T(#{<<"backend">> => <<"riak">>})), - ?cfgh(P ++ [riak, bucket_type], <<"private_stuff">>, - T(#{<<"riak">> => #{<<"bucket_type">> => <<"private_stuff">>}})), + ?cfgh(P ++ [backend], riak, T(#{<<"backend">> => <<"riak">>})), + ?cfgh(P ++ [riak, bucket_type], <<"private">>, T(#{<<"backend">> => <<"riak">>})), + ?cfgh(P ++ [riak, bucket_type], <<"private_stuff">>, T(#{<<"backend">> => <<"riak">>, + <<"riak">> => #{<<"bucket_type">> => <<"private_stuff">>}})), ?errh(T(#{<<"backend">> => <<"mssql">>})), ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})).