Put mod_disco opts in a map with defaults

big_tests/tests/disco_and_caps_SUITE.erl

@@ -5,6 +5,7 @@
 -include_lib("escalus/include/escalus_xmlns.hrl").
 
 -import(domain_helper, [host_type/0, domain/0]).
+-import(config_parser_helper, [default_mod_config/1, mod_config/2]).
 
 all() ->
     [{group, disco_with_caps},
@@ -193,14 +194,15 @@ user_can_query_server_info(Config) ->
 
 required_modules(disco_with_caps) ->
     [{mod_caps, []},
-     {mod_disco, []}];
+     {mod_disco, default_mod_config(mod_disco)}];
 required_modules(disco_with_extra_features) ->
-    [{mod_disco, [{extra_domains, [extra_domain()]},
-                  {server_info, [server_info(abuse, []),
-                                 server_info(admin, [{modules, [mod_disco]}]),
-                                 server_info(sales, [{modules, [mod_pubsub]}])]
-                  }]
-     }].
+    [{mod_disco, mod_config(mod_disco, extra_disco_opts())}].
+
+extra_disco_opts() ->
+    #{extra_domains => [extra_domain()],
+      server_info => [server_info(abuse, #{}),
+                      server_info(admin, #{modules => [mod_disco]}),
+                      server_info(sales, #{modules => [mod_pubsub]})]}.
 
 get_form_fields(Stanza) ->
      exml_query:paths(Stanza, [{element_with_ns, <<"query">>, ?NS_DISCO_INFO},
@@ -211,7 +213,7 @@ extra_domain() ->
     <<"eXtra.example.com">>.
 
 server_info(Type, Extra) ->
-    [{name, name(Type)}, {urls, urls(Type)} | Extra].
+    maps:merge(#{name => name(Type), urls => urls(Type)}, Extra).
 
 name(abuse) -> <<"abuse-addresses">>;
 name(admin) -> <<"admin-addresses">>;

big_tests/tests/muc_SUITE.erl

@@ -54,8 +54,8 @@
          ]).
 
 -import(domain_helper, [host_type/0, domain/0]).
-
 -import(mongoose_helper, [backup_and_set_config_option/3, restore_config_option/2]).
+-import(config_parser_helper, [default_mod_config/1]).
 
 -define(PASSWORD, <<"pa5sw0rd">>).
 -define(SUBJECT, <<"subject">>).
@@ -322,7 +322,7 @@ init_per_suite(Config) ->
     mongoose_helper:inject_module(?MODULE),
     Config2 = escalus:init_per_suite(Config),
     Config3 = dynamic_modules:save_modules(host_type(), Config2),
-    dynamic_modules:restart(host_type(), mod_disco, []),
+    dynamic_modules:restart(host_type(), mod_disco, default_mod_config(mod_disco)),
     load_muc(),
     mongoose_helper:ensure_muc_clean(),
     Config3.

doc/modules/mod_disco.md

@@ -5,7 +5,7 @@ Implements [XEP-0030: Service Discovery](http://xmpp.org/extensions/xep-0030.htm
 ## Options
 ### `modules.mod_disco.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_caps.erl

@@ -500,7 +500,7 @@ make_my_disco_hash(HostType, LServer) ->
             <<>>;
         {result, FeaturesXML} ->
             IdentityXML = mongoose_disco:get_local_identity(HostType, JID, JID, <<>>, <<>>),
-            InfoXML = mongoose_disco:get_info(HostType, undefined, <<>>, <<>>),
+            InfoXML = mongoose_disco:get_info(HostType, mod_disco, <<>>, <<>>),
             make_disco_hash(IdentityXML ++ InfoXML ++ FeaturesXML, sha1)
     end.
 

src/mod_disco.erl

@@ -58,10 +58,10 @@
 -include("mongoose_config_spec.hrl").
 
 -type return_hidden() :: ejabberd_router:return_hidden().
+-type server_info() :: #{name := binary(), urls := [binary()], modules => module()}.
 
--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}) ->
     [gen_iq_handler:add_iq_handler_for_domain(HostType, NS, Component, Handler, #{}, IQDisc) ||
         {Component, NS, Handler} <- iq_handlers()],
     ejabberd_hooks:add(hooks(HostType)).
@@ -95,7 +95,12 @@ config_spec() ->
                  <<"server_info">> => #list{items = server_info_spec()},
                  <<"users_can_see_hidden_services">> => #option{type = boolean},
                  <<"iqdisc">> => mongoose_config_spec:iqdisc()
-                }
+                },
+       defaults = #{<<"extra_domains">> => [],
+                    <<"server_info">> => [],
+                    <<"users_can_see_hidden_services">> => true,
+                    <<"iqdisc">> => one_queue},
+       format_items = map
       }.
 
 server_info_spec() ->
@@ -107,7 +112,8 @@ server_info_spec() ->
                  <<"modules">> => #list{items = #option{type = atom,
                                                         validate = module}}
                 },
