forked from AkkomaGang/akkoma
Don't try to cleanup remote attachments
The cleanup attachment worker was run for every deleted post, even if it’s a remote post whose attachments we don't even store. This was especially bad due to attachment cleanup involving a particularly heavy query wasting a bunch of database perf for nil. This was uncovered by comparing statistics from AkkomaGang/akkoma#784 and AkkomaGang/akkoma#765 (comment)
This commit is contained in:
parent
c0a99df06a
commit
615ac14d6d
3 changed files with 101 additions and 23 deletions
|
@ -9,7 +9,6 @@ defmodule Pleroma.Object do
|
|||
import Ecto.Changeset
|
||||
|
||||
alias Pleroma.Activity
|
||||
alias Pleroma.Config
|
||||
alias Pleroma.Hashtag
|
||||
alias Pleroma.Object
|
||||
alias Pleroma.Object.Fetcher
|
||||
|
@ -241,23 +240,11 @@ def delete(%Object{data: %{"id" => id}} = object) do
|
|||
with {:ok, _obj} = swap_object_with_tombstone(object),
|
||||
deleted_activity = Activity.delete_all_by_object_ap_id(id),
|
||||
{:ok, _} <- invalid_object_cache(object) do
|
||||
cleanup_attachments(
|
||||
Config.get([:instance, :cleanup_attachments]),
|
||||
%{object: object}
|
||||
)
|
||||
|
||||
AttachmentsCleanupWorker.enqueue_if_needed(object.data)
|
||||
{:ok, object, deleted_activity}
|
||||
end
|
||||
end
|
||||
|
||||
@spec cleanup_attachments(boolean(), %{required(:object) => map()}) ::
|
||||
{:ok, Oban.Job.t() | nil}
|
||||
def cleanup_attachments(true, %{object: _} = params) do
|
||||
AttachmentsCleanupWorker.enqueue("cleanup_attachments", params)
|
||||
end
|
||||
|
||||
def cleanup_attachments(_, _), do: {:ok, nil}
|
||||
|
||||
def prune(%Object{data: %{"id" => _id}} = object) do
|
||||
with {:ok, object} <- Repo.delete(object),
|
||||
{:ok, _} <- invalid_object_cache(object) do
|
||||
|
|
|
@ -5,30 +5,61 @@
|
|||
defmodule Pleroma.Workers.AttachmentsCleanupWorker do
|
||||
import Ecto.Query
|
||||
|
||||
alias Pleroma.Config
|
||||
alias Pleroma.Object
|
||||
alias Pleroma.Repo
|
||||
|
||||
use Pleroma.Workers.WorkerHelper, queue: "attachments_cleanup"
|
||||
|
||||
@doc """
|
||||
Takes object data and if necessary enqueues a job,
|
||||
deleting all attachments of the post eligible for cleanup
|
||||
"""
|
||||
@spec enqueue_if_needed(map()) :: {:ok, Oban.Job.t()} | {:ok, :skip} | {:error, any()}
|
||||
def enqueue_if_needed(%{
|
||||
"actor" => actor,
|
||||
"attachment" => [_ | _] = attachments
|
||||
}) do
|
||||
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})
|
||||
else
|
||||
_ -> {:ok, :skip}
|
||||
end
|
||||
end
|
||||
|
||||
def enqueue_if_needed(_), do: {:ok, :skip}
|
||||
|
||||
@impl Oban.Worker
|
||||
def perform(%Job{
|
||||
args: %{
|
||||
"op" => "cleanup_attachments",
|
||||
"object" => %{"data" => %{"attachment" => [_ | _] = attachments, "actor" => actor}}
|
||||
"attachments" => [_ | _] = attachments,
|
||||
"actor" => actor
|
||||
}
|
||||
}) do
|
||||
if Pleroma.Config.get([:instance, :cleanup_attachments], false) do
|
||||
attachments
|
||||
|> Enum.flat_map(fn item -> Enum.map(item["url"], & &1["href"]) end)
|
||||
|> fetch_objects
|
||||
|> prepare_objects(actor, Enum.map(attachments, & &1["name"]))
|
||||
|> filter_objects
|
||||
|> do_clean
|
||||
end
|
||||
|
||||
{:ok, :success}
|
||||
end
|
||||
|
||||
# Left over already enqueued jobs in the old format
|
||||
# This function clause can be deleted once sufficient time passed after 3.14
|
||||
def perform(%Job{
|
||||
args: %{
|
||||
"op" => "cleanup_attachments",
|
||||
"object" => %{"data" => data}
|
||||
}
|
||||
}) do
|
||||
enqueue_if_needed(data)
|
||||
end
|
||||
|
||||
def perform(%Job{args: %{"op" => "cleanup_attachments", "object" => _object}}), do: {:ok, :skip}
|
||||
|
||||
defp do_clean({object_ids, attachment_urls}) do
|
||||
|
|
60
test/pleroma/workers/attachments_cleanup_worker_test.exs
Normal file
60
test/pleroma/workers/attachments_cleanup_worker_test.exs
Normal file
|
@ -0,0 +1,60 @@
|
|||
# Akkoma: Magically expressive social media
|
||||
# Copyright © 2024 Akkoma Authors <https://akkoma.dev/>
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
|
||||
defmodule Pleroma.Workers.AttachmentsCleanupWorkerTest do
|
||||
use Pleroma.DataCase, async: false
|
||||
use Oban.Testing, repo: Pleroma.Repo
|
||||
|
||||
import Pleroma.Factory
|
||||
|
||||
alias Pleroma.Workers.AttachmentsCleanupWorker
|
||||
|
||||
setup do
|
||||
clear_config([:instance, :cleanup_attachments], true)
|
||||
|
||||
file = %Plug.Upload{
|
||||
content_type: "image/jpeg",
|
||||
path: Path.absname("test/fixtures/image.jpg"),
|
||||
filename: "an_image.jpg"
|
||||
}
|
||||
|
||||
user = insert(:user)
|
||||
|
||||
{:ok, %Pleroma.Object{} = attachment} =
|
||||
Pleroma.Web.ActivityPub.ActivityPub.upload(file, actor: user.ap_id)
|
||||
|
||||
{:ok, attachment: attachment, user: user}
|
||||
end
|
||||
|
||||
test "does not enqueue remote post" do
|
||||
remote_data = %{
|
||||
"id" => "https://remote.example/obj/123",
|
||||
"actor" => "https://remote.example/user/1",
|
||||
"content" => "content",
|
||||
"attachment" => [
|
||||
%{
|
||||
"type" => "Document",
|
||||
"mediaType" => "image/png",
|
||||
"name" => "marvellous image",
|
||||
"url" => "https://remote.example/files/image.png"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
assert {:ok, :skip} = AttachmentsCleanupWorker.enqueue_if_needed(remote_data)
|
||||
end
|
||||
|
||||
test "enqueues local post", %{attachment: attachment, user: user} do
|
||||
local_url = Pleroma.Web.Endpoint.url()
|
||||
|
||||
local_data = %{
|
||||
"id" => local_url <> "/obj/123",
|
||||
"actor" => user.ap_id,
|
||||
"content" => "content",
|
||||
"attachment" => [attachment.data]
|
||||
}
|
||||
|
||||
assert {:ok, %Oban.Job{}} = AttachmentsCleanupWorker.enqueue_if_needed(local_data)
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue