From e117d9266e577675afe1462e37601c029e21091b Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 4 Mar 2018 15:32:46 +0900
Subject: `id_in_database` should be respected as primary key value for
persisted records
Currently primary key value can not be updated if a record has a locking
column because of `_update_record` in `Locking::Optimistic` doesn't
respect `id_in_database` as primary key value unlike in `Persistence`.
And also, if a record has dirty primary key value, it may destroy any
other record by the lock version of dirty record itself.
When updating/destroying persisted records, it should identify
themselves by `id_in_database`, not by dirty primary key value.
---
.../lib/active_record/locking/optimistic.rb | 2 +-
activerecord/lib/active_record/persistence.rb | 2 +-
activerecord/test/cases/locking_test.rb | 39 ++++++++++++++++++++++
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index bf17f27e68..052b5d23aa 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -94,7 +94,7 @@ module ActiveRecord
relation = self.class.unscoped
affected_rows = relation.where(
- self.class.primary_key => id,
+ self.class.primary_key => id_in_database,
lock_col => previous_lock_value
).update_all(
attributes_for_update(attribute_names).map do |name|
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index c9f929680b..8f9cb85a90 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -691,7 +691,7 @@ module ActiveRecord
end
def _relation_for_itself
- self.class.unscoped.where(self.class.primary_key => id)
+ self.class.unscoped.where(self.class.primary_key => id_in_database)
end
def create_or_update(*args, &block)
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 02be0aa9e3..9d04750712 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -194,6 +194,45 @@ class OptimisticLockingTest < ActiveRecord::TestCase
end
end
+ def test_update_with_dirty_primary_key
+ assert_raises(ActiveRecord::RecordNotUnique) do
+ person = Person.find(1)
+ person.id = 2
+ person.save!
+ end
+
+ person = Person.find(1)
+ person.id = 42
+ person.save!
+
+ assert Person.find(42)
+ assert_raises(ActiveRecord::RecordNotFound) do
+ Person.find(1)
+ end
+ end
+
+ def test_delete_with_dirty_primary_key
+ person = Person.find(1)
+ person.id = 2
+ person.delete
+
+ assert Person.find(2)
+ assert_raises(ActiveRecord::RecordNotFound) do
+ Person.find(1)
+ end
+ end
+
+ def test_destroy_with_dirty_primary_key
+ person = Person.find(1)
+ person.id = 2
+ person.destroy
+
+ assert Person.find(2)
+ assert_raises(ActiveRecord::RecordNotFound) do
+ Person.find(1)
+ end
+ end
+
def test_explicit_update_lock_column_raise_error
person = Person.find(1)
--
cgit v1.2.3