Merge pull request #16089 from eileencodes/refactor-reflections-from-sub-classes-to-delegates

Refactor reflections from sub classes to delegates

activerecord/lib/active_record/associations.rb

@@ -1587,7 +1587,7 @@ def has_and_belongs_to_many(name, scope = nil, options = {}, &extension)
           scope   = nil
         end
 
-        habtm_reflection = ActiveRecord::Reflection::HasAndBelongsToManyReflection.new(:has_and_belongs_to_many, name, scope, options, self)
+        habtm_reflection = ActiveRecord::Reflection::HasAndBelongsToManyReflection.new(name, scope, options, self)
 
         builder = Builder::HasAndBelongsToMany.new name, self, options
 

activerecord/lib/active_record/reflection.rb

@@ -13,14 +13,21 @@ module Reflection # :nodoc:
     end
 
     def self.create(macro, name, scope, options, ar)
-      case macro
-      when :has_many, :belongs_to, :has_one
-        klass = options[:through] ? ThroughReflection : AssociationReflection
-      when :composed_of
-        klass = AggregateReflection
-      end
-
-      klass.new(macro, name, scope, options, ar)
+      klass = case macro
+              when :composed_of
+                AggregateReflection
+              when :has_many
+                HasManyReflection
+              when :has_one
+                HasOneReflection
+              when :belongs_to
+                BelongsToReflection
+              else
+                raise "Unsupported Macro: #{macro}"
+              end
+
+      reflection = klass.new(name, scope, options, ar)
+      options[:through] ? ThroughReflection.new(reflection) : reflection
     end
 
     def self.add_reflection(ar, name, reflection)
@@ -110,6 +117,52 @@ def reflect_on_all_autosave_associations
       end
     end
 
+    # Holds all the methods that are shared between MacroReflection, AssociationReflection
+    # and ThroughReflection
+    class AbstractReflection # :nodoc:
+      def table_name
+        klass.table_name
+      end
+
+      # Returns a new, unsaved instance of the associated class. +attributes+ will
+      # be passed to the class's constructor.
+      def build_association(attributes, &block)
+        klass.new(attributes, &block)
+      end
+
+      def quoted_table_name
+        klass.quoted_table_name
+      end
+
+      def primary_key_type
+        klass.type_for_attribute(klass.primary_key)
+      end
+
+      # Returns the class name for the macro.
+      #
+      # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>'Money'</tt>
+      # <tt>has_many :clients</tt> returns <tt>'Client'</tt>
+      def class_name
+        @class_name ||= (options[:class_name] || derive_class_name).to_s
+      end
+
+      JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
+
+      def join_keys(assoc_klass)
+        if source_macro == :belongs_to
+          if polymorphic?
+            reflection_key = association_primary_key(assoc_klass)
+          else
+            reflection_key = association_primary_key
+          end
+          reflection_foreign_key = foreign_key
+        else
+          reflection_foreign_key = active_record_primary_key
+          reflection_key = foreign_key
+        end
+        JoinKeys.new(reflection_key, reflection_foreign_key)
+      end
+    end
     # Base class for AggregateReflection and AssociationReflection. Objects of
     # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods.
     #
@@ -117,7 +170,7 @@ def reflect_on_all_autosave_associations
     #     AggregateReflection
     #     AssociationReflection
     #       ThroughReflection
-    class MacroReflection
+    class MacroReflection < AbstractReflection
       # Returns the name of the macro.
       #
       # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>:balance</tt>
@@ -142,8 +195,7 @@ class MacroReflection
 
       attr_reader :plural_name # :nodoc:
 
-      def initialize(macro, name, scope, options, active_record)
-        @macro         = macro
+      def initialize(name, scope, options, active_record)
         @name          = name
         @scope         = scope
         @options       = options
@@ -167,15 +219,11 @@ def autosave=(autosave)
       # <tt>composed_of :balance, class_name: 'Money'</tt> returns the Money class
       # <tt>has_many :clients</tt> returns the Client class
       def klass
