From 382426e0338d7918cd2db7c72ede446a2a8f7f4f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 12:41:06 -0400 Subject: Remove Object.get_by_id_and_maybe_refetch/2 This was only used for poll refreshing and is not a good approach to the problem. --- lib/pleroma/object.ex | 21 --------------------- .../web/mastodon_api/controllers/poll_controller.ex | 2 +- 2 files changed, 1 insertion(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 748f18e6c..77dfda851 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -99,27 +99,6 @@ defmodule Pleroma.Object do def get_by_id(nil), do: nil def get_by_id(id), do: Repo.get(Object, id) - @spec get_by_id_and_maybe_refetch(integer(), list()) :: Object.t() | nil - def get_by_id_and_maybe_refetch(id, opts \\ []) do - with %Object{updated_at: updated_at} = object <- get_by_id(id) do - if opts[:interval] && - NaiveDateTime.diff(NaiveDateTime.utc_now(), updated_at) > opts[:interval] do - case Fetcher.refetch_object(object) do - {:ok, %Object{} = object} -> - object - - e -> - Logger.error("Couldn't refresh #{object.data["id"]}:\n#{inspect(e)}") - object - end - else - object - end - else - nil -> nil - end - end - def get_by_ap_id(nil), do: nil def get_by_ap_id(ap_id) do diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index a2af8148c..303b995f6 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -30,7 +30,7 @@ defmodule Pleroma.Web.MastodonAPI.PollController do @doc "GET /api/v1/polls/:id" def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do - with %Object{} = object <- Object.get_by_id_and_maybe_refetch(id, interval: 60), + with %Object{} = object <- Object.get_by_id(id), %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), true <- Visibility.visible_for_user?(activity, user) do try_render(conn, "show.json", %{object: object, for: user}) -- cgit v1.2.3 From c077a14ce1343f5515fa11938df7d808f23a566c Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 13:54:56 -0400 Subject: Add Oban job to handle poll refreshing and stream out the update --- .../mastodon_api/controllers/poll_controller.ex | 4 +++ lib/pleroma/workers/poll_worker.ex | 34 +++++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index 303b995f6..0d5a57518 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.PollController do alias Pleroma.Activity alias Pleroma.Object + alias Pleroma.Workers.PollWorker alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.CommonAPI alias Pleroma.Web.Plugs.OAuthScopesPlug @@ -33,6 +34,9 @@ defmodule Pleroma.Web.MastodonAPI.PollController do with %Object{} = object <- Object.get_by_id(id), %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), true <- Visibility.visible_for_user?(activity, user) do + PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) + |> Oban.insert(unique: [period: 60]) + try_render(conn, "show.json", %{object: object, for: user}) else error when is_nil(error) or error == false -> diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index d263aa1b9..0d2d67326 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -11,27 +11,41 @@ defmodule Pleroma.Workers.PollWorker do alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.Object + alias Pleroma.Object.Fetcher + + @stream_out_impl Pleroma.Config.get( + [__MODULE__, :stream_out], + Pleroma.Web.ActivityPub.ActivityPub + ) @impl true def perform(%Job{args: %{"op" => "poll_end", "activity_id" => activity_id}}) do - with %Activity{} = activity <- find_poll_activity(activity_id), + with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id(activity_id)}, {:ok, notifications} <- Notification.create_poll_notifications(activity) do Notification.stream(notifications) else - {:error, :poll_activity_not_found} = e -> {:cancel, e} + {:activity, nil} -> {:cancel, :poll_activity_not_found} e -> {:error, e} end end - @impl true - def timeout(_job), do: :timer.seconds(5) + def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do + with {_, %Activity{object: object}} <- + {:activity, Activity.get_by_id_with_object(activity_id)}, + {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do + stream_update(activity_id) - defp find_poll_activity(activity_id) do - with nil <- Activity.get_by_id(activity_id) do - {:error, :poll_activity_not_found} + :ok + else + {:activity, nil} -> {:cancel, :poll_activity_not_found} + {:refetch, _} = e -> {:cancel, e} + e -> {:error, e} end end + @impl true + def timeout(_job), do: :timer.seconds(5) + def schedule_poll_end(%Activity{data: %{"type" => "Create"}, id: activity_id} = activity) do with %Object{data: %{"type" => "Question", "closed" => closed}} when is_binary(closed) <- Object.normalize(activity), @@ -49,4 +63,10 @@ defmodule Pleroma.Workers.PollWorker do end def schedule_poll_end(activity), do: {:error, activity} + + defp stream_update(activity_id) do + Activity.get_by_id(activity_id) + |> Activity.normalize() + |> @stream_out_impl.stream_out() + end end -- cgit v1.2.3 From 4b3f604f9529c9ced23f747cb6f6d82fedfadab0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 14:02:41 -0400 Subject: Skip refetching poll results if the object's updated_at is newer than the poll closed timestamp --- lib/pleroma/workers/poll_worker.ex | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index 0d2d67326..a61c5eac1 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -32,6 +32,8 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do with {_, %Activity{object: object}} <- {:activity, Activity.get_by_id_with_object(activity_id)}, + {:ok, naive_closed} <- NaiveDateTime.from_iso8601(object.data["closed"]), + {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, naive_closed)}, {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do stream_update(activity_id) @@ -39,6 +41,7 @@ defmodule Pleroma.Workers.PollWorker do else {:activity, nil} -> {:cancel, :poll_activity_not_found} {:refetch, _} = e -> {:cancel, e} + {:closed_compare, _} -> {:cancel, :poll_finalized} e -> {:error, e} end end -- cgit v1.2.3 From 47ce3a4a961bd7496f8105bc957dbf958b77d342 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 14:17:35 -0400 Subject: Schedule a final poll refresh before streaming out the notifications --- lib/pleroma/workers/poll_worker.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index a61c5eac1..574daa9ba 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -22,6 +22,10 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "poll_end", "activity_id" => activity_id}}) do with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id(activity_id)}, {:ok, notifications} <- Notification.create_poll_notifications(activity) do + # Schedule a final refresh + __MODULE__.new(%{"op" => "refresh", "activity_id" => activity_id}) + |> Oban.insert() + Notification.stream(notifications) else {:activity, nil} -> {:cancel, :poll_activity_not_found} @@ -32,8 +36,8 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do with {_, %Activity{object: object}} <- {:activity, Activity.get_by_id_with_object(activity_id)}, - {:ok, naive_closed} <- NaiveDateTime.from_iso8601(object.data["closed"]), - {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, naive_closed)}, + {:ok, naive_closed} <- NaiveDateTime.from_iso8601(object.data["closed"]), + {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, naive_closed)}, {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do stream_update(activity_id) -- cgit v1.2.3 From a2e7db43aa3636569f4d770df980347a03c957fe Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 14:23:04 -0400 Subject: Rename assignment for consistency --- lib/pleroma/workers/poll_worker.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index 574daa9ba..f70ab48a4 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -36,8 +36,8 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do with {_, %Activity{object: object}} <- {:activity, Activity.get_by_id_with_object(activity_id)}, - {:ok, naive_closed} <- NaiveDateTime.from_iso8601(object.data["closed"]), - {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, naive_closed)}, + {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]), + {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)}, {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do stream_update(activity_id) -- cgit v1.2.3 From 9ff57946e7d6fa7dabaf90457e11041ce46991c4 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 30 Sep 2024 15:25:13 -0400 Subject: Credo --- lib/pleroma/web/mastodon_api/controllers/poll_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index 0d5a57518..f89bfa7f2 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -9,10 +9,10 @@ defmodule Pleroma.Web.MastodonAPI.PollController do alias Pleroma.Activity alias Pleroma.Object - alias Pleroma.Workers.PollWorker alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.CommonAPI alias Pleroma.Web.Plugs.OAuthScopesPlug + alias Pleroma.Workers.PollWorker action_fallback(Pleroma.Web.MastodonAPI.FallbackController) -- cgit v1.2.3 From 0a42a3f2eaf53fa87d934226874de5919320de26 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 2 Oct 2024 11:05:17 -0400 Subject: Do not attempt to schedule poll refresh jobs for local activities --- lib/pleroma/web/mastodon_api/controllers/poll_controller.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index f89bfa7f2..495f89278 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -34,8 +34,10 @@ defmodule Pleroma.Web.MastodonAPI.PollController do with %Object{} = object <- Object.get_by_id(id), %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), true <- Visibility.visible_for_user?(activity, user) do - PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) - |> Oban.insert(unique: [period: 60]) + unless activity.local do + PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) + |> Oban.insert(unique: [period: 60]) + end try_render(conn, "show.json", %{object: object, for: user}) else -- cgit v1.2.3 From ba2ae5e40bbe98d20be083d331222a9aea8b61de Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 3 Oct 2024 10:14:02 -0400 Subject: Check if a refresh is permitted by comparing timestamps before attempting to insert an Oban job It's better to avoid inserting an Oban job that will just be rejected if it's not expensive to check. --- .../web/mastodon_api/controllers/poll_controller.ex | 17 ++++++++++++----- lib/pleroma/workers/poll_worker.ex | 3 --- 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index 495f89278..4b347a6a7 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -32,12 +32,10 @@ defmodule Pleroma.Web.MastodonAPI.PollController do @doc "GET /api/v1/polls/:id" def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do with %Object{} = object <- Object.get_by_id(id), - %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), + %Activity{} = activity <- + Activity.get_create_by_object_ap_id_with_object(object.data["id"]), true <- Visibility.visible_for_user?(activity, user) do - unless activity.local do - PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) - |> Oban.insert(unique: [period: 60]) - end + maybe_refresh_poll(activity) try_render(conn, "show.json", %{object: object, for: user}) else @@ -76,4 +74,13 @@ defmodule Pleroma.Web.MastodonAPI.PollController do end end) end + + defp maybe_refresh_poll(%Activity{object: %Object{} = object} = activity) do + with false <- activity.local, + {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]), + {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)} do + PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) + |> Oban.insert(unique: [period: 60]) + end + end end diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index f70ab48a4..bb92634c9 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -36,8 +36,6 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do with {_, %Activity{object: object}} <- {:activity, Activity.get_by_id_with_object(activity_id)}, - {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]), - {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)}, {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do stream_update(activity_id) @@ -45,7 +43,6 @@ defmodule Pleroma.Workers.PollWorker do else {:activity, nil} -> {:cancel, :poll_activity_not_found} {:refetch, _} = e -> {:cancel, e} - {:closed_compare, _} -> {:cancel, :poll_finalized} e -> {:error, e} end end -- cgit v1.2.3 From b854e3836fd22a2589a6a6b97478998675d72048 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 3 Oct 2024 10:30:32 -0400 Subject: Remove pattern that can never match --- lib/pleroma/workers/poll_worker.ex | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index bb92634c9..7d69bea54 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -43,7 +43,6 @@ defmodule Pleroma.Workers.PollWorker do else {:activity, nil} -> {:cancel, :poll_activity_not_found} {:refetch, _} = e -> {:cancel, e} - e -> {:error, e} end end -- cgit v1.2.3 From a3038aa6a2189ced1e5c394a4e6e8be76f2644d0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 3 Oct 2024 11:01:33 -0400 Subject: Increase poll refresh interval to 120 seconds --- lib/pleroma/web/mastodon_api/controllers/poll_controller.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex index 4b347a6a7..6526457df 100644 --- a/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/poll_controller.ex @@ -28,6 +28,7 @@ defmodule Pleroma.Web.MastodonAPI.PollController do defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.PollOperation @cachex Pleroma.Config.get([:cachex, :provider], Cachex) + @poll_refresh_interval 120 @doc "GET /api/v1/polls/:id" def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do @@ -80,7 +81,7 @@ defmodule Pleroma.Web.MastodonAPI.PollController do {:ok, end_time} <- NaiveDateTime.from_iso8601(object.data["closed"]), {_, :lt} <- {:closed_compare, NaiveDateTime.compare(object.updated_at, end_time)} do PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id}) - |> Oban.insert(unique: [period: 60]) + |> Oban.insert(unique: [period: @poll_refresh_interval]) end end end -- cgit v1.2.3 From 03a6e33b81281256f2e9b6ffb75910fdd1a7894f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 9 Oct 2024 16:25:58 -0400 Subject: Skip the final refresh job if the activity is local --- lib/pleroma/workers/poll_worker.ex | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index 7d69bea54..a9afe9d63 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -22,9 +22,11 @@ defmodule Pleroma.Workers.PollWorker do def perform(%Job{args: %{"op" => "poll_end", "activity_id" => activity_id}}) do with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id(activity_id)}, {:ok, notifications} <- Notification.create_poll_notifications(activity) do - # Schedule a final refresh - __MODULE__.new(%{"op" => "refresh", "activity_id" => activity_id}) - |> Oban.insert() + unless activity.local do + # Schedule a final refresh + __MODULE__.new(%{"op" => "refresh", "activity_id" => activity_id}) + |> Oban.insert() + end Notification.stream(notifications) else -- cgit v1.2.3