diff --git a/lib/carrierwave/uploader/store.rb b/lib/carrierwave/uploader/store.rb index 55d0c0640..bc86cb41c 100644 --- a/lib/carrierwave/uploader/store.rb +++ b/lib/carrierwave/uploader/store.rb @@ -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 ## @@ -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 diff --git a/spec/uploader/store_spec.rb b/spec/uploader/store_spec.rb index a3bdca975..819f70cf3 100644 --- a/spec/uploader/store_spec.rb +++ b/spec/uploader/store_spec.rb @@ -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