From 053276ef6c6780ed65b6683cb45d4d361985157b Mon Sep 17 00:00:00 2001 From: ilja Date: Sun, 12 Mar 2023 10:07:47 +0100 Subject: [PATCH] Refactor upload, get url from modules To get the url and base url, we had a function in upload.ex. This included a `case` to check on what module is used. This means that adding a new upload module implies changing code besides just adding a new module. Here we try to get the logic properly into the respective modules. This doesn't add new functionality, but should make it easier to add new uploaders in the future. --- lib/pleroma/upload.ex | 47 ++++++------------------------- lib/pleroma/uploaders/local.ex | 8 +++++- lib/pleroma/uploaders/s3.ex | 27 ++++++++++++++++++ lib/pleroma/uploaders/uploader.ex | 2 ++ test/pleroma/upload_test.exs | 2 ++ 5 files changed, 46 insertions(+), 40 deletions(-) diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index 3b5419db7..17db44ae9 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -86,7 +86,7 @@ defmodule Pleroma.Upload do {_, true} <- {:description_limit, String.length(description) <= Pleroma.Config.get([:instance, :description_limit])}, - {:ok, url_spec} <- Pleroma.Uploaders.Uploader.put_file(opts.uploader, upload) do + {:ok, file} <- Pleroma.Uploaders.Uploader.put_file(opts.uploader, upload) do {:ok, %{ "id" => Utils.generate_object_id(), @@ -96,7 +96,7 @@ defmodule Pleroma.Upload do %{ "type" => "Link", "mediaType" => upload.content_type, - "href" => url_from_spec(upload, opts.base_url, url_spec) + "href" => get_url(upload, file) } |> Maps.put_if_present("width", upload.width) |> Maps.put_if_present("height", upload.height) @@ -142,8 +142,7 @@ defmodule Pleroma.Upload do size_limit: Keyword.get(opts, :size_limit, size_limit), uploader: Keyword.get(opts, :uploader, Pleroma.Config.get([__MODULE__, :uploader])), filters: Keyword.get(opts, :filters, Pleroma.Config.get([__MODULE__, :filters])), - description: Keyword.get(opts, :description), - base_url: base_url() + description: Keyword.get(opts, :description) } end @@ -215,7 +214,9 @@ defmodule Pleroma.Upload do tmp_path end - defp url_from_spec(%__MODULE__{name: name}, base_url, {:file, path}) do + defp get_url(%__MODULE__{name: name}, {:file, path}) do + base_url = base_url() + path = URI.encode(path, &char_unescaped?/1) <> if Pleroma.Config.get([__MODULE__, :link_name], false) do @@ -228,42 +229,10 @@ defmodule Pleroma.Upload do |> Path.join() end - defp url_from_spec(_upload, _base_url, {:url, url}), do: url + defp get_url(_upload, {:url, url}), do: url def base_url do uploader = Config.get([Pleroma.Upload, :uploader]) - upload_base_url = Config.get([Pleroma.Upload, :base_url]) - public_endpoint = Config.get([uploader, :public_endpoint]) - - case uploader do - Pleroma.Uploaders.Local -> - upload_base_url || Pleroma.Web.Endpoint.url() <> "/media/" - - Pleroma.Uploaders.S3 -> - bucket = Config.get([Pleroma.Uploaders.S3, :bucket]) - truncated_namespace = Config.get([Pleroma.Uploaders.S3, :truncated_namespace]) - namespace = Config.get([Pleroma.Uploaders.S3, :bucket_namespace]) - - bucket_with_namespace = - cond do - !is_nil(truncated_namespace) -> - truncated_namespace - - !is_nil(namespace) -> - namespace <> ":" <> bucket - - true -> - bucket - end - - if public_endpoint do - Path.join([public_endpoint, bucket_with_namespace]) - else - Path.join([upload_base_url, bucket_with_namespace]) - end - - _ -> - public_endpoint || upload_base_url || Pleroma.Web.Endpoint.url() <> "/media/" - end + uploader.base_url() end end diff --git a/lib/pleroma/uploaders/local.ex b/lib/pleroma/uploaders/local.ex index 0e1ba4b90..c4e0d912d 100644 --- a/lib/pleroma/uploaders/local.ex +++ b/lib/pleroma/uploaders/local.ex @@ -4,6 +4,7 @@ defmodule Pleroma.Uploaders.Local do @behaviour Pleroma.Uploaders.Uploader + alias Pleroma.Config @impl true def get_file(_) do @@ -33,7 +34,7 @@ defmodule Pleroma.Uploaders.Local do end def upload_path do - Pleroma.Config.get!([__MODULE__, :uploads]) + Config.get!([__MODULE__, :uploads]) end @impl true @@ -46,4 +47,9 @@ defmodule Pleroma.Uploaders.Local do {:error, posix_error} -> {:error, to_string(posix_error)} end end + + @impl true + def base_url() do + Config.get([Pleroma.Upload, :base_url]) || Pleroma.Web.Endpoint.url() <> "/media/" + end end diff --git a/lib/pleroma/uploaders/s3.ex b/lib/pleroma/uploaders/s3.ex index 481153fe8..de5a7892f 100644 --- a/lib/pleroma/uploaders/s3.ex +++ b/lib/pleroma/uploaders/s3.ex @@ -67,6 +67,33 @@ defmodule Pleroma.Uploaders.S3 do end end + @impl true + def base_url() do + upload_base_url = Config.get([Pleroma.Upload, :base_url]) + public_endpoint = Config.get([Pleroma.Uploaders.S3, :public_endpoint]) + bucket = Config.get([Pleroma.Uploaders.S3, :bucket]) + truncated_namespace = Config.get([Pleroma.Uploaders.S3, :truncated_namespace]) + namespace = Config.get([Pleroma.Uploaders.S3, :bucket_namespace]) + + bucket_with_namespace = + cond do + !is_nil(truncated_namespace) -> + truncated_namespace + + !is_nil(namespace) -> + namespace <> ":" <> bucket + + true -> + bucket + end + + if public_endpoint do + Path.join([public_endpoint, bucket_with_namespace]) + else + Path.join([upload_base_url, bucket_with_namespace]) + 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 deba548b7..96545a4a4 100644 --- a/lib/pleroma/uploaders/uploader.ex +++ b/lib/pleroma/uploaders/uploader.ex @@ -40,6 +40,8 @@ defmodule Pleroma.Uploaders.Uploader do @callback delete_file(file :: String.t()) :: :ok | {:error, String.t()} + @callback base_url() :: String.t() + @callback http_callback(Plug.Conn.t(), Map.t()) :: {:ok, Plug.Conn.t()} | {:ok, Plug.Conn.t(), file_spec()} diff --git a/test/pleroma/upload_test.exs b/test/pleroma/upload_test.exs index 8f242630f..165e77a57 100644 --- a/test/pleroma/upload_test.exs +++ b/test/pleroma/upload_test.exs @@ -41,6 +41,8 @@ defmodule Pleroma.UploadTest do do: {:ok, conn, {:file, "post-process-file.jpg"}} def put_file(upload), do: TestUploaderBase.put_file(upload, __MODULE__) + + def base_url(), do: "http://localhost:4001/media/" end setup do: [uploader: TestUploaderSuccess]