From b34debe61540cf845ccf4ac93066e45a1d9c8f85 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 5 May 2020 16:17:09 +0200 Subject: [PATCH] Undoing: Move undoing reactions to the pipeline everywhere. --- lib/pleroma/web/activity_pub/activity_pub.ex | 25 --------- lib/pleroma/web/activity_pub/side_effects.ex | 8 +++ .../web/activity_pub/transmogrifier.ex | 27 +-------- lib/pleroma/web/common_api/common_api.ex | 8 ++- .../controllers/pleroma_api_controller.ex | 3 +- test/web/activity_pub/activity_pub_test.exs | 56 ------------------- test/web/activity_pub/side_effects_test.exs | 30 +++++++++- test/web/common_api/common_api_test.exs | 3 +- .../pleroma_api_controller_test.exs | 10 +++- 9 files changed, 57 insertions(+), 113 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index daad4d751..c94af3b5f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -373,31 +373,6 @@ defp do_react_with_emoji(user, object, emoji, options) do end end - @spec unreact_with_emoji(User.t(), String.t(), keyword()) :: - {:ok, Activity.t(), Object.t()} | {:error, any()} - def unreact_with_emoji(user, reaction_id, options \\ []) do - with {:ok, result} <- - Repo.transaction(fn -> do_unreact_with_emoji(user, reaction_id, options) end) do - result - end - end - - defp do_unreact_with_emoji(user, reaction_id, options) do - with local <- Keyword.get(options, :local, true), - activity_id <- Keyword.get(options, :activity_id, nil), - user_ap_id <- user.ap_id, - %Activity{actor: ^user_ap_id} = reaction_activity <- Activity.get_by_ap_id(reaction_id), - object <- Object.normalize(reaction_activity), - unreact_data <- make_undo_data(user, reaction_activity, activity_id), - {:ok, activity} <- insert(unreact_data, local), - {:ok, object} <- remove_emoji_reaction_from_object(reaction_activity, object), - :ok <- maybe_federate(activity) do - {:ok, activity, object} - else - {: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 8ed91e257..d58df9394 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -45,5 +45,13 @@ def handle_undoing(%{data: %{"type" => "Like"}} = object) do end end + def handle_undoing(%{data: %{"type" => "EmojiReact"}} = object) do + with %Object{} = reacted_object <- Object.get_by_ap_id(object.data["object"]), + {:ok, _} <- Utils.remove_emoji_reaction_from_object(object, reacted_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 a60b27bea..94849b5f5 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -806,28 +806,6 @@ def handle_incoming( end end - def handle_incoming( - %{ - "type" => "Undo", - "object" => %{"type" => "EmojiReact", "id" => reaction_activity_id}, - "actor" => _actor, - "id" => id - } = data, - _options - ) do - with actor <- Containment.get_actor(data), - {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), - {:ok, activity, _} <- - ActivityPub.unreact_with_emoji(actor, reaction_activity_id, - activity_id: id, - local: false - ) do - {:ok, activity} - else - _e -> :error - end - end - def handle_incoming( %{ "type" => "Undo", @@ -865,10 +843,11 @@ def handle_incoming( def handle_incoming( %{ "type" => "Undo", - "object" => %{"type" => "Like"} + "object" => %{"type" => type} } = data, _options - ) do + ) + when type in ["Like", "EmojiReact"] do with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index a670ea5bc..067ac875e 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -170,7 +170,7 @@ def unfavorite(id, user) do %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, _} <- Pipeline.common_pipeline(undo, local: true) do {:ok, activity} else {:find_activity, _} -> {:error, :not_found} @@ -189,8 +189,10 @@ def react_with_emoji(id, user, emoji) do end def unreact_with_emoji(id, user, emoji) do - with %Activity{} = reaction_activity <- Utils.get_latest_reaction(id, user, emoji) do - ActivityPub.unreact_with_emoji(user, reaction_activity.data["id"]) + with %Activity{} = reaction_activity <- Utils.get_latest_reaction(id, user, emoji), + {:ok, undo, _} <- Builder.undo(user, reaction_activity), + {:ok, activity, _} <- Pipeline.common_pipeline(undo, local: true) do + {:ok, activity} else _ -> {:error, dgettext("errors", "Could not remove reaction emoji")} diff --git a/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex b/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex index 1bdb3aa4d..4aa5c1dd8 100644 --- a/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex +++ b/lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex @@ -98,7 +98,8 @@ def unreact_with_emoji(%{assigns: %{user: user}} = conn, %{ "id" => activity_id, "emoji" => emoji }) do - with {:ok, _activity, _object} <- CommonAPI.unreact_with_emoji(activity_id, user, emoji), + with {:ok, _activity} <- + CommonAPI.unreact_with_emoji(activity_id, user, emoji), activity <- Activity.get_by_id(activity_id) do conn |> put_view(StatusView) diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 797af66a0..cb2d41f0b 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -939,62 +939,6 @@ test "reverts emoji reaction on error" do end end - describe "unreacting to an object" do - test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do - Config.put([:instance, :federating], true) - user = insert(:user) - reactor = insert(:user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "YASSSS queen slay"}) - assert object = Object.normalize(activity) - - {:ok, reaction_activity, _object} = ActivityPub.react_with_emoji(reactor, object, "🔥") - - assert called(Federator.publish(reaction_activity)) - - {:ok, unreaction_activity, _object} = - ActivityPub.unreact_with_emoji(reactor, reaction_activity.data["id"]) - - assert called(Federator.publish(unreaction_activity)) - end - - test "adds an undo activity to the db" do - user = insert(:user) - reactor = insert(:user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "YASSSS queen slay"}) - assert object = Object.normalize(activity) - - {:ok, reaction_activity, _object} = ActivityPub.react_with_emoji(reactor, object, "🔥") - - {:ok, unreaction_activity, _object} = - ActivityPub.unreact_with_emoji(reactor, reaction_activity.data["id"]) - - assert unreaction_activity.actor == reactor.ap_id - assert unreaction_activity.data["object"] == reaction_activity.data["id"] - - object = Object.get_by_ap_id(object.data["id"]) - assert object.data["reaction_count"] == 0 - assert object.data["reactions"] == [] - end - - test "reverts emoji unreact on error" do - [user, reactor] = insert_list(2, :user) - {:ok, activity} = CommonAPI.post(user, %{"status" => "Status"}) - object = Object.normalize(activity) - - {:ok, reaction_activity, _object} = ActivityPub.react_with_emoji(reactor, object, "😀") - - with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do - assert {:error, :reverted} = - ActivityPub.unreact_with_emoji(reactor, reaction_activity.data["id"]) - end - - object = Object.get_by_ap_id(object.data["id"]) - - assert object.data["reaction_count"] == 1 - assert object.data["reactions"] == [["😀", [reactor.ap_id]]] - 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 61ef72742..abcfdfa2f 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -23,10 +23,38 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do {:ok, post} = CommonAPI.post(poster, %{"status" => "hey"}) {:ok, like} = CommonAPI.favorite(user, post.id) + {:ok, reaction, _} = CommonAPI.react_with_emoji(post.id, user, "👍") + {: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} + {:ok, undo_data, _meta} = Builder.undo(user, reaction) + {:ok, reaction_undo, _meta} = ActivityPub.persist(undo_data, local: true) + + %{ + like_undo: like_undo, + post: post, + like: like, + reaction_undo: reaction_undo, + reaction: reaction + } + end + + test "a reaction undo removes the reaction from the object", %{ + reaction_undo: reaction_undo, + post: post + } do + {:ok, _reaction_undo, _} = SideEffects.handle(reaction_undo) + + object = Object.get_by_ap_id(post.data["object"]) + + assert object.data["reaction_count"] == 0 + assert object.data["reactions"] == [] + end + + test "deletes the original reaction", %{reaction_undo: reaction_undo, reaction: reaction} do + {:ok, _reaction_undo, _} = SideEffects.handle(reaction_undo) + refute Activity.get_by_id(reaction.id) end test "a like undo removes the like from the object", %{like_undo: like_undo, post: post} do diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index bc0c1a791..0664b7f90 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -295,10 +295,11 @@ test "unreacting to a status with an emoji" do {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"}) {:ok, reaction, _} = CommonAPI.react_with_emoji(activity.id, user, "👍") - {:ok, unreaction, _} = CommonAPI.unreact_with_emoji(activity.id, user, "👍") + {:ok, unreaction} = CommonAPI.unreact_with_emoji(activity.id, user, "👍") assert unreaction.data["type"] == "Undo" assert unreaction.data["object"] == reaction.data["id"] + assert unreaction.local end test "repeating a status" do diff --git a/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs b/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs index 61a1689b9..299dbad41 100644 --- a/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs +++ b/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs @@ -3,12 +3,14 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.PleromaAPI.PleromaAPIControllerTest do + use Oban.Testing, repo: Pleroma.Repo use Pleroma.Web.ConnCase alias Pleroma.Conversation.Participation alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo + alias Pleroma.Tests.ObanHelpers alias Pleroma.User alias Pleroma.Web.CommonAPI @@ -41,7 +43,9 @@ test "DELETE /api/v1/pleroma/statuses/:id/reactions/:emoji", %{conn: conn} do other_user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "#cofe"}) - {:ok, activity, _object} = CommonAPI.react_with_emoji(activity.id, other_user, "☕") + {:ok, _reaction, _object} = CommonAPI.react_with_emoji(activity.id, other_user, "☕") + + ObanHelpers.perform_all() result = conn @@ -52,7 +56,9 @@ test "DELETE /api/v1/pleroma/statuses/:id/reactions/:emoji", %{conn: conn} do assert %{"id" => id} = json_response(result, 200) assert to_string(activity.id) == id - object = Object.normalize(activity) + ObanHelpers.perform_all() + + object = Object.get_by_ap_id(activity.data["object"]) assert object.data["reaction_count"] == 0 end