diff --git a/modules/zim/lib.py b/modules/zim/lib.py index dc848fb4..d60d2ce8 100644 --- a/modules/zim/lib.py +++ b/modules/zim/lib.py @@ -595,7 +595,7 @@ async def restart_kiwix(): if DOCKERIZED: logger.info('Restarting Kiwix via controller API') try: - async with aiohttp_post(f'{CONTROLLER_SERVICE}/api/services/zim/restart', + async with aiohttp_post(f'{CONTROLLER_SERVICE}/api/services/zim/restart', None, timeout=Seconds.minute) as response: if response.status == 200: logger.info('Successfully restarted Kiwix container') diff --git a/wrolpi/files/test/test_worker.py b/wrolpi/files/test/test_worker.py index ad703573..b44a338f 100644 --- a/wrolpi/files/test/test_worker.py +++ b/wrolpi/files/test/test_worker.py @@ -1388,6 +1388,81 @@ async def test_upsert_file_groups_tracks_actual_db_progress(async_client, test_s assert len(upserting_updates) > 0 +@pytest.mark.asyncio +async def test_compare_file_groups_duplicate_deleted_filegroups(test_session, test_directory, make_files_structure): + """When multiple FileGroups share the same (directory, stem) and all files are deleted, + all duplicates should appear in the deleted diffs — not just the last one processed.""" + make_files_structure([ + 'docs/file1.txt', + 'docs/file1.json', + ]) + + docs_dir = str(test_directory / 'docs') + + # Create 3 separate FileGroups for the same (directory, stem) — simulating a botched move. + # Use different primary_paths (unique constraint) but same directory and stem. + fg1 = FileGroup(directory=docs_dir, primary_path=str(test_directory / 'docs/file1.txt'), + files=[{'path': 'file1.txt', 'mimetype': 'text/plain', 'size': 0}]) + fg2 = FileGroup(directory=docs_dir, primary_path=str(test_directory / 'docs/file1.json'), + files=[{'path': 'file1.json', 'mimetype': 'application/json', 'size': 0}]) + # Third ghost with empty files list — use a unique primary_path + (test_directory / 'docs/file1.bak').touch() + fg3 = FileGroup(directory=docs_dir, primary_path=str(test_directory / 'docs/file1.bak'), + files=[]) + test_session.add_all([fg1, fg2, fg3]) + test_session.commit() + + fg1_id, fg2_id, fg3_id = fg1.id, fg2.id, fg3.id + + # Delete all files from disk + for f in (test_directory / 'docs').iterdir(): + f.unlink() + + result = await compare_file_groups(test_directory) + + # ALL 3 FileGroup IDs must appear in deleted diffs + deleted_ids = {d.file_group_id for d in result.deleted} + assert fg1_id in deleted_ids, f'fg1 ({fg1_id}) missing from deleted: {deleted_ids}' + assert fg2_id in deleted_ids, f'fg2 ({fg2_id}) missing from deleted: {deleted_ids}' + assert fg3_id in deleted_ids, f'fg3 ({fg3_id}) missing from deleted: {deleted_ids}' + assert len(result.new) == 0 + + +@pytest.mark.asyncio +async def test_compare_file_groups_duplicate_keeps_one(test_session, test_directory, make_files_structure): + """When duplicate FileGroups exist and files are on disk, one should be kept (modified/unchanged) + and the others should be marked for deletion.""" + make_files_structure([ + 'docs/file1.txt', + ]) + + docs_dir = str(test_directory / 'docs') + + # Create 2 FileGroups with same (directory, stem) + fg1 = FileGroup(directory=docs_dir, primary_path=str(test_directory / 'docs/file1.txt'), + files=[{'path': 'file1.txt', 'mimetype': 'text/plain', 'size': 0}]) + (test_directory / 'docs/file1.json').touch() + fg2 = FileGroup(directory=docs_dir, primary_path=str(test_directory / 'docs/file1.json'), + files=[{'path': 'file1.json', 'mimetype': 'application/json', 'size': 0}]) + test_session.add_all([fg1, fg2]) + test_session.commit() + + fg1_id, fg2_id = fg1.id, fg2.id + # Remove the .json so only .txt remains + (test_directory / 'docs/file1.json').unlink() + + result = await compare_file_groups(test_directory) + + # One FG should have the file on disk, the other should be deleted as duplicate + deleted_ids = {d.file_group_id for d in result.deleted} + all_diff_ids = {d.file_group_id for d in result.deleted + result.modified + result.unchanged} + assert fg1_id in all_diff_ids + assert fg2_id in all_diff_ids + # At least one must be in deleted (the duplicate) + assert len(deleted_ids) >= 1 + assert deleted_ids & {fg1_id, fg2_id} + + @pytest.mark.asyncio async def test_move_planning_tracks_progress(async_client, test_session, test_directory, make_files_structure): """Move planning phase tracks progress.""" diff --git a/wrolpi/files/worker.py b/wrolpi/files/worker.py index 834b0331..c59d36f4 100644 --- a/wrolpi/files/worker.py +++ b/wrolpi/files/worker.py @@ -11,7 +11,7 @@ import shlex import shutil import stat as stat_module import time -from collections import OrderedDict +from collections import OrderedDict, defaultdict from dataclasses import dataclass, field from enum import Enum, auto from multiprocessing.queues import Queue @@ -39,6 +39,10 @@ logger = logger.getChild(__name__) # Update status every N items to avoid excessive overhead PROGRESS_UPDATE_INTERVAL = 100 +# Maximum mtime difference (seconds) before a file is considered "modified". +# 0.5s accommodates SD card / FAT32 / exFAT timestamp granularity on Pi 4. +MTIME_TOLERANCE_SECONDS = 0.5 + __all__ = [ 'FileGroupDiff', 'FileComparisonResult', @@ -99,6 +103,31 @@ class FileComparisonResult: modified: list # FileGroupDiff where needs_update +def _deduplicate_db_groups( + db_groups: Dict[Tuple[str, str], List[Tuple[int, Set[str], float]]], +) -> Tuple[Dict[Tuple[str, str], Tuple[int, Set[str], float]], List[FileGroupDiff]]: + """Pick one FileGroup per (directory, stem) key, mark the rest for deletion. + + Prefers the FG that has files (non-empty), then highest ID as a stable tiebreaker. + """ + deduplicated: Dict[Tuple[str, str], Tuple[int, Set[str], float]] = {} + duplicate_diffs: List[FileGroupDiff] = [] + for key, entries in db_groups.items(): + # Sort: prefer non-empty files first, then highest ID + entries.sort(key=lambda e: (bool(e[1]), e[0]), reverse=True) + deduplicated[key] = entries[0] + for fg_id, fg_files, _ in entries[1:]: + directory, stem = key + duplicate_diffs.append(FileGroupDiff( + directory=Path(directory), + stem=stem, + db_files=fg_files or {'__empty__'}, + fs_files=set(), + file_group_id=fg_id, + )) + return deduplicated, duplicate_diffs + + def _diff_to_paths(diff: FileGroupDiff) -> list[pathlib.Path]: """Convert a FileGroupDiff to full file paths.""" directory = pathlib.Path(diff.directory) @@ -352,15 +381,9 @@ async def compare_file_groups( logger.info(f'Found {len(fs_groups)} file groups on filesystem') # Get all DB FileGroups within root directory (both with files and empty) - # Include modification_datetime for mtime comparison - db_groups: Dict[Tuple[str, str], Tuple[int, Set[str], float]] = {} # key -> (id, files, mtime_timestamp) + # Collect ALL FileGroups per (directory, stem) key — duplicates are deduped after. + db_groups: Dict[Tuple[str, str], List[Tuple[int, Set[str], float]]] = defaultdict(list) root_str = str(root) - - # Initialize result lists before the loop so empty FileGroups can be processed inline - unchanged = [] - new = [] - deleted = [] - modified = [] empty_count = 0 result = session.execute(text(""" @@ -383,41 +406,34 @@ async def compare_file_groups( for row in result: if row.is_empty: - # Process empty FileGroup immediately (no intermediate list) empty_count += 1 stem, _ = split_path_stem_and_suffix(Path(row.primary_path)) key = (row.directory, stem) - fs_files = fs_groups.pop(key, set()) # Remove from fs_groups to avoid double-processing - diff = FileGroupDiff( - directory=Path(row.directory), - stem=stem, - db_files=set(), - fs_files=fs_files, - file_group_id=row.id, - ) - if fs_files: - # Files exist on disk but not in DB - update the FileGroup - modified.append(diff) - else: - # No files on disk and no files in DB - delete the FileGroup - deleted.append(diff) + db_groups[key].append((row.id, set(), row.mtime_epoch or 0)) elif row.files: - # Compute stem from first file (all files in group share stem) first_file = row.files[0] stem, _ = split_path_stem_and_suffix(Path(row.directory) / first_file) - db_groups[(row.directory, stem)] = (row.id, set(row.files), row.mtime_epoch or 0) + key = (row.directory, stem) + db_groups[key].append((row.id, set(row.files), row.mtime_epoch or 0)) logger.info(f'Found {len(db_groups)} file groups in database') if empty_count: logger.debug(f'Found {empty_count} FileGroups with empty files arrays') - # Recompute all_keys after removing empty FileGroup keys from fs_groups - all_keys = set(fs_groups.keys()) | set(db_groups.keys()) + # Deduplicate: keep one FileGroup per key, mark the rest for deletion + deduplicated, duplicate_diffs = _deduplicate_db_groups(db_groups) + + unchanged = [] + new = [] + deleted = list(duplicate_diffs) + modified = [] + + all_keys = set(fs_groups.keys()) | set(deduplicated.keys()) for key in all_keys: directory, stem = key fs_files = fs_groups.get(key, set()) - db_id, db_files, db_mtime = db_groups.get(key, (None, set(), 0)) + db_id, db_files, db_mtime = deduplicated.get(key, (None, set(), 0)) diff = FileGroupDiff( directory=Path(directory), @@ -443,13 +459,16 @@ async def compare_file_groups( for filename in fs_files ) # If filesystem mtime is newer than DB mtime, content changed - if fs_mtime > float(db_mtime) + 0.001: # small tolerance for float comparison + if fs_mtime > float(db_mtime) + MTIME_TOLERANCE_SECONDS: modified.append(diff) else: unchanged.append(diff) except (OSError, ValueError): # If we can't stat files, treat as unchanged unchanged.append(diff) + elif db_id is not None and not fs_files and not db_files: + # Empty FileGroup with no files on disk — orphan, delete it + deleted.append(diff) logger.info( f'Comparison complete: {len(unchanged)} unchanged, {len(new)} new, ' @@ -1683,7 +1702,8 @@ class FileWorker: # Query existing FileGroups that might match our files directories = list(set(str(f.parent) for f in all_fs_files)) - db_groups: Dict[Tuple[str, str], Tuple[int, Set[str], float]] = {} + # Collect ALL FileGroups per (directory, stem) key — duplicates are deduped after. + db_groups: Dict[Tuple[str, str], List[Tuple[int, Set[str], float]]] = defaultdict(list) with get_db_session() as session: # Query FileGroups in our directories @@ -1692,75 +1712,78 @@ class FileWorker: for fg in file_groups: if not fg.files: - # Empty FileGroup - compute stem from primary_path stem, _ = split_path_stem_and_suffix(fg.primary_path) key = (fg.directory, stem) if key in fs_groups: mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 - db_groups[key] = (fg.id, set(), mtime) + db_groups[key].append((fg.id, set(), mtime)) else: - # Compute stem from first file first_file = fg.files[0]['path'] stem, _ = split_path_stem_and_suffix(pathlib.Path(fg.directory) / first_file) key = (fg.directory, stem) - - # Include this FileGroup if its stem matches our targets if key in fs_groups: fg_filenames = {pathlib.Path(f['path']).name for f in fg.files} mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 - db_groups[key] = (fg.id, fg_filenames, mtime) + db_groups[key].append((fg.id, fg_filenames, mtime)) + + # Track FG IDs already loaded by the directory query to avoid double-appending + seen_fg_ids: set[int] = set() + for entries in db_groups.values(): + for fg_id, _, _ in entries: + seen_fg_ids.add(fg_id) # Also query for FileGroups by primary_path for deleted files # This handles cases where the file was deleted and no related files exist if deleted_paths: deleted_path_strs = [str(p) for p in deleted_paths] deleted_fgs = session.query(FileGroup).filter( - FileGroup.primary_path.in_(deleted_path_strs) + FileGroup.primary_path.in_(deleted_path_strs), + ~FileGroup.id.in_(seen_fg_ids) if seen_fg_ids else True, ).all() for fg in deleted_fgs: + seen_fg_ids.add(fg.id) stem, _ = split_path_stem_and_suffix(fg.primary_path) key = (fg.directory, stem) - if key not in db_groups: - fg_filenames = {pathlib.Path(f['path']).name for f in fg.files} if fg.files else set() - mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 - db_groups[key] = (fg.id, fg_filenames, mtime) - # Ensure the key is in fs_groups (with empty set for deleted) - if key not in fs_groups: - fs_groups[key] = set() + fg_filenames = {pathlib.Path(f['path']).name for f in fg.files} if fg.files else set() + mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 + db_groups[key].append((fg.id, fg_filenames, mtime)) + if key not in fs_groups: + fs_groups[key] = set() # Also query for FileGroups contained in deleted directories - # This handles cases where a directory was deleted with files inside for deleted_path in deleted_paths: deleted_path_str = str(deleted_path) dir_deleted_fgs = session.query(FileGroup).filter( or_( FileGroup.directory == deleted_path_str, FileGroup.directory.like(f'{deleted_path_str}/%') - ) + ), + ~FileGroup.id.in_(seen_fg_ids) if seen_fg_ids else True, ).all() for fg in dir_deleted_fgs: + seen_fg_ids.add(fg.id) stem, _ = split_path_stem_and_suffix(fg.primary_path) key = (fg.directory, stem) - if key not in db_groups: - fg_filenames = {pathlib.Path(f['path']).name for f in fg.files} if fg.files else set() - mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 - db_groups[key] = (fg.id, fg_filenames, mtime) - # Ensure the key is in fs_groups (with empty set for deleted) - if key not in fs_groups: - fs_groups[key] = set() + fg_filenames = {pathlib.Path(f['path']).name for f in fg.files} if fg.files else set() + mtime = fg.modification_datetime.timestamp() if fg.modification_datetime else 0 + db_groups[key].append((fg.id, fg_filenames, mtime)) + if key not in fs_groups: + fs_groups[key] = set() + + # Deduplicate: keep one FileGroup per key, mark the rest for deletion + deduplicated, duplicate_diffs = _deduplicate_db_groups(db_groups) - # Build diffs by comparing fs_groups with db_groups unchanged = [] new = [] - deleted = [] + deleted = list(duplicate_diffs) modified = [] - all_keys = set(fs_groups.keys()) | set(db_groups.keys()) + all_keys = set(fs_groups.keys()) | set(deduplicated.keys()) for key in all_keys: directory, stem = key fs_files = fs_groups.get(key, set()) - db_id, db_files, db_mtime = db_groups.get(key, (None, set(), 0)) + db_id, db_files, db_mtime = deduplicated.get(key, (None, set(), 0)) diff = FileGroupDiff( directory=Path(directory), @@ -1784,12 +1807,15 @@ class FileWorker: (dir_path / filename).stat().st_mtime for filename in fs_files ) - if fs_mtime > db_mtime + 1: + if fs_mtime > db_mtime + MTIME_TOLERANCE_SECONDS: modified.append(diff) else: unchanged.append(diff) except (OSError, ValueError): unchanged.append(diff) + elif db_id is not None and not fs_files and not db_files: + # Empty FileGroup with no files on disk — orphan, delete it + deleted.append(diff) logger.info( f'Direct file refresh: {len(unchanged)} unchanged, {len(new)} new, '