diff --git a/big_tests/tests/graphql_last_SUITE.erl b/big_tests/tests/graphql_last_SUITE.erl index f208fd243a..8d936187ae 100644 --- a/big_tests/tests/graphql_last_SUITE.erl +++ b/big_tests/tests/graphql_last_SUITE.erl @@ -5,7 +5,7 @@ -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_command/4, execute_user_command/5, user_to_bin/1, user_to_jid/1, get_ok_value/2, get_err_msg/1, get_err_code/1, get_unauthorized/1, - get_not_loaded/1]). + get_not_loaded/1, get_coercion_err_msg/1]). -include_lib("eunit/include/eunit.hrl"). @@ -13,7 +13,10 @@ -define(assertErrCode(Res, Code), assert_err_code(Code, Res)). -define(NONEXISTENT_JID, <<"user@user.com">>). +-define(NONEXISTENT_NAME, <<"user@", (domain_helper:domain())/binary>>). +-define(EMPTY_NAME_JID, <<"@", (domain_helper:domain())/binary>>). -define(DEFAULT_DT, <<"2022-04-17T12:58:30.000000Z">>). +-define(INVALID_TIMESTAMP, <<"20222-04-17T12:58:30.000000Z">>). -define(NONEXISTENT_DOMAIN, <<"nonexistent">>). suite() -> @@ -73,18 +76,22 @@ user_last_not_configured() -> admin_last_tests() -> [admin_set_last, admin_try_set_nonexistent_user_last, + admin_try_set_last_invalid_timestamp, admin_get_last, admin_get_nonexistent_user_last, admin_try_get_nonexistent_last, admin_count_active_users, - admin_try_count_nonexistent_domain_active_users]. + admin_try_count_nonexistent_domain_active_users, + admin_try_count_active_users_invalid_timestamp]. admin_old_users_tests() -> [admin_list_old_users_domain, admin_try_list_old_users_nonexistent_domain, + admin_try_list_old_users_invalid_timestamp, admin_list_old_users_global, admin_remove_old_users_domain, admin_try_remove_old_users_nonexistent_domain, + admin_try_remove_old_users_invalid_timestamp, admin_remove_old_users_global, admin_user_without_last_info_is_old_user, admin_logged_user_is_not_old_user]. @@ -92,14 +99,18 @@ admin_old_users_tests() -> domain_admin_last_tests() -> [admin_set_last, domain_admin_set_user_last_no_permission, + admin_try_set_last_invalid_timestamp, admin_get_last, domain_admin_get_user_last_no_permission, admin_try_get_nonexistent_last, admin_count_active_users, - domain_admin_try_count_external_domain_active_users]. + domain_admin_try_count_external_domain_active_users, + admin_try_count_active_users_invalid_timestamp]. domain_admin_old_users_tests() -> [admin_list_old_users_domain, + admin_try_list_old_users_invalid_timestamp, + admin_try_remove_old_users_invalid_timestamp, domain_admin_try_list_old_users_external_domain, domain_admin_list_old_users_global, domain_admin_remove_old_users_global, @@ -244,7 +255,20 @@ admin_set_last_story(Config, Alice) -> admin_try_set_nonexistent_user_last(Config) -> Res = admin_set_last(?NONEXISTENT_JID, <<"status">>, null, Config), ?assertErrMsg(Res, <<"not exist">>), - ?assertErrCode(Res, user_does_not_exist). + ?assertErrCode(Res, user_does_not_exist), + Res2 = admin_set_last(?NONEXISTENT_NAME, <<"status">>, null, Config), + ?assertErrMsg(Res2, <<"not exist">>), + ?assertErrCode(Res2, user_does_not_exist), + Res3 = admin_set_last(?EMPTY_NAME_JID, <<"status">>, null, Config), + get_coercion_err_msg(Res3). + +admin_try_set_last_invalid_timestamp(Config) -> + escalus:fresh_story_with_config(Config, [{alice, 1}], + fun admin_try_set_last_invalid_timestamp_story/2). + +admin_try_set_last_invalid_timestamp_story(Config, Alice) -> + Res = admin_set_last(Alice, <<"status">>, ?INVALID_TIMESTAMP, Config), + get_coercion_err_msg(Res). admin_get_last(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], @@ -261,7 +285,12 @@ admin_get_last_story(Config, Alice) -> admin_get_nonexistent_user_last(Config) -> Res = admin_get_last(?NONEXISTENT_JID, Config), ?assertErrMsg(Res, <<"not exist">>), - ?assertErrCode(Res, user_does_not_exist). + ?assertErrCode(Res, user_does_not_exist), + Res2 = admin_get_last(?NONEXISTENT_NAME, Config), + ?assertErrMsg(Res2, <<"not exist">>), + ?assertErrCode(Res2, user_does_not_exist), + Res3 = admin_get_last(?EMPTY_NAME_JID, Config), + get_coercion_err_msg(Res3). admin_try_get_nonexistent_last(Config) -> escalus:fresh_story_with_config(Config, [{alice, 1}], @@ -290,6 +319,11 @@ admin_try_count_nonexistent_domain_active_users(Config) -> ?assertErrMsg(Res, <<"not found">>), ?assertErrCode(Res, domain_not_found). +admin_try_count_active_users_invalid_timestamp(Config) -> + Domain = domain_helper:domain(), + Res = admin_count_active_users(Domain, ?INVALID_TIMESTAMP, Config), + get_coercion_err_msg(Res). + %% Admin old users test cases admin_remove_old_users_domain(Config) -> @@ -315,6 +349,11 @@ admin_try_remove_old_users_nonexistent_domain(Config) -> ?assertErrMsg(Res, <<"not found">>), ?assertErrCode(Res, domain_not_found). +admin_try_remove_old_users_invalid_timestamp(Config) -> + Domain = domain_helper:domain(), + Res = admin_remove_old_users(Domain, ?INVALID_TIMESTAMP, Config), + get_coercion_err_msg(Res). + admin_remove_old_users_global(Config) -> jids_with_config(Config, [alice, alice_bis, bob], fun admin_remove_old_users_global_story/4). @@ -355,6 +394,11 @@ admin_try_list_old_users_nonexistent_domain(Config) -> ?assertErrMsg(Res, <<"not found">>), ?assertErrCode(Res, domain_not_found). +admin_try_list_old_users_invalid_timestamp(Config) -> + Domain = domain_helper:domain(), + Res = admin_list_old_users(Domain, ?INVALID_TIMESTAMP, Config), + get_coercion_err_msg(Res). + admin_list_old_users_global(Config) -> jids_with_config(Config, [alice, alice_bis, bob], fun admin_list_old_users_global_story/4). diff --git a/src/admin_extra/service_admin_extra_accounts.erl b/src/admin_extra/service_admin_extra_accounts.erl index 3ffea82374..108aa35d11 100644 --- a/src/admin_extra/service_admin_extra_accounts.erl +++ b/src/admin_extra/service_admin_extra_accounts.erl @@ -133,7 +133,7 @@ num_active_users(Domain, Days) -> -spec delete_old_users(integer()) -> {ok, iolist()}. delete_old_users(Days) -> Timestamp = days_to_timestamp(Days), - OldUsers = mod_last_api:remove_old_users(Timestamp), + {ok, OldUsers} = mod_last_api:remove_old_users(Timestamp), {ok, format_deleted_users(OldUsers)}. -spec delete_old_users_for_domain(jid:server(), integer()) -> {ok | domain_not_found, iolist()}. diff --git a/src/graphql/admin/mongoose_graphql_last_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_last_admin_mutation.erl index 55a1a6f2ae..d55a9f7564 100644 --- a/src/graphql/admin/mongoose_graphql_last_admin_mutation.erl +++ b/src/graphql/admin/mongoose_graphql_last_admin_mutation.erl @@ -26,7 +26,7 @@ set_last(#{<<"user">> := JID, <<"timestamp">> := Timestamp, <<"status">> := Stat -spec remove_old_users(args()) -> {ok, old_users()} | {error, resolver_error()}. remove_old_users(#{<<"domain">> := null, <<"timestamp">> := Timestamp}) -> Timestamp2 = mongoose_graphql_last_helper:microseconds_to_seconds(Timestamp), - OldUsers = mod_last_api:remove_old_users(Timestamp2), + {ok, OldUsers} = mod_last_api:remove_old_users(Timestamp2), {ok, format_old_users(OldUsers)}; remove_old_users(#{<<"domain">> := Domain, <<"timestamp">> := Timestamp}) -> Timestamp2 = mongoose_graphql_last_helper:microseconds_to_seconds(Timestamp), diff --git a/src/graphql/admin/mongoose_graphql_last_admin_query.erl b/src/graphql/admin/mongoose_graphql_last_admin_query.erl index d15be950ac..07d93b02c1 100644 --- a/src/graphql/admin/mongoose_graphql_last_admin_query.erl +++ b/src/graphql/admin/mongoose_graphql_last_admin_query.erl @@ -35,7 +35,7 @@ count_active_users(#{<<"domain">> := Domain, <<"timestamp">> := Timestamp}) -> -spec list_old_users(args()) -> {ok, old_users()} | {error, resolver_error()}. list_old_users(#{<<"domain">> := null, <<"timestamp">> := Timestamp}) -> Timestamp2 = mongoose_graphql_last_helper:microseconds_to_seconds(Timestamp), - OldUsers = mod_last_api:list_old_users(Timestamp2), + {ok, OldUsers} = mod_last_api:list_old_users(Timestamp2), {ok, format_old_users(OldUsers)}; list_old_users(#{<<"domain">> := Domain, <<"timestamp">> := Timestamp}) -> Timestamp2 = mongoose_graphql_last_helper:microseconds_to_seconds(Timestamp), diff --git a/src/mod_last_api.erl b/src/mod_last_api.erl index 6c15f2c59f..61f9d98bd5 100644 --- a/src/mod_last_api.erl +++ b/src/mod_last_api.erl @@ -57,11 +57,12 @@ count_active_users(Domain, Timestamp) -> ?DOMAIN_NOT_FOUND_RESULT end. --spec list_old_users(timestamp()) -> [old_user()]. +-spec list_old_users(timestamp()) -> {ok, [old_user()]}. list_old_users(Timestamp) -> - lists:append([list_old_users(HostType, Domain, Timestamp) || - HostType <- ?ALL_HOST_TYPES, - Domain <- mongoose_domain_api:get_domains_by_host_type(HostType)]). + OldUsers = lists:append([list_old_users(HostType, Domain, Timestamp) || + HostType <- ?ALL_HOST_TYPES, + Domain <- mongoose_domain_api:get_domains_by_host_type(HostType)]), + {ok, OldUsers}. -spec list_old_users(jid:server(), timestamp()) -> {ok, [old_user()]} | {domain_not_found, binary()}. @@ -74,11 +75,11 @@ list_old_users(Domain, Timestamp) -> ?DOMAIN_NOT_FOUND_RESULT end. --spec remove_old_users(timestamp()) -> [old_user()]. +-spec remove_old_users(timestamp()) -> {ok, [old_user()]}. remove_old_users(Timestamp) -> - OldUsers = list_old_users(Timestamp), + {ok, OldUsers} = list_old_users(Timestamp), ok = remove_users(OldUsers), - OldUsers. + {ok, OldUsers}. -spec remove_old_users(jid:server(), timestamp()) -> {ok, [old_user()]} | {domain_not_found, binary()}.