Merge pull request #3569 from esl/mod_roster-map-config

Put mod_roster config options in a map with defaults

big_tests/tests/domain_removal_SUITE.erl

@@ -99,7 +99,7 @@ group_to_modules(inbox_removal) ->
 group_to_modules(private_removal) ->
     [{mod_private, #{iqdisc => one_queue, backend => rdbms}}];
 group_to_modules(roster_removal) ->
-    [{mod_roster, [{backend, rdbms}]}];
+    [{mod_roster, config_parser_helper:mod_config(mod_roster, #{backend => rdbms})}];
 group_to_modules(offline_removal) ->
     [{mod_offline, [{backend, rdbms}]}];
 group_to_modules(markers_removal) ->

big_tests/tests/gdpr_SUITE.erl

@@ -294,8 +294,7 @@ init_per_testcase(CN, Config) when CN =:= retrieve_mam_muc;
             escalus:init_per_testcase(CN, [{mam_modules, RequiredModules} | Config])
     end;
 init_per_testcase(remove_roster = CN, Config) ->
-    Backend = pick_enabled_backend(),
-    dynamic_modules:ensure_modules(host_type(), [{mod_roster, [{backend, Backend}]}]),
+    roster_started(),
     escalus:init_per_testcase(CN, Config);
 init_per_testcase(CN, Config) ->
     GN = proplists:get_value(group, Config),
@@ -391,6 +390,15 @@ pick_enabled_backend() ->
     ],
     proplists:get_value(true, BackendsList, mnesia).
 
+roster_required_modules() ->
+    Backend = pick_enabled_backend(),
+    [{mod_roster, roster_backend_opts(Backend)}].
+
+roster_backend_opts(riak) ->
+    RiakDefaults = config_parser_helper:default_config([modules, mod_roster, riak]),
+    config_parser_helper:mod_config(mod_roster, #{backend => riak, riak => RiakDefaults});
+roster_backend_opts(Backend) ->
+    config_parser_helper:mod_config(mod_roster, #{backend => Backend}).
 
 vcard_required_modules() ->
     Backend = pick_enabled_backend(),
@@ -424,6 +432,9 @@ is_mim2_started() ->
         _ -> false
     end.
 
+roster_started() ->
+    dynamic_modules:ensure_modules(host_type(), roster_required_modules()).
+
 vcard_started() ->
     dynamic_modules:ensure_modules(host_type(), vcard_required_modules()).
 

big_tests/tests/metrics_roster_SUITE.erl

@@ -118,16 +118,15 @@ get_roster(ConfigIn) ->
         ],
     Config = mongoose_metrics(ConfigIn, Metrics),
 
-    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice,_Bob) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice,_Bob) ->
         escalus_client:send(Alice, escalus_stanza:roster_get()),
         escalus_client:wait_for_stanza(Alice)
-
         end).
 
 add_contact(ConfigIn) ->
     Config = mongoose_metrics(ConfigIn, [{['_', modRosterSets], 1}]),
 
-    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) ->
 
         %% add contact
         escalus_client:send(Alice,
@@ -144,7 +143,7 @@ roster_push(ConfigIn) ->
     Config = mongoose_metrics(ConfigIn, [{['_', modRosterSets], 1},
                                          {['_', modRosterPush], 2}]),
 
-    escalus:story(Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) ->
+    escalus:fresh_story(Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) ->
 
         %% add contact
         escalus_client:send(Alice1,
@@ -164,7 +163,7 @@ roster_push(ConfigIn) ->
 subscribe(ConfigIn) ->
     Config = mongoose_metrics(ConfigIn, [{['_', modPresenceSubscriptions], 1}]),
 
-    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
         BobJid = escalus_client:short_jid(Bob),
         AliceJid = escalus_client:short_jid(Alice),
 
@@ -203,7 +202,7 @@ subscribe(ConfigIn) ->
 decline_subscription(ConfigIn) ->
     Config = mongoose_metrics(ConfigIn, [{['_', modPresenceUnsubscriptions], 1}]),
 
-    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
         BobJid = escalus_client:short_jid(Bob),
         AliceJid = escalus_client:short_jid(Alice),
 
@@ -230,7 +229,7 @@ decline_subscription(ConfigIn) ->
 unsubscribe(ConfigIn) ->
     Config = mongoose_metrics(ConfigIn, [{['_', modPresenceUnsubscriptions], 1}]),
 
-    escalus:story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
+    escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun(Alice,Bob) ->
         BobJid = escalus_client:short_jid(Bob),
         AliceJid = escalus_client:short_jid(Alice),
 

big_tests/tests/roster_helper.erl

@@ -7,6 +7,6 @@
 -spec set_versioning(boolean(), boolean(), escalus_config:config()) -> escalus_config:config().
 set_versioning(Versioning, VersionStore, Config) ->
     Opts = dynamic_modules:get_saved_config(host_type(), mod_roster, Config),
-    dynamic_modules:ensure_modules(host_type(), [{mod_roster, [{versioning, Versioning},
-                                                               {store_current_id, VersionStore} | Opts]}]),
+    dynamic_modules:ensure_modules(host_type(), [{mod_roster, Opts#{versioning => Versioning,
+                                                                    store_current_id => VersionStore}}]),
     Config.

src/gen_mod.erl

@@ -59,7 +59,7 @@
 
 -export([is_app_running/1]). % we have to mock it in some tests
 
--ignore_xref([behaviour_info/1, loaded_modules_with_opts/0,
+-ignore_xref([loaded_modules_with_opts/0,
               loaded_modules_with_opts/1, hosts_and_opts_with_module/1]).
 
 -include("mongoose.hrl").
@@ -73,12 +73,6 @@
 -type module_opts() :: [{opt_key(), opt_value()}] % deprecated, will be removed
                      | #{opt_key() => opt_value()}. % recommended
 
-%% -export([behaviour_info/1]).
-%% behaviour_info(callbacks) ->
-%%     [{start, 2},
-%%      {stop, 1}];
-%% behaviour_info(_Other) ->
-%%     undefined.
 -callback start(HostType :: host_type(), Opts :: module_opts()) -> any().
 -callback stop(HostType :: host_type()) -> any().
 -callback supported_features() -> [module_feature()].

src/mod_roster.erl

@@ -23,7 +23,7 @@
 %%%
 %%%----------------------------------------------------------------------
 
-%%% @doc Roster management (Mnesia storage).
+%%% @doc Roster management.
 %%%
 %%% Includes support for XEP-0237: Roster Versioning.
 %%% The roster versioning follows an all-or-nothing strategy:
@@ -53,7 +53,7 @@
          item_to_xml/1
         ]).
 
-% Main hooks
+% Hook handlers
 -export([
          get_user_roster/2,
          in_subscription/5,
@@ -73,7 +73,7 @@
 -export([config_metrics/1]).
 
 -ignore_xref([
-    behaviour_info/1, get_jid_info/4, get_personal_data/3, get_subscription_lists/2,
+    get_jid_info/4, get_personal_data/3, get_subscription_lists/2,
     get_user_roster/2, get_user_rosters_length/2, get_versioning_feature/2,
     in_subscription/5, item_to_xml/1, out_subscription/4, process_subscription_t/6,
     remove_user/3, remove_domain/3, transaction/2
@@ -133,9 +133,8 @@ roster_record_to_gdpr_entry(#roster{ jid = JID, name = Name,
 %% mod_roster's callbacks
 %%--------------------------------------------------------------------
 
--spec start(mongooseim:host_type(), list()) -> any().
-start(HostType, Opts) ->
-    IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue),
+-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> any().
+start(HostType, Opts = #{iqdisc := IQDisc}) ->
     mod_roster_backend:init(HostType, Opts),
     ejabberd_hooks:add(hooks(HostType)),
     gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_ROSTER, ejabberd_sm,
@@ -155,16 +154,28 @@ config_spec() ->
                  <<"backend">> => #option{type = atom,
                                           validate = {module, mod_roster}},
                  <<"riak">> => riak_config_spec()
-                }
+                },
+       format_items = map,
+       defaults = #{<<"iqdisc">> => one_queue,
+                    <<"versioning">> => false,
+                    <<"store_current_id">> => false,
+                    <<"backend">> => mnesia},
+       process = fun remove_unused_backend_opts/1
       }.
 
 riak_config_spec() ->
     #section{items = #{<<"bucket_type">> => #option{type = binary,
                                                     validate = non_empty},
                        <<"version_bucket_type">> => #option{type = binary,
                                                             validate = non_empty}},
