Skip to content
This repository

Fix for #5200 #5210

Closed
wants to merge 2 commits into from

4 participants

Dave Desrochers Steve Klabnik Richard Schneeman Rafael Mendonça França
Dave Desrochers

reset_counters() was crashing when there were multiple belongs_to associations with the same foreign key.. with only one of the belongs_to associations having a custom counter_cache.

reset_counters() made the false assumption that each belongs_to association has a counter_cache.

Steve Klabnik
Collaborator

This doesn't merge properly any longer, mind updating it for HEAD?

Dave Desrochers
Pliny commented May 03, 2012

Ok, I merged the changes. Let me know if you need anything else from me. thanks!

Steve Klabnik
Collaborator

I'm not allowed to merge this, but yes, it merges cleanly now, thanks! It's all in @tenderlove or @jonleighton's hands now.

Richard Schneeman
Collaborator

3 months old PR, the fix looks straightforward, tests are included. Can we get some love from AR?

Rafael Mendonça França
Owner

This seems fine, but needs a rebase.

Dave Desrochers

@rafaelfranca I rebased it. Please let me know if you need anything else. thanks.

Rafael Mendonça França
Owner

Done

Rafael Mendonça França rafaelfranca closed this August 21, 2012
Mack Earnhardt macksmind referenced this pull request from a commit March 16, 2013
Commit has since been removed from the repository and is no longer available.
Mack Earnhardt macksmind referenced this pull request from a commit March 16, 2013
Commit has since been removed from the repository and is no longer available.
Mack Earnhardt macksmind referenced this pull request from a commit in macksmind/rails March 16, 2013
Mack Earnhardt Refactor Person/Friendship relationships to be more intuitive
PR #5210 added a Friendship model to illustrate a bug, but in doing so
created a confusing structure because both belongs_to declarations in
Friendship referred to the same side of the join. The new structure
maintains the integrity of the bug test while changing the follower
relationship to be more useful for other testing.
1d6eabb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Aug 06, 2012
Dave Desrochers This closes issue #5200. reset_counters() was crashing when there wer…
…e multiple belongs_to associations with the same foreign key.
5717654
Dave Desrochers Added required test files for #5200 0ce9f2d
This page is out of date. Refresh to see the latest.
2  activerecord/lib/active_record/counter_cache.rb
@@ -25,7 +25,7 @@ def reset_counters(id, *counters)
25 25
           foreign_key  = has_many_association.foreign_key.to_s
26 26
           child_class  = has_many_association.klass
27 27
           belongs_to   = child_class.reflect_on_all_associations(:belongs_to)
28  
-          reflection   = belongs_to.find { |e| e.foreign_key.to_s == foreign_key }
  28
+          reflection   = belongs_to.find { |e| e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? }
29 29
           counter_name = reflection.counter_cache_column
30 30
 
31 31
           stmt = unscoped.where(arel_table[primary_key].eq(object.id)).arel.compile_update({
11  activerecord/test/cases/counter_cache_test.rb
@@ -8,9 +8,11 @@
8 8
 require 'models/categorization'
9 9
 require 'models/dog'
10 10
 require 'models/dog_lover'
  11
+require 'models/person'
  12
+require 'models/friendship'
11 13
 
12 14
 class CounterCacheTest < ActiveRecord::TestCase
13  
-  fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers
  15
+  fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers, :people, :friendships
14 16
 
15 17
   class ::SpecialTopic < ::Topic
16 18
     has_many :special_replies, :foreign_key => 'parent_id'
@@ -109,4 +111,11 @@ class ::SpecialReply < ::Reply
109 111
       Topic.update_counters([t1.id, t2.id], :replies_count => 2)
110 112
     end
111 113
   end
  114
+
  115
+  test "reset the right counter if two have the same foreign key" do
  116
+    michael = people(:michael)
  117
+    assert_nothing_raised(ActiveRecord::StatementInvalid) do
  118
+      Person.reset_counters(michael.id, :followers)
  119
+    end
  120
+  end
112 121
 end
4  activerecord/test/fixtures/friendships.yml
... ...
@@ -0,0 +1,4 @@
  1
+Connection 1:
  2
+  id: 1
  3
+  person_id: 1
  4
+  friend_id: 2
3  activerecord/test/fixtures/people.yml
@@ -4,15 +4,18 @@ michael:
4 4
   primary_contact_id: 2
5 5
   number1_fan_id: 3
6 6
   gender: M
  7
+  followers_count: 1
7 8
 david:
8 9
   id: 2
9 10
   first_name: David
10 11
   primary_contact_id: 3
11 12
   number1_fan_id: 1
12 13
   gender: M
  14
+  followers_count: 1
13 15
 susan:
14 16
   id: 3
15 17
   first_name: Susan
16 18
   primary_contact_id: 2
17 19
   number1_fan_id: 1
18 20
   gender: F
  21
+  followers_count: 1
4  activerecord/test/models/friendship.rb
... ...
@@ -0,0 +1,4 @@
  1
+class Friendship < ActiveRecord::Base
  2
+  belongs_to :friend, class_name: 'Person'
  3
+  belongs_to :follower, foreign_key: 'friend_id', class_name: 'Person', counter_cache: :followers_count
  4
+end
2  activerecord/test/models/person.rb
@@ -8,6 +8,8 @@ class Person < ActiveRecord::Base
8 8
   has_many :posts_with_no_comments, -> { includes(:comments).where('comments.id is null').references(:comments) },
9 9
                                     :through => :readers, :source => :post
10 10
 
  11
+  has_many :followers, foreign_key: 'friend_id', class_name: 'Friendship'
  12
+
11 13
   has_many :references
12 14
   has_many :bad_references
13 15
   has_many :fixed_bad_references, -> { where :favourite => true }, :class_name => 'BadReference'
6  activerecord/test/schema/schema.rb
@@ -270,6 +270,11 @@ def create_table(*args, &block)
270 270
     t.string :name
271 271
   end
272 272
 
  273
+  create_table :friendships, :force => true do |t|
  274
+    t.integer :friend_id
  275
+    t.integer :person_id
  276
+  end
  277
+
273 278
   create_table :goofy_string_id, :force => true, :id => false do |t|
274 279
     t.string :id, :null => false
275 280
     t.string :info
@@ -476,6 +481,7 @@ def create_table(*args, &block)
476 481
     t.references :number1_fan
477 482
     t.integer    :lock_version, :null => false, :default => 0
478 483
     t.string     :comments
  484
+    t.integer    :followers_count, :default => 0
479 485
     t.references :best_friend
480 486
     t.references :best_friend_of
481 487
     t.timestamps
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.