From 380d0cce6b802baf4d13031a4a39f169dd65bffd Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 29 Jan 2021 00:17:33 +0300 Subject: [PATCH] [#3213] Reinstated DISTINCT clause for hashtag "any" filtering with 2+ terms. Added test. --- lib/pleroma/web/activity_pub/activity_pub.ex | 17 ++++++++++++----- .../controllers/timeline_controller.ex | 2 +- .../web/activity_pub/activity_pub_test.exs | 17 +++++++++++++++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index be81e0833..0a21ac2f2 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -846,11 +846,18 @@ defp restrict_hashtag_any(_query, %{tag: _tag, skip_preload: true}) do end defp restrict_hashtag_any(query, %{tag: tags}) when is_list(tags) do - from( - [_activity, object] in query, - join: hashtag in assoc(object, :hashtags), - where: hashtag.name in ^tags - ) + query = + from( + [_activity, object] in query, + join: hashtag in assoc(object, :hashtags), + where: hashtag.name in ^tags + ) + + if length(tags) > 1 do + distinct(query, [activity], true) + else + query + end end defp restrict_hashtag_any(query, %{tag: tag}) when is_binary(tag) do diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 08e6f23b9..1fb954a9b 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -134,9 +134,9 @@ defp hashtag_fetching(params, user, local_only) do tags = [params[:tag], params[:any]] |> List.flatten() - |> Enum.uniq() |> Enum.reject(&is_nil/1) |> Enum.map(&String.downcase/1) + |> Enum.uniq() tag_all = params diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 1fcaf74d3..0b18269cd 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -217,6 +217,9 @@ test "it fetches the appropriate tag-restricted posts" do {:ok, status_two} = CommonAPI.post(user, %{status: ". #essais"}) {:ok, status_three} = CommonAPI.post(user, %{status: ". #test #reject"}) + {:ok, status_four} = CommonAPI.post(user, %{status: ". #any1 #any2"}) + {:ok, status_five} = CommonAPI.post(user, %{status: ". #any2 #any1"}) + for hashtag_timeline_strategy <- [true, :prefer_aggregation, false] do clear_config([:instance, :improved_hashtag_timeline], hashtag_timeline_strategy) @@ -238,8 +241,17 @@ test "it fetches the appropriate tag-restricted posts" do tag_all: ["test", "reject"] }) - [fetch_one, fetch_two, fetch_three, fetch_four] = - Enum.map([fetch_one, fetch_two, fetch_three, fetch_four], fn statuses -> + # This test would fail if JOIN with 2+ terms in "any" clause is done without DISTINCT. + # The :limit is important (w/o DISTINCT 2 records are deduped by Ecto to 1 b/c of preload). + fetch_five = + ActivityPub.fetch_activities([], %{ + type: "Create", + tag: ["any1", "any2"], + limit: 2 + }) + + [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(statuses, fn s -> Repo.preload(s, object: :hashtags) end) end) @@ -247,6 +259,7 @@ test "it fetches the appropriate tag-restricted posts" do assert fetch_two == [status_one, status_two, status_three] assert fetch_three == [status_one, status_two] assert fetch_four == [status_three] + assert fetch_five == [status_four, status_five] end end