From ee4ed87fb47fa6c395e0f77b614f1630f3a12637 Mon Sep 17 00:00:00 2001 From: Maksim Date: Fri, 14 Jun 2019 11:39:57 +0000 Subject: [PATCH] [#948] /api/v1/account_search added optional parameters (limit, offset, following) --- lib/pleroma/user/search.ex | 58 +++++--- lib/pleroma/web/controller_helper.ex | 18 +++ .../mastodon_api/mastodon_api_controller.ex | 52 ------- .../web/mastodon_api/search_controller.ex | 79 +++++++++++ lib/pleroma/web/router.ex | 6 +- test/user_test.exs | 30 ++++ .../mastodon_api_controller_test.exs | 110 --------------- .../mastodon_api/search_controller_test.exs | 128 ++++++++++++++++++ 8 files changed, 300 insertions(+), 181 deletions(-) create mode 100644 lib/pleroma/web/mastodon_api/search_controller.ex create mode 100644 test/web/mastodon_api/search_controller_test.exs diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index f88dffa7b..ed06c2ab9 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -7,45 +7,69 @@ defmodule Pleroma.User.Search do alias Pleroma.User import Ecto.Query - def search(query, opts \\ []) do + @similarity_threshold 0.25 + @limit 20 + + def search(query_string, opts \\ []) do resolve = Keyword.get(opts, :resolve, false) + following = Keyword.get(opts, :following, false) + result_limit = Keyword.get(opts, :limit, @limit) + offset = Keyword.get(opts, :offset, 0) + for_user = Keyword.get(opts, :for_user) # Strip the beginning @ off if there is a query - query = String.trim_leading(query, "@") + query_string = String.trim_leading(query_string, "@") - maybe_resolve(resolve, for_user, query) + maybe_resolve(resolve, for_user, query_string) {:ok, results} = Repo.transaction(fn -> - Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) + Ecto.Adapters.SQL.query( + Repo, + "select set_limit(#{@similarity_threshold})", + [] + ) - query - |> search_query(for_user) + query_string + |> search_query(for_user, following) + |> paginate(result_limit, offset) |> Repo.all() end) results end - defp search_query(query, for_user) do - query - |> union_query() + defp search_query(query_string, for_user, following) do + for_user + |> base_query(following) + |> search_subqueries(query_string) + |> union_subqueries |> distinct_query() |> boost_search_rank_query(for_user) |> subquery() |> order_by(desc: :search_rank) - |> limit(20) |> maybe_restrict_local(for_user) end - defp union_query(query) do - fts_subquery = fts_search_subquery(query) - trigram_subquery = trigram_search_subquery(query) + defp base_query(_user, false), do: User + defp base_query(user, true), do: User.get_followers_query(user) + 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 + 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 @@ -102,7 +126,8 @@ defp boost_search_rank_query(query, for_user) do ) end - defp fts_search_subquery(term, query \\ User) do + @spec fts_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t() + defp fts_search_subquery(query, term) do processed_query = term |> String.replace(~r/\W+/, " ") @@ -144,9 +169,10 @@ defp fts_search_subquery(term, query \\ User) do |> User.restrict_deactivated() end - defp trigram_search_subquery(term) do + @spec trigram_search_subquery(User.t() | Ecto.Query.t(), String.t()) :: Ecto.Query.t() + defp trigram_search_subquery(query, term) do from( - u in User, + u in query, select_merge: %{ # ^1 gives 'Postgrex expected a binary, got 1' for some weird reason search_type: fragment("?", 1), diff --git a/lib/pleroma/web/controller_helper.ex b/lib/pleroma/web/controller_helper.ex index 55706eeb8..8a753bb4f 100644 --- a/lib/pleroma/web/controller_helper.ex +++ b/lib/pleroma/web/controller_helper.ex @@ -15,4 +15,22 @@ def json_response(conn, status, json) do |> put_status(status) |> json(json) end + + @spec fetch_integer_param(map(), String.t(), integer() | nil) :: integer() | nil + def fetch_integer_param(params, name, default \\ nil) do + params + |> Map.get(name, default) + |> param_to_integer(default) + end + + defp param_to_integer(val, _) when is_integer(val), do: val + + defp param_to_integer(val, default) when is_binary(val) do + case Integer.parse(val) do + {res, _} -> res + _ -> default + end + end + + defp param_to_integer(_, default), do: default end diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 46049dd24..84359eea6 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -1118,58 +1118,6 @@ def unsubscribe(%{assigns: %{user: user}} = conn, %{"id" => id}) do end end - def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do - accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) - statuses = Activity.search(user, query) - tags_path = Web.base_url() <> "/tag/" - - tags = - query - |> String.split() - |> Enum.uniq() - |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end) - |> Enum.map(fn tag -> String.slice(tag, 1..-1) end) - |> 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 = User.search(query, resolve: params["resolve"] == "true", for_user: user) - statuses = Activity.search(user, query) - - tags = - query - |> String.split() - |> Enum.uniq() - |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end) - |> Enum.map(fn tag -> String.slice(tag, 1..-1) 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 account_search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do - accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) - - res = AccountView.render("accounts.json", users: accounts, for: user, as: :user) - - json(conn, res) - end - def favourites(%{assigns: %{user: user}} = conn, params) do params = params diff --git a/lib/pleroma/web/mastodon_api/search_controller.ex b/lib/pleroma/web/mastodon_api/search_controller.ex new file mode 100644 index 000000000..0d1e2355d --- /dev/null +++ b/lib/pleroma/web/mastodon_api/search_controller.ex @@ -0,0 +1,79 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.SearchController do + use Pleroma.Web, :controller + alias Pleroma.Activity + alias Pleroma.User + alias Pleroma.Web + alias Pleroma.Web.MastodonAPI.AccountView + alias Pleroma.Web.MastodonAPI.StatusView + + alias Pleroma.Web.ControllerHelper + + require Logger + + plug(Pleroma.Plugs.RateLimiter, :search when action in [:search, :search2, :account_search]) + + def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do + accounts = User.search(query, search_options(params, user)) + statuses = Activity.search(user, query) + tags_path = Web.base_url() <> "/tag/" + + tags = + query + |> String.split() + |> Enum.uniq() + |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end) + |> Enum.map(fn tag -> String.slice(tag, 1..-1) end) + |> 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 = User.search(query, search_options(params, user)) + statuses = Activity.search(user, query) + + tags = + query + |> String.split() + |> Enum.uniq() + |> Enum.filter(fn tag -> String.starts_with?(tag, "#") end) + |> Enum.map(fn tag -> String.slice(tag, 1..-1) 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 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) + + json(conn, res) + end + + defp search_options(params, user) do + [ + resolve: params["resolve"] == "true", + following: params["following"] == "true", + limit: ControllerHelper.fetch_integer_param(params, "limit"), + offset: ControllerHelper.fetch_integer_param(params, "offset"), + for_user: user + ] + end +end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 1b37d6a93..17733a77b 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -412,7 +412,7 @@ defmodule Pleroma.Web.Router do get("/trends", MastodonAPIController, :empty_array) - get("/accounts/search", MastodonAPIController, :account_search) + get("/accounts/search", SearchController, :account_search) scope [] do pipe_through(:oauth_read_or_public) @@ -431,7 +431,7 @@ defmodule Pleroma.Web.Router do get("/accounts/:id/following", MastodonAPIController, :following) get("/accounts/:id", MastodonAPIController, :user) - get("/search", MastodonAPIController, :search) + get("/search", SearchController, :search) get("/pleroma/accounts/:id/favourites", MastodonAPIController, :user_favourites) end @@ -439,7 +439,7 @@ defmodule Pleroma.Web.Router do scope "/api/v2", Pleroma.Web.MastodonAPI do pipe_through([:api, :oauth_read_or_public]) - get("/search", MastodonAPIController, :search2) + get("/search", SearchController, :search2) end scope "/api", Pleroma.Web do diff --git a/test/user_test.exs b/test/user_test.exs index 473f545ff..a8176025c 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -1011,6 +1011,18 @@ test "User.delete() plugs any possible zombie objects" do end describe "User.search" do + test "accepts limit parameter" do + Enum.each(0..4, &insert(:user, %{nickname: "john#{&1}"})) + assert length(User.search("john", limit: 3)) == 3 + assert length(User.search("john")) == 5 + end + + test "accepts offset parameter" do + Enum.each(0..4, &insert(:user, %{nickname: "john#{&1}"})) + assert length(User.search("john", limit: 3)) == 3 + assert length(User.search("john", limit: 3, offset: 3)) == 2 + end + test "finds a user by full or partial nickname" do user = insert(:user, %{nickname: "john"}) @@ -1077,6 +1089,24 @@ test "finds users, boosting ranks of friends and followers" do Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) == [] end + test "finds followers of user by partial name" do + u1 = insert(:user) + u2 = insert(:user, %{name: "Jimi"}) + follower_jimi = insert(:user, %{name: "Jimi Hendrix"}) + follower_lizz = insert(:user, %{name: "Lizz Wright"}) + friend = insert(:user, %{name: "Jimi"}) + + {:ok, follower_jimi} = User.follow(follower_jimi, u1) + {:ok, _follower_lizz} = User.follow(follower_lizz, u2) + {:ok, u1} = User.follow(u1, friend) + + assert Enum.map(User.search("jimi", following: true, for_user: u1), & &1.id) == [ + follower_jimi.id + ] + + assert User.search("lizz", following: true, for_user: u1) == [] + end + test "find local and remote users for authenticated users" do u1 = insert(:user, %{name: "lain"}) u2 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 15d3fdb65..3558aa192 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -2134,116 +2134,6 @@ test "unimplemented follow_requests, blocks, domain blocks" do end) end - test "account search", %{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"}) - - results = - conn - |> assign(:user, user) - |> get("/api/v1/accounts/search", %{"q" => "shp"}) - |> json_response(200) - - result_ids = for result <- results, do: result["acct"] - - assert user_two.nickname in result_ids - assert user_three.nickname in result_ids - - results = - conn - |> assign(:user, user) - |> get("/api/v1/accounts/search", %{"q" => "2hu"}) - |> json_response(200) - - result_ids = for result <- results, do: result["acct"] - - assert user_three.nickname in result_ids - end - - test "search", %{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, activity} = CommonAPI.post(user, %{"status" => "This is about 2hu"}) - - {:ok, _activity} = - CommonAPI.post(user, %{ - "status" => "This is about 2hu, but private", - "visibility" => "private" - }) - - {:ok, _} = CommonAPI.post(user_two, %{"status" => "This isn't"}) - - conn = - conn - |> get("/api/v1/search", %{"q" => "2hu"}) - - assert results = json_response(conn, 200) - - [account | _] = results["accounts"] - assert account["id"] == to_string(user_three.id) - - assert results["hashtags"] == [] - - [status] = results["statuses"] - assert status["id"] == to_string(activity.id) - end - - test "search fetches remote statuses", %{conn: conn} do - capture_log(fn -> - conn = - conn - |> get("/api/v1/search", %{"q" => "https://shitposter.club/notice/2827873"}) - - assert results = json_response(conn, 200) - - [status] = results["statuses"] - assert status["uri"] == "tag:shitposter.club,2017-05-05:noticeId=2827873:objectType=comment" - end) - end - - test "search doesn't show statuses that it shouldn't", %{conn: conn} do - {:ok, activity} = - CommonAPI.post(insert(:user), %{ - "status" => "This is about 2hu, but private", - "visibility" => "private" - }) - - capture_log(fn -> - conn = - conn - |> get("/api/v1/search", %{"q" => Object.normalize(activity).data["id"]}) - - assert results = json_response(conn, 200) - - [] = results["statuses"] - end) - end - - test "search fetches remote accounts", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> assign(:user, user) - |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "true"}) - - assert results = json_response(conn, 200) - [account] = results["accounts"] - assert account["acct"] == "shp@social.heldscal.la" - end - - test "search doesn't fetch remote accounts if resolve is false", %{conn: conn} do - conn = - conn - |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "false"}) - - assert results = json_response(conn, 200) - assert [] == results["accounts"] - end - test "returns the favorites of a user", %{conn: conn} do user = insert(:user) other_user = insert(:user) diff --git a/test/web/mastodon_api/search_controller_test.exs b/test/web/mastodon_api/search_controller_test.exs new file mode 100644 index 000000000..c3f531590 --- /dev/null +++ b/test/web/mastodon_api/search_controller_test.exs @@ -0,0 +1,128 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do + use Pleroma.Web.ConnCase + + alias Pleroma.Object + alias Pleroma.Web.CommonAPI + import Pleroma.Factory + import ExUnit.CaptureLog + import Tesla.Mock + + setup do + mock(fn env -> apply(HttpRequestMock, :request, [env]) end) + :ok + end + + test "account search", %{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"}) + + results = + conn + |> assign(:user, user) + |> get("/api/v1/accounts/search", %{"q" => "shp"}) + |> json_response(200) + + result_ids = for result <- results, do: result["acct"] + + assert user_two.nickname in result_ids + assert user_three.nickname in result_ids + + results = + conn + |> assign(:user, user) + |> get("/api/v1/accounts/search", %{"q" => "2hu"}) + |> json_response(200) + + result_ids = for result <- results, do: result["acct"] + + assert user_three.nickname in result_ids + end + + test "search", %{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, activity} = CommonAPI.post(user, %{"status" => "This is about 2hu"}) + + {:ok, _activity} = + CommonAPI.post(user, %{ + "status" => "This is about 2hu, but private", + "visibility" => "private" + }) + + {:ok, _} = CommonAPI.post(user_two, %{"status" => "This isn't"}) + + conn = + conn + |> get("/api/v1/search", %{"q" => "2hu"}) + + assert results = json_response(conn, 200) + + [account | _] = results["accounts"] + assert account["id"] == to_string(user_three.id) + + assert results["hashtags"] == [] + + [status] = results["statuses"] + assert status["id"] == to_string(activity.id) + end + + test "search fetches remote statuses", %{conn: conn} do + capture_log(fn -> + conn = + conn + |> get("/api/v1/search", %{"q" => "https://shitposter.club/notice/2827873"}) + + assert results = json_response(conn, 200) + + [status] = results["statuses"] + assert status["uri"] == "tag:shitposter.club,2017-05-05:noticeId=2827873:objectType=comment" + end) + end + + test "search doesn't show statuses that it shouldn't", %{conn: conn} do + {:ok, activity} = + CommonAPI.post(insert(:user), %{ + "status" => "This is about 2hu, but private", + "visibility" => "private" + }) + + capture_log(fn -> + conn = + conn + |> get("/api/v1/search", %{"q" => Object.normalize(activity).data["id"]}) + + assert results = json_response(conn, 200) + + [] = results["statuses"] + end) + end + + test "search fetches remote accounts", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "true"}) + + assert results = json_response(conn, 200) + [account] = results["accounts"] + assert account["acct"] == "shp@social.heldscal.la" + end + + test "search doesn't fetch remote accounts if resolve is false", %{conn: conn} do + conn = + conn + |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "false"}) + + assert results = json_response(conn, 200) + assert [] == results["accounts"] + end +end