From 568819c08afee68636a4871e78838db1ac1f590c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 11 Jun 2024 17:58:02 -0400 Subject: [PATCH] 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 @@ def format_body( 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 @@ def perform(%Job{args: %{"op" => "web_push", "notification_id" => notification_i |> 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 @@ test "performs sending notifications" 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