Merge branch 'backport/1.1-hackney-and-outbox-fixes' into 'maint/1.1'

Backport/1.1 hackney and outbox fixes

See merge request pleroma/pleroma!1718

CHANGELOG.md

@@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - AdminAPI: Add "godmode" while fetching user statuses (i.e. admin can see private statuses)
 - Improve digest email template
 – Pagination: (optional) return `total` alongside with `items` when paginating
+- ActivityPub: The first page in inboxes/outboxes is no longer embedded.
 
 ### Fixed
 - Following from Osada

lib/pleroma/web/activity_pub/activity_pub.ex

@@ -864,7 +864,7 @@ defp restrict_muted_reblogs(query, %{"muting_user" => %User{info: info}}) 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

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -197,12 +197,43 @@ def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) d
     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),
          {:ok, user} <- User.ensure_keys_present(user) do
       conn
       |> put_resp_content_type("application/activity+json")
-      |> json(UserView.render("outbox.json", %{user: user, max_id: params["max_id"]}))
+      |> put_view(UserView)
+      |> render("activity_collection.json", %{iri: "#{user.ap_id}/outbox"})
     end
   end
 
@@ -278,12 +309,37 @@ def whoami(_conn, _params), do: {:error, :not_found}
 
   def read_inbox(
         %{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
     |> put_resp_content_type("application/activity+json")
     |> 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
 
   def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do

lib/pleroma/web/activity_pub/views/user_view.ex

@@ -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
@@ -207,25 +206,22 @@ def render("followers.json", %{user: user} = opts) do
     |> Map.merge(Utils.make_json_ld_header())
   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
+    # this is sorted chronologically, so first activity is the newest (max)
-        Map.put(params, "max_id", max_qid)
-      else
-        params
-      end
-
-    activities = ActivityPub.fetch_user_activities(user, nil, params)
-
     {max_id, min_id, collection} =
       if length(activities) > 0 do
         {
-          Enum.at(Enum.reverse(activities), 0).id,
           Enum.at(activities, 0).id,
+          Enum.at(Enum.reverse(activities), 0).id,
           Enum.map(activities, fn act ->
             {:ok, data} = Transmogrifier.prepare_outgoing(act.data)
             data
@@ -239,71 +235,14 @@ def render("outbox.json", %{user: user, max_id: max_qid}) do
         }
       end
 
-    iri = "#{user.ap_id}/outbox"
+    %{
-
+      "id" => "#{iri}?max_id=#{max_id}&page=true",
-    page = %{
-      "id" => "#{iri}?max_id=#{max_id}",
       "type" => "OrderedCollectionPage",
       "partOf" => iri,
       "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
 
   def collection(collection, iri, page, show_items \\ true, total \\ nil) do

mix.lock

@@ -37,7 +37,7 @@
   "floki": {:hex, :floki, "0.20.4", "be42ac911fece24b4c72f3b5846774b6e61b83fe685c2fc9d62093277fb3bc86", [:mix], [{:html_entities, "~> 0.4.0", [hex: :html_entities, repo: "hexpm", optional: false]}, {:mochiweb, "~> 2.15", [hex: :mochiweb, repo: "hexpm", optional: false]}], "hexpm"},
   "gen_smtp": {:hex, :gen_smtp, "0.14.0", "39846a03522456077c6429b4badfd1d55e5e7d0fdfb65e935b7c5e38549d9202", [:rebar3], [], "hexpm"},
   "gettext": {:hex, :gettext, "0.17.0", "abe21542c831887a2b16f4c94556db9c421ab301aee417b7c4fbde7fbdbe01ec", [:mix], [], "hexpm"},
-  "hackney": {:hex, :hackney, "1.15.1", "9f8f471c844b8ce395f7b6d8398139e26ddca9ebc171a8b91342ee15a19963f4", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.4", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
+  "hackney": {:hex, :hackney, "1.15.2", "07e33c794f8f8964ee86cebec1a8ed88db5070e52e904b8f12209773c1036085", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.5", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
   "html_entities": {:hex, :html_entities, "0.4.0", "f2fee876858cf6aaa9db608820a3209e45a087c5177332799592142b50e89a6b", [:mix], [], "hexpm"},
   "html_sanitize_ex": {:hex, :html_sanitize_ex, "1.3.0", "f005ad692b717691203f940c686208aa3d8ffd9dd4bb3699240096a51fa9564e", [:mix], [{:mochiweb, "~> 2.15", [hex: :mochiweb, repo: "hexpm", optional: false]}], "hexpm"},
   "http_signatures": {:git, "https://git.pleroma.social/pleroma/http_signatures.git", "293d77bb6f4a67ac8bde1428735c3b42f22cbb30", [ref: "293d77bb6f4a67ac8bde1428735c3b42f22cbb30"]},
@@ -79,7 +79,7 @@
   "quack": {:hex, :quack, "0.1.1", "cca7b4da1a233757fdb44b3334fce80c94785b3ad5a602053b7a002b5a8967bf", [:mix], [{:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: false]}, {:tesla, "~> 1.2.0", [hex: :tesla, repo: "hexpm", optional: false]}], "hexpm"},
   "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm"},
   "recon": {:git, "https://github.com/ferd/recon.git", "75d70c7c08926d2f24f1ee6de14ee50fe8a52763", [tag: "2.4.0"]},
-  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm"},
+  "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.5", "6eaf7ad16cb568bb01753dbbd7a95ff8b91c7979482b95f38443fe2c8852a79b", [:make, :mix, :rebar3], [], "hexpm"},
   "sweet_xml": {:hex, :sweet_xml, "0.6.6", "fc3e91ec5dd7c787b6195757fbcf0abc670cee1e4172687b45183032221b66b8", [:mix], [], "hexpm"},
   "swoosh": {:hex, :swoosh, "0.23.2", "7dda95ff0bf54a2298328d6899c74dae1223777b43563ccebebb4b5d2b61df38", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm"},
   "syslog": {:git, "https://github.com/Vagabond/erlang-syslog.git", "4a6c6f2c996483e86c1320e9553f91d337bcb6aa", [tag: "1.0.5"]},

test/web/activity_pub/activity_pub_controller_test.exs

@@ -473,7 +473,7 @@ test "it returns a note activity in a collection", %{conn: conn} 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
@@ -559,7 +559,7 @@ test "it returns a note activity in a collection", %{conn: conn} 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
@@ -571,7 +571,7 @@ test "it returns an announce activity in a collection", %{conn: conn} 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

test/web/activity_pub/views/user_view_test.exs

@@ -121,5 +121,36 @@ test "sets totalItems to zero when follows are hidden" do
       user = Map.put(user, :info, info)
       assert %{"totalItems" => 0} = UserView.render("following.json", %{user: user})
     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