User: Move inbox & shared_inbox to own fields

lib/pleroma/user.ex

@@ -134,6 +134,8 @@ defmodule Pleroma.User do
     field(:skip_thread_containment, :boolean, default: false)
     field(:actor_type, :string, default: "Person")
     field(:also_known_as, {:array, :string}, default: [])
+    field(:inbox, :string)
+    field(:shared_inbox, :string)
 
     embeds_one(
       :notification_settings,
@@ -367,6 +369,8 @@ def remote_user_creation(params) do
         :bio,
         :name,
         :ap_id,
+        :inbox,
+        :shared_inbox,
         :nickname,
         :public_key,
         :avatar,
@@ -411,6 +415,8 @@ def update_changeset(struct, params \\ %{}) do
         :name,
         :avatar,
         :public_key,
+        :inbox,
+        :shared_inbox,
         :locked,
         :no_rich_text,
         :default_scope,
@@ -508,6 +514,8 @@ def upgrade_changeset(struct, params \\ %{}, remote? \\ false) do
         :follower_address,
         :following_address,
         :public_key,
+        :inbox,
+        :shared_inbox,
         :avatar,
         :last_refreshed_at,
         :ap_enabled,

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -1432,7 +1432,20 @@ defp object_to_user_data(data) do
     discoverable = data["discoverable"] || false
     invisible = data["invisible"] || false
     actor_type = data["type"] || "Person"
-    public_key = data["publicKey"]["publicKeyPem"]
+
+    public_key =
+      if is_map(data["publicKey"]) && is_binary(data["publicKey"]["publicKeyPem"]) do
+        data["publicKey"]["publicKeyPem"]
+      else
+        nil
+      end
+
+    shared_inbox =
+      if is_map(data["endpoints"]) && is_binary(data["endpoints"]["sharedInbox"]) do
+        data["endpoints"]["sharedInbox"]
+      else
+        nil
+      end
 
     user_data = %{
       ap_id: data["id"],
@@ -1451,7 +1464,9 @@ defp object_to_user_data(data) do
       bio: data["summary"],
       actor_type: actor_type,
       also_known_as: Map.get(data, "alsoKnownAs", []),
-      public_key: public_key
+      public_key: public_key,
+      inbox: data["inbox"],
+      shared_inbox: shared_inbox
     }
 
     # nickname can be nil because of virtual actors

lib/pleroma/web/activity_pub/publisher.ex

@@ -141,8 +141,8 @@ defp get_cc_ap_ids(ap_id, recipients) do
     |> Enum.map(& &1.ap_id)
   end
 
-  defp maybe_use_sharedinbox(%User{source_data: data}),
+  defp maybe_use_sharedinbox(%User{shared_inbox: nil, inbox: inbox}), do: inbox
-    do: (is_map(data["endpoints"]) && Map.get(data["endpoints"], "sharedInbox")) || data["inbox"]
+  defp maybe_use_sharedinbox(%User{shared_inbox: shared_inbox}), do: shared_inbox
 
   @doc """
   Determine a user inbox to use based on heuristics.  These heuristics
@@ -157,7 +157,7 @@ defp maybe_use_sharedinbox(%User{source_data: data}),
   """
   def determine_inbox(
         %Activity{data: activity_data},
-        %User{source_data: data} = user
+        %User{inbox: inbox} = user
       ) do
     to = activity_data["to"] || []
     cc = activity_data["cc"] || []
@@ -174,7 +174,7 @@ def determine_inbox(
         maybe_use_sharedinbox(user)
 
       true ->
-        data["inbox"]
+        inbox
     end
   end
 
@@ -192,14 +192,13 @@ def publish(%User{} = actor, %{data: %{"bcc" => bcc}} = activity)
     inboxes =
       recipients
       |> Enum.filter(&User.ap_enabled?/1)
-      |> Enum.map(fn %{source_data: data} -> data["inbox"] end)
+      |> Enum.map(fn actor -> actor.inbox end)
       |> Enum.filter(fn inbox -> should_federate?(inbox, public) end)
       |> Instances.filter_reachable()
 
     Repo.checkout(fn ->
       Enum.each(inboxes, fn {inbox, unreachable_since} ->
-        %User{ap_id: ap_id} =
+        %User{ap_id: ap_id} = Enum.find(recipients, fn actor -> actor.inbox == inbox end)
-          Enum.find(recipients, fn %{source_data: data} -> data["inbox"] == inbox end)
 
         # Get all the recipients on the same host and add them to cc. Otherwise, a remote
         # instance would only accept a first message for the first recipient and ignore the rest.

priv/repo/migrations/20200401072456_users_add_inboxes.exs

@@ -0,0 +1,20 @@
+defmodule Pleroma.Repo.Migrations.UsersAddInboxes do
+  use Ecto.Migration
+
+  def up do
+    alter table(:users) do
+      add_if_not_exists(:inbox, :text)
+      add_if_not_exists(:shared_inbox, :text)
+    end
+
+    execute("UPDATE users SET inbox = source_data->>'inbox'")
+    execute("UPDATE users SET shared_inbox = source_data->'endpoints'->>'sharedInbox'")
+  end
+
+  def down do
+    alter table(:users) do
+      remove_if_exists(:inbox, :text)
+      remove_if_exists(:shared_inbox, :text)
+    end
+  end
+end

test/web/activity_pub/publisher_test.exs

@@ -48,10 +48,7 @@ test "it returns links" do
 
   describe "determine_inbox/2" do
     test "it returns sharedInbox for messages involving as:Public in to" do
-      user =
+      user = insert(:user, %{shared_inbox: "http://example.com/inbox"})
-        insert(:user, %{
-          source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}}
-        })
 
       activity = %Activity{
         data: %{"to" => [@as_public], "cc" => [user.follower_address]}
@@ -61,10 +58,7 @@ test "it returns sharedInbox for messages involving as:Public in to" do
     end
 
     test "it returns sharedInbox for messages involving as:Public in cc" do
-      user =
+      user = insert(:user, %{shared_inbox: "http://example.com/inbox"})
-        insert(:user, %{
-          source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}}
-        })
 
       activity = %Activity{
         data: %{"cc" => [@as_public], "to" => [user.follower_address]}
@@ -74,11 +68,7 @@ test "it returns sharedInbox for messages involving as:Public in cc" do
     end
 
     test "it returns sharedInbox for messages involving multiple recipients in to" do
-      user =
+      user = insert(:user, %{shared_inbox: "http://example.com/inbox"})
-        insert(:user, %{
-          source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}}
-        })
-
       user_two = insert(:user)
       user_three = insert(:user)
 
