Merge pull request #3577 from esl/mod_push_service_mongoosepush-map-c…

…onfig

Put push_service_mongoosepush opts in a map with defaults

big_tests/tests/push_integration_SUITE.erl

@@ -796,15 +796,15 @@ no_push_notification_for_expired_device(Config) ->
 
 mongoose_push_unregistered_device_resp(Config) ->
     case ?config(api_v, Config) of
-        "v3" ->
+        <<"v3">> ->
             {410, jiffy:encode(#{<<"reason">> => <<"unregistered">>})};
-        "v2" ->
+        <<"v2">> ->
             {500, jiffy:encode(#{<<"details">> => <<"probably_unregistered">>})}
     end.
 
-maybe_check_if_push_node_was_disabled("v2", _, _) ->
+maybe_check_if_push_node_was_disabled(<<"v2">>, _, _) ->
     ok;
-maybe_check_if_push_node_was_disabled("v3", User, PushNode) ->
+maybe_check_if_push_node_was_disabled(<<"v3">>, User, PushNode) ->
     JID = rpc(?RPC_SPEC, jid, binary_to_bare, [escalus_utils:get_jid(User)]),
     Host = escalus_utils:get_server(User),
     Fun = fun() ->
@@ -959,9 +959,9 @@ init_modules(G, Config) ->
     [{api_v, MongoosePushAPI}, {required_modules, Modules} | C].
 
 mongoose_push_api_for_group(failure_cases_v2) ->
-    "v2";
+    <<"v2">>;
 mongoose_push_api_for_group(_) ->
-    "v3".
+    <<"v3">>.
 
 required_modules_for_group(pm_notifications_with_inbox, API, PubSubHost) ->
     [{mod_inbox, inbox_opts()},
@@ -1009,8 +1009,9 @@ required_modules(API, PubSubHost, PluginModule) ->
              end,
     PushBackend = {push, [{backend, mongoose_helper:mnesia_or_rdbms_backend()} | PushOpts]},
     [
-        {mod_push_service_mongoosepush, [{pool_name, mongoose_push_http},
-                                         {api_version, API}]},
+        {mod_push_service_mongoosepush, config_parser_helper:mod_config(mod_push_service_mongoosepush,
+                                                                        #{pool_name => mongoose_push_http,
+                                                                          api_version => API})},
         {mod_event_pusher, [{backends, [PushBackend]}]} |
         PubSub
     ].

big_tests/tests/push_pubsub_SUITE.erl

@@ -65,11 +65,11 @@ end_per_suite(Config) ->
     escalus:end_per_suite(Config).
 
 init_per_group(rest_integration_v1, Config) ->
-    restart_modules(Config, "v1");
+    restart_modules(Config, <<"v1">>);
 init_per_group(rest_integration_v2, Config) ->
-    restart_modules(Config, "v2");
+    restart_modules(Config, <<"v2">>);
 init_per_group(_, Config) ->
-    restart_modules(Config, "v2").
+    restart_modules(Config, <<"v2">>).
 
 end_per_group(_, Config) ->
     Config.
@@ -435,10 +435,10 @@ required_modules(APIVersion) ->
         {nodetree, <<"dag">>},
         {host, subhost_pattern(?PUBSUB_SUB_DOMAIN ++ ".@HOST@")}
     ]},
-     {mod_push_service_mongoosepush, [
-         {pool_name, mongoose_push_http},
-         {api_version, APIVersion}
-     ]}].
+     {mod_push_service_mongoosepush,
+      config_parser_helper:mod_config(mod_push_service_mongoosepush, #{pool_name => mongoose_push_http,
+                                                                       api_version => APIVersion})
+     }].
 
 restart_modules(Config, APIVersion) ->
     dynamic_modules:restore_modules(Config),

doc/modules/mod_push_service_mongoosepush.md

