Skip to content
Browse files

Raise error when trying to select many polymorphic objects with has_m…

…any :through or :include (closes #4226) [josh@hasmanythrough.com]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3961 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 49efa02 commit 57af961a80ee01d1876dfc50d93544906a995617 @technoweenie technoweenie committed
View
2 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]
View
48 activerecord/lib/active_record/associations.rb
@@ -9,6 +9,49 @@
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 @@ class JoinAssociation < JoinBase
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
View
1 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)/ }
View
33 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 @@ def reset
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 @@ def find_target
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 @@ def construct_sql
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
View
31 activerecord/lib/active_record/reflection.rb
@@ -144,6 +144,37 @@ def through_reflection
@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 =~ /::/
View
11 activerecord/test/associations_join_model_test.rb
@@ -213,7 +213,7 @@ def test_belongs_to_polymorphic_with_counter_cache
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 @@ def test_has_many_through_join_model_with_conditions
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)
View
5 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

0 comments on commit 57af961

Please sign in to comment.
Something went wrong with that request. Please try again.