diff --git a/README.md b/README.md index 531e7b8e5..e1184d8bd 100644 --- a/README.md +++ b/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 @@ -913,67 +936,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: diff --git a/lib/carrierwave/uploader/cache.rb b/lib/carrierwave/uploader/cache.rb index 715eccff9..d61e5b44d 100644 --- a/lib/carrierwave/uploader/cache.rb +++ b/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 diff --git a/lib/carrierwave/uploader/configuration.rb b/lib/carrierwave/uploader/configuration.rb index 469eb1a44..adf722460 100644 --- a/lib/carrierwave/uploader/configuration.rb +++ b/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 diff --git a/lib/carrierwave/uploader/processing.rb b/lib/carrierwave/uploader/processing.rb index 346e3ee81..1b638abdb 100644 --- a/lib/carrierwave/uploader/processing.rb +++ b/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 diff --git a/lib/carrierwave/uploader/store.rb b/lib/carrierwave/uploader/store.rb index 7f473dda2..9d87dacc7 100644 --- a/lib/carrierwave/uploader/store.rb +++ b/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 diff --git a/spec/processing/rmagick_spec.rb b/spec/processing/rmagick_spec.rb index 11668da79..ae21149ac 100644 --- a/spec/processing/rmagick_spec.rb +++ b/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 diff --git a/spec/uploader/processing_spec.rb b/spec/uploader/processing_spec.rb index 0fbcd0022..911acd4e4 100644 --- a/spec/uploader/processing_spec.rb +++ b/spec/uploader/processing_spec.rb @@ -135,41 +135,6 @@ end end - context "when using RMagick", :rmagick => true do - before do - def uploader.cover - manipulate! { |frame, index| frame if index.zero? } - end - - uploader_class.send :include, CarrierWave::RMagick - end - - after { uploader.instance_eval { undef cover } } - - context "with a multi-page PDF" do - before { uploader.cache! File.open(file_path("multi_page.pdf")) } - - it "successfully processes" do - uploader_class.process :convert => 'jpg' - uploader.process! - end - - it "supports page specific transformations" do - uploader_class.process :cover - uploader.process! - end - end - - context "with a simple image" do - before { uploader.cache! File.open(file_path("portrait.jpg")) } - - it "allows page specific transformations" do - uploader_class.process :cover - uploader.process! - end - end - end - context "with 'enable_processing' set to false" do before { uploader_class.enable_processing = false } @@ -193,4 +158,160 @@ def uploader.cover uploader.cache!(File.open(file_path('test.jpg'))) end end + + describe '#forcing_extension' do + it "works with a symbol" do + uploader.force_extension = :png + expect(uploader.send(:forcing_extension, 'test.jpg')).to eq 'test.png' + end + + it "works with a string without dot" do + uploader.force_extension = 'png' + expect(uploader.send(:forcing_extension, 'test.jpg')).to eq 'test.png' + end + + it "works with a string with dot" do + uploader.force_extension = '.png' + expect(uploader.send(:forcing_extension, 'test.jpg')).to eq 'test.png' + end + + it "does nothing when force_extension is false" do + uploader.force_extension = false + expect(uploader.send(:forcing_extension, 'test.jpg')).to eq 'test.jpg' + end + end + + context "when using #convert" do + let(:another_uploader) { uploader_class.new } + before do + uploader_class.class_eval do + include CarrierWave::MiniMagick + process convert: :png + end + end + + it "performs the processing" do + uploader.cache!(File.open(file_path('landscape.jpg'))) + expect(uploader).to be_format('png') + expect(uploader.file.filename).to eq 'landscape.png' + end + + it "does not change #original_filename but changes #cache_path and #url to have new extension" do + uploader.cache!(File.open(file_path('landscape.jpg'))) + expect(uploader.send(:original_filename)).to eq 'landscape.jpg' + expect(uploader.cache_name.split('/').last).to eq 'landscape.jpg' + expect(File.basename(uploader.cache_path)).to eq 'landscape.png' + expect(File.basename(uploader.url)).to eq 'landscape.png' + end + + it "changes #filename to have new extension" do + uploader.store!(File.open(file_path('landscape.jpg'))) + expect(uploader.identifier).to eq 'landscape.jpg' + expect(File.basename(uploader.store_path)).to eq 'landscape.png' + expect(File.basename(uploader.url)).to eq 'landscape.png' + end + + it "allows the cached file to be retrieved" do + uploader.cache!(File.open(file_path('landscape.jpg'))) + another_uploader.retrieve_from_cache!(uploader.cache_name) + expect(another_uploader.cache_path).to eq uploader.cache_path + expect(another_uploader.url).to eq uploader.url + end + + it "allows the stored file to be retrieved" do + uploader.store!(File.open(file_path('landscape.jpg'))) + another_uploader.retrieve_from_store!(uploader.identifier) + expect(another_uploader.identifier).to eq uploader.identifier + expect(another_uploader.url).to eq uploader.url + end + + context "with #filename overridden" do + let(:changed_extension) { '.png' } + + before do + uploader_class.class_eval <<-RUBY, __FILE__, __LINE__+1 + def filename + super.chomp(File.extname(super)) + '#{changed_extension}' + end + RUBY + end + + it "stores the file" do + uploader.store!(File.open(file_path('landscape.jpg'))) + expect(uploader.filename).to eq 'landscape.png' + end + + it "retrieves the file" do + uploader.store!(File.open(file_path('landscape.jpg'))) + another_uploader.retrieve_from_store!(uploader.identifier) + expect(another_uploader.identifier).to eq uploader.identifier + expect(another_uploader.url).to eq uploader.url + end + + context "to have a wrong extension" do + let(:changed_extension) { '.gif' } + + it "uses the wrong one" do + uploader.store!(File.open(file_path('landscape.jpg'))) + expect(uploader.filename).to eq 'landscape.gif' + expect(uploader).to be_format('png') + end + end + end + end + + context "when file extension changes not using #convert" do + let(:another_uploader) { uploader_class.new } + before do + uploader_class.class_eval do + def rename + file.move_to 'landscape.bin' + end + process :rename + end + end + + it "performs the processing without changing #idenfitier" do + uploader.cache!(File.open(file_path('landscape.jpg'))) + expect(uploader.file.filename).to eq 'landscape.bin' + expect(uploader.identifier).to eq 'landscape.jpg' + end + + context "but applying #force_extension" do + before do + uploader_class.class_eval do + force_extension '.bin' + end + end + + it "changes #filename to have the extension" do + uploader.store!(File.open(file_path('landscape.jpg'))) + expect(uploader.identifier).to eq 'landscape.jpg' + expect(File.basename(uploader.store_path)).to eq 'landscape.bin' + end + end + + context "but overriding #filename" do + before do + uploader_class.class_eval <<-RUBY, __FILE__, __LINE__+1 + def filename + super.chomp(File.extname(super)) + '.bin' + end + RUBY + end + + it "changes #filename to have the extension" do + uploader.store!(File.open(file_path('landscape.jpg'))) + expect(uploader.identifier).to eq 'landscape.bin' + expect(File.basename(uploader.store_path)).to eq 'landscape.bin' + end + + it "retrieves the file by using the overridden name" do + uploader.store!(File.open(file_path('landscape.jpg'))) + another_uploader.retrieve_from_store!(uploader.identifier) + expect(another_uploader.identifier).to eq uploader.identifier + expect(another_uploader.url).to eq uploader.url + end + end + end end diff --git a/spec/uploader/versions_spec.rb b/spec/uploader/versions_spec.rb index f3fb300f2..2f9644653 100644 --- a/spec/uploader/versions_spec.rb +++ b/spec/uploader/versions_spec.rb @@ -826,4 +826,128 @@ def condition(_); false; end end end end + + describe 'with a version using #convert' do + before do + @uploader_class.class_eval do + include CarrierWave::MiniMagick + end + + @uploader_class.version(:thumb) do + process convert: :png + end + + @another_uploader = @uploader_class.new + end + + it 'caches the file with given extension' do + @uploader.cache!(File.open(file_path('landscape.jpg'))) + expect(File.basename(@uploader.thumb.cache_path)).to eq 'thumb_landscape.png' + expect(File.basename(@uploader.thumb.url)).to eq 'thumb_landscape.png' + expect(@uploader.thumb).to be_format('png') + end + + it 'retrieves the cached file' do + @uploader.cache!(File.open(file_path('landscape.jpg'))) + @another_uploader.retrieve_from_cache!(@uploader.cache_name) + expect(@another_uploader.url).to eq @uploader.url + expect(@another_uploader.thumb.url).to eq @uploader.thumb.url + end + + it 'stores the file with given extension' do + @uploader.store!(File.open(file_path('landscape.jpg'))) + expect(File.basename(@uploader.thumb.store_path)).to eq 'thumb_landscape.png' + expect(File.basename(@uploader.thumb.url)).to eq 'thumb_landscape.png' + expect(@uploader.thumb).to be_format('png') + end + + it 'retrieves the stored file' do + @uploader.store!(File.open(file_path('landscape.jpg'))) + @another_uploader.retrieve_from_store!(@uploader.identifier) + expect(@another_uploader.identifier).to eq @uploader.identifier + expect(@another_uploader.url).to eq @uploader.url + expect(@another_uploader.thumb.url).to eq @uploader.thumb.url + end + + context 'with base filename overridden' do + before do + @uploader_class.class_eval do + def filename + "image.#{file.extension}" + end + end + end + + it "stores the file" do + @uploader.store!(File.open(file_path('landscape.jpg'))) + expect(File.basename(@uploader.thumb.url)).to eq 'thumb_image.png' + end + + it "retrieves the file without inconsistency" do + @uploader.store!(File.open(file_path('landscape.jpg'))) + @another_uploader.retrieve_from_store!(@uploader.identifier) + expect(@another_uploader.identifier).to eq @uploader.identifier + expect(@another_uploader.url).to eq @uploader.url + expect(@another_uploader.thumb.url).to eq @uploader.thumb.url + end + end + + context 'with version full_filename overridden' do + before do + @uploader_class.version(:thumb) do + def full_filename(for_file) + 'thumb_image.png' + end + end + end + + it "stores the file" do + @uploader.store!(File.open(file_path('landscape.jpg'))) + expect(File.basename(@uploader.thumb.url)).to eq 'thumb_image.png' + end + + it "retrieves the file without inconsistency" do + @uploader.store!(File.open(file_path('landscape.jpg'))) + @another_uploader.retrieve_from_store!(@uploader.identifier) + expect(@another_uploader.identifier).to eq @uploader.identifier + expect(@another_uploader.url).to eq @uploader.url + expect(@another_uploader.thumb.url).to eq @uploader.thumb.url + end + end + end + + describe 'with a version with file extension change' do + before do + @uploader_class.class_eval do + def rename_to_bin + file.move_to("#{File.basename(file.filename, '.*')}.bin") + end + end + + @uploader_class.version(:thumb) do + process :rename_to_bin + end + end + + it "does not change the version's #filename" do + @uploader.cache!(File.open(file_path('landscape.jpg'))) + expect(@uploader.thumb.file.filename).to eq 'landscape.bin' + expect(@uploader.thumb.filename).to eq 'landscape.jpg' + expect(File.basename(@uploader.thumb.store_path)).to eq 'thumb_landscape.jpg' + end + + context "but applying #force_extension" do + before do + @uploader_class.version(:thumb) do + force_extension '.bin' + end + end + + it "changes #filename to have the extension" do + @uploader.store!(File.open(file_path('landscape.jpg'))) + expect(@uploader.thumb.identifier).to eq 'landscape.jpg' + expect(File.basename(@uploader.thumb.store_path)).to eq 'thumb_landscape.bin' + end + end + end end