Don't embed the first page in inboxes/outboxes and refactor the views to
authorrinpatch <rinpatch@sdf.org>
Wed, 25 Sep 2019 12:59:04 +0000 (15:59 +0300)
committerrinpatch <rinpatch@sdf.org>
Wed, 25 Sep 2019 12:59:04 +0000 (15:59 +0300)
follow View/Controller pattern

Note that I mentioned the change in 1.1 section because I intend to
backport this, if this is not needed I will move it back to Unreleased.

CHANGELOG.md
lib/pleroma/web/activity_pub/activity_pub.ex
lib/pleroma/web/activity_pub/activity_pub_controller.ex
lib/pleroma/web/activity_pub/views/user_view.ex
test/web/activity_pub/activity_pub_controller_test.exs
test/web/activity_pub/views/user_view_test.exs

index 649fbc0be038a75786f84556a31ab87b3f7c248d..2d6ddd5d6274d7a304bdf7d43cbe436401606b1e 100644 (file)
@@ -44,6 +44,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Improve digest email template
 – Pagination: (optional) return `total` alongside with `items` when paginating
 - Add `rel="ugc"` to all links in statuses, to prevent SEO spam
+- ActivityPub: The first page in inboxes/outboxes is no longer embedded.
 
 ### Fixed
 - Following from Osada
index 1cf8b61516b7d82522940fcb232ba43ddb2dc341..a97afa66572e74b48dcb5af95849b73aac6d75b1 100644 (file)
@@ -834,7 +834,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
 
   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
     if has_named_binding?(query, :object) do
index 9eb86106f0851a3037aeb6917f08db18b1c6ff5f..c3e7edf573dd54e6107315add5090d624c8021c7 100644 (file)
@@ -231,13 +231,42 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
     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),
+         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) do
+      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),
          {:ok, user} <- User.ensure_keys_present(user) do
       conn
       |> put_resp_content_type("application/activity+json")
       |> put_view(UserView)
-      |> render("outbox.json", %{user: user, max_id: params["max_id"]})
+      |> render("activity_collection.json", %{iri: "#{user.ap_id}/outbox"})
     end
   end
 
@@ -315,12 +344,37 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
 
   def read_inbox(
         %{assigns: %{user: %{nickname: nickname} = user}} = conn,
-        %{"nickname" => nickname} = params
-      ) do
-    conn
-    |> put_resp_content_type("application/activity+json")
-    |> put_view(UserView)
-    |> render("inbox.json", user: user, max_id: params["max_id"])
+        %{"nickname" => nickname, "page" => page?} = params
+      )
+      when page? in [true, "true"] do
+    with 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) do
+      conn
+      |> put_resp_content_type("application/activity+json")
+      |> put_view(UserView)
+      |> render("activity_collection_page.json", %{
+        activities: activities,
+        iri: "#{user.ap_id}/inbox"
+      })
+    end
+  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
 
   def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do
index 352d856fac3b85fe5808b8994475ab5b6e1f9f8c..5dbb5992f6692f8fc30f2f33a87360af86e4f8e5 100644 (file)
@@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.UserView do
   alias Pleroma.Keys
   alias Pleroma.Repo
   alias Pleroma.User
-  alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.Endpoint
@@ -210,20 +209,16 @@ defmodule Pleroma.Web.ActivityPub.UserView do
     |> Map.merge(Utils.make_json_ld_header())
   end
 
-  def render("outbox.json", %{user: user, max_id: max_qid}) do
-    params = %{
-      "limit" => "10"
+  def render("activity_collection.json", %{iri: iri}) do
+    %{
+      "id" => iri,
+      "type" => "OrderedCollection",
+      "first" => "#{iri}?page=true"
     }
+    |> Map.merge(Utils.make_json_ld_header())
+  end
 
-    params =
-      if max_qid != nil do
-        Map.put(params, "max_id", max_qid)
-      else
-        params
-      end
-
-    activities = ActivityPub.fetch_user_activities(user, nil, params)
-
+  def render("activity_collection_page.json", %{activities: activities, iri: iri}) do
     # this is sorted chronologically, so first activity is the newest (max)
     {max_id, min_id, collection} =
       if length(activities) > 0 do
@@ -243,71 +238,15 @@ defmodule Pleroma.Web.ActivityPub.UserView do
         }
       end
 
-    iri = "#{user.ap_id}/outbox"
-
     page = %{
-      "id" => "#{iri}?max_id=#{max_id}",
+      "id" => "#{iri}?max_id=#{max_id}&page=true",
       "type" => "OrderedCollectionPage",
       "partOf" => iri,
       "orderedItems" => collection,
-      "next" => "#{iri}?max_id=#{min_id}"
+      "next" => "#{iri}?max_id=#{min_id}&page=true"
     }
 
-    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
+    page |> Map.merge(Utils.make_json_ld_header())
   end
 
   def collection(collection, iri, page, show_items \\ true, total \\ nil) do
index 9e8e420ec35cd6edafd183868e1ed739e538932f..ab52044ae40cc751689e7d4545133ef9c5e5d84e 100644 (file)
@@ -479,7 +479,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
         conn
         |> assign(:user, user)
         |> 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"]
     end
@@ -567,7 +567,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       conn =
         conn
         |> 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"]
     end
@@ -579,7 +579,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       conn =
         conn
         |> 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"]
     end
index 78b0408eec977058308b3b762a3f71d41837f638..3155749aac87c9f21504b80763e89675a987e951 100644 (file)
@@ -159,7 +159,7 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
     end
   end
 
-  test "outbox paginates correctly" do
+  test "activity collection page aginates correctly" do
     user = insert(:user)
 
     posts =
@@ -171,13 +171,21 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
     # outbox sorts chronologically, newest first, with ten per page
     posts = Enum.reverse(posts)
 
-    %{"first" => %{"next" => next_url}} =
-      UserView.render("outbox.json", %{user: user, max_id: nil})
+    %{"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("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
     assert next_url =~ next_id
   end