From a3071f023166cb5364ce56e3666d5a77baa16434 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 5 May 2020 15:08:41 +0200 Subject: [PATCH] Undoing: Move undoing likes to the pipeline everywhere. --- lib/pleroma/user.ex | 12 ++-- lib/pleroma/web/activity_pub/activity_pub.ex | 23 ------- lib/pleroma/web/activity_pub/side_effects.ex | 19 ++++++ .../web/activity_pub/transmogrifier.ex | 11 +--- lib/pleroma/web/common_api/common_api.ex | 9 ++- .../controllers/status_controller.ex | 6 +- test/notification_test.exs | 2 +- test/web/activity_pub/activity_pub_test.exs | 60 ------------------- test/web/activity_pub/side_effects_test.exs | 29 +++++++++ .../transmogrifier/undo_handling_test.exs | 9 ++- 10 files changed, 75 insertions(+), 105 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 99358ddaf..0136ba119 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -29,7 +29,9 @@ defmodule Pleroma.User do alias Pleroma.UserRelationship alias Pleroma.Web alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.ObjectValidators.Types + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI.Utils, as: CommonUtils @@ -1553,11 +1555,13 @@ defp delete_activity(%{data: %{"type" => "Create"}} = activity) do end defp delete_activity(%{data: %{"type" => "Like"}} = activity) do - object = Object.normalize(activity) + actor = + activity.actor + |> get_cached_by_ap_id() - activity.actor - |> get_cached_by_ap_id() - |> ActivityPub.unlike(object) + {:ok, undo, _} = Builder.undo(actor, activity) + + Pipeline.common_pipeline(undo, local: true) end defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1c21d78af..daad4d751 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -398,29 +398,6 @@ defp do_unreact_with_emoji(user, reaction_id, options) do end end - @spec unlike(User.t(), Object.t(), String.t() | nil, boolean()) :: - {:ok, Activity.t(), Activity.t(), Object.t()} | {:ok, Object.t()} | {:error, any()} - def unlike(%User{} = actor, %Object{} = object, activity_id \\ nil, local \\ true) do - with {:ok, result} <- - Repo.transaction(fn -> do_unlike(actor, object, activity_id, local) end) do - result - end - end - - defp do_unlike(actor, object, activity_id, local) do - with %Activity{} = like_activity <- get_existing_like(actor.ap_id, object), - unlike_data <- make_unlike_data(actor, like_activity, activity_id), - {:ok, unlike_activity} <- insert(unlike_data, local), - {:ok, _activity} <- Repo.delete(like_activity), - {:ok, object} <- remove_like_from_object(like_activity, object), - :ok <- maybe_federate(unlike_activity) do - {:ok, unlike_activity, like_activity, object} - else - nil -> {:ok, object} - {:error, error} -> Repo.rollback(error) - end - end - @spec announce(User.t(), Object.t(), String.t() | nil, boolean(), boolean()) :: {:ok, Activity.t(), Object.t()} | {:error, any()} def announce( diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 6a8f1af96..8ed91e257 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -5,8 +5,10 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do liked object, a `Follow` activity will add the user to the follower collection, and so on. """ + alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.Object + alias Pleroma.Repo alias Pleroma.Web.ActivityPub.Utils def handle(object, meta \\ []) @@ -23,8 +25,25 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do {:ok, object, meta} end + def handle(%{data: %{"type" => "Undo", "object" => undone_object}} = object, meta) do + with undone_object <- Activity.get_by_ap_id(undone_object), + :ok <- handle_undoing(undone_object) do + {:ok, object, meta} + end + end + # Nothing to do def handle(object, meta) do {:ok, object, meta} end + + def handle_undoing(%{data: %{"type" => "Like"}} = object) do + with %Object{} = liked_object <- Object.get_by_ap_id(object.data["object"]), + {:ok, _} <- Utils.remove_like_from_object(object, liked_object), + {:ok, _} <- Repo.delete(object) do + :ok + end + end + + def handle_undoing(object), do: {:error, ["don't know how to handle", object]} end diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 581e7040b..a60b27bea 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -865,19 +865,12 @@ def handle_incoming( def handle_incoming( %{ "type" => "Undo", - "object" => %{"type" => "Like", "object" => object_id}, - "actor" => _actor, - "id" => id + "object" => %{"type" => "Like"} } = data, _options ) do - with actor <- Containment.get_actor(data), - {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), - {:ok, object} <- get_obj_helper(object_id), - {:ok, activity, _, _} <- ActivityPub.unlike(actor, object, id, false) do + with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} - else - _e -> :error end end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index f9db97d24..a670ea5bc 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -166,9 +166,12 @@ def favorite_helper(user, id) do def unfavorite(id, user) do with {_, %Activity{data: %{"type" => "Create"}} = activity} <- - {:find_activity, Activity.get_by_id(id)} do - object = Object.normalize(activity) - ActivityPub.unlike(user, object) + {:find_activity, Activity.get_by_id(id)}, + %Object{} = note <- Object.normalize(activity, false), + %Activity{} = like <- Utils.get_existing_like(user.ap_id, note), + {:ok, undo, _} <- Builder.undo(user, like), + {:ok, activity, _} <- Pipeline.common_pipeline(undo, local: false) do + {:ok, activity} else {:find_activity, _} -> {:error, :not_found} _ -> {:error, dgettext("errors", "Could not unfavorite")} diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 9eea2e9eb..2a5eac9d9 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -222,9 +222,9 @@ def favourite(%{assigns: %{user: user}} = conn, %{"id" => activity_id}) do end @doc "POST /api/v1/statuses/:id/unfavourite" - def unfavourite(%{assigns: %{user: user}} = conn, %{"id" => ap_id_or_id}) do - with {:ok, _, _, %{data: %{"id" => id}}} <- CommonAPI.unfavorite(ap_id_or_id, user), - %Activity{} = activity <- Activity.get_create_by_object_ap_id(id) do + def unfavourite(%{assigns: %{user: user}} = conn, %{"id" => activity_id}) do + with {:ok, _unfav} <- CommonAPI.unfavorite(activity_id, user), + %Activity{} = activity <- Activity.get_by_id(activity_id) do try_render(conn, "show.json", activity: activity, for: user, as: :activity) end end diff --git a/test/notification_test.exs b/test/notification_test.exs index 601a6c0ca..7d5b82993 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -724,7 +724,7 @@ test "liking an activity results in 1 notification, then 0 if the activity is un assert length(Notification.for_user(user)) == 1 - {:ok, _, _, _} = CommonAPI.unfavorite(activity.id, other_user) + {:ok, _} = CommonAPI.unfavorite(activity.id, other_user) assert Enum.empty?(Notification.for_user(user)) end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 84ead93bb..797af66a0 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -995,66 +995,6 @@ test "reverts emoji unreact on error" do end end - describe "unliking" do - test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do - Config.put([:instance, :federating], true) - - note_activity = insert(:note_activity) - object = Object.normalize(note_activity) - user = insert(:user) - - {:ok, object} = ActivityPub.unlike(user, object) - refute called(Federator.publish()) - - {:ok, _like_activity} = CommonAPI.favorite(user, note_activity.id) - object = Object.get_by_id(object.id) - assert object.data["like_count"] == 1 - - {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) - assert object.data["like_count"] == 0 - - assert called(Federator.publish(unlike_activity)) - end - - test "reverts unliking on error" do - note_activity = insert(:note_activity) - user = insert(:user) - - {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) - object = Object.normalize(note_activity) - assert object.data["like_count"] == 1 - - with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do - assert {:error, :reverted} = ActivityPub.unlike(user, object) - end - - assert Object.get_by_ap_id(object.data["id"]) == object - assert object.data["like_count"] == 1 - assert Activity.get_by_id(like_activity.id) - end - - test "unliking a previously liked object" do - note_activity = insert(:note_activity) - object = Object.normalize(note_activity) - user = insert(:user) - - # Unliking something that hasn't been liked does nothing - {:ok, object} = ActivityPub.unlike(user, object) - assert object.data["like_count"] == 0 - - {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) - - object = Object.get_by_id(object.id) - assert object.data["like_count"] == 1 - - {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) - assert object.data["like_count"] == 0 - - assert Activity.get_by_id(like_activity.id) == nil - assert note_activity.actor in unlike_activity.recipients - end - end - describe "announcing an object" do test "adds an announce activity to the db" do note_activity = insert(:note_activity) diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index 0b6b55156..61ef72742 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -5,6 +5,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do use Pleroma.DataCase + alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo @@ -15,6 +16,34 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do import Pleroma.Factory + describe "Undo objects" do + setup do + poster = insert(:user) + user = insert(:user) + {:ok, post} = CommonAPI.post(poster, %{"status" => "hey"}) + {:ok, like} = CommonAPI.favorite(user, post.id) + + {:ok, undo_data, _meta} = Builder.undo(user, like) + {:ok, like_undo, _meta} = ActivityPub.persist(undo_data, local: true) + + %{like_undo: like_undo, post: post, like: like} + end + + test "a like undo removes the like from the object", %{like_undo: like_undo, post: post} do + {:ok, _like_undo, _} = SideEffects.handle(like_undo) + + object = Object.get_by_ap_id(post.data["object"]) + + assert object.data["like_count"] == 0 + assert object.data["likes"] == [] + end + + test "deletes the original like", %{like_undo: like_undo, like: like} do + {:ok, _like_undo, _} = SideEffects.handle(like_undo) + refute Activity.get_by_id(like.id) + end + end + describe "like objects" do setup do poster = insert(:user) diff --git a/test/web/activity_pub/transmogrifier/undo_handling_test.exs b/test/web/activity_pub/transmogrifier/undo_handling_test.exs index a9ebfdb18..bf2a6bc5b 100644 --- a/test/web/activity_pub/transmogrifier/undo_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/undo_handling_test.exs @@ -6,6 +6,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UndoHandlingTest do use Pleroma.DataCase alias Pleroma.Activity + alias Pleroma.Object alias Pleroma.User alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.CommonAPI @@ -67,7 +68,11 @@ test "it works for incoming unlikes with an existing like activity" do assert data["actor"] == "http://mastodon.example.org/users/admin" assert data["type"] == "Undo" assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo" - assert data["object"]["id"] == "http://mastodon.example.org/users/admin#likes/2" + assert data["object"] == "http://mastodon.example.org/users/admin#likes/2" + + note = Object.get_by_ap_id(like_data["object"]) + assert note.data["like_count"] == 0 + assert note.data["likes"] == [] end test "it works for incoming unlikes with an existing like activity and a compact object" do @@ -94,7 +99,7 @@ test "it works for incoming unlikes with an existing like activity and a compact assert data["actor"] == "http://mastodon.example.org/users/admin" assert data["type"] == "Undo" assert data["id"] == "http://mastodon.example.org/users/admin#likes/2/undo" - assert data["object"]["id"] == "http://mastodon.example.org/users/admin#likes/2" + assert data["object"] == "http://mastodon.example.org/users/admin#likes/2" end test "it works for incoming unannounces with an existing notice" do