Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
# Changelog

## 5.0.0 (unreleased) — Security & performance hardening
## 5.3.0 (unreleased)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. 5.0.0 through 5.2.1 are all tagged, so that heading was stale; 1e24553 marks 5.0.0 with its tag date (2026-07-21), leaving 5.3.0 as the only unreleased section. (5.1.x/5.2.x never got changelog entries; left that as-is rather than backfilling here.)


### 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` 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 — 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
Expand Down
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,34 @@ 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` 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

TychoDB offers rich querying capabilities.
Expand Down
294 changes: 294 additions & 0 deletions TychoDB.UnitTests/UpsertObjectTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
#nullable enable

using System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Shouldly;

namespace TychoDB.UnitTests;
Comment thread
Copilot marked this conversation as resolved.

/// <summary>
/// <c>UpsertObjectAsync</c> 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.
/// </summary>
[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<Doc>("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<Doc>("k1", PartitionA))!.Description.ShouldBe("two");
(await db.CountObjectsAsync<Doc>(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<Doc>("k1", PartitionA))!.Description.ShouldBe("same");
}

[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<Doc>("k1", PartitionA))!.Description.ShouldBe("a");
(await db.ReadObjectAsync<Doc>("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<Doc>("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<Doc>("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<ArgumentNullException>(
async () => await db.UpsertObjectAsync<Doc>(null!, x => x.Key, PartitionA));
}

[TestMethod]
public async Task NullKeySelector_Throws()
{
using var db = Connect();

await Should.ThrowAsync<ArgumentNullException>(
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<TychoException>(
async () => await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "one" }, x => x.Key, PartitionA));

(await db.CountObjectsAsync<Doc>(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<TychoException>(
async () => await db.UpsertObjectAsync(new Doc { Key = "k1", Description = "two" }, x => x.Key, PartitionA));

(await db.ReadObjectAsync<Doc>("k1", PartitionA))!.Description.ShouldBe("one");
(await db.CountObjectsAsync<Doc>(PartitionA)).ShouldBe(1);
}
}

[TestMethod]
public async Task AfterAFailure_TheNextUpsertStillWorks()
{
using var db = Connect(serializer: new ThrowingSerializer(failFromCall: 1, failCount: 1));

await Should.ThrowAsync<TychoException>(
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<Doc>("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<Doc>(x => x.Key);
}

return (db.Connect(), Path.Combine(dir, name));
}

/// <summary>
/// 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".
/// </summary>
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;
}

/// <summary>
/// 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.
/// </summary>
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>(T obj)
{
return ShouldFail() ? throw new InvalidOperationException("serializer refused") : _inner.Serialize(obj);
}

public void Serialize<T>(T obj, System.Buffers.IBufferWriter<byte> bufferWriter)
{
if (ShouldFail())
{
throw new InvalidOperationException("serializer refused");
}

_inner.Serialize(obj, bufferWriter);
}

public System.Threading.Tasks.ValueTask<T> DeserializeAsync<T>(Stream stream, System.Threading.CancellationToken cancellationToken)
{
return _inner.DeserializeAsync<T>(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;
}
}
27 changes: 27 additions & 0 deletions TychoDB/Queries.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,33 @@ INSERT OR REPLACE INTO JsonValue(Key, FullTypeName, Data, Partition)
SELECT last_insert_rowid();
""";

/// <summary>
/// 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 <see cref="UpdateDataWithKeyAndFullTypeName"/>
/// runs next.
/// </summary>
public const string InsertOrIgnore =
"""
INSERT OR IGNORE INTO JsonValue(Key, FullTypeName, Data, Partition)
VALUES ($key, $fullTypeName, json($json), $partition);
""";

/// <summary>
/// Second half of the outcome-reporting upsert: replaces the stored JSON of an existing row.
/// </summary>
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 ";

Expand Down
Loading
Loading