Skip to content

Commit

Permalink
Restore the ability that update/destroy optimistic locking object wit…
Browse files Browse the repository at this point in the history
…hout default

The ability has lost due to reverted #39321 in #41049.

We should allow updating with dirty locking value to work the documented
usage, but if casted value has no difference (i.e. regarded as no dirty),
identify the object by the original (uninitialized default) value.
  • Loading branch information
kamipo committed Jan 12, 2021
1 parent 8c60a21 commit 4db95d8
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 14 deletions.
20 changes: 10 additions & 10 deletions activemodel/lib/active_model/attribute.rb
Expand Up @@ -133,14 +133,13 @@ def encode_with(coder)
coder["value"] = value if defined?(@value)
end

protected
def original_value_for_database
if assigned?
original_attribute.original_value_for_database
else
_original_value_for_database
end
def original_value_for_database
if assigned?
original_attribute.original_value_for_database
else
_original_value_for_database
end
end

private
attr_reader :original_attribute
Expand All @@ -165,9 +164,10 @@ def type_cast(value)
type.deserialize(value)
end

def _original_value_for_database
value_before_type_cast
end
private
def _original_value_for_database
value_before_type_cast
end
end

class FromUser < Attribute # :nodoc:
Expand Down
18 changes: 14 additions & 4 deletions activerecord/lib/active_record/locking/optimistic.rb
Expand Up @@ -89,7 +89,9 @@ def _update_row(attribute_names, attempted_action = "update")

begin
locking_column = self.class.locking_column
previous_lock_value = attribute_before_type_cast(locking_column)
lock_attribute_was = @attributes[locking_column]
lock_value_for_database = _lock_value_for_database(locking_column)

attribute_names = attribute_names.dup if attribute_names.frozen?
attribute_names << locking_column

Expand All @@ -98,7 +100,7 @@ def _update_row(attribute_names, attempted_action = "update")
affected_rows = self.class._update_record(
attributes_with_values(attribute_names),
@primary_key => id_in_database,
locking_column => previous_lock_value
locking_column => lock_value_for_database
)

if affected_rows != 1
Expand All @@ -109,7 +111,7 @@ def _update_row(attribute_names, attempted_action = "update")

# If something went wrong, revert the locking_column value.
rescue Exception
self[locking_column] = previous_lock_value.to_i
@attributes[locking_column] = lock_attribute_was
raise
end
end
Expand All @@ -121,7 +123,7 @@ def destroy_row

affected_rows = self.class._delete_record(
@primary_key => id_in_database,
locking_column => attribute_before_type_cast(locking_column)
locking_column => _lock_value_for_database(locking_column)
)

if affected_rows != 1
Expand All @@ -131,6 +133,14 @@ def destroy_row
affected_rows
end

def _lock_value_for_database(locking_column)
if will_save_change_to_attribute?(locking_column)
@attributes[locking_column].value_for_database
else
@attributes[locking_column].original_value_for_database
end
end

module ClassMethods
DEFAULT_LOCKING_COLUMN = "lock_version"

Expand Down
33 changes: 33 additions & 0 deletions activerecord/test/cases/locking_test.rb
Expand Up @@ -340,6 +340,39 @@ def test_lock_without_default_should_work_with_null_in_the_database
assert_equal "new title2", t2.title
end

def test_update_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast
ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')")
t1 = LockWithoutDefault.last

assert_equal 0, t1.lock_version
assert_nil t1.lock_version_before_type_cast

t1.lock_version = t1.lock_version

assert_equal 0, t1.lock_version
assert_equal 0, t1.lock_version_before_type_cast

assert_nothing_raised { t1.update!(title: "new title1") }
assert_equal 1, t1.lock_version
assert_equal "new title1", t1.title
end

def test_destroy_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast
ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')")
t1 = LockWithoutDefault.last

assert_equal 0, t1.lock_version
assert_nil t1.lock_version_before_type_cast

t1.lock_version = t1.lock_version

assert_equal 0, t1.lock_version
assert_equal 0, t1.lock_version_before_type_cast

assert_nothing_raised { t1.destroy! }
assert_predicate t1, :destroyed?
end

def test_lock_without_default_queries_count
t1 = LockWithoutDefault.create(title: "title1")

Expand Down

0 comments on commit 4db95d8

Please sign in to comment.