Merge pull request #3557 from esl/module-opts-map-last

Module opts map last

big_tests/tests/domain_removal_SUITE.erl

@@ -107,7 +107,7 @@ group_to_modules(markers_removal) ->
 group_to_modules(vcard_removal) ->
     [{mod_vcard, config_parser_helper:mod_config(mod_vcard, #{backend => rdbms})}];
 group_to_modules(last_removal) ->
-    [{mod_last, [{backend, rdbms}]}].
+    [{mod_last, config_parser_helper:mod_config(mod_last, #{backend => rdbms})}].
 
 %%%===================================================================
 %%% Testcase specific setup/teardown

big_tests/tests/last_SUITE.erl

@@ -162,5 +162,10 @@ get_last_activity(Stanza) ->
 get_last_status(Stanza) ->
     exml_query:path(Stanza, [{element, <<"query">>}, cdata]).
 
+required_modules(riak) ->
+    [{mod_last, #{backend => riak,
+                  iqdisc => one_queue,
+                  riak => #{bucket_type => <<"last">>}}}];
 required_modules(Backend) ->
-    [{mod_last, [{backend, Backend}]}].
+    [{mod_last, #{backend => Backend,
+                  iqdisc => one_queue}}].

big_tests/tests/mongooseimctl_SUITE.erl

@@ -193,8 +193,10 @@ init_per_suite(Config) ->
     Config1 = ejabberd_node_utils:init(Node, Config),
     Config2 = escalus:init_per_suite([{ctl_auth_mods, AuthMods},
                                       {roster_template, TemplatePath} | Config1]),
-    dynamic_modules:ensure_modules(domain_helper:host_type(), [{mod_last, []}]),
-    dynamic_modules:ensure_modules(domain_helper:secondary_host_type(), [{mod_last, []}]),
+    dynamic_modules:ensure_modules(domain_helper:host_type(), [{mod_last,
+        config_parser_helper:default_mod_config(mod_last)}]),
+    dynamic_modules:ensure_modules(domain_helper:secondary_host_type(),
+        [{mod_last, config_parser_helper:default_mod_config(mod_last)}]),
     prepare_roster_template(TemplatePath, domain()),
     %% dump_and_load requires at least one mnesia table
     %% ensure, that passwd table is available

doc/modules/mod_last.md

@@ -8,7 +8,7 @@ Use with caution, as it was observed that a user disconnect spike might result i
 
 ### `modules.mod_last.iqdisc.type`
 * **Syntax:** string, one of `"one_queue"`, `"no_queue"`, `"queues"`, `"parallel"`
-* **Default:** `"no_queue"`
+* **Default:** `"one_queue"`
 
 Strategy to handle incoming stanzas. For details, please refer to
 [IQ processing policies](../configuration/Modules.md#iq-processing-policies).

src/mod_last.erl

@@ -44,7 +44,8 @@
          remove_user/3,
          on_presence_update/5,
          session_cleanup/5,
-         remove_domain/3]).
+         remove_domain/3,
+         remove_unused_backend_opts/1]).
 
 %% API
 -export([store_last_info/5,
@@ -71,12 +72,10 @@
 -type timestamp() :: non_neg_integer().
 -type status() :: binary().
 
--spec start(mongooseim:host_type(), list()) -> 'ok'.
-start(HostType, Opts) ->
-    IQDisc = gen_mod:get_opt(iqdisc, Opts, one_queue),
+-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
+start(HostType, #{iqdisc := IQDisc} = Opts) ->
 
     mod_last_backend:init(HostType, Opts),
-
     [gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_LAST, Component, Fn, #{}, IQDisc) ||
         {Component, Fn} <- iq_handlers()],
     ejabberd_hooks:add(hooks(HostType)).
@@ -110,14 +109,24 @@ config_spec() ->
                  <<"backend">> => #option{type = atom,
                                           validate = {module, mod_last}},
                  <<"riak">> => riak_config_spec()
-                }
+                },
+       defaults = #{<<"iqdisc">> => one_queue,
+                    <<"backend">> => mnesia
+                   },
+       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{items = #{<<"bucket_type">> => #option{type = binary,
                                                     validate = non_empty}
                       },
-             wrap = none
+             defaults = #{<<"bucket_type">> => <<"last">>},
+             include = always,
+             format_items = map
             }.
 
 supported_features() -> [dynamic_domains].
@@ -276,6 +285,6 @@ get_last(HostType, LUser, LServer) ->
 count_active_users(HostType, LServer, Timestamp) ->
     mod_last_backend:count_active_users(HostType, LServer, Timestamp).
 
-config_metrics(Host) ->
-    OptsToReport = [{backend, mnesia}], %list of tuples {option, defualt_value}
-    mongoose_module_metrics:opts_for_module(Host, ?MODULE, OptsToReport).
+-spec config_metrics(mongooseim:host_type()) -> [{gen_mod:opt_key(), gen_mod:opt_value()}].
+config_metrics(HostType) ->
+    mongoose_module_metrics:opts_for_module(HostType, ?MODULE, [backend]).

src/mod_last_riak.erl

@@ -111,7 +111,7 @@ remove_domain(_HostType, _Domain) ->
 
 -spec bucket_type(host_type(), jid:lserver()) -> riakc_obj:bucket().
 bucket_type(HostType, LServer) ->
-    {gen_mod:get_module_opt(HostType, mod_last, bucket_type, <<"last">>), LServer}.
+    {gen_mod:get_module_opt(HostType, mod_last, [riak, bucket_type]), LServer}.
 
 -spec infinity() -> non_neg_integer().
 infinity() ->

test/common/config_parser_helper.erl

@@ -501,7 +501,7 @@ all_modules() ->
                             #{name => <<"friendly-spirits">>,
                               urls => [<<"spirit1@localhost">>, <<"spirit2@localhost">>],
                               modules => [mod_muc, mod_disco]}]}),
