Merge branch 'fix/do-not-embed-first-pages-in-collections' into 'develop'

Don't embed the first page in inboxes/outboxes and refactor the views to follow View/Controller pattern

See merge request pleroma/pleroma!1716
This commit is contained in:
kaniini 2019-09-26 00:03:47 +00:00
commit 1b9cd4c866
6 changed files with 89 additions and 87 deletions

View file

@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Improve digest email template - Improve digest email template
Pagination: (optional) return `total` alongside with `items` when paginating Pagination: (optional) return `total` alongside with `items` when paginating
- Add `rel="ugc"` to all links in statuses, to prevent SEO spam - Add `rel="ugc"` to all links in statuses, to prevent SEO spam
- ActivityPub: The first page in inboxes/outboxes is no longer embedded.
### Fixed ### Fixed
- Following from Osada - Following from Osada

View file

@ -834,7 +834,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
defp restrict_muted_reblogs(query, _), do: query defp restrict_muted_reblogs(query, _), do: query
defp exclude_poll_votes(query, %{"include_poll_votes" => "true"}), do: query defp exclude_poll_votes(query, %{"include_poll_votes" => true}), do: query
defp exclude_poll_votes(query, _) do defp exclude_poll_votes(query, _) do
if has_named_binding?(query, :object) do if has_named_binding?(query, :object) do

View file

@ -231,13 +231,43 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
end end
end end
def outbox(conn, %{"nickname" => nickname} = params) do def outbox(conn, %{"nickname" => nickname, "page" => page?} = params)
when page? in [true, "true"] do
with %User{} = user <- User.get_cached_by_nickname(nickname),
{:ok, user} <- User.ensure_keys_present(user) do
activities =
if params["max_id"] do
ActivityPub.fetch_user_activities(user, nil, %{
"max_id" => params["max_id"],
# This is a hack because postgres generates inefficient queries when filtering by
# 'Answer', poll votes will be hidden by the visibility filter in this case anyway
"include_poll_votes" => true,
"limit" => 10
})
else
ActivityPub.fetch_user_activities(user, nil, %{
"limit" => 10,
"include_poll_votes" => true
})
end
conn
|> put_resp_content_type("application/activity+json")
|> put_view(UserView)
|> render("activity_collection_page.json", %{
activities: activities,
iri: "#{user.ap_id}/outbox"
})
end
end
def outbox(conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname), with %User{} = user <- User.get_cached_by_nickname(nickname),
{:ok, user} <- User.ensure_keys_present(user) do {:ok, user} <- User.ensure_keys_present(user) do
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
|> render("outbox.json", %{user: user, max_id: params["max_id"]}) |> render("activity_collection.json", %{iri: "#{user.ap_id}/outbox"})
end end
end end
@ -315,12 +345,37 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
def read_inbox( def read_inbox(
%{assigns: %{user: %{nickname: nickname} = user}} = conn, %{assigns: %{user: %{nickname: nickname} = user}} = conn,
%{"nickname" => nickname} = params %{"nickname" => nickname, "page" => page?} = params
) do )
when page? in [true, "true"] do
activities =
if params["max_id"] do
ActivityPub.fetch_activities([user.ap_id | user.following], %{
"max_id" => params["max_id"],
"limit" => 10
})
else
ActivityPub.fetch_activities([user.ap_id | user.following], %{"limit" => 10})
end
conn conn
|> put_resp_content_type("application/activity+json") |> put_resp_content_type("application/activity+json")
|> put_view(UserView) |> put_view(UserView)
|> render("inbox.json", user: user, max_id: params["max_id"]) |> render("activity_collection_page.json", %{
activities: activities,
iri: "#{user.ap_id}/inbox"
})
end
def read_inbox(%{assigns: %{user: %{nickname: nickname} = user}} = conn, %{
"nickname" => nickname
}) do
with {:ok, user} <- User.ensure_keys_present(user) do
conn
|> put_resp_content_type("application/activity+json")
|> put_view(UserView)
|> render("activity_collection.json", %{iri: "#{user.ap_id}/inbox"})
end
end end
def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do

View file

