From 62bccddde02edbf825c1806805cc9d7d7c9a0f13 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Fri, 22 Mar 2019 23:34:47 +0000 Subject: [PATCH 01/25] object: add support for preloading objects when walking an activity graph in normal form --- lib/pleroma/activity.ex | 62 ++++++++++++++++++++++++++++++++++++++++++++++++- lib/pleroma/object.ex | 25 ++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 04c3bb673..2f91b3c77 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Activity do alias Pleroma.Activity alias Pleroma.Notification + alias Pleroma.Object alias Pleroma.Repo import Ecto.Query @@ -33,6 +34,18 @@ defmodule Pleroma.Activity do field(:recipients, {:array, :string}) has_many(:notifications, Notification, on_delete: :delete_all) + # Attention: this is a fake relation, don't try to preload it blindly and expect it to work! + # The foreign key is embedded in a jsonb field. + # + # To use it, you probably want to do an inner join and a preload: + # + # ``` + # |> join(:inner, [activity], o in Object, + # fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", o.data, activity.data)) + # |> preload([activity, object], [object: object]) + # ``` + has_one(:object, Object, on_delete: :nothing, foreign_key: :id) + timestamps() end @@ -49,6 +62,21 @@ defmodule Pleroma.Activity do Repo.get(Activity, id) end + def get_by_id_with_object(id) do + from(activity in Activity, + where: activity.id == ^id, + inner_join: o in Object, + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + o.data, + activity.data + ), + preload: [object: o] + ) + |> Repo.one() + end + def by_object_ap_id(ap_id) do from( activity in Activity, @@ -76,7 +104,7 @@ defmodule Pleroma.Activity do ) end - def create_by_object_ap_id(ap_id) do + def create_by_object_ap_id(ap_id) when is_binary(ap_id) do from( activity in Activity, where: @@ -90,6 +118,8 @@ defmodule Pleroma.Activity do ) end + def create_by_object_ap_id(_), do: nil + def get_all_create_by_object_ap_id(ap_id) do Repo.all(create_by_object_ap_id(ap_id)) end @@ -101,6 +131,36 @@ defmodule Pleroma.Activity do def get_create_by_object_ap_id(_), do: nil + def create_by_object_ap_id_with_object(ap_id) when is_binary(ap_id) do + from( + activity in Activity, + where: + fragment( + "coalesce((?)->'object'->>'id', (?)->>'object') = ?", + activity.data, + activity.data, + ^to_string(ap_id) + ), + where: fragment("(?)->>'type' = 'Create'", activity.data), + inner_join: o in Object, + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + o.data, + activity.data + ), + preload: [object: o] + ) + end + + def create_by_object_ap_id_with_object(_), do: nil + + def get_create_by_object_ap_id_with_object(ap_id) do + ap_id + |> create_by_object_ap_id_with_object() + |> Repo.one() + end + def normalize(obj) when is_map(obj), do: Activity.get_by_ap_id(obj["id"]) def normalize(ap_id) when is_binary(ap_id), do: Activity.get_by_ap_id(ap_id) def normalize(_), do: nil diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 58e46ef1d..0f5c532ec 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -14,6 +14,8 @@ defmodule Pleroma.Object do import Ecto.Query import Ecto.Changeset + require Logger + schema "objects" do field(:data, :map) @@ -38,6 +40,29 @@ defmodule Pleroma.Object do Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id))) end + # If we pass an Activity to Object.normalize(), we can try to use the preloaded object. + # Use this whenever possible, especially when walking graphs in an O(N) loop! + def normalize(%Activity{object: %Object{} = object}), do: object + + # Catch and log Object.normalize() calls where the Activity's child object is not + # preloaded. + def normalize(%Activity{data: %{"object" => %{"id" => ap_id}}}) do + Logger.info( + "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" + ) + + normalize(ap_id) + end + + def normalize(%Activity{data: %{"object" => ap_id}}) do + Logger.info( + "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" + ) + + normalize(ap_id) + end + + # Old way, try fetching the object through cache. def normalize(%{"id" => ap_id}), do: normalize(ap_id) def normalize(ap_id) when is_binary(ap_id), do: get_cached_by_ap_id(ap_id) def normalize(_), do: nil From 092cedede507b3e89134deb6f5d09c5db339dfae Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:09:56 +0000 Subject: [PATCH 02/25] activity: add with_preloaded_object() convenience --- lib/pleroma/activity.ex | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 2f91b3c77..370721070 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -44,11 +44,29 @@ defmodule Pleroma.Activity do # fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", o.data, activity.data)) # |> preload([activity, object], [object: object]) # ``` + # + # As a convenience, Activity.with_preloaded_object() sets up an inner join and preload for the + # typical case. has_one(:object, Object, on_delete: :nothing, foreign_key: :id) timestamps() end + def with_preloaded_object(query) do + query + |> join( + :inner, + [activity], + o in Object, + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + o.data, + activity.data + ) + ) + |> preload([activity, object], object: object) + end + def get_by_ap_id(ap_id) do Repo.one( from( From 9aea7cc224c09e37a9a46277a9fbfb0af383fc0e Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:10:17 +0000 Subject: [PATCH 03/25] activitypub: preload child objects when fetching timelines --- lib/pleroma/web/activity_pub/activity_pub.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 2470b4a71..7d38a46e5 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -716,6 +716,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do limit: 20, order_by: [fragment("? desc nulls last", activity.id)] ) + |> Activity.with_preloaded_object() base_query |> restrict_recipients(recipients, opts["user"]) From e75e43b949ab6f6fb5751eeff10644d04259c149 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:22:14 +0000 Subject: [PATCH 04/25] common api: use the optimized Object.normalize whenever possible --- lib/pleroma/web/common_api/common_api.ex | 13 ++++++------- lib/pleroma/web/common_api/utils.ex | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 50d60aade..8625f6621 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -6,7 +6,6 @@ defmodule Pleroma.Web.CommonAPI do alias Pleroma.Activity alias Pleroma.Formatter alias Pleroma.Object - alias Pleroma.Repo alias Pleroma.ThreadMute alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub @@ -64,8 +63,8 @@ defmodule Pleroma.Web.CommonAPI do end def delete(activity_id, user) do - with %Activity{data: %{"object" => %{"id" => object_id}}} <- Repo.get(Activity, activity_id), - %Object{} = object <- Object.normalize(object_id), + with %Activity{data: %{"object" => _}} = activity <- Activity.get_by_id_with_object(activity_id), + %Object{} = object <- Object.normalize(activity), true <- User.superuser?(user) || user.ap_id == object.data["actor"], {:ok, _} <- unpin(activity_id, user), {:ok, delete} <- ActivityPub.delete(object) do @@ -75,7 +74,7 @@ defmodule Pleroma.Web.CommonAPI do def repeat(id_or_ap_id, user) do with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), - object <- Object.normalize(activity.data["object"]["id"]), + object <- Object.normalize(activity), nil <- Utils.get_existing_announce(user.ap_id, object) do ActivityPub.announce(user, object) else @@ -86,7 +85,7 @@ defmodule Pleroma.Web.CommonAPI do def unrepeat(id_or_ap_id, user) do with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), - object <- Object.normalize(activity.data["object"]["id"]) do + object <- Object.normalize(activity) do ActivityPub.unannounce(user, object) else _ -> @@ -96,7 +95,7 @@ defmodule Pleroma.Web.CommonAPI do def favorite(id_or_ap_id, user) do with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), - object <- Object.normalize(activity.data["object"]["id"]), + object <- Object.normalize(activity), nil <- Utils.get_existing_like(user.ap_id, object) do ActivityPub.like(user, object) else @@ -107,7 +106,7 @@ defmodule Pleroma.Web.CommonAPI do def unfavorite(id_or_ap_id, user) do with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), - object <- Object.normalize(activity.data["object"]["id"]) do + object <- Object.normalize(activity) do ActivityPub.unlike(user, object) else _ -> diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 3e807a5b7..6a94c2d26 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -17,13 +17,13 @@ defmodule Pleroma.Web.CommonAPI.Utils do # This is a hack for twidere. def get_by_id_or_ap_id(id) do - activity = Repo.get(Activity, id) || Activity.get_create_by_object_ap_id(id) + activity = Activity.get_by_id_with_object(id) || Activity.get_create_by_object_ap_id_with_object(id) activity && if activity.data["type"] == "Create" do activity else - Activity.get_create_by_object_ap_id(activity.data["object"]) + Activity.get_create_by_object_ap_id_with_object(activity.data["object"]) end end @@ -302,10 +302,10 @@ defmodule Pleroma.Web.CommonAPI.Utils do def maybe_notify_mentioned_recipients( recipients, - %Activity{data: %{"to" => _to, "type" => type} = data} = _activity + %Activity{data: %{"to" => _to, "type" => type} = data} = activity ) when type == "Create" do - object = Object.normalize(data["object"]) + object = Object.normalize(activity) object_data = cond do From b3bf523c0967e27616b78a42ba50a6b4c432749e Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:22:57 +0000 Subject: [PATCH 05/25] rich media: use optimized Object.normalize() --- lib/pleroma/web/rich_media/helpers.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/rich_media/helpers.ex b/lib/pleroma/web/rich_media/helpers.ex index 92c61ff51..f97d1863c 100644 --- a/lib/pleroma/web/rich_media/helpers.ex +++ b/lib/pleroma/web/rich_media/helpers.ex @@ -23,7 +23,7 @@ defmodule Pleroma.Web.RichMedia.Helpers do def fetch_data_for_activity(%Activity{} = activity) do with true <- Pleroma.Config.get([:rich_media, :enabled]), - %Object{} = object <- Object.normalize(activity.data["object"]), + %Object{} = object <- Object.normalize(activity), {:ok, page_url} <- HTML.extract_first_external_url(object, object.data["content"]), :ok <- validate_page_url(page_url), {:ok, rich_media} <- Parser.parse(page_url) do From 59518cbcd8bcc0fe98f2d05ef88d3e2ff68a256f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:24:23 +0000 Subject: [PATCH 06/25] activity: fix credo nitpick --- lib/pleroma/activity.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 370721070..f041d0e53 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -41,7 +41,8 @@ defmodule Pleroma.Activity do # # ``` # |> join(:inner, [activity], o in Object, - # fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", o.data, activity.data)) + # on: fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + # o.data, activity.data)) # |> preload([activity, object], [object: object]) # ``` # @@ -58,7 +59,7 @@ defmodule Pleroma.Activity do :inner, [activity], o in Object, - fragment( + on: fragment( "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", o.data, activity.data From a6973a668e40645f9c0940a4fb5aeee45003b66f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:28:16 +0000 Subject: [PATCH 07/25] formatting --- lib/pleroma/activity.ex | 11 ++++++----- lib/pleroma/web/common_api/common_api.ex | 3 ++- lib/pleroma/web/common_api/utils.ex | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index f041d0e53..a762d66ef 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -59,11 +59,12 @@ defmodule Pleroma.Activity do :inner, [activity], o in Object, - on: fragment( - "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", - o.data, - activity.data - ) + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + o.data, + activity.data + ) ) |> preload([activity, object], object: object) end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 8625f6621..25b990677 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -63,7 +63,8 @@ defmodule Pleroma.Web.CommonAPI do end def delete(activity_id, user) do - with %Activity{data: %{"object" => _}} = activity <- Activity.get_by_id_with_object(activity_id), + with %Activity{data: %{"object" => _}} = activity <- + Activity.get_by_id_with_object(activity_id), %Object{} = object <- Object.normalize(activity), true <- User.superuser?(user) || user.ap_id == object.data["actor"], {:ok, _} <- unpin(activity_id, user), diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 6a94c2d26..f596f703b 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -17,7 +17,8 @@ defmodule Pleroma.Web.CommonAPI.Utils do # This is a hack for twidere. def get_by_id_or_ap_id(id) do - activity = Activity.get_by_id_with_object(id) || Activity.get_create_by_object_ap_id_with_object(id) + activity = + Activity.get_by_id_with_object(id) || Activity.get_create_by_object_ap_id_with_object(id) activity && if activity.data["type"] == "Create" do From e4307cadc86018914cbe1298bcd06299e81006e5 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:40:08 +0000 Subject: [PATCH 08/25] activitypub: splice in the child object if we have one --- lib/pleroma/web/activity_pub/activity_pub.ex | 10 +++++++++- lib/pleroma/web/activity_pub/utils.ex | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 7d38a46e5..9cca7ec6f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -95,7 +95,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do :ok <- check_actor_is_active(map["actor"]), {_, true} <- {:remote_limit_error, check_remote_limit(map)}, {:ok, map} <- MRF.filter(map), - :ok <- insert_full_object(map) do + {:ok, object} <- insert_full_object(map) do {recipients, _, _} = get_recipients(map) {:ok, activity} = @@ -106,6 +106,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do recipients: recipients }) + # Splice in the child object if we have one. + activity = + if !is_nil(object) do + Map.put(activity, :object, object) + else + activity + end + Task.start(fn -> Pleroma.Web.RichMedia.Helpers.fetch_data_for_activity(activity) end) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index af317245f..2e9ffe41c 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -209,12 +209,12 @@ defmodule Pleroma.Web.ActivityPub.Utils do """ def insert_full_object(%{"object" => %{"type" => type} = object_data}) when is_map(object_data) and type in @supported_object_types do - with {:ok, _} <- Object.create(object_data) do - :ok + with {:ok, object} <- Object.create(object_data) do + {:ok, object} end end - def insert_full_object(_), do: :ok + def insert_full_object(_), do: {:ok, nil} def update_object_in_activities(%{data: %{"id" => id}} = object) do # TODO From ba7299fc875adc95102ddb1332bec2e6e89b6155 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 00:53:35 +0000 Subject: [PATCH 09/25] activitypub: add missing with_preloaded_object() --- lib/pleroma/web/activity_pub/activity_pub.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 9cca7ec6f..95bbadec1 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -438,6 +438,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do ), order_by: [desc: :id] ) + |> Activity.with_preloaded_object() Repo.all(query) end From 73efe95368dfc910c965e4025f47b36b6eb37aaa Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 01:09:12 +0000 Subject: [PATCH 10/25] activitypub: allow skipping preload in some cases (like certain tests where the preload is obnoxious) --- lib/pleroma/web/activity_pub/activity_pub.ex | 9 ++++++++- test/web/activity_pub/activity_pub_test.exs | 30 +++++++++++++++++++--------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 95bbadec1..0441376e3 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -718,6 +718,13 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do defp restrict_muted_reblogs(query, _), do: query + defp maybe_preload_objects(query, %{"skip_preload" => true}), do: query + + defp maybe_preload_objects(query, _) do + query + |> Activity.with_preloaded_object() + end + def fetch_activities_query(recipients, opts \\ %{}) do base_query = from( @@ -725,9 +732,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do limit: 20, order_by: [fragment("? desc nulls last", activity.id)] ) - |> Activity.with_preloaded_object() base_query + |> maybe_preload_objects(opts) |> restrict_recipients(recipients, opts["user"]) |> restrict_tag(opts) |> restrict_tag_reject(opts) diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 035778218..c11140f1c 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -270,7 +270,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do booster = insert(:user) {:ok, user} = User.block(user, %{ap_id: activity_one.data["actor"]}) - activities = ActivityPub.fetch_activities([], %{"blocking_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"blocking_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) @@ -278,7 +279,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do {:ok, user} = User.unblock(user, %{ap_id: activity_one.data["actor"]}) - activities = ActivityPub.fetch_activities([], %{"blocking_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"blocking_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) @@ -289,14 +291,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do %Activity{} = boost_activity = Activity.get_create_by_object_ap_id(id) activity_three = Repo.get(Activity, activity_three.id) - activities = ActivityPub.fetch_activities([], %{"blocking_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"blocking_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) refute Enum.member?(activities, activity_three) refute Enum.member?(activities, boost_activity) assert Enum.member?(activities, activity_one) - activities = ActivityPub.fetch_activities([], %{"blocking_user" => nil}) + activities = + ActivityPub.fetch_activities([], %{"blocking_user" => nil, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) @@ -312,14 +316,20 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do booster = insert(:user) {:ok, user} = User.mute(user, %User{ap_id: activity_one.data["actor"]}) - activities = ActivityPub.fetch_activities([], %{"muting_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"muting_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) refute Enum.member?(activities, activity_one) # Calling with 'with_muted' will deliver muted activities, too. - activities = ActivityPub.fetch_activities([], %{"muting_user" => user, "with_muted" => true}) + activities = + ActivityPub.fetch_activities([], %{ + "muting_user" => user, + "with_muted" => true, + "skip_preload" => true + }) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) @@ -327,7 +337,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do {:ok, user} = User.unmute(user, %User{ap_id: activity_one.data["actor"]}) - activities = ActivityPub.fetch_activities([], %{"muting_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"muting_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) @@ -338,14 +349,15 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do %Activity{} = boost_activity = Activity.get_create_by_object_ap_id(id) activity_three = Repo.get(Activity, activity_three.id) - activities = ActivityPub.fetch_activities([], %{"muting_user" => user}) + activities = + ActivityPub.fetch_activities([], %{"muting_user" => user, "skip_preload" => true}) assert Enum.member?(activities, activity_two) refute Enum.member?(activities, activity_three) refute Enum.member?(activities, boost_activity) assert Enum.member?(activities, activity_one) - activities = ActivityPub.fetch_activities([], %{"muting_user" => nil}) + activities = ActivityPub.fetch_activities([], %{"muting_user" => nil, "skip_preload" => true}) assert Enum.member?(activities, activity_two) assert Enum.member?(activities, activity_three) From e430a71d373a15b105a52771551b0ec4ed436ba4 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 01:17:26 +0000 Subject: [PATCH 11/25] ostatus: fetch preloaded object in note handler for testsuite --- lib/pleroma/web/ostatus/handlers/note_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/ostatus/handlers/note_handler.ex b/lib/pleroma/web/ostatus/handlers/note_handler.ex index 770a71a0a..db995ec77 100644 --- a/lib/pleroma/web/ostatus/handlers/note_handler.ex +++ b/lib/pleroma/web/ostatus/handlers/note_handler.ex @@ -106,7 +106,7 @@ defmodule Pleroma.Web.OStatus.NoteHandler do # TODO: Clean this up a bit. def handle_note(entry, doc \\ nil) do with id <- XML.string_from_xpath("//id", entry), - activity when is_nil(activity) <- Activity.get_create_by_object_ap_id(id), + activity when is_nil(activity) <- Activity.get_create_by_object_ap_id_with_object(id), [author] <- :xmerl_xpath.string('//author[1]', doc), {:ok, actor} <- OStatus.find_make_or_update_user(author), content_html <- OStatus.get_content(entry), From aaec91b9a126a4f36c996485c3a3149b3ba8745f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 01:23:02 +0000 Subject: [PATCH 12/25] relay test: don't do preloading (since follow objects are activities, it's a mess) --- test/tasks/relay_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/tasks/relay_test.exs b/test/tasks/relay_test.exs index c9d90fa2e..535dc3756 100644 --- a/test/tasks/relay_test.exs +++ b/test/tasks/relay_test.exs @@ -60,7 +60,8 @@ defmodule Mix.Tasks.Pleroma.RelayTest do ActivityPub.fetch_activities([], %{ "type" => "Undo", "actor_id" => follower_id, - "limit" => 1 + "limit" => 1, + "skip_preload" => true }) assert undo_activity.data["type"] == "Undo" From 4cedf454236c73b9337e58f5bf0e20ae65c65313 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 01:33:41 +0000 Subject: [PATCH 13/25] relay: use preloaded object since we always have it --- lib/pleroma/web/activity_pub/relay.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/relay.ex b/lib/pleroma/web/activity_pub/relay.ex index 01fef71b9..a7a20ca37 100644 --- a/lib/pleroma/web/activity_pub/relay.ex +++ b/lib/pleroma/web/activity_pub/relay.ex @@ -41,7 +41,7 @@ defmodule Pleroma.Web.ActivityPub.Relay do def publish(%Activity{data: %{"type" => "Create"}} = activity) do with %User{} = user <- get_actor(), - %Object{} = object <- Object.normalize(activity.data["object"]["id"]) do + %Object{} = object <- Object.normalize(activity) do ActivityPub.announce(user, object, nil, true, false) else e -> Logger.error("error: #{inspect(e)}") From 9a06d9f6e87c948997b908456ff8838e6110757e Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:19:34 +0000 Subject: [PATCH 14/25] notification: preload child objects --- lib/pleroma/notification.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index a98649b63..ef8e0b4c1 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Notification do alias Pleroma.Activity alias Pleroma.Notification + alias Pleroma.Object alias Pleroma.Pagination alias Pleroma.Repo alias Pleroma.User @@ -33,7 +34,10 @@ defmodule Pleroma.Notification do Notification |> where(user_id: ^user.id) |> join(:inner, [n], activity in assoc(n, :activity)) - |> preload(:activity) + |> join(:left, [n, a], object in Object, + on: fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", object.data, a.data) + ) + |> preload([n, a, o], activity: {a, object: o}) end def for_user(user, opts \\ %{}) do From c62220c50078c648c33d7aad7a615b9c7e1f27c6 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:26:49 +0000 Subject: [PATCH 15/25] rich media: helpers: only crawl Create activities --- lib/pleroma/web/rich_media/helpers.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/rich_media/helpers.ex b/lib/pleroma/web/rich_media/helpers.ex index f97d1863c..f67aaf58b 100644 --- a/lib/pleroma/web/rich_media/helpers.ex +++ b/lib/pleroma/web/rich_media/helpers.ex @@ -21,7 +21,7 @@ defmodule Pleroma.Web.RichMedia.Helpers do defp validate_page_url(%URI{}), do: :ok defp validate_page_url(_), do: :error - def fetch_data_for_activity(%Activity{} = activity) do + def fetch_data_for_activity(%Activity{data: %{"type" => "Create"}} = activity) do with true <- Pleroma.Config.get([:rich_media, :enabled]), %Object{} = object <- Object.normalize(activity), {:ok, page_url} <- HTML.extract_first_external_url(object, object.data["content"]), @@ -32,4 +32,6 @@ defmodule Pleroma.Web.RichMedia.Helpers do _ -> %{} end end + + def fetch_data_for_activity(_), do: %{} end From 07cdd9ed02838069b58c3b6cf9aec73b1d1852c3 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:27:52 +0000 Subject: [PATCH 16/25] streamer: use the preloaded object if possible --- lib/pleroma/web/streamer.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/streamer.ex b/lib/pleroma/web/streamer.ex index 7425bfb54..592749b42 100644 --- a/lib/pleroma/web/streamer.ex +++ b/lib/pleroma/web/streamer.ex @@ -202,7 +202,7 @@ defmodule Pleroma.Web.Streamer do mutes = user.info.mutes || [] reblog_mutes = user.info.muted_reblogs || [] - parent = Object.normalize(item.data["object"]) + parent = Object.normalize(item) unless is_nil(parent) or item.actor in blocks or item.actor in mutes or item.actor in reblog_mutes or not ActivityPub.contain_activity(item, user) or From aabcecb26935475b5985b6438774ee1d3cc14514 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:30:53 +0000 Subject: [PATCH 17/25] notification: formatting --- lib/pleroma/notification.ex | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index ef8e0b4c1..cac10f24a 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -35,7 +35,12 @@ defmodule Pleroma.Notification do |> where(user_id: ^user.id) |> join(:inner, [n], activity in assoc(n, :activity)) |> join(:left, [n, a], object in Object, - on: fragment("(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", object.data, a.data) + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + object.data, + a.data + ) ) |> preload([n, a, o], activity: {a, object: o}) end From ce47eb8b29aaba8f59720597485d3c586fbb7831 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:38:59 +0000 Subject: [PATCH 18/25] activitypub: when fetching objects, use the preloaded object from the synthesized activity --- lib/pleroma/web/activity_pub/activity_pub.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 0441376e3..80c64ae04 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -957,7 +957,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do }, :ok <- Transmogrifier.contain_origin(id, params), {:ok, activity} <- Transmogrifier.handle_incoming(params) do - {:ok, Object.normalize(activity.data["object"])} + {:ok, Object.normalize(activity)} else {:error, {:reject, nil}} -> {:reject, nil} @@ -969,7 +969,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do Logger.info("Couldn't get object via AP, trying out OStatus fetching...") case OStatus.fetch_activity_from_url(id) do - {:ok, [activity | _]} -> {:ok, Object.normalize(activity.data["object"])} + {:ok, [activity | _]} -> {:ok, Object.normalize(activity)} e -> e end end From f9d5c13b21905b6839f489d4c8d75bc1e95d3875 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:49:10 +0000 Subject: [PATCH 19/25] activity: add get_by_ap_id_with_object() --- lib/pleroma/activity.ex | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index a762d66ef..fcdac3a3f 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -78,6 +78,23 @@ defmodule Pleroma.Activity do ) end + def get_by_ap_id_with_object(ap_id) do + Repo.one( + from( + activity in Activity, + where: fragment("(?)->>'id' = ?", activity.data, ^to_string(ap_id)), + inner_join: o in Object, + on: + fragment( + "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)", + o.data, + activity.data + ), + preload: [object: o] + ) + ) + end + def get_by_id(id) do Repo.get(Activity, id) end @@ -181,8 +198,8 @@ defmodule Pleroma.Activity do |> Repo.one() end - def normalize(obj) when is_map(obj), do: Activity.get_by_ap_id(obj["id"]) - def normalize(ap_id) when is_binary(ap_id), do: Activity.get_by_ap_id(ap_id) + def normalize(obj) when is_map(obj), do: get_by_ap_id_with_object(obj["id"]) + def normalize(ap_id) when is_binary(ap_id), do: get_by_ap_id_with_object(ap_id) def normalize(_), do: nil def get_in_reply_to_activity(%Activity{data: %{"object" => %{"inReplyTo" => ap_id}}}) do From 8c70156157fcb2e0091c21e566324bcb24886f6f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 02:49:40 +0000 Subject: [PATCH 20/25] activitypub: object view: use preloaded object when possible --- lib/pleroma/web/activity_pub/views/object_view.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/views/object_view.ex b/lib/pleroma/web/activity_pub/views/object_view.ex index 84fa94e32..6028b773c 100644 --- a/lib/pleroma/web/activity_pub/views/object_view.ex +++ b/lib/pleroma/web/activity_pub/views/object_view.ex @@ -17,7 +17,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do def render("object.json", %{object: %Activity{data: %{"type" => "Create"}} = activity}) do base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header() - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) additional = Transmogrifier.prepare_object(activity.data) @@ -28,7 +28,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectView do def render("object.json", %{object: %Activity{} = activity}) do base = Pleroma.Web.ActivityPub.Utils.make_json_ld_header() - object = Object.normalize(activity.data["object"]) + object = Object.normalize(activity) additional = Transmogrifier.prepare_object(activity.data) From 3c2350cbee95df3b6a31945d54bf7c46a8afff25 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 03:00:04 +0000 Subject: [PATCH 21/25] object: downgrade normalize warning to debug severity --- lib/pleroma/object.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 0f5c532ec..193ae3fa8 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -47,18 +47,22 @@ defmodule Pleroma.Object do # Catch and log Object.normalize() calls where the Activity's child object is not # preloaded. def normalize(%Activity{data: %{"object" => %{"id" => ap_id}}}) do - Logger.info( + Logger.debug( "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" ) + Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") + normalize(ap_id) end def normalize(%Activity{data: %{"object" => ap_id}}) do - Logger.info( + Logger.debug( "Object.normalize() called without preloaded object (#{ap_id}). Consider preloading the object!" ) + Logger.debug("Backtrace: #{inspect(Process.info(:erlang.self(), :current_stacktrace))}") + normalize(ap_id) end From debf7f016d5f09b5878ddb24051a28ddb2cf1e4a Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 23 Mar 2019 03:03:06 +0000 Subject: [PATCH 22/25] ostatus: use preload objects with Object.normalize() when opportunistic --- lib/pleroma/web/ostatus/ostatus.ex | 10 +++++----- lib/pleroma/web/ostatus/ostatus_controller.ex | 13 +++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/ostatus/ostatus.ex b/lib/pleroma/web/ostatus/ostatus.ex index 266f86bf4..9a34d7ad5 100644 --- a/lib/pleroma/web/ostatus/ostatus.ex +++ b/lib/pleroma/web/ostatus/ostatus.ex @@ -23,8 +23,8 @@ defmodule Pleroma.Web.OStatus do alias Pleroma.Web.WebFinger alias Pleroma.Web.Websub - def is_representable?(%Activity{data: data}) do - object = Object.normalize(data["object"]) + def is_representable?(%Activity{} = activity) do + object = Object.normalize(activity) cond do is_nil(object) -> @@ -119,7 +119,7 @@ defmodule Pleroma.Web.OStatus do def make_share(entry, doc, retweeted_activity) do with {:ok, actor} <- find_make_or_update_user(doc), - %Object{} = object <- Object.normalize(retweeted_activity.data["object"]), + %Object{} = object <- Object.normalize(retweeted_activity), id when not is_nil(id) <- string_from_xpath("/entry/id", entry), {:ok, activity, _object} = ActivityPub.announce(actor, object, id, false) do {:ok, activity} @@ -137,7 +137,7 @@ defmodule Pleroma.Web.OStatus do def make_favorite(entry, doc, favorited_activity) do with {:ok, actor} <- find_make_or_update_user(doc), - %Object{} = object <- Object.normalize(favorited_activity.data["object"]), + %Object{} = object <- Object.normalize(favorited_activity), id when not is_nil(id) <- string_from_xpath("/entry/id", entry), {:ok, activity, _object} = ActivityPub.like(actor, object, id, false) do {:ok, activity} @@ -159,7 +159,7 @@ defmodule Pleroma.Web.OStatus do Logger.debug("Trying to get entry from db") with id when not is_nil(id) <- string_from_xpath("//activity:object[1]/id", entry), - %Activity{} = activity <- Activity.get_create_by_object_ap_id(id) do + %Activity{} = activity <- Activity.get_create_by_object_ap_id_with_object(id) do {:ok, activity} else _ -> diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index 0579a5f3d..2fb6ce41b 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -102,7 +102,8 @@ defmodule Pleroma.Web.OStatus.OStatusController do ActivityPubController.call(conn, :object) else with id <- o_status_url(conn, :object, uuid), - {_, %Activity{} = activity} <- {:activity, Activity.get_create_by_object_ap_id(id)}, + {_, %Activity{} = activity} <- + {:activity, Activity.get_create_by_object_ap_id_with_object(id)}, {_, true} <- {:public?, Visibility.is_public?(activity)}, %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do case get_format(conn) do @@ -148,13 +149,13 @@ defmodule Pleroma.Web.OStatus.OStatusController do end def notice(conn, %{"id" => id}) do - with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id(id)}, + with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id_with_object(id)}, {_, true} <- {:public?, Visibility.is_public?(activity)}, %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do case format = get_format(conn) do "html" -> if activity.data["type"] == "Create" do - %Object{} = object = Object.normalize(activity.data["object"]) + %Object{} = object = Object.normalize(activity) Fallback.RedirectController.redirector_with_meta(conn, %{ activity_id: activity.id, @@ -191,9 +192,9 @@ defmodule Pleroma.Web.OStatus.OStatusController do # Returns an HTML embedded