diff options
Diffstat (limited to 'activestorage/lib')
18 files changed, 223 insertions, 53 deletions
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index e1bd974853..928db32691 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -45,7 +45,9 @@ module ActiveStorage mattr_accessor :queue mattr_accessor :previewers, default: [] mattr_accessor :analyzers, default: [] + mattr_accessor :variant_processor, default: :mini_magick mattr_accessor :paths, default: {} mattr_accessor :variable_content_types, default: [] mattr_accessor :content_types_to_serve_as_binary, default: [] + mattr_accessor :service_urls_expire_in, default: 5.minutes 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/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index e31bdb0edb..18d8ff8237 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -16,7 +16,7 @@ module ActiveStorage # # When a video's angle is 90 or 270 degrees, its width and height are automatically swapped for convenience. # - # This analyzer requires the {ffmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. + # This analyzer requires the {FFmpeg}[https://www.ffmpeg.org] system library, which is not provided by Rails. class Analyzer::VideoAnalyzer < Analyzer def self.accept?(blob) blob.video? @@ -107,7 +107,7 @@ module ActiveStorage JSON.parse(output.read) end rescue Errno::ENOENT - logger.info "Skipping video analysis because ffmpeg isn't installed" + logger.info "Skipping video analysis because FFmpeg isn't installed" {} end diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index 819f00cc06..6ad9fc43d7 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 @@ -48,6 +48,12 @@ module ActiveStorage else before_destroy { public_send(name).detach } end + + ActiveRecord::Reflection.add_attachment_reflection( + self, + name, + ActiveRecord::Reflection.create(:has_one_attached, name, nil, { dependent: dependent }, self) + ) end # Specifies the relation between multiple attachments and the model. @@ -75,7 +81,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 @@ -105,6 +111,12 @@ module ActiveStorage else before_destroy { public_send(name).detach } end + + ActiveRecord::Reflection.add_attachment_reflection( + self, + name, + ActiveRecord::Reflection.create(:has_many_attached, name, nil, { dependent: dependent }, self) + ) end end end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index a582ed0137..f992cb5f84 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -13,6 +13,10 @@ module ActiveStorage record.public_send("#{name}_attachment") end + def blank? + attachment.blank? + end + # Associates a given attachment with the current record, saving it to the database. # # person.avatar.attach(params[:avatar]) # ActionDispatch::Http::UploadedFile object diff --git a/activestorage/lib/active_storage/downloader.rb b/activestorage/lib/active_storage/downloader.rb new file mode 100644 index 0000000000..87be6efb05 --- /dev/null +++ b/activestorage/lib/active_storage/downloader.rb @@ -0,0 +1,44 @@ +# 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 + verify_integrity_of file + yield file + end + end + + private + attr_reader :blob, :tempdir + + def open_tempfile + file = Tempfile.open([ "ActiveStorage-#{blob.id}-", blob.filename.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 verify_integrity_of(file) + unless Digest::MD5.file(file).base64digest == blob.checksum + raise ActiveStorage::IntegrityError + end + end + end +end diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index f2a1fffdcb..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: @@ -27,6 +35,7 @@ module ActiveStorage def download_blob_to(file) #:doc: file.binmode blob.download { |chunk| file.write(chunk) } + file.flush file.rewind end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 1385e2aa84..93c5c55b95 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -10,6 +10,8 @@ require "active_storage/previewer/video_previewer" require "active_storage/analyzer/image_analyzer" require "active_storage/analyzer/video_analyzer" +require "active_storage/reflection" + module ActiveStorage class Engine < Rails::Engine # :nodoc: isolate_namespace ActiveStorage @@ -43,14 +45,16 @@ module ActiveStorage initializer "active_storage.configs" do config.after_initialize do |app| - ActiveStorage.logger = app.config.active_storage.logger || Rails.logger - ActiveStorage.queue = app.config.active_storage.queue - ActiveStorage.previewers = app.config.active_storage.previewers || [] - ActiveStorage.analyzers = app.config.active_storage.analyzers || [] - ActiveStorage.paths = app.config.active_storage.paths || {} + ActiveStorage.logger = app.config.active_storage.logger || Rails.logger + ActiveStorage.queue = app.config.active_storage.queue + ActiveStorage.variant_processor = app.config.active_storage.variant_processor || :mini_magick + ActiveStorage.previewers = app.config.active_storage.previewers || [] + ActiveStorage.analyzers = app.config.active_storage.analyzers || [] + ActiveStorage.paths = app.config.active_storage.paths || {} ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || [] ActiveStorage.content_types_to_serve_as_binary = app.config.active_storage.content_types_to_serve_as_binary || [] + ActiveStorage.service_urls_expire_in = app.config.active_storage.service_urls_expire_in || 5.minutes end end @@ -94,5 +98,12 @@ module ActiveStorage end end end + + initializer "active_storage.reflection" do + ActiveSupport.on_load(:active_record) do + include Reflection::ActiveRecordExtensions + ActiveRecord::Reflection.singleton_class.prepend(Reflection::ReflectionExtension) + end + end end end diff --git a/activestorage/lib/active_storage/errors.rb b/activestorage/lib/active_storage/errors.rb index f099b13f5b..bedcd080c4 100644 --- a/activestorage/lib/active_storage/errors.rb +++ b/activestorage/lib/active_storage/errors.rb @@ -4,4 +4,8 @@ module ActiveStorage class InvariableError < StandardError; end class UnpreviewableError < StandardError; end class UnrepresentableError < StandardError; end + + # Raised when uploaded or downloaded data does not match a precomputed checksum. + # Indicates that a network error or a software bug caused data corruption. + class IntegrityError < StandardError; end end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index cf19987d72..fb202f029a 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,9 +24,14 @@ 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 + # Use this method to shell out to a system library (e.g. muPDF or FFmpeg) for preview image # generation. The resulting tempfile can be used as the +:io+ value in an attachable Hash: # # def preview @@ -41,18 +42,19 @@ 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| + open_tempfile do |file| + instrument :preview, key: blob.key do capture(*argv, to: file) - yield file end + + yield file end end - def open_tempfile_for_drawing - tempfile = Tempfile.open("ActiveStorage", tempdir) + def open_tempfile + tempfile = Tempfile.open("ActiveStorage-", tempdir) begin yield tempfile @@ -61,6 +63,10 @@ module ActiveStorage end end + def instrument(operation, payload = {}, &block) + ActiveSupport::Notifications.instrument "#{operation}.active_storage", payload, &block + end + def capture(*argv, to:) to.binmode IO.popen(argv, err: File::NULL) { |out| IO.copy_stream(out, to) } @@ -70,5 +76,9 @@ module ActiveStorage def logger #:doc: ActiveStorage.logger end + + def tempdir #:doc: + Dir.tmpdir + end end end diff --git a/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb index 2a787362cf..69eb617d7b 100644 --- a/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb +++ b/activestorage/lib/active_storage/previewer/poppler_pdf_previewer.rb @@ -12,7 +12,7 @@ module ActiveStorage end def pdftoppm_exists? - return @pdftoppm_exists unless @pdftoppm_exists.nil? + return @pdftoppm_exists if defined?(@pdftoppm_exists) @pdftoppm_exists = system(pdftoppm_path, "-v", out: File::NULL, err: File::NULL) end diff --git a/activestorage/lib/active_storage/previewer/video_previewer.rb b/activestorage/lib/active_storage/previewer/video_previewer.rb index 2f28a3d341..50e13d202a 100644 --- a/activestorage/lib/active_storage/previewer/video_previewer.rb +++ b/activestorage/lib/active_storage/previewer/video_previewer.rb @@ -9,15 +9,14 @@ module ActiveStorage def preview download_blob_to_tempfile do |input| draw_relevant_frame_from input do |output| - yield io: output, filename: "#{blob.filename.base}.png", content_type: "image/png" + yield io: output, filename: "#{blob.filename.base}.jpg", content_type: "image/jpeg" end end end private def draw_relevant_frame_from(file, &block) - draw ffmpeg_path, "-i", file.path, "-y", "-vcodec", "png", - "-vf", "thumbnail", "-vframes", "1", "-f", "image2", "-", &block + draw ffmpeg_path, "-i", file.path, "-y", "-vframes", "1", "-f", "image2", "-", &block end def ffmpeg_path diff --git a/activestorage/lib/active_storage/reflection.rb b/activestorage/lib/active_storage/reflection.rb new file mode 100644 index 0000000000..04a1b20882 --- /dev/null +++ b/activestorage/lib/active_storage/reflection.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module ActiveStorage + module Reflection + # Holds all the metadata about a has_one_attached attachment as it was + # specified in the Active Record class. + class HasOneAttachedReflection < ActiveRecord::Reflection::MacroReflection #:nodoc: + def macro + :has_one_attached + end + end + + # Holds all the metadata about a has_many_attached attachment as it was + # specified in the Active Record class. + class HasManyAttachedReflection < ActiveRecord::Reflection::MacroReflection #:nodoc: + def macro + :has_many_attached + end + end + + module ReflectionExtension # :nodoc: + def add_attachment_reflection(ar, name, reflection) + ar.attachment_reflections.merge!(name.to_s => reflection) + end + + private + def reflection_class_for(macro) + case macro + when :has_one_attached + HasOneAttachedReflection + when :has_many_attached + HasManyAttachedReflection + else + super + end + end + end + + module ActiveRecordExtensions + extend ActiveSupport::Concern + + included do + class_attribute :attachment_reflections, instance_writer: false, default: {} + end + + module ClassMethods + # Returns an array of reflection objects for all the attachments in the + # class. + def reflect_on_all_attachments + attachment_reflections.values + end + + # Returns the reflection object for the named +attachment+. + # + # User.reflect_on_attachment(:avatar) + # # => the avatar reflection + # + def reflect_on_attachment(attachment) + attachment_reflections[attachment.to_s] + end + end + end + end +end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index 949969fc95..f915518f52 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -3,8 +3,6 @@ require "active_storage/log_subscriber" module ActiveStorage - class IntegrityError < StandardError; end - # Abstract class serving as an interface for concrete services. # # The available services are: @@ -41,8 +39,6 @@ module ActiveStorage extend ActiveSupport::Autoload autoload :Configurator - class_attribute :url_expires_in, default: 5.minutes - class << self # Configure an Active Storage service by name from a set of configurations, # typically loaded from a YAML file. The Active Storage engine uses this @@ -94,7 +90,7 @@ module ActiveStorage end # Returns a signed, temporary URL for the file at the +key+. The URL will be valid for the amount - # of seconds specified in +expires_in+. You most also provide the +disposition+ (+:inline+ or +:attachment+), + # of seconds specified in +expires_in+. You must also provide the +disposition+ (+:inline+ or +:attachment+), # +filename+, and +content_type+ that you wish the file to be served with on request. def url(key, expires_in:, disposition:, filename:, content_type:) raise NotImplementedError diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 5b652fe74e..b1b6f1ddcf 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -26,7 +26,7 @@ module ActiveStorage if block_given? instrument :streaming_download, key: key do File.open(path_for(key), "rb") do |file| - while data = file.read(64.kilobytes) + while data = file.read(5.megabytes) yield data end end diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index 369c33cbdb..38acef81f4 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -gem "google-cloud-storage", "~> 1.8" +gem "google-cloud-storage", "~> 1.11" require "google/cloud/storage" require "net/http" @@ -32,27 +32,21 @@ module ActiveStorage end end - # FIXME: Download in chunks when given a block. - def download(key) - instrument :download, key: key do - io = file_for(key).download - io.rewind - - if block_given? - yield io.read - else - io.read + def download(key, &block) + if block_given? + instrument :streaming_download, key: key do + stream(key, &block) + end + else + instrument :download, key: key do + file_for(key).download.string end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - uri = URI(url(key, expires_in: 30.seconds, filename: ActiveStorage::Filename.new(""), content_type: "application/octet-stream", disposition: "inline")) - - Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client| - client.get(uri, "Range" => "bytes=#{range.begin}-#{range.exclude_end? ? range.end - 1 : range.end}").body - end + file_for(key).download(range: range).string end end @@ -110,8 +104,21 @@ module ActiveStorage private attr_reader :config - def file_for(key) - bucket.file(key, skip_lookup: true) + def file_for(key, skip_lookup: true) + bucket.file(key, skip_lookup: skip_lookup) + end + + # Reads the file for the given key in chunks, yielding each to the block. + def stream(key) + file = file_for(key, skip_lookup: false) + + chunk_size = 5.megabytes + offset = 0 + + while offset < file.size + yield file.download(range: offset..(offset + chunk_size - 1)).string + offset += chunk_size + end end def bucket diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 5e489f4be1..0286e7ff21 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -33,7 +33,7 @@ module ActiveStorage end else instrument :download, key: key do - object_for(key).get.body.read.force_encoding(Encoding::BINARY) + object_for(key).get.body.string.force_encoding(Encoding::BINARY) end end end diff --git a/activestorage/lib/tasks/activestorage.rake b/activestorage/lib/tasks/activestorage.rake index 296e91afa1..ac254d717f 100644 --- a/activestorage/lib/tasks/activestorage.rake +++ b/activestorage/lib/tasks/activestorage.rake @@ -1,6 +1,9 @@ # frozen_string_literal: true namespace :active_storage do + # Prevent migration installation task from showing up twice. + Rake::Task["install:migrations"].clear_comments + desc "Copy over the migration needed to the application" task install: :environment do if Rake::Task.task_defined?("active_storage:install:migrations") |