From 1d8fad6f903d5065911bea3409c7a9082f47d3f8 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Thu, 21 Feb 2019 21:09:05 +0900
Subject: Ensure `update_all` series cares about optimistic locking

Incrementing the lock version invalidates any other process's optimistic
lock, which is the desired outcome: the record no longer looks the same
as it did when they loaded it.
---
 activerecord/test/cases/dirty_test.rb              |  2 +-
 .../test/cases/relation/update_all_test.rb         | 71 +++++++++++++++++++---
 2 files changed, 64 insertions(+), 9 deletions(-)

(limited to 'activerecord/test/cases')

diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index dfd74bfcb4..a2a501a794 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -352,7 +352,7 @@ class DirtyTest < ActiveRecord::TestCase
       Person.where(id: person.id).update_all(first_name: "baz")
     end
 
-    old_lock_version = person.lock_version
+    old_lock_version = person.lock_version + 1
 
     with_partial_writes Person, true do
       assert_no_queries { 2.times { person.save! } }
diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb
index bb6912148c..0500574f28 100644
--- a/activerecord/test/cases/relation/update_all_test.rb
+++ b/activerecord/test/cases/relation/update_all_test.rb
@@ -138,14 +138,6 @@ class UpdateAllTest < ActiveRecord::TestCase
     assert_equal new_time, developer.updated_at
   end
 
-  def test_touch_all_updates_locking_column
-    person = people(:david)
-
-    assert_difference -> { person.reload.lock_version }, +1 do
-      Person.where(first_name: "David").touch_all
-    end
-  end
-
   def test_update_on_relation
     topic1 = TopicWithCallbacks.create! title: "arel", author_name: nil
     topic2 = TopicWithCallbacks.create! title: "activerecord", author_name: nil
@@ -186,6 +178,69 @@ class UpdateAllTest < ActiveRecord::TestCase
     end
   end
 
+  def test_update_all_cares_about_optimistic_locking
+    david = people(:david)
+
+    travel 5.seconds do
+      now = Time.now.utc
+      assert_not_equal now, david.updated_at
+
+      people = Person.where(id: people(:michael, :david, :susan))
+      expected = people.pluck(:lock_version)
+      expected.map! { |version| version + 1 }
+      people.update_all(updated_at: now)
+
+      assert_equal [now] * 3, people.pluck(:updated_at)
+      assert_equal expected, people.pluck(:lock_version)
+
+      assert_raises(ActiveRecord::StaleObjectError) do
+        david.touch(time: now)
+      end
+    end
+  end
+
+  def test_update_counters_cares_about_optimistic_locking
+    david = people(:david)
+
+    travel 5.seconds do
+      now = Time.now.utc
+      assert_not_equal now, david.updated_at
+
+      people = Person.where(id: people(:michael, :david, :susan))
+      expected = people.pluck(:lock_version)
+      expected.map! { |version| version + 1 }
+      people.update_counters(touch: [time: now])
+
+      assert_equal [now] * 3, people.pluck(:updated_at)
+      assert_equal expected, people.pluck(:lock_version)
+
+      assert_raises(ActiveRecord::StaleObjectError) do
+        david.touch(time: now)
+      end
+    end
+  end
+
+  def test_touch_all_cares_about_optimistic_locking
+    david = people(:david)
+
+    travel 5.seconds do
+      now = Time.now.utc
+      assert_not_equal now, david.updated_at
+
+      people = Person.where(id: people(:michael, :david, :susan))
+      expected = people.pluck(:lock_version)
+      expected.map! { |version| version + 1 }
+      people.touch_all(time: now)
+
+      assert_equal [now] * 3, people.pluck(:updated_at)
+      assert_equal expected, people.pluck(:lock_version)
+
+      assert_raises(ActiveRecord::StaleObjectError) do
+        david.touch(time: now)
+      end
+    end
+  end
+
   # Oracle UPDATE does not support ORDER BY
   unless current_adapter?(:OracleAdapter)
     def test_update_all_ignores_order_without_limit_from_association
-- 
cgit v1.2.3