Merge pull request 'Federation: always prefer the shared inbox' (#8029) from lambadalambda/pleroma:perf/shared-inbox into develop

Reviewed-on: https://git.pleroma.social/pleroma/pleroma/pulls/8029

changelog.d/shared-inbox.change

@@ -0,0 +1 @@
+Deliver to shared inboxes whenever possible, including followers-only posts and DMs, so each server gets one copy

lib/pleroma/web/activity_pub/publisher.ex

@@ -281,36 +281,18 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
   defp maybe_use_sharedinbox(%User{shared_inbox: shared_inbox}), do: shared_inbox
 
   @doc """
-  Determine a user inbox to use based on heuristics.  These heuristics
+  Determines the inbox to deliver an activity to.
-  are based on an approximation of the ``sharedInbox`` rules in the
-  [ActivityPub specification][ap-sharedinbox].
 
-  Please do not edit this function (or its children) without reading
+  Like Mastodon, GoToSocial and Akkoma, this always prefers the shared inbox,
-  the spec, as editing the code is likely to introduce some breakage
+  which saves duplicate deliveries to the same server. Only activities with
-  without some familiarity.
+  `bcc` or `bto` go to personal inboxes, because the receiver rebuilds that
-
+  addressing from the inbox the activity arrived at.
-     [ap-sharedinbox]: https://www.w3.org/TR/activitypub/#shared-inbox-delivery
   """
-  def determine_inbox(
+  def determine_inbox(%Activity{data: activity_data}, %User{inbox: inbox} = user) do
-        %Activity{data: activity_data},
+    if Enum.empty?(activity_data["bcc"] || []) and Enum.empty?(activity_data["bto"] || []) do
-        %User{inbox: inbox} = user
+      maybe_use_sharedinbox(user)
-      ) do
+    else
-    to = activity_data["to"] || []
+      inbox
-    cc = activity_data["cc"] || []
-    type = activity_data["type"]
-
-    cond do
-      type == "Delete" ->
-        maybe_use_sharedinbox(user)
-
-      Pleroma.Constants.as_public() in to || Pleroma.Constants.as_public() in cc ->
-        maybe_use_sharedinbox(user)
-
-      length(to) + length(cc) > 1 ->
-        maybe_use_sharedinbox(user)
-
-      true ->
-        inbox
     end
   end
 

test/pleroma/web/activity_pub/publisher_test.exs

@@ -137,7 +137,7 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
       assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox"
     end
 
-    test "it returns inbox for messages involving single recipients in total" do
+    test "it returns sharedInbox for messages to a single recipient" do
       user =
         insert(:user, %{
           shared_inbox: "http://example.com/inbox",
@@ -148,6 +148,46 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
         data: %{"to" => [user.ap_id], "cc" => []}
       }
 
+      assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox"
+    end
+
+    test "it returns sharedInbox for followers-only messages" do
+      user =
+        insert(:user, %{
+          shared_inbox: "http://example.com/inbox",
+          inbox: "http://example.com/personal-inbox"
+        })
+
+      author = insert(:user)
+
+      activity = %Activity{
+        data: %{"to" => [author.follower_address], "cc" => []}
+      }
+
+      assert Publisher.determine_inbox(activity, user) == "http://example.com/inbox"
+    end
+
+    test "it returns inbox for messages with bcc or bto, which needs personal inboxes" do
+      user =
+        insert(:user, %{
+          shared_inbox: "http://example.com/inbox",
+          inbox: "http://example.com/personal-inbox"
+        })
+
+      for field <- ["bcc", "bto"] do
+        activity = %Activity{
+          data: %{"to" => [@as_public], "cc" => [], field => [user.ap_id]}
+        }
+
+        assert Publisher.determine_inbox(activity, user) == "http://example.com/personal-inbox"
+      end
+    end
+
+    test "it returns inbox when the user has no sharedInbox" do
+      user = insert(:user, %{shared_inbox: nil, inbox: "http://example.com/personal-inbox"})
+
+      activity = %Activity{data: %{"to" => [@as_public], "cc" => []}}
+
       assert Publisher.determine_inbox(activity, user) == "http://example.com/personal-inbox"
     end
   end