@@ -19,7 +19,7 @@ It must be defined in [outgoing_pools setting](../configuration/outgoing-connect
 * **Default:** `"undefined"`
 * **Example:** `pool_name = "mongoose_push_http"`
 
-The name of the pool to use (as defined in `outgoing_pools`).
+The name of the pool to use (as defined in [`outgoing_pools`](../configuration/outgoing-connections.md)).
 
 ### `modules.mod_push_service_mongoosepush.api_version`
 * **Syntax:** string, `"v2"` or `"v3"`

src/mod_push_service_mongoosepush.erl

@@ -29,22 +29,9 @@
 
 -export([http_notification/5]).
 
-%% Types
--export_type([]).
-
 -export([config_metrics/1]).
 
--ignore_xref([behaviour_info/1, http_notification/5, push_notifications/4]).
-
-%%--------------------------------------------------------------------
-%% Definitions
-%%--------------------------------------------------------------------
-
--define(DEFAULT_API_VERSION, "v3").
-
--callback init(term(), term()) -> ok.
-
-%% Types
+-ignore_xref([http_notification/5, push_notifications/4]).
 
 %%--------------------------------------------------------------------
 %% Module callbacks
@@ -63,28 +50,30 @@ start_pool(Host, Opts) ->
     {ok, _} = mongoose_wpool:start(generic, Host, mongoosepush_service, pool_opts(Opts)).
 
 -spec pool_opts(gen_mod:module_opts()) -> mongoose_wpool:pool_opts().
-pool_opts(Opts) ->
-    MaxHTTPConnections = gen_mod:get_opt(max_http_connections, Opts, 100),
+pool_opts(#{max_http_connections := MaxHTTPConnections}) ->
     [{strategy, available_worker},
      {workers, MaxHTTPConnections}].
 
 -spec stop(Host :: jid:server()) -> ok.
 stop(Host) ->
     ejabberd_hooks:delete(push_notifications, Host, ?MODULE, push_notifications, 10),
     mongoose_wpool:stop(generic, Host, mongoosepush_service),
-
     ok.
 
 -spec config_spec() -> mongoose_config_spec:config_section().
 config_spec() ->
     #section{
        items = #{<<"pool_name">> => #option{type = atom,
                                             validate = pool_name},
-                 <<"api_version">> => #option{type = string,
-                                              validate = {enum, ["v2", "v3"]}},
+                 <<"api_version">> => #option{type = binary,
+                                              validate = {enum, [<<"v2">>, <<"v3">>]}},
                  <<"max_http_connections">> => #option{type = integer,
                                                        validate = non_negative}
-                }
+                },
+       defaults = #{<<"pool_name">> => undefined,
+                    <<"api_version">> => <<"v3">>,
+                    <<"max_http_connections">> => 100},
+       format_items = map
       }.
 
 %%--------------------------------------------------------------------
@@ -96,13 +85,11 @@ config_spec() ->
                          Notifications :: [#{binary() => binary()}],
                          Options :: #{binary() => binary()}) ->
     ok | {error, Reason :: term()}.
-push_notifications(AccIn, Host, Notifications, Options) ->
+push_notifications(AccIn, Host, Notifications, Options = #{<<"device_id">> := DeviceId}) ->
     ?LOG_DEBUG(#{what => push_notifications, notifications => Notifications,
                  opts => Options, acc => AccIn}),
 
-    DeviceId = maps:get(<<"device_id">>, Options),
-    ProtocolVersionOpt = gen_mod:get_module_opt(Host, ?MODULE, api_version, ?DEFAULT_API_VERSION),
-    {ok, ProtocolVersion} = parse_api_version(ProtocolVersionOpt),
+    ProtocolVersion = gen_mod:get_module_opt(Host, ?MODULE, api_version),
     Path = <<ProtocolVersion/binary, "/notification/", DeviceId/binary>>,
     Fun = fun(Notification) ->
             ReqHeaders = [{<<"content-type">>, <<"application/json">>}],
@@ -139,7 +126,7 @@ send_push_notifications([Notification | Notifications], Fun, Result) ->
                         binary(), proplists:proplist(), binary()) ->
     ok | {error, Reason :: term()}.
 http_notification(Host, Method, URL, ReqHeaders, Payload) ->
-    PoolName = gen_mod:get_module_opt(Host, ?MODULE, pool_name, undefined),
+    PoolName = gen_mod:get_module_opt(Host, ?MODULE, pool_name),
     case mongoose_http_client:Method(Host, PoolName, URL, ReqHeaders, Payload) of
         {ok, {BinStatusCode, Body}} ->
             case binary_to_integer(BinStatusCode) of
@@ -173,20 +160,13 @@ http_notification(Host, Method, URL, ReqHeaders, Payload) ->
 %% Helper functions
 %%--------------------------------------------------------------------
 
