Merge pull request #3547 from esl/module-opts-map-inbox

Put mod_inbox in a map with defaults

big_tests/tests/domain_removal_SUITE.erl

@@ -91,7 +91,7 @@ group_to_modules(muc_removal) ->
     MucHost = subhost_pattern(muc_helper:muc_host_pattern()),
     [{mod_muc, [{backend, rdbms}, {host, MucHost}]}];
 group_to_modules(inbox_removal) ->
-    [{mod_inbox, []}];
+    [{mod_inbox, inbox_helper:inbox_opts()}];
 group_to_modules(private_removal) ->
     [{mod_private, [{backend, rdbms}]}];
 group_to_modules(roster_removal) ->

big_tests/tests/gdpr_SUITE.erl

@@ -344,10 +344,8 @@ init_inbox(CN, Config, GroupChatType) ->
     end.
 inbox_required_modules(Type) ->
     GroupChatModules = groupchat_module(Type),
-    Inbox = {mod_inbox, [{aff_changes, true},
-                         {remove_on_kicked, true},
-                         {groupchat, [Type]},
-                         {markers, [displayed]}]},
+    InboxOpts = (inbox_helper:inbox_opts())#{groupchat => [Type]},
+    Inbox = {mod_inbox, InboxOpts},
      GroupChatModules ++ [Inbox] .
 
 groupchat_module(muc) ->

big_tests/tests/inbox_SUITE.erl

@@ -139,11 +139,7 @@ init_per_suite(Config0) ->
            domain_helper:host_type(), inbox_modules()),
     ok = dynamic_modules:ensure_modules(
            ct:get_config({hosts, mim, secondary_host_type}),
-           [{mod_inbox,
-             [{aff_changes, false},
-              {remove_on_kicked, true},
-              {groupchat, [muclight]},
-              {markers, [displayed]}]}]),
+           [{mod_inbox, (inbox_opts())#{aff_changes := false}}]),
     InboxOptions = inbox_opts(),
     mongoose_helper:inject_module(?MODULE),
     escalus:init_per_suite([{inbox_opts, InboxOptions} | Config1]).

big_tests/tests/inbox_helper.erl

@@ -118,10 +118,7 @@ inbox_ns_conversation() ->
     ?NS_ESL_INBOX_CONVERSATION.
 
 inbox_opts() ->
-    [{aff_changes, true},
-     {remove_on_kicked, true},
-     {groupchat, [muclight]},
-     {reset_markers, [<<"displayed">>]}].
+    config_parser_helper:default_mod_config(mod_inbox).
 
 skip_or_run_inbox_tests(TestCases) ->
     case (not ct_helper:is_ct_running())
@@ -294,16 +291,14 @@ clear_inboxes(UserList) ->
 reload_inbox_option(Config, KeyValueList) ->
     HostType = domain_helper:host_type(mim),
     Args = proplists:get_value(inbox_opts, Config),
-    Args2 = lists:foldl(fun({K, V}, AccIn) ->
-        lists:keyreplace(K, 1, AccIn, {K, V})
-                end, Args, KeyValueList),
-    dynamic_modules:restart(HostType, mod_inbox, Args2),
-    lists:keyreplace(inbox_opts, 1, Config, {inbox_opts, Args2}).
+    Args1 = maps:merge(Args, maps:from_list(KeyValueList)),
+    dynamic_modules:restart(HostType, mod_inbox, Args1),
+    lists:keyreplace(inbox_opts, 1, Config, {inbox_opts, Args1}).
 
 reload_inbox_option(Config, Key, Value) ->
     HostType = domain_helper:host_type(mim),
     Args = proplists:get_value(inbox_opts, Config),
-    Args1 = lists:keyreplace(Key, 1, Args, {Key, Value}),
+    Args1 = Args#{Key => Value},
     dynamic_modules:restart(HostType, mod_inbox, Args1),
     lists:keyreplace(inbox_opts, 1, Config, {inbox_opts, Args1}).
 

big_tests/tests/push_integration_SUITE.erl

@@ -1022,8 +1022,4 @@ muc_light_opts() ->
      {rooms_in_rosters, true}
     ].
 inbox_opts() ->
-    [{aff_changes, false},
-     {remove_on_kicked, true},
-     {groupchat, [muclight]},
-     {markers, [displayed]}].
-
+    (inbox_helper:inbox_opts())#{aff_changes := false}.

src/inbox/mod_inbox.erl

@@ -19,11 +19,7 @@
 -export([get_personal_data/3]).
 
 %% gen_mod
--export([start/2]).
--export([stop/1]).
--export([supported_features/0]).
--export([deps/2]).
--export([config_spec/0]).
+-export([start/2, stop/1, deps/2, config_spec/0, supported_features/0]).
 
 -export([process_iq/5,
          user_send_packet/4,
@@ -89,14 +85,12 @@ process_entry(#{remote_jid := RemJID,
 %%--------------------------------------------------------------------
 -spec deps(jid:lserver(), list()) -> gen_mod:deps_list().
 deps(_Host, Opts) ->
-    groupchat_deps(Opts).
-
--spec start(HostType :: mongooseim:host_type(), Opts :: list()) -> ok.
-start(HostType, Opts) ->
-    FullOpts = add_default_backend(Opts),
-    IQDisc = gen_mod:get_opt(iqdisc, FullOpts, no_queue),
-    MucTypes = gen_mod:get_opt(groupchat, FullOpts, [muclight]),
-    mod_inbox_backend:init(HostType, FullOpts),
+    Groupchats = gen_mod:get_opt(groupchat, Opts),
+    muclight_dep(Groupchats) ++ muc_dep(Groupchats).
+
+-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok.
+start(HostType, #{iqdisc := IQDisc, groupchat := MucTypes} = Opts) ->
+    mod_inbox_backend:init(HostType, Opts),
     lists:member(muc, MucTypes) andalso mod_inbox_muc:start(HostType),
     ejabberd_hooks:add(hooks(HostType)),
     gen_iq_handler:add_iq_handler_for_domain(HostType, ?NS_ESL_INBOX, ejabberd_sm,
@@ -105,8 +99,6 @@ start(HostType, Opts) ->
                                              fun mod_inbox_entries:process_iq_conversation/5, #{}, IQDisc),
     ok.
 
-
-
 -spec stop(HostType :: mongooseim:host_type()) -> ok.
 stop(HostType) ->
     mod_inbox_muc:stop(HostType),
@@ -122,14 +114,23 @@ supported_features() ->
 config_spec() ->
     Markers = mongoose_chat_markers:chat_marker_names(),
     #section{
-        items = #{<<"reset_markers">> => #list{items = #option{type = binary,
+        items = #{<<"backend">> => #option{type = atom, validate = {enum, [rdbms]}},
+                  <<"reset_markers">> => #list{items = #option{type = binary,
                                                                validate = {enum, Markers}}},
                   <<"groupchat">> => #list{items = #option{type = atom,
                                                            validate = {enum, [muc, muclight]}}},
                   <<"aff_changes">> => #option{type = boolean},
                   <<"remove_on_kicked">> => #option{type = boolean},
                   <<"iqdisc">> => mongoose_config_spec:iqdisc()
-        }
+        },
+        defaults = #{<<"backend">> => rdbms,
+                     <<"groupchat">> => [muclight],
+                     <<"aff_changes">> => true,
+                     <<"remove_on_kicked">> => true,
+                     <<"reset_markers">> => [<<"displayed">>],
+                     <<"iqdisc">> => no_queue
+                    },
+        format_items = map
     }.
 
 %%%%%%%%%%%%%%%%%%%