-      mod_last => [{backend, mnesia}, {iqdisc, {queues, 10}}],
+      mod_last => #{backend => mnesia, iqdisc => {queues, 10}},
       mod_shared_roster_ldap =>
           [{ldap_base, "ou=Users,dc=ejd,dc=com"},
            {ldap_filter, "(objectClass=inetOrgPerson)"},
@@ -660,7 +660,7 @@ pgsql_modules() ->
       mod_amp => [], mod_blocking => [], mod_bosh => default_mod_config(mod_bosh),
       mod_carboncopy => [], mod_commands => [],
       mod_disco => mod_config(mod_disco, #{users_can_see_hidden_services => false}),
-      mod_last => [{backend, rdbms}],
+      mod_last => mod_config(mod_last, #{backend => rdbms}),
       mod_muc_commands => [], mod_muc_light_commands => [],
       mod_offline => [{backend, rdbms}],
       mod_privacy => [{backend, rdbms}],
@@ -835,6 +835,8 @@ default_mod_config(mod_disco) ->
       users_can_see_hidden_services => true, iqdisc => one_queue};
 default_mod_config(mod_extdisco) ->
     #{iqdisc => no_queue, service => []};
+default_mod_config(mod_last) ->
+    #{iqdisc => one_queue, backend => mnesia};
 default_mod_config(mod_inbox) ->
     #{backend => rdbms,
       groupchat => [muclight],

test/config_parser_SUITE.erl

@@ -2120,14 +2120,15 @@ mod_keystore_keys(_Config) ->
                            <<"path">> => <<"does/not/exists">>}])).
 
 mod_last(_Config) ->
-    check_iqdisc(mod_last),
+    check_iqdisc_map(mod_last),
+    check_module_defaults(mod_last),
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_last">> => Opts}} end,
-    M = fun(Cfg) -> modopts(mod_last, Cfg) end,
-    ?cfgh(M([{backend, mnesia}]),
-          T(#{<<"backend">> => <<"mnesia">>})),
-    ?cfgh(M([{bucket_type, <<"test">>}]),
-          T(#{<<"riak">> => #{<<"bucket_type">> => <<"test">>}})),
-
+    P = [modules, mod_last],
+    ?cfgh(P ++ [backend], mnesia, T(#{<<"backend">> => <<"mnesia">>})),
+    ?cfgh(P ++ [backend], rdbms, T(#{<<"backend">> => <<"rdbms">>})),
+    ?cfgh(P ++ [riak, bucket_type], <<"last">>, T(#{<<"backend">> => <<"riak">>})),
+    ?cfgh(P ++ [riak, bucket_type], <<"test">>,
+          T(#{<<"backend">> => <<"riak">>, <<"riak">> => #{<<"bucket_type">> => <<"test">>}})),
     ?errh(T(#{<<"backend">> => <<"frontend">>})),
     ?errh(T(#{<<"riak">> => #{<<"bucket_type">> => 1}})).
 

test/mongoose_cleanup_SUITE.erl

@@ -106,7 +106,7 @@ auth_anonymous(_Config) ->
 last(_Config) ->
     HostType = host_type(),
     {U, S, R, _JID, SID} = get_fake_session(),
-    mod_last:start(HostType, [{backend, mnesia}, {iqdisc, no_queue}]),
+    mod_last:start(HostType, config_parser_helper:mod_config(mod_last, #{iqdisc => no_queue})),
     not_found = mod_last:get_last_info(HostType, U, S),
     Status1 = <<"status1">>,
     #{} = mod_last:on_presence_update(new_acc(S), U, S, R, Status1),