Rails Lighthouse ticket #2907

adapted_fix_for_2907.diff

From 9c58b3d6f6846896e2bd17c2e5d0fdb9b7d82006 Mon Sep 17 00:00:00 2001
From: Tanja Otto <[email protected]>
Date: Sun, 16 May 2010 21:03:14 +0200
Subject: [PATCH 1/2] adapted testcases for #2907 of Balint Erdi because apply of his diff doesn't work in master [#2907 state:commited]

---
 .../associations/belongs_to_associations_test.rb   |    6 +++---
 activerecord/test/cases/base_test.rb               |    2 +-
 activerecord/test/cases/reflection_test.rb         |    6 +++---
 activerecord/test/models/reply.rb                  |    2 +-
 activerecord/test/schema/schema.rb                 |    1 +
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index be77ee4..dff55c5 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -328,15 +328,15 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
 
   def test_custom_counter_cache
     reply = Reply.create(:title => "re: zoom", :content => "speedy quick!")
-    assert_equal 0, reply[:replies_count]
+    assert_equal 0, reply[:inane_replies_count]
 
     silly = SillyReply.create(:title => "gaga", :content => "boo-boo")
     silly.reply = reply
 
-    assert_equal 1, reply.reload[:replies_count]
+    assert_equal 1, reply.reload[:inane_replies_count]
     assert_equal 1, reply.replies.size
 
-    reply[:replies_count] = 17
+    reply[:inane_replies_count] = 17
     assert_equal 17, reply.replies.size
   end
 
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index b7ae619..79621cf 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2237,7 +2237,7 @@ class BasicsTest < ActiveRecord::TestCase
 
   def test_inspect_instance
     topic = topics(:first)
-    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
+    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, inane_replies_count: 0, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
   end
 
   def test_inspect_new_instance
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 6781862..d99d985 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -24,18 +24,18 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_read_attribute_names
     assert_equal(
-      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count parent_id parent_title type ).sort,
+      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count inane_replies_count parent_id parent_title type ).sort,
       @first.attribute_names
     )
   end
 
   def test_columns
-    assert_equal 13, Topic.columns.length
+    assert_equal 14, Topic.columns.length
   end
 
   def test_columns_are_returned_in_the_order_they_were_declared
     column_names = Topic.columns.map { |column| column.name }
-    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count parent_id parent_title type), column_names
+    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count inane_replies_count parent_id parent_title type), column_names
   end
 
   def test_content_columns
diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb
index 6cc9ee0..2e71bc4 100644
--- a/activerecord/test/models/reply.rb
+++ b/activerecord/test/models/reply.rb
@@ -47,7 +47,7 @@ class WrongReply < Reply
 end
 
 class SillyReply < Reply
-  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :replies_count
+  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :inane_replies_count
 end
 
 module Web
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index f5fba2f..fbd9c4e 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -482,6 +482,7 @@ ActiveRecord::Schema.define do
     end
     t.boolean  :approved, :default => true
     t.integer  :replies_count, :default => 0
+    t.integer  :inane_replies_count, :default => 0
     t.integer  :parent_id
     t.string   :parent_title
     t.string   :type
-- 
1.7.0.3


From 988a4951373d098f561dce8e20c0fb60f3d7b2c2 Mon Sep 17 00:00:00 2001
From: Tanja Otto <[email protected]>
Date: Sun, 16 May 2010 22:12:55 +0200
Subject: [PATCH 2/2] adapted fix for #2907 of Balint Erdi because apply of his diff doesn't work in master [#2907 state:commited]

---
 .../associations/has_many_association.rb           |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 0464e8c..af0ff0e 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -57,7 +57,12 @@ module ActiveRecord
         end
 
         def cached_counter_attribute_name
-          "#{@reflection.name}_count"
+          ( belongs_to_association_in_target && belongs_to_association_in_target.counter_cache_column ) || "#{@reflection.name}_count"
+        end
+          
+        def belongs_to_association_in_target
+          target_belongs_to_associations = @reflection.klass.reflect_on_all_associations.select(&:belongs_to?)
+          target_belongs_to_associations.detect { |a| a.class_name == @owner.class.to_s }
         end
 
         def insert_record(record, force = false, validate = true)
-- 
1.7.0.3

adapted_test_cases_for_2907.diff

From 9c58b3d6f6846896e2bd17c2e5d0fdb9b7d82006 Mon Sep 17 00:00:00 2001
From: Tanja Otto <[email protected]>
Date: Sun, 16 May 2010 21:03:14 +0200
Subject: [PATCH] adapted testcases for #2907 of Balint Erdi because apply of his diff doesn't work in master [#2907 state:commited]

