`scope` is nil in `storage_dir`

[ndarilek]

I have a Document model like so:

defmodule Scribe.Documents.Document do
  use Ecto.Schema
  use Waffle.Ecto.Schema
  import Ecto.Changeset

  @primary_key {:id, :binary_id, autogenerate: true}
  @foreign_key_type :binary_id
  schema "documents" do
    field :name, :string
    field :original_url, EctoFields.URL
    field :input_format, :string
    field :input_password, :string
    field :input_page_count, :integer
    field :input_storage_key, :string
    field :input_processing_expected_page_count, :integer
    field :input_processing_completed_page_count, :integer
    field :is_pdf_with_text, :boolean, default: false
    field :is_input_password_needed, :boolean, default: false
    field :original, ScribeWeb.Document.Type
    belongs_to :owner, Scribe.Auth.User

    timestamps()
  end

  def changeset(document, params \\ %{}) do
    document
    |> cast(params, [:name, :original_url, :input_format, :input_password, :is_input_password_needed, :input_page_count, :input_storage_key, :is_pdf_with_text, :input_processing_expected_page_count, :input_processing_completed_page_count, :original])
    |> cast_attachments(params, [:original])
  end
end

Users can send us a document either by uploading it directly, or by providing a URL original_url. I have the following in my uploader definition:

  def storage_dir(_version, {_file, scope}) do
    IO.inspect(scope)
    "#{scope.id}"
  end

And my create action, which only handles the remote URL case currently:

  def create(conn, %{"document" => %{"original_url" => original_url} = document_params}) do
    user = conn.assigns[:current_user]
    case Documents.create_document(user, document_params) do
      {:ok, document} ->
        if document.original_url do
          IO.inspect(document)
          {:ok, document} = Documents.update_document(document, %{"original" => original_url})
        end
        conn
        |> put_flash(:info, "Importing document...")
        |> redirect(to: Routes.document_path(conn, :show, document, "html"))
      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

Unfortunately, this returns:

[scribe-7fd9964698-h84wz scribe] ** (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available)

And, indeed, I am getting nil in storage_dir.

I've had the same issues with Arc. I found an issue there claiming that I shouldn't cast my attachment parameter. If I don't, my record gets created, but the attachment field doesn't make it through my changeset function. I also read that my model/scope needs to exist, so to the best of my knowledge, I'm creating it and then passing in the URL after it already exists.

It isn't clear to me from the docs what needs to be stored in my attachment field--:original in this case. Do I actually set it to the URL or file input field? Do I run a Waffle storage function and set it to one of the returned values? I was calling another function--store if I recall correctly--and files were being downloaded from remote URLs and stored in my local Minio setup. But nothing was stored on my attachment column. So I can either download files and not correctly associate them, or fail to download and associate entirely.

Thanks for any assistance.

[achempion]

Hi, thank you for the question. The issue is that you don't have scope defined on creation stage. I'm working towards improving documentation how to properly setup file uploading.

Basically, we have two general approaches:

• generate uuid or any random value and use it with your filename/scope insted of id
• if you want to use an id you have to separate file uploading on two stages within transaction (create a record, update the filename field)

I can see you already separating stages inside the controller, but the issue here is you still stying to handle file upload on the first stage.

You can implement something like this:

def changeset(session_recording, attrs) do
  session_recording
  |> cast(attrs, [:session_id])
  |> validate_required([:session_id])
end

def audio_changeset(recording, attrs) do
  recording
  |> cast_attachments(attrs, [:audio])
  |> validate_required([:audio])
end

Where the :audio attrubute is your file field. Then proceed with a controller (or context):

Ecto.Multi.new()
|> Ecto.Multi.insert(:recording, Recording.changeset(recording, attrs))
|> Ecto.Multi.update(:recording_with_audio, &Recording.audio_changeset(&1.recording, attrs))
|> Repo.transaction()

[ndarilek]

GOt it, so what you're saying is that I can do the `Ecto.Multi` transaction and batch both updates in a single transaction, as long as I use two separate changesets? Also, what content should ultimately end up in my database's file field, assuming I'm using S3 storage? Is it the key under which the file is stored? I'm asking because we're porting a legacy app to Phoenix, so I already have a database, and I'm trying to make sure that our legacy rows have the content Waffle expects to find. Thanks.

[achempion]

Also, what content should ultimately end up in my database's file field,
assuming I'm using S3 storage?

You should have a filename, ideally with v field wich is a timestamp when upload had happened.

image.png?v=63587293261

You can generate timestamp like this:

defp version_url(updated_at, url) do
  stamp = :calendar.datetime_to_gregorian_seconds(NaiveDateTime.to_erl(updated_at))

[achempion]

so what you're saying is that I can do the Ecto.Multi
transaction and batch both updates in a single transaction, as long as I
use two separate changesets?

Exactly. You need two separate stages only if you want to use resourse's id in your filepath.

