From 568819c08afee68636a4871e78838db1ac1f590c Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 11 Jun 2024 17:58:02 -0400
Subject: [PATCH 1/8] WebPush refactoring: separate build and deliver steps

---
 lib/pleroma/web/push/impl.ex             | 95 ++++++++++++------------
 lib/pleroma/workers/web_pusher_worker.ex |  4 +-
 test/pleroma/web/push/impl_test.exs      | 77 +++++++++++--------
 3 files changed, 97 insertions(+), 79 deletions(-)

diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex
index 13c054e05..c5ba7ca65 100644
--- a/lib/pleroma/web/push/impl.ex
+++ b/lib/pleroma/web/push/impl.ex
@@ -19,69 +19,72 @@ defmodule Pleroma.Web.Push.Impl do
   @body_chars 140
   @types ["Create", "Follow", "Announce", "Like", "Move", "EmojiReact", "Update"]
 
-  @doc "Performs sending notifications for user subscriptions"
-  @spec perform(Notification.t()) :: list(any) | :error | {:error, :unknown_type}
-  def perform(
+  @doc "Builds webpush notification payloads for the subscriptions enabled by the receiving user"
+  @spec build(Notification.t()) ::
+          list(%{content: map(), subscription: Subscription.t()})
+          | :error
+          | {:error, :unknown_type}
+  def build(
         %{
           activity: %{data: %{"type" => activity_type}} = activity,
-          user: %User{id: user_id}
+          user: user
         } = notification
       )
       when activity_type in @types do
-    user = User.get_cached_by_ap_id(notification.activity.data["actor"])
+    notification_actor = User.get_cached_by_ap_id(notification.activity.data["actor"])
+    avatar_url = User.avatar_url(notification_actor)
 
-    gcm_api_key = Application.get_env(:web_push_encryption, :gcm_api_key)
-    avatar_url = User.avatar_url(user)
     object = Object.normalize(activity, fetch: false)
-    user = User.get_cached_by_id(user_id)
     direct_conversation_id = Activity.direct_conversation_id(activity, user)
 
-    for subscription <- fetch_subscriptions(user_id),
-        Subscription.enabled?(subscription, notification.type) do
-      %{
-        access_token: subscription.token.token,
-        notification_id: notification.id,
-        notification_type: notification.type,
-        icon: avatar_url,
-        preferred_locale: "en",
-        pleroma: %{
-          activity_id: notification.activity.id,
-          direct_conversation_id: direct_conversation_id
+    subscriptions = fetch_subscriptions(user.id)
+
+    subscriptions
+    |> Enum.filter(&Subscription.enabled?(&1, notification.type))
+    |> Enum.map(fn subscription ->
+      payload =
+        %{
+          access_token: subscription.token.token,
+          notification_id: notification.id,
+          notification_type: notification.type,
+          icon: avatar_url,
+          preferred_locale: "en",
+          pleroma: %{
+            activity_id: notification.activity.id,
+            direct_conversation_id: direct_conversation_id
+          }
         }
-      }
-      |> Map.merge(build_content(notification, user, object))
-      |> Jason.encode!()
-      |> push_message(build_sub(subscription), gcm_api_key, subscription)
-    end
-    |> (&{:ok, &1}).()
+        |> Map.merge(build_content(notification, notification_actor, object))
+        |> Jason.encode!()
+
+      %{payload: payload, subscription: subscription}
+    end)
   end
 
-  def perform(_) do
+  def build(_) do
     Logger.warning("Unknown notification type")
     {:error, :unknown_type}
   end
 
-  @doc "Push message to web"
-  def push_message(body, sub, api_key, subscription) do
-    try do
-      case WebPushEncryption.send_web_push(body, sub, api_key) do
-        {:ok, %{status: code}} when code in 400..499 ->
-          Logger.debug("Removing subscription record")
-          Repo.delete!(subscription)
-          :ok
+  @doc "Deliver push notification to the provided webpush subscription"
+  @spec deliver(%{payload: String.t(), subscription: Subscription.t()}) :: :ok | :error
+  def deliver(%{payload: payload, subscription: subscription}) do
+    gcm_api_key = Application.get_env(:web_push_encryption, :gcm_api_key)
+    formatted_subscription = build_sub(subscription)
 
-        {:ok, %{status: code}} when code in 200..299 ->
-          :ok
+    case WebPushEncryption.send_web_push(payload, formatted_subscription, gcm_api_key) do
+      {:ok, %{status: code}} when code in 200..299 ->
+        :ok
 
-        {:ok, %{status: code}} ->
-          Logger.error("Web Push Notification failed with code: #{code}")
-          :error
+      {:ok, %{status: code}} when code in 400..499 ->
+        Logger.debug("Removing subscription record")
+        Repo.delete!(subscription)
+        :ok
+
+      {:ok, %{status: code}} ->
+        Logger.error("Web Push Notification failed with code: #{code}")
+        :error
 
-        error ->
-          Logger.error("Web Push Notification failed with #{inspect(error)}")
-          :error
-      end
-    rescue
       error ->
         Logger.error("Web Push Notification failed with #{inspect(error)}")
         :error
@@ -140,9 +143,7 @@ defmodule Pleroma.Web.Push.Impl do
 
     content_text = content <> "\n"
 
-    options_text =
-      Enum.map(options, fn x -> "○ #{x["name"]}" end)
-      |> Enum.join("\n")
+    options_text = Enum.map_join(options, "\n", fn x -> "○ #{x["name"]}" end)
 
     [content_text, options_text]
     |> Enum.join("\n")
diff --git a/lib/pleroma/workers/web_pusher_worker.ex b/lib/pleroma/workers/web_pusher_worker.ex
index 67e84b0c9..c549d3cd6 100644
--- a/lib/pleroma/workers/web_pusher_worker.ex
+++ b/lib/pleroma/workers/web_pusher_worker.ex
@@ -5,6 +5,7 @@
 defmodule Pleroma.Workers.WebPusherWorker do
   alias Pleroma.Notification
   alias Pleroma.Repo
+  alias Pleroma.Web.Push.Impl
 
   use Pleroma.Workers.WorkerHelper, queue: "web_push"
 
@@ -15,7 +16,8 @@ defmodule Pleroma.Workers.WebPusherWorker do
       |> Repo.get(notification_id)
       |> Repo.preload([:activity, :user])
 
-    Pleroma.Web.Push.Impl.perform(notification)
+    Impl.build(notification)
+    |> Enum.each(&Impl.deliver(&1))
   end
 
   @impl Oban.Worker
diff --git a/test/pleroma/web/push/impl_test.exs b/test/pleroma/web/push/impl_test.exs
index c263a1280..7f8dc2e6e 100644
--- a/test/pleroma/web/push/impl_test.exs
+++ b/test/pleroma/web/push/impl_test.exs
@@ -32,17 +32,6 @@ defmodule Pleroma.Web.Push.ImplTest do
     :ok
   end
 
-  @sub %{
-    endpoint: "https://example.com/example/1234",
-    keys: %{
-      auth: "8eDyX_uCN0XRhSbY5hs7Hg==",
-      p256dh:
-        "BCIWgsnyXDv1VkhqL2P7YRBvdeuDnlwAPT2guNhdIoW3IP7GmHh1SMKPLxRf7x8vJy6ZFK3ol2ohgn_-0yP7QQA="
-    }
-  }
-  @api_key "BASgACIHpN1GYgzSRp"
-  @message "@Bob: Lorem ipsum dolor sit amet, consectetur  adipiscing elit. Fusce sagittis finibus turpis."
-
   test "performs sending notifications" do
     user = insert(:user)
     user2 = insert(:user)
@@ -68,39 +57,65 @@ defmodule Pleroma.Web.Push.ImplTest do
         type: "mention"
       )
 
-    assert Impl.perform(notif) == {:ok, [:ok, :ok]}
+    Impl.build(notif)
+    |> Enum.each(fn push -> assert match?(:ok, Impl.deliver(push)) end)
   end
 
   @tag capture_log: true
   test "returns error if notif does not match " do
-    assert Impl.perform(%{}) == {:error, :unknown_type}
-  end
-
-  test "successful message sending" do
-    assert Impl.push_message(@message, @sub, @api_key, %Subscription{}) == :ok
+    assert Impl.build(%{}) == {:error, :unknown_type}
   end
 
   @tag capture_log: true
   test "fail message sending" do
-    assert Impl.push_message(
-             @message,
-             Map.merge(@sub, %{endpoint: "https://example.com/example/bad"}),
-             @api_key,
-             %Subscription{}
-           ) == :error
+    user = insert(:user)
+
+    insert(:push_subscription,
+      user: user,
+      endpoint: "https://example.com/example/bad",
+      data: %{alerts: %{"follow" => true}}
+    )
+
+    other_user = insert(:user)
+    {:ok, _, _, activity} = CommonAPI.follow(user, other_user)
+
+    notif =
+      insert(:notification,
+        user: user,
+        activity: activity,
+        type: "follow"
+      )
+
+    [push] = Impl.build(notif)
+
+    assert Impl.deliver(push) == :error
   end
 
   test "delete subscription if result send message between 400..500" do
-    subscription = insert(:push_subscription)
+    user = insert(:user)
 
-    assert Impl.push_message(
-             @message,
-             Map.merge(@sub, %{endpoint: "https://example.com/example/not_found"}),
-             @api_key,
-             subscription
-           ) == :ok
+    bad_subscription =
+      insert(:push_subscription,
+        user: user,
+        endpoint: "https://example.com/example/not_found",
+        data: %{alerts: %{"follow" => true}}
+      )
 
-    refute Pleroma.Repo.get(Subscription, subscription.id)
+    other_user = insert(:user)
+    {:ok, _, _, activity} = CommonAPI.follow(user, other_user)
+
+    notif =
+      insert(:notification,
+        user: user,
+        activity: activity,
+        type: "follow"
+      )
+
+    [push] = Impl.build(notif)
+
+    assert Impl.deliver(push) == :ok
+
+    refute Pleroma.Repo.get(Subscription, bad_subscription.id)
   end
 
   test "deletes subscription when token has been deleted" do

From 603a57576638c9732fe873db3c2b8d56d0a413a5 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 11 Jun 2024 18:14:07 -0400
Subject: [PATCH 2/8] The user is not always preloaded into the notification

---
 lib/pleroma/web/push/impl.ex | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex
index c5ba7ca65..65801922d 100644
--- a/lib/pleroma/web/push/impl.ex
+++ b/lib/pleroma/web/push/impl.ex
@@ -27,7 +27,7 @@ defmodule Pleroma.Web.Push.Impl do
   def build(
         %{
           activity: %{data: %{"type" => activity_type}} = activity,
-          user: user
+          user_id: user_id
         } = notification
       )
       when activity_type in @types do
@@ -35,9 +35,10 @@ defmodule Pleroma.Web.Push.Impl do
     avatar_url = User.avatar_url(notification_actor)
 
     object = Object.normalize(activity, fetch: false)
+    user = User.get_cached_by_id(user_id)
     direct_conversation_id = Activity.direct_conversation_id(activity, user)
 
-    subscriptions = fetch_subscriptions(user.id)
+    subscriptions = fetch_subscriptions(user_id)
 
     subscriptions
     |> Enum.filter(&Subscription.enabled?(&1, notification.type))

From a291a6b8c0c6e27ba6f9a03d9a1183184fc2ef1d Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 11 Jun 2024 18:14:48 -0400
Subject: [PATCH 3/8] Ensure the webpush notification for e.g., mentions start
 with the nickname of the actor it originates from

---
 changelog.d/web_push_actor_regression.skip |  0
 test/pleroma/web/push/impl_test.exs        | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 changelog.d/web_push_actor_regression.skip

diff --git a/changelog.d/web_push_actor_regression.skip b/changelog.d/web_push_actor_regression.skip
new file mode 100644
index 000000000..e69de29bb
diff --git a/test/pleroma/web/push/impl_test.exs b/test/pleroma/web/push/impl_test.exs
index 7f8dc2e6e..0eead956f 100644
--- a/test/pleroma/web/push/impl_test.exs
+++ b/test/pleroma/web/push/impl_test.exs
@@ -417,4 +417,23 @@ defmodule Pleroma.Web.Push.ImplTest do
              }
     end
   end
