From da6d6a8094f1b87f78456067bfa500be91c3d464 Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Wed, 30 Aug 2023 14:01:58 -0600
Subject: [PATCH] Allowing starting a download of a channel without frequency. 
 Doing this creates a once-download.

---
 app/src/components/Channels.js         |  1 -
 modules/videos/channel/api.py          |  2 +-
 modules/videos/channel/lib.py          | 27 +++++---
 modules/videos/downloader.py           | 89 +++-----------------------
 modules/videos/models.py               |  6 +-
 modules/videos/test/test_api.py        |  6 +-
 modules/videos/test/test_downloader.py | 43 ++-----------
 wrolpi/downloader.py                   | 31 +++++----
 8 files changed, 54 insertions(+), 151 deletions(-)

diff --git a/app/src/components/Channels.js b/app/src/components/Channels.js
index 9c568b4a..42e026f5 100644
--- a/app/src/components/Channels.js
+++ b/app/src/components/Channels.js
@@ -415,7 +415,6 @@ function ChannelPage({create, header}) {
                                         color='green'
                                         size='small'
                                         onClick={handleDownloadChannel}
-                                        disabled={!channel.url || !channel.download_frequency}
                                         obeyWROLMode={true}
                                     >Download</APIButton>
                                     <APIButton
diff --git a/modules/videos/channel/api.py b/modules/videos/channel/api.py
index 019bad4b..276a3bd5 100644
--- a/modules/videos/channel/api.py
+++ b/modules/videos/channel/api.py
@@ -114,7 +114,7 @@ def channel_refresh(_: Request, channel_id: int):
 @openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse)
 @wrol_mode_check
 def channel_download(_: Request, channel_id: int):
-    lib.download_channel(channel_id)
+    lib.download_channel(channel_id, reset_attempts=True)
     if download_manager.disabled.is_set():
         # Warn the user that downloads are disabled.
         Events.send_downloads_disabled('Channel download created. But downloads are disabled.')
diff --git a/modules/videos/channel/lib.py b/modules/videos/channel/lib.py
index 70eef27b..3ff416b3 100644
--- a/modules/videos/channel/lib.py
+++ b/modules/videos/channel/lib.py
@@ -7,11 +7,12 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from wrolpi.common import run_after, logger, \
     get_media_directory
-from wrolpi.db import get_db_curs, optional_session
-from wrolpi.errors import UnknownDirectory, APIError, ValidationError, InvalidDownload
-from ..errors import UnknownChannel
+from wrolpi.db import get_db_curs, optional_session, get_db_session
+from wrolpi.downloader import download_manager
+from wrolpi.errors import UnknownDirectory, APIError, ValidationError
 from .. import schema
 from ..common import check_for_channel_conflicts
+from ..errors import UnknownChannel
 from ..lib import save_channels_config
 from ..models import Channel
 
@@ -182,16 +183,22 @@ def delete_channel(session: Session, *, channel_id: int):
     return channel_dict
 
 
-def download_channel(id_: int):
+def download_channel(id_: int, reset_attempts: bool = False):
     """Create a Download record for a Channel's entire catalog.  Start downloading."""
+    from modules.videos.downloader import ChannelDownloader, VideoDownloader
     channel: Channel = get_channel(channel_id=id_, return_dict=False)
-    session = Session.object_session(channel)
-    download = channel.get_download()
-    if not download:
-        raise InvalidDownload(f'Channel {channel.name} does not have a download!')
-    download.renew(reset_attempts=True)
+    with get_db_session(commit=True) as session:
+        download = channel.get_download()
+        if not download and channel.url:
+            # Channel does not have recurring download, schedule a once-download.
+            download = download_manager.create_download(
+                session=session,
+                url=channel.url,
+                downloader_name=ChannelDownloader.name,
+                sub_downloader_name=VideoDownloader.name,
+                reset_attempts=reset_attempts,
+            )
     logger.info(f'Created download for {channel} with {download}')
-    session.commit()
 
 
 @optional_session
diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py
index 4b130141..e0ebc0b5 100755
--- a/modules/videos/downloader.py
+++ b/modules/videos/downloader.py
@@ -5,7 +5,6 @@ import re
 import traceback
 from abc import ABC
 from typing import Tuple, List, Dict
