use akkoma collections for everything

lib/pleroma/collections/fetcher.ex

@@ -11,10 +11,7 @@ defmodule Akkoma.Collections.Fetcher do
   alias Pleroma.Config
   require Logger
 
-  def fetch_collection_by_ap_id(ap_id) when is_binary(ap_id) do
-    fetch_collection(ap_id)
-  end
-
+  @spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()}
   def fetch_collection(ap_id) when is_binary(ap_id) do
     with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do
       {:ok, objects_from_collection(page)}
@@ -26,7 +23,7 @@ defmodule Akkoma.Collections.Fetcher do
   end
 
   def fetch_collection(%{"type" => type} = page)
-      when type in ["Collection", "OrderedCollection"] do
+      when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do
     {:ok, objects_from_collection(page)}
   end
 
@@ -38,12 +35,13 @@ defmodule Akkoma.Collections.Fetcher do
        when is_list(items) and type in ["Collection", "CollectionPage"],
        do: items
 
-  defp objects_from_collection(%{"type" => "OrderedCollection", "orderedItems" => items})
-       when is_list(items),
-       do: items
+  defp objects_from_collection(%{"type" => type, "orderedItems" => items} = page)
+       when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"],
+       do: maybe_next_page(page, items)
 
-  defp objects_from_collection(%{"type" => "Collection", "items" => items}) when is_list(items),
-    do: items
+  defp objects_from_collection(%{"type" => type, "items" => items} = page)
+       when is_list(items) and type in ["Collection", "CollectionPage"],
+       do: maybe_next_page(page, items)
 
   defp objects_from_collection(%{"type" => type, "first" => first})
        when is_binary(first) and type in ["Collection", "OrderedCollection"] do
@@ -55,17 +53,27 @@ defmodule Akkoma.Collections.Fetcher do
     fetch_page_items(id)
   end
 
+  defp objects_from_collection(_page), do: []
+
   defp fetch_page_items(id, items \\ []) do
     if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do
       items
     else
-      {:ok, page} = Fetcher.fetch_and_contain_remote_object_from_id(id)
-      objects = items_in_page(page)
+      with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do
+        objects = items_in_page(page)
 
-      if Enum.count(objects) > 0 do
-        maybe_next_page(page, items ++ objects)
+        if Enum.count(objects) > 0 do
+          maybe_next_page(page, items ++ objects)
+        else
+          items
+        end
       else
-        items
+        {:error, "Object has been deleted"} ->
+          items
+
+        {:error, error} ->
+          Logger.error("Could not fetch page #{id} - #{inspect(error)}")
+          {:error, error}
       end
     end
   end

lib/pleroma/web/activity_pub/object_validators/article_note_page_validator.ex

@@ -6,7 +6,6 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do
   use Ecto.Schema
   alias Pleroma.User
   alias Pleroma.EctoType.ActivityPub.ObjectValidators
-  alias Pleroma.Object.Fetcher
   alias Pleroma.Web.CommonAPI.Utils
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
   alias Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
@@ -58,22 +57,10 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do
   defp fix_tag(%{"tag" => tag} = data) when is_map(tag), do: Map.put(data, "tag", [tag])
   defp fix_tag(data), do: Map.drop(data, ["tag"])
 
-  defp fix_replies(%{"replies" => %{"first" => %{"items" => replies}}} = data)
-       when is_list(replies),
-       do: Map.put(data, "replies", replies)
-
-  defp fix_replies(%{"replies" => %{"items" => replies}} = data) when is_list(replies),
-    do: Map.put(data, "replies", replies)
-
-  # TODO: Pleroma does not have any support for Collections at the moment.
-  # If the `replies` field is not something the ObjectID validator can handle,
-  # the activity/object would be rejected, which is bad behavior.
-  defp fix_replies(%{"replies" => replies} = data) when not is_list(replies),
-    do: Map.drop(data, ["replies"])
+  defp fix_replies(%{"replies" => replies} = data) when is_list(replies), do: data
 
   defp fix_replies(%{"replies" => %{"first" => first}} = data) do
-    with {:ok, %{"orderedItems" => replies}} <-
-           Fetcher.fetch_and_contain_remote_object_from_id(first) do
+    with {:ok, replies} <- Akkoma.Collections.Fetcher.fetch_collection(first) do
       Map.put(data, "replies", replies)
     else
       {:error, _} ->
@@ -82,7 +69,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.ArticleNotePageValidator do
     end
   end
 