-        @klass ||= class_name.constantize
+        @klass ||= compute_class(class_name)
       end
 
-      # Returns the class name for the macro.
-      #
-      # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>'Money'</tt>
-      # <tt>has_many :clients</tt> returns <tt>'Client'</tt>
-      def class_name
-        @class_name ||= (options[:class_name] || derive_class_name).to_s
+      def compute_class(name)
+        name.constantize
       end
 
       # Returns +true+ if +self+ and +other_aggregation+ have the same +name+ attribute, +active_record+ attribute,
@@ -188,23 +236,6 @@ def ==(other_aggregation)
           active_record == other_aggregation.active_record
       end
 
-      JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
-
-      def join_keys(assoc_klass)
-        if source_macro == :belongs_to
-          if polymorphic?
-            reflection_key = association_primary_key(assoc_klass)
-          else
-            reflection_key = association_primary_key
-          end
-          reflection_foreign_key = foreign_key
-        else
-          reflection_key = foreign_key
-          reflection_foreign_key = active_record_primary_key
-        end
-        JoinKeys.new(reflection_key, reflection_foreign_key)
-      end
-
       private
         def derive_class_name
           name.to_s.camelize
@@ -237,15 +268,18 @@ class AssociationReflection < MacroReflection #:nodoc:
       # a new association object. Use +build_association+ or +create_association+
       # instead. This allows plugins to hook into association object creation.
       def klass
-        @klass ||= active_record.send(:compute_type, class_name)
+        @klass ||= compute_class(class_name)
+      end
+
+      def compute_class(name)
+        active_record.send(:compute_type, name)
       end
 
       attr_reader :type, :foreign_type
       attr_accessor :parent_reflection # [:name, Reflection]
 
-      def initialize(macro, name, scope, options, active_record)
+      def initialize(name, scope, options, active_record)
         super
-        @collection = macro == :has_many
         @automatic_inverse_of = nil
         @type         = options[:as] && "#{options[:as]}_type"
         @foreign_type = options[:foreign_type] || "#{name}_type"
@@ -264,24 +298,10 @@ def association_scope_cache(conn, owner)
         }
       end
 
-      # Returns a new, unsaved instance of the associated class. +attributes+ will
-      # be passed to the class's constructor.
-      def build_association(attributes, &block)
-        klass.new(attributes, &block)
-      end
-
       def constructable? # :nodoc:
         @constructable
       end
 
-      def table_name
-        klass.table_name
-      end
-
-      def quoted_table_name
-        klass.quoted_table_name
-      end
-
       def join_table
         @join_table ||= options[:join_table] || derive_join_table
       end
@@ -290,10 +310,6 @@ def foreign_key
         @foreign_key ||= options[:foreign_key] || derive_foreign_key
       end
 
-      def primary_key_type
-        klass.type_for_attribute(klass.primary_key)
-      end
-
       def association_foreign_key
         @association_foreign_key ||= options[:association_foreign_key] || class_name.foreign_key
       end
@@ -396,7 +412,7 @@ def polymorphic_inverse_of(associated_class)
       # association. Returns +true+ if the +macro+ is either +has_many+ or
       # +has_and_belongs_to_many+, +false+ otherwise.
       def collection?
-        @collection
+        false
       end
 
       # Returns whether or not the association should be validated as part of
@@ -560,22 +576,57 @@ def primary_key(klass)
         end
     end
 
+    class HasManyReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :has_many
+        super(name, scope, options, active_record)
+      end
+
+      def collection?
+        true
+      end
+    end
+
+    class HasOneReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :has_one
+        super(name, scope, options, active_record)
+      end
+    end
+
+    class BelongsToReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :belongs_to
+        super(name, scope, options, active_record)
+      end
+    end
+
     class HasAndBelongsToManyReflection < AssociationReflection #:nodoc:
-      def initialize(macro, name, scope, options, active_record)
+      def initialize(name, scope, options, active_record)
+        @macro = :has_and_belongs_to_many
         super
