potsda.mn/mobilizon
10
4
Fork 1
Code Issues 22 Pull requests 1 Projects Releases Wiki Activity

Merge branch 'allow-removing-files' into 'master'

Browse source 
Allow removing files

Closes #127

See merge request framasoft/mobilizon!149
This commit is contained in:
Thomas Citharel 2019-06-07 15:11:47 +02:00
parent 10dbe14a52 e8cabd38d4
commit 0549b02b3e
10 changed files with 263 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
lib/mobilizon/actors/actor.ex
View file

@ -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

37
lib/mobilizon/actors/actors.ex
View file

@ -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

12
lib/mobilizon/media.ex
View file

@ -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 """

10
lib/mobilizon_web/upload.ex
View file

@ -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

48
lib/mobilizon_web/uploaders/local.ex
View file

@ -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

7
lib/mobilizon_web/uploaders/uploader.ex
View file

@ -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())

94
test/mobilizon/actors/actors_test.exs
View file

@ -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

14
test/mobilizon/media/media_test.exs
View file

@ -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

43
test/mobilizon_web/upload_test.exs
View file

@ -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

19
test/support/factory.ex
View file

@ -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
}