The code depends on @original_filename doesn't work since v3.0.4

[y-yagi]

The uploader.rb generated by generators has an example of how to override the filename. The code is the following.

carrierwave/lib/generators/templates/uploader.rb.erb

Lines 42 to 46 in 0c50a8b

	# Override the filename of the uploaded files:
	# Avoid using model.id or version_name here, see uploader/store.rb for details.
	# def filename
	# "something.jpg" if original_filename
	# end

But, I think this code doesn't work since v3.0.4 because @original_filename is set to nil after store by #2707.

So that example always returns nil now. The reproduction script is the following.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "activerecord"
  gem "sqlite3"
  # gem "carrierwave", "3.0.3"
  gem "carrierwave", "3.0.4"
end

require "active_record"
require "carrierwave"
require "carrierwave/orm/activerecord"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(File::NULL)

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :name
    t.string :avatar
  end
end

class AvatarUploader < CarrierWave::Uploader::Base
  storage :file

  def filename
    "sanitized_filename.txt" if original_filename
  end
end

class User < ActiveRecord::Base
  mount_uploader :avatar, AvatarUploader
end

class BugTest < Minitest::Test
  def test_filename
    u = User.new
    File.open("test.txt") { |f| u.avatar = f }
    u.save!
    assert_equal "sanitized_filename.txt", u.avatar.filename
  end
end

$ ruby bug_report_carrierwave.rb
# Running:

F

Finished in 0.008341s, 119.8938 runs/s, 119.8938 assertions/s.

  1) Failure:
BugTest#test_filename [bug_report_carrierwave.rb:48]:
Expected: "sanitized_filename.txt"
  Actual: nil

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

Is this an intended change?

[RachalCassity]

Also facing this same issue.

In previous versions, the original filename would be stored when called uploader.store!(file). Going through each line, the
only change was when store! was called.

I noticed this commit reassigns the original_filename.

Here are the two uploader outputs after calling uploader.store!(file):

carrier wave 3.0.3:
uploader
=> #<Uploader:0x000000014bb1da90
 @cache_id=nil,
 @cache_storage=
  #<CarrierWave::Storage::File:0x000000014be92c48
   @cache_called=nil,
   @uploader=#<Uploader:0x000000014bb1da90 ...>>,
 @deduplication_index=nil,
 @file=
  #<CarrierWave::SanitizedFile:0x000000011b0d3ad8
   @content=nil,
   @content_type=nil,
   @declared_content_type="application/pdf",
   @file=
    "....pdf",
   @original_filename=nil>,
 @filename="extras.pdf",
 @guid="ec45bd9e-4497-43ef-8bd3-7c578e127485",
 @identifier="extras.pdf",
 @model="ec45bd9e-4497-43ef-8bd3-7c578e127485",
 @mounted_as=nil,
 @original_filename="....pdf",
 @staged=false,
 @storage=
  #<CarrierWave::Storage::File:0x000000014be914b0
   @cache_called=nil,
   @uploader=#<Uploader:0x000000014bb1da90 ...>>,
 @versions={}>

carrierwave 3.0.4:

uploader
=> #<Uploader:0x00000001595fe920
 @cache_id=nil,
 @cache_storage=
  #<CarrierWave::Storage::File:0x0000000159c14300
   @cache_called=nil,
   @uploader=#<Uploader:0x00000001595fe920 ...>>,
 @deduplication_index=nil,
 @file=
  #<CarrierWave::SanitizedFile:0x000000011faad668
   @content=nil,
   @content_type=nil,
   @declared_content_type="application/pdf",
   @file=
    "......pdf",
   @original_filename=nil>,
 @filename=".pdf",
 @guid="d0d65670-d87a-4a92-8d70-fe5d948e9741",
 @identifier=".pdf",
 @model="d0d65670-d87a-4a92-8d70-fe5d948e9741",
 @mounted_as=nil,
 @original_filename=nil,
 @staged=false,
 @storage=
  #<CarrierWave::Storage::File:0x0000000159c12b40
   @cache_called=nil,
   @uploader=#<tUploader:0x00000001595fe920 ...>>,
 @versions={}>

[mshibuya]

First, the example implementation

  def filename
    "sanitized_filename.txt" if original_filename
  end

is safeguarded with if original_filename, but it's just an workaround for the old issue #1898 and no longer necessary in CarrierWave 3.x. If you remove the conditional, #filename will always return a value.

Also I want to understand the real use case of that #filename call. What #filename stands for is not the actual name that the stored file has, but the name you want it to be named. For example, when you have

  def filename
    "#{Time.now.to_i}.jpg"
  end

the return value changes time to time and does not reflect the stored file's name. So it's not a stable way to get the filename.
For what purpose do you use the value of #filename ?

[rajyan]

There is a note about changes in original_filename here
https://github.com/carrierwaveuploader/carrierwave/blob/master/README.md#changing-the-filename

and original_filename isn't the original filename as explained in #2710 (comment)

Although, it is still used in the generator,

carrierwave/lib/generators/templates/uploader.rb.erb

Lines 42 to 46 in 0c50a8b

	# Override the filename of the uploaded files:
	# Avoid using model.id or version_name here, see uploader/store.rb for details.
	# def filename
	# "something.jpg" if original_filename
	# end

and cfaf173 was definitely a breaking change...
I'm looking for a way to fix it without breaking the previous behavior in #2710.

[y-yagi]

I'm not sure about the detailed history, the code generated by the generator is wrong now, right?

carrierwave/lib/generators/templates/uploader.rb.erb

Lines 42 to 46 in 0c50a8b

	# Override the filename of the uploaded files:
	# Avoid using model.id or version_name here, see uploader/store.rb for details.
	# def filename
	# "something.jpg" if original_filename
	# end

If so, I'm OK with fixing our application code. But it looks like this is a breaking change and introducing a breaking change in a newer minor version violates semantic versioning. I think it's good to support old behavior at least in a minor update, what do you think?

[mshibuya]

cfaf173 was definitely a breaking change...

To tell if it actually is or not, it is important to understand the real use case. It can be either a behavior that should be considered as a specification, or that you're just depending on what happened to work in the past.

[mshibuya]

As many people (including this) seems to be struggling, I got an idea.
The problem is the safeguard if original_filename within #filename so if someone seems to have it we should be able to instruct them to remove it, in the form of warning. I can open a PR.