From 62b5da21e7eb7cfc82b635dfcac39258aacedb9c Mon Sep 17 00:00:00 2001 From: rinpatch Date: Fri, 31 Jan 2020 00:09:13 +0300 Subject: Revert "Merge branch 'feature/attachments-cleanup' into 'develop'" This reverts commit a431e8c9f7033c739e10b0e8b34c75f2cc1d38d4, reversing changes made to 8b4d81609d5627d62b826bcd3e87290cb513495f. --- CHANGELOG.md | 1 - lib/pleroma/object.ex | 14 ---- lib/pleroma/uploaders/local.ex | 13 ---- lib/pleroma/uploaders/s3.ex | 14 ---- lib/pleroma/uploaders/uploader.ex | 3 +- test/object_test.exs | 139 -------------------------------------- test/uploaders/local_test.exs | 21 ------ test/uploaders/s3_test.exs | 7 -- 8 files changed, 1 insertion(+), 211 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68ebb03a7..2b258da41 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:** attachment links (`config :pleroma, :instance, no_attachment_links` and `config :pleroma, Pleroma.Upload, link_name`) disabled by default - **Breaking:** OAuth: defaulted `[:auth, :enforce_oauth_admin_scope_usage]` setting to `true` which demands `admin` OAuth scope to perform admin actions (in addition to `is_admin` flag on User); make sure to use bundled or newer versions of AdminFE & PleromaFE to access admin / moderator features. diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 38e372f6d..9a91fba9a 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -83,20 +83,6 @@ def get_by_ap_id(ap_id) do Repo.one(from(object in Object, where: fragment("(?)->>'id' = ?", object.data, ^ap_id))) 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 "Object.normalize() called without preloaded object (#{inspect(ap_id)}). Consider preloading the object" |> Logger.debug() diff --git a/lib/pleroma/uploaders/local.ex b/lib/pleroma/uploaders/local.ex index 2e6fe3292..36b3c35ec 100644 --- a/lib/pleroma/uploaders/local.ex +++ b/lib/pleroma/uploaders/local.ex @@ -5,12 +5,10 @@ defmodule Pleroma.Uploaders.Local do @behaviour Pleroma.Uploaders.Uploader - @impl true def get_file(_) do {:ok, {:static_dir, upload_path()}} end - @impl true def put_file(upload) do {local_path, file} = case Enum.reverse(Path.split(upload.path)) do @@ -35,15 +33,4 @@ def put_file(upload) do def upload_path do Pleroma.Config.get!([__MODULE__, :uploads]) 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 diff --git a/lib/pleroma/uploaders/s3.ex b/lib/pleroma/uploaders/s3.ex index feb89cea6..9876b6398 100644 --- a/lib/pleroma/uploaders/s3.ex +++ b/lib/pleroma/uploaders/s3.ex @@ -10,7 +10,6 @@ defmodule Pleroma.Uploaders.S3 do # The file name is re-encoded with S3's constraints here to comply with previous # links with less strict filenames - @impl true def get_file(file) do config = Config.get([__MODULE__]) bucket = Keyword.fetch!(config, :bucket) @@ -36,7 +35,6 @@ def get_file(file) do ])}} end - @impl true def put_file(%Pleroma.Upload{} = upload) do config = Config.get([__MODULE__]) bucket = Keyword.get(config, :bucket) @@ -71,18 +69,6 @@ def put_file(%Pleroma.Upload{} = upload) do 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!.*/'()_-]") def strict_encode(name) do String.replace(name, @regex, "-") diff --git a/lib/pleroma/uploaders/uploader.ex b/lib/pleroma/uploaders/uploader.ex index d71e213d2..c0b22c28a 100644 --- a/lib/pleroma/uploaders/uploader.ex +++ b/lib/pleroma/uploaders/uploader.ex @@ -36,8 +36,6 @@ defmodule Pleroma.Uploaders.Uploader do @callback put_file(Pleroma.Upload.t()) :: :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()) :: {:ok, Plug.Conn.t()} | {:ok, Plug.Conn.t(), file_spec()} @@ -45,6 +43,7 @@ defmodule Pleroma.Uploaders.Uploader do @optional_callbacks http_callback: 2 @spec put_file(module(), Pleroma.Upload.t()) :: {:ok, file_spec()} | {:error, String.t()} + def put_file(uploader, upload) do case uploader.put_file(upload) do :ok -> {:ok, {:file, upload.path}} diff --git a/test/object_test.exs b/test/object_test.exs index c6b2bc399..6252975f2 100644 --- a/test/object_test.exs +++ b/test/object_test.exs @@ -73,145 +73,6 @@ test "ensures cache is cleared for the object" do 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) - - ObanHelpers.perform(all_enqueued(worker: Pleroma.Workers.AttachmentsCleanupWorker)) - - 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) - - ObanHelpers.perform(all_enqueued(worker: Pleroma.Workers.AttachmentsCleanupWorker)) - - assert Object.get_by_id(attachment.id) == nil - assert {:ok, files} = File.ls(uploads_dir) - refute filename in files - end - - test "with objects that have legacy data.url attribute" 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) - - {:ok, %Object{}} = Object.create(%{url: "https://google.com", 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(attachment.id) == nil - - assert {:ok, []} == File.ls("#{uploads_dir}/#{path}") - end - - 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/") - - 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(attachment.id) == nil - - assert {:ok, []} == File.ls("#{uploads_dir}/#{path}") - end - end - describe "normalizer" do test "fetches unknown objects by default" do %Object{} = diff --git a/test/uploaders/local_test.exs b/test/uploaders/local_test.exs index 1963dac23..fc442d0f1 100644 --- a/test/uploaders/local_test.exs +++ b/test/uploaders/local_test.exs @@ -29,25 +29,4 @@ test "put file to local folder" do |> File.exists?() 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 diff --git a/test/uploaders/s3_test.exs b/test/uploaders/s3_test.exs index ab7795c3b..171316340 100644 --- a/test/uploaders/s3_test.exs +++ b/test/uploaders/s3_test.exs @@ -79,11 +79,4 @@ test "returns error", %{file_upload: file_upload} do 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 -- cgit v1.2.3