diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/associations.rb | 48 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/association_proxy.rb | 1 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_many_through_association.rb | 33 | ||||
| -rw-r--r-- | activerecord/lib/active_record/reflection.rb | 31 | ||||
| -rw-r--r-- | activerecord/test/associations_join_model_test.rb | 11 | ||||
| -rw-r--r-- | activerecord/test/fixtures/tag.rb | 5 | 
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  | 
