From 8057157ee3172c370200f328373f0a7e32092b91 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Fri, 31 Jan 2020 01:20:37 +0300 Subject: [PATCH] Make attachments cleanup optional --- CHANGELOG.md | 2 +- config/config.exs | 3 ++- config/description.exs | 9 +++++++++ lib/pleroma/object.ex | 13 ++++++++----- test/object_test.exs | 44 +++++++++++++++++++++++++++++++++++++++++- 5 files changed, 63 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d0b3cecd..713ae4361 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - **Breaking:** Pleroma won't start if it detects unapplied migrations -- **Breaking:** attachments are removed along with statuses. Does not affect duplicate files and attachments without status. - **Breaking:** Elixir >=1.8 is now required (was >= 1.7) - **Breaking:** `Pleroma.Plugs.RemoteIp` and `:rate_limiter` enabled by default. Please ensure your reverse proxy forwards the real IP! - **Breaking:** attachment links (`config :pleroma, :instance, no_attachment_links` and `config :pleroma, Pleroma.Upload, link_name`) disabled by default @@ -54,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - `:chat_limit` option to limit chat characters. +- `cleanup_attachments` option to 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. - Refreshing poll results for remote polls - Authentication: Added rate limit for password-authorized actions / login existence checks - Static Frontend: Add the ability to render user profiles and notices server-side without requiring JS app. diff --git a/config/config.exs b/config/config.exs index c57ef1bf7..60c982557 100644 --- a/config/config.exs +++ b/config/config.exs @@ -271,7 +271,8 @@ config :pleroma, :instance, account_field_name_length: 512, account_field_value_length: 2048, external_user_synchronization: true, - extended_nickname_format: true + extended_nickname_format: true, + cleanup_attachments: false config :pleroma, :feed, post_title: %{ diff --git a/config/description.exs b/config/description.exs index 5f3c58b08..1ffb66287 100644 --- a/config/description.exs +++ b/config/description.exs @@ -764,6 +764,15 @@ config :pleroma, :config_description, [ "Set to `true` to use extended local nicknames format (allows underscores/dashes)." <> " This will break federation with older software for theses nicknames." }, + %{ + key: :cleanup_attachments, + type: :boolean, + description: """ + "Set to `true` to remove associated attachments when status is removed. + This will not affect duplicates and attachments without status. + Enabling this will increase load to database when deleting statuses on larger instances. + """ + }, %{ key: :max_pinned_statuses, type: :integer, diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 38e372f6d..52556bf31 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -184,11 +184,14 @@ defmodule Pleroma.Object do with {:ok, _obj} = swap_object_with_tombstone(object), deleted_activity = Activity.delete_all_by_object_ap_id(id), {:ok, true} <- Cachex.del(:object_cache, "object:#{id}"), - {:ok, _} <- Cachex.del(:web_resp_cache, URI.parse(id).path), - {:ok, _} <- - Pleroma.Workers.AttachmentsCleanupWorker.enqueue("cleanup_attachments", %{ - "object" => object - }) do + {:ok, _} <- Cachex.del(:web_resp_cache, URI.parse(id).path) do + with true <- Pleroma.Config.get([:instance, :cleanup_attachments]) do + {:ok, _} = + Pleroma.Workers.AttachmentsCleanupWorker.enqueue("cleanup_attachments", %{ + "object" => object + }) + end + {:ok, object, deleted_activity} end end diff --git a/test/object_test.exs b/test/object_test.exs index c6b2bc399..5690bedec 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -76,8 +76,9 @@ defmodule Pleroma.ObjectTest do describe "delete attachments" do clear_config([Pleroma.Upload]) - test "in subdirectories" do + test "Disabled via config" do Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + Pleroma.Config.put([:instance, :cleanup_attachments], false) file = %Plug.Upload{ content_type: "image/jpg", @@ -103,6 +104,41 @@ defmodule Pleroma.ObjectTest do ObanHelpers.perform(all_enqueued(worker: Pleroma.Workers.AttachmentsCleanupWorker)) + assert Object.get_by_id(note.id).data["deleted"] + refute Object.get_by_id(attachment.id) == nil + + assert {:ok, ["an_image.jpg"]} == File.ls("#{uploads_dir}/#{path}") + end + + test "in subdirectories" do + Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + Pleroma.Config.put([:instance, :cleanup_attachments], true) + + file = %Plug.Upload{ + content_type: "image/jpg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "an_image.jpg" + } + + user = insert(:user) + + {:ok, %Object{} = attachment} = + Pleroma.Web.ActivityPub.ActivityPub.upload(file, actor: user.ap_id) + + %{data: %{"attachment" => [%{"url" => [%{"href" => href}]}]}} = + note = insert(:note, %{user: user, data: %{"attachment" => [attachment.data]}}) + + uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads]) + + path = href |> Path.dirname() |> Path.basename() + + assert {:ok, ["an_image.jpg"]} == File.ls("#{uploads_dir}/#{path}") + + Object.delete(note) + + 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 assert {:ok, []} == File.ls("#{uploads_dir}/#{path}") @@ -111,6 +147,7 @@ defmodule Pleroma.ObjectTest do test "with dedupe enabled" do Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) Pleroma.Config.put([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe]) + Pleroma.Config.put([:instance, :cleanup_attachments], true) uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads]) @@ -139,6 +176,7 @@ defmodule Pleroma.ObjectTest do 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 assert {:ok, files} = File.ls(uploads_dir) refute filename in files @@ -146,6 +184,7 @@ defmodule Pleroma.ObjectTest do test "with objects that have legacy data.url attribute" do Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) + Pleroma.Config.put([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpg", @@ -173,6 +212,7 @@ defmodule Pleroma.ObjectTest do 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 assert {:ok, []} == File.ls("#{uploads_dir}/#{path}") @@ -181,6 +221,7 @@ defmodule Pleroma.ObjectTest do test "With custom base_url" do Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local) Pleroma.Config.put([Pleroma.Upload, :base_url], "https://sub.domain.tld/dir/") + Pleroma.Config.put([:instance, :cleanup_attachments], true) file = %Plug.Upload{ content_type: "image/jpg", @@ -206,6 +247,7 @@ defmodule Pleroma.ObjectTest do 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 assert {:ok, []} == File.ls("#{uploads_dir}/#{path}")