Credo fixes.

lib/pleroma/user.ex

@@ -1435,15 +1435,13 @@ defmodule Pleroma.User do
     user
     |> get_followers()
     |> Enum.each(fn follower ->
-      ActivityPub.unfollow(follower, user)
-      unfollow(follower, user)
+      CommonAPI.unfollow(follower, user)
     end)
 
     user
     |> get_friends()
     |> Enum.each(fn followed ->
-      ActivityPub.unfollow(user, followed)
-      unfollow(user, followed)
+      CommonAPI.unfollow(user, followed)
     end)
 
     delete_user_activities(user)

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -17,7 +17,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
   alias Pleroma.Repo
   alias Pleroma.Upload
   alias Pleroma.User
+  alias Pleroma.Web.ActivityPub.Builder
   alias Pleroma.Web.ActivityPub.MRF
+  alias Pleroma.Web.ActivityPub.Pipeline
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.Streamer
@@ -421,28 +423,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  @spec unfollow(User.t(), User.t(), String.t() | nil, boolean()) ::
-          {:ok, Activity.t()} | nil | {:error, any()}
-  def unfollow(follower, followed, activity_id \\ nil, local \\ true) do
-    with {:ok, result} <-
-           Repo.transaction(fn -> do_unfollow(follower, followed, activity_id, local) end) do
-      result
-    end
-  end
-
-  defp do_unfollow(follower, followed, activity_id, local) do
-    with %Activity{} = follow_activity <- fetch_latest_follow(follower, followed),
-         {:ok, follow_activity} <- update_follow_state(follow_activity, "cancelled"),
-         unfollow_data <- make_unfollow_data(follower, followed, follow_activity, activity_id),
-         {:ok, activity} <- insert(unfollow_data, local),
-         :ok <- maybe_federate(activity) do
-      {:ok, activity}
-    else
-      nil -> nil
-      {:error, error} -> Repo.rollback(error)
-    end
-  end
-
   @spec delete(User.t() | Object.t(), keyword()) :: {:ok, User.t() | Object.t()} | {:error, any()}
   def delete(entity, options \\ []) do
     with {:ok, result} <- Repo.transaction(fn -> do_delete(entity, options) end) do
@@ -518,8 +498,11 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     unfollow_blocked = Config.get([:activitypub, :unfollow_blocked])
 
     if unfollow_blocked do
-      follow_activity = fetch_latest_follow(blocker, blocked)
-      if follow_activity, do: unfollow(blocker, blocked, nil, local)
+      with %Activity{} = follow_activity <- fetch_latest_follow(blocker, blocked),
+           {:ok, undo_data, _} <- Builder.undo(blocker, follow_activity),
+           {:ok, _undo, _} <- Pipeline.common_pipeline(undo_data, local: local) do
+        :ok
+      end
     end
 
     with true <- outgoing_blocks,

lib/pleroma/web/activity_pub/builder.ex

@@ -10,6 +10,18 @@ defmodule Pleroma.Web.ActivityPub.Builder do
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.ActivityPub.Visibility
 
