From ae2d36cf21281f4b720332a086b021d867e80084 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sat, 3 Mar 2018 18:44:09 +0900
Subject: [PATCH] Introduce `_update_row` to decouple optimistic locking
 concern from `Persistence` module

---
 .../lib/active_record/locking/optimistic.rb   | 31 ++++++++++++++
 activerecord/lib/active_record/persistence.rb | 42 +++++++------------
 2 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb
index 29735f5490..af361a8d2d 100644
--- a/activerecord/lib/active_record/locking/optimistic.rb
+++ b/activerecord/lib/active_record/locking/optimistic.rb
@@ -111,6 +111,37 @@ def _update_record(attribute_names = self.attribute_names)
           end
         end
 
+        def _update_row(attribute_names, attempted_action)
+          return super unless locking_enabled?
+
+          begin
+            locking_column = self.class.locking_column
+            previous_lock_value = read_attribute_before_type_cast(locking_column)
+            attribute_names << locking_column
+
+            self[locking_column] += 1
+
+            affected_rows = self.class._update_record(
+              attributes_with_values(attribute_names),
+              self.class.primary_key => id_in_database,
+              locking_column => previous_lock_value
+            )
+
+            if affected_rows != 1
+              raise ActiveRecord::StaleObjectError.new(self, attempted_action)
+            end
+
+            affected_rows
+
+          # If something went wrong, revert the locking_column value.
+          rescue Exception
+            self[locking_column] = previous_lock_value
+            raise
+          ensure
+            clear_attribute_change(locking_column)
+          end
+        end
+
         def destroy_row
           return super unless locking_enabled?
 
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 7b0b7a18e3..ce16587326 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -663,31 +663,12 @@ def touch(*names, time: nil)
       unless attribute_names.empty?
         attribute_names.each do |attr_name|
           write_attribute(attr_name, time)
+          clear_attribute_change(attr_name)
         end
 
-        constraints = { self.class.primary_key => id_in_database }
+        affected_rows = _update_row(attribute_names, "touch")
 
-        if locking_enabled?
-          locking_column = self.class.locking_column
-          constraints[locking_column] = read_attribute_before_type_cast(locking_column)
-          attribute_names << locking_column
-          increment_lock
-        end
-
-        clear_attribute_changes(attribute_names)
-
-        affected_rows = self.class._update_record(
-          attributes_with_values_for_update(attribute_names),
-          constraints
-        )
-
-        result = affected_rows == 1
-
-        if !result && locking_enabled?
-          raise ActiveRecord::StaleObjectError.new(self, "touch")
-        end
-
-        @_trigger_update_callback = result
+        @_trigger_update_callback = affected_rows == 1
       else
         true
       end
@@ -707,6 +688,13 @@ def _delete_row
       self.class._delete_record(self.class.primary_key => id_in_database)
     end
 
+    def _update_row(attribute_names, attempted_action)
+      self.class._update_record(
+        attributes_with_values(attribute_names),
+        self.class.primary_key => id_in_database
+      )
+    end
+
     def create_or_update(*args, &block)
       _raise_readonly_record_error if readonly?
       return false if destroyed?
@@ -718,15 +706,13 @@ def create_or_update(*args, &block)
     # Returns the number of affected rows.
     def _update_record(attribute_names = self.attribute_names)
       attribute_names &= self.class.column_names
-      attributes_values = attributes_with_values_for_update(attribute_names)
-      if attributes_values.empty?
+      attribute_names = attributes_for_update(attribute_names)
+
+      if attribute_names.empty?
         affected_rows = 0
         @_trigger_update_callback = true
       else
-        affected_rows = self.class._update_record(
-          attributes_values,
-          self.class.primary_key => id_in_database
-        )
+        affected_rows = _update_row(attribute_names, "update")
         @_trigger_update_callback = affected_rows == 1
       end
 
-- 
GitLab