From: Eugenij <eugenijm@protonmail.com>
Date: Thu, 11 Jul 2019 13:55:31 +0000 (+0000)
Subject: Extend Pleroma.Pagination to support offset-based pagination, use async/await to... 
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=4198c3ac390edaab04a61a179b1f8bc5adaf89de;p=akkoma

Extend Pleroma.Pagination to support offset-based pagination, use async/await to execute status and account search in parallel
---

diff --git a/CHANGELOG.md b/CHANGELOG.md
index da2aee883..942733ab6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Metadata rendering errors resulting in the entire page being inaccessible
 - Mastodon API: Handling of search timeouts (`/api/v1/search` and `/api/v2/search`)
 - Mastodon API: Embedded relationships not being properly rendered in the Account entity of Status entity
+- Mastodon API: Add `account_id`, `type`, `offset`, and `limit` to search API (`/api/v1/search` and `/api/v2/search`)
 
 ### Added
 - MRF: Support for priming the mediaproxy cache (`Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy`)
diff --git a/config/test.exs b/config/test.exs
index 19d7cca5f..96ecf3592 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -65,7 +65,9 @@ config :pleroma, Pleroma.ScheduledActivity,
   total_user_limit: 3,
   enabled: false
 
-config :pleroma, :rate_limit, app_account_creation: {10_000, 5}
+config :pleroma, :rate_limit,
+  search: [{1000, 30}, {1000, 30}],
+  app_account_creation: {10_000, 5}
 
 config :pleroma, :http_security, report_uri: "https://endpoint.com"
 
diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex
index 6db41fe6e..46552c7be 100644
--- a/lib/pleroma/activity.ex
+++ b/lib/pleroma/activity.ex
@@ -344,5 +344,5 @@ defmodule Pleroma.Activity do
     )
   end
 
-  defdelegate search(user, query), to: Pleroma.Activity.Search
+  defdelegate search(user, query, options \\ []), to: Pleroma.Activity.Search
 end
diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex
index 0aa2aab23..0cc3770a7 100644
--- a/lib/pleroma/activity/search.ex
+++ b/lib/pleroma/activity/search.ex
@@ -5,14 +5,17 @@
 defmodule Pleroma.Activity.Search do
   alias Pleroma.Activity
   alias Pleroma.Object.Fetcher
-  alias Pleroma.Repo
+  alias Pleroma.Pagination
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.Visibility
 
   import Ecto.Query
 
-  def search(user, search_query) do
+  def search(user, search_query, options \\ []) do
     index_type = if Pleroma.Config.get([:database, :rum_enabled]), do: :rum, else: :gin
+    limit = Enum.min([Keyword.get(options, :limit), 40])
+    offset = Keyword.get(options, :offset, 0)
+    author = Keyword.get(options, :author)
 
     Activity
     |> Activity.with_preloaded_object()
@@ -20,15 +23,23 @@ defmodule Pleroma.Activity.Search do
     |> restrict_public()
     |> query_with(index_type, search_query)
     |> maybe_restrict_local(user)
-    |> Repo.all()
+    |> maybe_restrict_author(author)
+    |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => limit}, :offset)
     |> maybe_fetch(user, search_query)
   end
 
+  def maybe_restrict_author(query, %User{} = author) do
+    from([a, o] in query,
+      where: a.actor == ^author.ap_id
+    )
+  end
+
+  def maybe_restrict_author(query, _), do: query
+
   defp restrict_public(q) do
     from([a, o] in q,
       where: fragment("?->>'type' = 'Create'", a.data),
-      where: "https://www.w3.org/ns/activitystreams#Public" in a.recipients,
-      limit: 40
+      where: "https://www.w3.org/ns/activitystreams#Public" in a.recipients
     )
   end
 
diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex
index 3d7dd9e6a..2b869ccdc 100644
--- a/lib/pleroma/pagination.ex
+++ b/lib/pleroma/pagination.ex
@@ -14,16 +14,28 @@ defmodule Pleroma.Pagination do
 
   @default_limit 20
 
-  def fetch_paginated(query, params) do
+  def fetch_paginated(query, params, type \\ :keyset)
+
+  def fetch_paginated(query, params, :keyset) do
     options = cast_params(params)
 
     query
