mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Created a method to automatically find inverse associations and cache
the results. Added tests to check to make sure that inverse associations are automatically found when has_many, has_one, or belongs_to associations are defined.
This commit is contained in:
parent
a0f904143b
commit
26d19b4661
11 changed files with 194 additions and 10 deletions
|
@ -5,7 +5,7 @@ module ActiveRecord::Associations::Builder
|
|||
end
|
||||
|
||||
def valid_options
|
||||
super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache]
|
||||
super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :automatic_inverse_of, :counter_cache]
|
||||
end
|
||||
|
||||
def valid_dependent_options
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
module ActiveRecord::Associations::Builder
|
||||
class SingularAssociation < Association #:nodoc:
|
||||
def valid_options
|
||||
super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of]
|
||||
super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of, :automatic_inverse_of]
|
||||
end
|
||||
|
||||
def constructable?
|
||||
|
|
|
@ -305,6 +305,11 @@ module ActiveRecord
|
|||
reflection.options[:autosave] = true
|
||||
add_autosave_association_callbacks(reflection)
|
||||
|
||||
# Clear cached values of any inverse associations found in the
|
||||
# reflection and prevent the reflection from finding inverses
|
||||
# automatically in the future.
|
||||
reflection.remove_automatic_inverse_of!
|
||||
|
||||
nested_attributes_options = self.nested_attributes_options.dup
|
||||
nested_attributes_options[association_name.to_sym] = options
|
||||
self.nested_attributes_options = nested_attributes_options
|
||||
|
|
|
@ -181,6 +181,7 @@ module ActiveRecord
|
|||
def initialize(*args)
|
||||
super
|
||||
@collection = [:has_many, :has_and_belongs_to_many].include?(macro)
|
||||
@automatic_inverse_of = nil
|
||||
end
|
||||
|
||||
# Returns a new, unsaved instance of the associated class. +attributes+ will
|
||||
|
@ -289,15 +290,32 @@ module ActiveRecord
|
|||
alias :source_macro :macro
|
||||
|
||||
def has_inverse?
|
||||
@options[:inverse_of]
|
||||
@options[:inverse_of] || find_inverse_of_automatically
|
||||
end
|
||||
|
||||
def inverse_of
|
||||
if has_inverse?
|
||||
@inverse_of ||= klass.reflect_on_association(options[:inverse_of])
|
||||
@inverse_of ||= if options[:inverse_of]
|
||||
klass.reflect_on_association(options[:inverse_of])
|
||||
else
|
||||
find_inverse_of_automatically
|
||||
end
|
||||
end
|
||||
|
||||
# Clears the cached value of +@inverse_of+ on this object. This will
|
||||
# not remove the :inverse_of option however, so future calls on the
|
||||
# +inverse_of+ will have to recompute the inverse.
|
||||
def clear_inverse_of_cache!
|
||||
@inverse_of = nil
|
||||
end
|
||||
|
||||
# Removes the cached inverse association that was found automatically
|
||||
# and prevents this object from finding the inverse association
|
||||
# automatically in the future.
|
||||
def remove_automatic_inverse_of!
|
||||
@automatic_inverse_of = nil
|
||||
options[:automatic_inverse_of] = false
|
||||
end
|
||||
|
||||
def polymorphic_inverse_of(associated_class)
|
||||
if has_inverse?
|
||||
if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of])
|
||||
|
@ -366,7 +384,84 @@ module ActiveRecord
|
|||
options.key? :polymorphic
|
||||
end
|
||||
|
||||
VALID_AUTOMATIC_INVERSE_MACROS = [:has_many, :has_one, :belongs_to]
|
||||
INVALID_AUTOMATIC_INVERSE_OPTIONS = [:conditions, :through, :polymorphic, :foreign_key]
|
||||
|
||||
private
|
||||
# Attempts to find the inverse association automatically.
|
||||
# If it cannot find a suitable inverse association, it returns
|
||||
# nil.
|
||||
def find_inverse_of_automatically
|
||||
if @automatic_inverse_of == false
|
||||
nil
|
||||
elsif @automatic_inverse_of.nil?
|
||||
set_automatic_inverse_of
|
||||
else
|
||||
klass.reflect_on_association(@automatic_inverse_of)
|
||||
end
|
||||
end
|
||||
|
||||
# Sets the +@automatic_inverse_of+ instance variable, and returns
|
||||
# either nil or the inverse association that it finds.
|
||||
#
|
||||
# This method caches the inverse association that is found so that
|
||||
# future calls to +find_inverse_of_automatically+ have much less
|
||||
# overhead.
|
||||
def set_automatic_inverse_of
|
||||
if can_find_inverse_of_automatically?(self)
|
||||
inverse_name = active_record.name.downcase.to_sym
|
||||
|
||||
begin
|
||||
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.
|
||||
reflection = false
|
||||
end
|
||||
|
||||
if valid_inverse_reflection?(reflection)
|
||||
@automatic_inverse_of = inverse_name
|
||||
reflection
|
||||
else
|
||||
@automatic_inverse_of = false
|
||||
nil
|
||||
end
|
||||
else
|
||||
@automatic_inverse_of = false
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
# Checks if the inverse reflection that is returned from the
|
||||
# +set_automatic_inverse_of+ method is a valid reflection. We must
|
||||
# make sure that the reflection's active_record name matches up
|
||||
# with the current reflection's klass name.
|
||||
#
|
||||
# Note: klass will always be valid because when there's a NameError
|
||||
# from calling +klass+, +reflection+ will already be set to false.
|
||||
def valid_inverse_reflection?(reflection)
|
||||
reflection &&
|
||||
klass.name == reflection.active_record.try(:name) &&
|
||||
klass.primary_key == reflection.active_record_primary_key &&
|
||||
can_find_inverse_of_automatically?(reflection)
|
||||
end
|
||||
|
||||
# Checks to see if the reflection doesn't have any options that prevent
|
||||
# us from being able to guess the inverse automatically. First, the
|
||||
# +automatic_inverse_of+ option cannot be set to false. Second, we must
|
||||
# have :has_many, :has_one, :belongs_to associations. Third, we must
|
||||
# not have options such as :class_name or :polymorphic which prevent us
|
||||
# from correctly guessing the inverse association.
|
||||
#
|
||||
# Anything with a scope can additionally ruin our attempt at finding an
|
||||
# inverse, so we exclude reflections with scopes.
|
||||
def can_find_inverse_of_automatically?(reflection)
|
||||
reflection.options[:automatic_inverse_of] != false &&
|
||||
VALID_AUTOMATIC_INVERSE_MACROS.include?(reflection.macro) &&
|
||||
!INVALID_AUTOMATIC_INVERSE_OPTIONS.any? { |opt| reflection.options[opt] } &&
|
||||
!reflection.scope
|
||||
end
|
||||
|
||||
def derive_class_name
|
||||
class_name = name.to_s.camelize
|
||||
class_name = class_name.singularize if collection?
|
||||
|
|
|
@ -5,6 +5,88 @@ require 'models/interest'
|
|||
require 'models/zine'
|
||||
require 'models/club'
|
||||
require 'models/sponsor'
|
||||
require 'models/rating'
|
||||
require 'models/comment'
|
||||
require 'models/car'
|
||||
require 'models/bulb'
|
||||
|
||||
class AutomaticInverseFindingTests < ActiveRecord::TestCase
|
||||
fixtures :ratings, :comments, :cars
|
||||
|
||||
def test_has_one_and_belongs_to_should_find_inverse_automatically
|
||||
car_reflection = Car.reflect_on_association(:bulb)
|
||||
bulb_reflection = Bulb.reflect_on_association(:car)
|
||||
|
||||
assert_respond_to car_reflection, :has_inverse?
|
||||
assert car_reflection.has_inverse?, "The Car reflection should have an inverse"
|
||||
assert_equal bulb_reflection, car_reflection.inverse_of, "The Car reflection's inverse should be the Bulb reflection"
|
||||
|
||||
assert_respond_to bulb_reflection, :has_inverse?
|
||||
assert bulb_reflection.has_inverse?, "The Bulb reflection should have an inverse"
|
||||
assert_equal car_reflection, bulb_reflection.inverse_of, "The Bulb reflection's inverse should be the Car reflection"
|
||||
end
|
||||
|
||||
def test_has_many_and_belongs_to_should_find_inverse_automatically
|
||||
comment_reflection = Comment.reflect_on_association(:ratings)
|
||||
rating_reflection = Rating.reflect_on_association(:comment)
|
||||
|
||||
assert_respond_to comment_reflection, :has_inverse?
|
||||
assert comment_reflection.has_inverse?, "The Comment reflection should have an inverse"
|
||||
assert_equal rating_reflection, comment_reflection.inverse_of, "The Comment reflection's inverse should be the Rating reflection"
|
||||
end
|
||||
|
||||
def test_has_one_and_belongs_to_automatic_inverse_shares_objects
|
||||
car = Car.first
|
||||
bulb = Bulb.create!(car: car)
|
||||
|
||||
assert_equal car.bulb, bulb, "The Car's bulb should be the original bulb"
|
||||
|
||||
car.bulb.color = "Blue"
|
||||
assert_equal car.bulb.color, bulb.color, "Changing the bulb's color on the car association should change the bulb's color"
|
||||
|
||||
bulb.color = "Red"
|
||||
assert_equal bulb.color, car.bulb.color, "Changing the bulb's color should change the bulb's color on the car association"
|
||||
end
|
||||
|
||||
def test_has_many_and_belongs_to_automatic_inverse_shares_objects_on_rating
|
||||
comment = Comment.first
|
||||
rating = Rating.create!(comment: comment)
|
||||
|
||||
assert_equal rating.comment, comment, "The Rating's comment should be the original Comment"
|
||||
|
||||
rating.comment.body = "Brogramming is the act of programming, like a bro."
|
||||
assert_equal rating.comment.body, comment.body, "Changing the Comment's body on the association should change the original Comment's body"
|
||||
|
||||
comment.body = "Broseiden is the king of the sea of bros."
|
||||
assert_equal comment.body, rating.comment.body, "Changing the original Comment's body should change the Comment's body on the association"
|
||||
end
|
||||
|
||||
def test_has_many_and_belongs_to_automatic_inverse_shares_objects_on_comment
|
||||
rating = Rating.create!
|
||||
comment = Comment.first
|
||||
rating.comment = comment
|
||||
|
||||
assert_equal rating.comment, comment, "The Rating's comment should be the original Comment"
|
||||
|
||||
rating.comment.body = "Brogramming is the act of programming, like a bro."
|
||||
assert_equal rating.comment.body, comment.body, "Changing the Comment's body on the association should change the original Comment's body"
|
||||
|
||||
comment.body = "Broseiden is the king of the sea of bros."
|
||||
assert_equal comment.body, rating.comment.body, "Changing the original Comment's body should change the Comment's body on the association"
|
||||
end
|
||||
|
||||
def test_polymorphic_and_has_many_through_relationships_should_not_have_inverses
|
||||
sponsor_reflection = Sponsor.reflect_on_association(:sponsorable)
|
||||
|
||||
assert_respond_to sponsor_reflection, :has_inverse?
|
||||
assert !sponsor_reflection.has_inverse?, "A polymorphic association should not find an inverse automatically"
|
||||
|
||||
club_reflection = Club.reflect_on_association(:members)
|
||||
|
||||
assert_respond_to club_reflection, :has_inverse?
|
||||
assert !club_reflection.has_inverse?, "A has_many_through association should not find an inverse automatically"
|
||||
end
|
||||
end
|
||||
|
||||
class InverseAssociationTests < ActiveRecord::TestCase
|
||||
def test_should_allow_for_inverse_of_options_in_associations
|
||||
|
|
|
@ -800,7 +800,9 @@ module NestedAttributesOnACollectionAssociationTests
|
|||
def test_validate_presence_of_parent_fails_without_inverse_of
|
||||
Man.accepts_nested_attributes_for(:interests)
|
||||
Man.reflect_on_association(:interests).options.delete(:inverse_of)
|
||||
Man.reflect_on_association(:interests).clear_inverse_of_cache!
|
||||
Interest.reflect_on_association(:man).options.delete(:inverse_of)
|
||||
Interest.reflect_on_association(:man).clear_inverse_of_cache!
|
||||
|
||||
repair_validations(Interest) do
|
||||
Interest.validates_presence_of(:man)
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
class Club < ActiveRecord::Base
|
||||
has_one :membership
|
||||
has_many :memberships
|
||||
has_many :memberships, :automatic_inverse_of => false
|
||||
has_many :members, :through => :memberships
|
||||
has_many :current_memberships
|
||||
has_one :sponsor
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
class Interest < ActiveRecord::Base
|
||||
belongs_to :man, :inverse_of => :interests
|
||||
belongs_to :man, :inverse_of => :interests, :automatic_inverse_of => false
|
||||
belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_interests
|
||||
belongs_to :zine, :inverse_of => :interests
|
||||
end
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
class Man < ActiveRecord::Base
|
||||
has_one :face, :inverse_of => :man
|
||||
has_one :polymorphic_face, :class_name => 'Face', :as => :polymorphic_man, :inverse_of => :polymorphic_man
|
||||
has_many :interests, :inverse_of => :man
|
||||
has_many :interests, :inverse_of => :man, :automatic_inverse_of => false
|
||||
has_many :polymorphic_interests, :class_name => 'Interest', :as => :polymorphic_man, :inverse_of => :polymorphic_man
|
||||
# These are "broken" inverse_of associations for the purposes of testing
|
||||
has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man
|
||||
|
|
|
@ -9,7 +9,7 @@ class Member < ActiveRecord::Base
|
|||
has_one :hairy_club, -> { where :clubs => {:name => "Moustache and Eyebrow Fancier Club"} }, :through => :membership, :source => :club
|
||||
has_one :sponsor, :as => :sponsorable
|
||||
has_one :sponsor_club, :through => :sponsor
|
||||
has_one :member_detail
|
||||
has_one :member_detail, :automatic_inverse_of => false
|
||||
has_one :organization, :through => :member_detail
|
||||
belongs_to :member_type
|
||||
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
class MemberDetail < ActiveRecord::Base
|
||||
belongs_to :member
|
||||
belongs_to :member, :automatic_inverse_of => false
|
||||
belongs_to :organization
|
||||
has_one :member_type, :through => :member
|
||||
|
||||
|
|
Loading…
Reference in a new issue