Initial nested_has_many_through support [#1152]

10 files changed, 220 insertions, 15 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 565ebf8197..812abf5a55 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -111,6 +111,7 @@ module ActiveRecord
     autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association'
     autoload :HasManyAssociation, 'active_record/associations/has_many_association'
     autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association'
+    autoload :NestedHasManyThroughAssociation, 'active_record/associations/nested_has_many_through_association'
     autoload :HasOneAssociation, 'active_record/associations/has_one_association'
     autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association'
 
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 97883d8393..964c381c0d 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -1,4 +1,5 @@
 require "active_record/associations/through_association_scope"
+require "active_record/associations/nested_has_many_through"
 require 'active_support/core_ext/object/blank'
 
 module ActiveRecord
@@ -6,6 +7,7 @@ module ActiveRecord
   module Associations
     class HasManyThroughAssociation < HasManyAssociation #:nodoc:
       include ThroughAssociationScope
+      include NestedHasManyThrough
 
       alias_method :new, :build
 
diff --git a/activerecord/lib/active_record/associations/nested_has_many_through.rb b/activerecord/lib/active_record/associations/nested_has_many_through.rb
new file mode 100644
index 0000000000..2d03b81128
--- /dev/null
+++ b/activerecord/lib/active_record/associations/nested_has_many_through.rb
@@ -0,0 +1,156 @@
+module ActiveRecord
+  module Associations
+    module NestedHasManyThrough
+      def self.included(klass)
+        klass.alias_method_chain :construct_conditions, :nesting
+        klass.alias_method_chain :construct_joins,      :nesting
+      end
+
+      def construct_joins_with_nesting(custom_joins = nil)
+        if nested?
+          @nested_join_attributes ||= construct_nested_join_attributes
+          "#{construct_nested_join_attributes[:joins]} #{@reflection.options[:joins]} #{custom_joins}"
+        else
+          construct_joins_without_nesting(custom_joins)
+        end
+      end
+
+      def construct_conditions_with_nesting
+        if nested?
+          @nested_join_attributes ||= construct_nested_join_attributes
+          if @reflection.through_reflection && @reflection.through_reflection.macro == :belongs_to
+            "#{@nested_join_attributes[:remote_key]} = #{belongs_to_quoted_key} #{@nested_join_attributes[:conditions]}"
+          else
+            "#{@nested_join_attributes[:remote_key]} = #{@owner.quoted_id} #{@nested_join_attributes[:conditions]}"
+          end
+        else
+          construct_conditions_without_nesting
+        end
+      end
+
+      protected
+
+        # Given any belongs_to or has_many (including has_many :through) association,
+        # return the essential components of a join corresponding to that association, namely:
+        #
+        # * <tt>:joins</tt>: any additional joins required to get from the association's table
+        #   (reflection.table_name) to the table that's actually joining to the active record's table
+        # * <tt>:remote_key</tt>: the name of the key in the join table (qualified by table name) which will join
+        #   to a field of the active record's table
+        # * <tt>:local_key</tt>: the name of the key in the local table (not qualified by table name) which will
+        #   take part in the join
+        # * <tt>:conditions</tt>: any additional conditions (e.g. filtering by type for a polymorphic association,
+        #    or a :conditions clause explicitly given in the association), including a leading AND
+        def construct_nested_join_attributes(reflection = @reflection, association_class = reflection.klass, table_ids = {association_class.table_name => 1})
+          if (reflection.macro == :has_many || reflection.macro == :has_one) && reflection.through_reflection
+            construct_has_many_through_attributes(reflection, table_ids)
+          else
+            construct_has_many_or_belongs_to_attributes(reflection, association_class, table_ids)
+          end
+        end
+
+        def construct_has_many_through_attributes(reflection, table_ids)
+          # Construct the join components of the source association, so that we have a path from
+          # the eventual target table of the association up to the table named in :through, and
+          # all tables involved are allocated table IDs.
+          source_attrs = construct_nested_join_attributes(reflection.source_reflection, reflection.klass, table_ids)
+
+          # Determine the alias of the :through table; this will be the last table assigned
+          # when constructing the source join components above.
+          through_table_alias = through_table_name = reflection.through_reflection.table_name
+          through_table_alias += "_#{table_ids[through_table_name]}" unless table_ids[through_table_name] == 1
+
+          # Construct the join components of the through association, so that we have a path to
+          # the active record's table.
+          through_attrs = construct_nested_join_attributes(reflection.through_reflection, reflection.through_reflection.klass, table_ids)
+
+          # Any subsequent joins / filters on owner attributes will act on the through association,
+          # so that's what we return for the conditions/keys of the overall association.
+          conditions = through_attrs[:conditions]
+          conditions += " AND #{interpolate_sql(reflection.klass.send(:sanitize_sql, reflection.options[:conditions]))}" if reflection.options[:conditions]
+
+          {
+            :joins => "%s INNER JOIN %s ON ( %s = %s.%s %s) %s %s" % [
+              source_attrs[:joins],
+              through_table_name == through_table_alias ? through_table_name : "#{through_table_name} #{through_table_alias}",
+              source_attrs[:remote_key],
+              through_table_alias, source_attrs[:local_key],
+              source_attrs[:conditions],
+              through_attrs[:joins],
+              reflection.options[:joins]
+            ],
+            :remote_key => through_attrs[:remote_key],
+            :local_key => through_attrs[:local_key],
+            :conditions => conditions
+          }
+        end
+
+        # reflection is not has_many :through; it's a standard has_many / belongs_to instead
+        # TODO: see if we can defer to rails code here a bit more
+        def construct_has_many_or_belongs_to_attributes(reflection, association_class, table_ids)
+          # Determine the alias used for remote_table_name, if any. In all cases this will already
+          # have been assigned an ID in table_ids (either through being involved in a previous join,
+          # or - if it's the first table in the query - as the default value of table_ids)
+          remote_table_alias = remote_table_name = association_class.table_name
+          remote_table_alias += "_#{table_ids[remote_table_name]}" unless table_ids[remote_table_name] == 1
+
+          # Assign a new alias for the local table.
+          local_table_alias = local_table_name = reflection.active_record.table_name
+          if table_ids[local_table_name]
+            table_id = table_ids[local_table_name] += 1
+            local_table_alias += "_#{table_id}"
+          else
+            table_ids[local_table_name] = 1
+          end
+
+          conditions = ''
+          # Add type_condition, if applicable
+          conditions += " AND #{association_class.send(:type_condition).to_sql}" if association_class.finder_needs_type_condition?
+          # Add custom conditions
+          conditions += " AND (#{interpolate_sql(association_class.send(:sanitize_sql, reflection.options[:conditions]))})" if reflection.options[:conditions]
+
+          if reflection.macro == :belongs_to
+            if reflection.options[:polymorphic]
+              conditions += " AND #{local_table_alias}.#{reflection.options[:foreign_type]} = #{reflection.active_record.quote_value(association_class.base_class.name.to_s)}"
+            end
+            {
+              :joins => reflection.options[:joins],
+              :remote_key => "#{remote_table_alias}.#{association_class.primary_key}",
+              :local_key => reflection.primary_key_name,
+              :conditions => conditions
+            }
+          else
+            # Association is has_many (without :through)
+            if reflection.options[:as]
+              conditions += " AND #{remote_table_alias}.#{reflection.options[:as]}_type = #{reflection.active_record.quote_value(reflection.active_record.base_class.name.to_s)}"
+            end
+            {
+              :joins => "#{reflection.options[:joins]}",
+              :remote_key => "#{remote_table_alias}.#{reflection.primary_key_name}",
+              :local_key => reflection.klass.primary_key,
+              :conditions => conditions
+            }
+          end
+        end
+
+        def belongs_to_quoted_key
+          attribute = @reflection.through_reflection.primary_key_name
+          column    = @owner.column_for_attribute attribute
+
+          @owner.send(:quote_value, @owner.send(attribute), column)
+        end
+
+        def nested?
+          through_source_reflection? || through_through_reflection?
+        end
+
+        def through_source_reflection?
+          @reflection.source_reflection && @reflection.source_reflection.options[:through]
+        end
+
+        def through_through_reflection?
+          @reflection.through_reflection && @reflection.through_reflection.options[:through]
+        end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index db18fb7c0f..ae90d30b42 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -378,9 +378,9 @@ module ActiveRecord
           raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
         end
 
-        unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil?
-          raise HasManyThroughSourceAssociationMacroError.new(self)
-        end
+        # unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil?
+        #   raise HasManyThroughSourceAssociationMacroError.new(self)
+        # end
 
         check_validity_of_inverse!
       end
diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb
index f131dc01f6..0b1a3db1e4 100644
--- a/activerecord/test/cases/associations/join_model_test.rb
+++ b/activerecord/test/cases/associations/join_model_test.rb
@@ -394,14 +394,6 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
     end
   end
 
-  def test_has_many_through_has_many_through
-    assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).tags }
-  end
-
-  def test_has_many_through_habtm
-    assert_raise(ActiveRecord::HasManyThroughSourceAssociationMacroError) { authors(:david).post_categories }
-  end
-
   def test_eager_load_has_many_through_has_many
     author = Author.find :first, :conditions => ['name = ?', 'David'], :include => :comments, :order => 'comments.id'
     SpecialComment.new; VerySpecialComment.new
