commit: 4a28b81b591d6b87fa27378744052ccf8be44308
parent f47a1246985d6ce69ceca4104e43f630f1f33610
Author: feld <feld@feld.me>
Date: Tue, 11 Jun 2024 23:08:12 +0000
Merge branch 'fix-webpush-actor' into 'develop'
Fix WebPush actor regression
See merge request pleroma/pleroma!4146
Diffstat:
4 files changed, 134 insertions(+), 99 deletions(-)
diff --git a/changelog.d/web_push_actor_regression.skip b/changelog.d/web_push_actor_regression.skip
diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex
@@ -19,69 +19,71 @@ 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()}) | []
+ def build(
%{
activity: %{data: %{"type" => activity_type}} = activity,
- user: %User{id: user_id}
+ user_id: user_id
} = 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
- Logger.warning("Unknown notification type")
- {:error, :unknown_type}
+ def build(notif) do
+ Logger.warning("WebPush: unknown activity type: #{inspect(notif)}")
+ []
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
-
- {:ok, %{status: code}} when code in 200..299 ->
- :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
+ @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 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
@@ -140,9 +142,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")
@@ -199,19 +199,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
diff --git 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
@@ -5,6 +5,7 @@
defmodule Pleroma.Web.Push.ImplTest do
use Pleroma.DataCase, async: true
+ import ExUnit.CaptureLog
import Mox
import Pleroma.Factory
@@ -32,17 +33,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 +58,67 @@ 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
+ 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
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
@@ -402,4 +420,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
