stavro / arc_ecto

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

Changeset doesn't track changes. Changes are nil. #73

Closed ospaarmann closed 7 years ago

ospaarmann commented 7 years ago

Hey,

I'm using arc 0.8.0 and arc_ecto 0.7.0. I am encountering a really strange behavior. At some point uploading local files was broken (with a previous version of arc). I decided to upgrade in a hope to fix the issue. I now ran into a different problem: When I pass a path to a local file, a remote file or an Upload Plug into the changeset no changes are being tracked. And it is driving me insane for hours.

I created a new Phoenix app to more easily identify the problem. I can upload the code if that helps.

This is how I try to store an image (for an existing user with the ID 1).

user = Repo.get!(User, 1)
remote_path = "https://static.pexels.com/photos/104827/cat-pet-animal-domestic-104827.jpeg"
user_params = %{avatar: remote_path}
avatar_changeset = User.avatar_changeset(user, user_params)

The resulting changeset looks like this:

#Ecto.Changeset<action: nil, changes: %{}, errors: [], data: #Image.User<>,
 valid?: true>

No changes are tracked. When I on the other hand (with the same setup) upload it without the changeset the file is persisted at S3. So at least the config seems to be correct:

Avatar.store({remote_path, user})

mix.exs

  defp deps do
    [{:phoenix, "~> 1.2.1"},
     {:phoenix_pubsub, "~> 1.0"},
     {:phoenix_ecto, "~> 3.0"},
     {:postgrex, ">= 0.0.0"},
     {:phoenix_html, "~> 2.6"},
     {:phoenix_live_reload, "~> 1.0", only: :dev},
     {:gettext, "~> 0.11"},
     {:cowboy, "~> 1.0"},

     {:arc, "~> 0.8.0"},
     {:arc_ecto, "0.7.0"},
     {:ex_aws, "~> 1.1"}, 
     #{:httpoison, "0.11.0", override: true},  
     {:hackney, "1.6.5"}, 
     {:poison, "~> 2.1"},
     {:sweet_xml, "~> 0.6"}

   ]
  end

dev.exs

use Mix.Config

# For development, we disable any cache and enable
# debugging and code reloading.
#
# The watchers configuration can be used to run external
# watchers to your application. For example, we use it
# with brunch.io to recompile .js and .css sources.
config :image, Image.Endpoint,
  http: [port: 4000],
  debug_errors: true,
  code_reloader: true,
  check_origin: false,
  watchers: []

# Watch static and templates for browser reloading.
config :image, Image.Endpoint,
  live_reload: [
    patterns: [
      ~r{priv/static/.*(js|css|png|jpeg|jpg|gif|svg)$},
      ~r{priv/gettext/.*(po)$},
      ~r{web/views/.*(ex)$},
      ~r{web/templates/.*(eex)$}
    ]
  ]

# Do not include metadata nor timestamps in development logs
config :logger, :console, format: "[$level] $message\n"

# Set a higher stacktrace during development. Avoid configuring such
# in production as building large stacktraces may be expensive.
config :phoenix, :stacktrace_depth, 20

# Configure your database
config :image, Image.Repo,
  adapter: Ecto.Adapters.Postgres,
  username: System.get_env("POSTGRES_USERNAME"),
  password: System.get_env("POSTGRES_PASSWORD"),
  database: System.get_env("POSTGRES_DATABASE_NAME"),
  hostname: System.get_env("POSTGRES_HOSTNAME"),
  pool_size: 10

config :ex_aws,
  access_key_id: [System.get_env("AWS_ACCESS_KEY_ID"), :instance_role],
  secret_access_key: [System.get_env("AWS_SECRET_ACCESS_KEY"), :instance_role],
  s3: [
    scheme: "https://",
    host: "s3.eu-central-1.amazonaws.com",
    region: "eu-central-1"
  ]

config :ex_aws, :hackney_opts,
  recv_timeout: 300_000

config :arc,
  storage: Arc.Storage.S3,
  bucket: System.get_env("AWS_S3_BUCKET"),
  virtual_host: true,
  version_timeout: 15_000

/web/uploaders/avatar.exs


