diff --git a/src/ejabberd_sm_kiss.erl b/src/ejabberd_sm_kiss.erl index 28302a91704..d3483846e42 100644 --- a/src/ejabberd_sm_kiss.erl +++ b/src/ejabberd_sm_kiss.erl @@ -60,13 +60,19 @@ create_session(User, Server, Resource, Session) -> case get_sessions(User, Server, Resource) of [] -> kiss:insert(?TABLE, session_to_tuple(Session)); - Tuples when is_list(Tuples) -> + Sessions when is_list(Sessions) -> %% Fix potential race condition during XMPP bind, where %% multiple calls (> 2) to ejabberd_sm:open_session %% have been made, resulting in >1 sessions for this resource -% MergedSession = mongoose_session:merge_info -% (Session, hd(lists:sort(Sessions))), - kiss:insert(?TABLE, session_to_tuple(Session)) + %% XXX Why do we need that exactly? + %% Sessions are open from c2s and that specific process is updating + %% its session info. Adding info from other processes would cause some + %% strange bugs. On another hand, there is very limited usage + %% of that info field, so nothing would probably break if + %% we keep calling merge_info (and it would make ejabberd_sm_SUITE happy). + MergedSession = mongoose_session:merge_info + (Session, hd(lists:sort(Sessions))), + kiss:insert(?TABLE, session_to_tuple(MergedSession)) end. -spec update_session(User :: jid:luser(), diff --git a/test/ejabberd_sm_SUITE.erl b/test/ejabberd_sm_SUITE.erl index 2ed05789c65..0c327d68acc 100644 --- a/test/ejabberd_sm_SUITE.erl +++ b/test/ejabberd_sm_SUITE.erl @@ -17,7 +17,7 @@ -define(MAX_USER_SESSIONS, 2). -all() -> [{group, mnesia}, {group, redis}]. +all() -> [{group, mnesia}, {group, redis}, {group, kiss}]. init_per_suite(C) -> {ok, _} = application:ensure_all_started(jid), @@ -37,7 +37,8 @@ end_per_suite(C) -> groups() -> [{mnesia, [], tests()}, - {redis, [], tests()}]. + {redis, [], tests()}, + {kiss, [], tests()}]. tests() -> [open_session, @@ -68,7 +69,11 @@ init_per_group(mnesia, Config) -> ok = mnesia:start(), [{backend, ejabberd_sm_mnesia} | Config]; init_per_group(redis, Config) -> - init_redis_group(is_redis_running(), Config). + init_redis_group(is_redis_running(), Config); +init_per_group(kiss, Config) -> + DiscoOpts = #{name => mongoose_kiss_discovery, disco_file => "does_not_exist.txt"}, + {ok, Pid} = kiss_discovery:start(DiscoOpts), + [{backend, ejabberd_sm_kiss}, {kiss_disco_pid, Pid} | Config]. init_redis_group(true, Config) -> Self = self(), @@ -91,7 +96,10 @@ end_per_group(mnesia, Config) -> mnesia:stop(), mnesia:delete_schema([node()]), Config; -end_per_group(_, Config) -> +end_per_group(kiss, Config) -> + exit(proplists:get_value(kiss_disco_pid, Config), kill), + Config; +end_per_group(redis, Config) -> whereis(test_helper) ! stop, Config. @@ -388,6 +396,8 @@ get_fun_for_unique_count(ejabberd_sm_mnesia) -> fun() -> mnesia:abort({badarg,[session,{{1442,941593,580189},list_to_pid("<0.23291.6>")}]}) end; +get_fun_for_unique_count(ejabberd_sm_kiss) -> + fun() -> error(oops) end; get_fun_for_unique_count(ejabberd_sm_redis) -> fun() -> %% The code below is on purpose, it's to crash with badarg reason @@ -423,6 +433,8 @@ verify_session_opened(C, Sid, USR) -> do_verify_session_opened(ejabberd_sm_mnesia, Sid, {U, S, R} = USR) -> general_session_check(ejabberd_sm_mnesia, Sid, USR, U, S, R); +do_verify_session_opened(ejabberd_sm_kiss, Sid, {U, S, R} = USR) -> + general_session_check(ejabberd_sm_kiss, Sid, USR, U, S, R); do_verify_session_opened(ejabberd_sm_redis, Sid, {U, S, R} = USR) -> UHash = iolist_to_binary(hash(U, S, R, Sid)), Hashes = mongoose_redis:cmd(["SMEMBERS", n(node())]), @@ -451,7 +463,9 @@ clean_sessions(C) -> ejabberd_sm_mnesia -> mnesia:clear_table(session); ejabberd_sm_redis -> - mongoose_redis:cmd(["FLUSHALL"]) + mongoose_redis:cmd(["FLUSHALL"]); + ejabberd_sm_kiss -> + ets:delete_all_objects(kiss_session) end. generate_random_user(S) -> @@ -590,6 +604,8 @@ setup_sm(Config) -> ejabberd_sm_redis -> mongoose_redis:cmd(["FLUSHALL"]); ejabberd_sm_mnesia -> + ok; + ejabberd_sm_kiss -> ok end. @@ -611,7 +627,9 @@ opts(Config) -> sm_backend(ejabberd_sm_redis) -> {redis, [{pool_size, 3}, {worker_config, [{host, "localhost"}, {port, 6379}]}]}; sm_backend(ejabberd_sm_mnesia) -> - {mnesia, []}. + {mnesia, []}; +sm_backend(ejabberd_sm_kiss) -> + {kiss, []}. set_meck() -> meck:expect(gen_hook, add_handler, fun(_, _, _, _, _) -> ok end),