From 1966c188cfb06b39a47082e2f6c6e33a43668ae5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 11 Jul 2017 18:53:17 +0200 Subject: Very incomplete first stab --- lib/active_storage/variant.rb | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 lib/active_storage/variant.rb (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb new file mode 100644 index 0000000000..9b9dad43da --- /dev/null +++ b/lib/active_storage/variant.rb @@ -0,0 +1,52 @@ +require "active_storage/blob" +require "mini_magick" + +class ActiveStorage::Variant + class_attribute :verifier + + attr_reader :blob, :variation + delegate :service, to: :blob + + def self.lookup(blob_key:, variation_key:) + new ActiveStorage::Blob.find_by!(key: blob_key), variation: verifier.verify(variation_key) + end + + def self.encode_key(variation) + verifier.generate(variation) + end + + def initialize(blob, variation:) + @blob, @variation = blob, variation + end + + def url(expires_in: 5.minutes, disposition: :inline) + perform unless exist? + service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename + end + + def key + verifier.generate(variation) + end + + private + def perform + upload_variant transform(download_blob) + end + + def download_blob + service.download(blob.key) + end + + def upload_variant(variation) + service.upload key, variation + end + + def transform(io) + # FIXME: Actually do a variant based on the variation + File.open MiniMagick::Image.read(io).resize("500x500").path + end + + def exist? + service.exist?(key) + end +end -- cgit v1.2.3 From 5dbe5eaeb84b470624f1a2785315ddd4f7b1a4e3 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 14:05:18 -0500 Subject: Follow AR like naming of factory method --- lib/active_storage/variant.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 9b9dad43da..f005454b00 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -7,7 +7,7 @@ class ActiveStorage::Variant attr_reader :blob, :variation delegate :service, to: :blob - def self.lookup(blob_key:, variation_key:) + def self.find_or_create_by(blob_key:, variation_key:) new ActiveStorage::Blob.find_by!(key: blob_key), variation: verifier.verify(variation_key) end -- cgit v1.2.3 From 76395e3c1b997da7b3853b1b3e94b712b1a29ecf Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 14:05:40 -0500 Subject: Do real transformations in a safe way --- lib/active_storage/variant.rb | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index f005454b00..62262c7790 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -1,9 +1,15 @@ require "active_storage/blob" +require "active_support/core_ext/object/inclusion" require "mini_magick" class ActiveStorage::Variant class_attribute :verifier + ALLOWED_TRANSFORMATIONS = %i( + resize rotate format flip fill monochrome orient quality roll scale sharpen shave shear size thumbnail + transparent transpose transverse trim background bordercolor compress crop + ) + attr_reader :blob, :variation delegate :service, to: :blob @@ -42,11 +48,21 @@ class ActiveStorage::Variant end def transform(io) - # FIXME: Actually do a variant based on the variation - File.open MiniMagick::Image.read(io).resize("500x500").path + File.open \ + MiniMagick::Image.read(io).tap { |transforming_image| + variation.each do |(method, argument)| + if method = allowed_transformational_method(method.to_sym) + if argument.present? + transforming_image.public_send(method, argument) + else + transforming_image.public_send(method) + end + end + end + }.path end - def exist? - service.exist?(key) + def allowed_transformational_method(method) + method.presence_in(ALLOWED_TRANSFORMATIONS) end end -- cgit v1.2.3 From f1523ab39e38bdc031c3bfb61ed4b6decd23ffcd Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 14:06:00 -0500 Subject: Use a unique blob variant key for storage --- lib/active_storage/variant.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 62262c7790..3053f44211 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -27,7 +27,7 @@ class ActiveStorage::Variant def url(expires_in: 5.minutes, disposition: :inline) perform unless exist? - service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename + service.url blob_variant_key, expires_in: expires_in, disposition: disposition, filename: blob.filename end def key @@ -35,6 +35,10 @@ class ActiveStorage::Variant end private + def exist? + service.exist?(blob_variant_key) + end + def perform upload_variant transform(download_blob) end @@ -44,7 +48,11 @@ class ActiveStorage::Variant end def upload_variant(variation) - service.upload key, variation + service.upload blob_variant_key, variation + end + + def blob_variant_key + "variants/#{blob.key}/#{key}" end def transform(io) -- cgit v1.2.3 From af999681122bf583b6644974ba2033453935fd6d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 17:35:15 -0500 Subject: Make processing an explicit step --- lib/active_storage/variant.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 3053f44211..a07735eb57 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -13,8 +13,8 @@ class ActiveStorage::Variant attr_reader :blob, :variation delegate :service, to: :blob - def self.find_or_create_by(blob_key:, variation_key:) - new ActiveStorage::Blob.find_by!(key: blob_key), variation: verifier.verify(variation_key) + def self.find_or_process_by!(blob_key:, encoded_variant_key:) + new(ActiveStorage::Blob.find_by!(key: blob_key), variation: verifier.verify(encoded_variant_key)).processed end def self.encode_key(variation) @@ -25,8 +25,12 @@ class ActiveStorage::Variant @blob, @variation = blob, variation end + def processed + process unless exist? + self + end + def url(expires_in: 5.minutes, disposition: :inline) - perform unless exist? service.url blob_variant_key, expires_in: expires_in, disposition: disposition, filename: blob.filename end @@ -39,7 +43,7 @@ class ActiveStorage::Variant service.exist?(blob_variant_key) end - def perform + def process upload_variant transform(download_blob) end -- cgit v1.2.3 From a968e3c3c75df3f209275d31eb0bd4ed6effd51e Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 17:35:23 -0500 Subject: Consistent naming --- lib/active_storage/variant.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index a07735eb57..658fb2f5bd 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -51,8 +51,8 @@ class ActiveStorage::Variant service.download(blob.key) end - def upload_variant(variation) - service.upload blob_variant_key, variation + def upload_variant(variant) + service.upload blob_variant_key, variant end def blob_variant_key -- cgit v1.2.3 From beb60b9c3a3f1f51d10fa800b967402d79ffcf28 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 20 Jul 2017 17:35:41 -0500 Subject: True is the same as no arguments --- lib/active_storage/variant.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 658fb2f5bd..7fcd3924f4 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -64,10 +64,11 @@ class ActiveStorage::Variant MiniMagick::Image.read(io).tap { |transforming_image| variation.each do |(method, argument)| if method = allowed_transformational_method(method.to_sym) - if argument.present? - transforming_image.public_send(method, argument) - else + if argument.blank? || argument == true transforming_image.public_send(method) + else + # FIXME: Consider whitelisting allowed arguments as well? + transforming_image.public_send(method, argument) end end end -- cgit v1.2.3 From cbe89319de0d2eda8ee53ab7c9d3bc92751deeb4 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 21 Jul 2017 12:35:00 -0500 Subject: Better naming --- lib/active_storage/variant.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 7fcd3924f4..4145ee644d 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -26,7 +26,7 @@ class ActiveStorage::Variant end def processed - process unless exist? + process unless processed? self end @@ -38,8 +38,9 @@ class ActiveStorage::Variant verifier.generate(variation) end + private - def exist? + def processed? service.exist?(blob_variant_key) end -- cgit v1.2.3 From 7f4111185ca6286adbe0ffc1346d416c5fa7dfd3 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 21 Jul 2017 15:51:31 -0500 Subject: Extract variation value object --- lib/active_storage/variant.rb | 66 ++++++++----------------------------------- 1 file changed, 12 insertions(+), 54 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 4145ee644d..8be51eba92 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -1,82 +1,40 @@ require "active_storage/blob" -require "active_support/core_ext/object/inclusion" require "mini_magick" class ActiveStorage::Variant - class_attribute :verifier - - ALLOWED_TRANSFORMATIONS = %i( - resize rotate format flip fill monochrome orient quality roll scale sharpen shave shear size thumbnail - transparent transpose transverse trim background bordercolor compress crop - ) - attr_reader :blob, :variation delegate :service, to: :blob - def self.find_or_process_by!(blob_key:, encoded_variant_key:) - new(ActiveStorage::Blob.find_by!(key: blob_key), variation: verifier.verify(encoded_variant_key)).processed - end - - def self.encode_key(variation) - verifier.generate(variation) + class << self + def find_or_process_by!(blob_key:, variation_key:) + new(ActiveStorage::Blob.find_by!(key: blob_key), variation: ActiveStorage::Variation.decode(variation_key)).processed + end end - def initialize(blob, variation:) + def initialize(blob, variation) @blob, @variation = blob, variation end def processed - process unless processed? + process unless service.exist?(key) self end - def url(expires_in: 5.minutes, disposition: :inline) - service.url blob_variant_key, expires_in: expires_in, disposition: disposition, filename: blob.filename + def key + "variants/#{blob.key}/#{variation.key}" end - def key - verifier.generate(variation) + def url(expires_in: 5.minutes, disposition: :inline) + service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename end private - def processed? - service.exist?(blob_variant_key) - end - def process - upload_variant transform(download_blob) - end - - def download_blob - service.download(blob.key) - end - - def upload_variant(variant) - service.upload blob_variant_key, variant - end - - def blob_variant_key - "variants/#{blob.key}/#{key}" + service.upload key, transform(service.download(blob.key)) end def transform(io) - File.open \ - MiniMagick::Image.read(io).tap { |transforming_image| - variation.each do |(method, argument)| - if method = allowed_transformational_method(method.to_sym) - if argument.blank? || argument == true - transforming_image.public_send(method) - else - # FIXME: Consider whitelisting allowed arguments as well? - transforming_image.public_send(method, argument) - end - end - end - }.path - end - - def allowed_transformational_method(method) - method.presence_in(ALLOWED_TRANSFORMATIONS) + File.open MiniMagick::Image.read(io).tap { |image| variation.transform(image) }.path end end -- cgit v1.2.3 From 796f8330ad441e93590a57521ef8fb80a030fb66 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 21 Jul 2017 16:12:29 -0500 Subject: Fix and test VariantsController --- lib/active_storage/variant.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index 8be51eba92..ba2604eccf 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -5,12 +5,6 @@ class ActiveStorage::Variant attr_reader :blob, :variation delegate :service, to: :blob - class << self - def find_or_process_by!(blob_key:, variation_key:) - new(ActiveStorage::Blob.find_by!(key: blob_key), variation: ActiveStorage::Variation.decode(variation_key)).processed - end - end - def initialize(blob, variation) @blob, @variation = blob, variation end -- cgit v1.2.3 From fa33ec9e7decde0fc0ba3e2bd2b7fc9f06908065 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 21 Jul 2017 16:34:28 -0500 Subject: Anemic intro --- lib/active_storage/variant.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/active_storage/variant.rb') diff --git a/lib/active_storage/variant.rb b/lib/active_storage/variant.rb index ba2604eccf..435033f980 100644 --- a/lib/active_storage/variant.rb +++ b/lib/active_storage/variant.rb @@ -1,6 +1,7 @@ require "active_storage/blob" require "mini_magick" +# Image blobs can have variants that are the result of a set of transformations applied to the original. class ActiveStorage::Variant attr_reader :blob, :variation delegate :service, to: :blob -- cgit v1.2.3