From d89cd0a19733eec27b79b768df2e30a68bfc6d6b Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 18:25:26 +0200 Subject: [PATCH] Reply Filtering: Refactor. --- benchmarks/load_testing/fetcher.ex | 15 ++- benchmarks/mix/tasks/pleroma/load_testing.ex | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 114 ++++++++------------- .../controllers/timeline_controller.ex | 3 +- test/web/activity_pub/activity_pub_test.exs | 13 ++- 5 files changed, 69 insertions(+), 77 deletions(-) diff --git a/benchmarks/load_testing/fetcher.ex b/benchmarks/load_testing/fetcher.ex index 6503deb41..12c30f6f5 100644 --- a/benchmarks/load_testing/fetcher.ex +++ b/benchmarks/load_testing/fetcher.ex @@ -508,13 +508,13 @@ defmodule Pleroma.LoadTesting.Fetcher do "Public timeline with reply filtering - following" => fn -> public_params |> Map.put("reply_visibility", "following") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() end, "Public timeline with reply filtering - self" => fn -> public_params |> Map.put("reply_visibility", "self") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() end }, @@ -531,12 +531,19 @@ defmodule Pleroma.LoadTesting.Fetcher do ActivityPub.fetch_activities(recipients, private_params) end, "Home timeline with reply filtering - following" => fn -> - private_params = Map.put(private_params, "reply_visibility", "following") + private_params = + private_params + |> Map.put("reply_filtering_user", user) + |> Map.put("reply_visibility", "following") ActivityPub.fetch_activities(recipients, private_params) end, "Home timeline with reply filtering - self" => fn -> - private_params = Map.put(private_params, "reply_visibility", "self") + private_params = + private_params + |> Map.put("reply_filtering_user", user) + |> Map.put("reply_visibility", "self") + ActivityPub.fetch_activities(recipients, private_params) end }, diff --git a/benchmarks/mix/tasks/pleroma/load_testing.ex b/benchmarks/mix/tasks/pleroma/load_testing.ex index 72b225f09..388883240 100644 --- a/benchmarks/mix/tasks/pleroma/load_testing.ex +++ b/benchmarks/mix/tasks/pleroma/load_testing.ex @@ -44,6 +44,7 @@ defmodule Mix.Tasks.Pleroma.LoadTesting do ] def run(args) do + Logger.configure(level: :error) Mix.Pleroma.start_pleroma() clean_tables() {opts, _} = OptionParser.parse!(args, strict: @switches, aliases: @aliases) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 8b170b7f8..9ec31fb03 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -270,9 +270,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do ), {:ok, activity} <- insert(create_data, local, fake), {:fake, false, activity} <- {:fake, fake, activity}, - {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, _ <- increase_replies_count_if_reply(create_data), _ <- increase_poll_votes_if_vote(create_data), + {:quick_insert, false, activity} <- {:quick_insert, quick_insert?, activity}, {:ok, _actor} <- increase_note_count_if_public(actor, activity), :ok <- maybe_federate(activity) do {:ok, activity} @@ -700,14 +700,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do do: [opts["user"].ap_id | User.following(opts["user"])] ++ public, else: public - opts = Map.put(opts, "user", opts["user"]) - from(activity in Activity) |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_set_thread_muted_field(opts) |> restrict_blocked(opts) - |> restrict_recipients(recipients, opts) + |> restrict_recipients(recipients, opts["user"]) |> where( [activity], fragment( @@ -742,10 +740,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do @spec fetch_public_activities(map(), Pagination.type()) :: [Activity.t()] def fetch_public_activities(opts \\ %{}, pagination \\ :keyset) do - opts = - opts - |> Map.put("reply_user", opts["user"]) - |> Map.delete("user") + opts = Map.drop(opts, ["user"]) [Constants.as_public()] |> fetch_activities_query(opts) @@ -981,65 +976,13 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do defp restrict_tag(query, _), do: query - defp reply_recipients(user, "following") do - [user.ap_id | User.get_cached_user_friends_ap_ids(user)] + defp restrict_recipients(query, [], _user), do: query + + defp restrict_recipients(query, recipients, nil) do + from(activity in query, where: fragment("? && ?", ^recipients, activity.recipients)) end - defp reply_recipients(user, "self"), do: [user.ap_id] - - defp restrict_recipients(query, [], _opts), do: query - - defp restrict_recipients( - query, - recipients, - %{"user" => nil, "reply_user" => user, "reply_visibility" => visibility} - ) - when not is_nil(user) and visibility in ["following", "self"] do - reply_recipients = reply_recipients(user, visibility) - - from([activity, object] in query, - where: - fragment( - "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ? OR ? = ?)", - ^recipients, - activity.recipients, - object.data, - activity.recipients, - activity.actor, - ^reply_recipients, - activity.actor, - ^user.ap_id - ) - ) - end - - defp restrict_recipients(query, recipients, %{"user" => nil}) do - from(activity in query, - where: fragment("? && ?", ^recipients, activity.recipients) - ) - end - - defp restrict_recipients(query, recipients, %{"user" => user, "reply_visibility" => visibility}) - when visibility in ["following", "self"] do - reply_recipients = reply_recipients(user, visibility) - - from( - [activity, object] in query, - where: - fragment( - "? && ? AND (?->>'inReplyTo' IS NULL OR array_remove(?, ?) && ?)", - ^recipients, - activity.recipients, - object.data, - activity.recipients, - activity.actor, - ^reply_recipients - ), - or_where: activity.actor == ^user.ap_id - ) - end - - defp restrict_recipients(query, recipients, %{"user" => user}) do + defp restrict_recipients(query, recipients, user) do from( activity in query, where: fragment("? && ?", ^recipients, activity.recipients), @@ -1104,6 +1047,41 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do ) end + defp restrict_replies(query, %{ + "reply_filtering_user" => user, + "reply_visibility" => "self" + }) do + from( + [activity, object] in query, + where: + fragment( + "?->>'inReplyTo' is null OR ? = ANY(?)", + object.data, + ^user.ap_id, + activity.recipients + ) + ) + end + + defp restrict_replies(query, %{ + "reply_filtering_user" => user, + "reply_visibility" => "following" + }) do + from( + [activity, object] in query, + where: + fragment( + "?->>'inReplyTo' is null OR ? && array_remove(?, ?) OR ? = ?", + object.data, + ^[user.ap_id | User.get_cached_user_friends_ap_ids(user)], + activity.recipients, + activity.actor, + activity.actor, + ^user.ap_id + ) + ) + end + defp restrict_replies(query, _), do: query defp restrict_reblogs(query, %{"exclude_reblogs" => val}) when val == "true" or val == "1" do @@ -1311,15 +1289,14 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do skip_thread_containment: Config.get([:instance, :skip_thread_containment]) } - opts = Map.put(opts, "user", opts["user"]) - Activity |> maybe_preload_objects(opts) |> maybe_preload_bookmarks(opts) |> maybe_preload_report_notes(opts) |> maybe_set_thread_muted_field(opts) |> maybe_order(opts) - |> restrict_recipients(recipients, opts) + |> restrict_recipients(recipients, opts["user"]) + |> restrict_replies(opts) |> restrict_tag(opts) |> restrict_tag_reject(opts) |> restrict_tag_all(opts) @@ -1334,7 +1311,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do |> restrict_media(opts) |> restrict_visibility(opts) |> restrict_thread_visibility(opts, config) - |> restrict_replies(opts) |> restrict_reblogs(opts) |> restrict_pinned(opts) |> restrict_muted_reblogs(restrict_muted_reblogs_opts) diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index a2ac9301e..403d500e0 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -37,6 +37,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do |> Map.put("type", ["Create", "Announce"]) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) recipients = [user.ap_id | User.following(user)] @@ -100,7 +101,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do |> Map.put("local_only", local_only) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() conn diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 8a1638a23..edd7dfb22 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1936,7 +1936,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("local_only", false) |> Map.put("blocking_user", user) |> Map.put("muting_user", user) - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -1958,7 +1958,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "following") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -1985,7 +1985,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "self") - |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2008,6 +2008,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("user", user) + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2046,6 +2047,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2084,6 +2086,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2131,6 +2134,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2146,6 +2150,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("blocking_user", user) |> Map.put("muting_user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) |> Map.put("user", user) |> ActivityPub.fetch_public_activities() |> Enum.map(& &1.id) @@ -2176,6 +2181,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "following") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params) @@ -2199,6 +2205,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do |> Map.put("muting_user", user) |> Map.put("user", user) |> Map.put("reply_visibility", "self") + |> Map.put("reply_filtering_user", user) activities_ids = ActivityPub.fetch_activities([user.ap_id | User.following(user)], params)