Trigger before and after callbacks when Attachments are uploaded.

[rapito]

Triggers Callbacks when an attachment has been uploaded

Motivation / Background

This Pull Request has been created because during our migration process from paperclip to ActiveStorage we couldn't find any straightforward mechanism to notify our system exactly when the attachment was uploaded.

Our use case was very particular but long story short, we needed to run a process that validated some uploaded files after they were attached.

The lack of this feature made this effort extremely painful as we needed to execute several rake tasks to re-process hundreds of files that were uploaded incorrectly.

Detail

This Pull Request changes ActiveStorage::Blob#upload_without_unfurling and adds some helpers to ActiveStorage::Attachment to invoke a specific method :before and :after uploading to the configured service.
This is achieved by checking if the active record model respond_to? a method with the following signature pattern:

• before_{attachment_name}_attached or after_{attachment_name}_attached
• before_{attachment_name}_variant_attached or after_{attachment_name}_variant_attached

It also passes the processed blob object to the method in order to do any required post-processing on it.

Additional information

The development of this feature started during RailsConf 2024 during the HackDay Workshop with the Rails Core Team.
Contributors and participating members of this process were:

• @tenderlove
• @mattboldt
• @garyhtou

(Thanks for the help!, great working with you! 🙌🏻 )

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[MatheusRich]

IMHO this feels different from normal AR callbacks. Usually we'd have something like

class Message < ApplicationRecord
  has_one_attached :image do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]
  end

  before_image_attached do |image_blob|
    # do stuff
  end
end

Minor change, but it feels more similar to what we already have with Active Record callbacks. Thoughts?

[rapito]

IMHO this feels different from normal AR callbacks. Usually we'd have something like

class Message < ApplicationRecord
  has_one_attached :image do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]
  end

  before_image_attached do |image_blob|
    # do stuff
  end
end

Minor change, but it feels more similar to what we already have with Active Record callbacks. Thoughts?

Thanks @MatheusRich, for sure! I completely agree with you. And this was the original intent.
Actually that was the first iteration, the problem however was that ar callbacks would not allow us to yield parameters back to it.

We would have to add that ability to ar callbacks from the ground up and in the end we decided against it and follow this simpler approach.

[zzak]

This isn't a fully baked thought, but what about something like:

before_image_attached :do_stuff

def do_stuff(blob)
  # do stuff with blob
end

[rapito]

This isn't a fully baked thought, but what about something like:

before_image_attached :do_stuff

def do_stuff(blob)
  # do stuff with blob
end

Hi! That is the same proposition on the first comment, please read my answer to it above.

[mattboldt]

Great work here! Echoing Robert's earlier message: normal AM callbacks don't allow us to pass an argument. My assumption is that AM callbacks expect to be idempotent, and that all information is available on self. However, if you has_many_attached, and you want to do some processing on the specific image that was just attached, that info is harder to get on self alone. You could query the most recently added attachment, but that could cause race conditions.

I wonder if it would help prevent confusion by differentiating it from the usual ActiveModel Callback pattern. Something like...

class Message < ApplicationRecord
  # with a lambda
  has_one_attached :image, after_attached: ->(blob) { puts "image attached" } do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]
  end

  # OR, with a method name
  has_one_attached :image, after_attached: :example_after_attached do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]
  end

  def example_after_attached(blob)
  end
end

Now it's more like a part of has_one_attached's API rather than trying to mimic the ActiveModel Callback API. You could still send an argument to it like you have here so you get all the same functionality, and the user gets to customize the method name (or use a lambda).

However, if folks aren't concerned about this, I think your solution looks good as-is.

[rapito]

I actually love your suggestion Matt, it makes a lot of sense to me and feels more explicit (and makes the behavior more readable and deterministic).

I don't have a strong opinion about leaving it as-is or using the new approach. I'm fine with whichever.

[natematykiewicz]

That last suggestion feels a lot more like normal ActiveRecord. I like the flexibility of it compared to a magically named method that you have to constantly do respond_to? checks for.

One suggestion I was thinking that might be a little more ergonomic than a hash argument in a method that also expects a block, is to add 2 methods to ActiveStorage::Reflection::HasAttachedReflection.

class Message < ApplicationRecord
  # with a lambda
  has_one_attached :image do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]

    attachable.after_attached do |blob|
      puts "image attached"
    end
  end

  # OR, with a method name
  has_one_attached :image do |attachable|
    attachable.variant :thumb, resize_to_limit: [100, 100]

    attachable.before_attached :example_before_attached
  end

  def example_before_attached(blob)
  end
end

[natematykiewicz]

It'd also be more performant to have to actually specify at the time of defining the has_one_attached that there are callbacks, than to always attempt to run callbacks only to find that none are defined.