From 83752373b9c83bf585b5fe44836c65612b680cf9 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Wed, 7 Feb 2007 16:10:40 +0000 Subject: Made increment_counter/decrement_counter play nicely with optimistic locking, and added a more general update_counters method git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6139 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/base.rb | 23 +++++++++- .../lib/active_record/locking/optimistic.rb | 25 ++++++++++- activerecord/test/locking_test.rb | 51 ++++++++++++++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index ba42776c7f..ed1b83cdca 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Made increment_counter/decrement_counter play nicely with optimistic locking, and added a more general update_counters method [Jamis Buck] + * Reworked David's query cache to be available as Model.cache {...}. For the duration of the block no select query should be run more then once. Any inserts/deletes/executes will flush the whole cache however [Tobias Luetke] Task.cache { Task.find(1); Task.find(1) } #=> 1 query diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index af3d1997c5..2127f64a1b 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -519,18 +519,37 @@ module ActiveRecord #:nodoc: connection.select_value(sql, "#{name} Count").to_i end + # A generic "counter updater" implementation, intended primarily to be + # used by increment_counter and decrement_counter, but which may also + # be useful on its own. It simply does a direct SQL update for the record + # with the given ID, altering the given hash of counters by the amount + # given by the corresponding value: + # + # Post.update_counters 5, :comment_count => -1, :action_count => 1 + # # UPDATE posts + # # SET comment_count = comment_count - 1, + # # action_count = action_count + 1 + # # WHERE id = 5 + def update_counters(id, counters) + updates = counters.inject([]) { |list, (counter_name, increment)| + sign = increment < 0 ? "-" : "+" + list << "#{counter_name} = #{counter_name} #{sign} #{increment.abs}" + }.join(", ") + update_all(updates, "#{primary_key} = #{quote_value(id)}") + end + # Increments the specified counter by one. So DiscussionBoard.increment_counter("post_count", # discussion_board_id) would increment the "post_count" counter on the board responding to discussion_board_id. # This is used for caching aggregate values, so that they don't need to be computed every time. Especially important # for looping over a collection where each element require a number of aggregate values. Like the DiscussionBoard # that needs to list both the number of posts and comments. def increment_counter(counter_name, id) - update_all "#{counter_name} = #{counter_name} + 1", "#{primary_key} = #{quote_value(id)}" + update_counters(id, counter_name => 1) end # Works like increment_counter, but decrements instead. def decrement_counter(counter_name, id) - update_all "#{counter_name} = #{counter_name} - 1", "#{primary_key} = #{quote_value(id)}" + update_counters(id, counter_name => -1) end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 02cf565017..217bd3def7 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -31,14 +31,14 @@ module ActiveRecord base.alias_method_chain :update, :lock base.alias_method_chain :attributes_from_column_definition, :lock - + class << base alias_method :locking_column=, :set_locking_column end end def locking_enabled? #:nodoc: - lock_optimistically && respond_to?(self.class.locking_column) + self.class.locking_enabled? end def attributes_from_column_definition_with_lock @@ -80,6 +80,20 @@ module ActiveRecord module ClassMethods DEFAULT_LOCKING_COLUMN = 'lock_version' + def self.extended(base) + class < 1) if locking_enabled? + update_counters_without_lock(id, counters) + end end end end diff --git a/activerecord/test/locking_test.rb b/activerecord/test/locking_test.rb index 471771bfbb..f62f1ac61c 100644 --- a/activerecord/test/locking_test.rb +++ b/activerecord/test/locking_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'fixtures/person' +require 'fixtures/reader' require 'fixtures/legacy_thing' class LockWithoutDefault < ActiveRecord::Base; end @@ -12,6 +13,11 @@ end class OptimisticLockingTest < Test::Unit::TestCase fixtures :people, :legacy_things + # need to disable transactional fixtures, because otherwise the sqlite3 + # adapter (at least) chokes when we try and change the schema in the middle + # of a test (see test_increment_counter_*). + self.use_transactional_fixtures = false + def test_lock_existing p1 = Person.find(1) p2 = Person.find(1) @@ -73,6 +79,51 @@ class OptimisticLockingTest < Test::Unit::TestCase t1 = LockWithCustomColumnWithoutDefault.new assert_equal 0, t1.custom_lock_version end + + { :lock_version => Person, :custom_lock_version => LegacyThing }.each do |name, model| + define_method("test_increment_counter_updates_#{name}") do + counter_test model, 1 do |id| + model.increment_counter :test_count, id + end + end + + define_method("test_decrement_counter_updates_#{name}") do + counter_test model, -1 do |id| + model.decrement_counter :test_count, id + end + end + + define_method("test_update_counters_updates_#{name}") do + counter_test model, 1 do |id| + model.update_counters id, :test_count => 1 + end + end + end + + private + + def add_counter_column_to(model) + model.connection.add_column model.table_name, :test_count, :integer, :null => false, :default => 0 + model.reset_column_information + end + + def remove_counter_column_from(model) + model.connection.remove_column model.table_name, :test_count + model.reset_column_information + end + + def counter_test(model, expected_count) + add_counter_column_to(model) + object = model.find(:first) + assert_equal 0, object.test_count + assert_equal 0, object.send(model.locking_column) + yield object.id + object.reload + assert_equal expected_count, object.test_count + assert_equal 1, object.send(model.locking_column) + ensure + remove_counter_column_from(model) + end end -- cgit v1.2.3