From 3b797c8c8681c8f4619bce39d3f042244fe002d9 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 9 Jan 2011 17:13:05 +0000 Subject: DRY up the code which instantiates the association proxy --- activerecord/lib/active_record/associations.rb | 59 +++++++++------------- activerecord/lib/active_record/reflection.rb | 25 +++++++++ .../validations/uniqueness_validation_test.rb | 13 ----- activerecord/test/models/reply.rb | 7 +++ activerecord/test/models/topic.rb | 4 ++ 5 files changed, 60 insertions(+), 48 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3f9762383b..20f22f8d1c 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -137,6 +137,22 @@ module ActiveRecord # :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 + + association + end + private # Returns the specified association instance if it responds to :loaded?, nil otherwise. def association_instance_get(name) @@ -1468,12 +1484,7 @@ module ActiveRecord def association_accessor_methods(reflection, association_proxy_class) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_instance_get(reflection.name) - - if association.nil? - association = association_proxy_class.new(self, reflection) - association_instance_set(reflection.name, association) - end + association = association_proxy(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1485,38 +1496,26 @@ module ActiveRecord end redefine_method("loaded_#{reflection.name}?") do - association = association_instance_get(reflection.name) + association = association_proxy(reflection.name) association && association.loaded? end redefine_method("#{reflection.name}=") do |record| - association = association_instance_get(reflection.name) - - if association.nil? - association = association_proxy_class.new(self, reflection) - association_instance_set(reflection.name, association) - end - - association.replace(record) + association_proxy(reflection.name).replace(record) end redefine_method("set_#{reflection.name}_target") do |target| - association = association_proxy_class.new(self, reflection) + association = association_proxy(reflection.name) association.target = target association.loaded - association_instance_set(reflection.name, association) + association end end def collection_reader_method(reflection, association_proxy_class) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_instance_get(reflection.name) - - unless association - association = association_proxy_class.new(self, reflection) - association_instance_set(reflection.name, association) - end + association = association_proxy(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1541,10 +1540,7 @@ module ActiveRecord if writer redefine_method("#{reflection.name}=") do |new_value| - # Loads proxy class instance (defined in collection_reader_method) if not already loaded - association = send(reflection.name) - association.replace(new_value) - association + association_proxy(reflection.name).replace(new_value) end redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| @@ -1559,14 +1555,7 @@ module ActiveRecord def association_constructor_method(constructor, reflection, association_proxy_class) redefine_method("#{constructor}_#{reflection.name}") do |*params| attributes = params.first unless params.empty? - association = association_instance_get(reflection.name) - - unless association - association = association_proxy_class.new(self, reflection) - association_instance_set(reflection.name, association) - end - - association.send(constructor, attributes) + association_proxy(reflection.name).send(constructor, attributes) end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 937efe395f..ceeb0ec39d 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -313,6 +313,31 @@ module ActiveRecord macro == :belongs_to end + def proxy_class + case macro + when :belongs_to + if options[:polymorphic] + Associations::BelongsToPolymorphicAssociation + else + Associations::BelongsToAssociation + end + when :has_and_belongs_to_many + Associations::HasAndBelongsToManyAssociation + when :has_many + if options[:through] + Associations::HasManyThroughAssociation + else + Associations::HasManyAssociation + end + when :has_one + if options[:through] + Associations::HasOneThroughAssociation + else + Associations::HasOneAssociation + end + end + end + private def derive_class_name class_name = name.to_s.camelize diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 679d67553b..b4f3dd034c 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -6,19 +6,6 @@ require 'models/warehouse_thing' require 'models/guid' require 'models/event' -# The following methods in Topic are used in test_conditional_validation_* -class Topic - has_many :unique_replies, :dependent => :destroy, :foreign_key => "parent_id" - has_many :silly_unique_replies, :dependent => :destroy, :foreign_key => "parent_id" -end - -class UniqueReply < Reply - validates_uniqueness_of :content, :scope => 'parent_id' -end - -class SillyUniqueReply < UniqueReply -end - class Wizard < ActiveRecord::Base self.abstract_class = true diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 110d540120..6adfe0ae3c 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -10,6 +10,13 @@ class Reply < Topic attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read, :parent_title end +class UniqueReply < Reply + validates_uniqueness_of :content, :scope => 'parent_id' +end + +class SillyUniqueReply < UniqueReply +end + class WrongReply < Reply validate :errors_on_empty_content validate :title_is_wrong_create, :on => :create diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 6496f36f7e..6440dbe8ab 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -45,6 +45,10 @@ class Topic < ActiveRecord::Base has_many :replies, :dependent => :destroy, :foreign_key => "parent_id" has_many :replies_with_primary_key, :class_name => "Reply", :dependent => :destroy, :primary_key => "title", :foreign_key => "parent_title" + + has_many :unique_replies, :dependent => :destroy, :foreign_key => "parent_id" + has_many :silly_unique_replies, :dependent => :destroy, :foreign_key => "parent_id" + serialize :content before_create :default_written_on -- cgit v1.2.3