From c7fb78cc32a91255f4bdc6af0db6b9c88b9e95b3 Mon Sep 17 00:00:00 2001 From: ilja Date: Sun, 21 May 2023 11:47:38 +0200 Subject: [PATCH 1/2] Move deadline and old_insert_date to setup Several tests for prune_objetcs need a date older than the deadline for pruning, so I moved that to the setup --- test/mix/tasks/pleroma/database_test.exs | 73 ++++++++---------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/test/mix/tasks/pleroma/database_test.exs b/test/mix/tasks/pleroma/database_test.exs index 9edb2c115..21c365412 100644 --- a/test/mix/tasks/pleroma/database_test.exs +++ b/test/mix/tasks/pleroma/database_test.exs @@ -45,21 +45,25 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest 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 +73,7 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest 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 +87,14 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest 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 - deadline = Pleroma.Config.get([:instance, :remote_post_retention_days]) + 1 - - date = - Timex.now() - |> Timex.shift(days: -deadline) - |> Timex.to_naive_datetime() - |> NaiveDateTime.truncate(:second) - + 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 +104,7 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest 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!() @@ -120,16 +117,10 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do 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 +203,9 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do assert length(Repo.all(Object)) == 2 end - test "with the --keep-threads option it deletes old threads with no local interaction" do - 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) - + test "with the --keep-threads option it deletes old threads with no local interaction", %{ + old_insert_date: old_insert_date + } do remote_user = insert(:user, local: false) remote_user2 = insert(:user, local: false) @@ -261,15 +246,9 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do assert length(Repo.all(Object)) == 0 end - test "with the --keep-threads option it keeps old threads with local interaction" do - 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) - + test "with the --keep-threads option it keeps old threads with local interaction", %{ + old_insert_date: old_insert_date + } do remote_user = insert(:user, local: false) local_user = insert(:user, local: true) @@ -326,15 +305,9 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do assert length(Repo.all(Object)) == 4 end - test "with the --keep-threads option it keeps old threads with bookmarked posts" do - 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) - + test "with the --keep-threads option it keeps old threads with bookmarked posts", %{ + old_insert_date: old_insert_date + } do remote_user = insert(:user, local: false) local_user = insert(:user, local: true) From f49e9e6d4cb7cfdd0c2fbb2ea74c82a7ec7d13e4 Mon Sep 17 00:00:00 2001 From: ilja Date: Sun, 21 May 2023 13:02:28 +0200 Subject: [PATCH 2/2] Clean up bookmarks after prune_objects When doing prune_objects, it's possible that bookmarked objects are deleted. This gave problems when fetching the bookmark TL. Here we clean up the bookmarks during pruning in the case were it's possible that bookmarked objects are deleted. --- lib/mix/tasks/pleroma/database.ex | 15 +++++++++++++++ test/mix/tasks/pleroma/database_test.exs | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/mix/tasks/pleroma/database.ex b/lib/mix/tasks/pleroma/database.ex index 726a22d41..55d1c8ddc 100644 --- a/lib/mix/tasks/pleroma/database.ex +++ b/lib/mix/tasks/pleroma/database.ex @@ -171,6 +171,21 @@ defmodule Mix.Tasks.Pleroma.Database 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 """ diff --git a/test/mix/tasks/pleroma/database_test.exs b/test/mix/tasks/pleroma/database_test.exs index 21c365412..40c5fd402 100644 --- a/test/mix/tasks/pleroma/database_test.exs +++ b/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 @@ -87,6 +88,29 @@ defmodule Mix.Tasks.Pleroma.DatabaseTest do refute Object.get_by_id(note_remote_non_public_id) end + test "it cleans up bookmarks", %{old_insert_date: old_insert_date} do + user = insert(:user) + {:ok, old_object_activity} = CommonAPI.post(user, %{status: "yadayada"}) + + Repo.one(Object) + |> Ecto.Changeset.change(%{updated_at: old_insert_date}) + |> Repo.update!() + + {: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)