Skip to content
Browse files

Make the query built by has_many ...., :dependent => :____ lazy since…

… all the information is not really available yet.
  • Loading branch information...
1 parent 386b7bf commit 13004d4f849682772060371273fda3312dd3b884 Carl Lerche committed Apr 2, 2010
Showing with 32 additions and 50 deletions.
  1. +22 −50 activerecord/lib/active_record/associations.rb
  2. +10 −0 activerecord/lib/active_record/reflection.rb
View
72 activerecord/lib/active_record/associations.rb
@@ -1495,13 +1495,6 @@ def add_touch_callbacks(reflection, touch_attribute)
# finder conditions.
def configure_dependency_for_has_many(reflection)
if reflection.options.include?(:dependent)
- # Add polymorphic type if the :as option is present
- dependent_conditions = []
- dependent_conditions << "#{reflection.primary_key_name} = \#{record.#{reflection.name}.send(:owner_quoted_id)}"
- dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as]
- dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.table_name) if reflection.options[:conditions]
- dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ")
- dependent_conditions = dependent_conditions.gsub('@', '\@')
case reflection.options[:dependent]
when :destroy
method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym
@@ -1510,51 +1503,30 @@ def configure_dependency_for_has_many(reflection)
end
before_destroy method_name
when :delete_all
- # before_destroy do |record|
- # self.class.send(:delete_all_has_many_dependencies,
- # record,
- # "posts",
- # Post,
- # %@...@) # this is a string literal like %(...)
- # end
- # end
- module_eval <<-CALLBACK
- before_destroy do |record|
- self.class.send(:delete_all_has_many_dependencies,
- record,
- "#{reflection.name}",
- #{reflection.class_name},
- %@#{dependent_conditions}@)
- end
- CALLBACK
+ before_destroy do |record|
+ self.class.send(:delete_all_has_many_dependencies,
+ record,
+ reflection.name,
+ reflection.klass,
+ reflection.dependent_conditions(record, self.class))
+ end
when :nullify
- # before_destroy do |record|
- # self.class.send(:nullify_has_many_dependencies,
- # record,
- # "posts",
- # Post,
- # "user_id",
- # %@...@) # this is a string literal like %(...)
- # end
- # end
- module_eval <<-CALLBACK
- before_destroy do |record|
- self.class.send(:nullify_has_many_dependencies,
- record,
- "#{reflection.name}",
- #{reflection.class_name},
- "#{reflection.primary_key_name}",
- %@#{dependent_conditions}@)
- end
- CALLBACK
- when :restrict
- method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym
- define_method(method_name) do
- unless send(reflection.name).empty?
- raise DeleteRestrictionError.new(reflection)
- end
+ before_destroy do |record|
+ self.class.send(:nullify_has_many_dependencies,
+ record,
+ reflection.name,
+ reflection.klass,
+ reflection.primary_key_name,
+ reflection.dependent_conditions(record, self.class))
+ end
+ when :restrict
+ method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym
+ define_method(method_name) do
+ unless send(reflection.name).empty?
+ raise DeleteRestrictionError.new(reflection)
end
- before_destroy method_name
+ end
+ before_destroy method_name
else
raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})"
end
View
10 activerecord/lib/active_record/reflection.rb
@@ -277,6 +277,16 @@ def validate?
!options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many)
end
+ def dependent_conditions(record, base_class)
+ dependent_conditions = []
+ dependent_conditions << "#{primary_key_name} = #{record.send(name).send(:owner_quoted_id)}"
+ dependent_conditions << "#{options[:as]}_type = '#{base_class.name}'" if options[:as]
+ dependent_conditions << klass.send(:sanitize_sql, options[:conditions]) if options[:conditions]
+ dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ")
+ dependent_conditions = dependent_conditions.gsub('@', '\@')
+ dependent_conditions
+ end
+
private
def derive_class_name
class_name = name.to_s.camelize

5 comments on commit 13004d4

@pixeltrix
Ruby on Rails member

@carllerche I don't know whether your aware of this ticket but this commit should fix that. Is there a testing coming for this fix?

@josevalim
Ruby on Rails member

@pixeltrix, can you provide a test case? Assign the ticket with a test case to me and I will apply it!

@carllerche

@pixeltrix, as far as I was aware, this commit was not to change any functionality. It was a simple refactor. As such, no test is needed. I was not aware that it fixed any ticket.

@josevalim
Ruby on Rails member

This only proves that refactors leads to better code and less bugs. :)

@pixeltrix
Ruby on Rails member

@josevalim I've added a test only patch for master to the lighthouse ticket. Patch file is here.

Please sign in to comment.
Something went wrong with that request. Please try again.