Merge pull request #15210 from arthurnn/fix_hbtm_reflection

Fix habtm reflection
Conflicts:
    activerecord/CHANGELOG.md
    activerecord/lib/active_record/counter_cache.rb
    activerecord/lib/active_record/reflection.rb
    activerecord/test/cases/reflection_test.rb

activerecord/CHANGELOG.md

+*   Fix has_and_belongs_to_many public reflection.
+    When defining a has_and_belongs_to_many, internally we convert that to two has_many.
+    But as `reflections` is a public API, people expect to see the right macro.
+
+    Fixes #14682.
+
+    *arthurnn*
+
 *   Fixed serialization for records with an attribute named `format`.
 
     Fixes #15188.

activerecord/lib/active_record/associations.rb

@@ -50,7 +50,7 @@ class HasManyThroughSourceAssociationNotFoundError < ActiveRecordError #:nodoc:
     def initialize(reflection)
       through_reflection      = reflection.through_reflection
       source_reflection_names = reflection.source_reflection_names
-      source_associations     = reflection.through_reflection.klass.reflect_on_all_associations.collect { |a| a.name.inspect }
+      source_associations     = reflection.through_reflection.klass._reflections.keys
       super("Could not find the source association(s) #{source_reflection_names.collect{ |a| a.inspect }.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => <name>'. Is it one of #{source_associations.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)}?")
     end
   end
@@ -151,7 +151,7 @@ def association(name) #:nodoc:
       association = association_instance_get(name)
 
       if association.nil?
-        raise AssociationNotFoundError.new(self, name) unless reflection = self.class.reflect_on_association(name)
+        raise AssociationNotFoundError.new(self, name) unless reflection = self.class._reflect_on_association(name)
         association = reflection.association_class.new(self, reflection)
         association_instance_set(name, association)
       end
@@ -1610,6 +1610,9 @@ def destroy_associations
         end
 
         has_many name, scope, hm_options, &extension
+
+        reflection = ActiveRecord::Reflection::AssociationReflection.new(:has_and_belongs_to_many, name, scope, options, self)
+        self.reflections = self.reflections.except(middle_reflection.name).merge!(name => reflection)
       end
     end
   end

activerecord/lib/active_record/associations/association.rb

@@ -160,7 +160,7 @@ def marshal_dump
       def marshal_load(data)
         reflection_name, ivars = data
         ivars.each { |name, val| instance_variable_set(name, val) }
-        @reflection = @owner.class.reflect_on_association(reflection_name)
+        @reflection = @owner.class._reflect_on_association(reflection_name)
       end
 
       def initialize_attributes(record) #:nodoc:

activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb

@@ -66,13 +66,13 @@ def self.compute_type(class_name)
 
         def self.add_left_association(name, options)
           belongs_to name, options
-          self.left_reflection = reflect_on_association(name)
+          self.left_reflection = _reflect_on_association(name)
         end
 
         def self.add_right_association(name, options)
           rhs_name = name.to_s.singularize.to_sym
           belongs_to rhs_name, options
-          self.right_reflection = reflect_on_association(rhs_name)
+          self.right_reflection = _reflect_on_association(rhs_name)
         end
 
       }

activerecord/lib/active_record/associations/has_many_association.rb

@@ -100,7 +100,8 @@ def update_counter(difference, reflection = reflection())
         # Hence this method.
         def inverse_updates_counter_cache?(reflection = reflection())
           counter_name = cached_counter_attribute_name(reflection)
