Shrine: Retain original filename with version

[Grafikart]

My issue looks like https://github.com/janko-m/shrine/issues/195 but it's a bit different. I want to keep the original filename when generating the location

uploads/
   original_filename.jpg
   original_filename_thumb.jpg
   original_filename_medium.jpg

With the version plugin the original filename is not persisted and path looks like that

uploads/
   original_filename.jpg
   minimagick_UID_thumb.jpg
   minimagick_UID_medium.jpg

The extract_filename(io).to_s does not return the original filename when working on versions. My current workaround is doing something like that.

  def generate_location(io, context = {})
    if [:original, nil].include? context[:version]
      @filename = File.basename(extract_filename(io).to_s, '.*')
    end
    extension   = ".#{io.extension}" if io.is_a?(UploadedFile) && io.extension
    extension ||= File.extname(extract_filename(io).to_s).downcase
    version = context[:version] && context[:version] == :original ? "" : "_#{context[:version]}"
    "#{context[:record].class.name.downcase.pluralize}/#{@filename}#{version}#{extension}"
  end

I persist the filename when processing the original filename but I would like to know if you have a better version for this. If there isn't and you think this problem is a "edge case" feel free to close this issue.

[hmistry]

extract_filename() method gets the original_filename if io responds to that method or returns the io base filename. For versions, the file is a new temp file copy so its filename will have a unique generated string. I don't think io object holds any info on original filenames. For that we have to look at Shrine's stored metadata in the record.

Here's a solution I came up with:

  def generate_location(io, context)
    if [:small, :thumb].include? context[:version]
      file_data = JSON.parse context.dig(:record).file_data
      file_ext = File.extname(file_data.dig("metadata", "filename"))
      file_basename = File.basename(file_data.dig("metadata", "filename"), file_ext)
      location = file_basename + "_" + context[:version].to_s + file_ext
    else
      file_ext = File.extname(context.dig(:metadata, "filename"))
      file_basename = File.basename(context.dig(:metadata, "filename"), file_ext)
      location = file_basename + file_ext
    end
    context[:metadata]["filename"] = location
    location
  end

[janko]

@hmistry I think you were definitely on the right track, the solution would be to retrieve the original filename from cached file. But I think it could be simplified a bit (this example is assuming your attachment attribute is named #file):

def generate_location(io, context)
  original_filename = context[:record]&.file&.original_filename || context[:metadata]["filename"]
  version_suffix    = "_#{context[:version]}" if context[:version] && context[:version] != :original
  basename          = File.basename(original_filename, ".*")
  extension         = File.extname(original_filename).downcase

  "#{context[:record].class.name.downcase.pluralize}/#{basename}#{version}#{extension}"
end

EDIT: One trick that's worth mentioning here is fetching original filename that was already extracted from the current io from context[:metadata].

[janko]

@Grafikart But note that you probably want to at least add the record ID into the location, to allow users to upload files that have the same filename. Currently any new upload that has the same filename as an existing uploaded file will overwrite it.

[goelinsights]

Thanks for the detailed explanation! It works well between filename and custom generated location to create a location that maps to both multi-tenant needs and carrierwave migration.