@@ -526,27 +527,13 @@ hooks(HostType) ->
      {disco_local_features, HostType, ?MODULE, disco_local_features, 99}
     ].
 
-add_default_backend(Opts) ->
-    case lists:keyfind(backend, 1, Opts) of
-        false -> [{backend, rdbms} | Opts];
-        _ -> Opts
-    end.
-
 get_groupchat_types(HostType) ->
     gen_mod:get_module_opt(HostType, ?MODULE, groupchat, [muclight]).
 
 config_metrics(HostType) ->
     OptsToReport = [{backend, rdbms}], %list of tuples {option, defualt_value}
     mongoose_module_metrics:opts_for_module(HostType, ?MODULE, OptsToReport).
 
-groupchat_deps(Opts) ->
-    case lists:keyfind(groupchat, 1, Opts) of
-        {groupchat, List} ->
-            muclight_dep(List) ++ muc_dep(List);
-        false ->
-            []
-    end.
-
 muclight_dep(List) ->
     case lists:member(muclight, List) of
         true -> [{mod_muc_light, hard}];

src/inbox/mod_inbox_backend.erl

@@ -18,7 +18,7 @@
 
 -callback init(HostType, Opts) -> ok when
     HostType :: mongooseim:host_type(),
-    Opts :: list().
+    Opts :: gen_mod:module_opts().
 
 -callback get_inbox(HostType, LUser, LServer, Params) -> mod_inbox:get_inbox_res() when
     HostType :: mongooseim:host_type(),
@@ -78,7 +78,7 @@
 
 -spec init(HostType, Opts) -> ok when
     HostType :: mongooseim:host_type(),
-    Opts :: list().
+    Opts :: gen_mod:module_opts().
 init(HostType, Opts) ->
     mongoose_backend:init(HostType, ?MAIN_MODULE, callback_funs(), Opts),
     Args = [HostType, Opts],

src/inbox/mod_inbox_entries.erl

@@ -1,6 +1,5 @@
 -module(mod_inbox_entries).
 
--include("mongoose_logger.hrl").
 -include("mongoose_ns.hrl").
 -include("jlib.hrl").
 -include("mod_inbox.hrl").

src/inbox/mod_inbox_utils.erl

@@ -109,7 +109,7 @@ clear_inbox(HostType, User, Server) when is_binary(User) ->
 
 -spec get_reset_markers(HostType :: mongooseim:host_type()) -> list(marker()).
 get_reset_markers(HostType) ->
