kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Remove the dependent_restrict_raises option.

Browse source 
It's not really a good idea to have this as a global config option. We
should allow people to specify the behaviour per association.

There will now be two new values:

* :dependent => :restrict_with_exception implements the current
  behaviour of :restrict. :restrict itself is deprecated in favour of
  :restrict_with_exception.
* :dependent => :restrict_with_error implements the new behaviour - it
  adds an error to the owner if there are dependent records present

See #4727 for the original discussion of this.
This commit is contained in:
Jon Leighton 2012-08-10 12:33:51 +01:00
parent d1835db6b5
commit 5ad79989ef
12 changed files with 95 additions and 149 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
activerecord/CHANGELOG.md
View file

@ -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:
The `:restrict` option is renamed to `:restrict_with_exception` to
make this distinction explicit.
* 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.
*Manoj Kumar*
*Manoj Kumar & Jon Leighton*
* Added `create_join_table` migration helper to create HABTM join tables

27
activerecord/lib/active_record/associations.rb
View file

@ -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 objects when
# their owner is destroyed:
#
# * <tt>:destroy</tt> causes all the associated objects to also be destroyed
# * <tt>:delete</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
# [: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

42
activerecord/lib/active_record/associations/builder/association.rb
View file

@ -85,35 +85,35 @@ module ActiveRecord::Associations::Builder
raise ArgumentError, "The :dependent option expects either " \
"#{valid_options_message} (#{dependent.inspect})"
end
end
def dependent_restrict_raises?
ActiveRecord::Base.dependent_restrict_raises == true
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
if dependent == :restrict
ActiveSupport::Deprecation.warn(
"The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \
"provides the same functionality."
)
end
end
def define_restrict_dependency_method
def define_restrict_with_exception_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
raise ActiveRecord::DeleteRestrictionError.new(name)
end
end
end
alias define_restrict_dependency_method define_restrict_with_exception_dependency_method
def define_restrict_with_error_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?
key = has_one_macro ? "one" : "many"
errors.add(:base, :"restrict_dependent_destroy.#{key}",
:record => self.class.human_attribute_name(name).downcase)
false
end
end
end

4
activerecord/lib/active_record/associations/builder/has_many.rb
View file

@ -19,8 +19,8 @@ module ActiveRecord::Associations::Builder
def configure_dependency
if dependent = options[:dependent]
check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict]
dependent_restrict_deprecation_warning if dependent == :restrict
check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict,
:restrict_with_error, :restrict_with_exception]
send("define_#{dependent}_dependency_method")
model.before_destroy dependency_method_name

3
activerecord/lib/active_record/associations/builder/has_one.rb
View file

@ -25,8 +25,7 @@ module ActiveRecord::Associations::Builder
def configure_dependency
if dependent = options[:dependent]
check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict]
dependent_restrict_deprecation_warning if dependent == :restrict
check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
send("define_#{dependent}_dependency_method")
model.before_destroy dependency_method_name

9
activerecord/lib/active_record/core.rb
View file

@ -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

41
activerecord/test/cases/associations/has_many_associations_test.rb
View file

@ -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

68
activerecord/test/cases/associations/has_one_associations_test.rb
View file

@ -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,15 +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
assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy }
assert RestrictedWithExceptionFirm.exists?(:name => 'restrict')
assert firm.account.present?
end
def test_restrict_with_error
firm = RestrictedWithErrorFirm.create!(:name => 'restrict')
firm.create_account(:credit_limit => 10)
assert_not_nil firm.account
@ -185,34 +193,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
assert !firm.errors.empty?
assert_equal "Cannot delete record because a dependent account exists", firm.errors[:base].first
assert RestrictedFirm.exists?(:name => 'restrict')
assert RestrictedWithErrorFirm.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')
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 model exists", firm.errors[:base].first
assert RestrictedFirm.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,18 +506,6 @@ 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
klass = Class.new(ActiveRecord::Base)
assert_deprecated { klass.has_one :account, :dependent => :restrict }
assert_not_deprecated { klass.has_one :account }
ensure
ActiveRecord::Base.dependent_restrict_raises = option_before
end
def test_has_one_transaction
company = companies(:first_firm)
account = Account.find(1)

3
activerecord/test/cases/helper.rb
View file

@ -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

16
activerecord/test/models/company.rb
View file

@ -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

5
railties/lib/rails/generators/rails/app/templates/config/application.rb
View file

@ -56,11 +56,6 @@ module <%= app_const_base %>
# in your app. As such, your models will need to explicitly whitelist or blacklist accessible
# parameters by using an attr_accessible or attr_protected declaration.
<%= comment_if :skip_active_record %>config.active_record.whitelist_attributes = true
# Specifies whether or not has_many or has_one association option :dependent => :restrict raises
# an exception. If set to true, then an ActiveRecord::DeleteRestrictionError exception would be
# raised. If set to false, then an error will be added on the model instead.
<%= comment_if :skip_active_record %>config.active_record.dependent_restrict_raises = false
<% unless options.skip_sprockets? -%>
# Enable the asset pipeline.

5
railties/test/generators/app_generator_test.rb
View file

@ -367,11 +367,6 @@ class AppGeneratorTest < Rails::Generators::TestCase
assert_file "config/application.rb", /config\.active_record\.whitelist_attributes = true/
end
def test_active_record_dependent_restrict_raises_is_present_application_config
run_generator
assert_file "config/application.rb", /config\.active_record\.dependent_restrict_raises = false/
end
def test_pretend_option
output = run_generator [File.join(destination_root, "myapp"), "--pretend"]
assert_no_match(/run bundle install/, output)