Permalink
Browse files

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
  • Loading branch information...
arthurnn committed May 24, 2014
1 parent e2bd0eb commit 09ac448a8eff73ffb7a3f683207802e8cb1f6d0b
@@ -1,3 +1,11 @@
+* 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.
@@ -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
@@ -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:
@@ -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
}
@@ -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
@@ -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
@@ -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
@@ -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({
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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.
@@ -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
@@ -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

0 comments on commit 09ac448

Please sign in to comment.