Merge pull request #25873 from schuetzm/warn_about_dirty_lock

Deprecate locking of dirty records

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate locking records with unpersisted changes.
+
+    *Marc Schütz*
+
 *   Remove deprecated behavior that halts callbacks when the return is false.
 
     *Rafael Mendonça França*

activerecord/lib/active_record/locking/pessimistic.rb

@@ -59,7 +59,16 @@ module Pessimistic
       # or pass true for "FOR UPDATE" (the default, an exclusive row lock). Returns
       # the locked record.
       def lock!(lock = true)
-        reload(lock: lock) if persisted?
+        if persisted?
+          if changed?
+            ActiveSupport::Deprecation.warn(<<-MSG.squish)
+              Locking a record with unpersisted changes is deprecated and will raise an
+              exception in Rails 5.2. Use `save` to persist the changes, or `reload` to
+              discard them explicitly.
+            MSG
+          end
+          reload(lock: lock)
+        end
         self
       end
 

activerecord/test/cases/locking_test.rb

@@ -536,7 +536,10 @@ def test_sane_lock_method
         Person.transaction do
           person = Person.find 1
           old, person.first_name = person.first_name, "fooman"
-          person.lock!
+          # Locking a dirty record is deprecated
+          assert_deprecated do
+            person.lock!
+          end
           assert_equal old, person.first_name
         end
       end