mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Disable variant options when false or nil present
In response to https://github.com/rails/rails/issues/32917 In the current implementation, ActiveStorage passes all options to the underlying processor, including when a key has a value of false. For example, passing: ``` avatar.variant(resize: "100x100", monochrome: false, flip: "-90") ``` will return a monochrome image (or an error, pending on ImageMagick configuration) because it passes `-monochrome false` to the command (but the command line does not allow disabling flags this way, as usually a user would omit the flag entirely to disable that feature). This fix only passes those keys forward to the underlying processor if the value responds to `present?`. In practice, this means that `false` or `nil` will be filtered out before going to the processor. One possible use case would be for a user to be able to apply different filters to an avatar. The code might look something like: ``` variant_options = { monochrome: params[:monochrome], resize: params[:resize] } avatar.variant(*variant_options) ``` Obviously some sanitization may be beneficial in a real-world scenario, but this type of configuration object could be used in many other places as well. - Add removing falsy values from varaints to changelog - The entirety of #image_processing_transformation inject block was wrapped in `list.tap` to guard against the default `nil` being returned if no conditional was called. - add test for explicitly true variant options
This commit is contained in:
parent
ce4d467f7c
commit
0210ac0b43
3 changed files with 81 additions and 12 deletions
|
@ -1,3 +1,13 @@
|
|||
* Variant arguments of `false` or `nil` will no longer be passed to the
|
||||
processor. For example, the following will not have the monochrome
|
||||
variation applied:
|
||||
|
||||
```ruby
|
||||
avatar.variant(monochrome: false)
|
||||
```
|
||||
|
||||
*Jacob Smith*
|
||||
|
||||
* Generated attachment getter and setter methods are created
|
||||
within the model's `GeneratedAssociationMethods` module to
|
||||
allow overriding and composition using `super`.
|
||||
|
|
|
@ -66,13 +66,15 @@ class ActiveStorage::Variation
|
|||
# Applies image transformations using the ImageProcessing gem.
|
||||
def image_processing_transform(file, format)
|
||||
operations = transformations.inject([]) do |list, (name, argument)|
|
||||
list.tap do |list|
|
||||
if name.to_s == "combine_options"
|
||||
ActiveSupport::Deprecation.warn("The ImageProcessing ActiveStorage variant backend doesn't need :combine_options, as it already generates a single MiniMagick command. In Rails 6.1 :combine_options will not be supported anymore.")
|
||||
list.concat argument.to_a
|
||||
else
|
||||
list.concat argument.keep_if { |key, value| value.present? }.to_a
|
||||
elsif argument.present?
|
||||
list << [name, argument]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
processor
|
||||
.source(file)
|
||||
|
@ -116,14 +118,10 @@ class ActiveStorage::Variation
|
|||
end
|
||||
|
||||
def pass_transform_argument(command, method, argument)
|
||||
if eligible_argument?(argument)
|
||||
command.public_send(method, argument)
|
||||
else
|
||||
if argument == true
|
||||
command.public_send(method)
|
||||
elsif argument.present?
|
||||
command.public_send(method, argument)
|
||||
end
|
||||
end
|
||||
|
||||
def eligible_argument?(argument)
|
||||
argument.present? && argument != true
|
||||
end
|
||||
end
|
||||
|
|
|
@ -25,6 +25,67 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
|
|||
assert_match(/Gray/, image.colorspace)
|
||||
end
|
||||
|
||||
test "monochrome with default variant_processor" do
|
||||
begin
|
||||
ActiveStorage.variant_processor = nil
|
||||
|
||||
blob = create_file_blob(filename: "racecar.jpg")
|
||||
variant = blob.variant(monochrome: true).processed
|
||||
image = read_image(variant)
|
||||
assert_match(/Gray/, image.colorspace)
|
||||
ensure
|
||||
ActiveStorage.variant_processor = :mini_magick
|
||||
end
|
||||
end
|
||||
|
||||
test "disabled variation of JPEG blob" do
|
||||
blob = create_file_blob(filename: "racecar.jpg")
|
||||
variant = blob.variant(resize: "100x100", monochrome: false).processed
|
||||
assert_match(/racecar\.jpg/, variant.service_url)
|
||||
|
||||
image = read_image(variant)
|
||||
assert_equal 100, image.width
|
||||
assert_equal 67, image.height
|
||||
assert_match(/RGB/, image.colorspace)
|
||||
end
|
||||
|
||||
test "disabled variation of JPEG blob with :combine_options" do
|
||||
blob = create_file_blob(filename: "racecar.jpg")
|
||||
variant = ActiveSupport::Deprecation.silence do
|
||||
blob.variant(combine_options: {
|
||||
resize: "100x100",
|
||||
monochrome: false
|
||||
}).processed
|
||||
end
|
||||
assert_match(/racecar\.jpg/, variant.service_url)
|
||||
|
||||
image = read_image(variant)
|
||||
assert_equal 100, image.width
|
||||
assert_equal 67, image.height
|
||||
assert_match(/RGB/, image.colorspace)
|
||||
end
|
||||
|
||||
test "disabled variation using :combine_options" do
|
||||
begin
|
||||
ActiveStorage.variant_processor = nil
|
||||
blob = create_file_blob(filename: "racecar.jpg")
|
||||
variant = ActiveSupport::Deprecation.silence do
|
||||
blob.variant(combine_options: {
|
||||
crop: "100x100+0+0",
|
||||
monochrome: false
|
||||
}).processed
|
||||
end
|
||||
assert_match(/racecar\.jpg/, variant.service_url)
|
||||
|
||||
image = read_image(variant)
|
||||
assert_equal 100, image.width
|
||||
assert_equal 100, image.height
|
||||
assert_match(/RGB/, image.colorspace)
|
||||
ensure
|
||||
ActiveStorage.variant_processor = :mini_magick
|
||||
end
|
||||
end
|
||||
|
||||
test "center-weighted crop of JPEG blob using :combine_options" do
|
||||
begin
|
||||
ActiveStorage.variant_processor = nil
|
||||
|
|
Loading…
Reference in a new issue