From 23bb8d77c6c5ace445659d56cf5df2adcf6c8dc3 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 3 Feb 2015 13:25:15 -0700 Subject: Correct errors in counter cache updating The cache name should be converted to a string when given, not compared as a symbol. This edge case is already adequately covered by our tests, but was masked by another issue where we were incorrectly updating the counter cache twice. When paired with a bug where we didn't update the counter cache because we couldn't find a match with the name, this made it look like everything was working fine. Fixes #10865. --- activerecord/CHANGELOG.md | 7 +++++++ .../lib/active_record/associations/has_many_association.rb | 6 +++++- .../lib/active_record/associations/has_many_through_association.rb | 4 ++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index af692adad6..0105dfa78c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fixed several edge cases which could result in a counter cache updating + twice or not updating at all for `has_many` and `has_many :through`. + + Fixes #10865. + + *Sean Griffin* + * Foreign keys added by migrations were given random, generated names. This meant a different `structure.sql` would be generated every time a developer ran migrations on their machine. diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index b574185561..ca27c9fdde 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -85,7 +85,11 @@ module ActiveRecord end def cached_counter_attribute_name(reflection = reflection()) - options[:counter_cache] || "#{reflection.name}_count" + if reflection.options[:counter_cache] + reflection.options[:counter_cache].to_s + else + "#{reflection.name}_count" + end end def update_counter(difference, reflection = reflection()) 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 4fcbc98c62..4897ec44e9 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -160,9 +160,9 @@ module ActiveRecord if through_reflection.collection? && update_through_counter?(method) update_counter(-count, through_reflection) + else + update_counter(-count) end - - update_counter(-count) end def through_records_for(record) -- cgit v1.2.3