aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activerecord/CHANGELOG2
-rw-r--r--activerecord/lib/active_record/locking/optimistic.rb39
-rw-r--r--activerecord/test/locking_test.rb14
3 files changed, 39 insertions, 16 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 0bed07029b..a0a1addd62 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Optimistic locking: revert the lock version when an update fails. #7840 [plang]
+
* Migrations: add_column supports custom column types. #7742 [jsgarvin, Theory]
* Load database adapters on demand. Eliminates config.connection_adapters and RAILS_CONNECTION_ADAPTERS. Add your lib directory to the $LOAD_PATH and put your custom adapter in lib/active_record/connection_adapters/adaptername_adapter.rb. This way you can provide custom adapters as plugins or gems without modifying Rails. [Jeremy Kemper]
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 23e7239d5e..d8f9dee372 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -2,7 +2,7 @@ module ActiveRecord
module Locking
# == What is Optimistic Locking
#
- # Optimistic locking allows multiple users to access the same record for edits, and assumes a minimum of
+ # Optimistic locking allows multiple users to access the same record for edits, and assumes a minimum of
# conflicts with the data. It does this by checking whether another process has made changes to a record since
# it was opened, an ActiveRecord::StaleObjectError is thrown if that has occurred and the update is ignored.
#
@@ -16,10 +16,10 @@ module ActiveRecord
#
# p1 = Person.find(1)
# p2 = Person.find(1)
- #
+ #
# p1.first_name = "Michael"
# p1.save
- #
+ #
# p2.first_name = "should fail"
# p2.save # Raises a ActiveRecord::StaleObjectError
#
@@ -53,16 +53,16 @@ module ActiveRecord
private
def attributes_from_column_definition_with_lock
result = attributes_from_column_definition_without_lock
-
+
# If the locking column has no default value set,
# start the lock version at zero. Note we can't use
# locking_enabled? at this point as @attributes may
# not have been initialized yet
-
+
if lock_optimistically && result.include?(self.class.locking_column)
result[self.class.locking_column] ||= 0
end
-
+
return result
end
@@ -73,18 +73,25 @@ module ActiveRecord
previous_value = send(lock_col)
send(lock_col + '=', previous_value + 1)
- affected_rows = connection.update(<<-end_sql, "#{self.class.name} Update with optimistic locking")
- UPDATE #{self.class.table_name}
- SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))}
- WHERE #{self.class.primary_key} = #{quote_value(id)}
- AND #{self.class.quoted_locking_column} = #{quote_value(previous_value)}
- end_sql
+ begin
+ affected_rows = connection.update(<<-end_sql, "#{self.class.name} Update with optimistic locking")
+ UPDATE #{self.class.table_name}
+ SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))}
+ WHERE #{self.class.primary_key} = #{quote_value(id)}
+ AND #{self.class.quoted_locking_column} = #{quote_value(previous_value)}
+ end_sql
- unless affected_rows == 1
- raise ActiveRecord::StaleObjectError, "Attempted to update a stale object"
- end
+ unless affected_rows == 1
+ raise ActiveRecord::StaleObjectError, "Attempted to update a stale object"
+ end
- return true
+ affected_rows
+
+ # If something went wrong, revert the version.
+ rescue Exception
+ send(lock_col + '=', previous_value)
+ raise
+ end
end
module ClassMethods
diff --git a/activerecord/test/locking_test.rb b/activerecord/test/locking_test.rb
index f62f1ac61c..3194727822 100644
--- a/activerecord/test/locking_test.rb
+++ b/activerecord/test/locking_test.rb
@@ -30,6 +30,20 @@ class OptimisticLockingTest < Test::Unit::TestCase
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
end
+
+ def test_lock_repeating
+ p1 = Person.find(1)
+ p2 = Person.find(1)
+ assert_equal 0, p1.lock_version
+ assert_equal 0, p2.lock_version
+
+ p1.save!
+ assert_equal 1, p1.lock_version
+ assert_equal 0, p2.lock_version
+
+ assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
+ assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
+ end
def test_lock_new
p1 = Person.new(:first_name => 'anika')