+
+  test "build/1 notification payload body starts with nickname of actor the notification originated from" do
+    user = insert(:user, nickname: "Bob")
+    user2 = insert(:user, nickname: "Tom")
+    insert(:push_subscription, user: user2, data: %{alerts: %{"mention" => true}})
+
+    {:ok, activity} =
+      CommonAPI.post(user, %{
+        status: "@Tom Hey are you okay?"
+      })
+
+    {:ok, [notification]} = Notification.create_notifications(activity)
+
+    [push] = Impl.build(notification)
+
+    {:ok, payload} = Jason.decode(push.payload)
+
+    assert String.starts_with?(payload["body"], "@Bob:")
+  end
 end

From 6a9d9da26feefe9b19e8072fd378f01a977b9d1a Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 11 Jun 2024 18:38:41 -0400
Subject: [PATCH 4/8] Cyclical complexity

---
 lib/pleroma/web/push/impl.ex | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex
index 65801922d..2bcfa97f5 100644
--- a/lib/pleroma/web/push/impl.ex
+++ b/lib/pleroma/web/push/impl.ex
@@ -201,19 +201,15 @@ defmodule Pleroma.Web.Push.Impl do
     "New Direct Message"
   end
 
-  def format_title(%{type: type}) do
-    case type do
-      "mention" -> "New Mention"
-      "status" -> "New Status"
-      "follow" -> "New Follower"
-      "follow_request" -> "New Follow Request"
-      "reblog" -> "New Repeat"
-      "favourite" -> "New Favorite"
-      "update" -> "New Update"
-      "pleroma:chat_mention" -> "New Chat Message"
-      "pleroma:emoji_reaction" -> "New Reaction"
-      "poll" -> "Poll Results"
-      type -> "New #{String.capitalize(type || "event")}"
-    end
-  end
+  def format_title(%{type: "mention"}), do: "New Mention"
+  def format_title(%{type: "status"}), do: "New Status"
+  def format_title(%{type: "follow"}), do: "New Follower"
+  def format_title(%{type: "follow_request"}), do: "New Follow Request"
+  def format_title(%{type: "reblog"}), do: "New Repeat"
+  def format_title(%{type: "favourite"}), do: "New Favorite"
+  def format_title(%{type: "update"}), do: "New Update"
+  def format_title(%{type: "pleroma:chat_mention"}), do: "New Chat Message"
+  def format_title(%{type: "pleroma:emoji_reaction"}), do: "New Reaction"
+  def format_title(%{type: "poll"}), do: "Poll Results"
+  def format_title(%{type: type}), do: "New #{String.capitalize(type || "event")}"
 end