diff --git a/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb b/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb
new file mode 100644
index 0000000000..36de709ffc
--- /dev/null
+++ b/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb
@@ -0,0 +1,43 @@
+require "cases/helper"
+require 'models/author'
+require 'models/post'
+require 'models/person'
+require 'models/reference'
+require 'models/job'
+require 'models/reader'
+require 'models/comment'
+require 'models/tag'
+require 'models/tagging'
+require 'models/owner'
+require 'models/pet'
+require 'models/toy'
+require 'models/contract'
+require 'models/company'
+require 'models/developer'
+require 'models/subscriber'
+require 'models/book'
+require 'models/subscription'
+
+class NestedHasManyThroughAssociationsTest < ActiveRecord::TestCase
+  fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings
+
+  def test_has_many_through_a_has_many_through_association_on_source_reflection
+    author = authors(:david)
+    assert_equal [tags(:general), tags(:general)], author.tags
+  end
+
+  def test_has_many_through_a_has_many_through_association_on_through_reflection
+    author = authors(:david)
+    assert_equal [subscribers(:first), subscribers(:second), subscribers(:second)], author.subscribers
+  end
+
+  def test_distinct_has_many_through_a_has_many_through_association_on_source_reflection
+    author = authors(:david)
+    assert_equal [tags(:general)], author.distinct_tags
+  end
+
+  def test_distinct_has_many_through_a_has_many_through_association_on_through_reflection
+    author = authors(:david)
+    assert_equal [subscribers(:first), subscribers(:second)], author.distinct_subscribers
+  end
+end
\ No newline at end of file
diff --git a/activerecord/test/fixtures/books.yml b/activerecord/test/fixtures/books.yml
index 473663ff5b..fb48645456 100644
--- a/activerecord/test/fixtures/books.yml
+++ b/activerecord/test/fixtures/books.yml
@@ -1,7 +1,9 @@
 awdr:
