diff --git a/changelog.d/optimize-orphaned-apps-cleanup.change b/changelog.d/optimize-orphaned-apps-cleanup.change new file mode 100644 index 000000000..3f037f125 --- /dev/null +++ b/changelog.d/optimize-orphaned-apps-cleanup.change @@ -0,0 +1 @@ +- Optimize orphaned OAuth apps cleanup by deleting records in batches instead of one by one. diff --git a/lib/pleroma/web/o_auth/app.ex b/lib/pleroma/web/o_auth/app.ex index a2841b2bb..9c16c23b6 100644 --- a/lib/pleroma/web/o_auth/app.ex +++ b/lib/pleroma/web/o_auth/app.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Web.OAuth.App do import Ecto.Query alias Pleroma.Repo alias Pleroma.User + alias Pleroma.Web.OAuth.Authorization alias Pleroma.Web.OAuth.Token @type t :: %__MODULE__{} @@ -191,15 +192,26 @@ defmodule Pleroma.Web.OAuth.App do @spec remove_orphans(pos_integer()) :: :ok def remove_orphans(limit \\ 100) do - fifteen_mins_ago = DateTime.add(DateTime.utc_now(), -900, :second) + fifteen_mins_ago = NaiveDateTime.add(NaiveDateTime.utc_now(), -900) Repo.transaction(fn -> + app_ids = + from(a in __MODULE__, + where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago, + limit: ^limit, + lock: "FOR UPDATE SKIP LOCKED", + select: a.id + ) + |> Repo.all() + + # Bulk deletes bypass the schema's association cleanup. + from(a in Authorization, where: a.app_id in ^app_ids) |> Repo.delete_all() + from(t in Token, where: t.app_id in ^app_ids) |> Repo.delete_all() + from(a in __MODULE__, - where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago, - limit: ^limit + where: a.id in ^app_ids and is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago ) - |> Repo.all() - |> Enum.each(&Repo.delete(&1)) + |> Repo.delete_all() end) :ok diff --git a/test/pleroma/web/o_auth/app_test.exs b/test/pleroma/web/o_auth/app_test.exs index a69ba371e..9a5045155 100644 --- a/test/pleroma/web/o_auth/app_test.exs +++ b/test/pleroma/web/o_auth/app_test.exs @@ -5,7 +5,11 @@ defmodule Pleroma.Web.OAuth.AppTest do use Pleroma.DataCase, async: true + alias Pleroma.MFA.Token, as: MFAToken alias Pleroma.Web.OAuth.App + alias Pleroma.Web.OAuth.Authorization + alias Pleroma.Web.OAuth.Token + alias Pleroma.Web.Push.Subscription import Pleroma.Factory describe "get_or_make/2" do @@ -57,29 +61,32 @@ defmodule Pleroma.Web.OAuth.AppTest do test "removes orphaned apps" do attrs = %{client_name: "Mastodon-Local", redirect_uris: "."} {:ok, %App{} = old_app} = App.get_or_make(attrs, ["write"]) + authorization = insert(:oauth_authorization, app: old_app) + token = insert(:oauth_token, app: old_app) + mfa_token = insert(:mfa_token, authorization: authorization) + push_subscription = insert(:push_subscription, token: token) - # backdate the old app so it's within the threshold for being cleaned up - one_hour_ago = DateTime.add(DateTime.utc_now(), -3600) + user = insert(:user) + attrs = %{client_name: "OldButValid", redirect_uris: ".", user_id: user.id} + {:ok, %App{} = old_owned_app} = App.get_or_make(attrs, ["write"]) + + one_hour_ago = NaiveDateTime.add(NaiveDateTime.utc_now(), -3600) {:ok, _} = - "UPDATE apps SET inserted_at = $1, updated_at = $1 WHERE id = $2" - |> Pleroma.Repo.query([one_hour_ago, old_app.id]) + "UPDATE apps SET inserted_at = $1, updated_at = $1 WHERE id IN ($2, $3)" + |> Repo.query([one_hour_ago, old_app.id, old_owned_app.id]) - # Create the new app after backdating the old one attrs = %{client_name: "PleromaFE", redirect_uris: "."} - {:ok, %App{} = app} = App.get_or_make(attrs, ["write"]) - - # Ensure the new app has a recent timestamp - now = DateTime.utc_now() - - {:ok, _} = - "UPDATE apps SET inserted_at = $1, updated_at = $1 WHERE id = $2" - |> Pleroma.Repo.query([now, app.id]) + {:ok, %App{} = recent_app} = App.get_or_make(attrs, ["write"]) App.remove_orphans() - assert [returned_app] = Pleroma.Repo.all(App) - assert returned_app.client_name == "PleromaFE" - assert returned_app.id == app.id + refute Repo.get(App, old_app.id) + refute Repo.get(Authorization, authorization.id) + refute Repo.get(Token, token.id) + refute Repo.get(MFAToken, mfa_token.id) + refute Repo.get(Subscription, push_subscription.id) + assert Repo.get(App, recent_app.id) + assert Repo.get(App, old_owned_app.id) end end