Merge pull request #2659 from carrierwaveuploader/use-converted-file-extension

[BREAKING CHANGE] Use the resulting file extension on changing format by :convert

Author: mshibuya

README.md

@@ -35,6 +35,15 @@ gem 'carrierwave', '>= 3.0.0.beta', '< 4.0'
 
 Finally, restart the server to apply the changes.
 
+## Upgrading from 2.x or earlier
+
+CarrierWave 3.0 comes with a change in the way of handling the file extension on conversion. This results in following issues if you use `process convert: :format` to change the file format:
+
+- If you have it on the uploader itself (not within a version), the file extension of the cached file will change. That means if you serve both CarrierWave 2.x and 3.x simultaneously on the same workload (e.g. using blue-green deployment), a cache file stored by 2.x can't be retrieved by 3.x and vice versa.
+- If you have it within a version, the file extension of the stored file will change. You need to perform `#recreate_versions!` to make it usable again.
+
+To preserve the 2.x behavior, you can set `force_extension false` right after calling `process convert: :format`. See [#2659](https://github.com/carrierwaveuploader/carrierwave/pull/2659) for the detail.
+
 ## Getting Started
 
 Start off by generating an uploader:
@@ -224,6 +233,20 @@ class MyUploader < CarrierWave::Uploader::Base
 end
 ```
 
+## Changing the filename
+
+To change the filename of uploaded files, you can override `#filename` method in the uploader.
+
+```ruby
+class MyUploader < CarrierWave::Uploader::Base
+  def filename
+    "image.#{file.extension}" # If you upload 'file.jpg', you'll get 'image.jpg'
+  end
+end
+```
+
+Some old documentations (like [this](https://stackoverflow.com/a/5865117)) may instruct you to safeguard the filename value with `if original_filename`, but it's no longer necessary with CarrierWave 3.0 or later.
+
 ## Securing uploads
 
 Certain files might be dangerous if uploaded to the wrong location, such as PHP
@@ -302,17 +325,8 @@ You no longer need to do this manually.
 
 ## Adding versions
 
-Often you'll want to add different versions of the same file. The classic example is image thumbnails. There is built in support for this*:
-
-*Note:* You must have Imagemagick installed to do image resizing.
-
-Some documentation refers to RMagick instead of MiniMagick but MiniMagick is recommended.
-
-To install Imagemagick on OSX with homebrew type the following:
-
-```
-$ brew install imagemagick
-```
+Often you'll want to add different versions of the same file. The classic example is generating image thumbnails while preserving the original file to be used for high-quality representation.
+In this section we'll explore how CarrierWave supports working with multiple versions. The image manipulation itself is covered in [another section](#manipulating-images).
 
 ```ruby
 class MyUploader < CarrierWave::Uploader::Base
@@ -348,17 +362,7 @@ uploader.thumb.url # => '/url/to/thumb_my_file.png'   # size: 200x200
 One important thing to remember is that process is called *before* versions are
 created. This can cut down on processing cost.
 
-### Processing Methods: mini_magick
-
-- `convert` - Changes the image encoding format to the given format, eg. jpg
-- `resize_to_limit` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. Will only resize the image if it is larger than the specified dimensions. The resulting image may be shorter or narrower than specified in the smaller dimension but will not be larger than the specified values.
-- `resize_to_fit` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. The image may be shorter or narrower than specified in the smaller dimension but will not be larger than the specified values.
-- `resize_to_fill` - Resize the image to fit within the specified dimensions while retaining the aspect ratio of the original image. If necessary, crop the image in the larger dimension. Optionally, a "gravity" may be specified, for example "Center", or "NorthEast".
-- `resize_and_pad` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. If necessary, will pad the remaining area with the given color, which defaults to transparent (for gif and png, white for jpeg). Optionally, a "gravity" may be specified, as above.
-
-See `carrierwave/processing/mini_magick.rb` for details.
-
-### conditional process
+### Conditional processing
 
 If you want to use conditional process, you can only use `if` statement.
 
@@ -442,8 +446,27 @@ class MyUploader < CarrierWave::Uploader::Base
 end
 ```
 
-The option `:from_version` uses the file cached in the `:thumb` version instead
-of the original version, potentially resulting in faster processing.
+### Customizing version filenames
+
+CarrierWave supports [customization of filename](#changing-the-filename) by overriding an uploader's
+#filename method, but this doesn't work for versions because of the limitation on how CarrierWave
+re-constructs the filename on retrieval of the stored file.
+Instead, you can override `#full_filename` with providing a version-aware name.
+
+```ruby
+class MyUploader < CarrierWave::Uploader::Base
+  version :thumb do
+    def full_filename(for_file)
+      'thumb.png'
+    end
+    process convert: 'png'
+  end
+end
+```
+
+Please note that `#full_filename` mustn't be constructed based on a dynamic value
+that can change from the time of store and time of retrieval, since it will result in
+being unable to retrieve a file previously stored.
 
 ## Making uploads work across form redisplays
 
@@ -911,67 +934,81 @@ CarrierWave.configure do |config|
 end
 ```
 
-## Using RMagick
+## Manipulating images
 
 If you're uploading images, you'll probably want to manipulate them in some way,
-you might want to create thumbnail images for example. CarrierWave comes with a
-small library to make manipulating images with RMagick easier, you'll need to
-include it in your Uploader:
+you might want to create thumbnail images for example.
 
-```ruby
-class AvatarUploader < CarrierWave::Uploader::Base
-  include CarrierWave::RMagick
-end
+### Using MiniMagick
+
+MiniMagick performs all the operations using the 'convert' CLI which is part of the standard ImageMagick kit.
+This allows you to have the power of ImageMagick without having to worry about installing
+all the RMagick libraries, it often results in higher memory footprint.
+
+See the MiniMagick site for more details:
+
+https://github.com/minimagick/minimagick
+
+To install Imagemagick on OSX with homebrew type the following:
+
+```
+$ brew install imagemagick
 ```
 
-The RMagick module gives you a few methods, like
-`CarrierWave::RMagick#resize_to_fill` which manipulate the image file in some
-way. You can set a `process` callback, which will call that method any time a
-file is uploaded.
-There is a demonstration of convert here.
-Convert will only work if the file has the same file extension, thus the use of the filename method.
+And the ImageMagick command line options for more for what's on offer:
+
+http://www.imagemagick.org/script/command-line-options.php
+
+Currently, the MiniMagick carrierwave processor provides exactly the same methods as
+for the RMagick processor.
 
 ```ruby
 class AvatarUploader < CarrierWave::Uploader::Base
-  include CarrierWave::RMagick
+  include CarrierWave::MiniMagick
 
   process resize_to_fill: [200, 200]
-  process convert: 'png'
-
-  def filename
-    super.chomp(File.extname(super)) + '.png' if original_filename.present?
-  end
 end
 ```
 
-Check out the manipulate! method, which makes it easy for you to write your own
-manipulation methods.
-
-## Using MiniMagick
+#### List of available processing methods:
 
-MiniMagick is similar to RMagick but performs all the operations using the 'convert'
-CLI which is part of the standard ImageMagick kit. This allows you to have the power
-of ImageMagick without having to worry about installing all the RMagick libraries.
+- `convert` - Changes the image encoding format to the given format(eg. jpg). This operation is treated specially to trigger the change of the file extension, so it matches with the format of the resulting file.
+- `resize_to_limit` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. Will only resize the image if it is larger than the specified dimensions. The resulting image may be shorter or narrower than specified in the smaller dimension but will not be larger than the specified values.
+- `resize_to_fit` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. The image may be shorter or narrower than specified in the smaller dimension but will not be larger than the specified values.
+- `resize_to_fill` - Resize the image to fit within the specified dimensions while retaining the aspect ratio of the original image. If necessary, crop the image in the larger dimension. Optionally, a "gravity" may be specified, for example "Center", or "NorthEast".
+- `resize_and_pad` - Resize the image to fit within the specified dimensions while retaining the original aspect ratio. If necessary, will pad the remaining area with the given color, which defaults to transparent (for gif and png, white for jpeg). Optionally, a "gravity" may be specified, as above.
 
-See the MiniMagick site for more details:
+See `carrierwave/processing/mini_magick.rb` for details.
 
-https://github.com/minimagick/minimagick
+### Using RMagick
 
-And the ImageMagick command line options for more for what's on offer:
+CarrierWave also comes with support for RMagick, a well-known image processing library.
+To use it, you'll need to include this in your Uploader:
 
-http://www.imagemagick.org/script/command-line-options.php
+```ruby
+class AvatarUploader < CarrierWave::Uploader::Base
+  include CarrierWave::RMagick
+end
+```
 
-Currently, the MiniMagick carrierwave processor provides exactly the same methods as
-for the RMagick processor.
+The RMagick module gives you a few methods, like
+`CarrierWave::RMagick#resize_to_fill` which manipulate the image file in some
+way. You can set a `process` callback, which will call that method any time a
+file is uploaded.
+There is a demonstration of convert here.
 
 ```ruby
 class AvatarUploader < CarrierWave::Uploader::Base
-  include CarrierWave::MiniMagick
+  include CarrierWave::RMagick
 
   process resize_to_fill: [200, 200]
+  process convert: 'png'
 end
 ```
 
+Check out the manipulate! method, which makes it easy for you to write your own
+manipulation methods.
+
 ## Migrating from Paperclip
 
 If you are using Paperclip, you can use the provided compatibility module:

lib/carrierwave/uploader/cache.rb

@@ -103,7 +103,7 @@ def sanitized_file
       # [String] a cache name, in the format TIMEINT-PID-COUNTER-RND/filename.txt
       #
       def cache_name
-        File.join(cache_id, full_original_filename) if cache_id && original_filename
+        File.join(cache_id, original_filename) if cache_id && original_filename
       end
 
       ##
@@ -166,7 +166,7 @@ def retrieve_from_cache!(cache_name)
           self.cache_id, self.original_filename = cache_name.to_s.split('/', 2)
           @staged = true
           @filename = original_filename
-          @file = cache_storage.retrieve_from_cache!(full_filename(original_filename))
+          @file = cache_storage.retrieve_from_cache!(full_original_filename)
         end
       end
 
@@ -181,7 +181,7 @@ def retrieve_from_cache!(cache_name)
       #
       # [String] the cache path
       #
-      def cache_path(for_file=full_filename(original_filename))
+      def cache_path(for_file=full_original_filename)
         File.join(*[cache_dir, @cache_id, for_file].compact)
       end
 
@@ -197,9 +197,6 @@ def workfile_path(for_file=original_filename)
 
       attr_reader :original_filename
 
-      # We can override the full_original_filename method in other modules
-      alias_method :full_original_filename, :original_filename
-
       def cache_id=(cache_id)
         # Earlier version used 3 part cache_id. Thus we should allow for
         # the cache_id to have both 3 part and 4 part formats.
@@ -215,6 +212,11 @@ def original_filename=(filename)
       def cache_storage
         @cache_storage ||= (self.class.cache_storage || self.class.storage).new(self)
       end
+
+      # We can override the full_original_filename method in other modules
+      def full_original_filename
+        forcing_extension(original_filename)
+      end
     end # Cache
   end # Uploader
 end # CarrierWave

lib/carrierwave/uploader/configuration.rb

@@ -24,6 +24,7 @@ module Configuration
         add_config :move_to_store
         add_config :remove_previously_stored_files_after_update
         add_config :downloader
+        add_config :force_extension
 
         # fog
         add_deprecated_config :fog_provider
@@ -202,6 +203,7 @@ def reset_config
             config.move_to_store = false
             config.remove_previously_stored_files_after_update = true
             config.downloader = CarrierWave::Downloader::Base
+            config.force_extension = false
             config.ignore_integrity_errors = true
             config.ignore_processing_errors = true
             config.ignore_download_errors = true

lib/carrierwave/uploader/processing.rb

@@ -66,6 +66,11 @@ def process(*args)
           condition = new_processors.delete(:if) || new_processors.delete(:unless)
           new_processors.each do |processor, processor_args|
             self.processors += [[processor, processor_args, condition, condition_type]]
+
+            if processor == :convert
+              # Treat :convert specially, since it should trigger the file extension change
+              force_extension processor_args
+            end
           end
         end
       end # ClassMethods
@@ -106,6 +111,15 @@ def process!(new_file=nil)
         end
       end
 
+    private
+
+      def forcing_extension(filename)
+        if force_extension && filename
+          Pathname.new(filename).sub_ext(".#{force_extension.to_s.delete_prefix('.')}").to_s
+        else
+          filename
+        end
+      end
     end # Processing
   end # Uploader
 end # CarrierWave

lib/carrierwave/uploader/store.rb

@@ -92,7 +92,7 @@ def retrieve_from_store!(identifier)
     private
 
       def full_filename(for_file)
-        for_file
+        forcing_extension(for_file)
       end
 
       def storage

spec/processing/rmagick_spec.rb

@@ -348,4 +348,39 @@ def read
       end
     end
   end
+
+  describe "when working with frames" do
+    before do
+      def instance.cover
+        manipulate! { |frame, index| frame if index.zero? }
+      end
+
+      klass.send :include, CarrierWave::RMagick
+    end
+
+    after { instance.instance_eval { undef cover } }
+
+    context "with a multi-page PDF" do
+      before { instance.cache! File.open(file_path("multi_page.pdf")) }
+
+      it "successfully processes" do
+        klass.process :convert => 'jpg'
+        instance.process!
+      end
+
+      it "supports page specific transformations" do
+        klass.process :cover
+        instance.process!
+      end
+    end
+
+    context "with a simple image" do
+      before { instance.cache! File.open(file_path("portrait.jpg")) }
+
+      it "allows page specific transformations" do
+        klass.process :cover
+        instance.process!
+      end
+    end
+  end
 end