Merge pull request rails#41082 from kamipo/fix_update_optimistic_locking_without_default

Restore the ability that update/destroy optimistic locking object without default

Author: kamipo

activemodel/lib/active_model/attribute.rb

@@ -133,14 +133,13 @@ def encode_with(coder)
       coder["value"] = value if defined?(@value)
     end
 
-    protected
-      def original_value_for_database
-        if assigned?
-          original_attribute.original_value_for_database
-        else
-          _original_value_for_database
-        end
+    def original_value_for_database
+      if assigned?
+        original_attribute.original_value_for_database
+      else
+        _original_value_for_database
       end
+    end
 
     private
       attr_reader :original_attribute
@@ -165,9 +164,10 @@ def type_cast(value)
           type.deserialize(value)
         end
 
-        def _original_value_for_database
-          value_before_type_cast
-        end
+        private
+          def _original_value_for_database
+            value_before_type_cast
+          end
       end
 
       class FromUser < Attribute # :nodoc:

activerecord/lib/active_record/locking/optimistic.rb

@@ -89,7 +89,9 @@ def _update_row(attribute_names, attempted_action = "update")
 
           begin
             locking_column = self.class.locking_column
-            previous_lock_value = attribute_before_type_cast(locking_column)
+            lock_attribute_was = @attributes[locking_column]
+            lock_value_for_database = _lock_value_for_database(locking_column)
+
             attribute_names = attribute_names.dup if attribute_names.frozen?
             attribute_names << locking_column
 
@@ -98,7 +100,7 @@ def _update_row(attribute_names, attempted_action = "update")
             affected_rows = self.class._update_record(
               attributes_with_values(attribute_names),
               @primary_key => id_in_database,
-              locking_column => previous_lock_value
+              locking_column => lock_value_for_database
             )
 
             if affected_rows != 1
@@ -109,7 +111,7 @@ def _update_row(attribute_names, attempted_action = "update")
 
           # If something went wrong, revert the locking_column value.
           rescue Exception
-            self[locking_column] = previous_lock_value.to_i
+            @attributes[locking_column] = lock_attribute_was
             raise
           end
         end
@@ -121,7 +123,7 @@ def destroy_row
 
           affected_rows = self.class._delete_record(
             @primary_key => id_in_database,
-            locking_column => attribute_before_type_cast(locking_column)
+            locking_column => _lock_value_for_database(locking_column)
           )
 
           if affected_rows != 1
@@ -131,6 +133,14 @@ def destroy_row
           affected_rows
         end
 
+        def _lock_value_for_database(locking_column)
+          if will_save_change_to_attribute?(locking_column)
+            @attributes[locking_column].value_for_database
+          else
+            @attributes[locking_column].original_value_for_database
+          end
+        end
+
         module ClassMethods
           DEFAULT_LOCKING_COLUMN = "lock_version"
 

activerecord/test/cases/locking_test.rb

@@ -340,6 +340,39 @@ def test_lock_without_default_should_work_with_null_in_the_database
     assert_equal "new title2", t2.title
   end
 
+  def test_update_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast
+    ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')")
+    t1 = LockWithoutDefault.last
+
+    assert_equal 0, t1.lock_version
+    assert_nil t1.lock_version_before_type_cast
+
+    t1.lock_version = t1.lock_version
+
+    assert_equal 0, t1.lock_version
+    assert_equal 0, t1.lock_version_before_type_cast
+
+    assert_nothing_raised { t1.update!(title: "new title1") }
+    assert_equal 1, t1.lock_version
+    assert_equal "new title1", t1.title
+  end
+
+  def test_destroy_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast
+    ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')")
+    t1 = LockWithoutDefault.last
+
+    assert_equal 0, t1.lock_version
+    assert_nil t1.lock_version_before_type_cast
+
+    t1.lock_version = t1.lock_version
+
+    assert_equal 0, t1.lock_version
+    assert_equal 0, t1.lock_version_before_type_cast
+
+    assert_nothing_raised { t1.destroy! }
+    assert_predicate t1, :destroyed?
+  end
+
   def test_lock_without_default_queries_count
     t1 = LockWithoutDefault.create(title: "title1")