Merge branch 'feature/attachments-cleanup' into 'develop'

Delete attachments when status is deleted

See merge request pleroma/pleroma!2036
This commit is contained in:
feld 2020-01-12 18:48:59 +00:00
commit a431e8c9f7
8 changed files with 214 additions and 1 deletions

View File

@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- **Breaking**: MDII uploader - **Breaking**: MDII uploader
### Changed ### Changed
- **Breaking:** attachments are removed along with statuses when there are no other references to it
- **Breaking:** Elixir >=1.8 is now required (was >= 1.7) - **Breaking:** Elixir >=1.8 is now required (was >= 1.7)
- **Breaking:** attachment links (`config :pleroma, :instance, no_attachment_links` and `config :pleroma, Pleroma.Upload, link_name`) disabled by default - **Breaking:** attachment links (`config :pleroma, :instance, no_attachment_links` and `config :pleroma, Pleroma.Upload, link_name`) disabled by default
- Replaced [pleroma_job_queue](https://git.pleroma.social/pleroma/pleroma_job_queue) and `Pleroma.Web.Federator.RetryQueue` with [Oban](https://github.com/sorentwo/oban) (see [`docs/config.md`](docs/config.md) on migrating customized worker / retry settings) - Replaced [pleroma_job_queue](https://git.pleroma.social/pleroma/pleroma_job_queue) and `Pleroma.Web.Federator.RetryQueue` with [Oban](https://github.com/sorentwo/oban) (see [`docs/config.md`](docs/config.md) on migrating customized worker / retry settings)

View File

@ -17,6 +17,8 @@ defmodule Pleroma.Object do
require Logger require Logger
@type t() :: %__MODULE__{}
schema "objects" do schema "objects" do
field(:data, :map) field(:data, :map)
@ -79,6 +81,20 @@ def get_by_ap_id(ap_id) do
Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id))) Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id)))
end end
@doc """
Get a single attachment by it's name and href
"""
@spec get_attachment_by_name_and_href(String.t(), String.t()) :: Object.t() | nil
def get_attachment_by_name_and_href(name, href) do
query =
from(o in Object,
where: fragment("(?)->>'name' = ?", o.data, ^name),
where: fragment("(?)->>'href' = ?", o.data, ^href)
)
Repo.one(query)
end
defp warn_on_no_object_preloaded(ap_id) do defp warn_on_no_object_preloaded(ap_id) do
"Object.normalize() called without preloaded object (#{inspect(ap_id)}). Consider preloading the object" "Object.normalize() called without preloaded object (#{inspect(ap_id)}). Consider preloading the object"
|> Logger.debug() |> Logger.debug()
@ -164,6 +180,7 @@ def swap_object_with_tombstone(object) do
def delete(%Object{data: %{"id" => id}} = object) do def delete(%Object{data: %{"id" => id}} = object) do
with {:ok, _obj} = swap_object_with_tombstone(object), with {:ok, _obj} = swap_object_with_tombstone(object),
:ok <- delete_attachments(object),
deleted_activity = Activity.delete_all_by_object_ap_id(id), deleted_activity = Activity.delete_all_by_object_ap_id(id),
{:ok, true} <- Cachex.del(:object_cache, "object:#{id}"), {:ok, true} <- Cachex.del(:object_cache, "object:#{id}"),
{:ok, _} <- Cachex.del(:web_resp_cache, URI.parse(id).path) do {:ok, _} <- Cachex.del(:web_resp_cache, URI.parse(id).path) do
@ -171,6 +188,77 @@ def delete(%Object{data: %{"id" => id}} = object) do
end end
end end
defp delete_attachments(%{data: %{"attachment" => [_ | _] = attachments, "actor" => actor}}) do
hrefs =
Enum.flat_map(attachments, fn attachment ->
Enum.map(attachment["url"], & &1["href"])
end)
names = Enum.map(attachments, & &1["name"])
uploader = Pleroma.Config.get([Pleroma.Upload, :uploader])
# find all objects for copies of the attachments, name and actor doesn't matter here
delete_ids =
from(o in Object,
where:
fragment(
"to_jsonb(array(select jsonb_array_elements((?)#>'{url}') ->> 'href'))::jsonb \\?| (?)",
o.data,
^hrefs
)
)
|> Repo.all()
# we should delete 1 object for any given attachment, but don't delete files if
# there are more than 1 object for it
|> Enum.reduce(%{}, fn %{
id: id,
data: %{
"url" => [%{"href" => href}],
"actor" => obj_actor,
"name" => name
}
},
acc ->
Map.update(acc, href, %{id: id, count: 1}, fn val ->
case obj_actor == actor and name in names do
true ->
# set id of the actor's object that will be deleted
%{val | id: id, count: val.count + 1}
false ->
# another actor's object, just increase count to not delete file
%{val | count: val.count + 1}
end
end)
end)
|> Enum.map(fn {href, %{id: id, count: count}} ->
# only delete files that have single instance
with 1 <- count do
prefix =
case Pleroma.Config.get([Pleroma.Upload, :base_url]) do
nil -> "media"
_ -> ""
end
base_url = Pleroma.Config.get([__MODULE__, :base_url], Pleroma.Web.base_url())
file_path = String.trim_leading(href, "#{base_url}/#{prefix}")
uploader.delete_file(file_path)
end
id
end)
from(o in Object, where: o.id in ^delete_ids)
|> Repo.delete_all()
:ok
end
defp delete_attachments(%{data: _data}), do: :ok
def prune(%Object{data: %{"id" => id}} = object) do def prune(%Object{data: %{"id" => id}} = object) do
with {:ok, object} <- Repo.delete(object), with {:ok, object} <- Repo.delete(object),
{:ok, true} <- Cachex.del(:object_cache, "object:#{id}"), {:ok, true} <- Cachex.del(:object_cache, "object:#{id}"),

