diff options
20 files changed, 154 insertions, 49 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 0408ca4aa1..cec4fcc71a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,6 +16,7 @@ Style/AndOr: # method call. Style/BracesAroundHashParameters: Enabled: true + EnforcedStyle: context_dependent # Align `when` with `case`. Layout/CaseIndentation: @@ -28,10 +29,6 @@ Layout/CommentIndentation: Layout/EmptyLineAfterMagicComment: Enabled: true -# No extra empty lines. -Layout/EmptyLines: - Enabled: true - # In a regular class definition, no empty lines around the body. Layout/EmptyLinesAroundClassBody: Enabled: true diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index 41af6b6e3a..8b6322f8ad 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -9,7 +9,6 @@ require_relative "../model_naming" require_relative "../record_identifier" require "active_support/core_ext/module/attribute_accessors" require "active_support/core_ext/hash/slice" -require "active_support/core_ext/hash/compact" require "active_support/core_ext/string/output_safety" require "active_support/core_ext/string/inflections" @@ -1194,7 +1193,7 @@ module ActionView # file_field(:attachment, :file, class: 'file_input') # # => <input type="file" id="attachment_file" name="attachment[file]" class="file_input" /> def file_field(object_name, method, options = {}) - Tags::FileField.new(object_name, method, self, convert_direct_upload_option_to_url(options)).render + Tags::FileField.new(object_name, method, self, convert_direct_upload_option_to_url(options.dup)).render end # Returns a textarea opening and closing tag set tailored for accessing a specified attribute (identified by +method+) @@ -2317,10 +2316,6 @@ module ActionView options[:include_id] = !options.delete(:skip_id) end end - - def convert_direct_upload_option_to_url(options) - options.merge('data-direct-upload-url': options.delete(:direct_upload) ? rails_direct_uploads_url : nil).compact - end end end diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 83b600d871..2519ff2837 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -906,7 +906,10 @@ module ActionView end def convert_direct_upload_option_to_url(options) - options.merge('data-direct-upload-url': options.delete(:direct_upload) ? rails_direct_uploads_url : nil).compact + if options.delete(:direct_upload) && respond_to?(:rails_direct_uploads_url) + options["data-direct-upload-url"] = rails_direct_uploads_url + end + options end end end diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index efe8c87b9b..6913c31a20 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -281,6 +281,18 @@ module ActionView super end end + + def respond_to_missing?(name, include_private = false) + begin + routes = @controller.respond_to?(:_routes) && @controller._routes + rescue + # Dont call routes, if there is an error on _routes call + end + + routes && + (routes.named_routes.route_defined?(name) || + routes.mounted_helpers.method_defined?(name)) + end end include Behavior diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb index 8d689aebeb..246f52588d 100644 --- a/actionview/test/template/form_helper_test.rb +++ b/actionview/test/template/form_helper_test.rb @@ -8,6 +8,16 @@ class FormHelperTest < ActionView::TestCase tests ActionView::Helpers::FormHelper + class WithActiveStorageRoutesControllers < ActionController::Base + test_routes do + post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads + end + + def url_options + { host: "testtwo.host" } + end + end + def form_for(*) @output_buffer = super end @@ -542,6 +552,27 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, file_field("import", "file", multiple: true, name: "custom") end + def test_file_field_with_direct_upload_when_rails_direct_uploads_url_is_not_defined + expected = '<input type="file" name="import[file]" id="import_file" />' + assert_dom_equal expected, file_field("import", "file", direct_upload: true) + end + + def test_file_field_with_direct_upload_when_rails_direct_uploads_url_is_defined + @controller = WithActiveStorageRoutesControllers.new + + expected = '<input data-direct-upload-url="http://testtwo.host/rails/active_storage/direct_uploads" type="file" name="import[file]" id="import_file" />' + assert_dom_equal expected, file_field("import", "file", direct_upload: true) + end + + def test_file_field_with_direct_upload_dont_mutate_arguments + original_options = { class: "pix", direct_upload: true } + + expected = '<input class="pix" type="file" name="import[file]" id="import_file" />' + assert_dom_equal expected, file_field("import", "file", original_options) + + assert_equal({ class: "pix", direct_upload: true }, original_options) + end + def test_hidden_field assert_dom_equal( '<input id="post_title" name="post[title]" type="hidden" value="Hello World" />', diff --git a/actionview/test/template/form_tag_helper_test.rb b/actionview/test/template/form_tag_helper_test.rb index 62b67b0435..b985b9789b 100644 --- a/actionview/test/template/form_tag_helper_test.rb +++ b/actionview/test/template/form_tag_helper_test.rb @@ -7,6 +7,16 @@ class FormTagHelperTest < ActionView::TestCase tests ActionView::Helpers::FormTagHelper + class WithActiveStorageRoutesControllers < ActionController::Base + test_routes do + post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads + end + + def url_options + { host: "testtwo.host" } + end + end + def setup super @controller = BasicController.new @@ -178,6 +188,33 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal "<input name=\"picsplz\" type=\"file\" id=\"picsplz\" class=\"pix\"/>", file_field_tag("picsplz", class: "pix") end + def test_file_field_tag_with_direct_upload_when_rails_direct_uploads_url_is_not_defined + assert_dom_equal( + "<input name=\"picsplz\" type=\"file\" id=\"picsplz\" class=\"pix\"/>", + file_field_tag("picsplz", class: "pix", direct_upload: true) + ) + end + + def test_file_field_tag_with_direct_upload_when_rails_direct_uploads_url_is_defined + @controller = WithActiveStorageRoutesControllers.new + + assert_dom_equal( + "<input name=\"picsplz\" type=\"file\" id=\"picsplz\" class=\"pix\" data-direct-upload-url=\"http://testtwo.host/rails/active_storage/direct_uploads\"/>", + file_field_tag("picsplz", class: "pix", direct_upload: true) + ) + end + + def test_file_field_tag_with_direct_upload_dont_mutate_arguments + original_options = { class: "pix", direct_upload: true } + + assert_dom_equal( + "<input name=\"picsplz\" type=\"file\" id=\"picsplz\" class=\"pix\"/>", + file_field_tag("picsplz", original_options) + ) + + assert_equal({ class: "pix", direct_upload: true }, original_options) + end + def test_password_field_tag actual = password_field_tag expected = %(<input id="password" name="password" type="password" />) diff --git a/activestorage/README.md b/activestorage/README.md index f868e0d708..22e77e2837 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -88,7 +88,8 @@ Variation of image attachment: 1. Run `rails activestorage:install` to create needed directories, migrations, and configuration. 2. Optional: Add `gem "aws-sdk", "~> 2"` to your Gemfile if you want to use AWS S3. 3. Optional: Add `gem "google-cloud-storage", "~> 1.3"` to your Gemfile if you want to use Google Cloud Storage. -4. Optional: Add `gem "mini_magick"` to your Gemfile if you want to use variants. +4. Optional: Add `gem "azure-storage"` to your Gemfile if you want to use Microsoft Azure. +5. Optional: Add `gem "mini_magick"` to your Gemfile if you want to use variants. ## Direct uploads diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 2d4a1f7569..911e1a0469 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -27,4 +27,4 @@ Gem::Specification.new do |s| s.add_dependency "actionpack", version s.add_dependency "activerecord", version -end
\ No newline at end of file +end diff --git a/activestorage/app/controllers/active_storage/variants_controller.rb b/activestorage/app/controllers/active_storage/variants_controller.rb index aa38f8e928..994c57aafd 100644 --- a/activestorage/app/controllers/active_storage/variants_controller.rb +++ b/activestorage/app/controllers/active_storage/variants_controller.rb @@ -1,5 +1,3 @@ -require "active_storage/variant" - # Take a signed permanent reference for a variant and turn it into an expiring service URL for download. # Note: These URLs are publicly accessible. If you need to enforce access protection beyond the # security-through-obscurity factor of the signed blob and variation reference, you'll need to implement your own diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 2c8b7a9cf2..e94fc69bba 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -1,7 +1,6 @@ -require "active_storage/blob" require "active_support/core_ext/module/delegation" -# Attachments associate records with blobs. Usually that's a one record-many blobs relationship, +# Attachments associate records with blobs. Usually that's a one record-many blobs relationship, # but it is possible to associate many different records with the same blob. If you're doing that, # you'll want to declare with `has_one/many_attached :thingy, dependent: false`, so that destroying # any one record won't destroy the blob as well. (Then you'll need to do your own garbage collecting, though). diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 9208d36ee3..debc62bd41 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -1,9 +1,3 @@ -require "active_storage/service" -require "active_storage/filename" -require "active_storage/purge_job" -require "active_storage/variant" -require "active_storage/variation" - # 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: # @@ -85,16 +79,24 @@ class ActiveStorage::Blob < ActiveRecord::Base end # Returns true if the content_type of this blob is in the image range, like image/png. - def image?() content_type =~ /^image/ end + def image? + content_type =~ /^image/ + end # Returns true if the content_type of this blob is in the audio range, like audio/mpeg. - def audio?() content_type =~ /^audio/ end + def audio? + content_type =~ /^audio/ + end # Returns true if the content_type of this blob is in the video range, like video/mp4. - def video?() content_type =~ /^video/ end + def video? + content_type =~ /^video/ + end # Returns true if the content_type of this blob is in the text range, like text/plain. - def text?() content_type =~ /^text/ end + def text? + content_type =~ /^text/ + end # Returns a `ActiveStorage::Variant` instance with the set of `transformations` passed in. This is only relevant # for image files, and it allows any image to be transformed for size, colors, and the like. Example: diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 34b854fd9f..e784506b4c 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -1,6 +1,6 @@ require "active_support/core_ext/object/inclusion" -# A set of transformations that can be applied to a blob to create a variant. This class is exposed via +# A set of transformations that can be applied to a blob to create a variant. This class is exposed via # the `ActiveStorage::Blob#variant` method and should rarely be used directly. # # In case you do need to use this directly, it's instantiated using a hash of transformations where diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index 6aa99c42ac..fddbb93255 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -9,7 +9,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Attachment") { |attachment| route_for(:rails_blob, attachment.blob) } - get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation + get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation direct :rails_variant do |variant| signed_blob_id = variant.blob.signed_id diff --git a/activestorage/lib/active_storage/attached.rb b/activestorage/lib/active_storage/attached.rb index 4644d74bcc..2dbf841864 100644 --- a/activestorage/lib/active_storage/attached.rb +++ b/activestorage/lib/active_storage/attached.rb @@ -1,6 +1,4 @@ -require "active_storage/blob" -require "active_storage/attachment" - +require "action_dispatch" require "action_dispatch/http/upload" require "active_support/core_ext/module/delegation" diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 89e61163f5..1d345920fa 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -27,7 +27,9 @@ module ActiveStorage initializer "active_storage.verifier" do config.after_initialize do |app| - ActiveStorage.verifier = app.message_verifier("ActiveStorage") + if app.config.secret_key_base.present? + ActiveStorage.verifier = app.message_verifier("ActiveStorage") + end end end diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 35b0909297..c75c1caabe 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -81,8 +81,10 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service content_length: content_length, checksum: checksum }, - expires_in: expires_in, - purpose: :blob_token + { + expires_in: expires_in, + purpose: :blob_token + } ) generated_url = diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb index 23b94f25ed..ec80cbce61 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 end test "uploading with server-side encryption" do - config = SERVICE_CONFIGURATIONS.deep_merge(s3: { upload: { server_side_encryption: "AES256" }}) + config = SERVICE_CONFIGURATIONS.deep_merge(s3: { upload: { server_side_encryption: "AES256" } }) service = ActiveStorage::Service.configure(:s3, config) begin diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 9f62ca8eb8..381bc2694f 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1452,8 +1452,8 @@ module ApplicationTests test "raises with proper error message if no database configuration found" do FileUtils.rm("#{app_path}/config/database.yml") - app "development" err = assert_raises RuntimeError do + app "development" Rails.application.config.database_configuration end assert_match "config/database", err.message diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index c75a25bc6f..1c5055a413 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -115,11 +115,11 @@ class LoadingTest < ActiveSupport::TestCase require "#{rails_root}/config/environment" setup_ar! - assert_equal [ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata], ActiveRecord::Base.descendants + assert_equal [ActiveStorage::Blob, ActiveStorage::Attachment, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata], ActiveRecord::Base.descendants get "/load" - assert_equal [ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata, Post], ActiveRecord::Base.descendants + assert_equal [ActiveStorage::Blob, ActiveStorage::Attachment, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata, Post], ActiveRecord::Base.descendants get "/unload" - assert_equal [ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata], ActiveRecord::Base.descendants + assert_equal [ActiveStorage::Blob, ActiveStorage::Attachment, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata], ActiveRecord::Base.descendants end test "initialize cant be called twice" do diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 134106812d..6340f8f7b1 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -129,7 +129,15 @@ module ApplicationTests RUBY output = Dir.chdir(app_path) { `bin/rails routes` } - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal <<-MESSAGE.strip_heredoc, output + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + MESSAGE end def test_singular_resource_output_in_rake_routes @@ -162,10 +170,20 @@ module ApplicationTests RUBY output = Dir.chdir(app_path) { `bin/rails routes -g show` } - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + assert_equal <<-MESSAGE.strip_heredoc, output + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + MESSAGE output = Dir.chdir(app_path) { `bin/rails routes -g POST` } - assert_equal "Prefix Verb URI Pattern Controller#Action\n POST /cart(.:format) cart#create\n", output + assert_equal <<-MESSAGE.strip_heredoc, output + Prefix Verb URI Pattern Controller#Action + POST /cart(.:format) cart#create + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + MESSAGE output = Dir.chdir(app_path) { `bin/rails routes -g basketballs` } assert_equal " Prefix Verb URI Pattern Controller#Action\n" \ @@ -221,11 +239,12 @@ module ApplicationTests RUBY assert_equal <<-MESSAGE.strip_heredoc, Dir.chdir(app_path) { `bin/rails routes` } - You don't have any routes defined! - - Please add some routes in config/routes.rb. - - For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. + Prefix Verb URI Pattern Controller#Action + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create MESSAGE end @@ -237,7 +256,16 @@ module ApplicationTests RUBY output = Dir.chdir(app_path) { `bin/rake --rakefile Rakefile routes` } - assert_equal "Prefix Verb URI Pattern Controller#Action\n cart GET /cart(.:format) cart#show\n", output + + assert_equal <<-MESSAGE.strip_heredoc, output + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show + rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show + rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show + rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update + rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + MESSAGE end def test_logger_is_flushed_when_exiting_production_rake_tasks |