From 44af2efa2c7391681968c827ca47201a0a02e974 Mon Sep 17 00:00:00 2001
From: Ernie Miller <ernie@metautonomo.us>
Date: Thu, 28 Aug 2008 14:01:42 -0400
Subject: Refactored AssociationCollection#count for uniformity and Ruby 1.8.7
 support.

[#831 state:resolved]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/associations.rb     |  3 +++
 .../associations/association_collection.rb         | 29 ++++++++++++++++++++++
 .../has_and_belongs_to_many_association.rb         | 10 ++++++++
 .../associations/has_many_association.rb           | 26 -------------------
 .../associations/has_many_through_association.rb   | 10 --------
 .../has_and_belongs_to_many_associations_test.rb   |  7 ++++++
 6 files changed, 49 insertions(+), 36 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 4d935612ca..98710dee09 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1164,6 +1164,9 @@ module ActiveRecord
       #   If true, duplicate associated objects will be ignored by accessors and query methods.
       # [:finder_sql]
       #   Overwrite the default generated SQL statement used to fetch the association with a manual statement
+      # [:counter_sql]
+      #   Specify a complete SQL statement to fetch the size of the association. If <tt>:finder_sql</tt> is
+      #   specified but not <tt>:counter_sql</tt>, <tt>:counter_sql</tt> will be generated by replacing <tt>SELECT ... FROM</tt> with <tt>SELECT COUNT(*) FROM</tt>.
       # [:delete_sql]
       #   Overwrite the default generated SQL statement used to remove links between the associated
       #   classes with a manual statement.
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 9061037b39..5092ccc1dc 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -128,6 +128,35 @@ module ActiveRecord
         end
       end
 
+      # Count all records using SQL. If the +:counter_sql+ option is set for the association, it will
+      # be used for the query. If no +:counter_sql+ was supplied, but +:finder_sql+ was set, the
+      # descendant's +construct_sql+ method will have set :counter_sql automatically.
+      # Otherwise, construct options and pass them with scope to the target class's +count+.
+      def count(*args)
+        if @reflection.options[:counter_sql]
+          @reflection.klass.count_by_sql(@counter_sql)
+        else
+          column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args)
+          if @reflection.options[:uniq]
+            # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
+            column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" if column_name == :all
+            options.merge!(:distinct => true)
+          end
+
+          value = @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) }
+
+          limit  = @reflection.options[:limit]
+          offset = @reflection.options[:offset]
+
+          if limit || offset
+            [ [value - offset.to_i, 0].max, limit.to_i ].min
+          else
+            value
+          end
+        end
+      end
+
+
       # Remove +records+ from this association.  Does not destroy +records+.
       def delete(*records)
         records = flatten_deeper(records)
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index e7e433b6b6..3d689098b5 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -78,6 +78,16 @@ module ActiveRecord
           end
 
           @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
         end
 
         def construct_scope
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 1535995410..1838021d40 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -1,32 +1,6 @@
 module ActiveRecord
   module Associations
     class HasManyAssociation < AssociationCollection #:nodoc:
-      # Count the number of associated records. All arguments are optional.
-      def count(*args)
-        if @reflection.options[:counter_sql]
-          @reflection.klass.count_by_sql(@counter_sql)
-        elsif @reflection.options[:finder_sql]
-          @reflection.klass.count_by_sql(@finder_sql)
-        else
-          column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args)          
-          options[:conditions] = options[:conditions].blank? ?
-            @finder_sql :
-            @finder_sql + " AND (#{sanitize_sql(options[:conditions])})"
-          options[:include] ||= @reflection.options[:include]
-
-          value = @reflection.klass.count(column_name, options)
-
-          limit  = @reflection.options[:limit]
-          offset = @reflection.options[:offset]
-
-          if limit || offset
-            [ [value - offset.to_i, 0].max, limit.to_i ].min
-          else
-            value
-          end
-        end
-      end
-
       protected
         def owner_quoted_id
           if @reflection.options[:primary_key]
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 24b02efc35..84fa900f46 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -31,16 +31,6 @@ module ActiveRecord
         return count
       end
       
-      def count(*args)
-        column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args)
-        if @reflection.options[:uniq]
-          # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL statement.
-          column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" if column_name == :all
-          options.merge!(:distinct => true) 
-        end
-        @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) } 
-      end
-
       protected
         def construct_find_options!(options)
           options[:select]  = construct_select(options[:select])
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 0572418e3b..1b31d28679 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -703,4 +703,11 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     # due to Unknown column 'authors.id'
     assert Category.find(1).posts_with_authors_sorted_by_author_id.find_by_title('Welcome to the weblog')
   end
+
+  def test_counting_on_habtm_association_and_not_array
+    david = Developer.find(1)
+    # Extra parameter just to make sure we aren't falling back to
+    # Array#count in Ruby >=1.8.7, which would raise an ArgumentError
+    assert_nothing_raised { david.projects.count(:all, :conditions => '1=1') }
+  end
 end
-- 
cgit v1.2.3


From db22c89543f45d7f27847003af949afa21cb6fa1 Mon Sep 17 00:00:00 2001
From: Andrew White <andyw@pixeltrix.co.uk>
Date: Thu, 28 Aug 2008 17:00:18 +0100
Subject: Merge scoped :joins together instead of overwriting them. May expose
 scoping bugs in your code!

[#501 state:resolved]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/associations.rb |  4 +-
 activerecord/lib/active_record/base.rb         | 37 ++++++++----
 activerecord/lib/active_record/calculations.rb |  2 +-
 activerecord/test/cases/method_scoping_test.rb | 81 +++++++++++++++++++++++++-
 4 files changed, 109 insertions(+), 15 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 98710dee09..3ca93db10f 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1599,7 +1599,7 @@ module ActiveRecord
           sql = "SELECT #{column_aliases(join_dependency)} FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "
           sql << join_dependency.join_associations.collect{|join| join.association_join }.join
 
-          add_joins!(sql, options, scope)
+          add_joins!(sql, options[:joins], scope)
           add_conditions!(sql, options[:conditions], scope)
           add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
 
@@ -1655,7 +1655,7 @@ module ActiveRecord
 
           if is_distinct
             sql << distinct_join_associations.collect { |assoc| assoc.association_join }.join
-            add_joins!(sql, options, scope)
+            add_joins!(sql, options[:joins], scope)
           end
 
           add_conditions!(sql, options[:conditions], scope)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 5c30f80555..b5ffc471bc 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1549,7 +1549,7 @@ module ActiveRecord #:nodoc:
           sql  = "SELECT #{options[:select] || (scope && scope[:select]) || ((options[:joins] || (scope && scope[:joins])) && quoted_table_name + '.*') || '*'} "
           sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "
 
-          add_joins!(sql, options, scope)
+          add_joins!(sql, options[:joins], scope)
           add_conditions!(sql, options[:conditions], scope)
 
           add_group!(sql, options[:group], scope)
@@ -1565,6 +1565,22 @@ module ActiveRecord #:nodoc:
          (safe_to_array(first) + safe_to_array(second)).uniq
         end
 
+        def merge_joins(first, second)
+          if first.is_a?(String) && second.is_a?(String)
+            "#{first} #{second}"
+          elsif first.is_a?(String) || second.is_a?(String)
+            if first.is_a?(String)
+              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, second, nil)
+              "#{first} #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join}"
+            else
+              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, first, nil)
+              "#{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} #{second}"
+            end
+          else
+            (safe_to_array(first) + safe_to_array(second)).uniq
+          end
+        end
+
         # Object#to_a is deprecated, though it does have the desired behavior
         def safe_to_array(o)
           case o
@@ -1620,16 +1636,15 @@ module ActiveRecord #:nodoc:
         end
 
         # The optional scope argument is for the current <tt>:find</tt> scope.
-        def add_joins!(sql, options, scope = :auto)
+        def add_joins!(sql, joins, scope = :auto)
           scope = scope(:find) if :auto == scope
-          [(scope && scope[:joins]), options[:joins]].each do |join|
-            case join
-            when Symbol, Hash, Array
-              join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil)
-              sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
-            else
-              sql << " #{join} "
-            end
+          merged_joins = scope && scope[:joins] && joins ? merge_joins(scope[:joins], joins) : (joins || scope && scope[:joins])
+          case merged_joins
+          when Symbol, Hash, Array
+            join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil)
+            sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
+          when String
+            sql << " #{merged_joins} "
           end
         end
 
@@ -1879,6 +1894,8 @@ module ActiveRecord #:nodoc:
                         hash[method][key] = merge_conditions(params[key], hash[method][key])
                       elsif key == :include && merge
                         hash[method][key] = merge_includes(hash[method][key], params[key]).uniq
+                      elsif key == :joins && merge
+                        hash[method][key] = merge_joins(params[key], hash[method][key])
                       else
                         hash[method][key] = hash[method][key] || params[key]
                       end
diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 08306f361a..0325a8f8ca 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -188,7 +188,7 @@ module ActiveRecord
           end
 
           joins = ""
-          add_joins!(joins, options, scope)
+          add_joins!(joins, options[:joins], scope)
 
           if merged_includes.any?
             join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins)
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index ee66ac948d..af6fcd32ad 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -1,4 +1,5 @@
 require "cases/helper"
+require 'models/author'
 require 'models/developer'
 require 'models/project'
 require 'models/comment'
@@ -6,7 +7,7 @@ require 'models/post'
 require 'models/category'
 
 class MethodScopingTest < ActiveRecord::TestCase
-  fixtures :developers, :projects, :comments, :posts, :developers_projects
+  fixtures :authors, :developers, :projects, :comments, :posts, :developers_projects
 
   def test_set_conditions
     Developer.with_scope(:find => { :conditions => 'just a test...' }) do
@@ -97,6 +98,46 @@ class MethodScopingTest < ActiveRecord::TestCase
     assert_equal developers(:david).attributes, scoped_developers.first.attributes
   end
 
+  def test_scoped_find_using_new_style_joins
+    scoped_developers = Developer.with_scope(:find => { :joins => :projects }) do
+      Developer.find(:all, :conditions => 'projects.id = 2')
+    end
+    assert scoped_developers.include?(developers(:david))
+    assert !scoped_developers.include?(developers(:jamis))
+    assert_equal 1, scoped_developers.size
+    assert_equal developers(:david).attributes, scoped_developers.first.attributes
+  end
+
+  def test_scoped_find_merges_old_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id ' }) do
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'INNER JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1')
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
+
+  def test_scoped_find_merges_new_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => :posts }) do
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => :comments, :conditions => 'comments.id = 1')
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
+
+  def test_scoped_find_merges_new_and_old_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => :posts }) do
+      Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1')
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
+
   def test_scoped_count_include
     # with the include, will retrieve only developers for the given project
     Developer.with_scope(:find => { :include => :projects }) do
@@ -152,7 +193,7 @@ class MethodScopingTest < ActiveRecord::TestCase
 end
 
 class NestedScopingTest < ActiveRecord::TestCase
-  fixtures :developers, :projects, :comments, :posts
+  fixtures :authors, :developers, :projects, :comments, :posts
 
   def test_merge_options
     Developer.with_scope(:find => { :conditions => 'salary = 80000' }) do
@@ -357,6 +398,42 @@ class NestedScopingTest < ActiveRecord::TestCase
       assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods')
     end
   end
+
+  def test_nested_scoped_find_merges_old_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id' }) do
+      Author.with_scope(:find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do
+        Author.find(:all, :select => 'DISTINCT authors.*', :conditions => 'comments.id = 1')
+      end
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
+
+  def test_nested_scoped_find_merges_new_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => :posts }) do
+      Author.with_scope(:find => { :joins => :comments }) do
+        Author.find(:all, :select => 'DISTINCT authors.*', :conditions => 'comments.id = 1')
+      end
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
+
+  def test_nested_scoped_find_merges_new_and_old_style_joins
+    scoped_authors = Author.with_scope(:find => { :joins => :posts }) do
+      Author.with_scope(:find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do
+        Author.find(:all, :select => 'DISTINCT authors.*', :joins => '', :conditions => 'comments.id = 1')
+      end
+    end
+    assert scoped_authors.include?(authors(:david))
+    assert !scoped_authors.include?(authors(:mary))
+    assert_equal 1, scoped_authors.size
+    assert_equal authors(:david).attributes, scoped_authors.first.attributes
+  end
 end
 
 class HasManyScopingTest< ActiveRecord::TestCase
-- 
cgit v1.2.3


From 082c377954fda43ad3b84abe5864e3e181f8e757 Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremy@bitsweat.net>
Date: Thu, 28 Aug 2008 12:32:38 -0700
Subject: Missed changelog update for #501

---
 activerecord/CHANGELOG | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 12295ac799..dcbe354733 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Edge*
 
+* Merge scoped :joins together instead of overwriting them. May expose scoping bugs in your code!  #501 [Andrew White]
+
 * before_save, before_validation and before_destroy callbacks that return false will now ROLLBACK the transaction.  Previously this would have been committed before the processing was aborted. #891 [Xavier Noria]
 
 * Transactional migrations for databases which support them.  #834 [divoxx, Adam Wiggins, Tarmo Tänav]
-- 
cgit v1.2.3


From db116a2ed688d36570f412a42e9fc33dcbca56c7 Mon Sep 17 00:00:00 2001
From: Jan De Poorter <jan@defv.be>
Date: Mon, 25 Aug 2008 12:37:34 +0200
Subject: Fix NamedScope regex so methods containing "an" get delegated to
 proxy_found. [#901 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/named_scope.rb | 2 +-
 activerecord/test/cases/named_scope_test.rb   | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index c99c4beca9..b9b7eb5f00 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -103,7 +103,7 @@ module ActiveRecord
       attr_reader :proxy_scope, :proxy_options
 
       [].methods.each do |m|
-        unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/
+        unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty\?|any\?|respond_to\?)/
           delegate m, :to => :proxy_found
         end
       end
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 6f6ea1cbe9..4fe7f4f5b9 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -249,6 +249,10 @@ class NamedScopeTest < ActiveRecord::TestCase
     assert_equal Topic.base.select(&:approved), Topic.base.find_all(&:approved)
   end
 
+  def test_rand_should_select_a_random_object_from_proxy
+    assert Topic.approved.rand.is_a? Topic
+  end
+
   def test_should_use_where_in_query_for_named_scope
     assert_equal Developer.find_all_by_name('Jamis'), Developer.find_all_by_id(Developer.jamises)
   end
-- 
cgit v1.2.3


From 6769d824f9c20812a0e6cd6e0c19742984b2450e Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 29 Aug 2008 15:23:28 +0200
Subject: Fix parentheses warnings

---
 activerecord/test/cases/named_scope_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 4fe7f4f5b9..acc3b3016a 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -250,7 +250,7 @@ class NamedScopeTest < ActiveRecord::TestCase
   end
 
   def test_rand_should_select_a_random_object_from_proxy
-    assert Topic.approved.rand.is_a? Topic
+    assert Topic.approved.rand.is_a?(Topic)
   end
 
   def test_should_use_where_in_query_for_named_scope
-- 
cgit v1.2.3


From a9086b3daa0a5174ba2118a2131eb5121f32d41b Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 29 Aug 2008 10:19:26 -0500
Subject: Make query-cache thread-local

Signed-off-by: Joshua Peek <josh@joshpeek.com>
---
 .../connection_adapters/abstract/query_cache.rb    | 43 +++++++++++++++-------
 1 file changed, 29 insertions(+), 14 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
index 2afd6064ad..2fc50b9bfa 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
@@ -4,7 +4,6 @@ module ActiveRecord
       class << self
         def included(base)
           base.class_eval do
-            attr_accessor :query_cache_enabled
             alias_method_chain :columns, :query_cache
             alias_method_chain :select_all, :query_cache
           end
@@ -16,7 +15,7 @@ module ActiveRecord
           method_names.each do |method_name|
             base.class_eval <<-end_code, __FILE__, __LINE__
               def #{method_name}_with_query_dirty(*args)
-                clear_query_cache if @query_cache_enabled
+                clear_query_cache if query_cache_enabled
                 #{method_name}_without_query_dirty(*args)
               end
 
@@ -26,22 +25,38 @@ module ActiveRecord
         end
       end
 
+      def query_cache_enabled
+        Thread.current['query_cache_enabled']
+      end
+
+      def query_cache_enabled=(flag)
+        Thread.current['query_cache_enabled'] = flag
+      end
+
+      def query_cache
+        Thread.current['query_cache']
+      end
+
+      def query_cache=(cache)
+        Thread.current['query_cache'] = cache
+      end
+
       # Enable the query cache within the block.
       def cache
-        old, @query_cache_enabled = @query_cache_enabled, true
-        @query_cache ||= {}
+        old, self.query_cache_enabled = query_cache_enabled, true
+        self.query_cache ||= {}
         yield
       ensure
         clear_query_cache
-        @query_cache_enabled = old
+        self.query_cache_enabled = old
       end
 
       # Disable the query cache within the block.
       def uncached
-        old, @query_cache_enabled = @query_cache_enabled, false
+        old, self.query_cache_enabled = query_cache_enabled, false
         yield
       ensure
-        @query_cache_enabled = old
+        self.query_cache_enabled = old
       end
 
       # Clears the query cache.
@@ -51,11 +66,11 @@ module ActiveRecord
       # the same SQL query and repeatedly return the same result each time, silently
       # undermining the randomness you were expecting.
       def clear_query_cache
-        @query_cache.clear if @query_cache
+        query_cache.clear if query_cache
       end
 
       def select_all_with_query_cache(*args)
-        if @query_cache_enabled
+        if query_cache_enabled
           cache_sql(args.first) { select_all_without_query_cache(*args) }
         else
           select_all_without_query_cache(*args)
@@ -63,8 +78,8 @@ module ActiveRecord
       end
 
       def columns_with_query_cache(*args)
-        if @query_cache_enabled
-          @query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args)
+        if query_cache_enabled
+          query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args)
         else
           columns_without_query_cache(*args)
         end
@@ -73,11 +88,11 @@ module ActiveRecord
       private
         def cache_sql(sql)
           result =
-            if @query_cache.has_key?(sql)
+            if query_cache.has_key?(sql)
               log_info(sql, "CACHE", 0.0)
-              @query_cache[sql]
+              query_cache[sql]
             else
-              @query_cache[sql] = yield
+              query_cache[sql] = yield
             end
 
           if Array === result
-- 
cgit v1.2.3


From 743f0e7114b071bf7786a80227c12dcc7ccee6c1 Mon Sep 17 00:00:00 2001
From: Eugene Pimenov <libc@mac.com>
Date: Fri, 29 Aug 2008 14:36:00 +0400
Subject: Make case insensitive validates_uniqueness_of use unicode aware
 downcase method.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
 activerecord/lib/active_record/validations.rb |  2 +-
 activerecord/test/cases/validations_test.rb   | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 8fe4336bbc..9ee80e6655 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -664,7 +664,7 @@ module ActiveRecord
             condition_params = [value]
           else
             condition_sql = "LOWER(#{sql_attribute}) #{comparison_operator}"
-            condition_params = [value.downcase]
+            condition_params = [value.chars.downcase]
           end
 
           if scope = configuration[:scope]
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index a40bda2533..4999d93a86 100644
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -451,6 +451,18 @@ class ValidationsTest < ActiveRecord::TestCase
     t2.title = nil
     assert t2.valid?, "should validate with nil"
     assert t2.save, "should save with nil"
+
+    with_kcode('UTF8') do
+      t_utf8 = Topic.new("title" => "Я тоже уникальный!")
+      assert t_utf8.save, "Should save t_utf8 as unique"
+
+      # If database hasn't UTF-8 character set, this test fails
+      if Topic.find(t_utf8, :select => 'LOWER(title) AS title').title == "я тоже уникальный!"
+        t2_utf8 = Topic.new("title" => "я тоже УНИКАЛЬНЫЙ!")
+        assert !t2_utf8.valid?, "Shouldn't be valid"
+        assert !t2_utf8.save, "Shouldn't save t2_utf8 as unique"
+      end
+    end
   end
 
   def test_validate_case_sensitive_uniqueness