-    |> paginate(options)
+    |> paginate(options, :keyset)
     |> Repo.all()
     |> enforce_order(options)
   end
 
-  def paginate(query, options) do
+  def fetch_paginated(query, params, :offset) do
+    options = cast_params(params)
+
+    query
+    |> paginate(options, :offset)
+    |> Repo.all()
+  end
+
+  def paginate(query, options, method \\ :keyset)
+
+  def paginate(query, options, :keyset) do
     query
     |> restrict(:min_id, options)
     |> restrict(:since_id, options)
@@ -32,11 +44,18 @@ defmodule Pleroma.Pagination do
     |> restrict(:limit, options)
   end
 
+  def paginate(query, options, :offset) do
+    query
+    |> restrict(:offset, options)
+    |> restrict(:limit, options)
+  end
+
   defp cast_params(params) do
     param_types = %{
       min_id: :string,
       since_id: :string,
       max_id: :string,
+      offset: :integer,
       limit: :integer
     }
 
@@ -70,6 +89,10 @@ defmodule Pleroma.Pagination do
     order_by(query, [u], fragment("? desc nulls last", u.id))
   end
 
+  defp restrict(query, :offset, %{offset: offset}) do
+    offset(query, ^offset)
+  end
+
   defp restrict(query, :limit, options) do
     limit = Map.get(options, :limit, @default_limit)
 
diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex
index e0fc6daa6..46620b89a 100644
--- a/lib/pleroma/user/search.ex
+++ b/lib/pleroma/user/search.ex
@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.User.Search do
+  alias Pleroma.Pagination
   alias Pleroma.Repo
   alias Pleroma.User
   import Ecto.Query
@@ -32,8 +33,7 @@ defmodule Pleroma.User.Search do
 
         query_string
         |> search_query(for_user, following)
-        |> paginate(result_limit, offset)
-        |> Repo.all()
+        |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset)
       end)
 
     results
@@ -87,10 +87,6 @@ defmodule Pleroma.User.Search do
 
   defp filter_blocked_domains(query, _), do: query
 
-  defp paginate(query, limit, offset) do
-    from(q in query, limit: ^limit, offset: ^offset)
-  end
-
   defp union_subqueries({fts_subquery, trigram_subquery}) do
     from(s in trigram_subquery, union_all: ^fts_subquery)
   end
diff --git a/lib/pleroma/web/mastodon_api/search_controller.ex b/lib/pleroma/web/mastodon_api/search_controller.ex
index 939f7f6cb..9072aa7a4 100644
--- a/lib/pleroma/web/mastodon_api/search_controller.ex
+++ b/lib/pleroma/web/mastodon_api/search_controller.ex
@@ -7,6 +7,7 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
 
   alias Pleroma.Activity
   alias Pleroma.Plugs.RateLimiter
+  alias Pleroma.Repo
   alias Pleroma.User
   alias Pleroma.Web
   alias Pleroma.Web.ControllerHelper
@@ -16,43 +17,6 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
   require Logger
   plug(RateLimiter, :search when action in [:search, :search2, :account_search])
 
-  def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do
-    accounts = with_fallback(fn -> User.search(query, search_options(params, user)) end, [])
-    statuses = with_fallback(fn -> Activity.search(user, query) end, [])
-
-    tags_path = Web.base_url() <> "/tag/"
-
-    tags =
-      query
-      |> prepare_tags
-      |> Enum.map(fn tag -> %{name: tag, url: tags_path <> tag} end)
-
-    res = %{
-      "accounts" => AccountView.render("accounts.json", users: accounts, for: user, as: :user),
-      "statuses" =>
-        StatusView.render("index.json", activities: statuses, for: user, as: :activity),
-      "hashtags" => tags
-    }
-
-    json(conn, res)
-  end
-
-  def search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do
-    accounts = with_fallback(fn -> User.search(query, search_options(params, user)) end)
-    statuses = with_fallback(fn -> Activity.search(user, query) end)
-
-    tags = prepare_tags(query)
-
-    res = %{
-      "accounts" => AccountView.render("accounts.json", users: accounts, for: user, as: :user),
-      "statuses" =>
-        StatusView.render("index.json", activities: statuses, for: user, as: :activity),
-      "hashtags" => tags
-    }
-
-    json(conn, res)
-  end
-
   def account_search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do
     accounts = User.search(query, search_options(params, user))
     res = AccountView.render("accounts.json", users: accounts, for: user, as: :user)