-             wrap = none
-            }.
+             include = always,
+             format_items = map,
+             defaults = #{<<"bucket_type">> => <<"rosters">>,
+                          <<"version_bucket_type">> => <<"roster_versions">>}
+    }.
+
+remove_unused_backend_opts(Opts = #{backend := riak}) -> Opts;
+remove_unused_backend_opts(Opts) -> maps:remove(riak, Opts).
 
 supported_features() -> [dynamic_domains].
 
@@ -964,9 +975,9 @@ item_to_map(#roster{} = Roster) ->
     #{jid => ContactJid, name => ContactName, subscription => Subs,
       groups => Groups, ask => Ask}.
 
+-spec config_metrics(mongooseim:host_type()) -> [{gen_mod:opt_key(), gen_mod:opt_value()}].
 config_metrics(HostType) ->
-    OptsToReport = [{backend, mnesia}], % list of tuples {option, default_value}
-    mongoose_module_metrics:opts_for_module(HostType, ?MODULE, OptsToReport).
+    mongoose_module_metrics:opts_for_module(HostType, ?MODULE, [backend]).
 
 %% Backend API wrappers
 

src/mod_roster_backend.erl

@@ -20,7 +20,7 @@
 %% Callbacks
 %% (exactly the same as specs in this module)
 
--callback init(mongooseim:host_type(), list()) -> ok.
+-callback init(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 
 -callback transaction(mongooseim:host_type(), fun(() -> any())) ->
     {aborted, any()} | {atomic, any()} | {error, any()}.
@@ -51,7 +51,7 @@
 
 -optional_callbacks([remove_domain_t/2]).
 
--spec init(mongooseim:host_type(), list()) -> ok.
+-spec init(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 init(HostType, Opts) ->
     TrackedFuns = [read_roster_version,
                    write_roster_version,

src/mod_roster_mnesia.erl

@@ -28,7 +28,7 @@
          del_roster_t/4,
          remove_user_t/3]).
 
--spec init(mongooseim:host_type(), list()) -> ok.
+-spec init(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 init(_HostType, _Opts) ->
     mnesia:create_table(roster,
                         [{disc_copies, [node()]},

src/mod_roster_rdbms.erl

@@ -33,7 +33,7 @@
 
 %% mod_roster backend API
 
--spec init(mongooseim:host_type(), list()) -> ok.
+-spec init(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 init(HostType, _Opts) ->
     prepare_queries(HostType),
     ok.

src/mod_roster_riak.erl

@@ -29,16 +29,16 @@
          remove_user_t/3]).
 
 -define(ROSTER_BUCKET(HostType, LServer),
-        {get_opt(HostType, bucket_type, <<"rosters">>), LServer}).
+        {get_opt(HostType, [riak, bucket_type]), LServer}).
 -define(VER_BUCKET(HostType, LServer),
-        {get_opt(HostType, version_bucket_type, <<"roster_versions">>), LServer}).
+        {get_opt(HostType, [riak, version_bucket_type]), LServer}).
 
-get_opt(HostType, Opt, Def) ->
-    gen_mod:get_module_opt(HostType, mod_roster, Opt, Def).
+get_opt(HostType, Opt) ->
+    gen_mod:get_module_opt(HostType, mod_roster, Opt).
 
 %% --------------------- mod_roster backend API -------------------------------
 
--spec init(mongooseim:host_type(), list()) -> ok.
+-spec init(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
 init(_HostType, _Opts) ->
     ok. % Common Riak pool is used
 

test/common/config_parser_helper.erl

@@ -574,7 +574,7 @@ all_modules() ->
            {region, "eu-west-1"},
            {secret_access_key, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"},
            {sns_host, "sns.eu-west-1.amazonaws.com"}],
-      mod_roster => [{store_current_id, true}, {versioning, true}],
+      mod_roster => mod_config(mod_roster, #{store_current_id => true, versioning => true}),
       mod_event_pusher_http =>
           [{configs,
             [[{callback_module, mod_event_pusher_http_defaults},
@@ -670,7 +670,7 @@ pgsql_modules() ->
           [{access, register},
            {ip_access, [{allow, "127.0.0.0/8"}, {deny, "0.0.0.0/0"}]},
            {welcome_message, {"Hello", "I am MongooseIM"}}],
-      mod_roster => [{backend, rdbms}],
+      mod_roster => mod_config(mod_roster, #{backend => rdbms}),
       mod_sic => default_mod_config(mod_sic), mod_stream_management => [],
       mod_vcard => mod_config(mod_vcard, #{backend => rdbms, host => {prefix, <<"vjud.">>}})}.
 
@@ -848,6 +848,8 @@ default_mod_config(mod_inbox) ->
       iqdisc => no_queue};
 default_mod_config(mod_private) ->
     #{iqdisc => one_queue, backend => rdbms};
+default_mod_config(mod_roster) ->
+    #{iqdisc => one_queue, versioning => false, store_current_id => false, backend => mnesia};
 default_mod_config(mod_sic) ->
     #{iqdisc => one_queue};
 default_mod_config(mod_time) ->
@@ -931,7 +933,9 @@ default_config([modules, mod_mam_meta, async_writer]) ->
     #{batch_size => 30, enabled => true, flush_interval => 2000,
       pool_size => 4 * erlang:system_info(schedulers_online)};
 default_config([modules, mod_mam_meta, riak]) ->
-    #{bucket_type => <<"mam_yz">>, search_index => <<"mam">>}.
+    #{bucket_type => <<"mam_yz">>, search_index => <<"mam">>};
+default_config([modules, mod_roster, riak]) ->
+    #{bucket_type => <<"rosters">>, version_bucket_type => <<"roster_versions">>}.
 
 common_mam_config() ->
     #{no_stanzaid_element => false,

test/config_parser_SUITE.erl

@@ -2806,27 +2806,29 @@ registration_watchers(JidBins) ->
     #{<<"modules">> => #{<<"mod_register">> => Opts}}.
 
 mod_roster(_Config) ->
+    check_iqdisc_map(mod_roster),
+    check_module_defaults(mod_roster),
+    P = [modules, mod_roster],
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_roster">> => Opts}} end,
-    M = fun(Cfg) -> modopts(mod_roster, Cfg) end,
-
-    ?cfgh(M([{versioning, false}]),
-          T(#{<<"versioning">> => false})),
-    ?cfgh(M([{store_current_id, false}]),
-          T(#{<<"store_current_id">> => false})),
-    ?cfgh(M([{backend, mnesia}]),
-          T(#{<<"backend">> => <<"mnesia">>})),
-    ?cfgh(M([{bucket_type, <<"rosters">>}]),
-          T(#{<<"riak">> => #{<<"bucket_type">> => <<"rosters">>}})),
-    ?cfgh(M([{version_bucket_type, <<"roster_versions">>}]),
-          T(#{<<"riak">> => #{<<"version_bucket_type">> => <<"roster_versions">>}})),
+    ?cfgh(P ++ [versioning],  true,
+          T(#{<<"versioning">> => true})),
+    ?cfgh(P ++ [store_current_id], true,
+          T(#{<<"store_current_id">> => true})),
+    ?cfgh(P ++ [backend], rdbms,
+          T(#{<<"backend">> => <<"rdbms">>})),
+    ?cfgh(P ++ [riak], config_parser_helper:default_config(P ++ [riak]),
+          T(#{<<"backend">> => <<"riak">>})),
+    ?cfgh(P ++ [riak, bucket_type], <<"my_type">>,
+          T(#{<<"backend">> => <<"riak">>, <<"riak">> => #{<<"bucket_type">> => <<"my_type">>}})),
+    ?cfgh(P ++ [riak, version_bucket_type], <<"my_versions">>,
+          T(#{<<"backend">> => <<"riak">>, <<"riak">> => #{<<"version_bucket_type">> => <<"my_versions">>}})),
 
     ?errh(T(#{<<"versioning">> => 1})),
     ?errh(T(#{<<"store_current_id">> => 1})),
     ?errh(T(#{<<"backend">> => 1})),
     ?errh(T(#{<<"backend">> => <<"iloveyou">>})),
     ?errh(T(#{<<"riak">> => #{<<"version_bucket_type">> => 1}})),
-    ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})),
-    check_iqdisc(mod_roster).
+    ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})).
 
 mod_shared_roster_ldap(_Config) ->
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_shared_roster_ldap">> => Opts}} end,

test/roster_SUITE.erl

@@ -72,7 +72,7 @@ opts() ->
     [{hosts, [host_type()]},
      {host_types, []},
      {all_metrics_are_global, false},
-     {{modules, host_type()}, #{mod_roster => []}}].
+     {{modules, host_type()}, #{mod_roster => config_parser_helper:default_mod_config(mod_roster)}}].
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%%%%%%%%%%%%%%%%%%%%%%%%%% TESTS %%%%%%%%%%%%%%%%%%%%%%%