From 5c8afbe646c874eea32d7063aa499c97191f3a6e Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 11 Jun 2024 18:54:22 -0400
Subject: [PATCH 5/8] Fix tests

---
 lib/pleroma/web/push/impl.ex        | 10 ++++------
 test/pleroma/web/push/impl_test.exs |  7 +++++--
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex
index 2bcfa97f5..d71e134cb 100644
--- a/lib/pleroma/web/push/impl.ex
+++ b/lib/pleroma/web/push/impl.ex
@@ -21,9 +21,7 @@ defmodule Pleroma.Web.Push.Impl do
 
   @doc "Builds webpush notification payloads for the subscriptions enabled by the receiving user"
   @spec build(Notification.t()) ::
-          list(%{content: map(), subscription: Subscription.t()})
-          | :error
-          | {:error, :unknown_type}
+          list(%{content: map(), subscription: Subscription.t()}) | []
   def build(
         %{
           activity: %{data: %{"type" => activity_type}} = activity,
@@ -62,9 +60,9 @@ defmodule Pleroma.Web.Push.Impl do
     end)
   end
 
-  def build(_) do
-    Logger.warning("Unknown notification type")
-    {:error, :unknown_type}
+  def build(notif) do
+    Logger.warning("WebPush: unknown activity type: #{inspect(notif)}")
+    []
   end
 
   @doc "Deliver push notification to the provided webpush subscription"
diff --git a/test/pleroma/web/push/impl_test.exs b/test/pleroma/web/push/impl_test.exs
index 0eead956f..169c380c7 100644
--- a/test/pleroma/web/push/impl_test.exs
+++ b/test/pleroma/web/push/impl_test.exs
@@ -5,6 +5,7 @@
 defmodule Pleroma.Web.Push.ImplTest do
   use Pleroma.DataCase, async: true
 
+  import ExUnit.CaptureLog
   import Mox
   import Pleroma.Factory
 
@@ -62,8 +63,10 @@ defmodule Pleroma.Web.Push.ImplTest do
   end
 
   @tag capture_log: true
-  test "returns error if notif does not match " do
-    assert Impl.build(%{}) == {:error, :unknown_type}
+  test "returns error if notification activity type does not match" do
+    assert capture_log(fn ->
+             assert Impl.build(%{}) == []
+           end) =~ "WebPush: unknown activity type"
   end
 
   @tag capture_log: true

From 1ae5c2b020810eda7243e7e6b52cf89e6bb7f8d0 Mon Sep 17 00:00:00 2001
From: Lain Soykaf <lain@lain.com>
Date: Wed, 12 Jun 2024 12:40:01 +0400
Subject: [PATCH 6/8] Transmogrifier: Encode Emoji id to be valid.

---
 changelog.d/3280-fix-emoji-ids.fix                 |  1 +
 lib/pleroma/web/activity_pub/transmogrifier.ex     |  6 ++++--
 .../transmogrifier/emoji_tag_building_test.exs     | 14 ++++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/3280-fix-emoji-ids.fix
 create mode 100644 test/pleroma/web/activity_pub/transmogrifier/emoji_tag_building_test.exs

diff --git a/changelog.d/3280-fix-emoji-ids.fix b/changelog.d/3280-fix-emoji-ids.fix
new file mode 100644
index 000000000..1bce5b653
--- /dev/null
+++ b/changelog.d/3280-fix-emoji-ids.fix
@@ -0,0 +1 @@
+Fix Emoji object IDs not always being valid
diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
index edfe73a25..4d851559f 100644
--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -913,9 +913,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
 
   def add_emoji_tags(object), do: object
 
-  defp build_emoji_tag({name, url}) do
+  def build_emoji_tag({name, url}) do
+    url = URI.encode(url)
+
     %{
-      "icon" => %{"url" => "#{URI.encode(url)}", "type" => "Image"},
+      "icon" => %{"url" => "#{url}", "type" => "Image"},
       "name" => ":" <> name <> ":",
       "type" => "Emoji",
       "updated" => "1970-01-01T00:00:00Z",
diff --git a/test/pleroma/web/activity_pub/transmogrifier/emoji_tag_building_test.exs b/test/pleroma/web/activity_pub/transmogrifier/emoji_tag_building_test.exs
new file mode 100644
index 000000000..c632c199c
--- /dev/null
+++ b/test/pleroma/web/activity_pub/transmogrifier/emoji_tag_building_test.exs
@@ -0,0 +1,14 @@
+defmodule Pleroma.Web.ActivityPub.Transmogrifier.EmojiTagBuildingTest do
+  use Pleroma.DataCase, async: true
+
+  alias Pleroma.Web.ActivityPub.Transmogrifier
+
+  test "it encodes the id to be a valid url" do
+    name = "hanapog"
+    url = "https://misskey.local.live/emojis/hana pog.png"
+
+    tag = Transmogrifier.build_emoji_tag({name, url})
+
+    assert tag["id"] == "https://misskey.local.live/emojis/hana%20pog.png"
+  end
+end

From e37845cd351d0d9cbdae469b75a532edbaa3c0ed Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Sun, 16 Jun 2024 16:26:24 -0400
Subject: [PATCH 7/8] Stale user refreshing should be done async to prevent
 blocking of rendering activities

---
 changelog.d/user-refresh.change |  1 +
 config/test.exs                 |  2 ++
 lib/pleroma/user.ex             | 25 ++++++++++++++-----------
 test/pleroma/user_test.exs      |  7 +++++--
 4 files changed, 22 insertions(+), 13 deletions(-)
 create mode 100644 changelog.d/user-refresh.change

diff --git a/changelog.d/user-refresh.change b/changelog.d/user-refresh.change
new file mode 100644
index 000000000..b91169a9e
--- /dev/null
+++ b/changelog.d/user-refresh.change
@@ -0,0 +1 @@
+User profile refreshes are now asynchronous
diff --git a/config/test.exs b/config/test.exs
index 0d4c82e0e..8cd3b0216 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -183,6 +183,8 @@ config :pleroma, Pleroma.Emoji.Loader, test_emoji: true
 
 config :pleroma, Pleroma.Web.RichMedia.Backfill, provider: Pleroma.Web.RichMedia.Backfill
 
+config :pleroma, Pleroma.User, sync_refreshing: true
+
 if File.exists?("./config/test.secret.exs") do
   import_config "test.secret.exs"
 else
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 884c1f302..440dc9210 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -2154,20 +2154,23 @@ defmodule Pleroma.User do
 
   def fetch_by_ap_id(ap_id), do: ActivityPub.make_user_from_ap_id(ap_id)
 
+  @spec get_or_fetch_by_ap_id(String.t()) :: {:ok, User.t()} | {:error, any()}
   def get_or_fetch_by_ap_id(ap_id) do
-    cached_user = get_cached_by_ap_id(ap_id)
+    with cached_user = %User{} <- get_cached_by_ap_id(ap_id),
+         _ <- maybe_refresh(cached_user) do
+      {:ok, cached_user}
+    else
+      _ -> fetch_by_ap_id(ap_id)
+    end
+  end
 
-    maybe_fetched_user = needs_update?(cached_user) && fetch_by_ap_id(ap_id)
+  defp maybe_refresh(user) do
+    fun = fn -> needs_update?(user) && fetch_by_ap_id(user.ap_id) end
 
-    case {cached_user, maybe_fetched_user} do
-      {_, {:ok, %User{} = user}} ->
-        {:ok, user}
-
-      {%User{} = user, _} ->
-        {:ok, user}
-
-      _ ->
-        {:error, :not_found}
+    if Config.get([__MODULE__, :sync_refreshing], false) do
+      fun.()
+    else
+      Task.start(fun)
     end
   end
 
diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs
index 5b7a65658..0da9969d0 100644
--- a/test/pleroma/user_test.exs
+++ b/test/pleroma/user_test.exs
@@ -953,9 +953,12 @@ defmodule Pleroma.UserTest do
 
       {:ok, user} = User.get_or_fetch_by_ap_id("http://mastodon.example.org/users/admin")
 
-      assert user.inbox
+      # User was updated async, fetch from cache now
+      updated_user = User.get_cached_by_ap_id(user.ap_id)
 
-      refute user.last_refreshed_at == orig_user.last_refreshed_at
+      assert updated_user.inbox
+
+      refute updated_user.last_refreshed_at == orig_user.last_refreshed_at
     end
 
     test "if nicknames clash, the old user gets a prefix with the old id to the nickname" do

From 9c6763725547e4927d09cf3cd8d33949a28c4824 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Mon, 17 Jun 2024 10:08:54 -0400
Subject: [PATCH 8/8] Refactor the async user refreshing to use Oban

Previous implementation could cause duplicate simultaneous profile fetches which is not polite.
---
 changelog.d/user-refresh-rework.skip       |  0
 config/test.exs                            |  2 --
 lib/pleroma/user.ex                        | 10 ++++------
 lib/pleroma/workers/user_refresh_worker.ex | 14 ++++++++++++++
 test/pleroma/user_test.exs                 |  8 ++++++--
 5 files changed, 24 insertions(+), 10 deletions(-)
 create mode 100644 changelog.d/user-refresh-rework.skip
 create mode 100644 lib/pleroma/workers/user_refresh_worker.ex

diff --git a/changelog.d/user-refresh-rework.skip b/changelog.d/user-refresh-rework.skip
new file mode 100644
index 000000000..e69de29bb
diff --git a/config/test.exs b/config/test.exs
index 8cd3b0216..0d4c82e0e 100644
--- a/config/test.exs
+++ b/config/test.exs
@@ -183,8 +183,6 @@ config :pleroma, Pleroma.Emoji.Loader, test_emoji: true
 
 config :pleroma, Pleroma.Web.RichMedia.Backfill, provider: Pleroma.Web.RichMedia.Backfill
 
-config :pleroma, Pleroma.User, sync_refreshing: true
-
 if File.exists?("./config/test.secret.exs") do
   import_config "test.secret.exs"
 else
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 440dc9210..7a8a68931 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -38,6 +38,7 @@ defmodule Pleroma.User do
   alias Pleroma.Web.OAuth
   alias Pleroma.Web.RelMe
   alias Pleroma.Workers.BackgroundWorker
+  alias Pleroma.Workers.UserRefreshWorker
 
   require Logger
   require Pleroma.Constants
@@ -2165,12 +2166,9 @@ defmodule Pleroma.User do
   end
 
   defp maybe_refresh(user) do
-    fun = fn -> needs_update?(user) && fetch_by_ap_id(user.ap_id) end
-
-    if Config.get([__MODULE__, :sync_refreshing], false) do
-      fun.()
-    else
-      Task.start(fun)
+    if needs_update?(user) do
+      UserRefreshWorker.new(%{"ap_id" => user.ap_id})
+      |> Oban.insert()
     end
   end
 
diff --git a/lib/pleroma/workers/user_refresh_worker.ex b/lib/pleroma/workers/user_refresh_worker.ex
new file mode 100644
index 000000000..5842143f8
--- /dev/null
+++ b/lib/pleroma/workers/user_refresh_worker.ex
@@ -0,0 +1,14 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Workers.UserRefreshWorker do
+  use Pleroma.Workers.WorkerHelper, queue: "background", max_attempts: 1, unique: [period: 300]
+
+  alias Pleroma.User
+
+  @impl Oban.Worker
+  def perform(%Job{args: %{"ap_id" => ap_id}}) do
+    User.fetch_by_ap_id(ap_id)
+  end
+end
diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs
index 0da9969d0..031621875 100644
--- a/test/pleroma/user_test.exs
+++ b/test/pleroma/user_test.exs
@@ -953,8 +953,12 @@ defmodule Pleroma.UserTest do
 
       {:ok, user} = User.get_or_fetch_by_ap_id("http://mastodon.example.org/users/admin")
 
-      # User was updated async, fetch from cache now
-      updated_user = User.get_cached_by_ap_id(user.ap_id)
+      # Oban job was generated to refresh the stale user
+      assert_enqueued(worker: "Pleroma.Workers.UserRefreshWorker", args: %{"ap_id" => user.ap_id})
+
+      # Run job to refresh the user; just capture its output instead of fetching it again
+      assert {:ok, updated_user} =
+               perform_job(Pleroma.Workers.UserRefreshWorker, %{"ap_id" => user.ap_id})
 
       assert updated_user.inbox