Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Providing support for :inverse_of as an option to associations.

You can now add an :inverse_of option to has_one, has_many and belongs_to associations.  This is best described with an example:

class Man < ActiveRecord::Base
  has_one :face, :inverse_of => :man
end

class Face < ActiveRecord::Base
  belongs_to :man, :inverse_of => :face
end

m = Man.first
f = m.face

Without :inverse_of m and f.man would be different instances of the same object (f.man being pulled from the database again).  With these new :inverse_of options m and f.man are the same in memory instance.

Currently :inverse_of supports has_one and has_many (but not the :through variants) associations.  It also supplies inverse support for belongs_to associations where the inverse is a has_one and it's not a polymorphic.

Signed-off-by: Murray Steele <muz@h-lame.com>
Signed-off-by: Michael Koziarski <michael@koziarski.com>
Signed-off-by: José Valim <jose.valim@gmail.com>
Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

Conflicts:

	activerecord/lib/active_record/associations/belongs_to_association.rb
	activerecord/lib/active_record/associations/has_one_association.rb
  • Loading branch information...
commit 5374fb3cad2970c95aa9d294d006b5804b760754 1 parent 8bb3b9b
Murray Steele h-lame authored jeremy committed
Showing with 555 additions and 15 deletions.
  1. +7 −0 activerecord/CHANGELOG
  2. +3 −1 activerecord/lib/active_record/association_preload.rb
  3. +17 −5 activerecord/lib/active_record/associations.rb
  4. +6 −2 activerecord/lib/active_record/associations/association_collection.rb
  5. +14 −0 activerecord/lib/active_record/associations/association_proxy.rb
  6. +11 −1 activerecord/lib/active_record/associations/belongs_to_association.rb
  7. +5 −0 activerecord/lib/active_record/associations/has_many_association.rb
  8. +5 −5 activerecord/lib/active_record/associations/has_many_through_association.rb
  9. +10 −1 activerecord/lib/active_record/associations/has_one_association.rb
  10. +21 −0 activerecord/lib/active_record/reflection.rb
  11. +313 −0 activerecord/test/cases/associations/inverse_associations_test.rb
  12. +37 −0 activerecord/test/cases/nested_attributes_test.rb
  13. +21 −0 activerecord/test/cases/validations_test.rb
  14. +7 −0 activerecord/test/fixtures/faces.yml
  15. +29 −0 activerecord/test/fixtures/interests.yml
  16. +5 −0 activerecord/test/fixtures/men.yml
  17. +5 −0 activerecord/test/fixtures/zines.yml
  18. +5 −0 activerecord/test/models/face.rb
  19. +4 −0 activerecord/test/models/interest.rb
  20. +7 −0 activerecord/test/models/man.rb
  21. +3 −0  activerecord/test/models/zine.rb
  22. +20 −0 activerecord/test/schema/schema.rb
