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.
This commit is contained in:
Mark Felder 2024-10-03 10:14:02 -04:00
parent 0a42a3f2ea
commit ba2ae5e40b
3 changed files with 16 additions and 9 deletions

View file

@ -32,12 +32,10 @@ defmodule Pleroma.Web.MastodonAPI.PollController do
@doc "GET /api/v1/polls/:id" @doc "GET /api/v1/polls/:id"
def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do def show(%{assigns: %{user: user}, private: %{open_api_spex: %{params: %{id: id}}}} = conn, _) do
with %Object{} = object <- Object.get_by_id(id), 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 true <- Visibility.visible_for_user?(activity, user) do
unless activity.local do maybe_refresh_poll(activity)
PollWorker.new(%{"op" => "refresh", "activity_id" => activity.id})
|> Oban.insert(unique: [period: 60])
end
try_render(conn, "show.json", %{object: object, for: user}) try_render(conn, "show.json", %{object: object, for: user})
else else
@ -76,4 +74,13 @@ defp get_cached_vote_or_vote(object, user, choices) do
end end
end) 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 end

View file

@ -36,8 +36,6 @@ def perform(%Job{args: %{"op" => "poll_end", "activity_id" => activity_id}}) do
def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do
with {_, %Activity{object: object}} <- with {_, %Activity{object: object}} <-
{:activity, Activity.get_by_id_with_object(activity_id)}, {: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 {_, {:ok, _object}} <- {:refetch, Fetcher.refetch_object(object)} do
stream_update(activity_id) stream_update(activity_id)
@ -45,7 +43,6 @@ def perform(%Job{args: %{"op" => "refresh", "activity_id" => activity_id}}) do
else else
{:activity, nil} -> {:cancel, :poll_activity_not_found} {:activity, nil} -> {:cancel, :poll_activity_not_found}
{:refetch, _} = e -> {:cancel, e} {:refetch, _} = e -> {:cancel, e}
{:closed_compare, _} -> {:cancel, :poll_finalized}
e -> {:error, e} e -> {:error, e}
end end
end end

View file

@ -29,13 +29,16 @@ test "returns poll entity for object id", %{user: user, conn: conn} do
id = to_string(object.id) id = to_string(object.id)
assert %{"id" => ^id, "expired" => false, "multiple" => false} = response assert %{"id" => ^id, "expired" => false, "multiple" => false} = response
# Local activities should not generate an Oban job to refresh
assert activity.local
refute_enqueued( refute_enqueued(
worker: Pleroma.Workers.PollWorker, worker: Pleroma.Workers.PollWorker,
args: %{"op" => "refresh", "activity_id" => activity.id} args: %{"op" => "refresh", "activity_id" => activity.id}
) )
end end
test "does not create oban job to refresh poll if activity is local", %{conn: conn} do test "creates an oban job to refresh poll if activity is remote", %{conn: conn} do
user = insert(:user, local: false) user = insert(:user, local: false)
question = insert(:question, user: user) question = insert(:question, user: user)
activity = insert(:question_activity, question: question, local: false) activity = insert(:question_activity, question: question, local: false)