fix(oauth): preserve dependent app cleanup
This commit is contained in:
parent
b4c624911c
commit
d30d205f0b
3 changed files with 31 additions and 14 deletions
|
|
@ -1 +1 @@
|
||||||
- Optimize orphaned OAuth apps cleanup by using a single delete query instead of deleting records one by one.
|
- Optimize orphaned OAuth apps cleanup by deleting records in batches instead of one by one.
|
||||||
|
|
|
||||||
|
|
@ -8,7 +8,7 @@ defmodule Pleroma.Web.OAuth.App do
|
||||||
import Ecto.Query
|
import Ecto.Query
|
||||||
alias Pleroma.Repo
|
alias Pleroma.Repo
|
||||||
alias Pleroma.User
|
alias Pleroma.User
|
||||||
alias Pleroma.Web.OAuth.Token
|
alias Pleroma.Web.OAuth.{Authorization, Token}
|
||||||
|
|
||||||
@type t :: %__MODULE__{}
|
@type t :: %__MODULE__{}
|
||||||
|
|
||||||
|
|
@ -193,18 +193,25 @@ defmodule Pleroma.Web.OAuth.App do
|
||||||
def remove_orphans(limit \\ 100) do
|
def remove_orphans(limit \\ 100) do
|
||||||
fifteen_mins_ago = NaiveDateTime.add(NaiveDateTime.utc_now(), -900)
|
fifteen_mins_ago = NaiveDateTime.add(NaiveDateTime.utc_now(), -900)
|
||||||
|
|
||||||
# First get the IDs of apps to delete
|
Repo.transaction(fn ->
|
||||||
app_ids =
|
app_ids =
|
||||||
from(a in __MODULE__,
|
from(a in __MODULE__,
|
||||||
where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago,
|
where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago,
|
||||||
limit: ^limit,
|
limit: ^limit,
|
||||||
select: a.id
|
lock: "FOR UPDATE SKIP LOCKED",
|
||||||
)
|
select: a.id
|
||||||
|> Repo.all()
|
)
|
||||||
|
|> Repo.all()
|
||||||
|
|
||||||
# Then delete those specific apps
|
# Bulk deletes bypass the schema's association cleanup.
|
||||||
from(a in __MODULE__, where: a.id in ^app_ids)
|
from(a in Authorization, where: a.app_id in ^app_ids) |> Repo.delete_all()
|
||||||
|> Repo.delete_all()
|
from(t in Token, where: t.app_id in ^app_ids) |> Repo.delete_all()
|
||||||
|
|
||||||
|
from(a in __MODULE__,
|
||||||
|
where: a.id in ^app_ids and is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago
|
||||||
|
)
|
||||||
|
|> Repo.delete_all()
|
||||||
|
end)
|
||||||
|
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,9 @@
|
||||||
defmodule Pleroma.Web.OAuth.AppTest do
|
defmodule Pleroma.Web.OAuth.AppTest do
|
||||||
use Pleroma.DataCase, async: true
|
use Pleroma.DataCase, async: true
|
||||||
|
|
||||||
alias Pleroma.Web.OAuth.App
|
alias Pleroma.MFA.Token, as: MFAToken
|
||||||
|
alias Pleroma.Web.OAuth.{App, Authorization, Token}
|
||||||
|
alias Pleroma.Web.Push.Subscription
|
||||||
import Pleroma.Factory
|
import Pleroma.Factory
|
||||||
|
|
||||||
describe "get_or_make/2" do
|
describe "get_or_make/2" do
|
||||||
|
|
@ -57,6 +59,10 @@ defmodule Pleroma.Web.OAuth.AppTest do
|
||||||
test "removes orphaned apps" do
|
test "removes orphaned apps" do
|
||||||
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
|
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
|
||||||
{:ok, %App{} = old_app} = App.get_or_make(attrs, ["write"])
|
{: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)
|
||||||
|
|
||||||
user = insert(:user)
|
user = insert(:user)
|
||||||
attrs = %{client_name: "OldButValid", redirect_uris: ".", user_id: user.id}
|
attrs = %{client_name: "OldButValid", redirect_uris: ".", user_id: user.id}
|
||||||
|
|
@ -74,6 +80,10 @@ defmodule Pleroma.Web.OAuth.AppTest do
|
||||||
App.remove_orphans()
|
App.remove_orphans()
|
||||||
|
|
||||||
refute Repo.get(App, old_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, recent_app.id)
|
||||||
assert Repo.get(App, old_owned_app.id)
|
assert Repo.get(App, old_owned_app.id)
|
||||||
end
|
end
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue