Permalink
Browse files

Split AssociationProxy into an Association class (and subclasses) whi…

…ch manages the association, and a CollectionProxy class which is *only* a proxy. Singular associations no longer have a proxy. See CHANGELOG for more.
  • Loading branch information...
jonleighton committed Feb 17, 2011
1 parent 1d9f26e commit 1644663ba7f678d178deab2bf1629dc05626f85b
Showing with 392 additions and 502 deletions.
  1. +12 −0 activerecord/CHANGELOG
  2. +11 −11 activerecord/lib/active_record/association_preload.rb
  3. +30 −34 activerecord/lib/active_record/associations.rb
  4. +65 −152 activerecord/lib/active_record/associations/{association_proxy.rb → association.rb}
  5. +7 −7 activerecord/lib/active_record/associations/class_methods/join_dependency.rb
  6. +54 −77 activerecord/lib/active_record/associations/{association_collection.rb → collection_association.rb}
  7. +127 −0 activerecord/lib/active_record/associations/collection_proxy.rb
  8. +17 −19 activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
  9. +5 −7 activerecord/lib/active_record/associations/has_many_association.rb
  10. +9 −11 activerecord/lib/active_record/associations/has_many_through_association.rb
  11. +1 −1 activerecord/lib/active_record/associations/has_one_through_association.rb
  12. +2 −2 activerecord/lib/active_record/associations/singular_association.rb
  13. +26 −22 activerecord/lib/active_record/autosave_association.rb
  14. +5 −5 activerecord/lib/active_record/nested_attributes.rb
  15. +1 −1 activerecord/lib/active_record/reflection.rb
  16. +1 −1 activerecord/lib/active_record/relation/predicate_builder.rb
  17. +0 −52 activerecord/test/cases/associations/association_proxy_test.rb
  18. +5 −8 activerecord/test/cases/associations/belongs_to_associations_test.rb
  19. +2 −7 activerecord/test/cases/associations/has_one_associations_test.rb
  20. +2 −2 activerecord/test/cases/associations/has_one_through_associations_test.rb
  21. +2 −39 activerecord/test/cases/associations/inverse_associations_test.rb
  22. +1 −1 activerecord/test/cases/associations/join_model_test.rb
  23. +0 −36 activerecord/test/cases/associations_test.rb
  24. +1 −1 activerecord/test/cases/autosave_association_test.rb
  25. +1 −1 activerecord/test/cases/named_scope_test.rb
  26. +5 −5 activerecord/test/cases/nested_attributes_test.rb
View
@@ -1,5 +1,17 @@
*Rails 3.1.0 (unreleased)*
+* ActiveRecord::Associations::AssociationProxy has been split. There is now an Association class
+ (and subclasses) which are responsible for operating on associations, and then a separate,
+ thin wrapper called CollectionProxy, which proxies collection associations.
+
+ This prevents namespace pollution, separates concerns, and will allow further refactorings.
+
+ Singular associations (has_one, belongs_to) no longer have a proxy at all. They simply return
+ the associated record or nil. This means that you should not use undocumented methods such
+ as bob.mother.create - use bob.create_mother instead.
+
+ [Jon Leighton]
+
* Make has_many :through associations work correctly when you build a record and then save it. This
requires you to set the :inverse_of option on the source reflection on the join model, like so:
@@ -124,16 +124,16 @@ def preload_one_association(records, association, preload_options={})
def add_preloaded_records_to_collection(parent_records, reflection_name, associated_record)
parent_records.each do |parent_record|
- association_proxy = parent_record.send(reflection_name)
- association_proxy.loaded!
- association_proxy.target.concat(Array.wrap(associated_record))
- association_proxy.send(:set_inverse_instance, associated_record)
+ association = parent_record.association(reflection_name)
+ association.loaded!
+ association.target.concat(Array.wrap(associated_record))
+ association.set_inverse_instance(associated_record)
end
end
def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record)
parent_records.each do |parent_record|
- parent_record.send(:association_proxy, reflection_name).target = associated_record
+ parent_record.association(reflection_name).target = associated_record
end
end
@@ -158,7 +158,7 @@ def set_association_single_records(id_to_record_map, reflection_name, associated
seen_keys[seen_key] = true
mapped_records = id_to_record_map[seen_key]
mapped_records.each do |mapped_record|
- association_proxy = mapped_record.send(:association_proxy, reflection_name)
+ association_proxy = mapped_record.association(reflection_name)
association_proxy.target = associated_record
association_proxy.send(:set_inverse_instance, associated_record)
end
@@ -187,7 +187,7 @@ def preload_has_and_belongs_to_many_association(records, reflection, preload_opt
id_to_record_map = construct_id_map(records)
- records.each { |record| record.send(reflection.name).loaded! }
+ records.each { |record| record.association(reflection.name).loaded! }
options = reflection.options
right = Arel::Table.new(options[:join_table]).alias('t0')
@@ -233,7 +233,7 @@ def preload_has_and_belongs_to_many_association(records, reflection, preload_opt
end
def preload_has_one_association(records, reflection, preload_options={})
- return if records.first.send(:association_proxy, reflection.name).loaded?
+ return if records.first.association(reflection.name).loaded?
id_to_record_map = construct_id_map(records, reflection.options[:primary_key])
options = reflection.options
@@ -268,7 +268,7 @@ def preload_has_many_association(records, reflection, preload_options={})
foreign_key = reflection.through_reflection_foreign_key
id_to_record_map = construct_id_map(records, foreign_key || reflection.options[:primary_key])
- records.each { |record| record.send(reflection.name).loaded! }
+ records.each { |record| record.association(reflection.name).loaded! }
if options[:through]
through_records = preload_through_records(records, reflection, options[:through])
@@ -298,7 +298,7 @@ def preload_through_records(records, reflection, through_association)
# Dont cache the association - we would only be caching a subset
records.map { |record|
- proxy = record.send(through_association)
+ proxy = record.association(through_association)
if proxy.respond_to?(:target)
Array.wrap(proxy.target).tap { proxy.reset }
@@ -320,7 +320,7 @@ def preload_through_records(records, reflection, through_association)
end
def preload_belongs_to_association(records, reflection, preload_options={})
- return if records.first.send(:association_proxy, reflection.name).loaded?
+ return if records.first.association(reflection.name).loaded?
options = reflection.options
klasses_and_ids = {}
@@ -118,17 +118,19 @@ module Associations # :nodoc:
# These classes will be loaded when associations are created.
# So there is no need to eager load them.
- autoload :AssociationCollection, 'active_record/associations/association_collection'
- autoload :SingularAssociation, 'active_record/associations/singular_association'
- autoload :AssociationProxy, 'active_record/associations/association_proxy'
- autoload :ThroughAssociation, 'active_record/associations/through_association'
- autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association'
+ autoload :Association, 'active_record/associations/association'
+ autoload :SingularAssociation, 'active_record/associations/singular_association'
+ autoload :CollectionAssociation, 'active_record/associations/collection_association'
+ autoload :CollectionProxy, 'active_record/associations/collection_proxy'
+
+ autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association'
autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association'
- autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association'
- autoload :HasManyAssociation, 'active_record/associations/has_many_association'
- autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association'
- autoload :HasOneAssociation, 'active_record/associations/has_one_association'
- autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association'
+ autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association'
+ autoload :HasManyAssociation, 'active_record/associations/has_many_association'
+ autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association'
+ autoload :HasOneAssociation, 'active_record/associations/has_one_association'
+ autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association'
+ autoload :ThroughAssociation, 'active_record/associations/through_association'
# Clears out the association cache.
def clear_association_cache #:nodoc:
@@ -138,29 +140,23 @@ def clear_association_cache #:nodoc:
# :nodoc:
attr_reader :association_cache
- protected
-
- # Returns the proxy for the given association name, instantiating it if it doesn't
- # already exist
- def association_proxy(name)
- association = association_instance_get(name)
-
- if association.nil?
- reflection = self.class.reflect_on_association(name)
- association = reflection.proxy_class.new(self, reflection)
- association_instance_set(name, association)
- end
+ # Returns the association instance for the given name, instantiating it if it doesn't already exist
+ def association(name) #:nodoc:
+ association = association_instance_get(name)
- association
+ if association.nil?
+ reflection = self.class.reflect_on_association(name)
+ association = reflection.association_class.new(self, reflection)
+ association_instance_set(name, association)
end
+ association
+ end
+
private
# Returns the specified association instance if it responds to :loaded?, nil otherwise.
def association_instance_get(name)
- if @association_cache.key? name
- association = @association_cache[name]
- association if association.respond_to?(:loaded?)
- end
+ @association_cache[name]
end
# Set the specified association instance.
@@ -1574,34 +1570,34 @@ def join_table_name(first_table_name, second_table_name)
def association_accessor_methods(reflection)
redefine_method(reflection.name) do |*params|
force_reload = params.first unless params.empty?
- association = association_proxy(reflection.name)
+ association = association(reflection.name)
if force_reload
reflection.klass.uncached { association.reload }
elsif !association.loaded? || association.stale_target?
association.reload
end
- association.target.nil? ? nil : association
+ association.target
end
redefine_method("#{reflection.name}=") do |record|
- association_proxy(reflection.name).replace(record)
+ association(reflection.name).replace(record)
end
end
def collection_reader_method(reflection)
redefine_method(reflection.name) do |*params|
force_reload = params.first unless params.empty?
- association = association_proxy(reflection.name)
+ association = association(reflection.name)
if force_reload
reflection.klass.uncached { association.reload }
elsif association.stale_target?
association.reload
end
- association
+ association.proxy
end
redefine_method("#{reflection.name.to_s.singularize}_ids") do
@@ -1621,7 +1617,7 @@ def collection_accessor_methods(reflection, writer = true)
if writer
redefine_method("#{reflection.name}=") do |new_value|
- association_proxy(reflection.name).replace(new_value)
+ association(reflection.name).replace(new_value)
end
redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value|
@@ -1643,7 +1639,7 @@ def association_constructor_methods(reflection)
constructors.each do |name, proxy_name|
redefine_method(name) do |*params|
attributes = params.first unless params.empty?
- association_proxy(reflection.name).send(proxy_name, attributes)
+ association(reflection.name).send(proxy_name, attributes)
end
end
end
Oops, something went wrong.

0 comments on commit 1644663

Please sign in to comment.