Introduce ActiveRecord::Reflection::ThroughReflection to simplify hm:…

…t reflection logic

activerecord/lib/active_record/reflection.rb

@@ -13,14 +13,15 @@ module ClassMethods
       def create_reflection(macro, name, options, active_record)
         case macro
           when :has_many, :belongs_to, :has_one, :has_and_belongs_to_many
-            reflection = AssociationReflection.new(macro, name, options, active_record)
+            klass = options[:through] ? ThroughReflection : AssociationReflection
+            reflection = klass.new(macro, name, options, active_record)
           when :composed_of
             reflection = AggregateReflection.new(macro, name, options, active_record)
         end
         write_inheritable_hash :reflections, name => reflection
         reflection
       end
-      
+
       # Returns a hash containing all AssociationReflection objects for the current class
       # Example:
       #
@@ -30,7 +31,7 @@ def create_reflection(macro, name, options, active_record)
       def reflections
         read_inheritable_attribute(:reflections) || write_inheritable_attribute(:reflections, {})
       end
-       
+
       # Returns an array of AggregateReflection objects for all the aggregations in the class.
       def reflect_on_all_aggregations
         reflections.values.select { |reflection| reflection.is_a?(AggregateReflection) }
@@ -192,6 +193,49 @@ def counter_cache_column
         end
       end
 
+      def check_validity!
+      end
+
+      def through_reflection
+        false
+      end
+
+      def source_reflection
+        nil
+      end
+
+      private
+        def derive_class_name
+          class_name = name.to_s.camelize
+          class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
+          class_name
+        end
+
+        def derive_primary_key_name
+          if macro == :belongs_to
+            "#{name}_id"
+          elsif options[:as]
+            "#{options[:as]}_id"
+          else
+            active_record.name.foreign_key
+          end
+        end
+    end
+
+    # Holds all the meta-data about a :through association as it was specified in the Active Record class.
+    class ThroughReflection < AssociationReflection #:nodoc:
+      # Gets the source of the through reflection.  It checks both a singularized and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>.
+      # (The <tt>:tags</tt> association on Tagging below.)
+      #
+      #   class Post < ActiveRecord::Base
+      #     has_many :taggings
+      #     has_many :tags, :through => :taggings
+      #   end
+      #
+      def source_reflection
+        @source_reflection ||= source_reflection_names.collect { |name| through_reflection.klass.reflect_on_association(name) }.compact.first
+      end
+
       # Returns the AssociationReflection object specified in the <tt>:through</tt> option
       # of a HasManyThrough or HasOneThrough association. Example:
       #
@@ -204,7 +248,7 @@ def counter_cache_column
       #   taggings_reflection = tags_reflection.through_reflection
       #
       def through_reflection
-        @through_reflection ||= options[:through] ? active_record.reflect_on_association(options[:through]) : false
+        @through_reflection ||= active_record.reflect_on_association(options[:through])
       end
 
       # Gets an array of possible <tt>:through</tt> source reflection names:
@@ -215,63 +259,32 @@ def source_reflection_names
         @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }
       end
 
-      # Gets the source of the through reflection.  It checks both a singularized and pluralized form for <tt>:belongs_to</tt> or <tt>:has_many</tt>.
-      # (The <tt>:tags</tt> association on Tagging below.)
-      # 
-      #   class Post < ActiveRecord::Base
-      #     has_many :taggings
-      #     has_many :tags, :through => :taggings
-      #   end
-      #
-      def source_reflection
-        return nil unless through_reflection
-        @source_reflection ||= source_reflection_names.collect { |name| through_reflection.klass.reflect_on_association(name) }.compact.first
-      end
-
       def check_validity!
-        if options[:through]
-          if through_reflection.nil?
-            raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
-          end
-
-          if source_reflection.nil?
-            raise HasManyThroughSourceAssociationNotFoundError.new(self)
-          end
+        if through_reflection.nil?
+          raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)
+        end
 
-          if options[:source_type] && source_reflection.options[:polymorphic].nil?
-            raise HasManyThroughAssociationPointlessSourceTypeError.new(active_record.name, self, source_reflection)
-          end
-
-          if source_reflection.options[:polymorphic] && options[:source_type].nil?
-            raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
-          end
-
-          unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil?
-            raise HasManyThroughSourceAssociationMacroError.new(self)
-          end
+        if source_reflection.nil?
+          raise HasManyThroughSourceAssociationNotFoundError.new(self)
+        end
+
+        if options[:source_type] && source_reflection.options[:polymorphic].nil?
+          raise HasManyThroughAssociationPointlessSourceTypeError.new(active_record.name, self, source_reflection)
+        end
+
+        if source_reflection.options[:polymorphic] && options[:source_type].nil?
+          raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
+        end
+
+        unless [:belongs_to, :has_many].include?(source_reflection.macro) && source_reflection.options[:through].nil?
+          raise HasManyThroughSourceAssociationMacroError.new(self)
         end
       end
 
       private
         def derive_class_name
           # get the class_name of the belongs_to association of the through reflection
-          if through_reflection
-            options[:source_type] || source_reflection.class_name
-          else
-            class_name = name.to_s.camelize
-            class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
-            class_name
-          end
-        end
-
-        def derive_primary_key_name
-          if macro == :belongs_to
-            "#{name}_id"
-          elsif options[:as]
-            "#{options[:as]}_id"
-          else
-            active_record.name.foreign_key
-          end
+          options[:source_type] || source_reflection.class_name
         end
     end
   end

activerecord/test/cases/reflection_test.rb

@@ -170,6 +170,10 @@ def test_reflection_should_not_raise_error_when_compared_to_other_object
     assert_nothing_raised { Firm.reflections[:clients] == Object.new }
   end
 
+  def test_has_many_through_reflection
+    assert_kind_of ActiveRecord::Reflection::ThroughReflection, Subscriber.reflect_on_association(:books)
+  end
+
   private
     def assert_reflection(klass, association, options)
       assert reflection = klass.reflect_on_association(association)