Raise error when trying to select many polymorphic objects with has_many :through or :include (closes #4226) [josh@hasmanythrough.com]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3961 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

7 files changed, 104 insertions, 27 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 707fe2de24..ef959c97c5 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Raise error when trying to select many polymorphic objects with has_many :through or :include [Rick Olson]
+
 * Fixed has_many :through to include :conditions set on the :through association. closes #4020 [jonathan@bluewire.net.nz]
 
 * Fix that has_many :through honors the foreign key set by the belongs_to association in the join model (closes #4259) [andylien@gmail.com / Rick]
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index ede8fd1223..9d4ba329bc 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -9,6 +9,49 @@ require 'active_record/associations/has_and_belongs_to_many_association'
 require 'active_record/deprecated_associations'
 
 module ActiveRecord
+  class HasManyThroughAssociationNotFoundError < ActiveRecordError
+    def initialize(reflection)
+      @reflection = reflection
+    end
+    
+    def message
+      "Could not find the association '#{@reflection.options[:through]}' in model #{@reflection.klass}"
+    end
+  end
+
+  class HasManyThroughAssociationPolymorphicError < ActiveRecordError
+    def initialize(owner_class_name, reflection, source_reflection)
+      @owner_class_name  = owner_class_name
+      @reflection        = reflection
+      @source_reflection = source_reflection
+    end
+    
+    def message
+      "Cannot have a has_many :through association '#{@owner_class_name}##{@reflection.name}' on the polymorphic object '#{@source_reflection.class_name}##{@source_reflection.name}'."
+    end
+  end
+
+  class HasManyThroughSourceAssociationNotFoundError < ActiveRecordError
+    def initialize(through_reflection, source_reflection_name)
+      @through_reflection      = through_reflection
+      @source_reflection_name  = source_reflection_name
+    end
+    
+    def message
+      "Could not find the source association '#{@source_reflection_name}' in model #{@through_reflection.klass}"
+    end
+  end
+
+  class EagerLoadPolymorphicError < ActiveRecordError
+    def initialize(reflection)
+      @reflection = reflection
+    end
+    
+    def message
+      "Can not eagerly load the polymorphic association '#{@reflection.name}'"
+    end
+  end
+
   module Associations # :nodoc:
     def self.append_features(base)
       super
@@ -1204,6 +1247,11 @@ module ActiveRecord
             delegate    :options, :klass, :to => :reflection
 
             def initialize(reflection, join_dependency, parent = nil)
+              reflection.check_validity!
+              if reflection.options[:polymorphic]
+                raise EagerLoadPolymorphicError.new(reflection)
+              end
+
               super(reflection.klass)
               @parent             = parent
               @reflection         = reflection
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index 583f8c04b1..403c036db5 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -1,6 +1,7 @@
 module ActiveRecord
   module Associations
     class AssociationProxy #:nodoc:
+      attr_reader :reflection
       alias_method :proxy_respond_to?, :respond_to?
       alias_method :proxy_extend, :extend
       instance_methods.each { |m| undef_method m unless m =~ /(^__|^nil\?|^proxy_respond_to\?|^proxy_extend|^send)/ }
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 573e29ca31..562d0e93fb 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -1,9 +1,9 @@
 module ActiveRecord
   module Associations
     class HasManyThroughAssociation < AssociationProxy #:nodoc:
-
       def initialize(owner, reflection)
         super
+        reflection.check_validity!
         @finder_sql = construct_conditions
         construct_sql
       end
@@ -40,21 +40,6 @@ module ActiveRecord
       end
 
       protected
-        def through_reflection
-          unless @through_reflection ||= @owner.class.reflections[@reflection.options[:through]]
-            raise ActiveRecordError, "Could not find the association '#{@reflection.options[:through]}' in model #{@reflection.klass}"
-          end
-          @through_reflection
-        end
-
-        def source_reflection
-          @source_reflection_name ||= @reflection.name.to_s.singularize.to_sym
-          unless @source_reflection ||= through_reflection.klass.reflect_on_association(@source_reflection_name)
-            raise ActiveRecordError, "Could not find the source association '#{@source_reflection_name}' in model #{@through_reflection.klass}"
-          end
-          @source_reflection
-        end
-
         def method_missing(method, *args, &block)
           if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
             super
@@ -77,17 +62,17 @@ module ActiveRecord
 
         def construct_conditions
           # Get the actual primary key of the belongs_to association that the reflection is going through
-          source_primary_key = source_reflection.primary_key_name
+          source_primary_key = @reflection.source_reflection.primary_key_name
           
-          if through_reflection.options[:as]
+          if @reflection.through_reflection.options[:as]
             conditions = 
-              "#{@reflection.table_name}.#{@reflection.klass.primary_key} = #{through_reflection.table_name}.#{source_primary_key} " +
-              "AND #{through_reflection.table_name}.#{through_reflection.options[:as]}_id = #{@owner.quoted_id} " + 
-              "AND #{through_reflection.table_name}.#{through_reflection.options[:as]}_type = #{@owner.class.quote @owner.class.base_class.name.to_s}"
+              "#{@reflection.table_name}.#{@reflection.klass.primary_key} = #{@reflection.through_reflection.table_name}.#{source_primary_key} " +
+              "AND #{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_id = #{@owner.quoted_id} " + 
+              "AND #{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.options[:as]}_type = #{@owner.class.quote @owner.class.base_class.name.to_s}"
           else
             conditions = 
-              "#{@reflection.klass.table_name}.#{@reflection.klass.primary_key} = #{through_reflection.table_name}.#{source_primary_key} " +
-              "AND #{through_reflection.table_name}.#{through_reflection.primary_key_name} = #{@owner.quoted_id}"
+              "#{@reflection.klass.table_name}.#{@reflection.klass.primary_key} = #{@reflection.through_reflection.table_name}.#{source_primary_key} " +
+              "AND #{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.primary_key_name} = #{@owner.quoted_id}"
           end
           
           conditions << " AND (#{sql_conditions})" if sql_conditions
@@ -131,7 +116,7 @@ module ActiveRecord
         end
         
         def sql_conditions
-          @conditions ||= interpolate_sql(@reflection.active_record.send(:sanitize_sql, through_reflection.options[:conditions])) if through_reflection.options[:conditions]
+          @conditions ||= interpolate_sql(@reflection.active_record.send(:sanitize_sql, @reflection.through_reflection.options[:conditions])) if @reflection.through_reflection.options[:conditions]
         end
     end
   end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index e56682ec92..3011ba6625 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -144,6 +144,37 @@ module ActiveRecord
         @through_reflection ||= options[:through] ? active_record.reflect_on_association(options[:through]) : false
       end
 
+      def source_reflection_name
+        @source_reflection_name ||= name.to_s.singularize.to_sym
+      end
+
+      # Gets the source of the through reflection.  (The :tags association on Tagging below)
+      # 
+      #   class Post
+      #     has_many :tags, :through => :taggings
+      #   end
+      #
+      def source_reflection
+        return nil unless through_reflection
+        @source_reflection ||= through_reflection.klass.reflect_on_association(source_reflection_name)
+      end
+
+      def check_validity!
+        if options[:through]
+          if through_reflection.nil?
+            raise HasManyThroughAssociationNotFoundError.new(self)
+          end
+          
+          if source_reflection.nil?
+            raise HasManyThroughSourceAssociationNotFoundError.new(through_reflection, source_reflection_name)
+          end
+          
+          if source_reflection.options[:polymorphic]
+            raise HasManyThroughAssociationPolymorphicError.new(class_name, @reflection, source_reflection)
+          end
+        end
+      end
+
       private
         def name_to_class_name(name)
           if name =~ /::/
diff --git a/activerecord/test/associations_join_model_test.rb b/activerecord/test/associations_join_model_test.rb
index 9477e69e55..f53a7669f4 100644
--- a/activerecord/test/associations_join_model_test.rb
+++ b/activerecord/test/associations_join_model_test.rb
@@ -213,7 +213,7 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
   end
 
   def test_unavailable_through_reflection
-    assert_raises (ActiveRecord::ActiveRecordError) { authors(:david).nothings }
+    assert_raises (ActiveRecord::HasManyThroughAssociationNotFoundError) { authors(:david).nothings }
   end
 
   def test_has_many_through_join_model_with_conditions
@@ -221,6 +221,15 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
     assert_equal [], posts(:welcome).invalid_tags
   end
 
+  def test_has_many_polymorphic
+    assert_raises ActiveRecord::HasManyThroughAssociationPolymorphicError do
+      assert_equal [posts(:welcome), posts(:thinking)], tags(:general).taggables
+    end
+    assert_raises ActiveRecord::EagerLoadPolymorphicError do
+      assert_equal [posts(:welcome), posts(:thinking)], tags(:general).taggings.find(:all, :include => :taggable)
+    end
+  end
+
   private
     # create dynamic Post models to allow different dependency options
     def find_post_with_dependency(post_id, association, association_name, dependency)
diff --git a/activerecord/test/fixtures/tag.rb b/activerecord/test/fixtures/tag.rb
index 78b6f6666c..ed3c6ce34a 100644
--- a/activerecord/test/fixtures/tag.rb
+++ b/activerecord/test/fixtures/tag.rb
@@ -1,4 +1,5 @@
 class Tag < ActiveRecord::Base
-  has_many :taggings, :as => :taggable
-  has_one  :tagging,  :as => :taggable
+  has_many :taggings,  :as => :taggable
+  has_many :taggables, :through => :taggings
+  has_one  :tagging,   :as => :taggable
 end
\ No newline at end of file