From 7edc3cf7f6e1c0c8624c0df0c8d2579878144ca7 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 09:12:34 -0400 Subject: [PATCH] test(search): add behavioral coverage for 0012 legacy-blocklist rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The brownfield migration path — where 0012 DROP + re-ADDs search_tsv on populated production databases still on the pre-#1771 negative blocklist — was exercised by zero tests. The fresh-install (allowlist) path always no-oped via setup(), and migration.rs only string-matched the SQL text. Add two integration tests: - migration_0012_rewrites_legacy_blocklist_to_exclude_drafts: constructs the legacy state (0001–0007 → seed row → 0008 skips → 0009–0011), seeds kind:9 control + kind:31234 draft, applies 0012, and asserts the rewrite took effect: expression contains 31234, GIN index rebuilt, control row searchable, draft row's search_tsv NULL, discriminator correctly classifies the post-rewrite expression as blocklist-shaped. - migration_0012_noops_on_fresh_install_allowlist: verifies the fresh-install expression has the ELSE NULL shape that 0012's discriminator detects. Also applies migrations 0009–0011 in setup() so the test chain matches production exactly (previously omitted). --- crates/buzz-search/tests/fts_integration.rs | 338 ++++++++++++++++++++ 1 file changed, 338 insertions(+) diff --git a/crates/buzz-search/tests/fts_integration.rs b/crates/buzz-search/tests/fts_integration.rs index 55d698abf3..b21a81417a 100644 --- a/crates/buzz-search/tests/fts_integration.rs +++ b/crates/buzz-search/tests/fts_integration.rs @@ -27,6 +27,12 @@ const MIGRATION_0006_SQL: &str = include_str!("../../../migrations/0006_moderati const MIGRATION_0007_SQL: &str = include_str!("../../../migrations/0007_nip_rs_retention.sql"); const MIGRATION_0008_SQL: &str = include_str!("../../../migrations/0008_fresh_install_search_allowlist.sql"); +const MIGRATION_0009_SQL: &str = + include_str!("../../../migrations/0009_nip_rs_database_guards.sql"); +const MIGRATION_0010_SQL: &str = + include_str!("../../../migrations/0010_nip_rs_exact_replay_guard.sql"); +const MIGRATION_0011_SQL: &str = + include_str!("../../../migrations/0011_nip_rs_exact_tag_cardinality.sql"); const MIGRATION_0012_SQL: &str = include_str!("../../../migrations/0012_draft_wrap_fts.sql"); async fn setup() -> (PgPool, String) { @@ -103,6 +109,15 @@ async fn setup() -> (PgPool, String) { pool.execute(MIGRATION_0008_SQL) .await .expect("apply 0008 migration"); + pool.execute(MIGRATION_0009_SQL) + .await + .expect("apply 0009 migration"); + pool.execute(MIGRATION_0010_SQL) + .await + .expect("apply 0010 migration"); + pool.execute(MIGRATION_0011_SQL) + .await + .expect("apply 0011 migration"); pool.execute(MIGRATION_0012_SQL) .await .expect("apply 0012 migration"); @@ -1486,3 +1501,326 @@ async fn p_gated_persistent_kinds_have_storage_null_tsvector() { teardown(pool, &schema).await; } + +/// Set up a schema with the LEGACY blocklist expression (pre-#1771 state). +/// +/// Applies 0001–0007 (establishing the legacy negative-blocklist `search_tsv`), +/// inserts a dummy row so the `events` table is non-empty, then applies 0008 +/// (which skips on non-empty tables) and 0009–0011 (NIP-RS guards, no FTS +/// changes). The resulting schema has the legacy blocklist expression where +/// kind 31234 is NOT excluded — exactly the brownfield state that 0012's +/// rewrite branch must handle. +/// +/// Does NOT apply 0012 — the caller controls that step. +async fn setup_legacy_blocklist() -> (PgPool, String) { + let url = std::env::var("BUZZ_TEST_DATABASE_URL").unwrap_or_else(|_| TEST_DB_URL.to_string()); + let schema = format!("fts_legacy_{}", Uuid::new_v4().simple()); + let admin_pool = PgPoolOptions::new() + .max_connections(1) + .connect(&url) + .await + .expect("connect"); + + sqlx::query("SELECT pg_advisory_lock(3723742987654321)") + .execute(&admin_pool) + .await + .expect("acquire pgcrypto advisory lock"); + sqlx::query("CREATE EXTENSION IF NOT EXISTS pgcrypto") + .execute(&admin_pool) + .await + .expect("install pgcrypto extension"); + sqlx::query("SELECT pg_advisory_unlock(3723742987654321)") + .execute(&admin_pool) + .await + .expect("release pgcrypto advisory lock"); + + let create_sql = format!("CREATE SCHEMA \"{schema}\""); + sqlx::query(sqlx::AssertSqlSafe(create_sql)) + .execute(&admin_pool) + .await + .expect("create schema"); + admin_pool.close().await; + + let url_with_search_path = format!("{url}?options=-c%20search_path%3D{schema}"); + let pool = PgPoolOptions::new() + .max_connections(2) + .connect(&url_with_search_path) + .await + .expect("connect with search_path"); + + // Apply 0001–0007: establishes the legacy negative-blocklist search_tsv. + pool.execute(MIGRATION_0001_SQL) + .await + .expect("apply 0001 migration"); + pool.execute(MIGRATION_0002_SQL) + .await + .expect("apply 0002 migration"); + pool.execute(MIGRATION_0003_SQL) + .await + .expect("apply 0003 migration"); + pool.execute(MIGRATION_0004_SQL) + .await + .expect("apply 0004 migration"); + pool.execute(MIGRATION_0005_SQL) + .await + .expect("apply 0005 migration"); + pool.execute(MIGRATION_0006_SQL) + .await + .expect("apply 0006 migration"); + pool.execute(MIGRATION_0007_SQL) + .await + .expect("apply 0007 migration"); + + // Insert a dummy community + event so the table is non-empty. + // 0008 checks `IF NOT EXISTS (SELECT 1 FROM events LIMIT 1)` and SKIPS + // on non-empty tables, preserving the legacy blocklist expression. + let dummy_community_id = Uuid::new_v4(); + sqlx::query("INSERT INTO communities (id, host, signing_key) VALUES ($1, $2, $3)") + .bind(dummy_community_id) + .bind("legacy-seed.example") + .bind(b"signingkey".as_slice()) + .execute(&pool) + .await + .expect("insert seed community"); + sqlx::query( + "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig) \ + VALUES ($1, $2, $3, to_timestamp($4), $5, '[]'::jsonb, $6, $7)", + ) + .bind(dummy_community_id) + .bind(&rand_bytes32()[..]) + .bind(&rand_bytes32()[..]) + .bind(1_600_000_000_i64) + .bind(9_i32) + .bind("seed event for non-empty table") + .bind(b"signature".as_slice()) + .execute(&pool) + .await + .expect("insert seed event"); + + // Apply 0008: skips because table is non-empty → legacy blocklist preserved. + pool.execute(MIGRATION_0008_SQL) + .await + .expect("apply 0008 migration"); + // Apply 0009–0011: NIP-RS guards, no FTS changes. + pool.execute(MIGRATION_0009_SQL) + .await + .expect("apply 0009 migration"); + pool.execute(MIGRATION_0010_SQL) + .await + .expect("apply 0010 migration"); + pool.execute(MIGRATION_0011_SQL) + .await + .expect("apply 0011 migration"); + + (pool, schema) +} + +/// Behavioral test for migration 0012's legacy-blocklist rewrite branch. +/// +/// This is the operationally risky path: populated production databases still +/// on the pre-#1771 negative blocklist need 0012 to DROP + re-ADD `search_tsv` +/// with kind 31234 added to the exclusion list. The fresh-install (allowlist) +/// path is covered by the existing tests via `setup()`. +/// +/// The test constructs the legacy state, seeds control and draft rows, applies +/// 0012, and asserts: +/// 1. 0012 took the REWRITE branch (the live expression now contains 31234). +/// 2. The GIN index was rebuilt. +/// 3. The kind:9 control row remains searchable via `search_tsv`. +/// 4. The kind:31234 draft row's `search_tsv` IS NULL (storage-level unsearchable). +/// 5. The discriminator correctly classifies both forms: the post-rewrite +/// expression remains blocklist-shaped (not ELSE NULL). +/// +/// Bite-proof: temporarily commenting out the DROP+re-ADD in 0012's legacy +/// branch causes assertions 1 and 4 to fail (31234 not in exclusion list, +/// draft row's `search_tsv` is non-NULL). +#[tokio::test] +#[ignore = "requires Postgres"] +async fn migration_0012_rewrites_legacy_blocklist_to_exclude_drafts() { + let (pool, schema) = setup_legacy_blocklist().await; + + // Verify we have the legacy blocklist expression BEFORE applying 0012. + let pre_expr: String = sqlx::query_scalar( + "SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef \ + WHERE adrelid = 'events'::regclass \ + AND adnum = (SELECT attnum FROM pg_attribute \ + WHERE attrelid = 'events'::regclass AND attname = 'search_tsv')", + ) + .fetch_one(&pool) + .await + .expect("read pre-0012 search_tsv expression"); + + // Legacy blocklist: THEN NULL (not ELSE NULL) — 0012 must NOT see this as allowlist. + assert!( + !pre_expr.contains("ELSE NULL"), + "pre-0012 expression should be legacy blocklist (THEN NULL, not ELSE NULL), got: {pre_expr}" + ); + assert!( + !pre_expr.contains("31234"), + "pre-0012 expression should not already exclude 31234, got: {pre_expr}" + ); + + // Seed test rows: a kind:9 control and a kind:31234 draft. + let community_id: Uuid = + sqlx::query_scalar("SELECT id FROM communities WHERE host = 'legacy-seed.example'") + .fetch_one(&pool) + .await + .expect("fetch seed community"); + let community = CommunityId::from_uuid(community_id); + + let control_id = rand_bytes32(); + let draft_id = rand_bytes32(); + let token = "legacy_rewrite_marker_unique"; + + insert_event( + &pool, + community, + control_id, + rand_bytes32(), + 9, + &format!("public chat — {token}"), + None, + 1_700_000_000, + ) + .await; + insert_event( + &pool, + community, + draft_id, + rand_bytes32(), + KIND_DRAFT as i32, + &format!("draft wrap — {token}"), + None, + 1_700_000_001, + ) + .await; + + // Before 0012: the draft row IS searchable under the legacy blocklist + // (31234 is not in the exclusion list). + let pre_tsv: Option = + sqlx::query_scalar("SELECT search_tsv::text FROM events WHERE id = $1") + .bind(&draft_id[..]) + .fetch_one(&pool) + .await + .expect("read pre-0012 draft search_tsv"); + assert!( + pre_tsv.is_some(), + "before 0012, kind:31234 should have non-NULL search_tsv under legacy blocklist" + ); + + // Apply 0012: should take the rewrite branch. + pool.execute(MIGRATION_0012_SQL) + .await + .expect("apply 0012 migration"); + + // 1. Verify 0012 took the REWRITE branch: expression now contains 31234. + let post_expr: String = sqlx::query_scalar( + "SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef \ + WHERE adrelid = 'events'::regclass \ + AND adnum = (SELECT attnum FROM pg_attribute \ + WHERE attrelid = 'events'::regclass AND attname = 'search_tsv')", + ) + .fetch_one(&pool) + .await + .expect("read post-0012 search_tsv expression"); + assert!( + post_expr.contains("31234"), + "after 0012, legacy-rewrite expression must include 31234, got: {post_expr}" + ); + + // 2. Verify GIN index was rebuilt. + let index_exists: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_indexes WHERE tablename = 'events' \ + AND indexname = 'idx_events_search_tsv')", + ) + .fetch_one(&pool) + .await + .expect("check GIN index existence"); + assert!( + index_exists, + "GIN index idx_events_search_tsv must exist after rewrite" + ); + + // 3. Control row (kind:9) remains searchable. + let svc = SearchService::new(pool.clone()); + let result = svc + .search(&SearchQuery { + community, + q: token.into(), + channel_scope: ChannelScope::Any, + kinds: None, + authors: None, + since: None, + until: None, + page: 1, + per_page: 10, + mode: buzz_search::SearchMode::FullText, + }) + .await + .expect("search ok"); + let kinds: Vec = result.hits.iter().map(|h| h.kind).collect(); + assert!( + kinds.contains(&9), + "kind:9 control row MUST remain searchable after legacy rewrite, got kinds={kinds:?}" + ); + + // 4. Draft row's search_tsv is now NULL (storage-level unsearchable). + let post_tsv: Option = + sqlx::query_scalar("SELECT search_tsv::text FROM events WHERE id = $1") + .bind(&draft_id[..]) + .fetch_one(&pool) + .await + .expect("read post-0012 draft search_tsv"); + assert!( + post_tsv.is_none(), + "after 0012, kind:31234 draft row's search_tsv MUST be NULL, got: {post_tsv:?}" + ); + assert!( + !kinds.contains(&(KIND_DRAFT as i32)), + "kind:31234 MUST NOT appear in search results after legacy rewrite, got kinds={kinds:?}" + ); + + // 5. Verify discriminator correctness: the post-rewrite expression is still + // blocklist-shaped (THEN NULL, not ELSE NULL) — 0012 extended it, didn't + // flip to allowlist. And the allowlist form (from a fresh setup) has ELSE NULL. + assert!( + !post_expr.contains("ELSE NULL"), + "post-rewrite expression should remain blocklist-shaped (not allowlist ELSE NULL), \ + got: {post_expr}" + ); + + teardown(pool, &schema).await; +} + +/// Companion to `migration_0012_rewrites_legacy_blocklist_to_exclude_drafts`: +/// verifies that 0012 correctly no-ops on the fresh-install allowlist form +/// and that the discriminator classifies it as allowlist (ELSE NULL shape). +#[tokio::test] +#[ignore = "requires Postgres"] +async fn migration_0012_noops_on_fresh_install_allowlist() { + let (pool, schema) = setup().await; + + let expr: String = sqlx::query_scalar( + "SELECT pg_get_expr(adbin, adrelid) FROM pg_attrdef \ + WHERE adrelid = 'events'::regclass \ + AND adnum = (SELECT attnum FROM pg_attribute \ + WHERE attrelid = 'events'::regclass AND attname = 'search_tsv')", + ) + .fetch_one(&pool) + .await + .expect("read search_tsv expression"); + + // The allowlist expression (from 0008) must have the ELSE NULL shape + // that 0012's discriminator detects. + assert!( + expr.contains("ELSE NULL"), + "fresh-install expression must contain ELSE NULL (allowlist shape), got: {expr}" + ); + // 31234 should NOT appear in the expression — it's excluded by omission. + assert!( + !expr.contains("31234"), + "fresh-install allowlist should not mention 31234, got: {expr}" + ); + + teardown(pool, &schema).await; +}