@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.UserView do
alias Pleroma.Keys alias Pleroma.Keys
alias Pleroma.Repo alias Pleroma.Repo
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.Transmogrifier
alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Utils
alias Pleroma.Web.Endpoint alias Pleroma.Web.Endpoint
@ -210,20 +209,16 @@ defmodule Pleroma.Web.ActivityPub.UserView do
|> Map.merge(Utils.make_json_ld_header()) |> Map.merge(Utils.make_json_ld_header())
end end
def render("outbox.json", %{user: user, max_id: max_qid}) do def render("activity_collection.json", %{iri: iri}) do
params = %{ %{
"limit" => "10" "id" => iri,
"type" => "OrderedCollection",
"first" => "#{iri}?page=true"
} }
|> Map.merge(Utils.make_json_ld_header())
end
params = def render("activity_collection_page.json", %{activities: activities, iri: iri}) do
if max_qid != nil do
Map.put(params, "max_id", max_qid)
else
params
end
activities = ActivityPub.fetch_user_activities(user, nil, params)
# this is sorted chronologically, so first activity is the newest (max) # this is sorted chronologically, so first activity is the newest (max)
{max_id, min_id, collection} = {max_id, min_id, collection} =
if length(activities) > 0 do if length(activities) > 0 do
@ -243,71 +238,14 @@ defmodule Pleroma.Web.ActivityPub.UserView do
} }
end end
iri = "#{user.ap_id}/outbox" %{
"id" => "#{iri}?max_id=#{max_id}&page=true",
page = %{
"id" => "#{iri}?max_id=#{max_id}",
"type" => "OrderedCollectionPage", "type" => "OrderedCollectionPage",
"partOf" => iri, "partOf" => iri,
"orderedItems" => collection, "orderedItems" => collection,
"next" => "#{iri}?max_id=#{min_id}" "next" => "#{iri}?max_id=#{min_id}&page=true"
} }
|> Map.merge(Utils.make_json_ld_header())
if max_qid == nil do
%{
"id" => iri,
"type" => "OrderedCollection",
"first" => page
}
|> Map.merge(Utils.make_json_ld_header())
else
page |> Map.merge(Utils.make_json_ld_header())
end
end
def render("inbox.json", %{user: user, max_id: max_qid}) do
params = %{
"limit" => "10"
}
params =
if max_qid != nil do
Map.put(params, "max_id", max_qid)
else
params
end
activities = ActivityPub.fetch_activities([user.ap_id | user.following], params)
min_id = Enum.at(Enum.reverse(activities), 0).id
max_id = Enum.at(activities, 0).id
collection =
Enum.map(activities, fn act ->
{:ok, data} = Transmogrifier.prepare_outgoing(act.data)
data
end)
iri = "#{user.ap_id}/inbox"
page = %{
"id" => "#{iri}?max_id=#{max_id}",
"type" => "OrderedCollectionPage",
"partOf" => iri,
"orderedItems" => collection,
"next" => "#{iri}?max_id=#{min_id}"
}
if max_qid == nil do
%{
"id" => iri,
"type" => "OrderedCollection",
"first" => page
}
|> Map.merge(Utils.make_json_ld_header())
else
page |> Map.merge(Utils.make_json_ld_header())
end
end end
def collection(collection, iri, page, show_items \\ true, total \\ nil) do def collection(collection, iri, page, show_items \\ true, total \\ nil) do

View file

@ -479,7 +479,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
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}/inbox") |> get("/users/#{user.nickname}/inbox?page=true")
assert response(conn, 200) =~ note_object.data["content"] assert response(conn, 200) =~ note_object.data["content"]
end end
@ -567,7 +567,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
conn = conn =
conn conn
|> put_req_header("accept", "application/activity+json") |> put_req_header("accept", "application/activity+json")
|> get("/users/#{user.nickname}/outbox") |> get("/users/#{user.nickname}/outbox?page=true")
assert response(conn, 200) =~ note_object.data["content"] assert response(conn, 200) =~ note_object.data["content"]
end end
@ -579,7 +579,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
conn = conn =
conn conn
|> put_req_header("accept", "application/activity+json") |> put_req_header("accept", "application/activity+json")
|> get("/users/#{user.nickname}/outbox") |> get("/users/#{user.nickname}/outbox?page=true")
assert response(conn, 200) =~ announce_activity.data["object"] assert response(conn, 200) =~ announce_activity.data["object"]
end end

View file

@ -159,7 +159,7 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
end end
end end
test "outbox paginates correctly" do test "activity collection page aginates correctly" do
user = insert(:user) user = insert(:user)
posts = posts =
@ -171,13 +171,21 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
# outbox sorts chronologically, newest first, with ten per page # outbox sorts chronologically, newest first, with ten per page
posts = Enum.reverse(posts) posts = Enum.reverse(posts)
%{"first" => %{"next" => next_url}} = %{"next" => next_url} =
UserView.render("outbox.json", %{user: user, max_id: nil}) UserView.render("activity_collection_page.json", %{
iri: "#{user.ap_id}/outbox",
activities: Enum.take(posts, 10)
})
next_id = Enum.at(posts, 9).id next_id = Enum.at(posts, 9).id
assert next_url =~ next_id assert next_url =~ next_id
%{"next" => next_url} = UserView.render("outbox.json", %{user: user, max_id: 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 next_id = Enum.at(posts, 19).id
assert next_url =~ next_id assert next_url =~ next_id
end end