Improve visibility of mam options' defaults.

esl · Jan 18, 2018 · 42fd92f · 42fd92f
1 parent a685bb2
commit 42fd92f
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 41 deletions.
diff --git a/src/gen_mod.erl b/src/gen_mod.erl
@@ -289,33 +289,27 @@ get_module_opt_by_subhost(SubHost, Module, Opt, Default) ->
     {ok, Host} = mongoose_subhosts:get_host(SubHost),
     get_module_opt(Host, Module, Opt, Default).
 
+
 %% @doc Non-atomic! You have been warned.
 -spec set_module_opt(ejabberd:server(), module(), _Opt, _Value) -> boolean().
 set_module_opt(Host, Module, Opt, Value) ->
-    Key = {Module, Host},
-    OptsList = ets:lookup(ejabberd_modules, Key),
-    case OptsList of
+    case ets:lookup(ejabberd_modules, {Module, Host}) of
         [] ->
             false;
         [#ejabberd_module{opts = Opts}] ->
             Updated = set_opt(Opt, Opts, Value),
-            ets:update_element(ejabberd_modules, Key,
-                               {#ejabberd_module.opts, Updated})
+            set_module_opts(Host, Module, Updated)
     end.
 
-%% @doc Non-atomic! You have been warned.
--spec set_module_opts(ejabberd:server(), module(), _Opts) -> boolean().
+
+%% @doc Replaces all module options
+-spec set_module_opts(ejabberd:server(), module(), _Opts) -> true.
 set_module_opts(Host, Module, Opts0) ->
     Opts = proplists:unfold(Opts0),
-    Key = {Module, Host},
-    OptsList = ets:lookup(ejabberd_modules, Key),
-    case OptsList of
-        [] ->
-            false;
-        [#ejabberd_module{opts = _Opts}] ->
-            ets:update_element(ejabberd_modules, Key,
-                               {#ejabberd_module.opts, Opts})
-    end.
+    ets:insert(ejabberd_modules,
+               #ejabberd_module{module_host = {Module, Host},
+                                opts = Opts}).
+
 
 -spec set_module_opt_by_subhost(
         SubHost :: ejabberd:server(),

diff --git a/src/mod_mam.erl b/src/mod_mam.erl
@@ -435,10 +435,9 @@ handle_lookup_messages(#jid{} = From, #jid{} = ArcJID,
     Host = server_host(ArcJID),
     ArcID = archive_id_int(Host, ArcJID),
     QueryID = exml_query:attr(QueryEl, <<"queryid">>, <<>>),
-    ExtraParamsModule = param(Host, extra_lookup_params, undefined),
-    Params0 = mam_iq:query_to_lookup_params(IQ, param(Host, max_result_limit, 50),
-                                            param(Host, default_result_limit, 50),
-                                            ExtraParamsModule),
+    Params0 = mam_iq:query_to_lookup_params(IQ, max_result_limit(Host),
+                                            default_result_limit(Host),
+                                            extra_params_module(Host)),
     Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID),
     case lookup_messages(Host, Params) of
         {error, 'policy-violation'} ->
@@ -469,10 +468,9 @@ handle_set_message_form(#jid{} = From, #jid{} = ArcJID,
     Host = server_host(ArcJID),
     ArcID = archive_id_int(Host, ArcJID),
     QueryID = exml_query:attr(QueryEl, <<"queryid">>, <<>>),
-    ExtraParamsModule = param(Host, extra_lookup_params, undefined),
-    Params0 = mam_iq:form_to_lookup_params(IQ, param(Host, max_result_limit, 50),
-                                           param(Host, default_result_limit, 50),
-                                           ExtraParamsModule),
+    Params0 = mam_iq:form_to_lookup_params(IQ, max_result_limit(Host),
+                                           default_result_limit(Host),
+                                           extra_params_module(Host)),
     Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID),
     PageSize = maps:get(page_size, Params),
     case lookup_messages(Host, Params) of
@@ -815,10 +813,16 @@ report_issue(Reason, Stacktrace, Issue, #jid{lserver=LServer, luser=LUser}, IQ)
 
 
 %% ----------------------------------------------------------------------
-%% Dynamic params module
+%% Dynamic params
 
-param(Host, Opt, Default) ->
-    mod_mam_utils:param(?MODULE, Host, Opt, Default).
+extra_params_module(Host) ->
+    mod_mam_utils:param(?MODULE, Host, extra_lookup_params, undefined).
+
+max_result_limit(Host) ->
+    mod_mam_utils:param(?MODULE, Host, max_result_limit, 50).
+
+default_result_limit(Host) ->
+    mod_mam_utils:param(?MODULE, Host, default_result_limit, 50).
 
 is_archivable_message(Host, Dir, Packet) ->
     mod_mam_utils:call_is_archivable_message(?MODULE, Host, Dir, Packet).

diff --git a/src/mod_mam_muc.erl b/src/mod_mam_muc.erl
@@ -402,10 +402,9 @@ handle_lookup_messages(#jid{} = From, #jid{} = ArcJID,
     {ok, Host} = mongoose_subhosts:get_host(ArcJID#jid.lserver),
     ArcID = archive_id_int(Host, ArcJID),
     QueryID = exml_query:attr(QueryEl, <<"queryid">>, <<>>),
-    ExtraParamsModule = gen_mod:get_module_opt(Host, ?MODULE, extra_lookup_params, undefined),
-    Params0 = mam_iq:query_to_lookup_params(IQ, param(Host, max_result_limit, 50),
-                                            param(Host, default_result_limit, 50),
-                                            ExtraParamsModule),
+    Params0 = mam_iq:query_to_lookup_params(IQ, max_result_limit(Host),
+                                            default_result_limit(Host),
+                                            extra_params_module(Host)),
     Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID),
     case lookup_messages(Host, Params) of
         {error, 'policy-violation'} ->
@@ -435,10 +434,9 @@ handle_lookup_messages(#jid{} = From, #jid{} = ArcJID,
 handle_set_message_form(#jid{} = From, #jid{} = ArcJID, IQ) ->
     {ok, Host} = mongoose_subhosts:get_host(ArcJID#jid.lserver),
     ArcID = archive_id_int(Host, ArcJID),
-    ExtraParamsModule = gen_mod:get_module_opt(Host, ?MODULE, extra_lookup_params, undefined),
-    Params0 = mam_iq:form_to_lookup_params(IQ, param(Host, max_result_limit, 50),
-                                           param(Host, default_result_limit, 50),
-                                           ExtraParamsModule),
+    Params0 = mam_iq:form_to_lookup_params(IQ, max_result_limit(Host),
+                                           default_result_limit(Host),
+                                           extra_params_module(Host)),
     Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID),
     Result = lookup_messages(Host, Params),
     handle_lookup_result(Result, From, IQ, Params).
@@ -765,12 +763,17 @@ report_issue(Reason, Stacktrace, Issue, #jid{lserver = LServer, luser = LUser},
     ?ERROR_MSG("issue=~p, server=~p, user=~p, reason=~p, iq=~p, stacktrace=~p",
                [Issue, LServer, LUser, Reason, IQ, Stacktrace]).
 
-
 %% ----------------------------------------------------------------------
-%% Dynamic params module
+%% Dynamic params
+
+extra_params_module(Host) ->
+    mod_mam_utils:param(?MODULE, Host, extra_lookup_params, undefined).
+
+max_result_limit(Host) ->
+    mod_mam_utils:param(?MODULE, Host, max_result_limit, 50).
 
-param(Host, Opt, Default) ->
-    mod_mam_utils:param(?MODULE, Host, Opt, Default).
+default_result_limit(Host) ->
+    mod_mam_utils:param(?MODULE, Host, default_result_limit, 50).
 
 is_archivable_message(Host, Dir, Packet) ->
     mod_mam_utils:call_is_archivable_message(?MODULE, Host, Dir, Packet).

diff --git a/src/mod_mam_utils.erl b/src/mod_mam_utils.erl
@@ -1067,9 +1067,12 @@ error_on_sql_error(_HostOrConn, _Query, Result) ->
 wrapper_id() ->
     uuid:uuid_to_string(uuid:get_v4(), binary_standard).
 
+-spec param(module(), Host :: ejabberd:lserver(), Opt :: term(), Default :: term()) -> term().
 param(Module, Host, Opt, Default) ->
     gen_mod:get_module_opt(Host, Module, Opt, Default).
 
+-spec call_is_archivable_message(module(), Host :: ejabberd:lserver(),
+                                 Dir :: incoming | outgoing, Packet :: jlib:xmlel()) -> boolean().
 call_is_archivable_message(Module, Host, Dir, Packet) ->
     {IsArchivableModule, IsArchivableFunction} =
         case param(Module, Host, is_archivable_message, undefined) of
@@ -1086,10 +1089,10 @@ call_is_archivable_message(Module, Host, Dir, Packet) ->
                  [Module, Dir, Packet, ArchiveChatMarkers]).
 
 %% @doc Enable support for `<archived/>' element from MAM v0.2
-%% -spec add_archived_element(Host :: ejabberd:lserver()) -> boolean().
+-spec add_archived_element(module(), Host :: ejabberd:lserver()) -> boolean().
 add_archived_element(Module, Host) ->
     param(Module, Host, add_archived_element, false).
 
-%% -spec add_stanzaid_element(Host :: ejabberd:lserver()) -> boolean().
+-spec add_stanzaid_element(module(), Host :: ejabberd:lserver()) -> boolean().
 add_stanzaid_element(Module, Host) ->
     not param(Module, Host, no_stanzaid_element, false).