fix(ui): schema-library delete-audit name + busy guard + edit-row guard + sanitized create-race test (#260)
This commit is contained in:
@@ -93,10 +93,13 @@
|
|||||||
<td>@(string.IsNullOrWhiteSpace(s.Scope) ? "—" : s.Scope)</td>
|
<td>@(string.IsNullOrWhiteSpace(s.Scope) ? "—" : s.Scope)</td>
|
||||||
<td><code>lib:@s.Name</code></td>
|
<td><code>lib:@s.Name</code></td>
|
||||||
<td class="text-end">
|
<td class="text-end">
|
||||||
|
@* Row actions are disabled while the editor is open so the
|
||||||
|
row under edit (and its siblings) can't be deleted out from
|
||||||
|
under the form, and while a delete is in flight (_busy). *@
|
||||||
<button class="btn btn-outline-primary btn-sm me-1"
|
<button class="btn btn-outline-primary btn-sm me-1"
|
||||||
@onclick="() => BeginEdit(s)">Edit</button>
|
@onclick="() => BeginEdit(s)" disabled="@(_editing || _busy)">Edit</button>
|
||||||
<button class="btn btn-outline-danger btn-sm"
|
<button class="btn btn-outline-danger btn-sm"
|
||||||
@onclick="() => DeleteSchema(s)">Delete</button>
|
@onclick="() => DeleteSchema(s)" disabled="@(_editing || _busy)">Delete</button>
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
}
|
}
|
||||||
@@ -208,21 +211,34 @@
|
|||||||
|
|
||||||
private async Task DeleteSchema(SharedSchema schema)
|
private async Task DeleteSchema(SharedSchema schema)
|
||||||
{
|
{
|
||||||
|
// In-flight guard: an editor-open row action is already disabled in the markup,
|
||||||
|
// but the _busy gate is the authoritative guard against a double-invoked delete
|
||||||
|
// (and mirrors the Save path's guard).
|
||||||
|
if (_busy) return;
|
||||||
|
|
||||||
var confirmed = await Dialog.ConfirmAsync(
|
var confirmed = await Dialog.ConfirmAsync(
|
||||||
"Delete Schema",
|
"Delete Schema",
|
||||||
$"Delete library schema '{schema.Name}'? References to lib:{schema.Name} will no longer resolve.",
|
$"Delete library schema '{schema.Name}'? References to lib:{schema.Name} will no longer resolve.",
|
||||||
danger: true);
|
danger: true);
|
||||||
if (!confirmed) return;
|
if (!confirmed) return;
|
||||||
|
|
||||||
var result = await SchemaLibraryService.DeleteAsync(schema.Id);
|
_busy = true;
|
||||||
if (result.Success)
|
try
|
||||||
{
|
{
|
||||||
_toast.ShowSuccess($"Schema '{schema.Name}' deleted.");
|
var result = await SchemaLibraryService.DeleteAsync(schema.Id);
|
||||||
await LoadAsync();
|
if (result.Success)
|
||||||
|
{
|
||||||
|
_toast.ShowSuccess($"Schema '{schema.Name}' deleted.");
|
||||||
|
await LoadAsync();
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
_toast.ShowError(result.Error ?? "Delete failed.");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
else
|
finally
|
||||||
{
|
{
|
||||||
_toast.ShowError(result.Error ?? "Delete failed.");
|
_busy = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,8 +31,11 @@ public sealed class SchemaLibraryQueryService : ISchemaLibraryQueryService
|
|||||||
var repo = scope.ServiceProvider.GetRequiredService<ISharedSchemaRepository>();
|
var repo = scope.ServiceProvider.GetRequiredService<ISharedSchemaRepository>();
|
||||||
var all = await repo.ListAsync(cancellationToken);
|
var all = await repo.ListAsync(cancellationToken);
|
||||||
|
|
||||||
// Ordinal-keyed to match the lib:Name resolver's exact-name lookup. Last-wins on
|
// Ordinal-keyed to match the lib:Name resolver's exact-name lookup. Name is
|
||||||
// the (DB-unique) name guards against a transient duplicate read.
|
// DB-unique, so a list yields at most one row per name and no real collision
|
||||||
|
// occurs; the indexer assignment is defensive only — should two rows ever share
|
||||||
|
// a name (e.g. a mid-write transient read), the later one in enumeration order
|
||||||
|
// overwrites the earlier rather than throwing.
|
||||||
var map = new Dictionary<string, string>(StringComparer.Ordinal);
|
var map = new Dictionary<string, string>(StringComparer.Ordinal);
|
||||||
foreach (var schema in all)
|
foreach (var schema in all)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -2323,8 +2323,13 @@ public class ManagementActor : ReceiveActor
|
|||||||
private static async Task<object?> HandleDeleteSharedSchema(IServiceProvider sp, DeleteSharedSchemaCommand cmd, string user)
|
private static async Task<object?> HandleDeleteSharedSchema(IServiceProvider sp, DeleteSharedSchemaCommand cmd, string user)
|
||||||
{
|
{
|
||||||
var repo = sp.GetRequiredService<ISharedSchemaRepository>();
|
var repo = sp.GetRequiredService<ISharedSchemaRepository>();
|
||||||
|
// Pre-fetch the human-readable name before the row is gone so the audit
|
||||||
|
// EntityName records "Address" rather than the numeric id — mirroring the
|
||||||
|
// Site delete handler. Falls back to the id when the row is already absent.
|
||||||
|
var schema = await repo.GetByIdAsync(cmd.SharedSchemaId);
|
||||||
await repo.DeleteAsync(cmd.SharedSchemaId);
|
await repo.DeleteAsync(cmd.SharedSchemaId);
|
||||||
await AuditAsync(sp, user, "Delete", "SharedSchema", cmd.SharedSchemaId.ToString(), cmd.SharedSchemaId.ToString(), null);
|
await AuditAsync(sp, user, "Delete", "SharedSchema", cmd.SharedSchemaId.ToString(),
|
||||||
|
schema?.Name ?? cmd.SharedSchemaId.ToString(), null);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -187,6 +187,26 @@ public class SchemaLibraryPageTests : BunitContext
|
|||||||
9, "Address", Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<CancellationToken>()));
|
9, "Address", Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<CancellationToken>()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Editing_DisablesRowActions_SoTheRowUnderEditCannotBeDeleted()
|
||||||
|
{
|
||||||
|
// #260: while the edit form is open the list still shows the row, but its
|
||||||
|
// Edit/Delete actions must be disabled so the row under edit (or a sibling)
|
||||||
|
// can't be deleted out from under the form.
|
||||||
|
SeedSchemas(new SharedSchema { Id = 9, Name = "Address", Scope = "us", SchemaJson = "{\"type\":\"object\"}" });
|
||||||
|
|
||||||
|
var cut = Render<SchemaLibraryPage>();
|
||||||
|
cut.WaitForState(() => cut.Markup.Contains("Address"));
|
||||||
|
|
||||||
|
cut.FindAll("tbody tr button").First(b => b.TextContent.Contains("Edit")).Click();
|
||||||
|
Assert.Contains("Edit Schema: Address", cut.Markup);
|
||||||
|
|
||||||
|
// Every row action button is now disabled while the editor is open.
|
||||||
|
var rowButtons = cut.FindAll("tbody tr button");
|
||||||
|
Assert.NotEmpty(rowButtons);
|
||||||
|
Assert.All(rowButtons, b => Assert.True(b.HasAttribute("disabled")));
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>A dialog service that auto-confirms, so the delete path runs end-to-end.</summary>
|
/// <summary>A dialog service that auto-confirms, so the delete path runs end-to-end.</summary>
|
||||||
private sealed class AlwaysConfirmDialogService : IDialogService
|
private sealed class AlwaysConfirmDialogService : IDialogService
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -302,6 +302,28 @@ public class SchemaLibraryHandlerTests : TestKit, IDisposable
|
|||||||
_schemaRepo.Received(1).DeleteAsync(5, Arg.Any<CancellationToken>());
|
_schemaRepo.Received(1).DeleteAsync(5, Arg.Any<CancellationToken>());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DeleteSharedSchema_RecordsHumanReadableNameInAudit()
|
||||||
|
{
|
||||||
|
// #260: the delete handler pre-fetches the row name BEFORE deleting so the
|
||||||
|
// audit EntityName records "Addr" rather than the numeric id (mirroring the
|
||||||
|
// Site delete handler).
|
||||||
|
_schemaRepo.GetByIdAsync(5, Arg.Any<CancellationToken>())
|
||||||
|
.Returns(new SharedSchema { Id = 5, Name = "Addr", SchemaJson = ValidSchema });
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = Envelope(new DeleteSharedSchemaCommand(5), "Designer");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||||
|
// LogAsync(user, action, entityType, entityId, entityName, afterState, ct):
|
||||||
|
// entityId is the id ("5"), entityName is the resolved name ("Addr").
|
||||||
|
_auditService.Received(1).LogAsync(
|
||||||
|
Arg.Any<string>(), "Delete", "SharedSchema", "5", "Addr",
|
||||||
|
Arg.Any<object?>(), Arg.Any<CancellationToken>());
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void ListSharedSchemas_WhenRepoThrows_ReturnsError()
|
public void ListSharedSchemas_WhenRepoThrows_ReturnsError()
|
||||||
{
|
{
|
||||||
@@ -317,4 +339,48 @@ public class SchemaLibraryHandlerTests : TestKit, IDisposable
|
|||||||
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||||
Assert.DoesNotContain("db down secret detail", response.Error);
|
Assert.DoesNotContain("db down secret detail", response.Error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Concurrent-create race (#260) ───────────────────────────────────────
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateSharedSchema_WhenInsertRacesUniqueViolation_SurfacesSanitizedError()
|
||||||
|
{
|
||||||
|
// Race: GetByNameAsync sees no row, but a concurrent create commits the
|
||||||
|
// same unique Name before our AddAsync, so the INSERT hits the DB unique
|
||||||
|
// constraint. EF surfaces this as a DbUpdateException whose message carries
|
||||||
|
// raw DB internals (server/constraint/SQL detail). The handler does NOT
|
||||||
|
// catch this as a curated ManagementCommandException, so the actor's
|
||||||
|
// top-level failure path must sanitize it to a generic, correlation-tagged
|
||||||
|
// message — never leaking the raw SQL/DB text to the caller.
|
||||||
|
//
|
||||||
|
// Modeled with a SqlException-shaped fault rather than a real EF
|
||||||
|
// DbUpdateException so the ManagementService.Tests project takes no new
|
||||||
|
// EntityFrameworkCore package reference; the actor's sanitization keys on
|
||||||
|
// the exception NOT being a ManagementCommandException, which is identical
|
||||||
|
// for both types — so this exercises the exact same code path.
|
||||||
|
const string rawDbDetail =
|
||||||
|
"Cannot insert duplicate key row in object 'dbo.SharedSchemas' with unique " +
|
||||||
|
"index 'IX_SharedSchemas_Name'. The duplicate key value is (Addr). " +
|
||||||
|
"INSERT INTO [SharedSchemas] ...";
|
||||||
|
|
||||||
|
_schemaRepo.GetByNameAsync("Addr", Arg.Any<CancellationToken>())
|
||||||
|
.Returns((SharedSchema?)null);
|
||||||
|
_schemaRepo.AddAsync(Arg.Any<SharedSchema>(), Arg.Any<CancellationToken>())
|
||||||
|
.ThrowsAsync(new InvalidOperationException(rawDbDetail));
|
||||||
|
|
||||||
|
var actor = CreateActor();
|
||||||
|
var envelope = Envelope(
|
||||||
|
new CreateSharedSchemaCommand("Addr", null, ValidSchema), "Designer");
|
||||||
|
|
||||||
|
actor.Tell(envelope);
|
||||||
|
|
||||||
|
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||||
|
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||||
|
// Sanitized: the generic internal-error message, NOT the raw DB detail.
|
||||||
|
Assert.Contains("internal error", response.Error, StringComparison.OrdinalIgnoreCase);
|
||||||
|
Assert.DoesNotContain("SharedSchemas", response.Error);
|
||||||
|
Assert.DoesNotContain("IX_SharedSchemas_Name", response.Error);
|
||||||
|
Assert.DoesNotContain("INSERT INTO", response.Error);
|
||||||
|
Assert.DoesNotContain("duplicate key", response.Error);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user