Refactor we_can_set_the_inverse_on_this? to use a less bizarre name a…

…mongst other things

activerecord/lib/active_record/association_preload.rb

@@ -127,8 +127,7 @@ def add_preloaded_records_to_collection(parent_records, reflection_name, associa
           association_proxy = parent_record.send(reflection_name)
           association_proxy.loaded
           association_proxy.target.push(*Array.wrap(associated_record))
-
-          association_proxy.__send__(:set_inverse_instance, associated_record, parent_record)
+          association_proxy.send(:set_inverse_instance, associated_record)
         end
       end
 
@@ -157,7 +156,7 @@ def set_association_single_records(id_to_record_map, reflection_name, associated
           mapped_records = id_to_record_map[associated_record[key].to_s]
           mapped_records.each do |mapped_record|
             association_proxy = mapped_record.send("set_#{reflection_name}_target", associated_record)
-            association_proxy.__send__(:set_inverse_instance, associated_record, mapped_record)
+            association_proxy.send(:set_inverse_instance, associated_record)
           end
         end
 

activerecord/lib/active_record/associations/association_collection.rb

@@ -454,9 +454,7 @@ def find_target
             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 @@ def add_record_to_target_with_callbacks(record)
             @target << record
           end
           callback(:after_add, record)
-          set_inverse_instance(record, @owner)
+          set_inverse_instance(record)
           record
         end
 

activerecord/lib/active_record/associations/association_proxy.rb

@@ -217,6 +217,13 @@ def aliased_table
           @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 @@ def owner_quoted_id
           @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

activerecord/lib/active_record/associations/belongs_to_association.rb

@@ -32,7 +32,7 @@ def replace(record)
           @updated = true
         end
 
-        set_inverse_instance(record, @owner)
+        set_inverse_instance(record)
 
         loaded
         record
@@ -69,7 +69,7 @@ def find_target
               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 @@ def foreign_key_present
 
         # 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)

activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb

@@ -14,7 +14,7 @@ def replace(record)
           @updated = true
         end
 
-        set_inverse_instance(record, @owner)
+        set_inverse_instance(record)
         loaded
         record
       end
@@ -38,23 +38,13 @@ def stale_target?
 
       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 @@ def find_target
               :include => @reflection.options[:include]
             )
           end
-          set_inverse_instance(target, @owner)
+          set_inverse_instance(target)
           target
         end
 

activerecord/lib/active_record/associations/class_methods/join_dependency.rb

@@ -212,7 +212,7 @@ def construct_association(record, join_part, row)
               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 @@ def construct_association(record, join_part, row)
 
         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

activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb

@@ -110,6 +110,10 @@ def record_timestamp_columns(record)
             []
           end
         end
+
+        def invertible_for?(record)
+          false
+        end
     end
   end
 end

activerecord/lib/active_record/associations/has_many_association.rb

@@ -92,10 +92,6 @@ def construct_find_scope
         def construct_create_scope
           construct_owner_attributes
         end
-
-        def we_can_set_the_inverse_on_this?(record)
-          @reflection.inverse_of
-        end
     end
   end
 end

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -67,7 +67,7 @@ def find_target
         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

activerecord/lib/active_record/associations/has_one_association.rb

@@ -55,7 +55,7 @@ def replace(obj, dont_save = false)
           @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 @@ def find_target
           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 @@ def new_record(replace_existing)
           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)