diff --git a/OpenNest.Server.Tests/NestApiTests.cs b/OpenNest.Server.Tests/NestApiTests.cs index 4269cda..8ab4364 100644 --- a/OpenNest.Server.Tests/NestApiTests.cs +++ b/OpenNest.Server.Tests/NestApiTests.cs @@ -4,6 +4,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using Microsoft.Extensions.DependencyInjection; using OpenNest.Data; using OpenNest.Geometry; using OpenNest.IO; @@ -38,6 +39,30 @@ public sealed class NestApiTests Assert.False(File.Exists(defaultPath)); } + [Fact] + public async Task Health_WithOpenDatabase_ReturnsExistingSuccessShape() + { + using var factory = new ServerFactory(); + using var client = factory.CreateClient(); + using var response = await client.GetAsync("/healthz"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("{\"status\":\"ok\"}", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task Health_WithDisposedDatabase_Returns503WithoutStorageDetails() + { + using var factory = new ServerFactory(); + using var client = factory.CreateClient(); + factory.Services.GetRequiredService().Dispose(); + + using var response = await client.GetAsync("/healthz"); + + Assert.Equal(HttpStatusCode.ServiceUnavailable, response.StatusCode); + Assert.Equal("{\"status\":\"unavailable\"}", await response.Content.ReadAsStringAsync()); + } + [Fact] public async Task Create_ThenListAndDownload_ReturnsExactRecordAndArchive() { diff --git a/OpenNest.Server.Tests/NestDatabaseConcurrencyTests.cs b/OpenNest.Server.Tests/NestDatabaseConcurrencyTests.cs new file mode 100644 index 0000000..f23ac16 --- /dev/null +++ b/OpenNest.Server.Tests/NestDatabaseConcurrencyTests.cs @@ -0,0 +1,249 @@ +using System.Net; +using System.Reflection; +using System.Text.Json; +using Microsoft.Data.Sqlite; +using OpenNest.Data; + +namespace OpenNest.Server.Tests; + +public sealed class NestDatabaseConcurrencyTests +{ + // Code-level ownership gate: hold the database's monitor and observe a dedicated + // synchronous caller waiting on it, not a sleep or a stress-run timing assumption. + [Theory] + [InlineData("List")] + [InlineData("Get")] + [InlineData("GetFile")] + [InlineData("Insert")] + [InlineData("UpdateFile")] + [InlineData("UpdateMetadata")] + [InlineData("Delete")] + [InlineData("Health")] + [InlineData("Dispose")] + public async Task Concurrency_EveryOperationWaitsForTheSameMonitor(string operation) + { + using var factory = new ServerFactory(); + using var database = new NestDatabase(factory.DatabasePath); + var id = Guid.NewGuid(); + var record = new NestRecord { Name = "Monitor ownership fixture" }; + database.Insert(id, record, new byte[] { 1, 2, 3 }); + var field = typeof(NestDatabase).GetField("_sync", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + var sync = field.GetValue(database)!; + Assert.NotNull(sync); + var completed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var worker = new Thread(() => + { + completed.SetResult(Record.Exception(() => + { + switch (operation) + { + case "List": database.List(); break; + case "Get": database.Get(id); break; + case "GetFile": database.GetFile(id); break; + case "Insert": database.Insert(Guid.NewGuid(), record, new byte[] { 4 }); break; + case "UpdateFile": database.Update(id, record, new byte[] { 5 }); break; + case "UpdateMetadata": database.Update(id, record, null); break; + case "Delete": database.Delete(id); break; + case "Health": Assert.True(database.IsHealthy()); break; + case "Dispose": database.Dispose(); break; + default: throw new InvalidOperationException($"Unknown test operation: {operation}"); + } + })); + }) + { IsBackground = true }; + + var observedWaiting = false; + var completedWhileHeld = false; + lock (sync) + { + worker.Start(); + observedWaiting = SpinWait.SpinUntil(() => completed.Task.IsCompleted || + (worker.ThreadState & ThreadState.WaitSleepJoin) != 0, TimeSpan.FromSeconds(10)); + completedWhileHeld = completed.Task.IsCompleted; + } + + Assert.True(worker.Join(TimeSpan.FromSeconds(10)), "Operation did not finish after releasing the monitor."); + Assert.True(observedWaiting, "Caller never reached the operation within the watchdog."); + Assert.False(completedWhileHeld, operation + " bypassed the shared monitor."); + Assert.Null(await completed.Task); + } + + [Fact] + public void Concurrency_AllSqlStepsIncludingReadbacks_OwnTheMonitor() + { + using var factory = new ServerFactory(); + using var database = new NestDatabase(factory.DatabasePath); + var sync = typeof(NestDatabase).GetField("_sync", BindingFlags.Instance | BindingFlags.NonPublic)! + .GetValue(database)!; + var connection = (SqliteConnection)typeof(NestDatabase) + .GetField("_connection", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(database)!; + var steps = 0; + var unownedSteps = 0; + // Observe every native VM step, including iteration of readers and nested + // write readbacks. No assertion/exception is thrown across the native callback. + SQLitePCL.raw.sqlite3_progress_handler(connection.Handle, 1, _ => + { + steps++; + if (!Monitor.IsEntered(sync)) + unownedSteps++; + return 0; + }, null); + try + { + var id = Guid.NewGuid(); + var record = Fixture(0, 0); + Action[] operations = + [ + () => database.Insert(id, record, new byte[] { 1 }), + () => database.List(), + () => database.Get(id), + () => database.GetFile(id), + () => database.Update(id, record, new byte[] { 2 }), + () => database.Update(id, record, null), + () => Assert.True(database.IsHealthy()), + () => database.Delete(id), + ]; + foreach (var operation in operations) + { + var before = steps; + operation(); + Assert.True(steps > before, "Operation did not execute the observed SQLite VM."); + Assert.Equal(0, unownedSteps); + } + } + finally + { + SQLitePCL.raw.sqlite3_progress_handler(connection.Handle, 0, null, null); + } + } + + [Fact] + public async Task Concurrency_DatabaseReadersAndWriters_PreserveExactRecords() + { + using var factory = new ServerFactory(); + using var database = new NestDatabase(factory.DatabasePath); + using var start = new Barrier(4); + var workers = Enumerable.Range(0, 4).Select(lane => Task.Factory.StartNew(() => + { + Assert.True(start.SignalAndWait(TimeSpan.FromSeconds(10))); + var records = new List<(NestRecord Record, byte[] File)>(); + for (var index = 0; index < 12; index++) + { + var id = Guid.NewGuid(); + var record = Fixture(lane, index); + var file = new byte[] { (byte)lane, (byte)index, 0, 255 }; + var saved = database.Insert(id, record, file); + AssertStoredRecord(record, id, file, saved); + AssertRecord(saved, database.Get(id)!); + Assert.Contains(database.List(), r => r.Id == id); + Assert.Equal(file, database.GetFile(id)); + record.Comments = "Archive updated"; + file = file.Concat(new byte[] { 42 }).ToArray(); + saved = database.Update(id, record, file)!; + AssertStoredRecord(record, id, file, saved); + record.Comments = "Metadata only"; + saved = database.Update(id, record, null)!; + AssertStoredRecord(record, id, file, saved); + Assert.Equal(file.LongLength, saved.FileSize); + Assert.Equal(file, database.GetFile(id)); + var transient = Guid.NewGuid(); + database.Insert(transient, record, new byte[] { 9 }); + Assert.True(database.Delete(transient)); + Assert.Null(database.Get(transient)); + records.Add((saved, file)); + } + return records; + }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default)).ToArray(); + + var expected = (await Task.WhenAll(workers).WaitAsync(TimeSpan.FromSeconds(60))).SelectMany(r => r).ToArray(); + Assert.Equal(expected.Select(r => r.Record.Id).Order(), database.List().Select(r => r.Id).Order()); + foreach (var (record, file) in expected) + { + AssertRecord(record, database.Get(record.Id)!); + Assert.Equal(file, database.GetFile(record.Id)); + } + } + + [Fact] + public async Task Concurrency_MultipleHttpClients_PreserveMetadataAndSyntheticArchives() + { + using var factory = new ServerFactory(); + using var watchdog = new CancellationTokenSource(TimeSpan.FromSeconds(60)); + var start = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var workers = Enumerable.Range(0, 8).Select(async lane => + { + using var client = factory.CreateClient(); + using var repository = new RemoteNestRepository(client, client.BaseAddress!.ToString()); + await start.Task; + var records = new List<(NestRecord Record, byte[] File)>(); + for (var index = 0; index < 4; index++) + { + var nest = NestApiTests.SyntheticNest(); + nest.Name = $"Synthetic concurrent nest {lane}-{index}"; + var session = new NestSaveSession(); + var file = NestApiTests.Archive(nest); + var saved = await session.SaveAsync(repository, client.BaseAddress.ToString(), nest, file, + cancellationToken: watchdog.Token); + AssertStoredRecord(NestRecordFactory.FromNest(nest, saved.Id, file.LongLength), saved.Id, file, saved); + AssertRecord(saved, (await repository.GetMetadataAsync(saved.Id, watchdog.Token))!); + Assert.Contains(await repository.ListAsync(watchdog.Token), r => r.Id == saved.Id); + nest.Notes = $"Archive change {lane}-{index}"; + file = NestApiTests.Archive(nest); + var updated = await session.SaveAsync(repository, client.BaseAddress.ToString(), nest, file, + cancellationToken: watchdog.Token); + AssertStoredRecord(NestRecordFactory.FromNest(nest, saved.Id, file.LongLength), saved.Id, file, updated); + updated.Comments = $"Metadata only {lane}-{index}"; + saved = await repository.UpdateMetadataAsync(updated.Id, updated, watchdog.Token); + AssertStoredRecord(updated, updated.Id, file, saved); + Assert.Equal(file.LongLength, saved.FileSize); + Assert.Equal(file, await repository.GetFileAsync(saved.Id, watchdog.Token)); + using var health = await client.GetAsync("/healthz", watchdog.Token); + Assert.Equal(HttpStatusCode.OK, health.StatusCode); + records.Add((saved, file)); + } + return records; + }).ToArray(); + start.SetResult(); + + var expected = (await Task.WhenAll(workers).WaitAsync(TimeSpan.FromSeconds(70))).SelectMany(r => r).ToArray(); + using var verifierClient = factory.CreateClient(); + using var verifier = new RemoteNestRepository(verifierClient, verifierClient.BaseAddress!.ToString()); + var listed = await verifier.ListAsync(watchdog.Token); + Assert.Equal(expected.Select(r => r.Record.Id).Order(), listed.Select(r => r.Id).Order()); + foreach (var (record, file) in expected) + { + AssertRecord(record, listed.Single(r => r.Id == record.Id)); + AssertRecord(record, (await verifier.GetMetadataAsync(record.Id, watchdog.Token))!); + Assert.Equal(file, await verifier.GetFileAsync(record.Id, watchdog.Token)); + } + } + + private static NestRecord Fixture(int lane, int index) => new() + { + Name = $"Synthetic database nest {lane}-{index}", + Customer = "Synthetic customer", + DateCreated = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Unspecified), + DateModified = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Unspecified), + Material = "Synthetic alloy", + Thickness = 0.25, + Status = NestStatus.ToBeCut, + PlateCount = lane + 1, + PartCount = index + 1, + Comments = "Initial archive", + MadeBy = "Synthetic author", + }; + + private static void AssertStoredRecord(NestRecord metadata, Guid id, byte[] file, NestRecord actual) + { + var expected = JsonSerializer.Deserialize(JsonSerializer.Serialize(metadata))!; + expected.Id = id; + expected.FileSize = file.LongLength; + expected.SavedAt = actual.SavedAt; + Assert.NotEqual(default, actual.SavedAt); + AssertRecord(expected, actual); + } + + private static void AssertRecord(NestRecord expected, NestRecord actual) => + Assert.Equal(JsonSerializer.Serialize(expected), JsonSerializer.Serialize(actual)); +} diff --git a/OpenNest.Server/NestDatabase.cs b/OpenNest.Server/NestDatabase.cs index 1e6adcd..bd22fbe 100644 --- a/OpenNest.Server/NestDatabase.cs +++ b/OpenNest.Server/NestDatabase.cs @@ -11,6 +11,9 @@ namespace OpenNest.Server; /// public sealed class NestDatabase : IDisposable { + // One reentrant monitor owns each complete connection operation, including + // readers, nested Get readbacks, health checks, and disposal. + private readonly object _sync = new(); private readonly SqliteConnection _connection; public NestDatabase(string databasePath) @@ -46,80 +49,120 @@ public sealed class NestDatabase : IDisposable public IReadOnlyList List() { - using var command = _connection.CreateCommand(); - command.CommandText = - $"SELECT {RecordColumns} FROM nests ORDER BY savedAt DESC"; - var records = new List(); - using var reader = command.ExecuteReader(); - while (reader.Read()) - records.Add(ReadRecord(reader)); - return records; + lock (_sync) + { + using var command = _connection.CreateCommand(); + command.CommandText = + $"SELECT {RecordColumns} FROM nests ORDER BY savedAt DESC"; + var records = new List(); + using var reader = command.ExecuteReader(); + while (reader.Read()) + records.Add(ReadRecord(reader)); + return records; + } } public NestRecord? Get(Guid id) { - using var command = _connection.CreateCommand(); - command.CommandText = $"SELECT {RecordColumns} FROM nests WHERE id = $id"; - command.Parameters.AddWithValue("$id", id.ToString()); - using var reader = command.ExecuteReader(); - return reader.Read() ? ReadRecord(reader) : null; + lock (_sync) + { + using var command = _connection.CreateCommand(); + command.CommandText = $"SELECT {RecordColumns} FROM nests WHERE id = $id"; + command.Parameters.AddWithValue("$id", id.ToString()); + using var reader = command.ExecuteReader(); + return reader.Read() ? ReadRecord(reader) : null; + } } public byte[]? GetFile(Guid id) { - using var command = _connection.CreateCommand(); - command.CommandText = "SELECT file FROM nests WHERE id = $id"; - command.Parameters.AddWithValue("$id", id.ToString()); - var value = command.ExecuteScalar(); - return value is byte[] bytes ? bytes : null; + lock (_sync) + { + using var command = _connection.CreateCommand(); + command.CommandText = "SELECT file FROM nests WHERE id = $id"; + command.Parameters.AddWithValue("$id", id.ToString()); + var value = command.ExecuteScalar(); + return value is byte[] bytes ? bytes : null; + } } public NestRecord Insert(Guid id, NestRecord record, byte[] file) { - using var command = _connection.CreateCommand(); - command.CommandText = """ - INSERT INTO nests (id, name, customer, dateCreated, dateModified, material, - thickness, status, plateCount, partCount, comments, madeBy, fileSize, - savedAt, file) - VALUES ($id, $name, $customer, $dateCreated, $dateModified, $material, - $thickness, $status, $plateCount, $partCount, $comments, $madeBy, - $fileSize, $savedAt, $file) - """; - AddRecordParameters(command, id, record, file, updateFile: true); - command.ExecuteNonQuery(); - return Get(id) ?? throw new InvalidOperationException("Insert did not persist."); + lock (_sync) + { + using var command = _connection.CreateCommand(); + command.CommandText = """ + INSERT INTO nests (id, name, customer, dateCreated, dateModified, material, + thickness, status, plateCount, partCount, comments, madeBy, fileSize, + savedAt, file) + VALUES ($id, $name, $customer, $dateCreated, $dateModified, $material, + $thickness, $status, $plateCount, $partCount, $comments, $madeBy, + $fileSize, $savedAt, $file) + """; + AddRecordParameters(command, id, record, file, updateFile: true); + command.ExecuteNonQuery(); + return Get(id) ?? throw new InvalidOperationException("Insert did not persist."); + } } public NestRecord? Update(Guid id, NestRecord record, byte[]? file) { - if (Get(id) is null) - return null; + lock (_sync) + { + if (Get(id) is null) + return null; - using var command = _connection.CreateCommand(); - var fileClause = file is null ? "" : ", file = $file, fileSize = $fileSize"; - command.CommandText = $""" - UPDATE nests SET - name = $name, customer = $customer, dateCreated = $dateCreated, - dateModified = $dateModified, material = $material, - thickness = $thickness, status = $status, plateCount = $plateCount, - partCount = $partCount, comments = $comments, madeBy = $madeBy, - savedAt = $savedAt{fileClause} - WHERE id = $id - """; - AddRecordParameters(command, id, record, file, updateFile: file is not null); - command.ExecuteNonQuery(); - return Get(id); + using var command = _connection.CreateCommand(); + var fileClause = file is null ? "" : ", file = $file, fileSize = $fileSize"; + command.CommandText = $""" + UPDATE nests SET + name = $name, customer = $customer, dateCreated = $dateCreated, + dateModified = $dateModified, material = $material, + thickness = $thickness, status = $status, plateCount = $plateCount, + partCount = $partCount, comments = $comments, madeBy = $madeBy, + savedAt = $savedAt{fileClause} + WHERE id = $id + """; + AddRecordParameters(command, id, record, file, updateFile: file is not null); + command.ExecuteNonQuery(); + return Get(id); + } } public bool Delete(Guid id) { - using var command = _connection.CreateCommand(); - command.CommandText = "DELETE FROM nests WHERE id = $id"; - command.Parameters.AddWithValue("$id", id.ToString()); - return command.ExecuteNonQuery() > 0; + lock (_sync) + { + using var command = _connection.CreateCommand(); + command.CommandText = "DELETE FROM nests WHERE id = $id"; + command.Parameters.AddWithValue("$id", id.ToString()); + return command.ExecuteNonQuery() > 0; + } } - public void Dispose() => _connection.Dispose(); + /// Checks the live connection without exposing storage error details. + public bool IsHealthy() + { + lock (_sync) + { + try + { + using var command = _connection.CreateCommand(); + command.CommandText = "SELECT 1"; + return command.ExecuteScalar() is long value && value == 1; + } + catch (Exception ex) when (ex is SqliteException or InvalidOperationException) + { + return false; + } + } + } + + public void Dispose() + { + lock (_sync) + _connection.Dispose(); + } private void Execute(string sql) { diff --git a/OpenNest.Server/Program.cs b/OpenNest.Server/Program.cs index 10a7bf5..9af6e91 100644 --- a/OpenNest.Server/Program.cs +++ b/OpenNest.Server/Program.cs @@ -33,7 +33,9 @@ var app = builder.Build(); // Open the database now so an unusable data path fails startup, not the first request. app.Services.GetRequiredService(); -app.MapGet("/healthz", () => Results.Ok(new { status = "ok" })); +app.MapGet("/healthz", (NestDatabase db) => db.IsHealthy() + ? Results.Ok(new { status = "ok" }) + : Results.Json(new { status = "unavailable" }, statusCode: StatusCodes.Status503ServiceUnavailable)); app.MapGet("/api/nests", (NestDatabase db) => Results.Ok(db.List())); diff --git a/docs/nest-storage.md b/docs/nest-storage.md index 06db02d..0e6492d 100644 --- a/docs/nest-storage.md +++ b/docs/nest-storage.md @@ -45,7 +45,7 @@ with enums serialized as strings (`JsonSerializerDefaults.Web` + | Method | Path | Body | Response | |---|---|---|---| -| GET | `/healthz` | — | `{ "status": "ok" }` | +| GET | `/healthz` | — | 200 `{ "status": "ok" }` after a live database query; 503 `{ "status": "unavailable" }` on storage failure (no internal details) | | GET | `/api/nests` | — | `NestRecord[]`, newest `savedAt` first | | GET | `/api/nests/{id}` | — | `NestRecord` or 404 | | GET | `/api/nests/{id}/file` | — | `.nest` archive bytes (`application/zip`) or 404 | @@ -65,6 +65,18 @@ archive as a `BLOB`. The database file path comes from `--database=`, then `OPENNEST_DB`, defaulting to `./data/nests.db`. The server opens the database at startup, so an unusable path fails startup rather than the first request. +Run **one service instance with its database on a local filesystem**. Multiple PCs +may use that instance, but do not share the SQLite file between replicas or place +it on SMB/NFS storage. A private reentrant lock serializes complete database +operations (including readers, write readbacks, health checks, and disposal); +HTTP upload/body reading happens outside that lock. `/healthz` executes `SELECT 1` +on the live connection; it is a connection check, not a backup, integrity scan, +or guarantee of future disk capacity. + +Concurrent edits of the same record remain **last-writer-wins**: there is no +optimistic version check or document lock. Coordinate editing with other operators +to avoid overwriting their changes. + ## Server integration tests ```sh @@ -78,7 +90,11 @@ download, same-record update, copy, metadata-only update (archive and `fileSize` unchanged), and delete. Missing/invalid upload parts must return 400 and unknown ids 404, both leaving every stored record and archive hash unchanged. Tests never open `data/nests.db` or `OPENNEST_DB`; the temporary directory is removed when the host is -disposed. The [container smoke](#isolated-container-smoke) remains the image-level check. +disposed. Concurrent HTTP clients and database-level readers/writers must retain +exact record membership, metadata, and archive bytes; a monitor-ownership test +checks every operation without relying on stress timing. Health tests check the +unchanged success response and a detail-free 503 after the database is disposed. +The [container smoke](#isolated-container-smoke) remains the image-level check. ## Running