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

[mshibuya]

Overview

With the current implementation, CarrierWave does not handle change of the file extension on converting a file. Suppose you have

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  process convert: :png
end

and upload a JPEG file named test.jpg, you'll get a PNG format file with the filename of test.jpg. This is often not what users want, as they expect the file extension also changes. The existing solution was to override #filename to set the desired extension, as suggested like this.

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  process convert: :png
  def filename
    super.chomp(File.extname(super)) + '.png' if original_filename.present?
  end
end

But even with this the cache file is still created with the original file extension, which differs with the actual format of the cached file. (#2254)

Furthermore, if we combine this with versions the situation gets worse, because there're several counterintuitive interactions between setting filename and version creation. (#2125, #2126)

This PR tries to solve those issues by performing extension change based on what is provided for :convert. That means just uploading test.jpg to this uploader:

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  process convert: :png
end

you'll get test.png.

Incompatibilities

As this modification comes with the change of filename, some incompatibilities arise with pre-3.x implementations. We'll explore 2 scenarios here.

Cache file retrieval issue

If your uploader performs a format conversion 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.
So when a user tries to upload test.jpg and the first request goes to 2.x which results in a validation error (the file will be stored with the name test.jpg), then the user fixes the validation error and the next request reaches 3.x, the uploader will look for test.png but it's not there. This results in the file not being stored at all.

To avoid this situation, I recommend minimizing the co-existence of CarrierWave 2.x and 3.x. Doing a canary deployment might be acceptable if you keep the ratio high enough (i.e. if you have many 2.x application processes and very small 3.x), the majority of requests will go to 2.x process and handled consistently. Also make the co-existing situation short enough before switching over everything to 3.x.

Another option will be keeping the old behavior only for caching by doing:

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  process convert: :png
  alias_method :full_original_filename, :original_filename
end

Stored version file retrieval issue

Another issue is versioning. With CarrierWave 2.x, when uploading a file test.jpg using an uploader like this

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  version :thumb do
    process convert: :png
  end
end

the version thumb will have the filename of test.jpg (though its format is in PNG). But with 3.x, it will be test.png.
If you have existing applications doing this, many version files should have been stored based on the old way. Switching over to new way results in unable to retrieve stored version files, as it will look for the new location.

The solution is either to preserve the old behavior by

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  version :thumb do
    process convert: :png
    force_extension false
  end
end

or performing version recreation to create the files in new locations.

Please note that if you're setting #full_filename explicitly like

class MyUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  version :thumb do
    process convert: :png
    def full_filename(for_file)
      'thumb.png'
    end
  end
end

then you won't be affected by this issue, as the overridden #full_filename takes precedence.