diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/indexer/indexer.py | 66 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_replug_restores_enrichment.py | 256 |
2 files changed, 232 insertions, 90 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/indexer/indexer.py b/packages/meshbay-node/src/meshbay_node/indexer/indexer.py index eea5b4f..33e7210 100644 --- a/packages/meshbay-node/src/meshbay_node/indexer/indexer.py +++ b/packages/meshbay-node/src/meshbay_node/indexer/indexer.py @@ -609,8 +609,7 @@ class DirectoryIndexer: for root, available in changed: if available: log.info("Root %r is back — rescanning", root.name) - self._drop_root_entries(root) - await self._scan_root(root) + await self._rescan_root(root) touched = True else: # Frozen: entries stay, marked unavailable to members through @@ -711,17 +710,61 @@ class DirectoryIndexer: return [e for e in self._index.entries if fold(e.path).split("/", 1)[0] == prefix] + # Everything on an IndexEntry that a scan does not produce. `_scan_root` + # fills id/name/path/size/type/added_at/hash_version from the file itself; + # every field below was derived by one of the enrichment passes and is + # nowhere on disk to be read back. + _ENRICHED_FIELDS = ( + "duration", "thumb_hash", "width", "height", + "display_title", "season", "episode", + "artist", "album", "track_no", "taken_at", "camera", + "uploader_id", "uploader_pk", + ) + + async def _rescan_root(self, root: Root) -> int: + """ + Rebuild one root's entries from disk, keeping what the files still say. + + The two callers — `reconcile` when a root reappears, `plug_root` when + the operator plugs one back in — have to re-walk: the drive may have + changed while it was away. What they must not do is throw away the + enrichment. An entry's id is its content hash, so an entry that comes + back under the same id, name and path is the same bytes in the same + place, and every field the Videos, Music and Photos passes derived from + it still holds. Re-deriving them means minutes of tag reads, ffprobe + runs and rate-limited metadata lookups during which the operator's + library sits empty — which is exactly what a replug looked like. + + Anything that does *not* match is left bare on purpose: a different id + is different content, and a different name or path can change the + filename and folder fallbacks that `display_title`, `track_no`, + `artist` and `album` fall back to. Those are the entries + `daemon._broadcast_index_change` re-enriches, off `rescanned_ids`. + """ + carried = {(e.id, e.name, e.path): e for e in self._entries_under(root)} + self._drop_root_entries(root) + count = await self._scan_root(root) + for entry in self._entries_under(root): + old = carried.get((entry.id, entry.name, entry.path)) + if old is None: + continue + for field in self._ENRICHED_FIELDS: + setattr(entry, field, getattr(old, field)) + # It came back intact, so it is not one of the entries the daemon + # needs to enrich again. + self.rescanned_ids.discard(entry.id) + return count + def _drop_root_entries(self, root: Root) -> None: """ - Throw away a root's entries, always in order to rescan it. + Throw away a root's entries. Only ever called to rebuild them. - Both callers — `reconcile` when a root reappears, `plug_root` when the - operator plugs one back in — rebuild immediately, so nothing outside - ever observes the gap: no deletion is broadcast, and the entries that - come back have the same content-hash ids they had before. What they do - not have is anything enrichment put on them, which is why the ids are - recorded for `daemon._broadcast_index_change` to re-enrich rather than - simply forgotten. + The ids are recorded because nothing outside can otherwise tell they + were rebuilt: no deletion is broadcast (the rescan is immediate) and + the entries come back under the same content-hash ids, so a diff + against the last broadcast reports neither an addition nor a deletion. + `_rescan_root` clears the ones it managed to carry over intact; what is + left is genuinely new to the apps and is re-enriched by the daemon. """ for entry in self._entries_under(root): self.rescanned_ids.add(entry.id) @@ -794,8 +837,7 @@ class DirectoryIndexer: root.available = root.is_live() if root.available: log.info("Root %r plugged — rescanning", root.name) - self._drop_root_entries(root) - await self._scan_root(root) + await self._rescan_root(root) self._restart_observer() self._index.roots = self.roots.describe() self._index.version = int(time.time()) diff --git a/packages/meshbay-node/tests/test_replug_restores_enrichment.py b/packages/meshbay-node/tests/test_replug_restores_enrichment.py index 23fc97e..04a06ae 100644 --- a/packages/meshbay-node/tests/test_replug_restores_enrichment.py +++ b/packages/meshbay-node/tests/test_replug_restores_enrichment.py @@ -1,36 +1,44 @@ """ A root that comes back keeps its Videos/Music/Photos metadata. -Reported live against a real library: a removable root ejected from the -Files app and plugged back in came back with its files but *without* its -albums — Music showed "no music found" and stayed that way through a force -reload, because the loss was on the node, not in the client. +Reported live: a removable root ejected from the Files app and plugged back +in returned with its files and without its albums. Music showed "no music +found", and it did not come back. -`DirectoryIndexer.plug_root` drops the root's entries and rescans, which is -right: the drive may have changed while it was away. What it produces is -bare `IndexEntry` objects — `_hash_or_cached` fills id/name/path/size/type -and nothing else. Every enrichment field (`artist`, `album`, `track_no`, -`duration`, `thumb_hash`, `display_title`, `taken_at`, ...) is gone, and -unlike the video/photo probe results those tag fields are cached nowhere: -`enrich_audio.py` deliberately re-reads them each time so a rename can -re-derive the filename fallback. Re-enrichment is the only way back. +`plug_root` has to re-walk the root — the drive may have changed while it +was away — and `_scan_root` produces bare entries: `_hash_or_cached` fills +id/name/path/size/type and nothing else. Every enrichment field went with +the old object, and the Music tag fields are cached nowhere by design +(`enrich_audio.py` re-reads them so a rename can re-derive the filename +fallback). -Two independent gates then made sure it never ran: +Two gates then stopped anything from filling them in again: -* `_broadcast_index_change` schedules enrichment for `delta.additions`. - Ejecting never broadcasts, so `_last_broadcast_snapshot` still held those - ids — the re-added entries diffed as *updates*, not additions. -* `_enrich_new_*_entries` skips anything already in `_enriched_attempted`, - which is only discarded for `delta.deletions`. Dropping and rescanning - inside one call means no deletion is ever broadcast, so the mark survived - a wipe of the very fields it was standing for. +* enrichment is scheduled for `delta.additions`, and ejecting broadcasts + nothing — the last snapshot still held those ids, so the rebuilt entries + diffed as *updates*; +* `_enrich_new_*_entries` skips anything in `_enriched_attempted`, which is + only discarded for `delta.deletions` — and dropping and rescanning inside + one call broadcasts no deletion either. -A restart cleared both (an empty snapshot makes every entry an addition), -which is why this looked like it might fix itself and never did. +Only a restart cleared both, an empty snapshot making every entry an +addition. That is why it looked like it might fix itself and never did. + +Re-enriching is now the *fallback*, not the fix. An entry's id is its +content hash, so one that comes back under the same id, name and path is +the same bytes in the same place and its enrichment still holds: +`_rescan_root` carries those fields across. Re-deriving them instead meant +tag reads, ffprobe runs and rate-limited lookups — measured at 14 seconds +of empty Music tab on a real library with a cold cache, which to the +operator is indistinguishable from the original bug. + +What these assert is therefore the field on the entry, not a call to an +enricher. Counting calls is what made an earlier version of this file pass +while the operator still watched their albums vanish. The same drop-and-rescan runs in `reconcile()` — "Root %r is back" — so a -USB drive that falls off and returns on its own hits this without anybody -touching the UI. +USB drive that falls off and returns on its own hits all of this without +anybody touching the UI. """ import asyncio @@ -44,6 +52,7 @@ from meshbay_node.config import (Config, GroupConfig, HubConfig, KeystoreConfig, NodeConfig) from meshbay_node.daemon import NodeDaemon from meshbay_node.indexer import DirectoryIndexer +from meshbay_node.indexer.enrich_audio import AudioEnricher from meshbay_node.media_cache import MediaCache from meshbay_node.roots import RootSet from meshbay_node.roster import Roster @@ -62,7 +71,7 @@ def _free_port() -> int: class _CountingEnricher: - """Stands in for AudioEnricher: records who it was asked to enrich.""" + """Stands in for an enricher: records who it was asked to enrich.""" def __init__(self): self.spawned: list[str] = [] @@ -96,94 +105,177 @@ async def _settled(daemon, indexer): await asyncio.sleep(0.05) -async def test_a_replugged_root_gets_its_music_metadata_back(tmp_path): - group_id = "a" * 32 +async def _library(tmp_path, group_id, *, enricher=None): + """A one-track library under <root>/<artist>/<album>, enriched once.""" library = tmp_path / "music" - (library / "an album").mkdir(parents=True) - (library / "an album" / "track.mp3").write_bytes(_AUDIO_BYTES) + (library / "an artist" / "a record").mkdir(parents=True) + (library / "an artist" / "a record" / "01 first track.mp3").write_bytes(_AUDIO_BYTES) daemon = await _daemon(tmp_path, library, group_id) + daemon._audio_enricher = enricher or AudioEnricher(daemon._media_cache) + await daemon._roster.set_app_directories( + group_id, "music", ["music"], set_by="op") + + roots = RootSet.build([{"path": str(library), "removable": True}]) + indexer = DirectoryIndexer( + roots=roots, group_id=group_id, + sk_node=Ed25519PrivateKey.generate(), gek=generate_gek()) + await indexer.initial_scan() + await _settled(daemon, indexer) + await asyncio.sleep(0.4) # the enricher runs off the broadcast + return daemon, indexer, roots, library + + +def _only(indexer): + return next(iter(indexer.index.entries)) + + +# ── The operator's own eject and plug ──────────────────────────────────────── + +async def test_the_albums_are_still_there_after_a_replug(tmp_path): + """ + No tags are written: `enrich_audio._artist_album_from_ancestors` derives + artist and album from the folder names when a file has none, which is the + <library>/<artist>/<album>/<track> layout this was reported against. + """ + group_id = "a" * 32 + daemon, indexer, _, _ = await _library(tmp_path, group_id) + try: + before = _only(indexer) + assert before.album == "a record" and before.artist == "an artist", ( + f"the first pass never filled the fields: {before}") + + indexer.eject_root("music") + await indexer.plug_root("music") + await _settled(daemon, indexer) + + after = _only(indexer) + assert after.album == "a record" and after.artist == "an artist", ( + "the entry came back from the rescan with no album — this is what " + "an empty Music tab after a replug looks like on the node") + finally: + await daemon._media_cache.close() + await daemon._roster.close() + + +async def test_the_fields_survive_without_re_deriving_them(tmp_path): + """ + Carried across, not recomputed. Re-deriving is correct and far too slow: + on a real library with a cold metadata cache it left Music empty for 14 + seconds, and an operator who looks in that window sees the bug. + """ + group_id = "b" * 32 enricher = _CountingEnricher() - daemon._audio_enricher = enricher + daemon, indexer, _, _ = await _library(tmp_path, group_id, enricher=enricher) try: - await daemon._roster.set_app_directories( - group_id, "music", ["music/an album"], set_by="op") + assert enricher.spawned == ["01 first track.mp3"] - roots = RootSet.build([{"path": str(library), "removable": True}]) - indexer = DirectoryIndexer( - roots=roots, group_id=group_id, - sk_node=Ed25519PrivateKey.generate(), gek=generate_gek()) - await indexer.initial_scan() + indexer.eject_root("music") + await indexer.plug_root("music") await _settled(daemon, indexer) - assert enricher.spawned == ["track.mp3"], "the first pass never ran" - # The operator ejects the drive from Files, then plugs it back in. + assert enricher.spawned == ["01 first track.mp3"], ( + "an unchanged file was enriched a second time — the whole point " + "of the content hash is that it did not need to be") + finally: + await daemon._media_cache.close() + await daemon._roster.close() + + +async def test_who_uploaded_a_file_survives_it_too(tmp_path): + """ + `uploader_id`/`uploader_pk` are the same shape of field — set once, on an + entry, readable from nowhere on disk — and they decide who may delete the + file. Losing them to a replug quietly takes a right away. + """ + group_id = "c" * 32 + daemon, indexer, _, _ = await _library(tmp_path, group_id) + try: + entry = _only(indexer) + entry.uploader_id = "alice" + entry.uploader_pk = "a-pinned-key" + indexer.eject_root("music") await indexer.plug_root("music") await _settled(daemon, indexer) - entry = next(iter(indexer.index.entries)) - assert entry.artist is None and entry.album is None, ( - "the rescan is supposed to produce a bare entry — if this ever " - "stops being true the rest of this test is measuring nothing") - assert enricher.spawned == ["track.mp3", "track.mp3"], ( - "a replugged root came back with its files and without its " - "albums, and nothing was ever going to fill them in again") + after = _only(indexer) + assert after.uploader_id == "alice" and after.uploader_pk == "a-pinned-key" finally: await daemon._media_cache.close() await daemon._roster.close() +# ── A drive that leaves and returns on its own ────────────────────────────── + async def test_a_root_that_returns_on_its_own_is_treated_the_same(tmp_path): """ `reconcile()` rescans a root that reappears without anyone asking — a USB - drive re-mounting. It goes through the same drop-and-rescan, so it loses - the same fields, with no click anywhere to blame it on. + drive re-mounting. Same drop-and-rescan, so it lost the same fields, with + no click anywhere to blame it on. """ - group_id = "b" * 32 - library = tmp_path / "music" - (library / "an album").mkdir(parents=True) - (library / "an album" / "track.mp3").write_bytes(_AUDIO_BYTES) - - daemon = await _daemon(tmp_path, library, group_id) - enricher = _CountingEnricher() - daemon._audio_enricher = enricher + group_id = "d" * 32 + daemon, indexer, roots, _ = await _library(tmp_path, group_id) try: - await daemon._roster.set_app_directories( - group_id, "music", ["music/an album"], set_by="op") - - roots = RootSet.build([{"path": str(library), "removable": True}]) - indexer = DirectoryIndexer( - roots=roots, group_id=group_id, - sk_node=Ed25519PrivateKey.generate(), gek=generate_gek()) - await indexer.initial_scan() - await _settled(daemon, indexer) - assert enricher.spawned == ["track.mp3"] + assert _only(indexer).album == "a record" - # Gone, then back — availability is what reconcile() watches. roots.roots[0].available = False await indexer.reconcile() await _settled(daemon, indexer) await indexer.reconcile() await _settled(daemon, indexer) + await asyncio.sleep(0.4) - assert enricher.spawned.count("track.mp3") >= 2, ( + assert _only(indexer).album == "a record", ( "a drive that fell off and came back left the library with no " - "metadata until the next daemon restart") + "metadata") finally: await daemon._media_cache.close() await daemon._roster.close() -async def test_videos_and_photos_lost_the_same_fields(tmp_path): +# ── What genuinely does have to be re-derived ─────────────────────────────── + +async def test_a_track_moved_while_the_drive_was_away_is_enriched_again(tmp_path): """ - Nothing here is specific to Music — Videos and Photos hang off the same - `new_entries` list in `_broadcast_index_change`, so a replug took their - durations, titles and thumbnails with it too. Music is simply where it - shows up loudest: a track with no tags has no album to file it under, so - the app goes empty rather than merely plain. + The counter-case, and the reason the carry-over is keyed on name and path + as well as id. `artist`, `album`, `display_title` and `track_no` all fall + back to the folder and filename when a file carries no tags, so the same + bytes under a new name are not the same metadata. Those are the entries + the daemon still re-enriches, off `rescanned_ids`. """ - group_id = "c" * 32 + group_id = "e" * 32 + enricher = _CountingEnricher() + daemon, indexer, _, library = await _library( + tmp_path, group_id, enricher=enricher) + try: + assert enricher.spawned == ["01 first track.mp3"] + + moved = library / "another artist" / "another record" + moved.mkdir(parents=True) + (library / "an artist" / "a record" / "01 first track.mp3").rename( + moved / "01 first track.mp3") + + indexer.eject_root("music") + await indexer.plug_root("music") + await _settled(daemon, indexer) + + assert enricher.spawned == ["01 first track.mp3"] * 2, ( + "the file is under a different artist and album now; carrying the " + "old ones across would file it under a folder it left") + finally: + await daemon._media_cache.close() + await daemon._roster.close() + + +async def test_videos_and_photos_are_covered_by_the_same_path(tmp_path): + """ + Nothing here is specific to Music — Videos and Photos lost their durations, + titles and thumbnails the same way. Music is simply where it shows up + loudest: a track with no tags has no album to file it under, so the app + goes empty rather than merely plain. + """ + group_id = "f" * 32 library = tmp_path / "media" (library / "films").mkdir(parents=True) (library / "films" / "clip.mkv").write_bytes(os.urandom(60 * 1024)) @@ -207,12 +299,20 @@ async def test_videos_and_photos_lost_the_same_fields(tmp_path): await _settled(daemon, indexer) assert video.spawned == ["clip.mkv"] and photo.spawned == ["shot.jpg"] + by_name = {e.name: e for e in indexer.index.entries} + by_name["clip.mkv"].duration = 1234 + by_name["shot.jpg"].thumb_hash = "a-thumbnail" + indexer.eject_root("media") await indexer.plug_root("media") await _settled(daemon, indexer) - assert video.spawned == ["clip.mkv"] * 2, "the film lost its probe" - assert photo.spawned == ["shot.jpg"] * 2, "the photo lost its thumbnail" + back = {e.name: e for e in indexer.index.entries} + assert back["clip.mkv"].duration == 1234, "the film lost its probe" + assert back["shot.jpg"].thumb_hash == "a-thumbnail", ( + "the photo lost its thumbnail") + assert video.spawned == ["clip.mkv"] and photo.spawned == ["shot.jpg"], ( + "unchanged files were probed and thumbnailed all over again") finally: await daemon._media_cache.close() await daemon._roster.close() |