-    gen_mod:get_module_opt(HostType, mod_inbox, reset_markers, [<<"displayed">>]).
+    gen_mod:get_module_opt(HostType, mod_inbox, reset_markers).
 
 -spec if_chat_marker_get_id(Packet :: exml:element(),
                             Markers :: list(marker())) -> undefined | id().
@@ -173,11 +173,11 @@ wrapper_id() ->
 
 -spec get_option_write_aff_changes(HostType :: mongooseim:host_type()) -> boolean().
 get_option_write_aff_changes(HostType) ->
-    gen_mod:get_module_opt(HostType, mod_inbox, aff_changes, true).
+    gen_mod:get_module_opt(HostType, mod_inbox, aff_changes).
 
 -spec get_option_remove_on_kicked(HostType :: mongooseim:host_type()) -> boolean().
 get_option_remove_on_kicked(HostType) ->
-    gen_mod:get_module_opt(HostType, mod_inbox, remove_on_kicked, true).
+    gen_mod:get_module_opt(HostType, mod_inbox, remove_on_kicked).
 
 extract_attr_jid(ResetStanza) ->
     case exml_query:attr(ResetStanza, <<"jid">>) of

test/common/config_parser_helper.erl

@@ -575,10 +575,12 @@ all_modules() ->
               {path, "/notifications"},
               {pool_name, http_pool}]]}],
       mod_inbox =>
-          [{aff_changes, true},
-           {groupchat, [muclight]},
-           {remove_on_kicked, true},
-           {reset_markers, [<<"displayed">>]}],
+          #{backend => rdbms,
+            iqdisc => no_queue,
+            aff_changes => true,
+            groupchat => [muclight],
+            remove_on_kicked => true,
+            reset_markers => [<<"displayed">>]},
       mod_mam_meta =>
           [{archive_chat_markers, true},
            {backend, rdbms},
@@ -752,6 +754,13 @@ pgsql_access() ->
       register => [#{acl => all, value => allow}],
       s2s_shaper => [#{acl => all, value => fast}]}.
 
+default_mod_config(mod_inbox) ->
+    #{backend => rdbms,
+      groupchat => [muclight],
+      aff_changes => true,
+      remove_on_kicked => true,
+      reset_markers => [<<"displayed">>],
+      iqdisc => no_queue};
 default_mod_config(mod_adhoc) ->
     #{iqdisc => one_queue, report_commands_node => false};
 default_mod_config(mod_auth_token) ->

test/config_parser_SUITE.erl

@@ -1723,23 +1723,24 @@ mod_extdisco(_Config) ->
     ?errh(T(RequiredOpts#{<<"password">> => <<>>})).
 
 mod_inbox(_Config) ->
+    check_module_defaults(mod_inbox),
+    check_iqdisc_map(mod_inbox),
+    P = [modules, mod_inbox],
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_inbox">> => Opts}} end,
-    M = fun(Opts) -> modopts(mod_inbox, Opts) end,
     ChatMarkers = [<<"displayed">>, <<"received">>, <<"acknowledged">>],
-    ?cfgh(M([{reset_markers, ChatMarkers}]),
+    ?cfgh(P ++ [reset_markers], ChatMarkers,
           T(#{<<"reset_markers">> => ChatMarkers})),
-    ?cfgh(M([{groupchat, [muc, muclight]}]),
+    ?cfgh(P ++ [groupchat], [muc, muclight],
           T(#{<<"groupchat">> => [<<"muc">>, <<"muclight">>]})),
-    ?cfgh(M([{aff_changes, true}]),
+    ?cfgh(P ++ [aff_changes], true,
           T(#{<<"aff_changes">> => true})),
-    ?cfgh(M([{remove_on_kicked, false}]),
+    ?cfgh(P ++ [remove_on_kicked], false,
           T(#{<<"remove_on_kicked">> => false})),
     ?errh(T(#{<<"reset_markers">> => 1})),
     ?errh(T(#{<<"reset_markers">> => [<<"destroyed">>]})),
     ?errh(T(#{<<"groupchat">> => [<<"test">>]})),
     ?errh(T(#{<<"aff_changes">> => 1})),
-    ?errh(T(#{<<"remove_on_kicked">> => 1})),
-    check_iqdisc(mod_inbox).
+    ?errh(T(#{<<"remove_on_kicked">> => 1})).
 
 mod_global_distrib(_Config) ->
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_global_distrib">> => Opts}} end,
@@ -3209,7 +3210,7 @@ maybe_insert_dummy_domain(M) ->
 -spec host_opts([{key_prefix(), mongoose_config:value()}]) ->
           [{mongoose_config:key() | mongoose_config:key_path(), mongoose_config:value()}].
 host_opts(ExpectedOptions) ->
-    [{host_key(Key), Value} || {Key, Value} <- ExpectedOptions].
+    lists:map(fun({Key, Value}) -> {host_key(Key), Value} end, ExpectedOptions).
 
 %% @doc Build full per-host config key for host-or-global options
 -spec host_key(top_level_key_prefix()) -> mongoose_config:key();