diff --git a/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs b/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs index d83bcad..a93d1e3 100644 --- a/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs +++ b/src/SharpMUTerm.Core/Configuration/AppConfiguration.cs @@ -8,7 +8,7 @@ namespace SharpMUTerm.Core.Configuration; public sealed class AppConfiguration { /// The current on-disk schema version. Older configs are upgraded by . - public const int CurrentVersion = 4; + public const int CurrentVersion = 5; /// Schema version, for future migrations. public int Version { get; set; } = CurrentVersion; diff --git a/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs b/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs index a166785..10da3ec 100644 --- a/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs +++ b/src/SharpMUTerm.Core/Configuration/ConfigurationMigrator.cs @@ -33,9 +33,114 @@ public static void Migrate(JsonObject root) MigrateV3ToV4(root); } + if (version < 5) + { + MigrateV4ToV5(root); + } + root["version"] = AppConfiguration.CurrentVersion; } + /// + /// v4's spawn window ids named only their target (spawn:Public), so a workspace held one + /// capture pane per target however many characters were capturing into it. v5 puts the owning + /// session in the id (), so two + /// connected characters running the same rule get a pane each. Every saved id is rewritten here — + /// in lastSession.windows and in every pane's tabs array that referenced it. + /// + /// It is a migration and not an adoption, and the reason is that a resumed workspace must not + /// come back holding a pane nothing writes to. Left alone, a saved spawn:Public would + /// match no id the running client can now produce: the pane would sit there for ever, empty, while + /// its channel filled a second pane beside it. Dropping it would have been the other way to avoid + /// that and it throws away a pane the user had, plus — through the window id the restore log is + /// keyed by — the scrollback in it. Rewriting keeps both: the pane stays where it was, and its log + /// file is carried across at startup by SharpMUTermApp.RestorePreviousSession. + /// + /// + /// What supplies the owner is the state itself. WorkspaceWindowState.SessionKey has + /// been persisted all along, so an old file already records which character each spawn window + /// belonged to and the rewrite is lossless rather than a guess. A window that recorded no owner + /// keeps that: it becomes the unowned form, which is exactly what the client produces for a spawn + /// window nobody owns, so it too stays reachable. + /// + /// + /// The decision is made by version, never by looking at the id. A target is free text and may + /// hold anything — including something that reads like a v5 id — so classifying ids by shape would + /// eventually mis-file one and produce the orphan pane this step exists to prevent. Everything under + /// a document that says version 4 is v4, by construction. + /// + /// + private static void MigrateV4ToV5(JsonObject root) + { + if (root["lastSession"] is not JsonObject session || session["windows"] is not JsonArray windows) + { + return; + } + + var rewritten = new Dictionary(StringComparer.Ordinal); + foreach (var window in windows.OfType()) + { + // Kind is written by the string enum converter, so it is the member name and not a number. + if (!string.Equals(Text(window["kind"]), nameof(Workspaces.WindowKind.Spawn), + StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (Text(window["id"]) is not { } id + || !id.StartsWith(Workspaces.Workspace.SpawnPrefix, StringComparison.Ordinal)) + { + continue; + } + + var target = id[Workspaces.Workspace.SpawnPrefix.Length..]; + if (target.Length == 0) + { + continue; + } + + var upgraded = Workspaces.Workspace.SpawnWindowId(Text(window["sessionKey"]), target); + window["id"] = upgraded; + rewritten[id] = upgraded; + } + + if (rewritten.Count > 0 && session["root"] is JsonObject layout) + { + RewriteTabs(layout, rewritten); + } + } + + /// Re-points every pane's tab list at the ids rewrote. + private static void RewriteTabs(JsonObject node, Dictionary rewritten) + { + if (node["tabs"] is JsonArray tabs) + { + for (var i = 0; i < tabs.Count; i++) + { + if (Text(tabs[i]) is { } tab && rewritten.TryGetValue(tab, out var upgraded)) + { + tabs[i] = upgraded; + } + } + } + + if (node["children"] is JsonArray children) + { + foreach (var child in children.OfType()) + { + RewriteTabs(child, rewritten); + } + } + } + + /// + /// A node's string value, or null when it is absent or is not a string. A hand-edited or + /// hand-truncated document must not stop the client starting, and GetValue<string> + /// throws on a number where this returns null. + /// + private static string? Text(JsonNode? node) => + node is JsonValue value && value.TryGetValue(out var text) ? text : null; + /// /// v3's autoLogin is gone: whether a character logs itself in is now derived from whether it /// has anything to send (, ). The diff --git a/src/SharpMUTerm.Core/Workspace/Workspace.cs b/src/SharpMUTerm.Core/Workspace/Workspace.cs index 6a1b242..fea867b 100644 --- a/src/SharpMUTerm.Core/Workspace/Workspace.cs +++ b/src/SharpMUTerm.Core/Workspace/Workspace.cs @@ -1,3 +1,5 @@ +using System.Globalization; + namespace SharpMUTerm.Core.Workspaces; /// @@ -154,14 +156,23 @@ public WorkspaceWindow OpenWindow( } /// - /// Routes trigger-spawned output to a spawn window named , creating and - /// placing the window on first use, and counts the line as unread unless the window is currently - /// visible. Returns the destination window. + /// Routes trigger-spawned output to 's spawn window named + /// , creating and placing the window on first use, and counts the line as + /// unread unless the window is currently visible. Returns the destination window. + /// + /// The destination is per session, not per workspace. Two connected characters running the + /// same capture rule each get a window of their own; the id carries the owner, so the second + /// session to match cannot land in the first's window. It used to: the id was the target alone, so + /// whoever matched first created the window with their own session key on it and everybody + /// else's lines were appended to somebody else's pane. That was not merely a mixed-up channel — the + /// rail draws window rows for the active character only, so the second character's own channel was + /// filed under the first and was invisible from the character it belonged to. + /// /// public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) { ArgumentException.ThrowIfNullOrEmpty(target); - var id = SpawnWindowId(target); + var id = SpawnWindowId(sessionKey, target); if (!_windows.TryGetValue(id, out var window)) { window = Register(new WorkspaceWindow(id, target, WindowKind.Spawn, sessionKey)); @@ -172,8 +183,93 @@ public WorkspaceWindow RouteSpawn(string target, string? sessionKey = null) return window; } - /// The window id a spawn routes to. - public static string SpawnWindowId(string target) => $"spawn:{target}"; + /// Every spawn window id starts with this. + public const string SpawnPrefix = "spawn:"; + + /// + /// The owner field of a spawn window that belongs to nobody. A single -, which is not a + /// decimal length and so can never be mistaken for one — that is the whole reason it is not the + /// empty string. + /// + private const string Unowned = "-"; + + /// + /// The window id the spawn of the session + /// routes to. Unique per (owner, target) and stable for ever, so a reconnect or a restart + /// comes back to the pane it left. + /// + /// Why the length prefix. A session key and a target are both user-controlled strings that may + /// hold any character, colons included — a world or character can be called a:b and a + /// trigger's SpawnTarget is free text. Joining them with a separator is therefore not + /// injective: (a, b:c) and (a:b, c) would produce one id and collapse two characters' + /// panes into one, which is this defect again in a rarer shape. Writing the owner's length in front + /// of it makes the encoding total and reversible: the digits up to the first colon give the length, + /// exactly that many characters are the owner, one more colon is consumed, and everything left is + /// the target — so distinct pairs cannot produce equal ids, whatever is in them. + /// + /// + /// It is legible on purpose rather than hashed. This id is a dictionary key, a value in + /// config.json, and the stem of a RestoreLog file name; a digest would be unambiguous + /// too and would make every one of those unreadable to whoever has to look at them, for no property + /// a reversible encoding does not already have. (The file name's own collision handling is + /// unchanged and unaffected: RestoreLog stores the full id in each file's header and refuses + /// a file whose header names a different window, so a CRC-32 clash on the stem costs one window's + /// log rather than mixing two.) + /// + /// + /// The owning world.character session, or null for a window nobody owns. + /// The capture target, which is also the window's title. + public static string SpawnWindowId(string? sessionKey, string target) + { + ArgumentException.ThrowIfNullOrEmpty(target); + return sessionKey is null + ? $"{SpawnPrefix}{Unowned}:{target}" + : $"{SpawnPrefix}{sessionKey.Length}:{sessionKey}:{target}"; + } + + /// + /// Reads a spawn window id back into the pair that made it. False when is not + /// one this build writes — including a spawn id from before the owner was in it, which is what + /// ConfigurationMigrator's v4→v5 step upgrades. + /// + public static bool TryReadSpawnWindowId(string id, out string? sessionKey, out string target) + { + sessionKey = null; + target = string.Empty; + if (id is null || !id.StartsWith(SpawnPrefix, StringComparison.Ordinal)) + { + return false; + } + + var rest = id.AsSpan(SpawnPrefix.Length); + if (rest.StartsWith(Unowned + ":", StringComparison.Ordinal)) + { + target = rest[(Unowned.Length + 1)..].ToString(); + return target.Length > 0; + } + + // NumberStyles.None: bare digits only, so a sign or surrounding space is not silently accepted + // into a field whose whole job is to say how many characters to take. + var separator = rest.IndexOf(':'); + if (separator <= 0 + || !int.TryParse(rest[..separator], NumberStyles.None, CultureInfo.InvariantCulture, out var length)) + { + return false; + } + + // The owner's exact length, then the colon that closes it: anything shorter is not this + // encoding. Written as `<=` rather than `< length + 1` so a declared length of int.MaxValue + // cannot overflow the comparison into passing and then index off the end. + var owner = rest[(separator + 1)..]; + if (owner.Length <= length || owner[length] != ':') + { + return false; + } + + sessionKey = owner[..length].ToString(); + target = owner[(length + 1)..].ToString(); + return target.Length > 0; + } /// /// Records a line arriving in a window: increments its unread badge unless the window is diff --git a/src/SharpMUTerm.Tui/DemoScene.cs b/src/SharpMUTerm.Tui/DemoScene.cs index d72b87f..9ed9fa9 100644 --- a/src/SharpMUTerm.Tui/DemoScene.cs +++ b/src/SharpMUTerm.Tui/DemoScene.cs @@ -19,6 +19,13 @@ internal static class DemoScene /// The world.character the demo resumes as focused/connected. public const string ActiveSessionKey = "Aetherfall.Corvid"; + /// + /// The demo's Chat spawn window. Spelt once, here, because a spawn window's id names its + /// owner as well as its target — the saved workspace, the snapshot's spawn view and + /// the scene's own backlog all have to mean the same window, and three hand-built ids would not. + /// + public static string ChatWindowId => Workspace.SpawnWindowId(ActiveSessionKey, "Chat"); + public static AppConfiguration Build() { var config = new AppConfiguration(); @@ -243,7 +250,7 @@ private static void AddTriggerSets(AppConfiguration config) /// private static WorkspaceState BuildLastSession() { - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = ChatWindowId; return new WorkspaceState { Windows = diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index a40addf..765c9b0 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -711,7 +711,7 @@ public string RenderSnapshot(string? view = null) // nothing else (TimestampGutterTests) are both claims about a spawn window in front. if (string.Equals(view, "spawn", StringComparison.OrdinalIgnoreCase)) { - _workspace.ActivateWindow(Workspace.SpawnWindowId("Chat")); + _workspace.ActivateWindow(DemoScene.ChatWindowId); RebuildPaneArea(); } @@ -1244,7 +1244,7 @@ void Feed(string windowId, string ansiLine) // The Chat spawn window already exists (opened by the resumed session); feed its backlog and // leave it in the background with unread, as if lines arrived while another tab was focused. - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = DemoScene.ChatWindowId; PaneContentFor(chatId, "Chat"); var chatParser = new AnsiParser(); foreach (var text in new[] @@ -1355,14 +1355,31 @@ private void RestorePreviousSession() var restoredWindows = 0; var restoredLines = 0; - foreach (var window in _restore.Read()) - { + var logged = _restore.Read(); + var carriedOver = CarryLegacySpawnLogsOver(logged); + foreach (var window in logged) + { + // A log written before spawn window ids carried their owner has been re-filed under the id + // its pane now has; its own file is already gone, and replaying it under the old id would + // buffer the previous session's channel where nothing can ever see it. A null replacement + // is one whose pane already had a log of its own, so there is nothing left to replay. + var windowId = window.WindowId; + if (carriedOver.TryGetValue(window.WindowId, out var carriedTo)) + { + if (carriedTo is null) + { + continue; + } + + windowId = carriedTo; + } + // A character who opted out gets their content dropped rather than merely un-drawn: an // opt-out that left the last session's text lying in the config directory would be // answering a different question from the one it was asked. - if (!RestoreLogWanted(_workspace.FindWindow(window.WindowId)?.SessionKey)) + if (!RestoreLogWanted(_workspace.FindWindow(windowId)?.SessionKey)) { - _restore.Forget(window.WindowId); + _restore.Forget(windowId); continue; } @@ -1373,13 +1390,13 @@ private void RestorePreviousSession() foreach (var line in window.Lines) { - AppendWindowLine(window.WindowId, _formatter.ToMarkup(line.Line), line.Stamp); + AppendWindowLine(windowId, _formatter.ToMarkup(line.Line), line.Stamp); } // The boundary marker carries no stamp: it did not arrive, it was drawn, and a timestamp // gutter beside it would be claiming a time for a row the game never sent. AppendWindowLine( - window.WindowId, + windowId, RestoreBarRenderer.Bar(window.Lines.Count, window.LastWritten, FrozenAccentHex())); restoredWindows++; @@ -1397,6 +1414,93 @@ private void RestorePreviousSession() restoredWindows); } + /// + /// Moves any restore log written before a spawn window id carried its owner onto the id that pane + /// has now, and hands back the old-id → new-id map the replay reads. + /// + /// This is the content half of ConfigurationMigrator's v4→v5 step, and without it that + /// step would lose the user's scrollback. The log is keyed by window id and the saved workspace + /// has just had its spawn ids rewritten, so the file holding the previous session's Public + /// pane is now filed under an id nothing refers to: the pane would come back in the right place and + /// empty. It is done by copying the lines onto the new id and dropping the old file rather than by + /// remembering a mapping for ever — after this launch there is nothing left to map, so the next + /// launch does no work and cannot replay the same content twice. + /// + /// + /// What counts as an old id is decided by shape here, and that is safe in a way it would not be + /// in the configuration: an id is old only if it does not parse as a current one + /// () and names no window this workspace holds + /// and exactly one live spawn window claims its target. Anything short of all three is left + /// alone, which costs nothing — a log the workspace cannot place is buffered under its own id + /// exactly as it has always been, so its pane refills if that channel ever speaks again. + /// + /// + /// + /// Old id → the id to replay it under, or where the old file was dropped and + /// there is nothing left to replay. Ids absent from the map are not old and are replayed as they are. + /// + private Dictionary CarryLegacySpawnLogsOver(IReadOnlyList logged) + { + var carried = new Dictionary(StringComparer.Ordinal); + if (_restore is null) + { + return carried; + } + + // Which live spawn window would have been written under which pre-v5 id. A target claimed by + // more than one window is ambiguous and is left alone rather than guessed at. + var claims = new Dictionary(StringComparer.Ordinal); + foreach (var window in _workspace.Windows.Where(w => w.Kind == WindowKind.Spawn)) + { + if (!Workspace.TryReadSpawnWindowId(window.Id, out _, out var target)) + { + continue; + } + + var legacy = Workspace.SpawnPrefix + target; + claims[legacy] = claims.ContainsKey(legacy) ? null : window.Id; + } + + var known = logged.Select(w => w.WindowId).ToHashSet(StringComparer.Ordinal); + foreach (var window in logged) + { + if (!window.WindowId.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal) + || Workspace.TryReadSpawnWindowId(window.WindowId, out _, out _) + || _workspace.FindWindow(window.WindowId) is not null + || !claims.TryGetValue(window.WindowId, out var replacement) + || replacement is null) + { + continue; + } + + // A log already standing under the new id means an earlier launch did this and was + // interrupted before it could drop the old file. Take the new one and drop the old, which + // loses nothing that is not already there and cannot double a pane's history. + carried[window.WindowId] = null; + if (!known.Contains(replacement) && RestoreLogWanted(_workspace.FindWindow(replacement)?.SessionKey)) + { + var title = _workspace.FindWindow(replacement)?.Title ?? string.Empty; + foreach (var line in window.Lines) + { + _restore.Append(replacement, title, line.Line, line.Stamp); + } + + carried[window.WindowId] = replacement; + } + + _restore.Forget(window.WindowId); + } + + var moved = carried.Count(entry => entry.Value is not null); + if (moved > 0) + { + _diagnostics.Logger.LogInformation( + "Carried {Count} pane(s) of restored content onto the per-session spawn window ids", moved); + } + + return carried; + } + /// /// Whether a window owned by takes part in the restore log. An /// unowned window — the main window before any session adopts it, the web view — is allowed: no @@ -2182,10 +2286,18 @@ private void OnLine(WorldSession session, string windowId, StyledLine line) /// cosmetic: the rail lists a character's windows by it, and resolves /// which world a link clicked in a spawn window sends to by it. /// + /// + /// The same session key also picks the window (), + /// which is what gives two characters running one capture rule a pane each. While the id was the + /// target alone there was one window per workspace: the first session to match created it with its + /// own key on it and every other session's lines were appended to a pane somebody else owned — + /// invisible from the character whose channel it was, because the rail lists window rows for the + /// active character only. + /// /// private void OnSpawnLine(WorldSession session, string target, StyledLine line) { - var existed = _workspace.FindWindow(Workspace.SpawnWindowId(target)) is not null; + var existed = _workspace.FindWindow(Workspace.SpawnWindowId(session.SessionKey, target)) is not null; var window = _workspace.RouteSpawn(target, session.SessionKey); // Its owner's own name, which for a session with no character is its world's. It used to fall back on @@ -4486,7 +4598,7 @@ private IReadOnlyList BuildRail() /// windows, plus the ones that belong to nobody. /// /// The owner test is the point. Every window a session or a trigger opens carries its owner - /// (, and RouteSpawn(target, _active?.SessionKey)), so a + /// (, and RouteSpawn(target, session.SessionKey)), so a /// window with no owner is an auxiliary like the web view — global, reachable from wherever you /// are, and listed here because the rail is the only place you can click back to it. Without the /// test this returned every window in the workspace, so the rail drew one character's tabs diff --git a/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs b/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs index 7091eef..31a6cf1 100644 --- a/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Configuration/ConfigurationTests.cs @@ -278,7 +278,8 @@ public async Task LastSession_RoundTripsThroughTheStore() await Assert.That(restored.LastSession).IsNotNull(); var workspace = restored.LastSession!.Restore(); - await Assert.That(workspace.FindWindow(Workspace.SpawnWindowId("Chat"))!.OwnerLabel).IsEqualTo("Corvid"); + await Assert.That(workspace.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.OwnerLabel) + .IsEqualTo("Corvid"); } [Test] diff --git a/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs new file mode 100644 index 0000000..a49d181 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Configuration/SpawnWindowIdMigrationTests.cs @@ -0,0 +1,233 @@ +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Workspaces; + +namespace SharpMUTerm.Core.Tests.Configuration; + +/// +/// The v4 → v5 upgrade: spawn window ids gain their owner, so two characters capturing one target stop +/// sharing a pane. The whole risk of the change is on this seam — every existing installation's +/// config.json holds spawn:Public rows, and both the pane placement in lastSession +/// and the scrollback in the restore log are keyed by those exact strings. +/// +/// The decision, and why it is the one to make. An old id is migrated, not adopted and +/// not dropped. Adoption — leaving the id alone and letting whichever session claims the target keep it +/// — cannot work, because nothing the running client now produces equals spawn:Public: the pane +/// would come back and then sit there for ever with nothing writing to it while its channel filled a +/// second pane beside it. Dropping is honest and throws away a pane the user had and, through the id +/// the restore log is keyed by, the text in it. Migrating keeps the pane, its place in the split tree +/// and its content, and produces exactly the id the session that owned it will route to. +/// +/// +/// What supplies the owner is the saved state itself. WorkspaceWindowState.SessionKey has +/// been persisted since spawn windows existed, so an old document already records which character each +/// pane belonged to; the rewrite is a lookup and not a guess. +/// +/// +public class SpawnWindowIdMigrationTests +{ + /// A v4 document: one character, a main window and a Chat spawn beside it in one pane. + private const string V4 = """ + { + "version": 4, + "worlds": [ { "name": "Aetherfall", "host": "aetherfall.mux", "port": 4201, + "characters": [ { "name": "Corvid" } ] } ], + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid", + "ownerLabel": "Corvid", "capturePattern": "^\\[Chat\\]" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """; + + /// The id the running client will route Corvid's Chat capture to. + private static string CorvidsChat => Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"); + + /// + /// The headline: the saved pane keeps everything it had, under the id the session that owns it now + /// produces. Placement included — the rewrite has to reach the pane's tabs array as well as + /// the window row, or the workspace comes back with a window in no pane and a pane naming no window. + /// + [Test] + public async Task AV4SpawnWindowKeepsItsPaneItsPlaceAndItsMetadata() + { + var config = ConfigurationStore.Deserialize(V4); + + await Assert.That(config.Version).IsEqualTo(AppConfiguration.CurrentVersion); + + var window = config.LastSession!.Windows.Single(w => w.Kind == WindowKind.Spawn); + await Assert.That(window.Id).IsEqualTo(CorvidsChat); + await Assert.That(window.Title).IsEqualTo("Chat"); + await Assert.That(window.SessionKey).IsEqualTo("Aetherfall.Corvid"); + await Assert.That(window.OwnerLabel).IsEqualTo("Corvid"); + + // There is no CapturePattern to assert any more: the capture header it fed was removed with the + // spawn pane's dim "⇱ capture …" row, and the field went with it. A v4 document may still carry + // the property — the fixture below does — and System.Text.Json drops what it cannot map, which is + // the behaviour that lets an old config load at all. + + await Assert.That(config.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + } + + /// + /// The property the migration exists for. Every window the resumed workspace holds is one the + /// running client can route to: the id of each spawn pane is exactly what + /// produces for the owner and target that pane records. A pane + /// left under a v4 id would fail this — and that failure, on a live client, is a pane nobody ever + /// writes to again. + /// + [Test] + public async Task NoResumedPaneIsLeftWithAnIdNothingRoutesTo() + { + var workspace = ConfigurationStore.Deserialize(V4).LastSession!.Restore(); + + foreach (var window in workspace.Windows.Where(w => w.Kind == WindowKind.Spawn).ToList()) + { + var routed = workspace.RouteSpawn(window.Title, window.SessionKey); + await Assert.That(routed.Id).IsEqualTo(window.Id); + } + + // …and routing them did not open anything new beside them. + await Assert.That(workspace.Windows.Count).IsEqualTo(2); + await Assert.That(workspace.Layout.FindWindow(CorvidsChat)).IsNotNull(); + } + + /// + /// A v4 spawn window that recorded no owner becomes the unowned form, which is the id the client + /// produces for a spawn window nobody owns — so it stays reachable rather than becoming an orphan by + /// a different route. + /// + [Test] + public async Task AV4SpawnWindowWithNoOwnerBecomesTheUnownedId() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ { "id": "spawn:Notes", "title": "Notes", "kind": "Spawn" } ], + "root": { "type": "pane", "id": "p1", "tabs": [ "spawn:Notes" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); + + await Assert.That(config.LastSession!.Windows.Single().Id) + .IsEqualTo(Workspace.SpawnWindowId(null, "Notes")); + } + + /// The rewrite reaches panes nested in a split, not only the root one. + [Test] + public async Task TabsAreRewrittenInsideASplitToo() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid" } + ], + "root": { "type": "split", "direction": "Row", "sizes": [ 0.5, 0.5 ], "children": [ + { "type": "pane", "id": "p1", "tabs": [ "main" ], "activeIndex": 0 }, + { "type": "pane", "id": "p2", "tabs": [ "spawn:Chat" ], "activeIndex": 0 } + ] }, + "focusedPaneId": "p1" + } + } + """); + + var workspace = config.LastSession!.Restore(); + await Assert.That(workspace.Layout.FindWindow(CorvidsChat)!.Id).IsEqualTo("p2"); + } + + /// + /// Only spawn windows move. The main window and any auxiliary keep the ids they had, because those + /// were never derived from a target and nothing about them was ambiguous. + /// + [Test] + public async Task NonSpawnWindowsAreLeftAlone() + { + var config = ConfigurationStore.Deserialize(""" + { + "version": 4, + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "web", "title": "Page", "kind": "Auxiliary" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "web" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); + + await Assert.That(config.LastSession!.Windows.Select(w => w.Id)).IsEquivalentTo(new[] { "main", "web" }); + await Assert.That(config.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", "web" }); + } + + /// + /// The step is idempotent in the way that matters: a document already at the current version is not + /// re-encoded. Running the rewrite twice would wrap one id inside another + /// (spawn:22:…:spawn:17:…:Chat) and lose the pane a second way, so this is the assertion that + /// keeps the version gate honest. + /// + [Test] + public async Task ADocumentAlreadyAtTheCurrentVersionIsNotRewrittenAgain() + { + var once = ConfigurationStore.Deserialize(V4); + var twice = ConfigurationStore.Deserialize(ConfigurationStore.Serialize(once)); + + await Assert.That(twice.LastSession!.Windows.Single(w => w.Kind == WindowKind.Spawn).Id) + .IsEqualTo(CorvidsChat); + await Assert.That(twice.LastSession.Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + } + + /// + /// A v4 document with no saved session at all — the overwhelmingly common shape of an old file — is + /// untouched apart from its version, and starting from it does not throw. + /// + [Test] + public async Task AV4DocumentWithNoSavedSessionIsFine() + { + var config = ConfigurationStore.Deserialize(""" + { "version": 4, "worlds": [ { "name": "Aetherfall", "host": "aetherfall.mux" } ] } + """); + + await Assert.That(config.LastSession).IsNull(); + await Assert.That(config.Version).IsEqualTo(AppConfiguration.CurrentVersion); + } + + /// + /// And a hand-edited document does not make the migrator throw. It is asserted at that + /// level rather than through because this step runs + /// before deserialization: a number where a window id belongs, a missing kind, a tabs array + /// with a number in it — all things GetValue<string> throws on and this reads as + /// "not a string, leave it alone". + /// + [Test] + public async Task AMangledSavedSessionDoesNotStopTheMigrator() + { + var root = (System.Text.Json.Nodes.JsonObject)System.Text.Json.Nodes.JsonNode.Parse(""" + { + "version": 4, + "lastSession": { + "windows": [ { "id": 7, "kind": 1 }, { "id": "spawn:Chat" }, { "kind": "Spawn" }, + { "id": "spawn:Chat", "kind": "Spawn" } ], + "root": { "type": "pane", "id": "p1", "tabs": [ 3, "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """)!; + + ConfigurationMigrator.Migrate(root); + + await Assert.That(root["version"]!.GetValue()).IsEqualTo(AppConfiguration.CurrentVersion); + + // The one row that was a spawn window with a readable id did move, and the tab moved with it. + var tabs = (System.Text.Json.Nodes.JsonArray)root["lastSession"]!["root"]!["tabs"]!; + await Assert.That(tabs[1]!.GetValue()).IsEqualTo(Workspace.SpawnWindowId(null, "Chat")); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs new file mode 100644 index 0000000..374d726 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/Workspace/SpawnWindowIdTests.cs @@ -0,0 +1,187 @@ +using SharpMUTerm.Core.Workspaces; + +namespace SharpMUTerm.Core.Tests.Workspaces; + +/// +/// The identity a capture pane is filed under. It has to be unique per (owner, target) — that is +/// the whole of the fix for two characters sharing one Public window — and stable for ever, since +/// it keys the workspace registry, a value in config.json, and a RestoreLog file name. +/// +/// The pressure this suite is really under is that both halves are user-controlled free text: a +/// world or a character may be called anything, and a trigger's SpawnTarget is whatever was typed +/// on F2. So "unique" cannot mean "unique for the names people usually pick" — the encoding has to be +/// injective over every pair, which is what the length prefix buys and what a plain owner:target +/// join does not. +/// +/// +public class SpawnWindowIdTests +{ + /// The ordinary case reads as what it is, which is why this is an encoding and not a digest. + [Test] + public async Task AnIdNamesItsOwnerAndItsTarget() + { + await Assert.That(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat")) + .IsEqualTo("spawn:17:Aetherfall.Corvid:Chat"); + await Assert.That(Workspace.SpawnWindowId(null, "Chat")).IsEqualTo("spawn:-:Chat"); + } + + /// Two characters, one capture rule, two windows. The report, at the level it is caused. + [Test] + public async Task TwoOwnersOfOneTargetGetTwoIds() + { + await Assert.That(Workspace.SpawnWindowId("Convergence.Ann", "Public")) + .IsNotEqualTo(Workspace.SpawnWindowId("Convergence.Bob", "Public")); + } + + /// + /// The pair that a separator-joined id would collapse. ("a", "b:c") and + /// ("a:b", "c") are two different windows — a character called a capturing a channel + /// called b:c, and a character called a:b capturing one called c — and + /// $"spawn:{owner}:{target}" spells both spawn:a:b:c. That is this very defect in a + /// rarer shape, and it is why the owner's length is written in front of it. + /// + [Test] + public async Task ColonsInEitherHalfDoNotCollapseTwoWindowsIntoOne() + { + await Assert.That(Workspace.SpawnWindowId("a", "b:c")).IsNotEqualTo(Workspace.SpawnWindowId("a:b", "c")); + } + + /// + /// And the property behind those examples, over a spread of names chosen to be awkward: distinct + /// pairs give distinct ids, every time. A table of cases can only ever say "not these"; this says + /// "none of them", which is the claim the design actually rests on. + /// + [Test] + public async Task DistinctPairsAlwaysGiveDistinctIds() + { + string?[] owners = [null, "", "a", "a:b", "1", "12:x", "-", "-:x", "World.Char", "spawn:", ":", "::"]; + string[] targets = ["Chat", "a", "b:c", "c", ":", "spawn:Chat", "-:Chat", "17:Aetherfall.Corvid:Chat", "1"]; + + var seen = new Dictionary(StringComparer.Ordinal); + foreach (var owner in owners) + { + foreach (var target in targets) + { + var id = Workspace.SpawnWindowId(owner, target); + await Assert.That(seen.TryAdd(id, (owner, target))) + .IsTrue() + .Because($"({owner ?? "null"}, {target}) collided with {seen.GetValueOrDefault(id)} on {id}"); + } + } + } + + /// + /// The id is readable back, which is what lets the restore log carry a pre-owner file onto the pane + /// that now holds its channel. Over the same awkward spread, so the reader is exercised on the + /// inputs that would break a naive split. + /// + [Test] + public async Task AnIdReadsBackAsThePairThatMadeIt() + { + string?[] owners = [null, "", "a:b", "-", "17:Aetherfall.Corvid", "World.Char"]; + string[] targets = ["Chat", "b:c", ":", "-:Chat", "spawn:Chat"]; + + foreach (var owner in owners) + { + foreach (var target in targets) + { + var id = Workspace.SpawnWindowId(owner, target); + await Assert.That(Workspace.TryReadSpawnWindowId(id, out var readOwner, out var readTarget)).IsTrue(); + await Assert.That(readOwner).IsEqualTo(owner); + await Assert.That(readTarget).IsEqualTo(target); + } + } + } + + /// + /// A window id from before the owner was in it does not read as one of these, which is what + /// lets the restore log tell an old file apart from a current one without being told. (The + /// configuration is not left to work it out by shape — it goes by the document's schema version — + /// because a target is free text and may be made to look like anything, including this.) + /// + [Test] + [Arguments("spawn:Chat")] + [Arguments("spawn:Public")] + [Arguments("spawn:")] + [Arguments("main")] + [Arguments("spawn:99:short:x")] + [Arguments("spawn:+3:abc:x")] + [Arguments("spawn: 3:abc:x")] + // A declared length at the top of the range: the bounds check has to be written so this cannot + // overflow into passing and then index off the end of the string. + [Arguments("spawn:2147483647:abc:x")] + [Arguments("spawn:99999999999999999999:abc:x")] + public async Task WhatIsNotOneOfTheseIsRefused(string id) + { + await Assert.That(Workspace.TryReadSpawnWindowId(id, out _, out _)).IsFalse(); + } + + /// A spawn window has to be called something; an empty target is not an id. + [Test] + public async Task AnEmptyTargetIsRefused() + { + await Assert.That(() => Workspace.SpawnWindowId("Aetherfall.Corvid", string.Empty)) + .Throws(); + } + + /// + /// The workspace routes by that identity: one target, two sessions, two windows, each owned by the + /// session whose rule fired, and neither holding the other's activity. + /// + [Test] + public async Task RouteSpawnGivesEachSessionItsOwnWindow() + { + var workspace = new Workspace(sessionKey: "Convergence.Ann"); + + var ann = workspace.RouteSpawn("Public", "Convergence.Ann"); + var bob = workspace.RouteSpawn("Public", "Convergence.Bob"); + + await Assert.That(ann).IsNotEqualTo(bob); + await Assert.That(ann.SessionKey).IsEqualTo("Convergence.Ann"); + await Assert.That(bob.SessionKey).IsEqualTo("Convergence.Bob"); + await Assert.That(ann.Title).IsEqualTo("Public"); + await Assert.That(bob.Title).IsEqualTo("Public"); + await Assert.That(ann.Unread).IsEqualTo(1); + await Assert.That(bob.Unread).IsEqualTo(1); + await Assert.That(workspace.Windows.Count).IsEqualTo(3); // main + one each + } + + /// + /// And the same session routing twice still lands in one window — the property that makes the id a + /// name rather than a fresh identity per line. + /// + [Test] + public async Task OneSessionRoutingTwiceKeepsOneWindow() + { + var workspace = new Workspace(sessionKey: "Convergence.Ann"); + + var first = workspace.RouteSpawn("Public", "Convergence.Ann"); + var second = workspace.RouteSpawn("Public", "Convergence.Ann"); + + await Assert.That(second).IsEqualTo(first); + await Assert.That(workspace.Windows.Count).IsEqualTo(2); + } + + /// + /// Windows nobody owns share one bucket, and that is right rather than an oversight. A live + /// WorldSession always has a key (its world's name when it has no character), so an unowned + /// spawn window cannot come from a connection at all. And the rail lists an unowned window under + /// every character precisely because it belongs to none — so two of them for one target + /// would draw two identical rows under everybody, with nothing in the client able to tell them apart + /// or route between them. Null is one identity, "nobody", and not an unknown owner. + /// + [Test] + public async Task WindowsWithNoOwnerAreOneWindowPerTarget() + { + var workspace = new Workspace(); + + var first = workspace.RouteSpawn("Public"); + var second = workspace.RouteSpawn("Public"); + + await Assert.That(second).IsEqualTo(first); + await Assert.That(first.SessionKey).IsNull(); + + // …and an unowned window is still nobody's, so it does not collide with an owned one. + await Assert.That(workspace.RouteSpawn("Public", "Convergence.Ann")).IsNotEqualTo(first); + } +} diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs index 4cc9c03..3d107e1 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WindowNumberingTests.cs @@ -37,7 +37,7 @@ public async Task WindowsAreNumberedInTheOrderTheyWereOpened() ws.RouteSpawn("Chat", "W.C"); ws.OpenWindow("web", "Web"); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Chat,web"); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Chat")},web"); } /// @@ -53,7 +53,7 @@ public async Task MovingAWindowToAnotherPaneDoesNotRenumberAnything() ws.RouteSpawn("Trade", "W.C"); var before = Order(ws); - ws.Layout.SplitWithWindow("spawn:Trade", ws.Layout.FocusedPaneId, Edge.Left); + ws.Layout.SplitWithWindow(Spawn("Trade"), ws.Layout.FocusedPaneId, Edge.Left); await Assert.That(Order(ws)).IsEqualTo(before); } @@ -69,7 +69,7 @@ public async Task ReorderingTabsDoesNotRenumberAnything() ws.RouteSpawn("Chat", "W.C"); var before = Order(ws); - ws.ActivateWindow("spawn:Chat"); + ws.ActivateWindow(Spawn("Chat")); await Assert.That(ws.Layout.ReorderActiveTab(-1)).IsTrue(); await Assert.That(Order(ws)).IsEqualTo(before); @@ -88,9 +88,9 @@ public async Task ClosingAWindowCompactsTheNumbering() ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.CloseWindow("spawn:Chat"); + ws.CloseWindow(Spawn("Chat")); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade"); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")}"); await Assert.That(ws.WindowsFor(Owner)[1].Sequence) .IsGreaterThan(2) .Because("the sequence keeps its hole; only the position closes up"); @@ -113,10 +113,10 @@ public async Task AWindowOpenedIntoAClosedOnesSlotStillTakesTheLastNumber() ws.RouteSpawn("Trade", "W.C"); ws.RouteSpawn("Newbie", "W.C"); - ws.CloseWindow("spawn:Chat"); // frees the second slot + ws.CloseWindow(Spawn("Chat")); // frees the second slot ws.RouteSpawn("Guild", "W.C"); // which the registry hands straight back out - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade,spawn:Newbie,spawn:Guild") + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")},{Spawn("Newbie")},{Spawn("Guild")}") .Because("the newcomer is ⌥4, and Trade and Newbie are still ⌥2 and ⌥3"); } @@ -133,10 +133,10 @@ public async Task AWindowNoPaneHoldsIsNotNumbered() ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.Layout.RemoveWindow("spawn:Chat"); // out of the tree, still in the registry + ws.Layout.RemoveWindow(Spawn("Chat")); // out of the tree, still in the registry - await Assert.That(ws.Windows.Select(w => w.Id)).Contains("spawn:Chat"); - await Assert.That(Order(ws)).IsEqualTo("main,spawn:Trade"); + await Assert.That(ws.Windows.Select(w => w.Id)).Contains(Spawn("Chat")); + await Assert.That(Order(ws)).IsEqualTo($"main,{Spawn("Trade")}"); } // --- across a restart --------------------------------------------------------------------------- @@ -151,7 +151,7 @@ public async Task TheNumberingComesBackAfterAResume() var ws = new Workspace(mainWindowId: "main", mainTitle: "Main", sessionKey: "W.C"); ws.RouteSpawn("Chat", "W.C"); ws.RouteSpawn("Trade", "W.C"); - ws.CloseWindow("spawn:Chat"); + ws.CloseWindow(Spawn("Chat")); ws.RouteSpawn("Newbie", "W.C"); var before = Order(ws); @@ -234,6 +234,9 @@ public async Task AHalfMigratedWorkspaceGivesNoTwoWindowsOneNumber() /// The character every window in these fixtures belongs to. private const string Owner = "W.C"; + /// The window id produces for . + private static string Spawn(string target) => Workspace.SpawnWindowId(Owner, target); + private static string Order(Workspace workspace) => string.Join(",", workspace.WindowsFor(Owner).Select(w => w.Id)); } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs index 2083109..b211ed3 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceStateTests.cs @@ -26,7 +26,7 @@ public async Task Capture_ThenRestore_PreservesWindowsAndTree() // Windows survive with their metadata. await Assert.That(restored.Windows.Count).IsEqualTo(2); - var chat = restored.FindWindow(Workspace.SpawnWindowId("Chat"))!; + var chat = restored.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!; await Assert.That(chat.Kind).IsEqualTo(WindowKind.Spawn); await Assert.That(chat.OwnerLabel).IsEqualTo("Corvid"); await Assert.That(chat.SessionKey).IsEqualTo("Aetherfall.Corvid"); @@ -34,9 +34,9 @@ public async Task Capture_ThenRestore_PreservesWindowsAndTree() // The split tree survives: two panes, main and Chat separated. await Assert.That(restored.Layout.Panes.Count).IsEqualTo(2); await Assert.That(restored.Layout.FindWindow("main")).IsNotNull(); - await Assert.That(restored.Layout.FindWindow(Workspace.SpawnWindowId("Chat"))).IsNotNull(); + await Assert.That(restored.Layout.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))).IsNotNull(); await Assert.That(restored.Layout.FindWindow("main")!.Id) - .IsNotEqualTo(restored.Layout.FindWindow(Workspace.SpawnWindowId("Chat"))!.Id); + .IsNotEqualTo(restored.Layout.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.Id); } [Test] @@ -78,6 +78,7 @@ public async Task State_RoundTripsThroughJson() await Assert.That(json).Contains("\"kind\": \"Spawn\""); await Assert.That(json).Contains("\"type\": \"split\""); await Assert.That(restored.Layout.Panes.Count).IsEqualTo(2); - await Assert.That(restored.FindWindow(Workspace.SpawnWindowId("Chat"))!.OwnerLabel).IsEqualTo("Corvid"); + await Assert.That(restored.FindWindow(Workspace.SpawnWindowId("Aetherfall.Corvid", "Chat"))!.OwnerLabel) + .IsEqualTo("Corvid"); } } diff --git a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs index 6b75dba..16caea6 100644 --- a/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs +++ b/tests/SharpMUTerm.Core.Tests/Workspace/WorkspaceTests.cs @@ -25,7 +25,7 @@ public async Task RouteSpawn_CreatesBackgroundWindow_AndAccruesUnread() var chat = w.RouteSpawn("Chat"); await Assert.That(chat.Kind).IsEqualTo(WindowKind.Spawn); - await Assert.That(chat.Id).IsEqualTo(Workspace.SpawnWindowId("Chat")); + await Assert.That(chat.Id).IsEqualTo(Workspace.SpawnWindowId(null, "Chat")); await Assert.That(w.IsVisible(chat.Id)).IsFalse(); // main stays the active tab await Assert.That(chat.Unread).IsEqualTo(1); @@ -93,7 +93,7 @@ public async Task CloseWindow_RemovesFromRegistryAndLayout() { var w = new Workspace(); w.RouteSpawn("Chat"); - var chatId = Workspace.SpawnWindowId("Chat"); + var chatId = Workspace.SpawnWindowId(null, "Chat"); var closed = w.CloseWindow(chatId); diff --git a/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs b/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs index a937435..7dff736 100644 --- a/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FocusIndicationTests.cs @@ -66,7 +66,7 @@ private static void Type(SharpMUTermApp app, string text) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; // --- item 1: the input bands ------------------------------------------------------------------ diff --git a/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs index 5816530..704f680 100644 --- a/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/HistorySearchEndToEndTests.cs @@ -594,11 +594,11 @@ public async Task CtrlWDoesNotKillTheWordBeforeTheCaret() public async Task CtrlWClosesTheWindow() { var (app, _) = await Demo(); - app.SimulateWindowChange("spawn:Chat"); - await Assert.That(app.WindowIds()).Contains("spawn:Chat"); + app.SimulateWindowChange(DemoScene.ChatWindowId); + await Assert.That(app.WindowIds()).Contains(DemoScene.ChatWindowId); app.SimulateKey(Chord(ConsoleKey.W)); - await Assert.That(app.WindowIds()).DoesNotContain("spawn:Chat"); + await Assert.That(app.WindowIds()).DoesNotContain(DemoScene.ChatWindowId); } } diff --git a/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs index 3046c3c..0c58b08 100644 --- a/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/InputAreaEndToEndTests.cs @@ -28,7 +28,9 @@ public class InputAreaEndToEndTests private const int Width = 120; private const int Height = 34; private const string MainWindow = "main"; - private const string ChatWindow = "spawn:Chat"; + + /// The demo's Chat spawn window. Its id names its owner, so it is asked for rather than spelt. + private static string ChatWindow => DemoScene.ChatWindowId; private static readonly TerminalCapabilities Headless = new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); diff --git a/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs b/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs index a2fa015..1cf7fc5 100644 --- a/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/NawsPaneReportTests.cs @@ -96,7 +96,7 @@ private static PaneRect OutputRectOf(SharpMUTermApp app, string windowId) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; // --- the bug --------------------------------------------------------------- diff --git a/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs index a621773..f4b1060 100644 --- a/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PanePrefixEndToEndTests.cs @@ -41,9 +41,9 @@ private static SharpMUTermApp OneTab() return new SharpMUTermApp(new AppConfiguration(), Headless, new HeadlessConsoleDriver(Width, Height)); } - private static readonly string ChatId = Workspace.SpawnWindowId("Chat"); + private static readonly string ChatId = Workspace.SpawnWindowId(null, "Chat"); - private static readonly string OocId = Workspace.SpawnWindowId("OOC"); + private static readonly string OocId = Workspace.SpawnWindowId(null, "OOC"); /// /// One pane holding three tabs — main, a Chat spawn and an OOC spawn — with the tab at diff --git a/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs index eaa98c3..38fde54 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneResizeEndToEndTests.cs @@ -70,7 +70,7 @@ private static void Type(SharpMUTermApp app, string text) } /// The demo workspace's spawn window — the second tab, and after a split the second pane. - private static string ChatWindowId => Workspace.SpawnWindowId("Chat"); + private static string ChatWindowId => DemoScene.ChatWindowId; /// Presses the chord and renders the frame it produces, the way the running app would. private static void Press(SharpMUTermApp app, ConsoleKey arrow) diff --git a/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs index 1d68d65..78ec30f 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneTabCloseTests.cs @@ -75,7 +75,7 @@ private static List Flatten(LayoutNodeState node) => public async Task TheActiveTabsCloseButtonClosesIt() { var harness = Rendered(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; await Assert.That(Tabs(harness.App)).Contains(chat); SweepRow(harness, 0); @@ -90,7 +90,7 @@ public async Task TheActiveTabsCloseButtonClosesIt() public async Task ClickingATabsLabelSelectsItWithoutClosing() { var harness = Rendered(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; // Column 1 is inside the first tab's label (the framework pads each tab with a space). harness.App.SimulateTabStripClick(harness.PaneId, 1, 0); @@ -121,6 +121,6 @@ public async Task TheMainWindowIsNeverClosable() SweepRow(harness, 0); await Assert.That(Tabs(harness.App)).Contains("main"); - await Assert.That(Tabs(harness.App)).Contains(Workspace.SpawnWindowId("Chat")); + await Assert.That(Tabs(harness.App)).Contains(DemoScene.ChatWindowId); } } diff --git a/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs b/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs index 89591fd..6103d93 100644 --- a/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PrefixWhichKeyTests.cs @@ -59,7 +59,7 @@ private static SharpMUTermApp Fresh() private static SharpMUTermApp ChatForward() { var app = App(); - app.SimulateWindowChange(Workspace.SpawnWindowId("Chat")); + app.SimulateWindowChange(DemoScene.ChatWindowId); return app; } diff --git a/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs b/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs index 5eb3b00..3391a0e 100644 --- a/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RailClickTests.cs @@ -151,9 +151,8 @@ public async Task ClickingAWindowRow_ActivatesThatWindow() app.RenderSnapshot(); // The demo's active character owns a spawn window as well as the main one. - var spawn = app.WindowIds().First(id => id.StartsWith("spawn:", StringComparison.Ordinal)); - var title = SpawnTitle(spawn); - ClickRailRow(app, title); + var spawn = app.WindowIds().First(id => id.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal)); + ClickRailRow(app, app.WindowTitleOf(spawn)!); await Assert.That(app.ActiveWindowId()).IsEqualTo(spawn); } @@ -417,5 +416,4 @@ private static void ClickRailRow(SharpMUTermApp app, string text) } } - private static string SpawnTitle(string spawnWindowId) => spawnWindowId["spawn:".Length..]; } diff --git a/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs b/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs index 6989461..c870098 100644 --- a/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RailWindowRowTests.cs @@ -168,7 +168,7 @@ public async Task ClosingAWindowRemovesItsRowRatherThanMarkingItClosed() { var app = App(); app.RenderSnapshot(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; await Assert.That(string.Join("\n", Rail(app))).Contains("Chat"); await Assert.That(app.DispatchCommand("win:" + chat)).IsTrue(); // bring it up so ⌃W can close it diff --git a/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs b/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs index 6598a52..b4be291 100644 --- a/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/RestoreLogEndToEndTests.cs @@ -5,6 +5,7 @@ using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Session; using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; using SharpMUTerm.Graphics; namespace SharpMUTerm.Tui.Tests; @@ -32,7 +33,14 @@ public class RestoreLogEndToEndTests private const int Width = 160; private const int Height = 40; private const string MainWindow = "main"; - private const string ChatWindow = "spawn:Chat"; + + /// + /// The one character in this fixture, and the Chat window its capture rule routes to. A spawn + /// window's id names its owner, so the id is asked for rather than spelt out. + /// + private const string Character = "Aetherfall.Corvid"; + + private static readonly string ChatWindow = Workspace.SpawnWindowId(Character, "Chat"); /// The startup ceiling asserted below — see that test for the measured figure it guards. private const int Ceiling = 400; @@ -385,7 +393,9 @@ public async Task RestoringAFullLogCostsLittleEnoughToRunBeforeTheFirstFrame() { using var root = new TempRoot(); var config = Configuration(); - var windows = new[] { MainWindow, ChatWindow, "spawn:OOC", "spawn:Tells", "spawn:Guild", "spawn:Events" }; + var windows = new[] { MainWindow, ChatWindow } + .Concat(new[] { "OOC", "Tells", "Guild", "Events" }.Select(t => Workspace.SpawnWindowId(Character, t))) + .ToArray(); using (var seed = new RestoreLog(root.Path, config.RestoreLog)) { diff --git a/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs b/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs index a7c65da..8e19063 100644 --- a/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/SessionStateAccountingTests.cs @@ -68,7 +68,7 @@ public async Task ASetAssignedWhileConnected_OpensTheSpawnWindowOnTheNextMatchin var mannaz = wired.Config.Worlds[0].Characters[0]; wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsFalse(); // the state the bug lived in + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsFalse(); // the state the bug lived in // WorldsScreenRenderer.Assignment: a character opts into a set by list membership… mannaz.TriggerSets.Add("Comms"); @@ -77,8 +77,8 @@ public async Task ASetAssignedWhileConnected_OpensTheSpawnWindowOnTheNextMatchin wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); - await Assert.That(string.Join("\n", wired.App.PaneLines(Workspace.SpawnWindowId("Public")))) + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); + await Assert.That(string.Join("\n", wired.App.PaneLines(Workspace.SpawnWindowId("Convergence.Mannaz", "Public")))) .Contains("Lol"); } @@ -97,7 +97,7 @@ public async Task ARuleAddedToAnAssignedSetWhileConnected_TakesEffectOnTheNextLi wired.App.SaveConfiguration(); wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsFalse(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsFalse(); wired.Config.TriggerSets.Single(s => s.Name == "Comms").Triggers.Add(new Trigger { @@ -109,7 +109,7 @@ public async Task ARuleAddedToAnAssignedSetWhileConnected_TakesEffectOnTheNextLi wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); } /// @@ -129,7 +129,7 @@ public async Task ACommittedEditOnAnySettingsScreen_IsWhatMakesAnAutomationChang wired.Receive(wired.MannazWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Convergence.Mannaz", "Public")).IsTrue(); } /// @@ -150,7 +150,7 @@ public async Task ASessionOpenedByTheCharacterSwitchPath_RoutesItsCaptures() wired.Receive(wired.RikoWire, PublicLine); - await Assert.That(SpawnWindowExists(wired.App, "Public")).IsTrue(); + await Assert.That(SpawnWindowExists(wired.App, "Grapevine.Riko", "Public")).IsTrue(); } /// @@ -293,8 +293,13 @@ public async Task ABackgroundConnectionDropping_ChangesTheCount() private const string PublicLine = " Starfall Empress Lucille Wolfsbane says, \"Lol\"\n"; - private static bool SpawnWindowExists(SharpMUTermApp app, string target) => - app.WindowIds().Contains(Workspace.SpawnWindowId(target), StringComparer.Ordinal); + /// + /// Whether has a spawn window for . The owner is + /// part of the question because it is part of the window id: two characters capturing one target + /// have a window each. + /// + private static bool SpawnWindowExists(SharpMUTermApp app, string owner, string target) => + app.WindowIds().Contains(Workspace.SpawnWindowId(owner, target), StringComparer.Ordinal); private static ConsoleKeyInfo CtrlQ() => new('\0', ConsoleKey.Q, false, false, true); diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs new file mode 100644 index 0000000..77b7fd4 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs @@ -0,0 +1,279 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The upgrade, driven the whole distance. A configuration written by the previous build — schema +/// v4, a saved workspace holding spawn:Chat in a pane — and a restore log written beside it under +/// that same id, opened by this build. The user must lose neither the pane nor the text in it, and the +/// pane must come back as something the client will still write to. +/// +/// The two stores are joined only by the window id, and this change moves it, so they have to be +/// upgraded together or the upgrade is worse than the bug: rewriting only config.json brings the +/// pane back empty for ever, and rewriting only the log leaves the content filed under an id no pane +/// has. ConfigurationMigrator's v4→v5 step does the first and +/// SharpMUTermApp.CarryLegacySpawnLogsOver does the second, at the launch that reads them. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class SpawnWindowIdUpgradeTests +{ + private const int Width = 160; + private const int Height = 40; + private const string Corvid = "Aetherfall.Corvid"; + private const string LegacyChat = "spawn:Chat"; + private const string Backlog = "Rivane: anyone up for the crypt run?"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + private static string CorvidsChat => Workspace.SpawnWindowId(Corvid, "Chat"); + + /// + /// The headline: the pane is still there, still in its pane, still called Chat, and still + /// holding the previous session's conversation — under the id its owner now routes to. The user sees + /// no difference at all, which is the point. + /// + [Test] + public async Task AnOldConfigKeepsItsPaneItsPlacementAndItsRestoredScrollback() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + // The pane, under the id this build routes to — and not under the one it was saved as. + await Assert.That(app.WindowIds()).Contains(CorvidsChat); + await Assert.That(app.WindowIds()).DoesNotContain(LegacyChat); + await Assert.That(app.WindowTitleOf(CorvidsChat)).IsEqualTo("Chat"); + await Assert.That(app.WindowOwnerOf(CorvidsChat)).IsEqualTo(Corvid); + + // Still beside the main window in the pane it was saved in. + await Assert.That(app.CaptureSession().Root.Tabs).IsEquivalentTo(new[] { "main", CorvidsChat }); + + // And holding what it held, closed off by the restore bar. + var lines = string.Join("\n", app.PaneLines(CorvidsChat)); + await Assert.That(lines).Contains(Backlog); + await Assert.That(lines).Contains(RestoreBarRenderer.Label); + } + + /// + /// No orphan. The upgraded pane is one the running client still writes to: the character + /// connects, its capture rule fires, and the line lands in the pane that came back rather than in a + /// second one beside it. This is the assertion that rules out the tempting cheaper answer of leaving + /// the old id alone. + /// + [Test] + public async Task TheUpgradedPaneIsTheOneTheLiveCaptureWritesTo() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + var telnet = new RecordingTelnetSession(); + app.TelnetFactory = _ => telnet; + app.DispatchCommand(CommandIds.Character(Corvid)); + await app.FindSession(Corvid)!.ConnectAsync(); + + telnet.Receive("[Chat] Bob: aye, meet me at the gate\n"); + app.RenderNextFrame(); + + // One Chat window, and the new line is under the restored history in it. + await Assert.That(app.WindowIds().Count(id => id.EndsWith(":Chat", StringComparison.Ordinal))).IsEqualTo(1); + var lines = app.PaneLines(CorvidsChat).ToList(); + await Assert.That(string.Join("\n", lines)).Contains(Backlog); + await Assert.That(lines[^1]).Contains("meet me at the gate"); + } + + /// + /// The carry-over happens once. The second launch reads a log that is already in the new shape, so + /// the pane comes back holding its history once rather than twice — which is what a mapping kept + /// for ever, or an old file left on disk beside the new one, would have cost on every launch after + /// the upgrade. + /// + [Test] + public async Task TheUpgradeHappensOnceAndDoesNotDoubleThePaneNextLaunch() + { + using var root = new TempRoot(); + SeedLegacyLog(root); + var config = OldConfiguration(); + + await Assert.That(root.Files.Any(f => Path.GetFileName(f).StartsWith("spawnChat-", StringComparison.Ordinal))) + .IsTrue() + .Because("the previous build's file has to be there for its removal to mean anything"); + + using (var first = new RestoreLog(root.Path, config.RestoreLog)) + { + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: first); + config.LastSession = app.CaptureSession(); + } + + // The file the old id was in is gone, and the new id has one of its own. + await Assert.That(root.Files.Any(f => Path.GetFileName(f).StartsWith("spawnChat-", StringComparison.Ordinal))) + .IsFalse(); + + using var second = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var relaunched = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: second); + + var lines = relaunched.PaneLines(CorvidsChat).ToList(); + await Assert.That(lines.Count(l => l.Contains(Backlog, StringComparison.Ordinal))).IsEqualTo(1); + await Assert.That(lines.Count(l => l.Contains(RestoreBarRenderer.Label, StringComparison.Ordinal))) + .IsEqualTo(1); + } + + /// + /// A log under an old id that no pane claims is left exactly as it always was: buffered under its + /// own id, not carried anywhere, not deleted — so if that channel ever speaks again its pane opens + /// with its history already in it. The upgrade may not turn "the saved workspace forgot this pane" + /// into "the content is gone". + /// + [Test] + public async Task AnOldLogNoPaneClaimsIsLeftWhereItIs() + { + using var root = new TempRoot(); + using (var seed = new RestoreLog(root.Path)) + { + seed.Append("spawn:Tells", "Tells", StyledLine.FromText("Rivane pages: hello", TextStyle.Default), "09:24"); + } + + var config = OldConfiguration(); + using var log = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: log); + + await Assert.That(log.Read().Any(w => w.WindowId == "spawn:Tells")).IsTrue(); + await Assert.That(app.WindowIds().Any(id => id.EndsWith(":Tells", StringComparison.Ordinal))).IsFalse(); + } + + /// + /// And the fix survives the round trip it is most likely to be undone by. Two characters capture one + /// target, the workspace is saved and reopened, and each still has a pane of their own holding their + /// own conversation — the ids are stable across a restart, and the restore log's per-window keying + /// keeps the two apart on disk as well as in memory. + /// + [Test] + public async Task TwoCharactersPanesComeBackSeparatelyAfterARestart() + { + using var root = new TempRoot(); + var config = SpawnWindowPerSessionTests.Configuration(); + var ann = Workspace.SpawnWindowId("Convergence.Ann", "Public"); + var bob = Workspace.SpawnWindowId("Convergence.Bob", "Public"); + + using (var first = new RestoreLog(root.Path, config.RestoreLog)) + { + Console.SetIn(TextReader.Null); + await using var app = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: first); + + foreach (var (key, text) in new[] { ("Convergence.Ann", "first"), ("Convergence.Bob", "second") }) + { + var telnet = new RecordingTelnetSession(); + app.TelnetFactory = _ => telnet; + app.DispatchCommand(CommandIds.Character(key)); + await app.FindSession(key)!.ConnectAsync(); + telnet.Receive($" {key} says, \"{text}\"\n"); + app.RenderNextFrame(); + } + + config.LastSession = app.CaptureSession(); + } + + using var second = new RestoreLog(root.Path, config.RestoreLog); + Console.SetIn(TextReader.Null); + await using var relaunched = new SharpMUTermApp( + config, Headless, new HeadlessConsoleDriver(Width, Height), restore: second); + + await Assert.That(string.Join("\n", relaunched.PaneLines(ann))).Contains("first"); + await Assert.That(string.Join("\n", relaunched.PaneLines(ann))).DoesNotContain("second"); + await Assert.That(string.Join("\n", relaunched.PaneLines(bob))).Contains("second"); + await Assert.That(string.Join("\n", relaunched.PaneLines(bob))).DoesNotContain("first"); + } + + // ---- Harness ---------------------------------------------------------------------------- + + /// A throwaway restore-log root, removed however the test ends. + private sealed class TempRoot : IDisposable + { + public TempRoot() => + Path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"smuterm-upgrade-{Guid.NewGuid():N}"); + + public string Path { get; } + + public IReadOnlyList Files => + Directory.Exists(Path) ? Directory.GetFiles(Path) : Array.Empty(); + + public void Dispose() + { + try + { + if (Directory.Exists(Path)) + { + Directory.Delete(Path, recursive: true); + } + } + catch (Exception) + { + // Nothing a test should fail over. + } + } + } + + /// The previous build's restore log: one window, keyed by the id that build wrote. + private static void SeedLegacyLog(TempRoot root) + { + using var seed = new RestoreLog(root.Path); + seed.Append(LegacyChat, "Chat", StyledLine.FromText("[Chat] " + Backlog, TextStyle.Default), "09:24"); + } + + /// + /// A genuine v4 document, parsed the way the client parses one at startup — so the migration under + /// test is the one that actually runs, rather than a hand-built object graph that has already had + /// the answer written into it. + /// + private static AppConfiguration OldConfiguration() => ConfigurationStore.Deserialize(""" + { + "version": 4, + "worlds": [ { + "name": "Aetherfall", "host": "aetherfall.example.org", "port": 4201, + "characters": [ { "name": "Corvid", "triggerSets": [ "chat" ], "logging": { "format": "None" } } ] + } ], + "triggerSets": [ { + "name": "chat", + "triggers": [ { "name": "chat", "pattern": "^\\[Chat\\]", + "actions": { "spawnTarget": "Chat", "gag": true } } ] + } ], + "lastSession": { + "windows": [ + { "id": "main", "title": "Corvid", "kind": "Main", "sessionKey": "Aetherfall.Corvid" }, + { "id": "spawn:Chat", "title": "Chat", "kind": "Spawn", "sessionKey": "Aetherfall.Corvid", + "ownerLabel": "Corvid", "capturePattern": "^\\[Chat\\]" } + ], + "root": { "type": "pane", "id": "p1", "tabs": [ "main", "spawn:Chat" ], "activeIndex": 0 }, + "focusedPaneId": "p1" + } + } + """); +} diff --git a/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs new file mode 100644 index 0000000..c8b3a8c --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/SpawnWindowPerSessionTests.cs @@ -0,0 +1,349 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Session; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The reported defect: two connected characters whose triggers match the same pattern shared one +/// capture pane. Whichever session matched first created the window — with its own +/// sessionKey on it — and every later session's lines were appended to that same window, which +/// somebody else owned. The second character's channel was therefore not merely mixed in with the +/// first's: it was filed under the wrong character, and draws +/// window rows for the active character only, so it was invisible from the character it +/// belonged to and reachable only from the one it did not. +/// +/// The cause was Workspace.SpawnWindowId, which was $"spawn:{target}" — no owner in it, so +/// one window per workspace per target rather than one per session per target. +/// +/// +/// The fixture is the smallest one that can show it: two characters on one world, both assigned the +/// same trigger set, both connected over their own recording transport. One character per target +/// passes either way, which is why this shipped. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class SpawnWindowPerSessionTests +{ + private const int Width = 160; + private const int Height = 40; + + private const string Ann = "Convergence.Ann"; + private const string Bob = "Convergence.Bob"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + // ---- The report ------------------------------------------------------------------------- + + /// + /// The headline. Both characters capture Public; there are two windows, one owned by each. + /// Before the fix there was one, owned by whoever matched first. + /// + [Test] + public async Task TwoCharactersCapturingOneTargetGetAWindowEach() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + var annWindow = Workspace.SpawnWindowId(Ann, "Public"); + var bobWindow = Workspace.SpawnWindowId(Bob, "Public"); + + await Assert.That(two.App.WindowIds()).Contains(annWindow); + await Assert.That(two.App.WindowIds()).Contains(bobWindow); + await Assert.That(annWindow).IsNotEqualTo(bobWindow); + + await Assert.That(two.App.WindowOwnerOf(annWindow)).IsEqualTo(Ann); + await Assert.That(two.App.WindowOwnerOf(bobWindow)).IsEqualTo(Bob); + } + + /// + /// And the lines land in their own pane and only there. This is the assertion the user's sentence is + /// about — "one wins and gets 2 redirects" — and it fails on the unfixed build with both lines in + /// Ann's pane and Bob's pane not existing at all. + /// + [Test] + public async Task EachCharactersCaptureLandsInItsOwnPaneAndNoOther() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + var ann = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Ann, "Public"))); + var bob = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Bob, "Public"))); + + await Assert.That(ann).Contains("first"); + await Assert.That(ann).DoesNotContain("second"); + await Assert.That(bob).Contains("second"); + await Assert.That(bob).DoesNotContain("first"); + } + + /// + /// The visible half of the same defect. The rail lists the active character's windows, so a + /// capture pane filed under the wrong owner cannot be seen or clicked from the character whose + /// channel it is. Each character's rail now carries exactly one Public row: their own. + /// + [Test] + public async Task EachCharactersRailListsItsOwnCapturePaneAndNotTheOthers() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + // Bob is the character switched to last, so the rail is drawn for Bob. + var bobsRail = two.App.RailLines; + await Assert.That(RailTargets(bobsRail)).Contains("win:" + Workspace.SpawnWindowId(Bob, "Public")); + await Assert.That(RailTargets(bobsRail)).DoesNotContain("win:" + Workspace.SpawnWindowId(Ann, "Public")); + + two.App.DispatchCommand(CommandIds.Character(Ann)); + var annsRail = two.App.RailLines; + await Assert.That(RailTargets(annsRail)).Contains("win:" + Workspace.SpawnWindowId(Ann, "Public")); + await Assert.That(RailTargets(annsRail)).DoesNotContain("win:" + Workspace.SpawnWindowId(Bob, "Public")); + } + + /// + /// The id changed and the name did not. Both panes are still called Public — on the tab + /// strip and in the sidebar — because every surface that names a window reads + /// and none derives a display name from the id. A pane labelled + /// Convergence.Ann:Public would be this fix leaking its own bookkeeping onto the screen. + /// + [Test] + public async Task BothPanesAreStillCalledPublic() + { + var two = await Two(); + + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + two.Receive(two.BobWire, " Bob says, \"second\"\n"); + + await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Ann, "Public"))).IsEqualTo("Public"); + await Assert.That(two.App.WindowTitleOf(Workspace.SpawnWindowId(Bob, "Public"))).IsEqualTo("Public"); + + // The tab strip and the sidebar say the same — asserted about *those two windows* rather than + // over the whole frame, so the test cannot pass because some unrelated surface happened to + // satisfy it. + // + // What must not leak is the *session key*. The character's own name is a different thing and is + // on the strip deliberately: a spawn tab reads "Ann - Public" precisely so two characters each + // capturing Public can be told apart — which is the feature this PR exists for. So the drawn + // title carries "Ann" and must never carry "Convergence.Ann". + two.App.RenderNextFrame(); + + foreach (var owner in new[] { Ann, Bob }) + { + var id = Workspace.SpawnWindowId(owner, "Public"); + + var pane = two.App.PaneIdOf(id); + await Assert.That(pane).IsNotNull().Because($"{owner}'s Public pane must be placed"); + + // Read through StripMarkup: the strip's labels are markup — #14 tints the tab a line + // arrived in — so the raw string carries colour tags around the name. + var tab = two.App.PaneTabTitles[pane!] + .Select(StripMarkup) + .SingleOrDefault(t => t.Contains("Public", StringComparison.Ordinal) && + t.Contains(CharacterOf(owner), StringComparison.Ordinal)); + + await Assert.That(tab).IsNotNull().Because($"{owner}'s Public pane must have a tab naming it"); + await Assert.That(tab!) + .DoesNotContain(owner) + .Because("the id carries the session key; the tab the reader sees must not"); + + // The rail row is found by its `win:` click payload — which does carry the owner — and + // then read for what it *draws*, which must not. Only the active character's window rows are + // drawn, so exactly one of the two is on screen. + var row = two.App.RailLines.SingleOrDefault(l => l.Contains("win:" + id, StringComparison.Ordinal)); + if (row is null) + { + continue; + } + + var drawn = StripMarkup(row); + await Assert.That(drawn).Contains("Public"); + await Assert.That(drawn) + .DoesNotContain(owner) + .Because("the id carries the session key; the row the reader sees must not"); + } + } + + /// The character half of a World.Character session key. + private static string CharacterOf(string sessionKey) => sessionKey[(sessionKey.IndexOf('.') + 1)..]; + + /// + /// The id is stable across a reconnect: the same character dropping and dialling back in keeps the + /// window it had, rather than accruing a second one beside it. + /// + [Test] + public async Task ReconnectingKeepsTheSameCapturePane() + { + var two = await Two(); + two.Receive(two.AnnWire, " Ann says, \"first\"\n"); + + var before = two.App.WindowIds().Count(id => id.Contains("Public", StringComparison.Ordinal)); + + await two.App.FindSession(Ann)!.DisconnectAsync(); + await two.App.FindSession(Ann)!.ConnectAsync(); + two.Receive(two.AnnWire, " Ann says, \"again\"\n"); + + await Assert.That(two.App.WindowIds().Count(id => id.Contains("Public", StringComparison.Ordinal))) + .IsEqualTo(before); + var pane = string.Join("\n", two.App.PaneLines(Workspace.SpawnWindowId(Ann, "Public"))); + await Assert.That(pane).Contains("first"); + await Assert.That(pane).Contains("again"); + } + + // ---- Harness ---------------------------------------------------------------------------- + + /// + /// One rail row with its Spectre markup removed, so an assertion can read what the sidebar + /// draws rather than the [link=win:<id>] payload it carries underneath. + /// + private static string StripMarkup(string line) + { + var text = new System.Text.StringBuilder(line.Length); + for (var i = 0; i < line.Length; i++) + { + if (line[i] != '[') + { + text.Append(line[i]); + continue; + } + + if (i + 1 < line.Length && line[i + 1] == '[') + { + text.Append('['); + i++; + continue; + } + + var close = line.IndexOf(']', i); + i = close < 0 ? line.Length : close; + } + + return text.ToString(); + } + + /// The win:… click payloads the rail is currently drawing. + private static IReadOnlyList RailTargets(IReadOnlyList lines) + { + var targets = new List(); + foreach (var line in lines) + { + var at = 0; + while ((at = line.IndexOf("[link=", at, StringComparison.Ordinal)) >= 0) + { + at += "[link=".Length; + var end = line.IndexOf(']', at); + if (end < 0) + { + break; + } + + targets.Add(line[at..end]); + at = end; + } + } + + return targets; + } + + private static Task Two() => TwoCharacters.Build(); + + /// + /// One world, two characters, one shared trigger set routing ^<Public> to a spawn window + /// called Public — and both characters connected over a transport of their own, because a + /// session that was never connected never runs its receive path and would make any capture + /// assertion true whatever the code does. + /// + private sealed class TwoCharacters + { + private TwoCharacters( + SharpMUTermApp app, AppConfiguration config, RecordingTelnetSession ann, RecordingTelnetSession bob) + { + App = app; + Config = config; + AnnWire = ann; + BobWire = bob; + } + + internal SharpMUTermApp App { get; } + + internal AppConfiguration Config { get; } + + internal RecordingTelnetSession AnnWire { get; } + + internal RecordingTelnetSession BobWire { get; } + + internal static async Task Build() + { + Console.SetIn(TextReader.Null); + var config = Configuration(); + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + var two = new TwoCharacters(app, config, new RecordingTelnetSession(), new RecordingTelnetSession()); + + await two.Open(Ann, two.AnnWire); + await two.Open(Bob, two.BobWire); + app.RenderNextFrame(); + return two; + } + + internal void Receive(RecordingTelnetSession telnet, string text) + { + telnet.Receive(text); + App.RenderNextFrame(); + } + + internal async Task Open(string sessionKey, RecordingTelnetSession telnet) + { + App.TelnetFactory = _ => telnet; + if (!App.DispatchCommand(CommandIds.Character(sessionKey))) + { + throw new InvalidOperationException($"the app would not switch to {sessionKey}"); + } + + await App.FindSession(sessionKey)!.ConnectAsync(); + } + } + + internal static AppConfiguration Configuration() + { + var config = new AppConfiguration(); + config.TriggerSets.Add(new TriggerSet + { + Name = "Comms", + Triggers = + { + new Trigger + { + Name = "Public", + Pattern = "^", + Actions = new TriggerActions { SpawnTarget = "Public", Gag = true }, + }, + }, + }); + + config.Worlds.Add(new WorldDefinition + { + Name = "Convergence", + Host = "convergence.example.org", + Port = 4201, + Characters = + { + new CharacterDefinition { Name = "Ann", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + new CharacterDefinition { Name = "Bob", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + }, + }); + + return config; + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs b/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs index ef2b20e..0181aed 100644 --- a/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/WindowJumpTests.cs @@ -203,7 +203,7 @@ public async Task AnUnownedWindowIsNumberedUnderEveryCharacter() public async Task ADigitReachesACaptureWindowSittingBehindAnotherTabInItsPane() { var scene = await BuildScene(); - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; var chatPane = scene.App.PaneIdOf(chat)!; // Stand in Chat's own pane, on the other tab, so nothing but the tab has to move. @@ -397,11 +397,11 @@ public async Task ClosingAWindowCompactsTheNumberingOnTheChordAndInTheSidebar() await Assert.That(WindowRowChord(scene.App, "Web")).IsEqualTo("⌥3"); scene.App.SimulateKey(Alt(2)); // Chat - await Assert.That(scene.App.ActiveWindowId()).IsEqualTo(Workspace.SpawnWindowId("Chat")); + await Assert.That(scene.App.ActiveWindowId()).IsEqualTo(DemoScene.ChatWindowId); await Assert.That(scene.App.DispatchCommand("layout:close")).IsTrue(); scene.App.RenderNextFrame(); - await Assert.That(scene.App.NumberedWindowIds).DoesNotContain(Workspace.SpawnWindowId("Chat")); + await Assert.That(scene.App.NumberedWindowIds).DoesNotContain(DemoScene.ChatWindowId); await Assert.That(WindowRowChord(scene.App, "Web")) .IsEqualTo("⌥2") .Because("the windows on the screen must be numbered without a hole where the closed one was"); @@ -440,7 +440,9 @@ public async Task AWindowOpenedByTheWireTakesTheNextDigitAndMovesNobodyElses() scene.Transports[0].Receive(" Ann offers a lamp\n"); scene.App.RenderNextFrame(); - var arrival = Workspace.SpawnWindowId("Trade"); + // Transports[0] is Alfa.Ann's, and the line says so — the window belongs to the session whose + // wire it arrived on, which is the whole point of the per-session id. + var arrival = Workspace.SpawnWindowId("Alfa.Ann", "Trade"); await Assert.That(scene.App.NumberedWindowIds).Contains(arrival); foreach (var (what, chord) in before) @@ -877,7 +879,7 @@ private static async Task BuildScene() config.Worlds.Add(definition); } - var chat = Workspace.SpawnWindowId("Chat"); + var chat = DemoScene.ChatWindowId; var windows = new[] { "main", chat, "char:Bravo.Bob", "char:Cara.Cal" }; // Ann's two, in creation order — which is the whole of ⌥N while she is active. Bob's and Cal's