+  @spec follow(User.t(), User.t()) :: {:ok, map(), keyword()}
+  def follow(follower, followed) do
+    {:ok,
+     %{
+       "id" => Utils.generate_activity_id(),
+       "actor" => follower.ap_id,
+       "type" => "Follow",
+       "object" => followed.ap_id,
+       "to" => [followed.ap_id]
+     }, []}
+  end
+
   @spec undo(User.t(), Activity.t()) :: {:ok, map(), keyword()}
   def undo(actor, object) do
     {:ok,

lib/pleroma/web/activity_pub/relay.ex

@@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.Relay do
   alias Pleroma.Object
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
+  alias Pleroma.Web.CommonAPI
   require Logger
 
   @relay_nickname "relay"
@@ -39,8 +40,7 @@ defmodule Pleroma.Web.ActivityPub.Relay do
   def unfollow(target_instance) do
     with %User{} = local_user <- get_actor(),
          {:ok, %User{} = target_user} <- User.get_or_fetch_by_ap_id(target_instance),
-         {:ok, activity} <- ActivityPub.unfollow(local_user, target_user) do
-      User.unfollow(local_user, target_user)
+         {:ok, activity} <- CommonAPI.unfollow(local_user, target_user) do
       Logger.info("relay: unfollowed instance: #{target_instance}: id=#{activity.data["id"]}")
       {:ok, activity}
     else

lib/pleroma/web/activity_pub/side_effects.ex

@@ -63,6 +63,18 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
   end
 
   def handle_undoing(
+        %{data: %{"type" => "Follow", "actor" => follower, "object" => followed}} = object
+      ) do
+    with %User{} = follower <- User.get_cached_by_ap_id(follower),
+         %User{} = followed <- User.get_cached_by_ap_id(followed),
+         {:ok, _, _} <- User.unfollow(follower, followed),
+         _ <- User.unsubscribe(follower, followed),
+         {:ok, _} <- Repo.delete(object) do
+      :ok
+    end
+  end
+
+  def handle_undoing(
         %{data: %{"type" => "Block", "actor" => blocker, "object" => blocked}} = object
       ) do
     with %User{} = blocker <- User.get_cached_by_ap_id(blocker),

lib/pleroma/web/activity_pub/transmogrifier.ex

@@ -666,6 +666,12 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
+  def handle_incoming(%{"type" => "Undo"} = data, _options) do
+    with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
+      {:ok, activity}
+    end
+  end
+
   def handle_incoming(
         %{
           "type" => "EmojiReact",
@@ -769,56 +775,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
   end
 
   def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => %{"type" => "Follow", "object" => followed},
-          "actor" => follower,
-          "id" => id
-        } = _data,
-        _options
-      ) do
-    with %User{local: true} = followed <- User.get_cached_by_ap_id(followed),
-         {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower),
-         {:ok, activity} <- ActivityPub.unfollow(follower, followed, id, false) do
-      User.unfollow(follower, followed)
-      {:ok, activity}
-    else
-      _e -> :error
-    end
-  end
-
-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => %{"type" => type}
-        } = data,
-        _options
-      )
-      when type in ["Like", "EmojiReact", "Announce", "Block"] do
-    with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do
-      {:ok, activity}
-    end
-  end
-
-  # For Undos that don't have the complete object attached, try to find it in our database.
-  def handle_incoming(
-        %{
-          "type" => "Undo",
-          "object" => object
-        } = activity,
-        options
-      )
-      when is_binary(object) do
-    with %Activity{data: data} <- Activity.get_by_ap_id(object) do
-      activity
-      |> Map.put("object", data)
-      |> handle_incoming(options)
-    else
-      _e -> :error
-    end
-  end
-
-  def handle_incoming(
         %{"type" => "Block", "object" => blocked, "actor" => blocker, "id" => id} = _data,
         _options
       ) do

lib/pleroma/web/activity_pub/utils.ex

@@ -614,18 +614,6 @@ defmodule Pleroma.Web.ActivityPub.Utils do
 
   defp take_announcements(_), do: []
 
-  #### Unfollow-related helpers
-
-  def make_unfollow_data(follower, followed, follow_activity, activity_id) do
-    %{
-      "type" => "Undo",
-      "actor" => follower.ap_id,
-      "to" => [followed.ap_id],
-      "object" => follow_activity.data
-    }
-    |> maybe_put("id", activity_id)
-  end
-
   #### Block-related helpers
   @spec fetch_latest_block(User.t(), User.t()) :: Activity.t() | nil
   def fetch_latest_block(%User{ap_id: blocker_id}, %User{ap_id: blocked_id}) do

lib/pleroma/web/common_api/common_api.ex

@@ -32,6 +32,28 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
+  def unfollow(follower, followed) do
+    with {_, %Activity{} = follow} <-
+           {:fetch_follow, Utils.fetch_latest_follow(follower, followed)},
+         {:ok, unfollow_data, _} <- Builder.undo(follower, follow),
+         {:ok, unfollow, _} <- Pipeline.common_pipeline(unfollow_data, local: true) do
+      {:ok, unfollow}
+    else
+      {:fetch_follow, nil} ->
+        Logger.warn(
+          "No follow activity found for #{follower.ap_id} and #{followed.ap_id}, inserting one and starting over"
+        )
+
+        with {:ok, follow_data, _} <- Builder.follow(follower, followed),
+             {:ok, _follow, _} <- ActivityPub.persist(follow_data, local: true) do
+          unfollow(follower, followed)
+        end
+
+      e ->
+        e
+    end
+  end
+
   def follow(follower, followed) do
     timeout = Pleroma.Config.get([:activitypub, :follow_handshake_timeout])
 
