Merge pull request #2718 from carrierwaveuploader/warn-if-filename-is…

…-safeguarded

Show warning if #filename is safeguarded as in the pre-3.x style

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

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