diff options
40 files changed, 305 insertions, 92 deletions
diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 99d0c06751..393141535b 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -69,6 +69,9 @@ module ActionDispatch # size of the browser screen. These two options are not applicable for # headless drivers and will be silently ignored if passed. # + # Headless browsers such as headless Chrome and headless Firefox are also supported. + # You can use these browsers by setting the +:using+ argument to +:headless_chrome+ or +:headless_firefox+. + # # To use a headless driver, like Poltergeist, update your Gemfile to use # Poltergeist instead of Selenium and then declare the driver name in the # +application_system_test_case.rb+ file. In this case, you would leave out diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c88a4e7695..73970c99d0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Using subselect for `delete_all` with `limit` or `offset`. + + *Ryuta Kamizono* + +* Undefine attribute methods on descendants when resetting column + information. + + *Chris Salzberg* + * Log database query callers Add `verbose_query_logs` configuration option to display the caller @@ -548,4 +557,5 @@ *Kevin McPhillips* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 8569c76317..bff2f51efd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -717,7 +717,8 @@ module ActiveRecord # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on' subselect.distinct unless select.limit || select.offset || select.orders.any? - Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key.name)) + key_name = quote_column_name(key.name) + Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key_name)) end def supports_rename_index? diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 773b0f6cde..fa7537c1a3 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -425,7 +425,7 @@ module ActiveRecord # end def reset_column_information connection.clear_cache! - undefine_attribute_methods + ([self] + descendants).each(&:undefine_attribute_methods) connection.schema_cache.clear_data_source_cache!(table_name) reload_schema_from_cache diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index dd0d52ad1c..4df3864d07 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -10,7 +10,7 @@ module ActiveRecord SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, :reverse_order, :distinct, :create_with, :skip_query_cache] CLAUSE_METHODS = [:where, :having, :from] - INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] + INVALID_METHODS_FOR_DELETE_ALL = [:distinct, :group, :having] VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS @@ -317,7 +317,7 @@ module ActiveRecord stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates)) stmt.table(table) - if has_join_values? + if has_join_values? || offset_value @klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key)) else stmt.key = arel_attribute(primary_key) @@ -365,8 +365,8 @@ module ActiveRecord # # If an invalid method is supplied, #delete_all raises an ActiveRecordError: # - # Post.limit(100).delete_all - # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit + # Post.distinct.delete_all + # # => ActiveRecord::ActiveRecordError: delete_all doesn't support distinct def delete_all invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method| value = get_value(method) @@ -384,7 +384,7 @@ module ActiveRecord stmt = Arel::DeleteManager.new stmt.from(table) - if has_join_values? + if has_join_values? || has_limit_or_offset? @klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key)) else stmt.wheres = arel.constraints diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6b3ff3d610..2056f9bb73 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1040,8 +1040,8 @@ module ActiveRecord def build_select(arel) if select_values.any? arel.project(*arel_columns(select_values.uniq)) - elsif @klass.ignored_columns.any? - arel.project(*arel_columns(@klass.column_names.map(&:to_sym))) + elsif klass.ignored_columns.any? + arel.project(*klass.column_names.map { |field| arel_attribute(field) }) else arel.project(table[Arel.star]) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7fc4a396e4..b076b452e7 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1498,10 +1498,14 @@ class BasicsTest < ActiveRecord::TestCase test "column names are quoted when using #from clause and model has ignored columns" do refute_empty Developer.ignored_columns - query = Developer.from("`developers`").to_sql - quoted_id = Developer.connection.quote_table_name("id") + query = Developer.from("developers").to_sql + quoted_id = "#{Developer.quoted_table_name}.#{Developer.quoted_primary_key}" - assert_match(/SELECT #{quoted_id}.* FROM `developers`/, query) + assert_match(/SELECT #{quoted_id}.* FROM developers/, query) + end + + test "using table name qualified column names unless having SELECT list explicitly" do + assert_equal developers(:david), Developer.from("developers").joins(:shared_computers).take end test "protected environments by default is an array with production" do diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 437a5a38a3..3701be4b11 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -406,7 +406,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, car.lock_version previously_car_updated_at = car.updated_at - travel(1.second) do + travel(2.second) do Wheel.create!(wheelable: car) end @@ -422,7 +422,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 2, car.lock_version previously_car_updated_at = car.updated_at - travel(1.second) do + travel(2.second) do car.wheels.first.destroy! end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index c0b9a3ef76..07c4a17fb1 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -59,13 +59,42 @@ class PersistenceTest < ActiveRecord::TestCase def test_update_all_with_order_and_limit_updates_subset_only author = authors(:david) - assert_nothing_raised do - assert_equal 1, author.posts_sorted_by_id_limited.size - assert_equal 2, author.posts_sorted_by_id_limited.limit(2).to_a.size - assert_equal 1, author.posts_sorted_by_id_limited.update_all([ "body = ?", "bulk update!" ]) - assert_equal "bulk update!", posts(:welcome).body - assert_not_equal "bulk update!", posts(:thinking).body - end + limited_posts = author.posts_sorted_by_id_limited + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.update_all([ "body = ?", "bulk update!" ]) + assert_equal "bulk update!", posts(:welcome).body + assert_not_equal "bulk update!", posts(:thinking).body + end + + def test_update_all_with_order_and_limit_and_offset_updates_subset_only + author = authors(:david) + limited_posts = author.posts_sorted_by_id_limited.offset(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.update_all([ "body = ?", "bulk update!" ]) + assert_equal "bulk update!", posts(:thinking).body + assert_not_equal "bulk update!", posts(:welcome).body + end + + def test_delete_all_with_order_and_limit_deletes_subset_only + author = authors(:david) + limited_posts = Post.where(author: author).order(:id).limit(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.delete_all + assert_raise(ActiveRecord::RecordNotFound) { posts(:welcome) } + assert posts(:thinking) + end + + def test_delete_all_with_order_and_limit_and_offset_deletes_subset_only + author = authors(:david) + limited_posts = Post.where(author: author).order(:id).limit(1).offset(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.delete_all + assert_raise(ActiveRecord::RecordNotFound) { posts(:thinking) } + assert posts(:welcome) end end @@ -1106,13 +1135,18 @@ class PersistenceTest < ActiveRecord::TestCase end def test_reset_column_information_resets_children - child = Class.new(Topic) - child.new # force schema to load + child_class = Class.new(Topic) + child_class.new # force schema to load ActiveRecord::Base.connection.add_column(:topics, :foo, :string) Topic.reset_column_information - assert_equal "bar", child.new(foo: :bar).foo + # this should redefine attribute methods + child_class.new + + assert child_class.instance_methods.include?(:foo) + assert child_class.instance_methods.include?(:foo_changed?) + assert_equal "bar", child_class.new(foo: :bar).foo ensure ActiveRecord::Base.connection.remove_column(:topics, :foo) Topic.reset_column_information diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 675aafabda..915ff03c2c 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -896,11 +896,9 @@ class RelationTest < ActiveRecord::TestCase end def test_delete_all_with_unpermitted_relation_raises_error - assert_raises(ActiveRecord::ActiveRecordError) { Author.limit(10).delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.distinct.delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.group(:name).delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.having("SUM(id) < 3").delete_all } - assert_raises(ActiveRecord::ActiveRecordError) { Author.offset(10).delete_all } end def test_select_with_aggregates diff --git a/activerecord/test/cases/reserved_word_test.rb b/activerecord/test/cases/reserved_word_test.rb index 0214dbec17..4f8ca392b9 100644 --- a/activerecord/test/cases/reserved_word_test.rb +++ b/activerecord/test/cases/reserved_word_test.rb @@ -39,7 +39,7 @@ class ReservedWordTest < ActiveRecord::TestCase t.string :order t.belongs_to :select end - @connection.create_table :values, force: true do |t| + @connection.create_table :values, primary_key: :as, force: true do |t| t.belongs_to :group end end @@ -88,6 +88,13 @@ class ReservedWordTest < ActiveRecord::TestCase assert_equal x, Group.find(x.id) end + def test_delete_all_with_subselect + create_test_fixtures :values + assert_equal 1, Values.order(:as).limit(1).offset(1).delete_all + assert_raise(ActiveRecord::RecordNotFound) { Values.find(2) } + assert Values.find(1) + end + def test_has_one_associations create_test_fixtures :group, :values v = Group.find(1).values diff --git a/activerecord/test/fixtures/reserved_words/values.yml b/activerecord/test/fixtures/reserved_words/values.yml index 7d109609ab..9ed9e5edc5 100644 --- a/activerecord/test/fixtures/reserved_words/values.yml +++ b/activerecord/test/fixtures/reserved_words/values.yml @@ -1,7 +1,7 @@ values1: - id: 1 + as: 1 group_id: 2 values2: - id: 2 + as: 2 group_id: 1 diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb index fa5aa69bd3..bdcb5e6a41 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -1,5 +1,7 @@ # 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. @@ -35,6 +37,10 @@ # # avatar.variant(resize: "100x100", monochrome: true, flip: "-90") class ActiveStorage::Variant + include ActiveStorage::Downloading + + WEB_IMAGE_CONTENT_TYPES = %w( image/png image/jpeg image/jpg image/gif ) + attr_reader :blob, :variation delegate :service, to: :blob @@ -62,7 +68,7 @@ class ActiveStorage::Variant # for a variant that points to the ActiveStorage::VariantsController, which in turn will use this +service_call+ method # for its redirection. def service_url(expires_in: service.url_expires_in, disposition: :inline) - service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type + service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type end # Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be used interchangeably. @@ -76,11 +82,51 @@ class ActiveStorage::Variant end def process - service.upload key, transform(service.download(blob.key)) + open_image do |image| + transform image + format image + upload image + end + end + + + def filename + if WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) + blob.filename + else + ActiveStorage::Filename.new("#{blob.filename.base}.png") + end + end + + def content_type + blob.content_type.presence_in(WEB_IMAGE_CONTENT_TYPES) || "image/png" + end + + + def open_image(&block) + image = download_image + + begin + yield image + ensure + image.destroy! + end end - def transform(io) + def download_image require "mini_magick" - File.open MiniMagick::Image.read(io).tap { |image| variation.transform(image) }.path + MiniMagick::Image.create { |file| download_blob_to(file) } + end + + def transform(image) + variation.transform(image) + end + + def format(image) + image.format("PNG") unless WEB_IMAGE_CONTENT_TYPES.include?(blob.content_type) + end + + def upload(image) + File.open(image.path, "r") { |file| service.upload(key, file) } end end diff --git a/activestorage/test/fixtures/files/icon.psd b/activestorage/test/fixtures/files/icon.psd Binary files differnew file mode 100644 index 0000000000..631fceeaab --- /dev/null +++ b/activestorage/test/fixtures/files/icon.psd diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 83ebce4446..8eced41ee0 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -4,12 +4,9 @@ require "test_helper" require "database/setup" class ActiveStorage::VariantTest < ActiveSupport::TestCase - setup do - @blob = create_file_blob filename: "racecar.jpg" - end - - test "resized variation" do - variant = @blob.variant(resize: "100x100").processed + test "resized variation of JPEG blob" do + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(resize: "100x100").processed assert_match(/racecar\.jpg/, variant.service_url) image = read_image(variant) @@ -17,8 +14,9 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_equal 67, image.height end - test "resized and monochrome variation" do - variant = @blob.variant(resize: "100x100", monochrome: true).processed + test "resized and monochrome variation of JPEG blob" do + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(resize: "100x100", monochrome: true).processed assert_match(/racecar\.jpg/, variant.service_url) image = read_image(variant) @@ -27,6 +25,17 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase assert_match(/Gray/, image.colorspace) end + test "resized variation of PSD blob" do + blob = create_file_blob(filename: "icon.psd", content_type: "image/vnd.adobe.photoshop") + 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 "variation of invariable blob" do assert_raises ActiveStorage::Blob::InvariableError do create_file_blob(filename: "report.pdf", content_type: "application/pdf").variant(resize: "100x100") @@ -34,7 +43,8 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end test "service_url doesn't grow in length despite long variant options" do - variant = @blob.variant(font: "a" * 10_000).processed + blob = create_file_blob(filename: "racecar.jpg") + variant = blob.variant(font: "a" * 10_000).processed assert_operator variant.service_url.length, :<, 500 end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index abbadd404f..fccaeb5d32 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,7 +1,7 @@ -* Introduced `ActiveSupport::Digest` that allows to specify hash function implementation - and defaults to `Digest::MD5`. +* Allow the hash function used to generate non-sensitive digests, such as the + ETag header, to be specified with `config.active_support.hash_digest_class`. - Replaced calls to `::Digest::MD5.hexdigest` with calls to `ActiveSupport::Digest.hexdigest`. + The object provided must respond to `#hexdigest`, e.g. `Digest::SHA1`. *Dmitri Dolguikh* diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index a77f903db5..4310df3024 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -115,11 +115,8 @@ class Module # invoice.customer_address # => 'Vimmersvej 13' # # If the target is +nil+ and does not respond to the delegated method a - # +Module::DelegationError+ is raised, as with any other value. Sometimes, - # however, it makes sense to be robust to that situation and that is the - # purpose of the <tt>:allow_nil</tt> option: If the target is not +nil+, or it - # is and responds to the method, everything works as usual. But if it is +nil+ - # and does not respond to the delegated method, +nil+ is returned. + # +Module::DelegationError+ is raised. If you wish to instead return +nil+, + # use the <tt>:allow_nil</tt> option. # # class User < ActiveRecord::Base # has_one :profile diff --git a/activesupport/lib/active_support/digest.rb b/activesupport/lib/active_support/digest.rb index d77eeb072b..fba10fbdcf 100644 --- a/activesupport/lib/active_support/digest.rb +++ b/activesupport/lib/active_support/digest.rb @@ -13,16 +13,8 @@ module ActiveSupport end def hexdigest(arg) - new.hexdigest(arg) + hash_digest_class.hexdigest(arg)[0...32] end end - - def initialize(digest_class: nil) - @digest_class = digest_class || self.class.hash_digest_class - end - - def hexdigest(arg) - @digest_class.hexdigest(arg).truncate(32) - end end end diff --git a/activesupport/lib/active_support/encrypted_configuration.rb b/activesupport/lib/active_support/encrypted_configuration.rb index c52d3869de..dab953d5d5 100644 --- a/activesupport/lib/active_support/encrypted_configuration.rb +++ b/activesupport/lib/active_support/encrypted_configuration.rb @@ -11,8 +11,9 @@ module ActiveSupport delegate :[], :fetch, to: :config delegate_missing_to :options - def initialize(config_path:, key_path:, env_key:) - super content_path: config_path, key_path: key_path, env_key: env_key + def initialize(config_path:, key_path:, env_key:, raise_if_missing_key:) + super content_path: config_path, key_path: key_path, + env_key: env_key, raise_if_missing_key: raise_if_missing_key end # Allow a config to be started without a file present diff --git a/activesupport/lib/active_support/encrypted_file.rb b/activesupport/lib/active_support/encrypted_file.rb index 3d1455fb95..671b6b6a69 100644 --- a/activesupport/lib/active_support/encrypted_file.rb +++ b/activesupport/lib/active_support/encrypted_file.rb @@ -26,11 +26,11 @@ module ActiveSupport end - attr_reader :content_path, :key_path, :env_key + attr_reader :content_path, :key_path, :env_key, :raise_if_missing_key - def initialize(content_path:, key_path:, env_key:) + def initialize(content_path:, key_path:, env_key:, raise_if_missing_key:) @content_path, @key_path = Pathname.new(content_path), Pathname.new(key_path) - @env_key = env_key + @env_key, @raise_if_missing_key = env_key, raise_if_missing_key end def key @@ -38,7 +38,7 @@ module ActiveSupport end def read - if content_path.exist? + if !key.nil? && content_path.exist? decrypt content_path.binread else raise MissingContentError, content_path @@ -93,7 +93,7 @@ module ActiveSupport end def handle_missing_key - raise MissingKeyError, key_path: key_path, env_key: env_key + raise MissingKeyError, key_path: key_path, env_key: env_key if raise_if_missing_key end end end diff --git a/activesupport/lib/active_support/railtie.rb b/activesupport/lib/active_support/railtie.rb index 3488721df9..91872e29c8 100644 --- a/activesupport/lib/active_support/railtie.rb +++ b/activesupport/lib/active_support/railtie.rb @@ -68,9 +68,9 @@ module ActiveSupport end initializer "active_support.set_hash_digest_class" do |app| - if app.config.active_support.respond_to?(:use_hash_digest_class) && app.config.active_support.use_hash_digest_class + if app.config.active_support.respond_to?(:hash_digest_class) && app.config.active_support.hash_digest_class ActiveSupport::Digest.hash_digest_class = - app.config.active_support.use_hash_digest_class + app.config.active_support.hash_digest_class end end end diff --git a/activesupport/test/digest_test.rb b/activesupport/test/digest_test.rb index 5dec75b9fe..83ff2a8d83 100644 --- a/activesupport/test/digest_test.rb +++ b/activesupport/test/digest_test.rb @@ -16,7 +16,7 @@ class DigestTest < ActiveSupport::TestCase digest = ActiveSupport::Digest.hexdigest("hello friend") assert_equal 32, digest.length - assert_equal ::Digest::SHA1.hexdigest("hello friend").truncate(32), digest + assert_equal ::Digest::SHA1.hexdigest("hello friend")[0...32], digest ensure ActiveSupport::Digest.hash_digest_class = original_hash_digest_class end diff --git a/activesupport/test/encrypted_configuration_test.rb b/activesupport/test/encrypted_configuration_test.rb index 0bc915be82..93ccf457de 100644 --- a/activesupport/test/encrypted_configuration_test.rb +++ b/activesupport/test/encrypted_configuration_test.rb @@ -10,8 +10,10 @@ class EncryptedConfigurationTest < ActiveSupport::TestCase @credentials_key_path = File.join(Dir.tmpdir, "master.key") File.write(@credentials_key_path, ActiveSupport::EncryptedConfiguration.generate_key) - @credentials = ActiveSupport::EncryptedConfiguration.new \ - config_path: @credentials_config_path, key_path: @credentials_key_path, env_key: "RAILS_MASTER_KEY" + @credentials = ActiveSupport::EncryptedConfiguration.new( + config_path: @credentials_config_path, key_path: @credentials_key_path, + env_key: "RAILS_MASTER_KEY", raise_if_missing_key: true + ) end teardown do diff --git a/activesupport/test/encrypted_file_test.rb b/activesupport/test/encrypted_file_test.rb index 7259726d08..ba3bbef903 100644 --- a/activesupport/test/encrypted_file_test.rb +++ b/activesupport/test/encrypted_file_test.rb @@ -12,8 +12,9 @@ class EncryptedFileTest < ActiveSupport::TestCase @key_path = File.join(Dir.tmpdir, "content.txt.key") File.write(@key_path, ActiveSupport::EncryptedFile.generate_key) - @encrypted_file = ActiveSupport::EncryptedFile.new \ - content_path: @content_path, key_path: @key_path, env_key: "CONTENT_KEY" + @encrypted_file = ActiveSupport::EncryptedFile.new( + content_path: @content_path, key_path: @key_path, env_key: "CONTENT_KEY", raise_if_missing_key: true + ) end teardown do @@ -47,4 +48,12 @@ class EncryptedFileTest < ActiveSupport::TestCase assert_equal "#{@content} and went by the lake", @encrypted_file.read end + + test "raise MissingKeyError when key is missing" do + assert_raise(ActiveSupport::EncryptedFile::MissingKeyError) do + ActiveSupport::EncryptedFile.new( + content_path: @content_path, key_path: "", env_key: "", raise_if_missing_key: true + ).read + end + end end diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index 069a624984..9be9c6c7b7 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -38,7 +38,7 @@ object on how to write to and read from the database. ### Object Relational Mapping -Object Relational Mapping, commonly referred to as its abbreviation ORM, is +[Object Relational Mapping](https://en.wikipedia.org/wiki/Object-relational_mapping), commonly referred to as its abbreviation ORM, is a technique that connects the rich objects of an application to tables in a relational database management system. Using ORM, the properties and relationships of the objects in an application can be easily stored and diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 7424818757..967c992c05 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -84,7 +84,9 @@ discussions new features require. Helping to Resolve Existing Issues ---------------------------------- -As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the [issues list](https://github.com/rails/rails/issues) in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: +As a next step beyond reporting issues, you can help the core team resolve existing ones by providing feedback about them. If you are new to Rails core development, that might be a great way to walk your first steps, you'll get familiar with the code base and the processes. + +If you check the [issues list](https://github.com/rails/rails/issues) in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: ### Verifying Bug Reports diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 293a736bfd..a200a1005c 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -472,7 +472,8 @@ module Rails ActiveSupport::EncryptedConfiguration.new( config_path: Rails.root.join(path), key_path: Rails.root.join(key_path), - env_key: env_key + env_key: env_key, + raise_if_missing_key: config.require_master_key ) end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index cbc04f8a48..5d8d6740c8 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -16,7 +16,8 @@ module Rails :ssl_options, :public_file_server, :session_options, :time_zone, :reload_classes_only_on_change, :beginning_of_week, :filter_redirect, :x, :enable_dependency_loading, - :read_encrypted_secrets, :log_level, :content_security_policy_report_only + :read_encrypted_secrets, :log_level, :content_security_policy_report_only, + :require_master_key attr_reader :encoding, :api_only @@ -56,6 +57,7 @@ module Rails @read_encrypted_secrets = false @content_security_policy = nil @content_security_policy_report_only = false + @require_master_key = false end def load_defaults(target_version) diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index 8085f07c2b..385d3976da 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -33,8 +33,7 @@ module Rails def show require_application_and_environment! - say Rails.application.credentials.read.presence || - "No credentials have been added yet. Use bin/rails credentials:edit to change that." + say Rails.application.credentials.read.presence || missing_credentials_message end private @@ -67,6 +66,14 @@ module Rails Rails::Generators::CredentialsGenerator.new end + + def missing_credentials_message + if Rails.application.credentials.key.nil? + "Missing master key to decrypt credentials. See bin/rails credentials:help" + else + "No credentials have been added yet. Use bin/rails credentials:edit to change that." + end + end end end end diff --git a/railties/lib/rails/commands/encrypted/encrypted_command.rb b/railties/lib/rails/commands/encrypted/encrypted_command.rb index 898094f1a4..912c453f09 100644 --- a/railties/lib/rails/commands/encrypted/encrypted_command.rb +++ b/railties/lib/rails/commands/encrypted/encrypted_command.rb @@ -37,9 +37,9 @@ module Rails def show(file_path) require_application_and_environment! + encrypted = Rails.application.encrypted(file_path, key_path: options[:key]) - say Rails.application.encrypted(file_path, key_path: options[:key]).read.presence || - "File '#{file_path}' does not exist. Use bin/rails encrypted:edit #{file_path} to change that." + say encrypted.read.presence || missing_encrypted_message(key: encrypted.key, key_path: options[:key], file_path: file_path) end private @@ -72,6 +72,14 @@ module Rails Rails::Generators::EncryptedFileGenerator.new end + + def missing_encrypted_message(key:, key_path:, file_path:) + if key.nil? + "Missing '#{key_path}' to decrypt data. See bin/rails encrypted:help" + else + "File '#{file_path}' does not exist. Use bin/rails encrypted:edit #{file_path} to change that." + end + end end end end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index b9ae24de59..400f954dcd 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -84,6 +84,9 @@ module Rails class_option :skip_system_test, type: :boolean, default: false, desc: "Skip system test files" + class_option :skip_bootsnap, type: :boolean, default: false, + desc: "Skip bootsnap gem" + class_option :dev, type: :boolean, default: false, desc: "Setup the #{name} with Gemfile pointing to your Rails checkout" @@ -435,6 +438,10 @@ module Rails !options[:skip_listen] && os_supports_listen_out_of_the_box? end + def depend_on_bootsnap? + !options[:skip_bootsnap] && !options[:dev] + end + def os_supports_listen_out_of_the_box? RbConfig::CONFIG["host_os"] =~ /darwin|linux/ end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index e3ed3e7c11..23bb89f4ce 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -29,9 +29,11 @@ ruby <%= "'#{RUBY_VERSION}'" -%> # Use Capistrano for deployment # gem 'capistrano-rails', group: :development +<% if depend_on_bootsnap? -%> # Reduces boot times through caching; required in config/boot.rb gem 'bootsnap', '>= 1.1.0', require: false +<%- end -%> <%- if options.api? -%> # Use Rack CORS for handling Cross-Origin Resource Sharing (CORS), making cross-origin AJAX possible # gem 'rack-cors' diff --git a/railties/lib/rails/generators/rails/app/templates/config/boot.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/boot.rb.tt index 6246e7bf85..720d36a2a4 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/boot.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/boot.rb.tt @@ -1,7 +1,9 @@ ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) require 'bundler/setup' # Set up gems listed in the Gemfile. +<% if depend_on_bootsnap? -%> require 'bootsnap/setup' # Speed up boot time by caching expensive operations. +<%- end -%> if %w[s server c console].any? { |a| ARGV.include?(a) } puts "=> Booting Rails" diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt index f630d9985a..8351d849ec 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt @@ -10,7 +10,7 @@ # This is needed for recyclable cache keys. # Rails.application.config.active_record.cache_versioning = true -# Use AES 256 GCM authenticated encryption for encrypted cookies. +# Use AES-256-GCM authenticated encryption for encrypted cookies. # Existing cookies will be converted on read then written with the new scheme. # Rails.application.config.action_dispatch.use_authenticated_cookie_encryption = true diff --git a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb index 01a5b502f9..9103b1122e 100644 --- a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb +++ b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb @@ -36,7 +36,8 @@ module Rails ActiveSupport::EncryptedConfiguration.new( config_path: "config/credentials.yml.enc", key_path: "config/master.key", - env_key: "RAILS_MASTER_KEY" + env_key: "RAILS_MASTER_KEY", + raise_if_missing_key: true ) end diff --git a/railties/lib/rails/generators/rails/encrypted_file/encrypted_file_generator.rb b/railties/lib/rails/generators/rails/encrypted_file/encrypted_file_generator.rb index ddce5f6fe2..4ce2fc1d86 100644 --- a/railties/lib/rails/generators/rails/encrypted_file/encrypted_file_generator.rb +++ b/railties/lib/rails/generators/rails/encrypted_file/encrypted_file_generator.rb @@ -24,7 +24,7 @@ module Rails def add_encrypted_file_silently(file_path, key_path, template = encrypted_file_template) unless File.exist?(file_path) - setup = { content_path: file_path, key_path: key_path, env_key: "RAILS_MASTER_KEY" } + setup = { content_path: file_path, key_path: key_path, env_key: "RAILS_MASTER_KEY", raise_if_missing_key: true } ActiveSupport::EncryptedFile.new(setup).write(template) end end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index d28f7ffc7f..ec745a397e 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -707,6 +707,14 @@ module ApplicationTests assert_match(/Missing.*RAILS_MASTER_KEY/, error) end + test "credentials does not raise error when require_master_key is false and master key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = false" + app "development" + + assert_not app.credentials.secret_key_base + end + test "protect from forgery is the default in a new app" do make_basic_app diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index f1bb1ef08a..7c464b3fde 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -26,10 +26,6 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - test "show credentials" do - assert_match(/access_key_id: 123/, run_show_command) - end - test "edit command does not add master key to gitignore when already exist" do run_edit_command @@ -47,6 +43,24 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase assert_match(/api_key: abc/, run_show_command) end + test "show credentials" do + assert_match(/access_key_id: 123/, run_show_command) + end + + test "show command raise error when require_master_key is specified and key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = true" + + assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true)) + end + + test "show command does not raise error when require_master_key is false and master key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = false" + + assert_match(/Missing master key to decrypt credentials/, run_show_command) + end + private def run_edit_command(editor: "cat") switch_env("EDITOR", editor) do @@ -54,7 +68,7 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - def run_show_command - rails "credentials:show" + def run_show_command(**options) + rails "credentials:show", **options end end diff --git a/railties/test/commands/encrypted_test.rb b/railties/test/commands/encrypted_test.rb index 0461493f2a..6647dcc902 100644 --- a/railties/test/commands/encrypted_test.rb +++ b/railties/test/commands/encrypted_test.rb @@ -52,6 +52,20 @@ class Rails::Command::EncryptedCommandTest < ActiveSupport::TestCase assert_match(/access_key_id: 123/, run_show_command("config/tokens.yml.enc", key: "config/tokens.key")) end + test "show command raise error when require_master_key is specified and key does not exist" do + add_to_config "config.require_master_key = true" + + assert_match(/Missing encryption key to decrypt file with/, + run_show_command("config/tokens.yml.enc", key: "unexist.key", allow_failure: true)) + end + + test "show command does not raise error when require_master_key is false and master key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = false" + + assert_match(/Missing 'config\/master\.key' to decrypt data/, run_show_command("config/tokens.yml.enc")) + end + test "won't corrupt encrypted file when passed wrong key" do run_edit_command("config/tokens.yml.enc", key: "config/tokens.key") @@ -68,8 +82,8 @@ class Rails::Command::EncryptedCommandTest < ActiveSupport::TestCase end end - def run_show_command(file, key: nil) - rails "encrypted:show", prepare_args(file, key) + def run_show_command(file, key: nil, **options) + rails "encrypted:show", prepare_args(file, key), **options end def prepare_args(file, key) diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 96803db838..110aca70c1 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -792,6 +792,37 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_bootsnap + run_generator + + assert_gem "bootsnap" + assert_file "config/boot.rb" do |content| + assert_match(/require 'bootsnap\/setup'/, content) + end + end + + def test_skip_bootsnap + run_generator [destination_root, "--skip-bootsnap"] + + assert_file "Gemfile" do |content| + assert_no_match(/bootsnap/, content) + end + assert_file "config/boot.rb" do |content| + assert_no_match(/require 'bootsnap\/setup'/, content) + end + end + + def test_bootsnap_with_dev_option + run_generator [destination_root, "--dev"] + + assert_file "Gemfile" do |content| + assert_no_match(/bootsnap/, content) + end + assert_file "config/boot.rb" do |content| + assert_no_match(/require 'bootsnap\/setup'/, content) + end + end + def test_inclusion_of_ruby_version run_generator |