diff --git a/modules/archive/__init__.py b/modules/archive/__init__.py index c15ce597..6e4e0f6c 100644 --- a/modules/archive/__init__.py +++ b/modules/archive/__init__.py @@ -69,6 +69,8 @@ class ArchiveDownloader(Downloader, ABC): archive = await lib.singlefile_to_archive(singlefile, destination=destination) archive_id = archive.id + settings = download.settings or dict() + with get_db_session() as session: archive = Archive.find_by_id(session, archive_id) need_commit = False @@ -80,6 +82,12 @@ class ArchiveDownloader(Downloader, ABC): if need_commit: session.commit() + if parent_download_url := settings.get('parent_download_url'): + try: + archive.file_group.update_wrolpi_json({'parent_download_url': parent_download_url}) + except ValueError as e: + logger.warning(f'Failed to update wrolpi json for {archive}: {e}') + logger.info(f'Successfully downloaded Archive {download.url} {archive}') return DownloadResult(success=True, location=f'/archive/{archive.id}') diff --git a/modules/archive/test/test_models.py b/modules/archive/test/test_models.py index a3a0097c..0db60fed 100644 --- a/modules/archive/test/test_models.py +++ b/modules/archive/test/test_models.py @@ -1,11 +1,13 @@ +import json import pathlib from datetime import datetime import pytest from modules.archive import Archive -from wrolpi.common import get_wrolpi_config +from wrolpi.common import get_media_directory, get_wrolpi_config from wrolpi.errors import UnknownArchive +from wrolpi.files.models import FileGroup def test_archive_get_by_id(test_session, archive_factory): @@ -178,3 +180,42 @@ async def test_archive_validate_rebuilds_data_after_filename_change(async_client archive.validate() assert archive.file_group.data['singlefile_path'] == new_filename + + +def test_update_wrolpi_json_prefers_info_json(test_session, make_files_structure): + """FileGroup.update_wrolpi_json prefers .info.json over .readability.json when both exist.""" + make_files_structure({'page.html': 'test'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'page.html') + test_session.add(fg) + test_session.flush() + + fg.replace_info_json({'title': 'Info Title'}) + fg.replace_readability_json({'title': 'Readability Title'}) + + fg.update_wrolpi_json({'key': 'value'}) + + # wrolpi section should be in .info.json, not .readability.json. + info_json = json.loads((get_media_directory() / 'page.info.json').read_text()) + assert info_json['wrolpi'] == {'key': 'value'} + + readability_json = json.loads((get_media_directory() / 'page.readability.json').read_text()) + assert 'wrolpi' not in readability_json + + +def test_metadata_json_path(test_session, make_files_structure): + """FileGroup.metadata_json_path returns .info.json first, then .readability.json.""" + make_files_structure({'page.html': 'test'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'page.html') + test_session.add(fg) + test_session.flush() + + # No metadata JSON yet. + assert fg.metadata_json_path is None + + # Add readability JSON. + fg.replace_readability_json({'title': 'Test'}) + assert fg.metadata_json_path == get_media_directory() / 'page.readability.json' + + # Add info JSON - it should now be preferred. + fg.replace_info_json({'title': 'Test'}) + assert fg.metadata_json_path == get_media_directory() / 'page.info.json' diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index fea29a79..8fd5dffe 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -672,6 +672,12 @@ class VideoDownloader(Downloader, ABC): if video_info_json_path and (new_info_json := video.clean_info_json()): video.replace_info_json(new_info_json, clean=False) + if parent_download_url := settings.get('parent_download_url'): + try: + video.file_group.update_wrolpi_json({'parent_download_url': parent_download_url}) + except ValueError as e: + logger.warning(f'Failed to update wrolpi json for {video}: {e}') + # Update channel's collection's file_format to track the format used if channel_id: from modules.videos.models import Channel @@ -903,11 +909,19 @@ class VideoDownloader(Downloader, ABC): logger.error('Failed to download video info json', exc_info=e) return DownloadResult(success=False, error='Failed to download video info json') + settings = download.settings or dict() + with get_db_session(commit=True) as session: # Find the existing video, replace its info json. video = Video.get_by_url(url, session) video.replace_info_json(info_json) + if parent_download_url := settings.get('parent_download_url'): + try: + video.file_group.update_wrolpi_json({'parent_download_url': parent_download_url}) + except ValueError as e: + logger.warning(f'Failed to update wrolpi json for {video}: {e}') + if video.get_comments(): video.have_comments = True diff --git a/modules/videos/models.py b/modules/videos/models.py index b5130284..87633d05 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -10,7 +10,7 @@ from sqlalchemy.orm.collections import InstrumentedList from modules.videos.errors import UnknownVideo, UnknownChannel from wrolpi.captions import read_captions from wrolpi.common import Base, ModelHelper, logger, get_media_directory, get_relative_to_media_directory, \ - background_task, replace_file + background_task from wrolpi.db import get_db_curs, get_db_session from wrolpi.downloader import Download from wrolpi.files.lib import split_path_stem_and_suffix @@ -546,19 +546,10 @@ class Video(ModelHelper, Base): def replace_info_json(self, info_json: dict, clean: bool = True, format_: bool = True): """Replace the info json file with the new json dict. Handles adding new info_json file, if necessary.""" - info_json_path = self.info_json_path or self.video_path.with_suffix('.info.json') if clean: info_json = self.clean_info_json(info_json) - if format_: - info_json = json.dumps(info_json, indent=2, sort_keys=True) - else: - info_json = json.dumps(info_json) - - replace_file(info_json_path, info_json, missing_ok=True) - - if not self.info_json_path: - self.file_group.append_files(info_json_path) + self.file_group.replace_info_json(info_json, format_=format_) class Channel(ModelHelper, Base): diff --git a/modules/videos/test/test_models.py b/modules/videos/test/test_models.py index 6b367fd0..b8a0ffda 100644 --- a/modules/videos/test/test_models.py +++ b/modules/videos/test/test_models.py @@ -1,7 +1,10 @@ +import json + import pytest from modules.videos.models import Video -from wrolpi.common import get_relative_to_media_directory +from wrolpi.common import get_media_directory, get_relative_to_media_directory +from wrolpi.files.models import FileGroup @pytest.mark.asyncio() @@ -47,3 +50,51 @@ async def test_channel_move(async_client, test_session, test_directory, channel_ assert not extra_file.is_file() assert (test_directory / 'foo/New Channel Directory/extra.txt').is_file() assert (test_directory / 'foo/New Channel Directory/extra.txt').read_text() == 'extra stuff' + + +def test_replace_info_json_compact(test_session, make_files_structure): + """FileGroup.replace_info_json can write compact JSON.""" + make_files_structure({'video.mp4': 'fake video'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + info = {'title': 'Test', 'id': '123'} + fg.replace_info_json(info, format_=False) + + json_path = get_media_directory() / 'video.info.json' + content = json_path.read_text() + assert '\n' not in content + assert json.loads(content) == info + + +def test_replace_info_json_multiple_json_files(test_session, make_files_structure): + """FileGroup.replace_info_json raises ValueError when multiple .info.json files are tracked.""" + make_files_structure({ + 'video.mp4': 'fake video', + 'video.info.json': '{"title": "one"}', + 'video.other.info.json': '{"title": "two"}', + }) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + fg.append_files(get_media_directory() / 'video.info.json', get_media_directory() / 'video.other.info.json') + + with pytest.raises(ValueError, match='multiple .info.json files'): + fg.replace_info_json({'title': 'new'}) + + +def test_update_wrolpi_json_merges(test_session, make_files_structure): + """FileGroup.update_wrolpi_json merges new keys without clobbering existing ones.""" + make_files_structure({'video.mp4': 'fake video'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + fg.replace_info_json({'title': 'Test', 'wrolpi': {'existing_key': 'keep'}}) + fg.update_wrolpi_json({'new_key': 'new_value'}) + + json_path = get_media_directory() / 'video.info.json' + written = json.loads(json_path.read_text()) + assert written['wrolpi'] == {'existing_key': 'keep', 'new_key': 'new_value'} diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index 9ca97e64..8e3d39cc 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -1,4 +1,5 @@ import copy +import json import pathlib import shutil import urllib.parse @@ -11,7 +12,7 @@ from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import deferred, relationship, Session from wrolpi.common import Base, ModelHelper, tsvector, logger, recursive_map, get_media_directory, \ - get_relative_to_media_directory, unique_by_predicate + get_relative_to_media_directory, unique_by_predicate, replace_file from wrolpi.dates import TZDateTime, now, from_timestamp, strptime_ms, strftime from wrolpi.db import get_db_session from wrolpi.downloader import Download @@ -356,6 +357,92 @@ class FileGroup(ModelHelper, Base): def my_html_paths(self) -> List[pathlib.Path]: return [i['path'] for i in self.my_html_files()] + def _find_unique_json(self, suffix: str) -> pathlib.Path | None: + """Find a JSON file ending with `suffix` in this FileGroup's files. + + @raise ValueError: If multiple files matching the suffix are found.""" + matches = [f for f in self.my_json_files() if f['path'].name.endswith(suffix)] + if len(matches) > 1: + raise ValueError(f'{self} has multiple {suffix} files: {[f["path"] for f in matches]}') + if matches: + return matches[0]['path'] + return None + + @property + def info_json_path(self) -> pathlib.Path | None: + """Find the .info.json file in this FileGroup's files. + + @raise ValueError: If multiple .info.json files are found.""" + return self._find_unique_json('.info.json') + + @property + def readability_json_path(self) -> pathlib.Path | None: + """Find the .readability.json file in this FileGroup's files. + + @raise ValueError: If multiple .readability.json files are found.""" + return self._find_unique_json('.readability.json') + + @property + def metadata_json_path(self) -> pathlib.Path | None: + """Find the primary metadata JSON file for this FileGroup. + + Returns .info.json if it exists, otherwise .readability.json. + @raise ValueError: If multiple files of either type are found.""" + return self.info_json_path or self.readability_json_path + + def replace_info_json(self, info_json: dict, format_: bool = True): + """Write `info_json` to this FileGroup's .info.json file. + + Finds the .info.json via self. Creates the file if it doesn't exist. + Raises ValueError if multiple .info.json files are tracked. + """ + json_path = self.info_json_path or self.primary_path.with_suffix('.info.json') + self._write_json(json_path, info_json, format_=format_) + + def replace_readability_json(self, data: dict, format_: bool = True): + """Write `data` to this FileGroup's .readability.json file. + + Finds the .readability.json via self. Creates the file if it doesn't exist. + Raises ValueError if multiple .readability.json files are tracked. + """ + json_path = self.readability_json_path or self.primary_path.with_suffix('.readability.json') + self._write_json(json_path, data, format_=format_) + + def _write_json(self, json_path: pathlib.Path, data: dict, format_: bool = True): + """Write `data` as JSON to `json_path` and register the file with this FileGroup.""" + if format_: + contents = json.dumps(data, indent=2, sort_keys=True) + else: + contents = json.dumps(data) + + replace_file(json_path, contents, missing_ok=True) + + # Register the file if not already tracked. + tracked_paths = {f['path'] for f in self.files} if self.files else set() + if json_path.name not in tracked_paths: + self.append_files(json_path) + + def update_wrolpi_json(self, data: dict): + """Read this FileGroup's metadata JSON, merge `data` into its `wrolpi` section, and write back. + + Uses .info.json if it exists, otherwise .readability.json. + No-op if no metadata JSON file exists. + """ + json_path = self.metadata_json_path + if not json_path or not json_path.is_file(): + return + + contents = json.loads(json_path.read_text()) + wrolpi_section = contents.get('wrolpi', {}) + wrolpi_section.update(data) + contents['wrolpi'] = wrolpi_section + + # Write back using the appropriate method based on file type. + if json_path.name.endswith('.readability.json'): + self.replace_readability_json(contents) + else: + self.replace_info_json(contents) + def delete(self, add_to_skip_list: bool = True): """Delete this FileGroup record, and all of its files. diff --git a/wrolpi/files/test/test_models.py b/wrolpi/files/test/test_models.py index 8ac7fdeb..14408b4a 100644 --- a/wrolpi/files/test/test_models.py +++ b/wrolpi/files/test/test_models.py @@ -1,3 +1,5 @@ +import json + import pytest from wrolpi.common import get_wrolpi_config @@ -66,3 +68,69 @@ async def test_add_tag_overload(async_client, test_session, make_files_structure assert set(foo.tag_names) == {'two'}, 'Tag should have been found by name' foo.untag(test_session, two.id) assert not foo.tag_names, 'Tag should have been found by ID.' + + +@pytest.mark.parametrize('method,suffix', [ + ('replace_info_json', '.info.json'), + ('replace_readability_json', '.readability.json'), +]) +def test_file_group_replace_json(test_session, make_files_structure, method, suffix): + """replace_info_json and replace_readability_json create, write, and register JSON files.""" + from wrolpi.common import get_media_directory + make_files_structure({'video.mp4': 'fake video'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + json_path = get_media_directory() / f'video{suffix}' + assert not json_path.exists(), 'File should not exist before writing' + + data = {'title': 'Test', 'id': 'abc123'} + getattr(fg, method)(data) + + # File should exist on disk with formatted JSON. + assert json_path.is_file() + assert json.loads(json_path.read_text()) == data + + # File should be tracked in FileGroup.files. + file_paths = [f['path'] for f in fg.my_files()] + assert json_path in file_paths + + +@pytest.mark.parametrize('method,suffix', [ + ('replace_info_json', '.info.json'), + ('replace_readability_json', '.readability.json'), +]) +def test_file_group_update_wrolpi_json(test_session, make_files_structure, method, suffix): + """FileGroup.update_wrolpi_json adds a 'wrolpi' section to .info.json or .readability.json.""" + from wrolpi.common import get_media_directory + make_files_structure({'video.mp4': 'fake video'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + # Write initial JSON using the appropriate method. + getattr(fg, method)({'title': 'Test Video', 'id': 'abc123'}) + + fg.update_wrolpi_json({'parent_download_url': 'https://example.com'}) + + json_path = get_media_directory() / f'video{suffix}' + written = json.loads(json_path.read_text()) + assert written['title'] == 'Test Video' + assert written['wrolpi'] == {'parent_download_url': 'https://example.com'} + + +def test_file_group_update_wrolpi_json_noop_no_metadata_json(test_session, make_files_structure): + """FileGroup.update_wrolpi_json is a no-op when no metadata JSON file exists.""" + from wrolpi.common import get_media_directory + make_files_structure({'video.mp4': 'fake video'}) + fg = FileGroup.from_paths(test_session, get_media_directory() / 'video.mp4') + test_session.add(fg) + test_session.flush() + + # Should not raise, and should not create any file. + fg.update_wrolpi_json({'key': 'value'}) + assert not (get_media_directory() / 'video.info.json').exists() + assert not (get_media_directory() / 'video.readability.json').exists() + +