Bugfix: Reuse Controller.Helper pagination for APC2S

This commit is contained in:
Haelwenn (lanodan) Monnier 2020-05-07 21:52:45 +02:00
parent 9848978109
commit b3b367b894
No known key found for this signature in database
GPG Key ID: D5B7A8E43C997DEE
6 changed files with 94 additions and 76 deletions

View File

@ -21,6 +21,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
alias Pleroma.Web.ActivityPub.UserView alias Pleroma.Web.ActivityPub.UserView
alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Utils
alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.ActivityPub.Visibility
alias Pleroma.Web.ControllerHelper
alias Pleroma.Web.Endpoint alias Pleroma.Web.Endpoint
alias Pleroma.Web.FederatingPlug alias Pleroma.Web.FederatingPlug
alias Pleroma.Web.Federator alias Pleroma.Web.Federator
@ -251,6 +252,7 @@ def outbox(
|> put_view(UserView) |> put_view(UserView)
|> render("activity_collection_page.json", %{ |> render("activity_collection_page.json", %{
activities: activities, activities: activities,
pagination: ControllerHelper.get_pagination_fields(conn, activities, %{"limit" => "10"}),
iri: "#{user.ap_id}/outbox" iri: "#{user.ap_id}/outbox"
}) })
end end
@ -368,6 +370,7 @@ def read_inbox(
|> put_view(UserView) |> put_view(UserView)
|> render("activity_collection_page.json", %{ |> render("activity_collection_page.json", %{
activities: activities, activities: activities,
pagination: ControllerHelper.get_pagination_fields(conn, activities, %{"limit" => "10"}),
iri: "#{user.ap_id}/inbox" iri: "#{user.ap_id}/inbox"
}) })
end end

View File

@ -213,34 +213,24 @@ def render("activity_collection.json", %{iri: iri}) do
|> Map.merge(Utils.make_json_ld_header()) |> Map.merge(Utils.make_json_ld_header())
end end
def render("activity_collection_page.json", %{activities: activities, iri: iri}) do def render("activity_collection_page.json", %{
# this is sorted chronologically, so first activity is the newest (max) activities: activities,
{max_id, min_id, collection} = iri: iri,
if length(activities) > 0 do pagination: pagination
{ }) do
Enum.at(activities, 0).id, collection =
Enum.at(Enum.reverse(activities), 0).id, Enum.map(activities, fn activity ->
Enum.map(activities, fn act -> {:ok, data} = Transmogrifier.prepare_outgoing(activity.data)
{:ok, data} = Transmogrifier.prepare_outgoing(act.data)
data data
end) end)
}
else
{
0,
0,
[]
}
end
%{ %{
"id" => "#{iri}?max_id=#{max_id}&page=true",
"type" => "OrderedCollectionPage", "type" => "OrderedCollectionPage",
"partOf" => iri, "partOf" => iri,
"orderedItems" => collection, "orderedItems" => collection
"next" => "#{iri}?max_id=#{min_id}&page=true"
} }
|> Map.merge(Utils.make_json_ld_header()) |> Map.merge(Utils.make_json_ld_header())
|> Map.merge(pagination)
end end
defp maybe_put_total_items(map, false, _total), do: map defp maybe_put_total_items(map, false, _total), do: map

View File

@ -5,6 +5,8 @@
defmodule Pleroma.Web.ControllerHelper do defmodule Pleroma.Web.ControllerHelper do
use Pleroma.Web, :controller use Pleroma.Web, :controller
alias Pleroma.Pagination
# As in Mastodon API, per https://api.rubyonrails.org/classes/ActiveModel/Type/Boolean.html # As in Mastodon API, per https://api.rubyonrails.org/classes/ActiveModel/Type/Boolean.html
@falsy_param_values [false, 0, "0", "f", "F", "false", "False", "FALSE", "off", "OFF"] @falsy_param_values [false, 0, "0", "f", "F", "false", "False", "FALSE", "off", "OFF"]
@ -46,6 +48,16 @@ def add_link_headers(%{assigns: %{skip_link_headers: true}} = conn, _activities,
do: conn do: conn
def add_link_headers(conn, activities, extra_params) do def add_link_headers(conn, activities, extra_params) do
case get_pagination_fields(conn, activities, extra_params) do
%{"next" => next_url, "prev" => prev_url} ->
put_resp_header(conn, "link", "<#{next_url}>; rel=\"next\", <#{prev_url}>; rel=\"prev\"")
_ ->
conn
end
end
def get_pagination_fields(conn, activities, extra_params \\ %{}) do
case List.last(activities) do case List.last(activities) do
%{id: max_id} -> %{id: max_id} ->
params = params =
@ -54,29 +66,29 @@ def add_link_headers(conn, activities, extra_params) do
|> Map.drop(["since_id", "max_id", "min_id"]) |> Map.drop(["since_id", "max_id", "min_id"])
|> Map.merge(extra_params) |> Map.merge(extra_params)
limit =
params
|> Map.get("limit", "20")
|> String.to_integer()
min_id = min_id =
if length(activities) <= limit do
activities activities
|> List.first() |> List.first()
|> Map.get(:id) |> Map.get(:id)
fields = %{
"next" => current_url(conn, Map.put(params, :max_id, max_id)),
"prev" => current_url(conn, Map.put(params, :min_id, min_id))
}
# Generating an `id` without already present pagination keys would
# need a query-restriction with an `q.id >= ^id` or `q.id <= ^id`
# instead of the `q.id > ^min_id` and `q.id < ^max_id`.
# This is because we only have ids present inside of the page, while
# `min_id`, `since_id` and `max_id` requires to know one outside of it.
if Map.take(conn.params, Pagination.page_keys() -- ["limit", "order"]) != [] do
Map.put(fields, "id", current_url(conn, conn.params))
else else
activities fields
|> Enum.at(limit * -1)
|> Map.get(:id)
end end
next_url = current_url(conn, Map.merge(params, %{max_id: max_id}))
prev_url = current_url(conn, Map.merge(params, %{min_id: min_id}))
put_resp_header(conn, "link", "<#{next_url}>; rel=\"next\", <#{prev_url}>; rel=\"prev\"")
_ -> _ ->
conn %{}
end end
end end

View File

@ -51,10 +51,8 @@ def home(%{assigns: %{user: user}} = conn, params) do
|> Map.put("reply_filtering_user", user) |> Map.put("reply_filtering_user", user)
|> Map.put("user", user) |> Map.put("user", user)
recipients = [user.ap_id | User.following(user)]
activities = activities =
recipients [user.ap_id | User.following(user)]
|> ActivityPub.fetch_activities(params) |> ActivityPub.fetch_activities(params)
|> Enum.reverse() |> Enum.reverse()

View File

@ -804,17 +804,63 @@ test "it requires authentication", %{conn: conn} do
end end
describe "GET /users/:nickname/outbox" do describe "GET /users/:nickname/outbox" do
test "it paginates correctly", %{conn: conn} do
user = insert(:user)
conn = assign(conn, :user, user)
outbox_endpoint = user.ap_id <> "/outbox"
_posts =
for i <- 0..15 do
{:ok, activity} = CommonAPI.post(user, %{status: "post #{i}"})
activity
end
result =
conn
|> put_req_header("accept", "application/activity+json")
|> get(outbox_endpoint <> "?page=true")
|> json_response(200)
result_ids = Enum.map(result["orderedItems"], fn x -> x["id"] end)
assert length(result["orderedItems"]) == 10
assert length(result_ids) == 10
assert result["next"]
assert String.starts_with?(result["next"], outbox_endpoint)
result_next =
conn
|> put_req_header("accept", "application/activity+json")
|> get(result["next"])
|> json_response(200)
result_next_ids = Enum.map(result_next["orderedItems"], fn x -> x["id"] end)
assert length(result_next["orderedItems"]) == 6
assert length(result_next_ids) == 6
refute Enum.find(result_next_ids, fn x -> x in result_ids end)
refute Enum.find(result_ids, fn x -> x in result_next_ids end)
assert String.starts_with?(result["id"], outbox_endpoint)
result_next_again =
conn
|> put_req_header("accept", "application/activity+json")
|> get(result_next["id"])
|> json_response(200)
assert result_next == result_next_again
end
test "it returns 200 even if there're no activities", %{conn: conn} do test "it returns 200 even if there're no activities", %{conn: conn} do
user = insert(:user) user = insert(:user)
outbox_endpoint = user.ap_id <> "/outbox"
conn = conn =
conn conn
|> assign(:user, user) |> assign(:user, user)
|> put_req_header("accept", "application/activity+json") |> put_req_header("accept", "application/activity+json")
|> get("/users/#{user.nickname}/outbox") |> get(outbox_endpoint)
result = json_response(conn, 200) result = json_response(conn, 200)
assert user.ap_id <> "/outbox" == result["id"] assert outbox_endpoint == result["id"]
end end
test "it returns a note activity in a collection", %{conn: conn} do test "it returns a note activity in a collection", %{conn: conn} do

View File

@ -158,35 +158,4 @@ test "sets correct totalItems when follows are hidden but the follow counter is
assert %{"totalItems" => 1} = UserView.render("following.json", %{user: user}) assert %{"totalItems" => 1} = UserView.render("following.json", %{user: user})
end end
end end
test "activity collection page aginates correctly" do
user = insert(:user)
posts =
for i <- 0..25 do
{:ok, activity} = CommonAPI.post(user, %{status: "post #{i}"})
activity
end
# outbox sorts chronologically, newest first, with ten per page
posts = Enum.reverse(posts)
%{"next" => next_url} =
UserView.render("activity_collection_page.json", %{
iri: "#{user.ap_id}/outbox",
activities: Enum.take(posts, 10)
})
next_id = Enum.at(posts, 9).id
assert next_url =~ next_id
%{"next" => next_url} =
UserView.render("activity_collection_page.json", %{
iri: "#{user.ap_id}/outbox",
activities: Enum.take(Enum.drop(posts, 10), 10)
})
next_id = Enum.at(posts, 19).id
assert next_url =~ next_id
end
end end