-       required = [<<"name">>, <<"urls">>]
+       required = [<<"name">>, <<"urls">>],
+       format_items = map
       }.
 
 supported_features() -> [dynamic_domains].
@@ -232,15 +238,16 @@ disco_local_features(Acc) ->
 %% @doc Support for: XEP-0157 Contact Addresses for XMPP Services
 -spec disco_info(mongoose_disco:info_acc()) -> mongoose_disco:info_acc().
 disco_info(Acc = #{host_type := HostType, module := Module, node := <<>>}) ->
-    ServerInfoList = gen_mod:get_module_opt(HostType, ?MODULE, server_info, []),
-    Fields = lists:filtermap(fun(Info) -> process_server_info(Module, Info) end, ServerInfoList),
+    ServerInfoList = gen_mod:get_module_opt(HostType, ?MODULE, server_info),
+    Fields = [server_info_to_field(ServerInfo) || ServerInfo <- ServerInfoList,
+                                                  is_module_allowed(Module, ServerInfo)],
     mongoose_disco:add_info([#{xmlns => ?NS_SERVERINFO, fields => Fields}], Acc);
 disco_info(Acc) ->
     Acc.
 
 -spec get_extra_domains(mongooseim:host_type()) -> [jid:lserver()].
 get_extra_domains(HostType) ->
-    gen_mod:get_module_opt(HostType, ?MODULE, extra_domains, []).
+    gen_mod:get_module_opt(HostType, ?MODULE, extra_domains).
 
 %% Internal functions
 
@@ -249,7 +256,7 @@ should_return_hidden(_HostType, #jid{ luser = <<>> } = _From) ->
     %% We respect "is hidden" flag only when a client performs the query
     all;
 should_return_hidden(HostType, _From) ->
-    case gen_mod:get_module_opt(HostType, ?MODULE, users_can_see_hidden_services, true) of
+    case gen_mod:get_module_opt(HostType, ?MODULE, users_can_see_hidden_services) of
         true -> all;
         false -> only_public
     end.
@@ -331,15 +338,10 @@ make_iq_result(IQ, NameSpace, Node, ChildrenXML) ->
 make_node_attrs(<<>>) -> [];
 make_node_attrs(Node) -> [{<<"node">>, Node}].
 
-process_server_info(Module, ServerInfo) ->
-    case is_module_allowed(Module, proplists:get_value(modules, ServerInfo, all)) of
-        true ->
-            {true, #{var => proplists:get_value(name, ServerInfo),
-                     values => proplists:get_value(urls, ServerInfo)}};
-        false ->
-            false
-    end.
+-spec server_info_to_field(server_info()) -> mongoose_disco:info_field().
+server_info_to_field(#{name := Name, urls := URLs}) ->
+    #{var => Name, values => URLs}.
 
-is_module_allowed(_Module, all) -> true;
-is_module_allowed(undefined, Modules) -> is_module_allowed(?MODULE, Modules);
-is_module_allowed(Module, Modules) -> lists:member(Module, Modules).
+-spec is_module_allowed(module(), server_info()) -> boolean().
+is_module_allowed(Module, #{modules := Modules}) -> lists:member(Module, Modules);
+is_module_allowed(_Module, #{}) -> true.

src/mongoose_disco.erl

@@ -40,7 +40,7 @@
 -type identity() :: #{category := binary(), type := binary(), name => binary()}.
 
 -type info_acc() :: #{host_type := mongooseim:host_type(),
-                      module := module() | undefined,
+                      module := module(),
                       node := binary(),
                       lang := ejabberd:lang(),
                       result := empty | [info()]}.
@@ -121,8 +121,7 @@ get_muc_features(HostType, From, To, Node, Lang, ExtraFeatures) ->
     Features = ExtraFeatures ++ extract_result(FinalAcc),
     features_to_xml(Features).
 
--spec get_info(mongooseim:host_type(), module() | undefined, binary(), ejabberd:lang()) ->
-          [exml:element()].
+-spec get_info(mongooseim:host_type(), module() , binary(), ejabberd:lang()) -> [exml:element()].
 get_info(HostType, Module, Node, Lang) ->
     InitialAcc = new_info_acc(HostType, Module, Node, Lang),
     FinalAcc = mongoose_hooks:disco_info(InitialAcc),

test/common/config_parser_helper.erl

@@ -489,14 +489,14 @@ all_modules() ->
            {is_archivable_message, mod_mam_utils},
            {no_stanzaid_element, true}],
       mod_disco =>
-          [{extra_domains, [<<"some_domain">>, <<"another_domain">>]},
-           {iqdisc, one_queue},
-           {server_info,
-            [[{name, <<"abuse-address">>}, {urls, [<<"admin@example.com">>]}],
-             [{modules, [mod_muc, mod_disco]},
-              {name, <<"friendly-spirits">>},
-              {urls, [<<"spirit1@localhost">>, <<"spirit2@localhost">>]}]]},
-           {users_can_see_hidden_services, true}],
+          mod_config(mod_disco,
+                     #{extra_domains => [<<"some_domain">>, <<"another_domain">>],
+                       server_info =>
+                           [#{name => <<"abuse-address">>,
+                              urls => [<<"admin@example.com">>]},
+                            #{name => <<"friendly-spirits">>,
+                              urls => [<<"spirit1@localhost">>, <<"spirit2@localhost">>],
+                              modules => [mod_muc, mod_disco]}]}),
       mod_last => [{backend, mnesia}, {iqdisc, {queues, 10}}],
       mod_shared_roster_ldap =>
           [{ldap_base, "ou=Users,dc=ejd,dc=com"},
@@ -650,7 +650,7 @@ pgsql_modules() ->
     #{mod_adhoc => default_mod_config(mod_adhoc),
       mod_amp => [], mod_blocking => [], mod_bosh => default_mod_config(mod_bosh),
       mod_carboncopy => [], mod_commands => [],
-      mod_disco => [{users_can_see_hidden_services, false}],
+      mod_disco => mod_config(mod_disco, #{users_can_see_hidden_services => false}),
       mod_last => [{backend, rdbms}],
       mod_muc_commands => [], mod_muc_light_commands => [],
       mod_offline => [{backend, rdbms}],
@@ -809,13 +809,9 @@ default_pool_conn_opts(redis) ->
 default_pool_conn_opts(_Type) ->
     #{}.
 
-default_mod_config(mod_inbox) ->
-    #{backend => rdbms,
-      groupchat => [muclight],
-      aff_changes => true,
-      remove_on_kicked => true,
-      reset_markers => [<<"displayed">>],
-      iqdisc => no_queue};
+mod_config(Module, ExtraOpts) ->
+    maps:merge(default_mod_config(Module), ExtraOpts).
+
 default_mod_config(mod_adhoc) ->
     #{iqdisc => one_queue, report_commands_node => false};
 default_mod_config(mod_auth_token) ->
@@ -825,5 +821,15 @@ default_mod_config(mod_auth_token) ->
 default_mod_config(mod_bosh) ->
     #{backend => mnesia, inactivity => 30, max_wait => infinity,
       server_acks => false, max_pause => 120};
+default_mod_config(mod_disco) ->
+    #{extra_domains => [], server_info => [],
+      users_can_see_hidden_services => true, iqdisc => one_queue};
 default_mod_config(mod_extdisco) ->
