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.

1 files changed, 15 insertions, 10 deletions

diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index f6b21eaa3a..d612ff53c1 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -372,6 +372,12 @@ module ActiveRecord
       stmt.wheres = arel.constraints
 
       if updates.is_a?(Hash)
+        if klass.locking_enabled? &&
+            !updates.key?(klass.locking_column) &&
+            !updates.key?(klass.locking_column.to_sym)
+          attr = arel_attribute(klass.locking_column)
+          updates[attr.name] = _increment_attribute(attr)
+        end
         stmt.set _substitute_values(updates)
       else
         stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name))
@@ -394,10 +400,7 @@ module ActiveRecord
       updates = {}
       counters.each do |counter_name, value|
         attr = arel_attribute(counter_name)
-        bind = predicate_builder.build_bind_attribute(attr.name, value.abs)
-        expr = table.coalesce(Arel::Nodes::UnqualifiedColumn.new(attr), 0)
-        expr = value < 0 ? expr - bind : expr + bind
-        updates[counter_name] = expr.expr
+        updates[attr.name] = _increment_attribute(attr, value)
       end
 
       if touch
@@ -433,12 +436,7 @@ module ActiveRecord
     #   Person.where(name: 'David').touch_all
     #   # => "UPDATE \"people\" SET \"updated_at\" = '2018-01-04 22:55:23.132670' WHERE \"people\".\"name\" = 'David'"
     def touch_all(*names, time: nil)
-      if klass.locking_enabled?
-        names << { time: time }
-        update_counters(klass.locking_column => 1, touch: names)
-      else
-        update_all klass.touch_attributes_with_time(*names, time: time)
-      end
+      update_all klass.touch_attributes_with_time(*names, time: time)
     end
 
     # Destroys the records by instantiating each
@@ -709,6 +707,13 @@ module ActiveRecord
         end
       end
 
+      def _increment_attribute(attribute, value = 1)
+        bind = predicate_builder.build_bind_attribute(attribute.name, value.abs)
+        expr = table.coalesce(Arel::Nodes::UnqualifiedColumn.new(attribute), 0)
+        expr = value < 0 ? expr - bind : expr + bind
+        expr.expr
+      end
+
       def exec_queries(&block)
         skip_query_cache_if_necessary do
           @records =