Undoing: Move undoing blocks to the pipeline everywhere.
authorlain <lain@soykaf.club>
Tue, 5 May 2020 16:00:37 +0000 (18:00 +0200)
committerlain <lain@soykaf.club>
Tue, 5 May 2020 16:02:24 +0000 (18:02 +0200)
lib/pleroma/web/activity_pub/activity_pub.ex
lib/pleroma/web/activity_pub/side_effects.ex
lib/pleroma/web/activity_pub/transmogrifier.ex
lib/pleroma/web/activity_pub/utils.ex
lib/pleroma/web/common_api/common_api.ex
lib/pleroma/web/mastodon_api/controllers/account_controller.ex
test/web/activity_pub/activity_pub_test.exs
test/web/activity_pub/side_effects_test.exs
test/web/activity_pub/transmogrifier/undo_handling_test.exs
test/web/activity_pub/utils_test.exs

index be3d72c829ee4103959f45d7b3960f2cece0cdb2..78e8c0cbe2a6f72237e937aab66d2806b767de1c 100644 (file)
@@ -532,27 +532,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  @spec unblock(User.t(), User.t(), String.t() | nil, boolean()) ::
-          {:ok, Activity.t()} | {:error, any()} | nil
-  def unblock(blocker, blocked, activity_id \\ nil, local \\ true) do
-    with {:ok, result} <-
-           Repo.transaction(fn -> do_unblock(blocker, blocked, activity_id, local) end) do
-      result
-    end
-  end
-
-  defp do_unblock(blocker, blocked, activity_id, local) do
-    with %Activity{} = block_activity <- fetch_latest_block(blocker, blocked),
-         unblock_data <- make_unblock_data(blocker, blocked, block_activity, activity_id),
-         {:ok, activity} <- insert(unblock_data, local),
-         :ok <- maybe_federate(activity) do
-      {:ok, activity}
-    else
-      nil -> nil
-      {:error, error} -> Repo.rollback(error)
-    end
-  end
-
   @spec flag(map()) :: {:ok, Activity.t()} | {:error, any()}
   def flag(
         %{
index 146d30ac13a0e86d034aa5dc4a9a039da87b3c01..3fad6e4d8d7fe50fd720f6ff8c77da9d538c4956 100644 (file)
@@ -9,6 +9,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
   alias Pleroma.Notification
   alias Pleroma.Object
   alias Pleroma.Repo
+  alias Pleroma.User
   alias Pleroma.Web.ActivityPub.Utils
 
   def handle(object, meta \\ [])
@@ -61,5 +62,16 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
     end
   end
 
+  def handle_undoing(
+        %{data: %{"type" => "Block", "actor" => blocker, "object" => blocked}} = object
+      ) do
+    with %User{} = blocker <- User.get_cached_by_ap_id(blocker),
+         %User{} = blocked <- User.get_cached_by_ap_id(blocked),
+         {:ok, _} <- User.unblock(blocker, blocked),
+         {:ok, _} <- Repo.delete(object) do
+      :ok
+    end
+  end
+
   def handle_undoing(object), do: {:error, ["don't know how to handle", object]}
 end
index afa1714486be9a5a0ae1fb73de5a69f77e2fddd3..65ae643ed52151f20b8eff2c173174db3560c43a 100644 (file)
@@ -787,40 +787,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => %{"type" => "Block", "object" => blocked},
-          "actor" => blocker,
-          "id" => id
-        } = _data,
-        _options
-      ) do
-    with %User{local: true} = blocked <- User.get_cached_by_ap_id(blocked),
-         {:ok, %User{} = blocker} <- User.get_or_fetch_by_ap_id(blocker),
-         {:ok, activity} <- ActivityPub.unblock(blocker, blocked, id, false) do
-      User.unblock(blocker, blocked)
-      {:ok, activity}
-    else
-      _e -> :error
-    end
-  end
-
-  def handle_incoming(
-        %{"type" => "Block", "object" => blocked, "actor" => blocker, "id" => id} = _data,
-        _options
-      ) do
-    with %User{local: true} = blocked = User.get_cached_by_ap_id(blocked),
-         {:ok, %User{} = blocker} = User.get_or_fetch_by_ap_id(blocker),
-         {:ok, activity} <- ActivityPub.block(blocker, blocked, id, false) do
-      User.unfollow(blocker, blocked)
-      User.block(blocker, blocked)
-      {:ok, activity}
-    else
-      _e -> :error
-    end
-  end
-
   def handle_incoming(
         %{
           "type" => "Undo",
@@ -828,7 +794,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
         } = data,
         _options
       )
-      when type in ["Like", "EmojiReact", "Announce"] do
+      when type in ["Like", "EmojiReact", "Announce", "Block"] do
     with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
       {:ok, activity}
     end
