From c556efb761a3e7fc2beb4540d6f58dbfe8e4abfe Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 12 Apr 2020 21:53:03 +0300 Subject: [PATCH] [#1364] Enabled notifications on followed domain-blocked users' activities. --- lib/pleroma/following_relationship.ex | 35 +++++++++++++-- lib/pleroma/notification.ex | 61 +++++++++++++++++++++------ test/notification_test.exs | 35 +++++++++++++-- 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/lib/pleroma/following_relationship.ex b/lib/pleroma/following_relationship.ex index a9538ea4e..11e06c5cc 100644 --- a/lib/pleroma/following_relationship.ex +++ b/lib/pleroma/following_relationship.ex @@ -69,6 +69,29 @@ def follower_count(%User{} = user) do |> Repo.aggregate(:count, :id) end + def followers_query(%User{} = user) do + __MODULE__ + |> join(:inner, [r], u in User, on: r.follower_id == u.id) + |> where([r], r.following_id == ^user.id) + |> where([r], r.state == "accept") + end + + def followers_ap_ids(%User{} = user, from_ap_ids \\ nil) do + query = + user + |> followers_query() + |> select([r, u], u.ap_id) + + query = + if from_ap_ids do + where(query, [r, u], u.ap_id in ^from_ap_ids) + else + query + end + + Repo.all(query) + end + def following_count(%User{id: nil}), do: 0 def following_count(%User{} = user) do @@ -92,12 +115,16 @@ def following?(%User{id: follower_id}, %User{id: followed_id}) do |> Repo.exists?() end + def following_query(%User{} = user) do + __MODULE__ + |> join(:inner, [r], u in User, on: r.following_id == u.id) + |> where([r], r.follower_id == ^user.id) + |> where([r], r.state == "accept") + end + def following(%User{} = user) do following = - __MODULE__ - |> join(:inner, [r], u in User, on: r.following_id == u.id) - |> where([r], r.follower_id == ^user.id) - |> where([r], r.state == "accept") + following_query(user) |> select([r, u], u.follower_address) |> Repo.all() diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 02363ddb0..da05ff2e4 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -6,6 +6,7 @@ defmodule Pleroma.Notification do use Ecto.Schema alias Pleroma.Activity + alias Pleroma.FollowingRelationship alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Pagination @@ -81,6 +82,7 @@ def for_user_query(user, opts \\ %{}) do |> exclude_visibility(opts) end + # Excludes blocked users and non-followed domain-blocked users defp exclude_blocked(query, user, opts) do blocked_ap_ids = opts[:blocked_users_ap_ids] || User.blocked_users_ap_ids(user) @@ -88,7 +90,16 @@ defp exclude_blocked(query, user, opts) do |> where([n, a], a.actor not in ^blocked_ap_ids) |> where( [n, a], - fragment("substring(? from '.*://([^/]*)')", a.actor) not in ^user.domain_blocks + fragment( + # "NOT (actor's domain in domain_blocks) OR (actor is in followed AP IDs)" + "NOT (substring(? from '.*://([^/]*)') = ANY(?)) OR \ + ? = ANY(SELECT ap_id FROM users AS u INNER JOIN following_relationships AS fr \ + ON u.id = fr.following_id WHERE fr.follower_id = ? AND fr.state = 'accept')", + a.actor, + ^user.domain_blocks, + a.actor, + ^User.binary_id(user.id) + ) ) end @@ -338,19 +349,11 @@ def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, lo |> Utils.maybe_notify_followers(activity) |> Enum.uniq() - potential_receivers = + potential_receivers = User.get_users_from_set(potential_receiver_ap_ids, local_only) + + notification_enabled_ap_ids = potential_receiver_ap_ids - |> Enum.uniq() - |> User.get_users_from_set(local_only) - - activity_actor_domain = activity.actor && URI.parse(activity.actor).host - - notification_enabled_ap_ids = - for u <- potential_receivers, activity_actor_domain not in u.domain_blocks, do: u.ap_id - - # Since even subscribers and followers can mute / thread-mute, filtering all above AP IDs - notification_enabled_ap_ids = - notification_enabled_ap_ids + |> exclude_domain_blocker_ap_ids(activity, potential_receivers) |> exclude_relationship_restricted_ap_ids(activity) |> exclude_thread_muter_ap_ids(activity) @@ -362,6 +365,38 @@ def get_notified_from_activity(%Activity{data: %{"type" => type}} = activity, lo def get_notified_from_activity(_, _local_only), do: {[], []} + @doc "Filters out AP IDs of users who domain-block and not follow activity actor" + def exclude_domain_blocker_ap_ids(ap_ids, activity, preloaded_users \\ []) + + def exclude_domain_blocker_ap_ids([], _activity, _preloaded_users), do: [] + + def exclude_domain_blocker_ap_ids(ap_ids, %Activity{} = activity, preloaded_users) do + activity_actor_domain = activity.actor && URI.parse(activity.actor).host + + users = + ap_ids + |> Enum.map(fn ap_id -> + Enum.find(preloaded_users, &(&1.ap_id == ap_id)) || + User.get_cached_by_ap_id(ap_id) + end) + |> Enum.filter(& &1) + + domain_blocker_ap_ids = for u <- users, activity_actor_domain in u.domain_blocks, do: u.ap_id + + domain_blocker_follower_ap_ids = + if Enum.any?(domain_blocker_ap_ids) do + activity + |> Activity.user_actor() + |> FollowingRelationship.followers_ap_ids(domain_blocker_ap_ids) + else + [] + end + + ap_ids + |> Kernel.--(domain_blocker_ap_ids) + |> Kernel.++(domain_blocker_follower_ap_ids) + end + @doc "Filters out AP IDs of users basing on their relationships with activity actor user" def exclude_relationship_restricted_ap_ids([], _activity), do: [] diff --git a/test/notification_test.exs b/test/notification_test.exs index caa941934..4e5559bb1 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -610,7 +610,7 @@ test "it returns thread-muting recipient in disabled recipients list" do refute other_user in enabled_receivers end - test "it returns domain-blocking recipient in disabled recipients list" do + test "it returns non-following domain-blocking recipient in disabled recipients list" do blocked_domain = "blocked.domain" user = insert(:user, %{ap_id: "https://#{blocked_domain}/@actor"}) other_user = insert(:user) @@ -624,6 +624,22 @@ test "it returns domain-blocking recipient in disabled recipients list" do assert [] == enabled_receivers assert [other_user] == disabled_receivers end + + test "it returns following domain-blocking recipient in enabled recipients list" do + blocked_domain = "blocked.domain" + user = insert(:user, %{ap_id: "https://#{blocked_domain}/@actor"}) + other_user = insert(:user) + + {:ok, other_user} = User.block_domain(other_user, blocked_domain) + {:ok, other_user} = User.follow(other_user, user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey @#{other_user.nickname}!"}) + + {enabled_receivers, disabled_receivers} = Notification.get_notified_from_activity(activity) + + assert [other_user] == enabled_receivers + assert [] == disabled_receivers + end end describe "notification lifecycle" do @@ -886,7 +902,7 @@ test "it doesn't return notifications for blocked user" do assert Notification.for_user(user) == [] end - test "it doesn't return notifications for blocked domain" do + test "it doesn't return notifications for domain-blocked non-followed user" do user = insert(:user) blocked = insert(:user, ap_id: "http://some-domain.com") {:ok, user} = User.block_domain(user, "some-domain.com") @@ -896,6 +912,18 @@ test "it doesn't return notifications for blocked domain" do assert Notification.for_user(user) == [] end + test "it returns notifications for domain-blocked but followed user" do + user = insert(:user) + blocked = insert(:user, ap_id: "http://some-domain.com") + + {:ok, user} = User.block_domain(user, "some-domain.com") + {:ok, _} = User.follow(user, blocked) + + {:ok, _activity} = CommonAPI.post(blocked, %{"status" => "hey @#{user.nickname}"}) + + assert length(Notification.for_user(user)) == 1 + end + test "it doesn't return notifications for muted thread" do user = insert(:user) another_user = insert(:user) @@ -926,7 +954,8 @@ test "it doesn't return notifications from a blocked user when with_muted is set assert Enum.empty?(Notification.for_user(user, %{with_muted: true})) end - test "it doesn't return notifications from a domain-blocked user when with_muted is set" do + test "when with_muted is set, " <> + "it doesn't return notifications from a domain-blocked non-followed user" do user = insert(:user) blocked = insert(:user, ap_id: "http://some-domain.com") {:ok, user} = User.block_domain(user, "some-domain.com")