defmodule Image.Avatar do
  @moduledoc """
  Uploader to handle avatars of members (with Arc).
  """

  use Arc.Definition

  # Include ecto support (requires package arc_ecto installed):
  use Arc.Ecto.Definition

  @acl :public_read

  # To add a thumbnail version:
  @versions [:original, :medium, :thumb, :tiny]

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

  # Define a thumbnail transformation:
  def transform(:original, _) do
    {:convert, "-format png -quality 85%", :png}
  end

  def transform(:medium, _) do
    {:convert, "-strip -thumbnail 600x600^ -gravity center -extent 600x600 -format png", :png}
  end

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

  def transform(:tiny, _) do
    {:convert, "-strip -thumbnail 100x100^ -gravity center -extent 100x100 -format png", :png}
  end

  # def __storage, do: Arc.Storage.S3

  # def filename(version, {file, scope}), do: "#{version}-#{file.file_name}"
  def filename(version, _) do
    version
  end

  # Override the storage directory:
  def storage_dir(_, {file, user}) do
    "uploads/test/#{user.id}"
  end

  def s3_object_headers(_version, {file, _scope}) do
    [content_type: Plug.MIME.path(file.file_name)] # for "image.png", would produce: "image/png"
  end

end

/web/models/user.exs

defmodule Image.User do
  use Image.Web, :model
  use Arc.Ecto.Schema

  schema "user" do
    field :name, :string
    field :avatar, Image.Avatar.Type

    timestamps()
  end

  @doc """
  Builds a changeset based on the `struct` and `params`.
  """
  def changeset(user, params \\ :invalid) do
    user
    |> cast(params, [:name])
    |> validate_required([:name])
  end

  # This is the changeset I'm using
  def avatar_changeset(model, params \\ :invalid) do
    model
    |> cast(params, [:name, :id])
    |> cast_attachments(params, [:avatar])
  end
end
ospaarmann commented 7 years ago

I uploaded the phoenix app. You can clone it from here: https://github.com/ospaarmann/arc_bugfix This should help to easily reproduce the problem.

ospaarmann commented 7 years ago

Additionally when I try to create a record with an avatar the scope (record) in the Avatar.storage_dir/2 function is nil. Not the id, but the scope itself. Which is super strange:

iex(5)> changeset = User.changeset(%User{}, %{name: "Klaus", avatar: remote_path})
** (EXIT from #PID<0.391.0>) an exception was raised:
    ** (UndefinedFunctionError) function nil.name/0 is undefined or private
        nil.name()
        (image) web/uploaders/avatar.ex:52: Image.Avatar.storage_dir/2
        lib/arc/storage/s3.ex:6: Arc.Storage.S3.put/3
        (elixir) lib/task/supervised.ex:85: Task.Supervised.do_apply/2
        (elixir) lib/task/supervised.ex:36: Task.Supervised.reply/5
        (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3

Interactive Elixir (1.4.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> [error] Task #PID<0.404.0> started from #PID<0.391.0> terminating
** (UndefinedFunctionError) function nil.name/0 is undefined (module nil is not available)
    nil.name()
    (image) web/uploaders/avatar.ex:52: Image.Avatar.storage_dir/2
    lib/arc/storage/s3.ex:6: Arc.Storage.S3.put/3
    (elixir) lib/task/supervised.ex:85: Task.Supervised.do_apply/2
    (elixir) lib/task/supervised.ex:36: Task.Supervised.reply/5
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
Function: #Function<0.124346436/0 in Arc.Actions.Store.async_put_version/3>
    Args: []
ospaarmann commented 7 years ago

So I seem to have fixed it. It was a bug on my side. But it would be great if arc would produce better error messages. If I have time I'll submit a PR.

In the beginning I forgot the sweet_xml dependency. But arc didn't complain about that. This lead to an error where a src attribute wasn't found in the response by AWS (this is more an arc than an arc_ecto issue though). That means sweet_xml should be in the arc dependencies.

Next the empty changeset. This was stupidity on my side. It only appeared after upgrading and was because I was missing the allow_paths: true option. The correct changeset for using local or remote paths is

cast_attachments(params, ~w(avatar), allow_paths: true)

Then we had the strange Task.await timeout which lead to the process crashing and in turn the model being nil in the storage_dir/2. This was caused by the transform functions. Apparantly the string that I pass into ImageMagick started to produce an error.

The old transform function:

def transform(:medium, _) do
    {:convert, "-strip -thumbnail 600x600^ -gravity center -extent 600x600 -quality 85% -format png", :png}
  end

The new, now working, transform function:

  def transform(:medium, _) do
    {:convert, "-strip -thumbnail 600x600^ -gravity center -extent 600x600 -format png", :png}
  end

Again this was a mistake on my side but it would be nice to get more informative error messages.