From fd70f3e38dac779c3eeaaff15b990a038b3136ef Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 11:24:05 -0500 Subject: [PATCH 1/2] A long connect screen cost a game its listing, for ever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the first crawl large enough to find it: seeding 701 addresses from the directories, three games of four hundred died with 54000: index row size 3048 exceeds btree version 4 maximum 2704 for index "game_field_field_value_idx" PostgreSQL cannot index a btree row past about 2704 bytes, and a connect screen is routinely thousands of characters -- the longest in this catalogue is 9,376. The failure is total rather than partial: the INSERT is refused, so the whole probe's ingestion is lost for that game, and it is lost again on every future probe. A game with a generous piece of ASCII art was permanently unlistable and nothing said so. WHY THREE AND NOT FIFTY-ONE, WHICH IS HOW MANY ROWS ARE NOW OVER THAT SIZE. Index tuples cannot be stored out of line but they can be compressed, so whether a game was listable depended on how well its ASCII art compressed. That is why it presented as three unrelated failures rather than as a rule, and it is why looking at the length of the longest value would not have predicted it. Both indexes over game_field had the flaw, and the first fix caught only one -- the very next probe failed on the other. They are bounded differently because they are read differently: - game_field_field_value_idx serves §9's faceted search and is indexed on a 256-character prefix. Nothing has ever searched by connect screen and nothing will; it is a display asset, and its fingerprint has its own column. - game_field_folded_value_idx serves §7.3's identity lookup, which asks an equality question. A prefix there would silently turn that into a starts-with -- over-matching where the raw index merely refused -- so it is partial instead, and CatalogueDirectories carries the same predicate or the planner cannot use it. Every identity signal §7.3 names is short: a name, a year, a hostname, a hash, a token. A value longer than that is not one. THE STORED VALUE IS UNTOUCHED. Truncating what a game sent in order to fit our own index would be the quiet lossiness this schema refuses everywhere else. It is the index that is bounded, never the fact. The test asserts the property over EVERY index on game_field rather than over the one that was noticed, which is the mistake the first pass made. Verified against the live catalogue: the games that had been failing re-probed clean, and the one that raised the original error now holds its 7,535-character connect screen. Co-Authored-By: Claude Opus 5 --- migrations/0008_bounded_field_value_index.sql | 37 +++++ .../Persistence/CatalogueDirectories.cs | 8 ++ .../Persistence/OversizedFieldValueTests.cs | 127 ++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 migrations/0008_bounded_field_value_index.sql create mode 100644 tests/MUI.Catalog.Tests/Persistence/OversizedFieldValueTests.cs diff --git a/migrations/0008_bounded_field_value_index.sql b/migrations/0008_bounded_field_value_index.sql new file mode 100644 index 0000000..a12d538 --- /dev/null +++ b/migrations/0008_bounded_field_value_index.sql @@ -0,0 +1,37 @@ +-- The (field, value) index could not hold a connect screen, and a game whose screen was long enough +-- failed to ingest at all. +-- +-- Observed on the first crawl large enough to find it: three of four hundred games died with +-- "54000: index row size 3048 exceeds btree version 4 maximum 2704 for index +-- game_field_field_value_idx". PostgreSQL's btree cannot index a row wider than about 2704 bytes, +-- and a connect screen is routinely thousands of characters — the longest in this catalogue is 9,376. +-- The failure is not partial: the INSERT is refused, so the whole probe's ingestion is lost for that +-- game, and it is lost again on every future probe, for ever. A game with a generous piece of ASCII +-- art was permanently unlistable. +-- +-- The index's stated purpose (0002) is §9's faceted search: which games have CODEBASE = PennMUSH, or +-- capability.gmcp.measured = true. Every value that purpose looks up is short. NOTHING HAS EVER +-- SEARCHED BY CONNECT SCREEN and nothing ever will — it is a display asset and a fingerprint, and +-- the fingerprint has its own column. So the index covers a bounded prefix, which serves the lookups +-- it was built for and cannot overflow: 256 characters is under the limit even at four bytes each. +-- +-- The stored value is untouched. Truncating what a game said in order to fit our own index would be +-- exactly the kind of quiet lossiness this schema refuses everywhere else; it is the *index* that is +-- bounded, not the fact. +DROP INDEX IF EXISTS game_field_field_value_idx; + +CREATE INDEX game_field_field_value_idx ON game_field (field, left(value, 256)); + +-- The same flaw, one index over: §7.3's identity lookup folds case and whitespace on both columns, +-- and folding does not shorten a connect screen. This one is partial rather than prefixed, because +-- its reader asks an equality question and a prefix would silently turn that into a +-- starts-with — over-matching where the raw index merely refused. Every identity signal §7.3 names +-- is short: a name, a year, a hostname, a hash, a token. A value longer than this is not one of +-- them, so excluding it from the lookup changes no correct answer. +-- +-- CatalogueDirectories carries the same predicate, or the planner cannot use a partial index. +DROP INDEX IF EXISTS game_field_folded_value_idx; + +CREATE INDEX game_field_folded_value_idx + ON game_field (lower(btrim(field)), lower(btrim(value))) + WHERE length(value) <= 256; diff --git a/src/MUI.Crawler/Persistence/CatalogueDirectories.cs b/src/MUI.Crawler/Persistence/CatalogueDirectories.cs index d668463..aaa69c5 100644 --- a/src/MUI.Crawler/Persistence/CatalogueDirectories.cs +++ b/src/MUI.Crawler/Persistence/CatalogueDirectories.cs @@ -101,6 +101,14 @@ SELECT DISTINCT game_id FROM game_field WHERE lower(btrim(field)) = lower(btrim(@field)) AND lower(btrim(value)) = lower(btrim(@value)) + -- The bound is here as well as on the index, and both are deliberate. PostgreSQL's + -- btree cannot hold a row past ~2704 bytes, and a connect screen is thousands of + -- characters, so an unbounded index refused the INSERT and cost the game its whole + -- ingestion. The index is now partial; repeating its predicate here is what lets the + -- planner use it rather than sequentially scanning game_field once per identity + -- signal per probe. It changes no answer: every §7.3 signal — a name, a year, a + -- hostname, a hash, a token — is short, and a value longer than this is not one. + AND length(value) <= 256 """, new { field, value }, cancellationToken: ct)); diff --git a/tests/MUI.Catalog.Tests/Persistence/OversizedFieldValueTests.cs b/tests/MUI.Catalog.Tests/Persistence/OversizedFieldValueTests.cs new file mode 100644 index 0000000..d0fb497 --- /dev/null +++ b/tests/MUI.Catalog.Tests/Persistence/OversizedFieldValueTests.cs @@ -0,0 +1,127 @@ +using Dapper; + +using MUI.Catalog.Persistence; +using MUI.Catalog.Tests.Persistence.Support; + +namespace MUI.Catalog.Tests.Persistence; + +/// +/// A field value too large for an index must not cost a game its listing. +/// +/// +/// Found on the first crawl big enough to find it: three games of four hundred died with +/// 54000: index row size … exceeds btree version 4 maximum 2704, because a connect screen is +/// routinely thousands of characters and the (field, value) index tried to hold one. The +/// failure is total rather than partial — the insert is refused, the whole probe's ingestion is lost, +/// and it is lost again on every future probe. A game with a generous piece of ASCII art was +/// permanently unlistable, and nothing said so. +/// +public class OversizedFieldValueTests +{ + private static readonly DateTimeOffset Now = Seed.Now; + + /// The real shape: a connect screen far past the btree limit, stored whole. + [Test] + public async Task AConnectScreenTooLargeToIndexIsStillStored() + { + await using var db = await PostgresFixture.MigratedAsync(); + var game = await Seed.GameAsync(db); + var store = new NpgsqlGameFieldStore(db.DataSource); + + // Longer than the longest observed in the wild (9,376 characters) and several times the + // index limit, so this fails against the old index and passes against the bounded one. + var screen = string.Join('\n', Enumerable.Repeat(new string('=', 78), 160)); + + await store.UpsertAsync(new GameField( + game, InternalFields.ConnectScreen, FieldSource.Banner, screen, Now, Now)); + + var stored = (await store.ForGameAsync(game)) + .Single(f => f.Field == InternalFields.ConnectScreen); + + // Stored whole. It is the index that is bounded, never the fact — truncating what a game + // sent in order to fit our own index is the kind of quiet lossiness this schema refuses. + await Assert.That(stored.Value).IsEqualTo(screen); + await Assert.That(stored.Value.Length).IsGreaterThan(2704); + } + + /// + /// Two long values that differ only past the indexed prefix are still two distinct rows. + /// + /// + /// The prefix is an index, not a key. If bounding it had collapsed rows that share their first + /// 256 characters — which two connect screens from one codebase easily do — the fix would have + /// traded a loud failure for a silent one. + /// + [Test] + public async Task TwoValuesSharingTheirFirstBytesRemainDistinct() + { + await using var db = await PostgresFixture.MigratedAsync(); + var one = await Seed.GameAsync(db, slug: "one", name: "One"); + var two = await Seed.GameAsync(db, slug: "two", name: "Two"); + var store = new NpgsqlGameFieldStore(db.DataSource); + + var shared = new string('#', 4000); + + await store.UpsertAsync(new GameField( + one, InternalFields.ConnectScreen, FieldSource.Banner, shared + "ONE", Now, Now)); + await store.UpsertAsync(new GameField( + two, InternalFields.ConnectScreen, FieldSource.Banner, shared + "TWO", Now, Now)); + + var first = (await store.ForGameAsync(one)).Single(f => f.Field == InternalFields.ConnectScreen); + var second = (await store.ForGameAsync(two)).Single(f => f.Field == InternalFields.ConnectScreen); + + await Assert.That(first.Value).EndsWith("ONE"); + await Assert.That(second.Value).EndsWith("TWO"); + } + + /// + /// Both indexes over this table are bounded, because both could refuse a connect screen. + /// + /// + /// The first fix caught only one of them and the very next probe failed on the other — so this + /// asserts the property over every index on game_field rather than over the one that was + /// noticed. An index on a raw or merely case-folded value is the shape of the bug: folding does + /// not shorten anything. + /// + [Test] + public async Task NoIndexOnThisTableCanRefuseALongValue() + { + await using var db = await PostgresFixture.MigratedAsync(); + + await using var connection = await db.DataSource.OpenConnectionAsync(); + + var definitions = (await connection.QueryAsync( + "SELECT indexdef FROM pg_indexes WHERE tablename = 'game_field'")).ToList(); + + foreach (var definition in definitions.Where(d => d.Contains("value", StringComparison.Ordinal))) + { + // Either the indexed expression is bounded, or the index only covers rows short enough. + var bounded = definition.Contains("256", StringComparison.Ordinal); + + await Assert.That(bounded) + .IsTrue() + .Because($"an unbounded index over `value` refuses a connect screen: {definition}"); + } + } + + /// The index still exists and still leads on the field, which is what it is for. + [Test] + public async Task TheFacetLookupIsStillIndexed() + { + await using var db = await PostgresFixture.MigratedAsync(); + + await using var connection = await db.DataSource.OpenConnectionAsync(); + + var definition = await connection.ExecuteScalarAsync( + "SELECT indexdef FROM pg_indexes WHERE indexname = 'game_field_field_value_idx'"); + + await Assert.That(definition).IsNotNull(); + await Assert.That(definition!).Contains("field"); + + // Asserted on the bound rather than the spelling: PostgreSQL reports the expression back as + // "left"(value, 256), quoted, and a test that matched the source text would break on a + // formatting difference while saying nothing about whether the index can overflow. + await Assert.That(definition!).Contains("256"); + await Assert.That(definition!.Contains("(field, value)", StringComparison.Ordinal)).IsFalse(); + } +} From 172f2294a6c6bdf619cb6b1951664b871c0b31a7 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 11:33:48 -0500 Subject: [PATCH 2/2] Storage for a submitted game, which stays hidden until it is claimed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Half the feature: the schema and the reads. The form and the scope gate follow. A signed-in operator may add their own game, and it stays off the listing until a claim proves they run it. That is ONE NULLABLE COLUMN and one sentence: a game is public if nobody submitted it, or if it has been claimed. game.submitted_by is not a lifecycle state and could not be. `state` is derived from availability history and never set by hand, and this is a different axis entirely -- how a game reached us rather than how it has been behaving. Two axes in one column would leave a submitted game that goes dark unable to say both things. WHY THE ASYMMETRY WITH §7.1's AUTO-LISTING IS NOT ARBITRARY. Discovery is something we did: a referral we walked and a resolved-address gate we applied. A submission is a stranger pointing us at a host. If it listed on sight, the form would be a way to put any address on a public page under any description, and the answer to "who says this is a game" would be "somebody". Requiring the claim makes the form mean add YOUR game rather than add A game. AND IT IS NOT THE MODERATION QUEUE §3 CONDEMNS. The incumbents' queues waited on a human at their end, which is why listings sat unapproved for a year. This waits on the submitter, settles in one probe, and has nobody in the middle. The migration says so, because the day somebody adds a staff approval screen the distinction is gone and so is the argument for the feature. The rule is one constant applied to all seven public reads. Writing the predicate per query is how the eighth query gets added without it, and the failure mode is a game on a public page nobody vouched for. FOUR OF THE SIX SQL LITERALS WERE NOT INTERPOLATED, so `{Public}` would have gone to PostgreSQL as text -- it compiled cleanly and would have failed at runtime, which is exactly why the suite runs against a real database rather than a fake. The guard is a test that walks IGameQueries by reflection and requires every member to be named as considered, so a query added later fails until somebody decides whether a submitted game can reach it. Alongside it, the case that would have been catastrophic to get backwards: an unclaimed game the crawler found is still listed. Reading the column as "unclaimed games are hidden" rather than "unclaimed SUBMISSIONS are hidden" would have emptied the catalogue -- all 409 games in it are unclaimed. Co-Authored-By: Claude Opus 5 --- migrations/0010_submitted_games.sql | 48 ++++++ .../Persistence/NpgsqlGameQueries.cs | 44 +++-- .../Persistence/SubmittedGameTests.cs | 155 ++++++++++++++++++ 3 files changed, 237 insertions(+), 10 deletions(-) create mode 100644 migrations/0010_submitted_games.sql create mode 100644 tests/MUI.Catalog.Tests/Persistence/SubmittedGameTests.cs diff --git a/migrations/0010_submitted_games.sql b/migrations/0010_submitted_games.sql new file mode 100644 index 0000000..7dfed18 --- /dev/null +++ b/migrations/0010_submitted_games.sql @@ -0,0 +1,48 @@ +-- spec §8 — a signed-in operator may add their own game, and it stays out of the listing until they +-- have proved they run it. +-- +-- ONE NULLABLE COLUMN, AND IT IS NOT A LIFECYCLE STATE. `game.state` is derived from availability +-- history and never set by hand (0001), so a submission cannot live there: it is a different axis +-- entirely — how a game reached us, rather than how it has been behaving. Two axes in one column +-- would mean a submitted game that goes dark has nowhere to say both things. +-- +-- The listing rule is one sentence and reads off this column alone: +-- +-- a game is public if nobody submitted it, or if it has been claimed. +-- +-- Which keeps §7.1's auto-listing exactly as it was — anything the crawler found for itself is +-- listed immediately as discovered-and-unclaimed — while a stranger's assertion that some address is +-- a game waits for that stranger to prove they run it. +-- +-- WHY THE ASYMMETRY IS NOT ARBITRARY. Discovery is something we did: a referral we walked and a +-- resolved-address gate we applied (§7.2). A submission is somebody else pointing us at a host. If a +-- submitted game listed on sight, the form would be a way to put any address on a public page under +-- any description, and the answer to "who says this is a game?" would be "a stranger". Requiring the +-- claim makes the form mean *add your own game*, not *add somebody's*. +-- +-- THIS IS NOT THE MODERATION QUEUE §3 CONDEMNS, AND THE DIFFERENCE MUST BE KEPT. The incumbents' +-- queues waited on a human at their end, which is why listings sat unapproved for a year. This waits +-- on the submitter, is settled by one probe, and has nobody in the middle. If a screen is ever added +-- where staff approve submissions, that distinction is gone and so is the argument for the feature. +ALTER TABLE game + ADD COLUMN submitted_by uuid REFERENCES app_user (id); + +-- The listing filters on this on every read, and the overwhelming majority of rows are NULL, so it +-- is worth an index only over the rows that are not. +CREATE INDEX game_submitted_by_idx ON game (submitted_by) WHERE submitted_by IS NOT NULL; + +-- How many submissions one account may make in a day (§8). Recorded as a table rather than a +-- column so the bound is auditable: a burst of submissions from one account is a thing somebody +-- will want to look at, and a counter that only counts would not say what happened. +CREATE TABLE game_submission ( + id uuid PRIMARY KEY, + user_id uuid NOT NULL REFERENCES app_user (id), + game_id uuid NOT NULL REFERENCES game (id), + host text NOT NULL, + port integer NOT NULL, + submitted_at timestamptz NOT NULL, + + CONSTRAINT game_submission_port_is_a_port CHECK (port >= 1 AND port <= 65535) +); + +CREATE INDEX game_submission_user_idx ON game_submission (user_id, submitted_at DESC); diff --git a/src/MUI.Catalog/Persistence/NpgsqlGameQueries.cs b/src/MUI.Catalog/Persistence/NpgsqlGameQueries.cs index 59a1ab9..5ce420f 100644 --- a/src/MUI.Catalog/Persistence/NpgsqlGameQueries.cs +++ b/src/MUI.Catalog/Persistence/NpgsqlGameQueries.cs @@ -31,6 +31,29 @@ namespace MUI.Catalog.Persistence; public sealed class NpgsqlGameQueries(NpgsqlDataSource source, IFieldRegistry? registry = null) : IGameQueries { + /// + /// The rule that keeps an unclaimed submission off every public surface (spec §8, migration 0010). + /// + /// + /// + /// A game is public if nobody submitted it, or if it has been claimed. Anything the + /// crawler found for itself is listed on sight exactly as §7.1 says; a stranger's assertion that + /// some address is a game waits until that stranger proves they run it. + /// + /// + /// It is one constant because it has to hold on every read — the listing, the search, + /// the three feeds, the ecosystem shares, the rankings, and both lookups. A predicate written + /// out per query is a predicate that will be forgotten on the next query somebody adds, and the + /// failure mode is a game on a public page that nobody vouched for. The test that enumerates + /// IGameQueries and asserts an unclaimed submission appears in none of it is the real + /// guard; this constant is what makes passing it easy. + /// + /// + private const string Public = "(submitted_by IS NULL OR is_claimed)"; + + /// The same rule where the table is aliased. + private const string PublicG = "(g.submitted_by IS NULL OR g.is_claimed)"; + /// The heatmap's window (spec §5.2). public static readonly TimeSpan ActivityWindow = TimeSpan.FromDays(56); @@ -119,11 +142,12 @@ public async Task SearchAsync( var includeArchived = filter.IncludeArchived || filter.Band is ActivityBand.Archived; var rows = (await connection.QueryAsync(new CommandDefinition( - """ + $""" SELECT g.id AS Id, g.slug AS Slug, g.name AS Name, g.tagline AS Tagline, g.state AS State, g.is_claimed AS IsClaimed, g.last_reachable_at AS LastReachableAt FROM game g WHERE (@includeArchived OR g.state <> 'archived') + AND {PublicG} ORDER BY g.name """, new { includeArchived }, @@ -229,11 +253,11 @@ FROM game_endpoint await using var connection = await source.OpenConnectionAsync(cancellationToken); var row = await connection.QuerySingleOrDefaultAsync(new CommandDefinition( - """ + $""" SELECT id AS Id, slug AS Slug, name AS Name, tagline AS Tagline, state AS State, is_claimed AS IsClaimed, last_reachable_at AS LastReachableAt FROM game - WHERE id = @id + WHERE id = @id AND {Public} """, new { id }, cancellationToken: cancellationToken)); @@ -268,11 +292,11 @@ FROM game await using var connection = await source.OpenConnectionAsync(cancellationToken); var row = await connection.QuerySingleOrDefaultAsync(new CommandDefinition( - """ + $""" SELECT id AS Id, slug AS Slug, name AS Name, tagline AS Tagline, state AS State, is_claimed AS IsClaimed, last_reachable_at AS LastReachableAt FROM game - WHERE slug = @slug + WHERE slug = @slug AND {Public} """, new { slug }, cancellationToken: cancellationToken)); @@ -333,10 +357,10 @@ public async Task FeedsAsync(CancellationToken cancellationToken // §9's three liveness feeds — the differentiator no incumbent can publish, because none of // them measured continuously enough to know when a game came back. var discovered = await connection.QueryAsync(new CommandDefinition( - """ + $""" SELECT slug AS Slug, name AS Name, first_seen_at AS At, NULL AS Cause FROM game - WHERE first_seen_at >= @since + WHERE first_seen_at >= @since AND {Public} ORDER BY first_seen_at DESC LIMIT @limit """, @@ -411,9 +435,9 @@ public async Task EcosystemAsync(CancellationToken cancellat await using var connection = await source.OpenConnectionAsync(cancellationToken); var totals = await connection.QuerySingleAsync(new CommandDefinition( - """ + $""" SELECT - (SELECT count(*)::int FROM game WHERE state <> 'archived') AS Listed, + (SELECT count(*)::int FROM game WHERE state <> 'archived' AND {Public}) AS Listed, -- A completed session, which is what a measured capability is a capability of. (SELECT count(DISTINCT a.game_id)::int @@ -507,7 +531,7 @@ public async Task RankingsAsync(CancellationToken cancellationToken = await using var connection = await source.OpenConnectionAsync(cancellationToken); var listed = await connection.ExecuteScalarAsync(new CommandDefinition( - "SELECT count(*)::int FROM game WHERE state <> 'archived'", + $"SELECT count(*)::int FROM game WHERE state <> 'archived' AND {Public}", cancellationToken: cancellationToken)); var busiest = (await connection.QueryAsync(new CommandDefinition( diff --git a/tests/MUI.Catalog.Tests/Persistence/SubmittedGameTests.cs b/tests/MUI.Catalog.Tests/Persistence/SubmittedGameTests.cs new file mode 100644 index 0000000..5d6b1dc --- /dev/null +++ b/tests/MUI.Catalog.Tests/Persistence/SubmittedGameTests.cs @@ -0,0 +1,155 @@ +using Dapper; + +using MUI.Catalog.Persistence; +using MUI.Catalog.Tests.Persistence.Support; + +namespace MUI.Catalog.Tests.Persistence; + +/// +/// A submitted game stays off every public surface until it is claimed (spec §8, migration 0010). +/// +/// +/// +/// The rule is one sentence — a game is public if nobody submitted it, or if it has been +/// claimed — and the danger is not the rule, it is the number of places it has to hold. The +/// listing, the faceted search, the three liveness feeds, the ecosystem shares, the rankings, the +/// by-slug lookup and the by-id lookup are seven surfaces, and the failure mode of forgetting one is +/// a game on a public page that nobody vouched for. +/// +/// +/// So the test does not check the surfaces somebody remembered. It walks +/// by reflection and requires every member to be covered here — a method added later fails this +/// until it is either filtered or explicitly declared to need no filtering. +/// +/// +public class SubmittedGameTests +{ + private static readonly DateTimeOffset Now = Seed.Now; + + [Test] + public async Task AnUnclaimedSubmissionIsOnNoPublicSurface() + { + await using var db = await PostgresFixture.MigratedAsync(); + var queries = new NpgsqlGameQueries(db.DataSource); + + var found = await Seed.GameAsync(db, slug: "found", name: "Found By Us"); + var submitted = await SubmittedAsync(db, slug: "submitted", name: "Somebody Said So"); + + var listed = await queries.ListAsync(new GameFilter { IncludeArchived = true }); + var searched = await queries.SearchAsync(new GameFilter { IncludeArchived = true }); + var feeds = await queries.FeedsAsync(); + + await Assert.That(listed.Select(g => g.Id)).Contains(found); + await Assert.That(listed.Select(g => g.Id)).DoesNotContain(submitted); + await Assert.That(searched.Games.Select(g => g.Id)).DoesNotContain(submitted); + + // A submission is not "newly discovered" — nothing discovered it. + await Assert.That(feeds.NewlyDiscovered.Select(e => e.Slug)).DoesNotContain("submitted"); + + // Not reachable by guessing its address either, on either lookup. + await Assert.That(await queries.FindAsync("submitted")).IsNull(); + await Assert.That(await queries.FindByIdAsync(submitted)).IsNull(); + + // And not in the denominator of any published share, which would let its existence be + // inferred from arithmetic even while its page is hidden. + var ecosystem = await queries.EcosystemAsync(); + var everything = await queries.ListAsync(new GameFilter { IncludeArchived = true }); + + await Assert.That(ecosystem.ListedGames).IsEqualTo(everything.Count); + } + + /// Claiming it lists it, by the same rule and with nothing else changed. + [Test] + public async Task ClaimingASubmissionListsIt() + { + await using var db = await PostgresFixture.MigratedAsync(); + var queries = new NpgsqlGameQueries(db.DataSource); + var submitted = await SubmittedAsync(db, slug: "submitted", name: "Somebody Said So"); + + await Assert.That(await queries.FindAsync("submitted")).IsNull(); + + await new NpgsqlGameStore(db.DataSource).SetClaimedAsync(submitted, true); + + await Assert.That(await queries.FindAsync("submitted")).IsNotNull(); + await Assert.That((await queries.ListAsync(new GameFilter())).Select(g => g.Id)) + .Contains(submitted); + } + + /// + /// A game the crawler found for itself is listed on sight, claimed or not. + /// + /// + /// §7.1's auto-listing is the feature this must not break. If the new column had been read as + /// "unclaimed games are hidden" rather than "unclaimed submissions are hidden", the + /// whole catalogue would have vanished — 409 of 409 games here are unclaimed. + /// + [Test] + public async Task AnUnclaimedGameTheCrawlerFoundIsStillListed() + { + await using var db = await PostgresFixture.MigratedAsync(); + var queries = new NpgsqlGameQueries(db.DataSource); + var found = await Seed.GameAsync(db, slug: "found", name: "Found By Us"); + + await Assert.That((await queries.ListAsync(new GameFilter())).Select(g => g.Id)) + .Contains(found); + await Assert.That(await queries.FindAsync("found")).IsNotNull(); + } + + /// + /// Every read on is covered above, by name. + /// + /// + /// The list is written out so that adding a query fails this test rather than silently shipping + /// an eighth surface nobody filtered. If a new member genuinely needs no filtering, add it here + /// with the reason — the point is that the decision is made, not that the list is long. + /// + [Test] + public async Task EveryQueryOnTheInterfaceHasBeenConsidered() + { + var covered = new HashSet(StringComparer.Ordinal) + { + nameof(IGameQueries.ListAsync), + nameof(IGameQueries.SearchAsync), + nameof(IGameQueries.FeedsAsync), + nameof(IGameQueries.FindAsync), + nameof(IGameQueries.FindByIdAsync), + nameof(IGameQueries.EcosystemAsync), + + // Ranks over presence samples of listed games; it reaches game rows through the same + // filtered path and has no lookup of its own. + nameof(IGameQueries.RankingsAsync), + }; + + var declared = typeof(IGameQueries).GetMethods().Select(m => m.Name).ToHashSet(StringComparer.Ordinal); + + foreach (var member in declared) + { + await Assert.That(covered.Contains(member)) + .IsTrue() + .Because($"{member} is a public read and nothing here says whether a submitted game " + + "can reach it"); + } + } + + /// A game submitted by an account, unclaimed, exactly as the web form would make it. + private static async Task SubmittedAsync(TestDatabase db, string slug, string name) + { + var id = await Seed.GameAsync(db, slug: slug, name: name); + var user = Guid.CreateVersion7(); + + await using var connection = await db.DataSource.OpenConnectionAsync(); + + await connection.ExecuteAsync( + """ + INSERT INTO app_user (id, display_name, normalised_name, security_stamp, + concurrency_stamp, created_at) + VALUES (@user, 'submitter', 'SUBMITTER', @stamp, @stamp, @now) + """, + new { user, stamp = Guid.NewGuid().ToString(), now = Now }); + + await connection.ExecuteAsync( + "UPDATE game SET submitted_by = @user WHERE id = @id", new { user, id }); + + return id; + } +}