The first action will create a record in your datebase and on the second stage you can upload your file using record's id within your target path.

[ndarilek]

Got it. Two questions: 1. Is it OK if the filename doesn't have a timestamp? For my use case, the timestamp isn't important. All that is important is the presence or absence of the file. 2. Should the filename be the full S3 storage key? Thanks a bunch.

[ndarilek]

OK, I *do* want to use the resource's ID in my path. Essentially, someone will either upload a file or submit a URL to my app. Say the uploaded file is document.pdf, or the URL downloads a file named document.pdf. In either case, if my created Document model has ID 12345, I want the S3 storage key to be 12345/document.pdf. I want to make sure whatever filename is submitted either via upload or URL is preserved. The only way to preserve the filename and ensure uniqueness is to prepend the model ID to the path. So, given that, does that change how you'd advise me to fix this? Thanks so much for your help, and for taking up maintenance of this library.

[achempion]

1. Is it OK if the filename doesn't have a timestamp? For my use case,
   the timestamp isn't important. All that is important is the presence or
   absence of the file.

It's not required anywhere, just a handy way to work around caching in browsers.

2. Should the filename be the full S3 storage key?

No, only the actual name of the file. Full key will be calculated on the uploading stage by the library.

[ndarilek]

OK, almost there. Here is my document model:

  def changeset(document, params \\ %{}) do
    document
    |> cast(params, [:title, :url, :password, :is_password_needed, :page_count, :expected_page_count, :completed_page_count])
  end

  def file_changeset(document, params \\ %{}) do
    document
    |> cast(params, [:file])
    |> validate_required(:file)
    |> cast_attachments(params, [:file])
  end

My context:

  def create_document(owner, attrs \\ %{})

  def create_document(%User{} = owner, %{"file" => _} = attrs) do
    IO.inspect(attrs)
    IO.puts("Creating with file")
    document = %Document{}
    |> Document.changeset(attrs)
    |> Ecto.Changeset.put_assoc(:owner, owner)
    Ecto.Multi.new()
    |> Ecto.Multi.insert(:document, document)
    |> Ecto.Multi.update(:document_with_file, &Document.file_changeset(&1.document, attrs))
    |> Repo.transaction()
  end

  def create_document(%User{} = owner, attrs) do
    IO.inspect(attrs)
    IO.puts("Creating without file")
    %Document{}
    |> Document.changeset(attrs)
    |> Ecto.Changeset.put_assoc(:owner, owner)
    |> Repo.insert()
  end

And my controller:

  def create(conn, %{"document" => %{"url" => url} = document_params}) do
    user = conn.assigns[:current_user]
    document_params = Map.put(document_params, "file", url)
    case Documents.create_document(user, document_params) do
      {:ok, document} ->
        conn
        |> put_flash(:info, "Importing document...")
        |> redirect(to: Routes.document_path(conn, :show, document, "html"))
      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

"Creating with file" is printed, and the params look correct, but the insert statement is the only one run, and I still get the same error.

What am I getting wrong?

[achempion]

What error exactly you are getting? function nil.id/0 is undefined (module nil is not available)?

[ndarilek]

Sorry, yeah, and `scope` is still `nil` in `storage_dir`.

[ndarilek]

Hmm, maybe I need to set the `allow_paths` and/or the `allow_urls` option as well? I don't see these documented anywhere--found them while digging through the code. My only use case right now is importing remote URLs. I'll try tomorrow when I'm back at work and will report back.

[achempion]

Yeah, allow_urls is exactly what you need if you want to allow fetching remote files. Thank you for pointing out. I'll add documentation about this.

[ndarilek]

OK, after lots and lots of trial and error I've cracked it. Here's what I discovered. Again, I'm just passing in a URL, nothing else. So maybe the URL path isn't very well tested/understood.

The issue is in my changeset. Here is the only changeset that works:

  def file_changeset(document, params \\ %{}) do
    document
    |> cast_attachments(params, [:file], allow_urls: true)
    |> validate_required(:file)
  end

If I do this:

  def file_changeset(document, params \\ %{}) do
    document
    |> cast(params, [:file])
    |> cast_attachments(params, [:file], allow_urls: true)
    |> validate_required(:file)
  end

I get:

[scribe-666dfd977c-z2jsr scribe] ** (UndefinedFunctionError) function nil.id/0 is undefined (module nil is not available) 
[scribe-666dfd977c-z2jsr scribe]     nil.id()
[scribe-666dfd977c-z2jsr scribe]     (scribe) lib/scribe_web/uploaders/document.ex:34: ScribeWeb.Document.storage_dir/2
[scribe-666dfd977c-z2jsr scribe]     (waffle) lib/waffle/storage/s3.ex:6: Waffle.Storage.S3.put/3
[scribe-666dfd977c-z2jsr scribe]     (waffle) lib/waffle/actions/store.ex:86: Waffle.Actions.Store.put_version/3
...