@@ -60,12 +24,36 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
     json(conn, res)
   end
 
-  defp prepare_tags(query) do
-    query
-    |> String.split()
-    |> Enum.uniq()
-    |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end)
-    |> Enum.map(fn tag -> String.slice(tag, 1..-1) end)
+  def search2(conn, params), do: do_search(:v2, conn, params)
+  def search(conn, params), do: do_search(:v1, conn, params)
+
+  defp do_search(version, %{assigns: %{user: user}} = conn, %{"q" => query} = params) do
+    options = search_options(params, user)
+    timeout = Keyword.get(Repo.config(), :timeout, 15_000)
+    default_values = %{"statuses" => [], "accounts" => [], "hashtags" => []}
+
+    result =
+      default_values
+      |> Enum.map(fn {resource, default_value} ->
+        if params["type"] == nil or params["type"] == resource do
+          {resource, fn -> resource_search(version, resource, query, options) end}
+        else
+          {resource, fn -> default_value end}
+        end
+      end)
+      |> Task.async_stream(fn {resource, f} -> {resource, with_fallback(f)} end,
+        timeout: timeout,
+        on_timeout: :kill_task
+      )
+      |> Enum.reduce(default_values, fn
+        {:ok, {resource, result}}, acc ->
+          Map.put(acc, resource, result)
+
+        _error, acc ->
+          acc
+      end)
+
+    json(conn, result)
   end
 
   defp search_options(params, user) do
@@ -74,8 +62,45 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
       following: params["following"] == "true",
       limit: ControllerHelper.fetch_integer_param(params, "limit"),
       offset: ControllerHelper.fetch_integer_param(params, "offset"),
+      type: params["type"],
+      author: get_author(params),
       for_user: user
     ]
+    |> Enum.filter(&elem(&1, 1))
+  end
+
+  defp resource_search(_, "accounts", query, options) do
+    accounts = with_fallback(fn -> User.search(query, options) end)
+    AccountView.render("accounts.json", users: accounts, for: options[:for_user], as: :user)
+  end
+
+  defp resource_search(_, "statuses", query, options) do
+    statuses = with_fallback(fn -> Activity.search(options[:for_user], query, options) end)
+    StatusView.render("index.json", activities: statuses, for: options[:for_user], as: :activity)
+  end
+
+  defp resource_search(:v2, "hashtags", query, _options) do
+    tags_path = Web.base_url() <> "/tag/"
+
+    query
+    |> prepare_tags()
+    |> Enum.map(fn tag ->
+      tag = String.trim_leading(tag, "#")
+      %{name: tag, url: tags_path <> tag}
+    end)
+  end
+
+  defp resource_search(:v1, "hashtags", query, _options) do
+    query
+    |> prepare_tags()
+    |> Enum.map(fn tag -> String.trim_leading(tag, "#") end)
+  end
+
+  defp prepare_tags(query) do
+    query
+    |> String.split()
+    |> Enum.uniq()
+    |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end)
   end
 
   defp with_fallback(f, fallback \\ []) do
@@ -87,4 +112,9 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
         fallback
     end
   end
+
+  defp get_author(%{"account_id" => account_id}) when is_binary(account_id),
+    do: User.get_cached_by_id(account_id)
+
+  defp get_author(_params), do: nil
 end
diff --git a/test/web/mastodon_api/search_controller_test.exs b/test/web/mastodon_api/search_controller_test.exs
index ea534b393..9f50c09f4 100644
--- a/test/web/mastodon_api/search_controller_test.exs
+++ b/test/web/mastodon_api/search_controller_test.exs
@@ -22,7 +22,7 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
     test "it returns empty result if user or status search return undefined error", %{conn: conn} do
       with_mocks [
         {Pleroma.User, [], [search: fn _q, _o -> raise "Oops" end]},
-        {Pleroma.Activity, [], [search: fn _u, _q -> raise "Oops" end]}
+        {Pleroma.Activity, [], [search: fn _u, _q, _o -> raise "Oops" end]}
       ] do
         conn = get(conn, "/api/v2/search", %{"q" => "2hu"})
 
