[#3213] ActivityPub: fixed subquery-based hashtags filtering implementation (addressed empty list options issue). Added regression test.

This commit is contained in:
Ivan Tashkinov 2021-01-31 23:06:38 +03:00
parent 10207f840c
commit cf4765af40
2 changed files with 68 additions and 60 deletions

View file

@ -673,7 +673,7 @@ defp restrict_embedded_tag_all(_query, %{tag_all: _tag_all, skip_preload: true})
raise_on_missing_preload() raise_on_missing_preload()
end end
defp restrict_embedded_tag_all(query, %{tag_all: tag_all}) when is_list(tag_all) do defp restrict_embedded_tag_all(query, %{tag_all: [_ | _] = tag_all}) do
from( from(
[_activity, object] in query, [_activity, object] in query,
where: fragment("(?)->'tag' \\?& (?)", object.data, ^tag_all) where: fragment("(?)->'tag' \\?& (?)", object.data, ^tag_all)
@ -690,7 +690,7 @@ defp restrict_embedded_tag_any(_query, %{tag: _tag, skip_preload: true}) do
raise_on_missing_preload() raise_on_missing_preload()
end end
defp restrict_embedded_tag_any(query, %{tag: tag}) when is_list(tag) do defp restrict_embedded_tag_any(query, %{tag: [_ | _] = tag}) do
from( from(
[_activity, object] in query, [_activity, object] in query,
where: fragment("(?)->'tag' \\?| (?)", object.data, ^tag) where: fragment("(?)->'tag' \\?| (?)", object.data, ^tag)
@ -707,8 +707,7 @@ defp restrict_embedded_tag_reject_any(_query, %{tag_reject: _tag_reject, skip_pr
raise_on_missing_preload() raise_on_missing_preload()
end end
defp restrict_embedded_tag_reject_any(query, %{tag_reject: tag_reject}) defp restrict_embedded_tag_reject_any(query, %{tag_reject: [_ | _] = tag_reject}) do
when is_list(tag_reject) do
from( from(
[_activity, object] in query, [_activity, object] in query,
where: fragment("not (?)->'tag' \\?| (?)", object.data, ^tag_reject) where: fragment("not (?)->'tag' \\?| (?)", object.data, ^tag_reject)
@ -722,53 +721,24 @@ defp restrict_embedded_tag_reject_any(query, %{tag_reject: tag_reject})
defp restrict_embedded_tag_reject_any(query, _), do: query defp restrict_embedded_tag_reject_any(query, _), do: query
# Groups by all bindings to allow aggregation on hashtags
defp group_by_all_bindings(query) do
# Expecting named bindings: :object, :bookmark, :thread_mute, :report_note
cond do
Enum.count(query.aliases) == 4 ->
from([a, o, b3, b4, b5] in query, group_by: [a.id, o.id, b3.id, b4.id, b5.id])
Enum.count(query.aliases) == 3 ->
from([a, o, b3, b4] in query, group_by: [a.id, o.id, b3.id, b4.id])
Enum.count(query.aliases) == 2 ->
from([a, o, b3] in query, group_by: [a.id, o.id, b3.id])
true ->
from([a, o] in query, group_by: [a.id, o.id])
end
end
defp restrict_hashtag_reject_any(_query, %{tag_reject: _tag_reject, skip_preload: true}) do
raise_on_missing_preload()
end
defp restrict_hashtag_reject_any(query, %{tag_reject: tags_reject}) when is_list(tags_reject) do
query
|> group_by_all_bindings()
|> join(:left, [_activity, object], hashtag in assoc(object, :hashtags), as: :hashtag)
|> having(
[hashtag: hashtag],
fragment("not(array_agg(?) && (?))", hashtag.name, ^tags_reject)
)
end
defp restrict_hashtag_reject_any(query, %{tag_reject: tag_reject}) when is_binary(tag_reject) do
restrict_hashtag_reject_any(query, %{tag_reject: [tag_reject]})
end
defp restrict_hashtag_reject_any(query, _), do: query
defp restrict_hashtag_all(_query, %{tag_all: _tag, skip_preload: true}) do defp restrict_hashtag_all(_query, %{tag_all: _tag, skip_preload: true}) do
raise_on_missing_preload() raise_on_missing_preload()
end end
defp restrict_hashtag_all(query, %{tag_all: tags}) when is_list(tags) do defp restrict_hashtag_all(query, %{tag_all: [_ | _] = tags}) do
Enum.reduce( from(
tags, [_activity, object] in query,
query, where:
fn tag, acc -> restrict_hashtag_any(acc, %{tag: tag}) end fragment(
"""
(SELECT array_agg(hashtags.name) FROM hashtags JOIN hashtags_objects
ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
AND hashtags_objects.object_id = ?) @> ?
""",
^tags,
object.id,
^tags
)
) )
end end
@ -782,19 +752,20 @@ defp restrict_hashtag_any(_query, %{tag: _tag, skip_preload: true}) do
raise_on_missing_preload() raise_on_missing_preload()
end end
defp restrict_hashtag_any(query, %{tag: tags}) when is_list(tags) do defp restrict_hashtag_any(query, %{tag: [_ | _] = tags}) do
query =
from( from(
[_activity, object] in query, [_activity, object] in query,
join: hashtag in assoc(object, :hashtags), where:
where: hashtag.name in ^tags fragment(
"""
EXISTS (SELECT 1 FROM hashtags JOIN hashtags_objects
ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
AND hashtags_objects.object_id = ? LIMIT 1)
""",
^tags,
object.id
)
) )
if length(tags) > 1 do
distinct(query, [activity], true)
else
query
end
end end
defp restrict_hashtag_any(query, %{tag: tag}) when is_binary(tag) do defp restrict_hashtag_any(query, %{tag: tag}) when is_binary(tag) do
@ -803,6 +774,32 @@ defp restrict_hashtag_any(query, %{tag: tag}) when is_binary(tag) do
defp restrict_hashtag_any(query, _), do: query defp restrict_hashtag_any(query, _), do: query
defp restrict_hashtag_reject_any(_query, %{tag_reject: _tag_reject, skip_preload: true}) do
raise_on_missing_preload()
end
defp restrict_hashtag_reject_any(query, %{tag_reject: [_ | _] = tags_reject}) do
from(
[_activity, object] in query,
where:
fragment(
"""
NOT EXISTS (SELECT 1 FROM hashtags JOIN hashtags_objects
ON hashtags_objects.hashtag_id = hashtags.id WHERE hashtags.name = ANY(?)
AND hashtags_objects.object_id = ? LIMIT 1)
""",
^tags_reject,
object.id
)
)
end
defp restrict_hashtag_reject_any(query, %{tag_reject: tag_reject}) when is_binary(tag_reject) do
restrict_hashtag_reject_any(query, %{tag_reject: [tag_reject]})
end
defp restrict_hashtag_reject_any(query, _), do: query
defp raise_on_missing_preload do defp raise_on_missing_preload do
raise "Can't use the child object without preloading!" raise "Can't use the child object without preloading!"
end end

View file

@ -249,6 +249,17 @@ test "it fetches the appropriate tag-restricted posts" do
limit: 2 limit: 2
}) })
fetch_six =
ActivityPub.fetch_activities([], %{
type: "Create",
tag: ["any1", "any2"],
tag_all: [],
tag_reject: []
})
# Regression test: passing empty lists as filter options shouldn't affect the results
assert fetch_five == fetch_six
[fetch_one, fetch_two, fetch_three, fetch_four, fetch_five] = [fetch_one, fetch_two, fetch_three, fetch_four, fetch_five] =
Enum.map([fetch_one, fetch_two, fetch_three, fetch_four, fetch_five], fn statuses -> Enum.map([fetch_one, fetch_two, fetch_three, fetch_four, fetch_five], fn statuses ->
Enum.map(statuses, fn s -> Repo.preload(s, object: :hashtags) end) Enum.map(statuses, fn s -> Repo.preload(s, object: :hashtags) end)