diff --git a/alembic/versions/3b6918aeca4b_file_group.py b/alembic/versions/3b6918aeca4b_file_group.py index 39509d91..613ad752 100644 --- a/alembic/versions/3b6918aeca4b_file_group.py +++ b/alembic/versions/3b6918aeca4b_file_group.py @@ -56,7 +56,6 @@ def upgrade(): id BIGSERIAL PRIMARY KEY, data JSON, files JSON, - full_stem TEXT NOT NULL UNIQUE, idempotency TIMESTAMP WITH TIME ZONE, indexed BOOLEAN DEFAULT FALSE, mimetype TEXT NOT NULL, diff --git a/modules/archive/test/test_lib.py b/modules/archive/test/test_lib.py index 28e58550..d242a1b2 100644 --- a/modules/archive/test/test_lib.py +++ b/modules/archive/test/test_lib.py @@ -486,7 +486,7 @@ def test_refresh_archives(test_session, test_directory, test_client, make_files_ # Cause a re-index of the archive. test_session.query(FileGroup).filter_by( - full_stem=str(test_directory / 'archive/example.com/2021-10-05-16-20-10_NA')).one().indexed = False + primary_path=str(test_directory / 'archive/example.com/2021-10-05-16-20-10_NA.html')).one().indexed = False test_session.commit() # Running the refresh does not result in a new archive. diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 375720e4..b676a90e 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -608,7 +608,7 @@ def find_orphaned_video_files(directory: pathlib.Path) -> Generator[pathlib.Path FROM file_group WHERE mimetype NOT LIKE 'video%' - AND full_stem LIKE '{directory}/%' + AND primary_path LIKE '{directory}/%' ''') results = (pathlib.Path(j['path']) for i in curs.fetchall() for j in i[0]) yield from results diff --git a/modules/videos/test/test_api.py b/modules/videos/test/test_api.py index bddd9bb0..a1271dea 100644 --- a/modules/videos/test/test_api.py +++ b/modules/videos/test/test_api.py @@ -73,10 +73,9 @@ def test_refresh_videos(test_client, test_session, test_directory, simple_channe # Create a bogus video in the channel. video_file = test_directory / 'foo.mp4' with get_db_curs(commit=True) as curs: - full_stem, _ = split_path_stem_and_suffix(video_file, full=True) - stmt = "INSERT INTO file_group (mimetype, full_stem, primary_path, indexed, files, model)" \ - " values ('video/mp4', %(full_stem)s, %(primary_path)s, true, %(files)s, 'video') RETURNING id" - params = {'full_stem': full_stem, 'primary_path': str(video_file), 'files': list()} + stmt = "INSERT INTO file_group (mimetype, primary_path, indexed, files, model)" \ + " values ('video/mp4', %(primary_path)s, true, %(files)s, 'video') RETURNING id" + params = {'primary_path': str(video_file), 'files': list()} curs.execute(stmt, params) video4_id = curs.fetchall()[0][0] stmt = "INSERT INTO video (file_group_id) values (%(video_id)s)" diff --git a/wrolpi/conftest.py b/wrolpi/conftest.py index 3e3cd21f..901fc7b3 100644 --- a/wrolpi/conftest.py +++ b/wrolpi/conftest.py @@ -9,6 +9,7 @@ import multiprocessing import pathlib import shutil import tempfile +import zipfile from abc import ABC from itertools import zip_longest from typing import List, Callable, Union, Dict @@ -240,6 +241,11 @@ def video_file_factory(test_directory): return _ +@pytest.fixture +def video_bytes(): + return (PROJECT_DIR / 'test/big_buck_bunny_720p_1mb.mp4').read_bytes() + + @pytest.fixture def corrupted_video_file(test_directory) -> pathlib.Path: """Return a copy of the corrupted video file in the `test_directory`.""" @@ -415,17 +421,16 @@ def assert_files(test_session, test_directory): def _(files: List[Dict]): from wrolpi.files.models import FileGroup for expected in files: - from wrolpi.files.lib import split_path_stem_and_suffix - full_stem, _ = split_path_stem_and_suffix(test_directory / expected.pop('path'), full=True) + primary_path = test_directory / expected.pop('path') try: - file: FileGroup = test_session.query(FileGroup).filter_by(full_stem=full_stem).one() + file: FileGroup = test_session.query(FileGroup).filter_by(primary_path=primary_path).one() except sqlalchemy.orm.exc.NoResultFound as e: - raise ValueError(f'No FileGroup found with {full_stem=}') from e + raise ValueError(f'No FileGroup found with {primary_path=}') from e for key, value in expected.items(): attr = getattr(file, key) if attr != value: - raise AssertionError(f'{full_stem} {attr} != {value}') + raise AssertionError(f'{primary_path} {attr} != {value}') return _ @@ -437,18 +442,14 @@ def assert_file_groups(test_session, test_directory): def _(file_groups: List[Dict], assert_count: bool = True): from wrolpi.files.models import FileGroup for expected in file_groups: - if 'full_stem' in expected: - full_stem = str(test_directory / expected.pop('full_stem')) - try: - file_group: FileGroup = test_session.query(FileGroup).filter_by(full_stem=full_stem).one() - except sqlalchemy.exc.NoResultFound as e: - raise ValueError(f'No FileGroup found with {full_stem=}') from e - elif 'primary_path' in expected: - primary_path = str(test_directory / expected.pop('primary_path')) - try: - file_group: FileGroup = test_session.query(FileGroup).filter_by(primary_path=primary_path).one() - except sqlalchemy.exc.NoResultFound as e: - raise ValueError(f'No FileGroup found with {primary_path=}') from e + if 'primary_path' not in expected: + raise Exception('You must specify the primary path for this fixture!') + + primary_path = str(test_directory / expected.pop('primary_path')) + try: + file_group: FileGroup = test_session.query(FileGroup).filter_by(primary_path=primary_path).one() + except Exception as e: + raise ValueError(f'No FileGroup found with {primary_path=}') from e # Compare file dictionaries. files = expected.pop('files', None) @@ -544,13 +545,12 @@ def insert_file_group(test_session, test_directory): def _(paths: List[pathlib.Path]): files = [dict(path=str(i), mimetype='fake') for i in paths] params = dict( - full_stem=str(test_directory / files_lib.split_path_stem_and_suffix(paths[0])[0]), primary_path=str(paths[0]), files=json.dumps(files), ) test_session.execute('INSERT INTO file_group ' - '(indexed, full_stem, primary_path, files) VALUES ' - '(true, :full_stem, :primary_path, :files)', params) + '(indexed, primary_path, files) VALUES ' + '(true, :primary_path, :files)', params) return _ @@ -574,9 +574,22 @@ def assert_tags_config(test_tags_config): if tags: for key, item in tags.items(): assert key in contents['tags'] - assert_dict_contains(contents['tags'][key], item) + assert_dict_contains(item, contents['tags'][key]) else: # Expect no tags to be saved. assert not contents['tags'] return _ + + +@pytest.fixture +def zip_file_factory(test_directory): + def _() -> bytes: + with tempfile.NamedTemporaryFile() as fh: + with zipfile.ZipFile(fh, 'w') as zip_file: + zip_file.write(PROJECT_DIR / 'test/big_buck_bunny_720p_1mb.mp4') + + fh.seek(0) + return fh.read() + + return _ diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index fe869852..f35a4a80 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -252,7 +252,7 @@ def _paths_to_files_dict(group: List[pathlib.Path]) -> List[dict]: return files -def get_primary_file(files: Union[Tuple[pathlib.Path], List[pathlib.Path]], pre_sorted: bool = False) -> pathlib.Path: +def get_primary_file(files: Union[Tuple[pathlib.Path], List[pathlib.Path]]) -> Union[pathlib.Path, List[pathlib.Path]]: """Given a list of files, return the file that we can model or index.""" from modules.archive.lib import is_singlefile_file @@ -287,10 +287,8 @@ def get_primary_file(files: Union[Tuple[pathlib.Path], List[pathlib.Path]], pre_ logger.debug(f'Cannot find primary file for group: {files}') - # Can't find a typical primary file, just use the alphanumeric first file. - files = sorted(files) if pre_sorted is False else files - file = files[0] - return file + # Can't find a typical primary file. + return files def _upsert_files(files: List[pathlib.Path], idempotency: datetime.datetime): @@ -308,25 +306,39 @@ def _upsert_files(files: List[pathlib.Path], idempotency: datetime.datetime): # Convert the groups into file_groups. Extract common information into `file_group.data`. values = dict() for group in grouped: - full_stem, _ = split_path_stem_and_suffix(group[0], full=True) # The primary file is the video/SingleFile/epub, etc. - primary_path = get_primary_file(group, pre_sorted=True) - # 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.dumps(_paths_to_files_dict(group)) - values[full_stem] = (str(primary_path), modification_datetime, mimetype, size, files) - values = [(stem, False, idempotency, *i) for stem, i in values.items()] + primary_path_or_paths = get_primary_file(group) + if isinstance(primary_path_or_paths, list) or isinstance(primary_path_or_paths, tuple): + # Cannot find primary path, create group for each file. + for primary_path in primary_path_or_paths: + primary_path: pathlib.Path + # The primary mimetype allows modelers to find its file_groups. + mimetype = get_mimetype(primary_path) + modification_datetime = from_timestamp(primary_path.stat().st_mtime) + size = primary_path.stat().st_size + files = json.dumps(_paths_to_files_dict([primary_path])) + values[primary_path] = (modification_datetime, mimetype, size, files) + else: + # Multiple files in this group. + primary_path: pathlib.Path = primary_path_or_paths + # 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.dumps(_paths_to_files_dict(group)) + values[primary_path] = (modification_datetime, mimetype, size, files) + + # `False` is the `indexed` which is false to force indexing by default. + values = [(str(primary_path), False, idempotency, *i) for primary_path, i in values.items()] with get_db_curs(commit=True) as curs: values = mogrify(curs, values) stmt = f''' INSERT INTO file_group - (full_stem, indexed, idempotency, primary_path, modification_datetime, mimetype, size, files) + (primary_path, indexed, idempotency, modification_datetime, mimetype, size, files) VALUES {values} - ON CONFLICT (full_stem) DO UPDATE + ON CONFLICT (primary_path) DO UPDATE SET idempotency=EXCLUDED.idempotency, modification_datetime=EXCLUDED.modification_datetime, @@ -401,11 +413,11 @@ async def refresh_discover_paths(paths: List[pathlib.Path], idempotency: datetim wheres = '' if paths: # Use LIKE to delete any children of directories that are deleted. - wheres = ' OR '.join([curs.mogrify('full_stem LIKE %s', (f'{i}/%',)).decode() for i in paths]) + wheres = ' OR '.join([curs.mogrify('primary_path LIKE %s', (f'{i}/%',)).decode() for i in paths]) deleted_files = '' if deleted: # Delete any paths that do not exist. - deleted_files = ' OR '.join([curs.mogrify('full_stem = %s', (str(i),)).decode() for i in deleted]) + deleted_files = ' OR '.join([curs.mogrify('primary_path = %s', (str(i),)).decode() for i in deleted]) idempotency = curs.mogrify('%s', (idempotency,)).decode() wheres = f' ( (idempotency != {idempotency} OR idempotency is null) AND ({wheres}))' if wheres else '' stmt = f''' diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index 0c578da9..00ea5b74 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -64,7 +64,6 @@ class FileGroup(ModelHelper, Base): data = Column(FancyJSON) # populated by the modeler files = Column(FancyJSON, nullable=False) # populated during discovery - full_stem = Column(String, unique=True) idempotency = Column(TZDateTime) indexed = Column(Boolean, default=lambda: False, nullable=False) mimetype = Column(String) # wrolpi.files.lib.get_mimetype @@ -101,7 +100,6 @@ class FileGroup(ModelHelper, Base): 'data': self.data, 'directory': self.primary_path.parent, 'files': self.my_files(), - 'full_stem': pathlib.Path(self.full_stem) if self.full_stem else None, 'id': self.id, 'mimetype': self.mimetype, 'model': self.model, @@ -225,7 +223,6 @@ class FileGroup(ModelHelper, Base): file_group = FileGroup() file_group.append_files(*paths) - file_group.full_stem, _ = split_path_stem_and_suffix(paths[0], full=True) file_group.primary_path = get_primary_file(paths) file_group.mimetype = get_mimetype(file_group.primary_path) file_group.modification_datetime = from_timestamp(max(i.stat().st_mtime for i in paths)) diff --git a/wrolpi/files/test/test_api.py b/wrolpi/files/test/test_api.py index f4ed30e4..7ccf8102 100644 --- a/wrolpi/files/test/test_api.py +++ b/wrolpi/files/test/test_api.py @@ -172,10 +172,10 @@ def test_files_search(test_session, test_client, make_files_structure, assert_fi request, response = test_client.post('/api/files/refresh') assert response.status_code == HTTPStatus.NO_CONTENT - assert_files_search('foo', [dict(full_stem='foo_is_the_name')]) - assert_files_search('bar', [dict(full_stem='archives/bar')]) - assert_files_search('baz', [dict(full_stem='baz baz two'), dict(full_stem='baz')]) - assert_files_search('two', [dict(full_stem='baz baz two')]) + assert_files_search('foo', [dict(primary_path='foo_is_the_name.txt')]) + assert_files_search('bar', [dict(primary_path='archives/bar.txt')]) + assert_files_search('baz', [dict(primary_path='baz baz two.mp4'), dict(primary_path='baz.mp4')]) + assert_files_search('two', [dict(primary_path='baz baz two.mp4')]) assert_files_search('nothing', []) @@ -204,9 +204,12 @@ def test_directory_search(test_client, make_files_structure): assert_directories('food', []) -def test_refresh_files_list(test_session, test_client, make_files_structure, test_directory): +def test_refresh_files_list(test_session, test_client, make_files_structure, test_directory, video_bytes): """The user can request to refresh specific files.""" - make_files_structure(['bar.txt', 'bar.mp4']) + make_files_structure({ + 'bar.txt': 'hello', + 'bar.mp4': video_bytes, + }) # Only the single file that was refreshed is discovered. content = json.dumps({'paths': ['bar.txt']}) diff --git a/wrolpi/files/test/test_ebooks.py b/wrolpi/files/test/test_ebooks.py index d522dcdb..46339d9a 100644 --- a/wrolpi/files/test/test_ebooks.py +++ b/wrolpi/files/test/test_ebooks.py @@ -45,6 +45,7 @@ async def test_index(test_session, test_directory, example_epub, example_mobi): example_epub.unlink() await refresh_files() assert test_session.query(EBook).count() == 1 + ebook = test_session.query(EBook).one() assert not ebook.file_group.my_files('application/epub+zip') assert (mobis := ebook.file_group.my_files('application/x-mobipocket-ebook')) and len(mobis) == 1 \ and mobis[0]['path'] == example_mobi diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index 89d12d81..964d84a3 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -80,19 +80,19 @@ async def test_refresh_files(test_session, make_files_structure, assert_file_gro await lib.refresh_files() assert_file_groups([ - {'full_stem': 'foo', 'indexed': True}, - {'full_stem': 'bar', 'indexed': True}, - {'full_stem': 'baz', 'indexed': True}]) + {'primary_path': 'foo.txt', 'indexed': True}, + {'primary_path': 'bar.txt', 'indexed': True}, + {'primary_path': 'baz.txt', 'indexed': True}]) baz.unlink() await lib.refresh_files() - assert_file_groups([{'full_stem': 'foo', 'indexed': True}, {'full_stem': 'bar', 'indexed': True}]) + assert_file_groups([{'primary_path': 'foo.txt', 'indexed': True}, {'primary_path': 'bar.txt', 'indexed': True}]) foo.unlink() await lib.refresh_files() - assert_file_groups([{'full_stem': 'bar', 'indexed': True}]) + assert_file_groups([{'primary_path': 'bar.txt', 'indexed': True}]) @pytest.mark.asyncio @@ -106,14 +106,14 @@ async def test_refresh_bogus_files(test_session, make_files_structure, test_dire # Bogus file was inserted. assert_file_groups([ - {'full_stem': 'does exist', 'indexed': True}, - {'full_stem': 'does not exist', 'indexed': True}, + {'primary_path': 'does exist.txt', 'indexed': True}, + {'primary_path': 'does not exist.txt', 'indexed': True}, ]) await lib.refresh_files() assert test_session.query(FileGroup).count() == 1, 'Bogus file was not removed' - assert_file_groups([{'full_stem': 'does exist', 'indexed': True}]) + assert_file_groups([{'primary_path': 'does exist.txt', 'indexed': True}]) @pytest.mark.asyncio @@ -132,7 +132,6 @@ async def test__upsert_files(test_session, make_files_structure, test_directory, }) video_file = video_file.rename(test_directory / 'video.mp4') srt_file3 = srt_file3.rename(test_directory / 'video.en.srt') - video_stem = str(test_directory / video_file.stem) foo_mtime = from_timestamp(srt_file3.stat().st_mtime) # All files are found because they are in this refresh request, or in the `dir1` directory. @@ -159,7 +158,7 @@ async def test__upsert_files(test_session, make_files_structure, test_directory, test_session.query(FileGroup).filter_by(primary_path=video_file).one().indexed = True test_session.commit() assert_file_groups( - [{'full_stem': video_stem, 'idempotency': idempotency, 'modification_datetime': foo_mtime, 'indexed': True}], + [{'primary_path': str(video_file), 'idempotency': idempotency, 'modification_datetime': foo_mtime, 'indexed': True}], assert_count=False) # Only modified files need to be re-indexed. @@ -181,7 +180,7 @@ async def test__upsert_files(test_session, make_files_structure, test_directory, # Deleting SRT removes it from the video. srt_file3.unlink() lib._upsert_files([video_file, bar, baz], idempotency) - video_file_group: FileGroup = test_session.query(FileGroup).filter_by(full_stem=str(test_directory / 'video')).one() + video_file_group: FileGroup = test_session.query(FileGroup).filter_by(primary_path=str(video_file)).one() assert len(video_file_group.files) == 1, 'SRT file was not removed from files' @@ -230,8 +229,8 @@ async def test_refresh_discover_paths(test_session, make_files_structure, test_d @pytest.mark.asyncio -async def test_refresh_discover_paths_groups(test_session, make_files_structure, test_directory): - make_files_structure(['dir1/foo.mp4', 'dir1/foo.info.json', 'baz.txt']) +async def test_refresh_discover_paths_groups(test_session, make_files_structure, test_directory, video_bytes): + make_files_structure({'dir1/foo.mp4': video_bytes, 'dir1/foo.info.json': 'hello', 'baz.txt': 'hello'}) await lib.refresh_discover_paths([test_directory, ], now()) # Two "foo" files, one "baz" file. @@ -370,7 +369,7 @@ async def test_files_indexer(test_session, make_files_structure, test_directory) await lib.refresh_files() text_file, zip_file, image_file, unknown_file, video_file \ - = test_session.query(FileGroup).order_by(FileGroup.full_stem) + = test_session.query(FileGroup).order_by(FileGroup.primary_path) # Indexers are detected correctly. assert text_file.mimetype == 'text/plain' and text_file.indexer == indexers.TextIndexer @@ -582,3 +581,21 @@ def test_get_refresh_progress(test_client, test_session): assert 'refreshing' in progress assert 'total_files' in progress assert 'unindexed' in progress + + +@pytest.mark.asyncio +async def test_refresh_files_no_groups(test_session, test_directory, make_files_structure, zip_file_factory): + """Files that share a name, but cannot be grouped into a FileGroup have their own FileGroups.""" + foo_txt, foo_zip = make_files_structure({ + 'foo.txt': 'text', + 'foo.zip': zip_file_factory(), + }) + assert foo_txt.stat().st_size and foo_zip.stat().st_size + + await lib.refresh_files() + + # Two distinct FileGroups. + assert test_session.query(FileGroup).count() == 2 + txt, zip_ = test_session.query(FileGroup) + assert txt.primary_path == foo_txt and txt.size == foo_txt.stat().st_size + assert zip_.primary_path == foo_zip and zip_.size == foo_zip.stat().st_size diff --git a/wrolpi/tags.py b/wrolpi/tags.py index 847262bd..f4bfedc3 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -176,10 +176,10 @@ def schedule_save(session: Session = None): def get_tags() -> List[dict]: with get_db_curs() as curs: curs.execute(''' - SELECT t.name, t.color, COUNT(tf.tag_id) + SELECT t.id, t.name, t.color, COUNT(tf.tag_id) FROM tag t LEFT JOIN tag_file tf on t.id = tf.tag_id - GROUP BY t.name, t.color + GROUP BY t.id, t.name, t.color ORDER BY t.name ''') tags = list(map(dict, curs.fetchall())) diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index 0f6c824a..fb52f1aa 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -29,9 +29,11 @@ async def test_tags_file_group_json(test_session, make_files_structure, tag_fact @pytest.mark.asyncio -async def test_tags_file_group(test_session, make_files_structure, tag_factory): +async def test_tags_file_group(test_session, make_files_structure, tag_factory, video_bytes, image_bytes_factory): """A FileGroup can be tagged with multiple Tags.""" - make_files_structure(['video.mp4', 'video.png']) + make_files_structure({ + 'video.mp4': video_bytes, 'video.png': image_bytes_factory(), + }) await files_lib.refresh_files() video_group: FileGroup = test_session.query(FileGroup).one() @@ -126,7 +128,7 @@ def test_tags_crud(test_session, test_client, example_pdf, assert_tags_config): # The tag can be retrieved. request, response = test_client.get('/api/tag') assert response.status_code == HTTPStatus.OK - assert response.json['tags'] == [dict(name='foo', color='#123456', id=1)] + assert response.json['tags'] == [dict(name='foo', color='#123456', id=1, count=0)] # Apply the tag to the PDF. tag = test_session.query(tags.Tag).one() @@ -138,7 +140,7 @@ def test_tags_crud(test_session, test_client, example_pdf, assert_tags_config): request, response = test_client.post('/api/tag/1', content=json.dumps(content)) assert response.status_code == HTTPStatus.OK request, response = test_client.get('/api/tag') - assert response.json['tags'] == [dict(name='bar', color='#000000', id=1)] + assert response.json['tags'] == [dict(name='bar', color='#000000', id=1, count=1)] assert_tags_config(tags={'bar': {'color': '#000000'}}) # Conflicting names return an error.