stavro / arc_ecto

An integration with Arc and Ecto.
255 stars 149 forks source link

arc_ecto not working with ecto 3 #104

Closed AppyCat closed 5 years ago

AppyCat commented 5 years ago

Hi, I'm using Elixir 1.7.3 with Phoenix 1.4.0 with Arc 0.11.0 and Ecto 3.0

Problem is arc_ecto 0.11.0 wants Ecto 2.0

Can arc_ecto work with Ecto 3 on any other branch?

dbernheisel commented 5 years ago

Can you try from master now? Should be fixed with #103

AppyCat commented 5 years ago

Hi, it did install fine from Master :) and is working well. Thank you very much for your speedy update on that :)

Have 3 little glitches that may be of interest:

  1. Its creating 2 sets of each version. So I have 4 files instead of 2.
  2. When I try to override the local storage directory, it can't find the file or scope.
  3. If I set a UUID on version, it keeps changes for each pic processed and isn't reflected in the db.

Is it possible to pass in client_id or set one constant UUID for each version stored in the db?

My uploader file and controller look as follows. The function is to update, so ideally we can delete the existing versions automatically via arc or I can write some logic to do so.

CONTROLLER FUNCTION def update(conn, %{"id" => id, "user" => user_params}) do user = Auth.get_user!(id) case Auth.update_user(user, user_params) do {:ok, user} -> conn |> put_flash(:info, "User updated successfully.") |> redirect(to: Routes.page_path(conn, :show, user)) {:error, %Ecto.Changeset{} = changeset} -> render(conn, "edit.html", user: user, changeset: changeset) end end

UPLOADER FILE defmodule UserUploader.Avatar do

use Arc.Definition use Arc.Ecto.Definition

def __storage, do: Arc.Storage.Local

@versions [:original]

@versions [:original, :thumb]

def validate({file, _}) do ~w(.jpg .jpeg .gif .png) |> Enum.member?(Path.extname(file.file_name)) end

def transform(:thumb, _) do {:convert, "-strip -thumbnail 250x250^ -gravity center -extent 250x250 -format png", :png} end

def filename(version, _) do "#{UUID.uuid1()}" end

def storagedir(version, ) do "uploads/users/" end end

AppyCat commented 5 years ago

Everything seems to be working fine. Only problem seems to be extracting id out of scope.

{scope.id} isn’t working. If I inspect scope it shows:

