From 57af961a80ee01d1876dfc50d93544906a995617 Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Sun, 19 Mar 2006 00:53:24 +0000 Subject: 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 --- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/associations.rb | 48 ++++++++++++++++++++++ .../associations/association_proxy.rb | 1 + .../associations/has_many_through_association.rb | 33 ++++----------- activerecord/lib/active_record/reflection.rb | 31 ++++++++++++++ activerecord/test/associations_join_model_test.rb | 11 ++++- activerecord/test/fixtures/tag.rb | 5 ++- 7 files changed, 104 insertions(+), 27 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3