From 2ad363281f658e4840cb8dc039ee8f5c8c3cda47 Mon Sep 17 00:00:00 2001 From: Roland Date: Tue, 30 Jul 2024 17:14:08 -0600 Subject: [PATCH] Delaying map startup because it prevents the API from starting. --- app/src/components/Map.js | 2 ++ etc/raspberrypios/renderd | 11 +++++-- modules/archive/models.py | 2 +- modules/videos/lib.py | 5 +-- modules/videos/models.py | 9 +++--- modules/videos/test/test_lib.py | 4 ++- modules/zim/models.py | 4 +-- repair.sh | 1 + wrolpi/conftest.py | 37 +++++++++++++++------- wrolpi/errors.py | 8 ++++- wrolpi/files/api.py | 5 +-- wrolpi/files/lib.py | 46 ++++++++++++++++++--------- wrolpi/files/models.py | 17 +++++++--- wrolpi/files/test/test_api.py | 52 +++++++++++++++++-------------- wrolpi/files/test/test_lib.py | 34 +++++++++++--------- wrolpi/tags.py | 54 ++++++++++++++++---------------- wrolpi/test/test_tags.py | 55 +++++++++++++++++---------------- 17 files changed, 207 insertions(+), 139 deletions(-) diff --git a/app/src/components/Map.js b/app/src/components/Map.js index 05bab020..c5f81572 100644 --- a/app/src/components/Map.js +++ b/app/src/components/Map.js @@ -295,6 +295,8 @@ function MapPage() {
Failed to fetch Map service.

You may need to give permission to access the page: {MAP_VIEWER_URI}

+

Map startup is delayed on boot, if you have just started your WROLPi try again in a few minutes.

+

If the above does not work, try starting the service:

sudo systemctl start renderd
sudo systemctl start apache2
diff --git a/etc/raspberrypios/renderd b/etc/raspberrypios/renderd index 133ef16f..21550dc1 100644 --- a/etc/raspberrypios/renderd +++ b/etc/raspberrypios/renderd @@ -17,8 +17,8 @@ PATH=/sbin:/usr/sbin:/bin:/usr/bin DESC="Mapnik rendering daemon" NAME=renderd DAEMON=/usr/bin/$NAME -DAEMON_ARGS="-c /etc/renderd.conf" -PIDSOCKDIR=/var/run/$NAME +DAEMON_ARGS="" +PIDSOCKDIR=/run/$NAME PIDFILE=$PIDSOCKDIR/$NAME.pid SCRIPTNAME=/etc/init.d/$NAME RUNASUSER=_renderd @@ -42,6 +42,10 @@ RUNASUSER=_renderd do_start() { [ -d "$PIDSOCKDIR" ] || mkdir -p $PIDSOCKDIR && chown $RUNASUSER $PIDSOCKDIR + + # Sleep because starting renderd prevents API from starting. + sleep 60 + # Return # 0 if daemon has been started # 1 if daemon was already running @@ -143,6 +147,9 @@ case "$1" in ;; esac ;; + status) + status_of_proc "$DAEMON" "$NAME" && exit 0 || exit $? + ;; *) #echo "Usage: $SCRIPTNAME {start|stop|restart|reload|force-reload}" >&2 echo "Usage: $SCRIPTNAME {start|stop|restart|force-reload}" >&2 diff --git a/modules/archive/models.py b/modules/archive/models.py index b5043450..2d26d473 100644 --- a/modules/archive/models.py +++ b/modules/archive/models.py @@ -286,7 +286,7 @@ class Archive(Base, ModelHelper): return archive def add_tag(self, tag_or_tag_name: Union[Tag, str]) -> TagFile: - tag = Tag.find_by_name(tag_or_tag_name) if isinstance(tag_or_tag_name, str) else tag_or_tag_name + tag = Tag.get_by_name(tag_or_tag_name) if isinstance(tag_or_tag_name, str) else tag_or_tag_name return self.file_group.add_tag(tag) diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 80b31ac1..2cd84f16 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -17,6 +17,7 @@ from wrolpi.dates import Seconds, from_timestamp from wrolpi.db import get_db_curs, get_db_session, optional_session from wrolpi.downloader import Download from wrolpi.errors import UnknownDirectory +from wrolpi.files.lib import split_path_stem_and_suffix from wrolpi.vars import PYTEST, YTDLP_CACHE_DIR from .common import is_valid_poster, convert_image, \ generate_video_poster, logger, REQUIRED_OPTIONS, ConfigError, \ @@ -593,7 +594,7 @@ async def get_statistics(): return ret -NAME_PARSER = re.compile(r'(.*?)_((?:\d+?)|(?:NA))_(?:(.{5,15})_)?(.*)\.' +NAME_PARSER = re.compile(r'(.*?)_((?:\d+?)|(?:NA))_(?:(.{5,25})_)?(.*)\.' r'(jpg|webp|flv|mp4|part|info\.json|description|webm|..\.srt|..\.vtt)') @@ -618,7 +619,7 @@ def parse_video_file_name(video_path: pathlib.Path) -> \ return channel, date, source_id, title # Return the stem as a last resort - title = pathlib.Path(video_path).stem.strip() + title = split_path_stem_and_suffix(video_path)[0].strip() return None, None, None, title diff --git a/modules/videos/models.py b/modules/videos/models.py index e03661b3..2005efe1 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -339,10 +339,9 @@ class Video(ModelHelper, Base): raise UnknownVideo(f'Cannot find Video with id {id_}') return video - def add_tag(self, tag_or_tag_name: Union[Tag, str], session: Session = None) -> TagFile: + def add_tag(self, tag_id: int, session: Session = None) -> TagFile: session = session or Session.object_session(self) - tag = Tag.find_by_name(tag_or_tag_name) if isinstance(tag_or_tag_name, str) else tag_or_tag_name - return self.file_group.add_tag(tag, session=session) + return self.file_group.add_tag(tag_id, session=session) async def get_ffprobe_json(self) -> dict: """Return the ffprobe json object if previously stored. @@ -420,8 +419,8 @@ class Video(ModelHelper, Base): def delete_video(video_: Video): """Delete a Video with any of its tags.""" for tag_name_ in video_.file_group.tag_names: - tag_ = Tag.find_by_name(tag_name_) - video_.file_group.remove_tag(tag_, session) + tag_ = Tag.get_by_name(tag_name_) + video_.file_group.untag(tag_, session) video_.delete() changes = False diff --git a/modules/videos/test/test_lib.py b/modules/videos/test/test_lib.py index 069c1553..8090c77f 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -20,7 +20,7 @@ from wrolpi.vars import PROJECT_DIR ('20000101foo.mp4', (None, None, None, '20000101foo')), ('something 20000101 foo.mp4', (None, None, None, 'something 20000101 foo')), ('something_20000101_foo.mp4', ('something', '20000101', None, 'foo')), - ('foo .mp4', (None, None, None, 'foo')), + ('foo .mp4', (None, None, None, 'foo')), # Has trailing whitespace. ('NA_20000303_vp91w5_Bob's Pancakes.mp4', (None, '20000303', 'vp91w5', 'Bob's Pancakes')), ('NA_NA_vp91w5_Bob's Pancakes.mp4', (None, None, 'vp91w5', 'Bob's Pancakes')), ('Learning Self-Reliance_20170529_p_MzsCFkUPU_Beekeeping 2017 Part 6 - Merging Hives.mp4', @@ -29,6 +29,8 @@ from wrolpi.vars import PROJECT_DIR ('Learning Self-Reliance', '20170529', 'p_Mzs', 'Beekeeping 2017 Part 6 - Merging Hives')), ('Learning Self-Reliance_20170529_p_Beekeeping 2017 Part 6 - Merging Hives.mp4', ('Learning Self-Reliance', '20170529', None, 'p_Beekeeping 2017 Part 6 - Merging Hives')), + ('Learning Self-Reliance_20240722_clyx8ark92qk70860vtl997n5_Long Source Id File with _.mp4', + ('Learning Self-Reliance', '20240722', 'clyx8ark92qk70860vtl997n5', 'Long Source Id File with _')) ]) def test_parse_video_file_name(file_name, expected): """ diff --git a/modules/zim/models.py b/modules/zim/models.py index 18712710..a294460c 100644 --- a/modules/zim/models.py +++ b/modules/zim/models.py @@ -189,7 +189,7 @@ class Zim(Base): def tag_entry(self, tag_name: str, zim_entry: str) -> 'TagZimEntry': """Create a TagZimEntry for this Zim at the provided entry (path) for the provided Tag.""" session = Session.object_session(self) - tag = Tag.find_by_name(tag_name, session=session) + tag = Tag.get_by_name(tag_name, session=session) tag_zim_entry = TagZimEntry(tag=tag, zim=self, zim_entry=zim_entry) session.add(tag_zim_entry) tags.schedule_save() @@ -200,7 +200,7 @@ class Zim(Base): @raise UnknownZimTagEntry: No entry exists to remove.""" session = Session.object_session(self) - tag = Tag.find_by_name(tag_name, session=session) + tag = Tag.get_by_name(tag_name, session=session) tag_zim_entry = session.query(TagZimEntry).filter_by( tag_id=tag.id, zim_id=self.id, diff --git a/repair.sh b/repair.sh index a0b02acd..2b8ee30d 100755 --- a/repair.sh +++ b/repair.sh @@ -68,6 +68,7 @@ systemctl enable wrolpi-kiwix.service systemctl enable wrolpi-help.service cp /opt/wrolpi/etc/raspberrypios/renderd.conf /etc/renderd.conf +cp /opt/wrolpi/etc/raspberrypios/renderd /etc/init.d/renderd # Configure Apache2 to listen on 8084. cp /opt/wrolpi/etc/raspberrypios/ports.conf /etc/apache2/ports.conf # Copy Leaflet files to Apache's directory so they can be used offline. diff --git a/wrolpi/conftest.py b/wrolpi/conftest.py index adb521eb..c9a49647 100644 --- a/wrolpi/conftest.py +++ b/wrolpi/conftest.py @@ -710,23 +710,38 @@ def assert_directories(test_session, test_directory): return _ +FORM_PART = b'''-----------------------------sanic\r +Content-Disposition: form-data; name="{name}"\r +\r +{value}\r +\r +''' + +FORM_FILENAME_PART = b'''-----------------------------sanic\r +Content-Disposition: form-data; name="{name}"; filename="{filename}"\r +Content-Type: application/octet-stream\r +\r +{value}\r +\r +''' + + @pytest.fixture def make_multipart_form(): def _(forms: List[dict]): - boundary = '-----sanic\n' + boundary = b'-----------------------------sanic\r\n' new_forms = [] for form in forms: - name, value, filename = form['name'], form['value'], form.get('filename') + name, value, filename = form['name'].encode(), form['value'], form.get('filename', '').encode() + value = value if isinstance(value, bytes) else str(value).encode() + part = FORM_PART if filename: - new_forms.append(f'Content-Disposition: form-data; name="{name}"; filename="{filename}"\n\n{value}\n') - elif isinstance(value, list): - for part in value: - new_forms.append(f'Content-Disposition: form-data; name="{name}"\n\n{part}\n') - else: - new_forms.append(f'Content-Disposition: form-data; name="{name}"\n\n{value}\n') - body = f'\n{boundary}'.join(new_forms) - body = f'{boundary}{body}\n{boundary}' - body = '\r\n'.join(body.split('\n')).strip() + part = FORM_FILENAME_PART + part.replace(b'{filename}', filename) + part = part.replace(b'{name}', name) + part = part.replace(b'{value}', value) + new_forms.append(part) + body = boundary + boundary.join(new_forms) + boundary return body return _ diff --git a/wrolpi/errors.py b/wrolpi/errors.py index 58ac481c..34e10f85 100644 --- a/wrolpi/errors.py +++ b/wrolpi/errors.py @@ -113,7 +113,13 @@ class InvalidTag(APIError): class FileGroupIsTagged(APIError): code = 'FILE_GROUP_IS_TAGGED' - message = 'File group is tagged' + message = 'FileGroup is tagged' + status_code = HTTPStatus.CONFLICT + + +class FileGroupAlreadyTagged(APIError): + code = 'FILE_GROUP_ALREADY_TAGGED' + message = 'FileGroup is already tagged with this Tag' status_code = HTTPStatus.CONFLICT diff --git a/wrolpi/files/api.py b/wrolpi/files/api.py index d28841ce..046bba9d 100644 --- a/wrolpi/files/api.py +++ b/wrolpi/files/api.py @@ -304,7 +304,7 @@ async def post_upload(request: Request): destination = get_media_directory() / destination if not destination.is_dir(): - raise UnknownDirectory('Destination must be a relative directory that is already in the media directory!') + raise UnknownDirectory(f'Destination must be a relative directory that is already in the media directory: {destination}') try: filename = str(request.form['filename'][0]) @@ -327,7 +327,7 @@ async def post_upload(request: Request): raise FileUploadFailed('tag_names must be a list') tag_names = [str(i) for i in tag_names] for tag_name in tag_names: - if not Tag.find_by_name(tag_name): + if not Tag.get_by_name(tag_name): raise FileUploadFailed(f'Tag does not exist: {tag_name}') filename = pathlib.Path(filename.lstrip('/')) @@ -382,6 +382,7 @@ async def post_upload(request: Request): # Chunks start at 0. if chunk_num == total_chunks: # File upload is complete. + logger.info(f'Got final chunk of uploaded file: {output_str}') del api_app.shared_ctx.uploaded_files[output_str] # Upsert this new file (and any related files) into the DB. background_task(lib.upsert_file(output, tag_names=tag_names)) diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index 4439d33e..fa32d084 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -242,6 +242,8 @@ def _mimetype_suffix_map(path: Path, mimetype: str): if mimetype == 'application/x-subrip': # Fallback to old mimetype. return 'text/srt' + if mimetype == 'application/zip' and path.suffix == '.docx': + return 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' return mimetype @@ -266,10 +268,10 @@ def get_mimetype(path: Path) -> str: # Special suffixes within WROLPi. SUFFIXES = { '.info.json', + '.live_chat.json', + '.readability.html', '.readability.json', '.readability.txt', - '.readability.html', - '.live_chat.json' } SUFFIXES |= {f'.{i}.srt' for i in ISO_639_CODES} SUFFIXES |= {f'.{i}.vtt' for i in ISO_639_CODES} @@ -279,16 +281,28 @@ SUFFIXES |= {f'.{i}.vtt' for i in ISO_639_CODES} def split_path_stem_and_suffix(path: Union[pathlib.Path, str], full: bool = False) -> Tuple[str, str]: """Get the path's stem and suffix. - This function handles WROLPi suffixes like .info.json.""" - if isinstance(path, str): - path = pathlib.Path(path) + This function handles WROLPi suffixes like .info.json. - full_ = str(path) # May or may not be absolute. + >>> split_path_stem_and_suffix('/foo/bar.txt') + ('bar', '.txt') + >>> split_path_stem_and_suffix('/foo/bar.txt', full=True) + ('/foo/bar', '.txt') + >>> split_path_stem_and_suffix('/foo/bar.info.json', full=True) + ('/foo/bar', '.info.json') + """ + path = pathlib.Path(path) if isinstance(path, str) else path + + # May or may not be absolute. Convert to lowercase so any suffix case can be matched. + full_ = str(path).lower() + # Get the special matching suffix, if any. Match against `.en.srt` but could return `.EN.SRT` suffix = next(filter(lambda i: full_.endswith(i), SUFFIXES), path.suffix) - if suffix and full: - return f'{path.parent}/{path.name[:-1 * len(suffix)]}', suffix - elif suffix: - return path.name[:-1 * len(suffix)], suffix + # Return the suffix from the path's name in the original case. + if suffix: + idx = -1 * len(suffix) + if full: + return f'{path.parent}/{path.name[:idx]}', path.name[idx:] + else: + return path.name[:idx], path.name[idx:] # Path has no suffix. if full: @@ -991,7 +1005,7 @@ async def _get_tag_and_file_group(file_group_id: int, file_group_primary_path: s if not tag: raise UnknownTag(f'Cannot find Tag with id {tag_id}') elif tag_name: - tag: Tag = Tag.find_by_name(tag_name, session) + tag: Tag = Tag.get_by_name(tag_name, session) if not tag: raise UnknownTag(f'Cannot find Tag with name {tag_name}') else: @@ -1012,7 +1026,7 @@ async def add_file_group_tag(file_group_id: int, file_group_primary_path: str, t async def remove_file_group_tag(file_group_id: int, file_group_primary_path: str, tag_name: str, tag_id: int, session: Session = None): file_group, tag = await _get_tag_and_file_group(file_group_id, file_group_primary_path, tag_name, tag_id, session) - file_group.remove_tag(tag, session) + file_group.untag(tag, session) @dataclasses.dataclass @@ -1371,7 +1385,7 @@ def remove_ignored_directory(directory: Union[pathlib.Path, str]): raise UnknownDirectory('Directory is not ignored') -@optional_session(commit=True) +@optional_session async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_names: List[str] = None): """Insert/update all files in the provided file's FileGroup.""" # Update/Insert all files in the FileGroup. @@ -1380,6 +1394,7 @@ async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_nam paths = remove_files_in_ignored_directories(paths) if paths: file_group = FileGroup.from_paths(session, *paths) + logger.debug(f'upsert_file: {file_group}') # Re-index the contents of the file. session.add(file_group) session.flush([file_group, ]) @@ -1388,7 +1403,8 @@ async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_nam for tag_name in (tag_names or []): if tag_name not in file_group.tag_names: - tag = Tag.find_by_name(tag_name) - file_group.add_tag(tag) + tag = Tag.get_by_name(tag_name) + file_group.add_tag(tag.id) + session.commit() else: logger.warning('upsert_file called, but all files are in ignored directories!') diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index b2cc9188..e12c6f4f 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -144,12 +144,15 @@ class FileGroup(ModelHelper, Base): self.viewed = now() @optional_session - def add_tag(self, tag: Tag, session: Session = None) -> TagFile: - return tag.add_file_group_tag(self, session) + def add_tag(self, tag_id: int, session: Session = None) -> TagFile: + return Tag.tag_file_group(self.id, tag_id, session) @optional_session - def remove_tag(self, tag: Tag, session: Session = None): - tag.remove_file_group_tag(self, session) + def untag(self, tag_id: int, session: Session = None): + if not isinstance(tag_id, int): + raise RuntimeError('tag_id must be an integer') + + Tag.untag_file_group(self.id, tag_id, session) def append_files(self, *paths: pathlib.Path): """Add all `paths` to this FileGroup.files.""" @@ -247,7 +250,11 @@ 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 + from wrolpi.files.lib import get_primary_file, get_mimetype, split_path_stem_and_suffix + + unique_stems = {split_path_stem_and_suffix(i, full=True)[0] for i in paths} + if len(unique_stems) > 1: + raise RuntimeError(f'Refusing to create FileGroup with paths that do not share a stem.') existing_groups = session.query(FileGroup).filter(FileGroup.primary_path.in_(list(map(str, paths)))).all() if len(existing_groups) == 0: diff --git a/wrolpi/files/test/test_api.py b/wrolpi/files/test/test_api.py index b7949b1a..705ee5ce 100644 --- a/wrolpi/files/test/test_api.py +++ b/wrolpi/files/test/test_api.py @@ -1,3 +1,4 @@ +import hashlib import json from http import HTTPStatus from unittest import mock @@ -6,6 +7,7 @@ import pytest from wrolpi.common import get_wrolpi_config from wrolpi.files import lib +from wrolpi.files.lib import get_mimetype from wrolpi.files.models import FileGroup from wrolpi.tags import TagFile from wrolpi.test.common import assert_dict_contains @@ -467,52 +469,56 @@ def test_post_upload_directory(test_session, test_client, test_directory, make_f assert test_session.query(FileGroup).count() == 1 -def test_post_upload(test_session, test_client, test_directory, make_files_structure, make_multipart_form, tag_factory): +def test_post_upload(test_session, test_client, test_directory, make_files_structure, make_multipart_form, tag_factory, + video_bytes): """A file can be uploaded in chunks directly to the destination.""" make_files_structure(['uploads/']) tag1, tag2 = tag_factory(), tag_factory() + part1, part2 = video_bytes[:1_000_000], video_bytes[1_000_000:] forms = [ dict(name='chunkNumber', value='0'), - dict(name='filename', value='foo.txt'), + dict(name='filename', value='video.mp4'), dict(name='totalChunks', value='1'), dict(name='destination', value='uploads'), - dict(name='chunkSize', value='3'), - dict(name='chunk', value='foo', filename='chunk'), - dict(name='tagNames', value=[tag1.name, tag2.name]), + dict(name='chunkSize', value=1_000_000), + dict(name='tagNames', value=tag1.name), + dict(name='tagNames', value=tag2.name), + dict(name='chunk', value=part1, filename='chunk'), ] body = make_multipart_form(forms) - request, response = test_client.post('/api/files/upload', content=body, - headers={ - 'Content-Type': 'multipart/form-data; name=upload; filename="file.txt";' - ' boundary=-----sanic'}) - assert response.status_code == HTTPStatus.OK + headers = {'Content-Type': 'multipart/form-data; boundary=-----------------------------sanic'} + request, response = test_client.post('/api/files/upload', content=body, headers=headers) + assert response.status_code == HTTPStatus.OK, response.content.decode() - assert (test_directory / 'uploads/foo.txt').is_file() - assert (test_directory / 'uploads/foo.txt').read_text() == 'foo' + output = test_directory / 'uploads/video.mp4' + assert output.is_file() + assert output.stat().st_size == 1_000_000 forms = [ dict(name='chunkNumber', value='1'), - dict(name='filename', value='foo.txt'), + dict(name='filename', value='video.mp4'), dict(name='totalChunks', value='1'), dict(name='destination', value='uploads'), - dict(name='chunkSize', value='3'), - dict(name='chunk', value='bar', filename='chunk'), - dict(name='tagNames', value=[tag1.name, tag2.name]), + dict(name='chunkSize', value=56318), + dict(name='tagNames', value=tag1.name), + dict(name='tagNames', value=tag2.name), + dict(name='chunk', value=part2, filename='chunk'), ] body = make_multipart_form(forms) - request, response = test_client.post('/api/files/upload', content=body, - headers={ - 'Content-Type': 'multipart/form-data; name=upload; filename="file.txt";' - ' boundary=-----sanic'}) - assert response.status_code == HTTPStatus.CREATED + request, response = test_client.post('/api/files/upload', content=body, headers=headers) + assert response.status_code == HTTPStatus.CREATED, response.content.decode() - assert (test_directory / 'uploads/foo.txt').is_file() - assert (test_directory / 'uploads/foo.txt').read_text() == 'foobar' + assert output.is_file() + assert get_mimetype(output) == 'video/mp4' + assert output.stat().st_size == len(video_bytes) + assert output.read_bytes() == video_bytes + assert hashlib.md5(output.read_bytes()).hexdigest() == '2738c53bd7c01b01d408da11a55bfa36' file_group: FileGroup = test_session.query(FileGroup).one() assert set(file_group.tag_names) == {tag1.name, tag2.name}, 'Two tags should be applied.' assert file_group.indexed, 'File should be indexed after upload.' + assert file_group.model, 'File should be modeled' @pytest.mark.asyncio diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index 3cb38dbd..23a02d4c 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -132,22 +132,27 @@ async def test_delete_nested(test_session, make_files_structure): @pytest.mark.parametrize( - 'path,expected', + 'path,full,expected', [ - ('foo', ('foo', '')), - ('foo.mp4', ('foo', '.mp4')), - ('foo.info.json', ('foo', '.info.json')), - ('foo.something.info.json', ('foo.something', '.info.json')), - ('foo-something.info.json', ('foo-something', '.info.json')), - ('/absolute/foo-something.info.json', ('foo-something', '.info.json')), - ('/absolute/foo', ('foo', '')), - ('/absolute/foo.bar', ('foo', '.bar')), - ('foo.en.srt', ('foo', '.en.srt')), - ('foo.pl.srt', ('foo', '.pl.srt')), + ('foo', False, ('foo', '')), + ('foo.mp4', False, ('foo', '.mp4')), + ('foo.info.json', False, ('foo', '.info.json')), + ('foo.something.info.json', False, ('foo.something', '.info.json')), + ('foo-something.info.json', False, ('foo-something', '.info.json')), + ('/absolute/foo-something.info.json', False, ('foo-something', '.info.json')), + ('/absolute/foo', False, ('foo', '')), + ('/absolute/foo.bar', False, ('foo', '.bar')), + ('foo.en.srt', False, ('foo', '.en.srt')), + ('foo.pl.srt', False, ('foo', '.pl.srt')), + # Absolute path can be returned. + ('/absolute//foo.bar', True, ('/absolute/foo', '.bar')), + # Case is preserved. + ('foo.EN.SRT', False, ('foo', '.EN.SRT')), + ('foo.INFO.JSON', False, ('foo', '.INFO.JSON')), ] ) -def test_split_path_stem_and_suffix(path, expected): - assert lib.split_path_stem_and_suffix(Path(path)) == expected +def test_split_path_stem_and_suffix(path, full, expected): + assert lib.split_path_stem_and_suffix(Path(path), full) == expected @pytest.mark.asyncio @@ -717,7 +722,8 @@ async def test_refresh_directories(test_session, test_directory, assert_director assert_directories({'foo', 'baz'}) -def test_file_group_merge(test_async_client, test_session, test_directory, make_files_structure, tag_factory, video_bytes, srt_file3): +def test_file_group_merge(test_async_client, test_session, test_directory, make_files_structure, tag_factory, + video_bytes, srt_file3): """A FileGroup can be created from multiple existing FileGroups. Any Tags applied to the existing groups will be migrated.""" vid, srt = make_files_structure({ diff --git a/wrolpi/tags.py b/wrolpi/tags.py index 832eccaf..4da8d4cb 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -1,7 +1,7 @@ import contextlib import pathlib from datetime import datetime -from typing import List, Dict, Tuple +from typing import List, Dict, Tuple, Optional from sqlalchemy import Column, Integer, String, ForeignKey, BigInteger from sqlalchemy.exc import IntegrityError @@ -12,7 +12,7 @@ from wrolpi.common import ModelHelper, Base, logger, ConfigFile, get_media_direc register_refresh_cleanup, get_relative_to_media_directory from wrolpi.dates import TZDateTime from wrolpi.db import optional_session, get_db_curs -from wrolpi.errors import UnknownTag, UsedTag, InvalidTag, ValidationError +from wrolpi.errors import UnknownTag, UsedTag, InvalidTag, FileGroupAlreadyTagged from wrolpi.vars import PYTEST logger = logger.getChild(__name__) @@ -27,6 +27,11 @@ class TagFile(ModelHelper, Base): file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), primary_key=True) file_group = relationship('FileGroup', back_populates='tag_files') + @staticmethod + @optional_session + def get_by_primary_keys(file_group_id: int, tag_id: int, session: Session = None) -> Optional['TagFile']: + return session.query(TagFile).filter_by(tag_id=tag_id, file_group_id=file_group_id).one_or_none() + class Tag(ModelHelper, Base): __tablename__ = 'tag' @@ -49,37 +54,32 @@ class Tag(ModelHelper, Base): color=self.color, ) + @staticmethod @optional_session - def add_file_group_tag(self, file_group, session: Session = None) -> TagFile: + def tag_file_group(file_group_id, tag_id, session: Session = None) -> TagFile: """Add a TagFile for the provided FileGroup and this Tag. @warning: Commits the session to keep the config in sync.""" - from wrolpi.files.models import FileGroup - if not isinstance(file_group, FileGroup): - raise ValueError('Cannot apply tag to non-FileGroup') - - tag_file = TagFile(file_group_id=file_group.id, tag_id=self.id) - logger.info(f'Tagging {file_group} with {self}') + existing = TagFile.get_by_primary_keys(file_group_id, tag_id, session) + if existing: + raise FileGroupAlreadyTagged('Tag already used') + tag_file = TagFile(file_group_id=file_group_id, tag_id=tag_id) session.add(tag_file) session.flush([tag_file]) session.commit() + logger.info(f'Tagged FileGroup {file_group_id} with Tag {tag_id}') # Save changes to config. schedule_save(session) return tag_file + @staticmethod @optional_session - def remove_file_group_tag(self, file_group, session: Session = None): + def untag_file_group(file_group_id: int, tag_id: int, session: Session = None): """Remove the record of a Tag applied to the FileGroup. @warning: Commits the session to keep config in sync.""" - from wrolpi.files.models import FileGroup - if not isinstance(file_group, FileGroup): - raise ValueError('Cannot remove tag of non-FileGroup') - - tag_file = session.query(TagFile) \ - .filter(TagFile.file_group_id == file_group.id, TagFile.tag_id == self.id) \ - .one_or_none() + tag_file = TagFile.get_by_primary_keys(file_group_id, tag_id, session) if tag_file: session.delete(tag_file) session.commit() @@ -87,14 +87,20 @@ class Tag(ModelHelper, Base): # Save changes to config. schedule_save(session) else: - logger.warning(f'Could not find tag_file for FileGroup.id={file_group.id}/Tag.id={self.id=}') + logger.warning(f'Could not find TagFile for FileGroup.id={file_group_id}/Tag.id={tag_id}') @staticmethod @optional_session - def find_by_name(name: str, session: Session) -> 'Tag': + def get_by_name(name: str, session: Session) -> Optional['Tag']: tag = session.query(Tag).filter_by(name=name).one_or_none() return tag + @staticmethod + @optional_session + def get_by_id(id_: int, session: Session = None) -> Optional[Base]: + tag = session.query(Tag).filter_by(id=id_).one_or_none() + return tag + def has_relations(self) -> bool: """Returns True if this Tag has been used with any FileGroups or Zim Entries.""" return bool(self.tag_files or self.tag_zim_entries) @@ -354,7 +360,7 @@ def import_tags_config(session: Session = None): tag_file: TagFile = tag_files.get((tag.id, file_group.id)) if not tag_file: # This FileGroup has not been tagged with the Tag, add it. - tag_file = tag.add_file_group_tag(file_group, session) + tag_file = tag.tag_file_group(file_group, session) tag_file.created_at = dates.strptime_ms(created_at) if created_at else dates.now() need_commit = True elif not file_group: @@ -491,11 +497,3 @@ def tag_names_to_zim_sub_select(tag_names: List[str], zim_id: int = None) -> Tup HAVING array_agg(t.name)::TEXT[] @> %(tag_names)s::TEXT[] ''' return stmt, params - - -if __name__ == '__main__': - from wrolpi.db import get_db_session - - if input('Save Tags to config file? (y/n) ').strip().lower() == 'y': - with get_db_session() as session: - get_tags_config().save_tags(session) diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index ec3da763..8ecfafb7 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -8,29 +8,29 @@ from wrolpi import tags from wrolpi.errors import FileGroupIsTagged from wrolpi.files import lib as files_lib from wrolpi.files.models import FileGroup -from wrolpi.tags import TagFile +from wrolpi.tags import TagFile, Tag @pytest.mark.asyncio -async def test_tags_file_group_json(test_session, make_files_structure, tag_factory, example_pdf): - """The tags of a file are returned in it's JSON.""" +async def test_tags_file_group_json(test_async_client, test_session, make_files_structure, tag_factory, example_pdf): + """The tags of a file are returned in its JSON.""" tag_one = tag_factory() tag_two = tag_factory() await files_lib.refresh_files() file_group: FileGroup = test_session.query(FileGroup).one() - tag_one.add_file_group_tag(file_group) + Tag.tag_file_group(file_group.id, tag_one.id) assert file_group.__json__()['tags'] == ['one'] - file_group.add_tag(tag_two) + file_group.add_tag(tag_two.id) assert file_group.__json__()['tags'] == ['one', 'two'] - file_group.remove_tag(tag_one) + file_group.untag(tag_one.id) assert file_group.__json__()['tags'] == ['two'] @pytest.mark.asyncio -async def test_tags_file_group(test_session, make_files_structure, tag_factory, video_bytes, image_bytes_factory): +async def test_tags_file_group(test_async_client, 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_bytes, 'video.png': image_bytes_factory(), @@ -41,13 +41,13 @@ async def test_tags_file_group(test_session, make_files_structure, tag_factory, tag_one = tag_factory() assert len(tag_one.tag_files) == 0 - tag_one.add_file_group_tag(video_group) + Tag.tag_file_group(video_group.id, tag_one.id) test_session.commit() assert len(tag_one.tag_files) == 1 assert sorted([i.tag.name for i in video_group.tag_files]) == ['one', ] tag_two = tag_factory() - tag_two.add_file_group_tag(video_group) + Tag.tag_file_group(video_group.id, tag_two.id) test_session.commit() assert len(tag_one.tag_files) == 1 assert len(tag_two.tag_files) == 1 @@ -64,7 +64,7 @@ async def test_tags_file_group(test_session, make_files_structure, tag_factory, @pytest.mark.asyncio -async def test_tags_config_(test_session, test_directory, tag_factory, example_pdf, video_file, test_tags_config): +async def test_tags_config_(test_async_client, test_session, test_directory, tag_factory, example_pdf, video_file, test_tags_config): """Test that the config is updated when a FileGroup is tagged.""" await files_lib.refresh_files() pdf: FileGroup = FileGroup.get_by_path(example_pdf, test_session) @@ -77,27 +77,27 @@ async def test_tags_config_(test_session, test_directory, tag_factory, example_p assert tag1.name in test_tags_config.read_text() assert tag2.name in test_tags_config.read_text() - video.add_tag(tag1) + video.add_tag(tag1.id) assert str(video_file.relative_to(test_directory)) in test_tags_config.read_text() assert str(example_pdf.relative_to(test_directory)) not in test_tags_config.read_text() # Tag PDF twice. - pdf.add_tag(tag1) - pdf.add_tag(tag2) + pdf.add_tag(tag1.id) + pdf.add_tag(tag2.id) assert str(video_file.relative_to(test_directory)) in test_tags_config.read_text() assert str(example_pdf.relative_to(test_directory)) in test_tags_config.read_text() # One TagFile still exists. - pdf.remove_tag(tag1) + pdf.untag(tag1.id) assert str(video_file.relative_to(test_directory)) in test_tags_config.read_text() assert str(example_pdf.relative_to(test_directory)) in test_tags_config.read_text() # PDF is no longer tagged. - pdf.remove_tag(tag2) + pdf.untag(tag2.id) assert str(video_file.relative_to(test_directory)) in test_tags_config.read_text() assert str(example_pdf.relative_to(test_directory)) not in test_tags_config.read_text() - video.remove_tag(tag1) + video.untag(tag1.id) assert str(video_file.relative_to(test_directory)) not in test_tags_config.read_text() assert str(example_pdf.relative_to(test_directory)) not in test_tags_config.read_text() @@ -106,28 +106,29 @@ async def test_tags_config_(test_session, test_directory, tag_factory, example_p assert isinstance(tags_config.tag_files, list) and len(tags_config.tag_files) == 0 # Removing non-existent tag does not error. - video.remove_tag(tag1) + video.untag(tag1.id) -def test_tags_crud(test_session, test_client, example_pdf, assert_tags_config): +@pytest.mark.asyncio +async def test_tags_crud(test_async_client, test_session, example_pdf, assert_tags_config): """Test API Create/Retrieve/Update/Delete of Tags.""" pdf = FileGroup.from_paths(test_session, example_pdf) test_session.add(pdf) test_session.commit() # Can get empty tags. - request, response = test_client.get('/api/tag') + request, response = await test_async_client.get('/api/tag') assert response.status_code == HTTPStatus.OK assert response.json['tags'] == list() # Tags can be created. content = dict(name='jardín', color='#123456') - request, response = test_client.post('/api/tag', content=json.dumps(content)) + request, response = await test_async_client.post('/api/tag', content=json.dumps(content)) assert response.status_code == HTTPStatus.CREATED assert_tags_config(tags={'jardín': {'color': '#123456'}}) # The tag can be retrieved. - request, response = test_client.get('/api/tag') + request, response = await test_async_client.get('/api/tag') assert response.status_code == HTTPStatus.OK assert response.json['tags'] == [dict(name='jardín', color='#123456', id=1, file_group_count=0, zim_entry_count=0)] @@ -138,28 +139,28 @@ def test_tags_crud(test_session, test_client, example_pdf, assert_tags_config): # The tag can be updated. content = dict(name='ガーデン', color='#000000') - request, response = test_client.post('/api/tag/1', content=json.dumps(content)) + request, response = await test_async_client.post('/api/tag/1', content=json.dumps(content)) assert response.status_code == HTTPStatus.OK - request, response = test_client.get('/api/tag') + request, response = await test_async_client.get('/api/tag') assert response.json['tags'] == [ dict(name='ガーデン', color='#000000', id=1, file_group_count=1, zim_entry_count=0)] assert_tags_config(tags={'ガーデン': {'color': '#000000'}}) # Conflicting names return an error. content = dict(name='ガーデン', color='#111111') - request, response = test_client.post('/api/tag', content=json.dumps(content)) + request, response = await test_async_client.post('/api/tag', content=json.dumps(content)) assert response.status_code == HTTPStatus.BAD_REQUEST assert_tags_config(tags={'ガーデン': {'color': '#000000'}}) # Cannot delete Tag that is used. - request, response = test_client.delete('/api/tag/1') + request, response = await test_async_client.delete('/api/tag/1') assert response.status_code == HTTPStatus.BAD_REQUEST - pdf.remove_tag(tag, test_session) + pdf.untag(tag, test_session) test_session.commit() # Can delete unused Tag. - request, response = test_client.delete('/api/tag/1') + request, response = await test_async_client.delete('/api/tag/1') assert response.status_code == HTTPStatus.NO_CONTENT # Config is empty.