Merge branch 'release/1.1.1' into 'stable'

1.1.1 Release See merge request pleroma/pleroma!1857
2019-10-18 12:08:03 +00:00 · 2019-10-18 12:08:03 +00:00 · bb73f100b8
parent c00797d08e c00b55bc80
commit bb73f100b8
11 changed files with 115 additions and 139 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file.

 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

+## [1.1.1] - 2019-10-18
+### Fixed
+- One of the migrations between 1.0.0 and 1.1.0 wiping user info of the relay user because of unexpected behavior of postgresql's `jsonb_set`, resulting in inability to post in the default configuration. If you were affected, please run the following query in postgres console, the relay user will be recreated automatically:
+```
+delete from users where ap_id = 'https://your.instance.hostname/relay';
+```
+- Bad user search matches
+
 ## [1.1.0] - 2019-10-14
 **Breaking:** The stable branch has been changed from `master` to `stable`. If you want to keep using 1.0, the `release/1.0` branch will receive security updates for 6 months after 1.1 release.

--- 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
            )
        ]
--- 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,
--- 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
            )
--- a/lib/pleroma/user/search.ex
+++ b/lib/pleroma/user/search.ex
@ -4,11 +4,9 @@

 defmodule Pleroma.User.Search do
  alias Pleroma.Pagination
-  alias Pleroma.Repo
  alias Pleroma.User
  import Ecto.Query

-  @similarity_threshold 0.25
  @limit 20

  def search(query_string, opts \\ []) do
@ -23,18 +21,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 +46,65 @@ 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_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_search(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,
+      where:
+        fragment(
+          """
+          (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 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)

@ -87,21 +127,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 +151,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 +162,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 +179,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
--- 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]
      )

--- a/mix.exs
+++ b/mix.exs
@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do
  def project do
    [
      app: :pleroma,
-      version: version("1.1.0"),
+      version: version("1.1.1"),
      elixir: "~> 1.7",
      elixirc_paths: elixirc_paths(Mix.env()),
      compilers: [:phoenix, :gettext] ++ Mix.compilers(),
--- a/priv/repo/migrations/20190711042021_create_safe_jsonb_set.exs
+++ b/priv/repo/migrations/20190711042021_create_safe_jsonb_set.exs
@ -0,0 +1,22 @@
+defmodule Pleroma.Repo.Migrations.CreateSafeJsonbSet do
+  use Ecto.Migration
+  alias Pleroma.User
+
+  def change do
+  execute("""
+  create or replace function safe_jsonb_set(target jsonb, path text[], new_value jsonb, create_missing boolean default true) returns jsonb as $$
+  declare
+    result jsonb;
+  begin
+    result := jsonb_set(target, path, coalesce(new_value, 'null'::jsonb), create_missing);
+    if result is NULL then
+      raise 'jsonb_set tried to wipe the object, please report this incindent to Pleroma bug tracker. https://git.pleroma.social/pleroma/pleroma/issues/new';
+      return target;
+    else
+      return result;
+    end if;
+  end;
+  $$ language plpgsql;
+  """)
+  end
+end
--- a/priv/repo/migrations/20190711042024_copy_muted_to_muted_notifications.exs
+++ b/priv/repo/migrations/20190711042024_copy_muted_to_muted_notifications.exs
@ -3,6 +3,6 @@ defmodule Pleroma.Repo.Migrations.CopyMutedToMutedNotifications do
  alias Pleroma.User

  def change do
-  execute("update users set info = jsonb_set(info, '{muted_notifications}', info->'mutes', true) where local = true")
+  execute("update users set info = safe_jsonb_set(info, '{muted_notifications}', info->'mutes', true) where local = true")
  end
 end
--- a/test/safe_jsonb_set_test.exs
+++ b/test/safe_jsonb_set_test.exs
@ -0,0 +1,12 @@
+defmodule Pleroma.SafeJsonbSetTest do
+  use Pleroma.DataCase
+
+  test "it doesn't wipe the object when asked to set the value to NULL" do
+    assert %{rows: [[%{"key" => "value", "test" => nil}]]} =
+             Ecto.Adapters.SQL.query!(
+               Pleroma.Repo,
+               "select safe_jsonb_set('{\"key\": \"value\"}'::jsonb, '{test}', NULL);",
+               []
+             )
+  end
+end
--- a/test/user_search_test.exs
+++ b/test/user_search_test.exs
@ -65,21 +65,6 @@ defmodule Pleroma.UserSearchTest do
      assert [u2.id, u1.id] == Enum.map(User.search("bar word"), & &1.id)
    end

-    test "finds users, ranking by similarity" do
-      u1 = insert(:user, %{name: "lain"})
-      _u2 = insert(:user, %{name: "ean"})
-      u3 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social"})
-      u4 = insert(:user, %{nickname: "lain@pleroma.soykaf.com"})
-
-      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 +148,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"})