From 6f152240538e1b300446f3bad50977cca7203f70 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Mon, 25 Mar 2019 23:13:58 +0100 Subject: [PATCH 1/4] activity_pub.ex: Move limit/max_id restrictions to Pagination helpers --- lib/pleroma/web/activity_pub/activity_pub.ex | 30 ++++--------------- .../mastodon_api/mastodon_api_controller.ex | 3 +- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 6e1ed7ec9..531158338 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do alias Pleroma.Instances alias Pleroma.Notification alias Pleroma.Object + alias Pleroma.Pagination alias Pleroma.Repo alias Pleroma.Upload alias Pleroma.User @@ -474,7 +475,7 @@ def fetch_public_activities(opts \\ %{}) do q |> restrict_unlisted() - |> Repo.all() + |> Pagination.fetch_paginated(opts) |> Enum.reverse() end @@ -617,26 +618,12 @@ defp restrict_recipients(query, recipients, user) do ) end - defp restrict_limit(query, %{"limit" => limit}) do - from(activity in query, limit: ^limit) - end - - defp restrict_limit(query, _), do: query - defp restrict_local(query, %{"local_only" => true}) do from(activity in query, where: activity.local == true) end defp restrict_local(query, _), do: query - defp restrict_max(query, %{"max_id" => ""}), do: query - - defp restrict_max(query, %{"max_id" => max_id}) do - from(activity in query, where: activity.id < ^max_id) - end - - defp restrict_max(query, _), do: query - defp restrict_actor(query, %{"actor_id" => actor_id}) do from(activity in query, where: activity.actor == ^actor_id) end @@ -757,12 +744,7 @@ defp maybe_preload_objects(query, _) do end def fetch_activities_query(recipients, opts \\ %{}) do - base_query = - from( - activity in Activity, - limit: 20, - order_by: [fragment("? desc nulls last", activity.id)] - ) + base_query = from(activity in Activity) base_query |> maybe_preload_objects(opts) @@ -772,8 +754,6 @@ def fetch_activities_query(recipients, opts \\ %{}) do |> restrict_tag_all(opts) |> restrict_since(opts) |> restrict_local(opts) - |> restrict_limit(opts) - |> restrict_max(opts) |> restrict_actor(opts) |> restrict_type(opts) |> restrict_favorited_by(opts) @@ -789,14 +769,14 @@ def fetch_activities_query(recipients, opts \\ %{}) do def fetch_activities(recipients, opts \\ %{}) do fetch_activities_query(recipients, opts) - |> Repo.all() + |> Pagination.fetch_paginated(opts) |> Enum.reverse() end def fetch_activities_bounded(recipients_to, recipients_cc, opts \\ %{}) do fetch_activities_query([], opts) |> restrict_to_cc(recipients_to, recipients_cc) - |> Repo.all() + |> Pagination.fetch_paginated(opts) |> Enum.reverse() end diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index eee4e7678..4d0caecf4 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -10,6 +10,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do alias Pleroma.Filter alias Pleroma.Notification alias Pleroma.Object + alias Pleroma.Pagination alias Pleroma.Repo alias Pleroma.Stats alias Pleroma.User @@ -310,7 +311,7 @@ def dm_timeline(%{assigns: %{user: user}} = conn, params) do activities = [user.ap_id] |> ActivityPub.fetch_activities_query(params) - |> Repo.all() + |> Pagination.fetch_paginated(params) conn |> add_link_headers(:dm_timeline, activities) From e83ad12c5730eae7adac597b97707836122b8f7f Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Tue, 26 Mar 2019 00:07:33 +0100 Subject: [PATCH 2/4] pagination.ex: Drop atom keys in params Atom keys could also have been transformed to string, or the other way around but this one is more efficient and what we actually expect with the current param_types in Pagination --- lib/pleroma/pagination.ex | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/pleroma/pagination.ex b/lib/pleroma/pagination.ex index 7c864deef..f435e5c9c 100644 --- a/lib/pleroma/pagination.ex +++ b/lib/pleroma/pagination.ex @@ -36,6 +36,12 @@ defp cast_params(params) do limit: :integer } + params = + Enum.reduce(params, %{}, fn + {key, _value}, acc when is_atom(key) -> Map.drop(acc, [key]) + {key, value}, acc -> Map.put(acc, key, value) + end) + changeset = cast({%{}, param_types}, params, Map.keys(param_types)) changeset.changes end From 6b407872b402b72dba48bece5fa69ca5af54f2f0 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Mon, 25 Mar 2019 22:19:57 +0100 Subject: [PATCH 3/4] mastodon_api_controller.ex: Use min_id in link header instead of since_id --- .../mastodon_api/mastodon_api_controller.ex | 21 ++++++++++++------- .../mastodon_api_controller_test.exs | 6 +++--- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 4d0caecf4..5cc19e68f 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -199,15 +199,20 @@ def custom_emojis(conn, _params) do defp add_link_headers(conn, method, activities, param \\ nil, params \\ %{}) do params = conn.params - |> Map.drop(["since_id", "max_id"]) + |> Map.drop(["since_id", "max_id", "min_id"]) |> Map.merge(params) last = List.last(activities) - first = List.first(activities) if last do - min = last.id - max = first.id + max_id = last.id + + limit = + params + |> Map.get("limit", "20") + |> String.to_integer() + + min_id = Enum.at(activities, limit * -1) {next_url, prev_url} = if param do @@ -216,13 +221,13 @@ defp add_link_headers(conn, method, activities, param \\ nil, params \\ %{}) do Pleroma.Web.Endpoint, method, param, - Map.merge(params, %{max_id: min}) + Map.merge(params, %{max_id: max_id}) ), mastodon_api_url( Pleroma.Web.Endpoint, method, param, - Map.merge(params, %{since_id: max}) + Map.merge(params, %{min_id: min_id}) ) } else @@ -230,12 +235,12 @@ defp add_link_headers(conn, method, activities, param \\ nil, params \\ %{}) do mastodon_api_url( Pleroma.Web.Endpoint, method, - Map.merge(params, %{max_id: min}) + Map.merge(params, %{max_id: max_id}) ), mastodon_api_url( Pleroma.Web.Endpoint, method, - Map.merge(params, %{since_id: max}) + Map.merge(params, %{min_id: min_id}) ) } end diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index d9bcbf5a9..f506a847d 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -1371,7 +1371,7 @@ test "getting followers, pagination", %{conn: conn} do assert id2 == follower2.id assert [link_header] = get_resp_header(res_conn, "link") - assert link_header =~ ~r/since_id=#{follower2.id}/ + assert link_header =~ ~r/min_id=#{follower2.id}/ assert link_header =~ ~r/max_id=#{follower2.id}/ end @@ -1450,7 +1450,7 @@ test "getting following, pagination", %{conn: conn} do assert id2 == following2.id assert [link_header] = get_resp_header(res_conn, "link") - assert link_header =~ ~r/since_id=#{following2.id}/ + assert link_header =~ ~r/min_id=#{following2.id}/ assert link_header =~ ~r/max_id=#{following2.id}/ end @@ -2261,7 +2261,7 @@ test "preserves parameters in link headers", %{conn: conn} do assert [link_header] = get_resp_header(conn, "link") assert link_header =~ ~r/media_only=true/ - assert link_header =~ ~r/since_id=#{notification2.id}/ + assert link_header =~ ~r/min_id=#{notification2.id}/ assert link_header =~ ~r/max_id=#{notification1.id}/ end end From c8abef373b32313f94fc34b33dc235ca6aabceed Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 28 Mar 2019 17:18:44 +0100 Subject: [PATCH 4/4] mastodon_api_controller.ex: fallback to first for min_id --- .../web/mastodon_api/mastodon_api_controller.ex | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 5cc19e68f..b2fc68707 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -212,7 +212,16 @@ defp add_link_headers(conn, method, activities, param \\ nil, params \\ %{}) do |> Map.get("limit", "20") |> String.to_integer() - min_id = Enum.at(activities, limit * -1) + min_id = + if length(activities) <= limit do + activities + |> List.first() + |> Map.get(:id) + else + activities + |> Enum.at(limit * -1) + |> Map.get(:id) + end {next_url, prev_url} = if param do