WIP

esl · Jan 3, 2022 · 92a641f · 92a641f
1 parent aba80ee
commit 92a641f
Show file tree

Hide file tree

Showing 14 changed files with 139 additions and 182 deletions.
diff --git a/big_tests/tests/dynamic_modules.erl b/big_tests/tests/dynamic_modules.erl
@@ -22,23 +22,21 @@ get_saved_config(HostType, Module, Config) ->
 
 get_saved_config(#{node := NodeName}, HostType, Module, Config) ->
     SavedModules = proplists:get_value({saved_modules, NodeName, HostType}, Config),
-    proplists:get_value(Module, SavedModules).
+    maps:get(Module, SavedModules).
 
 ensure_modules(HostType, RequiredModules) ->
     ensure_modules(mim(), HostType, RequiredModules).
 
 ensure_modules(Node, HostType, RequiredModules) ->
     ToStop = [M || {M, stopped} <- RequiredModules],
     ToEnsure = [{M, Opts} || {M, Opts} <- RequiredModules, Opts =/= stopped],
-    ok = rpc(Node, mongoose_modules, replace_modules, [HostType, ToStop, ToEnsure]).
+    rpc(Node, mongoose_modules, replace_modules, [HostType, ToStop, ToEnsure]).
 
 ensure_stopped(HostType, ModulesToStop) ->
     ensure_stopped(mim(), HostType, ModulesToStop).
 
 ensure_stopped(Node, HostType, ModulesToStop) ->
-    CurrentModules = get_current_modules(HostType),
-    [stop(Node, HostType, Mod) || {Mod, _Opts} <- CurrentModules,
-                                  lists:member(Mod, ModulesToStop)].
+    [{Mod, stop(Node, HostType, Mod)} || Mod <- ModulesToStop].
 
 restore_modules(Config) ->
     restore_modules(#{}, Config).
@@ -50,8 +48,8 @@ restore_modules(RPCSpec, Config) when is_map(RPCSpec) ->
 
 restore_modules(Node, HostType, SavedModules) ->
     CurrentModules = get_current_modules(Node, HostType),
-    ToStop = proplists:get_keys(CurrentModules) -- proplists:get_keys(SavedModules),
-    rpc(Node, mongoose_modules, replace_modules, [HostType, ToStop, SavedModules]).
+    ToStop = maps:keys(CurrentModules) -- maps:keys(SavedModules),
+    rpc(Node, mongoose_modules, replace_modules, [HostType, ToStop, maps:to_list(SavedModules)]).
 
 get_current_modules(HostType) ->
     get_current_modules(mim(), HostType).
@@ -63,25 +61,13 @@ stop(HostType, Mod) ->
     stop(mim(), HostType, Mod).
 
 stop(Node, HostType, Mod) ->
-    case rpc(Node, mongoose_modules, ensure_stopped, [HostType, Mod]) of
-        no_change ->
-            {error, stopped};
-        {changed_from, {Mod, Opts}} ->
-            {ok, Opts}
-    end.
+    rpc(Node, mongoose_modules, ensure_stopped, [HostType, Mod]).
 
 start(HostType, Mod, Args) ->
     start(mim(), HostType, Mod, Args).
 
 start(Node, HostType, Mod, Args) ->
-    case rpc(Node, mongoose_modules, ensure_started, [HostType, Mod, Args]) of
-        {changed_from, {Mod, stopped}} ->
-            ok;
-        {changed_from, {Mod, OldOpts}} ->
-            {error, {restarted, OldOpts}};
-        no_change ->
-            {error, already_started}
-    end.
+    rpc(Node, mongoose_modules, ensure_started, [HostType, Mod, Args]).
 
 restart(HostType, Mod, Args) ->
     restart(mim(), HostType, Mod, Args).

diff --git a/big_tests/tests/mod_global_distrib_SUITE.erl b/big_tests/tests/mod_global_distrib_SUITE.erl
@@ -223,6 +223,7 @@ end_per_group(advertised_endpoints, Config) ->
     escalus_fresh:clean(),
     end_per_group_generic(Config);
 end_per_group(start_checks, Config) ->
+    dynamic_modules:stop(node_spec(europe_node1), <<"localhost">>, mod_global_distrib),
     escalus_fresh:clean(),
     Config;
 end_per_group(invalidation, Config) ->

diff --git a/big_tests/tests/rest_SUITE.erl b/big_tests/tests/rest_SUITE.erl
@@ -672,9 +672,9 @@ stop_start_command_module(_) ->
     %% described above we test both transition from `started' to
     %% `stopped' and from `stopped' to `started'.
     {?OK, _} = gett(admin, <<"/commands">>),
-    {ok, _Opts} = dynamic_modules:stop(host_type(), mod_commands),
+    {stopped, _} = dynamic_modules:stop(host_type(), mod_commands),
     {?NOT_FOUND, _} = gett(admin, <<"/commands">>),
-    ok = dynamic_modules:start(host_type(), mod_commands, []),
+    {started, _} = dynamic_modules:start(host_type(), mod_commands, []),
     timer:sleep(200), %% give the server some time to build the paths again
     {?OK, _} = gett(admin, <<"/commands">>).
 

diff --git a/src/config/mongoose_config_parser.erl b/src/config/mongoose_config_parser.erl
@@ -128,10 +128,13 @@ add_dep_modules_opts(Opts) ->
 
 add_dep_modules_opt({{modules, Host}, Modules}) ->
     ModulesWithDeps = gen_mod_deps:resolve_deps(Host, Modules),
-    {{modules, Host}, ModulesWithDeps};
+    {{modules, Host}, unfold_opts(ModulesWithDeps)};
 add_dep_modules_opt(Other) ->
     Other.
 
+unfold_opts(Modules) ->
+    maps:map(fun(_Mod, Opts) -> proplists:unfold(Opts) end, Modules).
+
 %% local functions
 
 -spec halt_with_msg(string(), [any()]) -> no_return().

diff --git a/src/ejabberd_app.erl b/src/ejabberd_app.erl
@@ -52,7 +52,6 @@ start(normal, _Args) ->
     ejabberd_commands:init(),
     mongoose_commands:init(),
     mongoose_service:start(),
-    gen_mod:start(),
     mongoose_config:start(),
     mongoose_logs:set_global_loglevel(mongoose_config:get_opt(loglevel)),
     mongoose_deprecations:start(),

diff --git a/src/gen_mod.erl b/src/gen_mod.erl
@@ -44,8 +44,7 @@
               module_opts/0]).
 
 -export([
-         % Modules start & stop
-         start/0,
+         % Modules start & stop, do NOT use in the shell, use mongoose_modules API instead
          start_module/3,
          start_backend_module/2,
          start_backend_module/3,
@@ -80,11 +79,6 @@
 
 -include("mongoose.hrl").
 
--record(ejabberd_module, {
-          module_host_type, % {module(), host_type()},
-          opts % list()
-         }).
-
 -type module_feature() :: atom().
 -type domain_name() :: mongooseim:domain_name().
 -type host_type() :: mongooseim:host_type().
@@ -116,27 +110,16 @@
 
 -optional_callbacks([config_spec/0, supported_features/0, deps/2]).
 
--spec start() -> 'ok'.
-start() ->
-    ets:new(ejabberd_modules, [named_table, public, {read_concurrency, true},
-                               {keypos, #ejabberd_module.module_host_type}]),
-    ok.
-
-
+%% @doc To start a new module at runtime, use mongoose:modules:ensure_module/3 instead.
 -spec start_module(HostType :: host_type(),
                    Module :: module(),
-                   Opts :: [any()]) -> {ok, term()} | {error, already_started}.
+                   Opts :: [any()]) -> {ok, term()}.
 start_module(HostType, Module, Opts) ->
-    case is_loaded(HostType, Module) of
-        true -> {error, already_started};
-        false -> start_module_for_host_type(HostType, Module, Opts)
-    end.
+    true = is_loaded(HostType, Module),
+    start_module_for_host_type(HostType, Module, Opts).
 
-start_module_for_host_type(HostType, Module, Opts0) ->
+start_module_for_host_type(HostType, Module, Opts) ->
     {links, LinksBefore} = erlang:process_info(self(), links),
-    Opts = proplists:unfold(Opts0),
-    ets:insert(ejabberd_modules, #ejabberd_module{module_host_type = {Module, HostType},
-                                                  opts = Opts}),
     try
         lists:map(fun mongoose_service:assert_loaded/1,
                   get_required_services(HostType, Module, Opts)),
@@ -163,7 +146,6 @@ start_module_for_host_type(HostType, Module, Opts0) ->
         end
     catch
         Class:Reason:StackTrace ->
-            ets:delete(ejabberd_modules, {Module, HostType}),
             ErrorText = io_lib:format("Problem starting the module ~p for "
                                       "host_type ~p~n options: ~p~n ~p: ~p~n~p",
                                       [Module, HostType, Opts, Class, Reason,
@@ -223,35 +205,29 @@ is_app_running(AppName) ->
     Timeout = 15000,
     lists:keymember(AppName, 1, application:which_applications(Timeout)).
 
-%% @doc Stop the module, and remove it from 'ejabberd_modules'
+%% @doc To stop a module at runtime, use mongoose:modules:ensure_stopped/2 instead.
 -spec stop_module(host_type(), module()) ->
-    {ok, list()} | {error, not_loaded} | {error, term()}.
+    ok | {error, not_loaded} | {error, term()}.
 stop_module(HostType, Module) ->
-    case is_loaded(HostType, Module) of
-        false -> {error, not_loaded};
-        true -> stop_module_for_host_type(HostType, Module)
-    end.
+    true = is_loaded(HostType, Module),
+    stop_module_for_host_type(HostType, Module).
 
--spec stop_module_for_host_type(host_type(), module()) -> {error, term()} | {ok, list()}.
+-spec stop_module_for_host_type(host_type(), module()) -> ok.
 stop_module_for_host_type(HostType, Module) ->
-    Opts = get_module_opts(HostType, Module),
     try Module:stop(HostType) of
         {wait, ProcList} when is_list(ProcList) ->
             lists:foreach(fun wait_for_process/1, ProcList),
-            ets:delete(ejabberd_modules, {Module, HostType}),
-            {ok, Opts};
+            ok;
         {wait, Process} ->
             wait_for_process(Process),
-            ets:delete(ejabberd_modules, {Module, HostType}),
-            {ok, Opts};
+            ok;
         _ ->
-            ets:delete(ejabberd_modules, {Module, HostType}),
-            {ok, Opts}
+            ok
     catch Class:Reason:Stacktrace ->
             ?LOG_ERROR(#{what => module_stopping_failed,
                          host_type => HostType, stop_module => Module,
                          class => Class, reason => Reason, stacktrace => Stacktrace}),
-            {error, Reason}
+            erlang:raise(Class, Reason, Stacktrace)
     end.
 
 -spec does_module_support(module(), module_feature()) -> boolean().
@@ -334,18 +310,10 @@ get_module_opt(HostType, Module, Opt, Default) ->
 
 
 get_module_opts(HostType, Module) ->
-    OptsList = ets:lookup(ejabberd_modules, {Module, HostType}),
-    case OptsList of
-        [] -> [];
-        [#ejabberd_module{opts = Opts} | _] -> Opts
-    end.
+    mongoose_config:get_opt([{modules, HostType}, Module], []).
 
 get_loaded_module_opts(HostType, Module) ->
-    OptsList = ets:lookup(ejabberd_modules, {Module, HostType}),
-    case OptsList of
-        [] -> error({module_not_loaded, HostType, Module});
-        [#ejabberd_module{opts = Opts} | _] -> Opts
-    end.
+    mongoose_config:get_opt([{modules, HostType}, Module]).
 
 -spec get_opt_subhost(domain_name(),
                       list(),
@@ -369,52 +337,35 @@ get_module_opt_subhost(Host, Module, Default) ->
 
 -spec loaded_modules() -> [module()].
 loaded_modules() ->
-    ModSet = ets:foldl(fun(#ejabberd_module{module_host_type = {Mod, _}}, Set) ->
-                               gb_sets:add_element(Mod, Set)
-                       end, gb_sets:new(), ejabberd_modules),
-    gb_sets:to_list(ModSet).
+    gb_sets:to_list(lists:foldl(
+                      fun(HostType, ModSetIn) ->
+                              gb_sets:union(ModSetIn, gb_sets:from_list(loaded_modules(HostType)))
+                      end, gb_sets:new(), ?ALL_HOST_TYPES)).
 
 -spec loaded_modules(host_type()) -> [module()].
 loaded_modules(HostType) ->
-    ets:select(ejabberd_modules,
-               [{#ejabberd_module{_ = '_', module_host_type = {'$1', HostType}},
-                 [],
-                 ['$1']}]).
-
+    maps:keys(mongoose_config:get_opt({modules, HostType})).
 
--spec loaded_modules_with_opts(host_type()) -> [{module(), list()}].
+-spec loaded_modules_with_opts(host_type()) -> #{module() => list()}.
 loaded_modules_with_opts(HostType) ->
-    ets:select(ejabberd_modules,
-               [{#ejabberd_module{_ = '_', module_host_type = {'$1', HostType},
-                                  opts = '$2'},
-                 [],
-                 [{{'$1', '$2'}}]}]).
+    mongoose_config:get_opt({modules, HostType}).
 
--spec loaded_modules_with_opts() -> #{host_type() => [{module(), list()}]}.
+-spec loaded_modules_with_opts() -> #{host_type() => #{module() => list()}}.
 loaded_modules_with_opts() ->
-    Res = ets:select(ejabberd_modules,
-               [{#ejabberd_module{_ = '_', module_host_type = {'$1', '$2'},
-                                  opts = '$3'},
-                 [],
-                 [{{'$2', '$1', '$3'}}]}]),
-    Hosts = lists:usort([H || {H, _, _} <- Res]),
-    maps:from_list([{H, [{M, Opts}
-                         || {HH, M, Opts} <- Res,
-                            H =:= HH]}
-                    || H <- Hosts]).
+    maps:from_list([{HostType, loaded_modules_with_opts(HostType)} || HostType <- ?ALL_HOST_TYPES]).
 
 -spec hosts_with_module(module()) -> [host_type()].
 hosts_with_module(Module) ->
-    ets:select(ejabberd_modules,
-               [{#ejabberd_module{_ = '_', module_host_type = {Module, '$1'}},
-                 [], ['$1']}]).
+    [HostType || HostType <- ?ALL_HOST_TYPES, is_loaded(HostType, Module)].
 
 -spec hosts_and_opts_with_module(module()) -> [{host_type(), module_opts()}].
 hosts_and_opts_with_module(Module) ->
-    ets:select(ejabberd_modules,
-               [{#ejabberd_module{_ = '_', module_host_type = {Module, '$1'},
-                                  opts = '$2'},
-                 [], [{{'$1', '$2'}}]}]).
+    lists:flatmap(fun(HostType) ->
+                          case mongoose_config:lookup_opt([{modules, HostType}, Module]) of
+                              {error, not_found} -> [];
+                              {ok, Opts} -> [{HostType, Opts}]
+                          end
+                  end, ?ALL_HOST_TYPES).
 
 -spec get_module_proc(binary() | string(), module()) -> atom().
 %% TODO:
@@ -429,7 +380,7 @@ get_module_proc(Host, Base) ->
 
 -spec is_loaded(HostType :: binary(), Module :: atom()) -> boolean().
 is_loaded(HostType, Module) ->
-    ets:member(ejabberd_modules, {Module, HostType}).
+    maps:is_key(Module, loaded_modules_with_opts(HostType)).
 
 -spec get_deps(HostType :: host_type(), Module :: module(),
                Opts :: proplists:proplist()) -> module_deps_list().