+  author_id: 1
   id: 1
   name: "Agile Web Development with Rails"
 
 rfr:
+  author_id: 1
   id: 2
   name: "Ruby for Rails"
diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb
index 34bfd2d881..94810e2f34 100644
--- a/activerecord/test/models/author.rb
+++ b/activerecord/test/models/author.rb
@@ -83,14 +83,20 @@ class Author < ActiveRecord::Base
   has_many :author_favorites
   has_many :favorite_authors, :through => :author_favorites, :order => 'name'
 
-  has_many :tagging,  :through => :posts # through polymorphic has_one
-  has_many :taggings, :through => :posts, :source => :taggings # through polymorphic has_many
-  has_many :tags,     :through => :posts # through has_many :through
+  has_many :tagging,         :through => :posts # through polymorphic has_one
+  has_many :taggings,        :through => :posts, :source => :taggings # through polymorphic has_many
+  has_many :tags,            :through => :posts # through has_many :through (on source reflection + polymorphic)
+  has_many :distinct_tags,   :through => :posts, :source => :tags, :select => "DISTINCT tags.*", :order => "tags.name"
   has_many :post_categories, :through => :posts, :source => :categories
 
+  has_many :books
+  has_many :subscriptions,        :through => :books
+  has_many :subscribers,          :through => :subscriptions # through has_many :through (on through reflection)
+  has_many :distinct_subscribers, :through => :subscriptions, :source => :subscriber, :select => "DISTINCT subscribers.*", :order => "subscribers.nick"
+
   has_one :essay, :primary_key => :name, :as => :writer
 
-  belongs_to :author_address, :dependent => :destroy
+  belongs_to :author_address,       :dependent => :destroy
   belongs_to :author_address_extra, :dependent => :delete, :class_name => "AuthorAddress"
 
   scope :relation_include_posts, includes(:posts)
diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb
index 1e030b4f59..d27d0af77c 100644
--- a/activerecord/test/models/book.rb
+++ b/activerecord/test/models/book.rb
@@ -1,4 +1,6 @@
 class Book < ActiveRecord::Base
+  has_many :authors
+
   has_many :citations, :foreign_key => 'book1_id'
   has_many :references, :through => :citations, :source => :reference_of, :uniq => true
 
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index ea62833d81..dbd5da45eb 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -71,6 +71,7 @@ ActiveRecord::Schema.define do
   end
 
   create_table :books, :force => true do |t|
+    t.integer :author_id
     t.column :name, :string
   end