Merge pull request 'Clean up bookmarks after prune_objects' (#544) from ilja/akkoma:clean_up_bookmarks_after_prune_objects into develop

Reviewed-on: AkkomaGang/akkoma#544

lib/mix/tasks/pleroma/database.ex

@@ -171,6 +171,21 @@ def run(["prune_objects" | args]) do
     end
     |> Repo.delete_all(timeout: :infinity)
 
+    if !Keyword.get(options, :keep_threads) do
+      # Without the --keep-threads option, it's possible that bookmarked
+      # objects have been deleted. We remove the corresponding bookmarks.
+      """
+      delete from public.bookmarks
+      where id in (
+        select b.id from public.bookmarks b
+        left join public.activities a on b.activity_id = a.id
+        left join public.objects o on a."data" ->> 'object' = o.data ->> 'id'
+        where o.id is null
+      )
+      """
+      |> Repo.query([], timeout: :infinity)
+    end
+
     if Keyword.get(options, :prune_orphaned_activities) do
       # Prune activities who link to a single object
       """

test/mix/tasks/pleroma/database_test.exs

@@ -7,6 +7,7 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do
   use Oban.Testing, repo: Pleroma.Repo
 
   alias Pleroma.Activity
+  alias Pleroma.Bookmark
   alias Pleroma.Object
   alias Pleroma.Repo
   alias Pleroma.User
@@ -45,21 +46,25 @@ test "it replaces objects with references" do
   end
 
   describe "prune_objects" do
-    test "it prunes old objects from the database" do
+    setup do
       deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
 
-      date =
+      old_insert_date =
         Timex.now()
         |> Timex.shift(days: -deadline)
         |> Timex.to_naive_datetime()
         |> NaiveDateTime.truncate(:second)
 
+      %{old_insert_date: old_insert_date}
+    end
+
+    test "it prunes old objects from the database", %{old_insert_date: old_insert_date} do
       insert(:note)
 
       %{id: note_remote_public_id} =
         :note
         |> insert()
-        |> Ecto.Changeset.change(%{updated_at: date})
+        |> Ecto.Changeset.change(%{updated_at: old_insert_date})
         |> Repo.update!()
 
       note_remote_non_public =
@@ -69,7 +74,7 @@ test "it prunes old objects from the database" do
 
       note_remote_non_public
       |> Ecto.Changeset.change(%{
-        updated_at: date,
+        updated_at: old_insert_date,
         data: note_remote_non_public_data |> update_in(["to"], fn _ -> [] end)
       })
       |> Repo.update!()
@@ -83,21 +88,37 @@ test "it prunes old objects from the database" do
       refute Object.get_by_id(note_remote_non_public_id)
     end
 
-    test "with the --keep-non-public option it still keeps non-public posts even if they are not local" do
+    test "it cleans up bookmarks", %{old_insert_date: old_insert_date} do
-      deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
+      user = insert(:user)
+      {:ok, old_object_activity} = CommonAPI.post(user, %{status: "yadayada"})
 
-      date =
+      Repo.one(Object)
-        Timex.now()
+      |> Ecto.Changeset.change(%{updated_at: old_insert_date})
-        |> Timex.shift(days: -deadline)
+      |> Repo.update!()
-        |> Timex.to_naive_datetime()
-        |> NaiveDateTime.truncate(:second)
 
+      {:ok, new_object_activity} = CommonAPI.post(user, %{status: "yadayada"})
+
+      {:ok, _} = Bookmark.create(user.id, old_object_activity.id)
+      {:ok, _} = Bookmark.create(user.id, new_object_activity.id)
+
+      assert length(Repo.all(Object)) == 2
+      assert length(Repo.all(Bookmark)) == 2
+
+      Mix.Tasks.Pleroma.Database.run(["prune_objects"])
+
+      assert length(Repo.all(Object)) == 1
+      assert length(Repo.all(Bookmark)) == 1
+      refute Bookmark.get(user.id, old_object_activity.id)
+    end
+
+    test "with the --keep-non-public option it still keeps non-public posts even if they are not local",
+         %{old_insert_date: old_insert_date} do
       insert(:note)
 
       %{id: note_remote_id} =
         :note
         |> insert()
-        |> Ecto.Changeset.change(%{updated_at: date})
+        |> Ecto.Changeset.change(%{updated_at: old_insert_date})
         |> Repo.update!()
 
       note_remote_non_public =
@@ -107,7 +128,7 @@ test "with the --keep-non-public option it still keeps non-public posts even if
 
       note_remote_non_public
       |> Ecto.Changeset.change(%{
-        updated_at: date,
+        updated_at: old_insert_date,
         data: note_remote_non_public_data |> update_in(["to"], fn _ -> [] end)
       })
       |> Repo.update!()
@@ -120,16 +141,10 @@ test "with the --keep-non-public option it still keeps non-public posts even if
       refute Object.get_by_id(note_remote_id)
     end
 
-    test "with the --keep-threads and --keep-non-public option it keeps old threads with non-public replies even if the interaction is not local" do
+    test "with the --keep-threads and --keep-non-public option it keeps old threads with non-public replies even if the interaction is not local",
+         %{old_insert_date: old_insert_date} do
       # For non-public we only check Create Activities because only these are relevant for threads
       # Flags are always non-public, Announces from relays can be non-public...
-      deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
-
-      old_insert_date =
-        Timex.now()
-        |> Timex.shift(days: -deadline)
-        |> Timex.to_naive_datetime()
-        |> NaiveDateTime.truncate(:second)
 
       remote_user1 = insert(:user, local: false)
       remote_user2 = insert(:user, local: false)
@@ -212,15 +227,9 @@ test "with the --keep-threads option it still keeps non-old threads even with no
       assert length(Repo.all(Object)) == 2
     end
 
-    test "with the --keep-threads option it deletes old threads with no local interaction" do
+    test "with the --keep-threads option it deletes old threads with no local interaction", %{
-      deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
+      old_insert_date: old_insert_date
-
+    } do
-      old_insert_date =
-        Timex.now()
-        |> Timex.shift(days: -deadline)
-        |> Timex.to_naive_datetime()
-        |> NaiveDateTime.truncate(:second)
-
       remote_user = insert(:user, local: false)
       remote_user2 = insert(:user, local: false)
 
@@ -261,15 +270,9 @@ test "with the --keep-threads option it deletes old threads with no local intera
       assert length(Repo.all(Object)) == 0
     end
 
-    test "with the --keep-threads option it keeps old threads with local interaction" do
+    test "with the --keep-threads option it keeps old threads with local interaction", %{
-      deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
+      old_insert_date: old_insert_date
-
+    } do
-      old_insert_date =
-        Timex.now()
-        |> Timex.shift(days: -deadline)
-        |> Timex.to_naive_datetime()
-        |> NaiveDateTime.truncate(:second)
-
       remote_user = insert(:user, local: false)
       local_user = insert(:user, local: true)
 
@@ -326,15 +329,9 @@ test "with the --keep-threads option it keeps old threads with local interaction
       assert length(Repo.all(Object)) == 4
     end
 
-    test "with the --keep-threads option it keeps old threads with bookmarked posts" do
+    test "with the --keep-threads option it keeps old threads with bookmarked posts", %{
-      deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1
+      old_insert_date: old_insert_date
-
+    } do
-      old_insert_date =
-        Timex.now()
-        |> Timex.shift(days: -deadline)
-        |> Timex.to_naive_datetime()
-        |> NaiveDateTime.truncate(:second)
-
       remote_user = insert(:user, local: false)
       local_user = insert(:user, local: true)