@@ -42,14 +64,6 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
-  def unfollow(follower, unfollowed) do
-    with {:ok, follower, _follow_activity} <- User.unfollow(follower, unfollowed),
-         {:ok, _activity} <- ActivityPub.unfollow(follower, unfollowed),
-         {:ok, _subscription} <- User.unsubscribe(follower, unfollowed) do
-      {:ok, follower}
-    end
-  end
-
   def accept_follow_request(follower, followed) do
     with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed),
          {:ok, follower} <- User.follow(follower, followed),

lib/pleroma/web/mastodon_api/controllers/account_controller.ex

@@ -321,7 +321,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   end
 
   def unfollow(%{assigns: %{user: follower, account: followed}} = conn, _params) do
-    with {:ok, follower} <- CommonAPI.unfollow(follower, followed) do
+    with {:ok, _unfollow} <- CommonAPI.unfollow(follower, followed) do
       render(conn, "relationship.json", user: follower, target: followed)
     end
   end

test/notification_test.exs

@@ -331,21 +331,6 @@ defmodule Pleroma.NotificationTest do
       assert %{type: "follow"} = NotificationView.render("show.json", render_opts)
     end
 
-    test "it doesn't create a notification for follow-unfollow-follow chains" do
-      user = insert(:user)
-      followed_user = insert(:user, locked: false)
-
-      {:ok, _, _, _activity} = CommonAPI.follow(user, followed_user)
-      assert FollowingRelationship.following?(user, followed_user)
-      assert [notification] = Notification.for_user(followed_user)
-
-      CommonAPI.unfollow(user, followed_user)
-      {:ok, _, _, _activity_dupe} = CommonAPI.follow(user, followed_user)
-
-      notification_id = notification.id
-      assert [%{id: ^notification_id}] = Notification.for_user(followed_user)
-    end
-
     test "dismisses the notification on follow request rejection" do
       user = insert(:user, locked: true)
       follower = insert(:user)

test/tasks/relay_test.exs

@@ -58,7 +58,7 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
       Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance])
 
       cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"])
