diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex
index 22ca83cb9..8ec368b4d 100644
--- a/lib/mobilizon/actors/actor.ex
+++ b/lib/mobilizon/actors/actor.ex
@@ -70,8 +70,8 @@ defmodule Mobilizon.Actors.Actor do
many_to_many(:memberships, Actor, join_through: Member)
belongs_to(:user, User)
has_many(:feed_tokens, FeedToken, foreign_key: :actor_id)
- embeds_one(:avatar, File)
- embeds_one(:banner, File)
+ embeds_one(:avatar, File, on_replace: :update)
+ embeds_one(:banner, File, on_replace: :update)
timestamps()
end
diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex
index e232e5bec..0fe5c6336 100644
--- a/lib/mobilizon/actors/actors.ex
+++ b/lib/mobilizon/actors/actors.ex
@@ -9,6 +9,8 @@ defmodule Mobilizon.Actors do
alias Mobilizon.Repo
alias Mobilizon.Actors.{Actor, Bot, Member, Follower}
+ alias Mobilizon.Media.File
+ alias Ecto.Multi
alias Mobilizon.Service.ActivityPub
require Logger
@@ -119,9 +121,24 @@ defmodule Mobilizon.Actors do
def update_actor(%Actor{} = actor, attrs) do
actor
|> Actor.changeset(attrs)
+ |> delete_files_if_media_changed()
|> Repo.update()
end
+ defp delete_files_if_media_changed(%Ecto.Changeset{changes: changes} = changeset) do
+ Enum.each([:avatar, :banner], fn key ->
+ if Map.has_key?(changes, key) do
+ with %Ecto.Changeset{changes: %{url: new_url}} <- changes[key],
+ old_url <- Map.from_struct(changeset.data) |> Map.get(key) |> Map.get(:url),
+ false <- new_url == old_url do
+ MobilizonWeb.Upload.remove(old_url)
+ end
+ end
+ end)
+
+ changeset
+ end
+
@doc """
Deletes a Actor.
@@ -135,6 +152,26 @@ defmodule Mobilizon.Actors do
"""
@spec delete_actor(Actor.t()) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
+ def delete_actor(%Actor{domain: nil} = actor) do
+ case Multi.new()
+ |> Multi.delete(:actor, actor)
+ |> Multi.run(:remove_banner, fn _repo,
+ %{actor: %Actor{banner: %File{url: url}}} = _picture ->
+ MobilizonWeb.Upload.remove(url)
+ end)
+ |> Multi.run(:remove_avatar, fn _repo,
+ %{actor: %Actor{avatar: %File{url: url}}} = _picture ->
+ MobilizonWeb.Upload.remove(url)
+ end)
+ |> Repo.transaction() do
+ {:ok, %{actor: %Actor{} = actor}} ->
+ {:ok, actor}
+
+ {:error, remove, error, _} when remove in [:remove_banner, :remove_avatar] ->
+ {:error, error}
+ end
+ end
+
def delete_actor(%Actor{} = actor) do
Repo.delete(actor)
end
diff --git a/lib/mobilizon/media.ex b/lib/mobilizon/media.ex
index 4182e0701..850b5274e 100644
--- a/lib/mobilizon/media.ex
+++ b/lib/mobilizon/media.ex
@@ -7,6 +7,8 @@ defmodule Mobilizon.Media do
alias Mobilizon.Repo
alias Mobilizon.Media.Picture
+ alias Mobilizon.Media.File
+ alias Ecto.Multi
@doc false
def data() do
@@ -97,7 +99,15 @@ defmodule Mobilizon.Media do
"""
def delete_picture(%Picture{} = picture) do
- Repo.delete(picture)
+ case Multi.new()
+ |> Multi.delete(:picture, picture)
+ |> Multi.run(:remove, fn _repo, %{picture: %Picture{file: %File{url: url}}} = _picture ->
+ MobilizonWeb.Upload.remove(url)
+ end)
+ |> Repo.transaction() do
+ {:ok, %{picture: %Picture{} = picture}} -> {:ok, picture}
+ {:error, :remove, error, _} -> {:error, error}
+ end
end
@doc """
diff --git a/lib/mobilizon_web/upload.ex b/lib/mobilizon_web/upload.ex
index 24ad765eb..d98931477 100644
--- a/lib/mobilizon_web/upload.ex
+++ b/lib/mobilizon_web/upload.ex
@@ -88,6 +88,16 @@ defmodule MobilizonWeb.Upload do
end
end
+ def remove(url, opts \\ []) do
+ with opts <- get_opts(opts),
+ %URI{path: "/media/" <> path, host: host} <- URI.parse(url),
+ true <- host == MobilizonWeb.Endpoint.host() do
+ MobilizonWeb.Uploaders.Uploader.remove_file(opts.uploader, path)
+ else
+ %URI{} = _uri -> {:error, "URL doesn't match pattern"}
+ end
+ end
+
def char_unescaped?(char) do
URI.char_unreserved?(char) or char == ?/
end
diff --git a/lib/mobilizon_web/uploaders/local.ex b/lib/mobilizon_web/uploaders/local.ex
index 9308a9ffe..82d87ae56 100644
--- a/lib/mobilizon_web/uploaders/local.ex
+++ b/lib/mobilizon_web/uploaders/local.ex
@@ -14,18 +14,8 @@ defmodule MobilizonWeb.Uploaders.Local do
end
def put_file(upload) do
- {local_path, file} =
- case Enum.reverse(String.split(upload.path, "/", trim: true)) do
- [file] ->
- {upload_path(), file}
-
- [file | folders] ->
- path = Path.join([upload_path()] ++ Enum.reverse(folders))
- File.mkdir_p!(path)
- {path, file}
- end
-
- result_file = Path.join(local_path, file)
+ {path, file} = local_path(upload.path)
+ result_file = Path.join(path, file)
unless File.exists?(result_file) do
File.cp!(upload.tempfile, result_file)
@@ -34,6 +24,40 @@ defmodule MobilizonWeb.Uploaders.Local do
:ok
end
+ 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}
+ else
+ false -> {:error, "File #{path} doesn't exist"}
+ end
+ end
+
+ 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"}
+ end
+ end
+
+ defp local_path(path) do
+ case Enum.reverse(String.split(path, "/", trim: true)) do
+ [file] ->
+ {upload_path(), file}
+
+ [file | folders] ->
+ path = Path.join([upload_path()] ++ Enum.reverse(folders))
+ File.mkdir_p!(path)
+ {path, file}
+ end
+ end
+
def upload_path do
Mobilizon.CommonConfig.get!([__MODULE__, :uploads])
end
diff --git a/lib/mobilizon_web/uploaders/uploader.ex b/lib/mobilizon_web/uploaders/uploader.ex
index 2154c6d84..1f8d0fda4 100644
--- a/lib/mobilizon_web/uploaders/uploader.ex
+++ b/lib/mobilizon_web/uploaders/uploader.ex
@@ -35,6 +35,8 @@ defmodule MobilizonWeb.Uploaders.Uploader do
@callback put_file(MobilizonWeb.Upload.t()) ::
:ok | {:ok, file_spec()} | {:error, String.t()} | :wait_callback
+ @callback remove_file(file_spec()) :: :ok | {:ok, file_spec()} | {:error, String.t()}
+
@callback http_callback(Plug.Conn.t(), Map.t()) ::
{:ok, Plug.Conn.t()}
| {:ok, Plug.Conn.t(), file_spec()}
@@ -42,7 +44,6 @@ defmodule MobilizonWeb.Uploaders.Uploader do
@optional_callbacks http_callback: 2
@spec put_file(module(), MobilizonWeb.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}}
@@ -52,6 +53,10 @@ defmodule MobilizonWeb.Uploaders.Uploader do
end
end
+ def remove_file(uploader, path) do
+ uploader.remove_file(path)
+ end
+
defp handle_callback(uploader, upload) do
:global.register_name({__MODULE__, upload.path}, self())
diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs
index e1e10dc26..a6d023df0 100644
--- a/test/mobilizon/actors/actors_test.exs
+++ b/test/mobilizon/actors/actors_test.exs
@@ -4,7 +4,7 @@ defmodule Mobilizon.ActorsTest do
alias Mobilizon.Actors
alias Mobilizon.Actors.{Actor, Member, Follower, Bot}
alias Mobilizon.Users
- alias Mobilizon.Media.File
+ alias Mobilizon.Media.File, as: FileModel
import Mobilizon.Factory
use ExVCR.Mock, adapter: ExVCR.Adapter.Hackney
@@ -97,14 +97,14 @@ defmodule Mobilizon.ActorsTest do
id: actor_id,
preferred_username: preferred_username,
domain: domain,
- avatar: %File{name: picture_name} = _picture
+ avatar: %FileModel{name: picture_name} = _picture
} = _actor} = Actors.get_or_fetch_by_url(@remote_account_url)
assert picture_name == "avatar"
%Actor{
id: actor_found_id,
- avatar: %File{name: picture_name} = _picture
+ avatar: %FileModel{name: picture_name} = _picture
} = Actors.get_actor_by_name("#{preferred_username}@#{domain}")
assert actor_found_id == actor_id
@@ -235,7 +235,8 @@ defmodule Mobilizon.ActorsTest do
test "test get_public_key_for_url/1 with remote actor and bad key" do
use_cassette "actors/remote_actor_mastodon_tcit_actor_deleted" do
- assert Actor.get_public_key_for_url(@remote_account_url) == {:error, :actor_fetch_error}
+ assert Actor.get_public_key_for_url(@remote_account_url) ==
+ {:error, :actor_fetch_error}
end
end
@@ -257,8 +258,15 @@ defmodule Mobilizon.ActorsTest do
assert {:error, %Ecto.Changeset{}} = Actors.create_actor(@invalid_attrs)
end
- test "update_actor/2 with valid data updates the actor", %{actor: actor} do
- assert {:ok, actor} = Actors.update_actor(actor, @update_attrs)
+ test "update_actor/2 with valid data updates the actor", %{
+ actor: %Actor{} = actor
+ } do
+ assert {:ok, actor} =
+ Actors.update_actor(
+ actor,
+ @update_attrs
+ )
+
assert %Actor{} = actor
assert actor.summary == "some updated description"
assert actor.name == "some updated name"
@@ -268,15 +276,85 @@ defmodule Mobilizon.ActorsTest do
assert actor.preferred_username == "some updated username"
end
+ test "update_actor/2 with valid data updates the actor and it's media files", %{
+ actor: %Actor{avatar: %{url: avatar_url}, banner: %{url: banner_url}} = actor
+ } do
+ %URI{path: "/media/" <> avatar_path} = URI.parse(avatar_url)
+ %URI{path: "/media/" <> banner_path} = URI.parse(banner_url)
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> avatar_path
+ )
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> banner_path
+ )
+
+ file = %Plug.Upload{
+ content_type: "image/jpg",
+ path: Path.absname("test/fixtures/image.jpg"),
+ filename: "image.jpg"
+ }
+
+ {:ok, data} = MobilizonWeb.Upload.store(file)
+ url = hd(data["url"])["href"]
+
+ assert {:ok, actor} =
+ Actors.update_actor(
+ actor,
+ Map.put(@update_attrs, :avatar, %{name: file.filename, url: url})
+ )
+
+ assert %Actor{} = actor
+ assert actor.summary == "some updated description"
+ assert actor.name == "some updated name"
+ assert actor.domain == "some updated domain"
+ assert actor.keys == "some updated keys"
+ refute actor.suspended
+ assert actor.preferred_username == "some updated username"
+
+ refute File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> avatar_path
+ )
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> banner_path
+ )
+ end
+
test "update_actor/2 with invalid data returns error changeset", %{actor: actor} do
assert {:error, %Ecto.Changeset{}} = Actors.update_actor(actor, @invalid_attrs)
actor_fetched = Actors.get_actor!(actor.id)
assert actor = actor_fetched
end
- test "delete_actor/1 deletes the actor", %{actor: actor} do
+ test "delete_actor/1 deletes the actor", %{
+ actor: %Actor{avatar: %{url: avatar_url}, banner: %{url: banner_url}, id: actor_id} = actor
+ } do
+ %URI{path: "/media/" <> avatar_path} = URI.parse(avatar_url)
+ %URI{path: "/media/" <> banner_path} = URI.parse(banner_url)
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> avatar_path
+ )
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> banner_path
+ )
+
assert {:ok, %Actor{}} = Actors.delete_actor(actor)
- assert_raise Ecto.NoResultsError, fn -> Actors.get_actor!(actor.id) end
+ assert_raise Ecto.NoResultsError, fn -> Actors.get_actor!(actor_id) end
+
+ refute File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> banner_path
+ )
end
test "change_actor/1 returns a actor changeset", %{actor: actor} do
diff --git a/test/mobilizon/media/media_test.exs b/test/mobilizon/media/media_test.exs
index 04dce976e..6ba645d6a 100644
--- a/test/mobilizon/media/media_test.exs
+++ b/test/mobilizon/media/media_test.exs
@@ -5,6 +5,7 @@ defmodule Mobilizon.MediaTest do
import Mobilizon.Factory
describe "media" do
+ setup [:ensure_local_uploader]
alias Mobilizon.Media.Picture
@valid_attrs %{
@@ -43,8 +44,21 @@ defmodule Mobilizon.MediaTest do
test "delete_picture/1 deletes the picture" do
picture = insert(:picture)
+
+ %URI{path: "/media/" <> path} = URI.parse(picture.file.url)
+
+ assert File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> path
+ )
+
assert {:ok, %Picture{}} = Media.delete_picture(picture)
assert_raise Ecto.NoResultsError, fn -> Media.get_picture!(picture.id) end
+
+ refute File.exists?(
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/" <> path
+ )
end
test "change_picture/1 returns a picture changeset" do
diff --git a/test/mobilizon_web/upload_test.exs b/test/mobilizon_web/upload_test.exs
index 39f1bb656..4434d0937 100644
--- a/test/mobilizon_web/upload_test.exs
+++ b/test/mobilizon_web/upload_test.exs
@@ -173,5 +173,48 @@ defmodule Mobilizon.UploadTest do
assert Path.basename(attachment_url["href"]) ==
"%3A%3F%23%5B%5D%40%21%24%26%5C%27%28%29%2A%2B%2C%3B%3D.jpg"
end
+
+ test "upload and delete successfully a file" do
+ {path, url} = upload()
+ assert File.exists?(path)
+ assert {:ok, _} = Upload.remove(url)
+ refute File.exists?(path)
+ path = path |> Path.split() |> Enum.reverse() |> tl |> Enum.reverse() |> Path.join()
+ refute File.exists?(path)
+ end
+
+ test "delete a not existing file" do
+ file =
+ Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <>
+ "/not_existing/definitely.jpg"
+
+ refute File.exists?(file)
+
+ assert {:error, "File not_existing/definitely.jpg doesn't exist"} =
+ Upload.remove("https://mobilizon.test/media/not_existing/definitely.jpg")
+ end
+ end
+
+ defp upload() do
+ File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg")
+
+ file = %Plug.Upload{
+ content_type: "image/jpg",
+ path: Path.absname("test/fixtures/image_tmp.jpg"),
+ filename: "image.jpg"
+ }
+
+ {:ok, data} = Upload.store(file)
+
+ assert %{
+ "url" => [%{"href" => url, "mediaType" => "image/jpeg"}],
+ "size" => 13_227,
+ "type" => "Image"
+ } = data
+
+ assert String.starts_with?(url, MobilizonWeb.Endpoint.url() <> "/media/")
+
+ %URI{path: "/media/" <> path} = URI.parse(url)
+ {Mobilizon.CommonConfig.get!([MobilizonWeb.Uploaders.Local, :uploads]) <> "/" <> path, url}
end
end
diff --git a/test/support/factory.ex b/test/support/factory.ex
index 5362bbe9c..002566d2f 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -7,6 +7,7 @@ defmodule Mobilizon.Factory do
alias Mobilizon.Actors.Actor
alias MobilizonWeb.Router.Helpers, as: Routes
alias MobilizonWeb.Endpoint
+ alias MobilizonWeb.Upload
def user_factory do
%Mobilizon.Users.User{
@@ -171,9 +172,25 @@ defmodule Mobilizon.Factory do
end
def file_factory do
+ File.cp!("test/fixtures/image.jpg", "test/fixtures/image_tmp.jpg")
+
+ file = %Plug.Upload{
+ content_type: "image/jpg",
+ path: Path.absname("test/fixtures/image_tmp.jpg"),
+ filename: "image.jpg"
+ }
+
+ {:ok, data} = Upload.store(file)
+
+ %{
+ "url" => [%{"href" => url, "mediaType" => "image/jpeg"}],
+ "size" => 13_227,
+ "type" => "Image"
+ } = data
+
%Mobilizon.Media.File{
name: "My Picture",
- url: MobilizonWeb.Endpoint.url() <> "/uploads/something",
+ url: url,
content_type: "image/png",
size: 13_120
}