Various typespec and compilation improvements

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/web/upload/filter/analyze_metadata.ex

@@ -13,7 +13,7 @@ defmodule Mobilizon.Web.Upload.Filter.AnalyzeMetadata do
   @behaviour Mobilizon.Web.Upload.Filter
 
   @spec filter(Upload.t()) ::
-          {:ok, :filtered, Upload.t()} | {:ok, :noop} | {:error, String.t()}
+          {:ok, :filtered, Upload.t()} | {:ok, :noop}
   def filter(%Upload{tempfile: file, content_type: "image" <> _} = upload) do
     image =
       file

lib/web/upload/filter/anonymize_filename.ex

@@ -14,19 +14,27 @@ defmodule Mobilizon.Web.Upload.Filter.AnonymizeFilename do
 
   alias Mobilizon.Config
   alias Mobilizon.Web.Upload
+  alias Mobilizon.Web.Upload.Filter
+  import Mobilizon.Service.Guards, only: [is_valid_string: 1]
 
+  @impl Filter
+  @spec filter(any) :: {:ok, :filtered, Upload.t()} | {:ok, :noop}
   def filter(%Upload{name: name} = upload) do
     extension = List.last(String.split(name, "."))
-    name = predefined_name(extension) || random(extension)
+    name = predefined_name(extension)
+    name = if is_nil(name), do: random(extension), else: name
     {:ok, :filtered, %Upload{upload | name: name}}
   end
 
+  @impl Filter
   def filter(_), do: {:ok, :noop}
 
   @spec predefined_name(String.t()) :: String.t() | nil
   defp predefined_name(extension) do
-    with name when not is_nil(name) <- Config.get([__MODULE__, :text]),
-         do: String.replace(name, "{extension}", extension)
+    case Config.get([__MODULE__, :text]) do
+      name when is_valid_string(name) -> String.replace(name, "{extension}", extension)
+      _ -> nil
+    end
   end
 
   defp random(extension) do

lib/web/upload/filter/blurhash.ex

@@ -8,7 +8,7 @@ defmodule Mobilizon.Web.Upload.Filter.BlurHash do
   @behaviour Mobilizon.Web.Upload.Filter
 
   @spec filter(Upload.t()) ::
-          {:ok, :filtered, Upload.t()} | {:ok, :noop} | {:error, String.t()}
+          {:ok, :filtered, Upload.t()} | {:ok, :noop}
   def filter(%Upload{tempfile: file, content_type: "image" <> _} = upload) do
     {:ok, :filtered, %Upload{upload | blurhash: generate_blurhash(file)}}
   rescue

lib/web/upload/filter/dedupe.ex

@@ -10,6 +10,7 @@ defmodule Mobilizon.Web.Upload.Filter.Dedupe do
   @behaviour Mobilizon.Web.Upload.Filter
   alias Mobilizon.Web.Upload
 
+  @spec filter(Upload.t()) :: {:ok, :filtered, Upload.t()} | {:ok, :noop}
   def filter(%Upload{name: name, tempfile: tempfile} = upload) do
     extension = name |> String.split(".") |> List.last()
     shasum = :crypto.hash(:sha256, File.read!(tempfile)) |> Base.encode16(case: :lower)

lib/web/upload/filter/exiftool.ex

@@ -11,7 +11,7 @@ defmodule Mobilizon.Web.Upload.Filter.Exiftool do
 
   @behaviour Mobilizon.Web.Upload.Filter
 
-  @spec filter(Upload.t()) :: {:ok, any()} | {:error, String.t()}
+  @spec filter(Upload.t()) :: {:ok, :filtered | :noop} | {:error, String.t()}
 
   # webp is not compatible with exiftool at this time
   def filter(%Upload{content_type: "image/webp"}), do: {:ok, :noop}

lib/web/upload/filter/filter.ex

@@ -20,11 +20,10 @@ defmodule Mobilizon.Web.Upload.Filter do
               {:ok, :filtered}
               | {:ok, :noop}
               | {:ok, :filtered, Mobilizon.Web.Upload.t()}
-              | {:error, any()}
+              | {:error, String.t() | atom}
 
   @spec filter([module()], Mobilizon.Web.Upload.t()) ::
