From a4c5f71e933c905433b80c90bcd626e7da703669 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Mon, 2 Sep 2019 22:48:52 +0300 Subject: [PATCH] Return total from pagination + tests --- CHANGELOG.md | 1 + lib/pleroma/activity/search.ex | 1 + lib/pleroma/conversation/participation.ex | 1 + lib/pleroma/notification.ex | 1 + lib/pleroma/pagination.ex | 24 ++++-- lib/pleroma/user/search.ex | 1 + lib/pleroma/web/activity_pub/activity_pub.ex | 3 + .../controllers/mastodon_api_controller.ex | 2 + lib/pleroma/web/mastodon_api/mastodon_api.ex | 4 + test/pagination_test.exs | 78 +++++++++++++++++++ 10 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 test/pagination_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4acb749ac..06ad303de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Unsubscribe followers when they unfollow a user - AdminAPI: Add "godmode" while fetching user statuses (i.e. admin can see private statuses) - Improve digest email template +– Pagination: return `total` alongside with `items` when paginating ### Fixed - Following from Osada diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex index f847ac238..f7156c81c 100644 --- a/lib/pleroma/activity/search.ex +++ b/lib/pleroma/activity/search.ex @@ -27,6 +27,7 @@ def search(user, search_query, options \\ []) do |> maybe_restrict_local(user) |> maybe_restrict_author(author) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => limit}, :offset) + |> Map.get(:items) |> maybe_fetch(user, search_query) end diff --git a/lib/pleroma/conversation/participation.ex b/lib/pleroma/conversation/participation.ex index ea5b9fe17..5fd8d3d41 100644 --- a/lib/pleroma/conversation/participation.ex +++ b/lib/pleroma/conversation/participation.ex @@ -67,6 +67,7 @@ def for_user(user, params \\ %{}) do preload: [conversation: [:users]] ) |> Pleroma.Pagination.fetch_paginated(params) + |> Map.get(:items) end def for_user_and_conversation(user, conversation) do diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 5d29af853..3e4ddd2ba 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -75,6 +75,7 @@ def for_user(user, opts \\ %{}) do user |> for_user_query(opts) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) end @doc """ diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 2b869ccdc..d21ecf628 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -18,19 +18,29 @@ def fetch_paginated(query, params, type \\ :keyset) def fetch_paginated(query, params, :keyset) do options = cast_params(params) + total = Repo.aggregate(query, :count, :id) - query - |> paginate(options, :keyset) - |> Repo.all() - |> enforce_order(options) + %{ + total: total, + items: + query + |> paginate(options, :keyset) + |> Repo.all() + |> enforce_order(options) + } end def fetch_paginated(query, params, :offset) do options = cast_params(params) + total = Repo.aggregate(query, :count, :id) - query - |> paginate(options, :offset) - |> Repo.all() + %{ + total: total, + items: + query + |> paginate(options, :offset) + |> Repo.all() + } end def paginate(query, options, method \\ :keyset) diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index 6fb2c2352..bc05639b5 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -34,6 +34,7 @@ def search(query_string, opts \\ []) do query_string |> search_query(for_user, following) |> Pagination.fetch_paginated(%{"offset" => offset, "limit" => result_limit}, :offset) + |> Map.get(:items) end) results diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index eeb826814..8f07638cd 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -556,6 +556,7 @@ def fetch_public_activities(opts \\ %{}) do q |> restrict_unlisted() |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() end @@ -953,6 +954,7 @@ def fetch_activities(recipients, opts \\ %{}) do fetch_activities_query(recipients ++ list_memberships, opts) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() |> maybe_update_cc(list_memberships, opts["user"]) end @@ -987,6 +989,7 @@ def fetch_activities_bounded(recipients, recipients_with_public, opts \\ %{}) do fetch_activities_query([], opts) |> fetch_activities_bounded_query(recipients, recipients_with_public) |> Pagination.fetch_paginated(opts) + |> Map.get(:items) |> Enum.reverse() end diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex index 83e877c0e..d532ba685 100644 --- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex @@ -420,6 +420,7 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do [user.ap_id] |> ActivityPub.fetch_activities_query(params) |> Pagination.fetch_paginated(params) + |> Map.get(:items) conn |> add_link_headers(:dm_timeline, activities) @@ -1194,6 +1195,7 @@ def bookmarks(%{assigns: %{user: user}} = conn, params) do bookmarks = Bookmark.for_user_query(user.id) |> Pagination.fetch_paginated(params) + |> Map.get(:items) activities = bookmarks diff --git a/lib/pleroma/web/mastodon_api/mastodon_api.ex b/lib/pleroma/web/mastodon_api/mastodon_api.ex index ac01d1ff3..cf3962944 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api.ex @@ -45,12 +45,14 @@ def get_followers(user, params \\ %{}) do user |> User.get_followers_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_friends(user, params \\ %{}) do user |> User.get_friends_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_notifications(user, params \\ %{}) do @@ -60,12 +62,14 @@ def get_notifications(user, params \\ %{}) do |> Notification.for_user_query(options) |> restrict(:exclude_types, options) |> Pagination.fetch_paginated(params) + |> Map.get(:items) end def get_scheduled_activities(user, params \\ %{}) do user |> ScheduledActivity.for_user_query() |> Pagination.fetch_paginated(params) + |> Map.get(:items) end defp cast_params(params) do diff --git a/test/pagination_test.exs b/test/pagination_test.exs new file mode 100644 index 000000000..048ab6f3c --- /dev/null +++ b/test/pagination_test.exs @@ -0,0 +1,78 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.PaginationTest do + use Pleroma.DataCase + + import Pleroma.Factory + + alias Pleroma.Object + alias Pleroma.Pagination + + describe "keyset" do + setup do + notes = insert_list(5, :note) + + %{notes: notes} + end + + test "paginates by min_id", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"min_id" => id}) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by since_id", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"since_id" => id}) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by max_id", %{notes: notes} do + id = Enum.at(notes, 1).id |> Integer.to_string() + %{total: total, items: paginated} = Pagination.fetch_paginated(Object, %{"max_id" => id}) + + assert length(paginated) == 1 + assert total == 5 + end + + test "paginates by min_id & limit", %{notes: notes} do + id = Enum.at(notes, 2).id |> Integer.to_string() + + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"min_id" => id, "limit" => 1}) + + assert length(paginated) == 1 + assert total == 5 + end + end + + describe "offset" do + setup do + notes = insert_list(5, :note) + + %{notes: notes} + end + + test "paginates by limit" do + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"limit" => 2}, :offset) + + assert length(paginated) == 2 + assert total == 5 + end + + test "paginates by limit & offset" do + %{total: total, items: paginated} = + Pagination.fetch_paginated(Object, %{"limit" => 2, "offset" => 4}, :offset) + + assert length(paginated) == 1 + assert total == 5 + end + end +end