Merge branch 'feature/sql-thread-sat' into 'develop'

SQL thread visibility solver

See merge request pleroma/pleroma!971
This commit is contained in:
lambda 2019-05-16 13:44:22 +00:00
commit fde30aee43
9 changed files with 105 additions and 42 deletions

View File

@ -95,7 +95,6 @@ def handle_command(state, "home") do
activities = activities =
[user.ap_id | user.following] [user.ap_id | user.following]
|> ActivityPub.fetch_activities(params) |> ActivityPub.fetch_activities(params)
|> ActivityPub.contain_timeline(user)
Enum.each(activities, fn activity -> Enum.each(activities, fn activity ->
puts_activity(activity) puts_activity(activity)

View File

@ -38,7 +38,8 @@ def get_filters(%User{id: user_id} = _user) do
query = query =
from( from(
f in Pleroma.Filter, f in Pleroma.Filter,
where: f.user_id == ^user_id where: f.user_id == ^user_id,
order_by: [desc: :id]
) )
Repo.all(query) Repo.all(query)

View File

@ -540,8 +540,6 @@ defp restrict_visibility(query, %{visibility: visibility})
) )
) )
Ecto.Adapters.SQL.to_sql(:all, Repo, query)
query query
else else
Logger.error("Could not restrict visibility to #{visibility}") Logger.error("Could not restrict visibility to #{visibility}")
@ -557,8 +555,6 @@ defp restrict_visibility(query, %{visibility: visibility})
fragment("activity_visibility(?, ?, ?) = ?", a.actor, a.recipients, a.data, ^visibility) fragment("activity_visibility(?, ?, ?) = ?", a.actor, a.recipients, a.data, ^visibility)
) )
Ecto.Adapters.SQL.to_sql(:all, Repo, query)
query query
end end
@ -569,6 +565,18 @@ defp restrict_visibility(_query, %{visibility: visibility})
defp restrict_visibility(query, _visibility), do: query defp restrict_visibility(query, _visibility), do: query
defp restrict_thread_visibility(query, %{"user" => %User{ap_id: ap_id}}) do
query =
from(
a in query,
where: fragment("thread_visibility(?, (?)->>'id') = true", ^ap_id, a.data)
)
query
end
defp restrict_thread_visibility(query, _), do: query
def fetch_user_activities(user, reading_user, params \\ %{}) do def fetch_user_activities(user, reading_user, params \\ %{}) do
params = params =
params params
@ -848,6 +856,7 @@ def fetch_activities_query(recipients, opts \\ %{}) do
|> restrict_muted(opts) |> restrict_muted(opts)
|> restrict_media(opts) |> restrict_media(opts)
|> restrict_visibility(opts) |> restrict_visibility(opts)
|> restrict_thread_visibility(opts)
|> restrict_replies(opts) |> restrict_replies(opts)
|> restrict_reblogs(opts) |> restrict_reblogs(opts)
|> restrict_pinned(opts) |> restrict_pinned(opts)
@ -965,12 +974,4 @@ def contain_broken_threads(%Activity{} = activity, %User{} = user) do
def contain_activity(%Activity{} = activity, %User{} = user) do def contain_activity(%Activity{} = activity, %User{} = user) do
contain_broken_threads(activity, user) contain_broken_threads(activity, user)
end end
# do post-processing on a timeline
def contain_timeline(timeline, user) do
timeline
|> Enum.filter(fn activity ->
contain_activity(activity, user)
end)
end
end end

View File

@ -1,6 +1,7 @@
defmodule Pleroma.Web.ActivityPub.Visibility do defmodule Pleroma.Web.ActivityPub.Visibility do
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.Repo
alias Pleroma.User alias Pleroma.User
def is_public?(%Object{data: %{"type" => "Tombstone"}}), do: false def is_public?(%Object{data: %{"type" => "Tombstone"}}), do: false
@ -38,25 +39,14 @@ def visible_for_user?(activity, user) do
visible_for_user?(activity, nil) || Enum.any?(x, &(&1 in y)) visible_for_user?(activity, nil) || Enum.any?(x, &(&1 in y))
end end
# guard def entire_thread_visible_for_user?(%Activity{} = activity, %User{} = user) do
def entire_thread_visible_for_user?(nil, _user), do: false {:ok, %{rows: [[result]]}} =
Ecto.Adapters.SQL.query(Repo, "SELECT thread_visibility($1, $2)", [
user.ap_id,
activity.data["id"]
])
# XXX: Probably even more inefficient than the previous implementation intended to be a placeholder untill https://git.pleroma.social/pleroma/pleroma/merge_requests/971 is in develop result
# credo:disable-for-previous-line Credo.Check.Readability.MaxLineLength
def entire_thread_visible_for_user?(
%Activity{} = tail,
# %Activity{data: %{"object" => %{"inReplyTo" => parent_id}}} = tail,
user
) do
case Object.normalize(tail) do
%{data: %{"inReplyTo" => parent_id}} when is_binary(parent_id) ->
parent = Activity.get_in_reply_to_activity(tail)
visible_for_user?(tail, user) && entire_thread_visible_for_user?(parent, user)
_ ->
visible_for_user?(tail, user)
end
end end
def get_visibility(object) do def get_visibility(object) do

View File

