diff options
27 files changed, 291 insertions, 74 deletions
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2dfde11707..b67a803b9d 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix to working before/after validation callbacks on multiple contexts. + + *Yoshiyuki Hirano* + ## Rails 5.2.0.beta2 (November 28, 2017) ## * No changes. diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index 4d0ab2a2fe..11a8b2b229 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -54,14 +54,16 @@ module ActiveModel # person.valid? # => true # person.name # => "bob" def before_validation(*args, &block) - options = args.last - if options.is_a?(Hash) && options[:on] - options[:if] = Array(options[:if]) - options[:on] = Array(options[:on]) + options = args.extract_options! + options[:if] = Array(options[:if]) + + if options.key?(:on) options[:if].unshift ->(o) { - options[:on].include? o.validation_context + !(Array(options[:on]) & Array(o.validation_context)).empty? } end + + args << options set_callback(:validation, :before, *args, &block) end @@ -95,13 +97,15 @@ module ActiveModel options = args.extract_options! options[:prepend] = true options[:if] = Array(options[:if]) - if options[:on] - options[:on] = Array(options[:on]) + + if options.key?(:on) options[:if].unshift ->(o) { - options[:on].include? o.validation_context + !(Array(options[:on]) & Array(o.validation_context)).empty? } end - set_callback(:validation, :after, *(args << options), &block) + + args << options + set_callback(:validation, :after, *args, &block) end end diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index d3a9a17a05..ff3cf61746 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -59,6 +59,18 @@ class DogValidatorWithOnCondition < Dog def set_after_validation_marker; history << "after_validation_marker" ; end end +class DogValidatorWithOnMultipleCondition < Dog + before_validation :set_before_validation_marker_on_context_a, on: :context_a + before_validation :set_before_validation_marker_on_context_b, on: :context_b + after_validation :set_after_validation_marker_on_context_a, on: :context_a + after_validation :set_after_validation_marker_on_context_b, on: :context_b + + def set_before_validation_marker_on_context_a; history << "before_validation_marker on context_a"; end + def set_before_validation_marker_on_context_b; history << "before_validation_marker on context_b"; end + def set_after_validation_marker_on_context_a; history << "after_validation_marker on context_a" ; end + def set_after_validation_marker_on_context_b; history << "after_validation_marker on context_b" ; end +end + class DogValidatorWithIfCondition < Dog before_validation :set_before_validation_marker1, if: -> { true } before_validation :set_before_validation_marker2, if: -> { false } @@ -98,6 +110,37 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase assert_equal [], d.history end + def test_on_multiple_condition_is_respected_for_validation_with_matching_context + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:context_a) + assert_equal ["before_validation_marker on context_a", "after_validation_marker on context_a"], d.history + + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:context_b) + assert_equal ["before_validation_marker on context_b", "after_validation_marker on context_b"], d.history + + d = DogValidatorWithOnMultipleCondition.new + d.valid?([:context_a, :context_b]) + assert_equal([ + "before_validation_marker on context_a", + "before_validation_marker on context_b", + "after_validation_marker on context_a", + "after_validation_marker on context_b" + ], d.history) + end + + def test_on_multiple_condition_is_respected_for_validation_without_matching_context + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:save) + assert_equal [], d.history + end + + def test_on_multiple_condition_is_respected_for_validation_without_context + d = DogValidatorWithOnMultipleCondition.new + d.valid? + assert_equal [], d.history + end + def test_before_validation_and_after_validation_callbacks_should_be_called d = DogWithMethodCallbacks.new d.valid? diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c88a4e7695..791de1f2d0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,12 +1,25 @@ +* Fix `count(:all)` to correctly work `distinct` with custom SELECT list. + + *Ryuta Kamizono* + +* 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 of database queries in the log to facilitate N+1 query resolution - and other debugging. Excludes Ruby and Rails callers but not gems. + and other debugging. Enabled in development only for new and upgraded applications. Not recommended for use in the production environment since it relies - on Ruby's `Kernel#caller` which is fairly slow. + on Ruby's `Kernel#caller_locations` which is fairly slow. *Olivier Lacan* @@ -548,4 +561,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/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index bd05fb8f6e..4a191d337c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -95,6 +95,7 @@ module ActiveRecord if options_sql = options[:options] create_sql << " #{options_sql}" end + create_sql end def column_options(o) 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..0afdd959f5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -117,11 +117,11 @@ module ActiveRecord end def get_advisory_lock(lock_name, timeout = 0) # :nodoc: - query_value("SELECT GET_LOCK(#{quote(lock_name)}, #{timeout})") == 1 + query_value("SELECT GET_LOCK(#{quote(lock_name.to_s)}, #{timeout})") == 1 end def release_advisory_lock(lock_name) # :nodoc: - query_value("SELECT RELEASE_LOCK(#{quote(lock_name)})") == 1 + query_value("SELECT RELEASE_LOCK(#{quote(lock_name.to_s)})") == 1 end def native_database_types @@ -292,10 +292,6 @@ module ActiveRecord SQL end - def create_table(table_name, **options) #:nodoc: - super(table_name, options: "ENGINE=InnoDB", **options) - end - def bulk_change_table(table_name, operations) #:nodoc: sqls = operations.flat_map do |command, args| table, arguments = args.shift, args @@ -717,7 +713,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/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index ba3419be6c..9234029c22 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -121,8 +121,7 @@ module ActiveRecord [ offending_line.path, - offending_line.lineno, - offending_line.label + offending_line.lineno ] end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index bd8c054c28..7ae8073478 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -28,6 +28,14 @@ module ActiveRecord super end end + + def create_table(table_name, options = {}) + if adapter_name == "Mysql2" + super(table_name, options: "ENGINE=InnoDB", **options) + else + super + end + end end class V5_0 < V5_1 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/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index d49472fc70..cb0b06cfdc 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -217,7 +217,7 @@ module ActiveRecord if operation == "count" column_name ||= select_for_count if column_name == :all - if distinct && (group_values.any? || !(has_limit_or_offset? && order_values.any?)) + if distinct && (group_values.any? || select_values.empty? && order_values.empty?) column_name = primary_key end elsif column_name =~ /\s*DISTINCT[\s(]+/i @@ -249,7 +249,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, distinct) #:nodoc: column_alias = column_name - if operation == "count" && has_limit_or_offset? + if operation == "count" && (column_name == :all && distinct || has_limit_or_offset?) # Shortcut when limit is zero. return 0 if limit_value == 0 @@ -391,14 +391,12 @@ module ActiveRecord end def build_count_subquery(relation, column_name, distinct) - relation.select_values = [ - if column_name == :all - distinct ? table[Arel.star] : Arel.sql(FinderMethods::ONE_AS_ONE) - else - column_alias = Arel.sql("count_column") - aggregate_column(column_name).as(column_alias) - end - ] + if column_name == :all + relation.select_values = [ Arel.sql(FinderMethods::ONE_AS_ONE) ] unless distinct + else + column_alias = Arel.sql("count_column") + relation.select_values = [ aggregate_column(column_name).as(column_alias) ] + end subquery = relation.arel.as(Arel.sql("subquery_for_count")) select_value = operation_over_aggregate_column(column_alias || Arel.star, "count", false) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2056f9bb73..0296101f81 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -444,15 +444,13 @@ module ActiveRecord # def left_outer_joins(*args) check_if_method_has_arguments!(__callee__, args) - - args.compact! - args.flatten! - spawn.left_outer_joins!(*args) end alias :left_joins :left_outer_joins def left_outer_joins!(*args) # :nodoc: + args.compact! + args.flatten! self.left_outer_joins_values += args self end diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index 4e73c557ed..6931b085a8 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -68,14 +68,14 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase def (ActiveRecord::Base.connection).data_source_exists?(*); false; end %w(SPATIAL FULLTEXT UNIQUE).each do |type| - expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`)) ENGINE=InnoDB" + expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`))" actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t| t.index :last_name, type: type end assert_equal expected, actual end - expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10))) ENGINE=InnoDB" + expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10)))" actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t| t.index :last_name, length: 10, using: :btree end @@ -160,7 +160,7 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase ActiveRecord::Base.connection.stubs(:data_source_exists?).with(:temp).returns(false) ActiveRecord::Base.connection.stubs(:index_name_exists?).with(:index_temp_on_zip).returns(false) - expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip` (`zip`)) ENGINE=InnoDB AS SELECT id, name, zip FROM a_really_complicated_query" + expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip` (`zip`)) AS SELECT id, name, zip FROM a_really_complicated_query" actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t| t.index :zip end diff --git a/activerecord/test/cases/adapters/mysql2/table_options_test.rb b/activerecord/test/cases/adapters/mysql2/table_options_test.rb index 9f6bd38a8c..1c92df940f 100644 --- a/activerecord/test/cases/adapters/mysql2/table_options_test.rb +++ b/activerecord/test/cases/adapters/mysql2/table_options_test.rb @@ -42,3 +42,78 @@ class Mysql2TableOptionsTest < ActiveRecord::Mysql2TestCase assert_match %r{COLLATE=utf8mb4_bin}, options end end + +class Mysql2DefaultEngineOptionSchemaDumpTest < ActiveRecord::Mysql2TestCase + include SchemaDumpingHelper + self.use_transactional_tests = false + + def setup + @verbose_was = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = false + end + + def teardown + ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true + ActiveRecord::Migration.verbose = @verbose_was + ActiveRecord::SchemaMigration.delete_all rescue nil + end + + test "schema dump includes ENGINE=InnoDB if not provided" do + ActiveRecord::Base.connection.create_table "mysql_table_options", force: true + + output = dump_table_schema("mysql_table_options") + options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options] + assert_match %r{ENGINE=InnoDB}, options + end + + test "schema dump includes ENGINE=InnoDB in legacy migrations" do + migration = Class.new(ActiveRecord::Migration[5.1]) do + def migrate(x) + create_table "mysql_table_options", force: true + end + end.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + output = dump_table_schema("mysql_table_options") + options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options] + assert_match %r{ENGINE=InnoDB}, options + end +end + +class Mysql2DefaultEngineOptionSqlOutputTest < ActiveRecord::Mysql2TestCase + self.use_transactional_tests = false + + def setup + @logger_was = ActiveRecord::Base.logger + @log = StringIO.new + @verbose_was = ActiveRecord::Migration.verbose + ActiveRecord::Base.logger = ActiveSupport::Logger.new(@log) + ActiveRecord::Migration.verbose = false + end + + def teardown + ActiveRecord::Base.logger = @logger_was + ActiveRecord::Migration.verbose = @verbose_was + ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true + ActiveRecord::SchemaMigration.delete_all rescue nil + end + + test "new migrations do not contain default ENGINE=InnoDB option" do + ActiveRecord::Base.connection.create_table "mysql_table_options", force: true + + assert_no_match %r{ENGINE=InnoDB}, @log.string + end + + test "legacy migrations contain default ENGINE=InnoDB option" do + migration = Class.new(ActiveRecord::Migration[5.1]) do + def migrate(x) + create_table "mysql_table_options", force: true + end + end.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert_match %r{ENGINE=InnoDB}, @log.string + end +end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 55b50e4f84..5eda39a0c7 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -236,6 +236,12 @@ class CalculationsTest < ActiveRecord::TestCase end end + def test_distinct_count_all_with_custom_select_and_order + accounts = Account.distinct.select("credit_limit % 10").order(Arel.sql("credit_limit % 10")) + assert_queries(1) { assert_equal 3, accounts.count(:all) } + assert_queries(1) { assert_equal 3, accounts.load.size } + end + def test_distinct_count_with_order_and_limit assert_equal 4, Account.distinct.order(:firm_id).limit(4).count end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 62d5d88fcc..8369a10b5a 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -287,10 +287,8 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association - assert_nothing_raised do - developer = developers(:david) - developer.ratings.includes(comment: :post).where(posts: { id: 1 }).exists? - end + developer = developers(:david) + assert_not_predicate developer.ratings.includes(comment: :post).where(posts: { id: 1 }), :exists? end def test_exists_with_empty_table_and_no_args_given 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..7785f8c99b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -780,8 +780,6 @@ class RelationTest < ActiveRecord::TestCase def test_find_all_using_where_with_relation david = authors(:david) - # switching the lines below would succeed in current rails - # assert_queries(2) { assert_queries(1) { relation = Author.where(id: Author.where(id: david.id)) assert_equal [david], relation.to_a @@ -820,8 +818,6 @@ class RelationTest < ActiveRecord::TestCase def test_find_all_using_where_with_relation_and_alternate_primary_key cool_first = minivans(:cool_first) - # switching the lines below would succeed in current rails - # assert_queries(2) { assert_queries(1) { relation = Minivan.where(minivan_id: Minivan.where(name: cool_first.name)) assert_equal [cool_first], relation.to_a @@ -896,11 +892,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 @@ -969,6 +963,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal 11, posts.distinct(false).select(:comments_count).count end + def test_size_with_distinct + posts = Post.distinct.select(:author_id, :comments_count) + assert_queries(1) { assert_equal 8, posts.size } + assert_queries(1) { assert_equal 8, posts.load.size } + end + def test_update_all_with_scope tag = Tag.first Post.tagged_with(tag.id).update_all title: "rofl" 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 ece99518be..bdcb5e6a41 100644 --- a/activestorage/app/models/active_storage/variant.rb +++ b/activestorage/app/models/active_storage/variant.rb @@ -104,7 +104,13 @@ class ActiveStorage::Variant def open_image(&block) - download_image.tap(&block).destroy! + image = download_image + + begin + yield image + ensure + image.destroy! + end end def download_image 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/active_record_migrations.md b/guides/source/active_record_migrations.md index f8f36bf600..ab3af438f5 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -353,8 +353,7 @@ create_table :products, options: "ENGINE=BLACKHOLE" do |t| end ``` -will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table -(when using MySQL or MariaDB, the default is `ENGINE=InnoDB`). +will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table. Also you can pass the `:comment` option with any description for the table that will be stored in database itself and can be viewed with database administration diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 6f5e631504..400f954dcd 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -439,7 +439,7 @@ module Rails end def depend_on_bootsnap? - !options[:skip_bootsnap] + !options[:skip_bootsnap] && !options[:dev] end def os_supports_listen_out_of_the_box? 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 8351d849ec..ae665b960a 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 @@ -25,6 +25,3 @@ # Store boolean values are in sqlite3 databases as 1 and 0 instead of 't' and # 'f' after migrating old data. # Rails.application.config.active_record.sqlite3.represent_boolean_as_integer = true - -# Highlight code that triggered database queries in logs. -Rails.application.config.active_record.verbose_query_logs = Rails.env.development? diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index ec745a397e..907eb4fa58 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1890,6 +1890,24 @@ module ApplicationTests assert_equal "https://example.org/", last_response.location end + test "config.active_support.hash_digest_class is Digest::MD5 by default" do + app "development" + + assert_equal Digest::MD5, ActiveSupport::Digest.hash_digest_class + end + + test "config.active_support.hash_digest_class can be configured" do + app_file "config/environments/development.rb", <<-RUBY + Rails.application.configure do + config.active_support.hash_digest_class = Digest::SHA1 + end + RUBY + + app "development" + + assert_equal Digest::SHA1, ActiveSupport::Digest.hash_digest_class + end + private def force_lazy_load_hooks yield # Tasty clarifying sugar, homie! We only need to reference a constant to load it. diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 9fc63755ae..110aca70c1 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -812,6 +812,17 @@ class AppGeneratorTest < Rails::Generators::TestCase 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 |