View File

@ -5,10 +5,12 @@
defmodule Pleroma.Uploaders.Local do defmodule Pleroma.Uploaders.Local do
@behaviour Pleroma.Uploaders.Uploader @behaviour Pleroma.Uploaders.Uploader
@impl true
def get_file(_) do def get_file(_) do
{:ok, {:static_dir, upload_path()}} {:ok, {:static_dir, upload_path()}}
end end
@impl true
def put_file(upload) do def put_file(upload) do
{local_path, file} = {local_path, file} =
case Enum.reverse(Path.split(upload.path)) do case Enum.reverse(Path.split(upload.path)) do
@ -33,4 +35,15 @@ def put_file(upload) do
def upload_path do def upload_path do
Pleroma.Config.get!([__MODULE__, :uploads]) Pleroma.Config.get!([__MODULE__, :uploads])
end end
@impl true
def delete_file(path) do
upload_path()
|> Path.join(path)
|> File.rm()
|> case do
:ok -> :ok
{:error, posix_error} -> {:error, to_string(posix_error)}
end
end
end end

View File

@ -10,6 +10,7 @@ defmodule Pleroma.Uploaders.S3 do
# The file name is re-encoded with S3's constraints here to comply with previous # The file name is re-encoded with S3's constraints here to comply with previous
# links with less strict filenames # links with less strict filenames
@impl true
def get_file(file) do def get_file(file) do
config = Config.get([__MODULE__]) config = Config.get([__MODULE__])
bucket = Keyword.fetch!(config, :bucket) bucket = Keyword.fetch!(config, :bucket)
@ -35,6 +36,7 @@ def get_file(file) do
])}} ])}}
end end
@impl true
def put_file(%Pleroma.Upload{} = upload) do def put_file(%Pleroma.Upload{} = upload) do
config = Config.get([__MODULE__]) config = Config.get([__MODULE__])
bucket = Keyword.get(config, :bucket) bucket = Keyword.get(config, :bucket)
@ -69,6 +71,18 @@ def put_file(%Pleroma.Upload{} = upload) do
end end
end end
@impl true
def delete_file(file) do
[__MODULE__, :bucket]
|> Config.get()
|> ExAws.S3.delete_object(file)
|> ExAws.request()
|> case do
{:ok, %{status_code: 204}} -> :ok
error -> {:error, inspect(error)}
end
end
@regex Regex.compile!("[^0-9a-zA-Z!.*/'()_-]") @regex Regex.compile!("[^0-9a-zA-Z!.*/'()_-]")
def strict_encode(name) do def strict_encode(name) do
String.replace(name, @regex, "-") String.replace(name, @regex, "-")