@ -303,7 +303,6 @@ def home_timeline(%{assigns: %{user: user}} = conn, params) do
activities = activities =
[user.ap_id | user.following] [user.ap_id | user.following]
|> ActivityPub.fetch_activities(params) |> ActivityPub.fetch_activities(params)
|> ActivityPub.contain_timeline(user)
|> Enum.reverse() |> Enum.reverse()
conn conn

View File

@ -101,9 +101,7 @@ def friends_timeline(%{assigns: %{user: user}} = conn, params) do
|> Map.put("blocking_user", user) |> Map.put("blocking_user", user)
|> Map.put("user", user) |> Map.put("user", user)
activities = activities = ActivityPub.fetch_activities([user.ap_id | user.following], params)
ActivityPub.fetch_activities([user.ap_id | user.following], params)
|> ActivityPub.contain_timeline(user)
conn conn
|> put_view(ActivityView) |> put_view(ActivityView)

View File

@ -0,0 +1,73 @@
defmodule Pleroma.Repo.Migrations.AddThreadVisibilityFunction do
use Ecto.Migration
@disable_ddl_transaction true
def up do
statement = """
CREATE OR REPLACE FUNCTION thread_visibility(actor varchar, activity_id varchar) RETURNS boolean AS $$
DECLARE
public varchar := 'https://www.w3.org/ns/activitystreams#Public';
child objects%ROWTYPE;
activity activities%ROWTYPE;
actor_user users%ROWTYPE;
author_fa varchar;
valid_recipients varchar[];
BEGIN
--- Fetch our actor.
SELECT * INTO actor_user FROM users WHERE users.ap_id = actor;
--- Fetch our initial activity.
SELECT * INTO activity FROM activities WHERE activities.data->>'id' = activity_id;
LOOP
--- Ensure that we have an activity before continuing.
--- If we don't, the thread is not satisfiable.
IF activity IS NULL THEN
RETURN false;
END IF;
--- We only care about Create activities.
IF activity.data->>'type' != 'Create' THEN
RETURN true;
END IF;
--- Normalize the child object into child.
SELECT * INTO child FROM objects
INNER JOIN activities ON COALESCE(activities.data->'object'->>'id', activities.data->>'object') = objects.data->>'id'
WHERE COALESCE(activity.data->'object'->>'id', activity.data->>'object') = objects.data->>'id';
--- Fetch the author's AS2 following collection.
SELECT COALESCE(users.follower_address, '') INTO author_fa FROM users WHERE users.ap_id = activity.actor;
--- Prepare valid recipients array.
valid_recipients := ARRAY[actor, public];
IF ARRAY[author_fa] && actor_user.following THEN
valid_recipients := valid_recipients || author_fa;
END IF;
--- Check visibility.
IF NOT valid_recipients && activity.recipients THEN
--- activity not visible, break out of the loop
RETURN false;
END IF;
--- If there's a parent, load it and do this all over again.
IF (child.data->'inReplyTo' IS NOT NULL) AND (child.data->'inReplyTo' != 'null'::jsonb) THEN
SELECT * INTO activity FROM activities
INNER JOIN objects ON COALESCE(activities.data->'object'->>'id', activities.data->>'object') = objects.data->>'id'
WHERE child.data->>'inReplyTo' = objects.data->>'id';
ELSE
RETURN true;
END IF;
END LOOP;
END;
$$ LANGUAGE plpgsql IMMUTABLE;
"""
execute(statement)
end
def down do
execute("drop function thread_visibility(actor varchar, activity_id varchar)")
end
end

View File

@ -873,7 +873,6 @@ test "hide a user's statuses from timelines and notifications" do
assert [activity] == assert [activity] ==
ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2}) ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2})
|> ActivityPub.contain_timeline(user2)
{:ok, _user} = User.deactivate(user) {:ok, _user} = User.deactivate(user)
@ -882,7 +881,6 @@ test "hide a user's statuses from timelines and notifications" do
assert [] == assert [] ==
ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2}) ActivityPub.fetch_activities([user2.ap_id | user2.following], %{"user" => user2})
|> ActivityPub.contain_timeline(user2)
end end
end end

View File

@ -960,17 +960,21 @@ test "it filters broken threads" do
"in_reply_to_status_id" => private_activity_2.id "in_reply_to_status_id" => private_activity_2.id
}) })
activities = ActivityPub.fetch_activities([user1.ap_id | user1.following]) activities =
ActivityPub.fetch_activities([user1.ap_id | user1.following])
|> Enum.map(fn a -> a.id end)
private_activity_1 = Activity.get_by_ap_id_with_object(private_activity_1.data["id"]) private_activity_1 = Activity.get_by_ap_id_with_object(private_activity_1.data["id"])
assert [public_activity, private_activity_1, private_activity_3] == activities assert [public_activity.id, private_activity_1.id, private_activity_3.id] == activities
assert length(activities) == 3 assert length(activities) == 3
activities = ActivityPub.contain_timeline(activities, user1) activities =
ActivityPub.fetch_activities([user1.ap_id | user1.following], %{"user" => user1})
|> Enum.map(fn a -> a.id end)
assert [public_activity, private_activity_1] == activities assert [public_activity.id, private_activity_1.id] == activities
assert length(activities) == 2 assert length(activities) == 2
end end
end end