-- 
cgit v1.2.3


From 6edaa267174dfedaf5b152b9eba25b4eb5e34c99 Mon Sep 17 00:00:00 2001
From: Nick <nick@nicksieger.com>
Date: Sat, 19 Apr 2008 00:24:01 -0500
Subject: Initial conversion to connection pool

So far so good, tests still run clean. Next steps: synchronize connection pool access
and modification, and change allow_concurrency to simply switch a real lock for a
null lock.
---
 .../abstract/connection_pool.rb                    | 128 +++++++++++++
 .../abstract/connection_specification.rb           | 198 +++------------------
 .../connection_adapters/abstract_adapter.rb        |   1 +
 .../test/cases/threaded_connections_test.rb        |  39 ++--
 4 files changed, 171 insertions(+), 195 deletions(-)
 create mode 100644 activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
 mode change 100644 => 100755 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
new file mode 100644
index 0000000000..8685988e80
--- /dev/null
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -0,0 +1,128 @@
+module ActiveRecord
+  module ConnectionAdapters
+    class ConnectionPool
+      # Check for activity after at least +verification_timeout+ seconds.
+      # Defaults to 0 (always check.)
+      attr_accessor :verification_timeout
+      attr_reader :active_connections, :spec
+
+      def initialize(spec)
+        @verification_timeout = 0
+
+        # The thread id -> adapter cache.
+        @active_connections = {}
+
+        # The ConnectionSpecification for this pool
+        @spec = spec
+      end
+
+      def active_connection_name #:nodoc:
+        Thread.current.object_id
+      end
+
+      def active_connection
+        active_connections[active_connection_name]
+      end
+
+      # Returns the connection currently associated with the class. This can
+      # also be used to "borrow" the connection to do database work unrelated
+      # to any of the specific Active Records.
+      def connection
+        if conn = active_connections[active_connection_name]
+          conn
+        else
+          # retrieve_connection sets the cache key.
+          conn = retrieve_connection
+          active_connections[active_connection_name] = conn
+        end
+      end
+
+      # Clears the cache which maps classes to connections.
+      def clear_active_connections!
+        clear_entries!(@active_connections, [active_connection_name]) do |name, conn|
+          conn.disconnect!
+        end
+      end
+
+      # Clears the cache which maps classes 
+      def clear_reloadable_connections!
+        @active_connections.each do |name, conn|
+          if conn.requires_reloading?
+            conn.disconnect!
+            @active_connections.delete(name)
+          end
+        end
+      end
+
+      # Verify active connections.
+      def verify_active_connections! #:nodoc:
+        remove_stale_cached_threads!(@active_connections) do |name, conn|
+          conn.disconnect!
+        end
+        active_connections.each_value do |connection|
+          connection.verify!(@verification_timeout)
+        end
+      end
+
+      def retrieve_connection #:nodoc:
+        # Name is nil if establish_connection hasn't been called for
+        # some class along the inheritance chain up to AR::Base yet.
+        name = active_connection_name
+        if conn = active_connections[name]
+          # Verify the connection.
+          conn.verify!(@verification_timeout)
+        else
+          self.connection = spec
+          conn = active_connections[name]
+        end
+
+        conn or raise ConnectionNotEstablished
+      end
+
+      # Returns true if a connection that's accessible to this class has already been opened.
+      def connected?
+        active_connections[active_connection_name] ? true : false
+      end
+
+      def disconnect!
+        clear_cache!(@active_connections) do |name, conn|
+          conn.disconnect!
+        end
+      end
+
+      # Set the connection for the class.
+      def connection=(spec) #:nodoc:
+        if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter)
+          active_connections[active_connection_name] = spec
+        elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification)
+          self.connection = ActiveRecord::Base.send(spec.adapter_method, spec.config)
+        else
+          raise ConnectionNotEstablished
+        end
+      end
+
+      private
+        def clear_cache!(cache, &block)
+          cache.each(&block) if block_given?
+          cache.clear
+        end
+
+        # Remove stale threads from the cache.
+        def remove_stale_cached_threads!(cache, &block)
+          stale = Set.new(cache.keys)
+
+          Thread.list.each do |thread|
+            stale.delete(thread.object_id) if thread.alive?
+          end
+          clear_entries!(cache, stale, &block)
+        end
+
+        def clear_entries!(cache, keys, &block)
+          keys.each do |key|
+            block.call(key, cache[key])
+            cache.delete(key)
+          end
+        end
+    end
+  end
+end
\ No newline at end of file
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 2a8807fb78..6eee1ac8c1 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -14,156 +14,45 @@ module ActiveRecord
     cattr_accessor :verification_timeout, :instance_writer => false
     @@verification_timeout = 0
 
-    # The class -> [adapter_method, config] map
+    # The class -> connection pool map
     @@defined_connections = {}
 
-    # The class -> thread id -> adapter cache. (class -> adapter if not allow_concurrency)
-    @@active_connections = {}
-
     class << self
-      # Retrieve the connection cache.
-      def thread_safe_active_connections #:nodoc:
-        @@active_connections[Thread.current.object_id] ||= {}
-      end
-     
-      def single_threaded_active_connections #:nodoc:
-        @@active_connections
-      end
-     
-      # pick up the right active_connection method from @@allow_concurrency
-      if @@allow_concurrency
-        alias_method :active_connections, :thread_safe_active_connections
-      else
-        alias_method :active_connections, :single_threaded_active_connections
-      end
-     
-      # set concurrency support flag (not thread safe, like most of the methods in this file)
-      def allow_concurrency=(threaded) #:nodoc:
-        logger.debug "allow_concurrency=#{threaded}" if logger
-        return if @@allow_concurrency == threaded
-        clear_all_cached_connections!
-        @@allow_concurrency = threaded
-        method_prefix = threaded ? "thread_safe" : "single_threaded"
-        sing = (class << self; self; end)
-        [:active_connections, :scoped_methods].each do |method|
-          sing.send(:alias_method, method, "#{method_prefix}_#{method}")
-        end
-        log_connections if logger
-      end
-      
-      def active_connection_name #:nodoc:
-        @active_connection_name ||=
-           if active_connections[name] || @@defined_connections[name]
-             name
-           elsif self == ActiveRecord::Base
-             nil
-           else
-             superclass.active_connection_name
-           end
-      end
-
-      def clear_active_connection_name #:nodoc:
-        @active_connection_name = nil
-        subclasses.each { |klass| klass.clear_active_connection_name }
+      # for internal use only
+      def active_connections
+        @@defined_connections.inject([]) {|arr,kv| arr << kv.last.active_connection}.compact.uniq
       end
 
       # Returns the connection currently associated with the class. This can
       # also be used to "borrow" the connection to do database work unrelated
       # to any of the specific Active Records.
       def connection
-        if defined?(@active_connection_name) && (conn = active_connections[@active_connection_name])
-          conn
-        else
-          # retrieve_connection sets the cache key.
-          conn = retrieve_connection
-          active_connections[@active_connection_name] = conn
-        end
+        retrieve_connection
       end
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        clear_cache!(@@active_connections) do |name, conn|
-          conn.disconnect!
+        clear_cache!(@@defined_connections) do |name, pool|
+          pool.disconnect!
         end
       end
       
       # Clears the cache which maps classes 
       def clear_reloadable_connections!
-        if @@allow_concurrency
-          # With concurrent connections @@active_connections is
-          # a hash keyed by thread id.
-          @@active_connections.each do |thread_id, conns|
-            conns.each do |name, conn|
-              if conn.requires_reloading?
-                conn.disconnect!
-                @@active_connections[thread_id].delete(name)
-              end
-            end
-          end
-        else
-          @@active_connections.each do |name, conn|
-            if conn.requires_reloading?
-              conn.disconnect!
-              @@active_connections.delete(name)
-            end
-          end
+        clear_cache!(@@defined_connections) do |name, pool|
+          pool.clear_reloadable_connections!
         end
       end
 
       # Verify active connections.
       def verify_active_connections! #:nodoc:
-        if @@allow_concurrency
-          remove_stale_cached_threads!(@@active_connections) do |name, conn|
-            conn.disconnect!
-          end
-        end
-        
-        active_connections.each_value do |connection|
-          connection.verify!(@@verification_timeout)
-        end
+        @@defined_connections.each_value {|pool| pool.verify_active_connections!}
       end
 
       private
-        def clear_cache!(cache, thread_id = nil, &block)
-          if cache
-            if @@allow_concurrency
-              thread_id ||= Thread.current.object_id
-              thread_cache, cache = cache, cache[thread_id]
-              return unless cache
-            end
-
-            cache.each(&block) if block_given?
-            cache.clear
-          end
-        ensure
-          if thread_cache && @@allow_concurrency
-            thread_cache.delete(thread_id)
-          end
-        end
-
-        # Remove stale threads from the cache.
-        def remove_stale_cached_threads!(cache, &block)
-          stale = Set.new(cache.keys)
-
-          Thread.list.each do |thread|
-            stale.delete(thread.object_id) if thread.alive?
-          end
-
-          stale.each do |thread_id|
-            clear_cache!(cache, thread_id, &block)
-          end
-        end
-        
-        def clear_all_cached_connections!
-          if @@allow_concurrency
-            @@active_connections.each_value do |connection_hash_for_thread|
-              connection_hash_for_thread.each_value {|conn| conn.disconnect! }
-              connection_hash_for_thread.clear
-            end
-          else
-            @@active_connections.each_value {|conn| conn.disconnect! }
-          end
-          @@active_connections.clear          
+        def clear_cache!(cache, &block)
+          cache.each(&block) if block_given?
+          cache.clear
         end
     end
 
@@ -208,9 +97,7 @@ module ActiveRecord
           raise AdapterNotSpecified unless defined? RAILS_ENV
           establish_connection(RAILS_ENV)
         when ConnectionSpecification
-          clear_active_connection_name
-          @active_connection_name = name
-          @@defined_connections[name] = spec
+          @@defined_connections[name] = ConnectionAdapters::ConnectionPool.new(spec)
         when Symbol, String
           if configuration = configurations[spec.to_s]
             establish_connection(configuration)
@@ -248,26 +135,20 @@ module ActiveRecord
     # opened and set as the active connection for the class it was defined
     # for (not necessarily the current class).
     def self.retrieve_connection #:nodoc:
-      # Name is nil if establish_connection hasn't been called for
-      # some class along the inheritance chain up to AR::Base yet.
-      if name = active_connection_name
-        if conn = active_connections[name]
-          # Verify the connection.
-          conn.verify!(@@verification_timeout)
-        elsif spec = @@defined_connections[name]
-          # Activate this connection specification.
-          klass = name.constantize
-          klass.connection = spec
-          conn = active_connections[name]
-        end
-      end
+      pool = retrieve_connection_pool
+      (pool && pool.connection) or raise ConnectionNotEstablished
+    end
 
-      conn or raise ConnectionNotEstablished
+    def self.retrieve_connection_pool
+      pool = @@defined_connections[name]
+      return pool if pool
+      return nil if ActiveRecord::Base == self
+      superclass.retrieve_connection_pool
     end
 
     # Returns true if a connection that's accessible to this class has already been opened.
     def self.connected?
-      active_connections[active_connection_name] ? true : false
+      retrieve_connection_pool.connected?
     end
 
     # Remove the connection for this class. This will close the active
@@ -275,35 +156,10 @@ module ActiveRecord
     # can be used as an argument for establish_connection, for easily
     # re-establishing the connection.
     def self.remove_connection(klass=self)
-      spec = @@defined_connections[klass.name]
-      konn = active_connections[klass.name]
-      @@defined_connections.delete_if { |key, value| value == spec }
-      active_connections.delete_if { |key, value| value == konn }
-      konn.disconnect! if konn
-      spec.config if spec
-    end
-
-    # Set the connection for the class.
-    def self.connection=(spec) #:nodoc:
-      if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter)
-        active_connections[name] = spec
-      elsif spec.kind_of?(ConnectionSpecification)
-        config = spec.config.reverse_merge(:allow_concurrency => @@allow_concurrency)
-        self.connection = self.send(spec.adapter_method, config)
-      elsif spec.nil?
-        raise ConnectionNotEstablished
-      else
-        establish_connection spec
-      end
-    end
-
-    # connection state logging
-    def self.log_connections #:nodoc:
-      if logger
-        logger.info "Defined connections: #{@@defined_connections.inspect}"
-        logger.info "Active connections: #{active_connections.inspect}"
-        logger.info "Active connection name: #{@active_connection_name}"
-      end
+      pool = @@defined_connections[klass.name]
+      @@defined_connections.delete_if { |key, value| value == pool }
+      pool.disconnect! if pool
+      pool.spec.config if pool
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
old mode 100644
new mode 100755
index 6924bb7e6f..af8cfbee7a
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -8,6 +8,7 @@ require 'active_record/connection_adapters/abstract/schema_statements'
 require 'active_record/connection_adapters/abstract/database_statements'
 require 'active_record/connection_adapters/abstract/quoting'
 require 'active_record/connection_adapters/abstract/connection_specification'
+require 'active_record/connection_adapters/abstract/connection_pool'
 require 'active_record/connection_adapters/abstract/query_cache'
 
 module ActiveRecord
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 28f8302367..fedcab98e3 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -8,41 +8,32 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
 
     fixtures :topics
 
-  def setup
-    @connection = ActiveRecord::Base.remove_connection
-    @connections = []
-    @allow_concurrency = ActiveRecord::Base.allow_concurrency
-  end
+    def setup
+      @connection = ActiveRecord::Base.remove_connection
+      @connections = []
+    end
 
-  def teardown
-    # clear the connection cache
-    ActiveRecord::Base.send(:clear_all_cached_connections!)
-    # set allow_concurrency to saved value
-    ActiveRecord::Base.allow_concurrency = @allow_concurrency
-    # reestablish old connection
-    ActiveRecord::Base.establish_connection(@connection)
-  end
+    def teardown
+      # clear the connection cache
+      ActiveRecord::Base.clear_active_connections!
+      # reestablish old connection
+      ActiveRecord::Base.establish_connection(@connection)
+    end
 
-  def gather_connections(use_threaded_connections)
-    ActiveRecord::Base.allow_concurrency = use_threaded_connections
-    ActiveRecord::Base.establish_connection(@connection)
+    def gather_connections
+      ActiveRecord::Base.establish_connection(@connection)
 
       5.times do
         Thread.new do
           Topic.find :first
-          @connections << ActiveRecord::Base.active_connections.values.first
+          @connections << ActiveRecord::Base.active_connections.first
         end.join
       end
     end
 
     def test_threaded_connections
-      gather_connections(true)
-      assert_equal @connections.uniq.length, 5
-    end
-
-    def test_unthreaded_connections
-      gather_connections(false)
-      assert_equal @connections.uniq.length, 1
+      gather_connections
+      assert_equal @connections.length, 5
     end
   end
 end
-- 
cgit v1.2.3


From 5879b15f23f080badedbbab7835eda8c2c748a52 Mon Sep 17 00:00:00 2001
From: Nick <nick@nicksieger.com>
Date: Sat, 19 Apr 2008 00:34:22 -0500
Subject: Rename defined_connections to connection_pools

- Distinguis meaning of "active_connections" to always mean connections
  associated with the current thread
---
 .../abstract/connection_specification.rb           | 30 ++++++++++++++--------
 .../test/cases/threaded_connections_test.rb        |  2 +-
 2 files changed, 21 insertions(+), 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 6eee1ac8c1..b2771c60e7 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -15,12 +15,16 @@ module ActiveRecord
     @@verification_timeout = 0
 
     # The class -> connection pool map
-    @@defined_connections = {}
+    @@connection_pools = {}
 
     class << self
       # for internal use only
       def active_connections
-        @@defined_connections.inject([]) {|arr,kv| arr << kv.last.active_connection}.compact.uniq
+        @@connection_pools.inject({}) do |hash,kv|
+          hash[kv.first] = kv.last.active_connection
+          hash.delete(kv.first) unless hash[kv.first]
+          hash
+        end
       end
 
       # Returns the connection currently associated with the class. This can
@@ -32,21 +36,27 @@ module ActiveRecord
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        clear_cache!(@@defined_connections) do |name, pool|
-          pool.disconnect!
+        clear_cache!(@@connection_pools) do |name, pool|
+          pool.clear_active_connections!
         end
       end
       
       # Clears the cache which maps classes 
       def clear_reloadable_connections!
-        clear_cache!(@@defined_connections) do |name, pool|
+        clear_cache!(@@connection_pools) do |name, pool|
           pool.clear_reloadable_connections!
         end
       end
 
+      def clear_all_connections!
+        clear_cache!(@@connection_pools) do |name, pool|
+          pool.disconnect!
+        end
+      end
+
       # Verify active connections.
       def verify_active_connections! #:nodoc:
-        @@defined_connections.each_value {|pool| pool.verify_active_connections!}
+        @@connection_pools.each_value {|pool| pool.verify_active_connections!}
       end
 
       private
@@ -97,7 +107,7 @@ module ActiveRecord
           raise AdapterNotSpecified unless defined? RAILS_ENV
           establish_connection(RAILS_ENV)
         when ConnectionSpecification
-          @@defined_connections[name] = ConnectionAdapters::ConnectionPool.new(spec)
+          @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
         when Symbol, String
           if configuration = configurations[spec.to_s]
             establish_connection(configuration)
@@ -140,7 +150,7 @@ module ActiveRecord
     end
 
     def self.retrieve_connection_pool
-      pool = @@defined_connections[name]
+      pool = @@connection_pools[name]
       return pool if pool
       return nil if ActiveRecord::Base == self
       superclass.retrieve_connection_pool
@@ -156,8 +166,8 @@ module ActiveRecord
     # can be used as an argument for establish_connection, for easily
     # re-establishing the connection.
     def self.remove_connection(klass=self)
-      pool = @@defined_connections[klass.name]
-      @@defined_connections.delete_if { |key, value| value == pool }
+      pool = @@connection_pools[klass.name]
+      @@connection_pools.delete_if { |key, value| value == pool }
       pool.disconnect! if pool
       pool.spec.config if pool
     end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index fedcab98e3..9bf7217958 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -26,7 +26,7 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
       5.times do
         Thread.new do
           Topic.find :first
-          @connections << ActiveRecord::Base.active_connections.first
+          @connections << ActiveRecord::Base.active_connections.values.first
         end.join
       end
     end
-- 
cgit v1.2.3


From 50cd4bdc99ebaf3ac879e4e7fea43c5b55ca5f68 Mon Sep 17 00:00:00 2001
From: Nick <nick@nicksieger.com>
Date: Sat, 19 Apr 2008 12:42:43 -0500
Subject: Introduce synchronization around connection pool access

- use new active support Module#synchronize
- allow_concurrency now switches between a null monitor and a
  regular monitor (defaulting to null monitor to avoid overhead)
---
 .../abstract/connection_pool.rb                    | 15 ++++---
 .../abstract/connection_specification.rb           | 49 ++++++++++++++++------
 .../test/cases/threaded_connections_test.rb        |  4 ++
 3 files changed, 47 insertions(+), 21 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 8685988e80..8f241a39ca 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -1,14 +1,12 @@
+require 'set'
+
 module ActiveRecord
   module ConnectionAdapters
     class ConnectionPool
-      # Check for activity after at least +verification_timeout+ seconds.
-      # Defaults to 0 (always check.)
-      attr_accessor :verification_timeout
+      delegate :verification_timeout, :to => "::ActiveRecord::Base"
       attr_reader :active_connections, :spec
 
       def initialize(spec)
-        @verification_timeout = 0
-
         # The thread id -> adapter cache.
         @active_connections = {}
 
@@ -44,7 +42,7 @@ module ActiveRecord
         end
       end
 
-      # Clears the cache which maps classes 
+      # Clears the cache which maps classes
       def clear_reloadable_connections!
         @active_connections.each do |name, conn|
           if conn.requires_reloading?
@@ -60,7 +58,7 @@ module ActiveRecord
           conn.disconnect!
         end
         active_connections.each_value do |connection|
