diff options
| author | Guillermo Iguaran <guilleiguaran@gmail.com> | 2018-02-24 18:03:47 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-02-24 18:03:47 -0500 |
| commit | 697dd48b5e5787126a91ce10739f8af31d1ffd1d (patch) | |
| tree | cc4ff296f2096afe1a5fd3057f101213caac149a /activestorage | |
| parent | 6f5cca77313e127313ea44c5c213fda3b9027a95 (diff) | |
| parent | 3915a470d2b8898fdbc384d0f9f31e2ad8a2c899 (diff) | |
| download | rails-697dd48b5e5787126a91ce10739f8af31d1ffd1d.tar.gz rails-697dd48b5e5787126a91ce10739f8af31d1ffd1d.tar.bz2 rails-697dd48b5e5787126a91ce10739f8af31d1ffd1d.zip | |
Merge branch 'master' into update_default_hsts_max_age
Diffstat (limited to 'activestorage')
39 files changed, 313 insertions, 132 deletions
diff --git a/activestorage/.gitignore b/activestorage/.gitignore index a532335bdd..3e78878ffc 100644 --- a/activestorage/.gitignore +++ b/activestorage/.gitignore @@ -1,6 +1,6 @@ -.byebug_history -node_modules -test/dummy/db/*.sqlite3 -test/dummy/db/*.sqlite3-journal -test/dummy/log/*.log -test/dummy/tmp/ +/src/ +/test/dummy/db/*.sqlite3 +/test/dummy/db/*.sqlite3-journal +/test/dummy/log/*.log +/test/dummy/tmp/ +/test/service/configurations.yml diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 5e01297fc1..cc21c1f75a 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,26 +1,15 @@ -* Use `after_destroy_commit` instead of `before_destroy` for purging - attachments when a record is destroyed. +## Rails 6.0.0.alpha (Unreleased) ## - *Hiroki Zenigami* +* Rails 6 requires Ruby 2.4.1 or newer. + *Jeremy Daer* -* Force `:attachment` disposition for specific, configurable content types. - This mitigates possible security issues such as XSS or phishing when - serving them inline. A list of such content types is included by default, - and can be configured via `content_types_to_serve_as_binary`. +* Add source code to published npm package - *Rosa Gutierrez* + This allows activestorage users to depend on the javascript source code + rather than the compiled code, which can produce smaller javascript bundles. + *Richard Macklin* -## Rails 5.2.0.beta2 (November 28, 2017) ## -* Fix the gem adding the migrations files to the package. - - *Yuji Yaginuma* - - -## Rails 5.2.0.beta1 (November 27, 2017) ## - -* Added to Rails. - - *DHH* +Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activestorage/CHANGELOG.md) for previous changes. diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index d135324700..df7801a671 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -9,7 +9,7 @@ Gem::Specification.new do |s| s.summary = "Local and cloud file storage framework." s.description = "Attach cloud and local files in Rails applications." - s.required_ruby_version = ">= 2.2.2" + s.required_ruby_version = ">= 2.4.1" s.license = "MIT" diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 7067c58259..31fbc66965 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -14,17 +14,25 @@ # update a blob's metadata on a subsequent pass, but you should not update the key or change the uploaded file. # If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one. class ActiveStorage::Blob < ActiveRecord::Base - include Analyzable, Identifiable, Representable + require_dependency "active_storage/blob/analyzable" + require_dependency "active_storage/blob/identifiable" + require_dependency "active_storage/blob/representable" + + include Analyzable + include Identifiable + include Representable self.table_name = "active_storage_blobs" has_secure_token :key - store :metadata, accessors: [ :analyzed, :identified ], coder: JSON + store :metadata, accessors: [ :analyzed, :identified ], coder: ActiveRecord::Coders::JSON class_attribute :service has_many :attachments + scope :unattached, -> { left_joins(:attachments).where(ActiveStorage::Attachment.table_name => { blob_id: nil }) } + class << self # You can used the signed ID of a blob to refer to it on the client side without fear of tampering. # This is particularly helpful for direct uploads where the client-side needs to refer to the blob @@ -109,8 +117,11 @@ class ActiveStorage::Blob < ActiveRecord::Base # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL. # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And # it allows permanent URLs that redirect to the +service_url+ to be cached in the view. - def service_url(expires_in: service.url_expires_in, disposition: :inline, filename: self.filename) - service.url key, expires_in: expires_in, disposition: forcibly_serve_as_binary? ? :attachment : disposition, filename: filename, content_type: content_type + def service_url(expires_in: service.url_expires_in, disposition: :inline, filename: nil, **options) + filename = ActiveStorage::Filename.wrap(filename || self.filename) + + service.url key, expires_in: expires_in, filename: filename, content_type: content_type, + disposition: forcibly_serve_as_binary? ? :attachment : disposition, **options end # Returns a URL that can be used to directly upload a file for this blob on the service. This URL is intended to be diff --git a/activestorage/app/models/active_storage/blob/identifiable.rb b/activestorage/app/models/active_storage/blob/identifiable.rb index 40ca84ac70..dbe03cfa6c 100644 --- a/activestorage/app/models/active_storage/blob/identifiable.rb +++ b/activestorage/app/models/active_storage/blob/identifiable.rb @@ -2,10 +2,15 @@ module ActiveStorage::Blob::Identifiable def identify - ActiveStorage::Identification.new(self).apply + update!(content_type: identification.content_type, identified: true) unless identified? end def identified? identified end + + private + def identification + ActiveStorage::Identification.new self + end end diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index b9413dec95..bebb5e61b3 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -3,8 +3,18 @@ # Encapsulates a string representing a filename to provide convenient access to parts of it and sanitization. # A Filename instance is returned by ActiveStorage::Blob#filename, and is comparable so it can be used for sorting. class ActiveStorage::Filename + require_dependency "active_storage/filename/parameters" + include Comparable + class << self + # Returns a Filename instance based on the given filename. If the filename is a Filename, it is + # returned unmodified. If it is a String, it is passed to ActiveStorage::Filename.new. + def wrap(filename) + filename.kind_of?(self) ? filename : new(filename) + end + end + def initialize(filename) @filename = filename end diff --git a/activestorage/app/models/active_storage/identification.rb b/activestorage/app/models/active_storage/identification.rb index 4f295257ae..8d334ae1ea 100644 --- a/activestorage/app/models/active_storage/identification.rb +++ b/activestorage/app/models/active_storage/identification.rb @@ -1,25 +1,22 @@ # frozen_string_literal: true -class ActiveStorage::Identification +require "net/http" + +class ActiveStorage::Identification #:nodoc: attr_reader :blob def initialize(blob) @blob = blob end - def apply - blob.update!(content_type: content_type, identified: true) unless blob.identified? + def content_type + Marcel::MimeType.for(identifiable_chunk, name: filename, declared_type: declared_content_type) end private - def content_type - Marcel::MimeType.for(identifiable_chunk, name: filename, declared_type: declared_content_type) - end - - def identifiable_chunk Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https") do |client| - client.get(uri, "Range" => "0-4096").body + client.get(uri, "Range" => "bytes=0-4095").body end end diff --git a/activestorage/app/models/active_storage/preview.rb b/activestorage/app/models/active_storage/preview.rb index be5053edae..45efd26214 100644 --- a/activestorage/app/models/active_storage/preview.rb +++ b/activestorage/app/models/active_storage/preview.rb @@ -24,7 +24,7 @@ # The built-in previewers rely on third-party system libraries: # # * {ffmpeg}[https://www.ffmpeg.org] -# * {mupdf}[https://mupdf.com] +# * {mupdf}[https://mupdf.com] (version 1.8 or newer) # # These libraries are not provided by Rails. You must install them yourself to use the built-in previewers. Before you # install and use third-party software, make sure you understand the licensing implications of doing so. diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index e08a2271ec..a95a4bfd7c 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -115,7 +115,7 @@ class ActiveStorage::Variant def download_image require "mini_magick" - MiniMagick::Image.create { |file| download_blob_to(file) } + MiniMagick::Image.create(blob.filename.extension_with_delimiter) { |file| download_blob_to(file) } end def transform(image) diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index da4af62666..12e7f9f0b5 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -8,6 +8,14 @@ # # ActiveStorage::Variation.new(resize: "100x100", monochrome: true, trim: true, rotate: "-90") # +# You can also combine multiple transformations in one step, e.g. for center-weighted cropping: +# +# ActiveStorage::Variation.new(combine_options: { +# resize: "100x100^", +# gravity: "center", +# crop: "100x100+0+0", +# }) +# # A list of all possible transformations is available at https://www.imagemagick.org/script/mogrify.php. class ActiveStorage::Variation attr_reader :transformations diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb index 5231168a7c..3b39de91be 100644 --- a/activestorage/lib/active_storage/analyzer/image_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb @@ -3,6 +3,8 @@ module ActiveStorage # Extracts width and height in pixels from an image blob. # + # If the image contains EXIF data indicating its angle is 90 or 270 degrees, its width and height are swapped for convenience. + # # Example: # # ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata @@ -17,7 +19,11 @@ module ActiveStorage def metadata read_image do |image| - { width: image.width, height: image.height } + if rotated_image?(image) + { width: image.height, height: image.width } + else + { width: image.width, height: image.height } + end end rescue LoadError logger.info "Skipping image analysis because the mini_magick gem isn't installed" @@ -31,5 +37,9 @@ module ActiveStorage yield MiniMagick::Image.new(file.path) end end + + def rotated_image?(image) + %w[ RightTop LeftBottom ].include?(image["%[orientation]"]) + end end end diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index aa532da201..656e362187 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -9,12 +9,12 @@ module ActiveStorage # * Height (pixels) # * Duration (seconds) # * Angle (degrees) - # * Aspect ratio + # * Display aspect ratio # # Example: # # ActiveStorage::VideoAnalyzer.new(blob).metadata - # # => { width: 640, height: 480, duration: 5.0, angle: 0, aspect_ratio: [4, 3] } + # # => { width: 640.0, height: 480.0, duration: 5.0, angle: 0, display_aspect_ratio: [4, 3] } # # When a video's angle is 90 or 270 degrees, its width and height are automatically swapped for convenience. # @@ -25,24 +25,24 @@ module ActiveStorage end def metadata - { width: width, height: height, duration: duration, angle: angle, aspect_ratio: aspect_ratio }.compact + { width: width, height: height, duration: duration, angle: angle, display_aspect_ratio: display_aspect_ratio }.compact end private def width - rotated? ? raw_height : raw_width + if rotated? + computed_height || encoded_height + else + encoded_width + end end def height - rotated? ? raw_width : raw_height - end - - def raw_width - Integer(video_stream["width"]) if video_stream["width"] - end - - def raw_height - Integer(video_stream["height"]) if video_stream["height"] + if rotated? + encoded_width + else + computed_height || encoded_height + end end def duration @@ -53,16 +53,40 @@ module ActiveStorage Integer(tags["rotate"]) if tags["rotate"] end - def aspect_ratio + def display_aspect_ratio if descriptor = video_stream["display_aspect_ratio"] - descriptor.split(":", 2).collect(&:to_i) + if terms = descriptor.split(":", 2) + numerator = Integer(terms[0]) + denominator = Integer(terms[1]) + + [numerator, denominator] unless numerator == 0 + end end end + def rotated? angle == 90 || angle == 270 end + def computed_height + if encoded_width && display_height_scale + encoded_width * display_height_scale + end + end + + def encoded_width + @encoded_width ||= Float(video_stream["width"]) if video_stream["width"] + end + + def encoded_height + @encoded_height ||= Float(video_stream["height"]) if video_stream["height"] + end + + def display_height_scale + @display_height_scale ||= Float(display_aspect_ratio.last) / display_aspect_ratio.first if display_aspect_ratio + end + def tags @tags ||= video_stream["tags"] || {} diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index a57fda49b4..f2a1fffdcb 100644 --- a/activestorage/lib/active_storage/downloading.rb +++ b/activestorage/lib/active_storage/downloading.rb @@ -1,25 +1,37 @@ # frozen_string_literal: true +require "tmpdir" + module ActiveStorage module Downloading private # Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile. - def download_blob_to_tempfile # :doc: - Tempfile.open([ "ActiveStorage", blob.filename.extension_with_delimiter ], tempdir) do |file| + def download_blob_to_tempfile #:doc: + open_tempfile_for_blob do |file| download_blob_to file yield file end end + def open_tempfile_for_blob + tempfile = Tempfile.open([ "ActiveStorage", blob.filename.extension_with_delimiter ], tempdir) + + begin + yield tempfile + ensure + tempfile.close! + end + end + # Efficiently downloads blob data into the given file. - def download_blob_to(file) # :doc: + def download_blob_to(file) #:doc: file.binmode blob.download { |chunk| file.write(chunk) } file.rewind end # Returns the directory in which tempfiles should be opened. Defaults to +Dir.tmpdir+. - def tempdir # :doc: + def tempdir #:doc: Dir.tmpdir end end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 8ba32490b1..9430dde028 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -18,7 +18,15 @@ module ActiveStorage config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ] config.active_storage.paths = ActiveSupport::OrderedOptions.new - config.active_storage.variable_content_types = %w( image/png image/gif image/jpg image/jpeg image/vnd.adobe.photoshop ) + config.active_storage.variable_content_types = %w( + image/png + image/gif + image/jpg + image/jpeg + image/vnd.adobe.photoshop + image/vnd.microsoft.icon + ) + config.active_storage.content_types_to_serve_as_binary = %w( text/html text/javascript diff --git a/activestorage/lib/active_storage/gem_version.rb b/activestorage/lib/active_storage/gem_version.rb index f048bb0b77..492620731b 100644 --- a/activestorage/lib/active_storage/gem_version.rb +++ b/activestorage/lib/active_storage/gem_version.rb @@ -7,10 +7,10 @@ module ActiveStorage end module VERSION - MAJOR = 5 - MINOR = 2 + MAJOR = 6 + MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "alpha" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index dacab1e7df..cf19987d72 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -44,13 +44,23 @@ module ActiveStorage # The output tempfile is opened in the directory returned by ActiveStorage::Downloading#tempdir. def draw(*argv) #:doc: ActiveSupport::Notifications.instrument("preview.active_storage") do - Tempfile.open("ActiveStorage", tempdir) do |file| + open_tempfile_for_drawing do |file| capture(*argv, to: file) yield file end end end + def open_tempfile_for_drawing + tempfile = Tempfile.open("ActiveStorage", tempdir) + + begin + yield tempfile + ensure + tempfile.close! + end + end + def capture(*argv, to:) to.binmode IO.popen(argv, err: File::NULL) { |out| IO.copy_stream(out, to) } diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 27769db584..d17eea9046 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -11,7 +11,7 @@ module ActiveStorage class Service::DiskService < Service attr_reader :root, :host - def initialize(root:, host:) + def initialize(root:, host: "http://localhost:3000") @root, @host = root, host end diff --git a/activestorage/package.json b/activestorage/package.json index 621706000b..d23f8b179e 100644 --- a/activestorage/package.json +++ b/activestorage/package.json @@ -1,10 +1,11 @@ { "name": "activestorage", - "version": "5.2.0-beta2", + "version": "6.0.0-alpha", "description": "Attach cloud and local files in Rails applications", "main": "app/assets/javascripts/activestorage.js", "files": [ - "app/assets/javascripts/*.js" + "app/assets/javascripts/*.js", + "src/*.js" ], "homepage": "http://rubyonrails.org/", "repository": { @@ -16,18 +17,21 @@ }, "author": "Javan Makhmali <javan@javan.us>", "license": "MIT", + "dependencies": { + "spark-md5": "^3.0.0" + }, "devDependencies": { "babel-core": "^6.25.0", "babel-loader": "^7.1.1", "babel-preset-env": "^1.6.0", "eslint": "^4.3.0", "eslint-plugin-import": "^2.7.0", - "spark-md5": "^3.0.0", "webpack": "^3.4.0" }, "scripts": { "prebuild": "yarn lint", "build": "webpack -p", - "lint": "eslint app/javascript" + "lint": "eslint app/javascript", + "prepublishOnly": "rm -rf src && cp -R app/javascript/activestorage src" } } diff --git a/activestorage/test/analyzer/image_analyzer_test.rb b/activestorage/test/analyzer/image_analyzer_test.rb index 0d9f24c5c1..f8a38f001a 100644 --- a/activestorage/test/analyzer/image_analyzer_test.rb +++ b/activestorage/test/analyzer/image_analyzer_test.rb @@ -8,17 +8,30 @@ require "active_storage/analyzer/image_analyzer" class ActiveStorage::Analyzer::ImageAnalyzerTest < ActiveSupport::TestCase test "analyzing a JPEG image" do blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 4104, metadata[:width] assert_equal 2736, metadata[:height] end + test "analyzing a rotated JPEG image" do + blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg") + metadata = extract_metadata_from(blob) + + assert_equal 2736, metadata[:width] + assert_equal 4104, metadata[:height] + end + test "analyzing an SVG image without an XML declaration" do blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 792, metadata[:width] assert_equal 584, metadata[:height] end + + private + def extract_metadata_from(blob) + blob.tap(&:analyze).metadata + end end diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 7fc1cfce06..2612006551 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -8,28 +8,52 @@ require "active_storage/analyzer/video_analyzer" class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase test "analyzing a video" do blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 640, metadata[:width] assert_equal 480, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.166648, metadata[:duration] assert_not_includes metadata, :angle end test "analyzing a rotated video" do blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 480, metadata[:width] assert_equal 640, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] end + test "analyzing a video with rectangular samples" do + blob = create_file_blob(filename: "video_with_rectangular_samples.mp4", content_type: "video/mp4") + metadata = extract_metadata_from(blob) + + assert_equal 1280, metadata[:width] + assert_equal 720, metadata[:height] + assert_equal [16, 9], metadata[:display_aspect_ratio] + end + + test "analyzing a video with an undefined display aspect ratio" do + blob = create_file_blob(filename: "video_with_undefined_display_aspect_ratio.mp4", content_type: "video/mp4") + metadata = extract_metadata_from(blob) + + assert_equal 640, metadata[:width] + assert_equal 480, metadata[:height] + assert_nil metadata[:display_aspect_ratio] + end + test "analyzing a video without a video stream" do blob = create_file_blob(filename: "video_without_video_stream.mp4", content_type: "video/mp4") - assert_equal({ "analyzed" => true, "identified" => true }, blob.tap(&:analyze).metadata) + metadata = extract_metadata_from(blob) + assert_equal({ "analyzed" => true, "identified" => true }, metadata) end + + private + def extract_metadata_from(blob) + blob.tap(&:analyze).metadata + end end diff --git a/activestorage/test/controllers/direct_uploads_controller_test.rb b/activestorage/test/controllers/direct_uploads_controller_test.rb index 888767086c..dfffb6bb9c 100644 --- a/activestorage/test/controllers/direct_uploads_controller_test.rb +++ b/activestorage/test/controllers/direct_uploads_controller_test.rb @@ -27,7 +27,7 @@ if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].pr assert_equal checksum, details["checksum"] assert_equal "text/plain", details["content_type"] assert_match SERVICE_CONFIGURATIONS[:s3][:bucket], details["direct_upload"]["url"] - assert_match(/s3\.(\S+)?amazonaws\.com/, details["direct_upload"]["url"]) + assert_match(/s3(-[-a-z0-9]+)?\.(\S+)?amazonaws\.com/, details["direct_upload"]["url"]) assert_equal({ "Content-Type" => "text/plain", "Content-MD5" => checksum }, details["direct_upload"]["headers"]) end end diff --git a/activestorage/test/controllers/previews_controller_test.rb b/activestorage/test/controllers/previews_controller_test.rb index 704a466160..b87be6c8b2 100644 --- a/activestorage/test/controllers/previews_controller_test.rb +++ b/activestorage/test/controllers/previews_controller_test.rb @@ -14,7 +14,7 @@ class ActiveStorage::PreviewsControllerTest < ActionDispatch::IntegrationTest signed_blob_id: @blob.signed_id, variation_key: ActiveStorage::Variation.encode(resize: "100x100")) - assert @blob.preview_image.attached? + assert_predicate @blob.preview_image, :attached? assert_redirected_to(/report\.png\?.*disposition=inline/) image = read_image(@blob.preview_image.variant(resize: "100x100")) diff --git a/activestorage/test/database/setup.rb b/activestorage/test/database/setup.rb index 705650a25d..daeeb5695b 100644 --- a/activestorage/test/database/setup.rb +++ b/activestorage/test/database/setup.rb @@ -3,5 +3,5 @@ require_relative "create_users_migration" ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") -ActiveRecord::Migrator.migrate File.expand_path("../../db/migrate", __dir__) +ActiveRecord::Base.connection.migration_context.migrate ActiveStorageCreateUsers.migrate(:up) diff --git a/activestorage/test/dummy/config/boot.rb b/activestorage/test/dummy/config/boot.rb index 72516d9255..59459d4ae3 100644 --- a/activestorage/test/dummy/config/boot.rb +++ b/activestorage/test/dummy/config/boot.rb @@ -5,7 +5,3 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../../Gemfile", __dir__) require "bundler/setup" if File.exist?(ENV["BUNDLE_GEMFILE"]) $LOAD_PATH.unshift File.expand_path("../../../lib", __dir__) - -if %w[s server c console].any? { |a| ARGV.include?(a) } - puts "=> Booting Rails" -end diff --git a/activestorage/test/dummy/config/storage.yml b/activestorage/test/dummy/config/storage.yml index 2acc78dc4c..2c6762e0d6 100644 --- a/activestorage/test/dummy/config/storage.yml +++ b/activestorage/test/dummy/config/storage.yml @@ -1,4 +1,3 @@ local: service: Disk root: <%= Rails.root.join("storage") %> - host: http://localhost:3000 diff --git a/activestorage/test/fixtures/files/favicon.ico b/activestorage/test/fixtures/files/favicon.ico Binary files differnew file mode 100644 index 0000000000..87192a8a07 --- /dev/null +++ b/activestorage/test/fixtures/files/favicon.ico diff --git a/activestorage/test/fixtures/files/racecar_rotated.jpg b/activestorage/test/fixtures/files/racecar_rotated.jpg Binary files differnew file mode 100644 index 0000000000..89e6d54f98 --- /dev/null +++ b/activestorage/test/fixtures/files/racecar_rotated.jpg diff --git a/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 Binary files differnew file mode 100644 index 0000000000..12b04afc87 --- /dev/null +++ b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 diff --git a/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 Binary files differnew file mode 100644 index 0000000000..eb354e756f --- /dev/null +++ b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index f0aa96b411..9ba2759893 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -65,14 +65,14 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase end end - assert user.avatar.attached? + assert_predicate user.avatar, :attached? assert_equal "funky.jpg", user.avatar.filename.to_s assert_difference -> { ActiveStorage::Attachment.count }, +1 do user.save! end - assert user.reload.avatar.attached? + assert_predicate user.reload.avatar, :attached? assert_equal "funky.jpg", user.avatar.filename.to_s end @@ -81,12 +81,12 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user = User.new(name: "Jason", avatar: { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }) end - assert @user.new_record? - assert @user.avatar.attached? + assert_predicate @user, :new_record? + assert_predicate @user.avatar, :attached? assert_equal "town.jpg", @user.avatar.filename.to_s @user.save! - assert @user.reload.avatar.attached? + assert_predicate @user.reload.avatar, :attached? assert_equal "town.jpg", @user.avatar.filename.to_s end @@ -106,12 +106,12 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.avatar.attach(blob) assert_equal "image/jpeg", @user.avatar.reload.content_type - assert @user.avatar.identified? + assert_predicate @user.avatar, :identified? end test "identify newly-attached blob only once" do blob = create_file_blob - assert blob.identified? + assert_predicate blob, :identified? # The blob's backing file is a PNG image. Fudge its content type so we can tell if it's identified when we attach it. blob.update! content_type: "application/octet-stream" @@ -136,7 +136,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.avatar.attach blob end - assert blob.reload.analyzed? + assert_predicate blob.reload, :analyzed? @user.avatar.detach @@ -161,7 +161,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase avatar_key = @user.avatar.key @user.avatar.detach - assert_not @user.avatar.attached? + assert_not_predicate @user.avatar, :attached? assert ActiveStorage::Blob.exists?(avatar_blob_id) assert ActiveStorage::Blob.service.exist?(avatar_key) end @@ -171,7 +171,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase avatar_key = @user.avatar.key @user.avatar.purge - assert_not @user.avatar.attached? + assert_not_predicate @user.avatar, :attached? assert_not ActiveStorage::Blob.service.exist?(avatar_key) end @@ -228,7 +228,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase end end - assert user.highlights.attached? + assert_predicate user.highlights, :attached? assert_equal "town.jpg", user.highlights.first.filename.to_s assert_equal "country.jpg", user.highlights.second.filename.to_s @@ -236,7 +236,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase user.save! end - assert user.reload.highlights.attached? + assert_predicate user.reload.highlights, :attached? assert_equal "town.jpg", user.highlights.first.filename.to_s assert_equal "country.jpg", user.highlights.second.filename.to_s end @@ -248,13 +248,13 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" }]) end - assert @user.new_record? - assert @user.highlights.attached? + assert_predicate @user, :new_record? + assert_predicate @user.highlights, :attached? assert_equal "town.jpg", @user.highlights.first.filename.to_s assert_equal "country.jpg", @user.highlights.second.filename.to_s @user.save! - assert @user.reload.highlights.attached? + assert_predicate @user.reload.highlights, :attached? assert_equal "town.jpg", @user.highlights.first.filename.to_s assert_equal "country.jpg", @user.highlights.second.filename.to_s end @@ -334,7 +334,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase highlight_keys = @user.highlights.collect(&:key) @user.highlights.detach - assert_not @user.highlights.attached? + assert_not_predicate @user.highlights, :attached? assert ActiveStorage::Blob.exists?(highlight_blob_ids.first) assert ActiveStorage::Blob.exists?(highlight_blob_ids.second) @@ -348,7 +348,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase highlight_keys = @user.highlights.collect(&:key) @user.highlights.purge - assert_not @user.highlights.attached? + assert_not_predicate @user.highlights, :attached? assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) assert_not ActiveStorage::Blob.service.exist?(highlight_keys.second) end diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index b5daee2b57..779e47ffb6 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -2,8 +2,28 @@ require "test_helper" require "database/setup" +require "active_support/testing/method_call_assertions" class ActiveStorage::BlobTest < ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions + + test ".unattached scope returns not attached blobs" do + class UserWithHasOneAttachedDependentFalse < User + has_one_attached :avatar, dependent: false + end + + ActiveStorage::Blob.delete_all + blob_1 = create_blob filename: "funky.jpg" + blob_2 = create_blob filename: "town.jpg" + + user = UserWithHasOneAttachedDependentFalse.create! + user.avatar.attach blob_1 + + assert_equal [blob_2], ActiveStorage::Blob.unattached + user.destroy + assert_equal [blob_1, blob_2].map(&:id).sort, ActiveStorage::Blob.unattached.pluck(:id).sort + end + test "create after upload sets byte size and checksum" do data = "Hello world!" blob = create_blob data: data @@ -23,10 +43,22 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase assert_equal "text/plain", blob.content_type end + test "image?" do + blob = create_file_blob filename: "racecar.jpg" + assert_predicate blob, :image? + assert_not_predicate blob, :audio? + end + + test "video?" do + blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") + assert_predicate blob, :video? + assert_not_predicate blob, :audio? + end + test "text?" do blob = create_blob data: "Hello world!" - assert blob.text? - assert_not blob.audio? + assert_predicate blob, :text? + assert_not_predicate blob, :audio? end test "download yields chunks" do @@ -67,6 +99,25 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase freeze_time do assert_equal expected_url_for(blob), blob.service_url assert_equal expected_url_for(blob, filename: new_filename), blob.service_url(filename: new_filename) + assert_equal expected_url_for(blob, filename: new_filename), blob.service_url(filename: "new.txt") + assert_equal expected_url_for(blob, filename: blob.filename), blob.service_url(filename: nil) + end + end + + test "urls allow for custom options" do + blob = create_blob(filename: "original.txt") + + options = [ + blob.key, + expires_in: blob.service.url_expires_in, + disposition: :inline, + content_type: blob.content_type, + filename: blob.filename, + thumb_size: "300x300", + thumb_mode: "crop" + ] + assert_called_with(blob.service, :url, options) do + blob.service_url(thumb_size: "300x300", thumb_mode: "crop") end end diff --git a/activestorage/test/models/preview_test.rb b/activestorage/test/models/preview_test.rb index b0c6d2c45b..55fdc228c8 100644 --- a/activestorage/test/models/preview_test.rb +++ b/activestorage/test/models/preview_test.rb @@ -8,7 +8,7 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf") preview = blob.preview(resize: "640x280").processed - assert preview.image.attached? + assert_predicate preview.image, :attached? assert_equal "report.png", preview.image.filename.to_s assert_equal "image/png", preview.image.content_type @@ -21,7 +21,7 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") preview = blob.preview(resize: "640x280").processed - assert preview.image.attached? + assert_predicate preview.image, :attached? assert_equal "video.png", preview.image.filename.to_s assert_equal "image/png", preview.image.content_type diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 0cf8a583bd..0f3ada25c0 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -50,6 +50,17 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_equal 20, image.height end + test "resized variation of ICO blob" do + blob = create_file_blob(filename: "favicon.ico", content_type: "image/vnd.microsoft.icon") + variant = blob.variant(resize: "20x20").processed + assert_match(/icon\.png/, variant.service_url) + + image = read_image(variant) + assert_equal "PNG", image.type + assert_equal 20, image.width + assert_equal 20, image.height + end + test "optimized variation of GIF blob" do blob = create_file_blob(filename: "image.gif", content_type: "image/gif") diff --git a/activestorage/test/service/configurator_test.rb b/activestorage/test/service/configurator_test.rb index 0a79b37aa4..fe8a637ad0 100644 --- a/activestorage/test/service/configurator_test.rb +++ b/activestorage/test/service/configurator_test.rb @@ -4,7 +4,7 @@ require "service/shared_service_tests" class ActiveStorage::Service::ConfiguratorTest < ActiveSupport::TestCase test "builds correct service instance based on service name" do - service = ActiveStorage::Service::Configurator.build(:foo, foo: { service: "Disk", root: "path", host: "http://localhost:3000" }) + service = ActiveStorage::Service::Configurator.build(:foo, foo: { service: "Disk", root: "path" }) assert_instance_of ActiveStorage::Service::DiskService, service assert_equal "path", service.root diff --git a/activestorage/test/service/disk_service_test.rb b/activestorage/test/service/disk_service_test.rb index f0171f8999..4a6361b920 100644 --- a/activestorage/test/service/disk_service_test.rb +++ b/activestorage/test/service/disk_service_test.rb @@ -3,7 +3,7 @@ require "service/shared_service_tests" class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase - SERVICE = ActiveStorage::Service::DiskService.new(root: File.join(Dir.tmpdir, "active_storage"), host: "http://localhost:3000") + SERVICE = ActiveStorage::Service::DiskService.new(root: File.join(Dir.tmpdir, "active_storage")) include ActiveStorage::Service::SharedServiceTests diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb index 6b601f3b2b..87306644c5 100644 --- a/activestorage/test/service/mirror_service_test.rb +++ b/activestorage/test/service/mirror_service_test.rb @@ -6,13 +6,12 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase mirror_config = (1..3).map do |i| [ "mirror_#{i}", service: "Disk", - root: Dir.mktmpdir("active_storage_tests_mirror_#{i}"), - host: "http://localhost:3000" ] + root: Dir.mktmpdir("active_storage_tests_mirror_#{i}") ] end.to_h config = mirror_config.merge \ mirror: { service: "Mirror", primary: "primary", mirrors: mirror_config.keys }, - primary: { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary"), host: "http://localhost:3000" } + primary: { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary") } SERVICE = ActiveStorage::Service.configure :mirror, config @@ -20,11 +19,16 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase test "uploading to all services" do begin - data = "Something else entirely!" - key = upload(data, to: @service) + key = SecureRandom.base58(24) + data = "Something else entirely!" + io = StringIO.new(data) + checksum = Digest::MD5.base64digest(data) - assert_equal data, SERVICE.primary.download(key) - SERVICE.mirrors.each do |mirror| + @service.upload key, io.tap(&:read), checksum: checksum + assert_predicate io, :eof? + + assert_equal data, @service.primary.download(key) + @service.mirrors.each do |mirror| assert_equal data, mirror.download(key) end ensure @@ -33,14 +37,18 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase end test "downloading from primary service" do - data = "Something else entirely!" - key = upload(data, to: SERVICE.primary) + key = SecureRandom.base58(24) + data = "Something else entirely!" + checksum = Digest::MD5.base64digest(data) + + @service.primary.upload key, StringIO.new(data), checksum: checksum assert_equal data, @service.download(key) end test "deleting from all services" do @service.delete FIXTURE_KEY + assert_not SERVICE.primary.exist?(FIXTURE_KEY) SERVICE.mirrors.each do |mirror| assert_not mirror.exist?(FIXTURE_KEY) @@ -51,17 +59,8 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase filename = ActiveStorage::Filename.new("test.txt") freeze_time do - assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain"), + assert_equal @service.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain"), @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: filename, content_type: "text/plain") end end - - private - def upload(data, to:) - SecureRandom.base58(24).tap do |key| - io = StringIO.new(data).tap(&:read) - @service.upload key, io, checksum: Digest::MD5.base64digest(data) - assert io.eof? - end - end end diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index c3818422aa..d6996209d2 100644 --- a/activestorage/test/service/s3_service_test.rb +++ b/activestorage/test/service/s3_service_test.rb @@ -35,7 +35,7 @@ if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].pr url = @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png") - assert_match(/s3\.(\S+)?amazonaws.com.*response-content-disposition=inline.*avatar\.png.*response-content-type=image%2Fpng/, url) + assert_match(/s3(-[-a-z0-9]+)?\.(\S+)?amazonaws.com.*response-content-disposition=inline.*avatar\.png.*response-content-type=image%2Fpng/, url) assert_match SERVICE_CONFIGURATIONS[:s3][:bucket], url end diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb index 80f183e0e3..f0b166c225 100644 --- a/activestorage/test/template/image_tag_test.rb +++ b/activestorage/test/template/image_tag_test.rb @@ -33,7 +33,7 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase test "error when attachment's empty" do @user = User.create!(name: "DHH") - assert_not @user.avatar.attached? + assert_not_predicate @user.avatar, :attached? assert_raises(ArgumentError) { image_tag(@user.avatar) } end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 51f8ebad18..98fa44a604 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -34,7 +34,7 @@ rescue Errno::ENOENT end require "tmpdir" -ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: Dir.mktmpdir("active_storage_tests"), host: "http://localhost:3000") +ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: Dir.mktmpdir("active_storage_tests")) ActiveStorage.logger = ActiveSupport::Logger.new(nil) ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing") |
