Merge pull request #4250 from esl/consistent-node-cleanup

Make CETS node cleanup consistent

src/component/mongoose_component_cets.erl

@@ -17,8 +17,8 @@ init(_) ->
     cets_discovery:add_table(mongoose_cets_discovery, ?TABLE).
 
 node_cleanup(Node) ->
-    ets:match_delete(?TABLE, #external_component{node = Node, _ = '_'}),
-    ok.
+    Components = ets:match_object(?TABLE, #external_component{node = Node, _ = '_'}),
+    unregister_components(Components).
 
 register_components(Components) ->
     cets:insert_many(?TABLE, Components),

src/ejabberd_sm_cets.erl

@@ -79,13 +79,11 @@ cleanup(Node) ->
     cets:ping_all(?TABLE),
     %% This is a full table scan, but cleanup is rare.
     Tuples = ets:select(?TABLE, [{R, [Guard], ['$_']}]),
-    lists:foreach(fun({_Key, _, _} = Tuple) ->
+    cets:delete_many(?TABLE, [Key || {Key, _, _} <- Tuples]),
+    lists:foreach(fun(Tuple) ->
                           Session = tuple_to_session(Tuple),
                           ejabberd_sm:run_session_cleanup_hook(Session)
-                  end, Tuples),
-    %% We don't need to replicate deletes
-    %% We remove the local content here
-    ets:select_delete(?TABLE, [{R, [Guard], [true]}]).
+                  end, Tuples).
 
 -spec total_count() -> integer().
 total_count() ->

src/mod_bosh_cets.erl

@@ -39,10 +39,8 @@ get_sessions() ->
 
 -spec node_cleanup(atom()) -> any().
 node_cleanup(Node) ->
-    Guard = {'==', {node, '$1'}, Node},
-    R = {'_', '_', '$1'},
+    Guard = {'==', {node, '$2'}, Node},
+    R = {'_', '$1', '$2'},
     cets:ping_all(?TABLE),
-    %% We don't need to replicate deletes
-    %% We remove the local content here
-    ets:select_delete(?TABLE, [{R, [Guard], [true]}]),
-    ok.
+    Keys = ets:select(?TABLE, [{R, [Guard], ['$1']}]),
+    cets:delete_many(?TABLE, Keys).

src/muc/mod_muc_online_cets.erl

@@ -105,10 +105,10 @@ get_online_rooms(HostType, MucHost) ->
 -spec node_cleanup(mongooseim:host_type(), node()) -> ok.
 node_cleanup(HostType, Node) ->
     Tab = table_name(HostType),
-    Pattern = {'_', '$1'},
-    Guard = {'==', {node, '$1'}, Node},
-    ets:select_delete(Tab, [{Pattern, [Guard], [true]}]),
-    ok.
+    Pattern = {'$1', '$2'},
+    Guard = {'==', {node, '$2'}, Node},
+    Keys = ets:select(Tab, [{Pattern, [Guard], ['$1']}]),
+    cets:delete_many(Tab, Keys).
 
 %% Clear table for tests
 -spec clear_table(mongooseim:host_type()) -> ok.

src/s2s/mongoose_s2s_cets.erl

@@ -13,8 +13,6 @@
 %% Internal usage (export so the callback would survive multiple code reloads)
 -export([handle_secret_conflict/2]).
 
--include("mongoose_logger.hrl").
-
 -define(TABLE, cets_s2s_session).
 -define(SECRET_TABLE, cets_s2s_secret).
 
@@ -74,8 +72,8 @@ node_cleanup(Node) ->
     KeyPattern = {'_', '$1'},
     R = {KeyPattern},
     Guard = {'==', {node, '$1'}, Node},
-    ets:select_delete(?TABLE, [{R, [Guard], [true]}]),
-    ok.
+    Objects = ets:select(?TABLE, [{R, [Guard], ['$_']}]),
+    cets:delete_objects(?TABLE, Objects).
 
 %% Secrets
 -spec register_secret(HostType :: mongooseim:host_type(),

test/mongoose_cleanup_SUITE.erl

@@ -1,30 +1,12 @@
 -module(mongoose_cleanup_SUITE).
 
 -include_lib("eunit/include/eunit.hrl").
+-include_lib("common_test/include/ct.hrl").
 -include("mongoose.hrl").
 
--export([all/0, groups/0,
-         init_per_suite/1, end_per_suite/1,
-         init_per_group/2, end_per_group/2,
-         init_per_testcase/2, end_per_testcase/2]).
--export([cleaner_runs_hook_on_nodedown/1,
-         cleaner_runs_hook_on_nodedown_for_host_type/1,
-         notify_self_hook/3,
-         notify_self_hook_for_host_type/3]).
--export([auth_anonymous/1,
-         last/1,
-         stream_management/1,
-         s2s/1,
-         bosh/1,
-         component/1,
-         component_from_other_node_remains/1,
-         muc_node_cleanup_for_host_type/1,
-         muc_room/1,
-         muc_room_from_other_node_remains/1
-        ]).
+-compile([export_all, nowarn_export_all]).
 
 -define(HOST, <<"localhost">>).
--define(NS_CC_2, <<"urn:xmpp:carbons:2">>).
 
 %% -----------------------------------------------------
 %% CT callbacks
@@ -36,17 +18,18 @@ all() ->
      cleaner_runs_hook_on_nodedown_for_host_type,
      auth_anonymous,
      last,
-     stream_management,
-     s2s,
-     bosh,
-     [{group, Group} || {Group, _, _} <- groups()]
+     {group, cets},
+     {group, mnesia}
     ].
 
 groups() ->
-    [{component_cets, [], component_cases()},
-     {component_mnesia, [], component_cases()},
-     {muc_cets, [], muc_cases()},
-     {muc_mnesia, [], muc_cases()}].
+    [{cets, [], backend_tests()},
+     {mnesia, [], backend_tests()},
+     {component, [], component_cases()},
+     {muc, [], muc_cases()}].
+
+backend_tests() ->
+    [{group, component}, {group, muc}, bosh, stream_management, s2s].
 
 component_cases() ->
     [component, component_from_other_node_remains].
@@ -59,62 +42,51 @@ init_per_suite(Config) ->
     ok = mnesia:create_schema([node()]),
     ok = mnesia:start(),
     mongoose_config:set_opts(opts()),
-    Config.
+    lists:foreach(fun setup_meck/1, meck_mods()),
+    async_helper:start(Config, [{mim_ct_sup, start_link, [ejabberd_sup]},
+                                {mongooseim_helper, start_link_loaded_hooks, []},
+                                {mongoose_domain_sup, start_link, []}]).
 
 end_per_suite(Config) ->
+    async_helper:stop_all(Config),
+    lists:foreach(fun unload_meck/1, meck_mods()),
     mongoose_config:erase_opts(),
     mnesia:stop(),
-    mnesia:delete_schema([node()]),
+    mnesia:delete_schema([node()]).
+
+init_per_group(cets, Config) ->
+    [{backend, cets} | start_cets_disco(Config)];
+init_per_group(mnesia, Config) ->
+    [{backend, mnesia} | Config];
+init_per_group(component, Config) ->
+    mongoose_config:set_opt(component_backend, ?config(backend, Config)),
+    [{needs_component, true} | Config];
+init_per_group(Group, Config) ->
+    start_modules(Group, Config),
     Config.
 
-init_per_group(component_mnesia, Config) ->
-    mongoose_config:set_opt(component_backend, mnesia),
-    Config;
-init_per_group(component_cets, Config) ->
-    mongoose_config:set_opt(component_backend, cets),
-    start_cets_disco(Config);
-init_per_group(muc_mnesia, Config) ->
-    [{muc_backend, mnesia} | Config];
-init_per_group(muc_cets, Config) ->
-    [{muc_backend, cets} | start_cets_disco(Config)].
-
-end_per_group(_Group, Config) ->
-    stop_cets_disco(Config).
+end_per_group(cets, Config) ->
+    stop_cets_disco(Config);
+end_per_group(Group, Config) ->
+    stop_modules(Group, Config).
 
+init_per_testcase(s2s, Config) ->
+    mongoose_config:set_opt(s2s_backend, ?config(backend, Config)),
+    Config;
 init_per_testcase(TestCase, Config) ->
-    mim_ct_sup:start_link(ejabberd_sup),
-    {ok, _HooksServer} = mongooseim_helper:start_link_loaded_hooks(),
-    {ok, _DomainSup} = mongoose_domain_sup:start_link(),
-    setup_meck(meck_mods(TestCase)),
-    start_component(TestCase),
-    start_muc(TestCase, Config),
+    start_component_if_needed(?config(needs_component, Config)),
+    start_modules(TestCase, Config),
     Config.
 
-end_per_testcase(TestCase, _Config) ->
-    stop_component(TestCase),
-    mongoose_modules:stop(),
-    mongoose_config:set_opt({modules, ?HOST}, #{}),
-    unload_meck(meck_mods(TestCase)).
-
-start_component(TestCase) ->
-    case needs_component(TestCase) of
-        true ->
-            mongoose_router:start(),
-            mongoose_component:start();
-        false ->
-            ok
-    end.
+end_per_testcase(TestCase, Config) ->
+    stop_modules(TestCase, Config),
+    stop_component_if_needed(?config(needs_component, Config)).
 
-stop_component(TestCase) ->
-    case needs_component(TestCase) of
-        true ->
-            mongoose_component:stop();
-        false ->
-            ok
-    end.
+start_modules(GroupOrCase, Config) ->
+    mongoose_modules:replace_modules(?HOST, [], required_modules(GroupOrCase, Config)).
 
-needs_component(TestCase) ->
-    lists:member(TestCase, component_cases()).
+stop_modules(GroupOrCase, Config) ->
+    mongoose_modules:replace_modules(?HOST, maps:keys(required_modules(GroupOrCase, Config)), #{}).
 
 opts() ->
     #{hosts => [?HOST],
@@ -124,10 +96,31 @@ opts() ->
       {auth, ?HOST} => config_parser_helper:extra_auth(),
       {modules, ?HOST} => #{}}.
 
-meck_mods(bosh) -> [exometer, mod_bosh_socket, cets_dist_blocker];
-meck_mods(s2s) -> [exometer, mongoose_bin, cets_dist_blocker];
-meck_mods(component) -> [exometer, cets_dist_blocker];
-meck_mods(_) -> [exometer, ejabberd_sm, ejabberd_local, cets_dist_blocker].
+meck_mods() ->
+    [exometer, mod_bosh_socket, cets_dist_blocker, mongoose_bin, ejabberd_sm, ejabberd_local].
+
+required_modules(muc, Config) ->
+    required_module(mod_muc, #{online_backend => ?config(backend, Config), backend => mnesia});
+required_modules(bosh, Config) ->
+    required_module(mod_bosh, #{backend => ?config(backend, Config)});
+required_modules(stream_management, Config) ->
+    required_module(mod_stream_management, #{backend => ?config(backend, Config)});
+required_modules(_GroupOrCase, _Config) ->
+    #{}.
+
+required_module(Module, ExtraOpts) ->
+    #{Module => config_parser_helper:mod_config(Module, ExtraOpts)}.
+
+start_component_if_needed(true) ->
+    mongoose_router:start(),
+    mongoose_component:start();
+start_component_if_needed(_) ->
+    ok.
+
+stop_component_if_needed(true) ->
+    mongoose_component:stop();
+stop_component_if_needed(_) ->
+    ok.
 
 %% -----------------------------------------------------
 %% Tests
@@ -204,7 +197,6 @@ last(_Config) ->
 stream_management(_Config) ->
     HostType = ?HOST,
     {U, S, R, _JID, SID} = get_fake_session(),
-    {started, ok} = start(HostType, mod_stream_management),
     SMID = <<"123">>,
     mod_stream_management:register_smid(HostType, SMID, SID),
     {sid, SID} = mod_stream_management:get_sid(HostType, SMID),
@@ -221,7 +213,6 @@ s2s(_Config) ->
     [] = ejabberd_s2s:get_s2s_out_pids(FromTo).
 
 bosh(_Config) ->
-    {started, ok} = start(?HOST, mod_bosh),
     SID = <<"sid">>,
     Self = self(),
     {error, _} = mod_bosh:get_session_socket(SID),
@@ -282,41 +273,33 @@ muc_room_from_other_node_remains(_Config) ->
 %% Internal
 %% -----------------------------------------------------
 
-setup_meck([exometer | R]) ->
-    meck:new(exometer),
+setup_meck(exometer) ->
+    meck:new(exometer, [no_link]),
     meck:expect(exometer, info, fun(_, _) -> undefined end),
     meck:expect(exometer, new, fun(_, _) -> ok end),
-    meck:expect(exometer, update, fun(_, _) -> ok end),
-    setup_meck(R);
-setup_meck([ejabberd_sm | R]) ->
-    meck:new(ejabberd_sm),
+    meck:expect(exometer, update, fun(_, _) -> ok end);
+setup_meck(ejabberd_sm) ->
+    meck:new(ejabberd_sm, [no_link]),
     meck:expect(ejabberd_sm, register_iq_handler,
-                fun(_A1, _A2, _A3) -> ok end),
-    setup_meck(R);
-setup_meck([ejabberd_local | R]) ->
-    meck:new(ejabberd_local),
+                fun(_A1, _A2, _A3) -> ok end);
+setup_meck(ejabberd_local) ->
+    meck:new(ejabberd_local, [no_link]),
     meck:expect(ejabberd_local, register_iq_handler,
-                fun(_A1, _A2, _A3) -> ok end),
-    setup_meck(R);
-setup_meck([mongoose_bin | R]) ->
-    meck:new(mongoose_bin, [passthrough]),
-    meck:expect(mongoose_bin, gen_from_crypto, fun() -> <<"123456">> end),
-    setup_meck(R);
-setup_meck([mod_bosh_socket | R]) ->
-    meck:new(mod_bosh_socket, [passthrough]),
-    meck:expect(mod_bosh_socket, start_supervisor, fun() -> {ok, self()} end),
-    setup_meck(R);
-setup_meck([cets_dist_blocker | R]) ->
-    meck:new(cets_dist_blocker, [passthrough]),
+                fun(_A1, _A2, _A3) -> ok end);
+setup_meck(mongoose_bin) ->
+    meck:new(mongoose_bin, [passthrough, no_link]),
+    meck:expect(mongoose_bin, gen_from_crypto, fun() -> <<"123456">> end);
+setup_meck(mod_bosh_socket) ->
+    meck:new(mod_bosh_socket, [passthrough, no_link]),
+    meck:expect(mod_bosh_socket, start_supervisor, fun() -> {ok, self()} end);
+setup_meck(cets_dist_blocker) ->
+    meck:new(cets_dist_blocker, [passthrough, no_link]),
     meck:expect(cets_dist_blocker, add_cleaner, fun(_Pid) -> ok end),
-    meck:expect(cets_dist_blocker, cleaning_done, fun(_Pid, _Node) -> ok end),
-    setup_meck(R);
-setup_meck([]) ->
-    ok.
+    meck:expect(cets_dist_blocker, cleaning_done, fun(_Pid, _Node) -> ok end).
 
-unload_meck(Mods) ->
-    [ {meck:validate(Mod), meck:unload(Mod)} ||
-      Mod <- Mods ].
+unload_meck(Module) ->
+    meck:validate(Module),
+    meck:unload(Module).
 
 -spec get_fake_session() ->
     {U :: binary(), S :: binary(), R :: binary(),
@@ -367,26 +350,3 @@ remote_pid_binary() ->
 
 remote_pid() ->
     binary_to_term(remote_pid_binary()).
-
-start_muc(TestCase, Config) ->
-    case proplists:get_value(muc_backend, Config) of
-        undefined -> ok;
-        Backend ->
-            case should_start_full_muc_module(TestCase) of
-                true ->
-                    start_muc_module(Backend);
-                false ->
-                    start_muc_backend(Backend)
-            end
-    end.
-
-start_muc_backend(Backend) ->
-    mod_muc_online_backend:start(?HOST, #{online_backend => Backend}).
-
-start_muc_module(Backend) ->
-    ExtraOpts = #{online_backend => Backend, backend => mnesia},
-    Opts = config_parser_helper:mod_config(mod_muc, ExtraOpts),
-    {started, ok} = start(?HOST, mod_muc, Opts).
-
-should_start_full_muc_module(TestCase) ->
-    lists:member(TestCase, [muc_node_cleanup_for_host_type]).