From 059005ff829c0313c62ddf5fbcd95f8892920228 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Fri, 11 Oct 2019 02:35:32 +0700 Subject: [PATCH] Replace `user.following` with Pleroma.FollowingRelationship --- lib/mix/tasks/pleroma/database.ex | 6 +- lib/mix/tasks/pleroma/relay.ex | 5 +- lib/mix/tasks/pleroma/user.ex | 2 +- lib/pleroma/bbs/handler.ex | 2 +- lib/pleroma/following_relationship.ex | 110 ++++++++++++++++++ lib/pleroma/user.ex | 99 ++++------------ lib/pleroma/user/query.ex | 34 +++++- lib/pleroma/web/activity_pub/activity_pub.ex | 6 +- .../activity_pub/activity_pub_controller.ex | 4 +- .../web/activity_pub/transmogrifier.ex | 63 ++++------ lib/pleroma/web/activity_pub/visibility.ex | 2 +- lib/pleroma/web/common_api/common_api.ex | 3 + .../controllers/timeline_controller.ex | 8 +- .../controllers/account_controller.ex | 2 +- ...7073319_create_following_relationships.exs | 2 +- test/support/factory.ex | 3 +- test/tasks/database_test.exs | 8 +- test/tasks/relay_test.exs | 24 ++-- test/tasks/user_test.exs | 5 +- test/user_test.exs | 81 +++++-------- test/web/activity_pub/activity_pub_test.exs | 6 +- test/web/activity_pub/relay_test.exs | 5 +- test/web/activity_pub/transmogrifier_test.exs | 7 +- test/web/activity_pub/visibilty_test.exs | 3 +- .../controllers/account_controller_test.exs | 2 +- .../follow_request_controller_test.exs | 5 +- test/web/streamer/streamer_test.exs | 9 +- test/web/twitter_api/util_controller_test.exs | 4 +- 28 files changed, 275 insertions(+), 235 deletions(-) create mode 100644 lib/pleroma/following_relationship.ex diff --git a/lib/mix/tasks/pleroma/database.ex b/lib/mix/tasks/pleroma/database.ex index cfd9eeada..72b706e1a 100644 --- a/lib/mix/tasks/pleroma/database.ex +++ b/lib/mix/tasks/pleroma/database.ex @@ -52,9 +52,9 @@ def run(["bump_all_conversations"]) do def run(["update_users_following_followers_counts"]) do start_pleroma() - users = Repo.all(User) - Enum.each(users, &User.remove_duplicated_following/1) - Enum.each(users, &User.update_follower_count/1) + User + |> Repo.all() + |> Enum.each(&User.update_follower_count/1) end def run(["prune_objects" | args]) do diff --git a/lib/mix/tasks/pleroma/relay.ex b/lib/mix/tasks/pleroma/relay.ex index d7a7b599f..eafddada6 100644 --- a/lib/mix/tasks/pleroma/relay.ex +++ b/lib/mix/tasks/pleroma/relay.ex @@ -36,8 +36,9 @@ def run(["unfollow", target]) do def run(["list"]) do start_pleroma() - with %User{following: following} = _user <- Relay.get_actor() do - following + with %User{} = user <- Relay.get_actor() do + user + |> User.following() |> Enum.map(fn entry -> URI.parse(entry).host end) |> Enum.uniq() |> Enum.each(&shell_info(&1)) diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index 134b5bccc..8866afdf6 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -162,7 +162,7 @@ def run(["unsubscribe", nickname]) do user = User.get_cached_by_id(user.id) - if Enum.empty?(user.following) do + if Enum.empty?(User.get_friends(user)) do shell_info("Successfully unsubscribed all followers from #{user.nickname}") end else diff --git a/lib/pleroma/bbs/handler.ex b/lib/pleroma/bbs/handler.ex index fa838a4e4..b0e9ebbd0 100644 --- a/lib/pleroma/bbs/handler.ex +++ b/lib/pleroma/bbs/handler.ex @@ -97,7 +97,7 @@ def handle_command(state, "home") do |> Map.put("user", user) activities = - [user.ap_id | user.following] + [user.ap_id | Pleroma.User.following(user)] |> ActivityPub.fetch_activities(params) Enum.each(activities, fn activity -> diff --git a/lib/pleroma/following_relationship.ex b/lib/pleroma/following_relationship.ex new file mode 100644 index 000000000..0d789b5a6 --- /dev/null +++ b/lib/pleroma/following_relationship.ex @@ -0,0 +1,110 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.FollowingRelationship do + use Ecto.Schema + + import Ecto.Changeset + import Ecto.Query + + alias FlakeId.Ecto.CompatType + alias Pleroma.Repo + alias Pleroma.User + + schema "following_relationships" do + field(:state, :string, default: "accept") + + belongs_to(:follower, User, type: CompatType) + belongs_to(:following, User, type: CompatType) + + timestamps() + end + + def changeset(%__MODULE__{} = following_relationship, attrs) do + following_relationship + |> cast(attrs, [:state]) + |> put_assoc(:follower, attrs.follower) + |> put_assoc(:following, attrs.following) + |> validate_required([:state, :follower, :following]) + end + + def get(%User{} = follower, %User{} = following) do + __MODULE__ + |> where(follower_id: ^follower.id, following_id: ^following.id) + |> Repo.one() + end + + def update(follower, following, "reject"), do: unfollow(follower, following) + + def update(%User{} = follower, %User{} = following, state) do + case get(follower, following) do + nil -> + follow(follower, following, state) + + following_relationship -> + following_relationship + |> cast(%{state: state}, [:state]) + |> validate_required([:state]) + |> Repo.update() + end + end + + def follow(%User{} = follower, %User{} = following, state \\ "accept") do + %__MODULE__{} + |> changeset(%{follower: follower, following: following, state: state}) + |> Repo.insert(on_conflict: :nothing) + end + + def unfollow(%User{} = follower, %User{} = following) do + case get(follower, following) do + nil -> {:ok, nil} + %__MODULE__{} = following_relationship -> Repo.delete(following_relationship) + end + end + + def follower_count(%User{} = user) do + %{followers: user, deactivated: false} + |> User.Query.build() + |> Repo.aggregate(:count, :id) + end + + def following_count(%User{id: nil}), do: 0 + + def following_count(%User{} = user) do + %{friends: user, deactivated: false} + |> User.Query.build() + |> Repo.aggregate(:count, :id) + end + + def get_follow_requests(%User{id: id}) do + __MODULE__ + |> join(:inner, [r], f in assoc(r, :follower)) + |> where([r], r.state == "pending") + |> where([r], r.following_id == ^id) + |> select([r, f], f) + |> Repo.all() + end + + def following?(%User{id: follower_id}, %User{id: followed_id}) do + __MODULE__ + |> where(follower_id: ^follower_id, following_id: ^followed_id, state: "accept") + |> Repo.exists?() + end + + def following(%User{} = user) do + following = + __MODULE__ + |> join(:inner, [r], u in User, on: r.following_id == u.id) + |> where([r], r.follower_id == ^user.id) + |> where([r], r.state == "accept") + |> select([r, u], u.follower_address) + |> Repo.all() + + if user.nickname in [nil, "internal.fetch"] do + following + else + [user.follower_address | following] + end + end +end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 2cfb13a8c..c32f6b429 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -13,6 +13,7 @@ defmodule Pleroma.User do alias Pleroma.Activity alias Pleroma.Conversation.Participation alias Pleroma.Delivery + alias Pleroma.FollowingRelationship alias Pleroma.Keys alias Pleroma.Notification alias Pleroma.Object @@ -52,7 +53,6 @@ defmodule Pleroma.User do field(:password, :string, virtual: true) field(:password_confirmation, :string, virtual: true) field(:keys, :string) - field(:following, {:array, :string}, default: []) field(:ap_id, :string) field(:avatar, :map) field(:local, :boolean, default: true) @@ -162,13 +162,7 @@ def restrict_deactivated(query) do ) end - def following_count(%User{following: []}), do: 0 - - def following_count(%User{} = user) do - user - |> get_friends_query() - |> Repo.aggregate(:count, :id) - end + defdelegate following_count(user), to: FollowingRelationship defp truncate_if_exists(params, key, max_length) do if Map.has_key?(params, key) and is_binary(params[key]) do @@ -216,7 +210,7 @@ def update_changeset(struct, params \\ %{}) do name_limit = Pleroma.Config.get([:instance, :user_name_length], 100) struct - |> cast(params, [:bio, :name, :avatar, :following]) + |> cast(params, [:bio, :name, :avatar]) |> unique_constraint(:nickname) |> validate_format(:nickname, local_nickname_regex()) |> validate_length(:bio, max: bio_limit) @@ -324,7 +318,6 @@ defp put_following_and_follower_address(changeset) do followers = ap_followers(%User{nickname: get_field(changeset, :nickname)}) changeset - |> put_change(:following, [followers]) |> put_change(:follower_address, followers) end @@ -378,8 +371,11 @@ def needs_update?(%User{local: false} = user) do def needs_update?(_), do: true @spec maybe_direct_follow(User.t(), User.t()) :: {:ok, User.t()} | {:error, String.t()} - def maybe_direct_follow(%User{} = follower, %User{local: true, info: %{locked: true}}) do - {:ok, follower} + def maybe_direct_follow( + %User{} = follower, + %User{local: true, info: %{locked: true}} = followed + ) do + follow(follower, followed, "pending") end def maybe_direct_follow(%User{} = follower, %User{local: true} = followed) do @@ -397,37 +393,22 @@ def maybe_direct_follow(%User{} = follower, %User{} = followed) do @doc "A mass follow for local users. Respects blocks in both directions but does not create activities." @spec follow_all(User.t(), list(User.t())) :: {atom(), User.t()} def follow_all(follower, followeds) do - followed_addresses = - followeds - |> Enum.reject(fn followed -> blocks?(follower, followed) || blocks?(followed, follower) end) - |> Enum.map(fn %{follower_address: fa} -> fa end) + followeds = + Enum.reject(followeds, fn followed -> + blocks?(follower, followed) || blocks?(followed, follower) + end) - q = - from(u in User, - where: u.id == ^follower.id, - update: [ - set: [ - following: - fragment( - "array(select distinct unnest (array_cat(?, ?)))", - u.following, - ^followed_addresses - ) - ] - ], - select: u - ) - - {1, [follower]} = Repo.update_all(q, []) + Enum.each(followeds, &follow(follower, &1, "accept")) Enum.each(followeds, &update_follower_count/1) set_cache(follower) end - def follow(%User{} = follower, %User{info: info} = followed) do + defdelegate following(user), to: FollowingRelationship + + def follow(%User{} = follower, %User{info: info} = followed, state \\ "accept") do deny_follow_blocked = Pleroma.Config.get([:user, :deny_follow_blocked]) - ap_followers = followed.follower_address cond do info.deactivated -> @@ -441,14 +422,7 @@ def follow(%User{} = follower, %User{info: info} = followed) do Websub.subscribe(follower, followed) end - q = - from(u in User, - where: u.id == ^follower.id, - update: [push: [following: ^ap_followers]], - select: u - ) - - {1, [follower]} = Repo.update_all(q, []) + FollowingRelationship.follow(follower, followed, state) follower = maybe_update_following_count(follower) @@ -459,17 +433,8 @@ def follow(%User{} = follower, %User{info: info} = followed) do end def unfollow(%User{} = follower, %User{} = followed) do - ap_followers = followed.follower_address - if following?(follower, followed) and follower.ap_id != followed.ap_id do - q = - from(u in User, - where: u.id == ^follower.id, - update: [pull: [following: ^ap_followers]], - select: u - ) - - {1, [follower]} = Repo.update_all(q, []) + FollowingRelationship.unfollow(follower, followed) follower = maybe_update_following_count(follower) @@ -483,10 +448,7 @@ def unfollow(%User{} = follower, %User{} = followed) do end end - @spec following?(User.t(), User.t()) :: boolean - def following?(%User{} = follower, %User{} = followed) do - Enum.member?(follower.following, followed.follower_address) - end + defdelegate following?(follower, followed), to: FollowingRelationship def locked?(%User{} = user) do user.info.locked || false @@ -707,16 +669,7 @@ def get_friends_ids(user, page \\ nil) do |> Repo.all() end - @spec get_follow_requests(User.t()) :: {:ok, [User.t()]} - def get_follow_requests(%User{} = user) do - user - |> Activity.follow_requests_for_actor() - |> join(:inner, [a], u in User, on: a.actor == u.ap_id) - |> where([a, u], not fragment("? @> ?", u.following, ^[user.follower_address])) - |> group_by([a, u], u.id) - |> select([a, u], u) - |> Repo.all() - end + defdelegate get_follow_requests(user), to: FollowingRelationship def increase_note_count(%User{} = user) do User @@ -899,18 +852,6 @@ def increment_unread_conversation_count(conversation, %User{local: true} = user) def increment_unread_conversation_count(_, _), do: :noop - def remove_duplicated_following(%User{following: following} = user) do - uniq_following = Enum.uniq(following) - - if length(following) == length(uniq_following) do - {:ok, user} - else - user - |> update_changeset(%{following: uniq_following}) - |> update_and_set_cache() - end - end - @spec get_users_from_set([String.t()], boolean()) :: [User.t()] def get_users_from_set(ap_ids, local_only \\ true) do criteria = %{ap_id: ap_ids, deactivated: false} diff --git a/lib/pleroma/user/query.ex b/lib/pleroma/user/query.ex index 2baf016cf..f32def4f5 100644 --- a/lib/pleroma/user/query.ex +++ b/lib/pleroma/user/query.ex @@ -28,6 +28,8 @@ defmodule Pleroma.User.Query do """ import Ecto.Query import Pleroma.Web.AdminAPI.Search, only: [not_empty_string: 1] + + alias Pleroma.FollowingRelationship alias Pleroma.User @type criteria :: @@ -130,18 +132,40 @@ defp compose_query({:deactivated, true}, query) do |> where([u], not is_nil(u.nickname)) end - defp compose_query({:followers, %User{id: id, follower_address: follower_address}}, query) do - where(query, [u], fragment("? <@ ?", ^[follower_address], u.following)) + defp compose_query({:followers, %User{id: id}}, query) do + query |> where([u], u.id != ^id) + |> join(:inner, [u], r in FollowingRelationship, + as: :relationships, + on: r.following_id == ^id and r.follower_id == u.id + ) + |> where([relationships: r], r.state == "accept") end - defp compose_query({:friends, %User{id: id, following: following}}, query) do - where(query, [u], u.follower_address in ^following) + defp compose_query({:friends, %User{id: id}}, query) do + query |> where([u], u.id != ^id) + |> join(:inner, [u], r in FollowingRelationship, + as: :relationships, + on: r.following_id == u.id and r.follower_id == ^id + ) + |> where([relationships: r], r.state == "accept") end defp compose_query({:recipients_from_activity, to}, query) do - where(query, [u], u.ap_id in ^to or fragment("? && ?", u.following, ^to)) + query + |> join(:left, [u], r in FollowingRelationship, + as: :relationships, + on: r.follower_id == u.id + ) + |> join(:left, [relationships: r], f in User, + as: :following, + on: f.id == r.following_id + ) + |> where( + [u, following: f, relationships: r], + u.ap_id in ^to or (f.follower_address in ^to and r.state == "accept") + ) end defp compose_query({:order_by, key}, query) do diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 9f29087df..db0855329 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -501,7 +501,9 @@ defp fetch_activities_for_context_query(context, opts) do public = [Pleroma.Constants.as_public()] recipients = - if opts["user"], do: [opts["user"].ap_id | opts["user"].following] ++ public, else: public + if opts["user"], + do: [opts["user"].ap_id | User.following(opts["user"])] ++ public, + else: public from(activity in Activity) |> maybe_preload_objects(opts) @@ -652,7 +654,7 @@ defp user_activities_recipients(%{"godmode" => true}) do defp user_activities_recipients(%{"reading_user" => reading_user}) do if reading_user do - [Pleroma.Constants.as_public()] ++ [reading_user.ap_id | reading_user.following] + [Pleroma.Constants.as_public()] ++ [reading_user.ap_id | User.following(reading_user)] else [Pleroma.Constants.as_public()] end diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 080030eb5..9010eab91 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -319,12 +319,12 @@ def read_inbox( when page? in [true, "true"] do activities = if params["max_id"] do - ActivityPub.fetch_activities([user.ap_id | user.following], %{ + ActivityPub.fetch_activities([user.ap_id | User.following(user)], %{ "max_id" => params["max_id"], "limit" => 10 }) else - ActivityPub.fetch_activities([user.ap_id | user.following], %{"limit" => 10}) + ActivityPub.fetch_activities([user.ap_id | User.following(user)], %{"limit" => 10}) end conn diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 872ed0eb2..54ba49520 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do A module to handle coding from internal to wire ActivityPub and back. """ alias Pleroma.Activity + alias Pleroma.FollowingRelationship alias Pleroma.Object alias Pleroma.Object.Containment alias Pleroma.Repo @@ -474,7 +475,8 @@ def handle_incoming( {_, false} <- {:user_locked, User.locked?(followed)}, {_, {:ok, follower}} <- {:follow, User.follow(follower, followed)}, {_, {:ok, _}} <- - {:follow_state_update, Utils.update_follow_state_for_all(activity, "accept")} do + {:follow_state_update, Utils.update_follow_state_for_all(activity, "accept")}, + {:ok, _relationship} <- FollowingRelationship.update(follower, followed, "accept") do ActivityPub.accept(%{ to: [follower.ap_id], actor: followed, @@ -484,6 +486,7 @@ def handle_incoming( else {:user_blocked, true} -> {:ok, _} = Utils.update_follow_state_for_all(activity, "reject") + {:ok, _relationship} = FollowingRelationship.update(follower, followed, "reject") ActivityPub.reject(%{ to: [follower.ap_id], @@ -494,6 +497,7 @@ def handle_incoming( {:follow, {:error, _}} -> {:ok, _} = Utils.update_follow_state_for_all(activity, "reject") + {:ok, _relationship} = FollowingRelationship.update(follower, followed, "reject") ActivityPub.reject(%{ to: [follower.ap_id], @@ -522,7 +526,7 @@ def handle_incoming( {:ok, follow_activity} <- get_follow_activity(follow_object, followed), {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "accept"), %User{local: true} = follower <- User.get_cached_by_ap_id(follow_activity.data["actor"]), - {:ok, _follower} = User.follow(follower, followed) do + {:ok, _relationship} <- FollowingRelationship.update(follower, followed, "accept") do ActivityPub.accept(%{ to: follow_activity.data["to"], type: "Accept", @@ -544,6 +548,7 @@ def handle_incoming( {:ok, follow_activity} <- get_follow_activity(follow_object, followed), {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "reject"), %User{local: true} = follower <- User.get_cached_by_ap_id(follow_activity.data["actor"]), + {:ok, _relationship} <- FollowingRelationship.update(follower, followed, "reject"), {:ok, activity} <- ActivityPub.reject(%{ to: follow_activity.data["to"], @@ -552,8 +557,6 @@ def handle_incoming( object: follow_activity.data["id"], local: false }) do - User.unfollow(follower, followed) - {:ok, activity} else _e -> :error @@ -1050,46 +1053,24 @@ defp strip_internal_tags(object), do: object def perform(:user_upgrade, user) do # we pass a fake user so that the followers collection is stripped away old_follower_address = User.ap_followers(%User{nickname: user.nickname}) - - q = - from( - u in User, - where: ^old_follower_address in u.following, - update: [ - set: [ - following: - fragment( - "array_replace(?,?,?)", - u.following, - ^old_follower_address, - ^user.follower_address - ) - ] - ] - ) - - Repo.update_all(q, []) - maybe_retire_websub(user.ap_id) - q = - from( - a in Activity, - where: ^old_follower_address in a.recipients, - update: [ - set: [ - recipients: - fragment( - "array_replace(?,?,?)", - a.recipients, - ^old_follower_address, - ^user.follower_address - ) - ] + from( + a in Activity, + where: ^old_follower_address in a.recipients, + update: [ + set: [ + recipients: + fragment( + "array_replace(?,?,?)", + a.recipients, + ^old_follower_address, + ^user.follower_address + ) ] - ) - - Repo.update_all(q, []) + ] + ) + |> Repo.update_all([]) end def upgrade_user_from_ap_id(ap_id) do diff --git a/lib/pleroma/web/activity_pub/visibility.ex b/lib/pleroma/web/activity_pub/visibility.ex index 270d0fa02..47bc4e1ca 100644 --- a/lib/pleroma/web/activity_pub/visibility.ex +++ b/lib/pleroma/web/activity_pub/visibility.ex @@ -58,7 +58,7 @@ def visible_for_user?(activity, nil) do end def visible_for_user?(activity, user) do - x = [user.ap_id | user.following] + x = [user.ap_id | User.following(user)] y = [activity.actor] ++ activity.data["to"] ++ (activity.data["cc"] || []) visible_for_user?(activity, nil) || Enum.any?(x, &(&1 in y)) end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 386408d51..40b3930fb 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -6,6 +6,7 @@ defmodule Pleroma.Web.CommonAPI do alias Pleroma.Activity alias Pleroma.ActivityExpiration alias Pleroma.Conversation.Participation + alias Pleroma.FollowingRelationship alias Pleroma.Object alias Pleroma.ThreadMute alias Pleroma.User @@ -40,6 +41,7 @@ def accept_follow_request(follower, followed) do with {:ok, follower} <- User.follow(follower, followed), %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "accept"), + {:ok, _relationship} <- FollowingRelationship.update(follower, followed, "accept"), {:ok, _activity} <- ActivityPub.accept(%{ to: [follower.ap_id], @@ -54,6 +56,7 @@ def accept_follow_request(follower, followed) do def reject_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "reject"), + {:ok, _relationship} <- FollowingRelationship.update(follower, followed, "reject"), {:ok, _activity} <- ActivityPub.reject(%{ to: [follower.ap_id], diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 9f086a8c2..f2d2d3ccb 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do alias Pleroma.Pagination alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action in [:home, :direct]) @@ -28,7 +29,7 @@ def home(%{assigns: %{user: user}} = conn, params) do |> Map.put("muting_user", user) |> Map.put("user", user) - recipients = [user.ap_id | user.following] + recipients = [user.ap_id | User.following(user)] activities = recipients @@ -128,9 +129,12 @@ def list(%{assigns: %{user: user}} = conn, %{"list_id" => id} = params) do # we must filter the following list for the user to avoid leaking statuses the user # does not actually have permission to see (for more info, peruse security issue #270). + + user_following = User.following(user) + activities = following - |> Enum.filter(fn x -> x in user.following end) + |> Enum.filter(fn x -> x in user_following end) |> ActivityPub.fetch_activities_bounded(following, params) |> Enum.reverse() diff --git a/lib/pleroma/web/pleroma_api/controllers/account_controller.ex b/lib/pleroma/web/pleroma_api/controllers/account_controller.ex index 9012e2175..7ce9d5da2 100644 --- a/lib/pleroma/web/pleroma_api/controllers/account_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/account_controller.ex @@ -132,7 +132,7 @@ def favourites(%{assigns: %{user: for_user, account: user}} = conn, params) do recipients = if for_user do - [Pleroma.Constants.as_public()] ++ [for_user.ap_id | for_user.following] + [Pleroma.Constants.as_public()] ++ [for_user.ap_id | User.following(for_user)] else [Pleroma.Constants.as_public()] end diff --git a/priv/repo/migrations/20191007073319_create_following_relationships.exs b/priv/repo/migrations/20191007073319_create_following_relationships.exs index 7daaf0575..d49e24ee4 100644 --- a/priv/repo/migrations/20191007073319_create_following_relationships.exs +++ b/priv/repo/migrations/20191007073319_create_following_relationships.exs @@ -16,7 +16,7 @@ def change do execute(update_thread_visibility(), restore_thread_visibility()) end - # The only difference with the original verion: `actor_user` replaced with `actor_user_following` + # The only difference between the original version: `actor_user` replaced with `actor_user_following` def update_thread_visibility do """ CREATE OR REPLACE FUNCTION thread_visibility(actor varchar, activity_id varchar) RETURNS boolean AS $$ diff --git a/test/support/factory.ex b/test/support/factory.ex index b180844cd..74f292a1d 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -39,8 +39,7 @@ def user_factory do user | ap_id: User.ap_id(user), follower_address: User.ap_followers(user), - following_address: User.ap_following(user), - following: [User.ap_id(user)] + following_address: User.ap_following(user) } end diff --git a/test/tasks/database_test.exs b/test/tasks/database_test.exs index b63dcac00..bf5ba7883 100644 --- a/test/tasks/database_test.exs +++ b/test/tasks/database_test.exs @@ -72,25 +72,25 @@ test "it prunes old objects from the database" do describe "running update_users_following_followers_counts" do test "following and followers count are updated" do [user, user2] = insert_pair(:user) - {:ok, %User{following: following, info: info} = user} = User.follow(user, user2) + {:ok, %User{info: info} = user} = User.follow(user, user2) + + following = User.following(user) assert length(following) == 2 assert info.follower_count == 0 {:ok, user} = user - |> Ecto.Changeset.change(%{following: following ++ following}) |> User.change_info(&Ecto.Changeset.change(&1, %{follower_count: 3})) |> Repo.update() - assert length(user.following) == 4 assert user.info.follower_count == 3 assert :ok == Mix.Tasks.Pleroma.Database.run(["update_users_following_followers_counts"]) user = User.get_by_id(user.id) - assert length(user.following) == 2 + assert length(User.following(user)) == 2 assert user.info.follower_count == 0 end end diff --git a/test/tasks/relay_test.exs b/test/tasks/relay_test.exs index c866608ab..04a1e45d7 100644 --- a/test/tasks/relay_test.exs +++ b/test/tasks/relay_test.exs @@ -51,7 +51,7 @@ test "relay is unfollowed" do target_user = User.get_cached_by_ap_id(target_instance) follow_activity = Utils.fetch_latest_follow(local_user, target_user) User.follow(local_user, target_user) - assert "#{target_instance}/followers" in refresh_record(local_user).following + assert "#{target_instance}/followers" in User.following(local_user) Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance]) cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"]) @@ -68,7 +68,7 @@ test "relay is unfollowed" do assert undo_activity.data["type"] == "Undo" assert undo_activity.data["actor"] == local_user.ap_id assert undo_activity.data["object"] == cancelled_activity.data - refute "#{target_instance}/followers" in refresh_record(local_user).following + refute "#{target_instance}/followers" in User.following(local_user) end end @@ -78,20 +78,18 @@ test "Prints relay subscription list" do refute_receive {:mix_shell, :info, _} - Pleroma.Web.ActivityPub.Relay.get_actor() - |> Ecto.Changeset.change( - following: [ - "http://test-app.com/user/test1", - "http://test-app.com/user/test1", - "http://test-app-42.com/user/test1" - ] - ) - |> Pleroma.User.update_and_set_cache() + relay_user = Relay.get_actor() + + ["http://mastodon.example.org/users/admin", "https://mstdn.io/users/mayuutann"] + |> Enum.each(fn ap_id -> + {:ok, user} = User.get_or_fetch_by_ap_id(ap_id) + User.follow(relay_user, user) + end) :ok = Mix.Tasks.Pleroma.Relay.run(["list"]) - assert_receive {:mix_shell, :info, ["test-app.com"]} - assert_receive {:mix_shell, :info, ["test-app-42.com"]} + assert_receive {:mix_shell, :info, ["mstdn.io"]} + assert_receive {:mix_shell, :info, ["mastodon.example.org"]} end end end diff --git a/test/tasks/user_test.exs b/test/tasks/user_test.exs index cf12d9ed6..c0e4ba85c 100644 --- a/test/tasks/user_test.exs +++ b/test/tasks/user_test.exs @@ -139,7 +139,8 @@ test "no user to toggle" do describe "running unsubscribe" do test "user is unsubscribed" do followed = insert(:user) - user = insert(:user, %{following: [User.ap_followers(followed)]}) + user = insert(:user) + User.follow(user, followed, "accept") Mix.Tasks.Pleroma.User.run(["unsubscribe", user.nickname]) @@ -154,7 +155,7 @@ test "user is unsubscribed" do assert message =~ "Successfully unsubscribed" user = User.get_cached_by_nickname(user.nickname) - assert Enum.empty?(user.following) + assert Enum.empty?(User.get_friends(user)) assert user.info.deactivated end diff --git a/test/user_test.exs b/test/user_test.exs index 019e7b400..85e55876d 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -88,10 +88,9 @@ test "doesn't return already accepted or duplicate follow requests" do CommonAPI.follow(pending_follower, locked) CommonAPI.follow(pending_follower, locked) CommonAPI.follow(accepted_follower, locked) - User.follow(accepted_follower, locked) + Pleroma.FollowingRelationship.update(accepted_follower, locked, "accept") - assert [activity] = User.get_follow_requests(locked) - assert activity + assert [^pending_follower] = User.get_follow_requests(locked) end test "clears follow requests when requester is blocked" do @@ -136,10 +135,10 @@ test "follow_all follows mutliple users without duplicating" do followed_two = insert(:user) {:ok, user} = User.follow_all(user, [followed_zero, followed_one]) - assert length(user.following) == 3 + assert length(User.following(user)) == 3 {:ok, user} = User.follow_all(user, [followed_one, followed_two]) - assert length(user.following) == 4 + assert length(User.following(user)) == 4 end test "follow takes a user and another user" do @@ -153,7 +152,7 @@ test "follow takes a user and another user" do followed = User.get_cached_by_ap_id(followed.ap_id) assert followed.info.follower_count == 1 - assert User.ap_followers(followed) in user.following + assert User.ap_followers(followed) in User.following(user) end test "can't follow a deactivated users" do @@ -198,7 +197,7 @@ test "local users do not automatically follow local locked accounts" do # assert followed.local == false # {:ok, user} = User.follow(user, followed) - # assert User.ap_followers(followed) in user.following + # assert User.ap_followers(followed) in User.following(user) # query = from w in WebsubClientSubscription, # where: w.topic == ^followed.info["topic"] @@ -235,26 +234,29 @@ test "unfollow with syncronizes external user" do nickname: "fuser2", ap_id: "http://localhost:4001/users/fuser2", follower_address: "http://localhost:4001/users/fuser2/followers", - following_address: "http://localhost:4001/users/fuser2/following", - following: [User.ap_followers(followed)] + following_address: "http://localhost:4001/users/fuser2/following" }) + {:ok, user} = User.follow(user, followed, "accept") + {:ok, user, _activity} = User.unfollow(user, followed) user = User.get_cached_by_id(user.id) - assert user.following == [] + assert User.following(user) == [user.follower_address] end test "unfollow takes a user and another user" do followed = insert(:user) - user = insert(:user, %{following: [User.ap_followers(followed)]}) + user = insert(:user) + + {:ok, user} = User.follow(user, followed, "accept") + + assert User.following(user) == [user.follower_address, followed.follower_address] {:ok, user, _activity} = User.unfollow(user, followed) - user = User.get_cached_by_id(user.id) - - assert user.following == [] + assert User.following(user) == [user.follower_address] end test "unfollow doesn't unfollow yourself" do @@ -262,14 +264,14 @@ test "unfollow doesn't unfollow yourself" do {:error, _} = User.unfollow(user, user) - user = User.get_cached_by_id(user.id) - assert user.following == [user.ap_id] + assert User.following(user) == [user.follower_address] end end test "test if a user is following another user" do followed = insert(:user) - user = insert(:user, %{following: [User.ap_followers(followed)]}) + user = insert(:user) + User.follow(user, followed, "accept") assert User.following?(user, followed) refute User.following?(followed, user) @@ -352,7 +354,7 @@ test "it restricts certain nicknames" do refute changeset.valid? end - test "it sets the password_hash, ap_id and following fields" do + test "it sets the password_hash and ap_id" do changeset = User.register_changeset(%User{}, @full_user_data) assert changeset.valid? @@ -360,10 +362,6 @@ test "it sets the password_hash, ap_id and following fields" do assert is_binary(changeset.changes[:password_hash]) assert changeset.changes[:ap_id] == User.ap_id(%User{nickname: @full_user_data.nickname}) - assert changeset.changes[:following] == [ - User.ap_followers(%User{nickname: @full_user_data.nickname}) - ] - assert changeset.changes.follower_address == "#{changeset.changes.ap_id}/followers" end @@ -671,37 +669,6 @@ test "it sets the info->follower_count property" do end end - describe "remove duplicates from following list" do - test "it removes duplicates" do - user = insert(:user) - follower = insert(:user) - - {:ok, %User{following: following} = follower} = User.follow(follower, user) - assert length(following) == 2 - - {:ok, follower} = - follower - |> User.update_changeset(%{following: following ++ following}) - |> Repo.update() - - assert length(follower.following) == 4 - - {:ok, follower} = User.remove_duplicated_following(follower) - assert length(follower.following) == 2 - end - - test "it does nothing when following is uniq" do - user = insert(:user) - follower = insert(:user) - - {:ok, follower} = User.follow(follower, user) - assert length(follower.following) == 2 - - {:ok, follower} = User.remove_duplicated_following(follower) - assert length(follower.following) == 2 - end - end - describe "follow_import" do test "it imports user followings from list" do [user1, user2, user3] = insert_list(3, :user) @@ -1010,7 +977,9 @@ test "hide a user's statuses from timelines and notifications" do assert [activity] == ActivityPub.fetch_public_activities(%{}) |> Repo.preload(:bookmark) assert [%{activity | thread_muted?: CommonAPI.thread_muted?(user2, activity)}] == - ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2}) + ActivityPub.fetch_activities([user2.ap_id | User.following(user2)], %{ + "user" => user2 + }) {:ok, _user} = User.deactivate(user) @@ -1018,7 +987,9 @@ test "hide a user's statuses from timelines and notifications" do assert [] == Pleroma.Notification.for_user(user2) assert [] == - ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2}) + ActivityPub.fetch_activities([user2.ap_id | User.following(user2)], %{ + "user" => user2 + }) end end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index c9f2a92e7..75e928a14 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -606,7 +606,7 @@ test "does include announces on request" do {:ok, announce, _object} = CommonAPI.repeat(activity_three.id, booster) - [announce_activity] = ActivityPub.fetch_activities([user.ap_id | user.following]) + [announce_activity] = ActivityPub.fetch_activities([user.ap_id | User.following(user)]) assert announce_activity.id == announce.id end @@ -1132,7 +1132,7 @@ test "it filters broken threads" do }) activities = - ActivityPub.fetch_activities([user1.ap_id | user1.following]) + ActivityPub.fetch_activities([user1.ap_id | User.following(user1)]) |> Enum.map(fn a -> a.id end) private_activity_1 = Activity.get_by_ap_id_with_object(private_activity_1.data["id"]) @@ -1142,7 +1142,7 @@ test "it filters broken threads" do assert length(activities) == 3 activities = - ActivityPub.fetch_activities([user1.ap_id | user1.following], %{"user" => user1}) + ActivityPub.fetch_activities([user1.ap_id | User.following(user1)], %{"user" => user1}) |> Enum.map(fn a -> a.id end) assert [public_activity.id, private_activity_1.id] == activities diff --git a/test/web/activity_pub/relay_test.exs b/test/web/activity_pub/relay_test.exs index 0f7556538..e270cd4c3 100644 --- a/test/web/activity_pub/relay_test.exs +++ b/test/web/activity_pub/relay_test.exs @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.RelayTest do alias Pleroma.Activity alias Pleroma.Object + alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Relay @@ -50,14 +51,14 @@ test "returns activity" do service_actor = Relay.get_actor() ActivityPub.follow(service_actor, user) Pleroma.User.follow(service_actor, user) - assert "#{user.ap_id}/followers" in refresh_record(service_actor).following + assert "#{user.ap_id}/followers" in User.following(service_actor) assert {:ok, %Activity{} = activity} = Relay.unfollow(user.ap_id) assert activity.actor == "#{Pleroma.Web.Endpoint.url()}/relay" assert user.ap_id in activity.recipients assert activity.data["type"] == "Undo" assert activity.data["actor"] == service_actor.ap_id assert activity.data["to"] == [user.ap_id] - refute "#{user.ap_id}/followers" in refresh_record(service_actor).following + refute "#{user.ap_id}/followers" in User.following(service_actor) end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 50c0bfb84..d96dcd2a0 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -1312,7 +1312,8 @@ test "it upgrades a user to activitypub" do follower_address: User.ap_followers(%User{nickname: "rye@niu.moe"}) }) - user_two = insert(:user, %{following: [user.follower_address]}) + user_two = insert(:user) + Pleroma.FollowingRelationship.follow(user_two, user, "accept") {:ok, activity} = CommonAPI.post(user, %{"status" => "test"}) {:ok, unrelated_activity} = CommonAPI.post(user_two, %{"status" => "test"}) @@ -1359,8 +1360,8 @@ test "it upgrades a user to activitypub" do refute user.follower_address in unrelated_activity.recipients user_two = User.get_cached_by_id(user_two.id) - assert user.follower_address in user_two.following - refute "..." in user_two.following + assert User.following?(user_two, user) + refute "..." in User.following(user_two) end end diff --git a/test/web/activity_pub/visibilty_test.exs b/test/web/activity_pub/visibilty_test.exs index b62a89e68..4c2e0d207 100644 --- a/test/web/activity_pub/visibilty_test.exs +++ b/test/web/activity_pub/visibilty_test.exs @@ -212,7 +212,8 @@ test "returns false when invalid recipients", %{user: user} do test "returns true if user following to author" do author = insert(:user) - user = insert(:user, following: [author.ap_id]) + user = insert(:user) + Pleroma.User.follow(user, author) activity = insert(:note_activity, diff --git a/test/web/mastodon_api/controllers/account_controller_test.exs b/test/web/mastodon_api/controllers/account_controller_test.exs index 6a59c3d94..a398ef76a 100644 --- a/test/web/mastodon_api/controllers/account_controller_test.exs +++ b/test/web/mastodon_api/controllers/account_controller_test.exs @@ -457,7 +457,7 @@ test "following without reblogs" do conn = build_conn() - |> assign(:user, follower) + |> assign(:user, User.get_cached_by_id(follower.id)) |> post("/api/v1/accounts/#{followed.id}/follow?reblogs=true") assert %{"showing_reblogs" => true} = json_response(conn, 200) diff --git a/test/web/mastodon_api/controllers/follow_request_controller_test.exs b/test/web/mastodon_api/controllers/follow_request_controller_test.exs index 4bf292df5..89b676201 100644 --- a/test/web/mastodon_api/controllers/follow_request_controller_test.exs +++ b/test/web/mastodon_api/controllers/follow_request_controller_test.exs @@ -16,9 +16,7 @@ test "/api/v1/follow_requests works" do other_user = insert(:user) {:ok, _activity} = ActivityPub.follow(other_user, user) - - user = User.get_cached_by_id(user.id) - other_user = User.get_cached_by_id(other_user.id) + {:ok, other_user} = User.follow(other_user, user, "pending") assert User.following?(other_user, user) == false @@ -36,6 +34,7 @@ test "/api/v1/follow_requests/:id/authorize works" do other_user = insert(:user) {:ok, _activity} = ActivityPub.follow(other_user, user) + {:ok, other_user} = User.follow(other_user, user, "pending") user = User.get_cached_by_id(user.id) other_user = User.get_cached_by_id(other_user.id) diff --git a/test/web/streamer/streamer_test.exs b/test/web/streamer/streamer_test.exs index d33eb1e42..c674e71f5 100644 --- a/test/web/streamer/streamer_test.exs +++ b/test/web/streamer/streamer_test.exs @@ -169,7 +169,8 @@ test "it sends to public" do test "it doesn't send to user if recipients invalid and thread containment is enabled" do Pleroma.Config.put([:instance, :skip_thread_containment], false) author = insert(:user) - user = insert(:user, following: [author.ap_id]) + user = insert(:user) + User.follow(user, author, "accept") activity = insert(:note_activity, @@ -191,7 +192,8 @@ test "it doesn't send to user if recipients invalid and thread containment is en test "it sends message if recipients invalid and thread containment is disabled" do Pleroma.Config.put([:instance, :skip_thread_containment], true) author = insert(:user) - user = insert(:user, following: [author.ap_id]) + user = insert(:user) + User.follow(user, author, "accept") activity = insert(:note_activity, @@ -213,7 +215,8 @@ test "it sends message if recipients invalid and thread containment is disabled" test "it sends message if recipients invalid and thread containment is enabled but user's thread containment is disabled" do Pleroma.Config.put([:instance, :skip_thread_containment], false) author = insert(:user) - user = insert(:user, following: [author.ap_id], info: %{skip_thread_containment: true}) + user = insert(:user, info: %{skip_thread_containment: true}) + User.follow(user, author, "accept") activity = insert(:note_activity, diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 9d4cb70f0..5234a5271 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -366,7 +366,7 @@ test "follows user", %{conn: conn} do |> response(200) assert response =~ "Account followed!" - assert user2.follower_address in refresh_record(user).following + assert user2.follower_address in User.following(user) end test "returns error when user is deactivated", %{conn: conn} do @@ -438,7 +438,7 @@ test "follows", %{conn: conn} do |> response(200) assert response =~ "Account followed!" - assert user2.follower_address in refresh_record(user).following + assert user2.follower_address in User.following(user) end test "returns error when followee not found", %{conn: conn} do