-        @collection = true
+      end
+
+      def collection?
+        true
       end
     end
 
     # Holds all the meta-data about a :through association as it was specified
     # in the Active Record class.
-    class ThroughReflection < AssociationReflection #:nodoc:
+    class ThroughReflection < AbstractReflection #:nodoc:
+      attr_reader :delegate_reflection
       delegate :foreign_key, :foreign_type, :association_foreign_key,
                :active_record_primary_key, :type, :to => :source_reflection
 
-      def initialize(macro, name, scope, options, active_record)
-        super
-        @source_reflection_name = options[:source]
+      def initialize(delegate_reflection)
+        @delegate_reflection = delegate_reflection
+        @klass         = delegate_reflection.options[:class]
+        @source_reflection_name = delegate_reflection.options[:source]
+      end
+
+      def klass
+        @klass ||= delegate_reflection.compute_class(class_name)
       end
 
       # Returns the source of the through reflection. It checks both a singularized
@@ -777,15 +828,25 @@ def check_validity!
 
       protected
 
-      def actual_source_reflection # FIXME: this is a horrible name
-        source_reflection.actual_source_reflection
-      end
+        def actual_source_reflection # FIXME: this is a horrible name
+          source_reflection.send(:actual_source_reflection)
+        end
+
+        def primary_key(klass)
+          klass.primary_key || raise(UnknownPrimaryKey.new(klass))
+        end
 
       private
         def derive_class_name
           # get the class_name of the belongs_to association of the through reflection
           options[:source_type] || source_reflection.class_name
         end
+
+        delegate_methods = AssociationReflection.public_instance_methods -
+          public_instance_methods
+
+        delegate(*delegate_methods, to: :delegate_reflection)
+
     end
   end
 end

activerecord/test/cases/reflection_test.rb

@@ -87,7 +87,7 @@ def test_non_existent_columns_return_nil
   end
 
   def test_reflection_klass_for_nested_class_name
-    reflection = MacroReflection.new(:company, nil, nil, { :class_name => 'MyApplication::Business::Company' }, ActiveRecord::Base)
+    reflection = ActiveRecord::Reflection.create(:has_many, nil, nil, { :class_name => 'MyApplication::Business::Company' }, ActiveRecord::Base)
     assert_nothing_raised do
       assert_equal MyApplication::Business::Company, reflection.klass
     end
@@ -97,21 +97,21 @@ def test_irregular_reflection_class_name
     ActiveSupport::Inflector.inflections do |inflect|
       inflect.irregular 'plural_irregular', 'plurales_irregulares'
     end