-          connection.verify!(@verification_timeout)
+          connection.verify!(verification_timeout)
         end
       end
 
@@ -70,7 +68,7 @@ module ActiveRecord
         name = active_connection_name
         if conn = active_connections[name]
           # Verify the connection.
-          conn.verify!(@verification_timeout)
+          conn.verify!(verification_timeout)
         else
           self.connection = spec
           conn = active_connections[name]
@@ -119,6 +117,7 @@ module ActiveRecord
 
         def clear_entries!(cache, keys, &block)
           keys.each do |key|
+            next unless cache.has_key?(key)
             block.call(key, cache[key])
             cache.delete(key)
           end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index b2771c60e7..e262b2bac5 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -1,4 +1,4 @@
-require 'set'
+require 'monitor'
 
 module ActiveRecord
   class Base
@@ -9,6 +9,15 @@ module ActiveRecord
       end
     end
 
+    class NullMonitor
+      def synchronize
+        yield
+      end
+    end
+
+    cattr_accessor :connection_pools_lock, :instance_writer => false
+    @@connection_pools_lock = NullMonitor.new
+
     # Check for activity after at least +verification_timeout+ seconds.
     # Defaults to 0 (always check.)
     cattr_accessor :verification_timeout, :instance_writer => false
@@ -18,8 +27,18 @@ module ActiveRecord
     @@connection_pools = {}
 
     class << self
+      def allow_concurrency=(flag)
+        if @@allow_concurrency != flag
+          if flag
+            self.connection_pools_lock = Monitor.new
+          else
+            self.connection_pools_lock = NullMonitor.new
+          end
+        end
+      end
+
       # for internal use only
-      def active_connections
+      def active_connections #:nodoc:
         @@connection_pools.inject({}) do |hash,kv|
           hash[kv.first] = kv.last.active_connection
           hash.delete(kv.first) unless hash[kv.first]
@@ -36,22 +55,16 @@ module ActiveRecord
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        clear_cache!(@@connection_pools) do |name, pool|
-          pool.clear_active_connections!
-        end
+        @@connection_pools.each_value {|pool| pool.clear_active_connections! }
       end
       
-      # Clears the cache which maps classes 
+      # Clears the cache which maps classes
       def clear_reloadable_connections!
-        clear_cache!(@@connection_pools) do |name, pool|
-          pool.clear_reloadable_connections!
-        end
+        @@connection_pools.each_value {|pool| pool.clear_reloadable_connections! }
       end
 
       def clear_all_connections!
-        clear_cache!(@@connection_pools) do |name, pool|
-          pool.disconnect!
-        end
+        clear_cache!(@@connection_pools) {|name, pool| pool.disconnect! }
       end
 
       # Verify active connections.
@@ -59,6 +72,9 @@ module ActiveRecord
         @@connection_pools.each_value {|pool| pool.verify_active_connections!}
       end
 
+      synchronize :active_connections, :clear_active_connections!, :clear_reloadable_connections!,
+        :clear_all_connections!, :verify_active_connections!, :with => :connection_pools_lock
+
       private
         def clear_cache!(cache, &block)
           cache.each(&block) if block_given?
@@ -107,7 +123,9 @@ module ActiveRecord
           raise AdapterNotSpecified unless defined? RAILS_ENV
           establish_connection(RAILS_ENV)
         when ConnectionSpecification
-          @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
+          connection_pools_lock.synchronize do
+            @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
+          end
         when Symbol, String
           if configuration = configurations[spec.to_s]
             establish_connection(configuration)
@@ -171,5 +189,10 @@ module ActiveRecord
       pool.disconnect! if pool
       pool.spec.config if pool
     end
+
+    class << self
+      synchronize :retrieve_connection, :retrieve_connection_pool, :connected?,
+        :remove_connection, :with => :connection_pools_lock
+    end
   end
 end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 9bf7217958..3f88f79189 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -11,11 +11,15 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
     def setup
       @connection = ActiveRecord::Base.remove_connection
       @connections = []
+      @allow_concurrency = ActiveRecord::Base.allow_concurrency
+      ActiveRecord::Base.allow_concurrency = true
     end
 
     def teardown
       # clear the connection cache
       ActiveRecord::Base.clear_active_connections!
+      # set allow_concurrency to saved value
+      ActiveRecord::Base.allow_concurrency = @allow_concurrency
       # reestablish old connection
       ActiveRecord::Base.establish_connection(@connection)
     end
-- 
cgit v1.2.3


From cab76ce6ac2983f59451e2d53b23746a2873aea0 Mon Sep 17 00:00:00 2001
From: Nick <nick@nicksieger.com>
Date: Sat, 19 Apr 2008 14:42:56 -0500
Subject: Add synchronization to connection pool also

---
 .../connection_adapters/abstract/connection_pool.rb        | 14 +++++++++++---
 .../abstract/connection_specification.rb                   |  4 +++-
 2 files changed, 14 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 8f241a39ca..2d13d02fad 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -12,6 +12,9 @@ module ActiveRecord
 
         # The ConnectionSpecification for this pool
         @spec = spec
+
+        # The mutex used to synchronize pool access
+        @connection_mutex = Monitor.new
       end
 
       def active_connection_name #:nodoc:
@@ -70,7 +73,7 @@ module ActiveRecord
           # Verify the connection.
           conn.verify!(verification_timeout)
         else
-          self.connection = spec
+          self.set_connection spec
           conn = active_connections[name]
         end
 
@@ -82,6 +85,7 @@ module ActiveRecord
         active_connections[active_connection_name] ? true : false
       end
 
+      # Disconnect all connections in the pool.
       def disconnect!
         clear_cache!(@active_connections) do |name, conn|
           conn.disconnect!
@@ -89,16 +93,20 @@ module ActiveRecord
       end
 
       # Set the connection for the class.
-      def connection=(spec) #:nodoc:
+      def set_connection(spec) #:nodoc:
         if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter)
           active_connections[active_connection_name] = spec
         elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification)
-          self.connection = ActiveRecord::Base.send(spec.adapter_method, spec.config)
+          self.set_connection ActiveRecord::Base.send(spec.adapter_method, spec.config)
         else
           raise ConnectionNotEstablished
         end
       end
 
+      synchronize :active_connection, :connection, :clear_active_connections!,
+        :clear_reloadable_connections!, :verify_active_connections!, :retrieve_connection,
+        :connected?, :disconnect!, :set_connection, :with => :@connection_mutex
+
       private
         def clear_cache!(cache, &block)
           cache.each(&block) if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index e262b2bac5..ed9d074506 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -27,6 +27,8 @@ module ActiveRecord
     @@connection_pools = {}
 
     class << self
+      # Turning on allow_concurrency basically switches a null mutex for a real one, so that
+      # multi-threaded access of the connection pools hash is synchronized.
       def allow_concurrency=(flag)
         if @@allow_concurrency != flag
           if flag
@@ -37,7 +39,7 @@ module ActiveRecord
         end
       end
 
-      # for internal use only
+      # for internal use only and for testing
       def active_connections #:nodoc:
         @@connection_pools.inject({}) do |hash,kv|
           hash[kv.first] = kv.last.active_connection
-- 
cgit v1.2.3


From 37b0b36918f14519b28326057bba38ca6fcfbd3b Mon Sep 17 00:00:00 2001
From: Nick <nick@nicksieger.com>
Date: Sat, 19 Apr 2008 23:11:28 -0500
Subject: Fix failure to retain value of allow_concurrency

- Also carry allow_concurrency value through to connection adapter
  (for postgresql)
---
 .../lib/active_record/connection_adapters/abstract/connection_pool.rb  | 3 ++-
 .../connection_adapters/abstract/connection_specification.rb           | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 2d13d02fad..08fc61daaa 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -97,7 +97,8 @@ module ActiveRecord
         if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter)
           active_connections[active_connection_name] = spec
         elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification)
-          self.set_connection ActiveRecord::Base.send(spec.adapter_method, spec.config)
+          config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
+          self.set_connection ActiveRecord::Base.send(spec.adapter_method, config)
         else
           raise ConnectionNotEstablished
         end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index ed9d074506..ddca97c3bf 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -31,6 +31,7 @@ module ActiveRecord
       # multi-threaded access of the connection pools hash is synchronized.
       def allow_concurrency=(flag)
         if @@allow_concurrency != flag
+          @@allow_concurrency = flag
           if flag
             self.connection_pools_lock = Monitor.new
           else
-- 
cgit v1.2.3


From ff97e9d029d6164fa2e921a5d0acab13f39058b0 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Sat, 7 Jun 2008 00:30:15 -0500
Subject: Connection handling methods extracted out into separate
 ConnectionHandler class

- delegating methods left behind
---
 .../abstract/connection_pool.rb                    |  89 ++++++++++++++
 .../abstract/connection_specification.rb           | 131 +++++----------------
 .../connection_adapters/abstract_adapter.rb        |   2 +-
 3 files changed, 117 insertions(+), 105 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 08fc61daaa..01a75365d3 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -1,3 +1,4 @@
+require 'monitor'
 require 'set'
 
 module ActiveRecord
@@ -132,5 +133,93 @@ module ActiveRecord
           end
         end
     end
+
+    class ConnectionHandler
+      attr_reader :connection_pools_lock
+
+      def initialize
+        @connection_pools = {}
+        @connection_pools_lock = Monitor.new
+      end
+
+      def establish_connection(name, spec)
+        connection_pools_lock.synchronize do
+          @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
+        end
+      end
+
+      # for internal use only and for testing
+      def active_connections #:nodoc:
+        @connection_pools.inject({}) do |hash,kv|
+          hash[kv.first] = kv.last.active_connection
+          hash.delete(kv.first) unless hash[kv.first]
+          hash
+        end
+      end
+
+      # Clears the cache which maps classes to connections.
+      def clear_active_connections!
+        @connection_pools.each_value {|pool| pool.clear_active_connections! }
+      end
+
+      # Clears the cache which maps classes
+      def clear_reloadable_connections!
+        @connection_pools.each_value {|pool| pool.clear_reloadable_connections! }
+      end
+
+      def clear_all_connections!
+        clear_cache!(@connection_pools) {|name, pool| pool.disconnect! }
+      end
+
+      # Verify active connections.
+      def verify_active_connections! #:nodoc:
+        @connection_pools.each_value {|pool| pool.verify_active_connections!}
+      end
+
+      # Locate the connection of the nearest super class. This can be an
+      # active or defined connection: if it is the latter, it will be
+      # opened and set as the active connection for the class it was defined
+      # for (not necessarily the current class).
+      def retrieve_connection(klass) #:nodoc:
+        pool = retrieve_connection_pool(klass)
+        (pool && pool.connection) or raise ConnectionNotEstablished
+      end
+
+      def retrieve_connection_pool(klass)
+        loop do
+          pool = @connection_pools[klass.name]
+          return pool if pool
+          return nil if ActiveRecord::Base == klass
+          klass = klass.superclass
+        end
+      end
+
+      # Returns true if a connection that's accessible to this class has already been opened.
+      def connected?(klass)
+        retrieve_connection_pool(klass).connected?
+      end
+
+      # Remove the connection for this class. This will close the active
+      # connection and the defined connection (if they exist). The result
+      # can be used as an argument for establish_connection, for easily
+      # re-establishing the connection.
+      def remove_connection(klass)
+        pool = @connection_pools[klass.name]
+        @connection_pools.delete_if { |key, value| value == pool }
+        pool.disconnect! if pool
+        pool.spec.config if pool
+      end
+
+      synchronize :retrieve_connection, :retrieve_connection_pool, :connected?,
+        :remove_connection, :active_connections, :clear_active_connections!,
+        :clear_reloadable_connections!, :clear_all_connections!,
+        :verify_active_connections!, :with => :connection_pools_lock
+
+      private
+        def clear_cache!(cache, &block)
+          cache.each(&block) if block_given?
+          cache.clear
+        end
+    end
   end
 end
\ No newline at end of file
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index ddca97c3bf..2dc3201208 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -1,5 +1,3 @@
-require 'monitor'
-
 module ActiveRecord
   class Base
     class ConnectionSpecification #:nodoc:
@@ -9,80 +7,19 @@ module ActiveRecord
       end
     end
 
-    class NullMonitor
-      def synchronize
-        yield
-      end
-    end
-
-    cattr_accessor :connection_pools_lock, :instance_writer => false
-    @@connection_pools_lock = NullMonitor.new
-
     # Check for activity after at least +verification_timeout+ seconds.
     # Defaults to 0 (always check.)
     cattr_accessor :verification_timeout, :instance_writer => false
     @@verification_timeout = 0
 
-    # The class -> connection pool map
-    @@connection_pools = {}
-
-    class << self
-      # Turning on allow_concurrency basically switches a null mutex for a real one, so that
-      # multi-threaded access of the connection pools hash is synchronized.
-      def allow_concurrency=(flag)
-        if @@allow_concurrency != flag
-          @@allow_concurrency = flag
-          if flag
-            self.connection_pools_lock = Monitor.new
-          else
-            self.connection_pools_lock = NullMonitor.new
-          end
-        end
-      end
-
-      # for internal use only and for testing
-      def active_connections #:nodoc:
-        @@connection_pools.inject({}) do |hash,kv|
-          hash[kv.first] = kv.last.active_connection
-          hash.delete(kv.first) unless hash[kv.first]
-          hash
-        end
-      end
-
-      # Returns the connection currently associated with the class. This can
-      # also be used to "borrow" the connection to do database work unrelated
-      # to any of the specific Active Records.
-      def connection
-        retrieve_connection
-      end
-
-      # Clears the cache which maps classes to connections.
-      def clear_active_connections!
-        @@connection_pools.each_value {|pool| pool.clear_active_connections! }
-      end
-      
-      # Clears the cache which maps classes
-      def clear_reloadable_connections!
-        @@connection_pools.each_value {|pool| pool.clear_reloadable_connections! }
-      end
+    # The connection handler
+    cattr_accessor :connection_handler, :instance_writer => false
+    @@connection_handler = ConnectionAdapters::ConnectionHandler.new
 
-      def clear_all_connections!
-        clear_cache!(@@connection_pools) {|name, pool| pool.disconnect! }
-      end
-
-      # Verify active connections.
-      def verify_active_connections! #:nodoc:
-        @@connection_pools.each_value {|pool| pool.verify_active_connections!}
-      end
-
-      synchronize :active_connections, :clear_active_connections!, :clear_reloadable_connections!,
-        :clear_all_connections!, :verify_active_connections!, :with => :connection_pools_lock
-
-      private
-        def clear_cache!(cache, &block)
-          cache.each(&block) if block_given?
-          cache.clear
-        end
+    # Turning on allow_concurrency basically switches a null mutex for a real one, so that
+    # multi-threaded access of the connection pools hash is synchronized.
+    def self.allow_concurrency=(flag)
+      @@allow_concurrency = flag
     end
 
     # Returns the connection currently associated with the class. This can
@@ -126,9 +63,7 @@ module ActiveRecord
           raise AdapterNotSpecified unless defined? RAILS_ENV
           establish_connection(RAILS_ENV)
         when ConnectionSpecification
-          connection_pools_lock.synchronize do
-            @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
-          end
+          @@connection_handler.establish_connection(name, spec)
         when Symbol, String
           if configuration = configurations[spec.to_s]
             establish_connection(configuration)
@@ -161,41 +96,29 @@ module ActiveRecord
       end
     end
 
-    # Locate the connection of the nearest super class. This can be an
-    # active or defined connection: if it is the latter, it will be
-    # opened and set as the active connection for the class it was defined
-    # for (not necessarily the current class).
-    def self.retrieve_connection #:nodoc:
-      pool = retrieve_connection_pool
-      (pool && pool.connection) or raise ConnectionNotEstablished
-    end
+    class << self
+      # Returns the connection currently associated with the class. This can
+      # also be used to "borrow" the connection to do database work unrelated
+      # to any of the specific Active Records.
+      def connection
+        retrieve_connection
+      end
 
-    def self.retrieve_connection_pool
-      pool = @@connection_pools[name]
-      return pool if pool
-      return nil if ActiveRecord::Base == self
-      superclass.retrieve_connection_pool
-    end
+      def retrieve_connection
+        connection_handler.retrieve_connection(self)
+      end
 
-    # Returns true if a connection that's accessible to this class has already been opened.
-    def self.connected?
-      retrieve_connection_pool.connected?
-    end
+      def connected?
+        connection_handler.connected?(self)
+      end
 
-    # Remove the connection for this class. This will close the active
-    # connection and the defined connection (if they exist). The result
-    # can be used as an argument for establish_connection, for easily
-    # re-establishing the connection.
-    def self.remove_connection(klass=self)
-      pool = @@connection_pools[klass.name]
-      @@connection_pools.delete_if { |key, value| value == pool }
-      pool.disconnect! if pool
-      pool.spec.config if pool
-    end
+      def remove_connection(klass=self)
+        connection_handler.remove_connection(klass)
+      end
 
-    class << self
-      synchronize :retrieve_connection, :retrieve_connection_pool, :connected?,
-        :remove_connection, :with => :connection_pools_lock
+      delegate :active_connections, :clear_active_connections!,
+        :clear_reloadable_connections!, :clear_all_connections!,
+        :verify_active_connections!, :to => :connection_handler
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index af8cfbee7a..4b78d9f2e9 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -7,8 +7,8 @@ require 'active_record/connection_adapters/abstract/schema_definitions'
 require 'active_record/connection_adapters/abstract/schema_statements'
 require 'active_record/connection_adapters/abstract/database_statements'
 require 'active_record/connection_adapters/abstract/quoting'
-require 'active_record/connection_adapters/abstract/connection_specification'
 require 'active_record/connection_adapters/abstract/connection_pool'
+require 'active_record/connection_adapters/abstract/connection_specification'
 require 'active_record/connection_adapters/abstract/query_cache'
 
 module ActiveRecord
-- 
cgit v1.2.3


From 72d959d9b5255a449a554a1f011386d3c7a568cf Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Sat, 7 Jun 2008 16:30:56 -0500
Subject: Split connection handler into single- and multiple-thread versions.

---
 .../abstract/connection_pool.rb                    | 66 +++++++++++++---------
 .../abstract/connection_specification.rb           | 22 ++++++--
 2 files changed, 57 insertions(+), 31 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 01a75365d3..ca9f0a5d9b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -134,18 +134,17 @@ module ActiveRecord
         end
     end
 
-    class ConnectionHandler
-      attr_reader :connection_pools_lock
+    module ConnectionHandlerMethods
+      def initialize(pools = {})
+        @connection_pools = pools
+      end
 
-      def initialize
-        @connection_pools = {}
-        @connection_pools_lock = Monitor.new
+      def connection_pools
+        @connection_pools ||= {}
       end
 
       def establish_connection(name, spec)
-        connection_pools_lock.synchronize do
-          @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
-        end
+        @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
       end
 
       # for internal use only and for testing
@@ -168,7 +167,7 @@ module ActiveRecord
       end
 
       def clear_all_connections!
-        clear_cache!(@connection_pools) {|name, pool| pool.disconnect! }
+        @connection_pools.each_value {|pool| pool.disconnect! }
       end
 
       # Verify active connections.
@@ -185,15 +184,6 @@ module ActiveRecord
         (pool && pool.connection) or raise ConnectionNotEstablished
       end
 
-      def retrieve_connection_pool(klass)
-        loop do
-          pool = @connection_pools[klass.name]
-          return pool if pool
-          return nil if ActiveRecord::Base == klass
-          klass = klass.superclass
-        end
-      end
-
       # Returns true if a connection that's accessible to this class has already been opened.
       def connected?(klass)
         retrieve_connection_pool(klass).connected?
@@ -210,16 +200,40 @@ module ActiveRecord
         pool.spec.config if pool
       end
 
-      synchronize :retrieve_connection, :retrieve_connection_pool, :connected?,
-        :remove_connection, :active_connections, :clear_active_connections!,
-        :clear_reloadable_connections!, :clear_all_connections!,
-        :verify_active_connections!, :with => :connection_pools_lock
-
       private
