mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-25 18:40:06 -04:00
files: keep tags when a sidecar joins an unmodeled group
choose_primary_file is the single fallback for unmodeled stem groups so _upsert_files and _upsert_file_groups cannot pick different primaries and DELETE the tagged row. FileGroup.move merges an existing (directory, stem) group and flushes the delete before taking that stem. refresh_files_directly deletes before upsert, matching the other two callers.
This commit is contained in:
Roland
parent
507b3cf1d1
commit
754cda2d49
4 changed files with 111 additions and 44 deletions
|
|
@ -52,7 +52,7 @@ except ImportError:
|
|||
logger = logger.getChild(__name__)
|
||||
|
||||
__all__ = ['list_directories_contents', 'delete', 'split_path_stem_and_suffix', 'search_files',
|
||||
'get_mimetype', 'split_file_name_words', 'get_primary_file', 'get_file_statistics',
|
||||
'get_mimetype', 'split_file_name_words', 'get_primary_file', 'choose_primary_file', 'get_file_statistics',
|
||||
'search_file_suggestion_count', 'glob_shared_stem', 'upsert_file', 'get_unique_files_by_stem',
|
||||
'rename', 'delete_directory', 'handle_file_group_search_results', 'get_file_location_href',
|
||||
'get_tagged_file_groups_by_ids', 'delete_file_groups', 'get_special_directories',
|
||||
|
|
@ -685,6 +685,22 @@ def get_primary_file(files: Union[Tuple[pathlib.Path], Iterable[pathlib.Path]])
|
|||
raise NoPrimaryFile(f'Cannot find primary file for group: {files}')
|
||||
|
||||
|
||||
def choose_primary_file(files: Union[Tuple[pathlib.Path], Iterable[pathlib.Path]]) -> pathlib.Path:
|
||||
"""Primary file for a stem group. Falls back to a sorted path so every caller agrees.
|
||||
|
||||
`_upsert_files` and `_upsert_file_groups` must pick the same path. A set's iteration
|
||||
order is not stable; using it as the fallback would DELETE the existing tagged row
|
||||
and INSERT a new untagged one.
|
||||
"""
|
||||
paths = [pathlib.Path(p) for p in files]
|
||||
if not paths:
|
||||
raise ValueError('Cannot find primary file without any files')
|
||||
try:
|
||||
return get_primary_file(paths)
|
||||
except NoPrimaryFile:
|
||||
return sorted(paths)[0]
|
||||
|
||||
|
||||
def _upsert_files(files: List[pathlib.Path],
|
||||
progress_callback: Callable[[int, int], None] = None):
|
||||
"""Insert/update all records of the provided files.
|
||||
|
|
@ -709,33 +725,18 @@ def _upsert_files(files: List[pathlib.Path],
|
|||
for group in grouped:
|
||||
# The primary file is the video/SingleFile/epub, etc.
|
||||
try:
|
||||
try:
|
||||
primary_path = get_primary_file(group)
|
||||
# Multiple files in this group.
|
||||
primary_path: pathlib.Path = primary_path
|
||||
# The primary mimetype allows modelers to find its file_groups.
|
||||
mimetype = get_mimetype(primary_path)
|
||||
# The group uses a common modification_datetime so the group will be re-indexed when any of it's
|
||||
# files are modified.
|
||||
modification_datetime = from_timestamp(max(i.stat().st_mtime for i in group))
|
||||
size = sum(i.stat().st_size for i in group)
|
||||
files_json = json.dumps(_paths_to_files_dict(group))
|
||||
stem, suffix = split_path_stem_and_suffix(primary_path)
|
||||
suffix = (suffix or '').lower() or None
|
||||
values[primary_path] = (modification_datetime, mimetype, size, files_json, suffix, stem)
|
||||
non_primary_files.update(i for i in group if i != primary_path)
|
||||
except NoPrimaryFile:
|
||||
# Same stem still means one FileGroup. Pick a stable primary so
|
||||
# (directory, stem) stays unique even when nothing is modelable.
|
||||
primary_path = sorted(group)[0]
|
||||
mimetype = get_mimetype(primary_path)
|
||||
modification_datetime = from_timestamp(max(i.stat().st_mtime for i in group))
|
||||
size = sum(i.stat().st_size for i in group)
|
||||
files_json = json.dumps(_paths_to_files_dict(group))
|
||||
stem, suffix = split_path_stem_and_suffix(primary_path)
|
||||
suffix = (suffix or '').lower() or None
|
||||
values[primary_path] = (modification_datetime, mimetype, size, files_json, suffix, stem)
|
||||
non_primary_files.update(i for i in group if i != primary_path)
|
||||
primary_path = choose_primary_file(group)
|
||||
# The primary mimetype allows modelers to find its file_groups.
|
||||
mimetype = get_mimetype(primary_path)
|
||||
# The group uses a common modification_datetime so the group will be re-indexed when any of it's
|
||||
# files are modified.
|
||||
modification_datetime = from_timestamp(max(i.stat().st_mtime for i in group))
|
||||
size = sum(i.stat().st_size for i in group)
|
||||
files_json = json.dumps(_paths_to_files_dict(group))
|
||||
stem, suffix = split_path_stem_and_suffix(primary_path)
|
||||
suffix = (suffix or '').lower() or None
|
||||
values[primary_path] = (modification_datetime, mimetype, size, files_json, suffix, stem)
|
||||
non_primary_files.update(i for i in group if i != primary_path)
|
||||
except FileNotFoundError as e:
|
||||
# A file was deleted between discovery and upsert (temp files, WAL sidecars, etc.).
|
||||
refresh_logger.warning(f'File vanished during refresh, skipping group near {group[0]}', exc_info=e)
|
||||
|
|
|
|||
|
|
@ -518,13 +518,13 @@ class FileGroup(ModelHelper, Base):
|
|||
@classmethod
|
||||
def from_paths(cls, session: Session, *paths: pathlib.Path) -> 'FileGroup':
|
||||
"""Create a new FileGroup which contains the provided file paths."""
|
||||
from wrolpi.files.lib import get_primary_file, get_mimetype, sanitize_filename_surrogates, \
|
||||
from wrolpi.files.lib import choose_primary_file, get_mimetype, sanitize_filename_surrogates, \
|
||||
split_path_stem_and_suffix
|
||||
|
||||
# Sanitize any paths with invalid UTF-8 characters (renames files on disk if needed)
|
||||
paths = tuple(sanitize_filename_surrogates(p) for p in paths)
|
||||
|
||||
primary_path = get_primary_file(paths) if paths else None
|
||||
primary_path = choose_primary_file(paths) if paths else None
|
||||
stem = split_path_stem_and_suffix(primary_path)[0] if primary_path else None
|
||||
directory = str(primary_path.parent) if primary_path else None
|
||||
filters = [FileGroup.primary_path.in_(list(map(str, paths)))]
|
||||
|
|
@ -538,7 +538,7 @@ class FileGroup(ModelHelper, Base):
|
|||
if len(existing_groups) == 0:
|
||||
# These paths have not been used previously, create a new FileGroup.
|
||||
file_group = FileGroup()
|
||||
primary_path = get_primary_file(paths)
|
||||
primary_path = choose_primary_file(paths)
|
||||
file_group.primary_path = primary_path
|
||||
file_group.append_files(*paths)
|
||||
session.add(file_group)
|
||||
|
|
@ -546,10 +546,10 @@ class FileGroup(ModelHelper, Base):
|
|||
# Found one FileGroup with these paths, no need to create a new FileGroup.
|
||||
file_group = existing_groups[0]
|
||||
file_group.append_files(*paths)
|
||||
primary_path = get_primary_file(file_group.my_paths())
|
||||
primary_path = choose_primary_file(file_group.my_paths())
|
||||
else:
|
||||
# Multiple FileGroups contain these paths as primary.
|
||||
primary_path = get_primary_file(paths)
|
||||
primary_path = choose_primary_file(paths)
|
||||
file_group: FileGroup = next(filter(lambda i: i.primary_path == primary_path, existing_groups), None)
|
||||
if not file_group:
|
||||
file_group = FileGroup.from_paths(session, primary_path)
|
||||
|
|
@ -699,15 +699,33 @@ class FileGroup(ModelHelper, Base):
|
|||
|
||||
new_name, _ = split_path_stem_and_suffix(new_primary_path, full=True)
|
||||
new_directory = new_primary_path.parent
|
||||
new_stem, _ = split_path_stem_and_suffix(new_primary_path)
|
||||
|
||||
# Unique (directory, stem): merge into the destination group before touching disk.
|
||||
# Otherwise shutil.move succeeds, then commit hits IntegrityError and disk/DB disagree.
|
||||
session = Session.object_session(self)
|
||||
if session is not None:
|
||||
existing = session.query(FileGroup).filter(
|
||||
FileGroup.directory == str(new_directory),
|
||||
FileGroup.stem == new_stem,
|
||||
FileGroup.id != self.id,
|
||||
).one_or_none()
|
||||
if existing is not None:
|
||||
self.merge([existing])
|
||||
# Flush the DELETE first. Otherwise SQLAlchemy UPDATEs our stem before
|
||||
# removing the other row and hits UNIQUE (directory, stem).
|
||||
session.flush()
|
||||
|
||||
# Get current files with resolved absolute paths for moving
|
||||
current_files = self.my_files()
|
||||
|
||||
# Ensure that all destination files do not yet exist before move.
|
||||
# After merging a same-stem group, some dest paths are the files we already own.
|
||||
for file in current_files:
|
||||
_, suffix = split_path_stem_and_suffix(file['path'])
|
||||
old_path = file['path']
|
||||
_, suffix = split_path_stem_and_suffix(old_path)
|
||||
new_path = pathlib.Path(f'{new_name}{suffix}')
|
||||
if new_path.exists():
|
||||
if new_path.exists() and new_path != old_path:
|
||||
raise FileExistsError(f'Cannot move {self} to {new_path} because it already exists.')
|
||||
|
||||
# Move physical files and collect new relative file entries
|
||||
|
|
@ -717,7 +735,8 @@ class FileGroup(ModelHelper, Base):
|
|||
_, suffix = split_path_stem_and_suffix(old_path)
|
||||
new_path = pathlib.Path(f'{new_name}{suffix}')
|
||||
if old_path.is_file():
|
||||
shutil.move(old_path, new_path)
|
||||
if old_path != new_path:
|
||||
shutil.move(old_path, new_path)
|
||||
# Store just the filename (relative path)
|
||||
new_files.append({'path': new_path.name, 'mimetype': file['mimetype']})
|
||||
|
||||
|
|
|
|||
|
|
@ -515,6 +515,58 @@ def test_file_group_directory_stem_is_unique(test_session, test_directory, make_
|
|||
test_session.commit()
|
||||
|
||||
|
||||
def test_choose_primary_file_is_sorted_and_set_stable(test_directory, make_files_structure):
|
||||
"""Unmodeled groups must pick the same primary regardless of set iteration order."""
|
||||
csv, txt = make_files_structure(['n.csv', 'n.txt'])
|
||||
assert lib.choose_primary_file([txt, csv]) == csv
|
||||
assert lib.choose_primary_file({txt, csv}) == csv
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_refresh_keeps_tag_when_sidecar_added(async_client, test_session, test_directory,
|
||||
make_files_structure, tag_factory, refresh_files):
|
||||
"""Adding a sidecar to a tagged unmodeled FileGroup must not drop the tag on refresh."""
|
||||
txt, = make_files_structure(['note.txt'])
|
||||
fg = FileGroup.from_paths(test_session, txt)
|
||||
test_session.commit()
|
||||
tag = await tag_factory('keep')
|
||||
fg.add_tag(test_session, tag.id)
|
||||
test_session.commit()
|
||||
fg_id = fg.id
|
||||
|
||||
(test_directory / 'note.csv').write_text('a,b\n')
|
||||
await refresh_files()
|
||||
test_session.expire_all()
|
||||
|
||||
remaining = test_session.query(FileGroup).filter_by(id=fg_id).one()
|
||||
assert remaining.tag_names == ['keep']
|
||||
names = {pathlib.Path(f['path']).name for f in remaining.files}
|
||||
assert names == {'note.txt', 'note.csv'}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rename_into_existing_stem_merges(async_client, test_session, test_directory, make_files_structure):
|
||||
"""Renaming onto another FileGroup's stem merges before moving files, so disk and DB agree."""
|
||||
make_files_structure({'foo.jpg': 'x', 'bar.mp4': 'video'})
|
||||
jpg = test_directory / 'foo.jpg'
|
||||
mp4 = test_directory / 'bar.mp4'
|
||||
FileGroup.from_paths(test_session, jpg)
|
||||
FileGroup.from_paths(test_session, mp4)
|
||||
test_session.commit()
|
||||
assert test_session.query(FileGroup).count() == 2
|
||||
assert {fg.primary_path.name for fg in test_session.query(FileGroup)} == {'foo.jpg', 'bar.mp4'}
|
||||
|
||||
await lib.rename_file(mp4, 'foo.mp4')
|
||||
test_session.expire_all()
|
||||
|
||||
remaining = test_session.query(FileGroup).one()
|
||||
names = {pathlib.Path(f['path']).name for f in remaining.files}
|
||||
assert names == {'foo.jpg', 'foo.mp4'}
|
||||
assert (test_directory / 'foo.mp4').is_file()
|
||||
assert (test_directory / 'foo.jpg').is_file()
|
||||
assert not (test_directory / 'bar.mp4').exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dedupe_file_groups_merges_tags_onto_winner(async_client, test_session, test_directory,
|
||||
make_files_structure, tag_factory):
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ from wrolpi.events import Events
|
|||
from wrolpi.vars import PYTEST, IS_MACOS
|
||||
from wrolpi.files.lib import (
|
||||
split_path_stem_and_suffix, _upsert_files, get_unique_files_by_stem, glob_shared_stem,
|
||||
group_files_by_stem, get_primary_file, delete_directory, apply_indexers,
|
||||
group_files_by_stem, get_primary_file, choose_primary_file, delete_directory, apply_indexers,
|
||||
_move_file_group_files, _bulk_update_file_groups_db, MOVE_CHUNK_SIZE,
|
||||
_bulk_update_file_groups_reorganize, get_normalized_ignored_directories, remove_files_in_ignored_directories,
|
||||
ensure_file_group_stems,
|
||||
|
|
@ -1273,8 +1273,8 @@ class FileWorker:
|
|||
try:
|
||||
result = await self._refresh_files_directly(paths)
|
||||
self._cleanup_modified_models(result.modified)
|
||||
await self._upsert_file_groups(result.new + result.modified)
|
||||
await self._delete_file_groups(result.deleted)
|
||||
await self._upsert_file_groups(result.new + result.modified)
|
||||
if post_processing and (result.new or result.modified or result.deleted):
|
||||
# Skip global modelers/indexers when the caller opts out (e.g. `lib.delete` from the
|
||||
# upload path defers that to the post-upload `upsert_file`) or when nothing changed.
|
||||
|
|
@ -1610,17 +1610,12 @@ class FileWorker:
|
|||
# For modified diffs where file_group_id exists, we need to handle primary_path changes.
|
||||
# If the primary file changed (e.g., video deleted, leaving only poster), the ON CONFLICT
|
||||
# won't match, so we need to update the existing FileGroup's primary_path first.
|
||||
from wrolpi.files.lib import NoPrimaryFile
|
||||
modified_ids_to_update = []
|
||||
for diff in diffs:
|
||||
if diff.file_group_id and diff.fs_files:
|
||||
# This is a modified FileGroup - may need to update primary_path
|
||||
new_paths = [diff.directory / f for f in diff.fs_files]
|
||||
try:
|
||||
new_primary = get_primary_file(new_paths)
|
||||
except NoPrimaryFile:
|
||||
# No clear primary file, just use the first one
|
||||
new_primary = new_paths[0] if new_paths else None
|
||||
new_primary = choose_primary_file(new_paths)
|
||||
if new_primary:
|
||||
modified_ids_to_update.append((diff.file_group_id, str(new_primary)))
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue