ActivityPub: Remove `follow` and fix issues.
authorlain <lain@soykaf.club>
Wed, 8 Jul 2020 15:07:24 +0000 (17:07 +0200)
committerlain <lain@soykaf.club>
Wed, 8 Jul 2020 15:07:24 +0000 (17:07 +0200)
lib/pleroma/user.ex
lib/pleroma/web/activity_pub/activity_pub.ex
lib/pleroma/web/activity_pub/relay.ex
test/tasks/relay_test.exs
test/web/activity_pub/activity_pub_test.exs
test/web/activity_pub/relay_test.exs
test/web/activity_pub/transmogrifier_test.exs
test/web/activity_pub/utils_test.exs
test/web/common_api/common_api_test.exs
test/web/mastodon_api/controllers/follow_request_controller_test.exs

index e98332744010433cecf68962182e2fe05be66016..9d1314f8126b639ae7585936629d8bf10e435654 100644 (file)
@@ -1543,7 +1543,7 @@ defmodule Pleroma.User do
       fn followed_identifier ->
         with {:ok, %User{} = followed} <- get_or_fetch(followed_identifier),
              {:ok, follower} <- maybe_direct_follow(follower, followed),
-             {:ok, _} <- ActivityPub.follow(follower, followed) do
+             {:ok, _, _, _} <- CommonAPI.follow(follower, followed) do
           followed
         else
           err ->
index 8abbef487fb0b70d6a0d1da465b64392a0c4cf32..1c29088059233c65b83c812edfece12ce2e65115 100644 (file)
@@ -322,28 +322,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  @spec follow(User.t(), User.t(), String.t() | nil, boolean(), keyword()) ::
-          {:ok, Activity.t()} | {:error, any()}
-  def follow(follower, followed, activity_id \\ nil, local \\ true, opts \\ []) do
-    with {:ok, result} <-
-           Repo.transaction(fn -> do_follow(follower, followed, activity_id, local, opts) end) do
-      result
-    end
-  end
-
-  defp do_follow(follower, followed, activity_id, local, opts) do
-    skip_notify_and_stream = Keyword.get(opts, :skip_notify_and_stream, false)
-    data = make_follow_data(follower, followed, activity_id)
-
-    with {:ok, activity} <- insert(data, local),
-         _ <- skip_notify_and_stream || notify_and_stream(activity),
-         :ok <- maybe_federate(activity) do
-      {:ok, activity}
-    else
-      {:error, error} -> Repo.rollback(error)
-    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
index 484178edd9e266098678dff3af0d8585309a7b36..b09764d2bf647e5141bed9de5eb4d87b44a53bc5 100644 (file)
@@ -28,7 +28,7 @@ defmodule Pleroma.Web.ActivityPub.Relay do
   def follow(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.follow(local_user, target_user) do
+         {:ok, _, _, activity} <- CommonAPI.follow(local_user, target_user) do
       Logger.info("relay: followed instance: #{target_instance}; id=#{activity.data["id"]}")
       {:ok, activity}
     else
index a8ba0658d0d7d8abf46b80ac0c070dbe0b9da477..79ab72002fb4229a14ac127796d3a406fe235213 100644 (file)
@@ -10,6 +10,8 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
   alias Pleroma.Web.ActivityPub.Utils
   use Pleroma.DataCase
 
+  import Pleroma.Factory
+
   setup_all do
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
 
@@ -46,7 +48,8 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
 
   describe "running unfollow" do
     test "relay is unfollowed" do
-      target_instance = "http://mastodon.example.org/users/admin"
+      user = insert(:user)
+      target_instance = user.ap_id
 
       Mix.Tasks.Pleroma.Relay.run(["follow", target_instance])
 
@@ -71,7 +74,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"]["id"] == cancelled_activity.data["id"]
       refute "#{target_instance}/followers" in User.following(local_user)
     end
   end
index 17e12a1a72aec61eaa11c207f570b77c71f3cecd..38c98f658004ed8e35655890b28d47e921e88cd2 100644 (file)
@@ -669,7 +669,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
     refute activity in activities
 
     followed_user = insert(:user)
-    ActivityPub.follow(user, followed_user)
+    CommonAPI.follow(user, followed_user)
     {:ok, repeat_activity} = CommonAPI.repeat(activity.id, followed_user)
 
     activities = ActivityPub.fetch_activities([], %{blocking_user: user, skip_preload: true})
@@ -1013,24 +1013,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
     end
   end
 
-  describe "following / unfollowing" do
-    test "it reverts follow activity" do
-      follower = insert(:user)
-      followed = insert(:user)
-
-      with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do
-        assert {:error, :reverted} = ActivityPub.follow(follower, followed)
-      end
-
-      assert Repo.aggregate(Activity, :count, :id) == 0
-      assert Repo.aggregate(Object, :count, :id) == 0
-    end
-
+  describe "unfollowing" do
     test "it reverts unfollow activity" do
       follower = insert(:user)
       followed = insert(:user)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
 
       with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do
         assert {:error, :reverted} = ActivityPub.unfollow(follower, followed)
@@ -1043,21 +1031,11 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert activity.data["object"] == followed.ap_id
     end
 
-    test "creates a follow activity" do
-      follower = insert(:user)
-      followed = insert(:user)
-
-      {:ok, activity} = ActivityPub.follow(follower, followed)
-      assert activity.data["type"] == "Follow"
-      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, _, _, follow_activity} = CommonAPI.follow(follower, followed)
       {:ok, activity} = ActivityPub.unfollow(follower, followed)
 
       assert activity.data["type"] == "Undo"
