diff options
26 files changed, 240 insertions, 120 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index b8bebe4d42..ab4fcd123d 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -28,3 +28,5 @@ engines: ratings: paths: - "**.rb" + +exclude_paths: [] diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index 307b852440..e602aa117a 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -25,7 +25,7 @@ module ActionView src = @src view = Class.new(ActionView::Base) { include action_view_erb_handler_context._routes.url_helpers - class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) + class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", defined?(@filename) ? @filename : "(erubi)", 0) }.empty view._run(:_template, nil, {}, ActionView::OutputBuffer.new) end diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index afeee03d4f..0075cd3c01 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -286,12 +286,12 @@ module ActiveModel method_name = matcher.method_name(attr_name) unless instance_method_already_implemented?(method_name) - generate_method = "define_method_#{matcher.method_missing_target}" + generate_method = "define_method_#{matcher.target}" if respond_to?(generate_method, true) send(generate_method, attr_name.to_s) else - define_proxy_call true, generated_attribute_methods, method_name, matcher.method_missing_target, attr_name.to_s + define_proxy_call true, generated_attribute_methods, method_name, matcher.target, attr_name.to_s end end end @@ -362,7 +362,7 @@ module ActiveModel # Define a method `name` in `mod` that dispatches to `send` # using the given `extra` args. This falls back on `define_method` # and `send` if the given names cannot be compiled. - def define_proxy_call(include_private, mod, name, send, *extra) + def define_proxy_call(include_private, mod, name, target, *extra) defn = if NAME_COMPILABLE_REGEXP.match?(name) "def #{name}(*args)" else @@ -371,34 +371,34 @@ module ActiveModel extra = (extra.map!(&:inspect) << "*args").join(", ") - target = if CALL_COMPILABLE_REGEXP.match?(send) - "#{"self." unless include_private}#{send}(#{extra})" + body = if CALL_COMPILABLE_REGEXP.match?(target) + "#{"self." unless include_private}#{target}(#{extra})" else - "send(:'#{send}', #{extra})" + "send(:'#{target}', #{extra})" end mod.module_eval <<-RUBY, __FILE__, __LINE__ + 1 #{defn} - #{target} + #{body} end RUBY end class AttributeMethodMatcher #:nodoc: - attr_reader :prefix, :suffix, :method_missing_target + attr_reader :prefix, :suffix, :target - AttributeMethodMatch = Struct.new(:target, :attr_name, :method_name) + AttributeMethodMatch = Struct.new(:target, :attr_name) def initialize(options = {}) @prefix, @suffix = options.fetch(:prefix, ""), options.fetch(:suffix, "") @regex = /^(?:#{Regexp.escape(@prefix)})(.*)(?:#{Regexp.escape(@suffix)})$/ - @method_missing_target = "#{@prefix}attribute#{@suffix}" + @target = "#{@prefix}attribute#{@suffix}" @method_name = "#{prefix}%s#{suffix}" end def match(method_name) if @regex =~ method_name - AttributeMethodMatch.new(method_missing_target, $1, method_name) + AttributeMethodMatch.new(target, $1) end end diff --git a/activemodel/lib/active_model/attributes.rb b/activemodel/lib/active_model/attributes.rb index c3a446098c..b7b2f35bcc 100644 --- a/activemodel/lib/active_model/attributes.rb +++ b/activemodel/lib/active_model/attributes.rb @@ -91,7 +91,7 @@ module ActiveModel @attributes.fetch_value(name) end - # Handle *= for method_missing. + # Dispatch target for <tt>*=</tt> attribute methods. def attribute=(attribute_name, value) write_attribute(attribute_name, value) end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index ab2c9d04ae..35a587658c 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -165,17 +165,17 @@ module ActiveModel mutations_from_database.changed_attribute_names end - # Handles <tt>*_changed?</tt> for +method_missing+. + # Dispatch target for <tt>*_changed?</tt> attribute methods. def attribute_changed?(attr_name, **options) # :nodoc: mutations_from_database.changed?(attr_name.to_s, options) end - # Handles <tt>*_was</tt> for +method_missing+. + # Dispatch target for <tt>*_was</tt> attribute methods. def attribute_was(attr_name) # :nodoc: mutations_from_database.original_value(attr_name.to_s) end - # Handles <tt>*_previously_changed?</tt> for +method_missing+. + # Dispatch target for <tt>*_previously_changed?</tt> attribute methods. def attribute_previously_changed?(attr_name) # :nodoc: mutations_before_last_save.changed?(attr_name.to_s) end @@ -253,22 +253,22 @@ module ActiveModel @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance end - # Handles <tt>*_change</tt> for +method_missing+. + # Dispatch target for <tt>*_change</tt> attribute methods. def attribute_change(attr_name) mutations_from_database.change_to_attribute(attr_name.to_s) end - # Handles <tt>*_previous_change</tt> for +method_missing+. + # Dispatch target for <tt>*_previous_change</tt> attribute methods. def attribute_previous_change(attr_name) mutations_before_last_save.change_to_attribute(attr_name.to_s) end - # Handles <tt>*_will_change!</tt> for +method_missing+. + # Dispatch target for <tt>*_will_change!</tt> attribute methods. def attribute_will_change!(attr_name) mutations_from_database.force_change(attr_name.to_s) end - # Handles <tt>restore_*!</tt> for +method_missing+. + # Dispatch target for <tt>restore_*!</tt> attribute methods. def restore_attribute!(attr_name) attr_name = attr_name.to_s if attribute_changed?(attr_name) diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb index ebb6cc542d..4e228032c3 100644 --- a/activemodel/test/cases/attribute_methods_test.rb +++ b/activemodel/test/cases/attribute_methods_test.rb @@ -264,6 +264,5 @@ class AttributeMethodsTest < ActiveModel::TestCase assert_equal "foo", match.attr_name assert_equal "attribute_test", match.target - assert_equal "foo_test", match.method_name end end diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index 7662f996ae..72baf6e7a7 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -7,21 +7,23 @@ require "models/reply" require "models/person" class AcceptanceValidationTest < ActiveModel::TestCase - def teardown - Topic.clear_validators! + teardown do + self.class.send(:remove_const, :TestClass) end def test_terms_of_service_agreement_no_acceptance - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - t = Topic.new("title" => "We should not be confirmed") + t = klass.new("title" => "We should not be confirmed") assert_predicate t, :valid? end def test_terms_of_service_agreement - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -30,9 +32,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_eula - Topic.validates_acceptance_of(:eula, message: "must be abided") + klass = define_test_class(Topic) + klass.validates_acceptance_of(:eula, message: "must be abided") - t = Topic.new("title" => "We should be confirmed", "eula" => "") + t = klass.new("title" => "We should be confirmed", "eula" => "") assert_predicate t, :invalid? assert_equal ["must be abided"], t.errors[:eula] @@ -41,9 +44,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_terms_of_service_agreement_with_accept_value - Topic.validates_acceptance_of(:terms_of_service, accept: "I agree.") + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service, accept: "I agree.") - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -52,9 +56,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_terms_of_service_agreement_with_multiple_accept_values - Topic.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."]) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."]) - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -66,9 +71,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_validates_acceptance_of_for_ruby_class - Person.validates_acceptance_of :karma + klass = define_test_class(Person) + klass.validates_acceptance_of :karma - p = Person.new + p = klass.new p.karma = "" assert_predicate p, :invalid? @@ -76,13 +82,21 @@ class AcceptanceValidationTest < ActiveModel::TestCase p.karma = "1" assert_predicate p, :valid? - ensure - Person.clear_validators! end def test_validates_acceptance_of_true - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - assert_predicate Topic.new(terms_of_service: true), :valid? + assert_predicate klass.new(terms_of_service: true), :valid? end + + private + + # Acceptance validator includes anonymous module into class, which cannot + # be cleared, so to avoid multiple inclusions we use a named subclass which + # we can remove in teardown. + def define_test_class(parent) + self.class.const_set(:TestClass, Class.new(parent)) + end end diff --git a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb index 5941f51a1a..dc239ff9ea 100644 --- a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb +++ b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb @@ -65,7 +65,7 @@ module ActiveRecord private - # Handle *_before_type_cast for method_missing. + # Dispatch target for <tt>*_before_type_cast</tt> attribute methods. def attribute_before_type_cast(attribute_name) read_attribute_before_type_cast(attribute_name) end diff --git a/activerecord/lib/active_record/attribute_methods/query.rb b/activerecord/lib/active_record/attribute_methods/query.rb index 6811f54b10..0cf67644af 100644 --- a/activerecord/lib/active_record/attribute_methods/query.rb +++ b/activerecord/lib/active_record/attribute_methods/query.rb @@ -32,7 +32,7 @@ module ActiveRecord end private - # Handle *? for method_missing. + # Dispatch target for <tt>*?</tt> attribute methods. def attribute?(attribute_name) query_attribute(attribute_name) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 455e67e19b..d5ba2f42cb 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -58,7 +58,7 @@ module ActiveRecord value end - # Handle *= for method_missing. + # Dispatch target for <tt>*=</tt> attribute methods. def attribute=(attribute_name, value) _write_attribute(attribute_name, value) end 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 4840307094..8ba9943f75 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1379,6 +1379,31 @@ module ActiveRecord options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty? end + def bulk_change_table(table_name, operations) + sql_fragments = [] + non_combinable_operations = [] + + operations.each do |command, args| + table, arguments = args.shift, args + method = :"#{command}_for_alter" + + if respond_to?(method, true) + sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } + sql_fragments << sqls + non_combinable_operations.concat(procs) + else + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + sql_fragments = [] + non_combinable_operations = [] + send(command, table, *arguments) + end + end + + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + end + def add_column_for_alter(table_name, column_name, type, options = {}) td = create_table_definition(table_name) cd = td.new_column_definition(column_name, type, options) 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 ca8bbc14da..2a2b234ef8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -63,7 +63,7 @@ module ActiveRecord /mariadb/i.match?(full_version) end - def supports_bulk_alter? #:nodoc: + def supports_bulk_alter? true end @@ -285,21 +285,6 @@ module ActiveRecord SQL end - def bulk_change_table(table_name, operations) #:nodoc: - sqls = operations.flat_map do |command, args| - table, arguments = args.shift, args - method = :"#{command}_for_alter" - - if respond_to?(method, true) - send(method, table, *arguments) - else - raise "Unknown method called : #{method}(#{arguments.inspect})" - end - end.join(", ") - - execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}") - end - def change_table_comment(table_name, comment) #:nodoc: comment = "" if comment.nil? execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}") 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 2a641cbe53..ec8af73d07 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -368,31 +368,6 @@ module ActiveRecord SQL end - def bulk_change_table(table_name, operations) - sql_fragments = [] - non_combinable_operations = [] - - operations.each do |command, args| - table, arguments = args.shift, args - method = :"#{command}_for_alter" - - if respond_to?(method, true) - sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } - sql_fragments << sqls - non_combinable_operations.concat(procs) - else - execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? - non_combinable_operations.each(&:call) - sql_fragments = [] - non_combinable_operations = [] - send(command, table, *arguments) - end - end - - execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? - non_combinable_operations.each(&:call) - end - # Renames a table. # Also renames a table's primary key sequence if the sequence name exists and # matches the Active Record default. diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index 5dc88fb26c..980e42664b 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -22,7 +22,7 @@ module ActiveRecord @_touch_time = current_time_from_proper_timezone surreptitiously_touch @_defer_touch_attrs - self.class.connection.add_transaction_record self + add_to_transaction # touch the parents as we are not calling the after_save callbacks self.class.reflect_on_all_associations(:belongs_to).each do |r| diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 333f1a5435..00a62a70d6 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -355,18 +355,6 @@ module ActiveRecord clear_transaction_record_state end - # Add the record to the current transaction so that the #after_rollback and #after_commit callbacks - # can be called. - def add_to_transaction - if has_transactional_callbacks? - self.class.connection.add_transaction_record(self) - else - sync_with_transaction_state - set_transaction_state(self.class.connection.transaction_state) - end - remember_transaction_record_state - end - # Executes +method+ within a transaction and captures its return value as a # status flag. If the status is true the transaction is committed, otherwise # a ROLLBACK is issued. In any case the status flag is returned. @@ -387,7 +375,7 @@ module ActiveRecord ensure if has_transactional_callbacks? && (@_new_record_before_last_commit && !new_record? || _trigger_update_callback || _trigger_destroy_callback) - self.class.connection.add_transaction_record(self) + add_to_transaction end end status @@ -459,6 +447,12 @@ module ActiveRecord end end + # Add the record to the current transaction so that the #after_rollback and #after_commit + # callbacks can be called. + def add_to_transaction + self.class.connection.add_transaction_record(self) + end + def set_transaction_state(state) @transaction_state = state end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index ba04859bf0..ce2ed06c1d 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -490,6 +490,8 @@ module ActiveRecord @connection.truncate("posts") assert_equal 0, Post.count + ensure + reset_fixtures("posts") end def test_truncate_with_query_cache @@ -501,6 +503,7 @@ module ActiveRecord assert_equal 0, Post.count ensure + reset_fixtures("posts") @connection.disable_query_cache! end @@ -514,6 +517,8 @@ module ActiveRecord assert_equal 0, Post.count assert_equal 0, Author.count assert_equal 0, AuthorAddress.count + ensure + reset_fixtures("posts", "authors", "author_addresses") end def test_truncate_tables_with_query_cache @@ -529,6 +534,7 @@ module ActiveRecord assert_equal 0, Author.count assert_equal 0, AuthorAddress.count ensure + reset_fixtures("posts", "authors", "author_addresses") @connection.disable_query_cache! end @@ -551,6 +557,16 @@ module ActiveRecord assert_nothing_raised { sub.save! } end end + + private + + def reset_fixtures(*fixture_names) + ActiveRecord::FixtureSet.reset_cache + + fixture_names.each do |fixture_name| + ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, fixture_name) + end + end end end diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index 88c2ac5d0a..c2c357d0c1 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -10,7 +10,15 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase ActiveRecord::Base.connection.send(:default_row_format) ActiveRecord::Base.connection.singleton_class.class_eval do alias_method :execute_without_stub, :execute - def execute(sql, name = nil) sql end + def execute(sql, name = nil) + ActiveSupport::Notifications.instrumenter.instrument( + "sql.active_record", + sql: sql, + name: name, + connection: self) do + sql + end + end end end @@ -89,17 +97,19 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase %w(SPATIAL FULLTEXT UNIQUE).each do |type| expected = "ALTER TABLE `people` ADD #{type} INDEX `index_people_on_last_name` (`last_name`)" - actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| - t.index :last_name, type: type + assert_sql(expected) do + ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| + t.index :last_name, type: type + end end - assert_equal expected, actual end expected = "ALTER TABLE `people` ADD INDEX `index_people_on_last_name` USING btree (`last_name`(10)), ALGORITHM = COPY" - actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| - t.index :last_name, length: 10, using: :btree, algorithm: :copy + assert_sql(expected) do + ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| + t.index :last_name, length: 10, using: :btree, algorithm: :copy + end end - assert_equal expected, actual end def test_drop_table diff --git a/activerecord/test/cases/ar_schema_test.rb b/activerecord/test/cases/ar_schema_test.rb index 9027cc582a..7aa6d089c5 100644 --- a/activerecord/test/cases/ar_schema_test.rb +++ b/activerecord/test/cases/ar_schema_test.rb @@ -160,7 +160,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end if subsecond_precision_supported? - def test_timestamps_sets_presicion_on_create_table + def test_timestamps_sets_precision_on_create_table ActiveRecord::Schema.define do create_table :has_timestamps do |t| t.timestamps @@ -171,7 +171,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) end - def test_timestamps_sets_presicion_on_change_table + def test_timestamps_sets_precision_on_change_table ActiveRecord::Schema.define do create_table :has_timestamps @@ -185,7 +185,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end if ActiveRecord::Base.connection.supports_bulk_alter? - def test_timestamps_sets_presicion_on_change_table_with_bulk + def test_timestamps_sets_precision_on_change_table_with_bulk ActiveRecord::Schema.define do create_table :has_timestamps @@ -199,7 +199,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end end - def test_timestamps_sets_presicion_on_add_timestamps + def test_timestamps_sets_precision_on_add_timestamps ActiveRecord::Schema.define do create_table :has_timestamps add_timestamps :has_timestamps, default: Time.now diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 5753bd7117..eaf88bf698 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -242,9 +242,9 @@ module ActiveRecord private def precision_implicit_default if current_adapter?(:Mysql2Adapter) - { presicion: 0 } + { precision: 0 } else - { presicion: nil } + { precision: nil } end end end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index cd73f6082e..53fe31e087 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -624,7 +624,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end assert_equal [:before_commit, :after_commit], @topic.history end @@ -634,7 +634,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.transaction(requires_new: true) do @topic.content = "foo" @topic.save! - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end end assert_equal [:before_commit, :after_commit], @topic.history @@ -655,7 +655,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) raise ActiveRecord::Rollback end assert_equal [:rollback], @topic.history diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index d5a4f12376..7bad3de343 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -65,7 +65,7 @@ class TransactionTest < ActiveRecord::TestCase def test_add_to_null_transaction topic = Topic.new - topic.add_to_transaction + topic.send(:add_to_transaction) end def test_successful_with_return diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 9a70934b7e..4f98a6b7fc 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -145,15 +145,17 @@ class ValidationsTest < ActiveRecord::TestCase end def test_validates_acceptance_of_with_undefined_attribute_methods - Topic.validates_acceptance_of(:approved) - topic = Topic.new(approved: true) - Topic.undefine_attribute_methods + klass = Class.new(Topic) + klass.validates_acceptance_of(:approved) + topic = klass.new(approved: true) + klass.undefine_attribute_methods assert topic.approved end def test_validates_acceptance_of_as_database_column - Topic.validates_acceptance_of(:approved) - topic = Topic.create("approved" => true) + klass = Class.new(Topic) + klass.validates_acceptance_of(:approved) + topic = klass.create("approved" => true) assert topic["approved"] end diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index ab400e5982..673cfde3d9 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Autoloading during initialization is deprecated. + + *Xavier Noria* + * Only force `:async` ActiveJob adapter to `:inline` during seeding. *BatedUrGonnaDie* diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 1cf44a480c..109c560c80 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "active_support/core_ext/string/inflections" +require "active_support/core_ext/array/conversions" + module Rails class Application module Finisher @@ -21,6 +24,40 @@ module Rails end end + # This will become an error if/when we remove classic mode. The plan is + # autoloaders won't be configured up to this point in the finisher, so + # constants just won't be found, raising regular NameError exceptions. + initializer :warn_if_autoloaded, before: :let_zeitwerk_take_over do + next if config.cache_classes + next if ActiveSupport::Dependencies.autoloaded_constants.empty? + + autoloaded = ActiveSupport::Dependencies.autoloaded_constants + constants = "constant".pluralize(autoloaded.size) + enum = autoloaded.to_sentence + have = autoloaded.size == 1 ? "has" : "have" + these = autoloaded.size == 1 ? "This" : "These" + example = autoloaded.first + example_klass = example.constantize.class + + ActiveSupport::DescendantsTracker.clear + ActiveSupport::Dependencies.clear + + ActiveSupport::Deprecation.warn(<<~WARNING) + Initialization autoloaded the #{constants} #{enum}. + + Being able to do this is deprecated. Autoloading during initialization is going + to be an error condition in future versions of Rails. + + Reloading does not reboot the application, and therefore code executed during + initialization does not run again. So, if you reload #{example}, for example, + the expected changes won't be reflected in that stale #{example_klass} object. + + #{these} autoloaded #{constants} #{have} been unloaded. + + Please, check the "Autoloading and Reloading Constants" guide for solutions. + WARNING + end + initializer :let_zeitwerk_take_over do if config.autoloader == :zeitwerk require "active_support/dependencies/zeitwerk_integration" diff --git a/railties/lib/rails/command/environment_argument.rb b/railties/lib/rails/command/environment_argument.rb index 0cb3f1ce1e..9945fd1430 100644 --- a/railties/lib/rails/command/environment_argument.rb +++ b/railties/lib/rails/command/environment_argument.rb @@ -9,7 +9,9 @@ module Rails extend ActiveSupport::Concern included do - class_attribute :environment_desc, default: "Specifies the environment to run this #{self.command_name} under (test/development/production)." + no_commands do + class_attribute :environment_desc, default: "Specifies the environment to run this #{self.command_name} under (test/development/production)." + end class_option :environment, aliases: "-e", type: :string, desc: environment_desc end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index b8e167b488..a2e3e781c0 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1704,6 +1704,61 @@ module ApplicationTests end end + test "autoloading during initialization gets deprecation message and clearing if config.cache_classes is false" do + app_file "lib/c.rb", <<~EOS + class C + extend ActiveSupport::DescendantsTracker + end + + class X < C + end + EOS + + app_file "app/models/d.rb", <<~EOS + require "c" + + class D < C + end + EOS + + app_file "config/initializers/autoload.rb", "D" + + app "development" + + # TODO: Test deprecation message, assert_depcrecated { app "development" } + # does not collect it. + + assert_equal [X], C.descendants + assert_empty ActiveSupport::Dependencies.autoloaded_constants + end + + test "autoloading during initialization triggers nothing if config.cache_classes is true" do + app_file "lib/c.rb", <<~EOS + class C + extend ActiveSupport::DescendantsTracker + end + + class X < C + end + EOS + + app_file "app/models/d.rb", <<~EOS + require "c" + + class D < C + end + EOS + + app_file "config/initializers/autoload.rb", "D" + + app "production" + + # TODO: Test no deprecation message is issued. + + assert_equal [X, D], C.descendants + end + + test "raises with proper error message if no database configuration found" do FileUtils.rm("#{app_path}/config/database.yml") err = assert_raises RuntimeError do |