From de993b856bc2145e7c4aaa47767c7edc826798c7 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Mon, 28 Sep 2020 09:16:42 +0300 Subject: [PATCH] added `force` option to the unfollow operation --- docs/API/admin_api.md | 6 +- lib/mix/tasks/pleroma/relay.ex | 13 +++- lib/pleroma/user.ex | 4 +- lib/pleroma/web/activity_pub/relay.ex | 20 ++++- .../admin_api/controllers/relay_controller.ex | 16 +--- .../operations/admin/relay_operation.ex | 12 ++- test/tasks/relay_test.exs | 74 +++++++++++++++++++ test/web/activity_pub/relay_test.exs | 40 ++++++++++ 8 files changed, 160 insertions(+), 25 deletions(-) diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index 7992db58f..ddcab1a2c 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -349,9 +349,9 @@ Response: ### Unfollow a Relay -Params: - -* `relay_url` +- Params: + - `relay_url` + - *optional* `force`: forcefully unfollow a relay even when the relay is not available. (default is `false`) Response: diff --git a/lib/mix/tasks/pleroma/relay.ex b/lib/mix/tasks/pleroma/relay.ex index a6d8d6c1c..bb808ca47 100644 --- a/lib/mix/tasks/pleroma/relay.ex +++ b/lib/mix/tasks/pleroma/relay.ex @@ -21,10 +21,19 @@ defmodule Mix.Tasks.Pleroma.Relay do end end - def run(["unfollow", target]) do + def run(["unfollow", target | rest]) do start_pleroma() - with {:ok, _activity} <- Relay.unfollow(target) do + {options, [], []} = + OptionParser.parse( + rest, + strict: [force: :boolean], + aliases: [f: :force] + ) + + force = Keyword.get(options, :force, false) + + with {:ok, _activity} <- Relay.unfollow(target, %{force: force}) do # put this task to sleep to allow the genserver to push out the messages :timer.sleep(500) else diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 03be61ccf..71ace1c34 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -915,9 +915,7 @@ defmodule Pleroma.User do FollowingRelationship.unfollow(follower, followed) {:ok, followed} = update_follower_count(followed) - {:ok, follower} = - follower - |> update_following_count() + {:ok, follower} = update_following_count(follower) {:ok, follower, followed} diff --git a/lib/pleroma/web/activity_pub/relay.ex b/lib/pleroma/web/activity_pub/relay.ex index b65710a94..6606e1780 100644 --- a/lib/pleroma/web/activity_pub/relay.ex +++ b/lib/pleroma/web/activity_pub/relay.ex @@ -30,12 +30,16 @@ defmodule Pleroma.Web.ActivityPub.Relay do end end - @spec unfollow(String.t()) :: {:ok, Activity.t()} | {:error, any()} - def unfollow(target_instance) do + @spec unfollow(String.t(), map()) :: {:ok, Activity.t()} | {:error, any()} + def unfollow(target_instance, opts \\ %{}) do with %User{} = local_user <- get_actor(), - {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_instance), + {:ok, target_user} <- fetch_target_user(target_instance, opts), {:ok, activity} <- ActivityPub.unfollow(local_user, target_user) do - User.unfollow(local_user, target_user) + case target_user.id do + nil -> User.update_following_count(local_user) + _ -> User.unfollow(local_user, target_user) + end + Logger.info("relay: unfollowed instance: #{target_instance}: id=#{activity.data["id"]}") {:ok, activity} else @@ -43,6 +47,14 @@ defmodule Pleroma.Web.ActivityPub.Relay do end end + defp fetch_target_user(ap_id, opts) do + case {opts[:force], User.get_or_fetch_by_ap_id(ap_id)} do + {_, {:ok, %User{} = user}} -> {:ok, user} + {true, _} -> {:ok, %User{ap_id: ap_id}} + {_, error} -> error + end + end + @spec publish(any()) :: {:ok, Activity.t()} | {:error, any()} def publish(%Activity{data: %{"type" => "Create"}} = activity) do with %User{} = user <- get_actor(), diff --git a/lib/pleroma/web/admin_api/controllers/relay_controller.ex b/lib/pleroma/web/admin_api/controllers/relay_controller.ex index 95d06dde7..6c19f09f7 100644 --- a/lib/pleroma/web/admin_api/controllers/relay_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/relay_controller.ex @@ -33,11 +33,7 @@ defmodule Pleroma.Web.AdminAPI.RelayController do def follow(%{assigns: %{user: admin}, body_params: %{relay_url: target}} = conn, _) do with {:ok, _message} <- Relay.follow(target) do - ModerationLog.insert_log(%{ - action: "relay_follow", - actor: admin, - target: target - }) + ModerationLog.insert_log(%{action: "relay_follow", actor: admin, target: target}) json(conn, %{actor: target, followed_back: target in Relay.following()}) else @@ -48,13 +44,9 @@ defmodule Pleroma.Web.AdminAPI.RelayController do end end - def unfollow(%{assigns: %{user: admin}, body_params: %{relay_url: target}} = conn, _) do - with {:ok, _message} <- Relay.unfollow(target) do - ModerationLog.insert_log(%{ - action: "relay_unfollow", - actor: admin, - target: target - }) + def unfollow(%{assigns: %{user: admin}, body_params: %{relay_url: target} = params} = conn, _) do + with {:ok, _message} <- Relay.unfollow(target, %{force: params[:force]}) do + ModerationLog.insert_log(%{action: "relay_unfollow", actor: admin, target: target}) json(conn, target) else diff --git a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex index e06b2d164..f754bb9f5 100644 --- a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex @@ -56,7 +56,7 @@ defmodule Pleroma.Web.ApiSpec.Admin.RelayOperation do operationId: "AdminAPI.RelayController.unfollow", security: [%{"oAuth" => ["write:follows"]}], parameters: admin_api_params(), - requestBody: request_body("Parameters", relay_url()), + requestBody: request_body("Parameters", relay_unfollow()), responses: %{ 200 => Operation.response("Status", "application/json", %Schema{ @@ -91,4 +91,14 @@ defmodule Pleroma.Web.ApiSpec.Admin.RelayOperation do } } end + + defp relay_unfollow do + %Schema{ + type: :object, + properties: %{ + relay_url: %Schema{type: :string, format: :uri}, + force: %Schema{type: :boolean, default: false} + } + } + end end diff --git a/test/tasks/relay_test.exs b/test/tasks/relay_test.exs index e5225b64c..cf48e7dda 100644 --- a/test/tasks/relay_test.exs +++ b/test/tasks/relay_test.exs @@ -81,6 +81,80 @@ defmodule Mix.Tasks.Pleroma.RelayTest do assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"] refute "#{target_instance}/followers" in User.following(local_user) end + + test "unfollow when relay is dead" do + user = insert(:user) + target_instance = user.ap_id + + Mix.Tasks.Pleroma.Relay.run(["follow", target_instance]) + + %User{ap_id: follower_id} = local_user = Relay.get_actor() + 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 User.following(local_user) + + Tesla.Mock.mock(fn %{method: :get, url: ^target_instance} -> + %Tesla.Env{status: 404} + end) + + Pleroma.Repo.delete(user) + Cachex.clear(:user_cache) + + Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance]) + + cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"]) + assert cancelled_activity.data["state"] == "accept" + + assert [] == + ActivityPub.fetch_activities( + [], + %{ + type: "Undo", + actor_id: follower_id, + skip_preload: true, + invisible_actors: true + } + ) + end + + test "force unfollow when relay is dead" do + user = insert(:user) + target_instance = user.ap_id + + Mix.Tasks.Pleroma.Relay.run(["follow", target_instance]) + + %User{ap_id: follower_id} = local_user = Relay.get_actor() + 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 User.following(local_user) + + Tesla.Mock.mock(fn %{method: :get, url: ^target_instance} -> + %Tesla.Env{status: 404} + end) + + Pleroma.Repo.delete(user) + Cachex.clear(:user_cache) + + Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance, "--force"]) + + cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"]) + assert cancelled_activity.data["state"] == "cancelled" + + [undo_activity] = + ActivityPub.fetch_activities( + [], + %{type: "Undo", actor_id: follower_id, skip_preload: true, invisible_actors: true} + ) + + assert undo_activity.data["type"] == "Undo" + assert undo_activity.data["actor"] == local_user.ap_id + assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"] + refute "#{target_instance}/followers" in User.following(local_user) + end end describe "mix pleroma.relay list" do diff --git a/test/web/activity_pub/relay_test.exs b/test/web/activity_pub/relay_test.exs index 9d657ac4f..3284980f7 100644 --- a/test/web/activity_pub/relay_test.exs +++ b/test/web/activity_pub/relay_test.exs @@ -63,6 +63,46 @@ defmodule Pleroma.Web.ActivityPub.RelayTest do assert activity.data["to"] == [user.ap_id] refute "#{user.ap_id}/followers" in User.following(service_actor) end + + test "force unfollow when target service is dead" do + user = insert(:user) + user_ap_id = user.ap_id + user_id = user.id + + Tesla.Mock.mock(fn %{method: :get, url: ^user_ap_id} -> + %Tesla.Env{status: 404} + end) + + service_actor = Relay.get_actor() + CommonAPI.follow(service_actor, user) + assert "#{user.ap_id}/followers" in User.following(service_actor) + + assert Pleroma.Repo.get_by( + Pleroma.FollowingRelationship, + follower_id: service_actor.id, + following_id: user_id + ) + + Pleroma.Repo.delete(user) + Cachex.clear(:user_cache) + + assert {:ok, %Activity{} = activity} = Relay.unfollow(user_ap_id, %{force: true}) + + assert refresh_record(service_actor).following_count == 0 + + refute Pleroma.Repo.get_by( + Pleroma.FollowingRelationship, + follower_id: service_actor.id, + following_id: user_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 User.following(service_actor) + end end describe "publish/1" do