diff --git a/src/MUI.Catalog/Facets.cs b/src/MUI.Catalog/Facets.cs index 1c99726..f2cf7fc 100644 --- a/src/MUI.Catalog/Facets.cs +++ b/src/MUI.Catalog/Facets.cs @@ -88,7 +88,7 @@ public enum FacetKind /// real and useful question; it is not "games with no codebase", and neither is it a games-with-any /// filter left blank. Modelling it as a value would let one be typed where the other was meant. /// -public sealed record FacetChoice(string? Value) +public sealed record FacetChoice(string? Value, bool Exclude = false) { /// /// The querystring spelling of the absence. Tilde-prefixed so it cannot collide with a real @@ -96,22 +96,89 @@ public sealed record FacetChoice(string? Value) /// public const string UnknownToken = "~unknown"; + /// + /// The prefix that turns a selection inside out: ?codebase=!Evennia is every game whose + /// codebase is not Evennia. + /// + /// + /// + /// A facet has three states, not two, and the third one is what makes the panel a filter + /// rather than a set of shortcuts. Absent means the facet is not being asked about; a value + /// means only these; an excluded value means anything but these. Without the + /// third, "show me the games that are not Evennia" is a question the catalogue can answer and + /// the interface cannot ask. + /// + /// + /// ! rather than - because a codebase, genre or language may legitimately begin + /// with a hyphen and none of the values observed in the wild begin with a bang. A literal + /// leading ! is written !!, so a value is never unreachable — see + /// . + /// + /// + public const string ExcludeToken = "!"; + /// Games for which this facet has no value at all. public static readonly FacetChoice Unknown = new((string?)null); public static FacetChoice Of(string value) => new(value); + /// The same selection, inside out. + public static FacetChoice Not(string value) => new(value, Exclude: true); + public bool IsUnknown => Value is null; - /// What this selection is called in a URL. - public string Token => Value ?? UnknownToken; + /// What this selection is called in a URL, polarity included. + public string Token => + (Exclude ? ExcludeToken : string.Empty) + Escaped(Value ?? UnknownToken); - public static FacetChoice Parse(string token) => - string.Equals(token, UnknownToken, StringComparison.Ordinal) ? Unknown : Of(token); + /// The same facet with its polarity flipped, which is what a panel's toggle emits. + /// + /// A method and not a property, deliberately. A record's generated ToString prints + /// every public property, so a property returning another makes + /// printing one recurse until the stack runs out — which is exactly what happened, and it + /// surfaced as an unrelated test dying inside an assertion message rather than as anything to do + /// with facets. + /// + public FacetChoice Invert() => this with { Exclude = !Exclude }; - /// Whether a game whose value for this facet is matches. - public bool Matches(string? actual) => + public static FacetChoice Parse(string token) + { + ArgumentNullException.ThrowIfNull(token); + + var exclude = token.StartsWith(ExcludeToken, StringComparison.Ordinal) + && !token.StartsWith(ExcludeToken + ExcludeToken, StringComparison.Ordinal); + + var body = exclude + ? token[ExcludeToken.Length..] + // A doubled bang is a literal one: a value that genuinely starts with "!" stays + // reachable rather than being silently reinterpreted as its own negation. + : token.StartsWith(ExcludeToken + ExcludeToken, StringComparison.Ordinal) + ? token[ExcludeToken.Length..] + : token; + + return string.Equals(body, UnknownToken, StringComparison.Ordinal) + ? Unknown with { Exclude = exclude } + : new FacetChoice(body, exclude); + } + + /// + /// Whether is the value this selection names — polarity ignored. + /// + /// + /// Deliberately separate from . A facet can hand a row several tokens (a + /// game reached an hour ago is in the last day, week and month), and inverting the comparison + /// per token would make an excluded selection mean "some token differs" — which every row with + /// more than one token satisfies. The polarity is applied once, to the answer. + /// + public bool Covers(string? actual) => IsUnknown ? actual is null : string.Equals(actual, Value, StringComparison.OrdinalIgnoreCase); + + /// Whether a row that did or did not match this selection survives it. + public bool Admits(bool covered) => covered != Exclude; + + /// A value that begins with the exclusion marker is doubled so it round-trips. + private static string Escaped(string value) => + value.StartsWith(ExcludeToken, StringComparison.Ordinal) ? ExcludeToken + value : value; } /// @@ -151,7 +218,36 @@ public enum LastSeenBand /// results it will not deliver. A value nothing matches is never offered at all — the one exception /// is a value that is currently selected, which stays visible at zero so it can be seen and undone. /// -public sealed record FacetValue(string Token, int Count, bool IsSelected, bool IsUnknown); +public sealed record FacetValue( + string Token, + int Count, + bool IsSelected, + bool IsUnknown, + bool IsExcluded = false) +{ + /// + /// The three states a value can be in, as one question a renderer can switch on. + /// + /// + /// A panel that only knew would draw an included and an excluded value + /// identically, which is the one thing a tri-state filter must not do — a reader would have no + /// way to tell "only Evennia" from "anything but Evennia" except by reading the URL. + /// + public FacetState State => (IsSelected, IsExcluded) switch + { + (true, true) => FacetState.Excluded, + (true, false) => FacetState.Included, + _ => FacetState.Unselected, + }; +} + +/// Whether a facet value is being filtered in, filtered out, or not asked about. +public enum FacetState +{ + Unselected, + Included, + Excluded, +} /// One facet, ready to render: what it is called, what it reads, and what it offers. /// @@ -257,7 +353,7 @@ public static GameListing Search(IReadOnlyList rows, GameFilter fi var baseRows = rows .Where(r => (wantsArchived || r.Band is not ActivityBand.Archived) && MatchesText(r, filter.Text) - && CodebaseFamily.Matches(r.Codebase, filter.CodebaseFamily)) + && AdmitsFamily(r, filter.CodebaseFamily)) .ToList(); var results = baseRows.Where(r => Chosen(r, filter, null) && Present(r, filter)).ToList(); @@ -324,6 +420,18 @@ private static bool MatchesText(GameFacetRow row, string? text) || (row.Codebase?.Contains(needle, StringComparison.OrdinalIgnoreCase) ?? false); } + /// + /// Whether a row survives the codebase-family filter, polarity included. + /// + /// + /// Separate from the choice facets because the test is a bounded prefix rather than an equality, + /// and because this is a filter rather than a counted facet — it narrows the set the panel's + /// counts are taken over, which is what makes a codebase page's facet counts counts within that + /// codebase. + /// + private static bool AdmitsFamily(GameFacetRow row, FacetChoice? family) => + family is null || family.Admits(CodebaseFamily.Matches(row.Codebase, family.Value)); + private static bool Chosen(GameFacetRow row, GameFilter filter, string? except) { foreach (var facet in Choices) @@ -333,8 +441,9 @@ private static bool Chosen(GameFacetRow row, GameFilter filter, string? except) continue; } + // Applied once to the answer, not per token: see FacetChoice.Covers. if (facet.SelectionOf(filter) is { } selection - && !facet.TokensOf(row).Any(selection.Matches)) + && !selection.Admits(facet.TokensOf(row).Any(selection.Covers))) { return false; } @@ -416,8 +525,9 @@ .. vocabulary .Select(token => new FacetValue( token, counts.GetValueOrDefault(token), - selection?.Matches(token) ?? false, - IsUnknown: false)) + selection?.Covers(token) ?? false, + IsUnknown: false, + IsExcluded: (selection?.Covers(token) ?? false) && selection!.Exclude)) .Where(v => v.Count > 0 || v.IsSelected), ]; } @@ -442,7 +552,11 @@ private static List Open( var named = counts .Where(c => !string.Equals(c.Key, FacetChoice.UnknownToken, StringComparison.Ordinal)) .Select(c => new FacetValue( - c.Key, c.Value, selection?.Matches(c.Key) ?? false, IsUnknown: false)) + c.Key, + c.Value, + selection?.Covers(c.Key) ?? false, + IsUnknown: false, + IsExcluded: (selection?.Covers(c.Key) ?? false) && selection!.Exclude)) .OrderByDescending(v => v.IsSelected) .ThenByDescending(v => v.Count) .ThenBy(v => v.Token, StringComparer.Ordinal) @@ -456,7 +570,12 @@ private static List Open( if (unknown > 0 || unknownSelected) { - named.Add(new FacetValue(FacetChoice.UnknownToken, unknown, unknownSelected, IsUnknown: true)); + named.Add(new FacetValue( + FacetChoice.UnknownToken, + unknown, + unknownSelected, + IsUnknown: true, + IsExcluded: unknownSelected && selection!.Exclude)); } return named; diff --git a/src/MUI.Catalog/Views.cs b/src/MUI.Catalog/Views.cs index a0f8b03..47c95b8 100644 --- a/src/MUI.Catalog/Views.cs +++ b/src/MUI.Catalog/Views.cs @@ -188,8 +188,15 @@ public sealed record GameFilter /// variable, which answers TinyMUD or DikuMUD; this is the codebase with its /// version taken off. A reference page for PennMUSH wants the third and neither of the others. /// + /// + /// A for its polarity rather than its matching: the choice carries the + /// value and whether it is being filtered in or out, and the test is supplied by the + /// caller — , a bounded prefix, so ROM does not gather + /// ROMulus. It is not offered as a counted facet in the panel, so it never appears in the + /// vocabulary the choice facets are drawn from. + /// /// - public string? CodebaseFamily { get; init; } + public FacetChoice? CodebaseFamily { get; init; } } /// diff --git a/src/MUI.Web/Api/ApiModels.cs b/src/MUI.Web/Api/ApiModels.cs index 1def9cf..1abfe5e 100644 --- a/src/MUI.Web/Api/ApiModels.cs +++ b/src/MUI.Web/Api/ApiModels.cs @@ -213,7 +213,7 @@ public static FilterView Of(GameFilter filter) filter.Family?.Token, filter.Genre?.Token, filter.Language?.Token, - filter.CodebaseFamily); + filter.CodebaseFamily?.Token); } } diff --git a/src/MUI.Web/Api/GameFilterBinding.cs b/src/MUI.Web/Api/GameFilterBinding.cs index 44d9545..4803263 100644 --- a/src/MUI.Web/Api/GameFilterBinding.cs +++ b/src/MUI.Web/Api/GameFilterBinding.cs @@ -89,7 +89,9 @@ private static bool TryRead( Family = Choice(read, FacetKeys.Family), Genre = Choice(read, FacetKeys.Genre), Language = Choice(read, FacetKeys.Language), - CodebaseFamily = string.IsNullOrWhiteSpace(codebaseFamily) ? null : codebaseFamily.Trim(), + CodebaseFamily = string.IsNullOrWhiteSpace(codebaseFamily) + ? null + : FacetChoice.Parse(codebaseFamily.Trim()), }; result = new GameQuery( diff --git a/src/MUI.Web/Components/FacetPanel.razor b/src/MUI.Web/Components/FacetPanel.razor index 2115076..b9bff4c 100644 --- a/src/MUI.Web/Components/FacetPanel.razor +++ b/src/MUI.Web/Components/FacetPanel.razor @@ -38,15 +38,36 @@ @FacetWords.Group(group.Key) @FacetWords.Evidence(group.Evidence) + @* + Every value appears twice — once to filter in, once to filter out — which + is the only way to offer the third state in a plain GET form with no + script. A facet has three states (spec §9 and FacetChoice): not asked + about, only these, anything but these. Two s rather than a + second control, so one @* Always first, always empty: a facet you cannot un-choose is a trap. *@ - @foreach (var value in group.Values) - { - - } + + @foreach (var value in group.Values) + { + + } + + + @foreach (var value in group.Values) + { + + } + } @@ -88,6 +109,17 @@ @code { + /// + /// The same value, spelled as an exclusion — Evennia becomes !Evennia. + /// + /// + /// Built through rather than by prefixing a string here, so the + /// panel's spelling and the binding's parser cannot drift: a value that itself begins with the + /// marker is escaped by the same code that unescapes it. + /// + private static FacetChoice Excluded(FacetValue value) => + (value.IsUnknown ? FacetChoice.Unknown : FacetChoice.Of(value.Token)) with { Exclude = true }; + [Parameter, EditorRequired] public IReadOnlyList Facets { get; set; } = []; [Parameter, EditorRequired] public GameFilter Filter { get; set; } = new(); diff --git a/src/MUI.Web/Components/Pages/Games.razor b/src/MUI.Web/Components/Pages/Games.razor index 9689e31..aa7096b 100644 --- a/src/MUI.Web/Components/Pages/Games.razor +++ b/src/MUI.Web/Components/Pages/Games.razor @@ -30,14 +30,14 @@ else { - @if (Filter.CodebaseFamily is { } family) + @if (Filter.CodebaseFamily is { Exclude: false, Value: { } family }) { @* The filter a reference page links in on. It is a filter and not a search, so it says which family it is showing and offers the way back out — a reader who arrived from /reference/codebases/pennmush should not have to guess why the listing is short. *@

