From 62656ab259cec1a8585abecf45096b283fa4c60a Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Wed, 1 Apr 2020 07:47:07 +0200 Subject: [PATCH] User: Move inbox & shared_inbox to own fields --- lib/pleroma/user.ex | 8 +++ lib/pleroma/web/activity_pub/activity_pub.ex | 19 ++++++- lib/pleroma/web/activity_pub/publisher.ex | 13 +++-- .../20200401072456_users_add_inboxes.exs | 20 +++++++ test/web/activity_pub/publisher_test.exs | 52 ++++++------------- test/web/federator_test.exs | 4 +- 6 files changed, 70 insertions(+), 46 deletions(-) create mode 100644 priv/repo/migrations/20200401072456_users_add_inboxes.exs diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 027386a22..7d8f3a76b 100644 --- a/lib/pleroma/user.ex +++ b/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, diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 0e4a9d842..f0bbecc9b 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/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 diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 6c558e7f0..b70cbd043 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/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}), - do: (is_map(data["endpoints"]) && Map.get(data["endpoints"], "sharedInbox")) || data["inbox"] + defp maybe_use_sharedinbox(%User{shared_inbox: nil, inbox: inbox}), do: 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} = - Enum.find(recipients, fn %{source_data: data} -> data["inbox"] == inbox end) + %User{ap_id: ap_id} = Enum.find(recipients, fn actor -> actor.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. diff --git a/priv/repo/migrations/20200401072456_users_add_inboxes.exs b/priv/repo/migrations/20200401072456_users_add_inboxes.exs new file mode 100644 index 000000000..0947f0ab2 --- /dev/null +++ b/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 diff --git a/test/web/activity_pub/publisher_test.exs b/test/web/activity_pub/publisher_test.exs index 801da03c1..c2bc38d52 100644 --- a/test/web/activity_pub/publisher_test.exs +++ b/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 = - insert(:user, %{ - source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}} - }) + user = insert(:user, %{shared_inbox: "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 = - insert(:user, %{ - source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}} - }) + user = insert(:user, %{shared_inbox: "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 = - insert(:user, %{ - source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}} - }) - + user = insert(:user, %{shared_inbox: "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 = - insert(:user, %{ - source_data: %{"endpoints" => %{"sharedInbox" => "http://example.com/inbox"}} - }) - + user = insert(:user, %{shared_inbox: "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, - source_data: %{ - "inbox" => "http://example.com/personal-inbox", - "endpoints" => %{"sharedInbox" => "http://example.com/inbox"} - } - ) + insert(:user, %{ + shared_inbox: "http://example.com/inbox", + inbox: "http://example.com/personal-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, - source_data: %{ - "inbox" => "http://example.com/personal-inbox", - "endpoints" => %{"sharedInbox" => "http://example.com/inbox"} - } - ) + insert(:user, %{ + shared_inbox: "http://example.com/inbox", + inbox: "http://example.com/personal-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 ) diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index da844c24c..59e53bb03 100644 --- a/test/web/federator_test.exs +++ b/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 })