If I do:

  def file_changeset(document, params \\ %{}) do
    document
    |> validate_required(:file)
    |> cast_attachments(params, [:file], allow_urls: true)
  end

I get:

[scribe-666dfd977c-z2jsr scribe]     ** (FunctionClauseError) no function clause matching in Ecto.Changeset.validate_required/3
[scribe-666dfd977c-z2jsr scribe]         (ecto) lib/ecto/changeset.ex:1715: Ecto.Changeset.validate_required(%Scribe.Documents.Document{__meta__: #Ecto.Schema.Metadata<:loaded, "documents">, completed_page_co
unt: nil, expected_page_count: nil, file: nil, id: "25408661-b08f-432b-ad2c-9e2a38e432e6", inserted_at: ~N[2019-12-03 15:37:06], is_password_needed: false, owner: %Scribe.Auth.User{__meta__: #Ecto.Schema.Meta
data<:loaded, "users">, document_settings: #Ecto.Association.NotLoaded<association :document_settings is not loaded>, documents: #Ecto.Association.NotLoaded<association :documents is not loaded>, email: "nola
n@thewordnerd.info", id: "ed90073b-203d-471c-ab09-e87befdb0cee", inserted_at: ~N[2019-12-03 14:20:24], invitation: #Ecto.Association.NotLoaded<association :invitation is not loaded>, page_limit: 0, phone_numb
er: nil, role: :authenticated, settings: #Ecto.Association.NotLoaded<association :settings is not loaded>, updated_at: ~N[2019-12-03 14:20:24]}, owner_id: "ed90073b-203d-471c-ab09-e87befdb0cee", page_count: n
il, password: nil, title: nil, updated_at: ~N[2019-12-03 15:37:06], url: "https://www.ninjakitchen.com/include/pdf/ig-ag302.pdf"}, :file, [])
[scribe-666dfd977c-z2jsr scribe]         (scribe) lib/scribe/documents/document.ex:29: Scribe.Documents.Document.file_changeset/2
[scribe-666dfd977c-z2jsr scribe]         (ecto) lib/ecto/multi.ex:627: anonymous fn/5 in Ecto.Multi.operation_fun/3
[scribe-666dfd977c-z2jsr scribe]         (ecto) lib/ecto/multi.ex:585: Ecto.Multi.apply_operation/5

It'd be great if this could be a little less finnicky. To be fair, your example does put the required validation after the cast_attachments, but I'm not clear why it has to be that way. Also looks like cast isn't necessary as per your example, so that's my mistake. I wonder if it's possible to make that fail in an obvious way, or at least make it a no-op?

Thanks for your help with this!

[ltenzil]

Hi folks, everything worked fine with waffle ecto with local storage, but now after updating from Local to S3 it started throwing the same error. Same code worked fine for Local Storage

def user_changeset(upload, attrs) do
upload
|> cast(attrs, [:user_id])
|> validate_required([:user_id])
end

def file_changeset(upload, attrs) do
upload
|> cast_attachments(attrs, [:file])
|> validate_required([:file])
end

def create_upload(user_id, attrs \ %{}) do
upload = %Upload{user_id: user_id}
Ecto.Multi.new()
|> Ecto.Multi.insert(:upload, Upload.user_changeset(upload, attrs))
|> Ecto.Multi.update(:upload_with_file, &Upload.file_changeset(&1.upload, attrs))
|> Repo.transaction()
end
Failing here on upload.id only for S3
def storage_dir(_, {_file, upload}) do
"uploads/users/#{upload.user_id}/albums/#{upload.id}"
end
`

[achempion]

Could you, please, show the error you are getting?

[ltenzil]

With cast_attachments(attrs, [:file]) and(cast_attachments(attrs, [:file], allow_urls: true)

** (CaseClauseError) no case clause matching: {:error, :upload_with_file, #Ecto.Changeset<action: :update, changes: %{}, errors: [file: {"is invalid", [type: Uploaders.UserUpload.Type, validation: :cast]}], data: #ProfileDetails.Upload<>, valid?: false>, %{upload: %ProfileDetails.Upload{meta: #Ecto.Schema.Metadata<:loaded, "uploads">, approved: false, file: nil, id: 26, inserted_at: ~N[2020-04-03 00:15:34], updated_at: ~N[2020-04-03 00:15:34], user: #Ecto.Association.NotLoaded, user_id: 2}}}

[achempion]

It looks like you might have an invalid file. I would advice to check is the file actually received by controller and passed to the `upload_with_file` function.

[ltenzil]

ok, will check that update you

[ltenzil]

hey i, figured out the issues
1, S3 bucket is read access is blocked
2. I gave acl only for thumb, forgot to add this. "def acl(:original, _), do: :public_read."
Now even without => allow_urls: true, its working fine, Thank you