codebase @family · - what this codebase is · + what this codebase is · every game

} diff --git a/src/MUI.Web/Reference/ReferenceFigures.cs b/src/MUI.Web/Reference/ReferenceFigures.cs index 6de4067..ccfdf04 100644 --- a/src/MUI.Web/Reference/ReferenceFigures.cs +++ b/src/MUI.Web/Reference/ReferenceFigures.cs @@ -33,7 +33,7 @@ public static async Task ReadAsync( ArgumentNullException.ThrowIfNull(queries); var games = await queries.ListAsync( - new GameFilter { CodebaseFamily = family, IncludeArchived = true }, + new GameFilter { CodebaseFamily = FacetChoice.Of(family), IncludeArchived = true }, cancellationToken); return new CodebaseFigures( diff --git a/tests/MUI.Catalog.Tests/FacetPolarityTests.cs b/tests/MUI.Catalog.Tests/FacetPolarityTests.cs new file mode 100644 index 0000000..4426bcc --- /dev/null +++ b/tests/MUI.Catalog.Tests/FacetPolarityTests.cs @@ -0,0 +1,163 @@ +using MUI.Catalog; + +namespace MUI.Catalog.Tests; + +/// +/// The third state a facet can be in: filtered out (spec §9). +/// +/// +/// Absent means the facet is not being asked about, a value means only these, and an +/// excluded value means anything but these. Without the third, "show me the games that are +/// not Evennia" is a question the catalogue can answer and the interface cannot ask. +/// +public class FacetPolarityTests +{ + private static readonly GameSummary Penn = Game("penn", "PennMUSH 1.8.8p0"); + private static readonly GameSummary Evennia = Game("evennia", "Evennia"); + private static readonly GameSummary Rom = Game("rom", "ROM"); + private static readonly GameSummary Nameless = Game("nameless", null); + + [Test] + public async Task ExcludingAValueReturnsEverythingElse() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "penn", "rom", "nameless" }); + } + + [Test] + public async Task IncludingAValueStillReturnsOnlyIt() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Of("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)).IsEquivalentTo(new[] { "evennia" }); + } + + [Test] + public async Task NoSelectionFiltersOnNothing() + { + await Assert.That(Search(new GameFilter()).Games.Count).IsEqualTo(4); + } + + /// + /// A game with no value for the facet survives an exclusion, because it is not the thing excluded. + /// + /// + /// The tempting bug is to treat "not Evennia" as "has a codebase, and it is not Evennia", which + /// would quietly drop every game whose codebase we never identified — turning our own gap in + /// measurement into a property of those games. Not identifying a codebase is a measurement, and + /// it is not a measurement of being Evennia. + /// + [Test] + public async Task AGameWithNoValueSurvivesAnExclusion() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)).Contains("nameless"); + } + + /// Excluding the unknown is its own question, and answers it. + [Test] + public async Task TheUnknownCanItselfBeExcluded() + { + var listing = Search(new GameFilter + { + Codebase = FacetChoice.Unknown with { Exclude = true }, + }); + + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "penn", "evennia", "rom" }); + } + + /// The family filter is a bounded prefix, and it inverts the same way. + [Test] + public async Task ExcludingACodebaseFamilyTakesEveryPatchlevelWithIt() + { + var listing = Search(new GameFilter { CodebaseFamily = FacetChoice.Not("PennMUSH") }); + + // PennMUSH 1.8.8p0 goes with the family it belongs to; ROM stays, and is not caught by a + // prefix that would have gathered ROMulus. + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "evennia", "rom", "nameless" }); + } + + /// A token round-trips through the URL with its polarity intact. + [Test] + public async Task PolaritySurvivesTheQuerystring() + { + foreach (var choice in new[] + { + FacetChoice.Of("Evennia"), + FacetChoice.Not("Evennia"), + FacetChoice.Unknown, + FacetChoice.Unknown with { Exclude = true }, + }) + { + await Assert.That(FacetChoice.Parse(choice.Token)).IsEqualTo(choice); + } + } + + /// + /// A value that begins with the marker stays reachable rather than reading as its own negation. + /// + [Test] + public async Task AValueBeginningWithTheMarkerIsNotMistakenForAnExclusion() + { + var literal = FacetChoice.Of("!important"); + + await Assert.That(literal.Token).IsEqualTo("!!important"); + await Assert.That(FacetChoice.Parse(literal.Token)).IsEqualTo(literal); + await Assert.That(FacetChoice.Parse(literal.Token).Exclude).IsFalse(); + } + + /// + /// Polarity is applied to the answer, never per token. + /// + /// + /// A facet can hand one row several tokens — a game reached an hour ago is in the last day, the + /// last week and the last month. Inverting the comparison per token would make an excluded + /// selection mean "some token differs", which every multi-token row satisfies, so "not seen in + /// the last day" would return everything. + /// + [Test] + public async Task AMultiTokenFacetInvertsOnceRatherThanPerToken() + { + var choice = FacetChoice.Not("day"); + + await Assert.That(choice.Admits(covered: true)).IsFalse(); + await Assert.That(choice.Admits(covered: false)).IsTrue(); + await Assert.That(FacetChoice.Of("day").Admits(covered: true)).IsTrue(); + } + + /// The panel can tell the three states apart, which is what lets it draw them apart. + [Test] + public async Task AFacetValueReportsWhichOfTheThreeStatesItIsIn() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + var codebase = listing.Facets.Single(f => f.Key == FacetKeys.Codebase); + + await Assert.That(codebase.Values.Single(v => v.Token == "Evennia").State) + .IsEqualTo(FacetState.Excluded); + await Assert.That(codebase.Values.Single(v => v.Token == "ROM").State) + .IsEqualTo(FacetState.Unselected); + } + + private static GameListing Search(GameFilter filter) => + FacetedSearch.Search([.. new[] { Penn, Evennia, Rom, Nameless }.Select(Row)], filter); + + private static GameFacetRow Row(GameSummary game) => new( + game, + ActivityBand.Quiet, + LastSeenBand.Week, + TlsMeasured: false, + Charset: null, + Language: null, + Codebase: game.Codebase, + Family: null, + Genre: null); + + private static GameSummary Game(string slug, string? codebase) => new( + Guid.NewGuid(), slug, slug, null, LifecycleState.Active, IsClaimed: false, + PlayersNow: 1, Codebase: codebase, MeasuredProtocols: []); +} diff --git a/tests/MUI.Web.Tests/ReferenceFiguresTests.cs b/tests/MUI.Web.Tests/ReferenceFiguresTests.cs index 8a12a3e..ad31192 100644 --- a/tests/MUI.Web.Tests/ReferenceFiguresTests.cs +++ b/tests/MUI.Web.Tests/ReferenceFiguresTests.cs @@ -121,7 +121,7 @@ public async Task TheCodebaseLinkAndTheCountAreOneFilter() var queries = new FixtureGameQueries(); var figures = await CodebaseFigures.ReadAsync(queries, page.Codebase!); - var listing = await queries.ListAsync(new GameFilter { CodebaseFamily = page.Codebase }); + var listing = await queries.ListAsync(new GameFilter { CodebaseFamily = FacetChoice.Of(page.Codebase!) }); await Assert.That(figures.Listed).IsEqualTo(listing.Count); await Assert.That(page.GamesPath).IsEqualTo("/games?codebase-family=Evennia"); @@ -144,7 +144,8 @@ public Task> ListAsync( [ .. games .Where(g => filter.IncludeArchived || g.State is not LifecycleState.Archived) - .Where(g => CodebaseFamily.Matches(g.Codebase, filter.CodebaseFamily)), + .Where(g => filter.CodebaseFamily is not { } family + || family.Admits(CodebaseFamily.Matches(g.Codebase, family.Value))), ]); public Task FindAsync(string slug, CancellationToken cancellationToken = default) =>