pleroma

commit: cdeeb4dcc5f6d91a96a7a5f407debcdcccc5087d
parent ff6f5a417f1f22d53b1196cc19e1ba9ad6e0ed2c
Author: lain <lain@soykaf.club>
Date:   Sat,  1 Jun 2024 09:54:12 +0000

Merge branch '3276-pinned-collection-fetch' into 'develop'

Use proper workers for fetching pins instead of an ad-hoc task

Closes #3276

See merge request pleroma/pleroma!4136
Diffstat:
A
changelog.d/pinned-collection-fetch.security
1
+
M
lib/pleroma/web/activity_pub/activity_pub.ex
25
+++++++++++++------------
M
test/pleroma/web/activity_pub/activity_pub_test.exs
30
++++++++++++++++++++++++------

3 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/changelog.d/pinned-collection-fetch.security b/changelog.d/pinned-collection-fetch.security
@@ -0,0 +1 @@
+Use proper workers for fetching pins instead of an ad-hoc task, fixing a potential fetch loop
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -1794,24 +1794,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  def pinned_fetch_task(nil), do: nil
-
-  def pinned_fetch_task(%{pinned_objects: pins}) do
-    if Enum.all?(pins, fn {ap_id, _} ->
-         Object.get_cached_by_ap_id(ap_id) ||
-           match?({:ok, _object}, Fetcher.fetch_object_from_id(ap_id))
-       end) do
-      :ok
-    else
-      :error
-    end
+  def enqueue_pin_fetches(%{pinned_objects: pins}) do
+    # enqueue a task to fetch all pinned objects
+    Enum.each(pins, fn {ap_id, _} ->
+      if is_nil(Object.get_cached_by_ap_id(ap_id)) do
+        Pleroma.Workers.RemoteFetcherWorker.enqueue("fetch_remote", %{
+          "id" => ap_id,
+          "depth" => 1
+        })
+      end
+    end)
   end
 
+  def enqueue_pin_fetches(_), do: nil
+
   def make_user_from_ap_id(ap_id, additional \\ []) do
     user = User.get_cached_by_ap_id(ap_id)
 
     with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do
-      {:ok, _pid} = Task.start(fn -> pinned_fetch_task(data) end)
+      enqueue_pin_fetches(data)
 
       if user do
         user
diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs
@@ -291,9 +291,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
             body: featured_data,
             headers: [{"content-type", "application/activity+json"}]
           }
-      end)
 
-      Tesla.Mock.mock_global(fn
         %{
           method: :get,
           url: ^object_url
@@ -306,7 +304,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       end)
 
       {:ok, user} = ActivityPub.make_user_from_ap_id(ap_id)
-      Process.sleep(50)
+
+      assert_enqueued(
+        worker: Pleroma.Workers.RemoteFetcherWorker,
+        args: %{
+          "op" => "fetch_remote",
+          "id" => object_url,
+          "depth" => 1
+        }
+      )
+
+      # wait for oban
+      Pleroma.Tests.ObanHelpers.perform_all()
 
       assert user.featured_address == featured_url
       assert Map.has_key?(user.pinned_objects, object_url)
@@ -368,9 +377,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
             body: featured_data,
             headers: [{"content-type", "application/activity+json"}]
           }
-      end)
 
-      Tesla.Mock.mock_global(fn
         %{
           method: :get,
           url: ^object_url
@@ -383,7 +390,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       end)
 
       {:ok, user} = ActivityPub.make_user_from_ap_id(ap_id)
-      Process.sleep(50)
+
+      assert_enqueued(
+        worker: Pleroma.Workers.RemoteFetcherWorker,
+        args: %{
+          "op" => "fetch_remote",
+          "id" => object_url,
+          "depth" => 1
+        }
+      )
+
+      # wait for oban
+      Pleroma.Tests.ObanHelpers.perform_all()
 
       assert user.featured_address == featured_url
       assert Map.has_key?(user.pinned_objects, object_url)