-parse_api_version("v3") ->
-    {ok, <<"v3">>};
-parse_api_version("v2") ->
-    {ok, <<"v2">>};
-parse_api_version(_) ->
-    {error, not_supported}.
-
 %% Create notification for API v2 and v3
 make_notification(Notification, Options) ->
     RequiredParameters = #{service => maps:get(<<"service">>, Options)},
     %% The full list of supported optional parameters can be found here:
     %%    https://github.com/esl/MongoosePush/blob/master/README.md#request
     %%
-    %% Note that <<"tags">> parameter is explicitely excluded to avoid any
+    %% Note that <<"tags">> parameter is explicitly excluded to avoid any
     %% security issues. User should not be allowed to select pools other than
     %% prod and dev (see <<"mode">> parameter description).
     OptionalKeys = [<<"mode">>, <<"priority">>, <<"topic">>,
@@ -214,6 +194,6 @@ make_notification(Notification, Options) ->
 call(Host, M, F, A) ->
     mongoose_wpool:call(generic, Host, mongoosepush_service, {M, F, A}).
 
+-spec config_metrics(mongooseim:host_type()) -> [{gen_mod:opt_key(), gen_mod:opt_value()}].
 config_metrics(Host) ->
-    OptsToReport = [{api_version, ?DEFAULT_API_VERSION}], %list of tuples {option, defualt_value}
-    mongoose_module_metrics:opts_for_module(Host, ?MODULE, OptsToReport).
+    mongoose_module_metrics:opts_for_module(Host, ?MODULE, [api_version]).

test/common/config_parser_helper.erl

@@ -561,9 +561,9 @@ all_modules() ->
                                       rooms_per_page => 5,
                                       rooms_per_user => 10}),
       mod_push_service_mongoosepush =>
-          [{api_version, "v3"},
-           {max_http_connections, 100},
-           {pool_name, mongoose_push_http}],
+          #{api_version => <<"v3">>,
+            max_http_connections => 100,
+            pool_name => mongoose_push_http},
       mod_event_pusher_sns =>
           [{access_key_id, "AKIAIOSFODNN7EXAMPLE"},
            {account_id, "123456789012"},
@@ -884,6 +884,8 @@ default_mod_config(mod_privacy) ->
     #{backend => mnesia};
 default_mod_config(mod_private) ->
     #{iqdisc => one_queue, backend => rdbms};
+default_mod_config(mod_push_service_mongoosepush) ->
+    #{pool_name => undefined, api_version => <<"v3">>, max_http_connections => 100};
 default_mod_config(mod_roster) ->
     #{iqdisc => one_queue, versioning => false, store_current_id => false, backend => mnesia};
 default_mod_config(mod_shared_roster_ldap) ->

test/config_parser_SUITE.erl

@@ -2690,14 +2690,15 @@ mod_pubsub_default_node_config(_Config) ->
     ?errh(T(#{<<"subscribe">> => <<"never">>})).
 
 mod_push_service_mongoosepush(_Config) ->
+    check_module_defaults(mod_push_service_mongoosepush),
+    P = [modules, mod_push_service_mongoosepush],
     T = fun(Opts) -> #{<<"modules">> => #{<<"mod_push_service_mongoosepush">> => Opts}} end,
-    M = fun(Cfg) -> modopts(mod_push_service_mongoosepush, Cfg) end,
-    ?cfgh(M([{pool_name, test_pool}]),
+    ?cfgh(P ++ [pool_name], test_pool,
           T(#{<<"pool_name">> => <<"test_pool">>})),
-    ?cfgh(M([{api_version, "v3"}]),
-          T(#{<<"api_version">> => <<"v3">>})),
-    ?cfgh(M([{max_http_connections, 100}]),
-          T(#{<<"max_http_connections">> => 100})),
+    ?cfgh(P ++ [api_version], <<"v2">>,
+          T(#{<<"api_version">> => <<"v2">>})),
+    ?cfgh(P ++ [max_http_connections], 999,
+          T(#{<<"max_http_connections">> => 999})),
     ?errh(T(#{<<"pool_name">> => 1})),
     ?errh(T(#{<<"api_version">> => <<"v4">>})),
     ?errh(T(#{<<"max_http_connections">> => -1})).