aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb4
-rw-r--r--activemodel/lib/active_model/type/helpers/time_value.rb1
-rw-r--r--activemodel/lib/active_model/type/time.rb2
-rw-r--r--activemodel/test/cases/type/time_test.rb15
-rw-r--r--activerecord/test/cases/attribute_methods_test.rb10
-rw-r--r--activestorage/CHANGELOG.md11
-rw-r--r--activestorage/app/models/active_storage/blob.rb7
-rw-r--r--activestorage/app/models/active_storage/variant.rb8
-rw-r--r--activestorage/lib/active_storage/analyzer.rb13
-rw-r--r--activestorage/lib/active_storage/attached/macros.rb4
-rw-r--r--activestorage/lib/active_storage/downloader.rb41
-rw-r--r--activestorage/lib/active_storage/downloading.rb8
-rw-r--r--activestorage/lib/active_storage/previewer.rb15
-rw-r--r--activestorage/test/models/attached_test.rb54
-rw-r--r--activestorage/test/models/blob_test.rb21
-rw-r--r--activesupport/lib/active_support/deprecation/behaviors.rb4
-rw-r--r--activesupport/test/deprecation_test.rb8
-rw-r--r--guides/source/active_storage_overview.md49
18 files changed, 216 insertions, 59 deletions
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index 5a06bf86e3..46c0e80194 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -560,12 +560,14 @@ module ActionController
# Returns a parameter for the given +key+. If the +key+
# can't be found, there are several options: With no other arguments,
# it will raise an <tt>ActionController::ParameterMissing</tt> error;
- # if more arguments are given, then that will be returned; if a block
+ # if a second argument is given, then that is returned (converted to an
+ # instance of ActionController::Parameters if possible); if a block
# is given, then that will be run and its result returned.
#
# params = ActionController::Parameters.new(person: { name: "Francesco" })
# params.fetch(:person) # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
# params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none
+ # params.fetch(:none, {}) # => <ActionController::Parameters {} permitted: false>
# params.fetch(:none, "Francesco") # => "Francesco"
# params.fetch(:none) { "Francesco" } # => "Francesco"
def fetch(key, *args)
diff --git a/activemodel/lib/active_model/type/helpers/time_value.rb b/activemodel/lib/active_model/type/helpers/time_value.rb
index 250c4021c6..cb6aa67a9d 100644
--- a/activemodel/lib/active_model/type/helpers/time_value.rb
+++ b/activemodel/lib/active_model/type/helpers/time_value.rb
@@ -1,5 +1,6 @@
# frozen_string_literal: true
+require "active_support/core_ext/string/zones"
require "active_support/core_ext/time/zones"
module ActiveModel
diff --git a/activemodel/lib/active_model/type/time.rb b/activemodel/lib/active_model/type/time.rb
index c094ee0013..b3056b1333 100644
--- a/activemodel/lib/active_model/type/time.rb
+++ b/activemodel/lib/active_model/type/time.rb
@@ -18,6 +18,8 @@ module ActiveModel
case value
when ::String
value = "2000-01-01 #{value}"
+ time_hash = ::Date._parse(value)
+ return if time_hash[:hour].nil?
when ::Time
value = value.change(year: 2000, day: 1, month: 1)
end
diff --git a/activemodel/test/cases/type/time_test.rb b/activemodel/test/cases/type/time_test.rb
index f7102d1e97..3fbae1a169 100644
--- a/activemodel/test/cases/type/time_test.rb
+++ b/activemodel/test/cases/type/time_test.rb
@@ -17,6 +17,21 @@ module ActiveModel
assert_equal ::Time.utc(2000, 1, 1, 16, 45, 54), type.cast("2015-06-13T19:45:54+03:00")
assert_equal ::Time.utc(1999, 12, 31, 21, 7, 8), type.cast("06:07:08+09:00")
end
+
+ def test_user_input_in_time_zone
+ ::Time.use_zone("Pacific Time (US & Canada)") do
+ type = Type::Time.new
+ assert_nil type.user_input_in_time_zone(nil)
+ assert_nil type.user_input_in_time_zone("")
+ assert_nil type.user_input_in_time_zone("ABC")
+
+ offset = ::Time.zone.formatted_offset
+ time_string = "2015-02-09T19:45:54#{offset}"
+
+ assert_equal 19, type.user_input_in_time_zone(time_string).hour
+ assert_equal offset, type.user_input_in_time_zone(time_string).formatted_offset
+ end
+ end
end
end
end
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 4a7427158a..434d32846c 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -736,6 +736,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase
end
end
+ test "setting invalid string to a zone-aware time attribute" do
+ in_time_zone "Pacific Time (US & Canada)" do
+ record = @target.new
+ time_string = "ABC"
+
+ record.bonus_time = time_string
+ assert_nil record.bonus_time
+ end
+ end
+
test "removing time zone-aware types" do
with_time_zone_aware_types(:datetime) do
in_time_zone "Pacific Time (US & Canada)" do
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 679ca0df03..7b724b7b81 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,14 @@
+* Generated attachment getter and setter methods are created
+ within the model's `GeneratedAssociationMethods` module to
+ allow overriding and composition using `super`.
+
+ *Josh Susser*, *Jamon Douglas*
+
+* Add `ActiveStorage::Blob#open`, which downloads a blob to a tempfile on disk
+ and yields the tempfile. Deprecate `ActiveStorage::Downloading`.
+
+ *David Robertson*, *George Claghorn*
+
* Pass in `identify: false` as an argument when providing a `content_type` for
`ActiveStorage::Attached::{One,Many}#attach` to bypass automatic content
type inference. For example:
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index b8b3b62f22..134d3bb2d9 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -1,5 +1,7 @@
# frozen_string_literal: true
+require "active_storage/downloader"
+
# A blob is a record that contains the metadata about a file and a key for where that file resides on the service.
# Blobs can be created in two ways:
#
@@ -164,6 +166,11 @@ class ActiveStorage::Blob < ActiveRecord::Base
service.download key, &block
end
+ # Downloads the blob to a tempfile on disk. Yields the tempfile.
+ def open(tempdir: nil, &block)
+ ActiveStorage::Downloader.new(self, tempdir: tempdir).download_blob_to_tempfile(&block)
+ end
+
# Deletes the file on the service that's associated with this blob. This should only be done if the blob is going to be
# deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index b782489a92..1df36e37d9 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-require "active_storage/downloading"
-
# Image blobs can have variants that are the result of a set of transformations applied to the original.
# These variants are used to create thumbnails, fixed-size avatars, or any other derivative image from the
# original.
@@ -44,7 +42,7 @@ require "active_storage/downloading"
# You can combine any number of ImageMagick/libvips operations into a variant, as well as any macros provided by the
# ImageProcessing gem (such as +resize_to_fit+):
#
-# avatar.variant(resize_to_fit: [800, 800], monochrome: true, flip: "-90")
+# avatar.variant(resize_to_fit: [800, 800], monochrome: true, rotate: "-90")
#
# Visit the following links for a list of available ImageProcessing commands and ImageMagick/libvips operations:
#
@@ -53,8 +51,6 @@ require "active_storage/downloading"
# * {ImageProcessing::Vips}[https://github.com/janko-m/image_processing/blob/master/doc/vips.md#methods]
# * {ruby-vips reference}[http://www.rubydoc.info/gems/ruby-vips/Vips/Image]
class ActiveStorage::Variant
- include ActiveStorage::Downloading
-
WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif )
attr_reader :blob, :variation
@@ -98,7 +94,7 @@ class ActiveStorage::Variant
end
def process
- download_blob_to_tempfile do |image|
+ blob.open do |image|
transform image do |output|
upload output
end
diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb
index 7c4168c1a0..caa25418a5 100644
--- a/activestorage/lib/active_storage/analyzer.rb
+++ b/activestorage/lib/active_storage/analyzer.rb
@@ -1,13 +1,9 @@
# frozen_string_literal: true
-require "active_storage/downloading"
-
module ActiveStorage
# This is an abstract base class for analyzers, which extract metadata from blobs. See
# ActiveStorage::Analyzer::ImageAnalyzer for an example of a concrete subclass.
class Analyzer
- include Downloading
-
attr_reader :blob
# Implement this method in a concrete subclass. Have it return true when given a blob from which
@@ -26,8 +22,17 @@ module ActiveStorage
end
private
+ # Downloads the blob to a tempfile on disk. Yields the tempfile.
+ def download_blob_to_tempfile(&block) #:doc:
+ blob.open tempdir: tempdir, &block
+ end
+
def logger #:doc:
ActiveStorage.logger
end
+
+ def tempdir #:doc:
+ Dir.tmpdir
+ end
end
end
diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb
index 819f00cc06..f99cf35680 100644
--- a/activestorage/lib/active_storage/attached/macros.rb
+++ b/activestorage/lib/active_storage/attached/macros.rb
@@ -28,7 +28,7 @@ module ActiveStorage
# If the +:dependent+ option isn't set, the attachment will be purged
# (i.e. destroyed) whenever the record is destroyed.
def has_one_attached(name, dependent: :purge_later)
- class_eval <<-CODE, __FILE__, __LINE__ + 1
+ generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
def #{name}
@active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
end
@@ -75,7 +75,7 @@ module ActiveStorage
# If the +:dependent+ option isn't set, all the attachments will be purged
# (i.e. destroyed) whenever the record is destroyed.
def has_many_attached(name, dependent: :purge_later)
- class_eval <<-CODE, __FILE__, __LINE__ + 1
+ generated_association_methods.class_eval <<-CODE, __FILE__, __LINE__ + 1
def #{name}
@active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
end
diff --git a/activestorage/lib/active_storage/downloader.rb b/activestorage/lib/active_storage/downloader.rb
new file mode 100644
index 0000000000..0e7039e104
--- /dev/null
+++ b/activestorage/lib/active_storage/downloader.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module ActiveStorage
+ class Downloader #:nodoc:
+ def initialize(blob, tempdir: nil)
+ @blob = blob
+ @tempdir = tempdir
+ end
+
+ def download_blob_to_tempfile
+ open_tempfile do |file|
+ download_blob_to file
+ yield file
+ end
+ end
+
+ private
+ attr_reader :blob, :tempdir
+
+ def open_tempfile
+ file = Tempfile.open([ "ActiveStorage", tempfile_extension_with_delimiter ], tempdir)
+
+ begin
+ yield file
+ ensure
+ file.close!
+ end
+ end
+
+ def download_blob_to(file)
+ file.binmode
+ blob.download { |chunk| file.write(chunk) }
+ file.flush
+ file.rewind
+ end
+
+ def tempfile_extension_with_delimiter
+ blob.filename.extension_with_delimiter
+ end
+ end
+end
diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb
index 7c3d20ade0..df820bc088 100644
--- a/activestorage/lib/active_storage/downloading.rb
+++ b/activestorage/lib/active_storage/downloading.rb
@@ -1,9 +1,17 @@
# frozen_string_literal: true
require "tmpdir"
+require "active_support/core_ext/string/filters"
module ActiveStorage
module Downloading
+ def self.included(klass)
+ ActiveSupport::Deprecation.warn <<~MESSAGE.squish, caller_locations(2)
+ ActiveStorage::Downloading is deprecated and will be removed in Active Storage 6.1.
+ Use ActiveStorage::Blob#open instead.
+ MESSAGE
+ end
+
private
# Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile.
def download_blob_to_tempfile #:doc:
diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb
index cf19987d72..bff5e42d41 100644
--- a/activestorage/lib/active_storage/previewer.rb
+++ b/activestorage/lib/active_storage/previewer.rb
@@ -1,14 +1,10 @@
# frozen_string_literal: true
-require "active_storage/downloading"
-
module ActiveStorage
# This is an abstract base class for previewers, which generate images from blobs. See
# ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of
# concrete subclasses.
class Previewer
- include Downloading
-
attr_reader :blob
# Implement this method in a concrete subclass. Have it return true when given a blob from which
@@ -28,6 +24,11 @@ module ActiveStorage
end
private
+ # Downloads the blob to a tempfile on disk. Yields the tempfile.
+ def download_blob_to_tempfile(&block) #:doc:
+ blob.open tempdir: tempdir, &block
+ end
+
# Executes a system command, capturing its binary output in a tempfile. Yields the tempfile.
#
# Use this method to shell out to a system library (e.g. mupdf or ffmpeg) for preview image
@@ -41,7 +42,7 @@ module ActiveStorage
# end
# end
#
- # The output tempfile is opened in the directory returned by ActiveStorage::Downloading#tempdir.
+ # The output tempfile is opened in the directory returned by #tempdir.
def draw(*argv) #:doc:
ActiveSupport::Notifications.instrument("preview.active_storage") do
open_tempfile_for_drawing do |file|
@@ -70,5 +71,9 @@ module ActiveStorage
def logger #:doc:
ActiveStorage.logger
end
+
+ def tempdir #:doc:
+ Dir.tmpdir
+ end
end
end
diff --git a/activestorage/test/models/attached_test.rb b/activestorage/test/models/attached_test.rb
new file mode 100644
index 0000000000..14395e12df
--- /dev/null
+++ b/activestorage/test/models/attached_test.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "database/setup"
+
+class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
+ include ActiveJob::TestHelper
+
+ setup do
+ @user = User.create!(name: "Josh")
+ end
+
+ teardown { ActiveStorage::Blob.all.each(&:purge) }
+
+ test "overriding has_one_attached methods works" do
+ # attach blob before messing with getter, which breaks `#attach`
+ @user.avatar.attach create_blob(filename: "funky.jpg")
+
+ # inherited only
+ assert_equal "funky.jpg", @user.avatar.filename.to_s
+
+ User.class_eval do
+ def avatar
+ super.filename.to_s.reverse
+ end
+ end
+
+ # override with super
+ assert_equal "funky.jpg".reverse, @user.avatar
+
+ User.send(:remove_method, :avatar)
+ end
+
+ test "overriding has_many_attached methods works" do
+ # attach blobs before messing with getter, which breaks `#attach`
+ @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
+
+ # inherited only
+ assert_equal "funky.jpg", @user.highlights.first.filename.to_s
+ assert_equal "wonky.jpg", @user.highlights.second.filename.to_s
+
+ User.class_eval do
+ def highlights
+ super.reverse
+ end
+ end
+
+ # override with super
+ assert_equal "wonky.jpg", @user.highlights.first.filename.to_s
+ assert_equal "funky.jpg", @user.highlights.second.filename.to_s
+
+ User.send(:remove_method, :highlights)
+ end
+end
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index 17151117db..a013b7a924 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -84,6 +84,27 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
assert_equal "a" * 64.kilobytes, chunks.second
end
+ test "open" do
+ create_file_blob(filename: "racecar.jpg").open do |file|
+ assert file.binmode?
+ assert_equal 0, file.pos
+ assert_match(/\.jpg\z/, file.path)
+ assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file"
+ end
+ end
+
+ test "open in a custom tempdir" do
+ tempdir = Dir.mktmpdir
+
+ create_file_blob(filename: "racecar.jpg").open(tempdir: tempdir) do |file|
+ assert file.binmode?
+ assert_equal 0, file.pos
+ assert_match(/\.jpg\z/, file.path)
+ assert file.path.starts_with?(tempdir)
+ assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file"
+ end
+ end
+
test "urls expiring in 5 minutes" do
blob = create_blob
diff --git a/activesupport/lib/active_support/deprecation/behaviors.rb b/activesupport/lib/active_support/deprecation/behaviors.rb
index 66d6f3225a..3abd25aa85 100644
--- a/activesupport/lib/active_support/deprecation/behaviors.rb
+++ b/activesupport/lib/active_support/deprecation/behaviors.rb
@@ -94,6 +94,10 @@ module ActiveSupport
private
def arity_coerce(behavior)
+ unless behavior.respond_to?(:call)
+ raise ArgumentError, "#{behavior.inspect} is not a valid deprecation behavior."
+ end
+
if behavior.arity == 4 || behavior.arity == -1
behavior
else
diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb
index 60673c032b..105153584d 100644
--- a/activesupport/test/deprecation_test.rb
+++ b/activesupport/test/deprecation_test.rb
@@ -182,6 +182,14 @@ class DeprecationTest < ActiveSupport::TestCase
end
end
+ def test_default_invalid_behavior
+ e = assert_raises(ArgumentError) do
+ ActiveSupport::Deprecation.behavior = :invalid
+ end
+
+ assert_equal ":invalid is not a valid deprecation behavior.", e.message
+ end
+
def test_deprecated_instance_variable_proxy
assert_not_deprecated { @dtc.request.size }
diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md
index 292928488b..91ad089d40 100644
--- a/guides/source/active_storage_overview.md
+++ b/guides/source/active_storage_overview.md
@@ -380,58 +380,25 @@ Rails.application.routes.url_helpers.rails_blob_path(user.avatar, only_path: tru
Downloading Files
-----------------
-If you need to process the blobs on the server side, such as, when performing
-analysis or further conversions, you can download the blob and get a binary
-object:
+Sometimes you need to process a blob after it’s uploaded—for example, to convert
+it to a different format. Use `ActiveStorage::Blob#download` to read a blob’s
+binary data into memory:
```ruby
binary = user.avatar.download
```
-In some cases you might want to convert that into an actual file on the disk to
-pass the file path to external programs (such as virus scanners, converters,
-optimizers, minifiers, etc.). In this case you can include the
-`ActiveStorage::Downloading` module into your class which provides helpers to
-download directly into files while avoiding to store the file into memory.
-`ActiveStorage::Downloading` expects a `blob` method to be defined.
+You might want to download a blob to a file on disk so an external program (e.g.
+a virus scanner or media transcoder) can operate on it. Use
+`ActiveStorage::Blob#open` to download a blob to a tempfile on disk:
```ruby
-class VirusScanner
- include ActiveStorage::Downloading
-
- attr_reader :blob
-
- def initialize(blob)
- @blob = blob
- end
-
- def scan
- download_blob_to_tempfile do |file|
- system 'scan_virus', file.path
- end
- end
-end
-```
-
-By default, `download_blob_to_tempfile` creates files in `Dir.tmpdir`. If you need to use another directory, override ActiveStorage::Downloading#tempdir in your class:
-
-```ruby
-class VirusScanner
- include ActiveStorage::Downloading
+message.video.open do |file|
+ system '/path/to/virus/scanner', file.path
# ...
-
- private
- def tempdir
- '/path/to/tmp'
- end
end
```
-If the external program is run as a separate program, you might also want to
-`chmod` the file and it's directory, as it is inaccessible by other users
-because `Tempfile` will set the permissions to `0600`.
-
-
Transforming Images
-------------------