-        def clear_cache!(cache, &block)
-          cache.each(&block) if block_given?
-          cache.clear
+        def retrieve_connection_pool(klass)
+          loop do
+            pool = @connection_pools[klass.name]
+            return pool if pool
+            return nil if ActiveRecord::Base == klass
+            klass = klass.superclass
+          end
         end
     end
+
+    # This connection handler is not thread-safe, as it does not protect access
+    # to the underlying connection pools.
+    class SingleThreadConnectionHandler
+      include ConnectionHandlerMethods
+    end
+
+    # This connection handler is thread-safe. Each access or modification of a thread
+    # pool is synchronized by an internal monitor.
+    class MultipleThreadConnectionHandler
+      attr_reader :connection_pools_lock
+      include ConnectionHandlerMethods
+
+      def initialize(pools = {})
+        super
+        @connection_pools_lock = Monitor.new
+      end
+
+      # Apply monitor to all public methods that access the pool.
+      synchronize :establish_connection, :retrieve_connection,
+        :connected?, :remove_connection, :active_connections,
+        :clear_active_connections!, :clear_reloadable_connections!,
+        :clear_all_connections!, :verify_active_connections!,
+        :with => :connection_pools_lock
+    end
   end
 end
\ No newline at end of file
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 2dc3201208..0ebb191381 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -14,12 +14,24 @@ module ActiveRecord
 
     # The connection handler
     cattr_accessor :connection_handler, :instance_writer => false
-    @@connection_handler = ConnectionAdapters::ConnectionHandler.new
+    @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new
 
-    # Turning on allow_concurrency basically switches a null mutex for a real one, so that
-    # multi-threaded access of the connection pools hash is synchronized.
+    # Turning on allow_concurrency changes the single threaded connection handler
+    # for a multiple threaded one, so that multi-threaded access of the
+    # connection pools is synchronized.
     def self.allow_concurrency=(flag)
-      @@allow_concurrency = flag
+      if @@allow_concurrency != flag
+        @@allow_concurrency = flag
+        # When switching connection handlers, preserve the existing pools so that
+        # #establish_connection doesn't need to be called again.
+        if @@allow_concurrency
+          self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new(
+            self.connection_handler.connection_pools)
+        else
+          self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new(
+            self.connection_handler.connection_pools)
+        end
+      end
     end
 
     # Returns the connection currently associated with the class. This can
@@ -112,7 +124,7 @@ module ActiveRecord
         connection_handler.connected?(self)
       end
 
-      def remove_connection(klass=self)
+      def remove_connection(klass = self)
         connection_handler.remove_connection(klass)
       end
 
-- 
cgit v1.2.3


From 029952edf464b94184d9b48f3bdff49d2746d721 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Mon, 4 Aug 2008 22:43:32 -0700
Subject: Extract a base class for connection pools, start to flesh out
 reserve/release API

---
 .../abstract/connection_pool.rb                    | 189 ++++++++++++---------
 1 file changed, 105 insertions(+), 84 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index ca9f0a5d9b..cebff0262d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -3,135 +3,156 @@ require 'set'
 
 module ActiveRecord
   module ConnectionAdapters
+    # Connection pool API for ActiveRecord database connections.
     class ConnectionPool
-      delegate :verification_timeout, :to => "::ActiveRecord::Base"
-      attr_reader :active_connections, :spec
+      # Factory method for connection pools.
+      # Determines pool type to use based on contents of connection specification.
+      # FIXME: specification configuration TBD.
+      def self.create(spec)
+        ConnectionPerThread.new(spec)
+      end
 
+      delegate :verification_timeout, :to => "::ActiveRecord::Base"
+      attr_reader :spec
       def initialize(spec)
-        # The thread id -> adapter cache.
-        @active_connections = {}
-
-        # The ConnectionSpecification for this pool
         @spec = spec
-
+        # The cache of reserved connections mapped to threads
+        @reserved_connections = {}
         # The mutex used to synchronize pool access
         @connection_mutex = Monitor.new
       end
 
-      def active_connection_name #:nodoc:
-        Thread.current.object_id
+      # Retrieve the connection reserved for the current thread, or call #reserve to obtain one
+      # if necessary.
+      def open_connection
+        if conn = @reserved_connections[active_connection_name]
+          conn.verify!(verification_timeout)
+          conn
+        else
+          @reserved_connections[active_connection_name] = reserve
+        end
       end
+      alias connection open_connection
 
-      def active_connection
-        active_connections[active_connection_name]
+      def close_connection
+        conn = @reserved_connections.delete(active_connection_name)
+        release conn if conn
       end
 
-      # Returns the connection currently associated with the class. This can
-      # also be used to "borrow" the connection to do database work unrelated
-      # to any of the specific Active Records.
-      def connection
-        if conn = active_connections[active_connection_name]
-          conn
-        else
-          # retrieve_connection sets the cache key.
-          conn = retrieve_connection
-          active_connections[active_connection_name] = conn
-        end
+      # Returns true if a connection has already been opened.
+      def connected?
+        !connections.empty?
       end
 
-      # Clears the cache which maps classes to connections.
-      def clear_active_connections!
-        clear_entries!(@active_connections, [active_connection_name]) do |name, conn|
+      # Reserve (check-out) a database connection for the current thread.
+      def reserve
+        raise NotImplementedError, "reserve is an abstract method"
+      end
+      alias checkout reserve
+
+      # Release (check-in) a database connection for the current thread.
+      def release(connection)
+        raise NotImplementedError, "release is an abstract method"
+      end
+      alias checkin release
+
+      # Disconnect all connections in the pool.
+      def disconnect!
+        @reserved_connections.each do |name,conn|
+          release(conn)
+        end
+        connections.each do |conn|
           conn.disconnect!
         end
+        @reserved_connections = {}
       end
 
       # Clears the cache which maps classes
       def clear_reloadable_connections!
-        @active_connections.each do |name, conn|
+        @reserved_connections.each do |name, conn|
+          release(conn)
+        end
+        @reserved_connections = {}
+        connections.each do |conn|
           if conn.requires_reloading?
             conn.disconnect!
-            @active_connections.delete(name)
+            remove_connection conn
           end
         end
       end
 
       # Verify active connections.
       def verify_active_connections! #:nodoc:
-        remove_stale_cached_threads!(@active_connections) do |name, conn|
-          conn.disconnect!
+        remove_stale_cached_threads!(@reserved_connections) do |name, conn|
+          release(conn)
         end
-        active_connections.each_value do |connection|
+        connections.each do |connection|
           connection.verify!(verification_timeout)
         end
       end
 
-      def retrieve_connection #:nodoc:
-        # Name is nil if establish_connection hasn't been called for
-        # some class along the inheritance chain up to AR::Base yet.
-        name = active_connection_name
-        if conn = active_connections[name]
-          # Verify the connection.
-          conn.verify!(verification_timeout)
-        else
-          self.set_connection spec
-          conn = active_connections[name]
-        end
+      synchronize :open_connection, :close_connection, :reserve, :release,
+        :clear_reloadable_connections!, :verify_active_connections!,
+        :connected?, :disconnect!, :with => :@connection_mutex
 
-        conn or raise ConnectionNotEstablished
+      private
+      def active_connection_name #:nodoc:
+        Thread.current.object_id
       end
 
-      # Returns true if a connection that's accessible to this class has already been opened.
-      def connected?
-        active_connections[active_connection_name] ? true : false
+      def remove_connection(conn)
+        raise NotImplementedError, "remove_connection is an abstract method"
       end
 
-      # Disconnect all connections in the pool.
-      def disconnect!
-        clear_cache!(@active_connections) do |name, conn|
-          conn.disconnect!
-        end
+      # Array containing all connections (reserved or available) in the pool.
+      def connections
+        raise NotImplementedError, "connections is an abstract method"
       end
 
-      # Set the connection for the class.
-      def set_connection(spec) #:nodoc:
-        if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter)
-          active_connections[active_connection_name] = spec
-        elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification)
-          config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
-          self.set_connection ActiveRecord::Base.send(spec.adapter_method, config)
-        else
-          raise ConnectionNotEstablished
+      # Remove stale threads from the cache.
+      def remove_stale_cached_threads!(cache, &block)
+        keys = Set.new(cache.keys)
+
+        Thread.list.each do |thread|
+          keys.delete(thread.object_id) if thread.alive?
+        end
+        keys.each do |key|
+          next unless cache.has_key?(key)
+          block.call(key, cache[key])
+          cache.delete(key)
         end
       end
+    end
+
+    class ConnectionPerThread < ConnectionPool
+      def active_connection
+        @reserved_connections[active_connection_name]
+      end
 
-      synchronize :active_connection, :connection, :clear_active_connections!,
-        :clear_reloadable_connections!, :verify_active_connections!, :retrieve_connection,
-        :connected?, :disconnect!, :set_connection, :with => :@connection_mutex
+      def active_connections; @reserved_connections; end
 
-      private
-        def clear_cache!(cache, &block)
-          cache.each(&block) if block_given?
-          cache.clear
-        end
+      def reserve
+        new_connection
+      end
 
-        # Remove stale threads from the cache.
-        def remove_stale_cached_threads!(cache, &block)
-          stale = Set.new(cache.keys)
+      def release(conn)
+        conn.disconnect!
+      end
 
-          Thread.list.each do |thread|
-            stale.delete(thread.object_id) if thread.alive?
-          end
-          clear_entries!(cache, stale, &block)
-        end
+      private
+      # Set the connection for the class.
+      def new_connection
+        config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
+        ActiveRecord::Base.send(spec.adapter_method, config)
+      end
 
-        def clear_entries!(cache, keys, &block)
-          keys.each do |key|
-            next unless cache.has_key?(key)
-            block.call(key, cache[key])
-            cache.delete(key)
-          end
-        end
+      def connections
+        @reserved_connections.values
+      end
+
+      def remove_connection(conn)
+        @reserved_connections.delete_if {|k,v| v == conn}
+      end
     end
 
     module ConnectionHandlerMethods
@@ -144,7 +165,7 @@ module ActiveRecord
       end
 
       def establish_connection(name, spec)
-        @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
+        @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
       end
 
       # for internal use only and for testing
@@ -158,7 +179,7 @@ module ActiveRecord
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        @connection_pools.each_value {|pool| pool.clear_active_connections! }
+        @connection_pools.each_value {|pool| pool.close_connection }
       end
 
       # Clears the cache which maps classes
-- 
cgit v1.2.3


From 82fcd9d85fe245e8041f8d375175dde13688fce4 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Wed, 6 Aug 2008 22:54:06 -0700
Subject: Clean up the code, get rid of reserve/release, add some more docs

---
 .../abstract/connection_pool.rb                    | 85 +++++++++++++---------
 1 file changed, 51 insertions(+), 34 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index cebff0262d..28446fd64d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -3,7 +3,7 @@ require 'set'
 
 module ActiveRecord
   module ConnectionAdapters
-    # Connection pool API for ActiveRecord database connections.
+    # Connection pool base class and ActiveRecord database connections.
     class ConnectionPool
       # Factory method for connection pools.
       # Determines pool type to use based on contents of connection specification.
@@ -14,6 +14,7 @@ module ActiveRecord
 
       delegate :verification_timeout, :to => "::ActiveRecord::Base"
       attr_reader :spec
+
       def initialize(spec)
         @spec = spec
         # The cache of reserved connections mapped to threads
@@ -22,21 +23,35 @@ module ActiveRecord
         @connection_mutex = Monitor.new
       end
 
-      # Retrieve the connection reserved for the current thread, or call #reserve to obtain one
-      # if necessary.
-      def open_connection
+      # Retrieve the connection associated with the current thread, or call
+      # #checkout to obtain one if necessary.
+      #
+      # #connection can be called any number of times; the connection is
+      # held in a hash keyed by the thread id.
+      def connection
         if conn = @reserved_connections[active_connection_name]
           conn.verify!(verification_timeout)
           conn
         else
-          @reserved_connections[active_connection_name] = reserve
+          @reserved_connections[active_connection_name] = checkout
         end
       end
-      alias connection open_connection
 
-      def close_connection
+      # Signal that the thread is finished with the current connection.
+      # #release_thread_connection releases the connection-thread association
+      # and returns the connection to the pool.
+      def release_thread_connection
         conn = @reserved_connections.delete(active_connection_name)
-        release conn if conn
+        checkin conn if conn
+      end
+
+      # Reserve a connection, and yield it to a block. Ensure the connection is
+      # checked back in when finished.
+      def with_connection
+        conn = checkout
+        yield conn
+      ensure
+        checkin conn
       end
 
       # Returns true if a connection has already been opened.
@@ -44,22 +59,10 @@ module ActiveRecord
         !connections.empty?
       end
 
-      # Reserve (check-out) a database connection for the current thread.
-      def reserve
-        raise NotImplementedError, "reserve is an abstract method"
-      end
-      alias checkout reserve
-
-      # Release (check-in) a database connection for the current thread.
-      def release(connection)
-        raise NotImplementedError, "release is an abstract method"
-      end
-      alias checkin release
-
       # Disconnect all connections in the pool.
       def disconnect!
         @reserved_connections.each do |name,conn|
-          release(conn)
+          checkin conn
         end
         connections.each do |conn|
           conn.disconnect!
@@ -70,7 +73,7 @@ module ActiveRecord
       # Clears the cache which maps classes
       def clear_reloadable_connections!
         @reserved_connections.each do |name, conn|
-          release(conn)
+          checkin conn
         end
         @reserved_connections = {}
         connections.each do |conn|
@@ -84,30 +87,42 @@ module ActiveRecord
       # Verify active connections.
       def verify_active_connections! #:nodoc:
         remove_stale_cached_threads!(@reserved_connections) do |name, conn|
-          release(conn)
+          checkin conn
         end
         connections.each do |connection|
           connection.verify!(verification_timeout)
         end
       end
 
-      synchronize :open_connection, :close_connection, :reserve, :release,
-        :clear_reloadable_connections!, :verify_active_connections!,
-        :connected?, :disconnect!, :with => :@connection_mutex
+      # Check-out a database connection from the pool.
+      def checkout
+        raise NotImplementedError, "checkout is an abstract method"
+      end
 
-      private
-      def active_connection_name #:nodoc:
-        Thread.current.object_id
+      # Check-in a database connection back into the pool.
+      def checkin(connection)
+        raise NotImplementedError, "checkin is an abstract method"
       end
 
       def remove_connection(conn)
         raise NotImplementedError, "remove_connection is an abstract method"
       end
+      private :remove_connection
 
       # Array containing all connections (reserved or available) in the pool.
       def connections
         raise NotImplementedError, "connections is an abstract method"
       end
+      private :connections
+
+      synchronize :connection, :release_thread_connection, :checkout, :checkin,
+        :clear_reloadable_connections!, :verify_active_connections!,
+        :connected?, :disconnect!, :with => :@connection_mutex
+
+      private
+      def active_connection_name #:nodoc:
+        Thread.current.object_id
+      end
 
       # Remove stale threads from the cache.
       def remove_stale_cached_threads!(cache, &block)
@@ -131,11 +146,11 @@ module ActiveRecord
 
       def active_connections; @reserved_connections; end
 
-      def reserve
+      def checkout
         new_connection
       end
 
-      def release(conn)
+      def checkin(conn)
         conn.disconnect!
       end
 
@@ -168,7 +183,8 @@ module ActiveRecord
         @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
       end
 
-      # for internal use only and for testing
+      # for internal use only and for testing;
+      # only works with ConnectionPerThread pool class
       def active_connections #:nodoc:
         @connection_pools.inject({}) do |hash,kv|
           hash[kv.first] = kv.last.active_connection
@@ -179,7 +195,7 @@ module ActiveRecord
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        @connection_pools.each_value {|pool| pool.close_connection }
+        @connection_pools.each_value {|pool| pool.release_thread_connection }
       end
 
       # Clears the cache which maps classes
@@ -205,7 +221,8 @@ module ActiveRecord
         (pool && pool.connection) or raise ConnectionNotEstablished
       end
 
-      # Returns true if a connection that's accessible to this class has already been opened.
+      # Returns true if a connection that's accessible to this class has
+      # already been opened.
       def connected?(klass)
         retrieve_connection_pool(klass).connected?
       end
-- 
cgit v1.2.3


From fe575dd4a9f0fa0e71a89fae9f4a951a9fb36058 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Thu, 7 Aug 2008 01:27:40 -0700
Subject: Nearing the finish line. Initial fixed-size connection pool
 implemented, more docs

---
 .../abstract/connection_pool.rb                    | 138 +++++++++++++++++----
 .../abstract/connection_specification.rb           |   4 +
 .../test/cases/threaded_connections_test.rb        |  36 ++++++
 3 files changed, 156 insertions(+), 22 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 28446fd64d..97c6e3e886 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -2,14 +2,43 @@ require 'monitor'
 require 'set'
 
 module ActiveRecord
+  # Raised when a connection could not be obtained within the connection
+  # acquisition timeout period.
+  class ConnectionTimeoutError < ConnectionNotEstablished
+  end
+
   module ConnectionAdapters
-    # Connection pool base class and ActiveRecord database connections.
-    class ConnectionPool
+    # Connection pool base class for managing ActiveRecord database
+    # connections.
+    #
+    # Connections can be obtained and used from a connection pool in several
+    # ways:
+    #
+    # 1. Simply use ActiveRecord::Base.connection as in pre-connection-pooled
+    #    ActiveRecord. Eventually, when you're done with the connection and
+    #    wish it to be returned to the pool, you call
+    #    ActiveRecord::Base.connection_pool.release_thread_connection.
+    # 2. Manually check out a connection from the pool with
+    #    ActiveRecord::Base.connection_pool.checkout. You are responsible for
+    #    returning this connection to the pool when finished by calling
+    #    ActiveRecord::Base.connection_pool.checkin(connection).
+    # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which
+    #    obtains a connection, yields it as the sole argument to the block,
+    #    and returns it to the pool after the block completes.
+    class AbstractConnectionPool
       # Factory method for connection pools.
-      # Determines pool type to use based on contents of connection specification.
-      # FIXME: specification configuration TBD.
+      # Determines pool type to use based on contents of connection
+      # specification. Additional options for connection specification:
+      #
+      # * +pool+: number indicating size of fixed connection pool to use
+      # * +wait_timeout+ (optional): number of seconds to block and wait
+      #   for a connection before giving up and raising a timeout error.
       def self.create(spec)
-        ConnectionPerThread.new(spec)
+        if spec.config[:pool] && spec.config[:pool].to_i > 0
+          ConnectionPool.new(spec)
+        else
+          ConnectionPerThread.new(spec)
+        end
       end
 
       delegate :verification_timeout, :to => "::ActiveRecord::Base"
@@ -115,11 +144,16 @@ module ActiveRecord
       end
       private :connections
 
-      synchronize :connection, :release_thread_connection, :checkout, :checkin,
+      synchronize :connection, :release_thread_connection,
         :clear_reloadable_connections!, :verify_active_connections!,
         :connected?, :disconnect!, :with => :@connection_mutex
 
       private
+      def new_connection
+        config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
+        ActiveRecord::Base.send(spec.adapter_method, config)
+      end
+
       def active_connection_name #:nodoc:
         Thread.current.object_id
       end
@@ -139,7 +173,10 @@ module ActiveRecord
       end
     end
 
-    class ConnectionPerThread < ConnectionPool
+    # ConnectionPerThread is a simple implementation: always create/disconnect
+    # on checkout/checkin, and use the base class reserved connections hash to
+    # manage the per-thread connections.
+    class ConnectionPerThread < AbstractConnectionPool
       def active_connection
         @reserved_connections[active_connection_name]
       end
@@ -155,12 +192,6 @@ module ActiveRecord
       end
 
       private
-      # Set the connection for the class.
-      def new_connection
-        config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
-        ActiveRecord::Base.send(spec.adapter_method, config)
-      end
-
       def connections
         @reserved_connections.values
       end
@@ -170,6 +201,70 @@ module ActiveRecord
       end
     end
 