@@ -90,11 +80,7 @@ test "it returns sharedInbox for messages involving multiple recipients in to" d
     end
 
     test "it returns sharedInbox for messages involving multiple recipients in cc" do
-      user =
+      user = insert(:user, %{shared_inbox: "http://example.com/inbox"})
-        insert(:user, %{
-          source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}}
-        })
-
       user_two = insert(:user)
       user_three = insert(:user)
 
@@ -107,12 +93,10 @@ test "it returns sharedInbox for messages involving multiple recipients in cc" d
 
     test "it returns sharedInbox for messages involving multiple recipients in total" do
       user =
-        insert(:user,
+        insert(:user, %{
-          source_data: %{
+          shared_inbox: "http://example.com/inbox",
-            "inbox" => "http://example.com/personal-inbox",
+          inbox: "http://example.com/personal-inbox"
-            "endpoints" => %{"sharedInbox" => "http://example.com/inbox"}
+        })
-          }
-        )
 
       user_two = insert(:user)
 
@@ -125,12 +109,10 @@ test "it returns sharedInbox for messages involving multiple recipients in total
 
     test "it returns inbox for messages involving single recipients in total" do
       user =
-        insert(:user,
+        insert(:user, %{
-          source_data: %{
+          shared_inbox: "http://example.com/inbox",
-            "inbox" => "http://example.com/personal-inbox",
+          inbox: "http://example.com/personal-inbox"
-            "endpoints" => %{"sharedInbox" => "http://example.com/inbox"}
+        })
-          }
-        )
 
       activity = %Activity{
         data: %{"to" => [user.ap_id], "cc" => []}
@@ -258,11 +240,11 @@ test "it returns inbox for messages involving single recipients in total" do
                    [:passthrough],
                    [] do
       follower =
-        insert(:user,
+        insert(:user, %{
           local: false,
-          source_data: %{"inbox" => "https://domain.com/users/nick1/inbox"},
+          inbox: "https://domain.com/users/nick1/inbox",
           ap_enabled: true
-        )
+        })
 
       actor = insert(:user, follower_address: follower.ap_id)
       user = insert(:user)
@@ -295,14 +277,14 @@ test "it returns inbox for messages involving single recipients in total" do
       fetcher =
         insert(:user,
           local: false,
-          source_data: %{"inbox" => "https://domain.com/users/nick1/inbox"},
+          inbox: "https://domain.com/users/nick1/inbox",
           ap_enabled: true
         )
 
       another_fetcher =
         insert(:user,
           local: false,
-          source_data: %{"inbox" => "https://domain2.com/users/nick1/inbox"},
+          inbox: "https://domain2.com/users/nick1/inbox",
           ap_enabled: true
         )
 

test/web/federator_test.exs

@@ -78,7 +78,7 @@ test "it federates only to reachable instances via AP" do
         local: false,
         nickname: "nick1@domain.com",
         ap_id: "https://domain.com/users/nick1",
-        source_data: %{"inbox" => inbox1},
+        inbox: inbox1,
         ap_enabled: true
       })
 
@@ -86,7 +86,7 @@ test "it federates only to reachable instances via AP" do
         local: false,
         nickname: "nick2@domain2.com",
         ap_id: "https://domain2.com/users/nick2",
-        source_data: %{"inbox" => inbox2},
+        inbox: inbox2,
         ap_enabled: true
       })