-    reflection = AssociationReflection.new(:has_many, 'plurales_irregulares', nil, {}, ActiveRecord::Base)
+    reflection = ActiveRecord::Reflection.create(:has_many, 'plurales_irregulares', nil, {}, ActiveRecord::Base)
     assert_equal 'PluralIrregular', reflection.class_name
   end
 
   def test_aggregation_reflection
     reflection_for_address = AggregateReflection.new(
-      :composed_of, :address, nil, { :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer
+      :address, nil, { :mapping => [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer
     )
 
     reflection_for_balance = AggregateReflection.new(
-      :composed_of, :balance, nil, { :class_name => "Money", :mapping => %w(balance amount) }, Customer
+      :balance, nil, { :class_name => "Money", :mapping => %w(balance amount) }, Customer
     )
 
     reflection_for_gps_location = AggregateReflection.new(
-      :composed_of, :gps_location, nil, { }, Customer
+      :gps_location, nil, { }, Customer
     )
 
     assert Customer.reflect_on_all_aggregations.include?(reflection_for_gps_location)
@@ -135,7 +135,7 @@ def test_reflect_on_all_autosave_associations
   end
 
   def test_has_many_reflection
-    reflection_for_clients = AssociationReflection.new(:has_many, :clients, nil, { :order => "id", :dependent => :destroy }, Firm)
+    reflection_for_clients = ActiveRecord::Reflection.create(:has_many, :clients, nil, { :order => "id", :dependent => :destroy }, Firm)
 
     assert_equal reflection_for_clients, Firm.reflect_on_association(:clients)
 
@@ -147,7 +147,7 @@ def test_has_many_reflection
   end
 
   def test_has_one_reflection
-    reflection_for_account = AssociationReflection.new(:has_one, :account, nil, { :foreign_key => "firm_id", :dependent => :destroy }, Firm)
+    reflection_for_account = ActiveRecord::Reflection.create(:has_one, :account, nil, { :foreign_key => "firm_id", :dependent => :destroy }, Firm)
     assert_equal reflection_for_account, Firm.reflect_on_association(:account)
 
     assert_equal Account, Firm.reflect_on_association(:account).klass
@@ -284,12 +284,12 @@ def test_association_primary_key
   end
 
   def test_association_primary_key_raises_when_missing_primary_key
-    reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :edge, nil, {}, Author)
+    reflection = ActiveRecord::Reflection.create(:has_many, :edge, nil, {}, Author)
     assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.association_primary_key }
 
     through = Class.new(ActiveRecord::Reflection::ThroughReflection) {
       define_method(:source_reflection) { reflection }
-    }.new(:fuu, :edge, nil, {}, Author)
+    }.new(reflection)
     assert_raises(ActiveRecord::UnknownPrimaryKey) { through.association_primary_key }
   end
 
@@ -299,7 +299,7 @@ def test_active_record_primary_key
   end
 
   def test_active_record_primary_key_raises_when_missing_primary_key
-    reflection = ActiveRecord::Reflection::AssociationReflection.new(:fuu, :author, nil, {}, Edge)
+    reflection = ActiveRecord::Reflection.create(:has_many, :author, nil, {}, Edge)
     assert_raises(ActiveRecord::UnknownPrimaryKey) { reflection.active_record_primary_key }
   end
 
@@ -317,32 +317,28 @@ def test_collection_association
   end
 
   def test_default_association_validation
-    assert AssociationReflection.new(:has_many, :clients, nil, {}, Firm).validate?
+    assert ActiveRecord::Reflection.create(:has_many, :clients, nil, {}, Firm).validate?
 
-    assert !AssociationReflection.new(:has_one, :client, nil, {}, Firm).validate?
-    assert !AssociationReflection.new(:belongs_to, :client, nil, {}, Firm).validate?
-    assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, {}, Firm).validate?
+    assert !ActiveRecord::Reflection.create(:has_one, :client, nil, {}, Firm).validate?
+    assert !ActiveRecord::Reflection.create(:belongs_to, :client, nil, {}, Firm).validate?
   end
 
   def test_always_validate_association_if_explicit
