Merge pull request #3534 from esl/module-opts-as-map

big_tests/dynamic_domains.spec

@@ -33,6 +33,8 @@
 
 {suites, "tests", dynamic_domains_SUITE}.
 
+{suites, "tests", extdisco_SUITE}.
+
 {suites, "tests", gdpr_SUITE}.
 {skip_groups, "tests", gdpr_SUITE,
  [retrieve_personal_data_pubsub,

big_tests/tests/extdisco_SUITE.erl

@@ -15,14 +15,13 @@
 %%==============================================================================
 -module(extdisco_SUITE).
 
--include_lib("common_test/include/ct.hrl").
 -include_lib("exml/include/exml.hrl").
 -include_lib("eunit/include/eunit.hrl").
 
 -import(distributed_helper, [mim/0,
                              rpc/4]).
 
--import(domain_helper, [domain/0]).
+-import(domain_helper, [domain/0, host_type/0]).
 
 -define(NS_EXTDISCO, <<"urn:xmpp:extdisco:2">>).
 
@@ -35,11 +34,10 @@ all() ->
      {group, extdisco_required_elements_configured}].
 
 groups() ->
-    G = [{extdisco_not_configured, [sequence], extdisco_not_configured_tests()},
-         {extdisco_configured, [sequence], extdisco_configured_tests()},
-         {multiple_extdisco_configured, [sequence], multiple_extdisco_configured_tests()},
-         {extdisco_required_elements_configured, [sequence], extdisco_required_elements_configured_tests()}],
-    ct_helper:repeat_all_until_all_ok(G).
+    [{extdisco_not_configured, [sequence], extdisco_not_configured_tests()},
+     {extdisco_configured, [sequence], extdisco_configured_tests()},
+     {multiple_extdisco_configured, [sequence], multiple_extdisco_configured_tests()},
+     {extdisco_required_elements_configured, [sequence], extdisco_required_elements_configured_tests()}].
 
 extdisco_not_configured_tests() ->
     [external_services_discovery_not_supported,
@@ -65,7 +63,7 @@ extdisco_required_elements_configured_tests() ->
     [external_service_required_elements_configured].
 
 init_per_suite(Config) ->
-    NewConfig = dynamic_modules:save_modules(domain(), Config),
+    NewConfig = dynamic_modules:save_modules(host_type(), Config),
     escalus:init_per_suite(NewConfig).
 
 init_per_group(extdisco_configured, Config) ->
@@ -75,7 +73,7 @@ init_per_group(multiple_extdisco_configured, Config) ->
     ExternalServices = [stun_service(), stun_service(), turn_service()],
     set_external_services(ExternalServices, Config);
 init_per_group(extdisco_required_elements_configured, Config) ->
-    ExternalServices = [[{type, ftp},{host, "3.3.3.3"}]],
+    ExternalServices = [#{type => ftp, host => <<"3.3.3.3">>}],
     set_external_services(ExternalServices, Config);
 init_per_group(_GroupName, Config) ->
    Config.
@@ -299,28 +297,28 @@ external_service_required_elements_configured(Config) ->
 %%-----------------------------------------------------------------
 
 stun_service() ->
-      [{type, stun},
-       {host, "1.1.1.1"},
-       {port, 3478},
-       {transport, "udp"},
-       {username, "username"},
-       {password, "secret"}].
+    #{type => stun,
+      host => <<"1.1.1.1">>,
+      port => 3478,
+      transport => <<"udp">>,
+      username => <<"username">>,
+      password => <<"secret">>}.
 
 turn_service() ->
-       [{type, turn},
-        {host, "2.2.2.2"},
-        {port, 3478},
-        {transport, "tcp"},
-        {username, "username"},
-        {password, "secret"}].
-
-set_external_services(Opts, Config) ->
-    Module = [{mod_extdisco, Opts}],
-    ok = dynamic_modules:ensure_modules(domain(), Module),
+    #{type => turn,
+      host => <<"2.2.2.2">>,
+      port => 3478,
+      transport => <<"tcp">>,
+      username => <<"username">>,
+      password => <<"secret">>}.
+
+set_external_services(Services, Config) ->
+    Module = [{mod_extdisco, #{iqdisc => no_queue, service => Services}}],
+    ok = dynamic_modules:ensure_modules(host_type(), Module),
     Config.
 
 remove_external_services(Config) ->
-    dynamic_modules:ensure_stopped(domain(), [mod_extdisco]),
+    dynamic_modules:ensure_stopped(host_type(), [mod_extdisco]),
     Config.
 
 request_external_services(To) ->

big_tests/tests/oauth_SUITE.erl

@@ -475,5 +475,6 @@ required_modules() ->
      {mod_auth_token, auth_token_opts()}].
 
 auth_token_opts() ->
-    [{ {validity_period, access}, {60, minutes} },
-     { {validity_period, refresh}, {1, days} }].
+    #{iqdisc => no_queue,
+      validity_period => #{access => #{value => 60, unit => minutes},
+                           refresh => #{value => 1, unit => days}}}.

doc/configuration/Modules.md

@@ -197,7 +197,6 @@ There are some modules that don't support dynamic domains for now.
 These must **not** be enabled when using host types in `modules` or [`host_config.modules`](./host_config.md#host_configmodules) sections:
 
 * [mod_event_pusher](../modules/mod_event_pusher.md)
-* [mod_extdisco](../modules/mod_extdisco.md)
 * [mod_global_distrib](../modules/mod_global_distrib.md)
 * [mod_jingle_sip](../modules/mod_jingle_sip.md)
 * [mod_pubsub](../modules/mod_pubsub.md)

doc/migrations/5.0.0_5.1.0.md

@@ -24,6 +24,10 @@ See the [auth configuration](../configuration/auth.md) for details.
 
 The rules for overriding global options in the `host_config` section have been simplified. The `auth` section and the `s2s.address` and `s2s.host_policy` options now completely override the corresponding general settings instead of being merged with them.
 
+### Extension modules
+
+* `mod_auth_token` has a new configuration format - if you are using this module, amend the [`validity_period`](../modules/mod_auth_token.md#modulesmod_auth_tokenvalidity_period) option.
+
 ## Async workers
 
 The `async_writer` flag of MAM is now a section on its own, that absorbs previous flags related to it: `flush_interval`, `max_batch_size` and `pool_size` now become subelements of the `async_writer` section, with one more parameter, `enabled`. Below an example:

doc/modules/mod_auth_token.md

@@ -15,12 +15,19 @@ Generation of keys necessary to sign binary tokens is delegated to module `mod_k
 
 ## Options
 
+### `modules.mod_auth_token.iqdisc.type`
+* **Syntax:** string, one of `"one_queue"`, `"no_queue"`, `"queues"`, `"parallel"`
+* **Default:** `"no_queue"`
+
+Strategy to handle incoming IQ stanzas. For details, please refer to
+[IQ processing policies](../configuration/Modules.md#iq-processing-policies).
+
 ### `modules.mod_auth_token.validity_period`
-* **Syntax:** Array of TOML tables with the following keys: `token`, `value`, `unit` and following values: {token = `values: "access", "refresh", "provision"`, value = `non-negative integer`, unit = `values: "days", "hours", "minutes", "seconds"`}.
-* **Default:** `[{token = "access", value = 1, unit = "hours"}, {token = "refresh", value = 25, unit = "days"}]`
-* **Example:** `[{token = "access", value = 13, unit = "minutes"}, {token = "refresh", value = 13, unit = "days"}]`
+* **Syntax:** TOML table. Each key is either `access` or `refresh`. Each value is a nested TOML table with the following mandatory keys: `value` (non-negative integer) and `unit` (`"days"`, `"hours"`, `"minutes"` or `"seconds"`).
+* **Default:** `{access = {value = 1, unit = "hours"}, refresh = {value = 25, unit = "days"}}`
+* **Example:** `validity_period.access = {value = 30, unit = "minutes"}`
 
-Validity periods of access and refresh tokens can be defined independently.
+Validity periods of access and refresh tokens can be defined independently - specifying one of them does not change the default value for the other one.
 Validity period configuration for provision tokens happens outside the module since the server does not generate provision tokens - it only validates them.
 
 ### Required keys
@@ -156,8 +163,6 @@ Access token validity can't be sidestepped right now.
 
 ```toml
 [modules.mod_auth_token]
-  validity_period = [
-    {token = "access", value = 13, unit = "minutes"},
-    {token = "refresh", value = 13, unit = "days"}
-  ]
+  validity_period.access = {value = 13, unit = "minutes"}
+  validity_period.refresh = {value = 13, unit = "days"}
 ```

doc/modules/mod_extdisco.md

@@ -2,47 +2,59 @@
 Implements [XEP-0215: External Service Discovery](http://xmpp.org/extensions/xep-0215.html) for discovering information about services external to the XMPP network.
 The main use-case is to help discover STUN/TURN servers to allow for negotiating media exchanges.
 
-!!! warning
-    This module does not support [dynamic domains](../configuration/general.md#generalhost_types).
-
 ## Options
 
-### `modules.mod_extdisco.service.type`
+### `modules.mod_extdisco.iqdisc.type`
+* **Syntax:** string, one of `"one_queue"`, `"no_queue"`, `"queues"`, `"parallel"`
+* **Default:** `"no_queue"`
+
+Strategy to handle incoming IQ stanzas. For details, please refer to
+[IQ processing policies](../configuration/Modules.md#iq-processing-policies).
+
+### `modules.mod_extdisco.service`
+* **Syntax:** TOML array with one table for each advertised service - see below for details.
+* **Default:** `[]` - no services advertised
+
+### Service options
+
+Each advertised service is specified as a TOML table containing the following options listed below.
+
+#### `modules.mod_extdisco.service.type`
 * **Syntax:** string
 * **Default:** none, this option is required
 * **Example:** `type = "stun"`
 
 Service type, common values are `"stun"`, `"turn"`, `"ftp"`.
 
-### `modules.mod_extdisco.service.host`
+#### `modules.mod_extdisco.service.host`
 * **Syntax:** string
 * **Default:** none, this option is required
 * **Example:** `host = "192.168.0.2"`
 
 Hostname or an IP address where the service is hosted.
 
-### `modules.mod_extdisco.service.port`
+#### `modules.mod_extdisco.service.port`
 * **Syntax:** string
 * **Default:** none, this option is recommended
 * **Example:** `port = "3478"`
 
 The communications port to be used at the host.
 
-### `modules.mod_extdisco.service.transport`
+#### `modules.mod_extdisco.service.transport`
 * **Syntax:** string, one of `"udp"`, `"tcp"`
 * **Default:** none, this option is optional
 * **Example:** `transport = "udp"`
 
 The underlying transport protocol to be used when communicating with the service.
 
-### `modules.mod_extdisco.service.username`
+#### `modules.mod_extdisco.service.username`
 * **Syntax:** string
 * **Default:** none, this option is optional
 * **Example:** `username = "username"`
 
 A service-generated username for use at the service.
 
-### `modules.mod_extdisco.service.password`
+#### `modules.mod_extdisco.service.password`
 * **Syntax:** string
 * **Default:** none, this option is optional
 * **Example:** `password = "password"`
@@ -53,20 +65,23 @@ A service-generated password for use at the service.
 
 ```toml
 [modules.mod_extdisco]
+
   [[modules.mod_extdisco.service]]
     type = "stun"
     host = "127.0.0.1"
     port = 3478
     transport = "udp"
     username = "username"
     password = "password"
+
   [[modules.mod_extdisco.service]]
     type = "stun"
     host = "stun.host.com"
     port = 3478
     transport = "tcp"
     username = "username2"
     password = "password2"
+
   [[modules.mod_extdisco.service]]
     type = "turn"
     host = "turn.host.com"

src/config/mongoose_config_parser.erl

@@ -137,7 +137,7 @@ post_process_modules_opt(Other) ->
     Other.
 
 unfold_opts(Modules) ->
-    maps:map(fun(_Mod, Opts) -> proplists:unfold(Opts) end, Modules).
+    maps:map(fun(_Mod, Opts) -> mongoose_modules:unfold_opts(Opts) end, Modules).
 
 %% local functions
 

src/ejabberd_cowboy.erl

@@ -71,11 +71,11 @@ start_listener({Port, IP, tcp}=Listener, Opts) ->
                             [#cowboy_state{ref = Ref, opts = Opts}]},
                  transient, infinity, worker, [?MODULE]},
     {ok, Pid} = supervisor:start_child(ejabberd_listeners, ChildSpec),
-    TransportOpts = gen_mod:get_opt(transport_options, Opts, []),
+    TransportOpts = proplists:get_value(transport_options, Opts, []),
     TransportOptsMap = maps:from_list(TransportOpts),
     TransportOptsMap2 = TransportOptsMap#{socket_opts => [{port, Port}, {ip, IP}]},
     TransportOptsMap3 = maybe_insert_max_connections(TransportOptsMap2, Opts),
-    Opts2 = gen_mod:set_opt(transport_options, Opts, TransportOptsMap3),
+    Opts2 = lists:keystore(transport_options, 1, Opts, {transport_options, TransportOptsMap3}),
     {ok, _} = start_cowboy(Ref, Opts2),
     {ok, Pid}.
 

src/gen_mod.erl

@@ -55,7 +55,7 @@
          get_opt/2,
          get_opt/3,
          get_opt/4,
-         set_opt/3,
+         get_module_opt/3,
          get_module_opt/4,
          get_module_opts/2,
          get_loaded_module_opts/2,
@@ -82,7 +82,11 @@
 -type module_feature() :: atom().
 -type domain_name() :: mongooseim:domain_name().
 -type host_type() :: mongooseim:host_type().
--type module_opts() :: list().
+-type key_path() :: mongoose_config:key_path().
+-type opt_key() :: atom().
+-type opt_value() :: mongoose_config:value().
+-type module_opts() :: [{opt_key(), opt_value()}] % deprecated, will be removed
+                     | #{opt_key() => opt_value()}. % recommended
 
 %% -export([behaviour_info/1]).
 %% behaviour_info(callbacks) ->
@@ -265,6 +269,11 @@ wait_for_stop(MonitorReference) ->
             timeout
     end.
 
+-spec get_opt(opt_key() | key_path(), module_opts()) -> opt_value().
+get_opt(Path, Opts) when is_list(Path), is_map(Opts) ->
+    lists:foldl(fun maps:get/2, Opts, Path);
+get_opt(Opt, Opts) when is_map(Opts) ->
+    maps:get(Opt, Opts);
 get_opt(Opt, Opts) ->
     case lists:keysearch(Opt, 1, Opts) of
         false ->
@@ -273,14 +282,16 @@ get_opt(Opt, Opts) ->
             Val
     end.
 
-get_opt(Opt, Opts, Default) ->
-    case lists:keysearch(Opt, 1, Opts) of
-        false ->
-            Default;
-        {value, {_, Val}} ->
-            Val
+-spec get_opt(opt_key() | key_path(), module_opts(), opt_value()) -> opt_value().
+get_opt(Path, Opts, Default) ->
+    try
+        get_opt(Path, Opts)
+    catch
+        error:{badkey, _} -> Default;
+        throw:{undefined_option, _} -> Default
     end.
 
+%% @deprecated Processing should be done in the config spec
 get_opt(Opt, Opts, F, Default) ->
     case lists:keysearch(Opt, 1, Opts) of
         false ->
@@ -289,16 +300,8 @@ get_opt(Opt, Opts, F, Default) ->
             F(Val)
     end.
 
--spec set_opt(_, [tuple()], _) -> [tuple(), ...].
-set_opt(Opt, Opts, Value) ->
-    lists:keystore(Opt, 1, Opts, {Opt, Value}).
-
-
-%%% TODO Make Opt an atom. Fix in mod_auth_token:
-%%% 374: The call gen_mod:get_module_opt(Domain::any(), 'mod_auth_token',
-%%% {'validity_period','access' | 'refresh'}, {1 | 25,'days' | 'hours'})
-%%% breaks the contract (mongooseim:host_type(), module(), atom(), term()) -> term()
--spec get_module_opt(mongooseim:host_type(), module(), term(), term()) -> term().
+-spec get_module_opt(mongooseim:host_type(), module(), opt_key() | key_path(), opt_value()) ->
+          opt_value().
 get_module_opt(HostType, Module, Opt, Default) ->
     %% Fail in dev builds.
     %% It protects against passing something weird as a Module argument
@@ -307,10 +310,17 @@ get_module_opt(HostType, Module, Opt, Default) ->
     ModuleOpts = get_module_opts(HostType, Module),
     get_opt(Opt, ModuleOpts, Default).
 
+-spec get_module_opt(mongooseim:host_type(), module(), opt_key() | key_path()) -> opt_value().
+get_module_opt(HostType, Module, Opt) ->
+    ?ASSERT_MODULE(Module),
+    ModuleOpts = get_loaded_module_opts(HostType, Module),
+    get_opt(Opt, ModuleOpts).
 
+-spec get_module_opts(mongooseim:host_type(), module()) -> module_opts().
 get_module_opts(HostType, Module) ->
     mongoose_config:get_opt([{modules, HostType}, Module], []).
 
+-spec get_loaded_module_opts(mongooseim:host_type(), module()) -> module_opts().
 get_loaded_module_opts(HostType, Module) ->
     mongoose_config:get_opt([{modules, HostType}, Module]).
 

src/http_upload/mod_http_upload.erl

@@ -198,7 +198,7 @@ get_urls(HostType, Filename, Size, ContentType, Timeout) ->
     UTCDateTime = calendar:universal_time(),
     Token = generate_token(HostType),
     Opts = module_opts(HostType),
-    NewOpts = gen_mod:set_opt(expiration_time, Opts, Timeout),
+    NewOpts = lists:keystore(expiration_time, 1, Opts, {expiration_time, Timeout}),
     mod_http_upload_backend:create_slot(HostType, UTCDateTime, Token, Filename,
                                         ContentType, Size, NewOpts).