diff --git a/src/FluentCMS.Admin/AdminLimits.cs b/src/FluentCMS.Admin/AdminLimits.cs new file mode 100644 index 000000000..4e2787cb8 --- /dev/null +++ b/src/FluentCMS.Admin/AdminLimits.cs @@ -0,0 +1,13 @@ +namespace FluentCMS.Admin; + +// Sizes the host and the admin's components must agree on. +public static class AdminLimits +{ + // The largest message the admin's circuit hub takes, so a component's prerendered state, carried in the circuit's first message, + // fits. + public const long CircuitStartBytes = 1024 * 1024; + + // The most a component keeps, measured as its JSON. Blazor encodes kept state twice and encrypts it on the way, so it reaches the + // hub about 1.8 times as large: 480 KB of JSON stays under the hub's limit with room for the page's other state. + public const int KeptStateBytes = 480 * 1024; +} diff --git a/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor b/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor index 90bfed11b..c12d622cc 100644 --- a/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor +++ b/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor @@ -185,6 +185,10 @@ @if (Openable(source) is { } address) { @source + @if (AsciiHost(address) is { } host) + { + @L["(host {0})", host] + } } else { @@ -607,6 +611,15 @@ _ => L["The set was reverted. Nothing more can be done with it."], }; + // A host written with other than ASCII letters, such as a Cyrillic "а" in a look-alike of apple.com, is shown as the host it + // resolves to, so a reader sees xn--pple-43d.com rather than what looks like apple.com. + public static string? AsciiHost(string address) + { + return Uri.TryCreate(address, UriKind.Absolute, out var uri) && uri.HostNameType == UriHostNameType.Dns && uri.Host != uri.IdnHost + ? uri.IdnHost + : null; + } + // Only a web address becomes a link, so a source such as "javascript:" or a file path is never followed. public static string? Openable(string source) { @@ -654,9 +667,9 @@ "change_set.published" => L["Published"], "change_set.rejected" => L["Rejected"], "change_set.reverted" => L["Reverted"], - _ => operation, + _ => L["Other event ({0})", operation], }; - private static RenderFragment Time(DateTimeOffset time) => - @; + private RenderFragment Time(DateTimeOffset time) => + @; } diff --git a/src/FluentCMS.Admin/Components/Review/ChangeSetItems.razor b/src/FluentCMS.Admin/Components/Review/ChangeSetItems.razor index a934aaf53..1fd2b688c 100644 --- a/src/FluentCMS.Admin/Components/Review/ChangeSetItems.razor +++ b/src/FluentCMS.Admin/Components/Review/ChangeSetItems.razor @@ -1,10 +1,14 @@ +@using System.Text.Json @using FluentCMS.Core.ChangeSets @using Microsoft.Extensions.DependencyInjection +@implements IDisposable @inject IServiceScopeFactory Scopes +@inject PersistentComponentState State @inject IStringLocalizer L @* The set view's interactive island: DiffView adds items in batches over the circuit (PERF-04) while the rest of the page stays static. *@ -@* The circuit replaces the prerendered items when it attaches; data-interactive says it has, for tests that drive the keyboard. *@ +@* The prerendered diff is carried into the circuit, so the circuit shows the same items without reading them again or emptying the + list. data-interactive says the circuit has attached, for tests that drive the keyboard. *@
@if (_error is not null) { @@ -23,9 +27,12 @@ @code { private ChangeSetDiff? _diff; private string? _error; + private PersistingComponentStateSubscription? _persisting; [Parameter, EditorRequired] public Guid ChangeSetId { get; set; } + private string Key => $"change-set-items-{ChangeSetId:N}"; + protected override async Task OnParametersSetAsync() { if (_diff?.ChangeSetId == ChangeSetId) @@ -33,6 +40,12 @@ return; } + if (State.TryTakeFromJson(Key, out var kept) && kept is not null) + { + _diff = kept.ToDiff(); + return; + } + // Prerendered while the set view may still be loading: an async read on PostgreSQL can finish at once, so the view renders the // island before its own reads are done. Its own scope keeps the two off one DbContext. await using var scope = Scopes.CreateAsyncScope(); @@ -40,6 +53,7 @@ if (diff.IsSuccess) { (_diff, _error) = (diff.Value, null); + _persisting ??= State.RegisterOnPersisting(Keep, RenderMode.InteractiveServer); } else { @@ -51,4 +65,59 @@ })); } } + + // A diff too large for the circuit's first message is left for the circuit to read again. + private Task Keep() + { + if (_diff is { } diff) + { + var kept = KeptDiff.From(diff); + if (JsonSerializer.SerializeToUtf8Bytes(kept).Length <= AdminLimits.KeptStateBytes) + { + State.PersistAsJson(Key, kept); + } + } + + return Task.CompletedTask; + } + + public void Dispose() => _persisting?.Dispose(); + + private sealed record KeptDiff(Guid ChangeSetId, KeptItem[] Items) + { + public static KeptDiff From(ChangeSetDiff diff) => new(diff.ChangeSetId, [.. diff.Items.Select(KeptItem.From)]); + + public ChangeSetDiff ToDiff() => new(ChangeSetId, [.. Items.Select(item => item.ToDiff())]); + } + + private sealed record KeptItem( + OperationKind Kind, + ItemKind ItemKind, + Guid ItemId, + int? BaseVersion, + int? ProposedVersion, + KeptValue[] Properties, + KeptValue[] Fields) + { + public static KeptItem From(ItemDiff item) => new( + item.Operation.Kind, + item.Operation.ItemKind, + item.Operation.ItemId, + item.Operation.BaseVersion, + item.Operation.ProposedVersion, + [.. item.Properties.Select(KeptValue.From)], + [.. item.Fields.Select(KeptValue.From)]); + + public ItemDiff ToDiff() => ItemDiff.Restore( + ChangeOperation.Restore(Kind, ItemKind, ItemId, BaseVersion, ProposedVersion), + [.. Properties.Select(value => value.ToDiff())], + [.. Fields.Select(value => value.ToDiff())]); + } + + private sealed record KeptValue(string Name, ValueChange Change, JsonElement? Before, JsonElement? After) + { + public static KeptValue From(ValueDiff value) => new(value.Name, value.Change, value.Before, value.After); + + public ValueDiff ToDiff() => ValueDiff.Restore(Name, Change, Before, After); + } } diff --git a/src/FluentCMS.Core/ChangeSets/ItemDiff.cs b/src/FluentCMS.Core/ChangeSets/ItemDiff.cs index 1671b30f2..28ee4b6fd 100644 --- a/src/FluentCMS.Core/ChangeSets/ItemDiff.cs +++ b/src/FluentCMS.Core/ChangeSets/ItemDiff.cs @@ -28,6 +28,15 @@ private ItemDiff(ChangeOperation operation, IReadOnlyList properties, public IReadOnlyList Fields { get; } + // A diff read back from where it was kept, such as the state the set view carries into its circuit. + public static ItemDiff Restore(ChangeOperation operation, IReadOnlyList properties, IReadOnlyList fields) + { + ArgumentNullException.ThrowIfNull(operation); + ArgumentNullException.ThrowIfNull(properties); + ArgumentNullException.ThrowIfNull(fields); + return new ItemDiff(operation, properties, fields); + } + // The base definition is absent for a create and the proposed one for a delete, as the operation's versions say. A field's // value is its definition without the name. public static ItemDiff ForSchema(ChangeOperation operation, ContentTypeDefinition? baseDefinition, ContentTypeDefinition? proposedDefinition) diff --git a/src/FluentCMS.Core/ChangeSets/ValueDiff.cs b/src/FluentCMS.Core/ChangeSets/ValueDiff.cs index a8fac5b90..bad9b02d5 100644 --- a/src/FluentCMS.Core/ChangeSets/ValueDiff.cs +++ b/src/FluentCMS.Core/ChangeSets/ValueDiff.cs @@ -23,6 +23,13 @@ private ValueDiff(string name, ValueChange change, JsonElement? before, JsonElem public JsonElement? After { get; } + // A diff read back from where it was kept, such as the state the set view carries into its circuit. + public static ValueDiff Restore(string name, ValueChange change, JsonElement? before, JsonElement? after) + { + ArgumentException.ThrowIfNullOrEmpty(name); + return new ValueDiff(name, change, before, after); + } + // Values are matched by exact name and compared as JSON, so property order and number formatting are not changes. The result // follows the order of the later version, then lists the removed values in the order of the earlier one. public static IReadOnlyList Compare( diff --git a/src/FluentCMS.Host/FluentCmsHost.cs b/src/FluentCMS.Host/FluentCmsHost.cs index a6cf30a8f..01975ba54 100644 --- a/src/FluentCMS.Host/FluentCmsHost.cs +++ b/src/FluentCMS.Host/FluentCmsHost.cs @@ -56,7 +56,10 @@ public static WebApplication BuildWebApplication(string[] args, Action hub.MaximumReceiveMessageSize = AdminLimits.CircuitStartBytes); RunJobsAsTheirInitiator(builder.Services); var app = builder.Build(); diff --git a/tests/FluentCMS.IntegrationTests/Browser/SetViewBrowserTests.cs b/tests/FluentCMS.IntegrationTests/Browser/SetViewBrowserTests.cs new file mode 100644 index 000000000..15c246a76 --- /dev/null +++ b/tests/FluentCMS.IntegrationTests/Browser/SetViewBrowserTests.cs @@ -0,0 +1,96 @@ +using FluentCMS.Core.ChangeSets; +using FluentCMS.Core.Schema; +using FluentCMS.Infrastructure.Persistence; +using Microsoft.Playwright; + +namespace FluentCMS.IntegrationTests.Browser; + +public sealed class SetViewBrowserTests +{ + // PERF-04: a 200-item set arrives in batches over the circuit. The circuit starts from the items the page arrived with, so the list + // never empties when it attaches; on PostgreSQL a second read used to show "Loading the items…" in their place. + [BrowserFact] + public async Task A_large_set_arrives_in_batches_and_the_list_never_empties_when_the_circuit_attaches() + { + var provider = string.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable(PostgresFactAttribute.VariableName)) + ? DatabaseProviders.Sqlite + : DatabaseProviders.PostgreSql; + await using var database = await TemporaryDatabase.CreateAsync(provider); + await using var session = await BrowserSession.StartAsync(database.Configuration); + var set = await AwaitingReviewAsync(session, 200); + var page = session.Page; + await session.SignInWithKeyboardAsync(); + + // The fewest items shown at any moment after the page arrived with its prerendered items. + await page.AddInitScriptAsync(""" + document.addEventListener("DOMContentLoaded", () => { + const shown = () => document.querySelectorAll('[data-slot="diff-view-item"]').length; + window.fewestItems = shown(); + new MutationObserver(() => { window.fewestItems = Math.min(window.fewestItems, shown()); }) + .observe(document.body, { childList: true, subtree: true }); + }); + """); + await page.GotoAsync($"{session.BaseUrl}/sets/{set.Id}"); + + await Assertions.Expect(page.Locator("[data-testid=change-set-items][data-interactive=true]")).ToBeVisibleAsync(); + await Assertions.Expect(page.Locator("[data-slot=diff-view-item]")).ToHaveCountAsync(200); + Assert.True(await page.EvaluateAsync("window.fewestItems") >= 25, "The list emptied when the circuit attached."); + } + + // A set whose diff is too large to carry into the circuit still gets its circuit: the circuit reads the diff again. + [BrowserFact] + public async Task A_set_whose_diff_is_too_large_to_keep_still_attaches_and_shows_every_item() + { + await using var session = await BrowserSession.StartAsync(); + await session.Host.AsUserAsync(session.OwnerId, async directory => (await directory.CreateAsync( + new NewContentType("Page", "page", new ContentTypeDefinition([new FieldDefinition("body", "richText")])), TestChangeSetTargets.ContentModel)).Value); + var body = new string('x', 6500); + var entries = new List(); + for (var index = 0; index < 100; index++) + { + entries.Add(await session.Host.AsUserAsync(session.OwnerId, async directory => (await directory.CreateAsync( + new FluentCMS.Core.Content.NewEntry("page", System.Text.Json.JsonSerializer.SerializeToElement(new { body = $"{index} {body}" })))).Value.Id)); + } + + var set = await session.Host.AsUserAsync(session.OwnerId, async directory => + { + var opened = (await directory.OpenAsync(new NewChangeSet("Add a hundred long pages.", []))).Value; + foreach (var entry in entries) + { + await directory.AddOperationAsync(opened.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry, null, 1).Value); + } + + await directory.TransitionAsync(opened.Id, ChangeSetState.ChecksRunning); + return (await directory.TransitionAsync(opened.Id, ChangeSetState.AwaitingReview)).Value; + }); + await session.SignInWithKeyboardAsync(); + + await session.Page.GotoAsync($"{session.BaseUrl}/sets/{set.Id}"); + + await Assertions.Expect(session.Page.Locator("[data-testid=change-set-items][data-interactive=true]")).ToBeVisibleAsync(); + await Assertions.Expect(session.Page.Locator("[data-slot=diff-view-item]")).ToHaveCountAsync(100); + } + + private static async Task AwaitingReviewAsync(BrowserSession session, int items) + { + var types = new List(); + for (var index = 0; index < items; index++) + { + var slug = $"type-{index}"; + types.Add(await session.Host.AsUserAsync(session.OwnerId, async directory => + (await directory.CreateAsync(new NewContentType(slug, slug, new ContentTypeDefinition([new FieldDefinition("title", "text")])), TestChangeSetTargets.ContentModel)).Value)); + } + + return await session.Host.AsUserAsync(session.OwnerId, async directory => + { + var set = (await directory.OpenAsync(new NewChangeSet("Add two hundred types.", []))).Value; + foreach (var type in types) + { + Assert.True((await directory.AddOperationAsync(set.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Schema, type.Id, null, 1).Value)).IsSuccess); + } + + await directory.TransitionAsync(set.Id, ChangeSetState.ChecksRunning); + return (await directory.TransitionAsync(set.Id, ChangeSetState.AwaitingReview)).Value; + }); + } +} diff --git a/tests/FluentCMS.IntegrationTests/Desk/ChangeSetScreenTests.cs b/tests/FluentCMS.IntegrationTests/Desk/ChangeSetScreenTests.cs index 5c6395bec..c63c7a89c 100644 --- a/tests/FluentCMS.IntegrationTests/Desk/ChangeSetScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Desk/ChangeSetScreenTests.cs @@ -2,6 +2,7 @@ using System.Text.RegularExpressions; using FluentCMS.Core.ChangeSets; using FluentCMS.Core.Schema; +using FluentCMS.Infrastructure.Persistence; namespace FluentCMS.IntegrationTests.Desk; @@ -35,11 +36,14 @@ public async Task An_unknown_set_is_not_found() Assert.Contains("There is nothing at this address.", await response.Content.ReadAsStringAsync()); } - [Fact] - public async Task A_set_shows_its_rationale_sources_history_and_first_items_and_the_desk_links_to_it() + [Theory] + [InlineData(DatabaseProviders.Sqlite)] + [PostgresData] + public async Task A_set_shows_its_rationale_sources_history_and_first_items_and_the_desk_links_to_it(string provider) { using var install = new TemporaryInstall(); - await using var host = await AdminHost.StartAsync(install); + await using var database = await TemporaryDatabase.CreateAsync(provider); + await using var host = await AdminHost.StartAsync(install, settings: database.Configuration); var ownerId = await host.CreateFirstAdministratorAsync(); var types = new List(); for (var index = 0; index < 30; index++) @@ -51,7 +55,7 @@ public async Task A_set_shows_its_rationale_sources_history_and_first_items_and_ { var opened = (await directory.OpenAsync(new NewChangeSet( "Add thirty types.", - ["https://example.com/spec", "javascript:alert(1)", "specs/types.md"]))).Value; + ["https://example.com/spec", "javascript:alert(1)", "specs/types.md", "https://\u0430pple.com/prices"]))).Value; foreach (var type in types) { Assert.True((await directory.AddOperationAsync(opened.Id, Create(type))).IsSuccess); @@ -70,6 +74,8 @@ public async Task A_set_shows_its_rationale_sources_history_and_first_items_and_ Assert.Contains("href=\"https://example.com/spec\" target=\"_blank\" rel=\"noopener noreferrer\"", page); Assert.DoesNotContain("href=\"javascript:", page); Assert.Contains("specs/types.md", page); + Assert.Contains("xn--pple-43d.com", page[page.IndexOf("data-testid=\"source-host\"", StringComparison.Ordinal)..]); + Assert.Single(Regex.Matches(page, "data-testid=\"source-host\"")); Assert.Equal( ["change_set.opened", "change_set.submitted", "change_set.awaiting_review"], Regex.Matches(page[page.IndexOf("data-testid=\"change-set-history\"", StringComparison.Ordinal)..], "data-operation=\"([a-z_.]+)\"").Select(match => match.Groups[1].Value)); @@ -80,11 +86,14 @@ public async Task A_set_shows_its_rationale_sources_history_and_first_items_and_ Assert.Contains("