-  defp fix_replies(data), do: data
+  defp fix_replies(data), do: Map.delete(data, "replies")
 
   defp remote_mention_resolver(
          %{"id" => ap_id, "tag" => tags},

lib/pleroma/web/plugs/http_signature_plug.ex

@@ -64,7 +64,9 @@ defmodule Pleroma.Web.Plugs.HTTPSignaturePlug do
     if has_signature_header?(conn) do
       # set (request-target) header to the appropriate value
       # we also replace the digest header with the one we computed
-      possible_paths = route_aliases(conn) ++ [conn.request_path, conn.request_path <> "?#{conn.query_string}"]
+      possible_paths =
+        route_aliases(conn) ++ [conn.request_path, conn.request_path <> "?#{conn.query_string}"]
+
       assign_valid_signature_on_route_aliases(conn, possible_paths)
     else
       Logger.debug("No signature header!")

test/pleroma/collections/collections_fetcher_test.exs

@@ -30,7 +30,7 @@ defmodule Akkoma.Collections.FetcherTest do
         }
     end)
 
-    {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id)
+    {:ok, objects} = Fetcher.fetch_collection(ap_id)
     assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects
   end
 
@@ -53,7 +53,7 @@ defmodule Akkoma.Collections.FetcherTest do
         }
     end)
 
-    {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id)
+    {:ok, objects} = Fetcher.fetch_collection(ap_id)
     assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects
   end
 
@@ -106,7 +106,7 @@ defmodule Akkoma.Collections.FetcherTest do
         }
     end)
 
-    {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id)
+    {:ok, objects} = Fetcher.fetch_collection(ap_id)
     assert [%{"type" => "Create"}, %{"type" => "Like"}] = objects
   end
 
@@ -161,7 +161,58 @@ defmodule Akkoma.Collections.FetcherTest do
         }
     end)
 
-    {:ok, objects} = Fetcher.fetch_collection_by_ap_id(ap_id)
+    {:ok, objects} = Fetcher.fetch_collection(ap_id)
+    assert [%{"type" => "Create"}] = objects
+  end
+
+  test "it should stop fetching when we hit a 404" do
+    clear_config([:activitypub, :max_collection_objects], 1)
+
+    unordered_collection =
+      "test/fixtures/collections/unordered_page_reference.json"
+      |> File.read!()
+
+    first_page =
+      "test/fixtures/collections/unordered_page_first.json"
+      |> File.read!()
+
+    ap_id = "https://example.com/collection/unordered_page_reference"
+    first_page_id = "https://example.com/collection/unordered_page_reference?page=1"
+    second_page_id = "https://example.com/collection/unordered_page_reference?page=2"
+
+    Tesla.Mock.mock(fn
+      %{
+        method: :get,
+        url: ^ap_id
+      } ->
+        %Tesla.Env{
+          status: 200,
+          body: unordered_collection,
+          headers: [{"content-type", "application/activity+json"}]
+        }
+
+      %{
+        method: :get,
+        url: ^first_page_id
+      } ->
+        %Tesla.Env{
+          status: 200,
+          body: first_page,
+          headers: [{"content-type", "application/activity+json"}]
+        }
+
+      %{
+        method: :get,
+        url: ^second_page_id
+      } ->
+        %Tesla.Env{
+          status: 404,
+          body: nil,
+          headers: [{"content-type", "application/activity+json"}]
+        }
+    end)
+
+    {:ok, objects} = Fetcher.fetch_collection(ap_id)
     assert [%{"type" => "Create"}] = objects
   end
 end

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -782,6 +782,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
         |> String.replace("{{status_id}}", status_id)
 
       status_url = "https://example.com/users/lain/statuses/#{status_id}"
+      replies_url = status_url <> "/replies?only_other_accounts=true&page=true"
 
       user =
         File.read!("test/fixtures/users_mock/user.json")
@@ -820,6 +821,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
               |> String.replace("{{nickname}}", "lain"),
             headers: [{"content-type", "application/activity+json"}]
           }
+
+        %{
+          method: :get,
+          url: ^replies_url
+        } ->
+          %Tesla.Env{
+            status: 404,
+            body: "",
+            headers: [{"content-type", "application/activity+json"}]
+          }
       end)
 
       data = %{

test/pleroma/web/activity_pub/transmogrifier/note_handling_test.exs

@@ -380,7 +380,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.NoteHandlingTest do
       clear_config([:instance, :federation_incoming_replies_max_depth], 10)
 
       {:ok, activity} = Transmogrifier.handle_incoming(data)
-
       object = Object.normalize(activity.data["object"])
 
       assert object.data["replies"] == items

test/support/http_request_mock.ex

@@ -407,6 +407,15 @@ defmodule HttpRequestMock do
      }}
   end
 
+  def get(
+        "http://mastodon.example.org/users/admin/statuses/99512778738411822/replies?min_id=99512778738411824&page=true",
+        _,
+        _,
+        _
+      ) do
+    {:ok, %Tesla.Env{status: 404, body: ""}}
+  end
+
   def get("http://mastodon.example.org/users/relay", _, _, [
         {"accept", "application/activity+json"}
       ]) do