#551: Keep fact replacement atomic

lib/fbe/delete.rb

@@ -33,8 +33,8 @@ def Fbe.delete(fact, *props, fb: Fbe.fb, id: '_id')
     next if props.include?(k)
     before[k] = fact[k]
   end
-  fb.query("(eq #{id} #{i})").delete!
   fb.txn do |fbt|
+    fbt.query("(eq #{id} #{i})").delete!
     c = fbt.insert
     f = c
     while f.instance_variable_defined?(:@fact) || f.instance_variable_defined?(:@origin)

lib/fbe/delete_one.rb

@@ -32,8 +32,8 @@ def Fbe.delete_one(fact, prop, value, fb: Fbe.fb, id: '_id')
   return if nv == before[prop]
   before[prop] = nv
   before.delete(prop) if nv.empty?
-  fb.query("(eq #{id} #{i})").delete!
   fb.txn do |fbt|
+    fbt.query("(eq #{id} #{i})").delete!
     c = fbt.insert
     before.each do |k, vv|
       next unless c[k].nil?

lib/fbe/overwrite.rb

@@ -65,8 +65,8 @@ def Fbe.overwrite(fact, property_or_hash, values = nil, fb: Fbe.fb, fid: '_id')
     end
     id = fact[fid]&.first
     raise(Fbe::Error, "There is no #{fid} in the fact, cannot use Fbe.overwrite") if id.nil?
-    raise(Fbe::Error, "No facts by #{fid} = #{id}") if fb.query("(eq #{fid} #{id})").delete!.zero?
     fb.txn do |fbt|
+      raise(Fbe::Error, "No facts by #{fid} = #{id}") if fbt.query("(eq #{fid} #{id})").delete!.zero?
       n = fbt.insert
       before.each do |k, vv|
         next unless n[k].nil?
@@ -94,8 +94,8 @@ def Fbe.overwrite(fact, property_or_hash, values = nil, fb: Fbe.fb, fid: '_id')
   end
   id = fact[fid]&.first
   raise(Fbe::Error, "There is no #{fid} in the fact, cannot use Fbe.overwrite") if id.nil?
-  raise(Fbe::Error, "No facts by #{fid} = #{id}") if fb.query("(eq #{fid} #{id})").delete!.zero?
   fb.txn do |fbt|
+    raise(Fbe::Error, "No facts by #{fid} = #{id}") if fbt.query("(eq #{fid} #{id})").delete!.zero?
     n = fbt.insert
     before[property.to_s] = values
     before.each do |k, vv|

test/fbe/test_delete.rb

@@ -113,6 +113,24 @@ def test_deletes_in_transaction
     assert_nil(after['foo'])
   end
 
+  def test_keeps_original_fact_when_reinsert_fails
+    reject = false
+    fb =
+      Factbase::Pre.new(Factbase.new) do |_f, _fbt|
+        raise(RuntimeError, 'insert failed') if reject
+      end
+    f = fb.insert
+    f._id = 44
+    f.foo = 42
+    f.bar = 'keep'
+    reject = true
+    assert_raises(RuntimeError) { Fbe.delete(f, 'foo', fb:) }
+    after = fb.query('(eq _id 44)').each.to_a
+    assert_equal(1, after.size)
+    assert_equal([42], after.first['foo'])
+    assert_equal(['keep'], after.first['bar'])
+  end
+
   def test_deletes_when_duplicate_id
     fb = Factbase.new
     f = fb.insert

test/fbe/test_delete_one.rb

@@ -69,4 +69,23 @@ def test_does_not_recreate_fact_when_value_not_present
     assert_equal(id, r._id, 'The _id must not change when value is not in the array')
     assert_equal([1, 2, 3], r['foo'])
   end
+
+  def test_keeps_original_fact_when_reinsert_fails
+    reject = false
+    fb =
+      Factbase::Pre.new(Factbase.new) do |_f, _fbt|
+        raise(RuntimeError, 'insert failed') if reject
+      end
+    f = fb.insert
+    f._id = 555
+    f.foo = 42
+    f.foo = 'hello'
+    f.bar = 'keep'
+    reject = true
+    assert_raises(RuntimeError) { Fbe.delete_one(f, 'foo', 42, fb:) }
+    after = fb.query('(eq _id 555)').each.to_a
+    assert_equal(1, after.size)
+    assert_equal([42, 'hello'], after.first['foo'])
+    assert_equal(['keep'], after.first['bar'])
+  end
 end

test/fbe/test_overwrite.rb

@@ -122,6 +122,24 @@ def test_overwrites_in_transaction
     assert_equal('bar', after.foo)
   end
 
+  def test_keeps_original_fact_when_scalar_reinsert_fails
+    reject = false
+    fb =
+      Factbase::Pre.new(Factbase.new) do |_f, _fbt|
+        raise(RuntimeError, 'insert failed') if reject
+      end
+    f = fb.insert
+    f._id = 1
+    f.foo = 42
+    f.bar = 'keep'
+    reject = true
+    assert_raises(RuntimeError) { Fbe.overwrite(f, 'foo', 55, fb:) }
+    after = fb.query('(eq _id 1)').each.to_a
+    assert_equal(1, after.size)
+    assert_equal([42], after.first['foo'])
+    assert_equal(['keep'], after.first['bar'])
+  end
+
   def test_overwrite_with_hash_single_property
     fb = Factbase.new
     f = fb.insert
@@ -236,6 +254,25 @@ def test_overwrite_with_hash_mix_addition_and_override
     assert_equal('new_property', result['baz'].first)
   end
 
+  def test_keeps_original_fact_when_hash_reinsert_fails
+    reject = false
+    fb =
+      Factbase::Pre.new(Factbase.new) do |_f, _fbt|
+        raise(RuntimeError, 'insert failed') if reject
+      end
+    f = fb.insert
+    f._id = 1
+    f.foo = 42
+    f.bar = 'keep'
+    reject = true
+    assert_raises(RuntimeError) { Fbe.overwrite(f, { foo: 55, baz: 'new' }, fb:) }
+    after = fb.query('(eq _id 1)').each.to_a
+    assert_equal(1, after.size)
+    assert_equal([42], after.first['foo'])
+    assert_equal(['keep'], after.first['bar'])
+    assert_nil(after.first['baz'])
+  end
+
   def test_overwrite_with_hash_mixed_key_types
     fb = Factbase.new
     f = fb.insert