Partial updates don't update lock_version if nothing changed. [#426 state:resolved]
This commit is contained in:
parent
0fd3e4cd2b
commit
3610997ba3
|
@ -1,5 +1,7 @@
|
||||||
*Edge*
|
*Edge*
|
||||||
|
|
||||||
|
* 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]
|
* 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]
|
* db:migrate:down and :up update schema_migrations. #369 [Michael Raidel, RaceCondition]
|
||||||
|
|
|
@ -68,6 +68,7 @@ module ActiveRecord
|
||||||
|
|
||||||
def update_with_lock(attribute_names = @attributes.keys) #:nodoc:
|
def update_with_lock(attribute_names = @attributes.keys) #:nodoc:
|
||||||
return update_without_lock(attribute_names) unless locking_enabled?
|
return update_without_lock(attribute_names) unless locking_enabled?
|
||||||
|
return 0 if attribute_names.empty?
|
||||||
|
|
||||||
lock_col = self.class.locking_column
|
lock_col = self.class.locking_column
|
||||||
previous_value = send(lock_col).to_i
|
previous_value = send(lock_col).to_i
|
||||||
|
|
|
@ -2,6 +2,7 @@ require 'cases/helper'
|
||||||
require 'models/topic' # For booleans
|
require 'models/topic' # For booleans
|
||||||
require 'models/pirate' # For timestamps
|
require 'models/pirate' # For timestamps
|
||||||
require 'models/parrot'
|
require 'models/parrot'
|
||||||
|
require 'models/person' # For optimistic locking
|
||||||
|
|
||||||
class Pirate # Just reopening it, not defining it
|
class Pirate # Just reopening it, not defining it
|
||||||
attr_accessor :detected_changes_in_after_update # Boolean for if changes are detected
|
attr_accessor :detected_changes_in_after_update # Boolean for if changes are detected
|
||||||
|
@ -125,6 +126,24 @@ class DirtyTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
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
|
def test_changed_attributes_should_be_preserved_if_save_failure
|
||||||
pirate = Pirate.new
|
pirate = Pirate.new
|
||||||
pirate.parrot_id = 1
|
pirate.parrot_id = 1
|
||||||
|
|
|
@ -29,10 +29,12 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
assert_equal 0, p1.lock_version
|
assert_equal 0, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p1.first_name = 'stu'
|
||||||
p1.save!
|
p1.save!
|
||||||
assert_equal 1, p1.lock_version
|
assert_equal 1, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p2.first_name = 'sue'
|
||||||
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -42,11 +44,14 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
assert_equal 0, p1.lock_version
|
assert_equal 0, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p1.first_name = 'stu'
|
||||||
p1.save!
|
p1.save!
|
||||||
assert_equal 1, p1.lock_version
|
assert_equal 1, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p2.first_name = 'sue'
|
||||||
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
||||||
|
p2.first_name = 'sue2'
|
||||||
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -54,15 +59,18 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
p1 = Person.new(:first_name => 'anika')
|
p1 = Person.new(:first_name => 'anika')
|
||||||
assert_equal 0, p1.lock_version
|
assert_equal 0, p1.lock_version
|
||||||
|
|
||||||
|
p1.first_name = 'anika2'
|
||||||
p1.save!
|
p1.save!
|
||||||
p2 = Person.find(p1.id)
|
p2 = Person.find(p1.id)
|
||||||
assert_equal 0, p1.lock_version
|
assert_equal 0, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p1.first_name = 'anika3'
|
||||||
p1.save!
|
p1.save!
|
||||||
assert_equal 1, p1.lock_version
|
assert_equal 1, p1.lock_version
|
||||||
assert_equal 0, p2.lock_version
|
assert_equal 0, p2.lock_version
|
||||||
|
|
||||||
|
p2.first_name = 'sue'
|
||||||
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
assert_raises(ActiveRecord::StaleObjectError) { p2.save! }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -81,10 +89,12 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
assert_equal 0, t1.version
|
assert_equal 0, t1.version
|
||||||
assert_equal 0, t2.version
|
assert_equal 0, t2.version
|
||||||
|
|
||||||
|
t1.tps_report_number = 700
|
||||||
t1.save!
|
t1.save!
|
||||||
assert_equal 1, t1.version
|
assert_equal 1, t1.version
|
||||||
assert_equal 0, t2.version
|
assert_equal 0, t2.version
|
||||||
|
|
||||||
|
t2.tps_report_number = 800
|
||||||
assert_raises(ActiveRecord::StaleObjectError) { t2.save! }
|
assert_raises(ActiveRecord::StaleObjectError) { t2.save! }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -93,6 +103,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
assert_equal 0, p1.lock_version
|
assert_equal 0, p1.lock_version
|
||||||
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
|
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
|
||||||
|
|
||||||
|
p1.first_name = 'bianca2'
|
||||||
p1.save!
|
p1.save!
|
||||||
assert_equal 1, p1.lock_version
|
assert_equal 1, p1.lock_version
|
||||||
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
|
assert_equal p1.lock_version, Person.new(p1.attributes).lock_version
|
||||||
|
@ -146,6 +157,15 @@ class OptimisticLockingTest < ActiveRecord::TestCase
|
||||||
assert ref.save
|
assert ref.save
|
||||||
end
|
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
|
private
|
||||||
|
|
||||||
def add_counter_column_to(model)
|
def add_counter_column_to(model)
|
||||||
|
|
Loading…
Reference in New Issue