Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not working with not-yet-persisted blobs #242

Open
marckohlbrugge opened this issue Feb 12, 2024 · 3 comments
Open

Not working with not-yet-persisted blobs #242

marckohlbrugge opened this issue Feb 12, 2024 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@marckohlbrugge
Copy link

I use the following to allow attaching of remote files:

module ActiveStorage
  module RemoteURLHelper
    # If the record is persisted and unchanged, the attachments are saved to
    # the database immediately. Otherwise, they'll be saved to the DB when the
    # record is next saved.

    def attach_from_url(url, max_size: nil)
      tempfile = Down.download(url, max_size: max_size)
      attach(io: tempfile, filename: tempfile.original_filename, content_type: tempfile.content_type)
    end
  end
end

Rails.application.config.after_initialize do
  ActiveStorage::Attached::One.include(ActiveStorage::RemoteURLHelper)
  ActiveStorage::Attached::Many.include(ActiveStorage::RemoteURLHelper)
end

This allows me to do things like:

gallery.photos.attach_from_url "https://example.com/photo1.png"
gallery.photos.attach_from_url "https://example.com/photo2.png"
gallery.photos.attach_from_url "https://example.com/photo3.png"
gallery.save!

This works as expected, but active_storage_validations 1.1.4 does not seem compatible with this approach.

The reason is that the above won't upload the photos to the cloud storage provider yet. So when we call gallery.save! and it runs the validations, we get an ActiveStorage::FileNotFoundError (ActiveStorage::FileNotFoundError) error in activestorage-7.1.3/lib/active_storage/service/s3_service.rb:159:in stream: ActiveStorage::FileNotFoundError

After a little digging, I found out the reason for this:

tempfile = Tempfile.new(["ActiveStorage-#{blob.id}-", blob.filename.extension_with_delimiter])
tempfile.binmode
blob.download do |chunk|
tempfile.write(chunk)
end
tempfile.flush
tempfile.rewind
image = new_image_from_path(tempfile.path)

This code assumes the blob is already saved and uploaded. However, with the above code example, the blob will not be persisted yet. So blob.id will be nil and blob.download raises the exception.

Here's a relevant code comment from activestorage/lib/active_storage/attached/many.rb and activestorage/lib/active_storage/attached/one.rb's attach(…) methods:

    # If the record is persisted and unchanged, the attachments are saved to
    # the database immediately. Otherwise, they'll be saved to the DB when the
    # record is next saved.
    def attach(…)

So it seems to be that approach from my ActiveStorage::RemoteURLHelper mixin is indeed a valid use of the ActiveStorage API's, but active_storage_validations is currently incompatible with the scenario where the blobs aren't persisted yet.

Unfortunately, I don't see an easy solution as ActiveStorageValidations::Metadata is only passed the file (a blob in this case), and an unsaved blob, as far as I know, does have no knowledge of its io as that's typically passed in externally right before it gets uploaded.

The proper approach might be to not pass the blob, but the ActiveStorage::Attached::One / ActiveStorage::Attached::Many instead.

@Mth0158
Copy link
Collaborator

Mth0158 commented Feb 15, 2024

Hi @marckohlbrugge

I read your issue, tried to explore it a bit, this one is not an easy one and requires further knowledge in ActiveStorage that I currently do not have. I'll explore the ActiveStorage API in the coming days to be able to offer a right approach.
Maybe @igorkasyanchuk can help you on that one? But unsure since he is quite busy these days.

Do you have the ability + time to make a PR on that topic?

@Mth0158 Mth0158 added bug Something isn't working help wanted Extra attention is needed labels Feb 15, 2024
@northeastprince
Copy link

I recently started encountering this issue as well after a routine Rails update.

@Mth0158
Copy link
Collaborator

Mth0158 commented Mar 12, 2024

@northeastprince could you tell us which version of Rails did you update from/to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants