From 309a1079129d29c40c7d3061edf7ebd80de9da3e Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:35:19 +0200 Subject: [PATCH] feat(downloads): let Android move the app and its downloads to adoptable storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declare android:installLocation="auto" so the app becomes eligible for the Settings "change storage" flow and pm move-package. Adoptable storage relocates the private data directory with the APK, so downloads follow the app onto a USB drive adopted by an Android TV. Moving the app changes the private data directory, which invalidated any download task already enqueued: those pinned BaseDirectory.root plus an absolute directory that background_downloader persists verbatim, so a queued or paused download resumed writing to a volume the app no longer owns. Enqueue app-storage targets against the base directory the downloader re-resolves from the live app context instead, and drop the tasks and records a previous location left behind so the download restarts under the current one. That sweep runs before the downloader is wired up, because initialization delivers statuses accumulated while suspended — which can mark the row failed, and a failed row is deliberately not restarted — and because rescheduleKilledTasks re-enqueues every killed record it finds, stale absolute directory included. Compare paths by containment rather than by string prefix while making a stored path relative. A custom download root that merely starts with the base directory's name is a sibling the app does not own, and stripping it re-rooted the download inside app storage. close #1794 --- android/app/src/main/AndroidManifest.xml | 8 +- lib/services/download_manager_service.dart | 215 +++++++++++++- lib/services/download_storage_service.dart | 46 ++- .../download_manager_service_test.dart | 263 ++++++++++++++++++ .../download_storage_service_test.dart | 95 ++++++- 5 files changed, 606 insertions(+), 21 deletions(-) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 06289af6..0f1db247 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -1,5 +1,11 @@ + + xmlns:tools="http://schemas.android.com/tools" + android:installLocation="auto"> diff --git a/lib/services/download_manager_service.dart b/lib/services/download_manager_service.dart index 1eb7e767..d4c015ae 100644 --- a/lib/services/download_manager_service.dart +++ b/lib/services/download_manager_service.dart @@ -43,6 +43,17 @@ typedef _NativeTaskForId = Future Function(String taskId); typedef _NativeResumeTask = Future Function(DownloadTask task); typedef _EpisodeStorageDeletion = ({String? seasonDirUri, String? showDirUri}); +/// The background_downloader entry points the recovery path drives. Injected as a whole +/// in tests so relocated-storage recovery can be exercised without platform channels. +typedef NativeDownloaderOps = ({ + Future> Function() allTasks, + Future> Function() allRecords, + Future Function(String taskId) deleteRecord, + Future Function(Iterable taskIds) cancelTaskIds, + Future Function() cleanUpOrphanedTempFiles, + Future<(List, List)> Function() rescheduleKilledTasks, +}); + typedef NativeTaskPartition = ({List current, List stale}); typedef DownloadLocationSnapshot = ({String? path, String? type}); @@ -61,6 +72,40 @@ NativeTaskPartition partitionNativeTasks(Iterable tasks, String? currentTa return (current: current, stale: stale); } +/// Whether [task] writes into a directory left behind by a previous location of the +/// app's private storage. +/// +/// A [BaseDirectory.root] task carries its whole target directory in the downloader's +/// own persisted store, so one enqueued before the app moved to adoptable storage (or +/// before an iOS container UUID change) resumes writing where the app owns nothing. +/// Legitimate root-anchored tasks sit under [baseAppDirPath] or under the configured +/// [customRootPath], which does not move with the app; anything else is a leftover with +/// no recoverable partial data. +/// +/// [rootBasePath] is the downloader's own resolved path for [BaseDirectory.root] +/// (`Task.baseDirectoryPath`). It is needed because the [Task] constructor strips one +/// leading separator from `directory`, so the stored value must be rejoined the same way +/// `Task.filePath` does before it can be compared with a real directory. +@visibleForTesting +bool isRelocatedRootTaskDirectory({ + required Task task, + required String rootBasePath, + required String baseAppDirPath, + String? customRootPath, +}) { + // A SAF task also declares BaseDirectory.root, but its directory is a content:// tree + // URI owned by a document provider, not a path that moves with the app. + if (task is UriTask) return false; + if (task.baseDirectory != BaseDirectory.root) return false; + if (task.directory.isEmpty) return false; + final directory = path.join(rootBasePath, task.directory); + if (path.equals(directory, baseAppDirPath) || path.isWithin(baseAppDirPath, directory)) return false; + if (customRootPath != null && (path.equals(directory, customRootPath) || path.isWithin(customRootPath, directory))) { + return false; + } + return true; +} + const bool _tvosBuild = bool.fromEnvironment('TVOS_BUILD'); class _DownloadContext { @@ -97,6 +142,7 @@ class DownloadManagerService { final Future Function(MediaServerClient)? _queueProcessorOverride; final Future Function()? _nativeRecoveryOverride; final Future Function()? _fileDownloaderInitializerOverride; + final NativeDownloaderOps? _nativeOpsOverride; final DownloadLocationSnapshot Function()? _downloadLocationReader; final Future Function(String?)? _downloadPathWriter; @@ -225,6 +271,7 @@ class DownloadManagerService { @visibleForTesting Future Function(MediaServerClient)? queueProcessorOverride, @visibleForTesting Future Function()? fileDownloaderInitializerOverride, @visibleForTesting Future Function()? nativeRecoveryOverride, + @visibleForTesting NativeDownloaderOps? nativeOpsOverride, @visibleForTesting DownloadLocationSnapshot Function()? downloadLocationReader, @visibleForTesting Future Function(String?)? downloadPathWriter, @visibleForTesting Future Function(String?)? downloadPathTypeWriter, @@ -235,6 +282,7 @@ class DownloadManagerService { _nativeRecoveryOverride = nativeRecoveryOverride, _database = database, _fileDownloaderInitializerOverride = fileDownloaderInitializerOverride, + _nativeOpsOverride = nativeOpsOverride, _storageService = storageService, _clientResolver = clientResolver, _http = http ?? httpClient, @@ -247,6 +295,17 @@ class DownloadManagerService { bool get downloadsSupported => _downloadsSupportedOverride ?? platformDownloadsSupported; + NativeDownloaderOps get _nativeOps => + _nativeOpsOverride ?? + ( + allTasks: () => FileDownloader().allTasks(group: _downloadGroup), + allRecords: () => FileDownloader().database.allRecords(group: _downloadGroup), + deleteRecord: FileDownloader().database.deleteRecordWithId, + cancelTaskIds: FileDownloader().cancelTasksWithIds, + cleanUpOrphanedTempFiles: FileDownloader().cleanUpOrphanedTempFiles, + rescheduleKilledTasks: FileDownloader().rescheduleKilledTasks, + ); + bool _skipDownloadsUnsupported(String operation) { if (downloadsSupported) return false; if (!_loggedDownloadsUnsupported) { @@ -836,17 +895,23 @@ class DownloadManagerService { await nativeRecoveryOverride(); return; } - unawaited(Sentry.addBreadcrumb(Breadcrumb(message: 'Initializing FileDownloader', category: 'downloads'))); - await _initializeFileDownloader(); + // Strictly before the downloader is wired up. Initialization registers our status + // callbacks and calls resumeFromBackground, which can deliver a failure a relocated + // task already hit on the old path — and a failed row is no longer restartable. + // rescheduleKilledTasks, further down, would re-enqueue it against that path again. + await _purgeRelocatedDownloadRecords(); - final deletedTempFiles = await FileDownloader().cleanUpOrphanedTempFiles(); + unawaited(Sentry.addBreadcrumb(Breadcrumb(message: 'Initializing FileDownloader', category: 'downloads'))); + await (_fileDownloaderInitializerOverride?.call() ?? _initializeFileDownloader()); + + final deletedTempFiles = await _nativeOps.cleanUpOrphanedTempFiles(); if (deletedTempFiles > 0) { appLogger.i('Deleted $deletedTempFiles orphaned downloader temp file(s)'); } // Let background_downloader re-enqueue tasks killed by the OS unawaited(Sentry.addBreadcrumb(Breadcrumb(message: 'Rescheduling killed tasks', category: 'downloads'))); - final (rescheduled, _) = await FileDownloader().rescheduleKilledTasks(); + final (rescheduled, _) = await _nativeOps.rescheduleKilledTasks(); if (rescheduled.isNotEmpty) { appLogger.i('Rescheduled ${rescheduled.length} killed download task(s)'); } @@ -933,11 +998,13 @@ class DownloadManagerService { } Future _reconcileNativeDownloadTasks() async { - if (!downloadsSupported || !_fileDownloaderInitialized) return; + if (!downloadsSupported) return; + // An injected ops seam stands in for a wired-up downloader. + if (_nativeOpsOverride == null && !_fileDownloaderInitialized) return; final List nativeTasks; try { - nativeTasks = await FileDownloader().allTasks(group: _downloadGroup); + nativeTasks = await _nativeOps.allTasks(); } catch (e) { appLogger.w('Failed to enumerate native download tasks during recovery', error: e); return; @@ -945,16 +1012,48 @@ class DownloadManagerService { if (nativeTasks.isEmpty) return; final tasksByGlobalKey = >{}; + final rootTasks = []; for (final task in nativeTasks) { + if (task.baseDirectory == BaseDirectory.root) { + rootTasks.add(task); + continue; + } final globalKey = task.metaData; if (globalKey.isEmpty) continue; (tasksByGlobalKey[globalKey] ??= []).add(task); } - if (tasksByGlobalKey.isEmpty) return; + + // A root-anchored task carries its absolute directory in the downloader's own + // persisted store, so one enqueued before the app's private storage moved resumes + // writing where the app no longer owns anything. Those cannot be resumed at all. + final relocatedTasksByGlobalKey = >{}; + if (rootTasks.isNotEmpty) { + final rootBasePath = await Task.baseDirectoryPath(BaseDirectory.root); + final baseAppDirPath = await _storageService.baseAppDirectoryPath(); + final customRootPath = _storageService.customFileRootPath; + for (final task in rootTasks) { + final relocated = isRelocatedRootTaskDirectory( + task: task, + rootBasePath: rootBasePath, + baseAppDirPath: baseAppDirPath, + customRootPath: customRootPath, + ); + if (relocated) { + (relocatedTasksByGlobalKey[task.metaData] ??= []).add(task); + } else if (task.metaData.isNotEmpty) { + (tasksByGlobalKey[task.metaData] ??= []).add(task); + } + } + } + if (relocatedTasksByGlobalKey.isEmpty && tasksByGlobalKey.isEmpty) return; final rows = await _database.select(_database.downloadedMedia).get(); final rowsByGlobalKey = {for (final row in rows) row.globalKey: row}; + for (final entry in relocatedTasksByGlobalKey.entries) { + await _discardRelocatedNativeTasks(entry.key, entry.value, rowsByGlobalKey[entry.key]); + } + for (final entry in tasksByGlobalKey.entries) { final globalKey = entry.key; final tasks = entry.value; @@ -993,6 +1092,98 @@ class DownloadManagerService { } } + /// Drop downloader records whose target directory belongs to a previous location of the + /// app's private storage, and requeue the download so it restarts under the current one. + /// + /// Runs before the downloader is initialized, and therefore before + /// [FileDownloader.rescheduleKilledTasks], for two reasons. Reschedule re-enqueues every + /// enqueued/running record it finds missing natively, carrying the relocated absolute + /// directory over verbatim. And initialization registers our status callbacks and calls + /// [FileDownloader.resumeFromBackground], which can deliver a failure such a task already + /// hit — marking the row failed, which [_requeueRelocatedDownload] deliberately will not + /// restart. Reading and deleting records needs no initialization: the record store is + /// Dart-side, as [discardInterruptedNativeDownloadsAfterStorageFailure] also relies on. + Future _purgeRelocatedDownloadRecords() async { + if (!downloadsSupported) return; + + final List records; + try { + records = await _nativeOps.allRecords(); + } catch (e) { + appLogger.w('Failed to enumerate download records during recovery', error: e); + return; + } + final rootRecords = records.where((record) => record.task.baseDirectory == BaseDirectory.root).toList(); + if (rootRecords.isEmpty) return; + + final rootBasePath = await Task.baseDirectoryPath(BaseDirectory.root); + final baseAppDirPath = await _storageService.baseAppDirectoryPath(); + final customRootPath = _storageService.customFileRootPath; + for (final record in rootRecords) { + if (!isRelocatedRootTaskDirectory( + task: record.task, + rootBasePath: rootBasePath, + baseAppDirPath: baseAppDirPath, + customRootPath: customRootPath, + )) { + continue; + } + + final globalKey = record.task.metaData; + appLogger.i( + 'Dropping download record ${record.taskId} for $globalKey targeting relocated ' + 'storage: ${record.task.directory}', + ); + try { + await _nativeOps.deleteRecord(record.taskId); + } catch (e) { + appLogger.w('Failed to drop relocated download record ${record.taskId}', error: e); + continue; + } + await _cancelNativeTaskIds(globalKey, [record.taskId], reason: 'relocated app storage before rescheduling'); + if (globalKey.isNotEmpty) await _requeueRelocatedDownload(globalKey); + } + } + + /// Cancel [tasks] that target storage the app no longer owns and put a restartable + /// download back in the queue so it downloads again under the current location. + Future _discardRelocatedNativeTasks(String globalKey, List tasks, DownloadedMediaItem? row) async { + appLogger.i( + 'Discarding ${tasks.length} download task(s) for $globalKey targeting relocated storage: ' + '${tasks.map((task) => task.directory).toSet().join(', ')}', + ); + await _cancelNativeTaskIds( + globalKey, + tasks.map((task) => task.taskId), + reason: 'relocated app storage during recovery', + ); + if (row != null) await _requeueRelocatedDownload(row.globalKey, row: row); + } + + /// Send a download whose bytes are stranded on a previous storage location back to the + /// queue. A finished or already-abandoned row keeps its status: there is nothing to + /// restart, and its stored path is relative and therefore still valid. + Future _requeueRelocatedDownload(String globalKey, {DownloadedMediaItem? row}) async { + final current = row ?? await _database.getDownloadedMedia(globalKey); + if (current == null) return; + + final restartable = switch (DownloadStatus.values[current.status]) { + DownloadStatus.queued || DownloadStatus.downloading || DownloadStatus.paused => true, + DownloadStatus.completed || DownloadStatus.failed || DownloadStatus.cancelled || DownloadStatus.partial => false, + }; + if (!restartable) return; + + await _database.updateBgTaskId(globalKey, null); + await _database.updateDownloadStatus(globalKey, DownloadStatus.queued.index); + await _database.addToQueue(mediaGlobalKey: globalKey); + } + + @visibleForTesting + Future debugRecoverRelocatedDownloads() async { + await _purgeRelocatedDownloadRecords(); + await _reconcileNativeDownloadTasks(); + } + @visibleForTesting Future debugReconcileSafGrantOwnership({List nativeTasks = const []}) { return _reconcileSafGrantOwnership(nativeTasks: nativeTasks); @@ -1005,7 +1196,7 @@ class DownloadManagerService { tasks = nativeTasks; } else { try { - tasks = await FileDownloader().allTasks(group: _downloadGroup); + tasks = await _nativeOps.allTasks(); } catch (error) { appLogger.w('SAF grant reconciliation deferred: native task enumeration failed', error: error); return; @@ -1148,7 +1339,7 @@ class DownloadManagerService { if (ids.isEmpty) return; try { - final cancelled = await FileDownloader().cancelTasksWithIds(ids); + final cancelled = await _nativeOps.cancelTaskIds(ids); if (cancelled) { appLogger.d('Cancelled ${ids.length} native task(s) for $globalKey ($reason): ${ids.join(', ')}'); } @@ -1834,11 +2025,13 @@ class DownloadManagerService { await File(downloadFilePath).parent.create(recursive: true); + final taskLocation = await _storageService.resolveTaskDirectory(downloadFilePath); + task = DownloadTask( url: resolution.videoUrl!, filename: path.basename(downloadFilePath), - directory: path.dirname(downloadFilePath), - baseDirectory: BaseDirectory.root, + directory: taskLocation.directory, + baseDirectory: taskLocation.baseDirectory, group: _downloadGroup, updates: Updates.statusAndProgress, requiresWiFi: requiresWiFi, diff --git a/lib/services/download_storage_service.dart b/lib/services/download_storage_service.dart index e7adb362..b357e948 100644 --- a/lib/services/download_storage_service.dart +++ b/lib/services/download_storage_service.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import '../media/ids.dart'; import 'dart:io'; +import 'package:background_downloader/background_downloader.dart'; import 'package:crypto/crypto.dart'; import 'package:flutter/foundation.dart'; import 'package:path_provider/path_provider.dart'; @@ -79,15 +80,28 @@ class DownloadStorageService { } } + /// Whether [_getBaseAppDir] resolves to the documents directory (mobile) or the + /// support directory (desktop). Single source of truth for that split, shared + /// with [resolveTaskDirectory] so the two can never disagree. + static bool get _baseAppDirIsDocuments => Platform.isAndroid || Platform.isIOS; + /// Get the base app directory for storing data. /// Uses ApplicationDocumentsDirectory on mobile, ApplicationSupportDirectory on desktop. Future _getBaseAppDir() { - if (Platform.isAndroid || Platform.isIOS) { + if (_baseAppDirIsDocuments) { return getApplicationDocumentsDirectory(); } return getApplicationSupportDirectory(); } + /// Absolute path of the app-private directory that relative download paths are + /// anchored to. Moves with the app, so it must be read fresh rather than stored. + Future baseAppDirectoryPath() async => (await _getBaseAppDir()).path; + + /// Configured custom download root when it is a filesystem path, otherwise null. + /// A `saf` root is a `content://` tree URI instead — see [safBaseUri]. + String? get customFileRootPath => _customPathType == 'file' ? _customDownloadPath : null; + /// Format episode filename base: S{XX}E{XX} - {Title} String _formatEpisodeFileName(MediaItem episode) { final season = padNumber(episode.parentIndex ?? 0, 2); @@ -348,18 +362,38 @@ class DownloadStorageService { // Strip the base directory prefix iteratively — background_downloader // recovery paths can contain the base dir doubled (e.g. // /data/.../app_flutter/data/.../app_flutter/downloads/...). + // + // Containment, not a string prefix: a custom download root that merely starts with the + // base dir's name (`-external`) is a sibling the app does not own, and stripping + // it would silently re-root the download inside app storage. var result = absolutePath; - while (result.startsWith(baseDir.path)) { - result = result.substring(baseDir.path.length); - if (result.startsWith('/') || result.startsWith('\\')) { - result = result.substring(1); - } + while (path.isWithin(baseDir.path, result)) { + result = path.relative(result, from: baseDir.path); } if (result != absolutePath) return result; return absolutePath; } + /// Base directory and directory to enqueue a download for [absolutePath] with. + /// + /// A target inside the app's own storage is described relative to a base directory + /// that background_downloader re-resolves from the live app context on every launch, + /// so a task persisted across a restart survives the private data directory moving — + /// an iOS container UUID change, or an Android app moved to adoptable storage. Only a + /// custom download root, which lives outside that storage and therefore does not move + /// with the app, keeps [BaseDirectory.root] and its absolute path. + Future<({BaseDirectory baseDirectory, String directory})> resolveTaskDirectory(String absolutePath) async { + final relativePath = await toRelativePath(absolutePath); + if (relativePath == absolutePath) { + return (baseDirectory: BaseDirectory.root, directory: path.dirname(absolutePath)); + } + return ( + baseDirectory: _baseAppDirIsDocuments ? BaseDirectory.applicationDocuments : BaseDirectory.applicationSupport, + directory: path.dirname(relativePath), + ); + } + /// Convert a relative file path to an absolute path (for file operations) /// Reconstructs the full path using the current app documents directory. Future toAbsolutePath(String relativePath) async { diff --git a/test/services/download_manager_service_test.dart b/test/services/download_manager_service_test.dart index cc089c0d..87aa9865 100644 --- a/test/services/download_manager_service_test.dart +++ b/test/services/download_manager_service_test.dart @@ -79,6 +79,255 @@ void main() { }); }); + group('isRelocatedRootTaskDirectory', () { + const baseAppDir = '/mnt/expand/9f2a/user/0/com.edde746.plezy/app_flutter'; + // Task strips one leading separator from `directory`, so a root-anchored task is + // rejoined against the downloader's own root base before it can be compared. + const rootBase = '/'; + + test('flags an absolute directory left behind by the previous app location', () { + final task = _rootTask('t', 'srv:item-1', '/data/user/0/com.edde746.plezy/app_flutter/downloads/srv/item-1'); + + expect(task.directory, 'data/user/0/com.edde746.plezy/app_flutter/downloads/srv/item-1'); + expect(isRelocatedRootTaskDirectory(task: task, rootBasePath: rootBase, baseAppDirPath: baseAppDir), isTrue); + }); + + test('accepts a directory inside the live app storage', () { + final task = _rootTask('t', 'srv:item-1', '$baseAppDir/downloads/srv/item-1'); + + expect(isRelocatedRootTaskDirectory(task: task, rootBasePath: rootBase, baseAppDirPath: baseAppDir), isFalse); + }); + + test('accepts a directory inside the configured custom download root', () { + final task = _rootTask('t', 'srv:item-1', '/Volumes/External/Plezy/Movies/Arrival (2016)'); + + expect( + isRelocatedRootTaskDirectory( + task: task, + rootBasePath: rootBase, + baseAppDirPath: baseAppDir, + customRootPath: '/Volumes/External/Plezy', + ), + isFalse, + ); + }); + + test('ignores a SAF task, whose root-anchored directory is a content tree URI', () { + final task = UriDownloadTask( + taskId: 't', + url: 'https://example.test/video.mp4', + filename: 'video.mp4', + directoryUri: Uri.parse('content://com.android.externalstorage.documents/tree/usb%3APlezy'), + metaData: 'srv:item-1', + ); + + expect(task.baseDirectory, BaseDirectory.root); + expect(isRelocatedRootTaskDirectory(task: task, rootBasePath: rootBase, baseAppDirPath: baseAppDir), isFalse); + }); + + test('ignores a task anchored to a base directory the downloader re-resolves itself', () { + final task = _downloadTask('t', 'srv:item-1'); + + expect(task.baseDirectory, isNot(BaseDirectory.root)); + expect(isRelocatedRootTaskDirectory(task: task, rootBasePath: rootBase, baseAppDirPath: baseAppDir), isFalse); + }); + }); + + group('relocated task recovery', () { + late Directory tmpRoot; + late PathProviderPlatform previousPathProvider; + + setUp(() async { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + DownloadStorageService.resetForTesting(); + tmpRoot = await Directory.systemTemp.createTemp('dms_relocated_'); + previousPathProvider = PathProviderPlatform.instance; + PathProviderPlatform.instance = FakePathProvider(tmpRoot); + await DownloadStorageService.instance.initialize(await SettingsService.getInstance()); + }); + + tearDown(() async { + DownloadStorageService.resetForTesting(); + SettingsService.resetForTesting(); + PathProviderPlatform.instance = previousPathProvider; + if (await tmpRoot.exists()) await tmpRoot.delete(recursive: true); + }); + + late List cancelledIds; + late List deletedRecordIds; + late List callOrder; + + Future managerFor( + AppDatabase db, { + List nativeTasks = const [], + List records = const [], + }) async { + cancelledIds = []; + deletedRecordIds = []; + callOrder = []; + final manager = DownloadManagerService( + database: db, + storageService: DownloadStorageService.instance, + clientResolver: (_, {clientScopeId}) => null, + downloadsSupportedOverride: true, + fileDownloaderInitializerOverride: () async => callOrder.add('initialize'), + nativeOpsOverride: ( + allTasks: () async => nativeTasks, + allRecords: () async { + callOrder.add('allRecords'); + return records; + }, + deleteRecord: (taskId) async { + callOrder.add('deleteRecord:$taskId'); + deletedRecordIds.add(taskId); + }, + cancelTaskIds: (taskIds) async { + callOrder.add('cancel:${taskIds.join(",")}'); + cancelledIds.addAll(taskIds); + return true; + }, + cleanUpOrphanedTempFiles: () async => 0, + rescheduleKilledTasks: () async { + callOrder.add('reschedule'); + return ([], []); + }, + ), + ); + addTearDown(manager.dispose); + return manager; + } + + Future seedRow(AppDatabase db, DownloadStatus status, {String? taskId, String? videoFilePath}) async { + await db.insertDownload( + serverId: ServerId('srv'), + ratingKey: 'item-1', + globalKey: 'srv:item-1', + type: 'movie', + status: status.index, + ); + if (taskId != null) await db.updateBgTaskId('srv:item-1', taskId); + if (videoFilePath != null) await db.updateVideoFilePath('srv:item-1', videoFilePath); + } + + test('cancels a still-live task targeting the previous app location and requeues it', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + addTearDown(db.close); + await seedRow(db, DownloadStatus.downloading, taskId: 'task-a'); + final manager = await managerFor( + db, + nativeTasks: [_rootTask('task-a', 'srv:item-1', '$_previousAppDir/downloads/srv/item-1')], + ); + + await manager.debugRecoverRelocatedDownloads(); + + expect(cancelledIds, ['task-a']); + final row = await db.getDownloadedMedia('srv:item-1'); + expect(row?.status, DownloadStatus.queued.index); + expect(row?.bgTaskId, isNull); + expect((await db.getNextQueueItem())?.mediaGlobalKey, 'srv:item-1'); + }); + + test('drops a relocated record before rescheduling can re-enqueue it', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + addTearDown(db.close); + await seedRow(db, DownloadStatus.downloading, taskId: 'task-a'); + // A killed task survives only as a downloader record: rescheduleKilledTasks would + // re-enqueue it against the old volume, so it must be gone before that runs. + final manager = await managerFor( + db, + records: [ + TaskRecord( + _rootTask('task-a', 'srv:item-1', '$_previousAppDir/downloads/srv/item-1'), + TaskStatus.enqueued, + 0.4, + 1024, + ), + ], + ); + + await manager.debugRecoverRelocatedDownloads(); + + expect(deletedRecordIds, ['task-a']); + expect(cancelledIds, ['task-a']); + final row = await db.getDownloadedMedia('srv:item-1'); + expect(row?.status, DownloadStatus.queued.index); + expect(row?.bgTaskId, isNull); + expect((await db.getNextQueueItem())?.mediaGlobalKey, 'srv:item-1'); + }); + + test('keeps a record that still points inside the live app storage', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + addTearDown(db.close); + await seedRow(db, DownloadStatus.downloading, taskId: 'task-a'); + final liveDir = p.join( + await DownloadStorageService.instance.baseAppDirectoryPath(), + 'downloads', + 'srv', + 'item-1', + ); + final task = _rootTask('task-a', 'srv:item-1', liveDir); + final manager = await managerFor( + db, + nativeTasks: [task], + records: [TaskRecord(task, TaskStatus.enqueued, 0.4, 1024)], + ); + + await manager.debugRecoverRelocatedDownloads(); + + expect(deletedRecordIds, isEmpty); + expect(cancelledIds, isEmpty); + final row = await db.getDownloadedMedia('srv:item-1'); + expect(row?.status, DownloadStatus.downloading.index); + expect(row?.bgTaskId, 'task-a'); + expect(await db.getNextQueueItem(), isNull); + }); + + test('cancels a stale task for a finished download without disturbing the row', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + addTearDown(db.close); + await seedRow(db, DownloadStatus.completed, videoFilePath: 'downloads/srv/item-1/video.mkv'); + final manager = await managerFor( + db, + nativeTasks: [_rootTask('task-a', 'srv:item-1', '$_previousAppDir/downloads/srv/item-1')], + ); + + await manager.debugRecoverRelocatedDownloads(); + + expect(cancelledIds, ['task-a']); + final row = await db.getDownloadedMedia('srv:item-1'); + expect(row?.status, DownloadStatus.completed.index); + expect(row?.videoFilePath, 'downloads/srv/item-1/video.mkv'); + expect(await db.getNextQueueItem(), isNull); + }); + + test('startup drops relocated records before wiring up the downloader', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + addTearDown(db.close); + await seedRow(db, DownloadStatus.downloading, taskId: 'task-a'); + final manager = await managerFor( + db, + records: [ + TaskRecord( + _rootTask('task-a', 'srv:item-1', '$_previousAppDir/downloads/srv/item-1'), + TaskStatus.enqueued, + 0.4, + 1024, + ), + ], + ); + + await manager.recoverInterruptedDownloads(); + + // Initialization delivers statuses accumulated while suspended, which can mark the + // row failed and make it unrestartable; rescheduleKilledTasks then re-enqueues every + // enqueued record it finds missing natively, stale absolute directory and all. The + // record has to be gone before either. + expect(callOrder, ['allRecords', 'deleteRecord:task-a', 'cancel:task-a', 'initialize', 'reschedule']); + expect((await db.getNextQueueItem())?.mediaGlobalKey, 'srv:item-1'); + }); + }); + group('artworkStorageKey', () { test('removes Jellyfin api_key from persisted artwork keys', () { final url = 'https://jf.example/Items/item-1/Images/Primary?tag=abc&api_key=secret-token'; @@ -2530,6 +2779,20 @@ DownloadTask _downloadTask(String taskId, String globalKey) { ); } +/// Where the app's private storage lived before it was moved to another volume. +const _previousAppDir = '/data/user/0/com.edde746.plezy/app_flutter'; + +DownloadTask _rootTask(String taskId, String globalKey, String directory) { + return DownloadTask( + taskId: taskId, + url: 'https://example.test/video.mp4', + filename: 'video.mp4', + directory: directory, + baseDirectory: BaseDirectory.root, + metaData: globalKey, + ); +} + MediaItem _movie({String? thumbPath}) { return testMediaItem( id: 'item-1', diff --git a/test/services/download_storage_service_test.dart b/test/services/download_storage_service_test.dart index 9caa9dfb..835205e7 100644 --- a/test/services/download_storage_service_test.dart +++ b/test/services/download_storage_service_test.dart @@ -1,6 +1,7 @@ import 'dart:io'; import 'package:plezy/media/ids.dart'; +import 'package:background_downloader/background_downloader.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:path/path.dart' as p; import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; @@ -271,13 +272,24 @@ void main() { final dss = DownloadStorageService.instance; await dss.initialize(settings); - // Content URIs and non-base absolute paths must round-trip untouched — - // the production code only strips paths that literally start with the - // base dir. + // Content URIs and non-base absolute paths must round-trip untouched — the + // production code only strips paths contained by the base dir. const uri = '/Volumes/External/Movies/x.mkv'; expect(await dss.toRelativePath(uri), uri); }); + test('leaves a sibling directory whose name merely starts with the base dir alone', () async { + final settings = await SettingsService.getInstance(); + final dss = DownloadStorageService.instance; + await dss.initialize(settings); + + // `-external` is a string prefix match but not inside the base dir. Stripping it + // would yield "-external/..." and silently re-root the file inside app storage. + final sibling = '${p.join(tmpRoot.path, 'support')}-external'; + final outside = p.join(sibling, 'downloads', 'srv', '1', 'video.mkv'); + expect(await dss.toRelativePath(outside), outside); + }); + test('toAbsolutePath joins relative paths against the base dir', () async { final settings = await SettingsService.getInstance(); final dss = DownloadStorageService.instance; @@ -310,6 +322,83 @@ void main() { }); }); + group('resolveTaskDirectory', () { + test('describes an app-storage target relative to the base directory', () async { + final settings = await SettingsService.getInstance(); + final dss = DownloadStorageService.instance; + await dss.initialize(settings); + + final videoPath = await dss.getVideoFilePath(ServerId('srv'), 'item-1', 'mkv'); + final location = await dss.resolveTaskDirectory(videoPath); + + // Desktop hosts anchor downloads at the support directory; mobile uses documents. + expect(location.baseDirectory, BaseDirectory.applicationSupport); + expect(location.directory, p.join('downloads', 'srv', 'item-1')); + expect(p.isAbsolute(location.directory), isFalse); + expect(location.directory, isNot(contains(tmpRoot.path))); + }); + + test('reanchors an enqueued target after the app storage directory moves', () async { + final settings = await SettingsService.getInstance(); + final dss = DownloadStorageService.instance; + await dss.initialize(settings); + + final videoPath = await dss.getVideoFilePath(ServerId('srv'), 'item-1', 'mkv'); + final location = await dss.resolveTaskDirectory(videoPath); + final storedTarget = p.join(location.directory, p.basename(videoPath)); + expect(await dss.toAbsolutePath(storedTarget), videoPath); + + // Stand in for the app being moved to another volume: the same base-directory + // lookup now resolves somewhere else, and the enqueued target must follow it. + final movedRoot = await Directory.systemTemp.createTemp('dss_moved_'); + addTearDown(() async { + if (await movedRoot.exists()) await movedRoot.delete(recursive: true); + }); + PathProviderPlatform.instance = FakePathProvider(movedRoot); + + expect( + await dss.toAbsolutePath(storedTarget), + p.join(movedRoot.path, 'support', 'downloads', 'srv', 'item-1', 'video.mkv'), + ); + }); + + test('keeps a custom download root absolute because it does not move with the app', () async { + final settings = await SettingsService.getInstance(); + await settings.write(SettingsService.customDownloadPathType, 'file'); + final customRoot = p.join(tmpRoot.path, 'external', 'PlezyDownloads'); + await settings.write(SettingsService.customDownloadPath, customRoot); + + final dss = DownloadStorageService.instance; + await dss.initialize(settings); + + final videoPath = await dss.getVideoFilePath(ServerId('srv'), 'item-1', 'mkv'); + expect(videoPath, startsWith(customRoot)); + + final location = await dss.resolveTaskDirectory(videoPath); + expect(location.baseDirectory, BaseDirectory.root); + expect(location.directory, p.dirname(videoPath)); + }); + + test('keeps a custom root that only shares a name prefix with the app base dir', () async { + final settings = await SettingsService.getInstance(); + await settings.write(SettingsService.customDownloadPathType, 'file'); + // Sibling of the base dir, not inside it: downloads must still land here, not be + // rewritten to "-external/..." underneath app storage. + final customRoot = '${p.join(tmpRoot.path, 'support')}-external'; + await settings.write(SettingsService.customDownloadPath, customRoot); + + final dss = DownloadStorageService.instance; + await dss.initialize(settings); + + final videoPath = await dss.getVideoFilePath(ServerId('srv'), 'item-1', 'mkv'); + expect(videoPath, startsWith(customRoot)); + + final location = await dss.resolveTaskDirectory(videoPath); + expect(location.baseDirectory, BaseDirectory.root); + expect(location.directory, p.dirname(videoPath)); + }); + }); + // ============================================================ // ensureAbsolutePath / getReadablePath // ============================================================