@@ -1074,7 +1052,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       follower = insert(:user)
       followed = insert(:user, %{locked: true})
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
       {:ok, activity} = ActivityPub.unfollow(follower, followed)
 
       assert activity.data["type"] == "Undo"
index b3b573c9b87632dfae25af71955f00f3357427ac..9d657ac4fcf84fdbe464b503e01451baeae7f005 100644 (file)
@@ -7,8 +7,8 @@ defmodule Pleroma.Web.ActivityPub.RelayTest do
 
   alias Pleroma.Activity
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Relay
+  alias Pleroma.Web.CommonAPI
 
   import ExUnit.CaptureLog
   import Pleroma.Factory
@@ -53,8 +53,7 @@ defmodule Pleroma.Web.ActivityPub.RelayTest do
     test "returns activity" do
       user = insert(:user)
       service_actor = Relay.get_actor()
-      ActivityPub.follow(service_actor, user)
-      Pleroma.User.follow(service_actor, user)
+      CommonAPI.follow(service_actor, user)
       assert "#{user.ap_id}/followers" in User.following(service_actor)
       assert {:ok, %Activity{} = activity} = Relay.unfollow(user.ap_id)
       assert activity.actor == "#{Pleroma.Web.Endpoint.url()}/relay"
@@ -74,6 +73,7 @@ defmodule Pleroma.Web.ActivityPub.RelayTest do
       assert Relay.publish(activity) == {:error, "Not implemented"}
     end
 
+    @tag capture_log: true
     test "returns error when activity not public" do
       activity = insert(:direct_note_activity)
       assert Relay.publish(activity) == {:error, false}
index 01179206c0e8001f48a698f34d99e65c022f4ad3..f7b7d1a9f2b89063495a584705ff0293da8a495e 100644 (file)
@@ -11,7 +11,6 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
   alias Pleroma.Object.Fetcher
   alias Pleroma.Tests.ObanHelpers
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.AdminAPI.AccountView
   alias Pleroma.Web.CommonAPI
@@ -452,7 +451,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       {:ok, follower} = User.follow(follower, followed)
       assert User.following?(follower, followed) == true
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
 
       accept_data =
         File.read!("test/fixtures/mastodon-accept-activity.json")
@@ -482,7 +481,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       follower = insert(:user)
       followed = insert(:user, locked: true)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
 
       accept_data =
         File.read!("test/fixtures/mastodon-accept-activity.json")
@@ -504,7 +503,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       follower = insert(:user)
       followed = insert(:user, locked: true)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
 
       accept_data =
         File.read!("test/fixtures/mastodon-accept-activity.json")
@@ -569,7 +568,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       followed = insert(:user, locked: true)
 
       {:ok, follower} = User.follow(follower, followed)
-      {:ok, _follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, _follow_activity} = CommonAPI.follow(follower, followed)
 
       assert User.following?(follower, followed) == true
 
@@ -595,7 +594,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       followed = insert(:user, locked: true)
 
       {:ok, follower} = User.follow(follower, followed)