%UserApp.Auth.User{ meta: #Ecto.Schema.Metadata<:loaded, "users">, id: 1, inserted_at: ~N[2018-11-16 02:25:49], password: "$2b$12$zn/rUYLyX73iJRYi00/H4OiXcF9VZSFiM1bzexWhPqio0zO4yBy3y", photo: %{file_name: "Andrea5.jpg", updated_at: ~N[2018-11-19 06:57:46]}, updated_at: ~N[2018-11-19 06:57:46], username: "scooby" }

dbernheisel commented 5 years ago

@appycat try using the ID from the scope instead of generating a uuid. Each time the filename function is called it will generate a new uuid which I think is the root of it misbehaving. If your model is using a uuid ID, then you'll need to make sure it's present before casting attachments.

AppyCat commented 5 years ago

Thanks, I dropped the UUID for filenaming, and am trying to use scope.id for a unique directory per user. I am working on a test app trying the Addict dep for authentication, bu dropped it in favor of writing custom auth

I get this message when I try to access scope.id

scope is accessible with IO.inspect and shows values, but scope.id throws the following error.

[error] Task #PID<0.532.0> started from #PID<0.516.0> terminating (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available) nil.id() (addict_test) lib/addict_test_web/uploaders/avatar.ex:35: AddictTest.Avatar.storage_dir/2 (arc) lib/arc/storage/local.ex:3: Arc.Storage.Local.put/3 (elixir) lib/task/supervised.ex:89: Task.Supervised.do_apply/2 (elixir) lib/task/supervised.ex:38: Task.Supervised.reply/5 (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3 Function: #Function<1.78569476/0 in Arc.Actions.Store.async_put_version/3> Args: [] [error] Task #PID<0.533.0> started from #PID<0.516.0> terminating (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available) nil.id() (addict_test) lib/addict_test_web/uploaders/avatar.ex:35: AddictTest.Avatar.storage_dir/2 (arc) lib/arc/storage/local.ex:3: Arc.Storage.Local.put/3 (elixir) lib/task/supervised.ex:89: Task.Supervised.do_apply/2 (elixir) lib/task/supervised.ex:38: Task.Supervised.reply/5 (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3 Function: #Function<1.78569476/0 in Arc.Actions.Store.async_put_version/3> Args: [] [error] Ranch protocol #PID<0.516.0> of listener AddictTestWeb.Endpoint.HTTP (connection #PID<0.515.0>, stream id 1) terminated ** (exit) :undef

dbernheisel commented 5 years ago

Here's some example code that works for me. Can you compare and see where the difference may be?


# document.ex
defmodule MyApp.Library.Document do
  @moduledoc """
  Defines how to store documents
  """

  use Arc.Definition
  use Arc.Ecto.Definition

  @acl :public_read

  @allowed_extensions ~w(.doc .docx .xls .xlsx .pdf)
  def allowed_extensions, do: @allowed_extensions

  @doc """
  Overrides Arc.Definition function.

  Validate the file before uploading.

  Accepts {version, {file, scope}}
  Return true/false
  """
  def validate({file, _}) do
    extension = Path.extname(file.file_name)
    Enum.member?(allowed_extensions(), extension)
  end

  @doc """
  Overrides Arc.Definition function.

  Set the uploaded file's filename

  Accepts {version, {file, scope}}
  Return a string of the filename without extension.
  """
  def filename(version, {file, resource}) do
    extension = Path.extname(file.file_name)

    basename =
      file.file_name
      |> Path.basename(extension)
      |> sanitize_filename()

    "#{resource.id}-#{basename}-#{version}"
  end

  @doc """
  Overrides Arc.Definition function.

  Set the uploaded file's directory

  Accepts {version, {file, scope}}
  Return a string of the relative file path from root.
  """
  def storage_dir(_version, {_file, _resource}) do
    "uploads/resources/activities"
  end

  @spec sanitize_filename(String.t()) :: String.t()
  def sanitize_filename(filename) do
    filename |> String.replace(~r/[^A-Za-z0-9]/, "_", global: true)
  end
end

# my schema
defmodule MyApp.Library.Resource do
  @moduledoc """
  Represents resources in the library.
  """

  use Ecto.Schema
  use Arc.Ecto.Schema
  import Ecto.Changeset

  @primary_key {:id, :binary_id, autogenerate: true}

  schema "resources" do
    field(:activity_file, Library.Document.Type)

    timestamps(type: :utc_datetime)
  end

  @required_fields ~w()a
  @optional_fields ~w()a
  @attachment_fields ~w(activity_file)a

  @doc false
  def changeset(struct, attrs) do
    struct
    |> cast(attrs, @required_fields ++ @optional_fields)
    |> validate_required(@required_fields)
    |> ensure_uuid(:id)
    |> cast_attachments(attrs, @attachment_fields)
  end

  @doc "Ensure there's a UUID in the given field"
  @spec ensure_uuid(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t()
  def ensure_uuid(changeset, field) do
    case get_field(changeset, field) do
      nil -> changeset |> put_change(field, Ecto.UUID.generate())
      _ -> changeset
    end
  end
end
AppyCat commented 5 years ago

filename and storage are not working if I override either. Filename change is reflected in the outputted file as version, but in the db its the original file name.

storage can't override as I'm unable to get scope.id

Am using Phoenix 1.4.0, Elixir 1.7.3 and arc_ecto 3.0.1

Thanks for all your help.

AppyCat commented 5 years ago

Here's a wish list if in sync with your project roadmap:

  1. Files delete from directory if you delete the record in the db
  2. Updating an image deletes previously uploaded files for the record in db
  3. Ability to override either of above 2 default settings
  4. Example in docs on creating a new record and getting id to pass for image creation

I tried a new test project using just arc and arc_ecto and filename and storage override not working on Phoenix 1.4.0, Elixir 1.7.3 and latest versions of arc and arc_ecto (from master branch)

If I can be of any help with the above, please let me know.

dbernheisel commented 5 years ago

@appycat it's hard to debug without any of your code.

Thanks for your thoughts on what you'd like from ArcEcto. Heads up, Arc provides the ability to delete your assets from storage. It's your responsibility to manage the deletion of the records, and if it's deleted, then also use Arc to delete the asset.

AppyCat commented 5 years ago

I tried some of your code, but am unable to pass in a scope that can be used within filename or storage override. I did get delete functionality working just fine, very smooth. Here's my code ref the other 2 issues.

defmodule Arco.Photo do use Arc.Definition

use Arc.Ecto.Definition

@versions [:original]

@versions [:original, :thumb]

def validate({file, _}) do ~w(.jpg .jpeg .gif .png) |> Enum.member?(Path.extname(file.file_name)) end

def transform(:thumb, _) do {:convert, "-strip -thumbnail 250x250^ -gravity center -extent 250x250 -format png", :png} end

def filename(version, {file, user}) do extension = Path.extname(file.file_name)

basename =
  file.file_name
  |> Path.basename(extension)
  |> sanitize_filename()

"#{user.id}-#{basename}-#{version}"

end

@spec sanitize_filename(String.t()) :: String.t() def sanitizefilename(filename) do filename |> String.replace(~r/[^A-Za-z0-9]/, "", global: true) end end

And my two controller functions:

CREATE

def create(conn, %{"user" => user_params}) do case Members.create_user(user_params) do {:ok, user} -> conn |> put_flash(:info, "User created successfully.") |> redirect(to: Routes.user_path(conn, :show, user))

  {:error, %Ecto.Changeset{} = changeset} ->
    render(conn, "new.html", changeset: changeset)
end

end

UPDATE

def update(conn, %{"id" => id, "user" => user_params}) do user = Members.get_user!(id)

case Members.update_user(user, user_params) do
  {:ok, user} ->
    conn
    |> put_flash(:info, "User updated successfully.")
    |> redirect(to: Routes.user_path(conn, :show, user))

  {:error, %Ecto.Changeset{} = changeset} ->
    render(conn, "edit.html", user: user, changeset: changeset)
end

end

dbernheisel commented 5 years ago

@appycat and the changeset code? And what error are you getting?

AppyCat commented 5 years ago

Here's the changeset:

defmodule Arco.Members.User do use Ecto.Schema import Ecto.Changeset use Arc.Ecto.Schema

schema "users" do field :photo, Arco.Photo.Type

timestamps()

end

@doc false def changeset(user, attrs) do user |> cast(attrs, [:photo]) |> cast_attachments(attrs, [:photo]) |> validate_required([:photo]) end end

Here's the error code trying to use user.id or scope.id once I set it to user in the controller:

[error] Task #PID<0.883.0> started from #PID<0.877.0> terminating (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available) nil.id() (arco) lib/arco_web/uploaders/photo.ex:23: Arco.Photo.storage_dir/2 (arc) lib/arc/storage/local.ex:3: Arc.Storage.Local.put/3 (elixir) lib/task/supervised.ex:89: Task.Supervised.do_apply/2 (elixir) lib/task/supervised.ex:38: Task.Supervised.reply/5 (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3 Function: #Function<1.78569476/0 in Arc.Actions.Store.async_put_version/3> Args: [] [error] Task #PID<0.882.0> started from #PID<0.877.0> terminating (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available) nil.id() (arco) lib/arco_web/uploaders/photo.ex:23: Arco.Photo.storage_dir/2 (arc) lib/arc/storage/local.ex:3: Arc.Storage.Local.put/3 (elixir) lib/task/supervised.ex:89: Task.Supervised.do_apply/2 (elixir) lib/task/supervised.ex:38: Task.Supervised.reply/5 (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3 Function: #Function<1.78569476/0 in Arc.Actions.Store.async_put_version/3> Args: [] [error] Ranch protocol #PID<0.877.0> of listener ArcoWeb.Endpoint.HTTP (connection #PID<0.876.0>, stream id 1) terminated ** (exit) :undef

dbernheisel commented 5 years ago

@appycat try not casting :photo with cast(), leave that for cast_attachments().

Also, when creating new records, they don't yet have an ID, which you're telling Arc to use to determine how it should be named or stored. You'll notice in my example I have a ensure_uuid(:id) which will have Ecto use that when inserting into the database.

Alternatively you could insert the record first without an attachment, and then immediately update it to add the attachment.

dbernheisel commented 5 years ago

Also, I don't think this is about Ecto 3.0 anymore or a compatibility issue. If you would please close this issue, I'll also be happy to continue debugging here.

AppyCat commented 5 years ago

It works beautifully by simply removing :photo from cast()

Now I just need to figure out how to save it to get the ID before processing arc. Thanks for your help.