diff options
Diffstat (limited to 'activerecord')
35 files changed, 373 insertions, 94 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d1ae41ab97..71dcecd346 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,25 @@ +* Add database configuration to disable advisory locks. + + ``` + production: + adapter: postgresql + advisory_locks: false + ``` + + *Guo Xiang* + +* SQLite3 adapter `alter_table` method restores foreign keys. + + *Yasuo Honda* + +* Allow `:to_table` option to `invert_remove_foreign_key`. + + Example: + + remove_foreign_key :accounts, to_table: :owners + + *Nikolay Epifanov*, *Rich Chen* + * Add environment & load_config dependency to `bin/rake db:seed` to enable seed load in environments without Rails and custom DB configuration diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index b5fb0092d7..0166ed98ca 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -34,7 +34,7 @@ module ActiveRecord::Associations::Builder # :nodoc: foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column - if (@_after_replace_counter_called ||= false) + if @_after_replace_counter_called ||= false @_after_replace_counter_called = false elsif association(reflection.name).target_changed? if reflection.polymorphic? diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 3be0906f2a..4702de1964 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -980,11 +980,18 @@ module ActiveRecord # # remove_foreign_key :accounts, column: :owner_id # + # Removes the foreign key on +accounts.owner_id+. + # + # remove_foreign_key :accounts, to_table: :owners + # # Removes the foreign key named +special_fk_name+ on the +accounts+ table. # # remove_foreign_key :accounts, name: :special_fk_name # - # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. + # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key + # with an addition of + # [<tt>:to_table</tt>] + # The name of the table that contains the referenced primary key. def remove_foreign_key(from_table, options_or_to_table = {}) return unless supports_foreign_keys? diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 359cc54cf8..8999d3232a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -121,6 +121,10 @@ module ActiveRecord else @prepared_statements = false end + + @advisory_locks_enabled = self.class.type_cast_config_to_boolean( + config.fetch(:advisory_locks, true) + ) end def migrations_paths # :nodoc: 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 88fff83a9e..ad045f85ef 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -111,7 +111,7 @@ module ActiveRecord end def supports_advisory_locks? - true + @advisory_locks_enabled end def get_advisory_lock(lock_name, timeout = 0) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb index 231278c184..79351bc3a4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/type_map_initializer.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/array/extract" + module ActiveRecord module ConnectionAdapters module PostgreSQL @@ -16,12 +18,12 @@ module ActiveRecord def run(records) nodes = records.reject { |row| @store.key? row["oid"].to_i } - mapped, nodes = nodes.partition { |row| @store.key? row["typname"] } - ranges, nodes = nodes.partition { |row| row["typtype"] == "r".freeze } - enums, nodes = nodes.partition { |row| row["typtype"] == "e".freeze } - domains, nodes = nodes.partition { |row| row["typtype"] == "d".freeze } - arrays, nodes = nodes.partition { |row| row["typinput"] == "array_in".freeze } - composites, nodes = nodes.partition { |row| row["typelem"].to_i != 0 } + mapped = nodes.extract! { |row| @store.key? row["typname"] } + ranges = nodes.extract! { |row| row["typtype"] == "r".freeze } + enums = nodes.extract! { |row| row["typtype"] == "e".freeze } + domains = nodes.extract! { |row| row["typtype"] == "d".freeze } + arrays = nodes.extract! { |row| row["typinput"] == "array_in".freeze } + composites = nodes.extract! { |row| row["typelem"].to_i != 0 } mapped.each { |row| register_mapped_type(row) } enums.each { |row| register_enum_type(row) } diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 26b8113b0d..00da7690a2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -700,6 +700,11 @@ module ActiveRecord sql end + def add_column_for_alter(table_name, column_name, type, options = {}) + return super unless options.key?(:comment) + [super, Proc.new { change_column_comment(table_name, column_name, options[:comment]) }] + end + def change_column_for_alter(table_name, column_name, type, options = {}) sqls = [change_column_sql(table_name, column_name, type, options)] sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) @@ -708,7 +713,6 @@ module ActiveRecord sqls end - # Changes the default value of a table column. def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: column = column_for(table_name, column_name) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 3ee344a249..30e651ee63 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -298,7 +298,7 @@ module ActiveRecord end def supports_advisory_locks? - true + @advisory_locks_enabled end def supports_explain? diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index e0523de484..efe454fa7f 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -417,12 +417,23 @@ module ActiveRecord def alter_table(table_name, options = {}) altered_table_name = "a#{table_name}" - caller = lambda { |definition| yield definition if block_given? } + foreign_keys = foreign_keys(table_name) + + caller = lambda do |definition| + rename = options[:rename] || {} + foreign_keys.each do |fk| + if column = rename[fk.options[:column]] + fk.options[:column] = column + end + definition.foreign_key(fk.to_table, fk.options) + end + + yield definition if block_given? + end transaction do disable_referential_integrity do - move_table(table_name, altered_table_name, - options.merge(temporary: true)) + move_table(table_name, altered_table_name, options.merge(temporary: true)) move_table(altered_table_name, table_name, &caller) end end @@ -454,6 +465,7 @@ module ActiveRecord primary_key: column_name == from_primary_key ) end + yield @definition if block_given? end copy_table_indexes(from, to, options[:rename] || {}) diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 1ae6840921..6b84431343 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -4,6 +4,8 @@ module ActiveRecord class LogSubscriber < ActiveSupport::LogSubscriber IGNORE_PAYLOAD_NAMES = ["SCHEMA", "EXPLAIN"] + class_attribute :backtrace_cleaner, default: ActiveSupport::BacktraceCleaner.new + def self.runtime=(value) ActiveRecord::RuntimeRegistry.sql_runtime = value end @@ -100,21 +102,15 @@ module ActiveRecord end def log_query_source - location = extract_query_source_location(caller_locations) - - if location - source = "#{location.path}:#{location.lineno}" - source = source.sub("#{::Rails.root}/", "") if defined?(::Rails.root) + source = extract_query_source_location(caller) + if source logger.debug(" ↳ #{source}") end end - RAILS_GEM_ROOT = File.expand_path("../../..", __dir__) + "/" - PATHS_TO_IGNORE = /\A(#{RAILS_GEM_ROOT}|#{RbConfig::CONFIG["rubylibdir"]})/ - def extract_query_source_location(locations) - locations.find { |line| line.absolute_path && !line.absolute_path.match?(PATHS_TO_IGNORE) } + backtrace_cleaner.clean(locations).first end end end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 087632b10f..dea6d4ec08 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -214,11 +214,24 @@ module ActiveRecord end def invert_remove_foreign_key(args) - from_table, to_table, remove_options = args - raise ActiveRecord::IrreversibleMigration, "remove_foreign_key is only reversible if given a second table" if to_table.nil? || to_table.is_a?(Hash) + from_table, options_or_to_table, options_or_nil = args + + to_table = if options_or_to_table.is_a?(Hash) + options_or_to_table[:to_table] + else + options_or_to_table + end + + remove_options = if options_or_to_table.is_a?(Hash) + options_or_to_table.except(:to_table) + else + options_or_nil + end + + raise ActiveRecord::IrreversibleMigration, "remove_foreign_key is only reversible if given a second table" if to_table.nil? reversed_args = [from_table, to_table] - reversed_args << remove_options if remove_options + reversed_args << remove_options if remove_options.present? [:add_foreign_key, reversed_args] end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 009f412234..7ece083fd4 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -77,6 +77,10 @@ module ActiveRecord ActiveSupport.on_load(:active_record) { self.logger ||= ::Rails.logger } end + initializer "active_record.backtrace_cleaner" do + ActiveSupport.on_load(:active_record) { LogSubscriber.backtrace_cleaner = ::Rails.backtrace_cleaner } + end + initializer "active_record.migration_error" do if config.active_record.delete(:migration_error) == :page_load config.app_middleware.insert_after ::ActionDispatch::Callbacks, diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 10e4779ca4..07b16a0740 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -152,10 +152,10 @@ module ActiveRecord def merge_multi_values if other.reordering_value # override any order specified in the original relation - relation.reorder! other.order_values + relation.reorder!(*other.order_values) elsif other.order_values.any? # merge in order_values from relation - relation.order! other.order_values + relation.order!(*other.order_values) end extensions = other.extensions - relation.extensions diff --git a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb index 64bf83e3c1..e5191fa38a 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/array_handler.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/array/extract" + module ActiveRecord class PredicateBuilder class ArrayHandler # :nodoc: @@ -11,8 +13,8 @@ module ActiveRecord return attribute.in([]) if value.empty? values = value.map { |x| x.is_a?(Base) ? x.id : x } - nils, values = values.partition(&:nil?) - ranges, values = values.partition { |v| v.is_a?(Range) } + nils = values.extract!(&:nil?) + ranges = values.extract! { |v| v.is_a?(Range) } values_predicate = case values.length diff --git a/activerecord/test/cases/adapters/helpers/test_supports_advisory_locks.rb b/activerecord/test/cases/adapters/helpers/test_supports_advisory_locks.rb new file mode 100644 index 0000000000..4905e17725 --- /dev/null +++ b/activerecord/test/cases/adapters/helpers/test_supports_advisory_locks.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require "support/connection_helper" + +module TestSupportsAdvisoryLocks + include ConnectionHelper + + def test_supports_advisory_locks? + assert ActiveRecord::Base.connection.supports_advisory_locks? + + run_without_connection do |orig_connection| + ActiveRecord::Base.establish_connection( + orig_connection.merge(advisory_locks: false) + ) + + assert_not ActiveRecord::Base.connection.supports_advisory_locks? + + ActiveRecord::Base.establish_connection( + orig_connection.merge(advisory_locks: true) + ) + + assert ActiveRecord::Base.connection.supports_advisory_locks? + end + end +end diff --git a/activerecord/test/cases/adapters/mysql2/test_advisory_locks_disabled_test.rb b/activerecord/test/cases/adapters/mysql2/test_advisory_locks_disabled_test.rb new file mode 100644 index 0000000000..4857900820 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql2/test_advisory_locks_disabled_test.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require "cases/helper" +require "cases/adapters/helpers/test_supports_advisory_locks" + +class Mysql2AdvisoryLocksDisabledTest < ActiveRecord::Mysql2TestCase + include TestSupportsAdvisoryLocks +end diff --git a/activerecord/test/cases/adapters/postgresql/advisory_locks_disabled_test.rb b/activerecord/test/cases/adapters/postgresql/advisory_locks_disabled_test.rb new file mode 100644 index 0000000000..f14e9baeb9 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/advisory_locks_disabled_test.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require "cases/helper" +require "cases/adapters/helpers/test_supports_advisory_locks" + +class PostgresqlAdvisoryLocksDisabledTest < ActiveRecord::PostgreSQLTestCase + include TestSupportsAdvisoryLocks +end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 5b8d4722af..a1fba8dc66 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -1571,8 +1571,9 @@ class EagerAssociationTest < ActiveRecord::TestCase # CollectionProxy#reader is expensive, so the preloader avoids calling it. test "preloading has_many_through association avoids calling association.reader" do - ActiveRecord::Associations::HasManyAssociation.any_instance.expects(:reader).never - Author.preload(:readonly_comments).first! + assert_not_called_on_instance_of(ActiveRecord::Associations::HasManyAssociation, :reader) do + Author.preload(:readonly_comments).first! + end end test "preloading through a polymorphic association doesn't require the association to exist" do diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f414fbf64b..482302055d 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -781,7 +781,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_polymorphic_has_manys_works - assert_equal [10, 20].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set + assert_equal ["$10.00", "$20.00"].to_set, pirates(:redbeard).treasure_estimates.map(&:price).to_set end def test_symbols_as_keys diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 0ca902385a..5e6bea17ea 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2134,21 +2134,29 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_defining_has_many_association_with_delete_all_dependency_lazily_evaluates_target_class - ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never - class_eval(<<-EOF, __FILE__, __LINE__ + 1) - class DeleteAllModel < ActiveRecord::Base - has_many :nonentities, :dependent => :delete_all - end - EOF + assert_not_called_on_instance_of( + ActiveRecord::Reflection::AssociationReflection, + :class_name, + ) do + class_eval(<<-EOF, __FILE__, __LINE__ + 1) + class DeleteAllModel < ActiveRecord::Base + has_many :nonentities, :dependent => :delete_all + end + EOF + end end def test_defining_has_many_association_with_nullify_dependency_lazily_evaluates_target_class - ActiveRecord::Reflection::AssociationReflection.any_instance.expects(:class_name).never - class_eval(<<-EOF, __FILE__, __LINE__ + 1) - class NullifyModel < ActiveRecord::Base - has_many :nonentities, :dependent => :nullify - end - EOF + assert_not_called_on_instance_of( + ActiveRecord::Reflection::AssociationReflection, + :class_name, + ) do + class_eval(<<-EOF, __FILE__, __LINE__ + 1) + class NullifyModel < ActiveRecord::Base + has_many :nonentities, :dependent => :nullify + end + EOF + end end def test_attributes_are_being_set_when_initialized_from_has_many_association_with_where_clause diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index d7e898a1c0..9eea34d2b9 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -661,6 +661,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase self.table_name = "books" belongs_to :author, class_name: "SpecialAuthor" has_one :subscription, class_name: "SpecialSupscription", foreign_key: "subscriber_id" + + enum status: [:proposed, :written, :published] end class SpecialAuthor < ActiveRecord::Base @@ -678,6 +680,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase book = SpecialBook.create!(status: "published") author.book = book + assert_equal "published", book.status assert_not_equal 0, SpecialAuthor.joins(:book).where(books: { status: "published" }).count end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 9ac03629c3..6aecf5fa35 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -91,7 +91,9 @@ module ActiveRecord end def test_full_pool_exception + @pool.checkout_timeout = 0.001 # no need to delay test suite by waiting the whole full default timeout @pool.size.times { assert @pool.checkout } + assert_raises(ConnectionTimeoutError) do @pool.checkout end @@ -156,6 +158,48 @@ module ActiveRecord @pool.connections.each { |conn| conn.close if conn.in_use? } end + def test_idle_timeout_configuration + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: "0.02") + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.01 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 0.02 + ) + + @pool.flush + assert_equal 0, @pool.connections.length + end + + def test_disable_flush + @pool.disconnect! + spec = ActiveRecord::Base.connection_pool.spec + spec.config.merge!(idle_timeout: -5) + @pool = ConnectionPool.new(spec) + idle_conn = @pool.checkout + @pool.checkin(idle_conn) + + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1 + ) + + @pool.flush + assert_equal 1, @pool.connections.length + end + def test_flush idle_conn = @pool.checkout recent_conn = @pool.checkout @@ -166,9 +210,10 @@ module ActiveRecord assert_equal 3, @pool.connections.length - def idle_conn.seconds_idle - 1000 - end + idle_conn.instance_variable_set( + :@idle_since, + Concurrent.monotonic_time - 1000 + ) @pool.flush(30) diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 66f11fe5bd..68be685e4b 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -183,5 +183,3 @@ module InTimeZone ActiveRecord::Base.time_zone_aware_attributes = old_tz end end - -require "mocha/minitest" # FIXME: stop using mocha diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 3a11bb081b..1a19b8dafd 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -329,11 +329,24 @@ module ActiveRecord assert_equal [:add_foreign_key, [:dogs, :people, primary_key: "person_id"]], enable end + def test_invert_remove_foreign_key_with_primary_key_and_to_table_in_options + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people, primary_key: "uuid"] + assert_equal [:add_foreign_key, [:dogs, :people, primary_key: "uuid"]], enable + end + def test_invert_remove_foreign_key_with_on_delete_on_update enable = @recorder.inverse_of :remove_foreign_key, [:dogs, :people, on_delete: :nullify, on_update: :cascade] assert_equal [:add_foreign_key, [:dogs, :people, on_delete: :nullify, on_update: :cascade]], enable end + def test_invert_remove_foreign_key_with_to_table_in_options + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people] + assert_equal [:add_foreign_key, [:dogs, :people]], enable + + enable = @recorder.inverse_of :remove_foreign_key, [:dogs, to_table: :people, column: :owner_id] + assert_equal [:add_foreign_key, [:dogs, :people, column: :owner_id]], enable + end + def test_invert_remove_foreign_key_is_irreversible_without_to_table assert_raises ActiveRecord::IrreversibleMigration do @recorder.inverse_of :remove_foreign_key, [:dogs, column: "owner_id"] diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index c471dd1106..bb233fbf74 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -53,16 +53,49 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? end def test_change_column_of_parent_table - foreign_keys = ActiveRecord::Base.connection.foreign_keys("astronauts") rocket = Rocket.create!(name: "myrocket") rocket.astronauts << Astronaut.create! @connection.change_column_null :rockets, :name, false + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert_equal "myrocket", Rocket.first.name + assert_equal "astronauts", fk.from_table + assert_equal "rockets", fk.to_table + end + + def test_rename_column_of_child_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.rename_column :astronauts, :name, :astronaut_name + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert_equal "myrocket", Rocket.first.name + assert_equal "astronauts", fk.from_table + assert_equal "rockets", fk.to_table + end + + def test_rename_reference_column_of_child_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.rename_column :astronauts, :rocket_id, :new_rocket_id + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + fk = foreign_keys.first assert_equal "myrocket", Rocket.first.name assert_equal "astronauts", fk.from_table assert_equal "rockets", fk.to_table + assert_equal "new_rocket_id", fk.options[:column] end end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index d1292dc53d..868bb40ab2 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -793,12 +793,20 @@ if ActiveRecord::Base.connection.supports_bulk_alter? end def test_adding_multiple_columns - assert_queries(1) do + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 1, + "PostgreSQLAdapter" => 2, # one for bulk change, one for comment + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count) do with_bulk_change_table do |t| t.column :name, :string t.string :qualification, :experience t.integer :age, default: 0 - t.date :birthdate + t.date :birthdate, comment: "This is a comment" t.timestamps null: true end end @@ -806,6 +814,7 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert_equal 8, columns.size [:name, :qualification, :experience].each { |s| assert_equal :string, column(s).type } assert_equal "0", column(:age).default + assert_equal "This is a comment", column(:birthdate).comment end def test_removing_columns diff --git a/activerecord/test/cases/reaper_test.rb b/activerecord/test/cases/reaper_test.rb index b034fe3e3b..b630f782bc 100644 --- a/activerecord/test/cases/reaper_test.rb +++ b/activerecord/test/cases/reaper_test.rb @@ -61,9 +61,9 @@ module ActiveRecord def test_reaping_frequency_configuration spec = ActiveRecord::Base.connection_pool.spec.dup - spec.config[:reaping_frequency] = 100 + spec.config[:reaping_frequency] = "10.01" pool = ConnectionPool.new spec - assert_equal 100, pool.reaper.frequency + assert_equal 10.01, pool.reaper.frequency end def test_connection_pool_starts_reaper diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index f53ef1fe35..6e7998d15a 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -121,6 +121,16 @@ class RelationMergingTest < ActiveRecord::TestCase relation = relation.merge(Post.from("posts")) assert_not_empty relation.from_clause end + + def test_merging_with_order_with_binds + relation = Post.all.merge(Post.order([Arel.sql("title LIKE ?"), "%suffix"])) + assert_equal ["title LIKE '%suffix'"], relation.order_values + end + + def test_merging_with_order_without_binds + relation = Post.all.merge(Post.order(Arel.sql("title LIKE '%?'"))) + assert_equal ["title LIKE '%?'"], relation.order_values + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 4cd4515c3b..1192b30b14 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -322,7 +322,7 @@ class SerializedAttributeTest < ActiveRecord::TestCase topic = Topic.create!(content: {}) topic2 = Topic.create!(content: nil) - assert_equal [topic, topic2], Topic.where(content: nil) + assert_equal [topic, topic2], Topic.where(content: nil).sort_by(&:id) end def test_nil_is_always_persisted_as_null diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index a6efe3fa5e..ee53d576a1 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -46,35 +46,46 @@ module ActiveRecord class DatabaseTasksUtilsTask < ActiveRecord::TestCase def test_raises_an_error_when_called_with_protected_environment - ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1) - protected_environments = ActiveRecord::Base.protected_environments current_env = ActiveRecord::Base.connection.migration_context.current_environment - assert_not_includes protected_environments, current_env - # Assert no error - ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! - - ActiveRecord::Base.protected_environments = [current_env] - assert_raise(ActiveRecord::ProtectedEnvironmentError) do + assert_called_on_instance_of( + ActiveRecord::MigrationContext, + :current_version, + times: 6, + returns: 1 + ) do + assert_not_includes protected_environments, current_env + # Assert no error ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + + ActiveRecord::Base.protected_environments = [current_env] + + assert_raise(ActiveRecord::ProtectedEnvironmentError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end end ensure ActiveRecord::Base.protected_environments = protected_environments end def test_raises_an_error_when_called_with_protected_environment_which_name_is_a_symbol - ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1) - protected_environments = ActiveRecord::Base.protected_environments current_env = ActiveRecord::Base.connection.migration_context.current_environment - assert_not_includes protected_environments, current_env - # Assert no error - ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! - - ActiveRecord::Base.protected_environments = [current_env.to_sym] - assert_raise(ActiveRecord::ProtectedEnvironmentError) do + assert_called_on_instance_of( + ActiveRecord::MigrationContext, + :current_version, + times: 6, + returns: 1 + ) do + assert_not_includes protected_environments, current_env + # Assert no error ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + + ActiveRecord::Base.protected_environments = [current_env.to_sym] + assert_raise(ActiveRecord::ProtectedEnvironmentError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end end ensure ActiveRecord::Base.protected_environments = protected_environments @@ -82,10 +93,14 @@ module ActiveRecord def test_raises_an_error_if_no_migrations_have_been_made ActiveRecord::InternalMetadata.stub(:table_exists?, false) do - ActiveRecord::MigrationContext.any_instance.stubs(:current_version).returns(1) - - assert_raise(ActiveRecord::NoEnvironmentInSchemaError) do - ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + assert_called_on_instance_of( + ActiveRecord::MigrationContext, + :current_version, + returns: 1 + ) do + assert_raise(ActiveRecord::NoEnvironmentInSchemaError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end end end end diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index eeb4222d97..4d6dff68f9 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -152,10 +152,14 @@ if current_adapter?(:Mysql2Adapter) end def test_establishes_connection_to_mysql_database - with_stubbed_connection_establish_connection do - ActiveRecord::Base.expects(:establish_connection).with @configuration - - ActiveRecord::Tasks::DatabaseTasks.drop @configuration + ActiveRecord::Base.stub(:connection, @connection) do + assert_called_with( + ActiveRecord::Base, + :establish_connection, + [@configuration] + ) do + ActiveRecord::Tasks::DatabaseTasks.drop @configuration + end end end @@ -196,10 +200,14 @@ if current_adapter?(:Mysql2Adapter) end def test_establishes_connection_to_the_appropriate_database - with_stubbed_connection_establish_connection do - ActiveRecord::Base.expects(:establish_connection).with(@configuration) - - ActiveRecord::Tasks::DatabaseTasks.purge @configuration + ActiveRecord::Base.stub(:connection, @connection) do + assert_called_with( + ActiveRecord::Base, + :establish_connection, + [@configuration] + ) do + ActiveRecord::Tasks::DatabaseTasks.purge @configuration + end end end diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index 00005e7a0d..e36c2b1e3f 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -166,12 +166,17 @@ if current_adapter?(:PostgreSQLAdapter) def test_establishes_connection_to_postgresql_database ActiveRecord::Base.stub(:connection, @connection) do - ActiveRecord::Base.expects(:establish_connection).with( - "adapter" => "postgresql", - "database" => "postgres", - "schema_search_path" => "public" - ) - ActiveRecord::Tasks::DatabaseTasks.drop @configuration + assert_called_with( + ActiveRecord::Base, + :establish_connection, + [ + "adapter" => "postgresql", + "database" => "postgres", + "schema_search_path" => "public" + ] + ) do + ActiveRecord::Tasks::DatabaseTasks.drop @configuration + end end end diff --git a/activerecord/test/cases/tasks/sqlite_rake_test.rb b/activerecord/test/cases/tasks/sqlite_rake_test.rb index 7eb062b456..c42afd0e42 100644 --- a/activerecord/test/cases/tasks/sqlite_rake_test.rb +++ b/activerecord/test/cases/tasks/sqlite_rake_test.rb @@ -47,9 +47,9 @@ if current_adapter?(:SQLite3Adapter) def test_db_create_with_file_does_nothing File.stub(:exist?, true) do - ActiveRecord::Base.expects(:establish_connection).never - - ActiveRecord::Tasks::DatabaseTasks.create @configuration, "/rails/root" + assert_not_called(ActiveRecord::Base, :establish_connection) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration, "/rails/root" + end end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index a33877f43a..66763c727f 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -3,11 +3,11 @@ require "cases/helper" require "models/topic" require "models/reply" -require "models/person" require "models/developer" require "models/computer" require "models/parrot" require "models/company" +require "models/price_estimate" class ValidationsTest < ActiveRecord::TestCase fixtures :topics, :developers @@ -183,6 +183,22 @@ class ValidationsTest < ActiveRecord::TestCase assert_not_predicate klass.new(wibble: BigDecimal("97.179")), :valid? end + def test_numericality_validator_wont_be_affected_by_custom_getter + price_estimate = PriceEstimate.new(price: 50) + + assert_equal "$50.00", price_estimate.price + assert_equal 50, price_estimate.price_before_type_cast + assert_equal 50, price_estimate.read_attribute(:price) + + assert_predicate price_estimate, :price_came_from_user? + assert_predicate price_estimate, :valid? + + price_estimate.save! + + assert_not_predicate price_estimate, :price_came_from_user? + assert_predicate price_estimate, :valid? + end + def test_acceptance_validator_doesnt_require_db_connection klass = Class.new(ActiveRecord::Base) do self.table_name = "posts" diff --git a/activerecord/test/models/price_estimate.rb b/activerecord/test/models/price_estimate.rb index f1f88d8d8d..669d0991f7 100644 --- a/activerecord/test/models/price_estimate.rb +++ b/activerecord/test/models/price_estimate.rb @@ -1,6 +1,14 @@ # frozen_string_literal: true class PriceEstimate < ActiveRecord::Base + include ActiveSupport::NumberHelper + belongs_to :estimate_of, polymorphic: true belongs_to :thing, polymorphic: true + + validates_numericality_of :price + + def price + number_to_currency super + end end |