From 10f8266e7152d7dc38dbfe2449327829bf020ad1 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 2 Sep 2026 16:21:48 +0200 Subject: fix(hub): merge duplicate sources in Search's Music and Photos too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phases 5-8 of docs/refactoring-search.md, extending the Videos merge outward. A library shared by two groups now lists each track once inside an album and each photo once inside a photo album, and a card served by several groups says "N sources" instead of naming one of them. Units come from each application's own grouping, never a copy of its keys. For Music that meant exporting foldKey: groupMusicEntries folds case to group but keeps the first-seen spelling to display, and which group is seen first is whichever index arrived first — so keying a unit on the display strings would let the chosen source change between page loads. A group whose connection fails is marked down and stops being chosen, so a unit fails over to another group that has the file. Eviction is not a failure. Every source being down still yields an entry: a tile that fails to load beats a film that vanished from the grid. sourceLabel now takes the whole unit rather than one entry. A show's poster entry is picked for its thumbnail, so a show in two groups whose cover episode sits in only one of them would have claimed a single source. SourceTag lives in group-name.js — source-merge.js must keep importing nothing (its test executes it standalone), and a copy in each of the three apps is three chances to disagree. test_search_files_unmerged.py holds the one thing that must not change: the Files explorer is not merged, because there each group is a folder and merging would remove a file from one of them. It also asserts the other three lists are merged, or deleting the merge outright would leave it passing and saying nothing. One plan item was dropped as wrong rather than built: the Music queue in onPreview needed no change. It filters by groupId and is reachable only from FilesPanel, which is not merged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AbwJDbNTkiRUh7HTWEoyss --- .../src/meshbay_hub/static/group-name.js | 35 ++ .../src/meshbay_hub/static/locales/de.js | 1 + .../src/meshbay_hub/static/locales/en.js | 1 + .../src/meshbay_hub/static/locales/es.js | 1 + .../src/meshbay_hub/static/locales/fr.js | 1 + .../src/meshbay_hub/static/locales/it.js | 1 + .../src/meshbay_hub/static/locales/ja.js | 1 + .../src/meshbay_hub/static/locales/nl.js | 1 + .../src/meshbay_hub/static/locales/pl.js | 1 + .../src/meshbay_hub/static/locales/pt-BR.js | 1 + .../src/meshbay_hub/static/locales/zh-CN.js | 1 + .../src/meshbay_hub/static/music-app.js | 13 +- .../src/meshbay_hub/static/photos-app.js | 5 +- .../src/meshbay_hub/static/search-page.js | 79 ++++- .../src/meshbay_hub/static/source-merge.js | 37 ++- .../src/meshbay_hub/static/video-app.js | 13 +- .../tests/test_search_files_unmerged.py | 80 +++++ .../meshbay-hub/tests/test_search_media_merge.py | 363 +++++++++++++++++++++ .../meshbay-hub/tests/test_search_source_merge.py | 30 +- .../meshbay-hub/tests/test_search_video_merge.py | 185 ----------- 20 files changed, 627 insertions(+), 223 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_search_files_unmerged.py create mode 100644 packages/meshbay-hub/tests/test_search_media_merge.py delete mode 100644 packages/meshbay-hub/tests/test_search_video_merge.py (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-name.js b/packages/meshbay-hub/src/meshbay_hub/static/group-name.js index af8879c..f109da7 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-name.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-name.js @@ -1,4 +1,6 @@ import { html } from './vendor/htm-preact.js'; +import { t } from './i18n.js'; +import { sourceLabel } from './source-merge.js'; /** * A group's name with the `@owner` handle under it. @@ -20,3 +22,36 @@ export function GroupName({ name, owner, inline = false }) { `; } + +/** + * Which group serves an entry, or how many groups have it. + * + * The Search view merges a file several groups share into one entry, so the + * badge under a card cannot always name a group. One source keeps naming it + * and keeps linking to it; more than one becomes a count, and *which* one was + * picked is deliberately not shown (docs/refactoring-search.md §5.5). + * + * `entries` is the whole unit — every episode of a show, every track of an + * album — not the entry the card was drawn from; `sourceLabel` explains why. + * Renders nothing at all on the single-group Group page, where an entry + * carries no group and there is only ever one source anyway. + * + * It lives here rather than in `source-merge.js` because that module is + * executed standalone by its test and must keep importing nothing; and here + * rather than in one of the three apps that need it, because a copy apiece is + * three chances to disagree about what a merged card says. + * + * A `div` by default: the three card badges rely on `text-overflow: ellipsis`, + * which does nothing on an inline box. `link` gives the flat row's inline + * pill instead. + */ +export function SourceTag({ entries, cls, link = false }) { + const { count, name, groupId } = sourceLabel(entries); + if (count > 1) return html`${t('search.n_sources', { n: count })}`; + if (!name) return null; + if (link && groupId) { + return html` e.stopPropagation()}>${name}`; + } + return html`
${name}
`; +} diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index f118ffe..b66c847 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -616,6 +616,7 @@ export default { 'search.hint': 'Tippen Sie einen Begriff ein, um in allen Ihren Gruppen zu suchen.', 'search.indexing': 'Indexierung von {done} / {total} Gruppen …', 'search.unreachable': { one: '{n} Gruppe nicht erreichbar', other: '{n} Gruppen nicht erreichbar' }, + 'search.n_sources': { one: '{n} Quelle', other: '{n} Quellen' }, 'search.view_files': 'Dateien', 'search.view_videos': 'Videos', 'search.view_music': 'Musik', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index b55359b..898f7f4 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -690,6 +690,7 @@ export default { 'search.hint': 'Type to search file names across all your groups.', 'search.indexing': 'Indexing {done} / {total} groups...', 'search.unreachable': { one: '{n} group unreachable', other: '{n} groups unreachable' }, + 'search.n_sources': { one: '{n} source', other: '{n} sources' }, 'search.view_files': 'Files', 'search.view_videos': 'Videos', 'search.view_music': 'Music', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index 8e90dca..8d06a2d 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -612,6 +612,7 @@ export default { 'search.hint': 'Escriba un término para buscar en todos sus grupos.', 'search.indexing': 'Indexando {done} / {total} grupos…', 'search.unreachable': { one: '{n} grupo inalcanzable', other: '{n} grupos inalcanzables' }, + 'search.n_sources': { one: '{n} fuente', other: '{n} fuentes' }, 'search.view_files': 'Archivos', 'search.view_videos': 'Vídeos', 'search.view_music': 'Música', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index 995d4b3..1999e7e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -615,6 +615,7 @@ export default { 'search.hint': 'Saisissez un terme pour rechercher dans tous vos groupes.', 'search.indexing': 'Indexation de {done} / {total} groupes…', 'search.unreachable': { one: '{n} groupe injoignable', other: '{n} groupes injoignables' }, + 'search.n_sources': { one: '{n} source', other: '{n} sources' }, 'search.view_files': 'Fichiers', 'search.view_videos': 'Vidéos', 'search.view_music': 'Musique', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index 467521f..52e18a8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -614,6 +614,7 @@ export default { 'search.hint': 'Digita un termine per cercare in tutti i tuoi gruppi.', 'search.indexing': 'Indicizzazione di {done} / {total} gruppi…', 'search.unreachable': { one: '{n} gruppo non raggiungibile', other: '{n} gruppi non raggiungibili' }, + 'search.n_sources': { one: '{n} fonte', other: '{n} fonti' }, 'search.view_files': 'File', 'search.view_videos': 'Video', 'search.view_music': 'Musica', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index 89e8bdd..c0560b3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -602,6 +602,7 @@ export default { 'search.hint': '入力してすべてのグループを検索します。', 'search.indexing': '{done} / {total} グループをインデックス中…', 'search.unreachable': { other: '{n} グループに接続できません' }, + 'search.n_sources': { other: '{n} 個のソース' }, 'search.view_files': 'ファイル', 'search.view_videos': '動画', 'search.view_music': '音楽', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 1ed0362..da6dc04 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -616,6 +616,7 @@ export default { 'search.hint': 'Typ een zoekterm om in al uw groepen te zoeken.', 'search.indexing': '{done} / {total} groepen indexeren…', 'search.unreachable': { one: '{n} groep onbereikbaar', other: '{n} groepen onbereikbaar' }, + 'search.n_sources': { one: '{n} bron', other: '{n} bronnen' }, 'search.view_files': 'Bestanden', 'search.view_videos': 'Video\'s', 'search.view_music': 'Muziek', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index f71d048..29feebe 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -634,6 +634,7 @@ export default { 'search.hint': 'Wpisz frazę, aby wyszukać we wszystkich grupach.', 'search.indexing': 'Indeksowanie {done} / {total} grup…', 'search.unreachable': { one: '{n} grupa nieosiągalna', few: '{n} grupy nieosiągalne', many: '{n} grup nieosiągalnych', other: '{n} grupy nieosiągalnej' }, + 'search.n_sources': { one: '{n} źródło', few: '{n} źródła', many: '{n} źródeł', other: '{n} źródła' }, 'search.view_files': 'Pliki', 'search.view_videos': 'Filmy', 'search.view_music': 'Muzyka', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index 1919edd..d2fc355 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -613,6 +613,7 @@ export default { 'search.hint': 'Digite um termo para pesquisar em todos os seus grupos.', 'search.indexing': 'Indexando {done} / {total} grupos…', 'search.unreachable': { one: '{n} grupo inacessível', other: '{n} grupos inacessíveis' }, + 'search.n_sources': { one: '{n} fonte', other: '{n} fontes' }, 'search.view_files': 'Arquivos', 'search.view_videos': 'Vídeos', 'search.view_music': 'Música', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index 489311e..6840774 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -589,6 +589,7 @@ export default { 'search.hint': '输入关键词搜索所有群组。', 'search.indexing': '正在索引 {done} / {total} 个群组…', 'search.unreachable': { other: '{n} 个群组无法连接' }, + 'search.n_sources': { other: '{n} 个来源' }, 'search.view_files': '文件', 'search.view_videos': '视频', 'search.view_music': '音乐', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/music-app.js b/packages/meshbay-hub/src/meshbay_hub/static/music-app.js index 418da89..616f169 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/music-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/music-app.js @@ -5,6 +5,7 @@ import { t } from './i18n.js'; import { Icon } from './icon.js'; import { MediaThumb, LazyTile } from './video-app.js'; import { formatTime } from './music-player.js'; +import { SourceTag } from './group-name.js'; // -- Music -------------------------------------------------------------------- // @@ -264,9 +265,7 @@ function AlbumCard({ album, transportRef, gekRef, musicbrainzEnabled, onOpen })
${album.album}
${album.artist}
- ${repTrack.groupName && html` -
${repTrack.groupName}
- `} + <${SourceTag} entries=${album.tracks} cls="music-card-group" />
`; @@ -525,4 +524,10 @@ function MusicApp({ `; } -export { MusicApp, groupMusicEntries, bumpMusicMetaGeneration }; +// foldKey rides along for the Search page's merge unit keys +// (docs/refactoring-search.md §5.2). An album's *display* strings are the +// first-seen spelling, and which group is seen first is the order its index +// happened to arrive in — so keying a unit on them would let the chosen source +// change between page loads. The folded key is the one grouping actually used, +// and is stable. +export { MusicApp, groupMusicEntries, bumpMusicMetaGeneration, foldKey }; 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 1755392..e6f3482 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/photos-app.js @@ -8,6 +8,7 @@ import { } from './file-utils.js'; import { transfers } from './transfers.js'; import { MediaThumb, LazyTile } from './video-app.js'; +import { SourceTag } from './group-name.js'; // ── Photos ─────────────────────────────────────────────────────────────────── // @@ -93,9 +94,7 @@ function AlbumCard({ album, transportRef, gekRef, onOpen }) {
${year}${year ? ' · ' : ''}${t('photo.n_photos', { n: album.photos.length })}
- ${cover.groupName && html` -
${cover.groupName}
- `} + <${SourceTag} entries=${album.photos} cls="photo-card-group" /> `; 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 5897acb..608bb81 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/search-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/search-page.js @@ -9,8 +9,8 @@ import { } from './hub-client.js'; import { FilesPanel, FilePreview } from './files-app.js'; import { VideoApp, groupVideoEntries } from './video-app.js'; -import { MusicApp } from './music-app.js'; -import { PhotosApp } from './photos-app.js'; +import { MusicApp, groupMusicEntries, foldKey } from './music-app.js'; +import { PhotosApp, groupPhotoAlbums } from './photos-app.js'; import { VideoPlayer } from './video-player.js'; import { transfers } from './transfers.js'; import { mergeUnitEntries } from './source-merge.js'; @@ -233,6 +233,30 @@ function videoUnits(entries) { ]; } +// An album is a unit, and so is a track loose enough to have no artist at all. +// `groupMusicEntries` has already folded the two groups' copies into one album +// object, so the key only has to name it stably — hence `foldKey` over the +// display strings, which are whichever spelling arrived first. +function musicUnits(entries) { + const { tracks, albums } = groupMusicEntries(entries, SEARCH_AUDIO_ROOT); + return [ + ...albums.map((a) => ({ + key: `album:${foldKey(a.artist)}/${foldKey(a.album)}`, + entries: a.tracks, + })), + ...tracks.map((e) => ({ key: `track:${e.id}`, entries: [e] })), + ]; +} + +// A photo album is its directory, which is already prefixed per group when the +// entries are built — so two groups whose roots have different basenames stay +// two albums, and the same photo belongs in both. Only same-named albums +// collapse, which is the reported shape. +function photoUnits(entries) { + return groupPhotoAlbums(entries, SEARCH_PHOTO_ROOTS) + .map((a) => ({ key: `album:${a.dir}`, entries: a.photos })); +} + function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) { const [indexedGroups, setIndexedGroups] = useState(new Map()); const [progress, setProgress] = useState({ done: 0, total: 0, unreachable: [] }); @@ -254,6 +278,8 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) // key, so a group's posters/thumbnails recover the moment it reconnects // (after a pool eviction) instead of staying stuck on a spinner. const connGenRef = useRef(new Map()); + // groupIds whose connection failed — see markGroupDown below. + const downGroups = useRef(new Set()); const mountedRef = useRef(true); const [connectionGen, setConnectionGen] = useState(0); const debounceRef = useRef(null); @@ -301,6 +327,27 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) // -- Connection management -- + // A group whose connection failed stops being chosen as a merged entry's + // source, so a unit fails over to another group that has the file + // (docs/refactoring-search.md §5.4). Without this the merge could make a + // file *less* available than it was before it, which would be a regression + // dressed as a feature. + // + // Held in a ref and read live by `mergeOpts.isDown`; what actually rebuilds + // the entry lists is the `connectionGen` bump, which they already depend on. + // Eviction is deliberately not a failure — `_onEvict` only drops the recorded + // connection, and the group is picked again as readily as before. + const markGroupDown = useCallback((groupId, down) => { + let changed; + if (down) { + changed = !downGroups.current.has(groupId); + downGroups.current.add(groupId); + } else { + changed = downGroups.current.delete(groupId); + } + if (changed && mountedRef.current) setConnectionGen((g) => g + 1); + }, []); + const connectGroup = useCallback(async (groupId) => { if (groupConns.current.has(groupId)) { const c = groupConns.current.get(groupId); @@ -308,7 +355,14 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) } if (!poolRef.current) throw new Error('no pool'); const bundleKey = session.bundleKey || await _loadBundleKey(); - const conn = await poolRef.current.connect(groupId, token, bundleKey, username, userId); + let conn; + try { + conn = await poolRef.current.connect(groupId, token, bundleKey, username, userId); + } catch (e) { + markGroupDown(groupId, true); + throw e; + } + markGroupDown(groupId, false); // A concurrent caller for the same group (several tiles mounting at once) // may already have recorded this exact transport while we awaited. Only @@ -339,7 +393,7 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) // module-wide bumps stay for an operator's TMDB override/rematch only. setConnectionGen((g) => g + 1); return entry; - }, [token, username, userId]); + }, [token, username, userId, markGroupDown]); // Warm up connections as soon as indexing finishes so thumbnails start // loading before the user switches views — but only up to the pool's @@ -431,6 +485,11 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) const data = indexedGroups.get(gid); return !!(data && data.isNodeAdmin); }, + // Read live off the ref rather than captured: what rebuilds the lists is + // the `connectionGen` bump markGroupDown fires, and they already depend on + // it. Putting the set in this memo's own dependencies would only add a + // second reason to rebuild the same thing. + isDown: (gid) => downGroups.current.has(gid), }), [indexedGroups, userId]); // Videos view: pre-filtered by videoRoot, path-prefixed, then merged so a @@ -460,7 +519,7 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) return mergeUnitEntries(videoUnits(result), mergeOpts); }, [indexedGroups, q, matchesQuery, connectionGen, mergeOpts]); - // Music view: pre-filtered by audioRoot, path-prefixed + // Music view: pre-filtered by audioRoot, path-prefixed, then merged per album const musicEntries = useMemo(() => { const result = []; for (const [groupId, data] of indexedGroups) { @@ -483,10 +542,10 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) }); } } - return result; - }, [indexedGroups, q, matchesQuery, connectionGen]); + return mergeUnitEntries(musicUnits(result), mergeOpts); + }, [indexedGroups, q, matchesQuery, connectionGen, mergeOpts]); - // Photos view: pre-filtered by photoRoots, path-prefixed + // Photos view: pre-filtered by photoRoots, path-prefixed, then merged per album const photoEntries = useMemo(() => { const result = []; for (const [groupId, data] of indexedGroups) { @@ -510,8 +569,8 @@ function SearchPage({ token, username, userId, groups, onPlayQueue, userPrefs }) }); } } - return result; - }, [indexedGroups, q, matchesQuery, connectionGen]); + return mergeUnitEntries(photoUnits(result), mergeOpts); + }, [indexedGroups, q, matchesQuery, connectionGen, mergeOpts]); // -- Callbacks -- diff --git a/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js b/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js index ccc4cfd..2c08e06 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/source-merge.js @@ -136,24 +136,41 @@ function mergeUnitEntries(units, opts) { } /** - * What the group badge should say for one entry. + * What the group badge should say — for one entry, or for a whole unit. + * + * Takes a list, because a card stands for a unit while the entry it is drawn + * from is one file. A show's poster entry is picked for its *thumbnail* + * (`episodes.find((e) => e.thumb_hash)`), so reading the badge off it would + * report that one episode's sources: a show in two groups whose cover episode + * sits in only one of them would claim a single source. The union over the + * unit is the number the reader is actually being told. * * 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 + * and a plural of `count` above that — 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. + * `count: 0` is an entry with no group at all, which is the single-group Group + * page: there is one source by construction and no badge is drawn there. */ -function sourceLabel(entry) { - const sources = (entry && entry._sources) || []; - if (sources.length > 1) return { count: sources.length, name: '', groupId: null }; +function sourceLabel(entries) { + const list = Array.isArray(entries) ? entries : [entries]; + const groups = new Map(); + for (const e of list) { + if (!e) continue; + // An un-merged entry carries no `_sources`; it is its own single source. + const sources = (e._sources && e._sources.length) + ? e._sources + : (e.groupId ? [_source(e)] : []); + for (const s of sources) if (!groups.has(s.groupId)) groups.set(s.groupId, s); + } + if (groups.size > 1) return { count: groups.size, name: '', groupId: null }; + const only = groups.values().next().value; return { - count: 1, - name: (entry && entry.groupName) || '', - groupId: (entry && entry.groupId) || null, + count: groups.size, + name: (only && only.groupName) || '', + groupId: (only && only.groupId) || null, }; } diff --git a/packages/meshbay-hub/src/meshbay_hub/static/video-app.js b/packages/meshbay-hub/src/meshbay_hub/static/video-app.js index 19b0f0f..4f52b16 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/video-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/video-app.js @@ -4,6 +4,7 @@ import { import { t } from './i18n.js'; import { Icon } from './icon.js'; import { formatSize, pipelinedDownload } from './file-utils.js'; +import { SourceTag } from './group-name.js'; // ── Videos ─────────────────────────────────────────────────────────────────── // @@ -319,6 +320,7 @@ function useSeasonMeta(transportRef, tmdbId, season, active) { function PosterCard({ title, subtitle, repEntry, transportRef, gekRef, onOpen, groupKey, onMetaResolved, onNeedConn, + sourceEntries, }) { const tRef = repEntry._tRef || transportRef; const gRef = repEntry._gRef || gekRef; @@ -402,9 +404,7 @@ function PosterCard({ ${confident && meta.first_air_date ? yearOf(meta.first_air_date) : ''} ${subtitle ? ` · ${subtitle}` : ''} - ${repEntry.groupName && html` -
${repEntry.groupName}
- `} + <${SourceTag} entries=${sourceEntries || repEntry} cls="video-card-group" /> `} @@ -892,6 +892,7 @@ function PosterGrid({ <${PosterCard} title=${e.display_title || e.name} subtitle=${formatDuration(e.duration)} repEntry=${e} groupKey=${`movie:${e.id}`} + sourceEntries=${e} transportRef=${transportRef} gekRef=${gekRef} onNeedConn=${onNeedConn} onOpen=${() => (tmdbEnabled @@ -925,6 +926,7 @@ function PosterGrid({ <${PosterCard} title=${s.title} subtitle=${subtitle} repEntry=${repEntry} + sourceEntries=${s.episodes} groupKey=${s.title} onMetaResolved=${handleMetaResolved} onNeedConn=${onNeedConn} @@ -983,10 +985,7 @@ function FlatMovieRow({ entry, transportRef, gekRef, onPreview, seasonContext, o ${' · '}${formatSize(entry.size)} - ${entry.groupName && html` - e.stopPropagation()}>${entry.groupName} - `} + <${SourceTag} entries=${entry} cls="badge search-group-badge" link=${true} /> `; } diff --git a/packages/meshbay-hub/tests/test_search_files_unmerged.py b/packages/meshbay-hub/tests/test_search_files_unmerged.py new file mode 100644 index 0000000..6956dde --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_files_unmerged.py @@ -0,0 +1,80 @@ +""" +The Files explorer in the Search view is not merged, and must not become so. + +The Search view folds a file several groups share into one entry, so a film +shared by two groups is one poster instead of two. The Files tab is the one +place where that would be wrong: there each group is a top-level folder, the +two copies live in two different folders, and walking into one is how a member +browses *that group*. Merging would silently delete one of the two branches of +the tree. + +This is the guarantee a later refactor is most likely to break — the four entry +lists in `search-page.js` are near-identical, and tidying them into one shared +builder is the obvious cleanup. It would also be the last thing anyone tests by +hand, because the Files tab looks unchanged until you notice a group's folder +has fewer files in it than the group does. + +So: read the source, and refuse a build where `fileEntries` goes through the +merge. Weak evidence, and the only kind available for the SPA — but the failure +it guards against is a one-line edit, which is exactly what a source-reading +test catches well. + +See docs/refactoring-search.md §6.1. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +SEARCH_PAGE = STATIC / "search-page.js" + +pytestmark = pytest.mark.skipif( + not SEARCH_PAGE.exists(), reason="the SPA sources are not available") + +MERGE_CALL = "mergeUnitEntries" + + +def _memo(name): + """The body of `const = useMemo(() => { ... }, [...]);`.""" + src = SEARCH_PAGE.read_text() + m = re.search( + r"^ const " + re.escape(name) + r" = useMemo\(\(\) => \{.*?^ \}, \[.*?\]\);", + src, re.M | re.S) + assert m, ( + f"{name} is no longer a useMemo where this test reads it — the Files " + "explorer's exemption from the merge is untested until this is fixed") + return m.group(0) + + +def test_the_files_list_is_not_merged(): + body = _memo("fileEntries") + assert MERGE_CALL not in body, ( + "fileEntries now goes through the source merge. The Files explorer " + "shows one folder per group and a member navigates into it; merging " + "two groups' copies of a file would remove it from one of those " + "folders. See docs/refactoring-search.md §6.1") + + +@pytest.mark.parametrize("name", ["videoEntries", "musicEntries", "photoEntries"]) +def test_the_media_lists_are_merged(name): + """ + The other half of the check. Without it, deleting the merge outright would + leave the test above passing and saying nothing. + """ + assert MERGE_CALL in _memo(name), ( + f"{name} no longer goes through the source merge — a file shared by " + "two groups is two entries again") + + +def test_the_files_list_still_carries_one_group_per_entry(): + """ + What makes the explorer work: the path is prefixed with the group's name, + so the top level of the tree is the set of groups. A merged entry could not + be prefixed with anything, having several. + """ + body = _memo("fileEntries") + assert "data.groupName + (e.path ? '/' + e.path : '')" in body, ( + "fileEntries no longer prefixes paths with the group name — the " + "explorer's per-group top level is what this whole exemption is for") diff --git a/packages/meshbay-hub/tests/test_search_media_merge.py b/packages/meshbay-hub/tests/test_search_media_merge.py new file mode 100644 index 0000000..0312c57 --- /dev/null +++ b/packages/meshbay-hub/tests/test_search_media_merge.py @@ -0,0 +1,363 @@ +""" +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: + + * the application's own grouping (`groupVideoEntries`, `groupMusicEntries`, + `groupPhotoAlbums`) turns entries into films, shows, albums; + * `videoUnits` / `musicUnits` / `photoUnits` (search-page.js) turn those into + merge units; + * `mergeUnitEntries` (source-merge.js) folds them on the content hash. + +All of them are read out of their real sources here rather than restated. Each +pipeline is assembled the way `search-page.js` assembles it, and the result is +passed through the grouping a second time — which is what the application does +with it — so what this counts is what the grid renders. + +`t()` is stubbed to return its key: `groupMusicEntries` uses it for the two +placeholder album names, and a string is not what is under test here. + +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" +MUSIC_APP = STATIC / "music-app.js" +PHOTOS_APP = STATIC / "photos-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) + + +def _const(name): + m = re.search(r"^const " + re.escape(name) + r" = .*?;$", + SEARCH_PAGE.read_text(), re.M) + assert m, f"{name} moved — the search-page units cannot be lifted" + return m.group(0) + + +@pytest.fixture(scope="module") +def pipeline(): + """Everything the three views need, in one script.""" + return "\n".join([ + "const t = (k) => k;", + 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) {"), + _block(MUSIC_APP, "function foldKey(s) {"), + _block(MUSIC_APP, "function underAudioRoot(entry, audioRoot) {"), + _block(MUSIC_APP, "function groupMusicEntries(entries, audioRoot) {"), + _block(PHOTOS_APP, "function underAnyPhotoRoot(entry, photoRoots) {"), + _block(PHOTOS_APP, "function groupPhotoAlbums(entries, photoRoots) {"), + _const("SEARCH_VIDEO_ROOT"), + _const("SEARCH_AUDIO_ROOT"), + _const("SEARCH_PHOTO_ROOTS"), + _block(SEARCH_PAGE, "function videoUnits(entries) {"), + _block(SEARCH_PAGE, "function musicUnits(entries) {"), + _block(SEARCH_PAGE, "function photoUnits(entries) {"), + ]) + + +def _node(tmp_path, pipeline, body): + 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) + + +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}}`), + }})), + }})), + }})); + """ + return _node(tmp_path, pipeline, body) + + +def _albums(tmp_path, pipeline, entries, salt="reader", local=()): + """What the album grid ends up with, after the merge and MusicApp's own + regrouping.""" + body = f""" + const raw = {json.dumps(entries)}; + const local = new Set({json.dumps(list(local))}); + const merged = mergeUnitEntries(musicUnits(raw), {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + }}); + const {{ albums, tracks }} = groupMusicEntries(merged, SEARCH_AUDIO_ROOT); + console.log(JSON.stringify({{ + albums: albums.map((a) => ({{ + artist: a.artist, album: a.album, + groups: [...new Set(a.tracks.map((e) => e.groupId))].sort(), + tracks: a.tracks.map((e) => e.name), + }})), + loose: tracks.map((e) => e.name), + }})); + """ + return _node(tmp_path, pipeline, body) + + +def _photo_albums(tmp_path, pipeline, entries, salt="reader", local=()): + """What the photo album grid ends up with, after the merge and PhotosApp's + own regrouping.""" + body = f""" + const raw = {json.dumps(entries)}; + const local = new Set({json.dumps(list(local))}); + const merged = mergeUnitEntries(photoUnits(raw), {{ + salt: {json.dumps(salt)}, + isLocal: (g) => local.has(g), + }}); + console.log(JSON.stringify( + groupPhotoAlbums(merged, SEARCH_PHOTO_ROOTS).map((a) => ({{ + dir: a.dir, + groups: [...new Set(a.photos.map((e) => e.groupId))].sort(), + photos: a.photos.map((e) => e.name), + }})))); + """ + return _node(tmp_path, pipeline, body) + + +# 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"] + + +# ── Music ──────────────────────────────────────────────────────────────────── + +def _record(group, tracks=5): + """One album, tagged, under the search audio root.""" + return [{ + "id": f"t{n}", "name": f"{n:02d}-track.flac", "type": "audio", + "path": "__search__/music/some-band/a-record", "size": 1, + "artist": "Some Band", "album": "A Record", "track_no": n, + "groupId": group, "groupName": group.upper(), "groupOwner": "someone", + "_tRef": f"t:{group}", "_gRef": f"g:{group}", "_connGen": 1, + } for n in range(1, tracks + 1)] + + +def test_an_album_shared_by_two_groups_lists_each_track_once(tmp_path, pipeline): + grid = _albums(tmp_path, pipeline, _record("demo35") + _record("media")) + assert len(grid["albums"]) == 1 + album = grid["albums"][0] + assert len(album["tracks"]) == 5, ( + "a track is listed more than once — the same bug as the Videos view, " + "inside an album") + assert album["groups"] == ["demo35"] or album["groups"] == ["media"] + + +def test_one_group_of_music_is_unchanged(tmp_path, pipeline): + grid = _albums(tmp_path, pipeline, _record("solo")) + assert len(grid["albums"]) == 1 + assert len(grid["albums"][0]["tracks"]) == 5 + assert grid["albums"][0]["groups"] == ["solo"] + + +def test_a_differently_cased_tag_does_not_split_the_unit(tmp_path, pipeline): + """ + `groupMusicEntries` folds case for grouping but keeps the first-seen + spelling for display, and which group is seen first is whichever index + arrived first. `musicUnits` keys on the folded form for exactly that + reason — on the display strings, the chosen source could change between + page loads. + """ + other = _record("media") + for track in other: + track["artist"] = "SOME BAND" + track["album"] = "a record" + grid = _albums(tmp_path, pipeline, _record("demo35") + other) + assert len(grid["albums"]) == 1 + assert len(grid["albums"][0]["tracks"]) == 5 + + +def test_a_track_only_one_group_has_is_kept(tmp_path, pipeline): + extra = _record("media") + extra.append({ + "id": "t9", "name": "09-bonus.flac", "type": "audio", + "path": "__search__/music/some-band/a-record", "size": 1, + "artist": "Some Band", "album": "A Record", "track_no": 9, + "groupId": "media", "groupName": "MEDIA", "groupOwner": "someone", + }) + grid = _albums(tmp_path, pipeline, _record("demo35") + extra) + assert len(grid["albums"][0]["tracks"]) == 6 + + +def test_an_untagged_track_is_its_own_unit(tmp_path, pipeline): + """A track with no artist at all never reaches an album bucket, so it has + to be a unit of its own or it would be dropped from the merge entirely.""" + loose = [{ + "id": "x1", "name": "unknown.mp3", "type": "audio", + "path": "__search__/music", "size": 1, + "groupId": g, "groupName": g.upper(), + } for g in ("demo35", "media")] + grid = _albums(tmp_path, pipeline, _record("demo35") + loose) + assert grid["loose"] == ["unknown.mp3"] + + +# ── Photos ─────────────────────────────────────────────────────────────────── + +def _photos(group, root="pics", album="a-trip", n=4): + return [{ + "id": f"p{i}", "name": f"IMG_{i:04d}.jpg", "type": "image", + "path": f"__search_photos__/{root}/{album}", "size": 1, + "groupId": group, "groupName": group.upper(), "groupOwner": "someone", + "_tRef": f"t:{group}", "_gRef": f"g:{group}", "_connGen": 1, + } for i in range(1, n + 1)] + + +def test_a_photo_album_shared_by_two_groups_shows_each_photo_once(tmp_path, pipeline): + albums = _photo_albums(tmp_path, pipeline, _photos("demo35") + _photos("media")) + assert len(albums) == 1 + assert len(albums[0]["photos"]) == 4 + assert len(albums[0]["groups"]) == 1 + + +def test_two_differently_named_albums_both_keep_the_photo(tmp_path, pipeline): + """ + Documented consequence, not a bug: photo albums are keyed by directory, so + two groups whose roots have different basenames are two albums, and a + photo in both belongs in both. + """ + albums = _photo_albums( + tmp_path, pipeline, _photos("demo35", root="pics") + _photos("media", root="images")) + assert len(albums) == 2 + assert all(len(a["photos"]) == 4 for a in albums) + + +def test_one_group_of_photos_is_unchanged(tmp_path, pipeline): + albums = _photo_albums(tmp_path, pipeline, _photos("solo")) + assert len(albums) == 1 + assert albums[0]["groups"] == ["solo"] + assert len(albums[0]["photos"]) == 4 diff --git a/packages/meshbay-hub/tests/test_search_source_merge.py b/packages/meshbay-hub/tests/test_search_source_merge.py index 078c38d..cadf095 100644 --- a/packages/meshbay-hub/tests/test_search_source_merge.py +++ b/packages/meshbay-hub/tests/test_search_source_merge.py @@ -328,13 +328,35 @@ def test_source_label(tmp_path, module_source): _sources: [{ groupId: 'g1', groupName: 'G1' }] }; const many = { groupId: 'g1', groupName: 'G1', _sources: [{ groupId: 'g1' }, { groupId: 'g2' }] }; + const unmerged = { groupId: 'g9', groupName: 'G9' }; const bare = { }; console.log(JSON.stringify( - [one, many, bare].map(sourceLabel))); + [one, many, unmerged, bare].map((e) => sourceLabel(e)))); """ - one, many, bare = _run(tmp_path, module_source, body) + one, many, unmerged, 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} + # An entry that never went through the merge is its own single source. + assert unmerged == {"count": 1, "name": "G9", "groupId": "g9"} + # The single-group Group page, where entries carry no group at all. + assert bare == {"count": 0, "name": "", "groupId": None} + + +def test_source_label_counts_the_unit_not_the_cover(tmp_path, module_source): + """ + A card stands for a unit; the entry it is drawn from is one file. A show's + poster entry is chosen for its *thumbnail*, so a show in two groups whose + cover episode sits in only one of them would have claimed a single source. + """ + body = """ + const cover = { id: 'e1', groupId: 'aa', groupName: 'AA', + _sources: [{ groupId: 'aa', groupName: 'AA' }] }; + const rest = { id: 'e2', groupId: 'aa', groupName: 'AA', + _sources: [{ groupId: 'aa' }, { groupId: 'bb' }] }; + console.log(JSON.stringify( + [sourceLabel(cover), sourceLabel([cover, rest])])); + """ + alone, unit = _run(tmp_path, module_source, body) + assert alone["count"] == 1 + assert unit["count"] == 2 diff --git a/packages/meshbay-hub/tests/test_search_video_merge.py b/packages/meshbay-hub/tests/test_search_video_merge.py deleted file mode 100644 index e9ab258..0000000 --- a/packages/meshbay-hub/tests/test_search_video_merge.py +++ /dev/null @@ -1,185 +0,0 @@ -""" -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"] -- cgit v1.2.3