From 9f5c18ce075179cfc73a00cba9a19d69aaf5274c Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 26 Dec 2010 21:37:25 +0000 Subject: Refactor we_can_set_the_inverse_on_this? to use a less bizarre name amongst other things --- .../associations/association_collection.rb | 6 ++---- .../associations/association_proxy.rb | 24 ++++++++++++++-------- .../associations/belongs_to_association.rb | 9 ++++---- .../belongs_to_polymorphic_association.rb | 24 +++++++--------------- .../associations/class_methods/join_dependency.rb | 4 ++-- .../has_and_belongs_to_many_association.rb | 4 ++++ .../associations/has_many_association.rb | 4 ---- .../associations/has_many_through_association.rb | 2 +- .../associations/has_one_association.rb | 11 +++------- 9 files changed, 39 insertions(+), 49 deletions(-) (limited to 'activerecord/lib/active_record/associations') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 86cc17394c..97d9288874 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -454,9 +454,7 @@ module ActiveRecord end records = @reflection.options[:uniq] ? uniq(records) : records - records.each do |record| - set_inverse_instance(record, @owner) - end + records.each { |record| set_inverse_instance(record) } records end @@ -470,7 +468,7 @@ module ActiveRecord @target << record end callback(:after_add, record) - set_inverse_instance(record, @owner) + set_inverse_instance(record) record end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index b60aa33c98..6720d83199 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -217,6 +217,13 @@ module ActiveRecord @reflection.klass.arel_table end + # Set the inverse association, if possible + def set_inverse_instance(record) + if record && invertible_for?(record) + record.send("set_#{inverse_reflection_for(record).name}_target", @owner) + end + end + private # Forwards any missing method call to the \target. def method_missing(method, *args) @@ -280,17 +287,16 @@ module ActiveRecord @owner.quoted_id end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.inverse_of - unless inverse_relationship.nil? - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + # Can be redefined by subclasses, notably polymorphic belongs_to + # The record parameter is necessary to support polymorphic inverses as we must check for + # the association in the specific class of the record. + def inverse_reflection_for(record) + @reflection.inverse_of end - # Override in subclasses - def we_can_set_the_inverse_on_this?(record) - false + # Is this association invertible? Can be redefined by subclasses. + def invertible_for?(record) + inverse_reflection_for(record) end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index bbfe18f9fb..98c1c13524 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -32,7 +32,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record @@ -69,7 +69,7 @@ module ActiveRecord options ) if @owner[@reflection.primary_key_name] end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end @@ -83,8 +83,9 @@ module ActiveRecord # NOTE - for now, we're only supporting inverse setting from belongs_to back onto # has_one associations. - def we_can_set_the_inverse_on_this?(record) - @reflection.has_inverse? && @reflection.inverse_of.macro == :has_one + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def record_id(record) diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index c580de7fbe..90eff7399c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -14,7 +14,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record end @@ -38,23 +38,13 @@ module ActiveRecord private - # NOTE - for now, we're only supporting inverse setting from belongs_to back onto - # has_one associations. - def we_can_set_the_inverse_on_this?(record) - if @reflection.has_inverse? - inverse_association = @reflection.polymorphic_inverse_of(record.class) - inverse_association && inverse_association.macro == :has_one - else - false - end + def inverse_reflection_for(record) + @reflection.polymorphic_inverse_of(record.class) end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.polymorphic_inverse_of(record.class) - if inverse_relationship - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def construct_find_scope @@ -71,7 +61,7 @@ module ActiveRecord :include => @reflection.options[:include] ) end - set_inverse_instance(target, @owner) + set_inverse_instance(target) target end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb index a74d0a4ca8..bb6145656e 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -212,7 +212,7 @@ module ActiveRecord collection = record.send(join_part.reflection.name) collection.loaded collection.target.push(association) - collection.__send__(:set_inverse_instance, association, record) + collection.send(:set_inverse_instance, association) when :belongs_to set_target_and_inverse(join_part, association, record) else @@ -224,7 +224,7 @@ module ActiveRecord def set_target_and_inverse(join_part, association, record) association_proxy = record.send("set_#{join_part.reflection.name}_target", association) - association_proxy.__send__(:set_inverse_instance, association, record) + association_proxy.send(:set_inverse_instance, association) end end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 24871303eb..b1d454545f 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -110,6 +110,10 @@ module ActiveRecord [] end end + + def invertible_for?(record) + false + end 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 d35ecfd603..c3360463cd 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -92,10 +92,6 @@ module ActiveRecord def construct_create_scope construct_owner_attributes end - - def we_can_set_the_inverse_on_this?(record) - @reflection.inverse_of - end end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 8569a2f004..ba464c8d79 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -67,7 +67,7 @@ module ActiveRecord end # NOTE - not sure that we can actually cope with inverses here - def we_can_set_the_inverse_on_this?(record) + def invertible_for?(record) false end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index ca0828ea7b..c32aaf986e 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -55,7 +55,7 @@ module ActiveRecord @target = (AssociationProxy === obj ? obj.target : obj) end - set_inverse_instance(obj, @owner) + set_inverse_instance(obj) @loaded = true unless !@owner.persisted? || obj.nil? || dont_save @@ -81,7 +81,7 @@ module ActiveRecord the_target = with_scope(:find => @scope[:find]) do @reflection.klass.find(:first, options) end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end @@ -107,17 +107,12 @@ module ActiveRecord else record[@reflection.primary_key_name] = @owner.id if @owner.persisted? self.target = record - set_inverse_instance(record, @owner) + set_inverse_instance(record) end record end - def we_can_set_the_inverse_on_this?(record) - inverse = @reflection.inverse_of - return !inverse.nil? - end - def merge_with_conditions(attrs={}) attrs ||= {} attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) -- cgit v1.2.3