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/changelog.d/user-refresh-rework.skip b/changelog.d/user-refresh-rework.skip new file mode 100644 index 000000000..e69de29bb 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/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/lib/pleroma/user.ex b/lib/pleroma/user.ex index 884c1f302..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 @@ -2154,20 +2155,20 @@ 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) - - case {cached_user, maybe_fetched_user} do - {_, {:ok, %User{} = user}} -> - {:ok, user} - - {%User{} = user, _} -> - {:ok, user} - - _ -> - {:error, :not_found} + defp maybe_refresh(user) do + if needs_update?(user) do + UserRefreshWorker.new(%{"ap_id" => user.ap_id}) + |> Oban.insert() end end 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/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex index 13c054e05..d71e134cb 100644 --- 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 + @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 +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/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 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/user_test.exs b/test/pleroma/user_test.exs index 99ab90134..78018fedc 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -952,9 +952,16 @@ defmodule Pleroma.UserTest do {:ok, user} = User.get_or_fetch_by_ap_id("http://mastodon.example.org/users/admin") - assert user.inbox + # Oban job was generated to refresh the stale user + assert_enqueued(worker: "Pleroma.Workers.UserRefreshWorker", args: %{"ap_id" => user.ap_id}) - refute user.last_refreshed_at == orig_user.last_refreshed_at + # 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 + + 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 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 diff --git a/test/pleroma/web/push/impl_test.exs b/test/pleroma/web/push/impl_test.exs index d98bd1040..adaae93d0 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 @@ -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,37 +58,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 - 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 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 @@ -403,4 +421,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