Merge commit 'rails/master'

activerecord/lib/active_record/associations/association_collection.rb

@@ -351,7 +351,19 @@ def proxy_respond_to?(method, include_private = false)
       protected
         def construct_find_options!(options)
         end
-
+
+        def construct_counter_sql
+          if @reflection.options[:counter_sql]
+            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
+          elsif @reflection.options[:finder_sql]
+            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
+            @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
+            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
+          else
+            @counter_sql = @finder_sql
+          end
+        end
+
         def load_target
           if !@owner.new_record? || foreign_key_present
             begin

activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb

@@ -85,15 +85,7 @@ def construct_sql
 
           @join_sql = "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}"
 
-          if @reflection.options[:counter_sql]
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          elsif @reflection.options[:finder_sql]
-            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          else
-            @counter_sql = @finder_sql
-          end
+          construct_counter_sql
         end
 
         def construct_scope

activerecord/lib/active_record/associations/has_many_association.rb

@@ -97,15 +97,7 @@ def construct_sql
               @finder_sql << " AND (#{conditions})" if conditions
           end
 
-          if @reflection.options[:counter_sql]
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          elsif @reflection.options[:finder_sql]
-            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          else
-            @counter_sql = @finder_sql
-          end
+          construct_counter_sql
         end
 
         def construct_scope

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -90,15 +90,7 @@ def construct_sql
               @finder_sql = construct_conditions
           end
 
-          if @reflection.options[:counter_sql]
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          elsif @reflection.options[:finder_sql]
-            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
-            @counter_sql = interpolate_sql(@reflection.options[:counter_sql])
-          else
-            @counter_sql = @finder_sql
-          end
+          construct_counter_sql
         end
 
         def has_cached_counter?

activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb

@@ -803,6 +803,13 @@ def test_count_with_counter_sql
     assert_equal 1, developer.projects.count
   end
 
+  unless current_adapter?(:PostgreSQLAdapter)
+    def test_count_with_finder_sql
+      assert_equal 3, projects(:active_record).developers_with_finder_sql.count
+      assert_equal 3, projects(:active_record).developers_with_multiline_finder_sql.count
+    end
+  end
+
   def test_association_proxy_transaction_method_starts_transaction_in_association_class
     Post.expects(:transaction)
     Category.find(:first).posts.transaction do

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -163,6 +163,11 @@ def test_counting_non_existant_items_using_sql
     assert_equal 0, Firm.find(:first).no_clients_using_counter_sql.size
   end
 
+  def test_counting_using_finder_sql
+    assert_equal 2, Firm.find(4).clients_using_sql.count
+    assert_equal 2, Firm.find(4).clients_using_multiline_sql.count
+  end
+
   def test_belongs_to_sanity
     c = Client.new
     assert_nil c.firm

activerecord/test/cases/base_test.rb

@@ -599,9 +599,9 @@ def test_destroy_all
   end
 
   def test_destroy_many
-    assert_equal 3, Client.count
-    Client.destroy([2, 3])
-    assert_equal 1, Client.count
+    assert_difference('Client.count', -2) do
+      Client.destroy([2, 3])
+    end
   end
 
   def test_delete_many

activerecord/test/cases/calculations_test.rb

@@ -203,15 +203,15 @@ def test_should_calculate_grouped_by_function
     c = Company.count(:all, :group => "UPPER(#{QUOTED_TYPE})")
     assert_equal 2, c[nil]
     assert_equal 1, c['DEPENDENTFIRM']
-    assert_equal 3, c['CLIENT']
+    assert_equal 4, c['CLIENT']
     assert_equal 2, c['FIRM']
   end
 
   def test_should_calculate_grouped_by_function_with_table_alias
     c = Company.count(:all, :group => "UPPER(companies.#{QUOTED_TYPE})")
     assert_equal 2, c[nil]
     assert_equal 1, c['DEPENDENTFIRM']
-    assert_equal 3, c['CLIENT']
+    assert_equal 4, c['CLIENT']
     assert_equal 2, c['FIRM']
   end
 

activerecord/test/cases/finder_test.rb

@@ -1044,8 +1044,8 @@ def test_select_value
   end
 
   def test_select_values
-    assert_equal ["1","2","3","4","5","6","7","8","9"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map! { |i| i.to_s }
-    assert_equal ["37signals","Summit","Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
+    assert_equal ["1","2","3","4","5","6","7","8","9", "10"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map! { |i| i.to_s }
+    assert_equal ["37signals","Summit","Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
   end
 
   def test_select_rows

activerecord/test/cases/inheritance_test.rb

@@ -112,9 +112,9 @@ def test_alt_inheritance_save
   end
 
   def test_inheritance_condition
-    assert_equal 9, Company.count
+    assert_equal 10, Company.count
     assert_equal 2, Firm.count
-    assert_equal 3, Client.count
+    assert_equal 4, Client.count
   end
 
   def test_alt_inheritance_condition

activerecord/test/cases/reflection_test.rb

@@ -176,8 +176,8 @@ def test_association_reflection_in_modules
 
   def test_reflection_of_all_associations
     # FIXME these assertions bust a lot
-    assert_equal 28, Firm.reflect_on_all_associations.size
-    assert_equal 21, Firm.reflect_on_all_associations(:has_many).size
+    assert_equal 29, Firm.reflect_on_all_associations.size
+    assert_equal 22, Firm.reflect_on_all_associations(:has_many).size
     assert_equal 7, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end

activerecord/test/fixtures/companies.yml

@@ -35,6 +35,14 @@ another_client:
   name: Ex Nihilo
   ruby_type: Client
 
+a_third_client:
+  id: 10
+  type: Client
+  firm_id: 4
+  client_of: 4
+  name: Ex Nihilo Part Deux
+  ruby_type: Client
+
 rails_core:
   id: 6          
   name: RailsCore

activerecord/test/models/company.rb

@@ -48,6 +48,10 @@ class Firm < Company
   has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => 'rating > #{rating}'
   has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id"
   has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}'
+  has_many :clients_using_multiline_sql, :class_name => "Client", :finder_sql => '
+  SELECT
+  companies.*
+  FROM companies WHERE companies.client_of = #{id}'
   has_many :clients_using_counter_sql, :class_name => "Client",
            :finder_sql  => 'SELECT * FROM companies WHERE client_of = #{id}',
            :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = #{id}'

activerecord/test/models/project.rb

@@ -8,6 +8,12 @@ class Project < ActiveRecord::Base
   has_and_belongs_to_many :developers_named_david_with_hash_conditions, :class_name => "Developer", :conditions => { :name => 'David' }, :uniq => true
   has_and_belongs_to_many :salaried_developers, :class_name => "Developer", :conditions => "salary > 0"
   has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => 'SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id'
+  has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => '
+    SELECT
+      t.*, j.*
+    FROM
+      developers_projects j,
+      developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id'
   has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => "DELETE FROM developers_projects WHERE project_id = \#{id} AND developer_id = \#{record.id}"
   has_and_belongs_to_many :developers_with_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || '<new>'}"},
                             :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || '<new>'}"},