diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 15 | ||||
| -rw-r--r-- | activerecord/lib/active_record/counter_cache.rb | 28 | ||||
| -rw-r--r-- | activerecord/test/cases/counter_cache_test.rb | 159 |
3 files changed, 98 insertions, 104 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5fa70e4aad..b59d24f88d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,16 @@ -* Add option to update specified timestamp columns when incrementing, - decrementing, resetting, or updating counter caches. +* Add `touch` option to counter cache modifying methods. + + Works when updating, resetting, incrementing and decrementing counters: + + # Touches `updated_at`/`updated_on`. + Topic.increment_counter(:messages_count, 1, touch: true) + Topic.decrement_counter(:messages_count, 1, touch: true) + + # Touches `last_discussed_at`. + Topic.reset_counters(18, :messages, touch: :last_discussed_at) + + # Touches `updated_at` and `last_discussed_at`. + Topic.update_counters(18, messages_count: 5, touch: %i( updated_at last_discussed_at )) Fixes #26724. diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index af29b4c5bb..9355a2f7a7 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -47,12 +47,9 @@ module ActiveRecord updates = { counter_name.to_sym => object.send(counter_association).count(:all) } - touch_time = object.send(:current_time_from_proper_timezone) - resolve_timestamp_columns(object, touch).each do |column| - updates[column] = touch_time - end - - unscoped.where(primary_key => object.id).update_all(updates) + unscoped.where(primary_key => object.id).update_all( + updates.merge(touch_updates(object, touch)) + ) end return true end @@ -109,10 +106,7 @@ module ActiveRecord if touch object = find(id) - touch_time = object.send(:current_time_from_proper_timezone) - timestamps = resolve_timestamp_columns(object, touch) - - timestamps.map do |column| + touch_updates(object, touch).map do |column, touch_time| updates << "#{connection.quote_column_name(column.to_s)} = #{connection.quote(touch_time)}" end end @@ -144,7 +138,7 @@ module ActiveRecord # # and update the updated_at value. # DiscussionBoard.increment_counter(:posts_count, 5, touch: true) def increment_counter(counter_name, id, touch: nil) - update_counters(id, { counter_name => 1 }.merge(touch: touch)) + update_counters(id, counter_name => 1, touch: touch) end # Decrement a numeric field by one, via a direct SQL update. @@ -169,16 +163,14 @@ module ActiveRecord # # and update the updated_at value. # DiscussionBoard.decrement_counter(:posts_count, 5, touch: true) def decrement_counter(counter_name, id, touch: nil) - update_counters(id, { counter_name => -1 }.merge(touch: touch)) + update_counters(id, counter_name => -1, touch: touch) end private - def resolve_timestamp_columns(object, touch) - if touch == true - object.send(:timestamp_attributes_for_update_in_model) - else - Array(touch) - end + def touch_updates(object, touch) + touch = object.send(:timestamp_attributes_for_update_in_model) if touch == true + touch_time = object.send(:current_time_from_proper_timezone) + Array(touch).map { |column| [ column, touch_time ] }.to_h end end diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 478b44008d..c735e13715 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -212,144 +212,135 @@ class CounterCacheTest < ActiveRecord::TestCase end end - test "does not update counters with touch: false" do - previous_updated_at = @topic.updated_at - Topic.update_counters(@topic.id, replies_count: -1, touch: false) - assert_equal previous_updated_at, @topic.reload.updated_at + test "update counters doesn't touch timestamps by default" do + @topic.update_column :updated_at, 5.minutes.ago + previously_updated_at = @topic.updated_at + + Topic.update_counters(@topic.id, replies_count: -1) + + assert_equal previously_updated_at, @topic.updated_at end test "update counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.update_counters(@topic.id, replies_count: -1, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.update_counters(@topic.id, replies_count: -1, touch: true) + end end test "update multiple counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: true) + end end test "reset counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.reset_counters(@topic.id, :replies, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.reset_counters(@topic.id, :replies, touch: true) + end end test "reset multiple counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) - Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) + Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: true) + end end test "increment counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.increment_counter(:replies_count, @topic.id, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.increment_counter(:replies_count, @topic.id, touch: true) + end end test "decrement counters with touch: true" do - previous_updated_at = @topic.updated_at - Topic.decrement_counter(:replies_count, @topic.id, touch: true) - assert_not_equal previous_updated_at, @topic.reload.updated_at + assert_touching @topic, :updated_at do + Topic.decrement_counter(:replies_count, @topic.id, touch: true) + end end test "update counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.update_counters(@topic.id, replies_count: -1, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.update_counters(@topic.id, replies_count: -1, touch: :written_on) + end end test "update multiple counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: :written_on) + end end test "reset counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.reset_counters(@topic.id, :replies, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.reset_counters(@topic.id, :replies, touch: :written_on) + end end test "reset multiple counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) - Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) + Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: :written_on) + end end test "increment counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.increment_counter(:replies_count, @topic.id, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.increment_counter(:replies_count, @topic.id, touch: :written_on) + end end test "decrement counters with touch: :written_on" do - previous_written_on = @topic.written_on - Topic.decrement_counter(:replies_count, @topic.id, touch: :written_on) - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :written_on do + Topic.decrement_counter(:replies_count, @topic.id, touch: :written_on) + end end test "update counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on - - Topic.update_counters(@topic.id, replies_count: -1, touch: %i( updated_at written_on )) - - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.update_counters(@topic.id, replies_count: -1, touch: %i( updated_at written_on )) + end end test "update multiple counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on - - Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: %i( updated_at written_on )) - - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.update_counters(@topic.id, replies_count: 2, unique_replies_count: 2, touch: %i( updated_at written_on )) + end end test "reset counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on - - Topic.reset_counters(@topic.id, :replies, touch: %i( updated_at written_on )) - - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.reset_counters(@topic.id, :replies, touch: %i( updated_at written_on )) + end end test "reset multiple counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on - - Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) - Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: %i( updated_at written_on )) - - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.update_counters(@topic.id, replies_count: 1, unique_replies_count: 1) + Topic.reset_counters(@topic.id, :replies, :unique_replies, touch: %i( updated_at written_on )) + end end test "increment counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on - - Topic.increment_counter(:replies_count, @topic.id, touch: %i( updated_at written_on )) - - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.increment_counter(:replies_count, @topic.id, touch: %i( updated_at written_on )) + end end test "decrement counters with touch: %i( updated_at written_on )" do - previous_updated_at = @topic.updated_at - previous_written_on = @topic.written_on + assert_touching @topic, :updated_at, :written_on do + Topic.decrement_counter(:replies_count, @topic.id, touch: %i( updated_at written_on )) + end + end - Topic.decrement_counter(:replies_count, @topic.id, touch: %i( updated_at written_on )) + private + def assert_touching(record, *attributes) + record.update_columns attributes.map { |attr| [ attr, 5.minutes.ago ] }.to_h + touch_times = attributes.map { |attr| [ attr, record.public_send(attr) ] }.to_h - assert_not_equal previous_updated_at, @topic.reload.updated_at - assert_not_equal previous_written_on, @topic.reload.written_on - end + yield + + touch_times.each do |attr, previous_touch_time| + assert_operator previous_touch_time, :<, record.reload.public_send(attr) + end + end end |