From c3c14f5860051d0311ae34900ef0181ada5c630d Mon Sep 17 00:00:00 2001
From: Lain Soykaf <lain@lain.com>
Date: Tue, 21 Jan 2025 14:36:50 +0400
Subject: [PATCH] OAuth App: Fix test, optimize code.

---
 .../optimize-orphaned-apps-cleanup.change     |  1 +
 lib/pleroma/web/o_auth/app.ex                 | 14 ++++---
 test/pleroma/web/o_auth/app_test.exs          | 42 ++++++++++++++++---
 3 files changed, 46 insertions(+), 11 deletions(-)
 create mode 100644 changelog.d/optimize-orphaned-apps-cleanup.change

diff --git a/changelog.d/optimize-orphaned-apps-cleanup.change b/changelog.d/optimize-orphaned-apps-cleanup.change
new file mode 100644
index 000000000..e86b75524
--- /dev/null
+++ b/changelog.d/optimize-orphaned-apps-cleanup.change
@@ -0,0 +1 @@
+- Optimize orphaned OAuth apps cleanup by using a single delete query instead of deleting records one by one.
diff --git a/lib/pleroma/web/o_auth/app.ex b/lib/pleroma/web/o_auth/app.ex
index 7661c2566..30a9d9134 100644
--- a/lib/pleroma/web/o_auth/app.ex
+++ b/lib/pleroma/web/o_auth/app.ex
@@ -168,16 +168,20 @@ 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 ->
+    # First get the IDs of apps to delete
+    app_ids =
       from(a in __MODULE__,
         where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago,
-        limit: ^limit
+        limit: ^limit,
+        select: a.id
       )
       |> Repo.all()
-      |> Enum.each(&Repo.delete(&1))
-    end)
+
+    # Then delete those specific apps
+    from(a in __MODULE__, where: a.id in ^app_ids)
+    |> Repo.delete_all()
 
     :ok
   end
diff --git a/test/pleroma/web/o_auth/app_test.exs b/test/pleroma/web/o_auth/app_test.exs
index 44219cf90..871b10ac3 100644
--- a/test/pleroma/web/o_auth/app_test.exs
+++ b/test/pleroma/web/o_auth/app_test.exs
@@ -55,19 +55,49 @@ defmodule Pleroma.Web.OAuth.AppTest do
   end
 
   test "removes orphaned apps" do
+    # Create an orphaned app (no user_id)
     attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
     {:ok, %App{} = old_app} = App.get_or_make(attrs, ["write"])
 
-    attrs = %{client_name: "PleromaFE", redirect_uris: "."}
-    {:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
+    # Create a non-orphaned app with a user
+    user = insert(:user)
+    attrs = %{client_name: "PleromaFE", redirect_uris: ".", user_id: user.id}
+    {:ok, %App{} = kept_app} = App.get_or_make(attrs, ["write"])
+
+    # Create an old but non-orphaned app
+    attrs = %{client_name: "OldButValid", redirect_uris: ".", user_id: user.id}
+    {:ok, %App{} = old_kept_app} = App.get_or_make(attrs, ["write"])
+
+    # backdate both old apps so they're within the threshold for being cleaned up
+    # 1 hour ago
+    past_time = NaiveDateTime.add(NaiveDateTime.utc_now(), -3600)
 
-    # backdate the old app so it's within the threshold for being cleaned up
     {:ok, _} =
-      "UPDATE apps SET inserted_at = now() - interval '1 hour' WHERE id = #{old_app.id}"
-      |> Pleroma.Repo.query()
+      Repo.query(
+        "UPDATE apps SET inserted_at = $1 WHERE id IN ($2, $3)",
+        [past_time, old_app.id, old_kept_app.id]
+      )
+
+    # Ensure the updates were applied
+    updated_app = Repo.get(App, old_app.id)
+    updated_kept_app = Repo.get(App, old_kept_app.id)
+
+    assert NaiveDateTime.compare(
+             updated_app.inserted_at,
+             NaiveDateTime.add(NaiveDateTime.utc_now(), -900)
+           ) == :lt
+
+    assert NaiveDateTime.compare(
+             updated_kept_app.inserted_at,
+             NaiveDateTime.add(NaiveDateTime.utc_now(), -900)
+           ) == :lt
 
     App.remove_orphans()
 
-    assert [app] == Pleroma.Repo.all(App)
+    # Verify the orphaned app was removed
+    assert is_nil(Repo.get(App, old_app.id))
+    # Verify both non-orphaned apps still exist, regardless of age
+    assert Repo.get(App, kept_app.id)
+    assert Repo.get(App, old_kept_app.id)
   end
 end