Skip to content

Commit

Permalink
Show warning if #filename is safeguarded as in the pre-3.x style
Browse files Browse the repository at this point in the history
Closes #2708
  • Loading branch information
mshibuya committed Jan 29, 2024
1 parent d27c987 commit 127ccd5
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
26 changes: 26 additions & 0 deletions lib/carrierwave/uploader/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ def initialize(*)
@file, @filename, @cache_id, @identifier, @deduplication_index = nil
end
}

after :store, :show_warning_when_filename_is_unavailable

class_attribute :filename_safeguard_checked
end

module ClassMethods
private

def inherited(subclass)
# To perform the filename safeguard check once per a class
self.filename_safeguard_checked = false
super
end
end

##
Expand Down Expand Up @@ -133,6 +147,18 @@ def full_filename(for_file)
forcing_extension(for_file)
end

def show_warning_when_filename_is_unavailable(_)
return if self.class.filename_safeguard_checked
self.class.filename_safeguard_checked = true
return if filename

warn <<~MESSAGE
[WARNING] Your uploader's #filename method defined at #{method(:filename).source_location.join(':')} didn't return value after storing the file.
It's likely that the method is safeguarded with `if original_filename`, which were necessary for pre-3.x CarrierWave but is no longer needed.
Removing it is recommended, as it is known to cause issues depending on the use case: https://github.com/carrierwaveuploader/carrierwave/issues/2708
MESSAGE
end

def storage
@storage ||= self.class.storage.new(self)
end
Expand Down
18 changes: 18 additions & 0 deletions spec/uploader/store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,24 @@ def filename; "foo.jpg"; end
end
end

context "with a filename safeguarded by 'if original_filename'" do
before do
@uploader_class.class_eval do
def filename
"foo.jpg" if original_filename
end
end
end

it "shows warning on store only once" do
expect(@uploader).to receive(:warn).with(/Your uploader's #filename method .+ didn't return value/).once
@file = File.open(file_path('test.jpg'))
@uploader.store!(@file)
@file = File.open(file_path('bork.txt'))
@uploader.store!(@file)
end
end

describe 'without a store dir' do
before do
@uploader_class.class_eval do
Expand Down

0 comments on commit 127ccd5

Please sign in to comment.