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

@@ -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(
       %{

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)

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,

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

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}

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,

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