From 568819c08afee68636a4871e78838db1ac1f590c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 11 Jun 2024 17:58:02 -0400 Subject: WebPush refactoring: separate build and deliver steps --- lib/pleroma/web/push/impl.ex | 95 ++++++++++++++++---------------- lib/pleroma/workers/web_pusher_worker.ex | 4 +- 2 files changed, 51 insertions(+), 48 deletions(-) (limited to 'lib') 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) + + case WebPushEncryption.send_web_push(payload, formatted_subscription, gcm_api_key) do + {:ok, %{status: code}} when code in 200..299 -> + :ok - {:ok, %{status: code}} when code in 200..299 -> - :ok + {: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 + {: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 -- cgit v1.2.3 From 603a57576638c9732fe873db3c2b8d56d0a413a5 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 11 Jun 2024 18:14:07 -0400 Subject: The user is not always preloaded into the notification --- lib/pleroma/web/push/impl.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib') 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)) -- cgit v1.2.3 From 6a9d9da26feefe9b19e8072fd378f01a977b9d1a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 11 Jun 2024 18:38:41 -0400 Subject: Cyclical complexity --- lib/pleroma/web/push/impl.ex | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) (limited to 'lib') 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 -- cgit v1.2.3 From 5c8afbe646c874eea32d7063aa499c97191f3a6e Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 11 Jun 2024 18:54:22 -0400 Subject: Fix tests --- lib/pleroma/web/push/impl.ex | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib') 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" -- cgit v1.2.3 From 1ae5c2b020810eda7243e7e6b52cf89e6bb7f8d0 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Wed, 12 Jun 2024 12:40:01 +0400 Subject: Transmogrifier: Encode Emoji id to be valid. --- lib/pleroma/web/activity_pub/transmogrifier.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') 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", -- cgit v1.2.3 From e37845cd351d0d9cbdae469b75a532edbaa3c0ed Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sun, 16 Jun 2024 16:26:24 -0400 Subject: Stale user refreshing should be done async to prevent blocking of rendering activities --- lib/pleroma/user.ex | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'lib') 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) - - maybe_fetched_user = needs_update?(cached_user) && fetch_by_ap_id(ap_id) - - case {cached_user, maybe_fetched_user} do - {_, {:ok, %User{} = user}} -> - {:ok, user} + 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 - {%User{} = user, _} -> - {:ok, user} + defp maybe_refresh(user) do + fun = fn -> needs_update?(user) && fetch_by_ap_id(user.ap_id) end - _ -> - {:error, :not_found} + if Config.get([__MODULE__, :sync_refreshing], false) do + fun.() + else + Task.start(fun) end end -- cgit v1.2.3 From 9c6763725547e4927d09cf3cd8d33949a28c4824 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 17 Jun 2024 10:08:54 -0400 Subject: Refactor the async user refreshing to use Oban Previous implementation could cause duplicate simultaneous profile fetches which is not polite. --- lib/pleroma/user.ex | 10 ++++------ lib/pleroma/workers/user_refresh_worker.ex | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 lib/pleroma/workers/user_refresh_worker.ex (limited to 'lib') 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 +# 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 -- cgit v1.2.3