From a1c4a5d7cf99247ef9964fb530a6d2d46468dc82 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 18 Oct 2019 14:11:30 +0300 Subject: Fix a migration wiping user info of users that don't have any mutes And introduce safe_jsonb_set --- lib/mix/tasks/pleroma/database.ex | 4 ++-- lib/pleroma/object.ex | 4 ++-- lib/pleroma/user.ex | 6 +++--- lib/pleroma/web/activity_pub/utils.ex | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/mix/tasks/pleroma/database.ex b/lib/mix/tasks/pleroma/database.ex index bcc2052d6..ab7529e08 100644 --- a/lib/mix/tasks/pleroma/database.ex +++ b/lib/mix/tasks/pleroma/database.ex @@ -54,7 +54,7 @@ defmodule Mix.Tasks.Pleroma.Database do Logger.info("Removing embedded objects") Repo.query!( - "update activities set data = jsonb_set(data, '{object}'::text[], data->'object'->'id') where data->'object'->>'id' is not null;", + "update activities set data = safe_jsonb_set(data, '{object}'::text[], data->'object'->'id') where data->'object'->>'id' is not null;", [], timeout: :infinity ) @@ -152,7 +152,7 @@ defmodule Mix.Tasks.Pleroma.Database do set: [ data: fragment( - "jsonb_set(?, '{likes}', '[]'::jsonb, true)", + "safe_jsonb_set(?, '{likes}', '[]'::jsonb, true)", object.data ) ] diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 3fa407931..bf37b28a7 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -181,7 +181,7 @@ defmodule Pleroma.Object do data: fragment( """ - jsonb_set(?, '{repliesCount}', + safe_jsonb_set(?, '{repliesCount}', (coalesce((?->>'repliesCount')::int, 0) + 1)::varchar::jsonb, true) """, o.data, @@ -204,7 +204,7 @@ defmodule Pleroma.Object do data: fragment( """ - jsonb_set(?, '{repliesCount}', + safe_jsonb_set(?, '{repliesCount}', (greatest(0, (?->>'repliesCount')::int - 1))::varchar::jsonb, true) """, o.data, diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 617f160e0..f0912fb10 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -718,7 +718,7 @@ defmodule Pleroma.User do set: [ info: fragment( - "jsonb_set(?, '{note_count}', ((?->>'note_count')::int + 1)::varchar::jsonb, true)", + "safe_jsonb_set(?, '{note_count}', ((?->>'note_count')::int + 1)::varchar::jsonb, true)", u.info, u.info ) @@ -739,7 +739,7 @@ defmodule Pleroma.User do set: [ info: fragment( - "jsonb_set(?, '{note_count}', (greatest(0, (?->>'note_count')::int - 1))::varchar::jsonb, true)", + "safe_jsonb_set(?, '{note_count}', (greatest(0, (?->>'note_count')::int - 1))::varchar::jsonb, true)", u.info, u.info ) @@ -812,7 +812,7 @@ defmodule Pleroma.User do set: [ info: fragment( - "jsonb_set(?, '{follower_count}', ?::varchar::jsonb, true)", + "safe_jsonb_set(?, '{follower_count}', ?::varchar::jsonb, true)", u.info, s.count ) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 39a532db3..f22cc2367 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -349,7 +349,7 @@ defmodule Pleroma.Web.ActivityPub.Utils do try do Ecto.Adapters.SQL.query!( Repo, - "UPDATE activities SET data = jsonb_set(data, '{state}', $1) WHERE data->>'type' = 'Follow' AND data->>'actor' = $2 AND data->>'object' = $3 AND data->>'state' = 'pending'", + "UPDATE activities SET data = safe_jsonb_set(data, '{state}', $1) WHERE data->>'type' = 'Follow' AND data->>'actor' = $2 AND data->>'object' = $3 AND data->>'state' = 'pending'", [state, actor, object] ) -- cgit v1.2.3 From 713b2187b93ebda5533e67173d82745eb15ba650 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 16 Oct 2019 12:52:47 +0300 Subject: User search: Remove trigram and refactor the module - Remove trigram as it tends to rank garbage results highly, resulting in it prioritized above fts, which gives actually decent results. ACKed by kaniini and lain on irc. - Remove a test for handling misspelled requests, since we no longer have trigram - Remove a test for searching users with `nil` display names, because it is unrealistic, we don't accept usernames that are not >1 char strings - Make rank boosting for followers/followees sane again, previous values resulted in garbage matches getting on top just because the users are followers/followees --- lib/pleroma/user/search.ex | 152 +++++++++++++++------------------------------ 1 file changed, 50 insertions(+), 102 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index 6fb2c2352..fb2f3fedb 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -4,7 +4,6 @@ defmodule Pleroma.User.Search do alias Pleroma.Pagination - alias Pleroma.Repo alias Pleroma.User import Ecto.Query @@ -23,18 +22,10 @@ defmodule Pleroma.User.Search do maybe_resolve(resolve, for_user, query_string) - {:ok, results} = - Repo.transaction(fn -> - Ecto.Adapters.SQL.query( - Repo, - "select set_limit(#{@similarity_threshold})", - [] - ) - - query_string - |> search_query(for_user, following) - |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset) - end) + results = + query_string + |> search_query(for_user, following) + |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset) results end @@ -56,15 +47,52 @@ defmodule Pleroma.User.Search do |> base_query(following) |> filter_blocked_user(for_user) |> filter_blocked_domains(for_user) - |> search_subqueries(query_string) - |> union_subqueries - |> distinct_query() - |> boost_search_rank_query(for_user) + |> fts_subquery(query_string) |> subquery() + |> where([u], u.search_rank > @similarity_threshold) + |> boost_search_rank(for_user) |> order_by(desc: :search_rank) |> maybe_restrict_local(for_user) end + @nickname_regex ~r/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~\-@]+$/ + defp fts_subquery(query, query_string) do + {nickname_weight, name_weight} = + if String.match?(query_string, @nickname_regex) do + {"A", "B"} + else + {"B", "A"} + end + + query_string = to_tsquery(query_string) + + from( + u in query, + select_merge: %{ + search_rank: + fragment( + """ + ts_rank_cd((setweight(to_tsvector('simple', ?), ?) || setweight(to_tsvector('simple', ?), ?)), to_tsquery('simple', ?)) + """, + u.name, + ^name_weight, + u.nickname, + ^nickname_weight, + ^query_string + ) + } + ) + end + + defp to_tsquery(query_string) do + String.trim_trailing(query_string, "@" <> local_domain()) + |> String.replace(~r/[!-\/|@|[-`|{-~|:-?]+/, " ") + |> String.trim() + |> String.split() + |> Enum.map(&(&1 <> ":*")) + |> Enum.join(" | ") + end + defp base_query(_user, false), do: User defp base_query(user, true), do: User.get_followers_query(user) @@ -87,21 +115,6 @@ defmodule Pleroma.User.Search do defp filter_blocked_domains(query, _), do: query - defp union_subqueries({fts_subquery, trigram_subquery}) do - from(s in trigram_subquery, union_all: ^fts_subquery) - end - - defp search_subqueries(base_query, query_string) do - { - fts_search_subquery(base_query, query_string), - trigram_search_subquery(base_query, query_string) - } - end - - defp distinct_query(q) do - from(s in subquery(q), order_by: s.search_type, distinct: s.id) - end - defp maybe_resolve(true, user, query) do case {limit(), user} do {:all, _} -> :noop @@ -126,9 +139,9 @@ defmodule Pleroma.User.Search do defp restrict_local(q), do: where(q, [u], u.local == true) - defp boost_search_rank_query(query, nil), do: query + defp local_domain, do: Pleroma.Config.get([Pleroma.Web.Endpoint, :url, :host]) - defp boost_search_rank_query(query, for_user) do + defp boost_search_rank(query, %User{} = for_user) do friends_ids = User.get_friends_ids(for_user) followers_ids = User.get_followers_ids(for_user) @@ -137,8 +150,8 @@ defmodule Pleroma.User.Search do search_rank: fragment( """ - CASE WHEN (?) THEN 0.5 + (?) * 1.3 - WHEN (?) THEN 0.5 + (?) * 1.2 + CASE WHEN (?) THEN (?) * 1.5 + WHEN (?) THEN (?) * 1.3 WHEN (?) THEN (?) * 1.1 ELSE (?) END """, @@ -154,70 +167,5 @@ defmodule Pleroma.User.Search do ) end - @spec fts_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t() - defp fts_search_subquery(query, term) do - processed_query = - String.trim_trailing(term, "@" <> local_domain()) - |> String.replace(~r/[!-\/|@|[-`|{-~|:-?]+/, " ") - |> String.trim() - |> String.split() - |> Enum.map(&(&1 <> ":*")) - |> Enum.join(" | ") - - from( - u in query, - select_merge: %{ - search_type: ^0, - search_rank: - fragment( - """ - ts_rank_cd( - setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || - setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B'), - to_tsquery('simple', ?), - 32 - ) - """, - u.nickname, - u.name, - ^processed_query - ) - }, - where: - fragment( - """ - (setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || - setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B')) @@ to_tsquery('simple', ?) - """, - u.nickname, - u.name, - ^processed_query - ) - ) - |> User.restrict_deactivated() - end - - @spec trigram_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t() - defp trigram_search_subquery(query, term) do - term = String.trim_trailing(term, "@" <> local_domain()) - - from( - u in query, - select_merge: %{ - # ^1 gives 'Postgrex expected a binary, got 1' for some weird reason - search_type: fragment("?", 1), - search_rank: - fragment( - "similarity(?, trim(? || ' ' || coalesce(?, '')))", - ^term, - u.nickname, - u.name - ) - }, - where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^term) - ) - |> User.restrict_deactivated() - end - - defp local_domain, do: Pleroma.Config.get([Pleroma.Web.Endpoint, :url, :host]) + defp boost_search_rank(query, _for_user), do: query end -- cgit v1.2.3 From 7a00acb3e49f441d975ff509efabb0117875c61e Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 16 Oct 2019 13:49:33 +0300 Subject: Order fts results by trigram --- lib/pleroma/user/search.ex | 48 +++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index fb2f3fedb..0d697fe3d 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -7,7 +7,6 @@ defmodule Pleroma.User.Search do alias Pleroma.User import Ecto.Query - @similarity_threshold 0.25 @limit 20 def search(query_string, opts \\ []) do @@ -47,16 +46,16 @@ defmodule Pleroma.User.Search do |> base_query(following) |> filter_blocked_user(for_user) |> filter_blocked_domains(for_user) - |> fts_subquery(query_string) - |> subquery() - |> where([u], u.search_rank > @similarity_threshold) + |> fts_search(query_string) + |> trigram_rank(query_string) |> boost_search_rank(for_user) + |> subquery() |> order_by(desc: :search_rank) |> maybe_restrict_local(for_user) end @nickname_regex ~r/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~\-@]+$/ - defp fts_subquery(query, query_string) do + defp fts_search(query, query_string) do {nickname_weight, name_weight} = if String.match?(query_string, @nickname_regex) do {"A", "B"} @@ -68,19 +67,17 @@ defmodule Pleroma.User.Search do from( u in query, - select_merge: %{ - search_rank: - fragment( - """ - ts_rank_cd((setweight(to_tsvector('simple', ?), ?) || setweight(to_tsvector('simple', ?), ?)), to_tsquery('simple', ?)) - """, - u.name, - ^name_weight, - u.nickname, - ^nickname_weight, - ^query_string - ) - } + where: + fragment( + """ + (setweight(to_tsvector('simple', ?), ?) || setweight(to_tsvector('simple', ?), ?)) @@ to_tsquery('simple', ?) + """, + u.name, + ^name_weight, + u.nickname, + ^nickname_weight, + ^query_string + ) ) end @@ -93,6 +90,21 @@ defmodule Pleroma.User.Search do |> Enum.join(" | ") end + defp trigram_rank(query, query_string) do + from( + u in query, + select_merge: %{ + search_rank: + fragment( + "similarity(?, trim(? || ' ' || coalesce(?, '')))", + ^query_string, + u.nickname, + u.name + ) + } + ) + end + defp base_query(_user, false), do: User defp base_query(user, true), do: User.get_followers_query(user) -- cgit v1.2.3