diff --git a/app/main.py b/app/main.py index f5154ca..f6aed10 100644 --- a/app/main.py +++ b/app/main.py @@ -130,6 +130,10 @@ class Config: ) sys.exit(1) + self._validate_int('MAX_CONCURRENT_DOWNLOADS', minimum=1) + self._validate_int('PORT', minimum=1, maximum=65535) + self._validate_int('CLEAR_COMPLETED_AFTER', minimum=0) + self._runtime_overrides = {} success,_ = self.load_ytdl_options() @@ -139,6 +143,20 @@ class Config: if not success: sys.exit(1) + def _validate_int(self, key, *, minimum=None, maximum=None): + raw = getattr(self, key) + try: + value = int(raw) + except (TypeError, ValueError): + log.error('Environment variable "%s" must be an integer, got "%s"', key, raw) + sys.exit(1) + if minimum is not None and value < minimum: + log.error('Environment variable "%s" must be >= %d, got "%s"', key, minimum, raw) + sys.exit(1) + if maximum is not None and value > maximum: + log.error('Environment variable "%s" must be <= %d, got "%s"', key, maximum, raw) + sys.exit(1) + def set_runtime_override(self, key, value): self._runtime_overrides[key] = value self.YTDL_OPTIONS[key] = value @@ -241,7 +259,13 @@ logging.getLogger().setLevel(parseLogLevel(str(config.LOGLEVEL)) or logging.INFO class ObjectSerializer(json.JSONEncoder): def default(self, obj): - # First try to use __dict__ for custom objects + # Prefer an explicit client-facing view when the object provides one + # (e.g. DownloadInfo / SubscriptionInfo) so server-only or bulky fields + # are never broadcast to browser clients. + to_public = getattr(obj, 'to_public_dict', None) + if callable(to_public): + return to_public() + # Fall back to __dict__ for other custom objects if hasattr(obj, '__dict__'): return obj.__dict__ # Convert iterables (generators, dict_items, etc.) to lists @@ -827,10 +851,7 @@ async def cancel_add(request): @routes.post(config.URL_PREFIX + 'subscribe') async def subscribe(request): post = await _read_json_request(request) - try: - o = parse_download_options(post) - except web.HTTPBadRequest: - raise + o = parse_download_options(post) cic = post.get('check_interval_minutes') if cic is None: cic = config.SUBSCRIPTION_DEFAULT_CHECK_INTERVAL @@ -964,6 +985,12 @@ async def upload_cookies(request): tmp_cookie_path = f"{COOKIES_PATH}.tmp" with open(tmp_cookie_path, 'wb') as f: f.write(content) + # Cookies are sensitive auth material; restrict to owner read/write only + # (the container's default umask would otherwise leave them group/world readable). + try: + os.chmod(tmp_cookie_path, 0o600) + except OSError as exc: + log.warning(f'Could not restrict permissions on cookies file: {exc}') os.replace(tmp_cookie_path, COOKIES_PATH) config.set_runtime_override('cookiefile', COOKIES_PATH) log.info(f'Cookies file uploaded ({size} bytes)') diff --git a/app/subscriptions.py b/app/subscriptions.py index 0b5774a..9a3c4a3 100644 --- a/app/subscriptions.py +++ b/app/subscriptions.py @@ -312,6 +312,7 @@ class SubscriptionManager: self._subs: dict[str, SubscriptionInfo] = {} self._url_index: dict[str, str] = {} # normalized url -> id self._pending_urls: set[str] = set() + self._checks_in_flight: set[str] = set() # subscription ids being checked right now self._lock = asyncio.Lock() self._loop_task: Optional[asyncio.Task] = None self._load_all() @@ -677,6 +678,22 @@ class SubscriptionManager: return {"status": "ok"} async def _check_one_unlocked(self, sub: SubscriptionInfo) -> None: + sid = sub.id + # Prevent overlapping checks for the same subscription (e.g. the periodic + # loop and a manual check-now firing together), which could double-queue + # entries and drop seen_ids via a read-modify-write race. + async with self._lock: + if sid in self._checks_in_flight: + log.info("Subscription check already in progress for %s, skipping", sub.name) + return + self._checks_in_flight.add(sid) + try: + await self._check_one_inner(sub) + finally: + async with self._lock: + self._checks_in_flight.discard(sid) + + async def _check_one_inner(self, sub: SubscriptionInfo) -> None: sid = sub.id scan = int(getattr(self.config, "SUBSCRIPTION_SCAN_PLAYLIST_END", 50)) log.info("Checking subscription: %s", sub.name) diff --git a/app/tests/test_config.py b/app/tests/test_config.py index 8105e79..bbb6c6d 100644 --- a/app/tests/test_config.py +++ b/app/tests/test_config.py @@ -123,6 +123,29 @@ class ConfigTests(unittest.TestCase): with self.assertRaises(SystemExit): Config() + def test_invalid_max_concurrent_downloads_exits(self): + for bad in ("0", "-1", "abc"): + with patch.dict(os.environ, _base_env(MAX_CONCURRENT_DOWNLOADS=bad), clear=False): + with self.assertRaises(SystemExit): + Config() + + def test_invalid_port_exits(self): + for bad in ("0", "70000", "notaport"): + with patch.dict(os.environ, _base_env(PORT=bad), clear=False): + with self.assertRaises(SystemExit): + Config() + + def test_invalid_clear_completed_after_exits(self): + for bad in ("-5", "soon"): + with patch.dict(os.environ, _base_env(CLEAR_COMPLETED_AFTER=bad), clear=False): + with self.assertRaises(SystemExit): + Config() + + def test_clear_completed_after_zero_allowed(self): + with patch.dict(os.environ, _base_env(CLEAR_COMPLETED_AFTER="0"), clear=False): + c = Config() + self.assertEqual(c.CLEAR_COMPLETED_AFTER, "0") + def test_runtime_override_roundtrip(self): with patch.dict(os.environ, _base_env(), clear=False): c = Config() diff --git a/app/tests/test_download_queue.py b/app/tests/test_download_queue.py index 7e07c58..695788f 100644 --- a/app/tests/test_download_queue.py +++ b/app/tests/test_download_queue.py @@ -627,3 +627,63 @@ def test_seconds_until_next_probe_none_when_empty(dq_env): notifier = AsyncMock() dq = DownloadQueue(dq_env, notifier) assert dq._seconds_until_next_probe() is None + + +def test_calc_download_path_allows_subfolder(dq_env): + notifier = AsyncMock() + dq = DownloadQueue(dq_env, notifier) + path, err = dq._DownloadQueue__calc_download_path("video", "sub/dir") + assert err is None + assert os.path.realpath(path) == os.path.join(os.path.realpath(dq_env.DOWNLOAD_DIR), "sub", "dir") + + +def test_calc_download_path_rejects_sibling_prefix_escape(dq_env): + """A folder resolving to a sibling sharing a name prefix must be rejected. + + Regression test: ``startswith`` would have accepted ``../downloads-secret`` + when the base directory is ``.../downloads``. + """ + notifier = AsyncMock() + base = os.path.realpath(dq_env.DOWNLOAD_DIR) + sibling = base + "-secret" + os.makedirs(sibling, exist_ok=True) + dq = DownloadQueue(dq_env, notifier) + escape_folder = os.path.join("..", os.path.basename(sibling), "x") + path, err = dq._DownloadQueue__calc_download_path("video", escape_folder) + assert path is None + assert err is not None and err["status"] == "error" + + +def test_calc_download_path_rejects_parent_escape(dq_env): + notifier = AsyncMock() + dq = DownloadQueue(dq_env, notifier) + path, err = dq._DownloadQueue__calc_download_path("video", "../../etc") + assert path is None + assert err is not None and err["status"] == "error" + + +def test_download_info_to_public_dict_excludes_server_only_fields(): + info = DownloadInfo( + id="vid1", + title="Test Video", + url="https://example.com/watch?v=1", + quality="best", + download_type="video", + codec="auto", + format="any", + folder="", + custom_name_prefix="", + error=None, + entry={"id": "vid1", "huge": "x" * 100000}, + playlist_item_limit=0, + split_by_chapters=False, + chapter_template="", + ) + info.subtitle_files = [{"filename": "a.srt", "size": 10}] + public = info.to_public_dict() + assert "entry" not in public + assert "subtitle_files" not in public + # Client-facing fields are still present. + assert public["url"] == "https://example.com/watch?v=1" + assert public["title"] == "Test Video" + assert public["status"] == "pending" diff --git a/app/ytdl.py b/app/ytdl.py index 31bca5e..78acc82 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -232,6 +232,20 @@ class DownloadInfo: self.live_release_timestamp = live_release_timestamp self.subtitle_files = [] + # Fields that are useful server-side but must not be broadcast to browser + # clients: ``entry`` is the full yt-dlp info-dict (potentially large and + # re-sent on every progress tick) and ``subtitle_files`` is only used + # internally to derive the primary caption ``filename``. + _PUBLIC_EXCLUDED_FIELDS = ("entry", "subtitle_files") + + def to_public_dict(self) -> dict: + """Return the client-facing view, omitting server-only/bulky fields.""" + return { + k: v + for k, v in self.__dict__.items() + if k not in self._PUBLIC_EXCLUDED_FIELDS + } + def __setstate__(self, state): """BACKWARD COMPATIBILITY: migrate old DownloadInfo from persistent queue files.""" self.__dict__.update(state) @@ -584,7 +598,10 @@ class Download: self.info.filename = rel_name self.info.size = os.path.getsize(fileName) if os.path.exists(fileName) else None if getattr(self.info, 'download_type', '') == 'thumbnail': - self.info.filename = re.sub(r'\.webm$', '.jpg', self.info.filename) + # The thumbnail convertor always emits a .jpg, but yt-dlp may + # report the pre-conversion media/thumbnail extension + # (.webm/.mp4/.png/.webp/...). Normalise to .jpg regardless. + self.info.filename = os.path.splitext(self.info.filename)[0] + '.jpg' # Handle chapter files log.debug(f"Update status for {self.info.title}: {status}") @@ -647,8 +664,8 @@ class PersistentQueue: def __init__(self, name, path): self.identifier = name pdir = os.path.dirname(path) - if not os.path.isdir(pdir): - os.mkdir(pdir) + if pdir and not os.path.isdir(pdir): + os.makedirs(pdir, exist_ok=True) self.legacy_path = path self.path = f"{path}.json" self.store = AtomicJsonStore(self.path, kind=f"persistent_queue:{name}") @@ -1026,7 +1043,16 @@ class DownloadQueue: return None, {'status': 'error', 'msg': 'A folder for the download was specified but CUSTOM_DIRS is not true in the configuration.'} dldirectory = os.path.realpath(os.path.join(base_directory, folder)) real_base_directory = os.path.realpath(base_directory) - if not dldirectory.startswith(real_base_directory): + # Use commonpath rather than startswith so that a sibling directory + # sharing a name prefix (e.g. base "/downloads" vs "/downloads-secret") + # cannot be reached via "../downloads-secret". + try: + inside_base = os.path.commonpath([real_base_directory, dldirectory]) == real_base_directory + except ValueError: + # Raised when paths are on different drives (Windows) or mix + # absolute/relative; treat as outside the base directory. + inside_base = False + if not inside_base: return None, {'status': 'error', 'msg': f'Folder "{folder}" must resolve inside the base download directory "{real_base_directory}"'} if not os.path.isdir(dldirectory): if not self.config.CREATE_CUSTOM_DIRS: @@ -1387,11 +1413,28 @@ class DownloadQueue: continue if self.config.DELETE_FILE_ON_TRASHCAN: dl = self.done.get(id) - try: - dldirectory, _ = self.__calc_download_path(dl.info.download_type, dl.info.folder) - os.remove(os.path.join(dldirectory, dl.info.filename)) - except Exception as e: - log.warning(f'deleting file for download {id} failed with error message {e!r}') + dldirectory, calc_error = self.__calc_download_path(dl.info.download_type, dl.info.folder) + if calc_error is not None or not dldirectory: + log.warning(f'deleting files for download {id} skipped: could not resolve download directory') + else: + # Remove the primary output plus any per-chapter / per-subtitle + # outputs. Each filename is relative to the download directory. + rel_names = [] + if getattr(dl.info, 'filename', None): + rel_names.append(dl.info.filename) + for extra in (getattr(dl.info, 'chapter_files', None) or []): + if isinstance(extra, dict) and extra.get('filename'): + rel_names.append(extra['filename']) + for extra in (getattr(dl.info, 'subtitle_files', None) or []): + if isinstance(extra, dict) and extra.get('filename'): + rel_names.append(extra['filename']) + for rel_name in rel_names: + try: + os.remove(os.path.join(dldirectory, rel_name)) + except FileNotFoundError: + pass + except OSError as e: + log.warning(f'deleting file "{rel_name}" for download {id} failed with error message {e!r}') self.done.delete(id) await self.notifier.cleared(id) return {'status': 'ok'} diff --git a/ui/src/app/app.html b/ui/src/app/app.html index 0bd8300..1299b05 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -1078,3 +1078,5 @@ } + + diff --git a/ui/src/app/app.spec.ts b/ui/src/app/app.spec.ts index a77b171..be19b65 100644 --- a/ui/src/app/app.spec.ts +++ b/ui/src/app/app.spec.ts @@ -4,6 +4,7 @@ import { Subject, of } from 'rxjs'; import { App } from './app'; import { DownloadsService } from './services/downloads.service'; import { SubscriptionsService } from './services/subscriptions.service'; +import { ToastService } from './services/toast.service'; import { CookieService } from 'ngx-cookie-service'; class DownloadsServiceStub { @@ -263,7 +264,8 @@ describe('App', () => { }); it('blocks subscribe with invalid title regex', () => { - const alertSpy = vi.spyOn(window, 'alert').mockImplementation(() => undefined); + const toasts = TestBed.inject(ToastService); + const errorSpy = vi.spyOn(toasts, 'error').mockImplementation(() => undefined); const fixture = TestBed.createComponent(App); const app = fixture.componentInstance; const subs = TestBed.inject(SubscriptionsService) as unknown as SubscriptionsServiceStub; @@ -271,7 +273,7 @@ describe('App', () => { app.titleRegex = '['; app.addSubscription(); expect(subs.subscribeCalls.length).toBe(0); - expect(alertSpy).toHaveBeenCalledWith('Invalid subscription title filter (regex)'); - alertSpy.mockRestore(); + expect(errorSpy).toHaveBeenCalledWith('Invalid subscription title filter (regex)'); + errorSpy.mockRestore(); }); }); diff --git a/ui/src/app/app.ts b/ui/src/app/app.ts index 7245ab3..4f58747 100644 --- a/ui/src/app/app.ts +++ b/ui/src/app/app.ts @@ -13,6 +13,8 @@ import { CookieService } from 'ngx-cookie-service'; import { AddDownloadPayload, DownloadsService } from './services/downloads.service'; import { MeTubeSocket } from './services/metube-socket.service'; import { SubscriptionsService } from './services/subscriptions.service'; +import { ToastService } from './services/toast.service'; +import { BatchUrlsService, BatchUrlFilter } from './services/batch-urls.service'; import { SubscriptionRow } from './interfaces/subscription'; import { Themes } from './theme'; import { @@ -32,7 +34,7 @@ import { State, } from './interfaces'; import { EtaPipe, SpeedPipe, FileSizePipe } from './pipes'; -import { SelectAllCheckboxComponent, ItemCheckboxComponent } from './components/'; +import { SelectAllCheckboxComponent, ItemCheckboxComponent, ToastContainerComponent } from './components/'; @Component({ selector: 'app-root', @@ -50,6 +52,7 @@ import { SelectAllCheckboxComponent, ItemCheckboxComponent } from './components/ FileSizePipe, SelectAllCheckboxComponent, ItemCheckboxComponent, + ToastContainerComponent, ], templateUrl: './app.html', styleUrl: './app.sass', @@ -57,6 +60,8 @@ import { SelectAllCheckboxComponent, ItemCheckboxComponent } from './components/ export class App implements AfterViewInit, OnInit, OnDestroy { downloads = inject(DownloadsService); subscriptionsSvc = inject(SubscriptionsService); + private toasts = inject(ToastService); + private batchUrls = inject(BatchUrlsService); private socket = inject(MeTubeSocket); private cookieService = inject(CookieService); private http = inject(HttpClient); @@ -415,7 +420,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { const date = new Date(data['update_time'] * 1000); this.ytDlpOptionsUpdateTime=date.toLocaleString(); }else{ - alert("Error reload yt-dlp options: "+data['msg']); + this.toasts.error("Error reloading yt-dlp options: " + data['msg']); } this.cdr.markForCheck(); } @@ -490,11 +495,11 @@ export class App implements AfterViewInit, OnInit, OnDestroy { try { const parsed = JSON.parse(trimmed); if (!parsed || Array.isArray(parsed) || typeof parsed !== 'object') { - alert('Custom yt-dlp options must be a JSON object'); + this.toasts.error('Custom yt-dlp options must be a JSON object'); return false; } } catch { - alert('Custom yt-dlp options must be valid JSON'); + this.toasts.error('Custom yt-dlp options must be valid JSON'); return false; } return true; @@ -525,7 +530,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.subscriptionsSvc.refreshList().pipe(takeUntilDestroyed(this.destroyRef)).subscribe((refreshRes) => { const error = this.getStatusError(refreshRes); if (error) { - alert(error || 'Refresh subscriptions failed'); + this.toasts.error(error || 'Refresh subscriptions failed'); return; } this.cdr.markForCheck(); @@ -569,7 +574,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { } const payload = this.buildAddPayload(); if (!payload.url?.trim()) { - alert('Please enter a URL'); + this.toasts.error('Please enter a URL'); return; } const tr = (this.titleRegex || '').trim(); @@ -577,12 +582,12 @@ export class App implements AfterViewInit, OnInit, OnDestroy { try { void RegExp(tr); } catch { - alert('Invalid subscription title filter (regex)'); + this.toasts.error('Invalid subscription title filter (regex)'); return; } } if (payload.splitByChapters && !payload.chapterTemplate.includes('%(section_number)')) { - alert('Chapter template must include %(section_number)'); + this.toasts.error('Chapter template must include %(section_number)'); return; } if (!this.validateYtdlOptionsOverrides(payload.ytdlOptionsOverrides)) { @@ -611,7 +616,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { next: (res) => { const r = res as { status?: string; msg?: string }; if (r.status === 'error') { - alert(r.msg || 'Subscribe failed'); + this.toasts.error(r.msg || 'Subscribe failed'); } else { this.addUrl = ''; this.titleRegex = ''; @@ -639,14 +644,14 @@ export class App implements AfterViewInit, OnInit, OnDestroy { try { void RegExp(raw); } catch { - alert('Invalid subscription title filter (regex)'); + this.toasts.error('Invalid subscription title filter (regex)'); return; } } this.subscriptionsSvc.update(id, { title_regex: raw }).subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Update subscription failed'); + this.toasts.error(error || 'Update subscription failed'); return; } this.cancelEditTitleRegex(); @@ -657,7 +662,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.subscriptionsSvc.delete([id]).subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Delete subscription failed'); + this.toasts.error(error || 'Delete subscription failed'); return; } this.selectedSubscriptionIds.delete(id); @@ -673,7 +678,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.subscriptionsSvc.delete(ids).subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Delete subscriptions failed'); + this.toasts.error(error || 'Delete subscriptions failed'); return; } this.selectedSubscriptionIds.clear(); @@ -699,7 +704,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { .subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Subscription check failed'); + this.toasts.error(error || 'Subscription check failed'); return; } this.refreshSubscriptionsWithAlert(); @@ -746,7 +751,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { .subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Subscription check failed'); + this.toasts.error(error || 'Subscription check failed'); return; } this.refreshSubscriptionsWithAlert(); @@ -769,7 +774,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.subscriptionsSvc.update(row.id, { enabled: !row.enabled }).subscribe((res) => { const error = this.getStatusError(res); if (error) { - alert(error || 'Update subscription failed'); + this.toasts.error(error || 'Update subscription failed'); } }); } @@ -1066,20 +1071,19 @@ export class App implements AfterViewInit, OnInit, OnDestroy { // Validate chapter template if chapter splitting is enabled if (payload.splitByChapters && !payload.chapterTemplate.includes('%(section_number)')) { - alert('Chapter template must include %(section_number)'); + this.toasts.error('Chapter template must include %(section_number)'); return; } if (!this.validateYtdlOptionsOverrides(payload.ytdlOptionsOverrides)) { return; } - console.debug('Downloading:', payload); this.addInProgress = true; this.cancelRequested = false; this.addRequestSub?.unsubscribe(); this.addRequestSub = this.downloads.add(payload).subscribe((status: Status) => { if (status.status === 'error' && !this.cancelRequested) { - alert(`Error adding URL: ${status.msg}`); + this.toasts.error(`Error adding URL: ${status.msg}`); } else if (status.status !== 'error') { this.addUrl = ''; } @@ -1241,10 +1245,12 @@ export class App implements AfterViewInit, OnInit, OnDestroy { // file into memory only to have navigator.canShare reject it. if (download.size && download.size > App.SHARE_SIZE_WARN_BYTES) { const sizeMb = Math.round(download.size / 1024 / 1024); - const proceed = window.confirm( + const proceed = await this.toasts.confirm( `This file is ${sizeMb} MB. iOS' share sheet often refuses files ` + `larger than ~100 MB and the share will silently fail. ` + - `Try anyway? (Use the download button instead if it fails.)` + `Try anyway? (Use the download button instead if it fails.)`, + 'Try anyway', + 'Cancel', ); if (!proceed) return; } @@ -1265,7 +1271,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { // download button right next to this one instead of staring at // a button that quietly did nothing. console.warn('navigator.canShare rejected payload for', download.filename); - window.alert( + this.toasts.error( `Your device's share sheet doesn't accept this file ` + `(most likely because it's too large). ` + `Please use the download button instead.` @@ -1278,7 +1284,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { // AbortError = user dismissed the share sheet → silent no-op. if (e.name === 'AbortError') return; console.error('Share failed:', err); - window.alert( + this.toasts.error( `Share failed: ${e.message || 'unknown error'}. ` + `Please use the download button instead.` ); @@ -1370,7 +1376,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { .map(url => url.trim()) .filter(url => url.length > 0); if (urls.length === 0) { - alert('No valid URLs found.'); + this.toasts.error('No valid URLs found.'); return; } this.importInProgress = true; @@ -1435,62 +1441,13 @@ export class App implements AfterViewInit, OnInit, OnDestroy { } // Export URLs based on filter: 'pending', 'completed', 'failed', or 'all' - exportBatchUrls(filter: 'pending' | 'completed' | 'failed' | 'all'): void { - let urls: string[]; - if (filter === 'pending') { - urls = Array.from(this.downloads.queue.values()).map(dl => dl.url); - } else if (filter === 'completed') { - // Only finished downloads in the "done" Map - urls = Array.from(this.downloads.done.values()).filter(dl => dl.status === 'finished').map(dl => dl.url); - } else if (filter === 'failed') { - // Only error downloads from the "done" Map - urls = Array.from(this.downloads.done.values()).filter(dl => dl.status === 'error').map(dl => dl.url); - } else { - // All: pending + both finished and error in done - urls = [ - ...Array.from(this.downloads.queue.values()).map(dl => dl.url), - ...Array.from(this.downloads.done.values()).map(dl => dl.url) - ]; - } - if (!urls.length) { - alert('No URLs found for the selected filter.'); - return; - } - const content = urls.join('\n'); - const blob = new Blob([content], { type: 'text/plain' }); - const downloadUrl = window.URL.createObjectURL(blob); - const a = document.createElement('a'); - a.href = downloadUrl; - a.download = 'metube_urls.txt'; - document.body.appendChild(a); - a.click(); - document.body.removeChild(a); - window.URL.revokeObjectURL(downloadUrl); + exportBatchUrls(filter: BatchUrlFilter): void { + this.batchUrls.export(filter); } // Copy URLs to clipboard based on filter: 'pending', 'completed', 'failed', or 'all' - copyBatchUrls(filter: 'pending' | 'completed' | 'failed' | 'all'): void { - let urls: string[]; - if (filter === 'pending') { - urls = Array.from(this.downloads.queue.values()).map(dl => dl.url); - } else if (filter === 'completed') { - urls = Array.from(this.downloads.done.values()).filter(dl => dl.status === 'finished').map(dl => dl.url); - } else if (filter === 'failed') { - urls = Array.from(this.downloads.done.values()).filter(dl => dl.status === 'error').map(dl => dl.url); - } else { - urls = [ - ...Array.from(this.downloads.queue.values()).map(dl => dl.url), - ...Array.from(this.downloads.done.values()).map(dl => dl.url) - ]; - } - if (!urls.length) { - alert('No URLs found for the selected filter.'); - return; - } - const content = urls.join('\n'); - navigator.clipboard.writeText(content) - .then(() => alert('URLs copied to clipboard.')) - .catch(() => alert('Failed to copy URLs.')); + copyBatchUrls(filter: BatchUrlFilter): void { + this.batchUrls.copy(filter); } fetchVersionInfo(): void { @@ -1550,7 +1507,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { }; const fail = (err?: unknown) => { console.error('Clipboard write failed:', err); - alert('Failed to copy to clipboard. Your browser may require HTTPS for clipboard access.'); + this.toasts.error('Failed to copy to clipboard. Your browser may require HTTPS for clipboard access.'); }; if (navigator.clipboard?.writeText) { navigator.clipboard.writeText(text).then(done).catch(fail); @@ -1586,7 +1543,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.hasCookies = true; } else { this.refreshCookieStatus(); - alert(`Error uploading cookies: ${this.formatErrorMessage(response?.msg)}`); + this.toasts.error(`Error uploading cookies: ${this.formatErrorMessage(response?.msg)}`); } this.cookieUploadInProgress = false; input.value = ''; @@ -1595,7 +1552,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { this.refreshCookieStatus(); this.cookieUploadInProgress = false; input.value = ''; - alert('Error uploading cookies.'); + this.toasts.error('Error uploading cookies.'); } }); } @@ -1629,11 +1586,11 @@ export class App implements AfterViewInit, OnInit, OnDestroy { return; } this.refreshCookieStatus(); - alert(`Error deleting cookies: ${this.formatErrorMessage(response?.msg)}`); + this.toasts.error(`Error deleting cookies: ${this.formatErrorMessage(response?.msg)}`); }, error: () => { this.refreshCookieStatus(); - alert('Error deleting cookies.'); + this.toasts.error('Error deleting cookies.'); } }); } diff --git a/ui/src/app/components/index.ts b/ui/src/app/components/index.ts index 6aafae3..ff2a21b 100644 --- a/ui/src/app/components/index.ts +++ b/ui/src/app/components/index.ts @@ -1,2 +1,3 @@ export { SelectAllCheckboxComponent } from './master-checkbox.component'; -export { ItemCheckboxComponent } from './slave-checkbox.component'; \ No newline at end of file +export { ItemCheckboxComponent } from './slave-checkbox.component'; +export { ToastContainerComponent } from './toast-container.component'; \ No newline at end of file diff --git a/ui/src/app/components/toast-container.component.ts b/ui/src/app/components/toast-container.component.ts new file mode 100644 index 0000000..d03718a --- /dev/null +++ b/ui/src/app/components/toast-container.component.ts @@ -0,0 +1,58 @@ +import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; +import { FontAwesomeModule } from '@fortawesome/angular-fontawesome'; +import { faCheckCircle, faTimesCircle, faInfoCircle, faXmark } from '@fortawesome/free-solid-svg-icons'; +import { ToastService } from '../services/toast.service'; + +@Component({ + selector: 'app-toast-container', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [FontAwesomeModule], + template: ` +
+ @for (toast of toasts.toasts(); track toast.id) { + + } +
+`, +}) +export class ToastContainerComponent { + protected readonly toasts = inject(ToastService); + protected readonly faCheckCircle = faCheckCircle; + protected readonly faTimesCircle = faTimesCircle; + protected readonly faInfoCircle = faInfoCircle; + protected readonly faXmark = faXmark; +} diff --git a/ui/src/app/services/batch-urls.service.ts b/ui/src/app/services/batch-urls.service.ts new file mode 100644 index 0000000..47447b8 --- /dev/null +++ b/ui/src/app/services/batch-urls.service.ts @@ -0,0 +1,62 @@ +import { inject, Injectable } from '@angular/core'; +import { DownloadsService } from './downloads.service'; +import { ToastService } from './toast.service'; + +export type BatchUrlFilter = 'pending' | 'completed' | 'failed' | 'all'; + +/** + * Encapsulates collecting download URLs by status and exporting/copying them. + * Extracted from the main app component to keep it focused on view concerns. + */ +@Injectable({ providedIn: 'root' }) +export class BatchUrlsService { + private downloads = inject(DownloadsService); + private toasts = inject(ToastService); + + collect(filter: BatchUrlFilter): string[] { + const queueUrls = () => Array.from(this.downloads.queue.values()).map((dl) => dl.url); + const doneUrls = (status?: string) => + Array.from(this.downloads.done.values()) + .filter((dl) => status === undefined || dl.status === status) + .map((dl) => dl.url); + switch (filter) { + case 'pending': + return queueUrls(); + case 'completed': + return doneUrls('finished'); + case 'failed': + return doneUrls('error'); + default: + return [...queueUrls(), ...doneUrls()]; + } + } + + export(filter: BatchUrlFilter): void { + const urls = this.collect(filter); + if (!urls.length) { + this.toasts.info('No URLs found for the selected filter.'); + return; + } + const blob = new Blob([urls.join('\n')], { type: 'text/plain' }); + const downloadUrl = window.URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = downloadUrl; + a.download = 'metube_urls.txt'; + document.body.appendChild(a); + a.click(); + document.body.removeChild(a); + window.URL.revokeObjectURL(downloadUrl); + } + + copy(filter: BatchUrlFilter): void { + const urls = this.collect(filter); + if (!urls.length) { + this.toasts.info('No URLs found for the selected filter.'); + return; + } + navigator.clipboard + .writeText(urls.join('\n')) + .then(() => this.toasts.success('URLs copied to clipboard.')) + .catch(() => this.toasts.error('Failed to copy URLs.')); + } +} diff --git a/ui/src/app/services/index.ts b/ui/src/app/services/index.ts index 21ef047..8db6fec 100644 --- a/ui/src/app/services/index.ts +++ b/ui/src/app/services/index.ts @@ -1,2 +1,4 @@ export { DownloadsService } from './downloads.service'; -export { MeTubeSocket } from './metube-socket.service'; \ No newline at end of file +export { MeTubeSocket } from './metube-socket.service'; +export { ToastService } from './toast.service'; +export { BatchUrlsService } from './batch-urls.service'; \ No newline at end of file diff --git a/ui/src/app/services/toast.service.ts b/ui/src/app/services/toast.service.ts new file mode 100644 index 0000000..71ab21b --- /dev/null +++ b/ui/src/app/services/toast.service.ts @@ -0,0 +1,86 @@ +import { Injectable, signal } from '@angular/core'; + +export type ToastLevel = 'info' | 'success' | 'error'; + +export interface ToastAction { + label: string; + value: boolean; + primary?: boolean; +} + +export interface Toast { + id: number; + level: ToastLevel; + message: string; + actions?: ToastAction[]; + /** Resolver for confirm() toasts; resolved when the user picks an action or dismisses. */ + _resolve?: (value: boolean) => void; +} + +/** + * Lightweight non-blocking notification service. Replaces the blocking + * window.alert()/confirm() dialogs that previously littered the app component. + */ +@Injectable({ providedIn: 'root' }) +export class ToastService { + private counter = 0; + readonly toasts = signal([]); + + info(message: string): void { + this.show('info', message, 4000); + } + + success(message: string): void { + this.show('success', message, 4000); + } + + error(message: string): void { + this.show('error', message, 8000); + } + + /** + * Show a confirmation toast with confirm/cancel actions. Resolves true when + * confirmed, false when cancelled or auto-dismissed. + */ + confirm(message: string, confirmLabel = 'OK', cancelLabel = 'Cancel'): Promise { + return new Promise((resolve) => { + const id = ++this.counter; + this.toasts.update((list) => [ + ...list, + { + id, + level: 'info', + message, + actions: [ + { label: cancelLabel, value: false }, + { label: confirmLabel, value: true, primary: true }, + ], + _resolve: resolve, + }, + ]); + }); + } + + respond(id: number, value: boolean): void { + const toast = this.toasts().find((t) => t.id === id); + toast?._resolve?.(value); + this.remove(id); + } + + dismiss(id: number): void { + const toast = this.toasts().find((t) => t.id === id); + // A confirm toast dismissed without an explicit choice resolves to false. + toast?._resolve?.(false); + this.remove(id); + } + + private remove(id: number): void { + this.toasts.update((list) => list.filter((t) => t.id !== id)); + } + + private show(level: ToastLevel, message: string, autoDismissMs: number): void { + const id = ++this.counter; + this.toasts.update((list) => [...list, { id, level, message }]); + setTimeout(() => this.remove(id), autoDismissMs); + } +}