-    #{iqdisc => no_queue, service => []}.
+    #{iqdisc => no_queue, service => []};
+default_mod_config(mod_inbox) ->
+    #{backend => rdbms,
+      groupchat => [muclight],
+      aff_changes => true,
+      remove_on_kicked => true,
+      reset_markers => [<<"displayed">>],
+      iqdisc => no_queue}.

test/config_parser_SUITE.erl

@@ -1652,31 +1652,28 @@ mod_csi(_Config) ->
     ?errh(T(<<"buffer_max">>, -1)).
 
 mod_disco(_Config) ->
-    check_iqdisc(mod_disco),
+    check_module_defaults(mod_disco),
+    check_iqdisc_map(mod_disco),
+    P = [modules, mod_disco],
     T = fun(K, V) -> #{<<"modules">> => #{<<"mod_disco">> => #{K => V}}} end,
-    ?cfgh(modopts(mod_disco, [{users_can_see_hidden_services, true}]),
+    ?cfgh(P ++ [users_can_see_hidden_services], true,
           T(<<"users_can_see_hidden_services">>, true)),
-    ?cfgh(modopts(mod_disco, [{users_can_see_hidden_services, false}]),
+    ?cfgh(P ++ [users_can_see_hidden_services], false,
           T(<<"users_can_see_hidden_services">>, false)),
     %% extra_domains are binaries
-    ?cfgh(modopts(mod_disco, [{extra_domains, [<<"localhost">>, <<"erlang-solutions.com">>]}]),
+    ?cfgh(P ++ [extra_domains], [<<"localhost">>, <<"erlang-solutions.com">>],
           T(<<"extra_domains">>, [<<"localhost">>, <<"erlang-solutions.com">>])),
-    ?cfgh(modopts(mod_disco, [{extra_domains, []}]),
+    ?cfgh(P ++ [extra_domains], [],
           T(<<"extra_domains">>, [])),
     Info = #{<<"name">> => <<"abuse-address">>,
              <<"urls">> => [<<"admin@example.com">>]},
     SpiritUrls = [<<"spirit1@localhost">>, <<"spirit2@localhost">>],
-    ?cfgh(modopts(mod_disco, [{server_info, [[{name, <<"abuse-address">>},
-                                              {urls, [<<"admin@example.com">>]}],
-                                             [{modules, [mod_muc, mod_disco]},
-                                              {name, <<"friendly-spirits">>},
-                                              {urls, SpiritUrls}]
-                                            ]}
-                             ]),
-          T(<<"server_info">>, [Info, #{<<"modules">> => [<<"mod_muc">>, <<"mod_disco">>],
-                                        <<"name">> => <<"friendly-spirits">>,
-                                        <<"urls">> => SpiritUrls}
-                               ])),
+    ?cfgh(P ++ [server_info],
+          [#{name => <<"abuse-address">>, urls => [<<"admin@example.com">>]},
+           #{name => <<"friendly-spirits">>, urls => SpiritUrls, modules => [mod_muc, mod_disco]}],
+          T(<<"server_info">>, [Info, #{<<"name">> => <<"friendly-spirits">>,
+                                        <<"urls">> => SpiritUrls,
+                                        <<"modules">> => [<<"mod_muc">>, <<"mod_disco">>]}])),
     ?errh(T(<<"users_can_see_hidden_services">>, 1)),
     ?errh(T(<<"users_can_see_hidden_services">>, <<"true">>)),
     ?errh(T(<<"extra_domains">>, [<<"user@localhost">>])),