@@ -51,7 +51,6 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
       conn = get(conn, "/api/v2/search", %{"q" => "2hu #private"})
 
       assert results = json_response(conn, 200)
-      # IO.inspect results
 
       [account | _] = results["accounts"]
       assert account["id"] == to_string(user_three.id)
@@ -98,7 +97,7 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
     test "it returns empty result if user or status search return undefined error", %{conn: conn} do
       with_mocks [
         {Pleroma.User, [], [search: fn _q, _o -> raise "Oops" end]},
-        {Pleroma.Activity, [], [search: fn _u, _q -> raise "Oops" end]}
+        {Pleroma.Activity, [], [search: fn _u, _q, _o -> raise "Oops" end]}
       ] do
         conn =
           conn
@@ -195,5 +194,74 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
       assert results = json_response(conn, 200)
       assert [] == results["accounts"]
     end
+
+    test "search with limit and offset", %{conn: conn} do
+      user = insert(:user)
+      _user_two = insert(:user, %{nickname: "shp@shitposter.club"})
+      _user_three = insert(:user, %{nickname: "shp@heldscal.la", name: "I love 2hu"})
+
+      {:ok, _activity1} = CommonAPI.post(user, %{"status" => "This is about 2hu"})
+      {:ok, _activity2} = CommonAPI.post(user, %{"status" => "This is also about 2hu"})
+
+      result =
+        conn
+        |> get("/api/v1/search", %{"q" => "2hu", "limit" => 1})
+
+      assert results = json_response(result, 200)
+      assert [%{"id" => activity_id1}] = results["statuses"]
+      assert [_] = results["accounts"]
+
+      results =
+        conn
+        |> get("/api/v1/search", %{"q" => "2hu", "limit" => 1, "offset" => 1})
+        |> json_response(200)
+
+      assert [%{"id" => activity_id2}] = results["statuses"]
+      assert [] = results["accounts"]
+
+      assert activity_id1 != activity_id2
+    end
+
+    test "search returns results only for the given type", %{conn: conn} do
+      user = insert(:user)
+      _user_two = insert(:user, %{nickname: "shp@heldscal.la", name: "I love 2hu"})
+
+      {:ok, _activity} = CommonAPI.post(user, %{"status" => "This is about 2hu"})
+
+      assert %{"statuses" => [_activity], "accounts" => [], "hashtags" => []} =
+               conn
+               |> get("/api/v1/search", %{"q" => "2hu", "type" => "statuses"})
+               |> json_response(200)
+
+      assert %{"statuses" => [], "accounts" => [_user_two], "hashtags" => []} =
+               conn
+               |> get("/api/v1/search", %{"q" => "2hu", "type" => "accounts"})
+               |> json_response(200)
+    end
+
+    test "search uses account_id to filter statuses by the author", %{conn: conn} do
+      user = insert(:user, %{nickname: "shp@shitposter.club"})
+      user_two = insert(:user, %{nickname: "shp@heldscal.la", name: "I love 2hu"})
+
+      {:ok, activity1} = CommonAPI.post(user, %{"status" => "This is about 2hu"})
+      {:ok, activity2} = CommonAPI.post(user_two, %{"status" => "This is also about 2hu"})
+
+      results =
+        conn
+        |> get("/api/v1/search", %{"q" => "2hu", "account_id" => user.id})
+        |> json_response(200)
+
+      assert [%{"id" => activity_id1}] = results["statuses"]
+      assert activity_id1 == activity1.id
+      assert [_] = results["accounts"]
+
+      results =
+        conn
+        |> get("/api/v1/search", %{"q" => "2hu", "account_id" => user_two.id})
+        |> json_response(200)
+
+      assert [%{"id" => activity_id2}] = results["statuses"]
+      assert activity_id2 == activity2.id
+    end
   end
 end