From 774f58f9ed89a54bdd0928edc337021e8c1c1384 Mon Sep 17 00:00:00 2001 From: Aleksey Sabilin Date: Mon, 3 Aug 2026 22:56:16 +0300 Subject: [PATCH] fix(user): transactional status updates and paginated list_users Protect status/login/bot writes with transactions, fix abort Reason shadowing, keep create_bot email_exists, and add list_users/2 pagination. --- src/core/core_user.erl | 218 ++++++++++++++++++++++++++--------------- 1 file changed, 141 insertions(+), 77 deletions(-) diff --git a/src/core/core_user.erl b/src/core/core_user.erl index 25633b2..ddf0ef5 100644 --- a/src/core/core_user.erl +++ b/src/core/core_user.erl @@ -1,9 +1,10 @@ -module(core_user). -include("records.hrl"). +-include_lib("stdlib/include/ms_transform.hrl"). -export([create/2, get_by_id/1, get_by_email/1, update/2, update_status/3, delete/1, update_last_login/1]). -export([email_exists/1]). --export([list_users/0]). +-export([list_users/0, list_users/2]). -export([block/2, unblock/2]). -export([count_users/0, count_users_by_date/2, count_pending_users/0, count_pending_users_by_date/2, list_all/0]). -export([count_users_by_role/0, count_users_by_status/0]). @@ -80,10 +81,10 @@ get_by_id(Id) -> %%%------------------------------------------------------------------- -spec get_by_email(Email :: binary()) -> {ok, #user{}} | {error, not_found}. get_by_email(Email) -> - Match = #user{email = Email, _ = '_'}, - case mnesia:dirty_match_object(Match) of + %% Use the index on #user.email instead of a full table scan. + case mnesia:dirty_index_read(user, Email, #user.email) of [] -> {error, not_found}; - [User] -> {ok, User} + [User | _] -> {ok, User} end. %%%------------------------------------------------------------------- @@ -125,14 +126,21 @@ update(Id, Updates) -> %%% Устанавливает `last_login` в текущее UTC-время. %%% @end %%%------------------------------------------------------------------- --spec update_last_login(Id :: binary()) -> {ok, #user{}} | {error, not_found}. +-spec update_last_login(Id :: binary()) -> {ok, #user{}} | {error, not_found | term()}. update_last_login(Id) -> - case get_by_id(Id) of - {ok, User} -> - Updated = User#user{last_login = calendar:universal_time()}, - mnesia:dirty_write(Updated), - {ok, Updated}; - Error -> Error + %% Transaction prevents lost updates when concurrent logins happen. + F = fun() -> + case mnesia:read(user, Id, write) of + [] -> {error, not_found}; + [User] -> + Updated = User#user{last_login = calendar:universal_time()}, + mnesia:write(Updated), + {ok, Updated} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, Reason} -> {error, Reason} end. %%%------------------------------------------------------------------- @@ -140,15 +148,23 @@ update_last_login(Id) -> %%% @end %%%------------------------------------------------------------------- -spec update_status(Id :: binary(), Status :: atom(), Reason :: binary()) -> - {ok, #user{}} | {error, not_found}. + {ok, #user{}} | {error, not_found | term()}. update_status(Id, Status, Reason) -> - case get_by_id(Id) of - {ok, User} -> - Updated = User#user{status = Status, reason = Reason, - updated_at = calendar:universal_time()}, - mnesia:dirty_write(Updated), - {ok, Updated}; - Error -> Error + %% Transaction ensures atomic status change — prevents lost updates + %% when two admins change the same user's status simultaneously. + F = fun() -> + case mnesia:read(user, Id, write) of + [] -> {error, not_found}; + [User] -> + Updated = User#user{status = Status, reason = Reason, + updated_at = calendar:universal_time()}, + mnesia:write(Updated), + {ok, Updated} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, AbortReason} -> {error, AbortReason} end. %%%------------------------------------------------------------------- @@ -167,9 +183,32 @@ delete(Id) -> %%%------------------------------------------------------------------- -spec list_users() -> {ok, [map()]}. list_users() -> - Users = mnesia:dirty_match_object(#user{_ = '_'}), - ActiveUsers = [U || U <- Users, U#user.status =/= deleted], - {ok, [user_to_map(U) || U <- ActiveUsers]}. + %% Default pagination: first 100 non-deleted users. + list_users(100, 0). + +%%%------------------------------------------------------------------- +%%% @doc Получить список активных пользователей с пагинацией. +%%% `Limit` — максимальное количество записей, `Offset` — смещение. +%%% Фильтрация status =/= deleted выполняется в match-спецификации. +%%% @end +%%%------------------------------------------------------------------- +-spec list_users(Limit :: pos_integer(), Offset :: non_neg_integer()) -> {ok, [map()]}. +list_users(Limit, Offset) -> + %% Use select with a match spec to filter out deleted users at DB level + %% and apply limit for pagination. + MS = ets:fun2ms(fun(#user{status = S} = U) when S =/= deleted -> U end), + case mnesia:dirty_select(user, MS) of + [] -> + {ok, []}; + All -> + %% Apply offset/limit at the list level (Mnesia dirty_select + %% doesn't support offset natively, but we filtered at DB level). + Paged = case length(All) > Offset of + true -> lists:sublist(lists:nthtail(Offset, All), Limit); + false -> [] + end, + {ok, [user_to_map(U) || U <- Paged]} + end. %%%------------------------------------------------------------------- %%% @doc Преобразование записи пользователя в map. @@ -203,15 +242,22 @@ user_to_map(User) -> %%% @end %%%------------------------------------------------------------------- -spec block(Id :: binary(), Reason :: binary()) -> - {ok, #user{}} | {error, not_found}. + {ok, #user{}} | {error, not_found | term()}. block(Id, Reason) -> - case get_by_id(Id) of - {ok, User} -> - Updated = User#user{status = blocked, reason = Reason, - updated_at = calendar:universal_time()}, - mnesia:dirty_write(Updated), - {ok, Updated}; - Error -> Error + %% Transaction prevents concurrent status changes from racing. + F = fun() -> + case mnesia:read(user, Id, write) of + [] -> {error, not_found}; + [User] -> + Updated = User#user{status = blocked, reason = Reason, + updated_at = calendar:universal_time()}, + mnesia:write(Updated), + {ok, Updated} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, AbortReason} -> {error, AbortReason} end. %%%------------------------------------------------------------------- @@ -219,15 +265,22 @@ block(Id, Reason) -> %%% @end %%%------------------------------------------------------------------- -spec unblock(Id :: binary(), Reason :: binary()) -> - {ok, #user{}} | {error, not_found}. + {ok, #user{}} | {error, not_found | term()}. unblock(Id, Reason) -> - case get_by_id(Id) of - {ok, User} -> - Updated = User#user{status = active, reason = Reason, - updated_at = calendar:universal_time()}, - mnesia:dirty_write(Updated), - {ok, Updated}; - Error -> Error + %% Transaction prevents concurrent status changes from racing. + F = fun() -> + case mnesia:read(user, Id, write) of + [] -> {error, not_found}; + [User] -> + Updated = User#user{status = active, reason = Reason, + updated_at = calendar:universal_time()}, + mnesia:write(Updated), + {ok, Updated} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, AbortReason} -> {error, AbortReason} end. %%%------------------------------------------------------------------- @@ -393,50 +446,61 @@ set_field(_, _, U) -> U. -spec create_bot(Email :: binary(), Password :: binary()) -> {ok, #user{}} | {error, email_exists | invalid_email}. create_bot(Email, Password) -> - case email_exists(Email) of - true -> - {error, email_exists}; - false -> - case mnesia:dirty_index_read(user, Email, email) of - [] -> - {ok, PasswordHash} = logic_auth:hash_password(Password), - Id = infra_utils:generate_id(16), - Now = calendar:universal_time(), - User = #user{ - id = Id, - email = Email, - password_hash = PasswordHash, - role = bot, - status = active, - reason = ?DEFAULT_REASON, - nickname = extract_nickname(Email), - avatar_url = ?DEFAULT_AVATAR_URL, - timezone = ?DEFAULT_TIMEZONE, - language = ?DEFAULT_LANGUAGE, - social_links = ?DEFAULT_SOCIAL_LINKS, - phone = ?DEFAULT_PHONE, - preferences = ?DEFAULT_PREFERENCES, - last_login = ?DEFAULT_LAST_LOGIN, - created_at = Now, - updated_at = Now - }, - ok = mnesia:dirty_write(User), - {ok, User}; - _ -> - {error, duplicate_email} - end + %% Hash password outside the transaction to minimize lock time. + {ok, PasswordHash} = logic_auth:hash_password(Password), + %% Transaction ensures the email-uniqueness check and write are atomic, + %% preventing duplicate bots under concurrent creation. + F = fun() -> + case mnesia:index_read(user, Email, #user.email) of + [] -> + Id = infra_utils:generate_id(16), + Now = calendar:universal_time(), + User = #user{ + id = Id, + email = Email, + password_hash = PasswordHash, + role = bot, + status = active, + reason = ?DEFAULT_REASON, + nickname = extract_nickname(Email), + avatar_url = ?DEFAULT_AVATAR_URL, + timezone = ?DEFAULT_TIMEZONE, + language = ?DEFAULT_LANGUAGE, + social_links = ?DEFAULT_SOCIAL_LINKS, + phone = ?DEFAULT_PHONE, + preferences = ?DEFAULT_PREFERENCES, + last_login = ?DEFAULT_LAST_LOGIN, + created_at = Now, + updated_at = Now + }, + ok = mnesia:write(User), + {ok, User}; + _ -> + {error, email_exists} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, AbortReason} -> {error, AbortReason} end. %%%------------------------------------------------------------------- %%% @doc Удаление бота (физическое удаление записи, не мягкое). %%% @end %%%------------------------------------------------------------------- --spec delete_bot(Id :: binary()) -> ok | {error, not_found}. +-spec delete_bot(Id :: binary()) -> ok | {error, not_found | term()}. delete_bot(Id) -> - case mnesia:dirty_read({user, Id}) of - [#user{role = bot}] -> - mnesia:dirty_delete({user, Id}), - ok; - [] -> - {error, not_found} + %% Transaction ensures the read-then-delete is atomic. + F = fun() -> + case mnesia:read(user, Id, write) of + [#user{role = bot}] -> + mnesia:delete({user, Id}), + ok; + _ -> + {error, not_found} + end + end, + case mnesia:transaction(F) of + {atomic, Result} -> Result; + {aborted, Reason} -> {error, Reason} end. \ No newline at end of file