Not working with not-yet-persisted blobs

[marckohlbrugge]

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:

active_storage_validations/lib/active_storage_validations/metadata.rb

Lines 74 to 84 in dd835e6

	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]

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?

[northeastprince]

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

[Mth0158]

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

[Mth0158]

Hi @marckohlbrugge (again !),

I think that we could handle this case soon. I am currently refactoring the metadata.rb file into several pieces following Rails ActiveStorage::Analyzer files (see example for ImageMagick here: https://github.com/rails/rails/blob/main/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb).

Then I can add the corresponding tests to your case. IMO it should work because we will pass an attachable to the analyzer (instead of a blob for Rails analyzers), therefore we will have access to its io.

[Mth0158]

Hi @marckohlbrugge,
While doing the #254 refacto, I do not get why your code is not working. You are doing the below:

attach(io: tempfile, filename: tempfile.original_filename, content_type: tempfile.content_type)

You are therefore not passing a blob to ActiveStorage (and therefore our validators) but a Hash with io (File) / content_type / filename. The code will instead go through this method:

def read_file_path
      case file
      when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile
        ...
      when Hash
        io = file.fetch(:io)
        if io.is_a?(StringIO)
          tempfile = Tempfile.new([File.basename(file[:filename], '.*'), File.extname(file[:filename])])
          tempfile.binmode
          IO.copy_stream(io, tempfile)
          io.rewind
          tempfile.flush
          tempfile.rewind
          tempfile.path
        else
          File.open(io).path
        end
      else
       ...
      end
    end

It should then not try to download the blob io IMO. Are you sure the error is raised by our code?

[Mth0158]

Hi @marckohlbrugge,
Long time no see, we have released a v2 for the gem, are you able to upgrade your project and tell us if the issue remains?
I tried in the last comment to reproduce your issue but I was not successful :/