Active Storage with custom path and validation

rubyonrails-talk
1

Hello, we want to upload a file to an already existing S3 structure and we want to validate the form that the user has uploaded the file otherwise show them the error.

Our code looks like this:

class ObjectController < ApplicationController
  load_resource only: [:create]
  def create
    if csv_is_present? && @object.save
      digest = Digest::MD5.hexdigest("#{@object.parent_id}#{@object.id}#{SALT}")
      @object.file.attach(
        key: "objects/#{@object.parent_id}/#{digest}",
        content_type: "text/csv",
        filename: params[:object][:file].original_filename,
        io: params[:object][:file]
      )
      redirect_to suppressed_numbers_path, notice: "Successfully created a object."
    else
      flash.now[:alert] = "Creation of the object was unsuccessful."
      render :new, status: :unprocessable_entity
    end
  end

  private
  def object_params
    # NOTE: Do not permit file as we are not doing assignment of attributes
    # We should be calling file.attach as this is the only way for us
    # to specify the key of the blob.
    params.require(:object).permit(:parent_id)
  end

  def csv_is_present?
    if params[:object][:file]
      true
    else
      @object.errors.add(:file, "can't be blank")
      false
    end
  end
end

class Object < ActiveRecord::Base
  has_one_attached :file
end

And we don’t like that, we don’t like that the upload and validation is happening in the controller. We could move the upload to the model with an after commit, but because we have

has_one_attached :file active_storage attaches the file before that and we need to have a ‘dummy’ attribute to hold the file from the param. Which looks like this:

class ObjectController < ApplicationController
  load_resource only: [:create]
  def create
    if @object.save
      redirect_to suppressed_numbers_path, notice: "Successfully created a object."
    else
      flash.now[:alert] = "Creation of the object was unsuccessful."
      render :new, status: :unprocessable_entity
    end
  end

  private
  def object_params
    params.require(:object).permit(:parent_id, :file_from_param)
  end
end


class Object < ActiveRecord::Base
  has_one_attached :file
  attribute :file_from_param

  validates :file_from_param, on: [:create], present:  true
  
  after_commit, on: [:create] do
    digest = Digest::MD5.hexdigest("#{parent_id}#{id}#{SALT}")
    file.attach(
      key: "objects/#{parent_id}/#{digest}",
      content_type: "text/csv",
      filename: params[:object][:file].original_filename,
      io: params[:object][:file]
    )
  end
end

But then we have a dummy attribute that is in the model and we need to have an API of that. Why is there no way of telling active_support to not upload the file directly as we can do some work an upload it on our own time. Are we looking at this wrong?

2

I agree with your assessment of the problem. Having a non-standard controller means more need to work harder, which I always take as a sign that I’m going to be shooting myself in the foot.

(side note: I hope that Object is just pseudocode. I doubt sincerely that you can name an ActiveRecord the same as the ur-object in Ruby.)

If I was going to optimize this, I would move the digest bits to a method on the model, or a concern you can bring into the model. Then I might add an attribute to the model to hold the uploaded file, and override the = writer to contain your attach code. My first instinct would be to use the attribute API in the model, but thinking about it more, this might be a case where just adding an attr_accessor would be the better approach.

You can get rid of the csv_is_present by adding a presence validation on your pseudo attribute, too. And allow it in strong parameters.

I don’t see how you get the ID of the object on a create, though. At the time that the digest is being calculated, that number is unknown, and will be nil. So the digest will be correct for a nil id, but once the object is saved, the digest will be wrong.

class Object < ApplicationModel
  has_one_attached :file
  attr_accessor :uploaded_file # (or try `attribute :uploaded_file`)
  validates :uploaded_file, presence: true

  def uploaded_file=(var)
    digest = Digest::MD5.hexdigest("#{parent_id}#{id}#{SALT}")
    self.file.attach(
      key: "objects/#{parent_id}/#{digest}",
      content_type: "text/csv",
      filename: var.original_filename,
      io: var # not 100% sure what the correct place to wire this might be
    )
  end

As you can see, this isn’t a complete solution yet, but hopefully it gives you some ideas where to explore. I don’t use ActiveStorage very much, and prefer Shrine for this sort of thing. There, you have API access to the attacher on the model, and can do a lot of very low-level stuff if you need to without any extra drama.

Walter