Permalink
Browse files

Partial updates don't update lock_version if nothing changed. [#426 s…

…tate:resolved]
  • Loading branch information...
1 parent 3558a50 commit 44b890740003d27a4d80a07df33efd57476d37e4 @danielmorrison danielmorrison committed with jeremy Jun 15, 2008
View
@@ -1,5 +1,7 @@
*2.1.1 (next release)*
+* Partial updates don't update lock_version if nothing changed. #426 [Daniel Morrison]
+
* Fix column collision with named_scope and :joins. #46 [Duncan Beevers, Mark Catley]
* db:migrate:down and :up update schema_migrations. #369 [Michael Raidel, RaceCondition]
@@ -68,6 +68,7 @@ def attributes_from_column_definition_with_lock
def update_with_lock(attribute_names = @attributes.keys) #:nodoc:
return update_without_lock(attribute_names) unless locking_enabled?
+ return 0 if attribute_names.empty?
lock_col = self.class.locking_column
previous_value = send(lock_col).to_i
@@ -2,6 +2,7 @@
require 'models/topic' # For booleans
require 'models/pirate' # For timestamps
require 'models/parrot'
+require 'models/person' # For optimistic locking
class Pirate # Just reopening it, not defining it
attr_accessor :detected_changes_in_after_update # Boolean for if changes are detected
@@ -125,6 +126,24 @@ def test_partial_update
end
end
+ def test_partial_update_with_optimistic_locking
+ person = Person.new(:first_name => 'foo')
+ old_lock_version = 1
+
+ with_partial_updates Person, false do
+ assert_queries(2) { 2.times { person.save! } }
+ Person.update_all({ :first_name => 'baz' }, :id => person.id)
+ end
+
+ with_partial_updates Person, true do
+ assert_queries(0) { 2.times { person.save! } }
+ assert_equal old_lock_version, person.reload.lock_version
+
+ assert_queries(1) { person.first_name = 'bar'; person.save! }
+ assert_not_equal old_lock_version, person.reload.lock_version
+ end
+ end
+
def test_changed_attributes_should_be_preserved_if_save_failure
pirate = Pirate.new
pirate.parrot_id = 1
@@ -29,10 +29,12 @@ def test_lock_existing
assert_equal 0, p1.lock_version
assert_equal 0, p2.lock_version
+ p1.first_name = 'stu'
p1.save!
assert_equal 1, p1.lock_version
assert_equal 0, p2.lock_version
+ p2.first_name = 'sue'
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
end
@@ -42,27 +44,33 @@ def test_lock_repeating
assert_equal 0, p1.lock_version
assert_equal 0, p2.lock_version
+ p1.first_name = 'stu'
p1.save!
assert_equal 1, p1.lock_version
assert_equal 0, p2.lock_version
+ p2.first_name = 'sue'
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
+ p2.first_name = 'sue2'
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
end
def test_lock_new
p1 = Person.new(:first_name => 'anika')
assert_equal 0, p1.lock_version
+ p1.first_name = 'anika2'
p1.save!
p2 = Person.find(p1.id)
assert_equal 0, p1.lock_version
assert_equal 0, p2.lock_version
+ p1.first_name = 'anika3'
p1.save!
assert_equal 1, p1.lock_version
assert_equal 0, p2.lock_version
+ p2.first_name = 'sue'
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
end
@@ -81,10 +89,12 @@ def test_lock_column_name_existing
assert_equal 0, t1.version
assert_equal 0, t2.version
+ t1.tps_report_number = 700
t1.save!
assert_equal 1, t1.version
assert_equal 0, t2.version
+ t2.tps_report_number = 800
assert_raises(ActiveRecord::StaleObjectError) { t2.save! }
end
@@ -93,6 +103,7 @@ def test_lock_column_is_mass_assignable
assert_equal 0, p1.lock_version
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
+ p1.first_name = 'bianca2'
p1.save!
assert_equal 1, p1.lock_version
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
@@ -146,6 +157,15 @@ def test_quote_table_name
assert ref.save
end
+ # Useful for partial updates, don't only update the lock_version if there
+ # is nothing else being updated.
+ def test_update_without_attributes_does_not_only_update_lock_version
+ assert_nothing_raised do
+ p1 = Person.new(:first_name => 'anika')
+ p1.send(:update_with_lock, [])
+ end
+ end
+
private
def add_counter_column_to(model)

0 comments on commit 44b8907

Please sign in to comment.