-          reflection.klass.reflect_on_all_associations(:belongs_to).any? { |inverse_reflection|
+          reflection.klass._reflections.values.any? { |inverse_reflection|
+            :belongs_to == inverse_reflection.macro &&
             inverse_reflection.counter_cache_column == counter_name
           }
         end

activerecord/lib/active_record/associations/join_dependency.rb

@@ -207,7 +207,7 @@ def walk(left, right)
       end
 
       def find_reflection(klass, name)
-        klass.reflect_on_association(name) or
+        klass._reflect_on_association(name) or
           raise ConfigurationError, "Association named '#{ name }' was not found on #{ klass.name }; perhaps you misspelled it?"
       end
 

activerecord/lib/active_record/autosave_association.rb

@@ -273,9 +273,11 @@ def associated_records_to_validate_or_save(association, new_record, autosave)
       # go through nested autosave associations that are loaded in memory (without loading
       # any new ones), and return true if is changed for autosave
       def nested_records_changed_for_autosave?
-        self.class.reflect_on_all_autosave_associations.any? do |reflection|
-          association = association_instance_get(reflection.name)
-          association && Array.wrap(association.target).any? { |a| a.changed_for_autosave? }
+        self.class._reflections.values.any? do |reflection|
+          if reflection.options[:autosave]
+            association = association_instance_get(reflection.name)
+            association && Array.wrap(association.target).any? { |a| a.changed_for_autosave? }
+          end
         end
       end
 

activerecord/lib/active_record/counter_cache.rb

@@ -20,7 +20,7 @@ module ClassMethods
       def reset_counters(id, *counters)
         object = find(id)
         counters.each do |counter_association|
-          has_many_association = reflect_on_association(counter_association.to_sym)
+          has_many_association = _reflect_on_association(counter_association.to_sym)
           unless has_many_association
             has_many = reflect_on_all_associations(:has_many)
             has_many_association = has_many.find { |association| association.counter_cache_column && association.counter_cache_column.to_sym == counter_association.to_sym }
@@ -34,8 +34,7 @@ def reset_counters(id, *counters)
 
           foreign_key  = has_many_association.foreign_key.to_s
           child_class  = has_many_association.klass
-          belongs_to   = child_class.reflect_on_all_associations(:belongs_to)
-          reflection   = belongs_to.find { |e| e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? }
+          reflection   = child_class._reflections.values.find { |e| :belongs_to == e.macro && e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? }
           counter_name = reflection.counter_cache_column
 
           stmt = unscoped.where(arel_table[primary_key].eq(object.id)).arel.compile_update({

activerecord/lib/active_record/fixtures.rb

@@ -649,7 +649,7 @@ def table_rows
               model_class
             end
 
-          reflection_class.reflect_on_all_associations.each do |association|
+          reflection_class._reflections.values.each do |association|
             case association.macro
             when :belongs_to
               # Do not replace association name with association foreign key if they are named the same

activerecord/lib/active_record/nested_attributes.rb

@@ -305,8 +305,9 @@ def accepts_nested_attributes_for(*attr_names)
         options[:reject_if] = REJECT_ALL_BLANK_PROC if options[:reject_if] == :all_blank
 
         attr_names.each do |association_name|
-          if reflection = reflect_on_association(association_name)
+          if reflection = _reflect_on_association(association_name)
             reflection.autosave = true
+            reflect_on_association(association_name).autosave = true
             add_autosave_association_callbacks(reflection)
 
             nested_attributes_options = self.nested_attributes_options.dup
@@ -542,7 +543,7 @@ def call_reject_if(association_name, attributes)
     end
 
     def raise_nested_attributes_record_not_found!(association_name, record_id)
-      raise RecordNotFound, "Couldn't find #{self.class.reflect_on_association(association_name).klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}"
+      raise RecordNotFound, "Couldn't find #{self.class._reflect_on_association(association_name).klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}"
     end
   end
 end

activerecord/lib/active_record/reflection.rb

@@ -6,8 +6,11 @@ module Reflection # :nodoc:
     extend ActiveSupport::Concern
 
     included do
+      class_attribute :_reflections
+      # @api public
       class_attribute :reflections
       class_attribute :aggregate_reflections
+      self._reflections = {}
       self.reflections = {}
       self.aggregate_reflections = {}
     end
@@ -24,6 +27,7 @@ def self.create(macro, name, scope, options, ar)
     end
 
     def self.add_reflection(ar, name, reflection)
+      ar._reflections = ar._reflections.merge(name.to_s => reflection)
       ar.reflections = ar.reflections.merge(name.to_s => reflection)
     end
 
@@ -63,6 +67,7 @@ def reflect_on_aggregation(aggregation)
       #   Account.reflect_on_all_associations             # returns an array of all associations
       #   Account.reflect_on_all_associations(:has_many)  # returns an array of all has_many associations
       #
+      #   @api public
       def reflect_on_all_associations(macro = nil)
         association_reflections = reflections.values
         macro ? association_reflections.select { |reflection| reflection.macro == macro } : association_reflections
@@ -73,11 +78,19 @@ def reflect_on_all_associations(macro = nil)
       #   Account.reflect_on_association(:owner)             # returns the owner AssociationReflection
       #   Invoice.reflect_on_association(:line_items).macro  # returns :has_many
       #
+      #   @api public
       def reflect_on_association(association)
         reflections[association.to_s]
       end
 
+      #   @api private
+      def _reflect_on_association(association) #:nodoc:
+        _reflections[association]
+      end
+
       # Returns an array of AssociationReflection objects for all associations which have <tt>:autosave</tt> enabled.
+      #
+      #   @api public
       def reflect_on_all_autosave_associations
         reflections.values.select { |reflection| reflection.options[:autosave] }
       end
@@ -196,7 +209,7 @@ def klass
 
       def initialize(macro, name, scope, options, active_record)
         super
-        @collection = :has_many == macro
+        @collection = [:has_many, :has_and_belongs_to_many].include?(macro)
         @automatic_inverse_of = nil
         @type         = options[:as] && "#{options[:as]}_type"
         @foreign_type = options[:foreign_type] || "#{name}_type"
@@ -330,12 +343,12 @@ def has_inverse?
       def inverse_of
         return unless inverse_name
 
-        @inverse_of ||= klass.reflect_on_association inverse_name
+        @inverse_of ||= klass._reflect_on_association inverse_name
       end
 
       def polymorphic_inverse_of(associated_class)
         if has_inverse?
-          if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of])
+          if inverse_relationship = associated_class._reflect_on_association(options[:inverse_of])
             inverse_relationship
           else
             raise InverseOfAssociationNotFoundError.new(self, associated_class)
@@ -436,7 +449,7 @@ def automatic_inverse_of
             inverse_name = ActiveSupport::Inflector.underscore(active_record.name).to_sym
 
             begin
-              reflection = klass.reflect_on_association(inverse_name)
+              reflection = klass._reflect_on_association(inverse_name)
             rescue NameError
               # Give up: we couldn't compute the klass type so we won't be able
               # to find any associations either.
@@ -535,7 +548,7 @@ def initialize(macro, name, scope, options, active_record)
       #   # => <ActiveRecord::Reflection::AssociationReflection: @macro=:belongs_to, @name=:tag, @active_record=Tagging, @plural_name="tags">
       #
       def source_reflection
-        through_reflection.klass.reflect_on_association(source_reflection_name)
+        through_reflection.klass._reflect_on_association(source_reflection_name)
       end
 
       # Returns the AssociationReflection object specified in the <tt>:through</tt> option
@@ -551,7 +564,7 @@ def source_reflection
       #   # => <ActiveRecord::Reflection::AssociationReflection: @macro=:has_many, @name=:taggings, @active_record=Post, @plural_name="taggings">
       #
       def through_reflection
-        active_record.reflect_on_association(options[:through])
+        active_record._reflect_on_association(options[:through])
       end
 
       # Returns an array of reflections which are involved in this association. Each item in the
@@ -658,7 +671,7 @@ def source_reflection_name # :nodoc:
 
         names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq
         names = names.find_all { |n|
-          through_reflection.klass.reflect_on_association(n)
+          through_reflection.klass._reflect_on_association(n)
         }
 
         if names.length > 1

activerecord/lib/active_record/relation/calculations.rb

@@ -277,7 +277,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
       group_attrs = group_values
 
       if group_attrs.first.respond_to?(:to_sym)
-        association  = @klass.reflect_on_association(group_attrs.first.to_sym)
+        association  = @klass._reflect_on_association(group_attrs.first.to_sym)
         associated   = group_attrs.size == 1 && association && association.macro == :belongs_to # only count belongs_to associations
         group_fields = Array(associated ? association.foreign_key : group_attrs)
       else

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -26,7 +26,7 @@ def self.build_from_hash(klass, attributes, default_table)
             queries << '1=0'
           else
             table       = Arel::Table.new(column, default_table.engine)
-            association = klass.reflect_on_association(column.to_sym)
+            association = klass._reflect_on_association(column.to_sym)
 
             value.each do |k, v|
               queries.concat expand(association && association.klass, table, k, v)
@@ -55,7 +55,7 @@ def self.expand(klass, table, column, value)
       #
       # For polymorphic relationships, find the foreign key and type:
       # PriceEstimate.where(estimate_of: treasure)
-      if klass && reflection = klass.reflect_on_association(column.to_sym)
+      if klass && reflection = klass._reflect_on_association(column.to_sym)
         if reflection.polymorphic? && base_class = polymorphic_base_class_from_value(value)
           queries << build(table[reflection.foreign_type], base_class)
         end

activerecord/lib/active_record/validations/presence.rb

@@ -4,7 +4,7 @@ class PresenceValidator < ActiveModel::Validations::PresenceValidator # :nodoc:
       def validate(record)
         super
         attributes.each do |attribute|
-          next unless record.class.reflect_on_association(attribute)
+          next unless record.class._reflect_on_association(attribute)
           associated_records = Array.wrap(record.send(attribute))
 
           # Superclass validates presence. Ensure present records aren't about to be destroyed.

activerecord/lib/active_record/validations/uniqueness.rb

@@ -47,7 +47,7 @@ def find_finder_class_for(record) #:nodoc:
       end
 
       def build_relation(klass, table, attribute, value) #:nodoc:
-        if reflection = klass.reflect_on_association(attribute)
+        if reflection = klass._reflect_on_association(attribute)
           attribute = reflection.foreign_key
           value = value.attributes[reflection.primary_key_column.name] unless value.nil?
         end
@@ -74,7 +74,7 @@ def build_relation(klass, table, attribute, value) #:nodoc:
 
       def scope_relation(record, table, relation)
         Array(options[:scope]).each do |scope_item|
-          if reflection = record.class.reflect_on_association(scope_item)
+          if reflection = record.class._reflect_on_association(scope_item)
             scope_value = record.send(reflection.foreign_key)
             scope_item  = reflection.foreign_key
           else

activerecord/test/cases/reflection_test.rb

@@ -200,7 +200,12 @@ def test_association_reflection_in_modules
   end
 
   def test_reflection_should_not_raise_error_when_compared_to_other_object
-    assert_nothing_raised { Firm.reflections['clients'] == Object.new }
+    assert_not_equal Object.new, Firm._reflections['clients']
+  end
+
+  def test_has_and_belongs_to_many_reflection
+    assert_equal :has_and_belongs_to_many, Category.reflections['posts'].macro
+    assert_equal :posts, Category.reflect_on_all_associations(:has_and_belongs_to_many).first.name
   end
 
   def test_has_many_through_reflection