From 9d8ab46cd6f7d31f80caaac1d3ab9132b02e553c Mon Sep 17 00:00:00 2001 From: ilja Date: Mon, 27 Mar 2023 09:25:44 +0200 Subject: [PATCH] Move building of url to modules We got a path and there's a "base_url" function. With that the url was build. But not all url's are nessecarely build like this. Building the url should be the responsability of the Uploader itself, so it's now moved to there. --- lib/pleroma/upload.ex | 24 +++++++++--------------- lib/pleroma/uploaders/local.ex | 4 +++- lib/pleroma/uploaders/s3.ex | 3 ++- lib/pleroma/uploaders/uploader.ex | 8 ++++---- test/pleroma/upload_test.exs | 7 ++++++- test/pleroma/uploaders/local_test.exs | 3 ++- test/pleroma/uploaders/s3_test.exs | 3 ++- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/lib/pleroma/upload.ex b/lib/pleroma/upload.ex index e2dc182db..7c9a99029 100644 --- a/lib/pleroma/upload.ex +++ b/lib/pleroma/upload.ex @@ -63,7 +63,7 @@ defmodule Pleroma.Upload do blurhash: String.t(), path: String.t() } - defstruct [:id, :name, :tempfile, :content_type, :width, :height, :blurhash, :path] + defstruct [:id, :name, :tempfile, :content_type, :width, :height, :blurhash, :path, :url] defp get_description(opts, upload) do case {opts[:description], Config.get([Pleroma.Upload, :default_description])} do @@ -97,7 +97,7 @@ defmodule Pleroma.Upload do %{ "type" => "Link", "mediaType" => upload.content_type, - "href" => get_url(upload.name, upload.path) + "href" => url_with_query_params(upload) } |> Maps.put_if_present("width", upload.width) |> Maps.put_if_present("height", upload.height) @@ -212,19 +212,13 @@ defmodule Pleroma.Upload do tmp_path end - defp get_url(name, path) do - base_url = base_url() - - path = - URI.encode(path, &char_unescaped?/1) <> - if Config.get([__MODULE__, :link_name], false) do - "?name=#{URI.encode(name, &char_unescaped?/1)}" - else - "" - end - - [base_url, path] - |> Path.join() + defp url_with_query_params(%__MODULE__{url: url, name: name}) do + url <> + if Config.get([__MODULE__, :link_name], false) do + "?name=#{URI.encode(name, &char_unescaped?/1)}" + else + "" + end end def base_url(), do: Config.get([__MODULE__, :uploader]).base_url() diff --git a/lib/pleroma/uploaders/local.ex b/lib/pleroma/uploaders/local.ex index d0202f076..83d9fbfab 100644 --- a/lib/pleroma/uploaders/local.ex +++ b/lib/pleroma/uploaders/local.ex @@ -30,7 +30,9 @@ defmodule Pleroma.Uploaders.Local do File.cp!(upload.tempfile, result_file) end - {:ok, upload} + url = [base_url(), URI.encode(upload.path, &Pleroma.Upload.char_unescaped?/1)] |> Path.join() + + {:ok, upload |> Map.put(:url, url)} end def upload_path do diff --git a/lib/pleroma/uploaders/s3.ex b/lib/pleroma/uploaders/s3.ex index e714f5719..4bfbf7857 100644 --- a/lib/pleroma/uploaders/s3.ex +++ b/lib/pleroma/uploaders/s3.ex @@ -47,7 +47,8 @@ defmodule Pleroma.Uploaders.S3 do case ExAws.request(op) do {:ok, _} -> - {:ok, Map.put(upload, :path, s3_name)} + {:ok, + upload |> Map.put(:path, s3_name) |> Map.put(:url, Path.join([base_url(), s3_name]))} error -> Logger.error("#{__MODULE__}: #{inspect(error)}") diff --git a/lib/pleroma/uploaders/uploader.ex b/lib/pleroma/uploaders/uploader.ex index 15840a049..e4d597bdc 100644 --- a/lib/pleroma/uploaders/uploader.ex +++ b/lib/pleroma/uploaders/uploader.ex @@ -35,18 +35,18 @@ defmodule Pleroma.Uploaders.Uploader do """ @callback put_file(upload :: struct()) :: - :ok | {:ok, Pleroma.Upload} | {:error, String.t()} | :wait_callback + :ok | {:ok, Pleroma.Upload.t()} | {:error, String.t()} | :wait_callback @callback delete_file(file :: String.t()) :: :ok | {:error, String.t()} @callback base_url() :: String.t() - @callback http_callback(Plug.Conn.t(), Map.t(), Pleroma.Upload) :: - {:ok, Plug.Conn.t(), Pleroma.Upload} + @callback http_callback(Plug.Conn.t(), Map.t(), Pleroma.Upload.t()) :: + {:ok, Plug.Conn.t(), Pleroma.Upload.t()} | {:error, Plug.Conn.t(), String.t()} @optional_callbacks http_callback: 3 - @spec put_file(module(), upload :: struct()) :: {:ok, Pleroma.Upload} | {:error, String.t()} + @spec put_file(module(), upload :: struct()) :: {:ok, Pleroma.Upload.t()} | {:error, String.t()} def put_file(uploader, upload) do case uploader.put_file(upload) do {:ok, %Pleroma.Upload{}} = ok -> ok diff --git a/test/pleroma/upload_test.exs b/test/pleroma/upload_test.exs index 5ae689c9d..389151edd 100644 --- a/test/pleroma/upload_test.exs +++ b/test/pleroma/upload_test.exs @@ -38,7 +38,12 @@ defmodule Pleroma.UploadTest do describe "Tried storing a file when http callback response success result" do defmodule TestUploaderSuccess do def http_callback(conn, _params, upload), - do: {:ok, conn, Map.put(upload, :path, "post-process-file.jpg")} + do: + {:ok, conn, + Map.merge(upload, %{ + path: "post-process-file.jpg", + url: "http://localhost:4001/media/post-process-file.jpg" + })} def put_file(upload), do: TestUploaderBase.put_file(upload, __MODULE__) diff --git a/test/pleroma/uploaders/local_test.exs b/test/pleroma/uploaders/local_test.exs index 5d405dac5..2c7518bac 100644 --- a/test/pleroma/uploaders/local_test.exs +++ b/test/pleroma/uploaders/local_test.exs @@ -21,7 +21,8 @@ defmodule Pleroma.Uploaders.LocalTest do name: "image.jpg", content_type: "image/jpeg", path: file_path, - tempfile: Path.absname("test/fixtures/image_tmp.jpg") + tempfile: Path.absname("test/fixtures/image_tmp.jpg"), + url: "http://localhost:4001/media/local_upload/files/image.jpg" } assert Local.put_file(file) == {:ok, file} diff --git a/test/pleroma/uploaders/s3_test.exs b/test/pleroma/uploaders/s3_test.exs index 4cc6d186d..6e63d431c 100644 --- a/test/pleroma/uploaders/s3_test.exs +++ b/test/pleroma/uploaders/s3_test.exs @@ -59,7 +59,8 @@ defmodule Pleroma.Uploaders.S3Test do name: "image-tet.jpg", content_type: "image/jpeg", path: "test_folder/image-tet.jpg", - tempfile: Path.absname("test/instance_static/add/shortcode.png") + tempfile: Path.absname("test/instance_static/add/shortcode.png"), + url: "https://s3.amazonaws.com/test_bucket/test_folder/image-tet.jpg" } [file_upload: file_upload]