diff options
Diffstat (limited to 'activerecord')
24 files changed, 287 insertions, 382 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a99d7fdde8..e6237ef437 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -486,24 +486,13 @@ * Added the `ActiveRecord::NullRelation` class implementing the null object pattern for the Relation class. *Juanjo Bazán* -* Added deprecation for the `:dependent => :restrict` association option. +* Added new `:dependent => :restrict_with_error` option. This will add + an error to the model, rather than raising an exception. - Please note: - - * Up until now `has_many` and `has_one`, `:dependent => :restrict` - option raised a `DeleteRestrictionError` at the time of destroying - the object. Instead, it will add an error on the model. - - * To fix this warning, make sure your code isn't relying on a - `DeleteRestrictionError` and then add - `config.active_record.dependent_restrict_raises = false` to your - application config. - - * New rails application would be generated with the - `config.active_record.dependent_restrict_raises = false` in the - application config. + The `:restrict` option is renamed to `:restrict_with_exception` to + make this distinction explicit. - *Manoj Kumar* + *Manoj Kumar & Jon Leighton* * Added `create_join_table` migration helper to create HABTM join tables diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index f5ee4f3ebe..9ba3323bc7 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1094,12 +1094,14 @@ module ActiveRecord # [:primary_key] # Specify the method that returns the primary key used for the association. By default this is +id+. # [:dependent] - # If set to <tt>:destroy</tt> all the associated objects are destroyed - # alongside this object by calling their +destroy+ method. If set to <tt>:delete_all</tt> all associated - # objects are deleted *without* calling their +destroy+ method. If set to <tt>:nullify</tt> all associated - # objects' foreign keys are set to +NULL+ *without* calling their +save+ callbacks. If set to - # <tt>:restrict</tt> an error will be added to the object, preventing its deletion, if any associated - # objects are present. + # Controls what happens to the associated objects when + # their owner is destroyed: + # + # * <tt>:destroy</tt> causes all the associated objects to also be destroyed + # * <tt>:delete_all</tt> causes all the asssociated objects to be deleted directly from the database (so callbacks will not execute) + # * <tt>:nullify</tt> causes the foreign keys to be set to +NULL+. Callbacks are not executed. + # * <tt>:restrict_with_exception</tt> causes an exception to be raised if there are any associated records + # * <tt>:restrict_with_error</tt> causes an error to be added to the owner if there are any associated objects # # If using with the <tt>:through</tt> option, the association on the join model must be # a +belongs_to+, and the records which get deleted are the join records, rather than @@ -1203,11 +1205,14 @@ module ActiveRecord # from the association name. So <tt>has_one :manager</tt> will by default be linked to the Manager class, but # if the real class name is Person, you'll have to specify it with this option. # [:dependent] - # If set to <tt>:destroy</tt>, the associated object is destroyed when this object is. If set to - # <tt>:delete</tt>, the associated object is deleted *without* calling its destroy method. - # If set to <tt>:nullify</tt>, the associated object's foreign key is set to +NULL+. - # If set to <tt>:restrict</tt>, an error will be added to the object, preventing its deletion, if an - # associated object is present. + # Controls what happens to the associated object when + # its owner is destroyed: + # + # * <tt>:destroy</tt> causes the associated object to also be destroyed + # * <tt>:delete</tt> causes the asssociated object to be deleted directly from the database (so callbacks will not execute) + # * <tt>:nullify</tt> causes the foreign key to be set to +NULL+. Callbacks are not executed. + # * <tt>:restrict_with_exception</tt> causes an exception to be raised if there is an associated record + # * <tt>:restrict_with_error</tt> causes an error to be added to the owner if there is an associated object # [:foreign_key] # Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a +has_one+ association diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index ddfc6f6c05..75f72c1a46 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -2,6 +2,11 @@ module ActiveRecord # = Active Record Belongs To Associations module Associations class BelongsToAssociation < SingularAssociation #:nodoc: + + def handle_dependency + target.send(options[:dependent]) if load_target + end + def replace(record) raise_on_type_mismatch(record) if record diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index c3f32b5ed9..1df876bf62 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -39,6 +39,7 @@ module ActiveRecord::Associations::Builder def build validate_options define_accessors + configure_dependency if options[:dependent] @reflection = model.create_reflection(macro, name, scope, options, model) super # provides an extension point @reflection @@ -52,70 +53,54 @@ module ActiveRecord::Associations::Builder Association.valid_options end - private - - def validate_options - options.assert_valid_keys(valid_options) - end + def validate_options + options.assert_valid_keys(valid_options) + end - def define_accessors - define_readers - define_writers - end + def define_accessors + define_readers + define_writers + end - def define_readers - name = self.name - mixin.redefine_method(name) do |*params| - association(name).reader(*params) + def define_readers + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name}(*args) + association(:#{name}).reader(*args) end - end + CODE + end - def define_writers - name = self.name - mixin.redefine_method("#{name}=") do |value| - association(name).writer(value) + def define_writers + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name}=(value) + association(:#{name}).writer(value) end - end - - def check_valid_dependent!(dependent, valid_options) - unless valid_options.include?(dependent) - valid_options_message = valid_options.map(&:inspect).to_sentence( - words_connector: ', ', two_words_connector: ' or ', last_word_connector: ' or ') + CODE + end - raise ArgumentError, "The :dependent option expects either " \ - "#{valid_options_message} (#{dependent.inspect})" - end + def configure_dependency + unless valid_dependent_options.include? options[:dependent] + raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{options[:dependent]}" end - def dependent_restrict_raises? - ActiveRecord::Base.dependent_restrict_raises == true + if options[:dependent] == :restrict + ActiveSupport::Deprecation.warn( + "The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \ + "provides the same functionality." + ) end - def dependent_restrict_deprecation_warning - if dependent_restrict_raises? - msg = "In the next release, `:dependent => :restrict` will not raise a `DeleteRestrictionError`. "\ - "Instead, it will add an error on the model. To fix this warning, make sure your code " \ - "isn't relying on a `DeleteRestrictionError` and then add " \ - "`config.active_record.dependent_restrict_raises = false` to your application config." - ActiveSupport::Deprecation.warn msg + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{macro}_dependent_for_#{name} + association(:#{name}).handle_dependency end - end + CODE - def define_restrict_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - has_one_macro = association(name).reflection.macro == :has_one - if has_one_macro ? !send(name).nil? : send(name).exists? - if dependent_restrict_raises? - raise ActiveRecord::DeleteRestrictionError.new(name) - else - key = has_one_macro ? "one" : "many" - errors.add(:base, :"restrict_dependent_destroy.#{key}", - :record => self.class.human_attribute_name(name).downcase) - return false - end - end - end - end + model.before_destroy "#{macro}_dependent_for_#{name}" + end + + def valid_dependent_options + raise NotImplementedError + end end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index f205a456f7..2f2600b7fb 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: def macro @@ -17,72 +16,51 @@ module ActiveRecord::Associations::Builder reflection = super add_counter_cache_callbacks(reflection) if options[:counter_cache] add_touch_callbacks(reflection) if options[:touch] - configure_dependency reflection end - private - - def add_counter_cache_callbacks(reflection) - cache_column = reflection.counter_cache_column - name = self.name + def add_counter_cache_callbacks(reflection) + cache_column = reflection.counter_cache_column - method_name = "belongs_to_counter_cache_after_create_for_#{name}" - mixin.redefine_method(method_name) do - record = send(name) - record.class.increment_counter(cache_column, record.id) unless record.nil? + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def belongs_to_counter_cache_after_create_for_#{name} + record = #{name} + record.class.increment_counter(:#{cache_column}, record.id) unless record.nil? end - model.after_create(method_name) - method_name = "belongs_to_counter_cache_before_destroy_for_#{name}" - mixin.redefine_method(method_name) do + def belongs_to_counter_cache_before_destroy_for_#{name} unless marked_for_destruction? - record = send(name) - record.class.decrement_counter(cache_column, record.id) unless record.nil? + record = #{name} + record.class.decrement_counter(:#{cache_column}, record.id) unless record.nil? end end - model.before_destroy(method_name) + CODE - model.send(:module_eval, - "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__ - ) - end + model.after_create "belongs_to_counter_cache_after_create_for_#{name}" + model.before_destroy "belongs_to_counter_cache_before_destroy_for_#{name}" - def add_touch_callbacks(reflection) - name = self.name - method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}" - touch = options[:touch] + klass = reflection.class_name.safe_constantize + klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) + end - mixin.redefine_method(method_name) do - record = send(name) + def add_touch_callbacks(reflection) + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def belongs_to_touch_after_save_or_destroy_for_#{name} + record = #{name} unless record.nil? - if touch == true - record.touch - else - record.touch(touch) - end + record.touch #{options[:touch].inspect if options[:touch] != true} end end + CODE - model.after_save(method_name) - model.after_touch(method_name) - model.after_destroy(method_name) - end - - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete] + model.after_save "belongs_to_touch_after_save_or_destroy_for_#{name}" + model.after_touch "belongs_to_touch_after_save_or_destroy_for_#{name}" + model.after_destroy "belongs_to_touch_after_save_or_destroy_for_#{name}" + end - method_name = "belongs_to_dependent_#{dependent}_for_#{name}" - model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1) - def #{method_name} - association = #{name} - association.#{dependent} if association - end - eoruby - model.after_destroy method_name - end - end + def valid_dependent_options + [:destroy, :delete] + end end end diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 3fb0a57450..1b382f7285 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] @@ -34,53 +33,53 @@ module ActiveRecord::Associations::Builder end end - private - - def wrap_block_extension - if block_extension - @extension_module = mod = Module.new(&block_extension) - silence_warnings do - model.parent.const_set(extension_module_name, mod) - end + def wrap_block_extension + if block_extension + @extension_module = mod = Module.new(&block_extension) + silence_warnings do + model.parent.const_set(extension_module_name, mod) + end - prev_scope = @scope + prev_scope = @scope - if prev_scope - @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) } - else - @scope = proc { extending(mod) } - end + if prev_scope + @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) } + else + @scope = proc { extending(mod) } end end + end - def extension_module_name - @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - end + def extension_module_name + @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" + end - def define_callback(callback_name) - full_callback_name = "#{callback_name}_for_#{name}" + def define_callback(callback_name) + full_callback_name = "#{callback_name}_for_#{name}" - # TODO : why do i need method_defined? I think its because of the inheritance chain - model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name) - model.send("#{full_callback_name}=", Array(options[callback_name.to_sym])) - end + # TODO : why do i need method_defined? I think its because of the inheritance chain + model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name) + model.send("#{full_callback_name}=", Array(options[callback_name.to_sym])) + end - def define_readers - super + def define_readers + super - name = self.name - mixin.redefine_method("#{name.to_s.singularize}_ids") do - association(name).ids_reader + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name.to_s.singularize}_ids + association(:#{name}).ids_reader end - end + CODE + end - def define_writers - super + def define_writers + super - name = self.name - mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids| - association(name).ids_writer(ids) + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{name.to_s.singularize}_ids=(ids) + association(:#{name}).ids_writer(ids) end - end + CODE + end end end diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 8df28ad876..bdac02b5bf 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -24,18 +24,16 @@ module ActiveRecord::Associations::Builder end end - private - - def define_destroy_hook - name = self.name - model.send(:include, Module.new { - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def destroy_associations - association(#{name.to_sym.inspect}).delete_all - super - end - RUBY - }) - end + def define_destroy_hook + name = self.name + model.send(:include, Module.new { + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def destroy_associations + association(:#{name}).delete_all + super + end + RUBY + }) + end end end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 9e60dbc30b..ab8225460a 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: def macro @@ -9,52 +8,8 @@ module ActiveRecord::Associations::Builder super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of] end - def build - reflection = super - configure_dependency - reflection + def valid_dependent_options + [:destroy, :delete_all, :nullify, :restrict, :restrict_with_error, :restrict_with_exception] end - - private - - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict] - dependent_restrict_deprecation_warning if dependent == :restrict - - send("define_#{dependent}_dependency_method") - model.before_destroy dependency_method_name - end - end - - def define_destroy_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - send(name).each do |o| - # No point in executing the counter update since we're going to destroy the parent anyway - o.mark_for_destruction - end - - send(name).delete_all - end - end - - def define_delete_all_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - association(name).delete_all - end - end - - def define_nullify_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - send(name).delete_all - end - end - - def dependency_method_name - "has_many_dependent_for_#{name}" - end end end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 9c84f1913a..0da564f402 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: def macro @@ -15,35 +14,12 @@ module ActiveRecord::Associations::Builder !options[:through] end - def build - reflection = super - configure_dependency unless options[:through] - reflection + def configure_dependency + super unless options[:through] end - private - - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict] - dependent_restrict_deprecation_warning if dependent == :restrict - - send("define_#{dependent}_dependency_method") - model.before_destroy dependency_method_name - end - end - - def define_destroy_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - association(name).delete - end - end - alias :define_delete_dependency_method :define_destroy_dependency_method - alias :define_nullify_dependency_method :define_destroy_dependency_method - - def dependency_method_name - "has_one_dependent_#{options[:dependent]}_for_#{name}" - end + def valid_dependent_options + [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception] + end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 90a4b7c2ef..6a5830e57f 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -13,22 +13,20 @@ module ActiveRecord::Associations::Builder define_constructors if constructable? end - private - - def define_constructors - name = self.name - - mixin.redefine_method("build_#{name}") do |*params, &block| - association(name).build(*params, &block) + def define_constructors + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def build_#{name}(*args, &block) + association(:#{name}).build(*args, &block) end - mixin.redefine_method("create_#{name}") do |*params, &block| - association(name).create(*params, &block) + def create_#{name}(*args, &block) + association(:#{name}).create(*args, &block) end - mixin.redefine_method("create_#{name}!") do |*params, &block| - association(name).create!(*params, &block) + def create_#{name}!(*args, &block) + association(:#{name}).create!(*args, &block) end - end + CODE + end end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 7a363c896e..74864d271f 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -7,6 +7,28 @@ module ActiveRecord # is provided by its child HasManyThroughAssociation. class HasManyAssociation < CollectionAssociation #:nodoc: + def handle_dependency + case options[:dependent] + when :restrict, :restrict_with_exception + raise ActiveRecord::DeleteRestrictionError.new(reflection.name) unless empty? + + when :restrict_with_error + unless empty? + record = klass.human_attribute_name(reflection.name).downcase + owner.errors.add(:base, :"restrict_dependent_destroy.many", record: record) + false + end + + else + if options[:dependent] == :destroy + # No point in executing the counter update since we're going to destroy the parent anyway + load_target.each(&:mark_for_destruction) + end + + delete_all + end + end + def insert_record(record, validate = true, raise = false) set_owner_attributes(record) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 191c4e8e39..dd7da59a86 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -3,23 +3,43 @@ module ActiveRecord # = Active Record Belongs To Has One Association module Associations class HasOneAssociation < SingularAssociation #:nodoc: - def replace(record, save = true) - raise_on_type_mismatch(record) if record - load_target - reflection.klass.transaction do - if target && target != record - remove_target!(options[:dependent]) unless target.destroyed? + def handle_dependency + case options[:dependent] + when :restrict, :restrict_with_exception + raise ActiveRecord::DeleteRestrictionError.new(reflection.name) if load_target + + when :restrict_with_error + if load_target + record = klass.human_attribute_name(reflection.name).downcase + owner.errors.add(:base, :"restrict_dependent_destroy.one", record: record) + false end - if record - set_owner_attributes(record) - set_inverse_instance(record) + else + delete + end + end - if owner.persisted? && save && !record.save - nullify_owner_attributes(record) - set_owner_attributes(target) if target - raise RecordNotSaved, "Failed to save the new associated #{reflection.name}." + def replace(record, save = true) + raise_on_type_mismatch(record) if record + load_target + + # If target and record are nil, or target is equal to record, + # we don't need to have transaction. + if (target || record) && target != record + reflection.klass.transaction do + remove_target!(options[:dependent]) if target && !target.destroyed? + + if record + set_owner_attributes(record) + set_inverse_instance(record) + + if owner.persisted? && save && !record.save + nullify_owner_attributes(record) + set_owner_attributes(target) if target + raise RecordNotSaved, "Failed to save the new associated #{reflection.name}." + end end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 1145d2138c..0fddfdf0cb 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -82,15 +82,6 @@ module ActiveRecord # The connection handler config_attribute :connection_handler - ## - # :singleton-method: - # Specifies whether or not has_many or has_one association option - # :dependent => :restrict raises an exception. If set to true, the - # ActiveRecord::DeleteRestrictionError exception will be raised - # along with a DEPRECATION WARNING. If set to false, an error would - # be added to the model instead. - config_attribute :dependent_restrict_raises - %w(logger configurations default_timezone schema_format timestamped_migrations).each do |name| config_attribute name, global: true end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index ec7e4f5fb7..5f7825783b 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -524,13 +524,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase def test_invalid_belongs_to_dependent_option_nullify_raises_exception assert_raise ArgumentError do - Author.belongs_to :special_author_address, :dependent => :nullify + Class.new(Author).belongs_to :special_author_address, :dependent => :nullify end end def test_invalid_belongs_to_dependent_option_restrict_raises_exception assert_raise ArgumentError do - Author.belongs_to :special_author_address, :dependent => :restrict + Class.new(Author).belongs_to :special_author_address, :dependent => :restrict end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 43440c1146..04714f42e9 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1091,9 +1091,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - firm = RestrictedFirm.create!(:name => 'restrict') firm.companies.create(:name => 'child') @@ -1101,15 +1098,25 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } assert RestrictedFirm.exists?(:name => 'restrict') assert firm.companies.exists?(:name => 'child') - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_restrict_when_dependent_restrict_raises_config_set_to_false - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false + def test_restrict_is_deprecated + klass = Class.new(ActiveRecord::Base) + assert_deprecated { klass.has_many :posts, dependent: :restrict } + end - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_exception + firm = RestrictedWithExceptionFirm.create!(:name => 'restrict') + firm.companies.create(:name => 'child') + + assert !firm.companies.empty? + assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } + assert RestrictedWithExceptionFirm.exists?(:name => 'restrict') + assert firm.companies.exists?(:name => 'child') + end + + def test_restrict_with_error + firm = RestrictedWithErrorFirm.create!(:name => 'restrict') firm.companies.create(:name => 'child') assert !firm.companies.empty? @@ -1119,10 +1126,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !firm.errors.empty? assert_equal "Cannot delete record because dependent companies exist", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert RestrictedWithErrorFirm.exists?(:name => 'restrict') assert firm.companies.exists?(:name => 'child') - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end def test_included_in_collection @@ -1602,18 +1607,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [bulb1, bulb3], result end - def test_building_has_many_association_with_restrict_dependency - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - - klass = Class.new(ActiveRecord::Base) - - assert_deprecated { klass.has_many :companies, :dependent => :restrict } - assert_not_deprecated { klass.has_many :companies } - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before - end - def test_collection_association_with_private_kernel_method firm = companies(:first_firm) assert_equal [accounts(:signals37)], firm.accounts.open diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 112735839f..8bc633f2b5 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -156,10 +156,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { firm.destroy } end - def test_dependence_with_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - + def test_restrict firm = RestrictedFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) @@ -168,38 +165,26 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } assert RestrictedFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_dependence_with_restrict_with_dependent_restrict_raises_config_set_to_false - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false + def test_restrict_is_deprecated + klass = Class.new(ActiveRecord::Base) + assert_deprecated { klass.has_one :post, dependent: :restrict } + end - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_exception + firm = RestrictedWithExceptionFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) assert_not_nil firm.account - firm.destroy - - assert !firm.errors.empty? - assert_equal "Cannot delete record because a dependent account exists", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } + assert RestrictedWithExceptionFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_dependence_with_restrict_with_dependent_restrict_raises_config_set_to_false_and_attribute_name - old_backend = I18n.backend - I18n.backend = I18n::Backend::Simple.new - I18n.backend.store_translations 'en', :activerecord => {:attributes => {:restricted_firm => {:account => "account model"}}} - - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false - - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_error + firm = RestrictedWithErrorFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) assert_not_nil firm.account @@ -207,12 +192,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm.destroy assert !firm.errors.empty? - assert_equal "Cannot delete record because a dependent account model exists", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert_equal "Cannot delete record because a dependent account exists", firm.errors[:base].first + assert RestrictedWithErrorFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before - I18n.backend = old_backend end def test_successful_build_association @@ -524,15 +506,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal car.id, bulb.attributes_after_initialize['car_id'] end - def test_building_has_one_association_with_dependent_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true + def test_has_one_transaction + company = companies(:first_firm) + account = Account.find(1) - klass = Class.new(ActiveRecord::Base) + company.account # force loading + assert_no_queries { company.account = account } - assert_deprecated { klass.has_one :account, :dependent => :restrict } - assert_not_deprecated { klass.has_one :account } - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before + company.account = nil + assert_no_queries { company.account = nil } + account = Account.find(2) + assert_queries { company.account = account } end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 3cea20527e..86893ec4b3 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -385,7 +385,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_polymorphic_has_one - assert_equal Tagging.find(1,2).sort_by { |t| t.id }, authors(:david).tagging + assert_equal Tagging.find(1,2).sort_by { |t| t.id }, authors(:david).taggings_2 end def test_has_many_through_polymorphic_has_many diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 018064233a..4c6d4666ed 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -19,9 +19,6 @@ require 'support/connection' # Show backtraces for deprecated behavior for quicker cleanup. ActiveSupport::Deprecation.debug = true -# Avoid deprecation warning setting dependent_restrict_raises to false. The default is true -ActiveRecord::Base.dependent_restrict_raises = false - # Connect to the database ARTest.connect diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index 06d6596725..42461e8ecb 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -1,9 +1,7 @@ require "cases/helper" require 'models/entrant' require 'models/bird' - -# So we can test whether Course.connection survives a reload. -require_dependency 'models/course' +require 'models/course' class MultipleDbTest < ActiveRecord::TestCase self.use_transactional_fixtures = false diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 7444dc5de1..bb034848e1 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -114,9 +114,12 @@ class TimestampTest < ActiveRecord::TestCase end def test_saving_a_record_with_a_belongs_to_that_specifies_touching_a_specific_attribute_the_parent_should_update_that_attribute - Pet.belongs_to :owner, :touch => :happy_at + klass = Class.new(ActiveRecord::Base) do + def self.name; 'Pet'; end + belongs_to :owner, :touch => :happy_at + end - pet = Pet.first + pet = klass.first owner = pet.owner previously_owner_happy_at = owner.happy_at @@ -124,14 +127,15 @@ class TimestampTest < ActiveRecord::TestCase pet.save assert_not_equal previously_owner_happy_at, pet.owner.happy_at - ensure - Pet.belongs_to :owner, :touch => true end def test_touching_a_record_with_a_belongs_to_that_uses_a_counter_cache_should_update_the_parent - Pet.belongs_to :owner, :counter_cache => :use_count, :touch => true + klass = Class.new(ActiveRecord::Base) do + def self.name; 'Pet'; end + belongs_to :owner, :counter_cache => :use_count, :touch => true + end - pet = Pet.first + pet = klass.first owner = pet.owner owner.update_columns(happy_at: 3.days.ago) previously_owner_updated_at = owner.updated_at @@ -140,15 +144,15 @@ class TimestampTest < ActiveRecord::TestCase pet.save assert_not_equal previously_owner_updated_at, pet.owner.updated_at - ensure - Pet.belongs_to :owner, :touch => true end def test_touching_a_record_touches_parent_record_and_grandparent_record - Toy.belongs_to :pet, :touch => true - Pet.belongs_to :owner, :touch => true + klass = Class.new(ActiveRecord::Base) do + def self.name; 'Toy'; end + belongs_to :pet, :touch => true + end - toy = Toy.first + toy = klass.first pet = toy.pet owner = pet.owner time = 3.days.ago @@ -158,8 +162,6 @@ class TimestampTest < ActiveRecord::TestCase owner.reload assert_not_equal time, owner.updated_at - ensure - Toy.belongs_to :pet end def test_timestamp_attributes_for_create diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 3157d8fe7f..77f4a2ec87 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -93,8 +93,8 @@ class Author < ActiveRecord::Base has_many :author_favorites has_many :favorite_authors, -> { order('name') }, :through => :author_favorites - has_many :tagging, :through => :posts has_many :taggings, :through => :posts + has_many :taggings_2, :through => :posts, :source => :tagging has_many :tags, :through => :posts has_many :post_categories, :through => :posts, :source => :categories has_many :tagging_tags, :through => :taggings, :source => :tag diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 5bfbb5e855..75f38d275c 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -115,8 +115,20 @@ class DependentFirm < Company end class RestrictedFirm < Company - has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict - has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict + ActiveSupport::Deprecation.silence do + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict + end +end + +class RestrictedWithExceptionFirm < Company + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict_with_exception + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict_with_exception +end + +class RestrictedWithErrorFirm < Company + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict_with_error + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict_with_error end class Client < Company diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index 359b29fac3..1134b09d8b 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -24,11 +24,10 @@ class Member < ActiveRecord::Base has_one :club_category, :through => :club, :source => :category - has_many :current_memberships - has_one :club_through_many, :through => :current_memberships, :source => :club - has_many :current_memberships, -> { where :favourite => true } has_many :clubs, :through => :current_memberships + + has_one :club_through_many, :through => :current_memberships, :source => :club end class SelfMember < ActiveRecord::Base diff --git a/activerecord/test/support/connection.rb b/activerecord/test/support/connection.rb index c176316a05..92736e0ca9 100644 --- a/activerecord/test/support/connection.rb +++ b/activerecord/test/support/connection.rb @@ -1,6 +1,6 @@ require 'active_support/logger' -require_dependency 'models/college' -require_dependency 'models/course' +require 'models/college' +require 'models/course' module ARTest def self.connection_name |