-          {:ok, Mobilizon.Web.Upload.t()} | {:error, any()}
-
+          {:ok, Mobilizon.Web.Upload.t()} | {:error, String.t() | atom}
   def filter([], upload) do
     {:ok, upload}
   end

lib/web/upload/filter/mogrify.ex

@@ -15,7 +15,7 @@ defmodule Mobilizon.Web.Upload.Filter.Mogrify do
   @type conversion :: action :: String.t() | {action :: String.t(), opts :: String.t()}
   @type conversions :: conversion() | [conversion()]
 
-  @spec filter(Mobilizon.Web.Upload.t()) :: {:ok, :atom} | {:error, String.t()}
+  @spec filter(Mobilizon.Web.Upload.t()) :: {:ok, :filtered | :noop} | {:error, String.t()}
   def filter(%Mobilizon.Web.Upload{tempfile: file, content_type: "image" <> _}) do
     do_filter(file, Config.get!([__MODULE__, :args]))
     {:ok, :filtered}

lib/web/upload/filter/optimize.ex

@@ -6,6 +6,8 @@ defmodule Mobilizon.Web.Upload.Filter.Optimize do
   @behaviour Mobilizon.Web.Upload.Filter
 
   alias Mobilizon.Config
+  alias Mobilizon.Web.Upload
+  require Logger
 
   @default_optimizers [
     JpegOptim,
@@ -16,24 +18,24 @@ defmodule Mobilizon.Web.Upload.Filter.Optimize do
     Cwebp
   ]
 
-  def filter(%Mobilizon.Web.Upload{tempfile: file, content_type: "image" <> _}) do
+  @spec filter(Upload.t()) :: {:ok, :filtered | :noop} | {:error, :file_not_found}
+  def filter(%Upload{tempfile: file, content_type: "image" <> _}) do
     optimizers = Config.get([__MODULE__, :optimizers], @default_optimizers)
 
     case ExOptimizer.optimize(file, deps: optimizers) do
       {:ok, _res} ->
         {:ok, :filtered}
 
-      {:error, err} ->
-        require Logger
+      {:error, :file_not_found} ->
+        Logger.warn("Unable to optimize file #{file}. File was not found")
+        {:error, :file_not_found}
 
+      {:error, err} ->
         Logger.warn(
           "Unable to optimize file #{file}. The return from the process was #{inspect(err)}"
         )
 
         {:ok, :noop}
-
-      err ->
-        {:error, err}
     end
   end
 

lib/web/upload/filter/resize.ex

@@ -11,6 +11,7 @@ defmodule Mobilizon.Web.Upload.Filter.Resize do
   @maximum_width 1_920
   @maximum_height 1_080
 