-      {:ok, follow_activity} = ActivityPub.follow(follower, followed)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
 
       assert User.following?(follower, followed) == true
 
index 2f9ecb5a3220873fa0c8a4c9cdf43c4822cd195a..361dc5a41e3da9ca631630fad2d718dd173f2f0e 100644 (file)
@@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
   alias Pleroma.Object
   alias Pleroma.Repo
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.AdminAPI.AccountView
   alias Pleroma.Web.CommonAPI
@@ -197,8 +196,8 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
       user = insert(:user, locked: true)
       follower = insert(:user)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_two} = CommonAPI.follow(follower, user)
 
       data =
         follow_activity_two.data
@@ -221,8 +220,8 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
       user = insert(:user, locked: true)
       follower = insert(:user)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_two} = CommonAPI.follow(follower, user)
 
       data =
         follow_activity_two.data
index 908ee5484aef2e42557ff40acfdbe4fddf33155f..7e11fede3d8247563bfe38a7ae7bbc94a56971f3 100644 (file)
@@ -934,6 +934,15 @@ defmodule Pleroma.Web.CommonAPITest do
     end
   end
 
+  describe "follow/2" do
+    test "directly follows a non-locked local user" do
+      [follower, followed] = insert_pair(:user)
+      {:ok, follower, followed, _} = CommonAPI.follow(follower, followed)
+
+      assert User.following?(follower, followed)
+    end
+  end
+
   describe "unfollow/2" do
     test "also unsubscribes a user" do
       [follower, followed] = insert_pair(:user)
@@ -998,9 +1007,9 @@ defmodule Pleroma.Web.CommonAPITest do
       follower = insert(:user)
       follower_two = insert(:user)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_two} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_three} = CommonAPI.follow(follower_two, user)
 
       assert follow_activity.data["state"] == "pending"
       assert follow_activity_two.data["state"] == "pending"
@@ -1018,9 +1027,9 @@ defmodule Pleroma.Web.CommonAPITest do
       follower = insert(:user)
       follower_two = insert(:user)
 
-      {:ok, follow_activity} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_two} = ActivityPub.follow(follower, user)
-      {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user)
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_two} = CommonAPI.follow(follower, user)
+      {:ok, _, _, follow_activity_three} = CommonAPI.follow(follower_two, user)
 
       assert follow_activity.data["state"] == "pending"
       assert follow_activity_two.data["state"] == "pending"
index 44e12d15a99f206ef54eda83013dfe4006eae605..6749e0e83a480dcf843b2348735cc2b3bc9e6a4c 100644 (file)
@@ -6,7 +6,7 @@ defmodule Pleroma.Web.MastodonAPI.FollowRequestControllerTest do
   use Pleroma.Web.ConnCase
 
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
+  alias Pleroma.Web.CommonAPI
 
   import Pleroma.Factory
 
@@ -20,7 +20,7 @@ defmodule Pleroma.Web.MastodonAPI.FollowRequestControllerTest do
     test "/api/v1/follow_requests works", %{user: user, conn: conn} do
       other_user = insert(:user)
 
-      {:ok, _activity} = ActivityPub.follow(other_user, user)
+      {:ok, _, _, _activity} = CommonAPI.follow(other_user, user)
       {:ok, other_user} = User.follow(other_user, user, :follow_pending)
 
       assert User.following?(other_user, user) == false
@@ -34,7 +34,7 @@ defmodule Pleroma.Web.MastodonAPI.FollowRequestControllerTest do
     test "/api/v1/follow_requests/:id/authorize works", %{user: user, conn: conn} do
       other_user = insert(:user)
 
-      {:ok, _activity} = ActivityPub.follow(other_user, user)
+      {:ok, _, _, _activity} = CommonAPI.follow(other_user, user)
       {:ok, other_user} = User.follow(other_user, user, :follow_pending)
 
       user = User.get_cached_by_id(user.id)
@@ -56,7 +56,7 @@ defmodule Pleroma.Web.MastodonAPI.FollowRequestControllerTest do
     test "/api/v1/follow_requests/:id/reject works", %{user: user, conn: conn} do
       other_user = insert(:user)
 
-      {:ok, _activity} = ActivityPub.follow(other_user, user)
+      {:ok, _, _, _activity} = CommonAPI.follow(other_user, user)
 
       user = User.get_cached_by_id(user.id)