@@ -852,6 +818,21 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
+  def handle_incoming(
+        %{"type" => "Block", "object" => blocked, "actor" => blocker, "id" => id} = _data,
+        _options
+      ) do
+    with %User{local: true} = blocked = User.get_cached_by_ap_id(blocked),
+         {:ok, %User{} = blocker} = User.get_or_fetch_by_ap_id(blocker),
+         {:ok, activity} <- ActivityPub.block(blocker, blocked, id, false) do
+      User.unfollow(blocker, blocked)
+      User.block(blocker, blocked)
+      {:ok, activity}
+    else
+      _e -> :error
+    end
+  end
+
   def handle_incoming(
         %{
           "type" => "Move",
index 2d685ecc09d11f39ad526f7ddbbefb5be69e6a36..95fb382f05fe1afb83f66dbdd44c85a4389fc2a6 100644 (file)
@@ -562,45 +562,6 @@ defmodule Pleroma.Web.ActivityPub.Utils do
     |> maybe_put("id", activity_id)
   end
 
-  @doc """
-  Make unannounce activity data for the given actor and object
-  """
-  def make_unannounce_data(
-        %User{ap_id: ap_id} = user,
-        %Activity{data: %{"context" => context, "object" => object}} = activity,
-        activity_id
-      ) do
-    object = Object.normalize(object)
-
-    %{
-      "type" => "Undo",
-      "actor" => ap_id,
-      "object" => activity.data,
-      "to" => [user.follower_address, object.data["actor"]],
-      "cc" => [Pleroma.Constants.as_public()],
-      "context" => context
-    }
-    |> maybe_put("id", activity_id)
-  end
-
-  def make_unlike_data(
-        %User{ap_id: ap_id} = user,
-        %Activity{data: %{"context" => context, "object" => object}} = activity,
-        activity_id
-      ) do
-    object = Object.normalize(object)
-
-    %{
-      "type" => "Undo",
-      "actor" => ap_id,
-      "object" => activity.data,
-      "to" => [user.follower_address, object.data["actor"]],
-      "cc" => [Pleroma.Constants.as_public()],
-      "context" => context
-    }
-    |> maybe_put("id", activity_id)
-  end
-
   def make_undo_data(
         %User{ap_id: actor, follower_address: follower_address},
         %Activity{
@@ -688,16 +649,6 @@ defmodule Pleroma.Web.ActivityPub.Utils do
     |> maybe_put("id", activity_id)
   end
 
-  def make_unblock_data(blocker, blocked, block_activity, activity_id) do
-    %{
-      "type" => "Undo",
-      "actor" => blocker.ap_id,
-      "to" => [blocked.ap_id],
-      "object" => block_activity.data
-    }
-    |> maybe_put("id", activity_id)
-  end
-
   #### Create-related helpers
 
   def make_create_data(params, additional) do
index fc82468717f77a0c3a5d10f9f8a0acb79338def6..2a1eb7f37b821dd7afe54e594c96062527683d69 100644 (file)
@@ -24,6 +24,14 @@ defmodule Pleroma.Web.CommonAPI do
   require Pleroma.Constants
   require Logger
 
+  def unblock(blocker, blocked) do
+    with %Activity{} = block <- Utils.fetch_latest_block(blocker, blocked),
+         {:ok, unblock_data, _} <- Builder.undo(blocker, block),
+         {:ok, unblock, _} <- Pipeline.common_pipeline(unblock_data, local: true) do
+      {:ok, unblock}
+    end
+  end
+
   def follow(follower, followed) do
     timeout = Pleroma.Config.get([:activitypub, :follow_handshake_timeout])
 
index 61b0e2f633c939599ca1d0ee56cb49e7eb523114..2b208ddab58d4870c85efd941c827e7375b488b6 100644 (file)
@@ -356,8 +356,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
 
   @doc "POST /api/v1/accounts/:id/unblock"
   def unblock(%{assigns: %{user: blocker, account: blocked}} = conn, _params) do
-    with {:ok, _user_block} <- User.unblock(blocker, blocked),
-         {:ok, _activity} <- ActivityPub.unblock(blocker, blocked) do
+    with {:ok, _activity} <- CommonAPI.unblock(blocker, blocked) do
       render(conn, "relationship.json", user: blocker, target: blocked)
     else
       {:error, message} -> json_response(conn, :forbidden, %{error: message})
index 2c3d354f27e6a83754f4c3c430c049ecef7a6477..7824095c78b53aa93c6c170069c3fc2b56061e36 100644 (file)
@@ -1114,7 +1114,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
     end
   end
 
-  describe "blocking / unblocking" do
+  describe "blocking" do
     test "reverts block activity on error" do
       [blocker, blocked] = insert_list(2, :user)
 
@@ -1136,38 +1136,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert activity.data["actor"] == blocker.ap_id
       assert activity.data["object"] == blocked.ap_id
     end
-
-    test "reverts unblock activity on error" do
-      [blocker, blocked] = insert_list(2, :user)
-      {:ok, block_activity} = ActivityPub.block(blocker, blocked)
-
-      with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do
-        assert {:error, :reverted} = ActivityPub.unblock(blocker, blocked)
-      end
-
-      assert block_activity.data["type"] == "Block"
-      assert block_activity.data["actor"] == blocker.ap_id
-
-      assert Repo.aggregate(Activity, :count, :id) == 1
-      assert Repo.aggregate(Object, :count, :id) == 1
-    end
-
-    test "creates an undo activity for the last block" do
-      blocker = insert(:user)
-      blocked = insert(:user)
-
-      {:ok, block_activity} = ActivityPub.block(blocker, blocked)
-      {:ok, activity} = ActivityPub.unblock(blocker, blocked)
-
-      assert activity.data["type"] == "Undo"
-      assert activity.data["actor"] == blocker.ap_id
-
-      embedded_object = activity.data["object"]
-      assert is_map(embedded_object)
-      assert embedded_object["type"] == "Block"
-      assert embedded_object["object"] == blocked.ap_id
-      assert embedded_object["id"] == block_activity.data["id"]
-    end
   end
 
   describe "deletion" do
index 00241320b02d05d748bbf93d1fff5ff12935fd4e..f41a7f3c1a935860bcdb2303646577a4b7725bcd 100644 (file)
@@ -9,6 +9,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
   alias Pleroma.Notification
   alias Pleroma.Object
   alias Pleroma.Repo
+  alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Builder
   alias Pleroma.Web.ActivityPub.SideEffects
@@ -24,6 +25,8 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
       {:ok, like} = CommonAPI.favorite(user, post.id)
       {:ok, reaction, _} = CommonAPI.react_with_emoji(post.id, user, "👍")
       {:ok, announce, _} = CommonAPI.repeat(post.id, user)
+      {:ok, block} = ActivityPub.block(user, poster)
+      User.block(user, poster)
 
       {:ok, undo_data, _meta} = Builder.undo(user, like)
       {:ok, like_undo, _meta} = ActivityPub.persist(undo_data, local: true)
@@ -34,6 +37,9 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
       {:ok, undo_data, _meta} = Builder.undo(user, announce)
       {:ok, announce_undo, _meta} = ActivityPub.persist(undo_data, local: true)
 
+      {:ok, undo_data, _meta} = Builder.undo(user, block)
+      {:ok, block_undo, _meta} = ActivityPub.persist(undo_data, local: true)
+
       %{
         like_undo: like_undo,
         post: post,
@@ -41,10 +47,27 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
         reaction_undo: reaction_undo,
         reaction: reaction,
         announce_undo: announce_undo,
-        announce: announce
+        announce: announce,
+        block_undo: block_undo,
+        block: block,
+        poster: poster,
+        user: user
       }
     end
 
+    test "deletes the original block", %{block_undo: block_undo, block: block} do
+      {:ok, _block_undo, _} = SideEffects.handle(block_undo)
+      refute Activity.get_by_id(block.id)
+    end
+
+    test "unblocks the blocked user", %{block_undo: block_undo, block: block} do
+      blocker = User.get_by_ap_id(block.data["actor"])
+      blocked = User.get_by_ap_id(block.data["object"])
+
+      {:ok, _block_undo, _} = SideEffects.handle(block_undo)
+      refute User.blocks?(blocker, blocked)
+    end
+
     test "an announce undo removes the announce from the object", %{
       announce_undo: announce_undo,
       post: post
index 281cf5b0da77d43a00d285e2d6715f119e883cf1..6f5e61ac3f6ed3d107114bbc420acea219747d80 100644 (file)
@@ -176,9 +176,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UndoHandlingTest do
 
     {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data)
     assert data["type"] == "Undo"
-    assert data["object"]["type"] == "Block"
-    assert data["object"]["object"] == user.ap_id
-    assert data["actor"] == "http://mastodon.example.org/users/admin"
+    assert data["object"] == block_data["id"]
 
     blocker = User.get_cached_by_ap_id(data["actor"])
 
index b0bfed9178a53cba7a5f034cbe0cc1060d4f487e..b8d811c73d96fc1e02bc740dcd1f29d6433e0107 100644 (file)
@@ -102,34 +102,6 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
     end
   end
 
-  describe "make_unlike_data/3" do
-    test "returns data for unlike activity" do
-      user = insert(:user)
-      like_activity = insert(:like_activity, data_attrs: %{"context" => "test context"})
-
-      object = Object.normalize(like_activity.data["object"])
-
-      assert Utils.make_unlike_data(user, like_activity, nil) == %{
-               "type" => "Undo",
-               "actor" => user.ap_id,
-               "object" => like_activity.data,
-               "to" => [user.follower_address, object.data["actor"]],
-               "cc" => [Pleroma.Constants.as_public()],
-               "context" => like_activity.data["context"]
-             }
-
-      assert Utils.make_unlike_data(user, like_activity, "9mJEZK0tky1w2xD2vY") == %{
-               "type" => "Undo",
-               "actor" => user.ap_id,
-               "object" => like_activity.data,
-               "to" => [user.follower_address, object.data["actor"]],
-               "cc" => [Pleroma.Constants.as_public()],
-               "context" => like_activity.data["context"],
-               "id" => "9mJEZK0tky1w2xD2vY"
-             }
-    end
-  end
-
   describe "make_like_data" do
     setup do
       user = insert(:user)