View File

@ -36,6 +36,8 @@ defmodule Pleroma.Uploaders.Uploader do
@callback put_file(Pleroma.Upload.t()) :: @callback put_file(Pleroma.Upload.t()) ::
:ok | {:ok, file_spec()} | {:error, String.t()} | :wait_callback :ok | {:ok, file_spec()} | {:error, String.t()} | :wait_callback
@callback delete_file(file :: String.t()) :: :ok | {:error, String.t()}
@callback http_callback(Plug.Conn.t(), Map.t()) :: @callback http_callback(Plug.Conn.t(), Map.t()) ::
{:ok, Plug.Conn.t()} {:ok, Plug.Conn.t()}
| {:ok, Plug.Conn.t(), file_spec()} | {:ok, Plug.Conn.t(), file_spec()}
@ -43,7 +45,6 @@ defmodule Pleroma.Uploaders.Uploader do
@optional_callbacks http_callback: 2 @optional_callbacks http_callback: 2
@spec put_file(module(), Pleroma.Upload.t()) :: {:ok, file_spec()} | {:error, String.t()} @spec put_file(module(), Pleroma.Upload.t()) :: {:ok, file_spec()} | {:error, String.t()}
def put_file(uploader, upload) do def put_file(uploader, upload) do
case uploader.put_file(upload) do case uploader.put_file(upload) do
:ok -> {:ok, {:file, upload.path}} :ok -> {:ok, {:file, upload.path}}

View File

@ -71,6 +71,74 @@ test "ensures cache is cleared for the object" do
end end
end end
describe "delete attachments" do
clear_config([Pleroma.Upload])
test "in subdirectories" do
Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
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)
assert Object.get_by_id(attachment.id) == nil
assert {:ok, []} == File.ls("#{uploads_dir}/#{path}")
end
test "with dedupe enabled" do
Pleroma.Config.put([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
Pleroma.Config.put([Pleroma.Upload, :filters], [Pleroma.Upload.Filter.Dedupe])
uploads_dir = Pleroma.Config.get!([Pleroma.Uploaders.Local, :uploads])
File.mkdir_p!(uploads_dir)
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]}})
filename = Path.basename(href)
assert {:ok, files} = File.ls(uploads_dir)
assert filename in files
Object.delete(note)
assert Object.get_by_id(attachment.id) == nil
assert {:ok, files} = File.ls(uploads_dir)
refute filename in files
end
end
describe "normalizer" do describe "normalizer" do
test "fetches unknown objects by default" do test "fetches unknown objects by default" do
%Object{} = %Object{} =

View File

@ -29,4 +29,25 @@ test "put file to local folder" do
|> File.exists?() |> File.exists?()
end end
end end
describe "delete_file/1" do
test "deletes local file" do
file_path = "local_upload/files/image.jpg"
file = %Pleroma.Upload{
name: "image.jpg",
content_type: "image/jpg",
path: file_path,
tempfile: Path.absname("test/fixtures/image_tmp.jpg")
}
:ok = Local.put_file(file)
local_path = Path.join([Local.upload_path(), file_path])
assert File.exists?(local_path)
Local.delete_file(file_path)
refute File.exists?(local_path)
end
end
end end

View File

@ -79,4 +79,11 @@ test "returns error", %{file_upload: file_upload} do
end end
end end
end end
describe "delete_file/1" do
test_with_mock "deletes file", ExAws, request: fn _req -> {:ok, %{status_code: 204}} end do
assert :ok = S3.delete_file("image.jpg")
assert_called(ExAws.request(:_))
end
end
end end