+    # ConnectionPool provides a full, fixed-size connection pool with timed
+    # waits when the pool is exhausted.
+    class ConnectionPool < AbstractConnectionPool
+      def initialize(spec)
+        super
+        # default 5 second timeout
+        @timeout = spec.config[:wait_timeout] || 5
+        @size = spec.config[:pool].to_i
+        @queue = @connection_mutex.new_cond
+        @connections = []
+        @checked_out = []
+      end
+
+      def checkout
+        # Checkout an available connection
+        conn = @connection_mutex.synchronize do
+          if @connections.length < @size
+            checkout_new_connection
+          elsif @checked_out.size < @connections.size
+            checkout_existing_connection
+          end
+        end
+        return conn if conn
+
+        # No connections available; wait for one
+        @connection_mutex.synchronize do
+          if @queue.wait(@timeout)
+            checkout_existing_connection
+          else
+            raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion"
+          end
+        end
+      end
+
+      def checkin(conn)
+        @connection_mutex.synchronize do
+          @checked_out -= conn
+          @queue.signal
+        end
+      end
+
+      private
+      def checkout_new_connection
+        c = new_connection
+        @connections << c
+        @checked_out << c
+        c
+      end
+
+      def checkout_existing_connection
+        c = [@connections - @checked_out].first
+        @checked_out << c
+        c
+      end
+
+      def connections
+        @connections
+      end
+
+      def remove_connection(conn)
+        @connections.delete conn
+      end
+    end
+
     module ConnectionHandlerMethods
       def initialize(pools = {})
         @connection_pools = pools
@@ -180,7 +275,7 @@ module ActiveRecord
       end
 
       def establish_connection(name, spec)
-        @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
+        @connection_pools[name] = ConnectionAdapters::AbstractConnectionPool.create(spec)
       end
 
       # for internal use only and for testing;
@@ -238,15 +333,14 @@ module ActiveRecord
         pool.spec.config if pool
       end
 
-      private
-        def retrieve_connection_pool(klass)
-          loop do
-            pool = @connection_pools[klass.name]
-            return pool if pool
-            return nil if ActiveRecord::Base == klass
-            klass = klass.superclass
-          end
+      def retrieve_connection_pool(klass)
+        loop do
+          pool = @connection_pools[klass.name]
+          return pool if pool
+          return nil if ActiveRecord::Base == klass
+          klass = klass.superclass
         end
+      end
     end
 
     # This connection handler is not thread-safe, as it does not protect access
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 0ebb191381..0910db1951 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -116,6 +116,10 @@ module ActiveRecord
         retrieve_connection
       end
 
+      def connection_pool
+        connection_handler.retrieve_connection_pool(self)
+      end
+
       def retrieve_connection
         connection_handler.retrieve_connection(self)
       end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 3f88f79189..3abe9aea56 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -40,4 +40,40 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
       assert_equal @connections.length, 5
     end
   end
+
+  class PooledConnectionsTest < ActiveRecord::TestCase
+    def setup
+      @connection = ActiveRecord::Base.remove_connection
+      @connections = []
+      @allow_concurrency = ActiveRecord::Base.allow_concurrency
+      ActiveRecord::Base.allow_concurrency = true
+    end
+
+    def teardown
+      ActiveRecord::Base.clear_all_connections!
+      ActiveRecord::Base.allow_concurrency = @allow_concurrency
+      ActiveRecord::Base.establish_connection(@connection)
+    end
+
+    def gather_connections
+      ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
+      @timed_out = 0
+
+      4.times do
+        Thread.new do
+          begin
+            @connections << ActiveRecord::Base.connection_pool.checkout
+          rescue ActiveRecord::ConnectionTimeoutError
+            @timed_out += 1
+          end
+        end.join
+      end
+    end
+
+    def test_threaded_connections
+      gather_connections
+      assert_equal @connections.length, 2
+      assert_equal @timed_out, 2
+    end
+  end
 end
-- 
cgit v1.2.3


From 3ce64d4f1608330072e1959a10f9b84205baebfa Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Thu, 7 Aug 2008 08:33:42 -0700
Subject: Fix checkin method, add a couple more tests

---
 .../abstract/connection_pool.rb                    |  2 +-
 .../test/cases/threaded_connections_test.rb        | 38 +++++++++++++++++++---
 2 files changed, 35 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 97c6e3e886..3d727bd0e6 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -237,7 +237,7 @@ module ActiveRecord
 
       def checkin(conn)
         @connection_mutex.synchronize do
-          @checked_out -= conn
+          @checked_out.delete conn
           @queue.signal
         end
       end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 3abe9aea56..a9d82037bc 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -44,7 +44,6 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
   class PooledConnectionsTest < ActiveRecord::TestCase
     def setup
       @connection = ActiveRecord::Base.remove_connection
-      @connections = []
       @allow_concurrency = ActiveRecord::Base.allow_concurrency
       ActiveRecord::Base.allow_concurrency = true
     end
@@ -55,8 +54,9 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
       ActiveRecord::Base.establish_connection(@connection)
     end
 
-    def gather_connections
+    def checkout_connections
       ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
+      @connections = []
       @timed_out = 0
 
       4.times do
@@ -70,10 +70,40 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
       end
     end
 
-    def test_threaded_connections
-      gather_connections
+    def test_pooled_connection_checkout
+      checkout_connections
       assert_equal @connections.length, 2
       assert_equal @timed_out, 2
     end
+
+    def checkout_checkin_connections(pool_size, threads)
+      ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
+      @connection_count = 0
+      @timed_out = 0
+      threads.times do
+        Thread.new do
+          begin
+            conn = ActiveRecord::Base.connection_pool.checkout
+            sleep 0.1
+            ActiveRecord::Base.connection_pool.checkin conn
+            @connection_count += 1
+          rescue ActiveRecord::ConnectionTimeoutError
+            @timed_out += 1
+          end
+        end.join
+      end
+    end
+
+    def test_pooled_connection_checkin_one
+      checkout_checkin_connections 1, 2
+      assert_equal 2, @connection_count
+      assert_equal 0, @timed_out
+    end
+
+    def test_pooled_connection_checkin_two
+      checkout_checkin_connections 2, 3
+      assert_equal 3, @connection_count
+      assert_equal 0, @timed_out
+    end
   end
 end
-- 
cgit v1.2.3


From 817a07b45105f7043846973525a9edc44028c0d4 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Thu, 7 Aug 2008 22:24:42 -0700
Subject: More doco and class/method renames. Now have a strategy for
 integration with ActionPack.

---
 .../abstract/connection_pool.rb                    | 63 +++++++++++++---------
 1 file changed, 39 insertions(+), 24 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 3d727bd0e6..50b2badbe6 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -14,10 +14,12 @@ module ActiveRecord
     # Connections can be obtained and used from a connection pool in several
     # ways:
     #
-    # 1. Simply use ActiveRecord::Base.connection as in pre-connection-pooled
-    #    ActiveRecord. Eventually, when you're done with the connection and
-    #    wish it to be returned to the pool, you call
-    #    ActiveRecord::Base.connection_pool.release_thread_connection.
+    # 1. Simply use ActiveRecord::Base.connection as with ActiveRecord 2.1 and
+    #    earlier (pre-connection-pooling). Eventually, when you're done with
+    #    the connection(s) and wish it to be returned to the pool, you call
+    #    ActiveRecord::Base.clear_active_connections!. This will be the
+    #    default behavior for ActiveRecord when used in conjunction with
+    #    ActionPack's request handling cycle.
     # 2. Manually check out a connection from the pool with
     #    ActiveRecord::Base.connection_pool.checkout. You are responsible for
     #    returning this connection to the pool when finished by calling
@@ -25,7 +27,7 @@ module ActiveRecord
     # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which
     #    obtains a connection, yields it as the sole argument to the block,
     #    and returns it to the pool after the block completes.
-    class AbstractConnectionPool
+    class ConnectionPool
       # Factory method for connection pools.
       # Determines pool type to use based on contents of connection
       # specification. Additional options for connection specification:
@@ -35,9 +37,11 @@ module ActiveRecord
       #   for a connection before giving up and raising a timeout error.
       def self.create(spec)
         if spec.config[:pool] && spec.config[:pool].to_i > 0
-          ConnectionPool.new(spec)
+          FixedSizeConnectionPool.new(spec)
+        elsif spec.config[:jndi] # JRuby appserver datasource pool; passthrough
+          NewConnectionEveryTime.new(spec)
         else
-          ConnectionPerThread.new(spec)
+          CachedConnectionPerThread.new(spec)
         end
       end
 
@@ -67,9 +71,9 @@ module ActiveRecord
       end
 
       # Signal that the thread is finished with the current connection.
-      # #release_thread_connection releases the connection-thread association
+      # #release_connection releases the connection-thread association
       # and returns the connection to the pool.
-      def release_thread_connection
+      def release_connection
         conn = @reserved_connections.delete(active_connection_name)
         checkin conn if conn
       end
@@ -113,7 +117,8 @@ module ActiveRecord
         end
       end
 
-      # Verify active connections.
+      # Verify active connections and remove and disconnect connections
+      # associated with stale threads.
       def verify_active_connections! #:nodoc:
         remove_stale_cached_threads!(@reserved_connections) do |name, conn|
           checkin conn
@@ -133,18 +138,17 @@ module ActiveRecord
         raise NotImplementedError, "checkin is an abstract method"
       end
 
-      def remove_connection(conn)
+      def remove_connection(conn) #:nodoc:
         raise NotImplementedError, "remove_connection is an abstract method"
       end
       private :remove_connection
 
-      # Array containing all connections (reserved or available) in the pool.
-      def connections
+      def connections #:nodoc:
         raise NotImplementedError, "connections is an abstract method"
       end
       private :connections
 
-      synchronize :connection, :release_thread_connection,
+      synchronize :connection, :release_connection,
         :clear_reloadable_connections!, :verify_active_connections!,
         :connected?, :disconnect!, :with => :@connection_mutex
 
@@ -173,10 +177,9 @@ module ActiveRecord
       end
     end
 
-    # ConnectionPerThread is a simple implementation: always create/disconnect
-    # on checkout/checkin, and use the base class reserved connections hash to
-    # manage the per-thread connections.
-    class ConnectionPerThread < AbstractConnectionPool
+    # NewConnectionEveryTime is a simple implementation: always
+    # create/disconnect on checkout/checkin.
+    class NewConnectionEveryTime < ConnectionPool
       def active_connection
         @reserved_connections[active_connection_name]
       end
@@ -201,9 +204,21 @@ module ActiveRecord
       end
     end
 
-    # ConnectionPool provides a full, fixed-size connection pool with timed
-    # waits when the pool is exhausted.
-    class ConnectionPool < AbstractConnectionPool
+    # CachedConnectionPerThread is a compatible pseudo-connection pool that
+    # caches connections per-thread. In order to hold onto threads in the same
+    # manner as ActiveRecord 2.1 and earlier, it only disconnects the
+    # connection when the connection is checked in, or when calling
+    # ActiveRecord::Base.clear_all_connections!, and not during
+    # #release_connection.
+    class CachedConnectionPerThread < NewConnectionEveryTime
+      def release_connection
+        # no-op; keep the connection
+      end
+    end
+
+    # FixedSizeConnectionPool provides a full, fixed-size connection pool with
+    # timed waits when the pool is exhausted.
+    class FixedSizeConnectionPool < ConnectionPool
       def initialize(spec)
         super
         # default 5 second timeout
@@ -275,7 +290,7 @@ module ActiveRecord
       end
 
       def establish_connection(name, spec)
-        @connection_pools[name] = ConnectionAdapters::AbstractConnectionPool.create(spec)
+        @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
       end
 
       # for internal use only and for testing;
@@ -290,7 +305,7 @@ module ActiveRecord
 
       # Clears the cache which maps classes to connections.
       def clear_active_connections!
-        @connection_pools.each_value {|pool| pool.release_thread_connection }
+        @connection_pools.each_value {|pool| pool.release_connection }
       end
 
       # Clears the cache which maps classes
@@ -304,7 +319,7 @@ module ActiveRecord
 
       # Verify active connections.
       def verify_active_connections! #:nodoc:
-        @connection_pools.each_value {|pool| pool.verify_active_connections!}
+        @connection_pools.each_value {|pool| pool.verify_active_connections! }
       end
 
       # Locate the connection of the nearest super class. This can be an
-- 
cgit v1.2.3


From 1712e37c90d0ac74b21589c0ee7b0365cb2b7beb Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Thu, 7 Aug 2008 23:13:21 -0700
Subject: Favor existing connections over new ones if available

---
 .../active_record/connection_adapters/abstract/connection_pool.rb   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 50b2badbe6..1d7efc89e1 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -232,10 +232,10 @@ module ActiveRecord
       def checkout
         # Checkout an available connection
         conn = @connection_mutex.synchronize do
-          if @connections.length < @size
-            checkout_new_connection
-          elsif @checked_out.size < @connections.size
+          if @checked_out.size < @connections.size
             checkout_existing_connection
+          elsif @connections.size < @size
+            checkout_new_connection
           end
         end
         return conn if conn
-- 
cgit v1.2.3


From d7d2d73d88e465cb0c618283862179accd8932e1 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 8 Aug 2008 00:45:16 -0700
Subject: Fix typo: was using brackets instead of parens. Must need more sleep.

---
 .../connection_adapters/abstract/connection_pool.rb            |  2 +-
 activerecord/test/cases/threaded_connections_test.rb           | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 1d7efc89e1..988f85e909 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -266,7 +266,7 @@ module ActiveRecord
       end
 
       def checkout_existing_connection
-        c = [@connections - @checked_out].first
+        c = (@connections - @checked_out).first
         @checked_out << c
         c
       end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index a9d82037bc..892a99f85a 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -105,5 +105,15 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
       assert_equal 3, @connection_count
       assert_equal 0, @timed_out
     end
+
+    def test_pooled_connection_checkout_existing_first
+      ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1}))
+      conn_pool = ActiveRecord::Base.connection_pool
+      conn = conn_pool.checkout
+      conn_pool.checkin(conn)
+      conn = conn_pool.checkout
+      assert ActiveRecord::ConnectionAdapters::AbstractAdapter === conn
+      conn_pool.checkin(conn)
+    end
   end
 end
-- 
cgit v1.2.3


From a96b7d4c33757364a19ed1fc34f0a89801b8b2d7 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 22 Aug 2008 11:43:20 -0500
Subject: Add connection reset and verification upon each connection checkout

---
 .../connection_adapters/abstract/connection_pool.rb         |  9 +++++++--
 .../active_record/connection_adapters/abstract_adapter.rb   | 13 +++++++++++--
 .../lib/active_record/connection_adapters/mysql_adapter.rb  |  9 +++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 988f85e909..365c80fe1d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -261,12 +261,17 @@ module ActiveRecord
       def checkout_new_connection
         c = new_connection
         @connections << c
-        @checked_out << c
-        c
+        checkout_and_verify(c)
       end
 
       def checkout_existing_connection
         c = (@connections - @checked_out).first
+        checkout_and_verify(c)
+      end
+
+      def checkout_and_verify(c)
+        c.reset!
+        c.verify!(verification_timeout)
         @checked_out << c
         c
       end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 4b78d9f2e9..c37ebf1410 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -103,14 +103,23 @@ module ActiveRecord
         @active = false
       end
 
+      # Reset the state of this connection, directing the DBMS to clear
+      # transactions and other connection-related server-side state. Usually a
+      # database-dependent operation; the default method simply executes a
+      # ROLLBACK and swallows any exceptions which is probably not enough to
+      # ensure the connection is clean.
+      def reset!
+        execute "ROLLBACK" rescue nil
+      end
+
       # Returns true if its safe to reload the connection between requests for development mode.
       # This is not the case for Ruby/MySQL and it's not necessary for any adapters except SQLite.
       def requires_reloading?
         false
       end
 
-      # Lazily verify this connection, calling <tt>active?</tt> only if it hasn't
-      # been called for +timeout+ seconds.
+      # Lazily verify this connection, calling <tt>active?</tt> only if it
+      # hasn't been called for +timeout+ seconds.
       def verify!(timeout)
         now = Time.now.to_i
         if (now - @last_verification) > timeout
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 204ebaa2e2..0a935a1b7a 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -280,6 +280,15 @@ module ActiveRecord
         @connection.close rescue nil
       end
 
+      def reset!
+        if @connection.respond_to?(:change_user)
+          # See http://bugs.mysql.com/bug.php?id=33540 -- the workaround way to
+          # reset the connection is to change the user to the same user.
+          @connection.change_user(@config[:username], @config[:password], @config[:database])
+        else
+          super
+        end
+      end
 
       # DATABASE STATEMENTS ======================================
 
-- 
cgit v1.2.3


From ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 22 Aug 2008 12:19:29 -0500
Subject: Deprecate allow_concurrency and make it have no effect

---
 activerecord/lib/active_record/base.rb             | 20 +----------
 .../abstract/connection_pool.rb                    | 25 +++----------
 .../abstract/connection_specification.rb           | 30 ++++++----------
 .../connection_adapters/abstract_adapter.rb        |  4 ++-
 activerecord/test/cases/base_test.rb               |  2 +-
 activerecord/test/cases/locking_test.rb            |  7 ----
 .../test/cases/threaded_connections_test.rb        | 42 ++++------------------
 activerecord/test/cases/transactions_test.rb       | 11 ------
 8 files changed, 27 insertions(+), 114 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b5ffc471bc..bc6d61301f 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -452,13 +452,6 @@ module ActiveRecord #:nodoc:
     cattr_accessor :default_timezone, :instance_writer => false
     @@default_timezone = :local
 
-    # Determines whether to use a connection for each thread, or a single shared connection for all threads.
-    # Defaults to false. If you're writing a threaded application, set to true
-    # and periodically call verify_active_connections! to clear out connections
-    # assigned to stale threads.
-    cattr_accessor :allow_concurrency, :instance_writer => false
-    @@allow_concurrency = false
-
     # Specifies the format to use when dumping the database schema with Rails'
     # Rakefile.  If :sql, the schema is dumped as (potentially database-
     # specific) SQL statements.  If :ruby, the schema is dumped as an
@@ -1943,22 +1936,11 @@ module ActiveRecord #:nodoc:
           end
         end
 
-        def thread_safe_scoped_methods #:nodoc:
+        def scoped_methods #:nodoc:
           scoped_methods = (Thread.current[:scoped_methods] ||= {})
           scoped_methods[self] ||= []
         end
 
-        def single_threaded_scoped_methods #:nodoc:
-          @scoped_methods ||= []
-        end
-
-        # pick up the correct scoped_methods version from @@allow_concurrency
-        if @@allow_concurrency
-          alias_method :scoped_methods, :thread_safe_scoped_methods
-        else
-          alias_method :scoped_methods, :single_threaded_scoped_methods
-        end
-
         def current_scoped_methods #:nodoc:
           scoped_methods.last
         end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 365c80fe1d..04c8361c64 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -154,7 +154,7 @@ module ActiveRecord
 
       private
       def new_connection
-        config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency)
+        config = spec.config.reverse_merge(:allow_concurrency => true)
         ActiveRecord::Base.send(spec.adapter_method, config)
       end
 
@@ -285,9 +285,12 @@ module ActiveRecord
       end
     end
 
-    module ConnectionHandlerMethods
+    class ConnectionHandler
+      attr_reader :connection_pools_lock
+
       def initialize(pools = {})
         @connection_pools = pools
+        @connection_pools_lock = Monitor.new
       end
 
       def connection_pools
@@ -361,24 +364,6 @@ module ActiveRecord
           klass = klass.superclass
         end
       end
-    end
-
-    # This connection handler is not thread-safe, as it does not protect access
-    # to the underlying connection pools.
-    class SingleThreadConnectionHandler
-      include ConnectionHandlerMethods
-    end
-
-    # This connection handler is thread-safe. Each access or modification of a thread
-    # pool is synchronized by an internal monitor.
-    class MultipleThreadConnectionHandler
-      attr_reader :connection_pools_lock
-      include ConnectionHandlerMethods
-
-      def initialize(pools = {})
-        super
-        @connection_pools_lock = Monitor.new
-      end
 
       # Apply monitor to all public methods that access the pool.
       synchronize :establish_connection, :retrieve_connection,
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 0910db1951..47fc11a620 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -14,25 +14,7 @@ module ActiveRecord
 
     # The connection handler
     cattr_accessor :connection_handler, :instance_writer => false