-    assert AssociationReflection.new(:has_one, :client, nil, { :validate => true }, Firm).validate?
-    assert AssociationReflection.new(:belongs_to, :client, nil, { :validate => true }, Firm).validate?
-    assert AssociationReflection.new(:has_many, :clients, nil, { :validate => true }, Firm).validate?
-    assert AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :validate => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:has_one, :client, nil, { :validate => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:belongs_to, :client, nil, { :validate => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:has_many, :clients, nil, { :validate => true }, Firm).validate?
   end
 
   def test_validate_association_if_autosave
-    assert AssociationReflection.new(:has_one, :client, nil, { :autosave => true }, Firm).validate?
-    assert AssociationReflection.new(:belongs_to, :client, nil, { :autosave => true }, Firm).validate?
-    assert AssociationReflection.new(:has_many, :clients, nil, { :autosave => true }, Firm).validate?
-    assert AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :autosave => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:has_one, :client, nil, { :autosave => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:belongs_to, :client, nil, { :autosave => true }, Firm).validate?
+    assert ActiveRecord::Reflection.create(:has_many, :clients, nil, { :autosave => true }, Firm).validate?
   end
 
   def test_never_validate_association_if_explicit
-    assert !AssociationReflection.new(:has_one, :client, nil, { :autosave => true, :validate => false }, Firm).validate?
-    assert !AssociationReflection.new(:belongs_to, :client, nil, { :autosave => true, :validate => false }, Firm).validate?
-    assert !AssociationReflection.new(:has_many, :clients, nil, { :autosave => true, :validate => false }, Firm).validate?
-    assert !AssociationReflection.new(:has_and_belongs_to_many, :clients, nil, { :autosave => true, :validate => false }, Firm).validate?
+    assert !ActiveRecord::Reflection.create(:has_one, :client, nil, { :autosave => true, :validate => false }, Firm).validate?
+    assert !ActiveRecord::Reflection.create(:belongs_to, :client, nil, { :autosave => true, :validate => false }, Firm).validate?
+    assert !ActiveRecord::Reflection.create(:has_many, :clients, nil, { :autosave => true, :validate => false }, Firm).validate?
   end
 
   def test_foreign_key
@@ -364,11 +360,11 @@ def test_join_table
     category = Struct.new(:table_name, :pluralize_table_names).new('categories', true)
     product = Struct.new(:table_name, :pluralize_table_names).new('products', true)
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product)
+    reflection = ActiveRecord::Reflection.create(:has_many, :categories, nil, {}, product)
     reflection.stubs(:klass).returns(category)
     assert_equal 'categories_products', reflection.join_table
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category)
+    reflection = ActiveRecord::Reflection.create(:has_many, :products, nil, {}, category)
     reflection.stubs(:klass).returns(product)
     assert_equal 'categories_products', reflection.join_table
   end
@@ -377,11 +373,11 @@ def test_join_table_with_common_prefix
     category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true)
     product = Struct.new(:table_name, :pluralize_table_names).new('catalog_products', true)
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, product)
+    reflection = ActiveRecord::Reflection.create(:has_many, :categories, nil, {}, product)
     reflection.stubs(:klass).returns(category)
     assert_equal 'catalog_categories_products', reflection.join_table
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, {}, category)
+    reflection = ActiveRecord::Reflection.create(:has_many, :products, nil, {}, category)
     reflection.stubs(:klass).returns(product)
     assert_equal 'catalog_categories_products', reflection.join_table
   end
@@ -390,11 +386,11 @@ def test_join_table_with_different_prefix
     category = Struct.new(:table_name, :pluralize_table_names).new('catalog_categories', true)
     page = Struct.new(:table_name, :pluralize_table_names).new('content_pages', true)
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, {}, page)
+    reflection = ActiveRecord::Reflection.create(:has_many, :categories, nil, {}, page)
     reflection.stubs(:klass).returns(category)
     assert_equal 'catalog_categories_content_pages', reflection.join_table
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :pages, nil, {}, category)
+    reflection = ActiveRecord::Reflection.create(:has_many, :pages, nil, {}, category)
     reflection.stubs(:klass).returns(page)
     assert_equal 'catalog_categories_content_pages', reflection.join_table
   end
@@ -403,11 +399,11 @@ def test_join_table_can_be_overridden
     category = Struct.new(:table_name, :pluralize_table_names).new('categories', true)
     product = Struct.new(:table_name, :pluralize_table_names).new('products', true)
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :categories, nil, { :join_table => 'product_categories' }, product)
+    reflection = ActiveRecord::Reflection.create(:has_many, :categories, nil, { :join_table => 'product_categories' }, product)
     reflection.stubs(:klass).returns(category)
     assert_equal 'product_categories', reflection.join_table
 
-    reflection = AssociationReflection.new(:has_and_belongs_to_many, :products, nil, { :join_table => 'product_categories' }, category)
+    reflection = ActiveRecord::Reflection.create(:has_many, :products, nil, { :join_table => 'product_categories' }, category)
     reflection.stubs(:klass).returns(product)
     assert_equal 'product_categories', reflection.join_table
   end