Immediately receiving attachments after attaching them with ActiveStorage

I’m a long time user of Rails and Ruby but this one has stumped me. It seems to me that it’d be a common use case to want to use an attached file straight after attaching it to a model. In my case I have a Page model that has images as Active Storage attachments. The attachment paths are used inside the HTML of the page, so I need to return a rails_blob_path to the image in the json response as soon as the file is attached.

I’m able to hack around the problem like so:

page.images.attach params[:file]
image = page.images.last

render json: { link: rails_blob_path(image), 'data-signed-id': image.signed_id }

This seems brittle as potentially .last may not mean the file that I just attached. attach just returns true.

I thought of first creating the blob unbound to the page and then attaching it, but couldn’t quite figure that out.

Any help would be greatly appreciated :slight_smile:

2 Likes

@Spike Good question, I feel like I might be missing some context here but I wonder if page.images.attach isn’t the right interface. I think ActiveStorage provides some Model level features so you could use that to return the attached image instead of using .last. :thinking:

You’re right @zzak, it does feel like the wrong interface. I thought of something like:

image = page.images.create params[:file]

but that doesn’t work.

Ok, looking at the docs that actually might be the right API.

Judging by the API reference for attach, it looks like it might actually return the image you need:

Sadly it doesn’t. It just returns the result of the save (so true or false):

def attach(*attachables)
  if record.persisted? && !record.changed?
    record.public_send("#{name}=", blobs + attachables.flatten)
    record.save
  else
    record.public_send("#{name}=", (change&.attachables || blobs) + attachables.flatten)
  end
end

Or in the case that the record is new, I guess it returns the attachments? Looks like the return value isn’t really defined for this method anyway. Perhaps it should explicitly return the attachments? (as in it should be changed to do this?)

@Spike Could you do something like:

image = page.images.new params[:file]
page.save # or image.save

render json: { link: rails_blob_path(image), ... }

Unfortunately it’s not that simple. Assigning to images is quite magical:

        generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
          # frozen_string_literal: true
          def #{name}
            @active_storage_attached ||= {}
            @active_storage_attached[:#{name}] ||= ActiveStorage::Attached::Many.new("#{name}", self)
          end

          def #{name}=(attachables)
            if ActiveStorage.replace_on_assign_to_many
              attachment_changes["#{name}"] =
                if Array(attachables).none?
                  ActiveStorage::Attached::Changes::DeleteMany.new("#{name}", self)
                else
                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, attachables)
                end
            else
              if Array(attachables).any?
                attachment_changes["#{name}"] =
                  ActiveStorage::Attached::Changes::CreateMany.new("#{name}", self, #{name}.blobs + attachables)
              end
            end
          end
        CODE

#{name} is images in my example. There’s a lot going on there including reassigning existing blobs so that they don’t get dropped when further attachments are added.

The more I look at this, the more I think this might not have been thought of. I assume the idea of the has_many is to allow dumping files in, then just being able to loop over them as a homogenous group later on (without regard for the significance of any particular one).

The way around this (that I don’t want to do) is to introduce an intermediate model between Page and the attachments that is the image and the image has_one_attached :file, but that’s just weird and redundant :smiley:

I really appreciate your help on this so far @zzak :slight_smile:

@Spike I found this issue which is interesting, they make it seem like it does indeed return the attachment: https://github.com/rails/rails/issues/33784

Btw, you’re using has_many_attached, right?

That’s a super interesting issue :slight_smile: I suspect it may have been the case in the past, but perhaps this has changed unintentionally, or this was never a documented return?

I’m definitely using has_many_attached :images.

In the attach method, if the parent record is persisted and unchanged it’ll return the result of record.save which will be true or false. Otherwise, it returns an instance of ActiveStorage::Attached::Changes::CreateMany (I tried marking the pages name as dirty to trigger this).

This could all be tidied up, but the main issue will be the fact that the parent record’s existing file blobs are merged with the ones being added: blobs + attachables.flatten and so the method that persists these has no way of knowing what the new ones are (other than maybe keeping a track of which ones aren’t blobs already and returning those?).

I wonder if this should be raised as an issue in GitHub?

I’ve managed to figure out how to do it, though it feels a bit messy:

image = ActiveStorage::Blob.create_and_upload!(
  io: params[:file],
  filename: params[:file].original_filename,
  content_type: params[:file].content_type
)

page.images.attach image

render json: { link: rails_blob_path(image), 'data-signed-id': image.signed_id }

Props to this article: Manual uploads using Active Storage | by Mirko Akov | Evermore | Medium

1 Like

@Spike I’m glad you found a workaround for the issue. I’m not sure yet if there is a better way to do this in Rails – but open to suggestions. :bow:

Thanks @zzak, It’s working pretty well for my purposes. When I save the Page I trawl it with Nokogiri for those data-signed-id attributes and add/remove the blobs to the page model then. Removed blobs will be purged later on by a cron task :slight_smile:

1 Like

Thanks here too, still relavant in 2023

All good @dschnell, I can’t remember if I mentioned but Rails does something similar with ActionText. This is my final code for linking attachments. I use Pundit to do security on blob fetches so I have to just allow full access to unattached blobs. Once they’re attached I can check the thing they’re attached to to see if the user can access the file.

  def assign_attachments
    signed_ids = Set.new

    body.scan %r{/storage/(?:blobs|representations)/redirect/([^/.?]+)} do |(signed_id)|
      signed_ids.add signed_id
    end

    self.attachments = signed_ids.filter_map do |signed_id|
      ActiveStorage::Blob.find_signed signed_id
    end
  end

I have been struggling for hours before finding this post (and I have 10+ years with Rails).

The way attach works is not intuitive and the solutions proposed here, unfortunately, are just workarounds that introduce more issues (e.g. validations of the attachment are skipped when you call directly ActiveStorage::Blob.create_and_upload!).

I think that it would be really useful to have a new append method that:

  1. Appends a new attachment (e.g. @image = @page.images.append params[:image])
  2. If it is a success, it returns only the new attachment
  3. If it is a failure, it returns nil or false

@Spike Your solution also introduces a possible security issue (when the call to attach fails, for example due to a validation error): users are free to upload arbitrary files that are stored on your server, but you don’t have any reference to those files and to the users that have uploaded them. Even if you delete the page or the user later the files will remain forever on your storage and can also increase your costs.

Hi @collimarco, I will have a database record pointing to the blob if I’m not mistaken? You’re correct in that that will stick around unattached for a time, but I have a garbage collector cron task that deletes unattached blobs that are more than 7 days old as recommended in the guide: Active Storage Overview — Ruby on Rails Guides

1 Like

@Spike Oh, that makes sense!