-    @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new
-
-    # Turning on allow_concurrency changes the single threaded connection handler
-    # for a multiple threaded one, so that multi-threaded access of the
-    # connection pools is synchronized.
-    def self.allow_concurrency=(flag)
-      if @@allow_concurrency != flag
-        @@allow_concurrency = flag
-        # When switching connection handlers, preserve the existing pools so that
-        # #establish_connection doesn't need to be called again.
-        if @@allow_concurrency
-          self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new(
-            self.connection_handler.connection_pools)
-        else
-          self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new(
-            self.connection_handler.connection_pools)
-        end
-      end
-    end
+    @@connection_handler = ConnectionAdapters::ConnectionHandler.new
 
     # Returns the connection currently associated with the class. This can
     # also be used to "borrow" the connection to do database work that isn't
@@ -109,6 +91,16 @@ module ActiveRecord
     end
 
     class << self
+      # Deprecated and no longer has any effect.
+      def allow_concurrency
+        ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency has been deprecated and no longer has any effect. Please remove all references to allow_concurrency.")
+      end
+
+      # Deprecated and no longer has any effect.
+      def allow_concurrency=(flag)
+        ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency= has been deprecated and no longer has any effect. Please remove all references to allow_concurrency=.")
+      end
+
       # Returns the connection currently associated with the class. This can
       # also be used to "borrow" the connection to do database work unrelated
       # to any of the specific Active Records.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index c37ebf1410..7ef8834547 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -109,7 +109,9 @@ module ActiveRecord
       # ROLLBACK and swallows any exceptions which is probably not enough to
       # ensure the connection is clean.
       def reset!
-        execute "ROLLBACK" rescue nil
+        silence_stderr do # postgres prints on stderr when you do this w/o a txn
+          execute "ROLLBACK" rescue nil
+        end
       end
 
       # Returns true if its safe to reload the connection between requests for development mode.
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index c8111358e3..ac9081e003 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -880,7 +880,7 @@ class BasicsTest < ActiveRecord::TestCase
 
   def test_mass_assignment_protection_against_class_attribute_writers
     [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names, :colorize_logging,
-      :default_timezone, :allow_concurrency, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method|
+      :default_timezone, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method|
       assert  Task.respond_to?(method)
       assert  Task.respond_to?("#{method}=")
       assert  Task.new.respond_to?(method)
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 701187223f..bbe8582466 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -210,13 +210,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter)
     def setup
       # Avoid introspection queries during tests.
       Person.columns; Reader.columns
-
-      @allow_concurrency = ActiveRecord::Base.allow_concurrency
-      ActiveRecord::Base.allow_concurrency = true
-    end
-
-    def teardown
-      ActiveRecord::Base.allow_concurrency = @allow_concurrency
     end
 
     # Test typical find.
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 892a99f85a..7246a8a62b 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -4,53 +4,23 @@ require 'models/reply'
 
 unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
   class ThreadedConnectionsTest < ActiveRecord::TestCase
-    self.use_transactional_fixtures = false
-
-    fixtures :topics
-
-    def setup
-      @connection = ActiveRecord::Base.remove_connection
-      @connections = []
-      @allow_concurrency = ActiveRecord::Base.allow_concurrency
-      ActiveRecord::Base.allow_concurrency = true
-    end
-
-    def teardown
-      # clear the connection cache
-      ActiveRecord::Base.clear_active_connections!
-      # set allow_concurrency to saved value
-      ActiveRecord::Base.allow_concurrency = @allow_concurrency
-      # reestablish old connection
-      ActiveRecord::Base.establish_connection(@connection)
-    end
-
-    def gather_connections
-      ActiveRecord::Base.establish_connection(@connection)
-
-      5.times do
-        Thread.new do
-          Topic.find :first
-          @connections << ActiveRecord::Base.active_connections.values.first
-        end.join
+    def test_allow_concurrency_is_deprecated
+      assert_deprecated('ActiveRecord::Base.allow_concurrency') do
+        ActiveRecord::Base.allow_concurrency
+      end
+      assert_deprecated('ActiveRecord::Base.allow_concurrency=') do
+        ActiveRecord::Base.allow_concurrency = true
       end
-    end
-
-    def test_threaded_connections
-      gather_connections
-      assert_equal @connections.length, 5
     end
   end
 
   class PooledConnectionsTest < ActiveRecord::TestCase
     def setup
       @connection = ActiveRecord::Base.remove_connection
-      @allow_concurrency = ActiveRecord::Base.allow_concurrency
-      ActiveRecord::Base.allow_concurrency = true
     end
 
     def teardown
       ActiveRecord::Base.clear_all_connections!
-      ActiveRecord::Base.allow_concurrency = @allow_concurrency
       ActiveRecord::Base.establish_connection(@connection)
     end
 
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index af3ee6ddba..8383ba58e9 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -283,17 +283,6 @@ end
 
 if current_adapter?(:PostgreSQLAdapter)
   class ConcurrentTransactionTest < TransactionTest
-    def setup
-      @allow_concurrency = ActiveRecord::Base.allow_concurrency
-      ActiveRecord::Base.allow_concurrency = true
-      super
-    end
-
-    def teardown
-      super
-      ActiveRecord::Base.allow_concurrency = @allow_concurrency
-    end
-
     # This will cause transactions to overlap and fail unless they are performed on
     # separate database connections.
     def test_transaction_per_thread
-- 
cgit v1.2.3


From 212134dce158db0ecb4169c28fd9ef80ea1a55b2 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 22 Aug 2008 13:45:10 -0500
Subject: Remove CachedConnectionPerThread per-thread pooling mechanism in
 favor of a fixed pool with default maximum of 5 connections

---
 .../connection_adapters/abstract/connection_pool.rb | 21 ++++-----------------
 .../connection_adapters/mysql_adapter.rb            |  5 +++++
 activerecord/lib/active_record/fixtures.rb          |  2 +-
 activerecord/test/cases/locking_test.rb             |  2 ++
 .../test/cases/threaded_connections_test.rb         |  2 ++
 activerecord/test/cases/transactions_test.rb        |  3 +++
 6 files changed, 17 insertions(+), 18 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 04c8361c64..1b908c3113 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -36,12 +36,10 @@ module ActiveRecord
       # * +wait_timeout+ (optional): number of seconds to block and wait
       #   for a connection before giving up and raising a timeout error.
       def self.create(spec)
-        if spec.config[:pool] && spec.config[:pool].to_i > 0
-          FixedSizeConnectionPool.new(spec)
-        elsif spec.config[:jndi] # JRuby appserver datasource pool; passthrough
+        if spec.config[:jndi] # JRuby appserver datasource pool; passthrough
           NewConnectionEveryTime.new(spec)
         else
-          CachedConnectionPerThread.new(spec)
+          FixedSizeConnectionPool.new(spec)
         end
       end
 
@@ -204,18 +202,6 @@ module ActiveRecord
       end
     end
 
-    # CachedConnectionPerThread is a compatible pseudo-connection pool that
-    # caches connections per-thread. In order to hold onto threads in the same
-    # manner as ActiveRecord 2.1 and earlier, it only disconnects the
-    # connection when the connection is checked in, or when calling
-    # ActiveRecord::Base.clear_all_connections!, and not during
-    # #release_connection.
-    class CachedConnectionPerThread < NewConnectionEveryTime
-      def release_connection
-        # no-op; keep the connection
-      end
-    end
-
     # FixedSizeConnectionPool provides a full, fixed-size connection pool with
     # timed waits when the pool is exhausted.
     class FixedSizeConnectionPool < ConnectionPool
@@ -223,7 +209,8 @@ module ActiveRecord
         super
         # default 5 second timeout
         @timeout = spec.config[:wait_timeout] || 5
-        @size = spec.config[:pool].to_i
+        # default max pool size to 5
+        @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
         @queue = @connection_mutex.new_cond
         @connections = []
         @checked_out = []
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 0a935a1b7a..14c76ac455 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -285,6 +285,7 @@ module ActiveRecord
           # See http://bugs.mysql.com/bug.php?id=33540 -- the workaround way to
           # reset the connection is to change the user to the same user.
           @connection.change_user(@config[:username], @config[:password], @config[:database])
+          configure_connection
         else
           super
         end
@@ -538,7 +539,11 @@ module ActiveRecord
           end
 
           @connection.real_connect(*@connection_options)
+          configure_connection
+        end
 
+        def configure_connection
+          encoding = @config[:encoding]
           execute("SET NAMES '#{encoding}'") if encoding
 
           # By default, MySQL 'where id is null' selects the last inserted id.
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 622cfc3c3f..114141a646 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -955,7 +955,7 @@ module Test #:nodoc:
           ActiveRecord::Base.connection.rollback_db_transaction
           ActiveRecord::Base.connection.decrement_open_transactions
         end
-        ActiveRecord::Base.verify_active_connections!
+        ActiveRecord::Base.clear_active_connections!
       end
 
       private
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index bbe8582466..df9829195c 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -280,6 +280,7 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter)
               sleep zzz       # block thread 2 for zzz seconds
             end
             t1 = Time.now
+            Person.clear_active_connections!
           end
 
           b = Thread.new do
@@ -287,6 +288,7 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter)
             t2 = Time.now
             Person.transaction { yield }
             t3 = Time.now
+            Person.clear_active_connections!
           end
 
           a.join
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
index 7246a8a62b..54c1470e69 100644
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ b/activerecord/test/cases/threaded_connections_test.rb
@@ -16,12 +16,14 @@ unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
 
   class PooledConnectionsTest < ActiveRecord::TestCase
     def setup
+      super
       @connection = ActiveRecord::Base.remove_connection
     end
 
     def teardown
       ActiveRecord::Base.clear_all_connections!
       ActiveRecord::Base.establish_connection(@connection)
+      super
     end
 
     def checkout_connections
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 8383ba58e9..d3cc69507a 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -296,6 +296,7 @@ if current_adapter?(:PostgreSQLAdapter)
               topic.approved = !topic.approved?
               topic.save!
             end
+            Topic.clear_active_connections!
           end
         end
 
@@ -331,6 +332,7 @@ if current_adapter?(:PostgreSQLAdapter)
               dev = Developer.find(1)
               assert_equal original_salary, dev.salary
             end
+            Developer.clear_active_connections!
           end
         end
 
@@ -342,6 +344,7 @@ if current_adapter?(:PostgreSQLAdapter)
               # Always expect original salary.
               assert_equal original_salary, Developer.find(1).salary
             end
+            Developer.clear_active_connections!
           end
         end
 
-- 
cgit v1.2.3


From d07a6b1a4a234908959650197f596329ca08b4f0 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 22 Aug 2008 14:11:44 -0500
Subject: Make clear_active_connections! also return stale connections back to
 the pool

- also clean up some cruft remaining from per-thread connection cache
---
 .../abstract/connection_pool.rb                    | 53 ++++++-------
 .../abstract/connection_specification.rb           |  5 +-
 activerecord/test/cases/locking_test.rb            |  2 -
 activerecord/test/cases/pooled_connections_test.rb | 87 +++++++++++++++++++++
 .../test/cases/threaded_connections_test.rb        | 91 ----------------------
 activerecord/test/cases/transactions_test.rb       |  3 -
 6 files changed, 112 insertions(+), 129 deletions(-)
 create mode 100644 activerecord/test/cases/pooled_connections_test.rb
 delete mode 100644 activerecord/test/cases/threaded_connections_test.rb

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 1b908c3113..472cf64c54 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -60,11 +60,11 @@ module ActiveRecord
       # #connection can be called any number of times; the connection is
       # held in a hash keyed by the thread id.
       def connection
-        if conn = @reserved_connections[active_connection_name]
+        if conn = @reserved_connections[current_connection_id]
           conn.verify!(verification_timeout)
           conn
         else
-          @reserved_connections[active_connection_name] = checkout
+          @reserved_connections[current_connection_id] = checkout
         end
       end
 
@@ -72,7 +72,7 @@ module ActiveRecord
       # #release_connection releases the connection-thread association
       # and returns the connection to the pool.
       def release_connection
-        conn = @reserved_connections.delete(active_connection_name)
+        conn = @reserved_connections.delete(current_connection_id)
         checkin conn if conn
       end
 
@@ -118,14 +118,20 @@ module ActiveRecord
       # Verify active connections and remove and disconnect connections
       # associated with stale threads.
       def verify_active_connections! #:nodoc:
-        remove_stale_cached_threads!(@reserved_connections) do |name, conn|
-          checkin conn
-        end
+        clear_stale_cached_connections!
         connections.each do |connection|
           connection.verify!(verification_timeout)
         end
       end
 
+      # Return any checked-out connections back to the pool by threads that
+      # are no longer alive.
+      def clear_stale_cached_connections!
+        remove_stale_cached_threads!(@reserved_connections) do |name, conn|
+          checkin conn
+        end
+      end
+
       # Check-out a database connection from the pool.
       def checkout
         raise NotImplementedError, "checkout is an abstract method"
@@ -156,7 +162,7 @@ module ActiveRecord
         ActiveRecord::Base.send(spec.adapter_method, config)
       end
 
-      def active_connection_name #:nodoc:
+      def current_connection_id #:nodoc:
         Thread.current.object_id
       end
 
@@ -178,12 +184,6 @@ module ActiveRecord
     # NewConnectionEveryTime is a simple implementation: always
     # create/disconnect on checkout/checkin.
     class NewConnectionEveryTime < ConnectionPool
-      def active_connection
-        @reserved_connections[active_connection_name]
-      end
-
-      def active_connections; @reserved_connections; end
-
       def checkout
         new_connection
       end
@@ -288,19 +288,14 @@ module ActiveRecord
         @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
       end
 
-      # for internal use only and for testing;
-      # only works with ConnectionPerThread pool class
-      def active_connections #:nodoc:
-        @connection_pools.inject({}) do |hash,kv|
-          hash[kv.first] = kv.last.active_connection
-          hash.delete(kv.first) unless hash[kv.first]
-          hash
-        end
-      end
-
-      # Clears the cache which maps classes to connections.
+      # Returns any connections in use by the current thread back to the pool,
+      # and also returns connections to the pool cached by threads that are no
+      # longer alive.
       def clear_active_connections!
-        @connection_pools.each_value {|pool| pool.release_connection }
+        @connection_pools.each_value do |pool|
+          pool.release_connection
+          pool.clear_stale_cached_connections!
+        end
       end
 
       # Clears the cache which maps classes
@@ -353,11 +348,9 @@ module ActiveRecord
       end
 
       # Apply monitor to all public methods that access the pool.
-      synchronize :establish_connection, :retrieve_connection,
-        :connected?, :remove_connection, :active_connections,
-        :clear_active_connections!, :clear_reloadable_connections!,
-        :clear_all_connections!, :verify_active_connections!,
-        :with => :connection_pools_lock
+      synchronize :establish_connection, :retrieve_connection, :connected?, :remove_connection,
+        :clear_active_connections!, :clear_reloadable_connections!, :clear_all_connections!,
+        :verify_active_connections!, :with => :connection_pools_lock
     end
   end
 end
\ No newline at end of file
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 47fc11a620..417a333aab 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -124,9 +124,8 @@ module ActiveRecord
         connection_handler.remove_connection(klass)
       end
 
-      delegate :active_connections, :clear_active_connections!,
-        :clear_reloadable_connections!, :clear_all_connections!,
-        :verify_active_connections!, :to => :connection_handler
+      delegate :clear_active_connections!, :clear_reloadable_connections!,
+        :clear_all_connections!,:verify_active_connections!, :to => :connection_handler
     end
   end
 end
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index df9829195c..bbe8582466 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -280,7 +280,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter)
               sleep zzz       # block thread 2 for zzz seconds
             end
             t1 = Time.now
-            Person.clear_active_connections!
           end
 
           b = Thread.new do
@@ -288,7 +287,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter)
             t2 = Time.now
             Person.transaction { yield }
             t3 = Time.now
-            Person.clear_active_connections!
           end
 
           a.join
diff --git a/activerecord/test/cases/pooled_connections_test.rb b/activerecord/test/cases/pooled_connections_test.rb
new file mode 100644
index 0000000000..078ca1d679
--- /dev/null
+++ b/activerecord/test/cases/pooled_connections_test.rb
@@ -0,0 +1,87 @@
+require "cases/helper"
+
+class PooledConnectionsTest < ActiveRecord::TestCase
+  def setup
+    super
+    @connection = ActiveRecord::Base.remove_connection
+  end
+
+  def teardown
+    ActiveRecord::Base.clear_all_connections!
+    ActiveRecord::Base.establish_connection(@connection)
+    super
+  end
+
+  def checkout_connections
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
+    @connections = []
+    @timed_out = 0
+
+    4.times do
+      Thread.new do
+        begin
+          @connections << ActiveRecord::Base.connection_pool.checkout
+        rescue ActiveRecord::ConnectionTimeoutError
+          @timed_out += 1
+        end
+      end.join
+    end
+  end
+
+  def test_pooled_connection_checkout
+    checkout_connections
+    assert_equal @connections.length, 2
+    assert_equal @timed_out, 2
+  end
+
+  def checkout_checkin_connections(pool_size, threads)
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
+    @connection_count = 0
+    @timed_out = 0
+    threads.times do
+      Thread.new do
+        begin
+          conn = ActiveRecord::Base.connection_pool.checkout
+          sleep 0.1
+          ActiveRecord::Base.connection_pool.checkin conn
+          @connection_count += 1
+        rescue ActiveRecord::ConnectionTimeoutError
+          @timed_out += 1
+        end
+      end.join
+    end
+  end
+
+  def test_pooled_connection_checkin_one
+    checkout_checkin_connections 1, 2
+    assert_equal 2, @connection_count
+    assert_equal 0, @timed_out
+  end
+
+  def test_pooled_connection_checkin_two
+    checkout_checkin_connections 2, 3
+    assert_equal 3, @connection_count
+    assert_equal 0, @timed_out
+  end
+
+  def test_pooled_connection_checkout_existing_first
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1}))
+    conn_pool = ActiveRecord::Base.connection_pool
+    conn = conn_pool.checkout
+    conn_pool.checkin(conn)
+    conn = conn_pool.checkout
+    assert ActiveRecord::ConnectionAdapters::AbstractAdapter === conn
+    conn_pool.checkin(conn)
+  end
+end unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
+
+class AllowConcurrencyDeprecatedTest < ActiveRecord::TestCase
+  def test_allow_concurrency_is_deprecated
+    assert_deprecated('ActiveRecord::Base.allow_concurrency') do
+      ActiveRecord::Base.allow_concurrency
+    end
+    assert_deprecated('ActiveRecord::Base.allow_concurrency=') do
+      ActiveRecord::Base.allow_concurrency = true
+    end
+  end
+end
diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb
deleted file mode 100644
index 54c1470e69..0000000000
--- a/activerecord/test/cases/threaded_connections_test.rb
+++ /dev/null
@@ -1,91 +0,0 @@
-require "cases/helper"
-require 'models/topic'
-require 'models/reply'
-
-unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
-  class ThreadedConnectionsTest < ActiveRecord::TestCase
-    def test_allow_concurrency_is_deprecated
-      assert_deprecated('ActiveRecord::Base.allow_concurrency') do
-        ActiveRecord::Base.allow_concurrency
-      end
-      assert_deprecated('ActiveRecord::Base.allow_concurrency=') do
-        ActiveRecord::Base.allow_concurrency = true
-      end
-    end
-  end
-
-  class PooledConnectionsTest < ActiveRecord::TestCase
-    def setup
-      super
-      @connection = ActiveRecord::Base.remove_connection
-    end
-
-    def teardown
-      ActiveRecord::Base.clear_all_connections!
-      ActiveRecord::Base.establish_connection(@connection)
-      super
-    end
-
-    def checkout_connections
-      ActiveRecord::Base.establish_connection(@connection.merge({:pool => 2, :wait_timeout => 0.3}))
-      @connections = []
-      @timed_out = 0
-
-      4.times do
-        Thread.new do
-          begin
-            @connections << ActiveRecord::Base.connection_pool.checkout
-          rescue ActiveRecord::ConnectionTimeoutError
-            @timed_out += 1
-          end
-        end.join
-      end
-    end
-
-    def test_pooled_connection_checkout
-      checkout_connections
-      assert_equal @connections.length, 2
-      assert_equal @timed_out, 2
-    end
-
-    def checkout_checkin_connections(pool_size, threads)
-      ActiveRecord::Base.establish_connection(@connection.merge({:pool => pool_size, :wait_timeout => 0.5}))
-      @connection_count = 0
-      @timed_out = 0
-      threads.times do
-        Thread.new do
-          begin
-            conn = ActiveRecord::Base.connection_pool.checkout
-            sleep 0.1
-            ActiveRecord::Base.connection_pool.checkin conn
-            @connection_count += 1
-          rescue ActiveRecord::ConnectionTimeoutError
-            @timed_out += 1
-          end
-        end.join
-      end
-    end
-
-    def test_pooled_connection_checkin_one
-      checkout_checkin_connections 1, 2
-      assert_equal 2, @connection_count
-      assert_equal 0, @timed_out
-    end
-
-    def test_pooled_connection_checkin_two
-      checkout_checkin_connections 2, 3
-      assert_equal 3, @connection_count
-      assert_equal 0, @timed_out
-    end
-
-    def test_pooled_connection_checkout_existing_first
-      ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1}))
-      conn_pool = ActiveRecord::Base.connection_pool
-      conn = conn_pool.checkout
-      conn_pool.checkin(conn)
-      conn = conn_pool.checkout
-      assert ActiveRecord::ConnectionAdapters::AbstractAdapter === conn
-      conn_pool.checkin(conn)
-    end
-  end
-end
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index d3cc69507a..8383ba58e9 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -296,7 +296,6 @@ if current_adapter?(:PostgreSQLAdapter)
               topic.approved = !topic.approved?
               topic.save!
             end
