diff options
| -rw-r--r-- | docs/refactoring-search.md | 26 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/photos-app.js | 7 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/search-page.js | 56 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/source-merge.js | 160 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_search_source_merge.py | 340 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_search_video_merge.py | 185 |
6 files changed, 762 insertions, 12 deletions
diff --git a/docs/refactoring-search.md b/docs/refactoring-search.md index 08774d1..f23e1ef 100644 --- a/docs/refactoring-search.md +++ b/docs/refactoring-search.md @@ -1,6 +1,9 @@ # Refactor: one file, one entry — merging duplicate sources in the Search view -> Status: **planned, not built** (2026-09-02). Branch `feat/search-source-merge`. +> Status: **phases 1–4 built** (2026-09-02), 5–9 planned. Branch +> `feat/search-source-merge`. The Videos view is merged and the reported bug is +> gone under test; Music, Photos, failover and the badge are still to come, so +> a merged film still shows its chosen group's name rather than `N sources`. > Scope: the cross-group Search page (`static/search-page.js`) and the three > media applications it reuses (Videos, Music, Photos). The Files **explorer** > inside Search is explicitly out of scope and must not change. @@ -328,10 +331,10 @@ Each phase is independently testable and leaves the tree working. | # | Phase | Files | |---|---|---| -| 1 | Export `groupPhotoAlbums`; record `is_node_admin` in `fetchGroupIndex`'s result | `photos-app.js`, `search-page.js` | -| 2 | `source-merge.js` — `pickSource`, `mergeUnitEntries`, `sourceLabel`. No caller yet | new file | -| 3 | Tests for phase 2, read out of the source | `test_search_source_merge.py` | -| 4 | Wire the Videos list through the merge | `search-page.js` | +| 1 ✅ | Export `groupPhotoAlbums`; record `is_node_admin` in `fetchGroupIndex`'s result | `photos-app.js`, `search-page.js` | +| 2 ✅ | `source-merge.js` — `pickSource`, `mergeUnitEntries`, `sourceLabel`. No caller yet | new file | +| 3 ✅ | Tests for phase 2, read out of the source | `test_search_source_merge.py`, `test_search_video_merge.py` | +| 4 ✅ | Wire the Videos list through the merge | `search-page.js` | | 5 | Same for Music, including the `onPreview` queue (§6.4) | `search-page.js` | | 6 | Same for Photos | `search-page.js` | | 7 | `downGroups` and failover | `search-page.js` | @@ -352,11 +355,22 @@ value from the real source rather than restating a constant. | Test | Holds | |---|---| -| `test_search_source_merge.py` | `pickSource` / `mergeUnitEntries` lifted out of `source-merge.js` and executed: a file in two groups yields one entry with two sources; a local source always wins; the pick is stable across calls and varies with the salt; a unit's files share the unit's source; a file the unit's source lacks falls back to its own; a down group is skipped; all-down falls back to the full list | +| `test_search_source_merge.py` ✅ | The whole of `source-merge.js` executed standalone — it has no imports precisely so that it can be, and the test refuses a build where it gains one. A file in two groups yields one entry with two sources; a local source always wins, over every salt; the pick is stable across calls, varies with the salt, and spreads one reader across units; source order does not decide it; a unit's files share the unit's source; a file the unit's source lacks falls back with its siblings; a down group is skipped, a down *local* group yields to a live remote, all-down still returns an entry; no field is back-filled from another source | +| `test_search_video_merge.py` ✅ | The reported symptom end to end. `groupVideoEntries` + `buildSeasons` (video-app.js) and `videoUnits` (search-page.js) are lifted from their real sources, the pipeline is assembled as the page assembles it, and the result is re-grouped the way `VideoApp` re-groups it — so what is counted is what the grid renders. Two groups sharing one library give one film card and one show whose seasons hold three episodes, not six; one group is unchanged; an episode only one group has survives | | extend `test_locales.py` | already fails on a key present in `en.js` and missing elsewhere — no change needed, listed so the ten-catalogue edit is not forgotten | | `test_search_files_unmerged.py` | reads `search-page.js` and refuses a build where `fileEntries` is fed through the merge — the one guarantee §6.1 makes, and the one a later refactor is most likely to break by tidying the four lists into one | | extend `test_transport_contracts.py` | the existing "declared vs. called setters" check covers the new state in `search-page.js` for free | +**Every one of these was checked against the fix removed**, which is where the +first version of "a unit's files share its source" turned out to prove nothing: +with every episode in every group, picking per file and picking per unit give +the same answer — the same key over the same set — so the test passed against a +per-file implementation. It now uses a unit whose files have *unequal* sources, +which is the only shape where the two rules come apart. Five mutations are +caught: dropping the local preference, picking per file, not de-duplicating a +group announcing a file twice, dropping the group-id sort, and hashing the salt +without the unit key. + Beyond the suite, this needs a person: two groups sharing one directory, one film and one multi-season show, checked in Posters, Flat list, the detail modal, Music and Photos, plus one playback and one download from a merged entry. The diff --git a/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js b/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js index d63b384..1755392 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js @@ -373,4 +373,9 @@ function PhotosApp({ `; } -export { PhotosApp }; +// groupPhotoAlbums is exported for the Search page, which needs the album a +// photo belongs to in order to merge duplicate sources per album rather than +// per file (docs/refactoring-search.md §5.2). It calls this one, never a copy: +// a second implementation of the album key would keep agreeing with this one +// right up until one of them changed. +export { PhotosApp, groupPhotoAlbums }; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/search-page.js b/packages/meshbay-hub/src/meshbay_hub/static/search-page.js index ec92e76..5897acb 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/search-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/search-page.js @@ -8,11 +8,12 @@ import { HUB, session, cacheGroupIndex, hubFetch, ensureFreshToken, _loadBundleKey, } from './hub-client.js'; import { FilesPanel, FilePreview } from './files-app.js'; -import { VideoApp } from './video-app.js'; +import { VideoApp, groupVideoEntries } from './video-app.js'; import { MusicApp } from './music-app.js'; import { PhotosApp } from './photos-app.js'; import { VideoPlayer } from './video-player.js'; import { transfers } from './transfers.js'; +import { mergeUnitEntries } from './source-merge.js'; const BATCH_SIZE = 3; // One WebRTC peer connection per group the search view touches. The cap bounds @@ -151,7 +152,13 @@ async function fetchGroupIndex(groupId, token, bundleKey, username, userId) { audioRoot: ack.audio_root || '', photoRoots: ack.photo_roots || [], }; - return { entries: indexMsg.entries || [], roots }; + // Which of the reader's groups sit on their own node — the tie-breaker + // when the same file is announced by several of them + // (docs/refactoring-search.md §5.3). Computed by the node from its own + // record of who it belongs to (webrtc_server.py's _is_node_admin), never + // from a hub claim, and deliberately not written to the index cache: it + // describes this connection, not the group's content. + return { entries: indexMsg.entries || [], roots, isNodeAdmin: !!ack.is_node_admin }; } finally { try { transport.close(); } catch {} } @@ -200,6 +207,32 @@ function underRoot(entry, root) { return p === root || p.startsWith(root + '/'); } +// -- Merging the same file announced by several groups ------------------------ +// +// A directory shared by two groups — the reason two groups exist at all: +// different people invited to different libraries — arrived here as two +// entries per file, so a film showed as two poster cards and every episode +// twice inside a show. `source-merge.js` folds them on the content hash and +// resolves one source per *unit*. See docs/refactoring-search.md. +// +// The units come from video-app.js's own `groupVideoEntries`, never from a +// second copy of its keys here: a copy would keep agreeing with the original +// right up until one of them changed, and the symptom would be a show whose +// episodes stream from two different nodes. Running it twice per recompute (it +// runs again inside VideoApp) is a linear pass over an index already in memory +// and already re-walked on every keystroke of the filter. +// +// One naive unit per copy of a film rather than a pre-grouped one: +// `mergeUnitEntries` folds lists that share a key, so the two copies become +// one unit without this having to group them first. +function videoUnits(entries) { + const { movies, shows } = groupVideoEntries(entries, SEARCH_VIDEO_ROOT); + return [ + ...movies.map((e) => ({ key: `movie:${e.id}`, entries: [e] })), + ...shows.map((s) => ({ key: `show:${s.title}`, entries: s.episodes })), + ]; +} + function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) { const [indexedGroups, setIndexedGroups] = useState(new Map()); const [progress, setProgress] = useState({ done: 0, total: 0, unreachable: [] }); @@ -388,7 +421,20 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) return dirs; }, [indexedGroups]); - // Videos view: pre-filtered by videoRoot, path-prefixed + // How a unit's source is chosen, shared by every merged view + // (docs/refactoring-search.md §5.3). `isLocal` reads the flag the node itself + // put in the handshake ack — computed from its own record of who it belongs + // to (webrtc_server.py's `_is_node_admin`), never from a hub claim. + const mergeOpts = useMemo(() => ({ + salt: userId || '', + isLocal: (gid) => { + const data = indexedGroups.get(gid); + return !!(data && data.isNodeAdmin); + }, + }), [indexedGroups, userId]); + + // Videos view: pre-filtered by videoRoot, path-prefixed, then merged so a + // file several groups share is one card and one list entry. const videoEntries = useMemo(() => { const result = []; for (const [groupId, data] of indexedGroups) { @@ -411,8 +457,8 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) }); } } - return result; - }, [indexedGroups, q, matchesQuery, connectionGen]); + return mergeUnitEntries(videoUnits(result), mergeOpts); + }, [indexedGroups, q, matchesQuery, connectionGen, mergeOpts]); // Music view: pre-filtered by audioRoot, path-prefixed const musicEntries = useMemo(() => { diff --git a/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js b/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js new file mode 100644 index 0000000..ccc4cfd --- /dev/null +++ b/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js @@ -0,0 +1,160 @@ +/** + * One file, one entry — merging the same content announced by several groups. + * + * Only the cross-group Search view needs this. Inside a single group the case + * cannot arise: `GroupIndex` is keyed by blake3, so the same bytes at two + * paths are already one entry. The Search page is what creates duplicates, by + * concatenating N independently keyed indexes into one list — a directory + * shared by two groups then shows every film twice, every episode twice, every + * track twice. See docs/refactoring-search.md. + * + * Two rules decide everything here: + * + * - **Identity is the content hash.** `IndexEntry.id` is blake3 of the file, + * so two entries sharing an id are the same file whatever group announced + * it and whatever path it sits at. + * - **A source is chosen per unit, not per file.** A unit is a film, a whole + * show, a whole album — whatever the reader thinks of as one thing. Picking + * per file would scatter a season's episodes across two nodes, opening two + * connections and two metadata lookups for one show. + * + * No imports, and there must be none: the whole module is executed standalone + * by `test_search_source_merge.py`, which is the only evidence the merge has. + */ + +// FNV-1a, 32 bits. Spreads the source choice deterministically across readers +// — nothing here is security-bearing, and a cryptographic hash would buy +// nothing a multiplication does not. +function stableHash(s) { + let h = 0x811c9dc5; + for (let i = 0; i < s.length; i++) { + h ^= s.charCodeAt(i); + h = Math.imul(h, 0x01000193); + } + return h >>> 0; +} + +function _byGroupId(a, b) { + return String(a.groupId).localeCompare(String(b.groupId)); +} + +function _source(e) { + return { groupId: e.groupId, groupName: e.groupName, groupOwner: e.groupOwner }; +} + +/** + * Which of `sources` serves this unit. + * + * A group hosted by the reader's own node wins outright — for someone browsing + * their own libraries that is the whole of the reported case, and it makes the + * answer predictable where it matters most. Failing that the pick is a hash of + * the unit key and a per-reader salt: stable for one reader across renders and + * reloads (a source that changed mid-stream would tear down the connection + * under a film that is playing), different from one reader to the next, which + * is what spreading the load across sources was asking for. + * + * `isDown` marks a source whose connection has failed. Every source being down + * still returns one: a tile that fails to load is better than a film that + * vanished from the grid. + */ +function pickSource(sources, unitKey, opts) { + if (!sources || !sources.length) return null; + const { salt = '', isLocal, isDown } = opts || {}; + const sorted = [...sources].sort(_byGroupId); + const up = isDown ? sorted.filter((s) => !isDown(s.groupId)) : sorted; + const live = up.length ? up : sorted; + const local = isLocal ? live.filter((s) => isLocal(s.groupId)) : []; + const pool = local.length ? local : live; + return pool[stableHash(`${unitKey}\u0000${salt}`) % pool.length]; +} + +/** + * Merge the entries of each unit by content hash and resolve one source each. + * + * `units` is `[{ key, entries }]`, and the caller builds it by running the + * application's *own* grouping function over the un-merged list — never a + * second copy of that logic, which would keep agreeing right up until one of + * them changed. Two lists carrying the same key are one unit, which is what + * lets a caller emit a naive `{key: 'movie:<id>', entries: [e]}` per copy of a + * film and still get one card. + * + * Every field of a merged entry comes from the chosen source's own entry, and + * nothing is back-filled from another source. A `thumb_hash` or a + * `display_title` only one node computed is only fetchable over *that* node's + * connection, so borrowing it would produce a poster request the chosen + * transport cannot answer — merging two records field by field is the obvious + * thing to write here and it is wrong. + * + * The result carries `_sources`: every group that has the file, sorted by + * group id. That is what the badge counts, and it is the only trace left that + * more than one group was involved. + */ +function mergeUnitEntries(units, opts) { + const byKey = new Map(); + for (const u of units || []) { + if (!byKey.has(u.key)) byKey.set(u.key, []); + byKey.get(u.key).push(...(u.entries || [])); + } + + const merged = []; + for (const [key, entries] of byKey) { + // id -> one entry per group announcing it, first seen wins within a group + const byId = new Map(); + for (const e of entries) { + if (!byId.has(e.id)) byId.set(e.id, []); + const copies = byId.get(e.id); + if (!copies.some((c) => c.groupId === e.groupId)) copies.push(e); + } + + const unitSources = []; + const seen = new Set(); + for (const copies of byId.values()) { + for (const e of copies) { + if (seen.has(e.groupId)) continue; + seen.add(e.groupId); + unitSources.push(_source(e)); + } + } + const unitSource = pickSource(unitSources, key, opts); + + for (const copies of byId.values()) { + const sources = copies.map(_source).sort(_byGroupId); + let chosen = unitSource + && copies.find((e) => e.groupId === unitSource.groupId); + if (!chosen) { + // A file the unit's source does not hold — an episode only one of the + // groups has. Resolved over its own sources by the same rule and the + // same unit key, so every such file in the unit lands on the same + // fallback instead of scattering one per file. + const alt = pickSource(sources, key, opts); + chosen = (alt && copies.find((e) => e.groupId === alt.groupId)) || copies[0]; + } + merged.push({ ...chosen, _sources: sources }); + } + } + return merged; +} + +/** + * What the group badge should say for one entry. + * + * Returns `{ count, name, groupId }` rather than a rendered string: this module + * is executed standalone under node by its test, so it holds no reference to + * `i18n.js`. A caller renders `name` (a link to `groupId`) when `count` is 1, + * and a plural of `count` otherwise — which group was picked is deliberately + * never shown. + * + * An entry with no `_sources` at all is the single-group Group page, where + * there is one source by construction and no badge is drawn. + */ +function sourceLabel(entry) { + const sources = (entry && entry._sources) || []; + if (sources.length > 1) return { count: sources.length, name: '', groupId: null }; + return { + count: 1, + name: (entry && entry.groupName) || '', + groupId: (entry && entry.groupId) || null, + }; +} + +export { pickSource, mergeUnitEntries, sourceLabel, stableHash }; diff --git a/packages/meshbay-hub/tests/test_search_source_merge.py b/packages/meshbay-hub/tests/test_search_source_merge.py new file mode 100644 index 0000000..078c38d --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_source_merge.py @@ -0,0 +1,340 @@ +""" +One file, one entry: merging the same content announced by several groups. + +Reported live: a node hosting two groups that were given the same video +directory — the point of two groups being that different people are invited to +different libraries, and one library may be in several of them. The Search +view, which concatenates every group's index into one list, then showed every +film twice, every episode twice inside a show, every track twice inside an +album. Inside one group this cannot happen: `GroupIndex` is keyed by blake3, so +the same bytes at two paths are already one entry. The duplication is the +Search page's own. + +`source-merge.js` merges on the content hash and resolves one source per +*unit* — a film, a whole show, a whole album — rather than per file, so a +season's episodes do not scatter across two nodes. This holds the rules that +decide which source that is, and what survives the merge. + +The whole module is executed here rather than a regex-extracted function of it: +it has no imports precisely so that it can be, and a copy of the picking rule +in a test would keep agreeing with the original right up until one of them +changed. + +See docs/refactoring-search.md. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +SRC = STATIC / "source-merge.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not SRC.exists(), + reason="node or the SPA sources are not available") + +IMPORT = re.compile(r"^\s*import\b", re.M) +EXPORT = re.compile(r"^export \{[^}]*\};?\s*$", re.M) + + +@pytest.fixture(scope="module") +def module_source(): + text = SRC.read_text() + assert not IMPORT.search(text), ( + "source-merge.js has gained an import. It is executed standalone here, " + "and the merge is untested from the moment it cannot be — keep the " + "module free of imports, or this test needs a bundler") + stripped, n = EXPORT.subn("", text) + assert n == 1, ( + "source-merge.js no longer ends in a single export statement — the " + "test can no longer strip it to run the module") + return stripped + + +def _run(tmp_path, module_source, body): + script = tmp_path / "case.js" + script.write_text(f"{module_source}\n{body}\n") + out = subprocess.run( + ["node", str(script)], capture_output=True, text=True, timeout=30) + assert out.returncode == 0, out.stderr + return json.loads(out.stdout) + + +def _entry(file_id, group, **kw): + e = { + "id": file_id, + "name": f"{file_id}.mkv", + "type": "video", + "groupId": group, + "groupName": group.upper(), + "groupOwner": "someone", + "_tRef": f"t:{group}", + } + e.update(kw) + return e + + +def _merge(tmp_path, module_source, units, salt="u1", local=(), down=()): + body = f""" + const units = {json.dumps(units)}; + const local = new Set({json.dumps(list(local))}); + const down = new Set({json.dumps(list(down))}); + const merged = mergeUnitEntries(units, {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + isDown: (g) => down.has(g), + }}); + console.log(JSON.stringify(merged)); + """ + return _run(tmp_path, module_source, body) + + +# ── the reported case ──────────────────────────────────────────────────────── + +def test_one_file_two_groups_becomes_one_entry(tmp_path, module_source): + """The whole bug: a film shared by two groups was two poster cards.""" + units = [ + {"key": "movie:aaa", "entries": [_entry("aaa", "demo35")]}, + {"key": "movie:aaa", "entries": [_entry("aaa", "media")]}, + ] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert merged[0]["id"] == "aaa" + assert [s["groupId"] for s in merged[0]["_sources"]] == ["demo35", "media"] + + +def test_a_show_is_one_list_of_episodes(tmp_path, module_source): + """Two seasons' worth of episodes announced twice, one list out.""" + eps = [f"e{n}" for n in range(6)] + units = [{ + "key": "show:Some Saga", + "entries": [_entry(i, g) for g in ("demo35", "media") for i in eps], + }] + merged = _merge(tmp_path, module_source, units) + assert sorted(e["id"] for e in merged) == sorted(eps) + + +def test_a_lone_source_is_left_alone(tmp_path, module_source): + """The overwhelmingly common case must come through untouched.""" + units = [{"key": "movie:aaa", "entries": [_entry("aaa", "solo")]}] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert merged[0]["groupId"] == "solo" + assert merged[0]["_tRef"] == "t:solo" + assert len(merged[0]["_sources"]) == 1 + + +# ── which source ───────────────────────────────────────────────────────────── + +def test_the_local_node_always_wins(tmp_path, module_source): + """ + Whatever the salt, a group on the reader's own node is the source. Every + salt is tried rather than one: a rule that only holds for the salt the test + happened to pick is not the rule. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "remote"), _entry("aaa", "mine")]}] + for salt in [f"user-{n}" for n in range(20)]: + merged = _merge(tmp_path, module_source, units, salt=salt, local=["mine"]) + assert merged[0]["groupId"] == "mine", f"salt {salt} escaped the local rule" + + +def test_the_pick_is_stable_and_spread(tmp_path, module_source): + """ + Stable for one reader — a source that changed between renders would tear + down the connection under a film that is playing — and different across + readers, which is what "random" was for. + """ + groups = [f"g{n}" for n in range(8)] + units = [{"key": "movie:aaa", "entries": [_entry("aaa", g) for g in groups]}] + + first = _merge(tmp_path, module_source, units, salt="u1")[0]["groupId"] + again = _merge(tmp_path, module_source, units, salt="u1")[0]["groupId"] + assert first == again + + picks = {_merge(tmp_path, module_source, units, salt=f"u{n}")[0]["groupId"] + for n in range(40)} + assert len(picks) > 1, "every reader lands on the same source — nothing is spread" + + +def test_one_reader_is_spread_across_sources(tmp_path, module_source): + """ + The spread is across units as well as across readers. Hashing the salt + alone would be just as stable and just as fair between readers, and would + point one reader's entire library at one node. + """ + groups = ("aa", "bb", "cc", "dd") + units = [{"key": f"movie:{n}", "entries": [_entry(f"f{n}", g) for g in groups]} + for n in range(40)] + merged = _merge(tmp_path, module_source, units, salt="u1") + assert len({e["groupId"] for e in merged}) > 1 + + +def test_source_order_does_not_decide(tmp_path, module_source): + """ + The pick sorts by group id first. Without that it would follow whichever + group the index fetch happened to answer first, which is a race. + """ + groups = [_entry("aaa", g) for g in ("zulu", "alpha", "mike")] + fwd = _merge(tmp_path, module_source, [{"key": "m", "entries": groups}]) + rev = _merge(tmp_path, module_source, [{"key": "m", "entries": groups[::-1]}]) + assert fwd[0]["groupId"] == rev[0]["groupId"] + + +def test_every_file_of_a_unit_shares_its_source(tmp_path, module_source): + """ + The reason the source is picked per unit at all: a season split across two + nodes would open two connections and two metadata lookups for one show. + + The episodes deliberately do NOT all have the same sources. With every + episode in every group, picking per file and picking per unit give the same + answer — the same key over the same set — and a test built that way passes + against a per-file implementation, which is how the first version of this + one got written. `e3` is missing from `cc`, so a per-file pick resolves it + over `{aa, bb}` while its siblings resolve over `{aa, bb, cc}`, and the two + rules come apart. + """ + full = ("aa", "bb", "cc") + units = [{ + "key": "show:Some Saga", + "entries": ([_entry(i, g) for g in full for i in ("e1", "e2")] + + [_entry("e3", g) for g in ("aa", "bb")]), + }] + exercised = 0 + for salt in [f"u{n}" for n in range(40)]: + by_id = {e["id"]: e for e in + _merge(tmp_path, module_source, units, salt=salt)} + assert by_id["e1"]["groupId"] == by_id["e2"]["groupId"] + chosen = by_id["e1"]["groupId"] + if chosen == "cc": + continue # e3 genuinely does not have it; it falls back + exercised += 1 + assert by_id["e3"]["groupId"] == chosen, ( + f"salt {salt}: e3 has {chosen} and went to " + f"{by_id['e3']['groupId']} instead — the source is being picked " + "per file, not per unit") + assert exercised > 5, ( + "no salt put the unit on a source e3 also has — this test proved " + "nothing about per-unit picking") + + +def test_a_file_the_unit_source_lacks_falls_back_together(tmp_path, module_source): + """ + An episode only one group holds still plays — and when several episodes are + missing from the chosen source they all land on the same fallback rather + than one node apiece. + """ + shared = [_entry(i, g) for g in ("aa", "bb") for i in ("e1", "e2")] + # e8/e9 exist in bb and cc only, whichever of the three the unit picked + extra = [_entry(i, g) for g in ("bb", "cc") for i in ("e8", "e9")] + merged = _merge(tmp_path, module_source, + [{"key": "show:Some Saga", "entries": shared + extra}]) + by_id = {e["id"]: e for e in merged} + assert sorted(by_id) == ["e1", "e2", "e8", "e9"] + assert by_id["e8"]["groupId"] == by_id["e9"]["groupId"] + assert by_id["e8"]["groupId"] in ("bb", "cc") + + +# ── failover ───────────────────────────────────────────────────────────────── + +def test_a_down_source_is_skipped(tmp_path, module_source): + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", g) for g in ("aa", "bb", "cc")]}] + for salt in [f"u{n}" for n in range(20)]: + merged = _merge(tmp_path, module_source, units, salt=salt, down=["aa", "bb"]) + assert merged[0]["groupId"] == "cc" + + +def test_a_down_local_source_yields_to_a_live_remote(tmp_path, module_source): + """ + The local preference is a preference, not an override. A local node that is + not answering must not hold a film hostage while another group serves it. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "mine"), _entry("aaa", "remote")]}] + merged = _merge(tmp_path, module_source, units, local=["mine"], down=["mine"]) + assert merged[0]["groupId"] == "remote" + + +def test_everything_down_still_yields_an_entry(tmp_path, module_source): + """A tile that fails to load beats a film that vanished from the grid.""" + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", g) for g in ("aa", "bb")]}] + merged = _merge(tmp_path, module_source, units, down=["aa", "bb"]) + assert len(merged) == 1 + assert merged[0]["groupId"] in ("aa", "bb") + assert len(merged[0]["_sources"]) == 2, "a down source is still a source" + + +# ── what survives the merge ────────────────────────────────────────────────── + +def test_fields_come_from_the_chosen_source_only(tmp_path, module_source): + """ + No field is back-filled from another source. A thumb_hash only one node + computed is only fetchable over *that* node's connection, so borrowing it + would produce a poster request the chosen transport cannot answer. + """ + units = [{"key": "movie:aaa", "entries": [ + _entry("aaa", "mine", thumb_hash=None, display_title="a"), + _entry("aaa", "remote", thumb_hash="deadbeef", display_title="b"), + ]}] + merged = _merge(tmp_path, module_source, units, local=["mine"]) + assert merged[0]["groupId"] == "mine" + assert merged[0]["thumb_hash"] is None + assert merged[0]["display_title"] == "a" + assert merged[0]["_tRef"] == "t:mine" + + +def test_units_are_kept_apart(tmp_path, module_source): + """ + Merging is scoped to a unit. Two albums that happen to hold the same photo + are two albums, and it belongs in both. + """ + units = [ + {"key": "album:trip", "entries": [_entry("p1", "aa"), _entry("p1", "bb")]}, + {"key": "album:party", "entries": [_entry("p1", "aa")]}, + ] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 2 + + +def test_one_group_announcing_a_file_twice_counts_once(tmp_path, module_source): + """ + Cannot happen through a well-behaved index (GroupIndex is keyed by id), so + a duplicate here is a node saying something odd. It must not inflate the + source count the badge shows. + """ + units = [{"key": "movie:aaa", + "entries": [_entry("aaa", "aa"), _entry("aaa", "aa")]}] + merged = _merge(tmp_path, module_source, units) + assert len(merged) == 1 + assert len(merged[0]["_sources"]) == 1 + + +def test_empty_input(tmp_path, module_source): + assert _merge(tmp_path, module_source, []) == [] + + +# ── the badge ──────────────────────────────────────────────────────────────── + +def test_source_label(tmp_path, module_source): + body = """ + const one = { groupId: 'g1', groupName: 'G1', + _sources: [{ groupId: 'g1', groupName: 'G1' }] }; + const many = { groupId: 'g1', groupName: 'G1', + _sources: [{ groupId: 'g1' }, { groupId: 'g2' }] }; + const bare = { }; + console.log(JSON.stringify( + [one, many, bare].map(sourceLabel))); + """ + one, many, bare = _run(tmp_path, module_source, body) + assert one == {"count": 1, "name": "G1", "groupId": "g1"} + # Which group was picked is deliberately not shown once there are several. + assert many == {"count": 2, "name": "", "groupId": None} + # The single-group Group page, where entries carry no sources at all. + assert bare == {"count": 1, "name": "", "groupId": None} diff --git a/packages/meshbay-hub/tests/test_search_video_merge.py b/packages/meshbay-hub/tests/test_search_video_merge.py new file mode 100644 index 0000000..e9ab258 --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_video_merge.py @@ -0,0 +1,185 @@ +""" +The reported symptom, end to end: one library shared by two groups. + +`test_search_source_merge.py` holds the merging rules in isolation. This holds +the thing an operator actually saw — a node hosting two groups that were given +the same video directory, and a Search view showing every film as two poster +cards and every episode twice inside a show. + +Three pieces have to agree for that to come out right, and each lives in a +different file: + + * `groupVideoEntries` (video-app.js) turns entries into films and shows; + * `videoUnits` (search-page.js) turns those into merge units; + * `mergeUnitEntries` (source-merge.js) folds them on the content hash. + +All three are read out of their real sources here rather than restated. The +pipeline is assembled the way `search-page.js` assembles it, and then the +result is passed through `groupVideoEntries` a second time — which is what +`VideoApp` does with it — so what this counts is what the grid renders. + +See docs/refactoring-search.md. +""" + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +VIDEO_APP = STATIC / "video-app.js" +SEARCH_PAGE = STATIC / "search-page.js" +MERGE = STATIC / "source-merge.js" + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not MERGE.exists(), + reason="node or the SPA sources are not available") + +EXPORT = re.compile(r"^export \{[^}]*\};?\s*$", re.M) + + +def _block(path, header): + """One top-level `function name(...) {` ... `}` read out of a module.""" + src = path.read_text() + m = re.search(r"^" + re.escape(header) + r".*?^\}", src, re.M | re.S) + assert m, ( + f"{header} is no longer where this test reads it from in {path.name} — " + "the Search view's de-duplication is untested until this is fixed") + return m.group(0) + + +@pytest.fixture(scope="module") +def pipeline(): + root = re.search(r"^const SEARCH_VIDEO_ROOT = .*?;$", SEARCH_PAGE.read_text(), re.M) + assert root, "SEARCH_VIDEO_ROOT moved — videoUnits cannot be lifted" + return "\n".join([ + EXPORT.sub("", MERGE.read_text()), + _block(VIDEO_APP, "function underVideoRoot(entry, videoRoot) {"), + _block(VIDEO_APP, "function buildSeasons(episodes) {"), + _block(VIDEO_APP, "function groupVideoEntries(entries, videoRoot) {"), + root.group(0), + _block(SEARCH_PAGE, "function videoUnits(entries) {"), + ]) + + +def _grid(tmp_path, pipeline, entries, salt="reader", local=()): + """What the poster grid ends up with, after the merge and VideoApp's own + regrouping.""" + body = f""" + const raw = {json.dumps(entries)}; + const local = new Set({json.dumps(list(local))}); + const merged = mergeUnitEntries(videoUnits(raw), {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + }}); + const {{ movies, shows }} = groupVideoEntries(merged, SEARCH_VIDEO_ROOT); + console.log(JSON.stringify({{ + movies: movies.map((e) => ({{ + id: e.id, title: e.display_title || e.name, groupId: e.groupId, + sources: e._sources.length, + }})), + shows: shows.map((s) => ({{ + title: s.title, + groups: [...new Set(s.episodes.map((e) => e.groupId))].sort(), + seasons: s.seasons.map((x) => ({{ + season: x.season, + episodes: x.episodes.map((e) => `S${{e.season}}E${{e.episode}}`), + }})), + }})), + }})); + """ + script = tmp_path / "case.js" + script.write_text(f"{pipeline}\n{body}\n") + out = subprocess.run( + ["node", str(script)], capture_output=True, text=True, timeout=30) + assert out.returncode == 0, out.stderr + return json.loads(out.stdout) + + +# The shape of one shared library: a film, and a two-season show. Invented +# titles — the real one this was found against is nobody's business here. +def _library(group): + """`path` is already prefixed the way search-page.js prefixes it.""" + def entry(file_id, name, **kw): + return { + "id": file_id, "name": name, "type": "video", + "path": "__search__/shows", "size": 1, + "groupId": group, "groupName": group.upper(), "groupOwner": "someone", + "_tRef": f"t:{group}", "_gRef": f"g:{group}", "_connGen": 1, + **kw, + } + files = [entry("film1", "a-film.mkv", display_title="Some Film")] + for season in (1, 2): + for ep in (1, 2, 3): + files.append(entry( + f"s{season}e{ep}", f"show.s0{season}e0{ep}.mkv", + display_title="Some Saga", season=season, episode=ep)) + return files + + +def test_a_shared_library_is_listed_once(tmp_path, pipeline): + """ + The bug as reported: two groups, one directory, everything twice. + """ + both = _library("demo35") + _library("media") + grid = _grid(tmp_path, pipeline, both) + + assert [m["title"] for m in grid["movies"]] == ["Some Film"] + assert grid["movies"][0]["sources"] == 2 + + assert len(grid["shows"]) == 1 + show = grid["shows"][0] + assert [s["season"] for s in show["seasons"]] == [1, 2] + for season in show["seasons"]: + assert season["episodes"] == [ + f"S{season['season']}E{n}" for n in (1, 2, 3)], ( + "an episode is listed more than once — this is the reported bug, " + "in the season list under the synopsis") + + +def test_a_show_streams_from_one_source(tmp_path, pipeline): + """A season split across two nodes would open two connections and two + metadata lookups for one show.""" + both = _library("demo35") + _library("media") + show = _grid(tmp_path, pipeline, both)["shows"][0] + assert len(show["groups"]) == 1 + + +def test_the_operators_own_node_serves_it(tmp_path, pipeline): + """Both groups are on the operator's node in the reported case; when only + one is, that one is the source.""" + both = _library("remote") + _library("mine") + grid = _grid(tmp_path, pipeline, both, local=["mine"]) + assert grid["movies"][0]["groupId"] == "mine" + assert grid["shows"][0]["groups"] == ["mine"] + + +def test_one_group_is_unchanged(tmp_path, pipeline): + """The overwhelmingly common case: nothing to merge, nothing different.""" + grid = _grid(tmp_path, pipeline, _library("solo")) + assert [m["title"] for m in grid["movies"]] == ["Some Film"] + assert grid["movies"][0]["sources"] == 1 + assert grid["movies"][0]["groupId"] == "solo" + show = grid["shows"][0] + assert show["groups"] == ["solo"] + assert sum(len(s["episodes"]) for s in show["seasons"]) == 6 + + +def test_an_episode_only_one_group_has_is_kept(tmp_path, pipeline): + """ + Merging must never subtract. A group holding one extra episode contributes + it, whichever source the show settled on. + """ + extra = _library("media") + extra.append({ + "id": "s2e4", "name": "show.s02e04.mkv", "type": "video", + "path": "__search__/shows", "size": 1, + "groupId": "media", "groupName": "MEDIA", "groupOwner": "someone", + "display_title": "Some Saga", "season": 2, "episode": 4, + }) + grid = _grid(tmp_path, pipeline, _library("demo35") + extra) + season2 = [s for s in grid["shows"][0]["seasons"] if s["season"] == 2][0] + assert season2["episodes"] == ["S2E1", "S2E2", "S2E3", "S2E4"] |