---
 .../associations/belongs_to_associations_test.rb   |    6 +++---
 activerecord/test/cases/base_test.rb               |    2 +-
 activerecord/test/cases/reflection_test.rb         |    6 +++---
 activerecord/test/models/reply.rb                  |    2 +-
 activerecord/test/schema/schema.rb                 |    1 +
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index be77ee4..dff55c5 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -328,15 +328,15 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
 
   def test_custom_counter_cache
     reply = Reply.create(:title => "re: zoom", :content => "speedy quick!")
-    assert_equal 0, reply[:replies_count]
+    assert_equal 0, reply[:inane_replies_count]
 
     silly = SillyReply.create(:title => "gaga", :content => "boo-boo")
     silly.reply = reply
 
-    assert_equal 1, reply.reload[:replies_count]
+    assert_equal 1, reply.reload[:inane_replies_count]
     assert_equal 1, reply.replies.size
 
-    reply[:replies_count] = 17
+    reply[:inane_replies_count] = 17
     assert_equal 17, reply.replies.size
   end
 
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index b7ae619..79621cf 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2237,7 +2237,7 @@ class BasicsTest < ActiveRecord::TestCase
 
   def test_inspect_instance
     topic = topics(:first)
-    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
+    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, inane_replies_count: 0, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
   end
 
   def test_inspect_new_instance
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 6781862..d99d985 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -24,18 +24,18 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_read_attribute_names
     assert_equal(
-      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count parent_id parent_title type ).sort,
+      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count inane_replies_count parent_id parent_title type ).sort,
       @first.attribute_names
     )
   end
 
   def test_columns
-    assert_equal 13, Topic.columns.length
+    assert_equal 14, Topic.columns.length
   end
 
   def test_columns_are_returned_in_the_order_they_were_declared
     column_names = Topic.columns.map { |column| column.name }
-    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count parent_id parent_title type), column_names
+    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count inane_replies_count parent_id parent_title type), column_names
   end
 
   def test_content_columns
diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb
index 6cc9ee0..2e71bc4 100644
--- a/activerecord/test/models/reply.rb
+++ b/activerecord/test/models/reply.rb
@@ -47,7 +47,7 @@ class WrongReply < Reply
 end
 
 class SillyReply < Reply
-  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :replies_count
+  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :inane_replies_count
 end
 
 module Web
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index f5fba2f..fbd9c4e 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -482,6 +482,7 @@ ActiveRecord::Schema.define do
     end
     t.boolean  :approved, :default => true
     t.integer  :replies_count, :default => 0
+    t.integer  :inane_replies_count, :default => 0
     t.integer  :parent_id
     t.string   :parent_title
     t.string   :type
-- 
1.7.0.3

collection_size_correct_through_custom_counter_cache.diff

From 615d15a37ab7dbeaa1e246af0a86e28c3ba63106 Mon Sep 17 00:00:00 2001
From: Balint Erdi <[email protected]>
Date: Fri, 15 Jan 2010 13:32:06 +0100
Subject: [PATCH] association collection size returns correct value through custom cache_counter

---
 .../associations/has_many_association.rb           |   17 +++++++++++------
 .../associations/belongs_to_associations_test.rb   |    4 ++--
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 4e113be..0ea5015 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -40,11 +40,11 @@ module ActiveRecord
           # we are certain the current target is an empty array. This is a
           # documented side-effect of the method that may avoid an extra SELECT.
           @target ||= [] and loaded if count == 0
-          
+
           if @reflection.options[:limit]
             count = [ @reflection.options[:limit], count ].min
           end
-          
+
           return count
         end
 
@@ -53,7 +53,12 @@ module ActiveRecord
         end
 
         def cached_counter_attribute_name
-          "#{@reflection.name}_count"
+          ( belongs_to_association_in_target && belongs_to_association_in_target.counter_cache_column ) || "#{@reflection.name}_count"
+        end
+
+        def belongs_to_association_in_target
+          target_belongs_to_associations = @reflection.klass.reflect_on_all_associations.select(&:belongs_to?)
+          target_belongs_to_associations.detect { |a| a.class_name == @owner.class.to_s }
         end
 
         def insert_record(record, force = false, validate = true)