-            Topic.clear_active_connections!
           end
         end
 
@@ -332,7 +331,6 @@ if current_adapter?(:PostgreSQLAdapter)
               dev = Developer.find(1)
               assert_equal original_salary, dev.salary
             end
-            Developer.clear_active_connections!
           end
         end
 
@@ -344,7 +342,6 @@ if current_adapter?(:PostgreSQLAdapter)
               # Always expect original salary.
               assert_equal original_salary, Developer.find(1).salary
             end
-            Developer.clear_active_connections!
           end
         end
 
-- 
cgit v1.2.3


From 8e5e02bdad5f5aaed8ea72e9da13f8d6aa22ab34 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Fri, 22 Aug 2008 14:40:06 -0500
Subject: Collapse connection pool class hierarchy; YAGNI.

- Add connection checkin and checkout callbacks to adapter to allow
  adapter-specific customization of behavior (e.g., JRuby w/ JNDI)
---
 .../abstract/connection_pool.rb                    | 160 +++++++--------------
 .../connection_adapters/abstract_adapter.rb        |   3 +
 2 files changed, 51 insertions(+), 112 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 472cf64c54..85ecd9d6c6 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -27,22 +27,14 @@ module ActiveRecord
     # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which
     #    obtains a connection, yields it as the sole argument to the block,
     #    and returns it to the pool after the block completes.
+    #
+    # There are two connection-pooling-related options that you can add to
+    # your database connection configuration:
+    #
+    # * +pool+: number indicating size of connection pool (default 5)
+    # * +wait_timeout+: number of seconds to block and wait for a connection
+    #   before giving up and raising a timeout error (default 5 seconds).
     class ConnectionPool
-      # Factory method for connection pools.
-      # Determines pool type to use based on contents of connection
-      # specification. Additional options for connection specification:
-      #
-      # * +pool+: number indicating size of fixed connection pool to use
-      # * +wait_timeout+ (optional): number of seconds to block and wait
-      #   for a connection before giving up and raising a timeout error.
-      def self.create(spec)
-        if spec.config[:jndi] # JRuby appserver datasource pool; passthrough
-          NewConnectionEveryTime.new(spec)
-        else
-          FixedSizeConnectionPool.new(spec)
-        end
-      end
-
       delegate :verification_timeout, :to => "::ActiveRecord::Base"
       attr_reader :spec
 
@@ -52,6 +44,13 @@ module ActiveRecord
         @reserved_connections = {}
         # The mutex used to synchronize pool access
         @connection_mutex = Monitor.new
+        @queue = @connection_mutex.new_cond
+        # default 5 second timeout
+        @timeout = spec.config[:wait_timeout] || 5
+        # default max pool size to 5
+        @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
+        @connections = []
+        @checked_out = []
       end
 
       # Retrieve the connection associated with the current thread, or call
@@ -87,7 +86,7 @@ module ActiveRecord
 
       # Returns true if a connection has already been opened.
       def connected?
-        !connections.empty?
+        !@connections.empty?
       end
 
       # Disconnect all connections in the pool.
@@ -95,10 +94,11 @@ module ActiveRecord
         @reserved_connections.each do |name,conn|
           checkin conn
         end
-        connections.each do |conn|
+        @reserved_connections = {}
+        @connections.each do |conn|
           conn.disconnect!
         end
-        @reserved_connections = {}
+        @connections = []
       end
 
       # Clears the cache which maps classes
@@ -107,19 +107,17 @@ module ActiveRecord
           checkin conn
         end
         @reserved_connections = {}
-        connections.each do |conn|
-          if conn.requires_reloading?
-            conn.disconnect!
-            remove_connection conn
-          end
+        @connections.each do |conn|
+          conn.disconnect! if conn.requires_reloading?
         end
+        @connections = []
       end
 
       # Verify active connections and remove and disconnect connections
       # associated with stale threads.
       def verify_active_connections! #:nodoc:
         clear_stale_cached_connections!
-        connections.each do |connection|
+        @connections.each do |connection|
           connection.verify!(verification_timeout)
         end
       end
@@ -134,23 +132,34 @@ module ActiveRecord
 
       # Check-out a database connection from the pool.
       def checkout
-        raise NotImplementedError, "checkout is an abstract method"
-      end
-
-      # Check-in a database connection back into the pool.
-      def checkin(connection)
-        raise NotImplementedError, "checkin is an abstract method"
-      end
+        # Checkout an available connection
+        conn = @connection_mutex.synchronize do
+          if @checked_out.size < @connections.size
+            checkout_existing_connection
+          elsif @connections.size < @size
+            checkout_new_connection
+          end
+        end
+        return conn if conn
 
-      def remove_connection(conn) #:nodoc:
-        raise NotImplementedError, "remove_connection is an abstract method"
+        # No connections available; wait for one
+        @connection_mutex.synchronize do
+          if @queue.wait(@timeout)
+            checkout_existing_connection
+          else
+            raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion"
+          end
+        end
       end
-      private :remove_connection
 
-      def connections #:nodoc:
-        raise NotImplementedError, "connections is an abstract method"
+      # Check-in a database connection back into the pool.
+      def checkin(conn)
+        @connection_mutex.synchronize do
+          conn.run_callbacks :checkin
+          @checked_out.delete conn
+          @queue.signal
+        end
       end
-      private :connections
 
       synchronize :connection, :release_connection,
         :clear_reloadable_connections!, :verify_active_connections!,
@@ -179,72 +188,7 @@ module ActiveRecord
           cache.delete(key)
         end
       end
-    end
-
-    # NewConnectionEveryTime is a simple implementation: always
-    # create/disconnect on checkout/checkin.
-    class NewConnectionEveryTime < ConnectionPool
-      def checkout
-        new_connection
-      end
-
-      def checkin(conn)
-        conn.disconnect!
-      end
-
-      private
-      def connections
-        @reserved_connections.values
-      end
-
-      def remove_connection(conn)
-        @reserved_connections.delete_if {|k,v| v == conn}
-      end
-    end
-
-    # FixedSizeConnectionPool provides a full, fixed-size connection pool with
-    # timed waits when the pool is exhausted.
-    class FixedSizeConnectionPool < ConnectionPool
-      def initialize(spec)
-        super
-        # default 5 second timeout
-        @timeout = spec.config[:wait_timeout] || 5
-        # default max pool size to 5
-        @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
-        @queue = @connection_mutex.new_cond
-        @connections = []
-        @checked_out = []
-      end
-
-      def checkout
-        # Checkout an available connection
-        conn = @connection_mutex.synchronize do
-          if @checked_out.size < @connections.size
-            checkout_existing_connection
-          elsif @connections.size < @size
-            checkout_new_connection
-          end
-        end
-        return conn if conn
-
-        # No connections available; wait for one
-        @connection_mutex.synchronize do
-          if @queue.wait(@timeout)
-            checkout_existing_connection
-          else
-            raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion"
-          end
-        end
-      end
 
-      def checkin(conn)
-        @connection_mutex.synchronize do
-          @checked_out.delete conn
-          @queue.signal
-        end
-      end
-
-      private
       def checkout_new_connection
         c = new_connection
         @connections << c
@@ -257,19 +201,11 @@ module ActiveRecord
       end
 
       def checkout_and_verify(c)
-        c.reset!
+        c.run_callbacks :checkout
         c.verify!(verification_timeout)
         @checked_out << c
         c
       end
-
-      def connections
-        @connections
-      end
-
-      def remove_connection(conn)
-        @connections.delete conn
-      end
     end
 
     class ConnectionHandler
@@ -285,7 +221,7 @@ module ActiveRecord
       end
 
       def establish_connection(name, spec)
-        @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec)
+        @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
       end
 
       # Returns any connections in use by the current thread back to the pool,
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 7ef8834547..005be9d72f 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -25,6 +25,9 @@ module ActiveRecord
     class AbstractAdapter
       include Quoting, DatabaseStatements, SchemaStatements
       include QueryCache
+      include ActiveSupport::Callbacks
+      define_callbacks :checkout, :checkin
+      checkout :reset!
       @@row_even = true
 
       def initialize(connection, logger = nil) #:nodoc:
-- 
cgit v1.2.3


From 113cc4e1c41b8246b8f6327b58bd315be72469e7 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Sat, 23 Aug 2008 01:24:34 -0500
Subject: Remove some synchronization that's probably overkill, assuming one
 doesn't establish connections frequently

---
 .../connection_adapters/abstract/connection_pool.rb           | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 85ecd9d6c6..fe6ba47d69 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -161,8 +161,7 @@ module ActiveRecord
         end
       end
 
-      synchronize :connection, :release_connection,
-        :clear_reloadable_connections!, :verify_active_connections!,
+      synchronize :clear_reloadable_connections!, :verify_active_connections!,
         :connected?, :disconnect!, :with => :@connection_mutex
 
       private
@@ -209,11 +208,8 @@ module ActiveRecord
     end
 
     class ConnectionHandler
-      attr_reader :connection_pools_lock
-
       def initialize(pools = {})
         @connection_pools = pools
-        @connection_pools_lock = Monitor.new
       end
 
       def connection_pools
@@ -282,11 +278,6 @@ module ActiveRecord
           klass = klass.superclass
         end
       end
-
-      # Apply monitor to all public methods that access the pool.
-      synchronize :establish_connection, :retrieve_connection, :connected?, :remove_connection,
-        :clear_active_connections!, :clear_reloadable_connections!, :clear_all_connections!,
-        :verify_active_connections!, :with => :connection_pools_lock
     end
   end
 end
\ No newline at end of file
-- 
cgit v1.2.3


From 300754509b6990b387b056c122e90f50a79eeb81 Mon Sep 17 00:00:00 2001
From: Nick Sieger <nick@nicksieger.com>
Date: Tue, 26 Aug 2008 11:08:06 -0500
Subject: Minor tweak to retrieve_connection_pool -- recurse instead of loop

---
 .../connection_adapters/abstract/connection_pool.rb            | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index fe6ba47d69..838b0434b0 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -271,12 +271,10 @@ module ActiveRecord
       end
 
       def retrieve_connection_pool(klass)
-        loop do
-          pool = @connection_pools[klass.name]
-          return pool if pool
-          return nil if ActiveRecord::Base == klass
-          klass = klass.superclass
-        end
+        pool = @connection_pools[klass.name]
+        return pool if pool
+        return nil if ActiveRecord::Base == klass
+        retrieve_connection_pool klass.superclass
       end
     end
   end
-- 
cgit v1.2.3


From 99492bad885aa0ee44c770e2c61ad36c3058c697 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 29 Aug 2008 21:12:37 +0200
Subject: Use a set for the named scope methods not a big regexp.

---
 activerecord/lib/active_record/named_scope.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index b9b7eb5f00..570416d4e0 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -101,9 +101,9 @@ module ActiveRecord
     
     class Scope
       attr_reader :proxy_scope, :proxy_options
-
+      NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
       [].methods.each do |m|
-        unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty\?|any\?|respond_to\?)/
+        unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m)
           delegate m, :to => :proxy_found
         end
       end
-- 
cgit v1.2.3


From c0361344d95162cd8573592d60e52986eaca0377 Mon Sep 17 00:00:00 2001
From: Joshua Peek <josh@joshpeek.com>
Date: Fri, 29 Aug 2008 15:43:07 -0500
Subject: 1.9: methods need to be coerced into strings

---
 activerecord/lib/active_record/named_scope.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 570416d4e0..7397d70279 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -103,7 +103,7 @@ module ActiveRecord
       attr_reader :proxy_scope, :proxy_options
       NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
       [].methods.each do |m|
-        unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m)
+        unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m.to_s)
           delegate m, :to => :proxy_found
         end
       end
-- 
cgit v1.2.3


From 7f179f8540ab92dbd9d3e650b465de5b694d93d4 Mon Sep 17 00:00:00 2001
From: Tom Stuart <tom@obsess.com>
Date: Fri, 29 Aug 2008 15:11:25 +0100
Subject: Make NamedScope#size behave identically to
 AssociationCollection#size. [#933 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/named_scope.rb |  6 +++++-
 activerecord/test/cases/named_scope_test.rb   | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 7397d70279..d7e152ed1d 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -101,7 +101,7 @@ module ActiveRecord
     
     class Scope
       attr_reader :proxy_scope, :proxy_options
-      NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
+      NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
       [].methods.each do |m|
         unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m.to_s)
           delegate m, :to => :proxy_found
@@ -136,6 +136,10 @@ module ActiveRecord
         end
       end
 
+      def size
+        @found ? @found.length : count
+      end
+
       def empty?
         @found ? @found.empty? : count.zero?
       end
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index acc3b3016a..444debd255 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -256,4 +256,19 @@ class NamedScopeTest < ActiveRecord::TestCase
   def test_should_use_where_in_query_for_named_scope
     assert_equal Developer.find_all_by_name('Jamis'), Developer.find_all_by_id(Developer.jamises)
   end
+
+  def test_size_should_use_count_when_results_are_not_loaded
+    topics = Topic.base
+    assert_queries(1) do
+      assert_sql(/COUNT/i) { topics.size }
+    end
+  end
+
+  def test_size_should_use_length_when_results_are_loaded
+    topics = Topic.base
+    topics.reload # force load
+    assert_no_queries do
+      topics.size # use loaded (no query)
+    end
+  end
 end
-- 
cgit v1.2.3


From b163d83b8bab9103dc0e73b86212c2629cb45ca2 Mon Sep 17 00:00:00 2001
From: miloops <miloops@gmail.com>
Date: Sat, 30 Aug 2008 19:17:29 -0300
Subject: Performance: Better query for ASSOCIATION_ids. Select only ids if the
 association hasn't been loaded.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/associations.rb     |  6 +++-
 .../has_and_belongs_to_many_associations_test.rb   | 34 ++++++++++++++++------
 .../associations/has_many_associations_test.rb     | 30 ++++++++++++++-----
 .../has_many_through_associations_test.rb          | 20 +++++++++++++
 4 files changed, 73 insertions(+), 17 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 3ca93db10f..62cc414555 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1304,7 +1304,11 @@ module ActiveRecord
           end
 
           define_method("#{reflection.name.to_s.singularize}_ids") do
-            send(reflection.name).map { |record| record.id }
+            if send(reflection.name).loaded?
+              send(reflection.name).map(&:id)
+            else
+              send(reflection.name).all(:select => "#{reflection.quoted_table_name}.id").map(&:id)
+            end
           end
         end
 
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 1b31d28679..edca3c622b 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -223,10 +223,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     devel = Developer.find(1)
     proj = assert_no_queries { devel.projects.build("name" => "Projekt") }
     assert !devel.projects.loaded?
-    
+
     assert_equal devel.projects.last, proj
     assert devel.projects.loaded?
-    
+
     assert proj.new_record?
     devel.save
     assert !proj.new_record?
@@ -251,10 +251,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     devel = Developer.find(1)
     proj = devel.projects.create("name" => "Projekt")
     assert !devel.projects.loaded?
-    
+
     assert_equal devel.projects.last, proj
     assert devel.projects.loaded?
-    
+
     assert !proj.new_record?
     assert_equal Developer.find(1).projects.sort_by(&:id).last, proj  # prove join table is updated
   end
@@ -274,10 +274,10 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_creation_respects_hash_condition
     post = categories(:general).post_with_conditions.build(:body => '')
-    
+
     assert        post.save
     assert_equal  'Yet Another Testing Title', post.title
-    
+
     another_post = categories(:general).post_with_conditions.create(:body => '')
 
     assert        !another_post.new_record?
@@ -288,7 +288,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     dev = developers(:jamis)
     dev.projects << projects(:active_record)
     dev.projects << projects(:active_record)
-    
+
     assert_equal 3, dev.projects.size
     assert_equal 1, dev.projects.uniq.size
   end
@@ -415,13 +415,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     project.developers.class # force load target
 
     developer = project.developers.first
-    
+
     assert_no_queries do
       assert project.developers.loaded?
       assert project.developers.include?(developer)
     end
   end
-  
+
   def test_include_checks_if_record_exists_if_target_not_loaded
     project = projects(:active_record)
     developer = project.developers.first
@@ -641,6 +641,22 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [projects(:active_record).id], developers(:jamis).project_ids
   end
 
+  def test_get_ids_for_loaded_associations
+    developer = developers(:david)
+    developer.projects(true)
+    assert_queries(0) do
+      developer.project_ids
+      developer.project_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    developer = developers(:david)
+    assert !developer.projects.loaded?
+    assert_equal projects(:active_record, :action_controller).map(&:id).sort, developer.project_ids.sort
+    assert !developer.projects.loaded?
+  end
+
   def test_assign_ids
     developer = Developer.new("name" => "Joe")
     developer.project_ids = projects(:active_record, :action_controller).map(&:id)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 17fd88ce65..feac4b002b 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -384,7 +384,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     company = companies(:first_firm)
     new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
     assert !company.clients_of_firm.loaded?
-    
+
     assert_equal "Another Client", new_client.name
     assert new_client.new_record?
     assert_equal new_client, company.clients_of_firm.last
@@ -416,7 +416,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_build_many
     company = companies(:first_firm)
     new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) }
-    
+
     assert_equal 2, new_clients.size
     company.name += '-changed'
     assert_queries(3) { assert company.save }
@@ -655,10 +655,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_creation_respects_hash_condition
     ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.build
-    
+
     assert        ms_client.save
     assert_equal  'Microsoft', ms_client.name
-    
+
     another_ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.create
 
     assert        !another_ms_client.new_record?
@@ -830,6 +830,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [companies(:first_client).id, companies(:second_client).id], companies(:first_firm).client_ids
   end
 
+  def test_get_ids_for_loaded_associations
+    company = companies(:first_firm)
+    company.clients(true)
+    assert_queries(0) do
+      company.client_ids
+      company.client_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    company = companies(:first_firm)
+    assert !company.clients.loaded?
+    assert_equal [companies(:first_client).id, companies(:second_client).id], company.client_ids
+    assert !company.clients.loaded?
+  end
+
   def test_assign_ids
     firm = Firm.new("name" => "Apple")
     firm.client_ids = [companies(:first_client).id, companies(:second_client).id]