-      assert cancelled_activity.data["state"] == "cancelled"
+      refute cancelled_activity
 
       [undo_activity] =
         ActivityPub.fetch_activities([], %{
@@ -70,7 +70,7 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
 
       assert undo_activity.data["type"] == "Undo"
       assert undo_activity.data["actor"] == local_user.ap_id
-      assert undo_activity.data["object"] == cancelled_activity.data
+      assert undo_activity.data["object"] == follow_activity.data["id"]
       refute "#{target_instance}/followers" in User.following(local_user)
     end
   end

test/user_test.exs

@@ -1149,8 +1149,8 @@ defmodule Pleroma.UserTest do
       {:ok, like_two} = CommonAPI.favorite(follower, activity.id)
       {:ok, repeat, _} = CommonAPI.repeat(activity_two.id, user)
 
-      {:ok, job} = User.delete(user)
-      {:ok, _user} = ObanHelpers.perform(job)
+      {:ok, _job} = User.delete(user)
+      ObanHelpers.perform_all()
 
       follower = User.get_cached_by_id(follower.id)
 

test/web/activity_pub/activity_pub_test.exs

@@ -1052,23 +1052,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert Repo.aggregate(Object, :count, :id) == 0
     end
 
-    test "it reverts unfollow activity" do
-      follower = insert(:user)
-      followed = insert(:user)
-
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
-
-      with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do
-        assert {:error, :reverted} = ActivityPub.unfollow(follower, followed)
-      end
-
-      activity = Activity.get_by_id(follow_activity.id)
-      assert activity.data["type"] == "Follow"
-      assert activity.data["actor"] == follower.ap_id
-
-      assert activity.data["object"] == followed.ap_id
-    end
-
     test "creates a follow activity" do
       follower = insert(:user)
       followed = insert(:user)
@@ -1078,40 +1061,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert activity.data["actor"] == follower.ap_id
       assert activity.data["object"] == followed.ap_id
     end
-
-    test "creates an undo activity for the last follow" do
-      follower = insert(:user)
-      followed = insert(:user)
-
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
-      {:ok, activity} = ActivityPub.unfollow(follower, followed)
-
-      assert activity.data["type"] == "Undo"
-      assert activity.data["actor"] == follower.ap_id
-
-      embedded_object = activity.data["object"]
-      assert is_map(embedded_object)
-      assert embedded_object["type"] == "Follow"
-      assert embedded_object["object"] == followed.ap_id
-      assert embedded_object["id"] == follow_activity.data["id"]
-    end
-
-    test "creates an undo activity for a pending follow request" do
-      follower = insert(:user)
-      followed = insert(:user, %{locked: true})
-
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
-      {:ok, activity} = ActivityPub.unfollow(follower, followed)
-
-      assert activity.data["type"] == "Undo"
-      assert activity.data["actor"] == follower.ap_id
-
-      embedded_object = activity.data["object"]
-      assert is_map(embedded_object)
-      assert embedded_object["type"] == "Follow"
-      assert embedded_object["object"] == followed.ap_id
-      assert embedded_object["id"] == follow_activity.data["id"]
-    end
   end
 
   describe "blocking" do

test/web/activity_pub/side_effects_test.exs

@@ -21,12 +21,15 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
     setup do
       poster = insert(:user)
       user = insert(:user)
+      followed = insert(:user)
       {:ok, post} = CommonAPI.post(poster, %{"status" => "hey"})
       {: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, user, followed, follow} = CommonAPI.follow(user, followed)
+      User.subscribe(user, followed)
 
       {:ok, undo_data, _meta} = Builder.undo(user, like)
       {:ok, like_undo, _meta} = ActivityPub.persist(undo_data, local: true)
@@ -40,6 +43,9 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
       {:ok, undo_data, _meta} = Builder.undo(user, block)
       {:ok, block_undo, _meta} = ActivityPub.persist(undo_data, local: true)
 
+      {:ok, undo_data, _meta} = Builder.undo(user, follow)
+      {:ok, follow_undo, _meta} = ActivityPub.persist(undo_data, local: true)
+
       %{
         like_undo: like_undo,
         post: post,
@@ -50,11 +56,31 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
         announce: announce,
         block_undo: block_undo,
         block: block,
+        follow_undo: follow_undo,
+        follow: follow,
         poster: poster,
-        user: user
+        user: user,
+        followed: followed
       }
     end
 
+    test "deletes the original follow", %{follow_undo: follow_undo, follow: follow} do
+      {:ok, _follow_undo, _} = SideEffects.handle(follow_undo)
+      refute Activity.get_by_id(follow.id)
+    end
+
+    test "unfollows and unsubscribes the followed user", %{
+      follow_undo: follow_undo,
+      follow: follow
+    } do
+      follower = User.get_by_ap_id(follow.data["actor"])
+      followed = User.get_by_ap_id(follow.data["object"])
+
+      {:ok, _follow_undo, _} = SideEffects.handle(follow_undo)
+      refute User.following?(follower, followed)
+      refute User.subscribed_to?(follower, followed)
+    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)

test/web/activity_pub/transmogrifier/undo_handling_test.exs

@@ -34,14 +34,15 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UndoHandlingTest do
 
   test "it returns an error for incoming unlikes wihout a like activity" do
     user = insert(:user)
-    {:ok, activity} = CommonAPI.post(user, %{"status" => "leave a like pls"})
+    {:ok, _activity} = CommonAPI.post(user, %{"status" => "leave a like pls"})
 
     data =
       File.read!("test/fixtures/mastodon-undo-like.json")
       |> Poison.decode!()
-      |> Map.put("object", activity.data["object"])
+      |> Map.put("object", user.ap_id <> "/activities/likes/1")
+      |> Map.put("actor", user.ap_id)
 
-    assert Transmogrifier.handle_incoming(data) == :error
+    assert {:error, _} = Transmogrifier.handle_incoming(data)
   end
 
   test "it works for incoming unlikes with an existing like activity" do
@@ -150,9 +151,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"] == "Follow"
-    assert data["object"]["object"] == user.ap_id
-    assert data["actor"] == "http://mastodon.example.org/users/admin"
+    assert data["object"] == follow_data["id"]
 
     refute User.following?(User.get_cached_by_ap_id(data["actor"]), user)
   end

test/web/common_api/common_api_test.exs

@@ -14,6 +14,7 @@ defmodule Pleroma.Web.CommonAPITest do
   alias Pleroma.Web.CommonAPI
 
   import Pleroma.Factory
+  import ExUnit.CaptureLog
 
   require Pleroma.Constants
 
@@ -601,6 +602,17 @@ defmodule Pleroma.Web.CommonAPITest do
   end
 
   describe "unfollow/2" do
+    test "still works if we don't have an existing follow activity" do
+      [follower, followed] = insert_pair(:user)
+
+      # No activity created here
+      {:ok, follower} = User.follow(follower, followed)
+
+      assert capture_log(fn ->
+               assert {:ok, _undo} = CommonAPI.unfollow(follower, followed)
+             end) =~ "No follow activity found"
+    end
+
     test "also unsubscribes a user" do
       [follower, followed] = insert_pair(:user)
       {:ok, follower, followed, _} = CommonAPI.follow(follower, followed)
@@ -608,7 +620,7 @@ defmodule Pleroma.Web.CommonAPITest do
 
       assert User.subscribed_to?(follower, followed)
 
-      {:ok, follower} = CommonAPI.unfollow(follower, followed)
+      {:ok, _undo} = CommonAPI.unfollow(follower, followed)
 
       refute User.subscribed_to?(follower, followed)
     end
@@ -621,18 +633,10 @@ defmodule Pleroma.Web.CommonAPITest do
                CommonAPI.follow(follower, followed)
 
       assert User.get_follow_state(follower, followed) == :follow_pending
-      assert {:ok, follower} = CommonAPI.unfollow(follower, followed)
+      assert {:ok, _undo} = CommonAPI.unfollow(follower, followed)
       assert User.get_follow_state(follower, followed) == nil
 
-      assert %{id: ^activity_id, data: %{"state" => "cancelled"}} =
-               Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
-
-      assert %{
-               data: %{
-                 "type" => "Undo",
-                 "object" => %{"type" => "Follow", "state" => "cancelled"}
-               }
-             } = Pleroma.Web.ActivityPub.Utils.fetch_latest_undo(follower)
+      refute Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
     end
 
     test "cancels a pending follow for a remote user" do
@@ -643,18 +647,10 @@ defmodule Pleroma.Web.CommonAPITest do
                CommonAPI.follow(follower, followed)
 
       assert User.get_follow_state(follower, followed) == :follow_pending
-      assert {:ok, follower} = CommonAPI.unfollow(follower, followed)
+      assert {:ok, _undo} = CommonAPI.unfollow(follower, followed)
       assert User.get_follow_state(follower, followed) == nil
 
-      assert %{id: ^activity_id, data: %{"state" => "cancelled"}} =
-               Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
-
-      assert %{
-               data: %{
-                 "type" => "Undo",
-                 "object" => %{"type" => "Follow", "state" => "cancelled"}
-               }
-             } = Pleroma.Web.ActivityPub.Utils.fetch_latest_undo(follower)
+      refute Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
     end
   end
 

test/web/mastodon_api/views/notification_view_test.exs

@@ -107,9 +107,10 @@ defmodule Pleroma.Web.MastodonAPI.NotificationViewTest do
     test_notifications_rendering([notification], followed, [expected])
 
     User.perform(:delete, follower)
-    notification = Notification |> Repo.one() |> Repo.preload(:activity)
 
-    test_notifications_rendering([notification], followed, [])
+    refute Notification
+           |> Repo.one()
+           |> Repo.preload(:activity)
   end
 
   @tag capture_log: true