-from urllib.parse import urlparse
 
 import yt_dlp.utils
 from sqlalchemy.orm import Session
@@ -24,11 +23,11 @@ from wrolpi.files.lib import glob_shared_stem
 from wrolpi.vars import PYTEST
 from .channel.lib import create_channel, get_channel
 from .common import get_no_channel_directory, get_videos_directory, update_view_counts, ffmpeg_video_complete
-from .errors import UnknownChannel, ChannelURLEmpty
+from .errors import UnknownChannel
 from .lib import get_downloader_config
 from .models import Video, Channel
 from .schema import ChannelPostRequest
-from .video_url_resolver import normalize_youtube_shorts_url, video_url_resolver
+from .video_url_resolver import normalize_youtube_shorts_url
 
 logger = logger.getChild(__name__)
 ydl_logger = logger.getChild('youtube-dl')
@@ -163,13 +162,21 @@ class ChannelDownloader(Downloader, ABC):
             # Only download Videos that have matching titles.
             match_regex = re.compile(channel.match_regex)
             downloads = [i for i in downloads if (title := i.get('title')) and match_regex.match(title)]
+
+        # Do not request videos in the skip list.
+        skip_download_videos = channel.skip_download_videos or []
+        downloads = [i for i in downloads if i['id'] not in skip_download_videos]
+
         # Prefer `webpage_url` before `url` for all entries.
         downloads = [i.get('webpage_url') or i.get('url') for i in downloads]
+
         # YouTube Shorts are handled specially.
         downloads = [normalize_youtube_shorts_url(i) for i in downloads]
+
         # Only download those that have not yet been downloaded.
         already_downloaded = [i.url for i in video_downloader.already_downloaded(*downloads)]
         downloads = [i for i in downloads if i not in already_downloaded]
+
         return downloads
 
 
@@ -506,82 +513,6 @@ def update_channel_catalog(channel: Channel, info: dict):
     background_task(update_view_counts(channel_id))
 
 
