From 935e65e2613d29d065166ba4605ee14349b74c53 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 30 Jan 2019 19:21:04 +0100 Subject: [PATCH 1/3] Use race-condition free following method. --- lib/pleroma/user.ex | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 1468cc133..62a4a3db1 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -343,18 +343,17 @@ def follow(%User{} = follower, %User{info: info} = followed) do Websub.subscribe(follower, followed) end - following = - [ap_followers | follower.following] - |> Enum.uniq() + q = + from(u in User, + where: u.id == ^follower.id, + update: [push: [following: ^ap_followers]] + ) - follower = - follower - |> follow_changeset(%{following: following}) - |> update_and_set_cache + {1, [follower]} = Repo.update_all(q, [], returning: true) {:ok, _} = update_follower_count(followed) - follower + set_cache(follower) end end @@ -362,17 +361,18 @@ def unfollow(%User{} = follower, %User{} = followed) do ap_followers = followed.follower_address if following?(follower, followed) and follower.ap_id != followed.ap_id do - following = - follower.following - |> List.delete(ap_followers) + q = + from(u in User, + where: u.id == ^follower.id, + update: [pull: [following: ^ap_followers]] + ) - {:ok, follower} = - follower - |> follow_changeset(%{following: following}) - |> update_and_set_cache + {1, [follower]} = Repo.update_all(q, [], returning: true) {:ok, followed} = update_follower_count(followed) + set_cache(follower) + {:ok, follower, Utils.fetch_latest_follow(follower, followed)} else {:error, "Not subscribed!"} @@ -423,12 +423,16 @@ def get_by_guessed_nickname(ap_id) do get_by_nickname(nickname) end + def set_cache(user) do + Cachex.put(:user_cache, "ap_id:#{user.ap_id}", user) + Cachex.put(:user_cache, "nickname:#{user.nickname}", user) + Cachex.put(:user_cache, "user_info:#{user.id}", user_info(user)) + {:ok, user} + end + def update_and_set_cache(changeset) do with {:ok, user} <- Repo.update(changeset) do - Cachex.put(:user_cache, "ap_id:#{user.ap_id}", user) - Cachex.put(:user_cache, "nickname:#{user.nickname}", user) - Cachex.put(:user_cache, "user_info:#{user.id}", user_info(user)) - {:ok, user} + set_cache(user) else e -> e end From 47ec690c54d8af9e317217990fb0756c7d37e2a5 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 30 Jan 2019 19:33:25 +0100 Subject: [PATCH 2/3] Use race-condition free mass follow. --- lib/pleroma/user.ex | 17 +++++++++-------- test/user_test.exs | 6 ++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 62a4a3db1..bd797db40 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -309,20 +309,21 @@ def maybe_follow(%User{} = follower, %User{info: _info} = followed) do @doc "A mass follow for local users. Ignores blocks and has no side effects" @spec follow_all(User.t(), list(User.t())) :: {atom(), User.t()} def follow_all(follower, followeds) do - following = - (follower.following ++ Enum.map(followeds, fn %{follower_address: fa} -> fa end)) - |> Enum.uniq() + followed_addresses = Enum.map(followeds, fn %{follower_address: fa} -> fa end) - {:ok, follower} = - follower - |> follow_changeset(%{following: following}) - |> update_and_set_cache + q = + from(u in User, + where: u.id == ^follower.id, + update: [set: [following: fragment("array_cat(?, ?)", u.following, ^followed_addresses)]] + ) + + {1, [follower]} = Repo.update_all(q, [], returning: true) Enum.each(followeds, fn followed -> update_follower_count(followed) end) - {:ok, follower} + set_cache(follower) end def follow(%User{} = follower, %User{info: info} = followed) do diff --git a/test/user_test.exs b/test/user_test.exs index a0657c7b6..cd202e360 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -50,13 +50,19 @@ test "ap_followers returns the followers collection for the user" do test "follow_all follows mutliple users" do user = insert(:user) + followed_zero = insert(:user) followed_one = insert(:user) followed_two = insert(:user) + not_followed = insert(:user) + + {:ok, user} = User.follow(user, followed_zero) {:ok, user} = User.follow_all(user, [followed_one, followed_two]) assert User.following?(user, followed_one) assert User.following?(user, followed_two) + assert User.following?(user, followed_zero) + refute User.following?(user, not_followed) end test "follow takes a user and another user" do From c53b96a024d4802a153ae3f561b2f6accb334ace Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 30 Jan 2019 19:45:31 +0100 Subject: [PATCH 3/3] Fix specs. --- test/web/activity_pub/activity_pub_test.exs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 7895cf21d..b826f5a1b 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -623,8 +623,6 @@ test "it filters broken threads" do "in_reply_to_status_id" => private_activity_2.id }) - assert user1.following == [user3.ap_id <> "/followers", user1.ap_id] - activities = ActivityPub.fetch_activities([user1.ap_id | user1.following]) assert [public_activity, private_activity_1, private_activity_3] == activities