@@ -71,7 +76,7 @@ module ActiveRecord
             else
               ids = quoted_record_ids(records)
               @reflection.klass.update_all(
-                "#{@reflection.primary_key_name} = NULL", 
+                "#{@reflection.primary_key_name} = NULL",
                 "#{@reflection.primary_key_name} = #{owner_quoted_id} AND #{@reflection.klass.primary_key} IN (#{ids})"
               )
               @owner.class.update_counters(@owner.id, cached_counter_attribute_name => -records.size) if has_cached_counter?
@@ -88,11 +93,11 @@ module ActiveRecord
               @finder_sql = interpolate_sql(@reflection.options[:finder_sql])
 
             when @reflection.options[:as]
-              @finder_sql = 
+              @finder_sql =
                 "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " +
                 "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}"
               @finder_sql << " AND (#{conditions})" if conditions
-            
+
             else
               @finder_sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}"
               @finder_sql << " AND (#{conditions})" if conditions
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 5769066..868acf4 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -307,7 +307,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
 
   def test_custom_counter_cache
     reply = Reply.create(:title => "re: zoom", :content => "speedy quick!")
-    assert_equal 0, reply[:replies_count]
+    assert_equal 0, reply[:inane_replies_count]
 
     silly = SillyReply.create(:title => "gaga", :content => "boo-boo")
     silly.reply = reply
@@ -315,7 +315,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_equal 1, reply.reload[:inane_replies_count]
     assert_equal 1, reply.replies.size
 
-    reply[:replies_count] = 17
+    reply[:inane_replies_count] = 17
     assert_equal 17, reply.replies.size
   end
 
-- 
1.6.5

failing_unit_test_for_2907.diff

From 4031493cf8eb85d25c2a1ff23ddd657c2123d387 Mon Sep 17 00:00:00 2001
From: Balint Erdi <[email protected]>
Date: Fri, 15 Jan 2010 11:22:10 +0100
Subject: [PATCH] failing unit test for ticket 2907-custom-counter_cache-is-not-used-when-using-collectionsize-method

https://rails.lighthouseapp.com/projects/8994/tickets/2907-custom-counter_cache-is-not-used-when-using-collectionsize-method
---
 .../associations/belongs_to_associations_test.rb   |    2 +-
 activerecord/test/cases/base_test.rb               |    2 +-
 activerecord/test/cases/reflection_test.rb         |    6 +++---
 activerecord/test/models/reply.rb                  |    2 +-
 activerecord/test/schema/schema.rb                 |    1 +
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 9f3945f..5769066 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -312,7 +312,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     silly = SillyReply.create(:title => "gaga", :content => "boo-boo")
     silly.reply = reply
 
-    assert_equal 1, reply.reload[:replies_count]
+    assert_equal 1, reply.reload[:inane_replies_count]
     assert_equal 1, reply.replies.size
 
     reply[:replies_count] = 17
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 6f7a779..ab47fc7 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2084,7 +2084,7 @@ class BasicsTest < ActiveRecord::TestCase
 
   def test_inspect_instance
     topic = topics(:first)
-    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
+    assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "[email protected]", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, inane_replies_count: 0, parent_id: nil, parent_title: nil, type: nil>), topic.inspect
   end
 
   def test_inspect_new_instance
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 3278300..d54a5da 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -24,18 +24,18 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_read_attribute_names
     assert_equal(
-      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count parent_id parent_title type ).sort,
+      %w( id title author_name author_email_address bonus_time written_on last_read content approved replies_count inane_replies_count parent_id parent_title type ).sort,
       @first.attribute_names
     )
   end
 
   def test_columns
-    assert_equal 13, Topic.columns.length
+    assert_equal 14, Topic.columns.length
   end
 
   def test_columns_are_returned_in_the_order_they_were_declared
     column_names = Topic.columns.map { |column| column.name }
-    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count parent_id parent_title type), column_names
+    assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count inane_replies_count parent_id parent_title type), column_names
   end
 
   def test_content_columns
diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb
index 4063785..a173678 100644
--- a/activerecord/test/models/reply.rb
+++ b/activerecord/test/models/reply.rb
@@ -36,7 +36,7 @@ class Reply < Topic
 end
 
 class SillyReply < Reply
-  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :replies_count
+  belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :inane_replies_count
 end
 
 module Web
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index c8e652a..8f62293 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -445,6 +445,7 @@ ActiveRecord::Schema.define do
     t.text     :content
     t.boolean  :approved, :default => true
     t.integer  :replies_count, :default => 0
+    t.integer  :inane_replies_count, :default => 0
     t.integer  :parent_id
     t.string   :parent_title
     t.string   :type
-- 
1.6.5