From 521dfa4670bfe62a69036998f3966e37aac1f587 Mon Sep 17 00:00:00 2001 From: Oneric Date: Thu, 9 Oct 2025 00:00:00 +0000 Subject: [PATCH] Use keyed lists for pagination with foreign id The old approach required adding a special virtual field to any table potentially needing such foreign-id pagination and also still required manually sorting according to pagiantion settings since the pagination helper does not know whether this virtual field was set or not. Using lists with each entry containing the pagination id and the actual entry insterad allows any table to use this mechanism unchanged and does not require manually sorting. Since it was unused, this also drops the pagination mode paramter from fetch_favourited_with_fav_id. Furthermore, as a side effect of this change a bug in the favourite benchmark is fixed. It used to incorrectly attempt to use IDs of the liked objects for pagination instead of the like IDs as advertised in Link headers. --- benchmarks/load_testing/fetcher.ex | 15 ++++++++------- lib/pleroma/activity.ex | 4 ---- lib/pleroma/pagination.ex | 10 ++++++++++ lib/pleroma/web/activity_pub/activity_pub.ex | 15 ++++++--------- lib/pleroma/web/controller_helper.ex | 4 ---- .../mastodon_api/controllers/status_controller.ex | 5 +++-- .../web/activity_pub/activity_pub_test.exs | 8 ++++---- 7 files changed, 31 insertions(+), 30 deletions(-) diff --git a/benchmarks/load_testing/fetcher.ex b/benchmarks/load_testing/fetcher.ex index 607b7d4cb..445463dea 100644 --- a/benchmarks/load_testing/fetcher.ex +++ b/benchmarks/load_testing/fetcher.ex @@ -53,7 +53,7 @@ defp fetch_timelines(user) do fetch_public_timeline(user, :local) fetch_public_timeline(user, :tag) fetch_notifications(user) - fetch_favourites(user) + fetch_favourited_with_fav_id(user) fetch_long_thread(user) fetch_timelines_with_reply_filtering(user) end @@ -378,21 +378,21 @@ defp fetch_notifications(user) do end defp fetch_favourites(user) do - first_page_last = ActivityPub.fetch_favourites(user) |> List.last() + first_page_last = ActivityPub.fetch_favourited_with_fav_id(user) |> List.last() second_page_last = - ActivityPub.fetch_favourites(user, %{:max_id => first_page_last.id}) |> List.last() + ActivityPub.fetch_favourited_with_fav_id(user, %{:max_id => first_page_last.id}) |> List.last() third_page_last = - ActivityPub.fetch_favourites(user, %{:max_id => second_page_last.id}) |> List.last() + ActivityPub.fetch_favourited_with_fav_id(user, %{:max_id => second_page_last.id}) |> List.last() forth_page_last = - ActivityPub.fetch_favourites(user, %{:max_id => third_page_last.id}) |> List.last() + ActivityPub.fetch_favourited_with_fav_id(user, %{:max_id => third_page_last.id}) |> List.last() Benchee.run( %{ "Favourites" => fn opts -> - ActivityPub.fetch_favourites(user, opts) + ActivityPub.fetch_favourited_with_fav_id(user, opts) end }, inputs: %{ @@ -465,7 +465,8 @@ defp render_timelines(user) do notifications = MastodonAPI.get_notifications(user, opts) - favourites = ActivityPub.fetch_favourites(user) + favourites_keyed = ActivityPub.fetch_favourited_with_fav_id(user) + favourites = Pagiation.unwrap(favourites_keyed) Benchee.run( %{ diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index f820cbdae..449c9beda 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -33,10 +33,6 @@ defmodule Pleroma.Activity do field(:recipients, {:array, :string}, default: []) field(:thread_muted?, :boolean, virtual: true) - # A field that can be used if you need to join some kind of other - # id to order / paginate this field by - field(:pagination_id, :string, virtual: true) - # This is a fake relation, # do not use outside of with_preloaded_user_actor/with_joined_user_actor has_one(:user_actor, User, on_delete: :nothing, foreign_key: :id) diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index f71502da9..f70e281a9 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -86,6 +86,16 @@ def paginate(query, options, :offset, table_binding) do |> restrict(:limit, options, table_binding) end + @doc """ + Unwraps a result list for a query paginated by a foreign id. + Usually you want to keep those foreign ids around until after pagination Link headers got generated. + """ + @spec unwrap([%{id: any(), entry: any()}]) :: [any()] + def unwrap(list) when is_list(list), do: do_unwrap(list, []) + + defp do_unwrap([%{entry: entry} | rest], acc), do: do_unwrap(rest, [entry | acc]) + defp do_unwrap([], acc), do: Enum.reverse(acc) + defp cast_params(params) do param_types = %{ min_id: params[:id_type] || :string, diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index ec14956a7..f3077b242 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1467,21 +1467,18 @@ def fetch_activities_query(recipients, opts \\ %{}) do end @doc """ - Fetch favorites activities of user with order by sort adds to favorites + Fetch posts liked by the given user wrapped in a paginated list with IDs taken from the like activity """ - @spec fetch_favourites(User.t(), map(), Pagination.type()) :: list(Activity.t()) - def fetch_favourites(user, params \\ %{}, pagination \\ :keyset) do + @spec fetch_favourited_with_fav_id(User.t(), map()) :: + list(%{id: binary(), entry: Activity.t()}) + def fetch_favourited_with_fav_id(user, params \\ %{}) do user.ap_id |> Activity.Queries.by_actor() |> Activity.Queries.by_type("Like") |> Activity.with_joined_object() |> Object.with_joined_activity() - |> select([like, object, activity], %{activity | object: object, pagination_id: like.id}) - |> order_by([like, _, _], desc_nulls_last: like.id) - |> Pagination.fetch_paginated( - Map.merge(params, %{skip_order: true}), - pagination - ) + |> select([like, object, create], %{id: like.id, entry: %{create | object: object}}) + |> Pagination.fetch_paginated(params, :keyset) end defp maybe_update_cc(activities, [_ | _] = list_memberships, %User{ap_id: user_ap_id}) do diff --git a/lib/pleroma/web/controller_helper.ex b/lib/pleroma/web/controller_helper.ex index f49e9ca66..a13d32eac 100644 --- a/lib/pleroma/web/controller_helper.ex +++ b/lib/pleroma/web/controller_helper.ex @@ -79,10 +79,6 @@ defp build_pagination_fields(conn, min_id, max_id, extra_params, order) do defp get_first_last_pagination_id(entries) do case List.last(entries) do - %{pagination_id: last_id} when not is_nil(last_id) -> - %{pagination_id: first_id} = List.first(entries) - {first_id, last_id} - %{id: last_id} -> %{id: first_id} = List.first(entries) {first_id, last_id} diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 79d590a7b..b1a2395ac 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -455,10 +455,11 @@ def context(%{assigns: %{user: user}} = conn, %{id: id}) do @doc "GET /api/v1/favourites" def favourites(%{assigns: %{user: %User{} = user}} = conn, params) do - activities = ActivityPub.fetch_favourites(user, params) + activities_keyed = ActivityPub.fetch_favourited_with_fav_id(user, params) + activities = Pleroma.Pagination.unwrap(activities_keyed) conn - |> add_link_headers(activities) + |> add_link_headers(activities_keyed) |> render("index.json", activities: activities, for: user, diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 65d3ab6a3..b8a02a16a 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1828,12 +1828,12 @@ test "returns a favourite activities sorted by adds to favorite" do {:ok, _} = CommonAPI.favorite(other_user, a4.id) {:ok, _} = CommonAPI.favorite(user, a1.id) {:ok, _} = CommonAPI.favorite(other_user, a1.id) - result = ActivityPub.fetch_favourites(user) + result = ActivityPub.fetch_favourited_with_fav_id(user) - assert Enum.map(result, & &1.id) == [a1.id, a5.id, a3.id, a4.id] + assert Enum.map(result, & &1.entry.id) == [a1.id, a5.id, a3.id, a4.id] - result = ActivityPub.fetch_favourites(user, %{limit: 2}) - assert Enum.map(result, & &1.id) == [a1.id, a5.id] + result = ActivityPub.fetch_favourited_with_fav_id(user, %{limit: 2}) + assert Enum.map(result, & &1.entry.id) == [a1.id, a5.id] end end