From 33c84d8fd148fd8eba56556254ce520ce1f04253 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Wed, 2 Sep 2026 07:57:21 -0500 Subject: [PATCH 1/3] feat: UpsertObjectAsync reports Inserted vs Updated Closes #32. WriteObjectAsync only says whether the write succeeded. A caller maintaining a derived view of the store (a queue count, an added/removed signal) needs to know whether the row was created or replaced, and a read-then-write pair of its own is not atomic against other writers - in PFM.Mobile the outer lock that made it atomic wedged the whole TPR queue once in the field. UpsertObjectAsync(obj, [keySelector], partition, withTransaction, ct) returns UpsertResult.Inserted / Updated. It runs INSERT OR IGNORE and, only when that affected no row, an in-place UPDATE of Data - both inside the existing connection gate and serializable transaction, one statement on the insert path and two on the update path, no pre-read. Anything other than exactly one affected row throws TychoException like the other write paths. Stored contents match WriteObjectAsync; the registered-id overload and the strict-mode key-divergence guard behave the same. Tests cover first/second write, rows written by WriteObjectAsync, the three primary-key axes (key, type, partition), the empty partition, the registered overload, withTransaction: false, and null arguments. Full suite passes in Debug and Encrypted. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 16 ++ README.md | 27 +++ TychoDB.UnitTests/UpsertObjectTests.cs | 278 +++++++++++++++++++++++++ TychoDB/Queries.cs | 27 +++ TychoDB/Tycho.cs | 158 ++++++++++++++ TychoDB/UpsertResult.cs | 14 ++ 6 files changed, 520 insertions(+) create mode 100644 TychoDB.UnitTests/UpsertObjectTests.cs create mode 100644 TychoDB/UpsertResult.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index d412978..a3d48e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## 5.3.0 (unreleased) + +### Added + +- **`UpsertObjectAsync` reports whether the write inserted or updated.** Returns + `UpsertResult.Inserted` when no row existed for the key in that partition and + `UpsertResult.Updated` when one did and its data was replaced; stored contents are identical + to `WriteObjectAsync`. Implemented as `INSERT OR IGNORE` plus, only when nothing was inserted, + an in-place `UPDATE`, both under the connection gate and the transaction — so callers that keep + an incremental view of the store (a queue count, an added/removed signal) no longer need a + read-then-write pair guarded by a lock of their own. Registered-id and explicit key-selector + overloads, same strict-mode divergence guard. There is deliberately no failure value: any + failure (insert ignored for another constraint, update not affecting exactly one row, a + serializer or SQLite error) throws `TychoException` after rolling back, leaving the row as it + was. (#32) + ## 5.0.0 (unreleased) — Security & performance hardening This release closes a critical SQL-injection vector and a data-integrity bug, and diff --git a/README.md b/README.md index 4e3ce25..0fe60fb 100644 --- a/README.md +++ b/README.md @@ -192,6 +192,33 @@ the call site. > cannot reach. Types registered with `AddTypeRegistrationWithCustomKeySelector` have no id > property to compare against and are unaffected. +## Knowing Whether a Write Inserted or Updated + +`WriteObjectAsync` reports only success. When you keep something derived from the store — a +queue count, an added/removed signal — you need to know whether the write **created** the row +or **replaced** one, and a read-then-write pair of your own is not atomic against other +writers. `UpsertObjectAsync` answers that from inside the connection gate and transaction: + +```csharp +var result = await db.UpsertObjectAsync(queuedItem, x => x.Key, "queue"); + +if (result == UpsertResult.Inserted) +{ + queueCount++; // exact: an edit of an already-queued item returns Updated +} +``` + +Stored contents are identical to `WriteObjectAsync`; the row is keyed by +`(Key, FullTypeName, Partition)`, so the same key in another partition or for another type is +`Inserted`. The registered-id overload `UpsertObjectAsync(obj, partition)` and the strict-mode +key-divergence guard behave exactly as they do for `WriteObjectAsync`. + +There is no third result. A call that returns has written the object; anything else — the +insert ignored for a reason other than an existing row, the update touching anything but one +row, a serializer or SQLite error on either statement — throws `TychoException` with the +transaction rolled back, so the stored row is exactly what it was before the call. Callers that +count on the result should let that exception surface rather than treat it as "updated". + ## Querying Objects TychoDB offers rich querying capabilities. diff --git a/TychoDB.UnitTests/UpsertObjectTests.cs b/TychoDB.UnitTests/UpsertObjectTests.cs new file mode 100644 index 0000000..4e86e5b --- /dev/null +++ b/TychoDB.UnitTests/UpsertObjectTests.cs @@ -0,0 +1,278 @@ +#nullable enable + +using System; +using System.IO; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Shouldly; + +namespace TychoDB.UnitTests; + +/// +/// UpsertObjectAsync exists so a caller can learn whether a write created a row or +/// replaced one without a read-then-write pair of its own. The tests pin the answer on the +/// three axes of the primary key (key, type, partition) and check the stored data matches the +/// last write. +/// +[TestClass] +public class UpsertObjectTests +{ + private const string PartitionA = "partitionA"; + private const string PartitionB = "partitionB"; + + [TestMethod] + public async Task FirstWriteOfAKey_IsInserted() + { + using var db = Connect(); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA); + + result.ShouldBe(UpsertResult.Inserted); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("one"); + } + + [TestMethod] + public async Task SecondWriteOfTheSameKey_IsUpdated_AndReplacesTheData() + { + using var db = Connect(); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA); + + result.ShouldBe(UpsertResult.Updated); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("two"); + (await db.CountObjectsAsync(PartitionA)).ShouldBe(1); + } + + [TestMethod] + public async Task ARowWrittenByWriteObjectAsync_CountsAsExisting() + { + using var db = Connect(); + await db.WriteObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA); + + result.ShouldBe(UpsertResult.Updated); + } + + [TestMethod] + public async Task SameKeyInAnotherPartition_IsInserted() + { + using var db = Connect(); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "a" }, x => x.Key, PartitionA); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "b" }, x => x.Key, PartitionB); + + result.ShouldBe(UpsertResult.Inserted); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("a"); + (await db.ReadObjectAsync("k1", PartitionB))!.Description.ShouldBe("b"); + } + + [TestMethod] + public async Task SameKeyForAnotherType_IsInserted() + { + using var db = Connect(); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "doc" }, x => x.Key, PartitionA); + + var result = await db.UpsertObjectAsync(new Other { Key = "k1", Name = "other" }, x => x.Key, PartitionA); + + result.ShouldBe(UpsertResult.Inserted); + } + + [TestMethod] + public async Task NoPartition_BehavesLikeTheEmptyPartition() + { + using var db = Connect(); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key); + + result.ShouldBe(UpsertResult.Updated); + (await db.ReadObjectAsync("k1"))!.Description.ShouldBe("two"); + } + + [TestMethod] + public async Task RegisteredIdOverload_UsesTheRegisteredKey() + { + using var db = Connect(register: true); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, PartitionA); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, PartitionA); + + result.ShouldBe(UpsertResult.Updated); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("two"); + } + + [TestMethod] + public async Task WithoutATransaction_StillReportsTheOutcome() + { + using var db = Connect(); + await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA, withTransaction: false); + + var result = await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA, withTransaction: false); + + result.ShouldBe(UpsertResult.Updated); + } + + [TestMethod] + public async Task NullObject_Throws() + { + using var db = Connect(); + + await Should.ThrowAsync( + async () => await db.UpsertObjectAsync(null!, x => x.Key, PartitionA)); + } + + [TestMethod] + public async Task NullKeySelector_Throws() + { + using var db = Connect(); + + await Should.ThrowAsync( + async () => await db.UpsertObjectAsync(new Doc { Key = "k1" }, null!, PartitionA)); + } + + [TestMethod] + public async Task InsertPathFailure_Throws_AndWritesNothing() + { + // The serializer blows up, so nothing reaches SQLite. That must surface as a + // TychoException (never as an outcome) and leave no row behind. + using var db = Connect(serializer: new ThrowingSerializer(failFromCall: 1)); + + await Should.ThrowAsync( + async () => await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA)); + + (await db.CountObjectsAsync(PartitionA)).ShouldBe(0); + } + + [TestMethod] + public async Task UpdatePathFailure_Throws_AndLeavesTheExistingRowUntouched() + { + // The row exists, so INSERT OR IGNORE is ignored and the follow-up UPDATE runs into a + // trigger that aborts it. The failure must be an exception, and the transaction must + // roll back to the original data - the "ignored, then failed" case from the review. + var (db, path) = ConnectWithPath(persistConnection: false); + using (db) + { + (await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA)) + .ShouldBe(UpsertResult.Inserted); + + AbortEveryUpdate(path); + + await Should.ThrowAsync( + async () => await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA)); + + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("one"); + (await db.CountObjectsAsync(PartitionA)).ShouldBe(1); + } + } + + [TestMethod] + public async Task AfterAFailure_TheNextUpsertStillWorks() + { + using var db = Connect(serializer: new ThrowingSerializer(failFromCall: 1, failCount: 1)); + + await Should.ThrowAsync( + async () => await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA)); + + (await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA)) + .ShouldBe(UpsertResult.Inserted); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("two"); + } + + private static Tycho Connect(bool register = false, IJsonSerializer? serializer = null) + { + return ConnectWithPath(register, serializer).Db; + } + + private static (Tycho Db, string Path) ConnectWithPath(bool register = false, IJsonSerializer? serializer = null, bool persistConnection = true) + { + var dir = Path.GetTempPath(); + var name = $"{Guid.NewGuid()}.db"; + + // Pooling keeps a closed connection's handle alive; a test that installs DDL from a + // second connection needs the first one really closed, so pooling follows persistence. + var db = new Tycho(dir, serializer ?? new NewtonsoftJsonSerializer(), dbName: name, persistConnection: persistConnection, rebuildCache: true, requireTypeRegistration: false, useConnectionPooling: persistConnection); + + if (register) + { + db.AddTypeRegistrationWithCustomKeySelector(x => x.Key); + } + + return (db.Connect(), Path.Combine(dir, name)); + } + + /// + /// Installs a trigger through a second connection so every UPDATE on JsonValue aborts. + /// Needs a Tycho opened with persistConnection: false, otherwise Tycho's held connection + /// keeps the writer lock and the DDL fails with "database is locked". + /// + private static void AbortEveryUpdate(string path) + { + using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={path}"); + connection.Open(); + + using var command = connection.CreateCommand(); + command.CommandText = + """ + CREATE TRIGGER abort_every_update BEFORE UPDATE ON JsonValue + BEGIN + SELECT RAISE(ABORT, 'update refused by test trigger'); + END; + """; + command.ExecuteNonQuery(); + } + + public class Doc + { + public string Key { get; set; } = string.Empty; + + public string Description { get; set; } = string.Empty; + } + + /// + /// Delegates to the real serializer except for a window of calls (1-based, counted per + /// serialize) during which it throws, standing in for any failure ahead of the INSERT. + /// + private sealed class ThrowingSerializer(int failFromCall, int failCount = int.MaxValue) : IJsonSerializer + { + private readonly NewtonsoftJsonSerializer _inner = new(); + + private int _calls; + + public string DateTimeSerializationFormat => _inner.DateTimeSerializationFormat; + + public object Serialize(T obj) + { + return ShouldFail() ? throw new InvalidOperationException("serializer refused") : _inner.Serialize(obj); + } + + public void Serialize(T obj, System.Buffers.IBufferWriter bufferWriter) + { + if (ShouldFail()) + { + throw new InvalidOperationException("serializer refused"); + } + + _inner.Serialize(obj, bufferWriter); + } + + public System.Threading.Tasks.ValueTask DeserializeAsync(Stream stream, System.Threading.CancellationToken cancellationToken) + { + return _inner.DeserializeAsync(stream, cancellationToken); + } + + private bool ShouldFail() + { + var call = ++_calls; + return call >= failFromCall && call - failFromCall < failCount; + } + } + + public class Other + { + public string Key { get; set; } = string.Empty; + + public string Name { get; set; } = string.Empty; + } +} diff --git a/TychoDB/Queries.cs b/TychoDB/Queries.cs index d3a7c79..3ac92fe 100644 --- a/TychoDB/Queries.cs +++ b/TychoDB/Queries.cs @@ -132,6 +132,33 @@ INSERT OR REPLACE INTO JsonValue(Key, FullTypeName, Data, Partition) SELECT last_insert_rowid(); """; + /// + /// First half of the outcome-reporting upsert: inserts only when the + /// (Key, FullTypeName, Partition) row is absent. One affected row means the object was + /// inserted; zero means it already existed and + /// runs next. + /// + public const string InsertOrIgnore = + """ + INSERT OR IGNORE INTO JsonValue(Key, FullTypeName, Data, Partition) + VALUES ($key, $fullTypeName, json($json), $partition); + """; + + /// + /// Second half of the outcome-reporting upsert: replaces the stored JSON of an existing row. + /// + public const string UpdateDataWithKeyAndFullTypeName = + """ + UPDATE JsonValue + SET Data = json($json) + WHERE + Key = $key + AND + FullTypeName = $fullTypeName + AND + Partition = $partition; + """; + private const string BatchInsertPrefix = "INSERT OR REPLACE INTO JsonValue(Key, FullTypeName, Data, Partition) VALUES "; diff --git a/TychoDB/Tycho.cs b/TychoDB/Tycho.cs index 3086ac1..a9766bd 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -556,6 +556,164 @@ public ValueTask WriteObjectsAsync(IEnumerable objs, Func cancellationToken); } + /// + /// Writes a single object and reports whether the write created the row or replaced an + /// existing one, using registered type information to determine the ID. + /// + /// The type of the object to write. + /// The object to write. + /// Optional partition key to organize objects. + /// Whether to use a transaction for the operation. + /// A token to cancel the asynchronous operation. + /// + /// when no row existed for the key in that partition, + /// when one did and its data was replaced. + /// + /// + /// Stored contents are identical to ; + /// the difference is only the answer. The insert/update decision is made inside the + /// connection gate and the transaction, so a caller keeping an incremental view of the + /// store (a queue count, an added/removed signal) can rely on it without a + /// read-then-write pair and an outer lock of its own. + /// + /// There is no failure value: the result is only ever one of the two outcomes, and a call + /// that returns has written the object. Every failure throws + /// after rolling the transaction back, so the row is left exactly as it was - that covers + /// the insert being ignored for a reason other than an existing row (the follow-up update + /// then affects nothing), the update affecting anything other than one row, and any + /// serializer or SQLite error on either statement. + /// + /// + public ValueTask UpsertObjectAsync(T obj, string? partition = null, bool withTransaction = true, + CancellationToken cancellationToken = default) + { + return UpsertObjectAsync(obj, GetIdSelectorFor(), partition, withTransaction, cancellationToken); + } + + /// + /// Writes a single object using a custom key selector and reports whether the write created + /// the row or replaced an existing one. + /// + /// The type of the object to write. + /// The object to write. + /// A function that extracts the key from the object. + /// Optional partition key to organize objects. + /// Whether to use a transaction for the operation. + /// A token to cancel the asynchronous operation. + /// + /// when no row existed for the key in that partition, + /// when one did and its data was replaced. + /// + /// + /// The key selector follows the same rules as + /// , + /// including the strict-mode divergence guard. Failure semantics are those of + /// : never a third + /// result value, always a with the transaction rolled back. + /// + public ValueTask UpsertObjectAsync(T obj, Func keySelector, string? partition = null, + bool withTransaction = true, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(obj); + ArgumentNullException.ThrowIfNull(keySelector); + ArgumentNullException.ThrowIfNull(_connection); + + keySelector = GuardAgainstKeyDivergence(keySelector); + + return _connection + .WithConnectionBlockAsync( + _connectionGate, + (obj, keySelector, partition, withTransaction, _commandTimeout, _jsonSerializer), + static (conn, state) => + { + SqliteTransaction? transaction = null; + + if (state.withTransaction) + { + transaction = conn.BeginTransaction(IsolationLevel.Serializable); + } + + try + { + var keyValue = state.keySelector(state.obj); + var fullTypeNameValue = TypeCache.FullName; + var partitionValue = state.partition.AsValueOrEmptyString(); + + using var serializationStream = _memoryStreamManager.GetStream("TychoDB.UpsertObject"); + state._jsonSerializer.Serialize(state.obj, serializationStream); + var json = serializationStream.ToArray(); + + // INSERT OR IGNORE affects one row only when the key was absent, which is + // exactly the answer; if it affected nothing the row exists and the data + // is replaced in place. Both statements run under the same gate and + // transaction, so no other writer can slip between them. + using var insertCommand = conn.CreateCommand(); + if (transaction is not null) + { + insertCommand.Transaction = transaction; + } + + insertCommand.CommandTimeout = state._commandTimeout; + insertCommand.CommandText = Queries.InsertOrIgnore; + insertCommand.Parameters.Add(ParameterKey, SqliteType.Text).Value = keyValue; + insertCommand.Parameters.Add(ParameterFullTypeName, SqliteType.Text).Value = fullTypeNameValue; + insertCommand.Parameters.Add(ParameterJson, SqliteType.Blob).Value = json; + insertCommand.Parameters.Add(ParameterPartition, SqliteType.Text).Value = partitionValue; + + var result = UpsertResult.Inserted; + var affected = insertCommand.ExecuteNonQuery(); + + if (affected == 0) + { + using var updateCommand = conn.CreateCommand(); + if (transaction is not null) + { + updateCommand.Transaction = transaction; + } + + updateCommand.CommandTimeout = state._commandTimeout; + updateCommand.CommandText = Queries.UpdateDataWithKeyAndFullTypeName; + updateCommand.Parameters.Add(ParameterKey, SqliteType.Text).Value = keyValue; + updateCommand.Parameters.Add(ParameterFullTypeName, SqliteType.Text).Value = fullTypeNameValue; + updateCommand.Parameters.Add(ParameterJson, SqliteType.Blob).Value = json; + updateCommand.Parameters.Add(ParameterPartition, SqliteType.Text).Value = partitionValue; + + affected = updateCommand.ExecuteNonQuery(); + result = UpsertResult.Updated; + } + + // Two outcomes only. INSERT OR IGNORE can be ignored for a constraint + // other than the existing-row case, and then the UPDATE finds nothing; + // either way anything but exactly one affected row is a failure, and a + // failure is an exception plus rollback - never a quiet "Updated". + if (affected != 1) + { + throw new TychoException($"Upsert affected {affected} rows; expected exactly one ({result})"); + } + + transaction?.Commit(); + + return result; + } + catch (TychoException) + { + transaction?.Rollback(); + throw; + } + catch (Exception ex) + { + transaction?.Rollback(); + throw new TychoException("Failed Upserting Object", ex); + } + finally + { + transaction?.Dispose(); + } + }, + _persistConnection, + cancellationToken); + } + /// /// Counts objects matching the optional filter criteria. /// diff --git a/TychoDB/UpsertResult.cs b/TychoDB/UpsertResult.cs new file mode 100644 index 0000000..6038974 --- /dev/null +++ b/TychoDB/UpsertResult.cs @@ -0,0 +1,14 @@ +namespace TychoDB; + +/// +/// What +/// did to the row for the object's key. +/// +public enum UpsertResult +{ + /// No row existed for the key in that partition; one was created. + Inserted, + + /// A row already existed for the key in that partition; its data was replaced. + Updated, +} From 1e245539c36649ec03665ea675a3ffa4c75a7f80 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Wed, 2 Sep 2026 08:46:35 -0500 Subject: [PATCH 2/3] review: pin idempotent-rewrite semantics, single unreleased changelog section, transaction wording - Test that rewriting identical JSON reports Updated: SQLite's change count is rows matched by the UPDATE, not rows whose bytes differed, so the exactly-one-row check cannot misfire on an idempotent upsert. - Remarks/README/CHANGELOG no longer promise a transaction when withTransaction is false; the row-left-as-it-was guarantee holds either way because only one of the two statements ever modifies data. - CHANGELOG: 5.0.0 marked released (tag date) so 5.3.0 is the only unreleased section. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 7 ++++--- README.md | 7 ++++--- TychoDB.UnitTests/UpsertObjectTests.cs | 16 ++++++++++++++++ TychoDB/Tycho.cs | 20 ++++++++++++-------- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3d48e9..8d0930a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,11 @@ read-then-write pair guarded by a lock of their own. Registered-id and explicit key-selector overloads, same strict-mode divergence guard. There is deliberately no failure value: any failure (insert ignored for another constraint, update not affecting exactly one row, a - serializer or SQLite error) throws `TychoException` after rolling back, leaving the row as it - was. (#32) + serializer or SQLite error) throws `TychoException` and leaves the row as it was — with a + transaction it is rolled back, without one the single failing statement is atomic on its own. + (#32) -## 5.0.0 (unreleased) — Security & performance hardening +## 5.0.0 — 2026-07-21 — Security & performance hardening This release closes a critical SQL-injection vector and a data-integrity bug, and adds proven write/startup performance improvements. It is a **major** version because diff --git a/README.md b/README.md index 0fe60fb..4e3aa37 100644 --- a/README.md +++ b/README.md @@ -215,9 +215,10 @@ key-divergence guard behave exactly as they do for `WriteObjectAsync`. There is no third result. A call that returns has written the object; anything else — the insert ignored for a reason other than an existing row, the update touching anything but one -row, a serializer or SQLite error on either statement — throws `TychoException` with the -transaction rolled back, so the stored row is exactly what it was before the call. Callers that -count on the result should let that exception surface rather than treat it as "updated". +row, a serializer or SQLite error on either statement — throws `TychoException` and leaves the +stored row exactly what it was before the call (only one of the two statements ever modifies +data, so this holds with `withTransaction: false` too). Callers that count on the result should +let that exception surface rather than treat it as "updated". ## Querying Objects diff --git a/TychoDB.UnitTests/UpsertObjectTests.cs b/TychoDB.UnitTests/UpsertObjectTests.cs index 4e86e5b..e2b5868 100644 --- a/TychoDB.UnitTests/UpsertObjectTests.cs +++ b/TychoDB.UnitTests/UpsertObjectTests.cs @@ -44,6 +44,22 @@ public async Task SecondWriteOfTheSameKey_IsUpdated_AndReplacesTheData() (await db.CountObjectsAsync(PartitionA)).ShouldBe(1); } + [TestMethod] + public async Task RewritingIdenticalData_IsUpdated_NotAFailure() + { + // SQLite's change count is the number of rows the UPDATE matched, not the number whose + // bytes differed, so an idempotent rewrite reports one affected row and must come back + // Updated rather than tripping the exactly-one-row check. + using var db = Connect(); + var doc = new Doc { Key = "k1", Description = "same" }; + await db.UpsertObjectAsync(doc, x => x.Key, PartitionA); + + var result = await db.UpsertObjectAsync(doc, x => x.Key, PartitionA); + + result.ShouldBe(UpsertResult.Updated); + (await db.ReadObjectAsync("k1", PartitionA))!.Description.ShouldBe("same"); + } + [TestMethod] public async Task ARowWrittenByWriteObjectAsync_CountsAsExisting() { diff --git a/TychoDB/Tycho.cs b/TychoDB/Tycho.cs index a9766bd..82fcfa7 100644 --- a/TychoDB/Tycho.cs +++ b/TychoDB/Tycho.cs @@ -572,16 +572,18 @@ public ValueTask WriteObjectsAsync(IEnumerable objs, Func /// /// Stored contents are identical to ; /// the difference is only the answer. The insert/update decision is made inside the - /// connection gate and the transaction, so a caller keeping an incremental view of the - /// store (a queue count, an added/removed signal) can rely on it without a - /// read-then-write pair and an outer lock of its own. + /// connection gate (and, when is true, the + /// transaction), so a caller keeping an incremental view of the store (a queue count, an + /// added/removed signal) can rely on it without a read-then-write pair and an outer lock + /// of its own. /// /// There is no failure value: the result is only ever one of the two outcomes, and a call /// that returns has written the object. Every failure throws - /// after rolling the transaction back, so the row is left exactly as it was - that covers - /// the insert being ignored for a reason other than an existing row (the follow-up update - /// then affects nothing), the update affecting anything other than one row, and any - /// serializer or SQLite error on either statement. + /// and leaves the row exactly as it was - only one of the two statements ever modifies + /// data, so with a transaction it is rolled back and without one the failed statement is + /// atomic on its own. That covers the insert being ignored for a reason other than an + /// existing row (the follow-up update then affects nothing), the update affecting anything + /// other than one row, and any serializer or SQLite error on either statement. /// /// public ValueTask UpsertObjectAsync(T obj, string? partition = null, bool withTransaction = true, @@ -609,7 +611,7 @@ public ValueTask UpsertObjectAsync(T obj, string? partition = n /// , /// including the strict-mode divergence guard. Failure semantics are those of /// : never a third - /// result value, always a with the transaction rolled back. + /// result value, always a with the row left as it was. /// public ValueTask UpsertObjectAsync(T obj, Func keySelector, string? partition = null, bool withTransaction = true, CancellationToken cancellationToken = default) @@ -686,6 +688,8 @@ public ValueTask UpsertObjectAsync(T obj, Func keySe // other than the existing-row case, and then the UPDATE finds nothing; // either way anything but exactly one affected row is a failure, and a // failure is an exception plus rollback - never a quiet "Updated". + // (SQLite's change count is the number of rows the UPDATE matched, so a + // rewrite with identical JSON still reports 1 - see the idempotent test.) if (affected != 1) { throw new TychoException($"Upsert affected {affected} rows; expected exactly one ({result})"); From 08e3e10bed22f78171e42e6180c94f9e0b13c0e5 Mon Sep 17 00:00:00 2001 From: Michael Stonis <120685+michaelstonis@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:45:05 -0500 Subject: [PATCH 3/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- TychoDB.UnitTests/UpsertObjectTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/TychoDB.UnitTests/UpsertObjectTests.cs b/TychoDB.UnitTests/UpsertObjectTests.cs index e2b5868..f2d8c64 100644 --- a/TychoDB.UnitTests/UpsertObjectTests.cs +++ b/TychoDB.UnitTests/UpsertObjectTests.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; using Shouldly; +using TychoDB; namespace TychoDB.UnitTests;