Fixed: Optimistic locking does not work well with null in the database

2 files changed, 57 insertions, 11 deletions

diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index 13b6f6daaf..8b6a969c9e 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -161,14 +161,6 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     assert_equal(error.record.object_id, p2.object_id)
   end
 
-  def test_lock_new_with_nil
-    p1 = Person.new(first_name: "anika")
-    p1.save!
-    p1.lock_version = nil # simulate bad fixture or column with no default
-    p1.save!
-    assert_equal 1, p1.lock_version
-  end
-
   def test_lock_new_when_explicitly_passing_nil
     p1 = Person.new(first_name: "anika", lock_version: nil)
     p1.save!
@@ -222,22 +214,73 @@ class OptimisticLockingTest < ActiveRecord::TestCase
 
   def test_lock_without_default_sets_version_to_zero
     t1 = LockWithoutDefault.new
+
     assert_equal 0, t1.lock_version
+    assert_nil t1.lock_version_before_type_cast
+
+    t1.save!
+    t1.reload
 
-    t1.save
-    t1 = LockWithoutDefault.find(t1.id)
     assert_equal 0, t1.lock_version
+    assert_equal 0, t1.lock_version_before_type_cast
+  end
+
+  def test_lock_without_default_should_work_with_null_in_the_database
+    ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')")
+    t1 = LockWithoutDefault.last
+    t2 = LockWithoutDefault.last
+
+    assert_equal 0, t1.lock_version
+    assert_nil t1.lock_version_before_type_cast
+    assert_equal 0, t2.lock_version
+    assert_nil t2.lock_version_before_type_cast
+
+    t1.title = "new title1"
+    t2.title = "new title2"
+
+    assert_nothing_raised { t1.save! }
+    assert_equal 1, t1.lock_version
+    assert_equal "new title1", t1.title
+
+    assert_raise(ActiveRecord::StaleObjectError) { t2.save! }
+    assert_equal 0, t2.lock_version
+    assert_equal "new title2", t2.title
   end
 
   def test_lock_with_custom_column_without_default_sets_version_to_zero
     t1 = LockWithCustomColumnWithoutDefault.new
+
     assert_equal 0, t1.custom_lock_version
     assert_nil t1.custom_lock_version_before_type_cast
 
     t1.save!
     t1.reload
+
     assert_equal 0, t1.custom_lock_version
-    assert [0, "0"].include?(t1.custom_lock_version_before_type_cast)
+    assert_equal 0, t1.custom_lock_version_before_type_cast
+  end
+
+  def test_lock_with_custom_column_without_default_should_work_with_null_in_the_database
+    ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults_cust(title) VALUES('title1')")
+
+    t1 = LockWithCustomColumnWithoutDefault.last
+    t2 = LockWithCustomColumnWithoutDefault.last
+
+    assert_equal 0, t1.custom_lock_version
+    assert_nil t1.custom_lock_version_before_type_cast
+    assert_equal 0, t2.custom_lock_version
+    assert_nil t2.custom_lock_version_before_type_cast
+
+    t1.title = "new title1"
+    t2.title = "new title2"
+
+    assert_nothing_raised { t1.save! }
+    assert_equal 1, t1.custom_lock_version
+    assert_equal "new title1", t1.title
+
+    assert_raise(ActiveRecord::StaleObjectError) { t2.save! }
+    assert_equal 0, t2.custom_lock_version
+    assert_equal "new title2", t2.title
   end
 
   def test_readonly_attributes
@@ -461,6 +504,7 @@ unless in_memory_db?
       end
 
       protected
+
       def duel(zzz = 5)
         t0, t1, t2, t3 = nil, nil, nil, nil
 
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index d2fb090118..983ac076a9 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -436,10 +436,12 @@ ActiveRecord::Schema.define do
   end
 
   create_table :lock_without_defaults, force: true do |t|
+    t.column :title, :string
     t.column :lock_version, :integer
   end
 
   create_table :lock_without_defaults_cust, force: true do |t|
+    t.column :title, :string
     t.column :custom_lock_version, :integer
   end