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

diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex
index 6fb2c2352f8aa9655f635ffc687ae40fb26f95ad..fb2f3fedbf627e79cbfe33014062697f8cfb7a19 100644 (file)
--- 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

diff --git a/test/user_search_test.exs b/test/user_search_test.exs
index f7ab312872c0c820a8919dd318513e227c47d6db..e413f08935e1cdb8ff0335669a94f5ac475e52e6 100644 (file)
--- a/test/user_search_test.exs
+++ b/test/user_search_test.exs
@@ -74,12 +74,6 @@ defmodule Pleroma.UserSearchTest do
       assert [u4.id, u3.id, u1.id] == Enum.map(User.search("lain@ple", for_user: u1), & &1.id)
     end
 
-    test "finds users, handling misspelled requests" do
-      u1 = insert(:user, %{name: "lain"})
-
-      assert [u1.id] == Enum.map(User.search("laiin"), & &1.id)
-    end
-
     test "finds users, boosting ranks of friends and followers" do
       u1 = insert(:user)
       u2 = insert(:user, %{name: "Doe"})
@@ -163,17 +157,6 @@ defmodule Pleroma.UserSearchTest do
       Pleroma.Config.put([:instance, :limit_to_local_content], :unauthenticated)
     end
 
-    test "finds a user whose name is nil" do
-      _user = insert(:user, %{name: "notamatch", nickname: "testuser@pleroma.amplifie.red"})
-      user_two = insert(:user, %{name: nil, nickname: "lain@pleroma.soykaf.com"})
-
-      assert user_two ==
-               User.search("lain@pleroma.soykaf.com")
-               |> List.first()
-               |> Map.put(:search_rank, nil)
-               |> Map.put(:search_type, nil)
-    end
-
     test "does not yield false-positive matches" do
       insert(:user, %{name: "John Doe"})