diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b535b35ba0..5fa70e4aad 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Add option to update specified timestamp columns when incrementing, + decrementing, resetting, or updating counter caches. + + Fixes #26724. + + *Jarred Trost* + * Remove deprecated `#uniq`, `#uniq!`, and `#uniq_value`. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index e2da512813..af29b4c5bb 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -12,13 +12,21 @@ module ActiveRecord # # * +id+ - The id of the object you wish to reset a counter on. # * +counters+ - One or more association counters to reset. Association name or counter name can be given. + # * :touch - Touch timestamp columns when updating. + # Pass +true+ to touch +updated_at+ and/or +updated_on+. Pass a symbol to + # touch that column or an array of symbols to touch just those ones. # # ==== Examples # # # For Post with id #1 records reset the comments_count # Post.reset_counters(1, :comments) - def reset_counters(id, *counters) + # + # # For Post with id #1 records reset the comments_count + # # and updates the +updated_at+ and/or +updated_on+ attributes. + # Post.reset_counters(1, :comments, touch: true) + def reset_counters(id, *counters, touch: nil) object = find(id) + counters.each do |counter_association| has_many_association = _reflect_on_association(counter_association) unless has_many_association @@ -37,9 +45,14 @@ module ActiveRecord reflection = child_class._reflections.values.find { |e| e.belongs_to? && e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? } counter_name = reflection.counter_cache_column - unscoped.where(primary_key => object.id).update_all( - counter_name => object.send(counter_association).count(:all) - ) + 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) end return true end @@ -55,6 +68,9 @@ module ActiveRecord # * +id+ - The id of the object you wish to update a counter on or an array of ids. # * +counters+ - A Hash containing the names of the fields # to update as keys and the amount to update the field by as values. + # * :touch option - Touch timestamp columns when updating. + # Pass +true+ to touch +updated_at+ and/or +updated_on+. Pass a symbol to + # touch that column or an array of symbols to touch just those ones. # # ==== Examples # @@ -73,13 +89,34 @@ module ActiveRecord # # UPDATE posts # # SET comment_count = COALESCE(comment_count, 0) + 1 # # WHERE id IN (10, 15) + # + # # For the Posts with id of 10 and 15, increment the comment_count by 1 + # # and update the updated_at value for each counter. + # Post.update_counters [10, 15], comment_count: 1, touch: true + # # Executes the following SQL: + # # UPDATE posts + # # SET comment_count = COALESCE(comment_count, 0) + 1, + # # `updated_at` = '2016-10-13T09:59:23-05:00' + # # WHERE id IN (10, 15) def update_counters(id, counters) + touch = counters.delete(:touch) + updates = counters.map do |counter_name, value| operator = value < 0 ? "-" : "+" quoted_column = connection.quote_column_name(counter_name) "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{value.abs}" end + if touch + object = find(id) + touch_time = object.send(:current_time_from_proper_timezone) + timestamps = resolve_timestamp_columns(object, touch) + + timestamps.map do |column| + updates << "#{connection.quote_column_name(column.to_s)} = #{connection.quote(touch_time)}" + end + end + unscoped.where(primary_key => id).update_all updates.join(", ") end @@ -94,13 +131,20 @@ module ActiveRecord # # * +counter_name+ - The name of the field that should be incremented. # * +id+ - The id of the object that should be incremented or an array of ids. + # * :touch - Touch timestamp columns when updating. + # Pass +true+ to touch +updated_at+ and/or +updated_on+. Pass a symbol to + # touch that column or an array of symbols to touch just those ones. # # ==== Examples # # # Increment the posts_count column for the record with an id of 5 # DiscussionBoard.increment_counter(:posts_count, 5) - def increment_counter(counter_name, id) - update_counters(id, counter_name => 1) + # + # # Increment the posts_count column for the record with an id of 5 + # # 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)) end # Decrement a numeric field by one, via a direct SQL update. @@ -112,14 +156,30 @@ module ActiveRecord # # * +counter_name+ - The name of the field that should be decremented. # * +id+ - The id of the object that should be decremented or an array of ids. + # * :touch - Touch timestamp columns when updating. + # Pass +true+ to touch +updated_at+ and/or +updated_on+. Pass a symbol to + # touch that column or an array of symbols to touch just those ones. # # ==== Examples # # # Decrement the posts_count column for the record with an id of 5 # DiscussionBoard.decrement_counter(:posts_count, 5) - def decrement_counter(counter_name, id) - update_counters(id, counter_name => -1) + # + # # Decrement the posts_count column for the record with an id of 5 + # # 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)) end + + private + def resolve_timestamp_columns(object, touch) + if touch == true + object.send(:timestamp_attributes_for_update_in_model) + else + Array(touch) + end + end end private diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index 84f2c3a465..478b44008d 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -211,4 +211,145 @@ class CounterCacheTest < ActiveRecord::TestCase aircraft.wheels.first.destroy 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + 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 + end + + test "decrement counters with touch: %i( updated_at written_on )" do + previous_updated_at = @topic.updated_at + previous_written_on = @topic.written_on + + Topic.decrement_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 + end end