@@ -900,7 +916,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 4, authors(:david).limited_comments.find(:all, :conditions => "comments.type = 'SpecialComment'", :limit => 9_000).length
     assert_equal 4, authors(:david).limited_comments.find_all_by_type('SpecialComment', :limit => 9_000).length
   end
-  
+
   def test_find_all_include_over_the_same_table_for_through
     assert_equal 2, people(:michael).posts.find(:all, :include => :people).length
   end
@@ -937,13 +953,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_include_loads_collection_if_target_uses_finder_sql
     firm = companies(:first_firm)
     client = firm.clients_using_sql.first
-    
+
     firm.reload
     assert ! firm.clients_using_sql.loaded?
     assert firm.clients_using_sql.include?(client)
     assert firm.clients_using_sql.loaded?
   end
-  
+
 
   def test_include_returns_false_for_non_matching_record_to_verify_scoping
     firm = companies(:first_firm)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 353262c81b..3e3c7a95b7 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -200,4 +200,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_count_with_include_should_alias_join_table
     assert_equal 2, people(:michael).posts.count(:include => :readers)
   end
+
+  def test_get_ids
+    assert_equal [posts(:welcome).id, posts(:authorless).id], people(:michael).post_ids
+  end
+
+  def test_get_ids_for_loaded_associations
+    person = people(:michael)
+    person.posts(true)
+    assert_queries(0) do
+      person.post_ids
+      person.post_ids
+    end
+  end
+
+  def test_get_ids_for_unloaded_associations_does_not_load_them
+    person = people(:michael)
+    assert !person.posts.loaded?
+    assert_equal [posts(:welcome).id, posts(:authorless).id], person.post_ids
+    assert !person.posts.loaded?
+  end
 end
-- 
cgit v1.2.3


From 6183e55f714b436335dc843528be7525c342d922 Mon Sep 17 00:00:00 2001
From: miloops <miloops@gmail.com>
Date: Sat, 30 Aug 2008 21:13:53 -0300
Subject: Use reflection primary_key instead of id for when selecting
 association ids.

[#906 state:resolved]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/associations.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 62cc414555..2470eb44b4 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1307,7 +1307,7 @@ module ActiveRecord
             if send(reflection.name).loaded?
               send(reflection.name).map(&:id)
             else
-              send(reflection.name).all(:select => "#{reflection.quoted_table_name}.id").map(&:id)
+              send(reflection.name).all(:select => "#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").map(&:id)
             end
           end
         end
-- 
cgit v1.2.3


From c50223b76fdb843dc7e4d4522627a5b20f955a03 Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremy@bitsweat.net>
Date: Sat, 30 Aug 2008 22:47:28 -0700
Subject: Fix tests that assumed implicit order by id

---
 .../test/cases/associations/has_many_through_associations_test.rb     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 3e3c7a95b7..0be050ec81 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -202,7 +202,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_get_ids
-    assert_equal [posts(:welcome).id, posts(:authorless).id], people(:michael).post_ids
+    assert_equal [posts(:welcome).id, posts(:authorless).id].sort, people(:michael).post_ids.sort
   end
 
   def test_get_ids_for_loaded_associations
@@ -217,7 +217,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_get_ids_for_unloaded_associations_does_not_load_them
     person = people(:michael)
     assert !person.posts.loaded?
-    assert_equal [posts(:welcome).id, posts(:authorless).id], person.post_ids
+    assert_equal [posts(:welcome).id, posts(:authorless).id].sort, person.post_ids.sort
     assert !person.posts.loaded?
   end
 end
-- 
cgit v1.2.3


From 76797b443929005f43512a147e97f02f3145ed81 Mon Sep 17 00:00:00 2001
From: Iain Hecker <github@iain.nl>
Date: Sun, 31 Aug 2008 12:14:24 +0200
Subject: translates when a message symbol has been set on builtin validations

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/validations.rb    |  3 ++-
 activerecord/test/cases/validations_i18n_test.rb | 33 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 9ee80e6655..dae4ae8122 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -87,6 +87,8 @@ module ActiveRecord
     # </ol>
     def generate_message(attribute, message = :invalid, options = {})
 
+      message, options[:default] = options[:default], message if options[:default].is_a?(Symbol)
+
       defaults = @base.class.self_and_descendents_from_active_record.map do |klass| 
         [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}", 
           :"models.#{klass.name.underscore}.#{message}" ]
@@ -95,7 +97,6 @@ module ActiveRecord
       defaults << options.delete(:default)
       defaults = defaults.compact.flatten << :"messages.#{message}"
 
-      model_name = @base.class.name
       key = defaults.shift
       value = @base.respond_to?(attribute) ? @base.send(attribute) : nil
 
diff --git a/activerecord/test/cases/validations_i18n_test.rb b/activerecord/test/cases/validations_i18n_test.rb
index 43592bcee3..090f347a20 100644
--- a/activerecord/test/cases/validations_i18n_test.rb
+++ b/activerecord/test/cases/validations_i18n_test.rb
@@ -675,6 +675,38 @@ class ActiveRecordValidationsI18nTests < Test::Unit::TestCase
     replied_topic.valid?
     assert_equal 'global message', replied_topic.errors.on(:replies)
   end
+
+  def test_validations_with_message_symbol_must_translate
+    I18n.backend.store_translations 'en-US', :activerecord => {:errors => {:messages => {:custom_error => "I am a custom error"}}}
+    Topic.validates_presence_of :title, :message => :custom_error
+    @topic.title = nil
+    @topic.valid?
+    assert_equal "I am a custom error", @topic.errors.on(:title)
+  end
+
+  def test_validates_with_message_symbol_must_translate_per_attribute
+    I18n.backend.store_translations 'en-US', :activerecord => {:errors => {:models => {:topic => {:attributes => {:title => {:custom_error => "I am a custom error"}}}}}}
+    Topic.validates_presence_of :title, :message => :custom_error
+    @topic.title = nil
+    @topic.valid?
+    assert_equal "I am a custom error", @topic.errors.on(:title)
+  end
+
+  def test_validates_with_message_symbol_must_translate_per_model
+    I18n.backend.store_translations 'en-US', :activerecord => {:errors => {:models => {:topic => {:custom_error => "I am a custom error"}}}}
+    Topic.validates_presence_of :title, :message => :custom_error
+    @topic.title = nil
+    @topic.valid?
+    assert_equal "I am a custom error", @topic.errors.on(:title)
+  end
+
+  def test_validates_with_message_string
+    Topic.validates_presence_of :title, :message => "I am a custom error"
+    @topic.title = nil
+    @topic.valid?
+    assert_equal "I am a custom error", @topic.errors.on(:title)
+  end
+
 end
 
 class ActiveRecordValidationsGenerateMessageI18nTests < Test::Unit::TestCase
@@ -855,4 +887,5 @@ class ActiveRecordValidationsGenerateMessageI18nTests < Test::Unit::TestCase
   def test_generate_message_even_with_default_message
     assert_equal "must be even", @topic.errors.generate_message(:title, :even, :default => nil, :value => 'title', :count => 10)
   end
+
 end
-- 
cgit v1.2.3


From 1646e8c36495680756304b23b7301dbda9cad07a Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Tue, 2 Sep 2008 10:04:53 +0200
Subject: More symbols for send and respond_to?.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/attribute_methods.rb | 4 ++--
 activerecord/lib/active_record/validations.rb       | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index fab16a4446..ace335ed87 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -330,8 +330,8 @@ module ActiveRecord
       end
     end
     
-    # A Person object with a name attribute can ask <tt>person.respond_to?("name")</tt>,
-    # <tt>person.respond_to?("name=")</tt>, and <tt>person.respond_to?("name?")</tt>
+    # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>,
+    # <tt>person.respond_to?(:name=)</tt>, and <tt>person.respond_to?(:name?)</tt>
     # which will all return +true+.
     alias :respond_to_without_attributes? :respond_to?
     def respond_to?(method, include_priv = false)
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index dae4ae8122..9941b752cb 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -54,7 +54,7 @@ module ActiveRecord
     def add_on_empty(attributes, custom_message = nil)
       for attr in [attributes].flatten
         value = @base.respond_to?(attr.to_s) ? @base.send(attr.to_s) : @base[attr.to_s]
-        is_empty = value.respond_to?("empty?") ? value.empty? : false        
+        is_empty = value.respond_to?(:empty?) ? value.empty? : false
         add(attr, :empty, :default => custom_message) unless !value.nil? && !is_empty
       end
     end
@@ -751,7 +751,7 @@ module ActiveRecord
 
         enum = configuration[:in] || configuration[:within]
 
-        raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?("include?")
+        raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?)
 
         validates_each(attr_names, configuration) do |record, attr_name, value|
           unless enum.include?(value)
@@ -785,7 +785,7 @@ module ActiveRecord
 
         enum = configuration[:in] || configuration[:within]
 
-        raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?("include?")
+        raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?)
 
         validates_each(attr_names, configuration) do |record, attr_name, value|
           if enum.include?(value)
-- 
cgit v1.2.3


From ba3ecf53b4902a9a5943f4dcf2073fe413de4778 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Tue, 2 Sep 2008 10:31:49 +0200
Subject: Some performance goodness for AR associations.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/associations.rb | 64 +++++++++++++-------------
 1 file changed, 32 insertions(+), 32 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 2470eb44b4..dccd9abf31 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -878,10 +878,10 @@ module ActiveRecord
 
           method_name = "has_one_after_save_for_#{reflection.name}".to_sym
           define_method(method_name) do
-            association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}")
+            association = instance_variable_get(ivar) if instance_variable_defined?(ivar)
 
-            if !association.nil? && (new_record? || association.new_record? || association["#{reflection.primary_key_name}"] != id)
-              association["#{reflection.primary_key_name}"] = id
+            if !association.nil? && (new_record? || association.new_record? || association[reflection.primary_key_name] != id)
+              association[reflection.primary_key_name] = id
               association.save(true)
             end
           end
@@ -994,7 +994,7 @@ module ActiveRecord
 
           method_name = "polymorphic_belongs_to_before_save_for_#{reflection.name}".to_sym
           define_method(method_name) do
-            association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}")
+            association = instance_variable_get(ivar) if instance_variable_defined?(ivar)
 
             if association && association.target
               if association.new_record?
@@ -1002,8 +1002,8 @@ module ActiveRecord
               end
 
               if association.updated?
-                self["#{reflection.primary_key_name}"] = association.id
-                self["#{reflection.options[:foreign_type]}"] = association.class.base_class.name.to_s
+                self[reflection.primary_key_name] = association.id
+                self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s
               end
             end
           end
@@ -1015,7 +1015,7 @@ module ActiveRecord
 
           method_name = "belongs_to_before_save_for_#{reflection.name}".to_sym
           define_method(method_name) do
-            association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}")
+            association = instance_variable_get(ivar) if instance_variable_defined?(ivar)
 
             if !association.nil?
               if association.new_record?
@@ -1023,7 +1023,7 @@ module ActiveRecord
               end
 
               if association.updated?
-                self["#{reflection.primary_key_name}"] = association.id
+                self[reflection.primary_key_name] = association.id
               end
             end
           end
@@ -1038,15 +1038,15 @@ module ActiveRecord
 
           method_name = "belongs_to_counter_cache_after_create_for_#{reflection.name}".to_sym
           define_method(method_name) do
-            association = send("#{reflection.name}")
-            association.class.increment_counter("#{cache_column}", send("#{reflection.primary_key_name}")) unless association.nil?
+            association = send(reflection.name)
+            association.class.increment_counter(cache_column, send(reflection.primary_key_name)) unless association.nil?
           end
           after_create method_name
 
           method_name = "belongs_to_counter_cache_before_destroy_for_#{reflection.name}".to_sym
           define_method(method_name) do
-            association = send("#{reflection.name}")
-            association.class.decrement_counter("#{cache_column}", send("#{reflection.primary_key_name}")) unless association.nil?
+            association = send(reflection.name)
+            association.class.decrement_counter(cache_column, send(reflection.primary_key_name)) unless association.nil?
           end
           before_destroy method_name
 
@@ -1329,19 +1329,19 @@ module ActiveRecord
             end
           end
         end
-        
+
         def add_single_associated_validation_callbacks(association_name)
           method_name = "validate_associated_records_for_#{association_name}".to_sym
           define_method(method_name) do
             association = instance_variable_get("@#{association_name}")
             if !association.nil?
-              errors.add "#{association_name}" unless association.target.nil? || association.valid?
+              errors.add association_name unless association.target.nil? || association.valid?
             end
           end
-        
+
           validate method_name
         end
-        
+
         def add_multiple_associated_validation_callbacks(association_name)
           method_name = "validate_associated_records_for_#{association_name}".to_sym
           ivar = "@#{association_name}"
@@ -1357,7 +1357,7 @@ module ActiveRecord
               else
                 association.target.select { |record| record.new_record? }
               end.each do |record|
-                errors.add "#{association_name}" unless record.valid?
+                errors.add association_name unless record.valid?
               end
             end
           end
@@ -1377,7 +1377,7 @@ module ActiveRecord
 
           method_name = "after_create_or_update_associated_records_for_#{association_name}".to_sym
           define_method(method_name) do
-            association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}")
+            association = instance_variable_get(ivar) if instance_variable_defined?(ivar)
 
             records_to_save = if @new_record_before_save
               association
@@ -1444,7 +1444,7 @@ module ActiveRecord
               when :destroy
                 method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  send("#{reflection.name}").each { |o| o.destroy }
+                  send(reflection.name).each { |o| o.destroy }
                 end
                 before_destroy method_name
               when :delete_all
@@ -1463,22 +1463,22 @@ module ActiveRecord
               when :destroy
                 method_name = "has_one_dependent_destroy_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  association = send("#{reflection.name}")
+                  association = send(reflection.name)
                   association.destroy unless association.nil?
                 end
                 before_destroy method_name
               when :delete
                 method_name = "has_one_dependent_delete_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  association = send("#{reflection.name}")
+                  association = send(reflection.name)
                   association.class.delete(association.id) unless association.nil?
                 end
                 before_destroy method_name
               when :nullify
                 method_name = "has_one_dependent_nullify_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  association = send("#{reflection.name}")
-                  association.update_attribute("#{reflection.primary_key_name}", nil) unless association.nil?
+                  association = send(reflection.name)
+                  association.update_attribute(reflection.primary_key_name, nil) unless association.nil?
                 end
                 before_destroy method_name
               else
@@ -1493,14 +1493,14 @@ module ActiveRecord
               when :destroy
                 method_name = "belongs_to_dependent_destroy_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  association = send("#{reflection.name}")
+                  association = send(reflection.name)
                   association.destroy unless association.nil?
                 end
                 before_destroy method_name
               when :delete
                 method_name = "belongs_to_dependent_delete_for_#{reflection.name}".to_sym
                 define_method(method_name) do
-                  association = send("#{reflection.name}")
+                  association = send(reflection.name)
                   association.class.delete(association.id) unless association.nil?
                 end
                 before_destroy method_name
@@ -1535,7 +1535,7 @@ module ActiveRecord
 
           create_reflection(:has_one, association_id, options, self)
         end
-        
+
         def create_has_one_through_reflection(association_id, options)
           options.assert_valid_keys(
             :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :through, :source, :source_type, :validate
@@ -1927,7 +1927,7 @@ module ActiveRecord
             end
 
             def aliased_primary_key
-              "#{ aliased_prefix }_r0"
+              "#{aliased_prefix}_r0"
             end
 
             def aliased_table_name
@@ -1939,7 +1939,7 @@ module ActiveRecord
                 @column_names_with_alias = []
 
                 ([primary_key] + (column_names - [primary_key])).each_with_index do |column_name, i|
-                  @column_names_with_alias << [column_name, "#{ aliased_prefix }_r#{ i }"]
+                  @column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"]
                 end
               end
 
@@ -1976,11 +1976,11 @@ module ActiveRecord
               @aliased_prefix     = "t#{ join_dependency.joins.size }"
               @parent_table_name  = parent.active_record.table_name
               @aliased_table_name = aliased_table_name_for(table_name)
-              
+
               if reflection.macro == :has_and_belongs_to_many
                 @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join")
               end
-        
+
               if [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through]
                 @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join")
               end
@@ -2117,7 +2117,7 @@ module ActiveRecord
             end
 
             protected
-            
+
               def aliased_table_name_for(name, suffix = nil)
                 if !parent.table_joins.blank? && parent.table_joins.to_s.downcase =~ %r{join(\s+\w+)?\s+#{name.downcase}\son}
                   @join_dependency.table_aliases[name] += 1
@@ -2135,7 +2135,7 @@ module ActiveRecord
 
                 name
               end
-              
+
               def pluralize(table_name)
                 ActiveRecord::Base.pluralize_table_names ? table_name.to_s.pluralize : table_name
               end
-- 
cgit v1.2.3


From 4d092ba2089de185cc8f5a8d16432b348e102046 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Tue, 2 Sep 2008 10:59:20 +0200
Subject: Some performance goodness for AR.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/attribute_methods.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index ace335ed87..0a1baff87d 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -214,7 +214,7 @@ module ActiveRecord
             if logger
               logger.warn "Exception occurred during reader method compilation."
               logger.warn "Maybe #{attr_name} is not a valid Ruby identifier?"
-              logger.warn "#{err.message}"
+              logger.warn err.message
             end
           end
         end
-- 
cgit v1.2.3


From 288e947ae1737645985fde76f5382baaff700505 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Tue, 2 Sep 2008 15:33:49 +0200
Subject: Some performance goodness for inheritable attributes.

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/base.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index bc6d61301f..2367277c03 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -920,12 +920,12 @@ module ActiveRecord #:nodoc:
       # To start from an all-closed default and enable attributes as needed,
       # have a look at +attr_accessible+.
       def attr_protected(*attributes)
-        write_inheritable_attribute("attr_protected", Set.new(attributes.map(&:to_s)) + (protected_attributes || []))
+        write_inheritable_attribute(:attr_protected, Set.new(attributes.map(&:to_s)) + (protected_attributes || []))
       end
 
       # Returns an array of all the attributes that have been protected from mass-assignment.
       def protected_attributes # :nodoc:
-        read_inheritable_attribute("attr_protected")
+        read_inheritable_attribute(:attr_protected)
       end
 
       # Specifies a white list of model attributes that can be set via
@@ -953,22 +953,22 @@ module ActiveRecord #:nodoc:
       #   customer.credit_rating = "Average"
       #   customer.credit_rating # => "Average"
       def attr_accessible(*attributes)
-        write_inheritable_attribute("attr_accessible", Set.new(attributes.map(&:to_s)) + (accessible_attributes || []))
+        write_inheritable_attribute(:attr_accessible, Set.new(attributes.map(&:to_s)) + (accessible_attributes || []))
       end
 
       # Returns an array of all the attributes that have been made accessible to mass-assignment.
       def accessible_attributes # :nodoc:
-        read_inheritable_attribute("attr_accessible")
+        read_inheritable_attribute(:attr_accessible)
       end
 
        # Attributes listed as readonly can be set for a new record, but will be ignored in database updates afterwards.
        def attr_readonly(*attributes)
-         write_inheritable_attribute("attr_readonly", Set.new(attributes.map(&:to_s)) + (readonly_attributes || []))
+         write_inheritable_attribute(:attr_readonly, Set.new(attributes.map(&:to_s)) + (readonly_attributes || []))
        end
 
        # Returns an array of all the attributes that have been specified as readonly.
        def readonly_attributes
-         read_inheritable_attribute("attr_readonly")
+         read_inheritable_attribute(:attr_readonly)
        end
 
       # If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object,
@@ -992,7 +992,7 @@ module ActiveRecord #:nodoc:
 
       # Returns a hash of all the attributes that have been specified for serialization as keys and their class restriction as values.
       def serialized_attributes
-        read_inheritable_attribute("attr_serialized") or write_inheritable_attribute("attr_serialized", {})
+        read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {})
       end
 
 
-- 
cgit v1.2.3