From a98aebfddae461b171ab125a3dde603e6051b056 Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 1 Aug 2024 13:26:31 -0600 Subject: [PATCH] A video that is uploaded is attached to its Channel. Saving Channels config with both download url and frequency in case downloads config is corrupted. --- modules/videos/channel/lib.py | 3 +- modules/videos/channel/test/test_models.py | 2 +- modules/videos/conftest.py | 4 +- modules/videos/models.py | 81 ++++++++++++---------- modules/videos/normalize_video_url.py | 2 +- modules/videos/test/test_common.py | 17 +++-- modules/videos/test/test_downloader.py | 6 +- modules/videos/test/test_lib.py | 2 +- requirements.txt | 2 +- scripts/install_ubuntu_20.04.sh | 6 -- wrolpi/admin.py | 6 +- wrolpi/downloader.py | 2 +- wrolpi/files/ebooks.py | 1 + wrolpi/files/lib.py | 6 +- wrolpi/files/models.py | 6 +- wrolpi/files/test/test_api.py | 6 ++ wrolpi/files/test/test_lib.py | 26 +++++++ 17 files changed, 109 insertions(+), 69 deletions(-) delete mode 100755 scripts/install_ubuntu_20.04.sh diff --git a/modules/videos/channel/lib.py b/modules/videos/channel/lib.py index cd27efec..bba55415 100644 --- a/modules/videos/channel/lib.py +++ b/modules/videos/channel/lib.py @@ -221,8 +221,7 @@ async def create_channel_download(channel_id: int, url: str, frequency: int, set """Create Download for Channel.""" with get_db_session(commit=True) as session: channel = Channel.find_by_id(channel_id, session=session) - download = channel.get_or_create_download(url, session, reset_attempts=True) - download.frequency = download.frequency or frequency + download = channel.get_or_create_download(url, frequency, session, reset_attempts=True) download.settings = settings save_channels_config() diff --git a/modules/videos/channel/test/test_models.py b/modules/videos/channel/test/test_models.py index 0103dccc..036ddacc 100644 --- a/modules/videos/channel/test/test_models.py +++ b/modules/videos/channel/test/test_models.py @@ -97,7 +97,7 @@ async def test_channel_download_relationships(test_session, download_channel): assert test_session.query(Channel).one(), 'Channel should not have been deleted.' # Create a Download again. - download_channel.get_or_create_download('https://example.com/1', test_session, reset_attempts=True) + download_channel.get_or_create_download('https://example.com/1', 60, test_session, reset_attempts=True) assert test_session.query(Download).count() == 1 # Delete the Channel, and any relationships. download_channel.delete_with_videos() diff --git a/modules/videos/conftest.py b/modules/videos/conftest.py index 25b054d2..0b417f96 100644 --- a/modules/videos/conftest.py +++ b/modules/videos/conftest.py @@ -53,7 +53,7 @@ def channel_factory(test_session, test_directory): test_session.flush([channel]) assert channel.id and channel.url if download_frequency: - download = channel.get_or_create_download(channel.url) + download = channel.get_or_create_download(channel.url, 60) download.frequency = download_frequency assert download.url == channel.url assert channel.downloads @@ -72,7 +72,7 @@ def download_channel(test_session, test_directory, video_download_manager) -> Ch test_session.add(channel) test_session.flush([channel, ]) assert channel and channel.id and channel.url - download = channel.get_or_create_download(channel.url, test_session) + download = channel.get_or_create_download(channel.url, 60, test_session) assert download.url == channel.url download.frequency = DownloadFrequency.weekly test_session.commit() diff --git a/modules/videos/models.py b/modules/videos/models.py index 2f77e990..9a3d1ac2 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -12,6 +12,7 @@ from wrolpi.captions import read_captions from wrolpi.common import Base, ModelHelper, logger, get_media_directory, background_task, replace_file from wrolpi.db import get_db_curs, get_db_session, optional_session from wrolpi.downloader import Download, download_manager +from wrolpi.errors import InvalidDownload from wrolpi.files.lib import refresh_files, split_path_stem_and_suffix from wrolpi.files.models import FileGroup from wrolpi.media_path import MediaPathType @@ -236,6 +237,11 @@ class Video(ModelHelper, Base): if (comments := self.get_comments()) and len(comments) >= 5: self.have_comments = True + # If this Video is in a Channel's directory, then it is part of that Channel. + session: Session = Session.object_session(self) + if session and (channel := Channel.get_by_path(self.video_path.parent, session)): + self.channel = channel + @staticmethod def from_paths(session: Session, *paths: pathlib.Path) -> 'Video': file_group = FileGroup.from_paths(session, *paths) @@ -542,14 +548,13 @@ class Channel(ModelHelper, Base): download.delete() # Must commit before final deletion. - # TODO why? - # session.commit() session.delete(self) def update(self, data: dict): """ Update the attributes of this Channel. Will also update the Channel's Download, if it has one. """ + channel_id = self.id data = data.copy() # URL should not be empty string. @@ -570,28 +575,28 @@ class Channel(ModelHelper, Base): session: Session = Session.object_session(self) for download in downloads: - # `download` may be string from config file. url = None - if isinstance(download, str): - url = download - elif isinstance(download, dict): - if not download.get('frequency'): - logger.error('Refusing to create channel download without frequency') - continue + frequency = None + if isinstance(download, dict): url = download['url'] + # Use download frequency from downloads config before the channels config. + if existing_download := download_manager.get_download(session, url): + frequency = existing_download.frequency + else: + frequency = download.get('frequency') elif isinstance(download, Download): - if not download.frequency: - logger.error('Refusing to create channel download without frequency') - continue url = download.url + frequency = download.frequency + if not url: raise RuntimeError(f'Unknown download type: {download}') - download_ = self.get_or_create_download(url, session=session, reset_attempts=True) - # If `download` is dict, use that frequency. Try to keep the currently set frequency. - frequency = download['frequency'] if isinstance(download, dict) else download_.frequency - download_.frequency = frequency + if not frequency: + logger.error(f'Refusing to create Download for Channel without frequency: {url}') + continue - session.flush() + download = self.get_or_create_download(url, frequency, session=session, reset_attempts=True) + download.channel_id = channel_id + session.add(download) def config_view(self) -> dict: """ @@ -604,7 +609,7 @@ class Channel(ModelHelper, Base): name=self.name, source_id=self.source_id, url=self.url or None, - downloads=[i.url for i in self.downloads] + downloads=[{'url': i.url, 'frequency': i.frequency} for i in self.downloads] ) return config @@ -616,7 +621,12 @@ class Channel(ModelHelper, Base): @staticmethod def get_by_path(path: pathlib.Path, session: Session) -> Optional['Channel']: - return session.query(Channel).filter_by(directory=path).one_or_none() + if not path: + raise RuntimeError('Must provide path to get Channel') + path = pathlib.Path(path) if isinstance(path, str) else path + path = str(path.absolute()) if path.is_absolute() else str(get_media_directory() / path) + channel = session.query(Channel).filter_by(directory=path).one_or_none() + return channel def __json__(self) -> dict: d = dict( @@ -630,7 +640,8 @@ class Channel(ModelHelper, Base): def dict(self, with_statistics: bool = False, with_downloads: bool = True) -> dict: d = super(Channel, self).dict() - d['directory'] = self.directory.relative_to(get_media_directory()) if self.directory else None + d['directory'] = \ + self.directory.relative_to(get_media_directory()) if self.directory else None if with_statistics: d['statistics'] = self.get_statistics() if with_downloads: @@ -713,21 +724,25 @@ class Channel(ModelHelper, Base): raise RuntimeError(f'More than one info_json entry matches {video_source_id}') @optional_session - def get_or_create_download(self, url: str, session: Session = None, reset_attempts: bool = False) -> Download: + def get_or_create_download(self, url: str, frequency: int, session: Session = None, + reset_attempts: bool = False) -> Download: """Get a Download record, if it does not exist, create it. Create a Download if necessary - which goes into this Channel's directory.""" + which goes into this Channel's directory. + """ if not isinstance(url, str) or not url: - raise RuntimeError(f'Cannot get Download without url') + raise InvalidDownload(f'Cannot get Download without url') + if not frequency: + raise InvalidDownload('Download for Channel must have a frequency') from modules.videos.downloader import ChannelDownloader, VideoDownloader download = download_manager.get_download(session, url) if not download: - download = download_manager.create_download(url, ChannelDownloader.name, session=session, - sub_downloader_name=VideoDownloader.name, - settings=dict(destination=str(self.directory)), - reset_attempts=reset_attempts, - ) + download = download_manager.recurring_download(url, frequency, ChannelDownloader.name, session=session, + sub_downloader_name=VideoDownloader.name, + settings=dict(destination=str(self.directory)), + reset_attempts=reset_attempts, + ) if reset_attempts: download.attempts = 0 download.channel_id = self.id @@ -746,13 +761,3 @@ class Channel(ModelHelper, Base): raise RuntimeError('Must provide URL to get Channel') channel = session.query(Channel).filter_by(url=url).one_or_none() return channel - - @staticmethod - @optional_session - def get_by_directory(directory: pathlib.Path | str, session: Session = None) -> Optional['Channel']: - if not directory: - raise RuntimeError('Must provide directory to get Channel') - directory = pathlib.Path(directory) if isinstance(directory, str) else directory - directory = str(directory.absolute()) if directory.is_absolute() else str(get_media_directory() / directory) - channel = session.query(Channel).filter_by(directory=directory).one_or_none() - return channel diff --git a/modules/videos/normalize_video_url.py b/modules/videos/normalize_video_url.py index 9b949bc2..d05b2536 100644 --- a/modules/videos/normalize_video_url.py +++ b/modules/videos/normalize_video_url.py @@ -21,7 +21,7 @@ def strip_extra_from_url(url: str) -> str: return url -# TODO Move this to a global location. We want an Archive's URL to mat a Video's URL. +# TODO Move this to a global location. We want an Archive's URL to match a Video's URL. def normalize_video_url(url: str) -> str: """Convert video URLs to what is expected to be in the yt-dlp info json.""" diff --git a/modules/videos/test/test_common.py b/modules/videos/test/test_common.py index f439264a..6bcf12f5 100644 --- a/modules/videos/test/test_common.py +++ b/modules/videos/test/test_common.py @@ -18,7 +18,7 @@ from ..common import convert_image, update_view_counts_and_censored, get_video_d from ..lib import save_channels_config, get_channels_config, import_channels_config, ChannelsConfig -def test_get_absolute_media_path(): +def test_get_absolute_media_path(test_directory): path = get_absolute_media_path('videos') assert str(path).endswith('videos') @@ -134,7 +134,8 @@ def test_generate_video_poster(video_file): assert duration == 5 -def test_import_channel_downloads(test_async_client, test_session, channel_factory, test_channels_config): +@pytest.mark.asyncio +async def test_import_channel_downloads(test_async_client, test_session, channel_factory, test_channels_config): """Importing the Channels' config should create any missing download records""" channel1 = channel_factory(source_id='foo', url='https://example.com/channel1') channel2 = channel_factory(source_id='bar', url='https://example.com/channel2') @@ -173,6 +174,7 @@ def test_import_channel_downloads(test_async_client, test_session, channel_facto download: Download = test_session.query(Download).one() assert download.url == channel1.url assert download.frequency + assert download.downloader == 'video_channel' # Download.next_download was not deleted. next_download = str(download.next_download) @@ -181,7 +183,7 @@ def test_import_channel_downloads(test_async_client, test_session, channel_facto assert next_download == str(download.next_download) # Creating Download that matches Channel2's URL means they are related. Delete it and it should be re-created. - channel2.get_or_create_download(channel2.url, test_session) + channel2.get_or_create_download(channel2.url, 60, test_session) save_channels_config() Download.find_by_url(channel2.url).delete(skip=False) test_session.commit() @@ -193,10 +195,11 @@ def test_import_channel_downloads(test_async_client, test_session, channel_facto assert len(downloads) == 1, downloads assert len(channel1.downloads) == 1 assert len(channel2.downloads) == 0 + assert downloads[0].downloader == 'video_channel' # Add a Download to Channel2 which does not match Channel.url. - channel2.get_or_create_download('https://example.org', session=test_session) - channel2.get_or_create_download('https://example.com/channel2', session=test_session) + channel2.get_or_create_download('https://example.org', 60, session=test_session) + channel2.get_or_create_download('https://example.com/channel2', 60, session=test_session) test_session.commit() save_channels_config() # Check config is written to match new URLs. @@ -206,7 +209,8 @@ def test_import_channel_downloads(test_async_client, test_session, channel_facto if channel_config['source_id'] == channel2.source_id: assert channel_config['source_id'] == 'bar' assert len(channel_config['downloads']) == 2 - assert set(channel_config['downloads']) == {'https://example.com/channel2', 'https://example.org'} + assert {i['url'] for i in channel_config['downloads']} \ + == {'https://example.com/channel2', 'https://example.org'} else: assert channel_config['source_id'] == 'foo' @@ -218,6 +222,7 @@ def test_import_channel_downloads(test_async_client, test_session, channel_facto get_channels_config().channels = channels_backup import_channels_config() channel1, channel2 = test_session.query(Channel).order_by(Channel.url).all() + assert test_session.query(Download).count() == 3 assert channel1.url == 'https://example.com/channel1' assert channel2.url == 'https://example.com/channel2' assert {i.url for i in channel1.downloads} == {'https://example.com/channel1'} diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index 863835ad..8af8e7d3 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -136,7 +136,7 @@ async def test_download_channel(test_session, simple_channel, video_download_man mock_video_extract_info.return_value = example_channel_json with mock.patch('modules.videos.downloader.get_channel') as mock_get_channel: mock_get_channel.return_value = simple_channel - simple_channel.get_or_create_download(url, test_session) + simple_channel.get_or_create_download(url, 60, test_session) # Download channel. await video_download_manager.wait_for_all_downloads() # Download videos. @@ -161,7 +161,7 @@ async def test_download_channel(test_session, simple_channel, video_download_man test_session.commit() with mock.patch('modules.videos.downloader.get_channel') as mock_get_channel: mock_get_channel.return_value = simple_channel - download = simple_channel.get_or_create_download(url, test_session) + download = simple_channel.get_or_create_download(url, 60, test_session) download.frequency = 100 download.settings = {'title_include': '2'} await video_download_manager.wait_for_all_downloads() @@ -178,7 +178,7 @@ async def test_download_channel(test_session, simple_channel, video_download_man test_session.commit() with mock.patch('modules.videos.downloader.get_channel') as mock_get_channel: mock_get_channel.return_value = simple_channel - download = simple_channel.get_or_create_download(url, test_session) + download = simple_channel.get_or_create_download(url, 60, test_session) download.frequency = 100 download.settings = {'title_exclude': '2'} await video_download_manager.wait_for_all_downloads() diff --git a/modules/videos/test/test_lib.py b/modules/videos/test/test_lib.py index 8090c77f..0bd3caf0 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -160,7 +160,7 @@ def test_link_channel_and_downloads(test_session, channel_factory, test_download source_id='UCng5u6ASda3LNRXJN0JCQJA', ) download1 = Download(url=channel.get_rss_url(), downloader=RSSDownloader.name) - download2 = Download(url='https://example.com/videos', downloader=RSSDownloader.name, frequency=99, + download2 = Download(url='https://example.com/videos', downloader=ChannelDownloader.name, frequency=99, settings=dict(destination=str(channel.directory))) download3 = Download(url='https://example.com/video/1', downloader=RSSDownloader.name, settings=dict(destination=str(channel.directory))) diff --git a/requirements.txt b/requirements.txt index 33d32372..0b276215 100644 --- a/requirements.txt +++ b/requirements.txt @@ -27,4 +27,4 @@ srt==3.5.3 vininfo[cli]==1.8.0 websockets==12.0 webvtt-py==0.4.6 -yt-dlp==2024.7.16 \ No newline at end of file +yt-dlp==2024.8.1 \ No newline at end of file diff --git a/scripts/install_ubuntu_20.04.sh b/scripts/install_ubuntu_20.04.sh deleted file mode 100755 index 94a59d51..00000000 --- a/scripts/install_ubuntu_20.04.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/usr/bin/env bash -# Ubuntu 20.04 is no longer supported. This script will forward the request to the RPi OS installer. - -# TODO delete this after beta. - -/bin/bash /opt/wrolpi/scripts/install_raspberrypios.sh diff --git a/wrolpi/admin.py b/wrolpi/admin.py index 7ac8240f..c214bfcf 100644 --- a/wrolpi/admin.py +++ b/wrolpi/admin.py @@ -52,7 +52,7 @@ def hotspot_status() -> HotspotStatus: def get_current_ssid(interface='wlan0') -> str | None: - """Returns the name of the SSID that is currently connected. Returns None if Hotspot, or no WIFI network is being + """Returns the name of the SSID that is currently connected. Returns None if Hotspot, or no WI-FI network is being used.""" try: # Run the iwgetid command and capture the output @@ -67,10 +67,10 @@ def get_current_ssid(interface='wlan0') -> str | None: # Hotspot is in use. return None else: - print(f"Error: {result.stderr}") + logger.error(f'Failed to get current ssid: {result.stderr}') return None except Exception as e: - print(f"Exception occurred: {e}") + logger.error('Failed to get current ssid', exc_info=e) return None diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index b3152534..aa326af5 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -541,7 +541,7 @@ class DownloadManager: if channel := Channel.get_by_url(url=download.url, session=session): download.channel_id = channel.id if destination := (download.settings or dict()).get('destination'): - if channel := Channel.get_by_directory(destination): + if channel := Channel.get_by_path(destination, session): download.channel_id = channel.id downloads.append(download) diff --git a/wrolpi/files/ebooks.py b/wrolpi/files/ebooks.py index bcf8e5f0..cec06296 100644 --- a/wrolpi/files/ebooks.py +++ b/wrolpi/files/ebooks.py @@ -268,6 +268,7 @@ def model_ebook(file_group: FileGroup, session: Session) -> EBook: ebook.file_group.title = stem ebook.size = size + session.flush([ebook, ]) return ebook diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index 5d6e00ac..559c1da8 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -1390,7 +1390,7 @@ def remove_ignored_directory(directory: Union[pathlib.Path, str]): @optional_session -async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_names: List[str] = None): +async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_names: List[str] = None) -> FileGroup: """Insert/update all files in the provided file's FileGroup.""" # Update/Insert all files in the FileGroup. paths = glob_shared_stem(pathlib.Path(file)) @@ -1404,7 +1404,8 @@ async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_nam session.flush([file_group, ]) try: - file_group.do_model(session) + if model := file_group.do_model(session): + session.add(model) except Exception as e: logger.error(f'Failed to model FileGroup: {file_group}', exc_info=e) if PYTEST: @@ -1416,5 +1417,6 @@ async def upsert_file(file: pathlib.Path | str, session: Session = None, tag_nam file_group.add_tag(tag.id) session.commit() + return file_group 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 4b35a603..649d5ea6 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -11,13 +11,13 @@ from sqlalchemy.orm import deferred, relationship, Session from sqlalchemy.orm.collections import InstrumentedList from wrolpi.common import Base, ModelHelper, tsvector, logger, recursive_map, get_media_directory, \ - get_relative_to_media_directory, escape_file_name + get_relative_to_media_directory from wrolpi.dates import TZDateTime, now, from_timestamp, strptime_ms, strftime from wrolpi.db import optional_session from wrolpi.errors import FileGroupIsTagged, UnknownFile from wrolpi.files import indexers from wrolpi.media_path import MediaPathType -from wrolpi.tags import Tag, TagFile, get_tags_directory +from wrolpi.tags import Tag, TagFile from wrolpi.vars import PYTEST logger = logger.getChild(__name__) @@ -390,8 +390,10 @@ class FileGroup(ModelHelper, Base): if self.mimetype.startswith('video/'): video = Video(file_group_id=self.id, file_group=self) + session.add(video) video.validate() self.indexed = True + session.flush([video, ]) return video elif self.mimetype.startswith('text') and is_singlefile_file(self.primary_path): archive = model_archive(self, session) diff --git a/wrolpi/files/test/test_api.py b/wrolpi/files/test/test_api.py index f0b7a921..0a5b33e0 100644 --- a/wrolpi/files/test/test_api.py +++ b/wrolpi/files/test/test_api.py @@ -5,6 +5,7 @@ from unittest import mock import pytest +from modules.videos import Video from wrolpi.common import get_wrolpi_config from wrolpi.files import lib from wrolpi.files.lib import get_mimetype @@ -518,6 +519,11 @@ def test_post_upload(test_session, test_client, test_directory, make_files_struc 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' + # Video was modeled, but has no Channel. + video: Video = test_session.query(Video).one() + assert video.video_path + assert not video.info_json_path + assert not video.channel and not video.channel_id @pytest.mark.asyncio diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index 10207b9f..450000d6 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -13,6 +13,7 @@ import pytest from PIL import Image import wrolpi.common +from modules.videos import Video from wrolpi.common import timer from wrolpi.dates import now from wrolpi.errors import InvalidFile, UnknownDirectory, FileGroupIsTagged, NoPrimaryFile @@ -1083,3 +1084,28 @@ def test_replace_file(test_directory): # Non-existent file can be created. wrolpi.common.replace_file(test_directory / 'will now exist', 'foo', missing_ok=True) + + +@pytest.mark.asyncio +async def test_upsert_file_video_with_channel(test_async_client, test_session, test_directory, video_bytes, + channel_factory, video_factory): + """A video file that is uploaded should be assigned to a Channel if it is in the Channel's directory.""" + channel = channel_factory() + # Put the video file in the Channel's directory. + video_file = channel.directory / 'video.mp4' + video_file.write_bytes(video_bytes) + # Put an info json file next to the video file. + info_json_file = video_file.with_suffix('.info.json') + info_json_file.write_text(json.dumps({'duration': 5})) + + # Upsert the file, it should be modeled. + fg: FileGroup = await lib.upsert_file(video_file, test_session) + assert fg.model == 'video', 'Upserted file should have been modeled.' + assert fg.my_video_files(), 'Video file was upserted.' + assert fg.my_json_files(), 'Info json file should have been found near video file.' + + # The video is in the correct Channel and has its files. + video: Video = test_session.query(Video).one() + assert video.channel_id == channel.id, "Videos should be assigned to the Channel their files are in" + assert video.video_path, 'Video file was upserted' + assert video.info_json_path, 'Info json file not found near video file.'