7 activerecord/CHANGELOG
View
@@ -1,5 +1,12 @@
*2.3.6 (pending)*
+* Association inverses for belongs_to, has_one, and has_many. Optimization to reduce database queries. #3533 [Murray Steele]
+
+ # post.comments sets each comment's post without needing to :include
+ class Post < ActiveRecord::Base
+ has_many :comments, :inverse_of => :post
+ end
+
* MySQL: add_ and change_column support positioning. #3286 [Ben Marini]
* Reset your Active Record counter caches with the reset_counter_cache class method. #1211 [Mike Breen, Gabe da Silveira]
4 activerecord/lib/active_record/association_preload.rb
View
@@ -126,6 +126,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(*[associated_record].flatten)
+ association_proxy.__send__(:set_inverse_instance, associated_record, parent_record)
end
end
@@ -152,7 +153,8 @@ def set_association_single_records(id_to_record_map, reflection_name, associated
seen_keys[associated_record[key].to_s] = true
mapped_records = id_to_record_map[associated_record[key].to_s]
mapped_records.each do |mapped_record|
- mapped_record.send("set_#{reflection_name}_target", associated_record)
+ association_proxy = mapped_record.send("set_#{reflection_name}_target", associated_record)
+ association_proxy.__send__(:set_inverse_instance, associated_record, mapped_record)
end
end
end
22 activerecord/lib/active_record/associations.rb
View
@@ -1,4 +1,10 @@
module ActiveRecord
+ class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
+ def initialize(reflection)
+ super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{reflection.class_name})")
+ end
+ end
+
class HasManyThroughAssociationNotFoundError < ActiveRecordError #:nodoc:
def initialize(owner_class_name, reflection)
super("Could not find the association #{reflection.options[:through].inspect} in model #{owner_class_name}")
@@ -1526,7 +1532,7 @@ def nullify_has_many_dependencies(record, reflection_name, association_class, pr
:finder_sql, :counter_sql,
:before_add, :after_add, :before_remove, :after_remove,
:extend, :readonly,
- :validate
+ :validate, :inverse_of
]
def create_has_many_reflection(association_id, options, &extension)
@@ -1540,7 +1546,7 @@ def create_has_many_reflection(association_id, options, &extension)
@@valid_keys_for_has_one_association = [
:class_name, :foreign_key, :remote, :select, :conditions, :order,
:include, :dependent, :counter_cache, :extend, :as, :readonly,
- :validate, :primary_key
+ :validate, :primary_key, :inverse_of
]
def create_has_one_reflection(association_id, options)
@@ -1559,7 +1565,7 @@ def create_has_one_through_reflection(association_id, options)
@@valid_keys_for_belongs_to_association = [
:class_name, :primary_key, :foreign_key, :foreign_type, :remote, :select, :conditions,
:include, :dependent, :counter_cache, :extend, :polymorphic, :readonly,
- :validate, :touch
+ :validate, :touch, :inverse_of
]
def create_belongs_to_reflection(association_id, options)
@@ -1960,21 +1966,27 @@ def construct_association(record, join, row)
return nil if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil?
association = join.instantiate(row)
collection.target.push(association)
+ collection.__send__(:set_inverse_instance, association, record)
when :has_one
return if record.id.to_s != join.parent.record_id(row).to_s
return if record.instance_variable_defined?("@#{join.reflection.name}")
association = join.instantiate(row) unless row[join.aliased_primary_key].nil?
- record.send("set_#{join.reflection.name}_target", association)
+ set_target_and_inverse(join, association, record)
when :belongs_to
return if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil?
association = join.instantiate(row)
- record.send("set_#{join.reflection.name}_target", association)
+ set_target_and_inverse(join, association, record)
else
raise ConfigurationError, "unknown macro: #{join.reflection.macro}"
end
return association
end
+ def set_target_and_inverse(join, association, record)
+ association_proxy = record.send("set_#{join.reflection.name}_target", association)
+ association_proxy.__send__(:set_inverse_instance, association, record)
+ end
+
class JoinBase # :nodoc:
attr_reader :active_record, :table_joins
delegate :table_name, :column_names, :primary_key, :reflections, :sanitize_sql, :to => :active_record
8 activerecord/lib/active_record/associations/association_collection.rb
View
@@ -400,11 +400,14 @@ def find_target
find(:all)
end
- @reflection.options[:uniq] ? uniq(records) : records
+ records = @reflection.options[:uniq] ? uniq(records) : records
+ records.each do |record|
+ set_inverse_instance(record, @owner)
+ end
+ records
end
private
-
def create_record(attrs)
attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
ensure_owner_is_not_new
@@ -434,6 +437,7 @@ def add_record_to_target_with_callbacks(record)
@target ||= [] unless loaded?
@target << record unless @reflection.options[:uniq] && @target.include?(record)
callback(:after_add, record)
+ set_inverse_instance(record, @owner)
record
end
14 activerecord/lib/active_record/associations/association_proxy.rb
View
@@ -53,6 +53,7 @@ class AssociationProxy #:nodoc:
def initialize(owner, reflection)
@owner, @reflection = owner, reflection
+ reflection.check_validity!
Array(reflection.options[:extend]).each { |ext| proxy_extend(ext) }
reset
end
@@ -269,6 +270,19 @@ def flatten_deeper(array)
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
+ end
+
+ # Override in subclasses
+ def we_can_set_the_inverse_on_this?(record)
+ false
+ end
end
end
end
12 activerecord/lib/active_record/associations/belongs_to_association.rb
View
@@ -31,6 +31,8 @@ def replace(record)
@updated = true
end
+ set_inverse_instance(record, @owner)
+
loaded
record
end
@@ -46,13 +48,15 @@ def find_target
else
"find"
end
- @reflection.klass.send(find_method,
+ the_target = @reflection.klass.send(find_method,
@owner[@reflection.primary_key_name],
:select => @reflection.options[:select],
:conditions => conditions,
:include => @reflection.options[:include],
:readonly => @reflection.options[:readonly]
) if @owner[@reflection.primary_key_name]
+ set_inverse_instance(the_target, @owner)
+ the_target
end
def foreign_key_present
@@ -71,6 +75,12 @@ def previous_record_id
@owner[@reflection.primary_key_name]
end
end
+
+ # 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
+ end
end
end
end
5 activerecord/lib/active_record/associations/has_many_association.rb
View
@@ -117,6 +117,11 @@ def construct_scope
:create => create_scoping
}
end
+
+ def we_can_set_the_inverse_on_this?(record)
+ inverse = @reflection.inverse_of
+ return !inverse.nil?
+ end
end
end
end
10 activerecord/lib/active_record/associations/has_many_through_association.rb
View
@@ -1,11 +1,6 @@
module ActiveRecord
module Associations
class HasManyThroughAssociation < HasManyAssociation #:nodoc:
- def initialize(owner, reflection)
- reflection.check_validity!
- super
- end
-
alias_method :new, :build
def create!(attrs = nil)
@@ -261,6 +256,11 @@ def has_cached_counter?
def cached_counter_attribute_name
"#{@reflection.name}_count"
end
+
+ # NOTE - not sure that we can actually cope with inverses here
+ def we_can_set_the_inverse_on_this?(record)
+ false
+ end
end
end
end
11 activerecord/lib/active_record/associations/has_one_association.rb
View
@@ -77,13 +77,15 @@ def owner_quoted_id
private
def find_target
- @reflection.klass.find(:first,
+ the_target = @reflection.klass.find(:first,
:conditions => @finder_sql,
:select => @reflection.options[:select],
:order => @reflection.options[:order],
:include => @reflection.options[:include],
:readonly => @reflection.options[:readonly]
)
+ set_inverse_instance(the_target, @owner)
+ the_target
end
def construct_sql
@@ -120,6 +122,8 @@ def new_record(replace_existing)
self.target = record
end
+ set_inverse_instance(record, @owner)
+
record
end
@@ -128,6 +132,11 @@ def merge_with_conditions(attrs={})
attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
attrs
end
+
+ def we_can_set_the_inverse_on_this?(record)
+ inverse = @reflection.inverse_of
+ return !inverse.nil?
+ end
end
end
end
21 activerecord/lib/active_record/reflection.rb
View
@@ -212,6 +212,13 @@ def reset_column_information
end
def check_validity!
+ check_validity_of_inverse!
+ end
+
+ def check_validity_of_inverse!
+ if has_inverse? && inverse_of.nil?
+ raise InverseOfAssociationNotFoundError.new(self)
+ end
end
def through_reflection
@@ -225,6 +232,18 @@ def source_reflection
nil
end
+ def has_inverse?
+ !@options[:inverse_of].nil?
+ end
+
+ def inverse_of
+ if has_inverse?
+ @inverse_of ||= klass.reflect_on_association(options[:inverse_of])
+ else
+ nil
+ end
+ end
+
private
def derive_class_name
class_name = name.to_s.camelize
@@ -300,6 +319,8 @@ def check_validity!
unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil?
raise HasManyThroughSourceAssociationMacroError.new(self)
end
+
+ check_validity_of_inverse!
end
def through_reflection_primary_key
313 activerecord/test/cases/associations/inverse_associations_test.rb
View
@@ -0,0 +1,313 @@
+require "cases/helper"
+require 'models/man'
+require 'models/face'
+require 'models/interest'
+require 'models/zine'
+require 'models/club'
+require 'models/sponsor'
+
+class InverseAssociationTests < ActiveRecord::TestCase
+ def test_should_allow_for_inverse_of_options_in_associations
+ assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_many') do
+ Class.new(ActiveRecord::Base).has_many(:wheels, :inverse_of => :car)
+ end
+
+ assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on has_one') do
+ Class.new(ActiveRecord::Base).has_one(:engine, :inverse_of => :car)
+ end
+
+ assert_nothing_raised(ArgumentError, 'ActiveRecord should allow the inverse_of options on belongs_to') do
+ Class.new(ActiveRecord::Base).belongs_to(:car, :inverse_of => :driver)
+ end
+ end
+
+ def test_should_be_able_to_ask_a_reflection_if_it_has_an_inverse
+ has_one_with_inverse_ref = Man.reflect_on_association(:face)
+ assert has_one_with_inverse_ref.respond_to?(:has_inverse?)
+ assert has_one_with_inverse_ref.has_inverse?
+
+ has_many_with_inverse_ref = Man.reflect_on_association(:interests)
+ assert has_many_with_inverse_ref.respond_to?(:has_inverse?)
+ assert has_many_with_inverse_ref.has_inverse?
+
+ belongs_to_with_inverse_ref = Face.reflect_on_association(:man)
+ assert belongs_to_with_inverse_ref.respond_to?(:has_inverse?)
+ assert belongs_to_with_inverse_ref.has_inverse?
+
+ has_one_without_inverse_ref = Club.reflect_on_association(:sponsor)
+ assert has_one_without_inverse_ref.respond_to?(:has_inverse?)
+ assert !has_one_without_inverse_ref.has_inverse?
+
+ has_many_without_inverse_ref = Club.reflect_on_association(:memberships)
+ assert has_many_without_inverse_ref.respond_to?(:has_inverse?)
+ assert !has_many_without_inverse_ref.has_inverse?
+
+ belongs_to_without_inverse_ref = Sponsor.reflect_on_association(:sponsor_club)
+ assert belongs_to_without_inverse_ref.respond_to?(:has_inverse?)
+ assert !belongs_to_without_inverse_ref.has_inverse?
+ end
+
+ def test_should_be_able_to_ask_a_reflection_what_it_is_the_inverse_of
+ has_one_ref = Man.reflect_on_association(:face)
+ assert has_one_ref.respond_to?(:inverse_of)
+
+ has_many_ref = Man.reflect_on_association(:interests)
+ assert has_many_ref.respond_to?(:inverse_of)
+
+ belongs_to_ref = Face.reflect_on_association(:man)
+ assert belongs_to_ref.respond_to?(:inverse_of)
+ end
+
+ def test_inverse_of_method_should_supply_the_actual_reflection_instance_it_is_the_inverse_of
+ has_one_ref = Man.reflect_on_association(:face)
+ assert_equal Face.reflect_on_association(:man), has_one_ref.inverse_of
+
+ has_many_ref = Man.reflect_on_association(:interests)
+ assert_equal Interest.reflect_on_association(:man), has_many_ref.inverse_of
+
+ belongs_to_ref = Face.reflect_on_association(:man)
+ assert_equal Man.reflect_on_association(:face), belongs_to_ref.inverse_of
+ end
+
+ def test_associations_with_no_inverse_of_should_return_nil
+ has_one_ref = Club.reflect_on_association(:sponsor)
+ assert_nil has_one_ref.inverse_of
+
+ has_many_ref = Club.reflect_on_association(:memberships)
+ assert_nil has_many_ref.inverse_of
+
+ belongs_to_ref = Sponsor.reflect_on_association(:sponsor_club)
+ assert_nil belongs_to_ref.inverse_of
+ end
+end
+
+class InverseHasOneTests < ActiveRecord::TestCase
+ fixtures :men, :faces
+
+ def test_parent_instance_should_be_shared_with_child_on_find
+ m = Man.find(:first)
+ f = m.face
+ assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+ f.man.name = 'Mungo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"
+ end
+
+
+ def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find
+ m = Man.find(:first, :include => :face)
+ f = m.face
+ assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+ f.man.name = 'Mungo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"
+
+ m = Man.find(:first, :include => :face, :order => 'faces.id')
+ f = m.face
+ assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+ f.man.name = 'Mungo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"
+ end
+
+ def test_parent_instance_should_be_shared_with_newly_built_child
+ m = Man.find(:first)
+ f = m.build_face(:description => 'haunted')
+ assert_not_nil f.man
+ assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+ f.man.name = 'Mungo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to just-built-child-owned instance"
+ end
+
+ def test_parent_instance_should_be_shared_with_newly_created_child
+ m = Man.find(:first)
+ f = m.create_face(:description => 'haunted')
+ assert_not_nil f.man
+ assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+ f.man.name = 'Mungo'
+ assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+ end
+
+ def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
+ assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face }
+ end
+end
+
+class InverseHasManyTests < ActiveRecord::TestCase
+ fixtures :men, :interests
+
+ def test_parent_instance_should_be_shared_with_every_child_on_find
+ m = Man.find(:first)
+ is = m.interests
+ is.each do |i|
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
+ end
+ end
+
+ def test_parent_instance_should_be_shared_with_eager_loaded_children
+ m = Man.find(:first, :include => :interests)
+ is = m.interests
+ is.each do |i|
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
+ end
+
+ m = Man.find(:first, :include => :interests, :order => 'interests.id')
+ is = m.interests
+ is.each do |i|
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
+ end
+
+ end
+
+ def test_parent_instance_should_be_shared_with_newly_built_child
+ m = Man.find(:first)
+ i = m.interests.build(:topic => 'Industrial Revolution Re-enactment')
+ assert_not_nil i.man
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance"
+ end
+
+ def test_parent_instance_should_be_shared_with_newly_created_child
+ m = Man.find(:first)
+ i = m.interests.create(:topic => 'Industrial Revolution Re-enactment')
+ assert_not_nil i.man
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+ end
+
+ def test_parent_instance_should_be_shared_with_poked_in_child
+ m = Man.find(:first)
+ i = Interest.create(:topic => 'Industrial Revolution Re-enactment')
+ m.interests << i
+ assert_not_nil i.man
+ assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+ m.name = 'Bongo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+ i.man.name = 'Mungo'
+ assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+ end
+
+ def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
+ assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests }
+ end
+end
+
+class InverseBelongsToTests < ActiveRecord::TestCase
+ fixtures :men, :faces, :interests
+
+ def test_child_instance_should_be_shared_with_parent_on_find
+ f = Face.find(:first)
+ m = f.man
+ assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+ f.description = 'gormless'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+ m.face.description = 'pleasing'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"
+ end
+
+ def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
+ f = Face.find(:first, :include => :man)
+ m = f.man
+ assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+ f.description = 'gormless'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+ m.face.description = 'pleasing'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"
+
+
+ f = Face.find(:first, :include => :man, :order => 'men.id')
+ m = f.man
+ assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+ f.description = 'gormless'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+ m.face.description = 'pleasing'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"
+ end
+
+ def test_child_instance_should_be_shared_with_newly_built_parent
+ f = Face.find(:first)
+ m = f.build_man(:name => 'Charles')
+ assert_not_nil m.face
+ assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+ f.description = 'gormless'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+ m.face.description = 'pleasing'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to just-built-parent-owned instance"
+ end
+
+ def test_child_instance_should_be_shared_with_newly_created_parent
+ f = Face.find(:first)
+ m = f.create_man(:name => 'Charles')
+ assert_not_nil m.face
+ assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+ f.description = 'gormless'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+ m.face.description = 'pleasing'
+ assert_equal f.description, m.face.description, "Description of face should be the same after changes to newly-created-parent-owned instance"
+ end
+
+ def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many
+ i = Interest.find(:first)
+ m = i.man
+ assert_not_nil m.interests
+ iz = m.interests.detect {|iz| iz.id == i.id}
+ assert_not_nil iz
+ assert_equal i.topic, iz.topic, "Interest topics should be the same before changes to child"
+ i.topic = 'Eating cheese with a spoon'
+ assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to child"
+ iz.topic = 'Cow tipping'
+ assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance"
+ end
+
+ def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
+ assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man }
+ end
+end
+
+# NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin
+# which would guess the inverse rather than look for an explicit configuration option.
+class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase
+ fixtures :men, :interests, :zines
+
+ def test_that_we_can_load_associations_that_have_the_same_reciprocal_name_from_different_models
+ assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do
+ i = Interest.find(:first)
+ z = i.zine
+ m = i.man
+ end
+ end
+
+ def test_that_we_can_create_associations_that_have_the_same_reciprocal_name_from_different_models
+ assert_nothing_raised(ActiveRecord::AssociationTypeMismatch) do
+ i = Interest.find(:first)
+ i.build_zine(:title => 'Get Some in Winter! 2008')
+ i.build_man(:name => 'Gordon')
+ i.save!
+ end
+ end
+end
37 activerecord/test/cases/nested_attributes_test.rb
View
@@ -4,6 +4,8 @@
require "models/bird"
require "models/parrot"
require "models/treasure"
+require "models/man"
+require "models/interest"
module AssertRaiseWithMessage
def assert_raise_with_message(expected_exception, expected_message)
@@ -495,6 +497,41 @@ def test_should_automatically_enable_autosave_on_the_association
assert Pirate.reflect_on_association(@association_name).options[:autosave]
end
+ def test_validate_presence_of_parent__works_with_inverse_of
+ Man.accepts_nested_attributes_for(:interests)
+ assert_equal :man, Man.reflect_on_association(:interests).options[:inverse_of]
+ assert_equal :interests, Interest.reflect_on_association(:man).options[:inverse_of]
+
+ repair_validations(Interest) do
+ Interest.validates_presence_of(:man)
+ assert_difference 'Man.count' do
+ assert_difference 'Interest.count', 2 do
+ man = Man.create!(:name => 'John',
+ :interests_attributes => [{:topic=>'Cars'}, {:topic=>'Sports'}])
+ assert_equal 2, man.interests.count
+ end
+ end
+ end
+ end
+
+ 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)
+ Interest.reflect_on_association(:man).options.delete(:inverse_of)
+
+ repair_validations(Interest) do
+ Interest.validates_presence_of(:man)
+ assert_no_difference ['Man.count', 'Interest.count'] do
+ man = Man.create(:name => 'John',
+ :interests_attributes => [{:topic=>'Cars'}, {:topic=>'Sports'}])
+ assert !man.errors[:'interests.man'].empty?
+ end
+ end
+ # restore :inverse_of
+ Man.reflect_on_association(:interests).options[:inverse_of] = :man
+ Interest.reflect_on_association(:man).options[:inverse_of] = :interests
+ end
+
private
def association_setter
21 activerecord/test/cases/validations_test.rb
View
@@ -9,6 +9,8 @@
require 'models/owner'
require 'models/pet'
require 'models/event'
+require 'models/man'
+require 'models/interest'
# The following methods in Topic are used in test_conditional_validation_*
class Topic
@@ -356,6 +358,25 @@ def test_validate_presences
assert t.save
end
+ def test_validates_presence_of_belongs_to_association__parent_is_new_record
+ repair_validations(Interest) do
+ # Note that Interest and Man have the :inverse_of option set
+ Interest.validates_presence_of(:man)
+ man = Man.new(:name => 'John')
+ interest = man.interests.build(:topic => 'Airplanes')
+ assert interest.valid?, "Expected interest to be valid, but was not. Interest should have a man object associated"
+ end
+ end
+
+ def test_validates_presence_of_belongs_to_association__existing_parent
+ repair_validations(Interest) do
+ Interest.validates_presence_of(:man)
+ man = Man.create!(:name => 'John')
+ interest = man.interests.build(:topic => 'Airplanes')
+ assert interest.valid?, "Expected interest to be valid, but was not. Interest should have a man object associated"
+ end
+ end
+
def test_validate_uniqueness
Topic.validates_uniqueness_of(:title)
7 activerecord/test/fixtures/faces.yml
View
@@ -0,0 +1,7 @@
+trusting:
+ description: trusting
+ man: gordon
+
+weather_beaten:
+ description: weather beaten
+ man: steve
29 activerecord/test/fixtures/interests.yml
View
@@ -0,0 +1,29 @@
+trainspotting:
+ topic: Trainspotting
+ zine: staying_in
+ man: gordon
+
+birdwatching:
+ topic: Birdwatching
+ zine: staying_in
+ man: gordon
+
+stamp_collecting:
+ topic: Stamp Collecting
+ zine: staying_in
+ man: gordon
+
+hunting:
+ topic: Hunting
+ zine: going_out
+ man: steve
+
+woodsmanship:
+ topic: Woodsmanship
+ zine: going_out
+ man: steve
+
+survial:
+ topic: Survival
+ zine: going_out
+ man: steve
5 activerecord/test/fixtures/men.yml
View
@@ -0,0 +1,5 @@
+gordon:
+ name: Gordon
+
+steve:
+ name: Steve
5 activerecord/test/fixtures/zines.yml
View
@@ -0,0 +1,5 @@
+staying_in:
+ title: Staying in '08
+
+going_out:
+ title: Outdoor Pursuits 2k+8
5 activerecord/test/models/face.rb
View
@@ -0,0 +1,5 @@
+class Face < ActiveRecord::Base
+ belongs_to :man, :inverse_of => :face
+ # This is a "broken" inverse_of for the purposes of testing
+ belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face
+end
4 activerecord/test/models/interest.rb
View
@@ -0,0 +1,4 @@
+class Interest < ActiveRecord::Base
+ belongs_to :man, :inverse_of => :interests
+ belongs_to :zine, :inverse_of => :interests
+end
7 activerecord/test/models/man.rb
View
@@ -0,0 +1,7 @@
+class Man < ActiveRecord::Base
+ has_one :face, :inverse_of => :man
+ has_many :interests, :inverse_of => :man
+ # These are "broken" inverse_of associations for the purposes of testing
+ has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man
+ has_many :secret_interests, :class_name => 'Interest', :inverse_of => :secret_man
+end
3  activerecord/test/models/zine.rb
View
@@ -0,0 +1,3 @@
+class Zine < ActiveRecord::Base
+ has_many :interests, :inverse_of => :zine
+end
20 activerecord/test/schema/schema.rb
View
@@ -479,6 +479,26 @@ def create_table(*args, &block)
end
end
+ # NOTE - the following 4 tables are used by models that have :inverse_of options on the associations
+ create_table :men, :force => true do |t|
+ t.string :name
+ end
+
+ create_table :faces, :force => true do |t|
+ t.string :description
+ t.integer :man_id
+ end
+
+ create_table :interests, :force => true do |t|
+ t.string :topic
+ t.integer :man_id
+ t.integer :zine_id
+ end
+
+ create_table :zines, :force => true do |t|
+ t.string :title
+ end
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|

1 comment on commit 5374fb3

Murray Steele

Great to see this! However, this single commit from master doesn't do everything. Check out https://rails.lighthouseapp.com/projects/8994/tickets/3533 for a patch with the other 3 or 4 commits that will bring 2-3-stable up to parity with master.

Please sign in to comment.
Something went wrong with that request. Please try again.