-def _find_all_missing_videos(channel_id: id) -> List[Tuple]:
-    """Get all Video entries which don't have the required media files (i.e. hasn't been downloaded)."""
-    with get_db_session() as session:
-        channel: Channel = Channel.find_by_id(channel_id)
-
-        def resolve_url(entry: dict) -> str:
-            parsed = urlparse(entry.get('webpage_url') or entry['url'])
-            domain = parsed.hostname
-            url_ = video_url_resolver(domain, entry)
-            url_ = normalize_youtube_shorts_url(url_)
-            return url_
-
-        info_json = channel.info_json
-        # (source_id, url, channel_id)
-        channel_videos = [(i['id'], resolve_url(i), channel_id) for i in info_json['entries']]
-
-        video_source_ids = [i[0] for i in channel_videos]
-        videos_with_source_id = list(session.query(Video).filter(Video.source_id.in_(video_source_ids)))
-        found_source_ids = set()
-        missing_files = []
-        for video in videos_with_source_id:
-            found_source_ids |= {video.source_id, }
-            if not video.poster_path:
-                missing_files.append(video)
-            elif not video.video_path:
-                missing_files.append(video)
-
-        missing_source_ids = set(video_source_ids) - found_source_ids
-        for source_id in missing_source_ids:
-            yield None, source_id, channel_id
-
-        for video in missing_files:
-            yield video.id, video.source_id, channel_id
-
-
-async def find_all_missing_videos(channel_id: int = None):
-    """
-    Find all videos that don't have a video file, but are found in the DB (taken from the channel's info_json).
-
-    Yields a Channel Dict object, our Video id, and the "entry" of the video from the channel's info_json['entries'].
-    """
-    channel: Channel = get_channel(channel_id=channel_id, return_dict=False)
-    if not channel.url:
-        raise ChannelURLEmpty('No URL for this channel')
-
-    # Check that the channel has some videos.  We can't be sure what is missing if we don't know what we have.
-    if channel.refreshed is False:
-        # Refresh this channel's videos.
-        await channel.refresh_files()
-
-    match_regex = re.compile(channel.match_regex) if channel.match_regex else None
-
-    # Convert the channel video entries into a form that allows them to be quickly retrieved without searching through
-    # the entire entries list.
-    channel_entries = {i['id']: i for i in channel.info_json['entries']}
-    # Yield all videos not skipped.
-    missing_videos = _find_all_missing_videos(channel_id)
-    for video_id, source_id, channel_id in missing_videos:
-        if channel.skip_download_videos and source_id in channel.skip_download_videos:
-            # This video has been marked to skip.
-            continue
-
-        try:
-            missing_video = channel_entries[source_id]
-        except KeyError:
-            logger.warning(f'Video {channel.name} / {source_id} is not in {channel.name} info_json')
-            continue
-
-        if not match_regex:
-            # No title match regex, yield all videos.
-            yield video_id, source_id, missing_video
-        if match_regex and missing_video['title'] and match_regex.match(missing_video['title']):
-            # Title matches the regex.
-            yield video_id, source_id, missing_video
-
-
 UNRECOVERABLE_ERRORS = {
     '404: Not Found',
     'requires payment',
diff --git a/modules/videos/models.py b/modules/videos/models.py
index a9844237..7e571809 100644
--- a/modules/videos/models.py
+++ b/modules/videos/models.py
@@ -100,6 +100,8 @@ class Video(ModelHelper, Base):
         """Add this video to it's Channel's skip list."""
         if self.channel and self.source_id:
             self.channel.add_video_to_skip_list(self.source_id)
+        if self.url:
+            download_manager.add_to_skip_list(self.url)
 
     def set_viewed(self):
         self.viewed = now()
@@ -494,8 +496,8 @@ class Channel(ModelHelper, Base):
         if not self.url:
             return None
 
-        session = Session.object_session(self)
-        download = session.query(Download).filter_by(url=self.url).one_or_none()
+        session: Session = Session.object_session(self)
+        download = download_manager.get_download(session, url=self.url)
         return download
 
     def __json__(self):
diff --git a/modules/videos/test/test_api.py b/modules/videos/test/test_api.py
index 047e52a4..4edd3008 100644
--- a/modules/videos/test/test_api.py
+++ b/modules/videos/test/test_api.py
@@ -147,16 +147,14 @@ def test_channels_with_videos(test_session, test_client, test_directory, channel
 
 def test_api_download_channel(test_session, test_client, simple_channel):
     """A Channel download (a catalog update) can be triggered via the API."""
-    # simple_channel does not have a download record.
-    request, response = test_client.post(f'/api/videos/channels/download/{simple_channel.id}')
-    assert response.status_code == HTTPStatus.BAD_REQUEST, response.json
-    assert 'not have a download' in response.json['error']
+    assert test_session.query(Download).count() == 0
 
     # Add a download frequency to the channel, this should also create a download.
     simple_channel.update(dict(download_frequency=DownloadFrequency.daily))
     test_session.commit()
     request, response = test_client.post(f'/api/videos/channels/download/{simple_channel.id}')
     assert response.status_code == HTTPStatus.NO_CONTENT, response.json
+    assert test_session.query(Download).count() == 1
 
 
 def test_api_download(test_session, test_client, test_directory):
diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py
index cc12e725..81c01a4a 100644
--- a/modules/videos/test/test_downloader.py
+++ b/modules/videos/test/test_downloader.py
@@ -7,7 +7,7 @@ import pytest
 from yt_dlp.utils import UnsupportedError
 
 from modules.videos.channel.lib import download_channel
-from modules.videos.downloader import find_all_missing_videos, VideoDownloader, \
+from modules.videos.downloader import VideoDownloader, \
     get_or_create_channel, channel_downloader, video_downloader
 from modules.videos.models import Channel, Video
 from wrolpi.downloader import Download, DownloadResult
@@ -15,38 +15,6 @@ from wrolpi.errors import InvalidDownload
 from wrolpi.test.common import skip_circleci
 from wrolpi.vars import PROJECT_DIR
 
-
-@pytest.mark.asyncio
-async def test_find_all_missing_videos(test_session, channel_factory, video_factory, image_file):
-    channel1 = channel_factory(url='some url')
-    channel1.info_json = {'entries': [{'id': 'foo', 'view_count': 0, 'webpage_url': 'https://example.com'}]}
-    test_session.commit()
-
-    missing = [i async for i in find_all_missing_videos(channel1.id)]
-    assert len(missing) == 1
-
-    # The video returned is the one we faked.
-    id_, source_id, entry = missing[0]
-    # No Video.id because the video is missing.
-    assert id_ is None
-    # The fake entry we added is regurgitated back.
-    assert entry == channel1.info_json['entries'][0]
-
-    # Create a video without a poster, attempt to download it.
-    video = video_factory(channel_id=channel1.id, with_video_file=True)
-    channel1.info_json = {'entries': [
-        {'id': 'foo', 'view_count': 0, 'webpage_url': 'https://example.com'},
-        {'id': video.source_id, 'view_count': 0, 'webpage_url': 'https://example.com/new'},
-    ]}
-    test_session.commit()
-
-    missing = [i async for i in find_all_missing_videos(channel1.id)]
-    assert len(missing) == 2
-
-    _, new = missing
-    assert new == (video.id, video.source_id, channel1.info_json['entries'][1])
-
-
 example_video_json = {
     'age_limit': 0,
     'average_rating': 4.6923075,
@@ -304,16 +272,15 @@ def test_bad_downloader(test_session, video_download_manager):
         video_download_manager.create_download('https://example.com', downloader_name='bad downloader')
 
 
-def test_channel_download_no_download(test_session, video_download_manager, simple_channel):
+@pytest.mark.asyncio
+async def test_channel_download_no_frequency(test_session, video_download_manager, simple_channel):
     """
-    A Channel without an existing Download cannot be downloaded.  This is to avoid the situation where a single
-    video has been downloaded in a Channel, but the User has not requested downloading of all videos in the channel.
+    A Channel without a download frequency creates a once-download.
     """
     # simple_channel has no download record.
     assert video_download_manager.get_downloads(test_session) == []
 
-    with pytest.raises(InvalidDownload):
-        download_channel(simple_channel.id)
+    download_channel(simple_channel.id)
 
 
 @pytest.mark.asyncio
diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py
index 148e0096..6e8bfa8b 100644
--- a/wrolpi/downloader.py
+++ b/wrolpi/downloader.py
@@ -524,29 +524,28 @@ class DownloadManager:
                          reset_attempts: bool = False, sub_downloader_name: str = None, settings: dict = None) \
             -> List[Download]:
         """Schedule all URLs for download.  If one cannot be downloaded, none will be added."""
-        if not all(urls):
+        if not urls or not all(urls):
             raise ValueError('Download must have a URL')
 
         downloads = []
         # Throws an error if no downloader is found.
         self.get_downloader_by_name(downloader_name)
 
-        with session.transaction:
-            for url in urls:
-                if url in get_download_manager_config().skip_urls and reset_attempts:
-                    # User manually entered this download, remove it from the skip list.
-                    self.remove_from_skip_list(url)
-                elif url in get_download_manager_config().skip_urls:
-                    self.log_warning(f'Skipping {url} because it is in the download_manager.yaml skip list.')
-                    continue
+        for url in urls:
+            if url in get_download_manager_config().skip_urls and reset_attempts:
+                # User manually entered this download, remove it from the skip list.
+                self.remove_from_skip_list(url)
+            elif url in get_download_manager_config().skip_urls:
+                self.log_warning(f'Skipping {url} because it is in the download_manager.yaml skip list.')
+                continue
 
-                download = self.get_or_create_download(url, session)
-                # Download may have failed, try again.
-                download.renew(reset_attempts=reset_attempts)
-                download.downloader = downloader_name
-                download.sub_downloader = sub_downloader_name
-                download.settings = settings if settings is not None else download.settings
-                downloads.append(download)
+            download = self.get_or_create_download(url, session, reset_attempts=reset_attempts)
+            # Download may have failed, try again.
+            download.renew(reset_attempts=reset_attempts)
+            download.downloader = downloader_name
+            download.sub_downloader = sub_downloader_name
+            download.settings = settings if settings is not None else download.settings
+            downloads.append(download)
 
         try:
             # Start downloading ASAP.