diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 9c765b34..71b084ea 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -65,6 +65,7 @@ def get_domain_directory(url: str) -> pathlib.Path: return directory +# File names include domain and datetime. MAXIMUM_ARCHIVE_FILE_CHARACTER_LENGTH = 200 diff --git a/modules/archive/test/test_lib.py b/modules/archive/test/test_lib.py index 78555dd4..4bc8c80a 100644 --- a/modules/archive/test/test_lib.py +++ b/modules/archive/test/test_lib.py @@ -359,7 +359,7 @@ async def test_get_title_from_html(test_directory, test_session, fake_now): @skip_circleci -def test_get_new_archive_files_length(fake_now): +def test_get_new_archive_files_length(test_directory, fake_now): """Archive titles are truncated to fit file system length limit. (255-character limit for most file systems)""" fake_now(datetime(2001, 1, 1)) archive_files = get_new_archive_files('https://example.com', 'a' * 500) @@ -372,7 +372,7 @@ def test_get_new_archive_files_length(fake_now): @skip_circleci def test_get_new_archive_files(fake_now): - """Archive files have a specific format so they are sorted by datetime, and are near each other.""" + """Archive files have a specific format, so they are sorted by datetime, and are near each other.""" fake_now(datetime(2001, 1, 1)) archive_files = get_new_archive_files('https://example.com/two', None) assert str(archive_files.singlefile).endswith('archive/example.com/2001-01-01-00-00-00_NA.html') diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index 53c89505..89e05dc3 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -1,5 +1,6 @@ #! /usr/bin/env python3 import json +import os.path import pathlib import traceback from abc import ABC @@ -12,7 +13,7 @@ from yt_dlp.extractor import YoutubeTabIE # noqa from wrolpi.cmd import YT_DLP_BIN from wrolpi.common import logger, get_media_directory, escape_file_name, resolve_generators, background_task, \ - format_json_file + format_json_file, trim_file_name from wrolpi.dates import now from wrolpi.db import get_db_session from wrolpi.db import optional_session @@ -62,7 +63,9 @@ def extract_info(url: str, ydl: YoutubeDL = YDL, process=False) -> dict: def prepare_filename(entry: dict, ydl: YoutubeDL = YDL) -> str: """Get filename from YoutubeDL. Separated for testing.""" - return ydl.prepare_filename(entry) + dir_name, file_name = os.path.split(ydl.prepare_filename(entry)) + file_name = trim_file_name(file_name) + return f'{dir_name}/{file_name}' class ChannelDownloader(Downloader, ABC): @@ -332,7 +335,6 @@ class VideoDownloader(Downloader, ABC): return await self.download_info_json(download, video_path) # Do the real download. - file_name_format = '%(uploader)s_%(upload_date)s_%(id)s_%(title)s.%(ext)s' cmd = ( str(YT_DLP_BIN), '-c', # Continue downloads @@ -347,7 +349,7 @@ class VideoDownloader(Downloader, ABC): '--write-info-json', '--merge-output-format', PREFERRED_VIDEO_EXTENSION, '--remux-video', PREFERRED_VIDEO_EXTENSION, - '-o', file_name_format, + '-o', video_path, '--no-cache-dir', '--compat-options', 'no-live-chat', # Get top 20 comments, 10 replies per parent. @@ -496,7 +498,7 @@ class VideoDownloader(Downloader, ABC): entry = extract_info(url, ydl=ydl, process=True) final_filename = pathlib.Path(prepare_filename(entry, ydl=ydl)).absolute() - logger.debug(f'Downloading {url} to {out_dir}') + logger.debug(f'Downloading {url} to {repr(str(final_filename))}') return final_filename, entry @staticmethod diff --git a/modules/videos/lib.py b/modules/videos/lib.py index 6273343a..4b5fb7b0 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -10,7 +10,7 @@ from sqlalchemy import or_ from sqlalchemy.orm import Session from yt_dlp import YoutubeDL -from wrolpi import before_startup, dates +from wrolpi import before_startup, dates, flags from wrolpi.captions import extract_captions from wrolpi.common import ConfigFile, get_media_directory, register_refresh_cleanup, limit_concurrent from wrolpi.dates import Seconds, from_timestamp @@ -427,14 +427,17 @@ def import_channels_config(): full_data.update(data) channel.update(full_data) - if not channel.source_id and channel.url: + if not channel.source_id and channel.url and flags.have_internet.is_set(): # If we can download from a channel, we must have its source_id. - channel.source_id = get_channel_source_id(channel.url) - save_config = True - if not channel.source_id: - channel_import_logger.warning(f'Unable to fetch source_id for {channel.url}') - else: - session.commit() + try: + channel.source_id = get_channel_source_id(channel.url) + save_config = True + if not channel.source_id: + channel_import_logger.warning(f'Unable to fetch source_id for {channel.url}') + else: + session.commit() + except Exception as e: + logger.error(f'Failed to get Channel source id of {channel}', exc_info=e) channel_import_logger.debug(f'Updated {repr(channel.name)}' f' url={channel.url}' diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index 0edb14eb..cea389e4 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -283,6 +283,9 @@ async def get_missing_videos_comments(limit: int = VIDEO_COMMENTS_FETCH_COUNT): elif 'not a valid URL' in str(e): logger.error(f'URL is not valid: {url=}', exc_info=e) add_video_to_skip_list(url, censored=True) + elif ' Sign in ' in str(e): + logger.error(f'Video requires sign-in: {url=}', exc_info=e) + add_video_to_skip_list(url, censored=True) else: logger.error(f'Got error when attempting to download video comments: {url=}', exc_info=e) continue diff --git a/modules/zim/downloader.py b/modules/zim/downloader.py index 4953121a..ad13db4c 100644 --- a/modules/zim/downloader.py +++ b/modules/zim/downloader.py @@ -2,7 +2,8 @@ from http import HTTPStatus from typing import List from modules.zim import lib -from wrolpi.common import logger, aiohttp_get, get_download_info, download_file, background_task, get_html_soup +from wrolpi.common import logger, aiohttp_get, get_download_info, download_file, background_task, get_html_soup, \ + trim_file_name from wrolpi.downloader import Downloader, Download, DownloadResult __all__ = ['KiwixCatalogDownloader', 'KiwixZimDownloader', 'kiwix_zim_downloader', 'kiwix_catalog_downloader'] @@ -72,7 +73,8 @@ class KiwixZimDownloader(Downloader): zim_directory = lib.get_zim_directory() zim_directory.mkdir(parents=True, exist_ok=True) - output_path = zim_directory / f'{download_info.name}' + name = trim_file_name(download_info.name) + output_path = zim_directory / name if output_path.is_file(): output_size = output_path.stat().st_size diff --git a/scripts/delete_similarly_named_videos.py b/scripts/delete_similarly_named_videos.py index 22a338b7..ccb7dcaf 100644 --- a/scripts/delete_similarly_named_videos.py +++ b/scripts/delete_similarly_named_videos.py @@ -8,12 +8,14 @@ import argparse import asyncio import difflib import functools +import os.path import re +import sys from difflib import SequenceMatcher from typing import List, Generator, Optional, Dict -from modules.videos.models import Video, Channel from modules.videos.lib import parse_video_file_name +from modules.videos.models import Video, Channel from wrolpi.common import get_relative_to_media_directory, chain from wrolpi.db import get_db_curs, get_db_session from wrolpi.files.models import FileGroup @@ -95,6 +97,8 @@ async def rank_video_quality(video: Video, sizes_by_source_id: Dict[str, int]) - rank += 2 if video_file_matches_channel_info_json(video): rank += 2 + if video.get_comments(): + rank += 2 # These are more common. if video.channel_id: rank += 1 @@ -124,7 +128,7 @@ async def rank_video_quality(video: Video, sizes_by_source_id: Dict[str, int]) - def delete_video(video_id: int): with get_db_session(commit=True) as session: - video = session.query(Video).filter_by(id=video_id).one() + video: Video = session.query(Video).filter_by(id=video_id).one() video.delete() @@ -183,12 +187,29 @@ async def handle_user_delete_duplicates(similar_videos: List[Video], video_url: if all_changes.isdigit(): print(f'\nSkipping {similar_videos[0]} and similar because they are parts') return + if delete_lower_ranked_automatically and any(PART_MATCH.match(i.file_group.title) for i in similar_videos): + # "Part" detection is far too inaccurate, do not delete part videos. + print(f'\nSkipping {similar_videos[0]} and similar because they may be parts') + return # Preserve the largest duplicate video. sizes_by_source_id = largest_size_by_source_id(similar_videos) print('\n') ranked_videos = [(await rank_video_quality(i, sizes_by_source_id), i) for i in similar_videos] + # Largest video gets a rank bump. + largest_video_idx = None + largest_video_size = None + for idx, video in enumerate([i[1] for i in ranked_videos]): + if largest_video_size and video.file_group.size > largest_video_size: + largest_video_idx = idx + largest_video_size = video.file_group.size + else: + largest_video_idx = idx + largest_video_size = video.file_group.size + ranked_videos[largest_video_idx] = \ + (ranked_videos[largest_video_idx][0] + 1, ranked_videos[largest_video_idx][1]) + # Order videos by rank, then size. ranked_videos = sorted(ranked_videos, key=lambda i: (i[0], i[1].file_group.size), reverse=True) for rank, video in ranked_videos: fg = video.file_group @@ -212,13 +233,17 @@ async def handle_user_delete_duplicates(similar_videos: List[Video], video_url: index = similar_videos.index(lowest_video) del similar_videos[index] continue - response = input('Delete the lowest ranked video? (y/N)').lower() + response = input('Delete the lowest ranked video? Merge? (y/m/N)').lower() if response == 'y': # Don't need to confirm when the video has been ranked low. delete_video(lowest_video.id) index = similar_videos.index(lowest_video) del similar_videos[index] - elif response == 'n': + elif response == 'm': + success = await handle_merge(ranked_videos) + if success: + break + elif response == 'n' or response == '': break elif response == 'c': raise SkipChannel() @@ -237,6 +262,47 @@ async def handle_user_delete_duplicates(similar_videos: List[Video], video_url: raise SkipChannel() +async def handle_merge(videos: List[tuple[int, Video]]) -> bool | None: + """Replace the video file of Video with better data. Delete all other Videos.""" + response = input( + 'Keep which video file? Empty will replace largest video with best rank. (starting at 0..., n to skip)') + if response == 'n': + return + if response == '': + # Largest Video. + keep_video = sorted(videos, key=lambda i: i[1].video_path.stat().st_size, reverse=True)[0][1] + # Highest ranked video. + keep_data = videos[0][1] + else: + if not response.isdigit() or ((keep_video := int(response)) and keep_video > len(videos) - 1): + print('Invalid file index', file=sys.stderr) + return + if len(videos) == 2: + keep_data = 0 if keep_video == 1 else 1 + else: + response = input('Keep which video data? (starting at 0..., n to skip)') + if response == 'n': + return + if not response.isdigit() or ((keep_data := int(response)) and keep_data > len(videos) - 1): + print('Invalid file index', file=sys.stderr) + return + keep_video = videos[keep_video][1] + keep_data = videos[keep_data][1] + if keep_video == keep_data: + print('Cannot keep and delete same video', file=sys.stderr), + return + # Replace video file of Video with better metadata. + print(f'{keep_video.video_path} -> {keep_data.video_path}') + os.replace(keep_video.video_path, keep_data.video_path) + + for _, video in videos: + if video.id != keep_data.id: + delete_video(video.id) + keep_data.validate() + + return True + + async def main(channel_id: int = None, minimum_rank: float = MINIMUM_RANK, video_url: str = None, directory: str = None, delete_lower_ranked_automatically: bool = False): with get_db_curs() as curs: diff --git a/wrolpi/common.py b/wrolpi/common.py index bff37547..5d94f610 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -223,6 +223,7 @@ __all__ = [ 'slow_logger', 'split_lines_by_length', 'timer', + 'trim_file_name', 'truncate_generator_bytes', 'truncate_object_bytes', 'tsvector', @@ -994,6 +995,25 @@ def escape_file_name(name: str) -> str: return name.strip() +# Maximum length is probably 255, but we need more length for large suffixes like `.readability.json`, and temporary +# downloading suffixes. +MAXIMUM_FILE_LENGTH = 180 + + +def trim_file_name(name: str) -> str: + """Shorten the file name only if it is longer than the file system supports. Trim from the end of the name until + the name is short enough, excluding suffix.""" + if len(name) < MAXIMUM_FILE_LENGTH: + return name + + # Don't trim the filename to exactly 256 characters. + # This is because a FileGroup will have varying filename lengths. + from wrolpi.files.lib import split_path_stem_and_suffix + stem, suffix = split_path_stem_and_suffix(name) + excess = MAXIMUM_FILE_LENGTH - len(suffix) + return stem[:excess].strip() + suffix + + def native_only(func: callable): """Wraps a function. Raises NativeOnly if run while Dockerized.""" diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index f6f9029f..96003085 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -496,7 +496,7 @@ class DownloadManager: -> List[Download]: """Schedule all URLs for download. If one cannot be downloaded, none will be added.""" if not urls or not all(urls): - raise ValueError('Download must have a URL') + raise ValueError(f'Download must have a URL: {urls=}') downloads = [] # Throws an error if no downloader is found. @@ -1093,9 +1093,9 @@ async def signal_download_download(download_id: int, download_url: str): download.error = result.error if result.error else None download.next_download = download_manager.calculate_next_download(download, session) - if result.downloads: + urls = download.filter_excluded(result.downloads) if result.downloads else None + if urls: worker_logger.info(f'Adding {len(result.downloads)} downloads from result of {download.url}') - urls = download.filter_excluded(result.downloads) download_manager.create_downloads(urls, session, downloader_name=download.sub_downloader, settings=result.settings) diff --git a/wrolpi/files/downloader.py b/wrolpi/files/downloader.py index e02dc29f..790445e4 100644 --- a/wrolpi/files/downloader.py +++ b/wrolpi/files/downloader.py @@ -4,7 +4,7 @@ from abc import ABC from typing import List from wrolpi.common import get_media_directory, logger, background_task, get_download_info, \ - download_file + download_file, trim_file_name from wrolpi.db import get_db_session from wrolpi.downloader import Downloader, Download, DownloadResult from wrolpi.errors import UnrecoverableDownloadError @@ -48,7 +48,7 @@ class FileDownloader(Downloader, ABC): info = await get_download_info(download.url) # TODO verify that this output_path is exclusive. - output_path = destination / info.name + output_path = destination / trim_file_name(info.name) try: await download_file(download.url, output_path, info) diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index c9f024a5..c6ffc5cd 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -626,6 +626,8 @@ async def refresh_files(paths: List[pathlib.Path] = None, send_events: bool = Tr if send_events: Events.send_refresh_completed() + api_app.shared_ctx.refresh['counted_files'] = 0 + async def apply_indexers(): """Indexes any Files that have not yet been indexed by Modelers, or by previous calls of this function.""" diff --git a/wrolpi/schema.py b/wrolpi/schema.py index 9b8e3db0..968a900f 100644 --- a/wrolpi/schema.py +++ b/wrolpi/schema.py @@ -68,7 +68,10 @@ class DownloadSettings: def __post_init__(self): from wrolpi.common import get_media_directory self.tag_names = [i.strip() for i in self.tag_names] if self.tag_names else [] - self.excluded_urls = [i.strip() for i in self.excluded_urls.split(',')] if self.excluded_urls else None + if isinstance(self.excluded_urls, str): + self.excluded_urls = [i.strip() for i in self.excluded_urls.split(',')] if self.excluded_urls else None + else: + self.excluded_urls = [i.strip() for i in self.excluded_urls] if self.excluded_urls else None self.destination = str(get_media_directory() / self.destination) if self.destination else None if self.suffix and not self.suffix.startswith('.'): raise ValidationError('suffix must start with .') diff --git a/wrolpi/test/test_common.py b/wrolpi/test/test_common.py index d5b20857..91827280 100644 --- a/wrolpi/test/test_common.py +++ b/wrolpi/test/test_common.py @@ -305,6 +305,17 @@ def test_escape_file_name(name, expected): raise AssertionError(f'Escape of {repr(name)}: {repr(result)} != {repr(expected)}') +@pytest.mark.parametrize('name,expected', [ + ('Channel_20240714_aaa.jpg', 'Channel_20240714_aaa.jpg'), + # Really long filename. The suffix is preserved. + ('Channel_20240714_' + ('a' * 244) + '.jpg', 'Channel_20240714_' + ('a' * 159) + '.jpg'), +]) +def test_trim_file_name(name, expected): + result = common.trim_file_name(name) + assert result == expected, f'Expected length of {len(expected)} but got length of {len(result)}' + assert len(result) <= 255, 'File name cannot be longer than 255 characters' + + def test_truncate_object_bytes(): """ Objects can be truncated (lists will be shortened) so they will fit in tsvector columns.