From 219fffa0c39161c8566d382b46a112a2de92879c Mon Sep 17 00:00:00 2001 From: Oneric Date: Sun, 2 Jun 2024 21:54:06 +0200 Subject: [PATCH] Allow deleting uploaded files after media migration Until now only the current base_url was checked. After a media domain migration all pre-existing files thus turned undeletable. Fix this with a new config option allowing to list all old media base urls. (This may also come in handy for a later db refactor, see https://akkoma.dev/AkkomaGang/akkoma/issues/765) --- CHANGELOG.md | 2 ++ config/config.exs | 1 + docs/docs/configuration/cheatsheet.md | 1 + .../workers/attachments_cleanup_worker.ex | 35 +++++++++++++++---- .../attachments_cleanup_worker_test.exs | 21 +++++++++++ 5 files changed, 54 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a3e50d4d..35d787a02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,10 +15,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - 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` +- New config option `Pleroma.Upload, :all_base_urls` ## Fixed - Meilisearch: order of results returned from our REST API now actually matches how Meilisearch ranks results - Fix “Delete & Redraft” often losing attachments if attachment cleanup was enabled +- If `Pleroma.Upload, :all_base_urls` is set accordingly, uploaded files can now be deleted after a domain migration. ## 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 39b53a010..22db62f07 100644 --- a/config/config.exs +++ b/config/config.exs @@ -65,6 +65,7 @@ link_name: false, filename_display_max_length: 30, base_url: nil, + all_base_urls: nil, allowed_mime_types: ["image", "audio", "video"] config :pleroma, Pleroma.Uploaders.Local, uploads: "uploads" diff --git a/docs/docs/configuration/cheatsheet.md b/docs/docs/configuration/cheatsheet.md index bbd353d70..dd38535ae 100644 --- a/docs/docs/configuration/cheatsheet.md +++ b/docs/docs/configuration/cheatsheet.md @@ -606,6 +606,7 @@ the source code is here: [kocaptcha](https://github.com/koto-bank/kocaptcha). Th * `link_name`: When enabled Akkoma will add a `name` parameter to the url of the upload, for example `https://instance.tld/media/corndog.png?name=corndog.png`. This is needed to provide the correct filename in Content-Disposition headers * `base_url`: The base URL to access a user-uploaded file; MUST be configured explicitly. Using a (sub)domain distinct from the instance endpoint is **strongly** recommended. A good value might be `https://media.myakkoma.instance/media/`. +* `all_base_url`: list of all base urls ever used *(**both** current and past)*; if unset defaults to a single-entry list containig the current `base_url` * `proxy_opts`: Proxy options, see `Pleroma.ReverseProxy` documentation. * `filename_display_max_length`: Set max length of a filename to display. 0 = no limit. Default: 30. diff --git a/lib/pleroma/workers/attachments_cleanup_worker.ex b/lib/pleroma/workers/attachments_cleanup_worker.ex index f1204a861..558c59755 100644 --- a/lib/pleroma/workers/attachments_cleanup_worker.ex +++ b/lib/pleroma/workers/attachments_cleanup_worker.ex @@ -22,6 +22,7 @@ def enqueue_if_needed(%{ }) do with true <- Config.get([:instance, :cleanup_attachments]), true <- URI.parse(actor).host == Pleroma.Web.Endpoint.host(), + attachments <- Enum.filter(attachments, &deletable_attachment/1), [_ | _] <- attachments do enqueue( "cleanup_attachments", @@ -35,6 +36,18 @@ def enqueue_if_needed(%{ def enqueue_if_needed(_), do: {:ok, :skip} + defp base_urls() do + Config.get([Pleroma.Upload, :all_base_urls]) || + [Config.get!([Pleroma.Upload, :base_url])] + end + + defp deletable_attachment(%{"id" => _id, "url" => [%{"href" => href} | _]}) do + # We can't delete files later if we can't strip the prefix + Enum.any?(base_urls(), fn url -> String.starts_with?(href, url) end) + end + + defp deletable_attachment(_), do: false + @impl Oban.Worker def perform(%Job{ args: %{ @@ -66,18 +79,28 @@ def perform(%Job{ def perform(%Job{args: %{"op" => "cleanup_attachments", "object" => _object}}), do: {:ok, :skip} + defp trim_first_leading(string, []), do: string + + defp trim_first_leading(string, [prefix | rest]) do + trimmed = String.trim_leading(string, prefix) + + if trimmed != string do + trimmed + else + trim_first_leading(string, rest) + end + end + defp do_clean({object_ids, attachment_urls}) do uploader = Pleroma.Config.get([Pleroma.Upload, :uploader]) - base_url = - String.trim_trailing( - Pleroma.Upload.base_url(), - "/" - ) + base_urls = + base_urls() + |> Enum.map(fn url -> String.trim_trailing(url, "/") end) Enum.each(attachment_urls, fn href -> href - |> String.trim_leading("#{base_url}") + |> trim_first_leading(base_urls) |> uploader.delete_file() end) diff --git a/test/pleroma/workers/attachments_cleanup_worker_test.exs b/test/pleroma/workers/attachments_cleanup_worker_test.exs index d180763fb..711492bb9 100644 --- a/test/pleroma/workers/attachments_cleanup_worker_test.exs +++ b/test/pleroma/workers/attachments_cleanup_worker_test.exs @@ -14,6 +14,7 @@ defmodule Pleroma.Workers.AttachmentsCleanupWorkerTest do setup do clear_config([:instance, :cleanup_attachments], true) + clear_config([Pleroma.Upload, :all_base_urls]) file = %Plug.Upload{ content_type: "image/jpeg", @@ -83,4 +84,24 @@ test "doesn't delete immediately", %{attachment: attachment, user: user} do assert Object.get_by_id(attachment.id) == nil refute File.exists?(path) end + + test "skips localpost with unmappable URLs", %{attachment: attachment, user: user} do + local_url = Pleroma.Web.Endpoint.url() + + attach_data = + attachment.data + |> Map.update!("url", fn + [%{"href" => _} = url | _] -> + [%{url | "href" => "https://oldmedia.example/files/123.png"}] + end) + + local_data = %{ + "id" => local_url <> "/obj/123", + "actor" => user.ap_id, + "content" => "content", + "attachment" => [attach_data] + } + + assert {:ok, :skip} = AttachmentsCleanupWorker.enqueue_if_needed(local_data) + end end