Hi,
I have written my first PhoenixController from scratch and was feeling quite proud of myself until it came time to dealing with branching logic in the action and my sudden realisation of how it should be done.
So the controller is simple. It takes a file upload, sanitizes the file name, saves it to local storage, gets file_stats and enters a record into the database for the upload. This is all just for a proof of concept, so later on I will do better filename collision (rather than :os.system_time), generate thumbnails etcā¦
However, if saving the file results in an error, then I want to respond appropriately via the connection and the same also applies if I canāt get the file stats. I believe my case statement is correct, but of course even if the file save does result in an :error then it will still continue onto getting the file stats - which of course would not be good.
I have tried using multi clause functions to handle the File.cp return value, which would then call another multi clause function for handling the File.stat return value, however, I then ran into issues of having variables like āsave_pathā not available in my multi-clause function and it seems wrong to need to pass those down to the multi clause function and would be a pain to refactor later onā¦
Any thoughts on where I am going wrong? I am not one of those that is adverse to if statements, but I am curious if there is another way of dealing with this.
Here is my controller so far.
defmodule ServerWeb.MediaController do
use ServerWeb, :controller
alias Server.Playback
alias Server.Playback.Media
def create(conn, %{"media" => upload}) do
extension = Path.extname(upload.filename)
filename = upload.filename
|> build_filename
filename = "#{filename}-#{:os.system_time(:seconds)}#{extension}"
relative_path = "uploads/original/#{filename}"
save_path = "./priv/static/#{relative_path}"
case result = File.cp(upload.path, save_path) do
{:error, reason} -> respond_with_reason(conn, reason)
_ -> result
end
file_stats = case File.stat(save_path) do
{:error, reason} -> respond_with_reason(conn, reason)
{:ok, file_stats} -> file_stats
end
attrs = %{"media_id" => Ecto.UUID.generate(), "path" => "", "customer_id" => 0, "filename" => filename, "type" => "image", "bytes" => file_stats.size}
case Playback.create_media(attrs) do
{:ok, %Media{} = _media} ->
conn
|> send_resp(200, "")
end
end
defp respond_with_reason(conn, reason) do
conn
|> send_resp(200, reason)
end
defp build_filename(original_filename) do
original_filename
|> Path.rootname
|> Zarex.sanitize(padding: 20)
|> String.replace(~r/\s/, "-")
end
end
And here is the code where I have started to implement multi-clause functions:
defmodule ServerWeb.MediaController do
use ServerWeb, :controller
alias Server.Playback
alias Server.Playback.Media
def create(conn, %{"media" => upload}) do
extension = Path.extname(upload.filename)
filename = upload.filename
|> build_filename
filename = "#{filename}-#{:os.system_time(:seconds)}#{extension}"
relative_path = "uploads/original/#{filename}"
save_path = "./priv/static/#{relative_path}"
file_saved(conn, File.cp(upload.path, save_path))
end
defp file_saved(conn, {:ok}) do
file_stats = case File.stat(save_path) do
{:error, reason} -> respond_with_reason(conn, reason)
{:ok, file_stats} -> file_stats
end
attrs = %{"media_id" => Ecto.UUID.generate(), "path" => "", "customer_id" => 0, "filename" => filename, "type" => "image", "bytes" => file_stats.size}
case Playback.create_media(attrs) do
{:ok, %Media{} = _media} ->
conn
|> send_resp(200, "")
end
end
defp file_saved(conn, {:error, reason}) do
respond_with_reason(conn, reason)
end
defp respond_with_reason(conn, reason) do
conn
|> send_resp(200, reason)
end
defp build_filename(original_filename) do
original_filename
|> Path.rootname
|> Zarex.sanitize(padding: 20)
|> String.replace(~r/\s/, "-")
end
end
