From b31388b5dfc62bd5f1b3383ff189a0a522a1f4ec Mon Sep 17 00:00:00 2001 From: Oneric Date: Mon, 3 Jun 2024 23:07:10 +0200 Subject: [PATCH] Delay attachment deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise attachments have a high chance to disappear with akkoma-fe’s “delete & redraft” feature when cleanup is enabled in the backend. Since we don't know whether a deletion was intended to be part of a redraft process or even if whether the redraft was abandoned we still have to delete attachments eventually. A thirty minute delay should provide sufficient time for redrafting. Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/775 --- CHANGELOG.md | 2 ++ config/config.exs | 1 + docs/docs/configuration/cheatsheet.md | 1 + .../workers/attachments_cleanup_worker.ex | 6 ++++- .../attachments_cleanup_worker_test.exs | 26 +++++++++++++++++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1644dad..937a56f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Meilisearch: it is now possible to use separate keys for search and admin actions - New standalone `prune_orphaned_activities` mix task with configurable batch limit - The `prune_objects` mix task now accepts a `--limit` parameter for initial object pruning +- New config option `:instance, :cleanup_attachments_delay` ## Fixed - Meilisearch: order of results returned from our REST API now actually matches how Meilisearch ranks results @@ -23,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - AP objects with additional JSON-LD profiles beyond ActivityStreams can now be fetched - Single-selection polls no longer expose the voter_count; MastoAPI demands it be null and this confused some clients leading to vote distributions >100% +- Fix “Delete & Redraft” often losing attachments if attachment cleanup was enabled ## Changed - Refactored Rich Media to cache the content in the database. Fetching operations that could block status rendering have been eliminated. diff --git a/config/config.exs b/config/config.exs index e919910b3..39b53a010 100644 --- a/config/config.exs +++ b/config/config.exs @@ -255,6 +255,7 @@ external_user_synchronization: true, extended_nickname_format: true, cleanup_attachments: false, + cleanup_attachments_delay: 1800, multi_factor_authentication: [ totp: [ # digits 6 or 8 diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index 916e1cc0c..9a50fc2bb 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -58,6 +58,7 @@ To add configuration to your config file, you can copy it from the base config. * `registration_reason_length`: Maximum registration reason length (default: `500`). * `external_user_synchronization`: Enabling following/followers counters synchronization for external users. * `cleanup_attachments`: Remove attachments along with statuses. Does not affect duplicate files and attachments without status. Enabling this will increase load to database when deleting statuses on larger instances. +* `cleanup_attachments_delay`: How many seconds to wait after post deletion before attempting to deletion; useful for “delete & redraft” functionality (default: `1800`) * `show_reactions`: Let favourites and emoji reactions be viewed through the API (default: `true`). * `password_reset_token_validity`: The time after which reset tokens aren't accepted anymore, in seconds (default: one day). * `local_bubble`: Array of domains representing instances closely related to yours. Used to populate the `bubble` timeline. e.g `["example.com"]`, (default: `[]`) diff --git a/lib/pleroma/workers/attachments_cleanup_worker.ex b/lib/pleroma/workers/attachments_cleanup_worker.ex index 58bbda94b..f1204a861 100644 --- a/lib/pleroma/workers/attachments_cleanup_worker.ex +++ b/lib/pleroma/workers/attachments_cleanup_worker.ex @@ -23,7 +23,11 @@ def enqueue_if_needed(%{ with true <- Config.get([:instance, :cleanup_attachments]), true <- URI.parse(actor).host == Pleroma.Web.Endpoint.host(), [_ | _] <- attachments do - enqueue("cleanup_attachments", %{"actor" => actor, "attachments" => attachments}) + enqueue( + "cleanup_attachments", + %{"actor" => actor, "attachments" => attachments}, + schedule_in: Config.get!([:instance, :cleanup_attachments_delay]) + ) else _ -> {:ok, :skip} end diff --git a/test/pleroma/workers/attachments_cleanup_worker_test.exs b/test/pleroma/workers/attachments_cleanup_worker_test.exs index 2212db927..d180763fb 100644 --- a/test/pleroma/workers/attachments_cleanup_worker_test.exs +++ b/test/pleroma/workers/attachments_cleanup_worker_test.exs @@ -8,7 +8,9 @@ defmodule Pleroma.Workers.AttachmentsCleanupWorkerTest do import Pleroma.Factory + alias Pleroma.Object alias Pleroma.Workers.AttachmentsCleanupWorker + alias Pleroma.Tests.ObanHelpers setup do clear_config([:instance, :cleanup_attachments], true) @@ -57,4 +59,28 @@ test "enqueues local post", %{attachment: attachment, user: user} do assert {:ok, %Oban.Job{}} = AttachmentsCleanupWorker.enqueue_if_needed(local_data) end + + test "doesn't delete immediately", %{attachment: attachment, user: user} do + delay = 6000 + clear_config([:instance, :cleanup_attachments_delay], delay) + + note = insert(:note, %{user: user, data: %{"attachment" => [attachment.data]}}) + + uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads]) + %{"url" => [%{"href" => href}]} = attachment.data + path = "#{uploads_dir}/#{Path.basename(href)}" + + assert File.exists?(path) + + Object.delete(note) + Process.sleep(2000) + + assert File.exists?(path) + + ObanHelpers.perform(all_enqueued(worker: Pleroma.Workers.AttachmentsCleanupWorker)) + + assert Object.get_by_id(note.id).data["deleted"] + assert Object.get_by_id(attachment.id) == nil + refute File.exists?(path) + end end