From 2ec8cf566569912b767e15ab467cadd04fd1fd1c Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Sat, 2 Mar 2019 17:21:18 +0300 Subject: [PATCH] Add pagination to search --- lib/pleroma/user.ex | 110 ++++++++++-------- .../web/admin_api/admin_api_controller.ex | 66 +++++++---- .../admin_api/admin_api_controller_test.exs | 46 +++++++- 3 files changed, 152 insertions(+), 70 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 3c6fb4f9b..230155c33 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -547,11 +547,8 @@ def get_followers_query(%User{id: id, follower_address: follower_address}, nil) end def get_followers_query(user, page) do - from( - u in get_followers_query(user, nil), - limit: 20, - offset: ^((page - 1) * 20) - ) + from(u in get_followers_query(user, nil)) + |> paginate(page, 20) end def get_followers_query(user), do: get_followers_query(user, nil) @@ -577,11 +574,8 @@ def get_friends_query(%User{id: id, following: following}, nil) do end def get_friends_query(user, page) do - from( - u in get_friends_query(user, nil), - limit: 20, - offset: ^((page - 1) * 20) - ) + from(u in get_friends_query(user, nil)) + |> paginate(page, 20) end def get_friends_query(user), do: get_friends_query(user, nil) @@ -755,47 +749,64 @@ def get_recipients_from_activity(%Activity{recipients: to}) do Repo.all(query) end - def search(term, options \\ %{}) do - # Strip the beginning @ off if there is a query + @spec search_for_admin(binary(), %{ + admin: Pleroma.User.t(), + local: boolean(), + page: number(), + page_size: number() + }) :: {:ok, [Pleroma.User.t()], number()} + def search_for_admin(term, %{admin: admin, local: local, page: page, page_size: page_size}) do term = String.trim_leading(term, "@") - query = options[:query] || User - if options[:resolve], do: get_or_fetch(term) + local_paginated_query = + User + |> maybe_local_user_query(local) + |> paginate(page, page_size) - fts_results = - do_search(fts_search_subquery(term, query), options[:for_user], limit: options[:limit]) + search_query = fts_search_subquery(term, local_paginated_query) + + count = + term + |> fts_search_subquery() + |> maybe_local_user_query(local) + |> Repo.aggregate(:count, :id) + + {:ok, do_search(search_query, admin), count} + end + + @spec all_for_admin(number(), number()) :: {:ok, [Pleroma.User.t()], number()} + def all_for_admin(page, page_size) do + query = from(u in User, order_by: u.id) + + paginated_query = + query + |> paginate(page, page_size) + + count = + query + |> Repo.aggregate(:count, :id) + + {:ok, Repo.all(paginated_query), count} + end + + def search(query, resolve \\ false, for_user \\ nil) do + # Strip the beginning @ off if there is a query + query = String.trim_leading(query, "@") + + if resolve, do: get_or_fetch(query) + + fts_results = do_search(fts_search_subquery(query), for_user) {:ok, trigram_results} = Repo.transaction(fn -> Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - - do_search(trigram_search_subquery(term, query), options[:for_user], limit: options[:limit]) + do_search(trigram_search_subquery(query), for_user) end) Enum.uniq_by(fts_results ++ trigram_results, & &1.id) end - def all(page, page_size) do - from( - u in User, - limit: ^page_size, - offset: ^((page - 1) * page_size), - order_by: u.id - ) - |> Repo.all() - end - - def count_all_except_one(user) do - query = - from( - u in User, - where: u.id != ^user.id - ) - - Repo.aggregate(query, :count, :id) - end - - defp do_search(subquery, for_user, options) do + defp do_search(subquery, for_user, options \\ []) do q = from( s in subquery(subquery), @@ -811,7 +822,7 @@ defp do_search(subquery, for_user, options) do boost_search_results(results, for_user) end - defp fts_search_subquery(term, query) do + defp fts_search_subquery(term, query \\ User) do processed_query = term |> String.replace(~r/\W+/, " ") @@ -851,9 +862,9 @@ defp fts_search_subquery(term, query) do ) end - defp trigram_search_subquery(term, query) do + defp trigram_search_subquery(term) do from( - u in query, + u in User, select_merge: %{ search_rank: fragment( @@ -1020,13 +1031,13 @@ def unblock_domain(user, domain) do update_and_set_cache(cng) end - def maybe_local_user_query(local) do - if local, do: local_user_query(), else: User + def maybe_local_user_query(query, local) do + if local, do: local_user_query(query), else: query end - def local_user_query do + def local_user_query(query \\ User) do from( - u in User, + u in query, where: u.local == true, where: not is_nil(u.nickname) ) @@ -1318,4 +1329,11 @@ def all_superusers do ) |> Repo.all() end + + defp paginate(query, page, page_size) do + from(u in query, + limit: ^page_size, + offset: ^((page - 1) * page_size) + ) + end end diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 33f9689cd..aae02cab8 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -63,38 +63,40 @@ def untag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do do: json_response(conn, :no_content, "") end - def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do - with {page, _} <- Integer.parse(page_string), - users <- User.all(page, @users_page_size), - count <- User.count_all_except_one(admin), + def list_users(conn, params) do + {page, page_size} = page_params(params) + + with {:ok, users, count} <- User.all_for_admin(page, page_size), do: conn |> json( AccountView.render("index.json", users: users, count: count, - page_size: @users_page_size + page_size: page_size ) ) end - def search_users(%{assigns: %{user: admin}} = conn, %{"query" => term} = params) do - users = - User.search(term, - query: User.maybe_local_user_query(params["local"] == "true"), - resolve: true, - for_user: admin, - limit: @users_page_size - ) + def search_users(%{assigns: %{user: admin}} = conn, %{"query" => query} = params) do + {page, page_size} = page_params(params) - conn - |> json( - AccountView.render("index.json", - users: users, - count: length(users), - page_size: @users_page_size - ) - ) + with {:ok, users, count} <- + User.search_for_admin(query, %{ + admin: admin, + local: params["local"] == "true", + page: page, + page_size: page_size + }), + do: + conn + |> json( + AccountView.render("index.json", + users: users, + count: count, + page_size: page_size + ) + ) end def right_add(conn, %{"permission_group" => permission_group, "nickname" => nickname}) @@ -240,4 +242,26 @@ def errors(conn, _) do |> put_status(500) |> json("Something went wrong") end + + defp page_params(params) do + {get_page(params["page"]), get_page_size(params["page_size"])} + end + + defp get_page(page_string) when is_nil(page_string), do: 1 + + defp get_page(page_string) do + case Integer.parse(page_string) do + {page, _} -> page + :error -> 1 + end + end + + defp get_page_size(page_size_string) when is_nil(page_size_string), do: @users_page_size + + defp get_page_size(page_size_string) do + case Integer.parse(page_size_string) do + {page_size, _} -> page_size + :error -> @users_page_size + end + end end diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index a3042fa05..42e0daf8e 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -342,7 +342,7 @@ test "renders users array for the first page" do |> get("/api/pleroma/admin/users?page=1") assert json_response(conn, 200) == %{ - "count" => 1, + "count" => 2, "page_size" => 50, "users" => [ %{ @@ -369,7 +369,7 @@ test "renders empty array for the second page" do |> get("/api/pleroma/admin/users?page=2") assert json_response(conn, 200) == %{ - "count" => 1, + "count" => 2, "page_size" => 50, "users" => [] } @@ -416,9 +416,49 @@ test "regular search" do } end - test "only local users" do + test "regular search with page size" do admin = insert(:user, info: %{is_admin: true}) user = insert(:user, nickname: "bob") + user2 = insert(:user, nickname: "bo") + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo&page_size=1&page=1") + + assert json_response(conn, 200) == %{ + "count" => 2, + "page_size" => 1, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo&page_size=1&page=2") + + assert json_response(conn, 200) == %{ + "count" => 2, + "page_size" => 1, + "users" => [ + %{ + "deactivated" => user2.info.deactivated, + "id" => user2.id, + "nickname" => user2.nickname + } + ] + } + end + + test "only local users" do + admin = insert(:user, info: %{is_admin: true}, nickname: "john") + user = insert(:user, nickname: "bob") insert(:user, nickname: "bobb", local: false)