commit: 3f6da9c45aeb6954a857566731f483cd3da8d068
parent: a511d2f9003870d267c143ee1aa12a0dfd805f8e
Author: kaniini <nenolod@gmail.com>
Date: Wed, 5 Jun 2019 15:52:40 +0000
Merge branch 'bugfix/377-stuck-follow-request' into 'develop'
Bugfix/377 stuck follow request
Closes #377
See merge request pleroma/pleroma!1250
Diffstat:
9 files changed, 143 insertions(+), 35 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -90,6 +90,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Respond with a 404 Not implemented JSON error message when requested API is not implemented
### Fixed
+- Follow requests don't get 'stuck' anymore.
- Added an FTS index on objects. Running `vacuum analyze` and setting a larger `work_mem` is recommended.
- Followers counter not being updated when a follower is blocked
- Deactivated users being able to request an access token
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
@@ -324,14 +324,6 @@ defmodule Pleroma.User do
end
end
- def maybe_follow(%User{} = follower, %User{info: _info} = followed) do
- if not following?(follower, followed) do
- follow(follower, followed)
- else
- {:ok, follower}
- end
- end
-
@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
diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -458,10 +458,12 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
{:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower),
{:ok, activity} <- ActivityPub.follow(follower, followed, id, false) do
with deny_follow_blocked <- Pleroma.Config.get([:user, :deny_follow_blocked]),
- {:user_blocked, false} <-
+ {_, false} <-
{:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked},
- {:user_locked, false} <- {:user_locked, User.locked?(followed)},
- {:follow, {:ok, follower}} <- {:follow, User.follow(follower, followed)} do
+ {_, 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
ActivityPub.accept(%{
to: [follower.ap_id],
actor: followed,
@@ -470,7 +472,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
})
else
{:user_blocked, true} ->
- {:ok, _} = Utils.update_follow_state(activity, "reject")
+ {:ok, _} = Utils.update_follow_state_for_all(activity, "reject")
ActivityPub.reject(%{
to: [follower.ap_id],
@@ -480,7 +482,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
})
{:follow, {:error, _}} ->
- {:ok, _} = Utils.update_follow_state(activity, "reject")
+ {:ok, _} = Utils.update_follow_state_for_all(activity, "reject")
ActivityPub.reject(%{
to: [follower.ap_id],
@@ -506,21 +508,16 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
with actor <- Containment.get_actor(data),
{:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor),
{:ok, follow_activity} <- get_follow_activity(follow_object, followed),
- {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"),
+ {: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, activity} <-
- ActivityPub.accept(%{
- to: follow_activity.data["to"],
- type: "Accept",
- actor: followed,
- object: follow_activity.data["id"],
- local: false
- }) do
- if not User.following?(follower, followed) do
- {:ok, _follower} = User.follow(follower, followed)
- end
-
- {:ok, activity}
+ {:ok, _follower} = User.follow(follower, followed) do
+ ActivityPub.accept(%{
+ to: follow_activity.data["to"],
+ type: "Accept",
+ actor: followed,
+ object: follow_activity.data["id"],
+ local: false
+ })
else
_e -> :error
end
@@ -532,7 +529,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
with actor <- Containment.get_actor(data),
{:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor),
{:ok, follow_activity} <- get_follow_activity(follow_object, followed),
- {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "reject"),
+ {: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, activity} <-
ActivityPub.reject(%{
diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex
@@ -376,8 +376,8 @@ defmodule Pleroma.Web.ActivityPub.Utils do
@doc """
Updates a follow activity's state (for locked accounts).
"""
- def update_follow_state(
- %Activity{data: %{"actor" => actor, "object" => object, "state" => "pending"}} = activity,
+ def update_follow_state_for_all(
+ %Activity{data: %{"actor" => actor, "object" => object}} = activity,
state
) do
try do
diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
@@ -35,9 +35,9 @@ defmodule Pleroma.Web.CommonAPI do
end
def accept_follow_request(follower, followed) do
- with {:ok, follower} <- User.maybe_follow(follower, followed),
+ with {:ok, follower} <- User.follow(follower, followed),
%Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed),
- {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"),
+ {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "accept"),
{:ok, _activity} <-
ActivityPub.accept(%{
to: [follower.ap_id],
@@ -51,7 +51,7 @@ defmodule Pleroma.Web.CommonAPI 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(follow_activity, "reject"),
+ {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "reject"),
{:ok, _activity} <-
ActivityPub.reject(%{
to: [follower.ap_id],
diff --git a/test/user_test.exs b/test/user_test.exs
@@ -75,7 +75,7 @@ defmodule Pleroma.UserTest do
Pleroma.Web.TwitterAPI.TwitterAPI.follow(pending_follower, %{"user_id" => locked.id})
Pleroma.Web.TwitterAPI.TwitterAPI.follow(pending_follower, %{"user_id" => locked.id})
Pleroma.Web.TwitterAPI.TwitterAPI.follow(accepted_follower, %{"user_id" => locked.id})
- User.maybe_follow(accepted_follower, locked)
+ User.follow(accepted_follower, locked)
assert {:ok, [activity]} = User.get_follow_requests(locked)
assert activity
diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs
@@ -27,14 +27,41 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do
|> Poison.decode!()
|> Map.put("object", user.ap_id)
- {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data)
+ {:ok, %Activity{data: data, local: false} = activity} = Transmogrifier.handle_incoming(data)
assert data["actor"] == "http://mastodon.example.org/users/admin"
assert data["type"] == "Follow"
assert data["id"] == "http://mastodon.example.org/users/admin#follows/2"
+
+ activity = Repo.get(Activity, activity.id)
+ assert activity.data["state"] == "accept"
assert User.following?(User.get_cached_by_ap_id(data["actor"]), user)
end
+ test "with locked accounts, it does not create a follow or an accept" do
+ user = insert(:user, info: %{locked: true})
+
+ data =
+ File.read!("test/fixtures/mastodon-follow-activity.json")
+ |> Poison.decode!()
+ |> Map.put("object", user.ap_id)
+
+ {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data)
+
+ assert data["state"] == "pending"
+
+ refute User.following?(User.get_cached_by_ap_id(data["actor"]), user)
+
+ accepts =
+ from(
+ a in Activity,
+ where: fragment("?->>'type' = ?", a.data, "Accept")
+ )
+ |> Repo.all()
+
+ assert length(accepts) == 0
+ end
+
test "it works for follow requests when you are already followed, creating a new accept activity" do
# This is important because the remote might have the wrong idea about the
# current follow status. This can lead to instance A thinking that x@A is
diff --git a/test/web/activity_pub/utils_test.exs b/test/web/activity_pub/utils_test.exs
@@ -2,6 +2,7 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
use Pleroma.DataCase
alias Pleroma.Activity
alias Pleroma.Object
+ alias Pleroma.Repo
alias Pleroma.User
alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.Utils
@@ -247,4 +248,51 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
assert fetched_vote.id == vote.id
end
end
+
+ describe "update_follow_state_for_all/2" do
+ test "updates the state of all Follow activities with the same actor and object" do
+ user = insert(:user, info: %{locked: true})
+ follower = insert(:user)
+
+ {:ok, follow_activity} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+
+ data =
+ follow_activity_two.data
+ |> Map.put("state", "accept")
+
+ cng = Ecto.Changeset.change(follow_activity_two, data: data)
+
+ {:ok, follow_activity_two} = Repo.update(cng)
+
+ {:ok, follow_activity_two} =
+ Utils.update_follow_state_for_all(follow_activity_two, "accept")
+
+ assert Repo.get(Activity, follow_activity.id).data["state"] == "accept"
+ assert Repo.get(Activity, follow_activity_two.id).data["state"] == "accept"
+ end
+ end
+
+ describe "update_follow_state/2" do
+ test "updates the state of the given follow activity" do
+ user = insert(:user, info: %{locked: true})
+ follower = insert(:user)
+
+ {:ok, follow_activity} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+
+ data =
+ follow_activity_two.data
+ |> Map.put("state", "accept")
+
+ cng = Ecto.Changeset.change(follow_activity_two, data: data)
+
+ {:ok, follow_activity_two} = Repo.update(cng)
+
+ {:ok, follow_activity_two} = Utils.update_follow_state(follow_activity_two, "reject")
+
+ assert Repo.get(Activity, follow_activity.id).data["state"] == "pending"
+ assert Repo.get(Activity, follow_activity_two.id).data["state"] == "reject"
+ end
+ end
end
diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs
@@ -7,6 +7,7 @@ defmodule Pleroma.Web.CommonAPITest do
alias Pleroma.Activity
alias Pleroma.Object
alias Pleroma.User
+ alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.CommonAPI
import Pleroma.Factory
@@ -339,4 +340,46 @@ defmodule Pleroma.Web.CommonAPITest do
assert User.showing_reblogs?(muter, muted) == true
end
end
+
+ describe "accept_follow_request/2" do
+ test "after acceptance, it sets all existing pending follow request states to 'accept'" do
+ user = insert(:user, info: %{locked: true})
+ follower = insert(:user)
+ follower_two = insert(:user)
+
+ {:ok, follow_activity} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user)
+
+ assert follow_activity.data["state"] == "pending"
+ assert follow_activity_two.data["state"] == "pending"
+ assert follow_activity_three.data["state"] == "pending"
+
+ {:ok, _follower} = CommonAPI.accept_follow_request(follower, user)
+
+ assert Repo.get(Activity, follow_activity.id).data["state"] == "accept"
+ assert Repo.get(Activity, follow_activity_two.id).data["state"] == "accept"
+ assert Repo.get(Activity, follow_activity_three.id).data["state"] == "pending"
+ end
+
+ test "after rejection, it sets all existing pending follow request states to 'reject'" do
+ user = insert(:user, info: %{locked: true})
+ follower = insert(:user)
+ follower_two = insert(:user)
+
+ {:ok, follow_activity} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+ {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user)
+
+ assert follow_activity.data["state"] == "pending"
+ assert follow_activity_two.data["state"] == "pending"
+ assert follow_activity_three.data["state"] == "pending"
+
+ {:ok, _follower} = CommonAPI.reject_follow_request(follower, user)
+
+ assert Repo.get(Activity, follow_activity.id).data["state"] == "reject"
+ assert Repo.get(Activity, follow_activity_two.id).data["state"] == "reject"
+ assert Repo.get(Activity, follow_activity_three.id).data["state"] == "pending"
+ end
+ end
end
