Merge branch 'better-following' into 'develop'

Use race condition free queries for following / unfollowing

See merge request pleroma/pleroma!740
This commit is contained in:
kaniini 2019-01-30 18:50:00 +00:00
commit 6fe01a3234
3 changed files with 38 additions and 29 deletions

View file

@ -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
@ -343,18 +344,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 +362,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 +424,16 @@ def get_by_guessed_nickname(ap_id) do
get_by_nickname(nickname)
end
def update_and_set_cache(changeset) do
with {:ok, user} <- Repo.update(changeset) do
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
set_cache(user)
else
e -> e
end

View file

@ -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

View file

@ -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