+  @spec filter(Upload.t()) :: {:ok, :filtered, Upload.t()} | {:ok, :noop}
   def filter(
         %Upload{
           tempfile: file,
@@ -31,6 +32,7 @@ defmodule Mobilizon.Web.Upload.Filter.Resize do
 
   def filter(_), do: {:ok, :noop}
 
+  @spec limit_sizes({non_neg_integer, non_neg_integer}) :: {non_neg_integer, non_neg_integer}
   def limit_sizes({width, height}) when width > @maximum_width do
     new_height = round(@maximum_width * height / width)
     limit_sizes({@maximum_width, new_height})
@@ -43,5 +45,6 @@ defmodule Mobilizon.Web.Upload.Filter.Resize do
 
   def limit_sizes({width, height}), do: {width, height}
 
+  @spec string({non_neg_integer, non_neg_integer}) :: String.t()
   defp string({width, height}), do: "#{width}x#{height}"
 end

lib/web/upload/mime.ex

@@ -10,8 +10,9 @@ defmodule Mobilizon.Web.Upload.MIME do
   @default "application/octet-stream"
   @read_bytes 35
 
-  @spec file_mime_type(String.t()) ::
+  @spec file_mime_type(String.t(), String.t()) ::
           {:ok, content_type :: String.t(), filename :: String.t()} | {:error, any()} | :error
+  @spec file_mime_type(String.t()) :: {:ok, String.t()} | {:error, any()} | :error
   def file_mime_type(path, filename) do
     with {:ok, content_type} <- file_mime_type(path),
          filename when is_binary(filename) <- fix_extension(filename, content_type) do
@@ -19,7 +20,6 @@ defmodule Mobilizon.Web.Upload.MIME do
     end
   end
 
-  @spec file_mime_type(String.t()) :: {:ok, String.t()} | {:error, any()} | :error
   def file_mime_type(filename) do
     File.open(filename, [:read], fn f ->
       check_mime_type(IO.binread(f, @read_bytes))

lib/web/upload/upload.ex

@@ -53,6 +53,7 @@ defmodule Mobilizon.Web.Upload do
           | {:size_limit, nil | non_neg_integer()}
           | {:uploader, module()}
           | {:filters, [module()]}
+          | {:allow_list_mime_types, boolean()}
 
   @type t :: %__MODULE__{
           id: String.t(),
@@ -65,33 +66,36 @@ defmodule Mobilizon.Web.Upload do
           height: integer(),
           blurhash: String.t()
         }
-  defstruct [:id, :name, :tempfile, :content_type, :path, :size, :width, :height, :blurhash]
+  defstruct [:id, :name, :url, :tempfile, :content_type, :path, :size, :width, :height, :blurhash]
 
-  @spec store(source, options :: [option()]) :: {:ok, map()} | {:error, any()}
+  @typep internal_options :: %{
+           activity_type: String.t() | nil,
+           size_limit: integer(),
+           uploader: module(),
+           filters: [module()],
+           description: String.t(),
+           allow_list_mime_types: list(String.t()),
+           base_url: String.t()
+         }
+
+  @spec store(source, options :: [option()]) ::
+          {:ok, map()} | {:error, String.t()} | {:error, atom()}
   def store(upload, opts \\ []) do
     opts = get_opts(opts)
 
-    with {:ok, upload} <- prepare_upload(upload, opts),
-         %__MODULE__{} = upload <- %__MODULE__{
-           upload
-           | path: upload.path || "#{upload.id}/#{upload.name}"
-         },
-         {:ok, upload} <- Filter.filter(opts.filters, upload),
-         {:ok, url_spec} <- Uploader.put_file(opts.uploader, upload) do
-      {:ok,
-       upload
-       |> Map.put(:name, Map.get(opts, :description) || upload.name)
-       |> Map.put(:url, url_from_spec(upload, opts.base_url, url_spec))}
-    else
-      {:error, error} ->
-        Logger.error(
-          "#{__MODULE__} store (using #{inspect(opts.uploader)}) failed: #{inspect(error)}"
-        )
+    case prepare_upload(upload, opts) do
+      {:ok, upload} ->
+        upload
+        |> set_default_upload_path()
+        |> perform_filter_and_put_file(opts)
 
-        {:error, error}
+      {:error, error} ->
+        error
     end
   end
 
+  @spec remove(String.t(), Keyword.t()) ::
+          {:ok, String.t()} | {:error, atom} | {:error, String.t()}
   def remove(url, opts \\ []) do
     with opts <- get_opts(opts),
          %URI{path: "/media/" <> path, host: host} <- URI.parse(url),
@@ -106,10 +110,52 @@ defmodule Mobilizon.Web.Upload do
     end
   end
 
-  def char_unescaped?(char) do
+  @spec char_unescaped?(byte()) :: boolean()
+  defp char_unescaped?(char) do
     URI.char_unreserved?(char) or char == ?/
   end
 
+  @spec set_default_upload_path(t) :: t
+  defp set_default_upload_path(%__MODULE__{} = upload) do
+    %__MODULE__{
+      upload
+      | path: upload.path || "#{upload.id}/#{upload.name}"
+    }
+  end
+
+  @spec perform_filter_and_put_file(t, map) ::
+          {:ok, t} | {:error, String.t()} | {:error, atom()}
+  defp perform_filter_and_put_file(%__MODULE__{} = upload, opts) do
+    case Filter.filter(opts.filters, upload) do
+      {:ok, upload} ->
+        perform_put_file(upload, opts)
+
+      {:error, error} ->
+        {:error, error}
+    end
+  end
+
+  @spec perform_put_file(t, map) :: {:ok, t} | {:error, atom()}
+  defp perform_put_file(%__MODULE__{} = upload, opts) do
+    case Uploader.put_file(opts.uploader, upload) do
+      {:ok, url_spec} ->
+        {:ok,
+         %__MODULE__{
+           upload
+           | name: Map.get(opts, :description) || upload.name,
+             url: url_from_spec(upload, opts.base_url, url_spec)
+         }}
+
+      {:error, error} ->
+        Logger.error(
+          "#{__MODULE__} store (using #{inspect(opts.uploader)}) failed: #{inspect(error)}"
+        )
+
+        {:error, error}
+    end
+  end
+
+  @spec get_opts(Keyword.t()) :: internal_options()
   defp get_opts(opts) do
     {size_limit, activity_type} =
       case Keyword.get(opts, :type) do
@@ -144,6 +190,7 @@ defmodule Mobilizon.Web.Upload do
     }
   end
 
+  @spec prepare_upload(t(), internal_options()) :: {:ok, t()}
   defp prepare_upload(%Plug.Upload{} = file, opts) do
     with {:ok, size} <- check_file_size(file.path, opts.size_limit),
          {:ok, content_type, name} <- MIME.file_mime_type(file.path, file.filename),
@@ -159,6 +206,7 @@ defmodule Mobilizon.Web.Upload do
     end
   end
 
+  @spec prepare_upload(%{body: String.t(), name: String.t()}, internal_options()) :: {:ok, t()}
   defp prepare_upload(%{body: body, name: name} = _file, opts) do
     with :ok <- check_binary_size(body, opts.size_limit),
          tmp_path <- tempfile_for_image(body),
@@ -175,8 +223,10 @@ defmodule Mobilizon.Web.Upload do
     end
   end
 
+  @spec check_file_size(String.t(), non_neg_integer()) ::
+          {:ok, non_neg_integer()} | {:error, :file_too_large} | {:error, :file.posix()}
   defp check_file_size(path, size_limit) when is_integer(size_limit) and size_limit > 0 do
-    with {:ok, %{size: size}} <- File.stat(path),
+    with {:ok, %File.Stat{size: size}} <- File.stat(path),
          true <- size <= size_limit do
       {:ok, size}
     else
@@ -185,8 +235,7 @@ defmodule Mobilizon.Web.Upload do
     end
   end
 
-  defp check_file_size(_, _), do: :ok
-
+  @spec check_binary_size(String.t(), non_neg_integer()) :: :ok | {:error, :file_too_large}
   defp check_binary_size(binary, size_limit)
        when is_integer(size_limit) and size_limit > 0 and byte_size(binary) >= size_limit do
     {:error, :file_too_large}
@@ -196,6 +245,7 @@ defmodule Mobilizon.Web.Upload do
 
   # Creates a tempfile using the Plug.Upload Genserver which cleans them up
   # automatically.
+  @spec tempfile_for_image(iodata) :: String.t()
   defp tempfile_for_image(data) do
     {:ok, tmp_path} = Plug.Upload.random_file("temp_files")
     {:ok, tmp_file} = File.open(tmp_path, [:write, :raw, :binary])
@@ -204,6 +254,7 @@ defmodule Mobilizon.Web.Upload do
     tmp_path
   end
 
+  @spec url_from_spec(t, String.t(), {:file | :url, String.t()}) :: String.t()
   defp url_from_spec(%__MODULE__{name: name}, base_url, {:file, path}) do
     path =
       URI.encode(path, &char_unescaped?/1) <>

lib/web/upload/uploader/local.ex

@@ -19,10 +19,24 @@ defmodule Mobilizon.Web.Upload.Uploader.Local do
   end
 
   @impl true
+  @spec put_file(Upload.t()) ::
+          :ok | {:ok, {:file, String.t()}} | {:error, :tempfile_no_longer_exists}
   def put_file(%Upload{path: path, tempfile: tempfile}) do
     {path, file} = local_path(path)
     result_file = Path.join(path, file)
 
+    if File.exists?(result_file) do
+      # If the resulting file already exists, it's because of the Dedupe filter
+      :ok
+    else
+      if File.exists?(tempfile) do
+        File.cp!(tempfile, result_file)
+        {:ok, {:file, result_file}}
+      else
+        {:error, :tempfile_no_longer_exists}
+      end
+    end
+
     with {:result_exists, false} <- {:result_exists, File.exists?(result_file)},
          {:temp_file_exists, true} <- {:temp_file_exists, File.exists?(tempfile)} do
       File.cp!(tempfile, result_file)
@@ -37,28 +51,54 @@ defmodule Mobilizon.Web.Upload.Uploader.Local do
   end
 
   @impl true
+  @spec remove_file(String.t()) ::
+          {:ok, {:file, String.t()}}
+          | {:error, :folder_not_empty}
+          | {:error, :enofile}
+          | {:error, File.posix()}
   def remove_file(path) do
-    with {path, file} <- local_path(path),
-         full_path <- Path.join(path, file),
-         true <- File.exists?(full_path),
-         :ok <- File.rm(full_path),
-         :ok <- remove_folder(path) do
-      {:ok, path}
+    {path, file} = local_path(path)
+    full_path = Path.join(path, file)
+
+    if File.exists?(full_path) do
+      do_remove_file(path, full_path)
     else
-      false -> {:error, "File #{path} doesn't exist"}
+      {:error, :enofile}
     end
   end
 
+  @spec do_remove_file(String.t(), String.t()) ::
+          {:ok, {:file, String.t()}}
+          | {:error, :folder_not_empty}
+          | {:error, File.posix()}
+  defp do_remove_file(path, full_path) do
+    case File.rm(full_path) do
+      :ok ->
+        case remove_folder(path) do
+          :ok ->
+            {:ok, {:file, path}}
+
+          {:error, err} ->
+            {:error, err}
+        end
+
+      {:error, err} ->
+        {:error, err}
+    end
+  end
+
+  @spec remove_folder(String.t()) :: :ok | {:error, :folder_not_empty} | {:error, File.posix()}
   defp remove_folder(path) do
     with {:subfolder, true} <- {:subfolder, path != upload_path()},
          {:empty_folder, {:ok, [] = _files}} <- {:empty_folder, File.ls(path)} do
       File.rmdir(path)
     else
       {:subfolder, _} -> :ok
-      {:empty_folder, _} -> {:error, "Error: Folder is not empty"}
+      {:empty_folder, _} -> {:error, :folder_not_empty}
     end
   end
 
+  @spec local_path(String.t()) :: {String.t(), String.t()}
   defp local_path(path) do
     case Enum.reverse(String.split(path, "/", trim: true)) do
       [file] ->
@@ -71,6 +111,7 @@ defmodule Mobilizon.Web.Upload.Uploader.Local do
     end
   end
 
+  @spec upload_path :: String.t()
   def upload_path do
     Config.get!([__MODULE__, :uploads])
   end

lib/web/upload/uploader/uploader.ex

@@ -33,9 +33,9 @@ defmodule Mobilizon.Web.Upload.Uploader do
   """
   @type file_spec :: {:file | :url, String.t()}
   @callback put_file(Mobilizon.Web.Upload.t()) ::
-              :ok | {:ok, file_spec()} | {:error, String.t()} | :wait_callback
+              :ok | {:ok, file_spec()} | {:error, atom()} | :wait_callback
 
-  @callback remove_file(file_spec()) :: :ok | {:ok, file_spec()} | {:error, String.t()}
+  @callback remove_file(file_spec()) :: :ok | {:ok, file_spec()} | {:error, atom()}
 
   @callback http_callback(Plug.Conn.t(), map()) ::
               {:ok, Plug.Conn.t()}
@@ -43,7 +43,7 @@ defmodule Mobilizon.Web.Upload.Uploader do
               | {:error, Plug.Conn.t(), String.t()}
   @optional_callbacks http_callback: 2
 
-  @spec put_file(module(), Mobilizon.Web.Upload.t()) :: {:ok, file_spec()} | {:error, String.t()}
+  @spec put_file(module(), Mobilizon.Web.Upload.t()) :: {:ok, file_spec()} | {:error, atom()}
   def put_file(uploader, upload) do
     case uploader.put_file(upload) do
       :ok -> {:ok, {:file, upload.path}}
@@ -53,6 +53,7 @@ defmodule Mobilizon.Web.Upload.Uploader do
     end
   end
 
+  @spec remove_file(module(), String.t()) :: {:ok, String.t()} | {:error, atom()}
   def remove_file(uploader, path) do
     uploader.remove_file(path)
   end