From 1bea67cb5e70ae28209a193c33b9da2d3c41cfb7 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 24 Sep 2019 14:14:34 +0700 Subject: [PATCH] Cleanup Pleroma.User --- lib/pleroma/user.ex | 231 ++++++++---------- lib/pleroma/web/common_api/common_api.ex | 25 +- .../web/twitter_api/twitter_api_controller.ex | 7 +- 3 files changed, 108 insertions(+), 155 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 422bc6fa6..640ef05c4 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -106,9 +106,7 @@ def profile_url(%User{info: %{source_data: %{"url" => url}}}), do: url def profile_url(%User{ap_id: ap_id}), do: ap_id def profile_url(_), do: nil - def ap_id(%User{nickname: nickname}) do - "#{Web.base_url()}/users/#{nickname}" - end + def ap_id(%User{nickname: nickname}), do: "#{Web.base_url()}/users/#{nickname}" def ap_followers(%User{follower_address: fa}) when is_binary(fa), do: fa def ap_followers(%User{} = user), do: "#{ap_id(user)}/followers" @@ -119,12 +117,9 @@ def ap_following(%User{} = user), do: "#{ap_id(user)}/following" def user_info(%User{} = user, args \\ %{}) do following_count = - if args[:following_count], - do: args[:following_count], - else: user.info.following_count || following_count(user) + Map.get(args, :following_count, user.info.following_count || following_count(user)) - follower_count = - if args[:follower_count], do: args[:follower_count], else: user.info.follower_count + follower_count = Map.get(args, :follower_count, user.info.follower_count) %{ note_count: user.info.note_count, @@ -137,12 +132,11 @@ def user_info(%User{} = user, args \\ %{}) do end def follow_state(%User{} = user, %User{} = target) do - follow_activity = Utils.fetch_latest_follow(user, target) - - if follow_activity, - do: follow_activity.data["state"], + case Utils.fetch_latest_follow(user, target) do + %{data: %{"state" => state}} -> state # Ideally this would be nil, but then Cachex does not commit the value - else: false + _ -> false + end end def get_cached_follow_state(user, target) do @@ -152,11 +146,7 @@ def get_cached_follow_state(user, target) do @spec set_follow_state_cache(String.t(), String.t(), String.t()) :: {:ok | :error, boolean()} def set_follow_state_cache(user_ap_id, target_ap_id, state) do - Cachex.put( - :user_cache, - "follow_state:#{user_ap_id}|#{target_ap_id}", - state - ) + Cachex.put(:user_cache, "follow_state:#{user_ap_id}|#{target_ap_id}", state) end def set_info_cache(user, args) do @@ -197,32 +187,25 @@ def remote_user_creation(params) do |> truncate_if_exists(:name, name_limit) |> truncate_if_exists(:bio, bio_limit) - changes = - %User{} + changeset = + %User{local: false} |> cast(params, [:bio, :name, :ap_id, :nickname, :avatar]) |> validate_required([:name, :ap_id]) |> unique_constraint(:nickname) |> validate_format(:nickname, @email_regex) |> validate_length(:bio, max: bio_limit) |> validate_length(:name, max: name_limit) - |> put_change(:local, false) |> change_info(&User.Info.remote_user_creation(&1, params[:info])) - if changes.valid? do - case info_cng.changes[:source_data] do - %{"followers" => followers, "following" => following} -> - changes - |> put_change(:follower_address, followers) - |> put_change(:following_address, following) + case params[:info][:source_data] do + %{"followers" => followers, "following" => following} -> + changeset + |> put_change(:follower_address, followers) + |> put_change(:following_address, following) - _ -> - followers = User.ap_followers(%User{nickname: changes.changes[:nickname]}) - - changes - |> put_change(:follower_address, followers) - end - else - changes + _ -> + followers = ap_followers(%User{nickname: get_field(changeset, :nickname)}) + put_change(changeset, :follower_address, followers) end end @@ -308,43 +291,39 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do opts[:need_confirmation] end - info_change = - User.Info.confirmation_changeset(%User.Info{}, need_confirmation: need_confirmation?) + struct + |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation]) + |> validate_required([:name, :nickname, :password, :password_confirmation]) + |> validate_confirmation(:password) + |> unique_constraint(:email) + |> unique_constraint(:nickname) + |> validate_exclusion(:nickname, Pleroma.Config.get([User, :restricted_nicknames])) + |> validate_format(:nickname, local_nickname_regex()) + |> validate_format(:email, @email_regex) + |> validate_length(:bio, max: bio_limit) + |> validate_length(:name, min: 1, max: name_limit) + |> change_info(&User.Info.confirmation_changeset(&1, need_confirmation: need_confirmation?)) + |> maybe_validate_required_email(opts[:external]) + |> put_password_hash + |> put_ap_id() + |> unique_constraint(:ap_id) + |> put_following_and_follower_address() + end - changeset = - struct - |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation]) - |> validate_required([:name, :nickname, :password, :password_confirmation]) - |> validate_confirmation(:password) - |> unique_constraint(:email) - |> unique_constraint(:nickname) - |> validate_exclusion(:nickname, Pleroma.Config.get([User, :restricted_nicknames])) - |> validate_format(:nickname, local_nickname_regex()) - |> validate_format(:email, @email_regex) - |> validate_length(:bio, max: bio_limit) - |> validate_length(:name, min: 1, max: name_limit) - |> put_change(:info, info_change) + def maybe_validate_required_email(changeset, true), do: changeset + def maybe_validate_required_email(changeset, _), do: validate_required(changeset, [:email]) - changeset = - if opts[:external] do - changeset - else - validate_required(changeset, [:email]) - end + defp put_ap_id(changeset) do + ap_id = ap_id(%User{nickname: get_field(changeset, :nickname)}) + put_change(changeset, :ap_id, ap_id) + end - if changeset.valid? do - ap_id = User.ap_id(%User{nickname: changeset.changes[:nickname]}) - followers = User.ap_followers(%User{nickname: changeset.changes[:nickname]}) + defp put_following_and_follower_address(changeset) do + followers = ap_followers(%User{nickname: get_field(changeset, :nickname)}) - changeset - |> put_password_hash - |> put_change(:ap_id, ap_id) - |> unique_constraint(:ap_id) - |> put_change(:following, [followers]) - |> put_change(:follower_address, followers) - else - changeset - end + changeset + |> put_change(:following, [followers]) + |> put_change(:follower_address, followers) end defp autofollow_users(user) do @@ -359,9 +338,8 @@ defp autofollow_users(user) do @doc "Inserts provided changeset, performs post-registration actions (confirmation email sending etc.)" def register(%Ecto.Changeset{} = changeset) do - with {:ok, user} <- Repo.insert(changeset), - {:ok, user} <- post_register_action(user) do - {:ok, user} + with {:ok, user} <- Repo.insert(changeset) do + post_register_action(user) end end @@ -407,7 +385,7 @@ def maybe_direct_follow(%User{} = follower, %User{local: true} = followed) do end def maybe_direct_follow(%User{} = follower, %User{} = followed) do - if not User.ap_enabled?(followed) do + if not ap_enabled?(followed) do follow(follower, followed) else {:ok, follower} @@ -440,9 +418,7 @@ def follow_all(follower, followeds) do {1, [follower]} = Repo.update_all(q, []) - Enum.each(followeds, fn followed -> - update_follower_count(followed) - end) + Enum.each(followeds, &update_follower_count/1) set_cache(follower) end @@ -552,8 +528,6 @@ def set_cache(%User{} = user) do def update_and_set_cache(changeset) do with {:ok, user} <- Repo.update(changeset, stale_error_field: :id) do set_cache(user) - else - e -> e end end @@ -590,9 +564,7 @@ def get_cached_by_nickname(nickname) do key = "nickname:#{nickname}" Cachex.fetch!(:user_cache, key, fn -> - user_result = get_or_fetch_by_nickname(nickname) - - case user_result do + case get_or_fetch_by_nickname(nickname) do {:ok, user} -> {:commit, user} {:error, _error} -> {:ignore, nil} end @@ -632,13 +604,11 @@ def get_by_nickname_or_email(nickname_or_email) do def get_cached_user_info(user) do key = "user_info:#{user.id}" - Cachex.fetch!(:user_cache, key, fn _ -> user_info(user) end) + Cachex.fetch!(:user_cache, key, fn -> user_info(user) end) end def fetch_by_nickname(nickname) do - ap_try = ActivityPub.make_user_from_nickname(nickname) - - case ap_try do + case ActivityPub.make_user_from_nickname(nickname) do {:ok, user} -> {:ok, user} _ -> OStatus.make_user(nickname) end @@ -673,7 +643,8 @@ def get_followers_query(%User{} = user, nil) do end def get_followers_query(user, page) do - from(u in get_followers_query(user, nil)) + user + |> get_followers_query(nil) |> User.Query.paginate(page, 20) end @@ -689,18 +660,17 @@ def get_followers(user, page \\ nil) do @spec get_external_followers(User.t(), pos_integer()) :: {:ok, list(User.t())} def get_external_followers(user, page \\ nil) do - q = - user - |> get_followers_query(page) - |> User.Query.build(%{external: true}) - - {:ok, Repo.all(q)} + user + |> get_followers_query(page) + |> User.Query.build(%{external: true}) + |> Repo.all() end def get_followers_ids(user, page \\ nil) do - q = get_followers_query(user, page) - - Repo.all(from(u in q, select: u.id)) + user + |> get_followers_query(page) + |> select([u], u.id) + |> Repo.all() end @spec get_friends_query(User.t(), pos_integer() | nil) :: Ecto.Query.t() @@ -709,7 +679,8 @@ def get_friends_query(%User{} = user, nil) do end def get_friends_query(user, page) do - from(u in get_friends_query(user, nil)) + user + |> get_friends_query(nil) |> User.Query.paginate(page, 20) end @@ -723,9 +694,10 @@ def get_friends(user, page \\ nil) do end def get_friends_ids(user, page \\ nil) do - q = get_friends_query(user, page) - - Repo.all(from(u in q, select: u.id)) + user + |> get_friends_query(page) + |> select([u], u.id) + |> Repo.all() end @spec get_follow_requests(User.t()) :: {:ok, [User.t()]} @@ -889,12 +861,10 @@ def unmute(muter, %{ap_id: ap_id}) do end def subscribe(subscriber, %{ap_id: ap_id}) do - deny_follow_blocked = Pleroma.Config.get([:user, :deny_follow_blocked]) - with %User{} = subscribed <- get_cached_by_ap_id(ap_id) do - blocked = blocks?(subscribed, subscriber) and deny_follow_blocked + deny_follow_blocked = Pleroma.Config.get([:user, :deny_follow_blocked]) - if blocked do + if blocks?(subscribed, subscriber) and deny_follow_blocked do {:error, "Could not subscribe: #{subscribed.nickname} is blocking you"} else update_info(subscribed, &User.Info.add_to_subscribers(&1, subscriber.ap_id)) @@ -933,9 +903,7 @@ def block(blocker, %User{ap_id: ap_id} = blocked) do blocker end - if following?(blocked, blocker) do - unfollow(blocked, blocker) - end + if following?(blocked, blocker), do: unfollow(blocked, blocker) {:ok, blocker} = update_follower_count(blocker) @@ -1168,17 +1136,19 @@ defp delete_activity(%{data: %{"type" => "Create"}} = activity) do end defp delete_activity(%{data: %{"type" => "Like"}} = activity) do - user = get_cached_by_ap_id(activity.actor) object = Object.normalize(activity) - ActivityPub.unlike(user, object) + activity.actor + |> get_cached_by_ap_id() + |> ActivityPub.unlike(object) end defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do - user = get_cached_by_ap_id(activity.actor) object = Object.normalize(activity) - ActivityPub.unannounce(user, object) + activity.actor + |> get_cached_by_ap_id() + |> ActivityPub.unannounce(object) end defp delete_activity(_activity), do: "Doing nothing" @@ -1190,9 +1160,7 @@ def html_filter_policy(%User{info: %{no_rich_text: true}}) do def html_filter_policy(_), do: Pleroma.Config.get([:markup, :scrub_policy]) def fetch_by_ap_id(ap_id) do - ap_try = ActivityPub.make_user_from_ap_id(ap_id) - - case ap_try do + case ActivityPub.make_user_from_ap_id(ap_id) do {:ok, user} -> {:ok, user} @@ -1207,7 +1175,7 @@ def fetch_by_ap_id(ap_id) do def get_or_fetch_by_ap_id(ap_id) do user = get_cached_by_ap_id(ap_id) - if !is_nil(user) and !User.needs_update?(user) do + if !is_nil(user) and !needs_update?(user) do {:ok, user} else # Whether to fetch initial posts for the user (if it's a new user & the fetching is enabled) @@ -1227,19 +1195,20 @@ def get_or_fetch_by_ap_id(ap_id) do @doc "Creates an internal service actor by URI if missing. Optionally takes nickname for addressing." def get_or_create_service_actor_by_ap_id(uri, nickname \\ nil) do - if user = get_cached_by_ap_id(uri) do + with %User{} = user <- get_cached_by_ap_id(uri) do user else - changes = - %User{info: %User.Info{}} - |> cast(%{}, [:ap_id, :nickname, :local]) - |> put_change(:ap_id, uri) - |> put_change(:nickname, nickname) - |> put_change(:local, true) - |> put_change(:follower_address, uri <> "/followers") + _ -> + {:ok, user} = + %User{info: %User.Info{}} + |> cast(%{}, [:ap_id, :nickname, :local]) + |> put_change(:ap_id, uri) + |> put_change(:nickname, nickname) + |> put_change(:local, true) + |> put_change(:follower_address, uri <> "/followers") + |> Repo.insert() - {:ok, user} = Repo.insert(changes) - user + user end end @@ -1296,23 +1265,21 @@ def get_or_fetch(nickname), do: get_or_fetch_by_nickname(nickname) # this is because we have synchronous follow APIs and need to simulate them # with an async handshake def wait_and_refresh(_, %User{local: true} = a, %User{local: true} = b) do - with %User{} = a <- User.get_cached_by_id(a.id), - %User{} = b <- User.get_cached_by_id(b.id) do + with %User{} = a <- get_cached_by_id(a.id), + %User{} = b <- get_cached_by_id(b.id) do {:ok, a, b} else - _e -> - :error + nil -> :error end end def wait_and_refresh(timeout, %User{} = a, %User{} = b) do with :ok <- :timer.sleep(timeout), - %User{} = a <- User.get_cached_by_id(a.id), - %User{} = b <- User.get_cached_by_id(b.id) do + %User{} = a <- get_cached_by_id(a.id), + %User{} = b <- get_cached_by_id(b.id) do {:ok, a, b} else - _e -> - :error + nil -> :error end end @@ -1374,7 +1341,7 @@ defp update_tags(%User{} = user, new_tags) do defp normalize_tags(tags) do [tags] |> List.flatten() - |> Enum.map(&String.downcase(&1)) + |> Enum.map(&String.downcase/1) end defp local_nickname_regex do diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 2f12ad43a..40eebe2aa 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -309,8 +309,7 @@ def update(user) do with {:ok, user} <- User.update_info(user, &User.Info.set_source_data(&1, source_data)) do user else - _e -> - user + _e -> user end ActivityPub.update(%{ @@ -338,11 +337,8 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do {:ok, _user} <- User.update_info(user, &User.Info.add_pinnned_activity(&1, activity)) do {:ok, activity} else - %{errors: [pinned_activities: {err, _}]} -> - {:error, err} - - _ -> - {:error, dgettext("errors", "Could not pin")} + {:error, %{changes: %{info: %{errors: [pinned_activities: {err, _}]}}}} -> {:error, err} + _ -> {:error, dgettext("errors", "Could not pin")} end end @@ -351,11 +347,8 @@ def unpin(id_or_ap_id, user) do {:ok, _user} <- User.update_info(user, &User.Info.remove_pinnned_activity(&1, activity)) do {:ok, activity} else - %{errors: [pinned_activities: {err, _}]} -> - {:error, err} - - _ -> - {:error, dgettext("errors", "Could not unpin")} + %{errors: [pinned_activities: {err, _}]} -> {:error, err} + _ -> {:error, dgettext("errors", "Could not unpin")} end end @@ -450,17 +443,13 @@ defp set_visibility(activity, %{"visibility" => visibility}) do defp set_visibility(activity, _), do: {:ok, activity} - def hide_reblogs(user, muted) do - ap_id = muted.ap_id - + def hide_reblogs(user, %{ap_id: ap_id} = _muted) do if ap_id not in user.info.muted_reblogs do User.update_info(user, &User.Info.add_reblog_mute(&1, ap_id)) end end - def show_reblogs(user, muted) do - ap_id = muted.ap_id - + def show_reblogs(user, %{ap_id: ap_id} = _muted) do if ap_id in user.info.muted_reblogs do User.update_info(user, &User.Info.remove_reblog_mute(&1, ap_id)) end diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 27f3664e0..aa06e2630 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -15,14 +15,11 @@ defmodule Pleroma.Web.TwitterAPI.Controller do action_fallback(:errors) def confirm_email(conn, %{"user_id" => uid, "token" => token}) do - true <- user.local, - true <- user.info.confirmation_pending, - true <- user.info.confirmation_token == token, - conn - |> redirect(to: "/") with %User{info: info} = user <- User.get_cached_by_id(uid), + true <- user.local and info.confirmation_pending and info.confirmation_token == token, {:ok, _} <- User.update_info(user, &User.Info.confirmation_changeset(&1, need_confirmation: false)) do + redirect(conn, to: "/") end end