mirror of
https://github.com/ajisaacs/OpenNest.git
synced 2026-10-29 22:51:06 -04:00
fix(server): reject duplicate nest ids, invalid metadata and bare --database
- POST /api/nests with an existing id (active or deleted) now returns 409 with the current record instead of a 500 from the UNIQUE constraint. - Uploaded nest metadata must have string text fields and a defined status; null fields and undefined statuses (e.g. 99) are 400 instead of a 500 or a stored invalid value. - --database <path> is accepted alongside --database=<path>; a missing or repeated path fails startup instead of silently using OPENNEST_DB/default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3a7073d38c
commit
5dff83cfb8
7 files changed
+309
-37
No files matched your search
@@ -0,0 +1,152 @@
|
||||
using System.Net;
|
||||
using System.Net.Http.Json;
|
||||
using System.Text;
|
||||
using System.Text.Json;
|
||||
using OpenNest.Data;
|
||||
|
||||
namespace OpenNest.Server.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Client-allocated ids and uploaded nest metadata: conflicts are 409 and invalid
|
||||
/// metadata is 400, never a 500, and neither changes stored records.
|
||||
/// </summary>
|
||||
public sealed class NestUploadValidationTests
|
||||
{
|
||||
private static readonly JsonSerializerOptions Options = new(JsonSerializerDefaults.Web)
|
||||
{ Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() } };
|
||||
|
||||
[Fact]
|
||||
public async Task Create_WithExistingId_Returns409WithCurrentRecordAndKeepsIt()
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
var id = Guid.NewGuid();
|
||||
using var created = await client.PostAsync("/api/nests", NestRevisionTests.Body(new NestRecord { Id = id, Name = "Original" }, [1]));
|
||||
Assert.Equal(HttpStatusCode.OK, created.StatusCode);
|
||||
|
||||
using var duplicate = await client.PostAsync("/api/nests", NestRevisionTests.Body(new NestRecord { Id = id, Name = "Replacement" }, [2]));
|
||||
|
||||
Assert.Equal(HttpStatusCode.Conflict, duplicate.StatusCode);
|
||||
var current = (await duplicate.Content.ReadFromJsonAsync<NestRecord>(Options))!;
|
||||
Assert.Equal(id, current.Id);
|
||||
Assert.Equal("Original", current.Name);
|
||||
Assert.Equal(new byte[] { 1 }, await client.GetByteArrayAsync($"/api/nests/{id}/file"));
|
||||
using var reopened = new NestDatabase(factory.DatabasePath, pooling: false);
|
||||
Assert.Equal(1, reopened.History(id, 0, 100)!.Total);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Create_WithIdOfDeletedNest_Returns409AndKeepsItRecoverable()
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
var id = Guid.NewGuid();
|
||||
using var created = await client.PostAsync("/api/nests", NestRevisionTests.Body(new NestRecord { Id = id, Name = "Original" }, [1]));
|
||||
using var delete = new HttpRequestMessage(HttpMethod.Delete, $"/api/nests/{id}");
|
||||
delete.Headers.TryAddWithoutValidation("If-Match", "\"1\"");
|
||||
using var deleted = await client.SendAsync(delete);
|
||||
Assert.Equal(HttpStatusCode.OK, deleted.StatusCode);
|
||||
|
||||
using var duplicate = await client.PostAsync("/api/nests", NestRevisionTests.Body(new NestRecord { Id = id, Name = "Replacement" }, [2]));
|
||||
|
||||
Assert.Equal(HttpStatusCode.Conflict, duplicate.StatusCode);
|
||||
var current = (await duplicate.Content.ReadFromJsonAsync<NestRecord>(Options))!;
|
||||
Assert.True(current.Deleted);
|
||||
Assert.Equal(2, current.Revision);
|
||||
Assert.Equal(HttpStatusCode.NotFound, (await client.GetAsync($"/api/nests/{id}")).StatusCode);
|
||||
Assert.Equal(new byte[] { 1 }, await client.GetByteArrayAsync($"/api/nests/deleted/{id}/file"));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("""{"name":null}""")]
|
||||
[InlineData("""{"name":"A","customer":null}""")]
|
||||
[InlineData("""{"name":"A","material":null}""")]
|
||||
[InlineData("""{"name":"A","comments":null}""")]
|
||||
[InlineData("""{"name":"A","madeBy":null}""")]
|
||||
[InlineData("""{"name":"A","status":99}""")]
|
||||
[InlineData("""{"name":"A","status":"cancelled"}""")]
|
||||
public async Task Create_WithInvalidMetadata_Returns400AndStoresNothing(string metadata)
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
|
||||
using var response = await client.PostAsync("/api/nests", RawBody(metadata, [1]));
|
||||
|
||||
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||
using var reopened = new NestDatabase(factory.DatabasePath, pooling: false);
|
||||
Assert.Empty(reopened.List());
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("""{"name":"A","status":"toBeCut"}""", NestStatus.ToBeCut)]
|
||||
[InlineData("""{"name":"A","status":2}""", NestStatus.HasBeenCut)]
|
||||
public async Task Create_WithDefinedStatus_StoresIt(string metadata, NestStatus expected)
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
|
||||
using var response = await client.PostAsync("/api/nests", RawBody(metadata, [1]));
|
||||
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
Assert.Equal(expected, (await response.Content.ReadFromJsonAsync<NestRecord>(Options))!.Status);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("""{"name":null}""")]
|
||||
[InlineData("""{"name":"A","status":99}""")]
|
||||
public async Task UpdateFile_WithInvalidMetadata_Returns400AndLeavesRecordUnchanged(string metadata)
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
var id = await Create(client);
|
||||
client.DefaultRequestHeaders.TryAddWithoutValidation("If-Match", "\"1\"");
|
||||
|
||||
using var response = await client.PutAsync($"/api/nests/{id}/file", RawBody(metadata, [2]));
|
||||
|
||||
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||
await AssertUnchanged(factory, client, id);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("""{"name":null}""")]
|
||||
[InlineData("""{"name":"A","comments":null}""")]
|
||||
[InlineData("""{"name":"A","status":99}""")]
|
||||
public async Task UpdateMetadata_WithInvalidMetadata_Returns400AndLeavesRecordUnchanged(string metadata)
|
||||
{
|
||||
using var factory = new ServerFactory();
|
||||
using var client = factory.CreateClient();
|
||||
var id = await Create(client);
|
||||
client.DefaultRequestHeaders.TryAddWithoutValidation("If-Match", "\"1\"");
|
||||
|
||||
using var response = await client.PutAsync($"/api/nests/{id}/metadata",
|
||||
new StringContent(metadata, Encoding.UTF8, "application/json"));
|
||||
|
||||
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
|
||||
await AssertUnchanged(factory, client, id);
|
||||
}
|
||||
|
||||
private static async Task<Guid> Create(HttpClient client)
|
||||
{
|
||||
using var created = await client.PostAsync("/api/nests", NestRevisionTests.Body(new NestRecord { Name = "Original" }, [1]));
|
||||
Assert.Equal(HttpStatusCode.OK, created.StatusCode);
|
||||
return (await created.Content.ReadFromJsonAsync<NestRecord>(Options))!.Id;
|
||||
}
|
||||
|
||||
private static async Task AssertUnchanged(ServerFactory factory, HttpClient client, Guid id)
|
||||
{
|
||||
Assert.Equal(new byte[] { 1 }, await client.GetByteArrayAsync($"/api/nests/{id}/file"));
|
||||
using var reopened = new NestDatabase(factory.DatabasePath, pooling: false);
|
||||
var current = reopened.Get(id)!;
|
||||
Assert.Equal("Original", current.Name);
|
||||
Assert.Equal(1, current.Revision);
|
||||
Assert.Equal(1, reopened.History(id, 0, 100)!.Total);
|
||||
}
|
||||
|
||||
private static MultipartFormDataContent RawBody(string metadata, byte[] bytes)
|
||||
{
|
||||
var body = new MultipartFormDataContent();
|
||||
body.Add(new StringContent(metadata), "metadata");
|
||||
body.Add(new ByteArrayContent(bytes), "file", "fixture.nest");
|
||||
return body;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
namespace OpenNest.Server.Tests;
|
||||
|
||||
public sealed class ServerOptionsTests
|
||||
{
|
||||
private static readonly string Default = Path.Combine("data", "nests.db");
|
||||
|
||||
[Theory]
|
||||
[InlineData(new[] { "--database=/srv/a.db" }, "/srv/a.db")]
|
||||
[InlineData(new[] { "--database", "/srv/a.db" }, "/srv/a.db")]
|
||||
[InlineData(new[] { "--urls=http://127.0.0.1:8090", "--database", "/srv/a.db" }, "/srv/a.db")]
|
||||
[InlineData(new[] { "--database=/srv/a=b.db" }, "/srv/a=b.db")]
|
||||
public void DatabasePath_AcceptsBothForms_AndOverridesEnvironment(string[] args, string expected)
|
||||
{
|
||||
Assert.Equal(expected, ServerOptions.DatabasePath(args, "/env/nests.db"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DatabasePath_WithoutArgument_UsesEnvironmentThenDefault()
|
||||
{
|
||||
Assert.Equal("/env/nests.db", ServerOptions.DatabasePath(["--urls=http://127.0.0.1:8090"], "/env/nests.db"));
|
||||
Assert.Equal(Default, ServerOptions.DatabasePath([], null));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("--database")]
|
||||
[InlineData("--database", "--urls=http://127.0.0.1:8090")]
|
||||
[InlineData("--database=")]
|
||||
[InlineData("--database= ")]
|
||||
[InlineData("--database=/a.db", "--database", "/b.db")]
|
||||
public void DatabasePath_MissingOrRepeated_FailsInsteadOfFallingBack(params string[] args)
|
||||
{
|
||||
Assert.Throws<ArgumentException>(() => ServerOptions.DatabasePath(args, "/env/nests.db"));
|
||||
}
|
||||
}
|
||||
@@ -142,6 +142,9 @@ public sealed partial class NestDatabase : IDisposable
|
||||
lock (_sync)
|
||||
{
|
||||
using var transaction = _connection.BeginTransaction();
|
||||
// Ids are client-allocated: an existing id, active or deleted, is a conflict, not a replacement.
|
||||
if ((Get(id) ?? Get(id, deleted: true)) is { } existing)
|
||||
throw new RevisionConflictException(existing, "A nest with this id already exists.");
|
||||
using var command = _connection.CreateCommand();
|
||||
command.Transaction = transaction;
|
||||
command.CommandText = """
|
||||
@@ -488,7 +491,8 @@ public sealed partial class NestDatabase : IDisposable
|
||||
}
|
||||
}
|
||||
|
||||
public sealed class RevisionConflictException(NestRecord current) : Exception("The nest has changed. Reload or Save As.")
|
||||
public sealed class RevisionConflictException(NestRecord current, string message = "The nest has changed. Reload or Save As.")
|
||||
: Exception(message)
|
||||
{
|
||||
public NestRecord Current { get; } = current;
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using OpenNest.Data;
|
||||
|
||||
namespace OpenNest.Server;
|
||||
|
||||
/// <summary>
|
||||
/// Reads and checks uploaded nest metadata. Text fields must be present and the status
|
||||
/// must be a defined value, so invalid metadata is a 400 and never reaches storage.
|
||||
/// </summary>
|
||||
internal static class NestMetadata
|
||||
{
|
||||
// Integer statuses stay accepted for existing callers; GetError refuses undefined ones.
|
||||
public static readonly JsonSerializerOptions Options = new(JsonSerializerDefaults.Web)
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) },
|
||||
};
|
||||
|
||||
public static bool TryRead(string? json, out NestRecord record, out string error)
|
||||
{
|
||||
record = new NestRecord();
|
||||
if (string.IsNullOrWhiteSpace(json))
|
||||
{
|
||||
error = "Missing 'metadata' part.";
|
||||
return false;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
if (JsonSerializer.Deserialize<NestRecord>(json, Options) is not { } parsed)
|
||||
{
|
||||
error = "Missing metadata.";
|
||||
return false;
|
||||
}
|
||||
|
||||
record = parsed;
|
||||
}
|
||||
catch (JsonException)
|
||||
{
|
||||
error = "Invalid metadata JSON.";
|
||||
return false;
|
||||
}
|
||||
|
||||
error = GetError(record) ?? "";
|
||||
return error.Length == 0;
|
||||
}
|
||||
|
||||
public static string? GetError(NestRecord record)
|
||||
{
|
||||
if (record.Name is null) return "name must be a string.";
|
||||
if (record.Customer is null) return "customer must be a string.";
|
||||
if (record.Material is null) return "material must be a string.";
|
||||
if (record.Comments is null) return "comments must be a string.";
|
||||
if (record.MadeBy is null) return "madeBy must be a string.";
|
||||
if (!Enum.IsDefined(record.Status)) return "status must be quote, toBeCut or hasBeenCut.";
|
||||
if (!double.IsFinite(record.Thickness)) return "thickness must be a finite number.";
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -20,11 +20,8 @@ builder.Services.ConfigureHttpJsonOptions(options =>
|
||||
options.SerializerOptions.Converters.Add(converter);
|
||||
});
|
||||
|
||||
// Storage path: --database <path> or OPENNEST_DB, default ./data/nests.db (a Docker volume).
|
||||
var databasePath =
|
||||
args.FirstOrDefault(a => a.StartsWith("--database=", StringComparison.Ordinal))?.Split('=', 2)[1]
|
||||
?? Environment.GetEnvironmentVariable("OPENNEST_DB")
|
||||
?? Path.Combine("data", "nests.db");
|
||||
// Storage path: --database=<path> or --database <path>, then OPENNEST_DB, default ./data/nests.db (a Docker volume).
|
||||
var databasePath = ServerOptions.DatabasePath(args, Environment.GetEnvironmentVariable(ServerOptions.DatabaseVariable));
|
||||
builder.Services.AddSingleton(_ => new NestDatabase(databasePath));
|
||||
|
||||
// Listen port: --urls or ASPNETCORE_URLS; Dockerfile defaults to 8090.
|
||||
@@ -60,7 +57,8 @@ app.MapPost("/api/nests", async (HttpContext http, NestDatabase db) =>
|
||||
return error;
|
||||
|
||||
var id = record!.Id == Guid.Empty ? Guid.NewGuid() : record.Id;
|
||||
return Results.Ok(db.Insert(id, record, file!));
|
||||
try { return Results.Ok(db.Insert(id, record, file!)); }
|
||||
catch (RevisionConflictException conflict) { return Results.Json(conflict.Current, statusCode: 409); }
|
||||
});
|
||||
app.MapGet("/api/nests/{id:guid}/revisions", (Guid id, HttpRequest request, NestDatabase db) =>
|
||||
ReadHistory(id, request, db));
|
||||
@@ -90,10 +88,11 @@ app.MapPut("/api/nests/{id:guid}/metadata", async (Guid id, HttpRequest request,
|
||||
var error = IfMatch.Read(request, out var expected);
|
||||
if (error is not null) return error;
|
||||
NestRecord? record;
|
||||
try { record = await request.ReadFromJsonAsync<NestRecord>(); }
|
||||
try { record = await request.ReadFromJsonAsync<NestRecord>(NestMetadata.Options); }
|
||||
catch (Exception ex) when (ex is JsonException or InvalidOperationException or BadHttpRequestException)
|
||||
{ return Results.BadRequest("Invalid metadata JSON."); }
|
||||
if (record is null) return Results.BadRequest("Missing metadata.");
|
||||
if (NestMetadata.GetError(record) is { } invalid) return Results.BadRequest(invalid);
|
||||
try
|
||||
{
|
||||
var updated = db.UpdateMetadata(id, record, expected);
|
||||
@@ -184,22 +183,8 @@ static async Task<(NestRecord? Record, byte[]? File, IResult? Error)> ReadMultip
|
||||
return (null, null, Results.BadRequest("Malformed multipart body."));
|
||||
}
|
||||
|
||||
var metadataPart = form["metadata"].FirstOrDefault();
|
||||
if (string.IsNullOrWhiteSpace(metadataPart))
|
||||
return (null, null, Results.BadRequest("Missing 'metadata' part."));
|
||||
|
||||
NestRecord? record;
|
||||
try
|
||||
{
|
||||
record = JsonSerializer.Deserialize<NestRecord>(metadataPart, MultipartJson.Options);
|
||||
}
|
||||
catch (JsonException)
|
||||
{
|
||||
return (null, null, Results.BadRequest("Invalid metadata JSON."));
|
||||
}
|
||||
|
||||
if (record is null)
|
||||
return (null, null, Results.BadRequest("Missing metadata."));
|
||||
if (!NestMetadata.TryRead(form["metadata"].FirstOrDefault(), out var record, out var metadataError))
|
||||
return (null, null, Results.BadRequest(metadataError));
|
||||
|
||||
var filePart = form.Files["file"];
|
||||
if (filePart is null || filePart.Length == 0)
|
||||
@@ -211,14 +196,5 @@ static async Task<(NestRecord? Record, byte[]? File, IResult? Error)> ReadMultip
|
||||
return (record, buffer.ToArray(), null);
|
||||
}
|
||||
|
||||
internal static class MultipartJson
|
||||
{
|
||||
public static readonly JsonSerializerOptions Options = new(JsonSerializerDefaults.Web)
|
||||
{
|
||||
PropertyNameCaseInsensitive = true,
|
||||
Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) },
|
||||
};
|
||||
}
|
||||
|
||||
// Entry-point type for in-memory integration tests (WebApplicationFactory<Program>).
|
||||
public partial class Program { }
|
||||
@@ -0,0 +1,43 @@
|
||||
namespace OpenNest.Server;
|
||||
|
||||
/// <summary>Startup settings read from the command line and environment.</summary>
|
||||
public static class ServerOptions
|
||||
{
|
||||
public const string DatabaseVariable = "OPENNEST_DB";
|
||||
|
||||
/// <summary>
|
||||
/// The database path: <c>--database=<path></c> or <c>--database <path></c>, then
|
||||
/// <c>OPENNEST_DB</c>, then <c>./data/nests.db</c>. A <c>--database</c> without a path,
|
||||
/// or given twice, fails startup instead of silently storing nests elsewhere.
|
||||
/// </summary>
|
||||
public static string DatabasePath(IReadOnlyList<string> args, string? environmentPath)
|
||||
{
|
||||
string? path = null;
|
||||
for (var i = 0; i < args.Count; i++)
|
||||
{
|
||||
string value;
|
||||
if (args[i] == "--database")
|
||||
{
|
||||
if (i + 1 >= args.Count || args[i + 1].StartsWith("--", StringComparison.Ordinal))
|
||||
throw new ArgumentException("--database requires a path.", nameof(args));
|
||||
value = args[++i];
|
||||
}
|
||||
else if (args[i].StartsWith("--database=", StringComparison.Ordinal))
|
||||
{
|
||||
value = args[i]["--database=".Length..];
|
||||
}
|
||||
else
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
if (string.IsNullOrWhiteSpace(value))
|
||||
throw new ArgumentException("--database requires a path.", nameof(args));
|
||||
if (path is not null)
|
||||
throw new ArgumentException("--database was given more than once.", nameof(args));
|
||||
path = value;
|
||||
}
|
||||
|
||||
return path ?? environmentPath ?? Path.Combine("data", "nests.db");
|
||||
}
|
||||
}
|
||||
@@ -65,7 +65,9 @@ installs are unaffected until an operator opts in via **File > Storage Mode...**
|
||||
(`RemoteNestRepository`) and the server (`OpenNest.Server`). JSON is camelCase
|
||||
with enums serialized as strings (`JsonSerializerDefaults.Web` +
|
||||
`JsonStringEnumConverter(JsonNamingPolicy.CamelCase)`), matching the rest of
|
||||
`OpenNest.Data`'s local JSON stores.
|
||||
`OpenNest.Data`'s local JSON stores. Uploaded metadata (POST and both PUTs) must
|
||||
carry string text fields (`null` is 400) and a defined `status`: a name, or for
|
||||
existing callers its integer value 0–2; an undefined value is 400 and stores nothing.
|
||||
|
||||
| Field | Type | Notes |
|
||||
|---|---|---|
|
||||
@@ -94,7 +96,7 @@ with enums serialized as strings (`JsonSerializerDefaults.Web` +
|
||||
| GET | `/api/nests/query?search=&sort=&order=&offset=&limit=` | — | `{ "items": NestRecord[], "total", "offset", "limit" }`: one bounded page filtered in SQL (see below), or 400 for an invalid parameter |
|
||||
| GET | `/api/nests/{id}` | — | `NestRecord` or 404 |
|
||||
| GET | `/api/nests/{id}/file` | — | `.nest` archive bytes (`application/zip`) or 404 |
|
||||
| POST | `/api/nests` | multipart: `metadata` (JSON `NestRecord`) + `file` (`.nest` bytes) | `NestRecord` with server-assigned `id` when the client sends an empty guid |
|
||||
| POST | `/api/nests` | multipart: `metadata` (JSON `NestRecord`) + `file` (`.nest` bytes) | `NestRecord` with server-assigned `id` when the client sends an empty guid; 409 with the current record (active or deleted) when the `id` already exists, storing nothing |
|
||||
| PUT | `/api/nests/{id}/file` | multipart: `metadata` + `file` | Updated `NestRecord`, or 404 if `id` is unknown |
|
||||
| PUT | `/api/nests/{id}/metadata` | JSON `NestRecord` | Updated `NestRecord` (archive untouched), or 404 |
|
||||
| DELETE | `/api/nests/{id}` | — | Updated hidden `NestRecord`, or 404 |
|
||||
@@ -291,8 +293,9 @@ revision; save the nest to keep it.
|
||||
|
||||
`OpenNest.Server.NestDatabase` uses SQLite (`Microsoft.Data.Sqlite`) with
|
||||
`journal_mode=WAL`. Each row holds the metadata columns plus the `.nest`
|
||||
archive as a `BLOB`. The database file path comes from `--database=<path>`,
|
||||
then `OPENNEST_DB`, defaulting to `./data/nests.db`. The server opens the
|
||||
archive as a `BLOB`. The database file path comes from `--database=<path>` or
|
||||
`--database <path>`, then `OPENNEST_DB`, defaulting to `./data/nests.db`; a
|
||||
`--database` without a path, or given twice, fails startup. 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
|
||||
|
||||
Reference in new issue
Block a user